Launchpad itself

Merge lp:~michael.nelson/launchpad/638090-base_version-property-for-differences into lp:launchpad/db-devel

  1. 638090-base_version-property-for-differences
  2. Merge into db-devel
Proposed by Michael Nelson
Status: Merged
Approved by: Stuart Bishop
Approved revision: no longer in the source branch.
Merged at revision: 9936
Proposed branch: lp:~michael.nelson/launchpad/638090-base_version-property-for-differences
Merge into: lp:launchpad/db-devel
Diff against target: 287 lines (+127/-40)
6 files modified
database/schema/comments.sql (+1/-0)
database/schema/patch-2208-24-0.sql (+7/-0)
lib/lp/registry/interfaces/distroseriesdifference.py (+7/-7)
lib/lp/registry/model/distroseriesdifference.py (+42/-11)
lib/lp/registry/tests/test_distroseriesdifference.py (+66/-3)
lib/lp/testing/factory.py (+4/-19)
To merge this branch: bzr merge lp:~michael.nelson/launchpad/638090-base_version-property-for-differences
Reviewer Review Type Date Requested Status
Stuart Bishop (community) db Approve
Edwin Grubbs (community) code Approve
Robert Collins db Pending
Review via email: mp+37742@code.launchpad.net

Commit message

Adds DistroSeriesDifference.base_version

Description of the change

Overview
========

This branch adds the base_version column to the schema for DistroSeriesDifferences, and ensures that it is calculated when the difference is created.

Details
=======
It also updates the versions created by factory.makeSourcePackageRelease() to be valid debian versions.

I've also simplified the factory for new distroseriesdifferences so that only the derived series and source package name are passed in - the rest (current versions in derived/parent series etc., status and type, are all evaluated for consistency).

Finally, removed a second factory.makePackageDiff() which crept in (independent devel/db-devel additions).

To test:
bin/test -vvm test_distroseriesdifference

To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Michael,

This branch looks good. I have included comments on the test changes that we already discussed on IRC.

-Edwin

>=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
>--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-09-22 11:03:20 +0000
>+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-10-06 14:44:02 +0000
>@@ -424,6 +422,38 @@
> self.assertContentEqual(
> ['source_pub', 'parent_source_pub'], cache)
>
>+ def test_base_version_none(self):
>+ # The attribute is set to None if there is no common base version.
>+ ds_diff = self.factory.makeDistroSeriesDifference()
>+
>+ self.assertIs(None, ds_diff.base_version)
>+
>+ def test_base_version_common(self):
>+ # The common base version is set when the difference is created.
>+ # Publish v1.0 of foo in both series.
>+ derived_series = self.factory.makeDistroSeries(
>+ parent_series=self.factory.makeDistroSeries())
>+ source_package_name = self.factory.getOrMakeSourcePackageName('foo')

The test should add multiple common versions to verify that it chooses
the right one (the latest one).

>+ self.factory.makeSourcePackagePublishingHistory(
>+ distroseries=derived_series,
>+ version='1.0',
>+ sourcepackagename=source_package_name,
>+ status = PackagePublishingStatus.PUBLISHED)

Remove spaces around "=".

>+ self.factory.makeSourcePackagePublishingHistory(
>+ distroseries=derived_series.parent_series,
>+ version='1.0',
>+ sourcepackagename=source_package_name,
>+ status = PackagePublishingStatus.PUBLISHED)

Same here.

>+ ds_diff = self.factory.makeDistroSeriesDifference(
>+ derived_series=derived_series, source_package_name_str='foo',
>+ versions={
>+ 'derived': '1.2',
>+ 'parent': '1.3',
>+ })
>+
>+ self.assertEqual('1.0', ds_diff.base_version)
>+
>
> class DistroSeriesDifferenceSourceTestCase(TestCaseWithFactory):

review: Approve (code)
Revision history for this message
Stuart Bishop (stub) wrote :

Fine. patch-2208-24-0.sql

review: Approve (db)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/comments.sql'
2--- database/schema/comments.sql 2010-10-07 18:54:00 +0000
3+++ database/schema/comments.sql 2010-10-08 10:37:44 +0000
4@@ -554,6 +554,7 @@
5 COMMENT ON COLUMN DistroSeriesDifference.difference_type IS 'The type of difference that this record represents - a package unique to the derived series, or missing, or in both.';
6 COMMENT ON COLUMN DistroSeriesDifference.source_version IS 'The version of the package in the derived series.';
7 COMMENT ON COLUMN DistroSeriesDifference.parent_source_version IS 'The version of the package in the parent series.';
8+COMMENT ON COLUMN DistroSeriesDifference.base_version IS 'The common base version of the package for the derived and parent series.';
9
10 -- DistroSeriesDifferenceMessage
11 COMMENT ON TABLE DistroSeriesDifferenceMessage IS 'A message/comment on a distro series difference.';
12
13=== added file 'database/schema/patch-2208-24-0.sql'
14--- database/schema/patch-2208-24-0.sql 1970-01-01 00:00:00 +0000
15+++ database/schema/patch-2208-24-0.sql 2010-10-08 10:37:44 +0000
16@@ -0,0 +1,7 @@
17+SET client_min_messages=ERROR;
18+
19+-- Add column to store the base_version for derived/parent versions.
20+ALTER TABLE DistroSeriesDifference ADD COLUMN base_version text;
21+ALTER TABLE DistroSeriesDifference ADD CONSTRAINT valid_base_version CHECK(valid_debian_version(base_version));
22+
23+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 24, 0);
24
25=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
26--- lib/lp/registry/interfaces/distroseriesdifference.py 2010-09-29 09:28:17 +0000
27+++ lib/lp/registry/interfaces/distroseriesdifference.py 2010-10-08 10:37:44 +0000
28@@ -110,6 +110,12 @@
29 "The version of the most recent source publishing in the "
30 "parent series."))
31
32+ base_version = TextLine(
33+ title=_("Base version"), readonly=True,
34+ description=_(
35+ "The common base version of the package for differences "
36+ "with different versions in the parent and derived series."))
37+
38 owner = Reference(
39 IPerson, title=_("Owning team of the derived series"), readonly=True,
40 description=_(
41@@ -171,8 +177,7 @@
42 class IDistroSeriesDifferenceSource(Interface):
43 """A utility of this interface can be used to create differences."""
44
45- def new(derived_series, source_package_name, difference_type,
46- status=DistroSeriesDifferenceStatus.NEEDS_ATTENTION):
47+ def new(derived_series, source_package_name):
48 """Create an `IDistroSeriesDifference`.
49
50 :param derived_series: The distribution series which was derived
51@@ -182,11 +187,6 @@
52 :param source_package_name: A source package name identifying the
53 package with a difference.
54 :type source_package_name: `ISourcePackageName`.
55- :param difference_type: Indicates the type of difference represented
56- by this record.
57- :type difference_type: `DistroSeriesDifferenceType`.
58- :param status: The current status of this difference.
59- :type status: `DistorSeriesDifferenceStatus`.
60 :raises NotADerivedSeriesError: When the passed distro series
61 is not a derived series.
62 :return: A new `DistroSeriesDifference` object.
63
64=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
65--- lib/lp/registry/model/distroseriesdifference.py 2010-09-28 14:42:41 +0000
66+++ lib/lp/registry/model/distroseriesdifference.py 2010-10-08 10:37:44 +0000
67@@ -28,6 +28,7 @@
68 IMasterStore,
69 IStore,
70 )
71+from lp.archivepublisher.debversion import Version
72 from lp.registry.enum import (
73 DistroSeriesDifferenceStatus,
74 DistroSeriesDifferenceType,
75@@ -83,10 +84,10 @@
76 source_version = Unicode(name='source_version', allow_none=True)
77 parent_source_version = Unicode(name='parent_source_version',
78 allow_none=True)
79+ base_version = Unicode(name='base_version', allow_none=True)
80
81 @staticmethod
82- def new(derived_series, source_package_name, difference_type,
83- status=DistroSeriesDifferenceStatus.NEEDS_ATTENTION):
84+ def new(derived_series, source_package_name):
85 """See `IDistroSeriesDifferenceSource`."""
86 if derived_series.parent_series is None:
87 raise NotADerivedSeriesError()
88@@ -95,16 +96,12 @@
89 diff = DistroSeriesDifference()
90 diff.derived_series = derived_series
91 diff.source_package_name = source_package_name
92- diff.status = status
93- diff.difference_type = difference_type
94
95- source_pub = diff.source_pub
96- if source_pub is not None:
97- diff.source_version = source_pub.source_package_version
98- parent_source_pub = diff.parent_source_pub
99- if parent_source_pub is not None:
100- diff.parent_source_version = (
101- parent_source_pub.source_package_version)
102+ # The status and type is set to default values - they will be
103+ # updated appropriately during the update() call.
104+ diff.status = DistroSeriesDifferenceStatus.NEEDS_ATTENTION
105+ diff.difference_type = DistroSeriesDifferenceType.DIFFERENT_VERSIONS
106+ diff.update()
107
108 return store.add(diff)
109
110@@ -251,8 +248,42 @@
111 updated = True
112 self.status = DistroSeriesDifferenceStatus.RESOLVED
113
114+ if self._updateBaseVersion():
115+ updated = True
116+
117 return updated
118
119+ def _updateBaseVersion(self):
120+ """Check for the most-recently published common version.
121+
122+ Return whether the record was updated or not.
123+ """
124+ if self.difference_type != (
125+ DistroSeriesDifferenceType.DIFFERENT_VERSIONS):
126+ return False
127+
128+ derived_pubs = self.derived_series.getPublishedSources(
129+ self.source_package_name)
130+ derived_versions = [
131+ Version(pub.sourcepackagerelease.version) for pub in derived_pubs]
132+ parent_pubs = self.derived_series.parent_series.getPublishedSources(
133+ self.source_package_name)
134+ parent_versions = [
135+ Version(pub.sourcepackagerelease.version) for pub in parent_pubs]
136+
137+ common_versions = list(
138+ set(derived_versions).intersection(parent_versions))
139+ if common_versions:
140+ common_versions.sort()
141+ self.base_version = unicode(common_versions.pop())
142+ return True
143+
144+ if self.base_version is None:
145+ return False
146+ else:
147+ self.base_version = None
148+ return True
149+
150 def addComment(self, commenter, comment):
151 """See `IDistroSeriesDifference`."""
152 return getUtility(IDistroSeriesDifferenceCommentSource).new(
153
154=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
155--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-10-06 18:53:53 +0000
156+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-10-08 10:37:44 +0000
157@@ -62,9 +62,7 @@
158
159 self.assertRaises(
160 NotADerivedSeriesError, distroseriesdifference_factory.new,
161- distro_series, source_package_name,
162- DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES,
163- )
164+ distro_series, source_package_name)
165
166 def test_source_pub(self):
167 # The related source pub is returned for the derived series.
168@@ -424,6 +422,71 @@
169 self.assertContentEqual(
170 ['source_pub', 'parent_source_pub'], cache)
171
172+ def test_base_version_none(self):
173+ # The attribute is set to None if there is no common base version.
174+ # Publish different versions in the series.
175+ derived_series = self.factory.makeDistroSeries(
176+ parent_series=self.factory.makeDistroSeries())
177+ source_package_name = self.factory.getOrMakeSourcePackageName('foo')
178+ self.factory.makeSourcePackagePublishingHistory(
179+ distroseries=derived_series,
180+ version='1.0deri1',
181+ sourcepackagename=source_package_name,
182+ status=PackagePublishingStatus.PUBLISHED)
183+ self.factory.makeSourcePackagePublishingHistory(
184+ distroseries=derived_series.parent_series,
185+ version='1.0ubu2',
186+ sourcepackagename=source_package_name,
187+ status=PackagePublishingStatus.PUBLISHED)
188+ ds_diff = self.factory.makeDistroSeriesDifference()
189+
190+ self.assertIs(None, ds_diff.base_version)
191+
192+ def test_base_version_common(self):
193+ # The common base version is set when the difference is created.
194+ # Publish v1.0 of foo in both series.
195+ derived_series = self.factory.makeDistroSeries(
196+ parent_series=self.factory.makeDistroSeries())
197+ source_package_name = self.factory.getOrMakeSourcePackageName('foo')
198+ for series in [derived_series, derived_series.parent_series]:
199+ self.factory.makeSourcePackagePublishingHistory(
200+ distroseries=series,
201+ version='1.0',
202+ sourcepackagename=source_package_name,
203+ status=PackagePublishingStatus.PUBLISHED)
204+
205+ ds_diff = self.factory.makeDistroSeriesDifference(
206+ derived_series=derived_series, source_package_name_str='foo',
207+ versions={
208+ 'derived': '1.2',
209+ 'parent': '1.3',
210+ })
211+
212+ self.assertEqual('1.0', ds_diff.base_version)
213+
214+ def test_base_version_multiple(self):
215+ # The latest common base version is set as the base-version.
216+ # Publish v1.0 and 1.1 of foo in both series.
217+ derived_series = self.factory.makeDistroSeries(
218+ parent_series=self.factory.makeDistroSeries())
219+ source_package_name = self.factory.getOrMakeSourcePackageName('foo')
220+ for series in [derived_series, derived_series.parent_series]:
221+ for version in ['1.0', '1.1']:
222+ self.factory.makeSourcePackagePublishingHistory(
223+ distroseries=series,
224+ version=version,
225+ sourcepackagename=source_package_name,
226+ status=PackagePublishingStatus.PUBLISHED)
227+
228+ ds_diff = self.factory.makeDistroSeriesDifference(
229+ derived_series=derived_series, source_package_name_str='foo',
230+ versions={
231+ 'derived': '1.2',
232+ 'parent': '1.3',
233+ })
234+
235+ self.assertEqual('1.1', ds_diff.base_version)
236+
237
238 class DistroSeriesDifferenceSourceTestCase(TestCaseWithFactory):
239
240
241=== modified file 'lib/lp/testing/factory.py'
242--- lib/lp/testing/factory.py 2010-10-06 18:53:53 +0000
243+++ lib/lp/testing/factory.py 2010-10-08 10:37:44 +0000
244@@ -1843,22 +1843,6 @@
245 expires=expires, restricted=restricted)
246 return library_file_alias
247
248- def makePackageDiff(self, from_spr=None, to_spr=None):
249- """Make a completed package diff."""
250- if from_spr is None:
251- from_spr = self.makeSourcePackageRelease()
252- if to_spr is None:
253- to_spr = self.makeSourcePackageRelease()
254-
255- diff = from_spr.requestDiffTo(
256- from_spr.creator, to_spr)
257-
258- naked_diff = removeSecurityProxy(diff)
259- naked_diff.status = PackageDiffStatus.COMPLETED
260- naked_diff.diff_content = self.makeLibraryFileAlias()
261- naked_diff.date_fulfilled = UTC_NOW
262- return diff
263-
264 def makeDistribution(self, name=None, displayname=None, owner=None,
265 members=None, title=None, aliases=None):
266 """Make a new distribution."""
267@@ -1950,8 +1934,9 @@
268 status = PackagePublishingStatus.PUBLISHED)
269
270 diff = getUtility(IDistroSeriesDifferenceSource).new(
271- derived_series, source_package_name, difference_type,
272- status=status)
273+ derived_series, source_package_name)
274+
275+ removeSecurityProxy(diff).status = status
276
277 # We clear the cache on the diff, returning the object as if it
278 # was just loaded from the store.
279@@ -2622,7 +2607,7 @@
280 creator = self.makePerson()
281
282 if version is None:
283- version = self.getUniqueString('version')
284+ version = unicode(self.getUniqueInteger()) + 'version'
285
286 return distroseries.createUploadedSourcePackageRelease(
287 sourcepackagename=sourcepackagename,

Subscribers

People subscribed via source and target branches

to status/vote changes: