Launchpad itself

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

  1. cross-product-spec-links-bug-3552
  2. Merge into devel
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
1=== modified file 'lib/lp/blueprints/browser/specificationdependency.py'
2--- lib/lp/blueprints/browser/specificationdependency.py 2010-08-26 03:02:29 +0000
3+++ lib/lp/blueprints/browser/specificationdependency.py 2010-08-27 04:52:44 +0000
4@@ -37,7 +37,9 @@
5 "If another blueprint needs to be fully implemented "
6 "before this feature can be started, then specify that "
7 "dependency here so Launchpad knows about it and can "
8- "give you an accurate project plan."))
9+ "give you an accurate project plan. You can enter the "
10+ "name of a blueprint that has the same target, or the "
11+ "URL of any blueprint."))
12
13
14 class SpecificationDependencyAddView(LaunchpadFormView):
15
16=== added file 'lib/lp/blueprints/browser/tests/test_specificationdependency.py'
17--- lib/lp/blueprints/browser/tests/test_specificationdependency.py 1970-01-01 00:00:00 +0000
18+++ lib/lp/blueprints/browser/tests/test_specificationdependency.py 2010-08-27 04:52:44 +0000
19@@ -0,0 +1,33 @@
20+# Copyright 2010 Canonical Ltd. This software is licensed under the
21+# GNU Affero General Public License version 3 (see the file LICENSE).
22+
23+"""Tests for the specification dependency views.
24+
25+There are also tests in lp/blueprints/stories/blueprints/xx-dependencies.txt.
26+"""
27+
28+__metaclass__ = type
29+
30+import unittest
31+
32+from canonical.launchpad.webapp import canonical_url
33+from canonical.testing import DatabaseFunctionalLayer
34+from lp.testing import BrowserTestCase
35+
36+
37+class TestAddDependency(BrowserTestCase):
38+ layer = DatabaseFunctionalLayer
39+
40+ def test_add_dependency_by_url(self):
41+ # It is possible to use the URL of a specification in the "Depends On"
42+ # field of the form to add a dependency to a spec.
43+ spec = self.factory.makeSpecification(owner=self.user)
44+ dependency = self.factory.makeSpecification()
45+ browser = self.getViewBrowser(spec, '+linkdependency')
46+ browser.getControl('Depends On').value = canonical_url(dependency)
47+ browser.getControl('Continue').click()
48+ self.assertIn(dependency, spec.dependencies)
49+
50+
51+def test_suite():
52+ return unittest.TestLoader().loadTestsFromName(__name__)
53
54=== modified file 'lib/lp/blueprints/vocabularies/specificationdependency.py'
55--- lib/lp/blueprints/vocabularies/specificationdependency.py 2010-08-26 03:30:31 +0000
56+++ lib/lp/blueprints/vocabularies/specificationdependency.py 2010-08-27 04:52:44 +0000
57@@ -6,11 +6,16 @@
58 __metaclass__ = type
59 __all__ = ['SpecificationDepCandidatesVocabulary']
60
61+from zope.component import getUtility
62 from zope.interface import implements
63 from zope.schema.vocabulary import SimpleTerm
64
65 from canonical.database.sqlbase import quote_like
66 from canonical.launchpad.helpers import shortlist
67+from canonical.launchpad.webapp import (
68+ canonical_url,
69+ urlparse,
70+ )
71 from canonical.launchpad.webapp.vocabulary import (
72 CountableIterator,
73 IHugeVocabulary,
74@@ -19,15 +24,26 @@
75
76 from lp.blueprints.interfaces.specification import SpecificationFilter
77 from lp.blueprints.model.specification import Specification
78-
79+from lp.registry.interfaces.pillar import IPillarNameSet
80
81 class SpecificationDepCandidatesVocabulary(SQLObjectVocabularyBase):
82 """Specifications that could be dependencies of this spec.
83
84- This includes only those specs that are not blocked by this spec
85- (directly or indirectly), unless they are already dependencies.
86-
87- The current spec is not included.
88+ This includes only those specs that are not blocked by this spec (directly
89+ or indirectly), unless they are already dependencies.
90+
91+ This vocabulary has a bit of a split personality.
92+
93+ Tokens are *either*:
94+
95+ - the name of a spec, in which case it must be a spec on the same target
96+ as the context, or
97+ - the full URL of the spec, in which case it can be any spec at all.
98+
99+ For the purposes of enumeration and searching we only consider the first
100+ sort of spec for now. The URL form of token only matches precisely,
101+ searching only looks for specs on the current target if the search term is
102+ not a URL.
103 """
104
105 implements(IHugeVocabulary)
106@@ -36,8 +52,8 @@
107 _orderBy = 'name'
108 displayname = 'Select a blueprint'
109
110- def _filter_specs(self, specs):
111- """Filter `specs` to remove invalid candidates.
112+ def _is_valid_candidate(self, spec, check_target=False):
113+ """Is `spec` a valid candidate spec for self.context?
114
115 Invalid candidates are:
116
117@@ -47,27 +63,64 @@
118
119 Preventing the last category prevents loops in the dependency graph.
120 """
121+ if check_target and spec.target != self.context.target:
122+ return False
123+ return spec != self.context and spec not in self.context.all_blocked
124+
125+ def _filter_specs(self, specs, check_target=False):
126+ """Filter `specs` to remove invalid candidates.
127+
128+ See `_is_valid_candidate` for what an invalid candidate is.
129+ """
130 # XXX intellectronica 2007-07-05: is 100 a reasonable count before
131 # starting to warn?
132- speclist = shortlist(specs, 100)
133- return [spec for spec in speclist
134- if (spec != self.context and
135- spec.target == self.context.target
136- and spec not in self.context.all_blocked)]
137+ return [spec for spec in shortlist(specs, 100)
138+ if self._is_valid_candidate(spec, check_target)]
139
140 def toTerm(self, obj):
141- return SimpleTerm(obj, obj.name, obj.title)
142+ if obj.target == self.context.target:
143+ token = obj.name
144+ else:
145+ token = canonical_url(obj)
146+ return SimpleTerm(obj, token, obj.title)
147+
148+ def _spec_from_url(self, url):
149+ """If `url` is the URL of a specification, return it.
150+
151+ This implementation is a little fuzzy and will return specs for URLs
152+ that, for example, don't have the host name right. This seems
153+ unlikely to cause confusion in practice, and being too anal probably
154+ would be confusing (e.g. not accepting edge URLs on lpnet).
155+ """
156+ scheme, netloc, path, params, args, fragment = urlparse(url)
157+ if not scheme or not netloc:
158+ # Not enough like a URL
159+ return None
160+ path_segments = path.strip('/').split('/')
161+ if len(path_segments) != 3:
162+ # Can't be a spec url
163+ return None
164+ pillar_name, plus_spec, spec_name = path_segments
165+ if plus_spec != '+spec':
166+ # Can't be a spec url
167+ return None
168+ pillar = getUtility(IPillarNameSet).getByName(
169+ pillar_name, ignore_inactive=True)
170+ if pillar is None:
171+ return None
172+ return pillar.getSpecification(spec_name)
173
174 def getTermByToken(self, token):
175 """See `zope.schema.interfaces.IVocabularyTokenized`.
176
177- The tokens for specifications are just the name of the spec.
178+ The tokens for specifications are either the name of a spec on the
179+ same target or a URL for a spec.
180 """
181- spec = self.context.target.getSpecification(token)
182- if spec is not None:
183- filtered = self._filter_specs([spec])
184- if len(filtered) > 0:
185- return self.toTerm(filtered[0])
186+ spec = self._spec_from_url(token)
187+ if spec is None:
188+ spec = self.context.target.getSpecification(token)
189+ if spec and self._is_valid_candidate(spec):
190+ return self.toTerm(spec)
191 raise LookupError(token)
192
193 def search(self, query):
194@@ -79,6 +132,9 @@
195 """
196 if not query:
197 return CountableIterator(0, [])
198+ spec = self._spec_from_url(query)
199+ if spec is not None and self._is_valid_candidate(spec):
200+ return CountableIterator(1, [spec])
201 quoted_query = quote_like(query)
202 sql_query = ("""
203 (Specification.name LIKE %s OR
204@@ -87,18 +143,18 @@
205 """
206 % (quoted_query, quoted_query, quoted_query))
207 all_specs = Specification.select(sql_query, orderBy=self._orderBy)
208- candidate_specs = self._filter_specs(all_specs)
209+ candidate_specs = self._filter_specs(all_specs, check_target=True)
210 return CountableIterator(len(candidate_specs), candidate_specs)
211
212 @property
213 def _all_specs(self):
214 return self.context.target.specifications(
215- filter=[SpecificationFilter.ALL],
216- prejoin_people=False)
217+ filter=[SpecificationFilter.ALL], prejoin_people=False)
218
219 def __iter__(self):
220- return (self.toTerm(spec)
221- for spec in self._filter_specs(self._all_specs))
222+ return (
223+ self.toTerm(spec) for spec in self._filter_specs(self._all_specs))
224
225 def __contains__(self, obj):
226- return obj in self._all_specs and len(self._filter_specs([obj])) > 0
227+ return self._is_valid_candidate(obj)
228+
229
230=== modified file 'lib/lp/blueprints/vocabularies/tests/specificationdepcandidates.txt'
231--- lib/lp/blueprints/vocabularies/tests/specificationdepcandidates.txt 2010-08-25 05:39:32 +0000
232+++ lib/lp/blueprints/vocabularies/tests/specificationdepcandidates.txt 2010-08-27 04:52:44 +0000
233@@ -22,26 +22,24 @@
234 >>> sorted([spec.name for spec in specced_product.specifications()])
235 [u'spec-a', u'spec-b', u'spec-c']
236
237-The dependency candidates for spec_a are all blueprints for
238-specced_product except for spec_a itself.
239+
240+Iterating over the vocabulary gives all blueprints for specced_product
241+except for spec_a itself.
242
243 >>> vocab = vocabulary_registry.get(
244 ... spec_a, "SpecificationDepCandidates")
245 >>> sorted([term.value.name for term in vocab])
246 [u'spec-b', u'spec-c']
247
248-Dependency candidate come only from the same product of the blueprint
249-they depend on.
250+Blueprints for other targets are considered to be 'in' the vocabulary
251+though.
252
253 >>> unrelated_spec = factory.makeSpecification(
254 ... product=factory.makeProduct())
255 >>> vocab = vocabulary_registry.get(
256 ... spec_a, "SpecificationDepCandidates")
257 >>> unrelated_spec in vocab
258- False
259- >>> [term.value.product for term in vocab
260- ... if term.value.product != specced_product]
261- []
262+ True
263
264 We mark spec_b as a dependency of spec_a and spec_c as a dependency of
265 spec_b.
266
267=== modified file 'lib/lp/blueprints/vocabularies/tests/test_specificationdependency.py'
268--- lib/lp/blueprints/vocabularies/tests/test_specificationdependency.py 2010-08-26 22:07:41 +0000
269+++ lib/lp/blueprints/vocabularies/tests/test_specificationdependency.py 2010-08-27 04:52:44 +0000
270@@ -10,6 +10,7 @@
271
272 from zope.schema.vocabulary import getVocabularyRegistry
273
274+from canonical.launchpad.webapp import canonical_url
275 from canonical.testing import DatabaseFunctionalLayer
276
277 from lp.testing import TestCaseWithFactory
278@@ -24,7 +25,7 @@
279 return getVocabularyRegistry().get(
280 spec, name='SpecificationDepCandidates')
281
282- def test_getTermByToken_product(self):
283+ def test_getTermByToken_by_name_for_product(self):
284 # Calling getTermByToken for a dependency vocab for a spec from a
285 # product with the name of an acceptable candidate spec returns the
286 # term for the candidate
287@@ -35,7 +36,7 @@
288 self.assertEqual(
289 candidate, vocab.getTermByToken(candidate.name).value)
290
291- def test_getTermByToken_distro(self):
292+ def test_getTermByToken_by_name_for_distro(self):
293 # Calling getTermByToken for a dependency vocab for a spec from a
294 # distribution with the name of an acceptable candidate spec returns
295 # the term for the candidate
296@@ -46,7 +47,55 @@
297 self.assertEqual(
298 candidate, vocab.getTermByToken(candidate.name).value)
299
300- def test_getTermByToken_disallows_blocked(self):
301+ def test_getTermByToken_by_url_for_product(self):
302+ # Calling getTermByToken with the full URL for a spec on a product
303+ # returns that spec, irrespective of the context's target.
304+ spec = self.factory.makeSpecification()
305+ candidate = self.factory.makeSpecification(
306+ product=self.factory.makeProduct())
307+ vocab = self.getVocabularyForSpec(spec)
308+ self.assertEqual(
309+ candidate, vocab.getTermByToken(canonical_url(candidate)).value)
310+
311+ def test_getTermByToken_by_url_for_distro(self):
312+ # Calling getTermByToken with the full URL for a spec on a
313+ # distribution returns that spec, irrespective of the context's
314+ # target.
315+ spec = self.factory.makeSpecification()
316+ candidate = self.factory.makeSpecification(
317+ distribution=self.factory.makeDistribution())
318+ vocab = self.getVocabularyForSpec(spec)
319+ self.assertEqual(
320+ candidate, vocab.getTermByToken(canonical_url(candidate)).value)
321+
322+ def test_getTermByToken_lookup_error_on_nonsense(self):
323+ # getTermByToken with the a string that does not name a spec raises
324+ # LookupError.
325+ product = self.factory.makeProduct()
326+ spec = self.factory.makeSpecification(product=product)
327+ vocab = self.getVocabularyForSpec(spec)
328+ self.assertRaises(
329+ LookupError, vocab.getTermByToken, self.factory.getUniqueString())
330+
331+ def test_getTermByToken_lookup_error_on_url_with_invalid_pillar(self):
332+ # getTermByToken with the a string that looks like a blueprint URL but
333+ # has an invalid pillar name raises LookupError.
334+ spec = self.factory.makeSpecification()
335+ url = canonical_url(spec).replace(
336+ spec.target.name, self.factory.getUniqueString())
337+ vocab = self.getVocabularyForSpec(spec)
338+ self.assertRaises(LookupError, vocab.getTermByToken, url)
339+
340+ def test_getTermByToken_lookup_error_on_url_with_invalid_spec_name(self):
341+ # getTermByToken with the a string that looks like a blueprint URL but
342+ # has an invalid spec name raises LookupError.
343+ spec = self.factory.makeSpecification()
344+ url = canonical_url(spec).replace(
345+ spec.name, self.factory.getUniqueString())
346+ vocab = self.getVocabularyForSpec(spec)
347+ self.assertRaises(LookupError, vocab.getTermByToken, url)
348+
349+ def test_getTermByToken_by_name_disallows_blocked(self):
350 # getTermByToken with the name of an candidate spec that is blocked by
351 # the vocab's context raises LookupError.
352 product = self.factory.makeProduct()
353@@ -56,17 +105,74 @@
354 vocab = self.getVocabularyForSpec(spec)
355 self.assertRaises(LookupError, vocab.getTermByToken, candidate.name)
356
357- def test_getTermByToken_disallows_context(self):
358+ def test_getTermByToken_by_url_disallows_blocked(self):
359+ # getTermByToken with the URL of an candidate spec that is blocked by
360+ # the vocab's context raises LookupError.
361+ spec = self.factory.makeSpecification()
362+ candidate = self.factory.makeSpecification()
363+ candidate.createDependency(spec)
364+ vocab = self.getVocabularyForSpec(spec)
365+ self.assertRaises(
366+ LookupError, vocab.getTermByToken, canonical_url(candidate))
367+
368+ def test_getTermByToken_by_name_disallows_context(self):
369 # getTermByToken with the name of the vocab's context raises
370 # LookupError.
371 spec = self.factory.makeSpecification()
372 vocab = self.getVocabularyForSpec(spec)
373 self.assertRaises(LookupError, vocab.getTermByToken, spec.name)
374
375- def test_getTermByToken_disallows_spec_for_other_target(self):
376+ def test_getTermByToken_by_url_disallows_context(self):
377+ # getTermByToken with the URL of the vocab's context raises
378+ # LookupError.
379+ spec = self.factory.makeSpecification()
380+ vocab = self.getVocabularyForSpec(spec)
381+ self.assertRaises(
382+ LookupError, vocab.getTermByToken, canonical_url(spec))
383+
384+ def test_getTermByToken_by_name_disallows_spec_for_other_target(self):
385 # getTermByToken with the name of a spec with a different target
386 # raises LookupError.
387 spec = self.factory.makeSpecification()
388 candidate = self.factory.makeSpecification()
389 vocab = self.getVocabularyForSpec(spec)
390 self.assertRaises(LookupError, vocab.getTermByToken, candidate.name)
391+
392+ def test_searchForTerms_by_url(self):
393+ # Calling searchForTerms with the URL of a valid candidate spec
394+ # returns just that spec.
395+ spec = self.factory.makeSpecification()
396+ candidate = self.factory.makeSpecification()
397+ vocab = self.getVocabularyForSpec(spec)
398+ results = vocab.searchForTerms(canonical_url(candidate))
399+ self.assertEqual(1, len(results))
400+ self.assertEqual(candidate, list(results)[0].value)
401+
402+ def test_searchForTerms_by_url_rejects_invalid(self):
403+ # Calling searchForTerms with the URL of a invalid candidate spec
404+ # returns an empty iterator.
405+ spec = self.factory.makeSpecification()
406+ candidate = self.factory.makeSpecification()
407+ candidate.createDependency(spec)
408+ vocab = self.getVocabularyForSpec(spec)
409+ results = vocab.searchForTerms(canonical_url(candidate))
410+ self.assertEqual(0, len(results))
411+
412+ def test_token_for_same_target_dep_is_name(self):
413+ # The 'token' part of the term for a dependency candidate that has the
414+ # same target is just the name of the candidate.
415+ product = self.factory.makeProduct()
416+ spec = self.factory.makeSpecification(product=product)
417+ candidate = self.factory.makeSpecification(product=product)
418+ vocab = self.getVocabularyForSpec(spec)
419+ term = vocab.getTermByToken(candidate.name)
420+ self.assertEqual(term.token, candidate.name)
421+
422+ def test_token_for_different_target_dep_is_url(self):
423+ # The 'token' part of the term for a dependency candidate that has a
424+ # different target is the canonical url of the candidate.
425+ spec = self.factory.makeSpecification()
426+ candidate = self.factory.makeSpecification()
427+ vocab = self.getVocabularyForSpec(spec)
428+ term = vocab.getTermByToken(canonical_url(candidate))
429+ self.assertEqual(term.token, canonical_url(candidate))
430
431=== modified file 'lib/lp/testing/factory.py'
432--- lib/lp/testing/factory.py 2010-08-26 22:44:30 +0000
433+++ lib/lp/testing/factory.py 2010-08-27 04:52:44 +0000
434@@ -1496,7 +1496,7 @@
435 return mail
436
437 def makeSpecification(self, product=None, title=None, distribution=None,
438- name=None, summary=None,
439+ name=None, summary=None, owner=None,
440 status=SpecificationDefinitionStatus.NEW):
441 """Create and return a new, arbitrary Blueprint.
442
443@@ -1511,13 +1511,15 @@
444 summary = self.getUniqueString('summary')
445 if title is None:
446 title = self.getUniqueString('title')
447+ if owner is None:
448+ owner = self.makePerson()
449 return getUtility(ISpecificationSet).new(
450 name=name,
451 title=title,
452 specurl=None,
453 summary=summary,
454 definition_status=status,
455- owner=self.makePerson(),
456+ owner=owner,
457 product=product,
458 distribution=distribution)
459