Merge lp:~allenap/launchpad/initseries-for-one-night-only-bug-793620 into lp:launchpad

Proposed by Gavin Panella
Status: Merged
Approved by: Julian Edwards
Approved revision: no longer in the source branch.
Merged at revision: 13200
Proposed branch: lp:~allenap/launchpad/initseries-for-one-night-only-bug-793620
Merge into: lp:launchpad
Diff against target: 587 lines (+199/-52)
11 files modified
lib/lp/registry/browser/distroseries.py (+23/-0)
lib/lp/registry/browser/tests/test_distroseries.py (+52/-16)
lib/lp/registry/interfaces/distroseries.py (+14/-11)
lib/lp/registry/model/distroseries.py (+11/-7)
lib/lp/registry/templates/distroseries-index.pt (+2/-1)
lib/lp/registry/templates/distroseries-initialize.pt (+19/-3)
lib/lp/registry/templates/distroseries-portlet-derivation.pt (+2/-2)
lib/lp/registry/tests/test_distroseries.py (+17/-8)
lib/lp/testing/__init__.py (+1/-1)
lib/lp/testing/matchers.py (+24/-3)
lib/lp/testing/tests/test_matchers.py (+34/-0)
To merge this branch: bzr merge lp:~allenap/launchpad/initseries-for-one-night-only-bug-793620
Reviewer Review Type Date Requested Status
Julian Edwards (community) Approve
Steve Kowalik (community) code Needs Fixing
Review via email: mp+63756@code.launchpad.net

Commit message

[r=julian-edwards][bug=793620] Show a message on DistroSeries:+initseries, and prevent attempts to use the page, when the distroseries is in the process of being initialized or when the distroseries has already been initialized.

Description of the change

Show a message on +initseries, and prevent attempts to use the page,
when the distroseries is in the process of being derived or when the
distroseries has already been derived.

To post a comment you must log in.
Revision history for this message
Steve Kowalik (stevenk) wrote :

I think you're conflating things were they don't need to be. For +initseries, you only really care if the distroseries has been initialised, not if it is a derived series.

The with featureflag(): section reads as a little messy -- I thought we had FeatureFixture for that which makes it easier?

Thank you for cleaning up the imports, but I don't think you can import DSDJ from lp.soyuz.model? Or if you can, why are you importing the whole module?

review: Needs Fixing (code)
Revision history for this message
Gavin Panella (allenap) wrote :

Thanks for the review!

> I think you're conflating things were they don't need to be. For
> +initseries, you only really care if the distroseries has been
> initialised, not if it is a derived series.

I have completely lost track of what this all means.

DistroSeries.is_derived_series checked if there are any DSPs, which
are, iirc, created by InitializeDistroSeries. As I understand it, they
are a marker that the distroseries has been initialized, that it is
now a derived distroseries. If initializing and deriving are very
different things that I'm conflating then I pity the users!

> The with featureflag(): section reads as a little messy -- I thought
> we had FeatureFixture for that which makes it easier?

I cargocult-n-pasted that. I've changed it to use FeatureFixture().

> Thank you for cleaning up the imports, but I don't think you can import DSDJ
> from lp.soyuz.model? Or if you can, why are you importing the whole module?

I didn't clean up the imports, utilities/format-imports took care of
that (by way of utilities/format-new-and-modified-imports). The
machine overlord has made its decision.

Revision history for this message
Gavin Panella (allenap) wrote :

Okay, Julian explained it to me. The follow-up is quite large, so I'll
explain what I've done:

- Introduced a new matched, EqualsIgnoringWhitespace. Yes, there are
  other ways to do this, but this seems to be a common test that was
  worth cementing. I suppose DocTestMatches is the closest thing to
  it, but this is simpler to grok.

- Removes DistroSeries.is_initialising. As Julian pointed out, this
  hits the database so would be better as a method...

- Introduce DistroSeries.isInitializing() and .isInitialized(). The
  former replaces .is_initialising and the latter reports if there are
  any published sources in the series' main archive.

- These are then used in the +initseries template.

- I also fixed some lint and imports.

1=== modified file 'lib/lp/registry/browser/distroseries.py'
2--- lib/lp/registry/browser/distroseries.py 2011-06-07 19:49:53 +0000
3+++ lib/lp/registry/browser/distroseries.py 2011-06-09 12:04:01 +0000
4@@ -654,20 +654,20 @@
5 def show_derivation_form(self):
6 return (
7 self.is_derived_series_feature_enabled and
8- not self.context.is_derived_series and
9- not self.context.is_initialising)
10+ not self.context.isInitializing() and
11+ not self.context.isInitialized())
12
13 @property
14- def show_already_derived_message(self):
15+ def show_already_initialized_message(self):
16 return (
17 self.is_derived_series_feature_enabled and
18- self.context.is_derived_series)
19+ self.context.isInitialized())
20
21 @property
22 def show_already_initializing_message(self):
23 return (
24 self.is_derived_series_feature_enabled and
25- self.context.is_initialising)
26+ self.context.isInitializing())
27
28 @property
29 def next_url(self):
30
31=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
32--- lib/lp/registry/browser/tests/test_distroseries.py 2011-06-08 08:22:07 +0000
33+++ lib/lp/registry/browser/tests/test_distroseries.py 2011-06-09 12:05:09 +0000
34@@ -85,7 +85,10 @@
35 with_celebrity_logged_in,
36 )
37 from lp.testing.fakemethod import FakeMethod
38-from lp.testing.matchers import HasQueryCount
39+from lp.testing.matchers import (
40+ EqualsIgnoringWhitespace,
41+ HasQueryCount,
42+ )
43 from lp.testing.views import create_initialized_view
44
45
46@@ -321,7 +324,7 @@
47 view.request.features = get_relevant_feature_controller()
48 html_content = view()
49
50- self.assertTrue(derived_series.is_initialising)
51+ self.assertTrue(derived_series.isInitializing())
52 self.assertThat(html_content, portlet_display)
53
54
55@@ -455,9 +458,10 @@
56
57 def test_form_hidden_when_distroseries_is_initialized(self):
58 # The form is hidden when the feature flag is set but the series has
59- # already been derived.
60+ # already been initialized.
61 distroseries = self.factory.makeDistroSeries()
62- self.factory.makeDistroSeriesParent(derived_series=distroseries)
63+ self.factory.makeSourcePackagePublishingHistory(
64+ distroseries=distroseries, archive=distroseries.main_archive)
65 view = create_initialized_view(distroseries, "+initseries")
66 flags = {u"soyuz.derived_series_ui.enabled": u"true"}
67 with FeatureFixture(flags):
68@@ -466,9 +470,10 @@
69 [], root.cssselect("#initseries-form-container"))
70 # Instead an explanatory message is shown.
71 [message] = root.cssselect("p.error.message")
72- self.assertEqual(
73- u"This series has already been derived.",
74- message.text.strip())
75+ self.assertThat(
76+ message.text.strip(), EqualsIgnoringWhitespace(
77+ u"This series already contains source packages "
78+ u"and cannot be initialized again."))
79
80 def test_form_hidden_when_distroseries_is_being_initialized(self):
81 # The form is hidden when the feature flag is set but the series has
82@@ -484,9 +489,10 @@
83 [], root.cssselect("#initseries-form-container"))
84 # Instead an explanatory message is shown.
85 [message] = root.cssselect("p.error.message")
86- self.assertEqual(
87- u"This series is already being initialized.",
88- message.text.strip())
89+ self.assertThat(
90+ message.text.strip(),
91+ EqualsIgnoringWhitespace(
92+ u"This series is already being initialized."))
93
94
95 class DistroSeriesDifferenceMixin:
96
97=== modified file 'lib/lp/registry/interfaces/distroseries.py'
98--- lib/lp/registry/interfaces/distroseries.py 2011-05-31 15:45:19 +0000
99+++ lib/lp/registry/interfaces/distroseries.py 2011-06-09 12:07:12 +0000
100@@ -213,7 +213,7 @@
101 description=_("The version string for this series.")))
102 distribution = exported(
103 Reference(
104- Interface, # Really IDistribution, see circular import fix below.
105+ Interface, # Really IDistribution, see circular import fix below.
106 title=_("Distribution"), required=True,
107 description=_("The distribution for which this is a series.")))
108 distributionID = Attribute('The distribution ID.')
109@@ -239,16 +239,13 @@
110 is_derived_series = Bool(
111 title=u'Is this series a derived series?', readonly=True,
112 description=(u"Whether or not this series is a derived series."))
113- is_initialising = Bool(
114- title=u'Is this series initialising?', readonly=True,
115- description=(u"Whether or not this series is initialising."))
116 datereleased = exported(
117 Datetime(title=_("Date released")))
118 previous_series = exported(
119 ReferenceChoice(
120 title=_("Parent series"),
121 description=_("The series from which this one was branched."),
122- required=True, schema=Interface, # Really IDistroSeries, see below
123+ required=True, schema=Interface, # Really IDistroSeries, see below
124 vocabulary='DistroSeries'),
125 ("devel", dict(exported_as="previous_series")),
126 ("1.0", dict(exported_as="parent_series")),
127@@ -370,7 +367,7 @@
128
129 main_archive = exported(
130 Reference(
131- Interface, # Really IArchive, see below for circular import fix.
132+ Interface, # Really IArchive, see below for circular import fix.
133 title=_('Distribution Main Archive')))
134
135 supported = exported(
136@@ -418,7 +415,7 @@
137 architectures = exported(
138 CollectionField(
139 title=_("All architectures in this series."),
140- value_type=Reference(schema=Interface), # IDistroArchSeries.
141+ value_type=Reference(schema=Interface), # IDistroArchSeries.
142 readonly=True))
143
144 enabled_architectures = Attribute(
145@@ -848,18 +845,18 @@
146
147 @operation_parameters(
148 parent_series=Reference(
149- schema=Interface, # IDistroSeries
150+ schema=Interface, # IDistroSeries
151 title=_("The parent series to consider."),
152 required=False),
153 difference_type=Choice(
154- vocabulary=DBEnumeratedType, # DistroSeriesDifferenceType
155+ vocabulary=DBEnumeratedType, # DistroSeriesDifferenceType
156 title=_("Only return differences of this type."), required=False),
157 source_package_name_filter=TextLine(
158 title=_("Only return differences for packages matching this "
159 "name."),
160 required=False),
161 status=Choice(
162- vocabulary=DBEnumeratedType, # DistroSeriesDifferenceStatus
163+ vocabulary=DBEnumeratedType, # DistroSeriesDifferenceStatus
164 title=_("Only return differences of this status."),
165 required=False),
166 child_version_higher=Bool(
167@@ -886,6 +883,12 @@
168 child's version is higher than the parent's version.
169 """
170
171+ def isInitializing(self):
172+ """Is this series initializing?"""
173+
174+ def isInitialized(self):
175+ """Has this series been initialized?"""
176+
177
178 class IDistroSeriesEditRestricted(Interface):
179 """IDistroSeries properties which require launchpad.Edit."""
180@@ -1032,7 +1035,7 @@
181
182 class DerivationError(Exception):
183 """Raised when there is a problem deriving a distroseries."""
184- webservice_error(400) # Bad Request
185+ webservice_error(400) # Bad Request
186 _message_prefix = "Error deriving distro series"
187
188
189
190=== modified file 'lib/lp/registry/model/distroseries.py'
191--- lib/lp/registry/model/distroseries.py 2011-05-31 15:45:19 +0000
192+++ lib/lp/registry/model/distroseries.py 2011-06-09 11:58:23 +0000
193@@ -801,13 +801,6 @@
194 return not self.getParentSeries() == []
195
196 @property
197- def is_initialising(self):
198- """See `IDistroSeries`."""
199- return not getUtility(
200- IInitialiseDistroSeriesJobSource).getPendingJobsForDistroseries(
201- self).is_empty()
202-
203- @property
204 def bugtargetname(self):
205 """See IBugTarget."""
206 # XXX mpt 2007-07-10 bugs 113258, 113262:
207@@ -2023,6 +2016,17 @@
208 status=status,
209 child_version_higher=child_version_higher)
210
211+ def isInitializing(self):
212+ """See `IDistroSeries`."""
213+ job_source = getUtility(IInitialiseDistroSeriesJobSource)
214+ pending_jobs = job_source.getPendingJobsForDistroseries(self)
215+ return not pending_jobs.is_empty()
216+
217+ def isInitialized(self):
218+ """See `IDistroSeries`."""
219+ published = self.main_archive.getPublishedSources(distroseries=self)
220+ return not published.is_empty()
221+
222
223 class DistroSeriesSet:
224 implements(IDistroSeriesSet)
225
226=== modified file 'lib/lp/registry/templates/distroseries-index.pt'
227--- lib/lp/registry/templates/distroseries-index.pt 2011-05-24 10:08:33 +0000
228+++ lib/lp/registry/templates/distroseries-index.pt 2011-06-09 12:00:16 +0000
229@@ -68,7 +68,8 @@
230 <tal:derivation
231 tal:condition="request/features/soyuz.derived_series_ui.enabled">
232 <div class="yui-u"
233- tal:condition="python:context.is_derived_series or context.is_initialising">
234+ tal:condition="python: context.is_derived_series or
235+ context.isInitializing()">
236 <div tal:replace="structure context/@@+portlet-derivation" />
237 </div>
238 </tal:derivation>
239
240=== modified file 'lib/lp/registry/templates/distroseries-initialize.pt'
241--- lib/lp/registry/templates/distroseries-initialize.pt 2011-06-07 19:49:53 +0000
242+++ lib/lp/registry/templates/distroseries-initialize.pt 2011-06-08 12:57:58 +0000
243@@ -50,11 +50,12 @@
244 </p>
245 </tal:disabled>
246
247- <tal:already-derived condition="view/show_already_derived_message">
248+ <tal:already-initialized condition="view/show_already_initialized_message">
249 <p class="error message">
250- This series has already been derived.
251+ This series already contains source packages and cannot be
252+ initialized again.
253 </p>
254- </tal:already-derived>
255+ </tal:already-initialized>
256
257 <tal:already-initializing
258 condition="view/show_already_initializing_message">
259
260=== modified file 'lib/lp/registry/templates/distroseries-portlet-derivation.pt'
261--- lib/lp/registry/templates/distroseries-portlet-derivation.pt 2011-05-31 11:55:38 +0000
262+++ lib/lp/registry/templates/distroseries-portlet-derivation.pt 2011-06-09 11:59:46 +0000
263@@ -5,7 +5,7 @@
264 id="series-derivation" class="portlet"
265 tal:define="overview_menu context/menu:overview">
266 <tal:is_derived condition="context/is_derived_series">
267- <tal:is_initialised condition="not: context/is_initialising">
268+ <tal:is_initialised condition="not: context/isInitializing()">
269 <tal:one_parent condition="view/has_unique_parent">
270 <h2>Derived from <tal:name replace="view/unique_parent/displayname"/></h2>
271 </tal:one_parent>
272@@ -61,7 +61,7 @@
273 </tal:diffs>
274 </tal:is_initialised>
275 </tal:is_derived>
276- <tal:is_initialising condition="context/is_initialising">
277+ <tal:is_initialising condition="context/isInitializing">
278 <h2>Series initialisation in progress</h2>
279 This series is initialising.
280 </tal:is_initialising>
281
282=== modified file 'lib/lp/registry/tests/test_distroseries.py'
283--- lib/lp/registry/tests/test_distroseries.py 2011-05-31 15:40:10 +0000
284+++ lib/lp/registry/tests/test_distroseries.py 2011-06-09 11:57:05 +0000
285@@ -222,22 +222,31 @@
286 self.assertEquals(registrant, distroseries.registrant)
287 self.assertNotEqual(distroseries.registrant, distroseries.owner)
288
289- def test_is_initialising(self):
290- # The series is_initialising only if there is an initialisation
291- # job with a pending status attached to this series.
292+ def test_isInitializing(self):
293+ # The series method isInitializing() returns True only if there is an
294+ # initialisation job with a pending status attached to this series.
295 distroseries = self.factory.makeDistroSeries()
296 parent_distroseries = self.factory.makeDistroSeries()
297- self.assertEquals(False, distroseries.is_initialising)
298+ self.assertFalse(distroseries.isInitializing())
299 job_source = getUtility(IInitialiseDistroSeriesJobSource)
300 job = job_source.create(distroseries, [parent_distroseries.id])
301- self.assertEquals(True, distroseries.is_initialising)
302+ self.assertTrue(distroseries.isInitializing())
303 job.start()
304- self.assertEquals(True, distroseries.is_initialising)
305+ self.assertTrue(distroseries.isInitializing())
306 job.queue()
307- self.assertEquals(True, distroseries.is_initialising)
308+ self.assertTrue(distroseries.isInitializing())
309 job.start()
310 job.complete()
311- self.assertEquals(False, distroseries.is_initialising)
312+ self.assertFalse(distroseries.isInitializing())
313+
314+ def test_isInitialized(self):
315+ # The series method isInitialized() returns True once the series has
316+ # been initialized.
317+ distroseries = self.factory.makeDistroSeries()
318+ self.assertFalse(distroseries.isInitialized())
319+ self.factory.makeSourcePackagePublishingHistory(
320+ distroseries=distroseries, archive=distroseries.main_archive)
321+ self.assertTrue(distroseries.isInitialized())
322
323
324 class TestDistroSeriesPackaging(TestCaseWithFactory):
325
326=== modified file 'lib/lp/testing/__init__.py'
327--- lib/lp/testing/__init__.py 2011-05-19 15:15:16 +0000
328+++ lib/lp/testing/__init__.py 2011-06-08 13:19:10 +0000
329@@ -163,7 +163,6 @@
330 )
331 from lp.testing.fixture import ZopeEventHandlerFixture
332 from lp.testing.karma import KarmaRecorder
333-from lp.testing.matchers import Provides
334 from lp.testing.windmill import (
335 constants,
336 lpuser,
337@@ -390,6 +389,7 @@
338
339 def assertProvides(self, obj, interface):
340 """Assert 'obj' correctly provides 'interface'."""
341+ from lp.testing.matchers import Provides
342 self.assertThat(obj, Provides(interface))
343
344 def assertClassImplements(self, cls, interface):
345
346=== modified file 'lib/lp/testing/matchers.py'
347--- lib/lp/testing/matchers.py 2011-03-02 23:54:25 +0000
348+++ lib/lp/testing/matchers.py 2011-06-08 13:33:03 +0000
349@@ -8,6 +8,7 @@
350 'DocTestMatches',
351 'DoesNotCorrectlyProvide',
352 'DoesNotProvide',
353+ 'EqualsIgnoringWhitespace',
354 'HasQueryCount',
355 'IsNotProxied',
356 'IsProxied',
357@@ -20,17 +21,17 @@
358 ]
359
360 from lazr.lifecycle.snapshot import Snapshot
361+from testtools import matchers
362 from testtools.content import Content
363 from testtools.content_type import UTF8_TEXT
364 from testtools.matchers import (
365+ DocTestMatches as OriginalDocTestMatches,
366 Equals,
367- DocTestMatches as OriginalDocTestMatches,
368 LessThan,
369 Matcher,
370 Mismatch,
371 MismatchesAll,
372 )
373-from testtools import matchers
374 from zope.interface.exceptions import (
375 BrokenImplementation,
376 BrokenMethodImplementation,
377@@ -44,6 +45,7 @@
378
379 from canonical.launchpad.webapp import canonical_url
380 from canonical.launchpad.webapp.batching import BatchNavigator
381+from lp.testing import normalize_whitespace
382 from lp.testing._login import person_logged_in
383 from lp.testing._webservice import QueryCollector
384
385@@ -290,7 +292,8 @@
386
387 :param singular: The singular header the batch should be using.
388 :param plural: The plural header the batch should be using.
389- :param batch_size: The batch size that should be configured by default.
390+ :param batch_size: The batch size that should be configured by
391+ default.
392 """
393 self._single = Equals(singular)
394 self._plural = Equals(plural)
395@@ -438,3 +441,21 @@
396 text = widget.findAll(attrs={'class': 'yui3-activator-data-box'})[0]
397 text_matcher = DocTestMatches(extract_text(text))
398 return text_matcher.match(matchee)
399+
400+
401+class EqualsIgnoringWhitespace(Equals):
402+ """Compare equality, ignoring whitespace in strings.
403+
404+ Whitespace in strings is normalized before comparison. All other objected
405+ are compared as they come.
406+ """
407+
408+ def __init__(self, expected):
409+ if isinstance(expected, (str, unicode)):
410+ expected = normalize_whitespace(expected)
411+ super(EqualsIgnoringWhitespace, self).__init__(expected)
412+
413+ def match(self, observed):
414+ if isinstance(observed, (str, unicode)):
415+ observed = normalize_whitespace(observed)
416+ return super(EqualsIgnoringWhitespace, self).match(observed)
417
418=== modified file 'lib/lp/testing/tests/test_matchers.py'
419--- lib/lp/testing/tests/test_matchers.py 2011-02-25 07:15:06 +0000
420+++ lib/lp/testing/tests/test_matchers.py 2011-06-08 13:31:16 +0000
421@@ -29,6 +29,7 @@
422 DoesNotContain,
423 DoesNotCorrectlyProvide,
424 DoesNotProvide,
425+ EqualsIgnoringWhitespace,
426 HasQueryCount,
427 IsNotProxied,
428 IsProxied,
429@@ -274,3 +275,36 @@
430 matcher = Contains("bar")
431 mismatch = matcher.match("foo")
432 self.assertEqual("bar", mismatch.expected)
433+
434+
435+class EqualsIgnoringWhitespaceTests(TestCase):
436+
437+ def test_str(self):
438+ matcher = EqualsIgnoringWhitespace("abc")
439+ self.assertEqual("EqualsIgnoringWhitespace('abc')", str(matcher))
440+
441+ def test_match_str(self):
442+ matcher = EqualsIgnoringWhitespace("one \t two \n three")
443+ self.assertIs(None, matcher.match(" one \r two three "))
444+
445+ def test_mismatch_str(self):
446+ matcher = EqualsIgnoringWhitespace("one \t two \n three")
447+ mismatch = matcher.match(" one \r three ")
448+ self.assertEqual(
449+ "'one two three' != 'one three'",
450+ mismatch.describe())
451+
452+ def test_match_unicode(self):
453+ matcher = EqualsIgnoringWhitespace(u"one \t two \n \u1234 ")
454+ self.assertIs(None, matcher.match(u" one \r two \u1234 "))
455+
456+ def test_mismatch_unicode(self):
457+ matcher = EqualsIgnoringWhitespace(u"one \t two \n \u1234 ")
458+ mismatch = matcher.match(u" one \r \u1234 ")
459+ self.assertEqual(
460+ u"u'one two \\u1234' != u'one \\u1234'",
461+ mismatch.describe())
462+
463+ def test_match_non_string(self):
464+ matcher = EqualsIgnoringWhitespace(1234)
465+ self.assertIs(None, matcher.match(1234))
Revision history for this message
Jonathan Lange (jml) wrote :

`EqualsIgnoringWhitespace` could be written as:

  EqualsIgnoringWhitespace = lambda s: AfterPreprocessing(normalize_whitespace, Equals(s))

This wouldn't have the (str, unicode) type guard, but I'm not really sure that adds much anyway.

Revision history for this message
Gavin Panella (allenap) wrote :

I hadn't seen AfterPreprocessing before, neat :)

However, AfterPreprocessing only processes the matchee, not the
matcher, so that lambda doesn't ignore whitespace in the same way as
EqualsIgnoringWhitespace does. In that respect I think
EqualsIgnoringWhitespace is less surprising.

Having said that, EqualsIgnoringWhitespace doesn't actually *ignore*
whitespace, it just treats any run of whitespace as equal in matcher
and matchee. It's misnamed, but I don't think people are going to be
surprised by its behaviour anyway.

The reason for the (str, unicode) guard is so that the assertion
doesn't fail with an AttributeError (i.e. '...' object has no
attribute 'split') when the matcher or, more significantly, the
matchee are None or some other unexpected result.

Revision history for this message
Jonathan Lange (jml) wrote :

On Thu, Jun 9, 2011 at 3:43 PM, Gavin Panella
<email address hidden> wrote:
> I hadn't seen AfterPreprocessing before, neat :)
>
> However, AfterPreprocessing only processes the matchee, not the
> matcher, so that lambda doesn't ignore whitespace in the same way as
> EqualsIgnoringWhitespace does. In that respect I think
> EqualsIgnoringWhitespace is less surprising.
>

Ahh, OK. I guess testtools could use a combinator like
AfterPreprocessing that maps both the matcher and the matchee.
Shouldn't have to write a class to do this.

jml

Revision history for this message
Julian Edwards (julian-edwards) wrote :

Lovely!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/distroseries.py'
2--- lib/lp/registry/browser/distroseries.py 2011-06-09 10:34:37 +0000
3+++ lib/lp/registry/browser/distroseries.py 2011-06-10 15:18:35 +0000
4@@ -647,6 +647,29 @@
5 return getFeatureFlag("soyuz.derived_series_ui.enabled") is not None
6
7 @property
8+ def show_derivation_not_yet_available(self):
9+ return not self.is_derived_series_feature_enabled
10+
11+ @property
12+ def show_derivation_form(self):
13+ return (
14+ self.is_derived_series_feature_enabled and
15+ not self.context.isInitializing() and
16+ not self.context.isInitialized())
17+
18+ @property
19+ def show_already_initialized_message(self):
20+ return (
21+ self.is_derived_series_feature_enabled and
22+ self.context.isInitialized())
23+
24+ @property
25+ def show_already_initializing_message(self):
26+ return (
27+ self.is_derived_series_feature_enabled and
28+ self.context.isInitializing())
29+
30+ @property
31 def rebuilding_allowed(self):
32 """If the distribution has got any initialized series already,
33 rebuilding is not allowed.
34
35=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
36--- lib/lp/registry/browser/tests/test_distroseries.py 2011-06-09 10:34:37 +0000
37+++ lib/lp/registry/browser/tests/test_distroseries.py 2011-06-10 15:18:35 +0000
38@@ -40,11 +40,11 @@
39 LaunchpadFunctionalLayer,
40 LaunchpadZopelessLayer,
41 )
42+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
43 from lp.archivepublisher.debversion import Version
44-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
45 from lp.registry.browser.distroseries import (
46+ HIGHER_VERSION_THAN_PARENT,
47 IGNORED,
48- HIGHER_VERSION_THAN_PARENT,
49 NON_IGNORED,
50 RESOLVED,
51 )
52@@ -73,22 +73,23 @@
53 from lp.soyuz.interfaces.sourcepackageformat import (
54 ISourcePackageFormatSelectionSet,
55 )
56+from lp.soyuz.model import distroseriesdifferencejob
57 from lp.soyuz.model.archivepermission import ArchivePermission
58 from lp.soyuz.model.packagecopyjob import PlainPackageCopyJob
59-from lp.soyuz.model import distroseriesdifferencejob
60 from lp.testing import (
61 anonymous_logged_in,
62 celebrity_logged_in,
63- feature_flags,
64 login_person,
65 person_logged_in,
66- set_feature_flag,
67 StormStatementRecorder,
68 TestCaseWithFactory,
69 with_celebrity_logged_in,
70 )
71 from lp.testing.fakemethod import FakeMethod
72-from lp.testing.matchers import HasQueryCount
73+from lp.testing.matchers import (
74+ EqualsIgnoringWhitespace,
75+ HasQueryCount,
76+ )
77 from lp.testing.views import create_initialized_view
78
79
80@@ -324,7 +325,7 @@
81 view.request.features = get_relevant_feature_controller()
82 html_content = view()
83
84- self.assertTrue(derived_series.is_initialising)
85+ self.assertTrue(derived_series.isInitializing())
86 self.assertThat(html_content, portlet_display)
87
88
89@@ -417,17 +418,17 @@
90 # the soyuz.derived_series_ui.enabled flag.
91 distroseries = self.factory.makeDistroSeries()
92 view = create_initialized_view(distroseries, "+initseries")
93- with feature_flags():
94+ with FeatureFixture({}):
95 self.assertFalse(view.is_derived_series_feature_enabled)
96- with feature_flags():
97- set_feature_flag(u"soyuz.derived_series_ui.enabled", u"true")
98+ flags = {u"soyuz.derived_series_ui.enabled": u"true"}
99+ with FeatureFixture(flags):
100 self.assertTrue(view.is_derived_series_feature_enabled)
101
102 def test_form_hidden_when_derived_series_feature_disabled(self):
103 # The form is hidden when the feature flag is not set.
104 distroseries = self.factory.makeDistroSeries()
105 view = create_initialized_view(distroseries, "+initseries")
106- with feature_flags():
107+ with FeatureFixture({}):
108 root = html.fromstring(view())
109 self.assertEqual(
110 [], root.cssselect("#initseries-form-container"))
111@@ -441,8 +442,8 @@
112 # The form is shown when the feature flag is set.
113 distroseries = self.factory.makeDistroSeries()
114 view = create_initialized_view(distroseries, "+initseries")
115- with feature_flags():
116- set_feature_flag(u"soyuz.derived_series_ui.enabled", u"true")
117+ flags = {u"soyuz.derived_series_ui.enabled": u"true"}
118+ with FeatureFixture(flags):
119 root = html.fromstring(view())
120 self.assertNotEqual(
121 [], root.cssselect("#initseries-form-container"))
122@@ -462,7 +463,6 @@
123 self.factory.makeDistroSeries(
124 distribution=distroseries.distribution)
125 view = create_initialized_view(distroseries, "+initseries")
126-
127 self.assertTrue(view.rebuilding_allowed)
128
129 def test_rebuilding_not_allowed(self):
130@@ -473,9 +473,45 @@
131 self.factory.makeSourcePackagePublishingHistory(
132 distroseries=another_distroseries)
133 view = create_initialized_view(distroseries, "+initseries")
134-
135 self.assertFalse(view.rebuilding_allowed)
136
137+ def test_form_hidden_when_distroseries_is_initialized(self):
138+ # The form is hidden when the feature flag is set but the series has
139+ # already been initialized.
140+ distroseries = self.factory.makeDistroSeries()
141+ self.factory.makeSourcePackagePublishingHistory(
142+ distroseries=distroseries, archive=distroseries.main_archive)
143+ view = create_initialized_view(distroseries, "+initseries")
144+ flags = {u"soyuz.derived_series_ui.enabled": u"true"}
145+ with FeatureFixture(flags):
146+ root = html.fromstring(view())
147+ self.assertEqual(
148+ [], root.cssselect("#initseries-form-container"))
149+ # Instead an explanatory message is shown.
150+ [message] = root.cssselect("p.error.message")
151+ self.assertThat(
152+ message.text, EqualsIgnoringWhitespace(
153+ u"This series already contains source packages "
154+ u"and cannot be initialized again."))
155+
156+ def test_form_hidden_when_distroseries_is_being_initialized(self):
157+ # The form is hidden when the feature flag is set but the series has
158+ # already been derived.
159+ distroseries = self.factory.makeDistroSeries()
160+ getUtility(IInitialiseDistroSeriesJobSource).create(
161+ distroseries, [self.factory.makeDistroSeries().id])
162+ view = create_initialized_view(distroseries, "+initseries")
163+ flags = {u"soyuz.derived_series_ui.enabled": u"true"}
164+ with FeatureFixture(flags):
165+ root = html.fromstring(view())
166+ self.assertEqual(
167+ [], root.cssselect("#initseries-form-container"))
168+ # Instead an explanatory message is shown.
169+ [message] = root.cssselect("p.error.message")
170+ self.assertThat(
171+ message.text, EqualsIgnoringWhitespace(
172+ u"This series is already being initialized."))
173+
174
175 class DistroSeriesDifferenceMixin:
176 """A helper class for testing differences pages"""
177@@ -1701,7 +1737,7 @@
178 person, sp_name)
179 self._syncAndGetView(
180 derived_series, person, [diff_id])
181- parent_pub = parent_series.main_archive.getPublishedSources(
182+ parent_series.main_archive.getPublishedSources(
183 name='my-src-name', version=versions['parent'],
184 distroseries=parent_series).one()
185
186
187=== modified file 'lib/lp/registry/interfaces/distroseries.py'
188--- lib/lp/registry/interfaces/distroseries.py 2011-06-08 11:06:04 +0000
189+++ lib/lp/registry/interfaces/distroseries.py 2011-06-10 15:18:35 +0000
190@@ -213,7 +213,7 @@
191 description=_("The version string for this series.")))
192 distribution = exported(
193 Reference(
194- Interface, # Really IDistribution, see circular import fix below.
195+ Interface, # Really IDistribution, see circular import fix below.
196 title=_("Distribution"), required=True,
197 description=_("The distribution for which this is a series.")))
198 distributionID = Attribute('The distribution ID.')
199@@ -239,16 +239,13 @@
200 is_derived_series = Bool(
201 title=u'Is this series a derived series?', readonly=True,
202 description=(u"Whether or not this series is a derived series."))
203- is_initialising = Bool(
204- title=u'Is this series initialising?', readonly=True,
205- description=(u"Whether or not this series is initialising."))
206 datereleased = exported(
207 Datetime(title=_("Date released")))
208 previous_series = exported(
209 ReferenceChoice(
210 title=_("Parent series"),
211 description=_("The series from which this one was branched."),
212- required=True, schema=Interface, # Really IDistroSeries, see below
213+ required=True, schema=Interface, # Really IDistroSeries
214 vocabulary='DistroSeries'),
215 ("devel", dict(exported_as="previous_series")),
216 ("1.0", dict(exported_as="parent_series")),
217@@ -370,7 +367,7 @@
218
219 main_archive = exported(
220 Reference(
221- Interface, # Really IArchive, see below for circular import fix.
222+ Interface, # Really IArchive, see below for circular import fix.
223 title=_('Distribution Main Archive')))
224
225 supported = exported(
226@@ -418,7 +415,7 @@
227 architectures = exported(
228 CollectionField(
229 title=_("All architectures in this series."),
230- value_type=Reference(schema=Interface), # IDistroArchSeries.
231+ value_type=Reference(schema=Interface), # IDistroArchSeries.
232 readonly=True))
233
234 enabled_architectures = Attribute(
235@@ -848,18 +845,18 @@
236
237 @operation_parameters(
238 parent_series=Reference(
239- schema=Interface, # IDistroSeries
240+ schema=Interface, # IDistroSeries
241 title=_("The parent series to consider."),
242 required=False),
243 difference_type=Choice(
244- vocabulary=DBEnumeratedType, # DistroSeriesDifferenceType
245+ vocabulary=DBEnumeratedType, # DistroSeriesDifferenceType
246 title=_("Only return differences of this type."), required=False),
247 source_package_name_filter=TextLine(
248 title=_("Only return differences for packages matching this "
249 "name."),
250 required=False),
251 status=Choice(
252- vocabulary=DBEnumeratedType, # DistroSeriesDifferenceStatus
253+ vocabulary=DBEnumeratedType, # DistroSeriesDifferenceStatus
254 title=_("Only return differences of this status."),
255 required=False),
256 child_version_higher=Bool(
257@@ -886,6 +883,12 @@
258 child's version is higher than the parent's version.
259 """
260
261+ def isInitializing():
262+ """Is this series initializing?"""
263+
264+ def isInitialized():
265+ """Has this series been initialized?"""
266+
267
268 class IDistroSeriesEditRestricted(Interface):
269 """IDistroSeries properties which require launchpad.Edit."""
270@@ -1032,7 +1035,7 @@
271
272 class DerivationError(Exception):
273 """Raised when there is a problem deriving a distroseries."""
274- webservice_error(400) # Bad Request
275+ webservice_error(400) # Bad Request
276 _message_prefix = "Error deriving distro series"
277
278
279
280=== modified file 'lib/lp/registry/model/distroseries.py'
281--- lib/lp/registry/model/distroseries.py 2011-06-08 15:27:40 +0000
282+++ lib/lp/registry/model/distroseries.py 2011-06-10 15:18:35 +0000
283@@ -802,13 +802,6 @@
284 return not self.getParentSeries() == []
285
286 @property
287- def is_initialising(self):
288- """See `IDistroSeries`."""
289- return not getUtility(
290- IInitialiseDistroSeriesJobSource).getPendingJobsForDistroseries(
291- self).is_empty()
292-
293- @property
294 def bugtargetname(self):
295 """See IBugTarget."""
296 # XXX mpt 2007-07-10 bugs 113258, 113262:
297@@ -2037,6 +2030,17 @@
298 status=status,
299 child_version_higher=child_version_higher)
300
301+ def isInitializing(self):
302+ """See `IDistroSeries`."""
303+ job_source = getUtility(IInitialiseDistroSeriesJobSource)
304+ pending_jobs = job_source.getPendingJobsForDistroseries(self)
305+ return not pending_jobs.is_empty()
306+
307+ def isInitialized(self):
308+ """See `IDistroSeries`."""
309+ published = self.main_archive.getPublishedSources(distroseries=self)
310+ return not published.is_empty()
311+
312
313 class DistroSeriesSet:
314 implements(IDistroSeriesSet)
315
316=== modified file 'lib/lp/registry/templates/distroseries-index.pt'
317--- lib/lp/registry/templates/distroseries-index.pt 2011-05-24 10:08:33 +0000
318+++ lib/lp/registry/templates/distroseries-index.pt 2011-06-10 15:18:35 +0000
319@@ -68,7 +68,8 @@
320 <tal:derivation
321 tal:condition="request/features/soyuz.derived_series_ui.enabled">
322 <div class="yui-u"
323- tal:condition="python:context.is_derived_series or context.is_initialising">
324+ tal:condition="python: context.is_derived_series or
325+ context.isInitializing()">
326 <div tal:replace="structure context/@@+portlet-derivation" />
327 </div>
328 </tal:derivation>
329
330=== modified file 'lib/lp/registry/templates/distroseries-initialize.pt'
331--- lib/lp/registry/templates/distroseries-initialize.pt 2011-06-06 09:32:14 +0000
332+++ lib/lp/registry/templates/distroseries-initialize.pt 2011-06-10 15:18:35 +0000
333@@ -12,13 +12,13 @@
334 </metal:head-epilogue>
335 <body>
336 <div metal:fill-slot="main">
337- <tal:enabled condition="view/is_derived_series_feature_enabled">
338+
339+ <tal:enabled condition="view/show_derivation_form">
340 <div class="top-portlet">
341 This page allows you to initialize a distribution series.
342 <a href="/+help/init-series-title-help.html"
343 target="help" class="sprite maybe">&nbsp;
344 <span class="invisible-link">Initialization help</span></a>
345-
346 </div>
347 <p class="error message javascript-disabled">
348 Javascript is required to use this page. Please enable
349@@ -42,7 +42,8 @@
350 many thousands of packages is likely to take hours to complete.
351 </p>
352 </tal:enabled>
353- <tal:disabled condition="not:view/is_derived_series_feature_enabled">
354+
355+ <tal:disabled condition="view/show_derivation_not_yet_available">
356 <p class="error message">
357 The Derivative Distributions feature is under development
358 and is not yet generally available. You can read more about
359@@ -51,6 +52,21 @@
360 page</a>.
361 </p>
362 </tal:disabled>
363+
364+ <tal:already-initialized condition="view/show_already_initialized_message">
365+ <p class="error message">
366+ This series already contains source packages and cannot be
367+ initialized again.
368+ </p>
369+ </tal:already-initialized>
370+
371+ <tal:already-initializing
372+ condition="view/show_already_initializing_message">
373+ <p class="error message">
374+ This series is already being initialized.
375+ </p>
376+ </tal:already-initializing>
377+
378 </div>
379 </body>
380 </html>
381
382=== modified file 'lib/lp/registry/templates/distroseries-portlet-derivation.pt'
383--- lib/lp/registry/templates/distroseries-portlet-derivation.pt 2011-05-31 11:55:38 +0000
384+++ lib/lp/registry/templates/distroseries-portlet-derivation.pt 2011-06-10 15:18:35 +0000
385@@ -5,7 +5,7 @@
386 id="series-derivation" class="portlet"
387 tal:define="overview_menu context/menu:overview">
388 <tal:is_derived condition="context/is_derived_series">
389- <tal:is_initialised condition="not: context/is_initialising">
390+ <tal:is_initialised condition="not: context/isInitializing">
391 <tal:one_parent condition="view/has_unique_parent">
392 <h2>Derived from <tal:name replace="view/unique_parent/displayname"/></h2>
393 </tal:one_parent>
394@@ -61,7 +61,7 @@
395 </tal:diffs>
396 </tal:is_initialised>
397 </tal:is_derived>
398- <tal:is_initialising condition="context/is_initialising">
399+ <tal:is_initialising condition="context/isInitializing">
400 <h2>Series initialisation in progress</h2>
401 This series is initialising.
402 </tal:is_initialising>
403
404=== modified file 'lib/lp/registry/tests/test_distroseries.py'
405--- lib/lp/registry/tests/test_distroseries.py 2011-05-31 15:40:10 +0000
406+++ lib/lp/registry/tests/test_distroseries.py 2011-06-10 15:18:35 +0000
407@@ -222,22 +222,31 @@
408 self.assertEquals(registrant, distroseries.registrant)
409 self.assertNotEqual(distroseries.registrant, distroseries.owner)
410
411- def test_is_initialising(self):
412- # The series is_initialising only if there is an initialisation
413- # job with a pending status attached to this series.
414+ def test_isInitializing(self):
415+ # The series method isInitializing() returns True only if there is an
416+ # initialisation job with a pending status attached to this series.
417 distroseries = self.factory.makeDistroSeries()
418 parent_distroseries = self.factory.makeDistroSeries()
419- self.assertEquals(False, distroseries.is_initialising)
420+ self.assertFalse(distroseries.isInitializing())
421 job_source = getUtility(IInitialiseDistroSeriesJobSource)
422 job = job_source.create(distroseries, [parent_distroseries.id])
423- self.assertEquals(True, distroseries.is_initialising)
424+ self.assertTrue(distroseries.isInitializing())
425 job.start()
426- self.assertEquals(True, distroseries.is_initialising)
427+ self.assertTrue(distroseries.isInitializing())
428 job.queue()
429- self.assertEquals(True, distroseries.is_initialising)
430+ self.assertTrue(distroseries.isInitializing())
431 job.start()
432 job.complete()
433- self.assertEquals(False, distroseries.is_initialising)
434+ self.assertFalse(distroseries.isInitializing())
435+
436+ def test_isInitialized(self):
437+ # The series method isInitialized() returns True once the series has
438+ # been initialized.
439+ distroseries = self.factory.makeDistroSeries()
440+ self.assertFalse(distroseries.isInitialized())
441+ self.factory.makeSourcePackagePublishingHistory(
442+ distroseries=distroseries, archive=distroseries.main_archive)
443+ self.assertTrue(distroseries.isInitialized())
444
445
446 class TestDistroSeriesPackaging(TestCaseWithFactory):
447
448=== modified file 'lib/lp/testing/__init__.py'
449--- lib/lp/testing/__init__.py 2011-05-19 15:15:16 +0000
450+++ lib/lp/testing/__init__.py 2011-06-10 15:18:35 +0000
451@@ -163,7 +163,6 @@
452 )
453 from lp.testing.fixture import ZopeEventHandlerFixture
454 from lp.testing.karma import KarmaRecorder
455-from lp.testing.matchers import Provides
456 from lp.testing.windmill import (
457 constants,
458 lpuser,
459@@ -390,6 +389,7 @@
460
461 def assertProvides(self, obj, interface):
462 """Assert 'obj' correctly provides 'interface'."""
463+ from lp.testing.matchers import Provides
464 self.assertThat(obj, Provides(interface))
465
466 def assertClassImplements(self, cls, interface):
467
468=== modified file 'lib/lp/testing/matchers.py'
469--- lib/lp/testing/matchers.py 2011-03-02 23:54:25 +0000
470+++ lib/lp/testing/matchers.py 2011-06-10 15:18:35 +0000
471@@ -8,6 +8,7 @@
472 'DocTestMatches',
473 'DoesNotCorrectlyProvide',
474 'DoesNotProvide',
475+ 'EqualsIgnoringWhitespace',
476 'HasQueryCount',
477 'IsNotProxied',
478 'IsProxied',
479@@ -20,17 +21,17 @@
480 ]
481
482 from lazr.lifecycle.snapshot import Snapshot
483+from testtools import matchers
484 from testtools.content import Content
485 from testtools.content_type import UTF8_TEXT
486 from testtools.matchers import (
487+ DocTestMatches as OriginalDocTestMatches,
488 Equals,
489- DocTestMatches as OriginalDocTestMatches,
490 LessThan,
491 Matcher,
492 Mismatch,
493 MismatchesAll,
494 )
495-from testtools import matchers
496 from zope.interface.exceptions import (
497 BrokenImplementation,
498 BrokenMethodImplementation,
499@@ -44,6 +45,7 @@
500
501 from canonical.launchpad.webapp import canonical_url
502 from canonical.launchpad.webapp.batching import BatchNavigator
503+from lp.testing import normalize_whitespace
504 from lp.testing._login import person_logged_in
505 from lp.testing._webservice import QueryCollector
506
507@@ -290,7 +292,8 @@
508
509 :param singular: The singular header the batch should be using.
510 :param plural: The plural header the batch should be using.
511- :param batch_size: The batch size that should be configured by default.
512+ :param batch_size: The batch size that should be configured by
513+ default.
514 """
515 self._single = Equals(singular)
516 self._plural = Equals(plural)
517@@ -438,3 +441,21 @@
518 text = widget.findAll(attrs={'class': 'yui3-activator-data-box'})[0]
519 text_matcher = DocTestMatches(extract_text(text))
520 return text_matcher.match(matchee)
521+
522+
523+class EqualsIgnoringWhitespace(Equals):
524+ """Compare equality, ignoring whitespace in strings.
525+
526+ Whitespace in strings is normalized before comparison. All other objects
527+ are compared as they come.
528+ """
529+
530+ def __init__(self, expected):
531+ if isinstance(expected, (str, unicode)):
532+ expected = normalize_whitespace(expected)
533+ super(EqualsIgnoringWhitespace, self).__init__(expected)
534+
535+ def match(self, observed):
536+ if isinstance(observed, (str, unicode)):
537+ observed = normalize_whitespace(observed)
538+ return super(EqualsIgnoringWhitespace, self).match(observed)
539
540=== modified file 'lib/lp/testing/tests/test_matchers.py'
541--- lib/lp/testing/tests/test_matchers.py 2011-02-25 07:15:06 +0000
542+++ lib/lp/testing/tests/test_matchers.py 2011-06-10 15:18:35 +0000
543@@ -29,6 +29,7 @@
544 DoesNotContain,
545 DoesNotCorrectlyProvide,
546 DoesNotProvide,
547+ EqualsIgnoringWhitespace,
548 HasQueryCount,
549 IsNotProxied,
550 IsProxied,
551@@ -274,3 +275,36 @@
552 matcher = Contains("bar")
553 mismatch = matcher.match("foo")
554 self.assertEqual("bar", mismatch.expected)
555+
556+
557+class EqualsIgnoringWhitespaceTests(TestCase):
558+
559+ def test_str(self):
560+ matcher = EqualsIgnoringWhitespace("abc")
561+ self.assertEqual("EqualsIgnoringWhitespace('abc')", str(matcher))
562+
563+ def test_match_str(self):
564+ matcher = EqualsIgnoringWhitespace("one \t two \n three")
565+ self.assertIs(None, matcher.match(" one \r two three "))
566+
567+ def test_mismatch_str(self):
568+ matcher = EqualsIgnoringWhitespace("one \t two \n three")
569+ mismatch = matcher.match(" one \r three ")
570+ self.assertEqual(
571+ "'one two three' != 'one three'",
572+ mismatch.describe())
573+
574+ def test_match_unicode(self):
575+ matcher = EqualsIgnoringWhitespace(u"one \t two \n \u1234 ")
576+ self.assertIs(None, matcher.match(u" one \r two \u1234 "))
577+
578+ def test_mismatch_unicode(self):
579+ matcher = EqualsIgnoringWhitespace(u"one \t two \n \u1234 ")
580+ mismatch = matcher.match(u" one \r \u1234 ")
581+ self.assertEqual(
582+ u"u'one two \\u1234' != u'one \\u1234'",
583+ mismatch.describe())
584+
585+ def test_match_non_string(self):
586+ matcher = EqualsIgnoringWhitespace(1234)
587+ self.assertIs(None, matcher.match(1234))