launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01401
lp:~michael.nelson/launchpad/638090-base_version-property-for-differences into lp:launchpad
Michael Nelson has proposed merging lp:~michael.nelson/launchpad/638090-base_version-property-for-differences into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#638090 base_version property for DistroSeriesDifference
https://bugs.launchpad.net/bugs/638090
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
--
https://code.launchpad.net/~michael.nelson/launchpad/638090-base_version-property-for-differences/+merge/37742
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~michael.nelson/launchpad/638090-base_version-property-for-differences into lp:launchpad.
=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql 2010-10-04 22:56:09 +0000
+++ database/schema/comments.sql 2010-10-06 14:07:48 +0000
@@ -553,6 +553,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-99-0.sql'
--- database/schema/patch-2208-99-0.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-99-0.sql 2010-10-06 14:07:48 +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, 99, 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-06 14:07:48 +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-06 14:07:48 +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-09-22 11:03:20 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-10-06 14:07:48 +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,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')
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=derived_series,
+ version='1.0',
+ sourcepackagename=source_package_name,
+ status = PackagePublishingStatus.PUBLISHED)
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=derived_series.parent_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)
+
class DistroSeriesDifferenceSourceTestCase(TestCaseWithFactory):
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-09-30 05:47:44 +0000
+++ lib/lp/testing/factory.py 2010-10-06 14:07:48 +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,
