launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/storm-vocabulary-base into lp:launchpad.

Commit message:
Add StormVocabularyBase, mostly copied from SQLObjectVocabularyBase, and use it for GitRepositoryVocabulary.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/storm-vocabulary-base/+merge/257655

Add StormVocabularyBase, mostly copied from SQLObjectVocabularyBase, and use it for GitRepositoryVocabulary.

(I ran into this when working on the browser code for Git merge proposals.)
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/storm-vocabulary-base into lp:launchpad.

=== modified file 'lib/lp/code/vocabularies/gitrepository.py'
--- lib/lp/code/vocabularies/gitrepository.py	2015-04-13 19:02:15 +0000
+++ lib/lp/code/vocabularies/gitrepository.py	2015-04-28 15:36:55 +0000
@@ -19,17 +19,17 @@
 from lp.services.webapp.vocabulary import (
     CountableIterator,
     IHugeVocabulary,
-    SQLObjectVocabularyBase,
+    StormVocabularyBase,
     )
 
 
-class GitRepositoryVocabulary(SQLObjectVocabularyBase):
+class GitRepositoryVocabulary(StormVocabularyBase):
     """A vocabulary for searching Git repositories."""
 
     implements(IHugeVocabulary)
 
     _table = GitRepository
-    _orderBy = ['name', 'id']
+    _order_by = ['name', 'id']
     displayname = 'Select a Git repository'
     step_title = 'Search'
 

=== modified file 'lib/lp/services/webapp/marshallers.py'
--- lib/lp/services/webapp/marshallers.py	2010-03-26 19:18:31 +0000
+++ lib/lp/services/webapp/marshallers.py	2015-04-28 15:36:55 +0000
@@ -10,12 +10,13 @@
 
 def choiceMarshallerError(field, request, vocabulary=None):
     # We don't support marshalling a normal Choice field with a
-    # SQLObjectVocabularyBase-based vocabulary.
+    # SQLObjectVocabularyBase/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, you should use "
-                         "lazr.restful.fields.ReferenceChoice instead."
+                         "SQLObjectVocabularyBase/StormVocabularyBase; you "
+                         "should use lazr.restful.fields.ReferenceChoice "
+                         "instead."
                          % field.__name__)

=== modified file 'lib/lp/services/webapp/vocabulary.py'
--- lib/lp/services/webapp/vocabulary.py	2013-01-07 02:40:55 +0000
+++ lib/lp/services/webapp/vocabulary.py	2015-04-28 15:36:55 +0000
@@ -10,14 +10,15 @@
 __metaclass__ = type
 
 __all__ = [
+    'BatchedCountableIterator',
+    'CountableIterator',
     'FilteredVocabularyBase',
     'ForgivingSimpleVocabulary',
     'IHugeVocabulary',
+    'NamedSQLObjectHugeVocabulary',
+    'NamedSQLObjectVocabulary',
     'SQLObjectVocabularyBase',
-    'NamedSQLObjectVocabulary',
-    'NamedSQLObjectHugeVocabulary',
-    'CountableIterator',
-    'BatchedCountableIterator',
+    'StormVocabularyBase',
     'VocabularyFilter',
 ]
 
@@ -28,6 +29,8 @@
     AND,
     CONTAINSSTRING,
     )
+from storm.base import Storm
+from storm.store import EmptyResultSet
 from zope.interface import (
     Attribute,
     implements,
@@ -43,6 +46,7 @@
     )
 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.helpers import ensure_unicode
 
@@ -464,3 +468,120 @@
             clause = AND(clause, self._filter)
         results = self._table.select(clause, orderBy=self._orderBy)
         return self.iterator(results.count(), results, self.toTerm)
+
+
+class StormVocabularyBase(FilteredVocabularyBase):
+    """A base class for widgets that are rendered to collect values
+    for attributes that are Storm references.
+
+    So if a content class behind some form looks like:
+
+    class Foo(StormBase):
+        id = Int(...)
+        bar_id = Int(...)
+        bar = Reference(bar_id, ...)
+        ...
+
+    Then the vocabulary for the widget that captures a value for bar
+    should derive from StormVocabularyBase.
+    """
+    implements(IVocabulary, IVocabularyTokenized)
+    _order_by = None
+    _clauses = []
+
+    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)
+
+    @property
+    def _entries(self):
+        entries = IStore(self._table).find(self._table, *self._clauses)
+        if self._order_by is not None:
+            entries = entries.order_by(self._order_by)
+        return entries
+
+    def __iter__(self):
+        """Return an iterator which provides the terms from the vocabulary."""
+        for obj in self._entries:
+            yield self.toTerm(obj)
+
+    def __len__(self):
+        return self._entries.count()
+
+    def __contains__(self, obj):
+        # Sometimes this method is called with a Storm instance, but z3 form
+        # machinery sends through integer ids.  This might be due to a bug
+        # somewhere.
+        if zisinstance(obj, Storm):
+            clauses = [self._table.id == obj.id]
+            if self._clauses:
+                # XXX kiko 2007-01-16: this code is untested.
+                clauses.extend(self._clauses)
+            found_obj = IStore(self._table).find(self._table, *clauses).one()
+            return found_obj is not None and found_obj == obj
+        else:
+            clauses = [self._table.id == int(obj)]
+            if self._clauses:
+                # XXX kiko 2007-01-16: this code is untested.
+                clauses.extend(self._clauses)
+            found_obj = IStore(self._table).find(self._table, *clauses).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 a Storm instance.
+        if zisinstance(value, Storm):
+            return self.toTerm(value)
+
+        try:
+            value = int(value)
+        except ValueError:
+            raise LookupError(value)
+
+        clauses = [self._table.id == value]
+        if self._clauses:
+            clauses.extend(self._clauses)
+        try:
+            obj = IStore(self._table).find(self._table, *clauses).one()
+        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 EmptyResultSet()

=== modified file 'lib/lp/services/webservice/configure.zcml'
--- lib/lp/services/webservice/configure.zcml	2012-01-31 01:19:45 +0000
+++ lib/lp/services/webservice/configure.zcml	2015-04-28 15:36:55 +0000
@@ -64,12 +64,26 @@
        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"
+       />
+   <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"
+       />
 
    <!-- The API documentation -->
     <browser:page

=== modified file 'lib/lp/services/webservice/doc/webservice-marshallers.txt'
--- lib/lp/services/webservice/doc/webservice-marshallers.txt	2011-12-24 17:49:30 +0000
+++ lib/lp/services/webservice/doc/webservice-marshallers.txt	2015-04-28 15:36:55 +0000
@@ -99,7 +99,7 @@
     Traceback (most recent call last):
     ...
     AssertionError: You exported some_person as an IChoice based on an
-    SQLObjectVocabularyBase, you should use
+    SQLObjectVocabularyBase/StormVocabularyBase; you should use
     lazr.restful.fields.ReferenceChoice instead.
 
 Cleanup.