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
=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql 2010-10-07 18:54:00 +0000
+++ database/schema/comments.sql 2010-10-08 10:37:44 +0000
@@ -554,6 +554,7 @@
554COMMENT 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.';554COMMENT 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.';
555COMMENT ON COLUMN DistroSeriesDifference.source_version IS 'The version of the package in the derived series.';555COMMENT ON COLUMN DistroSeriesDifference.source_version IS 'The version of the package in the derived series.';
556COMMENT ON COLUMN DistroSeriesDifference.parent_source_version IS 'The version of the package in the parent series.';556COMMENT ON COLUMN DistroSeriesDifference.parent_source_version IS 'The version of the package in the parent series.';
557COMMENT ON COLUMN DistroSeriesDifference.base_version IS 'The common base version of the package for the derived and parent series.';
557558
558-- DistroSeriesDifferenceMessage559-- DistroSeriesDifferenceMessage
559COMMENT ON TABLE DistroSeriesDifferenceMessage IS 'A message/comment on a distro series difference.';560COMMENT ON TABLE DistroSeriesDifferenceMessage IS 'A message/comment on a distro series difference.';
560561
=== added file 'database/schema/patch-2208-24-0.sql'
--- database/schema/patch-2208-24-0.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-24-0.sql 2010-10-08 10:37:44 +0000
@@ -0,0 +1,7 @@
1SET client_min_messages=ERROR;
2
3-- Add column to store the base_version for derived/parent versions.
4ALTER TABLE DistroSeriesDifference ADD COLUMN base_version text;
5ALTER TABLE DistroSeriesDifference ADD CONSTRAINT valid_base_version CHECK(valid_debian_version(base_version));
6
7INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 24, 0);
08
=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py 2010-09-29 09:28:17 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2010-10-08 10:37:44 +0000
@@ -110,6 +110,12 @@
110 "The version of the most recent source publishing in the "110 "The version of the most recent source publishing in the "
111 "parent series."))111 "parent series."))
112112
113 base_version = TextLine(
114 title=_("Base version"), readonly=True,
115 description=_(
116 "The common base version of the package for differences "
117 "with different versions in the parent and derived series."))
118
113 owner = Reference(119 owner = Reference(
114 IPerson, title=_("Owning team of the derived series"), readonly=True,120 IPerson, title=_("Owning team of the derived series"), readonly=True,
115 description=_(121 description=_(
@@ -171,8 +177,7 @@
171class IDistroSeriesDifferenceSource(Interface):177class IDistroSeriesDifferenceSource(Interface):
172 """A utility of this interface can be used to create differences."""178 """A utility of this interface can be used to create differences."""
173179
174 def new(derived_series, source_package_name, difference_type,180 def new(derived_series, source_package_name):
175 status=DistroSeriesDifferenceStatus.NEEDS_ATTENTION):
176 """Create an `IDistroSeriesDifference`.181 """Create an `IDistroSeriesDifference`.
177182
178 :param derived_series: The distribution series which was derived183 :param derived_series: The distribution series which was derived
@@ -182,11 +187,6 @@
182 :param source_package_name: A source package name identifying the187 :param source_package_name: A source package name identifying the
183 package with a difference.188 package with a difference.
184 :type source_package_name: `ISourcePackageName`.189 :type source_package_name: `ISourcePackageName`.
185 :param difference_type: Indicates the type of difference represented
186 by this record.
187 :type difference_type: `DistroSeriesDifferenceType`.
188 :param status: The current status of this difference.
189 :type status: `DistorSeriesDifferenceStatus`.
190 :raises NotADerivedSeriesError: When the passed distro series190 :raises NotADerivedSeriesError: When the passed distro series
191 is not a derived series.191 is not a derived series.
192 :return: A new `DistroSeriesDifference` object.192 :return: A new `DistroSeriesDifference` object.
193193
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2010-09-28 14:42:41 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2010-10-08 10:37:44 +0000
@@ -28,6 +28,7 @@
28 IMasterStore,28 IMasterStore,
29 IStore,29 IStore,
30 )30 )
31from lp.archivepublisher.debversion import Version
31from lp.registry.enum import (32from lp.registry.enum import (
32 DistroSeriesDifferenceStatus,33 DistroSeriesDifferenceStatus,
33 DistroSeriesDifferenceType,34 DistroSeriesDifferenceType,
@@ -83,10 +84,10 @@
83 source_version = Unicode(name='source_version', allow_none=True)84 source_version = Unicode(name='source_version', allow_none=True)
84 parent_source_version = Unicode(name='parent_source_version',85 parent_source_version = Unicode(name='parent_source_version',
85 allow_none=True)86 allow_none=True)
87 base_version = Unicode(name='base_version', allow_none=True)
8688
87 @staticmethod89 @staticmethod
88 def new(derived_series, source_package_name, difference_type,90 def new(derived_series, source_package_name):
89 status=DistroSeriesDifferenceStatus.NEEDS_ATTENTION):
90 """See `IDistroSeriesDifferenceSource`."""91 """See `IDistroSeriesDifferenceSource`."""
91 if derived_series.parent_series is None:92 if derived_series.parent_series is None:
92 raise NotADerivedSeriesError()93 raise NotADerivedSeriesError()
@@ -95,16 +96,12 @@
95 diff = DistroSeriesDifference()96 diff = DistroSeriesDifference()
96 diff.derived_series = derived_series97 diff.derived_series = derived_series
97 diff.source_package_name = source_package_name98 diff.source_package_name = source_package_name
98 diff.status = status
99 diff.difference_type = difference_type
10099
101 source_pub = diff.source_pub100 # The status and type is set to default values - they will be
102 if source_pub is not None:101 # updated appropriately during the update() call.
103 diff.source_version = source_pub.source_package_version102 diff.status = DistroSeriesDifferenceStatus.NEEDS_ATTENTION
104 parent_source_pub = diff.parent_source_pub103 diff.difference_type = DistroSeriesDifferenceType.DIFFERENT_VERSIONS
105 if parent_source_pub is not None:104 diff.update()
106 diff.parent_source_version = (
107 parent_source_pub.source_package_version)
108105
109 return store.add(diff)106 return store.add(diff)
110107
@@ -251,8 +248,42 @@
251 updated = True248 updated = True
252 self.status = DistroSeriesDifferenceStatus.RESOLVED249 self.status = DistroSeriesDifferenceStatus.RESOLVED
253250
251 if self._updateBaseVersion():
252 updated = True
253
254 return updated254 return updated
255255
256 def _updateBaseVersion(self):
257 """Check for the most-recently published common version.
258
259 Return whether the record was updated or not.
260 """
261 if self.difference_type != (
262 DistroSeriesDifferenceType.DIFFERENT_VERSIONS):
263 return False
264
265 derived_pubs = self.derived_series.getPublishedSources(
266 self.source_package_name)
267 derived_versions = [
268 Version(pub.sourcepackagerelease.version) for pub in derived_pubs]
269 parent_pubs = self.derived_series.parent_series.getPublishedSources(
270 self.source_package_name)
271 parent_versions = [
272 Version(pub.sourcepackagerelease.version) for pub in parent_pubs]
273
274 common_versions = list(
275 set(derived_versions).intersection(parent_versions))
276 if common_versions:
277 common_versions.sort()
278 self.base_version = unicode(common_versions.pop())
279 return True
280
281 if self.base_version is None:
282 return False
283 else:
284 self.base_version = None
285 return True
286
256 def addComment(self, commenter, comment):287 def addComment(self, commenter, comment):
257 """See `IDistroSeriesDifference`."""288 """See `IDistroSeriesDifference`."""
258 return getUtility(IDistroSeriesDifferenceCommentSource).new(289 return getUtility(IDistroSeriesDifferenceCommentSource).new(
259290
=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-10-06 18:53:53 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-10-08 10:37:44 +0000
@@ -62,9 +62,7 @@
6262
63 self.assertRaises(63 self.assertRaises(
64 NotADerivedSeriesError, distroseriesdifference_factory.new,64 NotADerivedSeriesError, distroseriesdifference_factory.new,
65 distro_series, source_package_name,65 distro_series, source_package_name)
66 DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES,
67 )
6866
69 def test_source_pub(self):67 def test_source_pub(self):
70 # The related source pub is returned for the derived series.68 # The related source pub is returned for the derived series.
@@ -424,6 +422,71 @@
424 self.assertContentEqual(422 self.assertContentEqual(
425 ['source_pub', 'parent_source_pub'], cache)423 ['source_pub', 'parent_source_pub'], cache)
426424
425 def test_base_version_none(self):
426 # The attribute is set to None if there is no common base version.
427 # Publish different versions in the series.
428 derived_series = self.factory.makeDistroSeries(
429 parent_series=self.factory.makeDistroSeries())
430 source_package_name = self.factory.getOrMakeSourcePackageName('foo')
431 self.factory.makeSourcePackagePublishingHistory(
432 distroseries=derived_series,
433 version='1.0deri1',
434 sourcepackagename=source_package_name,
435 status=PackagePublishingStatus.PUBLISHED)
436 self.factory.makeSourcePackagePublishingHistory(
437 distroseries=derived_series.parent_series,
438 version='1.0ubu2',
439 sourcepackagename=source_package_name,
440 status=PackagePublishingStatus.PUBLISHED)
441 ds_diff = self.factory.makeDistroSeriesDifference()
442
443 self.assertIs(None, ds_diff.base_version)
444
445 def test_base_version_common(self):
446 # The common base version is set when the difference is created.
447 # Publish v1.0 of foo in both series.
448 derived_series = self.factory.makeDistroSeries(
449 parent_series=self.factory.makeDistroSeries())
450 source_package_name = self.factory.getOrMakeSourcePackageName('foo')
451 for series in [derived_series, derived_series.parent_series]:
452 self.factory.makeSourcePackagePublishingHistory(
453 distroseries=series,
454 version='1.0',
455 sourcepackagename=source_package_name,
456 status=PackagePublishingStatus.PUBLISHED)
457
458 ds_diff = self.factory.makeDistroSeriesDifference(
459 derived_series=derived_series, source_package_name_str='foo',
460 versions={
461 'derived': '1.2',
462 'parent': '1.3',
463 })
464
465 self.assertEqual('1.0', ds_diff.base_version)
466
467 def test_base_version_multiple(self):
468 # The latest common base version is set as the base-version.
469 # Publish v1.0 and 1.1 of foo in both series.
470 derived_series = self.factory.makeDistroSeries(
471 parent_series=self.factory.makeDistroSeries())
472 source_package_name = self.factory.getOrMakeSourcePackageName('foo')
473 for series in [derived_series, derived_series.parent_series]:
474 for version in ['1.0', '1.1']:
475 self.factory.makeSourcePackagePublishingHistory(
476 distroseries=series,
477 version=version,
478 sourcepackagename=source_package_name,
479 status=PackagePublishingStatus.PUBLISHED)
480
481 ds_diff = self.factory.makeDistroSeriesDifference(
482 derived_series=derived_series, source_package_name_str='foo',
483 versions={
484 'derived': '1.2',
485 'parent': '1.3',
486 })
487
488 self.assertEqual('1.1', ds_diff.base_version)
489
427490
428class DistroSeriesDifferenceSourceTestCase(TestCaseWithFactory):491class DistroSeriesDifferenceSourceTestCase(TestCaseWithFactory):
429492
430493
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-10-06 18:53:53 +0000
+++ lib/lp/testing/factory.py 2010-10-08 10:37:44 +0000
@@ -1843,22 +1843,6 @@
1843 expires=expires, restricted=restricted)1843 expires=expires, restricted=restricted)
1844 return library_file_alias1844 return library_file_alias
18451845
1846 def makePackageDiff(self, from_spr=None, to_spr=None):
1847 """Make a completed package diff."""
1848 if from_spr is None:
1849 from_spr = self.makeSourcePackageRelease()
1850 if to_spr is None:
1851 to_spr = self.makeSourcePackageRelease()
1852
1853 diff = from_spr.requestDiffTo(
1854 from_spr.creator, to_spr)
1855
1856 naked_diff = removeSecurityProxy(diff)
1857 naked_diff.status = PackageDiffStatus.COMPLETED
1858 naked_diff.diff_content = self.makeLibraryFileAlias()
1859 naked_diff.date_fulfilled = UTC_NOW
1860 return diff
1861
1862 def makeDistribution(self, name=None, displayname=None, owner=None,1846 def makeDistribution(self, name=None, displayname=None, owner=None,
1863 members=None, title=None, aliases=None):1847 members=None, title=None, aliases=None):
1864 """Make a new distribution."""1848 """Make a new distribution."""
@@ -1950,8 +1934,9 @@
1950 status = PackagePublishingStatus.PUBLISHED)1934 status = PackagePublishingStatus.PUBLISHED)
19511935
1952 diff = getUtility(IDistroSeriesDifferenceSource).new(1936 diff = getUtility(IDistroSeriesDifferenceSource).new(
1953 derived_series, source_package_name, difference_type,1937 derived_series, source_package_name)
1954 status=status)1938
1939 removeSecurityProxy(diff).status = status
19551940
1956 # We clear the cache on the diff, returning the object as if it1941 # We clear the cache on the diff, returning the object as if it
1957 # was just loaded from the store.1942 # was just loaded from the store.
@@ -2622,7 +2607,7 @@
2622 creator = self.makePerson()2607 creator = self.makePerson()
26232608
2624 if version is None:2609 if version is None:
2625 version = self.getUniqueString('version')2610 version = unicode(self.getUniqueInteger()) + 'version'
26262611
2627 return distroseries.createUploadedSourcePackageRelease(2612 return distroseries.createUploadedSourcePackageRelease(
2628 sourcepackagename=sourcepackagename,2613 sourcepackagename=sourcepackagename,

Subscribers

People subscribed via source and target branches

to status/vote changes: