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/144626
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.
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/144626
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/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-24 03:46: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 = {
@@ -812,51 +820,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/vocabularies/specificationdependency.py'
--- lib/lp/blueprints/vocabularies/specificationdependency.py 2012-12-01 22:59:58 +0000
+++ lib/lp/blueprints/vocabularies/specificationdependency.py 2013-01-24 03:46: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())
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
