Merge ~cjwatson/launchpad:remove-sqlobjectvocabulary into launchpad:master

[Colin Watson]

Remove SQLObjectVocabularyBase and NamedSQLObjectVocabulary

Superseded by `StormVocabularyBase` and `NamedStormVocabulary` respectively.

[Guruprasad (lgp171188)]

LGTM 👍

review: Approve

diff --git a/lib/lp/blueprints/vocabularies/specificationdependency.py b/lib/lp/blueprints/vocabularies/specificationdependency.py
index d458c05..f6edb5f 100644
--- a/lib/lp/blueprints/vocabularies/specificationdependency.py
+++ b/lib/lp/blueprints/vocabularies/specificationdependency.py
@@ -168,7 +168,7 @@ class SpecificationDepCandidatesVocabulary(StormVocabularyBase):
         raise LookupError(token)
 
     def search(self, query, vocab_filter=None):
-        """See `SQLObjectVocabularyBase.search`.
+        """See `StormVocabularyBase.search`.
 
         We find specs where query is in the text of name or title, or matches
         the full text index and then filter out ineligible specs using
diff --git a/lib/lp/registry/vocabularies.py b/lib/lp/registry/vocabularies.py
index 0104a33..4f4a761 100644
--- a/lib/lp/registry/vocabularies.py
+++ b/lib/lp/registry/vocabularies.py
@@ -272,7 +272,7 @@ class ProductVocabulary(StormVocabularyBase):
         return self.toTerm(product)
 
     def search(self, query, vocab_filter=None):
-        """See `SQLObjectVocabularyBase`.
+        """See `StormVocabularyBase`.
 
         Returns products where the product name, displayname, title,
         summary, or description contain the given query. Returns an empty list
@@ -338,7 +338,7 @@ class ProjectGroupVocabulary(StormVocabularyBase):
         return self.toTerm(project)
 
     def search(self, query, vocab_filter=None):
-        """See `SQLObjectVocabularyBase`.
+        """See `StormVocabularyBase`.
 
         Returns projects where the project name, displayname, title,
         summary, or description contain the given query. Returns an empty list
@@ -1570,7 +1570,7 @@ class CommercialProjectsVocabulary(NamedStormVocabulary):
         raise LookupError(token)
 
     def searchForTerms(self, query=None, vocab_filter=None):
-        """See `SQLObjectVocabularyBase`."""
+        """See `StormVocabularyBase`."""
         results = self._doSearch(query)
         num = results.count()
         return CountableIterator(num, results, self.toTerm)
diff --git a/lib/lp/services/webapp/marshallers.py b/lib/lp/services/webapp/marshallers.py
index 7f13a36..48ab7ea 100644
--- a/lib/lp/services/webapp/marshallers.py
+++ b/lib/lp/services/webapp/marshallers.py
@@ -6,14 +6,13 @@ __all__ = ["choiceMarshallerError"]
 
 def choiceMarshallerError(field, request, vocabulary=None):
     # We don't support marshalling a normal Choice field with a
-    # SQLObjectVocabularyBase/StormVocabularyBase-based vocabulary.
+    # StormVocabularyBase-based vocabulary.
     # Normally for this kind of use case, one returns None and
     # lets the Zope machinery alert the user that the lookup has gone wrong.
     # However, we want to be more helpful, so we make an assertion,
     # with a comment on how to make things better.
     raise AssertionError(
-        "You exported %s as an IChoice based on an "
-        "SQLObjectVocabularyBase/StormVocabularyBase; you "
+        "You exported %s as an IChoice based on a StormVocabularyBase; you "
         "should use lazr.restful.fields.ReferenceChoice "
         "instead." % field.__name__
     )
diff --git a/lib/lp/services/webapp/vocabulary.py b/lib/lp/services/webapp/vocabulary.py
index 535588c..b13b797 100644
--- a/lib/lp/services/webapp/vocabulary.py
+++ b/lib/lp/services/webapp/vocabulary.py
@@ -13,20 +13,17 @@ __all__ = [
     "FilteredVocabularyBase",
     "ForgivingSimpleVocabulary",
     "IHugeVocabulary",
-    "NamedSQLObjectVocabulary",
     "NamedStormHugeVocabulary",
     "NamedStormVocabulary",
-    "SQLObjectVocabularyBase",
     "StormVocabularyBase",
     "VocabularyFilter",
 ]
 
 from collections import namedtuple
-from typing import Optional, Union
+from typing import Optional
 
 import six
 from storm.base import Storm  # noqa: B1
-from storm.expr import Expr
 from storm.store import EmptyResultSet
 from zope.interface import Attribute, Interface, implementer
 from zope.schema.interfaces import IVocabularyTokenized
@@ -34,8 +31,6 @@ from zope.schema.vocabulary import SimpleTerm, SimpleVocabulary
 from zope.security.proxy import isinstance as zisinstance
 
 from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import SQLBase
-from lp.services.database.sqlobject import AND, CONTAINSSTRING
 
 
 class ForgivingSimpleVocabulary(SimpleVocabulary):
@@ -266,164 +261,6 @@ class FilteredVocabularyBase:
 
 
 @implementer(IVocabularyTokenized)
-class SQLObjectVocabularyBase(FilteredVocabularyBase):
-    """A base class for widgets that are rendered to collect values
-    for attributes that are SQLObjects, e.g. ForeignKey.
-
-    So if a content class behind some form looks like:
-
-    class Foo(SQLObject):
-        id = IntCol(...)
-        bar = ForeignKey(...)
-        ...
-
-    Then the vocabulary for the widget that captures a value for bar
-    should derive from SQLObjectVocabularyBase.
-    """
-
-    _orderBy = None  # type: Optional[str]
-    _filter = None  # type: Optional[Union[Expr, bool]]
-    _clauseTables = None
-
-    def __init__(self, context=None):
-        self.context = context
-
-    # XXX kiko 2007-01-16: note that the method searchForTerms is part of
-    # IHugeVocabulary, and so should not necessarily need to be
-    # implemented here; however, many of our vocabularies depend on
-    # searchForTerms for popup functionality so I have chosen to just do
-    # that. It is possible that a better solution would be to have the
-    # search functionality produce a new vocabulary restricted to the
-    # desired subset.
-    def searchForTerms(self, query=None, vocab_filter=None):
-        results = self.search(query, vocab_filter)
-        return CountableIterator(results.count(), results, self.toTerm)
-
-    def search(self, query, vocab_filter=None):
-        # This default implementation of searchForTerms glues together
-        # the legacy API of search() with the toTerm method. If you
-        # don't reimplement searchForTerms you will need to at least
-        # provide your own search() method.
-        raise NotImplementedError
-
-    def toTerm(self, obj):
-        # This default implementation assumes that your object has a
-        # title attribute. If it does not you will need to reimplement
-        # toTerm, or reimplement the whole searchForTerms.
-        return SimpleTerm(obj, obj.id, obj.title)
-
-    def __iter__(self):
-        """Return an iterator which provides the terms from the vocabulary."""
-        params = {}
-        if self._orderBy is not None:
-            params["orderBy"] = self._orderBy
-        if self._clauseTables is not None:
-            params["clauseTables"] = self._clauseTables
-        for obj in self._table.select(self._filter, **params):
-            yield self.toTerm(obj)
-
-    def __len__(self):
-        return len(list(iter(self)))
-
-    def __contains__(self, obj):
-        # Sometimes this method is called with an SQLBase instance, but
-        # z3 form machinery sends through integer ids. This might be due
-        # to a bug somewhere.
-        if zisinstance(obj, (SQLBase, Storm)):  # noqa: B1
-            clause = self._table.id == obj.id
-            if self._filter:
-                # XXX kiko 2007-01-16: this code is untested.
-                clause = AND(clause, self._filter)
-            found_obj = IStore(self._table).find(self._table, clause).one()
-            return found_obj is not None and found_obj == obj
-        else:
-            clause = self._table.id == int(obj)
-            if self._filter:
-                # XXX kiko 2007-01-16: this code is untested.
-                clause = AND(clause, self._filter)
-            found_obj = IStore(self._table).find(self._table, clause).one()
-            return found_obj is not None
-
-    def getTerm(self, value):
-        # Short circuit. There is probably a design problem here since
-        # we sometimes get the id and sometimes an SQLBase instance.
-        if zisinstance(value, SQLBase):
-            return self.toTerm(value)
-
-        try:
-            value = int(value)
-        except ValueError:
-            raise LookupError(value)
-
-        clause = self._table.q.id == value
-        if self._filter:
-            clause = AND(clause, self._filter)
-        try:
-            obj = self._table.selectOne(clause)
-        except ValueError:
-            raise LookupError(value)
-
-        if obj is None:
-            raise LookupError(value)
-
-        return self.toTerm(obj)
-
-    def getTermByToken(self, token):
-        return self.getTerm(token)
-
-    def emptySelectResults(self):
-        """Return a SelectResults object without any elements.
-
-        This is to be used when no search string is given to the search()
-        method of subclasses, in order to be consistent and always return
-        a SelectResults object.
-        """
-        return self._table.select("1 = 2")
-
-
-class NamedSQLObjectVocabulary(SQLObjectVocabularyBase):
-    """A SQLObjectVocabulary base for database tables that have a unique
-    *and* ASCII name column.
-
-    Provides all methods required by IHugeVocabulary, although it
-    doesn't actually specify this interface since it may not actually
-    be huge and require the custom widgets.
-    """
-
-    _orderBy = "name"
-
-    def toTerm(self, obj):
-        """See SQLObjectVocabularyBase.
-
-        This implementation uses name as a token instead of the object's
-        ID, and tries to be smart about deciding to present an object's
-        title if it has one.
-        """
-        if getattr(obj, "title", None) is None:
-            return SimpleTerm(obj, obj.name, obj.name)
-        else:
-            return SimpleTerm(obj, obj.name, obj.title)
-
-    def getTermByToken(self, token):
-        clause = self._table.q.name == token
-        if self._filter:
-            clause = AND(clause, self._filter)
-        objs = list(self._table.select(clause))
-        if not objs:
-            raise LookupError(token)
-        return self.toTerm(objs[0])
-
-    def search(self, query, vocab_filter=None):
-        """Return terms where query is a substring of the name."""
-        if query:
-            clause = CONTAINSSTRING(self._table.q.name, six.ensure_text(query))
-            if self._filter:
-                clause = AND(clause, self._filter)
-            return self._table.select(clause, orderBy=self._orderBy)
-        return self.emptySelectResults()
-
-
-@implementer(IVocabularyTokenized)
 class StormVocabularyBase(FilteredVocabularyBase):
     """A base class for widgets that are rendered to collect values
     for attributes that are Storm references.
diff --git a/lib/lp/services/webservice/configure.zcml b/lib/lp/services/webservice/configure.zcml
index d3136b5..5e27c1f 100644
--- a/lib/lp/services/webservice/configure.zcml
+++ b/lib/lp/services/webservice/configure.zcml
@@ -60,13 +60,6 @@
    <adapter
        for="zope.schema.interfaces.IChoice
             zope.publisher.interfaces.http.IHTTPRequest
-            lp.services.webapp.vocabulary.SQLObjectVocabularyBase"
-       provides="lazr.restful.interfaces.IFieldMarshaller"
-       factory="lp.services.webapp.marshallers.choiceMarshallerError"
-       />
-   <adapter
-       for="zope.schema.interfaces.IChoice
-            zope.publisher.interfaces.http.IHTTPRequest
             lp.services.webapp.vocabulary.StormVocabularyBase"
        provides="lazr.restful.interfaces.IFieldMarshaller"
        factory="lp.services.webapp.marshallers.choiceMarshallerError"
@@ -74,13 +67,6 @@
    <adapter
        for="lazr.restful.interfaces.IReferenceChoice
             zope.publisher.interfaces.http.IHTTPRequest
-            lp.services.webapp.vocabulary.SQLObjectVocabularyBase"
-       provides="lazr.restful.interfaces.IFieldMarshaller"
-       factory="lazr.restful.marshallers.ObjectLookupFieldMarshaller"
-       />
-   <adapter
-       for="lazr.restful.interfaces.IReferenceChoice
-            zope.publisher.interfaces.http.IHTTPRequest
             lp.services.webapp.vocabulary.StormVocabularyBase"
        provides="lazr.restful.interfaces.IFieldMarshaller"
        factory="lazr.restful.marshallers.ObjectLookupFieldMarshaller"
diff --git a/lib/lp/services/webservice/doc/webservice-marshallers.rst b/lib/lp/services/webservice/doc/webservice-marshallers.rst
index 6dedcee..f49a033 100644
--- a/lib/lp/services/webservice/doc/webservice-marshallers.rst
+++ b/lib/lp/services/webservice/doc/webservice-marshallers.rst
@@ -25,10 +25,10 @@ application root.
     >>> getUtility(IOpenLaunchBag).add(root)
 
 
-Choice of SQLObjectVocabularyBase
-.................................
+Choice of StormVocabularyBase
+.............................
 
-For vocabularies based on SQLObjectVocabularyBase, the values are
+For vocabularies based on StormVocabularyBase, the values are
 interpreted as URLs referencing objects on the web service. If the given
 string is a URL corresponding to a vocabulary item, the marshaller
 returns that item. Otherwise it raises a ValueError.
@@ -94,7 +94,7 @@ resource and not a random string.
     >>> print(marshaller.representation_name)
     some_person_link
 
-If you export a Choice that uses an SQLObjectVocabularyBase then you
+If you export a Choice that uses a StormVocabularyBase then you
 get an error, as you should be using a ReferenceChoice instead to
 ensure that the resulting wadl matches lazr.restful conventions.
 
@@ -104,9 +104,9 @@ ensure that the resulting wadl matches lazr.restful conventions.
     ... # doctest: +NORMALIZE_WHITESPACE
     Traceback (most recent call last):
     ...
-    AssertionError: You exported some_person as an IChoice based on an
-    SQLObjectVocabularyBase/StormVocabularyBase; you should use
-    lazr.restful.fields.ReferenceChoice instead.
+    AssertionError: You exported some_person as an IChoice based on a
+    StormVocabularyBase; you should use lazr.restful.fields.ReferenceChoice
+    instead.
 
 Cleanup.