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

[Michael Nelson]

Adds DistroSeriesDifference.base_version

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

[Edwin Grubbs (edwin-grubbs)]

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)

[Stuart Bishop (stub)]

Fine. patch-2208-24-0.sql

review: Approve (db)

=== 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 @@
 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.';
 COMMENT ON COLUMN DistroSeriesDifference.source_version IS 'The version of the package in the derived series.';
 COMMENT ON COLUMN DistroSeriesDifference.parent_source_version IS 'The version of the package in the parent series.';
+COMMENT ON COLUMN DistroSeriesDifference.base_version IS 'The common base version of the package for the derived and parent series.';
 
 -- DistroSeriesDifferenceMessage
 COMMENT ON TABLE DistroSeriesDifferenceMessage IS 'A message/comment on a distro series difference.';

=== 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 @@
+SET client_min_messages=ERROR;
+
+-- Add column to store the base_version for derived/parent versions.
+ALTER TABLE DistroSeriesDifference ADD COLUMN base_version text;
+ALTER TABLE DistroSeriesDifference ADD CONSTRAINT valid_base_version CHECK(valid_debian_version(base_version));
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 24, 0);

=== 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 @@
             "The version of the most recent source publishing in the "
             "parent series."))
 
+    base_version = TextLine(
+        title=_("Base version"), readonly=True,
+        description=_(
+            "The common base version of the package for differences "
+            "with different versions in the parent and derived series."))
+
     owner = Reference(
         IPerson, title=_("Owning team of the derived series"), readonly=True,
         description=_(
@@ -171,8 +177,7 @@
 class IDistroSeriesDifferenceSource(Interface):
     """A utility of this interface can be used to create differences."""
 
-    def new(derived_series, source_package_name, difference_type,
-            status=DistroSeriesDifferenceStatus.NEEDS_ATTENTION):
+    def new(derived_series, source_package_name):
         """Create an `IDistroSeriesDifference`.
 
         :param derived_series: The distribution series which was derived
@@ -182,11 +187,6 @@
         :param source_package_name: A source package name identifying the
             package with a difference.
         :type source_package_name: `ISourcePackageName`.
-        :param difference_type: Indicates the type of difference represented
-            by this record.
-        :type difference_type: `DistroSeriesDifferenceType`.
-        :param status: The current status of this difference.
-        :type status: `DistorSeriesDifferenceStatus`.
         :raises NotADerivedSeriesError: When the passed distro series
             is not a derived series.
         :return: A new `DistroSeriesDifference` object.

=== 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 @@
     IMasterStore,
     IStore,
     )
+from lp.archivepublisher.debversion import Version
 from lp.registry.enum import (
     DistroSeriesDifferenceStatus,
     DistroSeriesDifferenceType,
@@ -83,10 +84,10 @@
     source_version = Unicode(name='source_version', allow_none=True)
     parent_source_version = Unicode(name='parent_source_version',
                                     allow_none=True)
+    base_version = Unicode(name='base_version', allow_none=True)
 
     @staticmethod
-    def new(derived_series, source_package_name, difference_type,
-            status=DistroSeriesDifferenceStatus.NEEDS_ATTENTION):
+    def new(derived_series, source_package_name):
         """See `IDistroSeriesDifferenceSource`."""
         if derived_series.parent_series is None:
             raise NotADerivedSeriesError()
@@ -95,16 +96,12 @@
         diff = DistroSeriesDifference()
         diff.derived_series = derived_series
         diff.source_package_name = source_package_name
-        diff.status = status
-        diff.difference_type = difference_type
 
-        source_pub = diff.source_pub
-        if source_pub is not None:
-            diff.source_version = source_pub.source_package_version
-        parent_source_pub = diff.parent_source_pub
-        if parent_source_pub is not None:
-            diff.parent_source_version = (
-                parent_source_pub.source_package_version)
+        # The status and type is set to default values - they will be
+        # updated appropriately during the update() call.
+        diff.status = DistroSeriesDifferenceStatus.NEEDS_ATTENTION
+        diff.difference_type = DistroSeriesDifferenceType.DIFFERENT_VERSIONS
+        diff.update()
 
         return store.add(diff)
 
@@ -251,8 +248,42 @@
                 updated = True
                 self.status = DistroSeriesDifferenceStatus.RESOLVED
 
+        if self._updateBaseVersion():
+            updated = True
+
         return updated
 
+    def _updateBaseVersion(self):
+        """Check for the most-recently published common version.
+
+        Return whether the record was updated or not.
+        """
+        if self.difference_type != (
+            DistroSeriesDifferenceType.DIFFERENT_VERSIONS):
+            return False
+
+        derived_pubs = self.derived_series.getPublishedSources(
+            self.source_package_name)
+        derived_versions = [
+            Version(pub.sourcepackagerelease.version) for pub in derived_pubs]
+        parent_pubs = self.derived_series.parent_series.getPublishedSources(
+            self.source_package_name)
+        parent_versions = [
+            Version(pub.sourcepackagerelease.version) for pub in parent_pubs]
+
+        common_versions = list(
+            set(derived_versions).intersection(parent_versions))
+        if common_versions:
+            common_versions.sort()
+            self.base_version = unicode(common_versions.pop())
+            return True
+
+        if self.base_version is None:
+            return False
+        else:
+            self.base_version = None
+            return True
+
     def addComment(self, commenter, comment):
         """See `IDistroSeriesDifference`."""
         return getUtility(IDistroSeriesDifferenceCommentSource).new(

=== 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 @@
 
         self.assertRaises(
             NotADerivedSeriesError, distroseriesdifference_factory.new,
-            distro_series, source_package_name,
-            DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES,
-            )
+            distro_series, source_package_name)
 
     def test_source_pub(self):
         # The related source pub is returned for the derived series.
@@ -424,6 +422,71 @@
         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.
+        # Publish different versions in the series.
+        derived_series = self.factory.makeDistroSeries(
+            parent_series=self.factory.makeDistroSeries())
+        source_package_name = self.factory.getOrMakeSourcePackageName('foo')
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=derived_series,
+            version='1.0deri1',
+            sourcepackagename=source_package_name,
+            status=PackagePublishingStatus.PUBLISHED)
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=derived_series.parent_series,
+            version='1.0ubu2',
+            sourcepackagename=source_package_name,
+            status=PackagePublishingStatus.PUBLISHED)
+        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')
+        for series in [derived_series, derived_series.parent_series]:
+            self.factory.makeSourcePackagePublishingHistory(
+                distroseries=series,
+                version='1.0',
+                sourcepackagename=source_package_name,
+                status=PackagePublishingStatus.PUBLISHED)
+
+        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)
+
+    def test_base_version_multiple(self):
+        # The latest common base version is set as the base-version.
+        # Publish v1.0 and 1.1 of foo in both series.
+        derived_series = self.factory.makeDistroSeries(
+            parent_series=self.factory.makeDistroSeries())
+        source_package_name = self.factory.getOrMakeSourcePackageName('foo')
+        for series in [derived_series, derived_series.parent_series]:
+            for version in ['1.0', '1.1']:
+                self.factory.makeSourcePackagePublishingHistory(
+                    distroseries=series,
+                    version=version,
+                    sourcepackagename=source_package_name,
+                    status=PackagePublishingStatus.PUBLISHED)
+
+        ds_diff = self.factory.makeDistroSeriesDifference(
+            derived_series=derived_series, source_package_name_str='foo',
+            versions={
+                'derived': '1.2',
+                'parent': '1.3',
+                })
+
+        self.assertEqual('1.1', ds_diff.base_version)
+
 
 class DistroSeriesDifferenceSourceTestCase(TestCaseWithFactory):
 

=== 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 @@
             expires=expires, restricted=restricted)
         return library_file_alias
 
-    def makePackageDiff(self, from_spr=None, to_spr=None):
-        """Make a completed package diff."""
-        if from_spr is None:
-            from_spr = self.makeSourcePackageRelease()
-        if to_spr is None:
-            to_spr = self.makeSourcePackageRelease()
-
-        diff = from_spr.requestDiffTo(
-            from_spr.creator, to_spr)
-
-        naked_diff = removeSecurityProxy(diff)
-        naked_diff.status = PackageDiffStatus.COMPLETED
-        naked_diff.diff_content = self.makeLibraryFileAlias()
-        naked_diff.date_fulfilled = UTC_NOW
-        return diff
-
     def makeDistribution(self, name=None, displayname=None, owner=None,
                          members=None, title=None, aliases=None):
         """Make a new distribution."""
@@ -1950,8 +1934,9 @@
                 status = PackagePublishingStatus.PUBLISHED)
 
         diff = getUtility(IDistroSeriesDifferenceSource).new(
-            derived_series, source_package_name, difference_type,
-            status=status)
+            derived_series, source_package_name)
+
+        removeSecurityProxy(diff).status = status
 
         # We clear the cache on the diff, returning the object as if it
         # was just loaded from the store.
@@ -2622,7 +2607,7 @@
             creator = self.makePerson()
 
         if version is None:
-            version = self.getUniqueString('version')
+            version = unicode(self.getUniqueInteger()) + 'version'
 
         return distroseries.createUploadedSourcePackageRelease(
             sourcepackagename=sourcepackagename,