launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01141
[Merge] lp:~michael.nelson/launchpad/641293-expose-distroseriesdiff into lp:launchpad
Michael Nelson has proposed merging lp:~michael.nelson/launchpad/641293-expose-distroseriesdiff into lp:launchpad with lp:~michael.nelson/launchpad/635005-difference-details-2 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#641293 Expose DistroSeriesDifferences to API
https://bugs.launchpad.net/bugs/641293
#641725 odd use of feature flags
https://bugs.launchpad.net/bugs/641725
Overview
========
This branch exposes IDistorSeriesDifference via the API (bug 641293), and as a driveby, fixes the way we were using the feature flag in the view (bug 641725).
Details
=======
IDistroSeriesDifference.blacklist() is added and exported.
To test
=======
bin/test -vv -m test_distroseriesdifference -m test_series_views
Only lint is the expected complaints about lib/canonical/launchpad/interfaces/__init__.py
--
https://code.launchpad.net/~michael.nelson/launchpad/641293-expose-distroseriesdiff/+merge/36113
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~michael.nelson/launchpad/641293-expose-distroseriesdiff into lp:launchpad.
=== modified file 'lib/canonical/launchpad/interfaces/__init__.py'
--- lib/canonical/launchpad/interfaces/__init__.py 2010-09-11 19:25:13 +0000
+++ lib/canonical/launchpad/interfaces/__init__.py 2010-09-21 08:58:55 +0000
@@ -55,6 +55,7 @@
from lp.registry.interfaces.distribution import *
from lp.registry.interfaces.distributionmirror import *
from lp.registry.interfaces.distributionsourcepackage import *
+from lp.registry.interfaces.distroseriesdifference import *
from lp.soyuz.interfaces.distributionsourcepackagecache import *
from lp.soyuz.interfaces.distributionsourcepackagerelease import *
from lp.registry.interfaces.series import *
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2010-09-13 10:04:20 +0000
+++ lib/lp/registry/browser/distroseries.py 2010-09-21 08:58:55 +0000
@@ -74,7 +74,7 @@
from lp.registry.interfaces.distroseriesdifference import (
IDistroSeriesDifferenceSource)
from lp.registry.interfaces.series import SeriesStatus
-from lp.services.features.flags import FeatureController
+from lp.services.features import getFeatureFlag
from lp.services.propertycache import cachedproperty
from lp.services.worlddata.interfaces.country import ICountry
from lp.services.worlddata.interfaces.language import ILanguageSet
@@ -543,11 +543,7 @@
def initialize(self):
"""Redirect to the derived series if the feature is not enabled."""
- def in_scope(value):
- return True
-
- feature_controller = FeatureController(in_scope)
- if feature_controller.getFlag('soyuz.derived-series-ui.enabled') != 'on':
+ if not getFeatureFlag('soyuz.derived-series-ui.enabled'):
self.request.response.redirect(canonical_url(self.context))
return
super(DistroSeriesLocalDifferences, self).initialize()
@@ -564,6 +560,6 @@
@cachedproperty
def cached_differences(self):
"""Return a batch navigator of potentially filtered results."""
- differences = getUtility(IDistroSeriesDifferenceSource).getForDistroSeries(
- self.context)
+ utility = getUtility(IDistroSeriesDifferenceSource)
+ differences = utility.getForDistroSeries(self.context)
return BatchNavigator(differences, self.request)
=== added file 'lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 2010-09-21 08:58:55 +0000
@@ -0,0 +1,71 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import with_statement
+
+__metaclass__ = type
+
+import transaction
+
+from lazr.restfulclient.errors import Unauthorized
+from zope.component import getUtility
+
+from canonical.testing import AppServerLayer
+from canonical.launchpad.webapp.publisher import canonical_url
+from lp.registry.enum import DistroSeriesDifferenceStatus
+from lp.registry.interfaces.distroseriesdifference import (
+ IDistroSeriesDifferenceSource,
+ )
+from lp.testing import (
+ launchpadlib_for,
+ login_person,
+ TestCaseWithFactory,
+ ws_object,
+ )
+
+
+class DistroSeriesDifferenceWebServiceTestCase(TestCaseWithFactory):
+
+ layer = AppServerLayer
+
+ def makeLaunchpadService(self, person=None):
+ if person is None:
+ person = self.factory.makePerson()
+ launchpad = launchpadlib_for("test", person,
+ service_root="http://api.launchpad.dev:8085";)
+ login_person(person)
+ return launchpad
+
+ def test_get_difference(self):
+ # DistroSeriesDifferences are available on the web service.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+ ds_diff_path = canonical_url(ds_diff).replace(
+ 'http://launchpad.dev', '')
+
+ ws_diff = ws_object(self.makeLaunchpadService(), ds_diff)
+
+ self.assertTrue(
+ ws_diff.self_link.endswith(ds_diff_path))
+
+ def test_blacklist_not_public(self):
+ # The blacklist method is not publically available.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+ ws_diff = ws_object(self.makeLaunchpadService(), ds_diff)
+
+ self.assertRaises(Unauthorized, ws_diff.blacklist)
+
+ def test_blacklist(self):
+ # The blacklist method can be called by people with edit access.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+ ws_diff = ws_object(self.makeLaunchpadService(
+ ds_diff.derived_series.owner), ds_diff)
+
+ result = ws_diff.blacklist()
+ transaction.commit()
+
+ utility = getUtility(IDistroSeriesDifferenceSource)
+ ds_diff = utility.getByDistroSeriesAndName(
+ ds_diff.derived_series, ds_diff.source_package_name.name)
+ self.assertEqual(
+ DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
+ ds_diff.status)
=== modified file 'lib/lp/registry/browser/tests/test_series_views.py'
--- lib/lp/registry/browser/tests/test_series_views.py 2010-09-21 08:58:51 +0000
+++ lib/lp/registry/browser/tests/test_series_views.py 2010-09-21 08:58:55 +0000
@@ -20,7 +20,14 @@
DistroSeriesDifferenceType,
)
from lp.services.features.flags import FeatureController
-from lp.services.features.model import FeatureFlag, getFeatureStore
+from lp.services.features.model import (
+ FeatureFlag,
+ getFeatureStore,
+ )
+from lp.services.features import (
+ getFeatureFlag,
+ per_thread,
+ )
from lp.testing import TestCaseWithFactory
from lp.testing.views import create_initialized_view
@@ -66,19 +73,31 @@
name=derived_name, parent_series=parent)
return derived_series
+ def makeControllerInScopes(self, scopes):
+ """Make a controller that will report it's in the given scopes."""
+ call_log = []
+
+ def scope_cb(scope):
+ call_log.append(scope)
+ return scope in scopes
+ return FeatureController(scope_cb), call_log
+
def setDerivedSeriesUIFeatureFlag(self):
# Helper to set the feature flag enabling the derived series ui.
ignore = getFeatureStore().add(FeatureFlag(
scope=u'default', flag=u'soyuz.derived-series-ui.enabled',
value=u'on', priority=1))
- def getDerivedSeriesUIFeatureFlag(self, flag):
- """Helper to return the given flag leaving tests more readable."""
+ # XXX Michael Nelson 2010-09-21 bug=631884
+ # Currently LaunchpadTestRequest doesn't set per-thread
+ # features.
def in_scope(value):
return True
+ per_thread.features = FeatureController(in_scope)
- feature_controller = FeatureController(in_scope)
- return feature_controller.getFlag(flag)
+ def reset_per_thread_features():
+ per_thread.features = None
+ self.addCleanup(reset_per_thread_features)
def test_view_redirects_without_feature_flag(self):
# If the feature flag soyuz.derived-series-ui.enabled is not set the
@@ -87,8 +106,7 @@
parent_name='lucid', derived_name='derilucid')
self.assertIs(
- None, self.getDerivedSeriesUIFeatureFlag(
- 'soyuz.derived-series-ui.enabled'))
+ None, getFeatureFlag('soyuz.derived-series-ui.enabled'))
view = create_initialized_view(
derived_series, '+localpackagediffs')
=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py 2010-09-13 10:18:16 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2010-09-21 08:58:55 +0000
@@ -13,6 +13,11 @@
'IDistroSeriesDifferenceSource',
]
+from lazr.restful.declarations import (
+ export_as_webservice_entry,
+ export_write_operation,
+ exported,
+ )
from lazr.restful.fields import Reference
from zope.interface import Interface
from zope.schema import (
@@ -39,11 +44,11 @@
id = Int(title=_('ID'), required=True, readonly=True)
- derived_series = Reference(
+ derived_series = exported(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."))
+ "identifies the two series with the difference.")))
source_package_name = Reference(
ISourcePackageName,
@@ -130,10 +135,19 @@
def addComment(owner, comment):
"""Add a comment on this difference."""
+ @export_write_operation()
+ def blacklist(all=False):
+ """Blacklist this version or all versions of this source package.
+
+ :param all: indicates whether all versions of this package should
+ be blacklisted or just the current (default).
+ """
+
class IDistroSeriesDifference(IDistroSeriesDifferencePublic,
IDistroSeriesDifferenceEdit):
"""An interface for a package difference between two distroseries."""
+ export_as_webservice_entry()
class IDistroSeriesDifferenceSource(Interface):
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2010-09-13 10:18:16 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2010-09-21 08:58:55 +0000
@@ -183,12 +183,12 @@
def update(self):
"""See `IDistroSeriesDifference`."""
- # Updating is expected to be a heavy operation (not called during
- # requests). We clear the cache beforehand - even though
+ # Updating is expected to be a heavy operation (not called
+ # during requests). We clear the cache beforehand - even though
# it is not currently be necessary so that in the future it
- # won't cause a hard-to find bug if a script ever creates a difference,
- # copies/publishes a new version and then calls update() (like the
- # tests for this method do).
+ # won't cause a hard-to find bug if a script ever creates a
+ # difference, copies/publishes a new version and then calls
+ # update() (like the tests for this method do).
IPropertyCacheManager(self).clear()
self._updateType()
updated = self._updateVersionsAndStatus()
@@ -265,3 +265,10 @@
DistroSeriesDifferenceComment,
DSDComment.distro_series_difference == self)
return comments.order_by(Desc(DSDComment.id))
+
+ def blacklist(self, all=False):
+ """See `IDistroSeriesDifference`."""
+ if all:
+ self.status = DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS
+ else:
+ self.status = DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT
=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-09-13 10:18:16 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-09-21 08:58:55 +0000
@@ -337,6 +337,38 @@
diff_comment = ds_diff.addComment(
ds_diff.derived_series.owner, "Boo")
+ def test_blacklist_not_public(self):
+ # Differences cannot be blacklisted without edit access.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+ person = self.factory.makePerson()
+
+ with person_logged_in(person):
+ self.assertTrue(check_permission('launchpad.View', ds_diff))
+ self.assertFalse(check_permission('launchpad.Edit', ds_diff))
+ self.assertRaises(Unauthorized, getattr, ds_diff, 'blacklist')
+
+ def test_blacklist_default(self):
+ # By default the current version is blacklisted.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+
+ with person_logged_in(ds_diff.owner):
+ ds_diff.blacklist()
+
+ self.assertEqual(
+ DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
+ ds_diff.status)
+
+ def test_blacklist_all(self):
+ # All versions are blacklisted with the all=True param.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+
+ with person_logged_in(ds_diff.owner):
+ ds_diff.blacklist(all=True)
+
+ self.assertEqual(
+ DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS,
+ ds_diff.status)
+
def test_source_package_name_unique_for_derived_series(self):
# We cannot create two differences for the same derived series
# for the same package.
