Merge lp:~mwhudson/launchpad/cross-product-spec-links-bug-3552 into lp:launchpad

Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: 11459
Proposed branch: lp:~mwhudson/launchpad/cross-product-spec-links-bug-3552
Merge into: lp:launchpad
Diff against target: 458 lines (+238/-41)
6 files modified
lib/lp/blueprints/browser/specificationdependency.py (+3/-1)
lib/lp/blueprints/browser/tests/test_specificationdependency.py (+33/-0)
lib/lp/blueprints/vocabularies/specificationdependency.py (+81/-25)
lib/lp/blueprints/vocabularies/tests/specificationdepcandidates.txt (+6/-8)
lib/lp/blueprints/vocabularies/tests/test_specificationdependency.py (+111/-5)
lib/lp/testing/factory.py (+4/-2)
To merge this branch: bzr merge lp:~mwhudson/launchpad/cross-product-spec-links-bug-3552
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+33866@code.launchpad.net

Commit message

Allow the creation of cross-project blueprint dependencies (a present from Linaro!)

Description of the change

Hi,

This branch changes the way the vocabulary for specification dependency candidates works. In particular, it allows you to create cross-pillar dependencies by entering the URL of a specification.

I had a pre-imp chat with Tim.

Cheers,
mwh

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

I love all the new tests :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/blueprints/browser/specificationdependency.py'
--- lib/lp/blueprints/browser/specificationdependency.py 2010-08-26 03:02:29 +0000
+++ lib/lp/blueprints/browser/specificationdependency.py 2010-08-27 04:52:44 +0000
@@ -37,7 +37,9 @@
37 "If another blueprint needs to be fully implemented "37 "If another blueprint needs to be fully implemented "
38 "before this feature can be started, then specify that "38 "before this feature can be started, then specify that "
39 "dependency here so Launchpad knows about it and can "39 "dependency here so Launchpad knows about it and can "
40 "give you an accurate project plan."))40 "give you an accurate project plan. You can enter the "
41 "name of a blueprint that has the same target, or the "
42 "URL of any blueprint."))
4143
4244
43class SpecificationDependencyAddView(LaunchpadFormView):45class SpecificationDependencyAddView(LaunchpadFormView):
4446
=== added file 'lib/lp/blueprints/browser/tests/test_specificationdependency.py'
--- lib/lp/blueprints/browser/tests/test_specificationdependency.py 1970-01-01 00:00:00 +0000
+++ lib/lp/blueprints/browser/tests/test_specificationdependency.py 2010-08-27 04:52:44 +0000
@@ -0,0 +1,33 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests for the specification dependency views.
5
6There are also tests in lp/blueprints/stories/blueprints/xx-dependencies.txt.
7"""
8
9__metaclass__ = type
10
11import unittest
12
13from canonical.launchpad.webapp import canonical_url
14from canonical.testing import DatabaseFunctionalLayer
15from lp.testing import BrowserTestCase
16
17
18class TestAddDependency(BrowserTestCase):
19 layer = DatabaseFunctionalLayer
20
21 def test_add_dependency_by_url(self):
22 # It is possible to use the URL of a specification in the "Depends On"
23 # field of the form to add a dependency to a spec.
24 spec = self.factory.makeSpecification(owner=self.user)
25 dependency = self.factory.makeSpecification()
26 browser = self.getViewBrowser(spec, '+linkdependency')
27 browser.getControl('Depends On').value = canonical_url(dependency)
28 browser.getControl('Continue').click()
29 self.assertIn(dependency, spec.dependencies)
30
31
32def test_suite():
33 return unittest.TestLoader().loadTestsFromName(__name__)
034
=== modified file 'lib/lp/blueprints/vocabularies/specificationdependency.py'
--- lib/lp/blueprints/vocabularies/specificationdependency.py 2010-08-26 03:30:31 +0000
+++ lib/lp/blueprints/vocabularies/specificationdependency.py 2010-08-27 04:52:44 +0000
@@ -6,11 +6,16 @@
6__metaclass__ = type6__metaclass__ = type
7__all__ = ['SpecificationDepCandidatesVocabulary']7__all__ = ['SpecificationDepCandidatesVocabulary']
88
9from zope.component import getUtility
9from zope.interface import implements10from zope.interface import implements
10from zope.schema.vocabulary import SimpleTerm11from zope.schema.vocabulary import SimpleTerm
1112
12from canonical.database.sqlbase import quote_like13from canonical.database.sqlbase import quote_like
13from canonical.launchpad.helpers import shortlist14from canonical.launchpad.helpers import shortlist
15from canonical.launchpad.webapp import (
16 canonical_url,
17 urlparse,
18 )
14from canonical.launchpad.webapp.vocabulary import (19from canonical.launchpad.webapp.vocabulary import (
15 CountableIterator,20 CountableIterator,
16 IHugeVocabulary,21 IHugeVocabulary,
@@ -19,15 +24,26 @@
1924
20from lp.blueprints.interfaces.specification import SpecificationFilter25from lp.blueprints.interfaces.specification import SpecificationFilter
21from lp.blueprints.model.specification import Specification26from lp.blueprints.model.specification import Specification
2227from lp.registry.interfaces.pillar import IPillarNameSet
2328
24class SpecificationDepCandidatesVocabulary(SQLObjectVocabularyBase):29class SpecificationDepCandidatesVocabulary(SQLObjectVocabularyBase):
25 """Specifications that could be dependencies of this spec.30 """Specifications that could be dependencies of this spec.
2631
27 This includes only those specs that are not blocked by this spec32 This includes only those specs that are not blocked by this spec (directly
28 (directly or indirectly), unless they are already dependencies.33 or indirectly), unless they are already dependencies.
2934
30 The current spec is not included.35 This vocabulary has a bit of a split personality.
36
37 Tokens are *either*:
38
39 - the name of a spec, in which case it must be a spec on the same target
40 as the context, or
41 - the full URL of the spec, in which case it can be any spec at all.
42
43 For the purposes of enumeration and searching we only consider the first
44 sort of spec for now. The URL form of token only matches precisely,
45 searching only looks for specs on the current target if the search term is
46 not a URL.
31 """47 """
3248
33 implements(IHugeVocabulary)49 implements(IHugeVocabulary)
@@ -36,8 +52,8 @@
36 _orderBy = 'name'52 _orderBy = 'name'
37 displayname = 'Select a blueprint'53 displayname = 'Select a blueprint'
3854
39 def _filter_specs(self, specs):55 def _is_valid_candidate(self, spec, check_target=False):
40 """Filter `specs` to remove invalid candidates.56 """Is `spec` a valid candidate spec for self.context?
4157
42 Invalid candidates are:58 Invalid candidates are:
4359
@@ -47,27 +63,64 @@
4763
48 Preventing the last category prevents loops in the dependency graph.64 Preventing the last category prevents loops in the dependency graph.
49 """65 """
66 if check_target and spec.target != self.context.target:
67 return False
68 return spec != self.context and spec not in self.context.all_blocked
69
70 def _filter_specs(self, specs, check_target=False):
71 """Filter `specs` to remove invalid candidates.
72
73 See `_is_valid_candidate` for what an invalid candidate is.
74 """
50 # XXX intellectronica 2007-07-05: is 100 a reasonable count before75 # XXX intellectronica 2007-07-05: is 100 a reasonable count before
51 # starting to warn?76 # starting to warn?
52 speclist = shortlist(specs, 100)77 return [spec for spec in shortlist(specs, 100)
53 return [spec for spec in speclist78 if self._is_valid_candidate(spec, check_target)]
54 if (spec != self.context and
55 spec.target == self.context.target
56 and spec not in self.context.all_blocked)]
5779
58 def toTerm(self, obj):80 def toTerm(self, obj):
59 return SimpleTerm(obj, obj.name, obj.title)81 if obj.target == self.context.target:
82 token = obj.name
83 else:
84 token = canonical_url(obj)
85 return SimpleTerm(obj, token, obj.title)
86
87 def _spec_from_url(self, url):
88 """If `url` is the URL of a specification, return it.
89
90 This implementation is a little fuzzy and will return specs for URLs
91 that, for example, don't have the host name right. This seems
92 unlikely to cause confusion in practice, and being too anal probably
93 would be confusing (e.g. not accepting edge URLs on lpnet).
94 """
95 scheme, netloc, path, params, args, fragment = urlparse(url)
96 if not scheme or not netloc:
97 # Not enough like a URL
98 return None
99 path_segments = path.strip('/').split('/')
100 if len(path_segments) != 3:
101 # Can't be a spec url
102 return None
103 pillar_name, plus_spec, spec_name = path_segments
104 if plus_spec != '+spec':
105 # Can't be a spec url
106 return None
107 pillar = getUtility(IPillarNameSet).getByName(
108 pillar_name, ignore_inactive=True)
109 if pillar is None:
110 return None
111 return pillar.getSpecification(spec_name)
60112
61 def getTermByToken(self, token):113 def getTermByToken(self, token):
62 """See `zope.schema.interfaces.IVocabularyTokenized`.114 """See `zope.schema.interfaces.IVocabularyTokenized`.
63115
64 The tokens for specifications are just the name of the spec.116 The tokens for specifications are either the name of a spec on the
117 same target or a URL for a spec.
65 """118 """
66 spec = self.context.target.getSpecification(token)119 spec = self._spec_from_url(token)
67 if spec is not None:120 if spec is None:
68 filtered = self._filter_specs([spec])121 spec = self.context.target.getSpecification(token)
69 if len(filtered) > 0:122 if spec and self._is_valid_candidate(spec):
70 return self.toTerm(filtered[0])123 return self.toTerm(spec)
71 raise LookupError(token)124 raise LookupError(token)
72125
73 def search(self, query):126 def search(self, query):
@@ -79,6 +132,9 @@
79 """132 """
80 if not query:133 if not query:
81 return CountableIterator(0, [])134 return CountableIterator(0, [])
135 spec = self._spec_from_url(query)
136 if spec is not None and self._is_valid_candidate(spec):
137 return CountableIterator(1, [spec])
82 quoted_query = quote_like(query)138 quoted_query = quote_like(query)
83 sql_query = ("""139 sql_query = ("""
84 (Specification.name LIKE %s OR140 (Specification.name LIKE %s OR
@@ -87,18 +143,18 @@
87 """143 """
88 % (quoted_query, quoted_query, quoted_query))144 % (quoted_query, quoted_query, quoted_query))
89 all_specs = Specification.select(sql_query, orderBy=self._orderBy)145 all_specs = Specification.select(sql_query, orderBy=self._orderBy)
90 candidate_specs = self._filter_specs(all_specs)146 candidate_specs = self._filter_specs(all_specs, check_target=True)
91 return CountableIterator(len(candidate_specs), candidate_specs)147 return CountableIterator(len(candidate_specs), candidate_specs)
92148
93 @property149 @property
94 def _all_specs(self):150 def _all_specs(self):
95 return self.context.target.specifications(151 return self.context.target.specifications(
96 filter=[SpecificationFilter.ALL],152 filter=[SpecificationFilter.ALL], prejoin_people=False)
97 prejoin_people=False)
98153
99 def __iter__(self):154 def __iter__(self):
100 return (self.toTerm(spec)155 return (
101 for spec in self._filter_specs(self._all_specs))156 self.toTerm(spec) for spec in self._filter_specs(self._all_specs))
102157
103 def __contains__(self, obj):158 def __contains__(self, obj):
104 return obj in self._all_specs and len(self._filter_specs([obj])) > 0159 return self._is_valid_candidate(obj)
160
105161
=== modified file 'lib/lp/blueprints/vocabularies/tests/specificationdepcandidates.txt'
--- lib/lp/blueprints/vocabularies/tests/specificationdepcandidates.txt 2010-08-25 05:39:32 +0000
+++ lib/lp/blueprints/vocabularies/tests/specificationdepcandidates.txt 2010-08-27 04:52:44 +0000
@@ -22,26 +22,24 @@
22 >>> sorted([spec.name for spec in specced_product.specifications()])22 >>> sorted([spec.name for spec in specced_product.specifications()])
23 [u'spec-a', u'spec-b', u'spec-c']23 [u'spec-a', u'spec-b', u'spec-c']
2424
25The dependency candidates for spec_a are all blueprints for25
26specced_product except for spec_a itself.26Iterating over the vocabulary gives all blueprints for specced_product
27except for spec_a itself.
2728
28 >>> vocab = vocabulary_registry.get(29 >>> vocab = vocabulary_registry.get(
29 ... spec_a, "SpecificationDepCandidates")30 ... spec_a, "SpecificationDepCandidates")
30 >>> sorted([term.value.name for term in vocab])31 >>> sorted([term.value.name for term in vocab])
31 [u'spec-b', u'spec-c']32 [u'spec-b', u'spec-c']
3233
33Dependency candidate come only from the same product of the blueprint34Blueprints for other targets are considered to be 'in' the vocabulary
34they depend on.35though.
3536
36 >>> unrelated_spec = factory.makeSpecification(37 >>> unrelated_spec = factory.makeSpecification(
37 ... product=factory.makeProduct())38 ... product=factory.makeProduct())
38 >>> vocab = vocabulary_registry.get(39 >>> vocab = vocabulary_registry.get(
39 ... spec_a, "SpecificationDepCandidates")40 ... spec_a, "SpecificationDepCandidates")
40 >>> unrelated_spec in vocab41 >>> unrelated_spec in vocab
41 False42 True
42 >>> [term.value.product for term in vocab
43 ... if term.value.product != specced_product]
44 []
4543
46We mark spec_b as a dependency of spec_a and spec_c as a dependency of44We mark spec_b as a dependency of spec_a and spec_c as a dependency of
47spec_b.45spec_b.
4846
=== modified file 'lib/lp/blueprints/vocabularies/tests/test_specificationdependency.py'
--- lib/lp/blueprints/vocabularies/tests/test_specificationdependency.py 2010-08-26 22:07:41 +0000
+++ lib/lp/blueprints/vocabularies/tests/test_specificationdependency.py 2010-08-27 04:52:44 +0000
@@ -10,6 +10,7 @@
1010
11from zope.schema.vocabulary import getVocabularyRegistry11from zope.schema.vocabulary import getVocabularyRegistry
1212
13from canonical.launchpad.webapp import canonical_url
13from canonical.testing import DatabaseFunctionalLayer14from canonical.testing import DatabaseFunctionalLayer
1415
15from lp.testing import TestCaseWithFactory16from lp.testing import TestCaseWithFactory
@@ -24,7 +25,7 @@
24 return getVocabularyRegistry().get(25 return getVocabularyRegistry().get(
25 spec, name='SpecificationDepCandidates')26 spec, name='SpecificationDepCandidates')
2627
27 def test_getTermByToken_product(self):28 def test_getTermByToken_by_name_for_product(self):
28 # Calling getTermByToken for a dependency vocab for a spec from a29 # Calling getTermByToken for a dependency vocab for a spec from a
29 # product with the name of an acceptable candidate spec returns the30 # product with the name of an acceptable candidate spec returns the
30 # term for the candidate31 # term for the candidate
@@ -35,7 +36,7 @@
35 self.assertEqual(36 self.assertEqual(
36 candidate, vocab.getTermByToken(candidate.name).value)37 candidate, vocab.getTermByToken(candidate.name).value)
3738
38 def test_getTermByToken_distro(self):39 def test_getTermByToken_by_name_for_distro(self):
39 # Calling getTermByToken for a dependency vocab for a spec from a40 # Calling getTermByToken for a dependency vocab for a spec from a
40 # distribution with the name of an acceptable candidate spec returns41 # distribution with the name of an acceptable candidate spec returns
41 # the term for the candidate42 # the term for the candidate
@@ -46,7 +47,55 @@
46 self.assertEqual(47 self.assertEqual(
47 candidate, vocab.getTermByToken(candidate.name).value)48 candidate, vocab.getTermByToken(candidate.name).value)
4849
49 def test_getTermByToken_disallows_blocked(self):50 def test_getTermByToken_by_url_for_product(self):
51 # Calling getTermByToken with the full URL for a spec on a product
52 # returns that spec, irrespective of the context's target.
53 spec = self.factory.makeSpecification()
54 candidate = self.factory.makeSpecification(
55 product=self.factory.makeProduct())
56 vocab = self.getVocabularyForSpec(spec)
57 self.assertEqual(
58 candidate, vocab.getTermByToken(canonical_url(candidate)).value)
59
60 def test_getTermByToken_by_url_for_distro(self):
61 # Calling getTermByToken with the full URL for a spec on a
62 # distribution returns that spec, irrespective of the context's
63 # target.
64 spec = self.factory.makeSpecification()
65 candidate = self.factory.makeSpecification(
66 distribution=self.factory.makeDistribution())
67 vocab = self.getVocabularyForSpec(spec)
68 self.assertEqual(
69 candidate, vocab.getTermByToken(canonical_url(candidate)).value)
70
71 def test_getTermByToken_lookup_error_on_nonsense(self):
72 # getTermByToken with the a string that does not name a spec raises
73 # LookupError.
74 product = self.factory.makeProduct()
75 spec = self.factory.makeSpecification(product=product)
76 vocab = self.getVocabularyForSpec(spec)
77 self.assertRaises(
78 LookupError, vocab.getTermByToken, self.factory.getUniqueString())
79
80 def test_getTermByToken_lookup_error_on_url_with_invalid_pillar(self):
81 # getTermByToken with the a string that looks like a blueprint URL but
82 # has an invalid pillar name raises LookupError.
83 spec = self.factory.makeSpecification()
84 url = canonical_url(spec).replace(
85 spec.target.name, self.factory.getUniqueString())
86 vocab = self.getVocabularyForSpec(spec)
87 self.assertRaises(LookupError, vocab.getTermByToken, url)
88
89 def test_getTermByToken_lookup_error_on_url_with_invalid_spec_name(self):
90 # getTermByToken with the a string that looks like a blueprint URL but
91 # has an invalid spec name raises LookupError.
92 spec = self.factory.makeSpecification()
93 url = canonical_url(spec).replace(
94 spec.name, self.factory.getUniqueString())
95 vocab = self.getVocabularyForSpec(spec)
96 self.assertRaises(LookupError, vocab.getTermByToken, url)
97
98 def test_getTermByToken_by_name_disallows_blocked(self):
50 # getTermByToken with the name of an candidate spec that is blocked by99 # getTermByToken with the name of an candidate spec that is blocked by
51 # the vocab's context raises LookupError.100 # the vocab's context raises LookupError.
52 product = self.factory.makeProduct()101 product = self.factory.makeProduct()
@@ -56,17 +105,74 @@
56 vocab = self.getVocabularyForSpec(spec)105 vocab = self.getVocabularyForSpec(spec)
57 self.assertRaises(LookupError, vocab.getTermByToken, candidate.name)106 self.assertRaises(LookupError, vocab.getTermByToken, candidate.name)
58107
59 def test_getTermByToken_disallows_context(self):108 def test_getTermByToken_by_url_disallows_blocked(self):
109 # getTermByToken with the URL of an candidate spec that is blocked by
110 # the vocab's context raises LookupError.
111 spec = self.factory.makeSpecification()
112 candidate = self.factory.makeSpecification()
113 candidate.createDependency(spec)
114 vocab = self.getVocabularyForSpec(spec)
115 self.assertRaises(
116 LookupError, vocab.getTermByToken, canonical_url(candidate))
117
118 def test_getTermByToken_by_name_disallows_context(self):
60 # getTermByToken with the name of the vocab's context raises119 # getTermByToken with the name of the vocab's context raises
61 # LookupError.120 # LookupError.
62 spec = self.factory.makeSpecification()121 spec = self.factory.makeSpecification()
63 vocab = self.getVocabularyForSpec(spec)122 vocab = self.getVocabularyForSpec(spec)
64 self.assertRaises(LookupError, vocab.getTermByToken, spec.name)123 self.assertRaises(LookupError, vocab.getTermByToken, spec.name)
65124
66 def test_getTermByToken_disallows_spec_for_other_target(self):125 def test_getTermByToken_by_url_disallows_context(self):
126 # getTermByToken with the URL of the vocab's context raises
127 # LookupError.
128 spec = self.factory.makeSpecification()
129 vocab = self.getVocabularyForSpec(spec)
130 self.assertRaises(
131 LookupError, vocab.getTermByToken, canonical_url(spec))
132
133 def test_getTermByToken_by_name_disallows_spec_for_other_target(self):
67 # getTermByToken with the name of a spec with a different target134 # getTermByToken with the name of a spec with a different target
68 # raises LookupError.135 # raises LookupError.
69 spec = self.factory.makeSpecification()136 spec = self.factory.makeSpecification()
70 candidate = self.factory.makeSpecification()137 candidate = self.factory.makeSpecification()
71 vocab = self.getVocabularyForSpec(spec)138 vocab = self.getVocabularyForSpec(spec)
72 self.assertRaises(LookupError, vocab.getTermByToken, candidate.name)139 self.assertRaises(LookupError, vocab.getTermByToken, candidate.name)
140
141 def test_searchForTerms_by_url(self):
142 # Calling searchForTerms with the URL of a valid candidate spec
143 # returns just that spec.
144 spec = self.factory.makeSpecification()
145 candidate = self.factory.makeSpecification()
146 vocab = self.getVocabularyForSpec(spec)
147 results = vocab.searchForTerms(canonical_url(candidate))
148 self.assertEqual(1, len(results))
149 self.assertEqual(candidate, list(results)[0].value)
150
151 def test_searchForTerms_by_url_rejects_invalid(self):
152 # Calling searchForTerms with the URL of a invalid candidate spec
153 # returns an empty iterator.
154 spec = self.factory.makeSpecification()
155 candidate = self.factory.makeSpecification()
156 candidate.createDependency(spec)
157 vocab = self.getVocabularyForSpec(spec)
158 results = vocab.searchForTerms(canonical_url(candidate))
159 self.assertEqual(0, len(results))
160
161 def test_token_for_same_target_dep_is_name(self):
162 # The 'token' part of the term for a dependency candidate that has the
163 # same target is just the name of the candidate.
164 product = self.factory.makeProduct()
165 spec = self.factory.makeSpecification(product=product)
166 candidate = self.factory.makeSpecification(product=product)
167 vocab = self.getVocabularyForSpec(spec)
168 term = vocab.getTermByToken(candidate.name)
169 self.assertEqual(term.token, candidate.name)
170
171 def test_token_for_different_target_dep_is_url(self):
172 # The 'token' part of the term for a dependency candidate that has a
173 # different target is the canonical url of the candidate.
174 spec = self.factory.makeSpecification()
175 candidate = self.factory.makeSpecification()
176 vocab = self.getVocabularyForSpec(spec)
177 term = vocab.getTermByToken(canonical_url(candidate))
178 self.assertEqual(term.token, canonical_url(candidate))
73179
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-08-26 22:44:30 +0000
+++ lib/lp/testing/factory.py 2010-08-27 04:52:44 +0000
@@ -1496,7 +1496,7 @@
1496 return mail1496 return mail
14971497
1498 def makeSpecification(self, product=None, title=None, distribution=None,1498 def makeSpecification(self, product=None, title=None, distribution=None,
1499 name=None, summary=None,1499 name=None, summary=None, owner=None,
1500 status=SpecificationDefinitionStatus.NEW):1500 status=SpecificationDefinitionStatus.NEW):
1501 """Create and return a new, arbitrary Blueprint.1501 """Create and return a new, arbitrary Blueprint.
15021502
@@ -1511,13 +1511,15 @@
1511 summary = self.getUniqueString('summary')1511 summary = self.getUniqueString('summary')
1512 if title is None:1512 if title is None:
1513 title = self.getUniqueString('title')1513 title = self.getUniqueString('title')
1514 if owner is None:
1515 owner = self.makePerson()
1514 return getUtility(ISpecificationSet).new(1516 return getUtility(ISpecificationSet).new(
1515 name=name,1517 name=name,
1516 title=title,1518 title=title,
1517 specurl=None,1519 specurl=None,
1518 summary=summary,1520 summary=summary,
1519 definition_status=status,1521 definition_status=status,
1520 owner=self.makePerson(),1522 owner=owner,
1521 product=product,1523 product=product,
1522 distribution=distribution)1524 distribution=distribution)
15231525