launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01436
[Merge] lp:~michael.nelson/launchpad/656166-expose-dsd-diffs into lp:launchpad
Michael Nelson has proposed merging lp:~michael.nelson/launchpad/656166-expose-dsd-diffs into lp:launchpad with lp:~michael.nelson/launchpad/638090-base_version-property-for-differences as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#656166 Cannot request an IPackageDiff for a DistroSeriesDifference via api
https://bugs.launchpad.net/bugs/656166
Overview
========
This branch adds the ability to request the generation of package diffs for DistroSeriesDifference objects, and exposes this to the API.
Details
=======
I removed lp.registry.exceptions (which I'd created a few branches ago), after seeing that lp.registry.errors already exists.
Generating the diffs also required adding a base_source_pub attribute to IDSDifference.
Test
====
bin/test -vvm test_distroseriesdifference
--
https://code.launchpad.net/~michael.nelson/launchpad/656166-expose-dsd-diffs/+merge/37858
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~michael.nelson/launchpad/656166-expose-dsd-diffs into lp:launchpad.
=== modified file 'lib/canonical/launchpad/interfaces/__init__.py'
--- lib/canonical/launchpad/interfaces/__init__.py 2010-10-06 18:53:53 +0000
+++ lib/canonical/launchpad/interfaces/__init__.py 2010-10-07 15:03:39 +0000
@@ -98,6 +98,10 @@
from canonical.launchpad.interfaces.packagerelationship import *
from canonical.launchpad.interfaces.pathlookup import *
from lp.registry.interfaces.poll import *
+<<<<<<< TREE
+=======
+from lp.registry.errors import *
+>>>>>>> MERGE-SOURCE
from lp.soyuz.interfaces.processor import *
from lp.registry.interfaces.product import *
from lp.registry.interfaces.productlicense import *
=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 2010-09-29 09:53:15 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 2010-10-07 15:03:39 +0000
@@ -7,8 +7,12 @@
import transaction
-from lazr.restfulclient.errors import Unauthorized
+from lazr.restfulclient.errors import (
+ BadRequest,
+ Unauthorized,
+ )
from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
from canonical.testing import AppServerLayer
from canonical.launchpad.webapp.publisher import canonical_url
@@ -16,6 +20,7 @@
from lp.registry.interfaces.distroseriesdifference import (
IDistroSeriesDifferenceSource,
)
+from lp.soyuz.enums import PackageDiffStatus
from lp.testing import (
TestCaseWithFactory,
ws_object,
@@ -97,3 +102,52 @@
self.assertTrue(
result['resource_type_link'].endswith(
'#distro_series_difference_comment'))
+
+ def test_requestDiffs(self):
+ # The generation of package diffs can be requested via the API.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ source_package_name_str='foo', versions={
+ 'derived': '1.2',
+ 'parent': '1.3',
+ 'base': '1.0',
+ })
+ ws_diff = ws_object(self.factory.makeLaunchpadService(
+ ds_diff.owner), ds_diff)
+
+ result = ws_diff.requestPackageDiffs()
+ transaction.commit()
+
+ # Reload and check that the package diffs are there.
+ utility = getUtility(IDistroSeriesDifferenceSource)
+ ds_diff = utility.getByDistroSeriesAndName(
+ ds_diff.derived_series, ds_diff.source_package_name.name)
+ self.assertIsNot(None, ds_diff.package_diff)
+ self.assertIsNot(None, ds_diff.parent_package_diff)
+
+ def test_requestPackageDiffs_exception(self):
+ # If one of the pubs is missing an exception is raised.
+ ds_diff = self.factory.makeDistroSeriesDifference(versions={
+ 'derived': '1.2',
+ 'parent': '1.3',
+ })
+
+ ws_diff = ws_object(self.factory.makeLaunchpadService(
+ ds_diff.owner), ds_diff)
+
+ self.assertRaises(
+ BadRequest, ws_diff.requestPackageDiffs)
+
+ def test_package_diffs(self):
+ # The package diff urls exposed.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+ naked_dsdiff = removeSecurityProxy(ds_diff)
+ naked_dsdiff.package_diff = self.factory.makePackageDiff(
+ status=PackageDiffStatus.PENDING)
+ naked_dsdiff.parent_package_diff = self.factory.makePackageDiff()
+
+ ws_diff = ws_object(self.factory.makeLaunchpadService(
+ ds_diff.owner), ds_diff)
+
+ self.assertEqual(None, ws_diff.package_diff_url)
+ self.assertTrue(ws_diff.parent_package_diff_url.startswith(
+ 'http://localhost:58000/'))
=== modified file 'lib/lp/registry/errors.py'
--- lib/lp/registry/errors.py 2010-09-30 15:13:57 +0000
+++ lib/lp/registry/errors.py 2010-10-07 15:03:39 +0000
@@ -3,6 +3,7 @@
__metaclass__ = type
__all__ = [
+<<<<<<< TREE
'PrivatePersonLinkageError',
'NameAlreadyTaken',
'NoSuchDistroSeries',
@@ -15,6 +16,10 @@
'MirrorNotProbed',
'DeleteSubscriptionError',
'UserCannotSubscribePerson',
+=======
+ 'DistroSeriesDifferenceError',
+ 'NotADerivedSeriesError',
+>>>>>>> MERGE-SOURCE
'TeamMembershipTransitionError',
]
@@ -107,6 +112,18 @@
webservice_error(httplib.UNAUTHORIZED)
+class DistroSeriesDifferenceError(Exception):
+ """Raised when package diffs cannot be created for a difference."""
+ webservice_error(httplib.BAD_REQUEST)
+
+
+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."""
+
+
class TeamMembershipTransitionError(ValueError):
"""Indicates something has gone wrong with the transtiion.
=== removed file 'lib/lp/registry/exceptions.py'
--- lib/lp/registry/exceptions.py 2010-08-27 08:17:00 +0000
+++ lib/lp/registry/exceptions.py 1970-01-01 00:00:00 +0000
@@ -1,17 +0,0 @@
-# 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."""
-
=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py 2010-10-07 15:03:24 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2010-10-07 15:03:39 +0000
@@ -68,12 +68,24 @@
"The most recently generated package diff from the base to the "
"derived version."))
+ package_diff_url = exported(TextLine(
+ title=_("Package diff url"), readonly=True, required=False,
+ description=_(
+ "The url for the diff between the base version and the "
+ "derived version.")))
+
parent_package_diff = Reference(
IPackageDiff, title=_("Parent package diff"), required=False,
readonly=True, description=_(
"The most recently generated package diff from the base to the "
"parent version."))
+ parent_package_diff_url = exported(TextLine(
+ title=_("Parent package diff url"), readonly=True, required=False,
+ description=_(
+ "The url for the diff between the base version and the "
+ "parent version.")))
+
status = Choice(
title=_('Distro series difference status.'),
description=_('The current status of this difference.'),
@@ -116,6 +128,12 @@
"The common base version of the package for differences "
"with different versions in the parent and derived series."))
+ base_source_pub = Reference(
+ ISourcePackagePublishingHistory,
+ title=_("Base source pub"), readonly=True,
+ description=_(
+ "The common base version published in the derived series."))
+
owner = Reference(
IPerson, title=_("Owning team of the derived series"), readonly=True,
description=_(
@@ -167,6 +185,15 @@
The status will be updated based on the versions.
"""
+ @call_with(requestor=REQUEST_USER)
+ @export_write_operation()
+ def requestPackageDiffs(requestor):
+ """Requests IPackageDiffs for the derived and parent version.
+
+ :raises DistroSeriesDifferenceError: When package diffs
+ cannot be requested.
+ """
+
class IDistroSeriesDifference(IDistroSeriesDifferencePublic,
IDistroSeriesDifferenceEdit):
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2010-10-07 15:03:24 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2010-10-07 15:03:39 +0000
@@ -33,7 +33,10 @@
DistroSeriesDifferenceStatus,
DistroSeriesDifferenceType,
)
-from lp.registry.exceptions import NotADerivedSeriesError
+from lp.registry.errors import (
+ DistroSeriesDifferenceError,
+ NotADerivedSeriesError,
+ )
from lp.registry.interfaces.distroseriesdifference import (
IDistroSeriesDifference,
IDistroSeriesDifferenceSource,
@@ -48,6 +51,7 @@
cachedproperty,
IPropertyCacheManager,
)
+from lp.soyuz.enums import PackageDiffStatus
class DistroSeriesDifference(Storm):
@@ -144,6 +148,17 @@
"""See `IDistroSeriesDifference`."""
return self._getLatestSourcePub(for_parent=True)
+ @cachedproperty
+ def base_source_pub(self):
+ """See `IDistroSeriesDifference`."""
+ if self.base_version is not None:
+ pubs = self.derived_series.getPublishedSources(
+ self.source_package_name, version=self.base_version,
+ include_pending=True)
+ return pubs[0]
+
+ return None
+
@property
def owner(self):
"""See `IDistroSeriesDifference`."""
@@ -163,6 +178,24 @@
'source_version': self.source_version,
})
+ def _getPackageDiffURL(self, package_diff):
+ """Check status and return URL if appropriate."""
+ if package_diff is None or (
+ package_diff.status != PackageDiffStatus.COMPLETED):
+ return None
+
+ return package_diff.diff_content.getURL()
+
+ @property
+ def package_diff_url(self):
+ """See `IDistroSeriesDifference`."""
+ return self._getPackageDiffURL(self.package_diff)
+
+ @property
+ def parent_package_diff_url(self):
+ """See `IDistroSeriesDifference`."""
+ return self._getPackageDiffURL(self.parent_package_diff)
+
def _getLatestSourcePub(self, for_parent=False):
"""Helper to keep source_pub/parent_source_pub DRY."""
distro_series = self.derived_series
@@ -308,3 +341,18 @@
"""See `IDistroSeriesDifference`."""
self.status = DistroSeriesDifferenceStatus.NEEDS_ATTENTION
self.update()
+
+ def requestPackageDiffs(self, requestor):
+ """See `IDistroSeriesDifference`."""
+ if (self.base_source_pub is None or self.source_pub is None or
+ self.parent_source_pub is None):
+ raise DistroSeriesDifferenceError(
+ "A derived, parent and base version are required to "
+ "generate package diffs.")
+ base_spr = self.base_source_pub.sourcepackagerelease
+ derived_spr = self.source_pub.sourcepackagerelease
+ parent_spr = self.parent_source_pub.sourcepackagerelease
+ self.package_diff = base_spr.requestDiffTo(
+ requestor, to_sourcepackagerelease=derived_spr)
+ self.parent_package_diff = base_spr.requestDiffTo(
+ requestor, to_sourcepackagerelease=parent_spr)
=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-10-07 15:03:24 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-10-07 15:03:39 +0000
@@ -13,20 +13,29 @@
from storm.store import Store
from zope.component import getUtility
from zope.security.interfaces import Unauthorized
+from zope.security.proxy import removeSecurityProxy
from canonical.launchpad.webapp.authorization import check_permission
from canonical.launchpad.webapp.testing import verifyObject
+<<<<<<< TREE
from canonical.testing.layers import DatabaseFunctionalLayer
+=======
+from canonical.testing import (
+ DatabaseFunctionalLayer,
+ LaunchpadFunctionalLayer,
+ )
+>>>>>>> MERGE-SOURCE
from lp.registry.enum import (
DistroSeriesDifferenceStatus,
DistroSeriesDifferenceType,
)
-from lp.registry.exceptions import NotADerivedSeriesError
+from lp.registry.errors import NotADerivedSeriesError
from lp.registry.interfaces.distroseriesdifference import (
IDistroSeriesDifference,
IDistroSeriesDifferenceSource,
)
from lp.services.propertycache import IPropertyCacheManager
+from lp.soyuz.enums import PackageDiffStatus
from lp.soyuz.interfaces.publishing import PackagePublishingStatus
from lp.testing import (
person_logged_in,
@@ -445,22 +454,11 @@
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',
- })
+ ds_diff = self.factory.makeDistroSeriesDifference(versions={
+ 'derived': '1.2',
+ 'parent': '1.3',
+ 'base': '1.0',
+ })
self.assertEqual('1.0', ds_diff.base_version)
@@ -487,6 +485,70 @@
self.assertEqual('1.1', ds_diff.base_version)
+ def test_base_source_pub_none(self):
+ # None is simply returned if there is no base version.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+
+ self.assertIs(None, ds_diff.base_version)
+ self.assertIs(None, ds_diff.base_source_pub)
+
+ def test_base_source_pub(self):
+ # The publishing in the derived series with base_version is returned.
+ ds_diff = self.factory.makeDistroSeriesDifference(versions={
+ 'derived': '1.2',
+ 'parent': '1.3',
+ 'base': '1.0',
+ })
+
+ base_pub = ds_diff.base_source_pub
+ self.assertEqual('1.0', base_pub.source_package_version)
+ self.assertEqual(ds_diff.derived_series, base_pub.distroseries)
+
+ def test_requestPackageDiffs(self):
+ # IPackageDiffs are created for the corresponding versions.
+ ds_diff = self.factory.makeDistroSeriesDifference(versions={
+ 'derived': '1.2',
+ 'parent': '1.3',
+ 'base': '1.0',
+ })
+
+ with person_logged_in(ds_diff.owner):
+ ds_diff.requestPackageDiffs(ds_diff.owner)
+
+ self.assertEqual(
+ '1.2', ds_diff.package_diff.to_source.version)
+ self.assertEqual(
+ '1.3', ds_diff.parent_package_diff.to_source.version)
+ self.assertEqual(
+ '1.0', ds_diff.package_diff.from_source.version)
+ self.assertEqual(
+ '1.0', ds_diff.parent_package_diff.from_source.version)
+
+ def test_package_diff_urls_none(self):
+ # URLs to the package diffs are only present when the diffs
+ # have been generated.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+
+ self.assertEqual(None, ds_diff.package_diff_url)
+ self.assertEqual(None, ds_diff.parent_package_diff_url)
+
+
+class DistroSeriesDifferenceLibrarianTestCase(TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+
+ def test_package_diff_urls(self):
+ # Only completed package diffs have urls.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+ naked_dsdiff = removeSecurityProxy(ds_diff)
+ naked_dsdiff.package_diff = self.factory.makePackageDiff(
+ status=PackageDiffStatus.PENDING)
+ naked_dsdiff.parent_package_diff = self.factory.makePackageDiff()
+
+ self.assertEqual(None, ds_diff.package_diff_url)
+ self.assertTrue(ds_diff.parent_package_diff_url.startswith(
+ 'http://localhost:58000/'))
+
class DistroSeriesDifferenceSourceTestCase(TestCaseWithFactory):
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-10-07 15:03:24 +0000
+++ lib/lp/testing/factory.py 2010-10-07 15:03:39 +0000
@@ -1915,6 +1915,15 @@
if versions is None:
versions = {}
+ base_version = versions.get('base')
+ if base_version:
+ for series in [derived_series, derived_series.parent_series]:
+ self.makeSourcePackagePublishingHistory(
+ distroseries=series,
+ version=base_version,
+ sourcepackagename=source_package_name,
+ status=PackagePublishingStatus.PUBLISHED)
+
if difference_type is not (
DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES):
