launchpad-reviewers team mailing list archive

[Abel Deuring <abel.deuring@xxxxxxxxxxxxx>]

Abel Deuring has proposed merging lp:~adeuring/launchpad/security-guarded-test-object-factory-3 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #609982 add security proxies to the object returned by the methods makeBranchMergeProposal(), makeCodeReviewVoteReference(), makeBinaryPackagePublishingHistory()makeDistributionSourcePackage() of LaunchpadObjectfactory
  https://bugs.launchpad.net/bugs/609982


this branch is another sequel to ensure that objects returned by methods of LPObjectFactory are wrapped in security proxies. It changes the methods makeBranchMergeProposal(), makeCodeReviewVoteReference(), makeBinaryPackagePublishingHistory(), makeDistributionSourcePackage() and affected tests.

See also the branches lp:~adeuring/launchpad/security-guarded-test-object-factory-1 and ~adeuring/launchpad/security-guarded-test-object-factory-2 and bug 607236
-- 
https://code.launchpad.net/~adeuring/launchpad/security-guarded-test-object-factory-3/+merge/30917
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/security-guarded-test-object-factory-3 into lp:launchpad/devel.

=== modified file 'lib/lp/bugs/doc/bug-heat.txt'
--- lib/lp/bugs/doc/bug-heat.txt	2010-06-22 16:08:05 +0000
+++ lib/lp/bugs/doc/bug-heat.txt	2010-07-26 08:41:50 +0000
@@ -311,15 +311,18 @@
 A DistributionSourcePackage also has a cached value for bug_count and
 total_bug_heat.
 
-    >>> print dsp.bug_count
+    >>> from lp.testing.factory import (
+    ...     remove_security_proxy_and_shout_at_engineer)
+    >>> naked_dsp = remove_security_proxy_and_shout_at_engineer(dsp)
+    >>> print naked_dsp.bug_count
     1
-    >>> print dsp.total_bug_heat
+    >>> print naked_dsp.total_bug_heat
     123
     >>> dsp_task2 = factory.makeBugTask(target=dsp)
     >>> dsp_task2.bug.setHeat(7)
-    >>> print dsp.bug_count
+    >>> print naked_dsp.bug_count
     2
-    >>> print dsp.total_bug_heat
+    >>> print naked_dsp.total_bug_heat
     130
 
 Transitioning from one target to another, calculates the value for the new

=== modified file 'lib/lp/bugs/tests/test_bugheat.py'
--- lib/lp/bugs/tests/test_bugheat.py	2010-07-18 00:55:24 +0000
+++ lib/lp/bugs/tests/test_bugheat.py	2010-07-26 08:41:50 +0000
@@ -12,7 +12,8 @@
 from canonical.testing import LaunchpadZopelessLayer
 
 from lp.testing import TestCaseWithFactory
-from lp.testing.factory import LaunchpadObjectFactory
+from lp.testing.factory import (
+    LaunchpadObjectFactory, remove_security_proxy_and_shout_at_engineer)
 
 
 class MaxHeatByTargetBase:
@@ -75,10 +76,14 @@
         self.assertEqual(None, self.target.max_bug_heat)
 
     def test_null_total_bug_heat(self):
-        self.assertEqual(None, self.target.total_bug_heat)
+        naked_target = remove_security_proxy_and_shout_at_engineer(
+            self.target)
+        self.assertEqual(None, naked_target.total_bug_heat)
 
     def test_null_bug_count(self):
-        self.assertEqual(None, self.target.bug_count)
+        naked_target = remove_security_proxy_and_shout_at_engineer(
+            self.target)
+        self.assertEqual(None, naked_target.bug_count)
 
 
 class DistributionSourcePackageZeroRecalculateBugHeatCacheTest(
@@ -96,10 +101,14 @@
         self.assertEqual(0, self.target.max_bug_heat)
 
     def test_zero_total_bug_heat(self):
-        self.assertEqual(0, self.target.total_bug_heat)
+        naked_target = remove_security_proxy_and_shout_at_engineer(
+            self.target)
+        self.assertEqual(0, naked_target.total_bug_heat)
 
     def test_zero_bug_count(self):
-        self.assertEqual(0, self.target.bug_count)
+        naked_target = remove_security_proxy_and_shout_at_engineer(
+            self.target)
+        self.assertEqual(0, naked_target.bug_count)
 
 
 class DistributionSourcePackageMultipleBugsRecalculateBugHeatCacheTest(
@@ -131,14 +140,18 @@
         self.assertEqual(self.max_heat, self.target.max_bug_heat)
 
     def test_total_bug_heat(self):
-        self.assertEqual(self.total_heat, self.target.total_bug_heat)
+        naked_target = remove_security_proxy_and_shout_at_engineer(
+            self.target)
+        self.assertEqual(self.total_heat, naked_target.total_bug_heat)
         self.failUnless(
-            self.target.total_bug_heat > self.target.max_bug_heat,
+            naked_target.total_bug_heat > naked_target.max_bug_heat,
             "Total bug heat should be more than the max bug heat, "
             "since we know that multiple bugs have nonzero heat.")
 
     def test_bug_count(self):
-        self.assertEqual(2, self.target.bug_count)
+        naked_target = remove_security_proxy_and_shout_at_engineer(
+            self.target)
+        self.assertEqual(2, naked_target.bug_count)
 
 
 class SourcePackageMaxHeatByTargetTest(

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py	2010-03-11 20:54:36 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py	2010-07-26 08:41:50 +0000
@@ -28,6 +28,7 @@
 from lp.code.tests.helpers import add_revision_to_branch
 from lp.testing import (
     login_person, TestCaseWithFactory, time_counter)
+from lp.testing.factory import remove_security_proxy_and_shout_at_engineer
 from lp.testing.views import create_initialized_view
 from lp.code.model.diff import PreviewDiff, StaticDiff
 from canonical.launchpad.database.message import MessageSet
@@ -78,7 +79,8 @@
         """It should be possible to review an unmergeable proposal."""
         request = self.factory.makeCodeReviewVoteReference()
         bmp = request.branch_merge_proposal
-        bmp.rejectBranch(bmp.target_branch.owner, 'foo')
+        naked_bmp = remove_security_proxy_and_shout_at_engineer(bmp)
+        naked_bmp.rejectBranch(bmp.target_branch.owner, 'foo')
         d = DecoratedCodeReviewVoteReference(request, request.reviewer, None)
         self.assertTrue(d.user_can_review)
         self.assertTrue(d.can_change_review)
@@ -752,7 +754,6 @@
 
     layer = LaunchpadFunctionalLayer
 
-
     def _makeCommentFromEmailWithAttachment(self, attachment_body):
         # Make an email message with an attachment, and create a code
         # review comment from it.

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-07-26 06:21:45 +0000
+++ lib/lp/testing/factory.py	2010-07-26 08:41:50 +0000
@@ -1026,7 +1026,7 @@
         else:
             raise AssertionError('Unknown status: %s' % set_state)
 
-        return proposal
+        return ProxyFactory(proposal)
 
     def makeBranchSubscription(self, branch=None, person=None,
                                subscribed_by=None):
@@ -1586,7 +1586,7 @@
     def makeCodeReviewVoteReference(self):
         bmp = removeSecurityProxy(self.makeBranchMergeProposal())
         candidate = self.makePerson()
-        return bmp.nominateReviewer(candidate, bmp.registrant)
+        return ProxyFactory(bmp.nominateReviewer(candidate, bmp.registrant))
 
     def makeMessage(self, subject=None, content=None, parent=None,
                     owner=None):
@@ -2466,7 +2466,7 @@
             section_name=section_name,
             priority=priority)
 
-        return BinaryPackagePublishingHistory(
+        return ProxyFactory(BinaryPackagePublishingHistory(
             distroarchseries=distroarchseries,
             binarypackagerelease=bpr,
             component=bpr.component,
@@ -2476,7 +2476,7 @@
             scheduleddeletiondate=scheduleddeletiondate,
             pocket=pocket,
             priority=priority,
-            archive=archive)
+            archive=archive))
 
     def makeBinaryPackageName(self, name=None):
         if name is None:
@@ -2565,7 +2565,8 @@
         if distribution is None:
             distribution = self.makeDistribution()
 
-        return DistributionSourcePackage(distribution, sourcepackagename)
+        return ProxyFactory(
+            DistributionSourcePackage(distribution, sourcepackagename))
 
     def makeEmailMessage(self, body=None, sender=None, to=None,
                          attachments=None, encode_attachments=False):