launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00918
[Merge] lp:~benji/launchpad/bug-413174 into lp:launchpad/devel
Benji York has proposed merging lp:~benji/launchpad/bug-413174 into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Often times functions have preconditions that must be met for the
functions to operate correctly. When users interact with those
functions via the web interface the precondition checks are often done
by view code. When called via the web service those functions instead
generate exceptions when called without the preconditions being met.
Previously those exceptions would be recorded as an OOPS and propagated
to the client as HTTP 500 errors with unhelpful response bodies.
This branch fixes bug 413174 by taking advantage of the
lazr.restful.error.expose function that signals lazr.restful to generate
an HTTP 400 error signaling a client error and causes the web service to
generate a response with the error message from the raised exception.
This also means that these client-induced errors will no longer generate
an OOPS.
In the future similar OOPSes that arrise can be given the same
treatment.
The tests added to verify the nice-error-message, non-OOPS producing
effects of this branch are in the TestLaunchpadlibAPI test cases in
lib/lp/registry/tests/test_project.py and TestDuplicateProductReleases
in lib/lp/registry/tests/test_project_milestone.py and can be run with
bin/test -t TestLaunchpadlibAPI -t TestDuplicateProductReleases
A small bit of whitespace lint was cleaned up in this branch.
--
https://code.launchpad.net/~benji/launchpad/bug-413174/+merge/34794
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/bug-413174 into lp:launchpad/devel.
=== modified file 'lib/lp/registry/model/milestone.py'
--- lib/lp/registry/model/milestone.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/model/milestone.py 2010-09-07 20:12:48 +0000
@@ -36,6 +36,7 @@
sqlvalues,
)
from canonical.launchpad.webapp.sorting import expand_numbers
+from lazr.restful.error import expose
from lp.app.errors import NotFoundError
from lp.blueprints.model.specification import Specification
from lp.bugs.interfaces.bugtarget import IHasBugs
@@ -109,6 +110,13 @@
return sorted(result, key=milestone_sort_key, reverse=True)
+class MultipleProductReleases(Exception):
+ """Raised when a second ProductRelease is created for a milestone."""
+
+ def __init__(self, msg='A milestone can only have one ProductRelease.'):
+ super(MultipleProductReleases, self).__init__(msg)
+
+
class Milestone(SQLBase, StructuralSubscriptionTargetMixin, HasBugsBase):
implements(IHasBugs, IMilestone)
@@ -194,8 +202,7 @@
changelog=None, release_notes=None):
"""See `IMilestone`."""
if self.product_release is not None:
- raise AssertionError(
- 'A milestone can only have one ProductRelease.')
+ raise expose(MultipleProductReleases())
release = ProductRelease(
owner=owner,
changelog=changelog,
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2010-09-03 11:05:21 +0000
+++ lib/lp/registry/model/product.py 2010-09-07 20:12:48 +0000
@@ -17,6 +17,7 @@
import operator
from lazr.delegates import delegates
+from lazr.restful.error import expose
import pytz
from sqlobject import (
BoolCol,
@@ -271,6 +272,13 @@
tables=[ProductLicense]))
+class UnDeactivateable(Exception):
+ """Raised when a project is requested to deactivate but can not."""
+
+ def __init__(self, msg):
+ super(UnDeactivateable, self).__init__(msg)
+
+
class Product(SQLBase, BugTargetBase, MakesAnnouncements,
HasSpecificationsMixin, HasSprintsMixin,
KarmaContextMixin, BranchVisibilityPolicyMixin,
@@ -440,9 +448,9 @@
# Validate deactivation.
if self.active == True and value == False:
if len(self.sourcepackages) > 0:
- raise AssertionError(
+ raise expose(UnDeactivateable(
'This project cannot be deactivated since it is '
- 'linked to source packages.')
+ 'linked to source packages.'))
return value
active = BoolCol(dbName='active', notNull=True, default=True,
=== modified file 'lib/lp/registry/tests/test_milestone.py'
--- lib/lp/registry/tests/test_milestone.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/tests/test_milestone.py 2010-09-07 20:12:48 +0000
@@ -15,6 +15,7 @@
logout,
)
from canonical.testing import LaunchpadFunctionalLayer
+
from lp.app.errors import NotFoundError
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.milestone import IMilestoneSet
@@ -79,6 +80,7 @@
all_visible_milestones_ids,
[1, 2, 3])
+
def test_suite():
"""Return the test suite for the tests in this module."""
return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py 2010-08-25 20:04:40 +0000
+++ lib/lp/registry/tests/test_product.py 2010-09-07 20:12:48 +0000
@@ -28,7 +28,7 @@
License,
)
from lp.registry.model.commercialsubscription import CommercialSubscription
-from lp.registry.model.product import Product
+from lp.registry.model.product import Product, UnDeactivateable
from lp.registry.model.productlicense import ProductLicense
from lp.testing import TestCaseWithFactory
@@ -48,7 +48,7 @@
source_package.setPackaging(
product.development_focus, self.factory.makePerson())
self.assertRaises(
- AssertionError,
+ UnDeactivateable,
setattr, product, 'active', False)
def test_deactivation_success(self):
=== modified file 'lib/lp/registry/tests/test_project.py'
--- lib/lp/registry/tests/test_project.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/tests/test_project.py 2010-09-07 20:12:48 +0000
@@ -6,11 +6,22 @@
import unittest
from zope.component import getUtility
+from lazr.restfulclient.errors import ClientError
+from canonical.launchpad.testing.pages import LaunchpadWebServiceCaller
from canonical.launchpad.ftests import login
-from canonical.testing import LaunchpadFunctionalLayer
+from canonical.launchpad.webapp.errorlog import globalErrorUtility
+from canonical.testing import (
+ LaunchpadFunctionalLayer,
+ DatabaseFunctionalLayer,
+ )
from lp.registry.interfaces.projectgroup import IProjectGroupSet
-from lp.testing import TestCaseWithFactory
+from lp.soyuz.enums import ArchivePurpose
+from lp.testing import (
+ celebrity_logged_in,
+ launchpadlib_for,
+ TestCaseWithFactory,
+ )
class ProjectGroupSearchTestCase(TestCaseWithFactory):
@@ -99,5 +110,31 @@
self.assertEqual(self.project3, results[0])
+def get_last_oops_id():
+ return getattr(globalErrorUtility.getLastOopsReport(), 'id', None)
+
+
+class TestLaunchpadlibAPI(TestCaseWithFactory):
+ layer = DatabaseFunctionalLayer
+
+ def test_inappropriate_deactivation_does_not_cause_an_OOPS(self):
+ # Make sure a 400 error and not an OOPS is returned when a ValueError
+ # is raised when trying to deactivate a project that has source
+ # releases.
+ last_oops = get_last_oops_id()
+ launchpad = launchpadlib_for("test", "salgado", "WRITE_PUBLIC")
+
+ project = launchpad.projects['evolution']
+ project.active = False
+ e = self.assertRaises(ClientError, project.lp_save)
+
+ # no OOPS was generated as a result of the exception
+ self.assertEqual(get_last_oops_id(), last_oops)
+ self.assertEqual(400, e.response.status)
+ self.assertIn(
+ 'This project cannot be deactivated since it is linked to source '
+ 'packages.', e.content)
+
+
def test_suite():
return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/registry/tests/test_project_milestone.py'
--- lib/lp/registry/tests/test_project_milestone.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/tests/test_project_milestone.py 2010-09-07 20:12:48 +0000
@@ -10,12 +10,19 @@
from storm.store import Store
from zope.component import getUtility
+import pytz
from canonical.launchpad.ftests import (
login,
syncUpdate,
)
-from canonical.testing import LaunchpadFunctionalLayer
+from canonical.launchpad.webapp.errorlog import globalErrorUtility
+from canonical.testing import (
+ LaunchpadFunctionalLayer,
+ DatabaseFunctionalLayer,
+ )
+from lazr.restfulclient.errors import ClientError
+
from lp.blueprints.interfaces.specification import (
ISpecificationSet,
SpecificationDefinitionStatus,
@@ -30,6 +37,11 @@
from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.product import IProductSet
from lp.registry.interfaces.projectgroup import IProjectGroupSet
+from lp.registry.model.milestone import MultipleProductReleases
+from lp.testing import (
+ launchpadlib_for,
+ TestCaseWithFactory,
+ )
class ProjectMilestoneTest(unittest.TestCase):
@@ -197,7 +209,7 @@
spec = specset.new(
name='%s-specification' % product_name,
title='Title %s specification' % product_name,
- specurl='http://www.example.com/spec/%s' %product_name ,
+ specurl='http://www.example.com/spec/%s' % product_name,
summary='summary',
definition_status=SpecificationDefinitionStatus.APPROVED,
priority=SpecificationPriority.HIGH,
@@ -317,6 +329,52 @@
self._createProductSeriesBugtask('evolution', 'trunk', '1.1')
+def get_last_oops_id():
+ return getattr(globalErrorUtility.getLastOopsReport(), 'id', None)
+
+
+class TestDuplicateProductReleases(TestCaseWithFactory):
+ layer = DatabaseFunctionalLayer
+
+ def test_inappropriate_release_raises(self):
+ # A milestone that already has a ProductRelease can not be given
+ # another one.
+ login('foo.bar@xxxxxxxxxxxxx')
+ product_set = getUtility(IProductSet)
+ product = product_set['evolution']
+ series = product.getSeries('trunk')
+ milestone = series.newMilestone(name='1.1', dateexpected=None)
+ now = datetime.now(pytz.UTC)
+ milestone.createProductRelease(1, now)
+ self.assertRaises(MultipleProductReleases,
+ milestone.createProductRelease, 1, now)
+ try:
+ milestone.createProductRelease(1, now)
+ except MultipleProductReleases, e:
+ self.assert_(
+ str(e), 'A milestone can only have one ProductRelease.')
+
+ def test_inappropriate_deactivation_does_not_cause_an_OOPS(self):
+ # Make sure a 400 error and not an OOPS is returned when an exception
+ # is raised when trying to create a product release when a milestone
+ # already has one.
+ last_oops = get_last_oops_id()
+ launchpad = launchpadlib_for("test", "salgado", "WRITE_PUBLIC")
+
+ project = launchpad.projects['evolution']
+ milestone = project.getMilestone(name='2.1.6')
+ now = datetime.now(pytz.UTC)
+
+ e = self.assertRaises(
+ ClientError, milestone.createProductRelease, date_released=now)
+
+ # no OOPS was generated as a result of the exception
+ self.assertEqual(get_last_oops_id(), last_oops)
+ self.assertEqual(400, e.response.status)
+ self.assertIn(
+ 'A milestone can only have one ProductRelease.', e.content)
+
+
def test_suite():
"""Return the test suite for the tests in this module."""
return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'versions.cfg'
--- versions.cfg 2010-09-06 18:23:01 +0000
+++ versions.cfg 2010-09-07 20:12:48 +0000
@@ -31,7 +31,7 @@
lazr.delegates = 1.2.0
lazr.enum = 1.1.2
lazr.lifecycle = 1.1
-lazr.restful = 0.11.3
+lazr.restful = 0.13.0
lazr.restfulclient = 0.10.0
lazr.smtptest = 1.1
lazr.testing = 0.1.1
