launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02935
[Merge] lp:~thumper/launchpad/blueprint-dependency-vocabulary into lp:launchpad
Tim Penhey has proposed merging lp:~thumper/launchpad/blueprint-dependency-vocabulary into lp:launchpad with lp:~thumper/launchpad/blueprint-dependencies as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #707111 in Launchpad itself: "Blueprint dependency picker doesn't allow searching outside the target"
https://bugs.launchpad.net/launchpad/+bug/707111
For more details, see:
https://code.launchpad.net/~thumper/launchpad/blueprint-dependency-vocabulary/+merge/53192
Allow the vocabulary to work with any dependency.
--
https://code.launchpad.net/~thumper/launchpad/blueprint-dependency-vocabulary/+merge/53192
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/blueprint-dependency-vocabulary into lp:launchpad.
=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py 2011-03-14 03:22:42 +0000
+++ lib/lp/blueprints/model/specification.py 2011-03-14 03:22:43 +0000
@@ -671,6 +671,9 @@
spec_branch = self.getBranchLink(branch)
spec_branch.destroySelf()
+ def __repr__(self):
+ return '<Specification %r for %r>' % (self.name, self.target.name)
+
class HasSpecificationsMixin:
"""A mixin class that implements many of the common shortcut properties
=== modified file 'lib/lp/blueprints/vocabularies/specificationdependency.py'
--- lib/lp/blueprints/vocabularies/specificationdependency.py 2010-12-01 18:58:44 +0000
+++ lib/lp/blueprints/vocabularies/specificationdependency.py 2011-03-14 03:22:43 +0000
@@ -11,12 +11,12 @@
from operator import attrgetter
+from storm.locals import Not, SQL, Store
from zope.component import getUtility
from zope.interface import implements
from zope.schema.vocabulary import SimpleTerm
-from canonical.database.sqlbase import quote_like
-from canonical.launchpad.helpers import shortlist
+from canonical.database.sqlbase import quote
from canonical.launchpad.webapp import (
canonical_url,
urlparse,
@@ -27,9 +27,11 @@
NamedSQLObjectVocabulary,
SQLObjectVocabularyBase,
)
-
-from lp.blueprints.enums import SpecificationFilter
-from lp.blueprints.model.specification import Specification
+from lp.blueprints.interfaces.specification import ISpecification
+from lp.blueprints.model.specification import (
+ recursive_blocked_query,
+ Specification,
+ )
from lp.registry.interfaces.pillar import IPillarNameSet
@@ -47,10 +49,10 @@
as the context, or
- the full URL of the spec, in which case it can be any spec at all.
- For the purposes of enumeration and searching we only consider the first
- sort of spec for now. The URL form of token only matches precisely,
- searching only looks for specs on the current target if the search term is
- not a URL.
+ For the purposes of enumeration and searching we look at all the possible
+ specifications, but order those of the same target first. If there is an
+ associated series as well, then those are shown before other matches not
+ linked to the same series.
"""
implements(IHugeVocabulary)
@@ -60,30 +62,51 @@
displayname = 'Select a blueprint'
step_title = 'Search'
- def _is_valid_candidate(self, spec, check_target=False):
+ def _is_valid_candidate(self, spec):
"""Is `spec` a valid candidate spec for self.context?
Invalid candidates are:
- * The spec that we're adding a depdency to,
- * Specs for a different target, and
- * Specs that depend on this one.
+ * Anything not a specification
+ * The spec that we're adding a depdency to
+ * Specs that depend on this one
Preventing the last category prevents loops in the dependency graph.
"""
- if check_target and spec.target != self.context.target:
+ if ISpecification(spec, None) is None:
return False
- return spec != self.context and spec not in self.context.all_blocked
-
- def _filter_specs(self, specs, check_target=False):
- """Filter `specs` to remove invalid candidates.
-
- See `_is_valid_candidate` for what an invalid candidate is.
- """
- # XXX intellectronica 2007-07-05: is 100 a reasonable count before
- # starting to warn?
- return [spec for spec in shortlist(specs, 100)
- if self._is_valid_candidate(spec, check_target)]
+ return spec != self.context and spec not in set(self.context.all_blocked)
+
+ def _order_by(self):
+ """Look at the context to provide grouping."""
+ order_statements = []
+ spec = self.context
+ if spec.product is not None:
+ order_statements.append(
+ "(CASE Specification.product WHEN %s THEN 0 ELSE 1 END)" %
+ spec.product.id)
+ if spec.productseries is not None:
+ order_statements.append(
+ "(CASE Specification.productseries"
+ " WHEN %s THEN 0 ELSE 1 END)" %
+ spec.productseries.id)
+ elif spec.distribution is not None:
+ order_statements.append(
+ "(CASE Specification.distribution WHEN %s THEN 0 ELSE 1 END)" %
+ spec.distribution.id)
+ if spec.distroseries is not None:
+ order_statements.append(
+ "(CASE Specification.distroseries"
+ " WHEN %s THEN 0 ELSE 1 END)" %
+ spec.distroseries.id)
+ order_statements.append("Specification.name")
+ order_statements.append("Specification.id")
+ return SQL(', '.join(order_statements))
+
+ def _blocked_subselect(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))
def toTerm(self, obj):
if obj.target == self.context.target:
@@ -127,7 +150,7 @@
spec = self._spec_from_url(token)
if spec is None:
spec = self.context.target.getSpecification(token)
- if spec and self._is_valid_candidate(spec):
+ if self._is_valid_candidate(spec):
return self.toTerm(spec)
raise LookupError(token)
@@ -141,27 +164,18 @@
if not query:
return CountableIterator(0, [])
spec = self._spec_from_url(query)
- if spec is not None and self._is_valid_candidate(spec):
+ if self._is_valid_candidate(spec):
return CountableIterator(1, [spec])
- quoted_query = quote_like(query)
- sql_query = ("""
- (Specification.name LIKE %s OR
- Specification.title LIKE %s OR
- fti @@ ftq(%s))
- """
- % (quoted_query, quoted_query, quoted_query))
- all_specs = Specification.select(sql_query, orderBy=self._orderBy)
- candidate_specs = self._filter_specs(all_specs, check_target=True)
- return CountableIterator(len(candidate_specs), candidate_specs)
- @property
- def _all_specs(self):
- return self.context.target.specifications(
- filter=[SpecificationFilter.ALL], prejoin_people=False)
+ return Store.of(self.context).find(
+ Specification,
+ SQL('Specification.fti @@ ftq(%s)' % quote(query)),
+ self._blocked_subselect(),
+ ).order_by(self._order_by())
def __iter__(self):
- return (
- self.toTerm(spec) for spec in self._filter_specs(self._all_specs))
+ # We don't ever want to iterate over everything.
+ raise NotImplementedError()
def __contains__(self, obj):
return self._is_valid_candidate(obj)
=== modified file 'lib/lp/blueprints/vocabularies/tests/test_specificationdependency.py'
--- lib/lp/blueprints/vocabularies/tests/test_specificationdependency.py 2010-10-04 19:50:45 +0000
+++ lib/lp/blueprints/vocabularies/tests/test_specificationdependency.py 2011-03-14 03:22:43 +0000
@@ -176,3 +176,72 @@
vocab = self.getVocabularyForSpec(spec)
term = vocab.getTermByToken(canonical_url(candidate))
self.assertEqual(term.token, canonical_url(candidate))
+
+ def test_searchForTerms_ordering_different_targets_by_name(self):
+ # If the searched for specs are on different targets, the ordering is
+ # by name.
+ spec = self.factory.makeSpecification()
+ foo_b = self.factory.makeSpecification(name='foo-b')
+ foo_a = self.factory.makeSpecification(name='foo-a')
+ vocab = self.getVocabularyForSpec(spec)
+ results = vocab.searchForTerms('foo')
+ self.assertEqual(2, len(results))
+ found = [item.value for item in results]
+ self.assertEqual([foo_a, foo_b], found)
+
+ def test_searchForTerms_ordering_same_product_first(self):
+ # Specs on the same product are returned first.
+ widget = self.factory.makeProduct()
+ spec = self.factory.makeSpecification(product=widget)
+ foo_b = self.factory.makeSpecification(name='foo-b', product=widget)
+ foo_a = self.factory.makeSpecification(name='foo-a')
+ vocab = self.getVocabularyForSpec(spec)
+ results = vocab.searchForTerms('foo')
+ self.assertEqual(2, len(results))
+ found = [item.value for item in results]
+ self.assertEqual([foo_b, foo_a], found)
+
+ def test_searchForTerms_ordering_same_productseries_first(self):
+ # Specs on the same product series are returned before the same
+ # products, and those before others.
+ widget = self.factory.makeProduct()
+ spec = self.factory.makeSpecification(product=widget)
+ spec.proposeGoal(widget.development_focus, widget.owner)
+ foo_c = self.factory.makeSpecification(name='foo-c', product=widget)
+ foo_c.proposeGoal(widget.development_focus, widget.owner)
+ foo_b = self.factory.makeSpecification(name='foo-b', product=widget)
+ foo_a = self.factory.makeSpecification(name='foo-a')
+ vocab = self.getVocabularyForSpec(spec)
+ results = vocab.searchForTerms('foo')
+ self.assertEqual(3, len(results))
+ found = [item.value for item in results]
+ self.assertEqual([foo_c, foo_b, foo_a], found)
+
+ def test_searchForTerms_ordering_same_distribution_first(self):
+ # Specs on the same distribution are returned first.
+ mint = self.factory.makeDistribution()
+ spec = self.factory.makeSpecification(distribution=mint)
+ foo_b = self.factory.makeSpecification(name='foo-b', distribution=mint)
+ foo_a = self.factory.makeSpecification(name='foo-a')
+ vocab = self.getVocabularyForSpec(spec)
+ results = vocab.searchForTerms('foo')
+ self.assertEqual(2, len(results))
+ found = [item.value for item in results]
+ self.assertEqual([foo_b, foo_a], found)
+
+ def test_searchForTerms_ordering_same_distroseries_first(self):
+ # Specs on the same distroseries are returned before the same
+ # distribution, and those before others.
+ mint = self.factory.makeDistribution()
+ next = self.factory.makeDistroSeries(mint)
+ spec = self.factory.makeSpecification(distribution=mint)
+ spec.proposeGoal(next, mint.owner)
+ foo_c = self.factory.makeSpecification(name='foo-c', distribution=mint)
+ foo_c.proposeGoal(next, mint.owner)
+ foo_b = self.factory.makeSpecification(name='foo-b', distribution=mint)
+ foo_a = self.factory.makeSpecification(name='foo-a')
+ vocab = self.getVocabularyForSpec(spec)
+ results = vocab.searchForTerms('foo')
+ self.assertEqual(3, len(results))
+ found = [item.value for item in results]
+ self.assertEqual([foo_c, foo_b, foo_a], found)
