launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #30471
[Merge] ~cjwatson/launchpad:remove-sqlobjectvocabulary into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:remove-sqlobjectvocabulary into launchpad:master.
Commit message:
Remove SQLObjectVocabularyBase and NamedSQLObjectVocabulary
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/451684
Superseded by `StormVocabularyBase` and `NamedStormVocabulary` respectively.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:remove-sqlobjectvocabulary into launchpad:master.
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.
