Merge lp:~michael.nelson/launchpad/distro-series-difference-basic-model into lp:launchpad/db-devel

[Michael Nelson]

Adds basic model functionality for the DistroSeriesDifference model (for https://dev.launchpad.net/LEP/DerivativeDistributions)

Overview
========

This branch continues on from:

https://code.edge.launchpad.net/~michael.nelson/launchpad/distro-series-difference-schema/+merge/33515

and implements the basic model, enums and tests for representing the distroseriesdifference pages such as:

https://dev.launchpad.net/LEP/DerivativeDistributions?action=AttachFile&do=get&target=derived-series-diffs_5.png

Details
=======

I've set the status and activity_log fields as readonly as a following branch will adds more functionality to the model including security for the status and activity_log fields.

Test with:
bin/test -vvm lp.registry.tests.test_distroseriesdifference

[Abel Deuring (adeuring)]

Hi Michael,

nice work! I have just two nitpicks:

> === added file 'lib/lp/registry/interfaces/distroseriesdifference.py'
> --- lib/lp/registry/interfaces/distroseriesdifference.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/interfaces/distroseriesdifference.py 2010-08-27 09:31:04 +0000

[...]

> + activity_log = Text(
> + title=_('A log of activity and comments for this difference'),
> + required=False, readonly=True)

s/difference/difference./

> === added file 'lib/lp/registry/tests/test_distroseriesdifference.py'
> --- lib/lp/registry/tests/test_distroseriesdifference.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-08-27 09:31:04 +0000

[...]

> + def test_new_non_derived_series(self):
> + # A DistroSeriesDifference cannot be created with a non-derived
> + # series.
> + distro_series = self.factory.makeDistroSeries()
> + self.assertRaises(
> + NotADerivedSeriesError,
> + self.factory.makeDistroSeriesDifference,
> + derived_series=distro_series)

I would prefer something like

    distroseriesdifferencesource_factory = getUtility(IDistroSeriesDifferenceSource)
    self.assertRaises(
        NotADerivedSeriesError, distroseriesdifferencesource_factory.new, ...)

This makes it a bit more explicit which method is tested.

review: Approve (code)

[Michael Nelson (michael.nelson)]

On Fri, Aug 27, 2010 at 1:39 PM, Abel Deuring
<email address hidden> wrote:
> Review: Approve code
> Hi Michael,
>
> nice work! I have just two nitpicks:

Thanks Abel, incremental attached. In addition to the changes you
mentioned, I've switched the activity_log to be a property rather than
a db field due to a schema change.

Cheers,
Michael

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2010-08-23 03:25:20 +0000
+++ lib/lp/registry/configure.zcml	2010-08-31 07:47:46 +0000
@@ -102,6 +102,17 @@
                 date_last_modified
                 person"/>
     </class>
+
+    <!-- DistroSeriesDifference -->
+    <securedutility
+        component="lp.registry.model.distroseriesdifference.DistroSeriesDifference"
+        provides="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifferenceSource">
+        <allow interface="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifferenceSource"/>
+    </securedutility>
+    <class
+        class="lp.registry.model.distroseriesdifference.DistroSeriesDifference">
+        <allow interface="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifference"/>
+    </class>
     <class
         class="lp.registry.model.distroseries.DistroSeries">
         <allow

=== modified file 'lib/lp/registry/enum.py'
--- lib/lp/registry/enum.py	2010-08-20 20:31:18 +0000
+++ lib/lp/registry/enum.py	2010-08-31 07:47:46 +0000
@@ -6,6 +6,8 @@
 __metaclass__ = type
 __all__ = [
     'BugNotificationLevel',
+    'DistroSeriesDifferenceStatus',
+    'DistroSeriesDifferenceType',
     ]
 
 from lazr.enum import (
@@ -47,3 +49,58 @@
         notifications about new events in the bugs's discussion, like new
         comments.
         """)
+
+
+class DistroSeriesDifferenceStatus(DBEnumeratedType):
+    """Distribution series difference status.
+
+    The status of a package difference between two DistroSeries.
+    """
+
+    NEEDS_ATTENTION = DBItem(1, """
+        Needs attention
+
+        This difference is current and needs attention.
+        """)
+
+    IGNORED = DBItem(2, """
+        Ignored
+
+        This difference is being ignored until a new package is uploaded
+        or the status is manually updated.
+        """)
+
+    IGNORED_ALWAYS = DBItem(3, """
+        Ignored always
+
+        This difference should always be ignored.
+        """)
+
+    RESOLVED = DBItem(4, """
+        Resolved
+
+        This difference has been resolved and versions are now equal.
+        """)
+
+class DistroSeriesDifferenceType(DBEnumeratedType):
+    """Distribution series difference type."""
+
+    UNIQUE_TO_DERIVED_SERIES = DBItem(1, """
+        Unique to derived series
+
+        This package is present in the derived series but not the parent
+        series.
+        """)
+
+    MISSING_FROM_DERIVED_SERIES = DBItem(2, """
+        Missing from derived series
+
+        This package is present in the parent series but missing from the
+        derived series.
+        """)
+
+    DIFFERENT_VERSIONS = DBItem(3, """
+        Different versions
+
+        This package is present in both series with different versions.
+        """)

=== added file 'lib/lp/registry/exceptions.py'
--- lib/lp/registry/exceptions.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/exceptions.py	2010-08-31 07:47:46 +0000
@@ -0,0 +1,17 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Exceptions for the Registry app."""
+
+__metaclass__ = type
+__all__ = [
+    'NotADerivedSeriesError',
+    ]
+
+
+class NotADerivedSeriesError(Exception):
+    """A distro series difference must be created with a derived series.
+
+    This is raised when a DistroSeriesDifference is created with a
+    non-derived series - that is, a distroseries with a null Parent."""
+

=== added file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py	2010-08-31 07:47:46 +0000
@@ -0,0 +1,108 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Interface classes for a difference between two distribution series."""
+
+__metaclass__ = type
+
+
+__all__ = [
+    'IDistroSeriesDifference',
+    'IDistroSeriesDifferenceSource',
+    ]
+
+from lazr.restful.fields import Reference
+from zope.interface import Interface
+from zope.schema import (
+    Choice,
+    Int,
+    Text,
+    )
+
+from canonical.launchpad import _
+from lp.registry.enum import (
+    DistroSeriesDifferenceStatus,
+    DistroSeriesDifferenceType,
+    )
+from lp.registry.interfaces.distroseries import IDistroSeries
+from lp.registry.interfaces.sourcepackagename import ISourcePackageName
+from lp.soyuz.interfaces.packagediff import IPackageDiff
+from lp.soyuz.interfaces.publishing import ISourcePackagePublishingHistory
+
+
+class IDistroSeriesDifference(Interface):
+    """An interface for a package difference between two distroseries."""
+
+    id = Int(title=_('ID'), required=True, readonly=True)
+
+    derived_series = Reference(
+        IDistroSeries, title=_("Derived series"), required=True,
+        readonly=True, description=_(
+            "The distribution series which, together with its parent, "
+            "identifies the two series with the difference."))
+
+    source_package_name = Reference(
+        ISourcePackageName,
+        title=_("Source package name"), required=True, readonly=True,
+        description=_(
+            "The package with a difference between the derived series "
+            "and its parent."))
+
+    package_diff = Reference(
+        IPackageDiff, title=_("Package diff"), required=False,
+        readonly=True, description=_(
+            "The most recently generated package diff for this difference."))
+
+    activity_log = Text(
+        title=_('A log of activity and comments for this difference.'),
+        required=False, readonly=True)
+
+    status = Choice(
+        title=_('Distro series difference status.'),
+        description=_('The current status of this difference.'),
+        vocabulary=DistroSeriesDifferenceStatus,
+        required=True, readonly=True)
+
+    difference_type = Choice(
+        title=_('Difference type'),
+        description=_('The type of difference for this package.'),
+        vocabulary=DistroSeriesDifferenceType,
+        required=True, readonly=True)
+
+    source_pub = Reference(
+        ISourcePackagePublishingHistory,
+        title=_("Derived source pub"), readonly=True,
+        description=_(
+            "The most recent published version in the derived series."))
+
+    parent_source_pub = Reference(
+        ISourcePackagePublishingHistory,
+        title=_("Parent source pub"), readonly=True,
+        description=_(
+            "The most recent published version in the parent series."))
+
+
+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):
+        """Create an `IDistroSeriesDifference`.
+
+        :param derived_series: The distribution series which was derived
+            from a parent. If a series without a parent is passed an
+            exception is raised.
+        :type derived_series: `IDistroSeries`.
+        :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.
+        """
+

=== added file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/model/distroseriesdifference.py	2010-08-31 07:47:46 +0000
@@ -0,0 +1,117 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Database classes for a difference between two distribution series."""
+
+__metaclass__ = type
+
+__all__ = [
+    'DistroSeriesDifference',
+    ]
+
+
+from storm.locals import (
+    Int,
+    Reference,
+    Storm,
+    Unicode,
+    )
+from zope.interface import (
+    classProvides,
+    implements,
+    )
+
+from canonical.database.enumcol import DBEnum
+from canonical.launchpad.interfaces.lpstorm import IMasterStore
+from lp.registry.enum import (
+    DistroSeriesDifferenceStatus,
+    DistroSeriesDifferenceType,
+    )
+from lp.registry.exceptions import NotADerivedSeriesError
+from lp.registry.interfaces.distroseriesdifference import (
+    IDistroSeriesDifference,
+    IDistroSeriesDifferenceSource,
+    )
+
+
+class DistroSeriesDifference(Storm):
+    """See `DistroSeriesDifference`."""
+    implements(IDistroSeriesDifference)
+    classProvides(IDistroSeriesDifferenceSource)
+    __storm_table__ = 'DistroSeriesDifference'
+
+    id = Int(primary=True)
+
+    derived_series_id = Int(name='derived_series', allow_none=False)
+    derived_series = Reference(
+        derived_series_id, 'DistroSeries.id')
+
+    source_package_name_id = Int(
+        name='source_package_name', allow_none=False)
+    source_package_name = Reference(
+        source_package_name_id, 'SourcePackageName.id')
+
+    package_diff_id = Int(
+        name='package_diff', allow_none=True)
+    package_diff = Reference(
+        package_diff_id, 'PackageDiff.id')
+
+    status = DBEnum(name='status', allow_none=False,
+                    enum=DistroSeriesDifferenceStatus)
+    difference_type = DBEnum(name='difference_type', allow_none=False,
+                             enum=DistroSeriesDifferenceType)
+
+    @staticmethod
+    def new(derived_series, source_package_name, difference_type,
+            status=DistroSeriesDifferenceStatus.NEEDS_ATTENTION):
+        """See `IDistroSeriesDifferenceSource`."""
+        if derived_series.parent_series is None:
+            raise NotADerivedSeriesError()
+
+        store = IMasterStore(DistroSeriesDifference)
+        diff = DistroSeriesDifference()
+        diff.derived_series = derived_series
+        diff.source_package_name = source_package_name
+        diff.status = status
+        diff.difference_type = difference_type
+
+        return store.add(diff)
+
+    @property
+    def source_pub(self):
+        """See `IDistroSeriesDifference`."""
+        return self._getLatestSourcePub()
+
+    @property
+    def parent_source_pub(self):
+        """See `IDistroSeriesDifference`."""
+        return self._getLatestSourcePub(for_parent=True)
+
+    @property
+    def activity_log(self):
+        """See `IDistroSeriesDifference`."""
+        return u""
+
+    def _getLatestSourcePub(self, for_parent=False):
+        """Helper to keep source_pub/parent_source_pub DRY."""
+        distro_series = self.derived_series
+        if for_parent:
+            distro_series = self.derived_series.parent_series
+
+        pubs = distro_series.getPublishedSources(
+            self.source_package_name, include_pending=True)
+
+        # The most recent published source is the first one.
+        if pubs:
+            return pubs[0]
+        else:
+            return None
+
+    def _getVersions(self):
+        """Helper method returning versions string."""
+        src_pub_ver = parent_src_pub_ver = "-"
+        if self.source_pub:
+            src_pub_ver = self.source_pub.source_package_version
+        if self.parent_source_pub is not None:
+            parent_src_pub_ver = self.parent_source_pub.source_package_version
+        return parent_src_pub_ver + "/" + src_pub_ver

=== added file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py	2010-08-31 07:47:46 +0000
@@ -0,0 +1,110 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Model tests for the DistroSeriesDifference class."""
+
+__metaclass__ = type
+
+import unittest
+
+from storm.store import Store
+from zope.component import getUtility
+
+from canonical.launchpad.webapp.testing import verifyObject
+from canonical.testing import DatabaseFunctionalLayer
+from lp.testing import TestCaseWithFactory
+from lp.registry.enum import DistroSeriesDifferenceType
+from lp.registry.exceptions import NotADerivedSeriesError
+from lp.registry.interfaces.distroseriesdifference import (
+    IDistroSeriesDifference,
+    IDistroSeriesDifferenceSource,
+    )
+from lp.soyuz.interfaces.publishing import PackagePublishingStatus
+
+
+class DistroSeriesDifferenceTestCase(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_implements_interface(self):
+        # The implementation implements the interface correctly.
+        ds_diff = self.factory.makeDistroSeriesDifference()
+        # Flush the store to ensure db constraints are triggered.
+        Store.of(ds_diff).flush()
+
+        verifyObject(IDistroSeriesDifference, ds_diff)
+
+    def test_source_implements_interface(self):
+        # The utility for creating differences implements its interface.
+        utility = getUtility(IDistroSeriesDifferenceSource)
+
+        verifyObject(IDistroSeriesDifferenceSource, utility)
+
+    def test_new_non_derived_series(self):
+        # A DistroSeriesDifference cannot be created with a non-derived
+        # series.
+        distro_series = self.factory.makeDistroSeries()
+        source_package_name = self.factory.makeSourcePackageName('myfoo')
+        distroseriesdifference_factory = getUtility(
+            IDistroSeriesDifferenceSource)
+
+        self.assertRaises(
+            NotADerivedSeriesError, distroseriesdifference_factory.new,
+            distro_series, source_package_name,
+            DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES
+            )
+
+    def test_source_pub(self):
+        # The related source pub is returned for the derived series.
+        ds_diff = self.factory.makeDistroSeriesDifference(
+            source_package_name_str="foonew")
+
+        self.assertEqual(
+            'foonew', ds_diff.source_pub.source_package_name)
+        self.assertEqual(
+            ds_diff.derived_series, ds_diff.source_pub.distroseries)
+
+    def test_source_pub_gets_latest_pending(self):
+        # The most recent publication is always returned, even if its pending.
+        ds_diff = self.factory.makeDistroSeriesDifference(
+            source_package_name_str="foonew")
+        pending_pub = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename=ds_diff.source_package_name,
+            distroseries=ds_diff.derived_series,
+            status=PackagePublishingStatus.PENDING)
+
+        self.assertEqual(pending_pub, ds_diff.source_pub)
+
+    def test_source_pub_returns_none(self):
+        # None is returned when there is no source pub.
+        ds_diff = self.factory.makeDistroSeriesDifference(
+            difference_type=(
+                DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES))
+
+        self.assertIs(None, ds_diff.source_pub)
+
+    def test_parent_source_pub(self):
+        # The related source pub for the parent distro series is returned.
+        ds_diff = self.factory.makeDistroSeriesDifference(
+            source_package_name_str="foonew")
+
+        self.assertEqual(
+            'foonew', ds_diff.parent_source_pub.source_package_name)
+        self.assertEqual(
+            ds_diff.derived_series.parent_series,
+            ds_diff.parent_source_pub.distroseries)
+
+    def test_paren_source_pub_gets_latest_pending(self):
+        # The most recent publication is always returned, even if its pending.
+        ds_diff = self.factory.makeDistroSeriesDifference(
+            source_package_name_str="foonew")
+        pending_pub = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename=ds_diff.source_package_name,
+            distroseries=ds_diff.derived_series.parent_series,
+            status=PackagePublishingStatus.PENDING)
+
+        self.assertEqual(pending_pub, ds_diff.parent_source_pub)
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-08-30 16:32:53 +0000
+++ lib/lp/testing/factory.py	2010-08-31 07:47:46 +0000
@@ -154,12 +154,16 @@
     IHWSubmissionDeviceSet,
     IHWSubmissionSet,
     )
+from lp.registry.enum import DistroSeriesDifferenceType
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.distributionmirror import (
     MirrorContent,
     MirrorSpeed,
     )
 from lp.registry.interfaces.distroseries import IDistroSeries
+from lp.registry.interfaces.distroseriesdifference import (
+    IDistroSeriesDifferenceSource,
+    )
 from lp.registry.interfaces.gpg import (
     GPGKeyAlgorithm,
     IGPGKeySet,
@@ -1817,6 +1821,44 @@
     # Most people think of distro releases as distro series.
     makeDistroSeries = makeDistroRelease
 
+    def makeDistroSeriesDifference(
+        self, derived_series=None, source_package_name_str=None,
+        versions=None,
+        difference_type=DistroSeriesDifferenceType.DIFFERENT_VERSIONS):
+        """Create a new distro series source package difference."""
+        if derived_series is None:
+            parent_series = self.makeDistroSeries()
+            derived_series = self.makeDistroSeries(
+                parent_series=parent_series)
+
+        if source_package_name_str is None:
+            source_package_name_str = self.getUniqueString('src-name')
+
+        source_package_name = self.getOrMakeSourcePackageName(
+            source_package_name_str)
+
+        if versions is None:
+            versions = {}
+
+        if difference_type is not (
+            DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES):
+
+            source_pub = self.makeSourcePackagePublishingHistory(
+                distroseries=derived_series,
+                version=versions.get('derived'),
+                sourcepackagename=source_package_name)
+
+        if difference_type is not (
+            DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES):
+
+            source_pub = self.makeSourcePackagePublishingHistory(
+                distroseries=derived_series.parent_series,
+                version=versions.get('parent'),
+                sourcepackagename=source_package_name)
+
+        return getUtility(IDistroSeriesDifferenceSource).new(
+            derived_series, source_package_name, difference_type)
+
     def makeDistroArchSeries(self, distroseries=None,
                              architecturetag=None, processorfamily=None,
                              official=True, owner=None,