← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~stevenk/launchpad/use-visibility-filter-spec-depends into lp:launchpad

 

Steve Kowalik has proposed merging lp:~stevenk/launchpad/use-visibility-filter-spec-depends into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1095235 in Launchpad itself: "Bogus dependencies in Blueprint graph"
  https://bugs.launchpad.net/launchpad/+bug/1095235

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/use-visibility-filter-spec-depends/+merge/144836

Rewrite the query fragment returned by recursive_{blocked,dependent}_query to no longer insert the specification id by string interpolation, and to make use of get_specification_privacy_filter() so that specifications that the user can not view are not included in the dep tree. This also drops the spec argument to the function, since the callers will wrap the resultant string in SQL(), they can use params=() to set the specification id.

Also rewrite ISpecification.{dependencies,blocked_specs} to be methods that require a user argument, and only return direct children/parents that the user can see. Call these methods from the deptree generation view, rather than their recursive cousins. I have renamed the old dependencies SQLRelatedJoin to _dependencies, since one internal specification method makes use of it, and deleted the old blocked_specs SQLRelatedJoin. This also means that the deptree view also drops its use of the sharing service, since it can just call the underlying methods on self.context directly.

This also means that ISpecification.all_{blocked,deps} no longer require the sharing service to filter against visible artifacts, and the SQL queries perform the heavy lifting.

I've dropped recursive_dependent_query from __all__ since they are both pretty much internal-only functions, and I'd like to discourage their use if at all possible, but specificationdependency's vocab sadly makes use of recursive_blocked_query, so it stays. For now.
-- 
https://code.launchpad.net/~stevenk/launchpad/use-visibility-filter-spec-depends/+merge/144836
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/use-visibility-filter-spec-depends into lp:launchpad.
=== modified file 'lib/lp/_schema_circular_imports.py'
--- lib/lp/_schema_circular_imports.py	2012-09-26 19:10:18 +0000
+++ lib/lp/_schema_circular_imports.py	2013-01-25 01:49:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Update the interface schema values due to circular imports.
@@ -724,9 +724,10 @@
     LAZR_WEBSERVICE_EXPORTED)['params']['bug'].schema = IBug
 ISpecification['unlinkBug'].queryTaggedValue(
     LAZR_WEBSERVICE_EXPORTED)['params']['bug'].schema = IBug
-patch_collection_property(ISpecification, 'dependencies', ISpecification)
 patch_collection_property(
     ISpecification, 'linked_branches', ISpecificationBranch)
+patch_collection_return_type(
+    ISpecification, 'dependencies', ISpecification)
 
 # ISpecificationTarget
 patch_entry_return_type(

=== modified file 'lib/lp/blueprints/browser/specification.py'
--- lib/lp/blueprints/browser/specification.py	2012-12-06 17:00:26 +0000
+++ lib/lp/blueprints/browser/specification.py	2013-01-25 01:49:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Specification views."""
@@ -604,8 +604,8 @@
 
     def dependencytree(self):
         text = 'Show dependencies'
-        enabled = (bool(self.context.dependencies) or
-                   bool(self.context.blocked_specs))
+        enabled = (bool(self.context.dependencies(self.user)) or
+                   bool(self.context.blocked_specs(self.user)))
         return Link('+deptree', text, icon='info', enabled=enabled)
 
     @enabled_with_permission('launchpad.AnyPerson')
@@ -643,7 +643,8 @@
 
     @cachedproperty
     def has_dep_tree(self):
-        return self.context.dependencies or self.context.blocked_specs
+        return (self.context.dependencies(self.user) or
+            self.context.blocked_specs(self.user))
 
     @cachedproperty
     def linked_branches(self):
@@ -1168,7 +1169,7 @@
         transitively.
         """
         def get_related_specs_fn(spec):
-            return spec.all_deps(user=self.user)
+            return spec.dependencies(self.user)
 
         def link_nodes_fn(node, dependency):
             self.link(dependency, node)
@@ -1177,7 +1178,7 @@
     def addBlockedNodes(self, spec):
         """Add nodes for specs that the given spec blocks, transitively."""
         def get_related_specs_fn(spec):
-            return spec.all_blocked(user=self.user)
+            return spec.blocked_specs(self.user)
 
         def link_nodes_fn(node, blocked_spec):
             self.link(node, blocked_spec)

=== modified file 'lib/lp/blueprints/browser/specificationdependency.py'
--- lib/lp/blueprints/browser/specificationdependency.py	2012-12-26 01:04:05 +0000
+++ lib/lp/blueprints/browser/specificationdependency.py	2013-01-25 01:49:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Views for SpecificationDependency."""
@@ -12,7 +12,6 @@
     ]
 
 from lazr.restful.interface import copy_field
-from zope.component import getUtility
 from zope.interface import Interface
 
 from lp import _
@@ -20,13 +19,10 @@
     action,
     LaunchpadFormView,
     )
-from lp.app.enums import InformationType
-from lp.app.interfaces.services import IService
 from lp.blueprints.interfaces.specificationdependency import (
     ISpecificationDependency,
     ISpecificationDependencyRemoval,
     )
-from lp.services.propertycache import cachedproperty
 from lp.services.webapp import (
     canonical_url,
     LaunchpadView,
@@ -99,40 +95,22 @@
 
     label = "Blueprint dependency tree"
 
-    def __init__(self, *args, **kwargs):
-        super(SpecificationDependencyTreeView, self).__init__(*args, **kwargs)
-        self.service = getUtility(IService, 'sharing')
-
     @property
     def page_title(self):
         return self.label
 
-    @cachedproperty
+    @property
     def all_blocked(self):
         return self.context.all_blocked(self.user)
 
-    @cachedproperty
+    @property
     def all_deps(self):
         return self.context.all_deps(self.user)
 
-    @cachedproperty
+    @property
     def dependencies(self):
-        deps = list(self.context.dependencies)
-        if self.user:
-            (ignore, ignore, deps) = self.service.getVisibleArtifacts(
-                self.user, specifications=deps)
-        else:
-            deps = [d for d in deps if
-                d.information_type == InformationType.PUBLIC]
-        return deps
+        return self.context.dependencies(self.user)
 
-    @cachedproperty
+    @property
     def blocked_specs(self):
-        blocked = list(self.context.blocked_specs)
-        if self.user:
-            (ignore, ignore, blocked) = self.service.getVisibleArtifacts(
-                self.user, specifications=blocked)
-        else:
-            blocked = [b for b in blocked if
-                b.information_type == InformationType.PUBLIC]
-        return blocked
+        return self.context.blocked_specs(self.user)

=== modified file 'lib/lp/blueprints/browser/tests/test_specificationdependency.py'
--- lib/lp/blueprints/browser/tests/test_specificationdependency.py	2012-12-02 05:05:40 +0000
+++ lib/lp/blueprints/browser/tests/test_specificationdependency.py	2013-01-25 01:49:24 +0000
@@ -40,7 +40,7 @@
         # We need a new interaction for the permission checks
         # on ISpecification objects.
         with person_logged_in(None):
-            self.assertIn(dependency, spec.dependencies)
+            self.assertIn(dependency, spec.dependencies(None))
 
 
 class TestDepTree(TestCaseWithFactory):
@@ -64,21 +64,17 @@
         # Anonymous can see only the public
         with anonymous_logged_in():
             view = create_view(root, name="+deptree")
-            self.assertEqual([public_dep], view.all_deps)
             self.assertEqual([public_dep], view.dependencies)
 
         # The owner can see everything.
         with person_logged_in(owner):
             view = create_view(root, name="+deptree")
             self.assertEqual(
-                [proprietary_dep, public_dep], view.all_deps)
-            self.assertEqual(
                 [proprietary_dep, public_dep], view.dependencies)
 
         # A random person cannot see the propriety dep.
         with person_logged_in(self.factory.makePerson()):
             view = create_view(root, name="+deptree")
-            self.assertEqual([public_dep], view.all_deps)
             self.assertEqual([public_dep], view.dependencies)
 
     def test_deptree_filters_blocked(self):
@@ -98,19 +94,15 @@
         # Anonymous can see only the public
         with anonymous_logged_in():
             view = create_view(root, name="+deptree")
-            self.assertEqual([public_blocked], view.all_blocked)
             self.assertEqual([public_blocked], view.blocked_specs)
 
         # The owner can see everything.
         with person_logged_in(owner):
             view = create_view(root, name="+deptree")
             self.assertEqual(
-                [proprietary_blocked, public_blocked], view.all_blocked)
-            self.assertEqual(
                 [proprietary_blocked, public_blocked], view.blocked_specs)
 
         # A random person cannot see the propriety dep.
         with person_logged_in(self.factory.makePerson()):
             view = create_view(root, name="+deptree")
-            self.assertEqual([public_blocked], view.all_blocked)
             self.assertEqual([public_blocked], view.blocked_specs)

=== modified file 'lib/lp/blueprints/doc/specgraph.txt'
--- lib/lp/blueprints/doc/specgraph.txt	2012-11-30 21:23:15 +0000
+++ lib/lp/blueprints/doc/specgraph.txt	2013-01-25 01:49:24 +0000
@@ -27,16 +27,18 @@
     ...         self.title = title or name
     ...         self.is_complete = is_complete
     ...         self.assignee = assignee
-    ...         # For purposes of this doc, we're only stubbing the
-    ...         # dependency methods.
-    ...         self._all_deps = []
-    ...         self._all_blocked = self._all_deps
-    ...
-    ...     def all_deps(self, user):
-    ...         return self._all_deps
-    ...
-    ...     def all_blocked(self, user):
-    ...         return self._all_blocked
+    ...         # Use lists here to ensure that the code converts them
+    ...         # to sets explicitly, like it has to do for SelectResults.
+    ...         self._dependencies = []
+    ...         # This is a hack for testing: we can set up dependencies,
+    ...         # and simply use their "mirror image" for blocked specs.
+    ...         self._blocked_specs = self._dependencies
+    ...
+    ...     def dependencies(self, user):
+    ...         return self._dependencies
+    ...
+    ...     def blocked_specs(self, user):
+    ...         return self._blocked_specs
 
     >>> foo = Spec('foo', title='something with " and \n in it')
     >>> root = g.newNode(foo, root=True)
@@ -74,16 +76,16 @@
     <fnord-foo>:
 
     >>> foo1 = Spec('foo1')
-    >>> foo._all_deps.append(foo1)
+    >>> foo._dependencies.append(foo1)
     >>> foo2 = Spec('foo2')
-    >>> foo._all_deps.append(foo2)
+    >>> foo._dependencies.append(foo2)
     >>> foo11 = Spec('foo11')
-    >>> foo1._all_deps.append(foo11)
+    >>> foo1._dependencies.append(foo11)
     >>> foo111 = Spec('foo111')
-    >>> foo11._all_deps.append(foo111)
+    >>> foo11._dependencies.append(foo111)
 
     >>> def make_graph(dependency, blocked):
-    ...     g = SpecGraph(user=None)
+    ...     g = SpecGraph()
     ...     g.url_pattern_for_testing = 'http://whatever/%s'
     ...     g.newNode(foo, root=True)
     ...     if dependency:
@@ -179,11 +181,11 @@
 
 The graph grows when specifications gain more dependencies.
 
-    >>> foo1._all_deps.append(foo)
-    >>> foo111._all_deps.append(foo1)
-    >>> foo111._all_deps.append(foo)
-    >>> foo2._all_deps.append(foo1)
-    >>> foo1._all_deps.append(foo2)
+    >>> foo1._dependencies.append(foo)
+    >>> foo111._dependencies.append(foo1)
+    >>> foo111._dependencies.append(foo)
+    >>> foo2._dependencies.append(foo1)
+    >>> foo1._dependencies.append(foo2)
     >>> print_graph()
     Root is <fnord-foo>
     <fnord-foo>:

=== modified file 'lib/lp/blueprints/doc/specification.txt'
--- lib/lp/blueprints/doc/specification.txt	2012-12-26 01:32:19 +0000
+++ lib/lp/blueprints/doc/specification.txt	2013-01-25 01:49:24 +0000
@@ -262,20 +262,20 @@
     >>> from lp.registry.interfaces.product import IProductSet
     >>> efourx = getUtility(IProductSet).getByName(
     ...     'firefox').getSpecification('e4x')
-    >>> for spec in efourx.dependencies: print spec.name
+    >>> for spec in efourx.dependencies(None): print spec.name
     svg-support
 
     >>> for spec in efourx.all_deps(): print spec.name
     svg-support
 
-    >>> for spec in efourx.blocked_specs: print spec.name
+    >>> for spec in efourx.blocked_specs(None): print spec.name
     canvas
 
     >>> for spec in efourx.all_blocked(): print spec.name
     canvas
 
-    >>> canvas = efourx.blocked_specs[0]
-    >>> svg = efourx.dependencies[0]
+    >>> canvas = efourx.blocked_specs(None)[0]
+    >>> svg = efourx.dependencies(None)[0]
     >>> for spec in svg.all_blocked(): print spec.name
     canvas
     e4x

=== modified file 'lib/lp/blueprints/interfaces/specification.py'
--- lib/lp/blueprints/interfaces/specification.py	2012-11-30 21:26:50 +0000
+++ lib/lp/blueprints/interfaces/specification.py	2013-01-25 01:49:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Specification interfaces."""
@@ -17,11 +17,13 @@
 from lazr.restful.declarations import (
     call_with,
     export_as_webservice_entry,
+    export_read_operation,
     export_write_operation,
     exported,
     mutator_for,
     operation_for_version,
     operation_parameters,
+    operation_returns_collection_of,
     REQUEST_USER,
     )
 from lazr.restful.fields import (
@@ -397,13 +399,6 @@
     subscribers = Attribute('The set of subscribers to this spec.')
     sprints = Attribute('The sprints at which this spec is discussed.')
     sprint_links = Attribute('The entries that link this spec to sprints.')
-    dependencies = exported(
-        CollectionField(
-            title=_('Specs on which this one depends.'),
-            value_type=Reference(schema=Interface),  # ISpecification, really.
-            readonly=True),
-        as_of="devel")
-    blocked_specs = Attribute('Specs for which this spec is a dependency.')
     linked_branches = exported(
         CollectionField(
             title=_("Branches associated with this spec, usually "
@@ -411,6 +406,16 @@
             value_type=Reference(schema=Interface),  # ISpecificationBranch
             readonly=True),
         as_of="devel")
+    
+    @call_with(user=REQUEST_USER)
+    @operation_returns_collection_of(Interface) # Really ISpecification
+    @export_read_operation()
+    @operation_for_version('devel')
+    def dependencies(user):
+        """Specs on which this one depends."""
+
+    def blocked_specs(user):
+        """Specs for which this spec is a dependency."""
 
     # emergent properties
     informational = Attribute('Is True if this spec is purely informational '

=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2013-01-22 05:07:31 +0000
+++ lib/lp/blueprints/model/specification.py	2013-01-25 01:49:24 +0000
@@ -5,7 +5,6 @@
 __all__ = [
     'HasSpecificationsMixin',
     'recursive_blocked_query',
-    'recursive_dependent_query',
     'Specification',
     'SPECIFICATION_POLICY_ALLOWED_TYPES',
     'SPECIFICATION_POLICY_DEFAULT_TYPES',
@@ -101,6 +100,7 @@
 from lp.services.database.enumcol import EnumCol
 from lp.services.database.lpstorm import IStore
 from lp.services.database.sqlbase import (
+    convert_storm_clause_to_string,
     cursor,
     SQLBase,
     sqlvalues,
@@ -113,26 +113,34 @@
 from lp.services.webapp.interfaces import ILaunchBag
 
 
-def recursive_blocked_query(spec):
+def recursive_blocked_query(user):
+    from lp.blueprints.model.specificationsearch import (
+        get_specification_privacy_filter)
     return """
         RECURSIVE blocked(id) AS (
-            SELECT %s
+            SELECT ?
         UNION
             SELECT sd.specification
-            FROM specificationdependency sd, blocked b
-            WHERE sd.dependency = b.id
-        )""" % spec.id
-
-
-def recursive_dependent_query(spec):
+            FROM blocked b, specificationdependency sd
+            JOIN specification ON sd.specification = specification.id
+            WHERE sd.dependency = b.id AND (%s))""" % (
+                convert_storm_clause_to_string(
+                    *get_specification_privacy_filter(user)))
+
+
+def recursive_dependent_query(user):
+    from lp.blueprints.model.specificationsearch import (
+        get_specification_privacy_filter)
     return """
         RECURSIVE dependencies(id) AS (
-            SELECT %s
+            SELECT ?
         UNION
             SELECT sd.dependency
-            FROM specificationdependency sd, dependencies d
-            WHERE sd.specification = d.id
-        )""" % spec.id
+            FROM dependencies d, specificationdependency sd
+            JOIN specification ON sd.dependency = specification.id
+            WHERE sd.specification = d.id AND (%s))""" % (
+                convert_storm_clause_to_string(
+                    *get_specification_privacy_filter(user)))
 
 
 SPECIFICATION_POLICY_ALLOWED_TYPES = {
@@ -249,15 +257,32 @@
     spec_dependency_links = SQLMultipleJoin('SpecificationDependency',
         joinColumn='specification', orderBy='id')
 
-    dependencies = SQLRelatedJoin('Specification', joinColumn='specification',
+    _dependencies = SQLRelatedJoin('Specification', joinColumn='specification',
         otherColumn='dependency', orderBy='title',
         intermediateTable='SpecificationDependency')
-    blocked_specs = SQLRelatedJoin('Specification', joinColumn='dependency',
-        otherColumn='specification', orderBy='title',
-        intermediateTable='SpecificationDependency')
     information_type = EnumCol(
         enum=InformationType, notNull=True, default=InformationType.PUBLIC)
 
+    def _fetch_children_or_parents(self, join_cond, cond, user):
+        from lp.blueprints.model.specificationsearch import (
+            get_specification_privacy_filter)
+        return list(Store.of(self).using(
+            Specification,
+            Join(SpecificationDependency, join_cond == self.id)).find(
+            Specification,
+            cond == Specification.id, *get_specification_privacy_filter(user)
+            ).order_by(Specification.title))
+
+    def dependencies(self, user):
+        return self._fetch_children_or_parents(
+            SpecificationDependency.specificationID,
+            SpecificationDependency.dependencyID, user)
+
+    def blocked_specs(self, user):
+        return self._fetch_children_or_parents(
+            SpecificationDependency.dependencyID,
+            SpecificationDependency.specificationID, user)
+
     def set_assignee(self, person):
         self.subscribeIfAccessGrantNeeded(person)
         self._assignee = person
@@ -636,7 +661,7 @@
     @property
     def is_blocked(self):
         """See ISpecification."""
-        for spec in self.dependencies:
+        for spec in self._dependencies:
             if spec.is_incomplete:
                 return True
         return False
@@ -812,51 +837,21 @@
                 return deplink
 
     def all_deps(self, user=None):
-        public_clause = True
-        if user is None:
-            public_clause = (
-                Specification.information_type == InformationType.PUBLIC,
-                )
-
-        results = Store.of(self).with_(
-            SQL(recursive_dependent_query(self))).find(
+        return list(Store.of(self).with_(
+            SQL(recursive_dependent_query(user), params=(self.id,))).find(
             Specification,
             Specification.id != self.id,
-            SQL('Specification.id in (select id from dependencies)'),
-            public_clause,
-            ).order_by(Specification.name, Specification.id)
-
-        results = list(results)
-
-        if user:
-            service = getUtility(IService, 'sharing')
-            (ignore, ignore, results) = service.getVisibleArtifacts(
-                user, specifications=results)
-        return results
+            Specification.id.is_in(SQL('select id from dependencies')),
+            ).order_by(Specification.name, Specification.id))
 
     def all_blocked(self, user=None):
         """See `ISpecification`."""
-        public_clause = True
-        if user is None:
-            public_clause = (
-                Specification.information_type == InformationType.PUBLIC,
-                )
-
-        results = Store.of(self).with_(
-            SQL(recursive_blocked_query(self))).find(
+        return list(Store.of(self).with_(
+            SQL(recursive_blocked_query(user), params=(self.id,))).find(
             Specification,
             Specification.id != self.id,
-            SQL('Specification.id in (select id from blocked)'),
-            public_clause,
-            ).order_by(Specification.name, Specification.id)
-
-        results = list(results)
-
-        if user:
-            service = getUtility(IService, 'sharing')
-            (ignore, ignore, results) = service.getVisibleArtifacts(
-                user, specifications=results)
-        return results
+            Specification.id.is_in(SQL('select id from blocked')),
+            ).order_by(Specification.name, Specification.id))
 
     # branches
     def getBranchLink(self, branch):

=== modified file 'lib/lp/blueprints/model/tests/test_specification.py'
--- lib/lp/blueprints/model/tests/test_specification.py	2012-12-10 13:43:47 +0000
+++ lib/lp/blueprints/model/tests/test_specification.py	2013-01-25 01:49:24 +0000
@@ -52,18 +52,18 @@
 
     def test_no_deps(self):
         blueprint = self.factory.makeBlueprint()
-        self.assertThat(list(blueprint.dependencies), Equals([]))
+        self.assertThat(list(blueprint.dependencies(None)), Equals([]))
         self.assertThat(list(blueprint.all_deps()), Equals([]))
-        self.assertThat(list(blueprint.blocked_specs), Equals([]))
+        self.assertThat(list(blueprint.blocked_specs(None)), Equals([]))
         self.assertThat(list(blueprint.all_blocked()), Equals([]))
 
     def test_single_dependency(self):
         do_first = self.factory.makeBlueprint()
         do_next = self.factory.makeBlueprint()
         do_next.createDependency(do_first)
-        self.assertThat(list(do_first.blocked_specs), Equals([do_next]))
+        self.assertThat(list(do_first.blocked_specs(None)), Equals([do_next]))
         self.assertThat(list(do_first.all_blocked()), Equals([do_next]))
-        self.assertThat(list(do_next.dependencies), Equals([do_first]))
+        self.assertThat(list(do_next.dependencies(None)), Equals([do_first]))
         self.assertThat(list(do_next.all_deps()), Equals([do_first]))
 
     def test_linear_dependency(self):
@@ -72,14 +72,14 @@
         do_next.createDependency(do_first)
         do_last = self.factory.makeBlueprint()
         do_last.createDependency(do_next)
-        self.assertThat(sorted(do_first.blocked_specs), Equals([do_next]))
-        self.assertThat(
-            sorted(do_first.all_blocked()),
-            Equals(sorted([do_next, do_last])))
-        self.assertThat(sorted(do_last.dependencies), Equals([do_next]))
-        self.assertThat(
-            sorted(do_last.all_deps()),
-            Equals(sorted([do_first, do_next])))
+        self.assertThat(
+            sorted(do_first.blocked_specs(None)), Equals([do_next]))
+        self.assertThat(
+            sorted(do_first.all_blocked()), Equals(sorted([do_next, do_last])))
+        self.assertThat(
+            sorted(do_last.dependencies(None)), Equals([do_next]))
+        self.assertThat(
+            sorted(do_last.all_deps()), Equals(sorted([do_first, do_next])))
 
     def test_diamond_dependency(self):
         #             do_first
@@ -96,13 +96,13 @@
         do_last.createDependency(do_next_lhs)
         do_last.createDependency(do_next_rhs)
         self.assertThat(
-            sorted(do_first.blocked_specs),
+            sorted(do_first.blocked_specs(None)),
             Equals(sorted([do_next_lhs, do_next_rhs])))
         self.assertThat(
             sorted(do_first.all_blocked()),
             Equals(sorted([do_next_lhs, do_next_rhs, do_last])))
         self.assertThat(
-            sorted(do_last.dependencies),
+            sorted(do_last.dependencies(None)),
             Equals(sorted([do_next_lhs, do_next_rhs])))
         self.assertThat(
             sorted(do_last.all_deps()),

=== modified file 'lib/lp/blueprints/tests/test_webservice.py'
--- lib/lp/blueprints/tests/test_webservice.py	2012-11-22 09:51:51 +0000
+++ lib/lp/blueprints/tests/test_webservice.py	2013-01-25 01:49:24 +0000
@@ -179,8 +179,8 @@
         spec2_name = spec2.name
         spec.createDependency(spec2)
         spec_webservice = self.getSpecOnWebservice(spec)
-        self.assertEqual(1, spec_webservice.dependencies.total_size)
-        self.assertEqual(spec2_name, spec_webservice.dependencies[0].name)
+        self.assertEqual(1, len(spec_webservice.dependencies()))
+        self.assertEqual(spec2_name, spec_webservice.dependencies()[0].name)
 
     def test_representation_contains_linked_branches(self):
         spec = self.factory.makeSpecification()

=== modified file 'lib/lp/blueprints/vocabularies/specificationdependency.py'
--- lib/lp/blueprints/vocabularies/specificationdependency.py	2012-12-01 22:59:58 +0000
+++ lib/lp/blueprints/vocabularies/specificationdependency.py	2013-01-25 01:49:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """The vocabularies relating to dependencies of specifications."""
@@ -24,7 +24,7 @@
     Specification,
     )
 from lp.registry.interfaces.pillar import IPillarNameSet
-from lp.services.database.sqlbase import quote
+from lp.services.database.stormexpr import fti_search
 from lp.services.webapp import (
     canonical_url,
     urlparse,
@@ -118,8 +118,10 @@
 
     def _exclude_blocked_query(self):
         """Return the select statement to exclude already blocked specs."""
-        return SQL("Specification.id not in (WITH %s select id from blocked)"
-                   % recursive_blocked_query(self.context))
+        user = getattr(getUtility(ILaunchBag), 'user', None)
+        return SQL(
+            "Specification.id not in (WITH %s select id from blocked)" % (
+                recursive_blocked_query(user)), params=(self.context.id,))
 
     def toTerm(self, obj):
         if obj.target == self.context.target:
@@ -182,7 +184,7 @@
 
         return Store.of(self.context).find(
             Specification,
-            SQL('Specification.fti @@ ftq(%s)' % quote(query)),
+            fti_search(Specification, query),
             self._exclude_blocked_query(),
             ).order_by(self._order_by())
 
@@ -201,6 +203,7 @@
     _orderBy = 'title'
 
     def __iter__(self):
+        user = getattr(getUtility(ILaunchBag), 'user', None)
         for spec in sorted(
-            self.context.dependencies, key=attrgetter('title')):
+            self.context.dependencies(user), key=attrgetter('title')):
             yield SimpleTerm(spec, spec.name, spec.title)


Follow ups