Merge lp:~gmb/launchpad/add-calculatebugheatjobs-bug-510876 into lp:launchpad/db-devel

[Graham Binns]

Relevant changes to a bug will now add CalculateBugHeatJobs to the job queue.

[Graham Binns (gmb)]

This branch updates the Bug model so that changes to a Bug add
CalculateBugHeatJobs to the job queue.

We've made changes in the following files:

=== lib/lp/bugs/configure.zcml ===

 - We've added IBugJob as an allowed interface for CalculateBugHeatJob
   so that we can access it as a BugJob.

=== lib/lp/bugs/interfaces/bugjob.py ===

 - We've fixed some lint by adding IBugJobSource to the modules __all__.

=== lib/lp/bugs/model/bug.py ===

 - We've added job creation code for the following events:
   - Subscriptions and unsubscriptions
   - Marking a user as affected or unaffected
   - Marking or unmarking a bug as a duplicate
   - Generic changes to the bug's attributes (this includes
     security_related and private, and uses the BugChange API, which is
     tested elsewhere).

=== lib/lp/bugs/scripts/bugheat.py ===

 - We've removed some unused imports.

=== lib/lp/bugs/tests/test_bugheat.py ===

 - We've added unit tests for the above event-driven job creation code.

[Deryck Hodge (deryck)]

Looks good to me. As mentioned in person, I would remove the comments that say something to the effect of "updating bug heat" since the method object makes this obvious. Otherwise, r=me.

Cheers,
deryck

review: Approve (code)

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-01-29 17:15:31 +0000
+++ database/schema/security.cfg	2010-02-01 08:45:27 +0000
@@ -546,6 +546,7 @@
 public.bugactivity                      = SELECT, INSERT
 public.bugaffectsperson                 = SELECT, INSERT, UPDATE, DELETE
 public.bugcve                           = SELECT, INSERT
+public.bugjob                           = SELECT, INSERT
 public.bugmessage                       = SELECT, INSERT, UPDATE
 public.bugnomination                    = SELECT
 public.bugnotification                  = SELECT, INSERT
@@ -561,6 +562,7 @@
 public.distribution                     = SELECT
 public.distroseries                     = SELECT
 public.emailaddress                     = SELECT, INSERT
+public.job                              = SELECT, INSERT, UPDATE
 public.language                         = SELECT
 public.libraryfilealias                 = SELECT, INSERT
 public.libraryfilecontent               = SELECT, INSERT
@@ -925,6 +927,7 @@
 public.bug                              = SELECT, INSERT, UPDATE
 public.bugactivity                      = SELECT, INSERT, UPDATE
 public.bugattachment                    = SELECT, INSERT, UPDATE
+public.bugjob                           = SELECT, INSERT
 public.bugmessage                       = SELECT, INSERT, UPDATE
 public.bugnomination                    = SELECT, INSERT, UPDATE
 public.bugpackageinfestation            = SELECT, INSERT, UPDATE
@@ -1147,6 +1150,7 @@
 public.bug                              = SELECT, UPDATE
 public.bugactivity                      = SELECT, INSERT
 public.bugaffectsperson                 = SELECT, INSERT, UPDATE, DELETE
+public.bugjob                           = SELECT, INSERT
 public.bugsubscription                  = SELECT
 public.bugnotification                  = SELECT, INSERT
 public.bugnotificationrecipient         = SELECT, INSERT
@@ -1246,6 +1250,7 @@
 public.bug                              = SELECT, UPDATE
 public.bugactivity                      = SELECT, INSERT
 public.bugaffectsperson                 = SELECT, INSERT, UPDATE, DELETE
+public.bugjob                           = SELECT, INSERT
 public.bugsubscription                  = SELECT
 public.bugnotification                  = SELECT, INSERT
 public.bugnotificationrecipient         = SELECT, INSERT
@@ -1315,10 +1320,12 @@
 public.bug                              = SELECT, INSERT, UPDATE
 public.bugactivity                      = SELECT, INSERT
 public.bugaffectsperson                 = SELECT, INSERT, UPDATE, DELETE
+public.bugjob                           = SELECT, INSERT
 public.bugmessage                       = SELECT, INSERT
 public.bugtag                           = SELECT
 public.bugtask                          = SELECT, INSERT, UPDATE
 public.bugwatch                         = SELECT
+public.job                              = SELECT, INSERT, UPDATE
 public.component                        = SELECT
 public.packagebugsupervisor             = SELECT
 public.person                           = SELECT
@@ -1515,6 +1522,7 @@
 public.bug                              = SELECT, INSERT, UPDATE
 public.bugactivity                      = SELECT, INSERT
 public.bugaffectsperson                 = SELECT, INSERT, UPDATE, DELETE
+public.bugjob                           = SELECT, INSERT
 public.bugsubscription                  = SELECT, INSERT
 public.bugnotification                  = SELECT, INSERT
 public.bugnotificationattachment        = SELECT
@@ -1783,6 +1791,7 @@
 public.bugtracker                       = SELECT, INSERT
 public.bugwatch                         = SELECT, INSERT
 public.bug                              = SELECT, INSERT, UPDATE
+public.bugjob                           = SELECT, INSERT
 public.bugaffectsperson                 = SELECT, INSERT, UPDATE, DELETE
 public.bugtask                          = SELECT, INSERT, UPDATE
 public.accountpassword                  = SELECT, INSERT
@@ -1796,6 +1805,7 @@
 public.bugsubscription                  = SELECT, INSERT
 public.bugmessage                       = SELECT, INSERT
 public.sourcepackagename                = SELECT
+public.job                              = SELECT, INSERT, UPDATE
 
 [updatesourceforgeremoteproduct]
 # Updates Product.remote_product using SourceForge project data.

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2010-01-24 12:40:12 +0000
+++ lib/lp/bugs/configure.zcml	2010-02-01 08:45:27 +0000
@@ -903,6 +903,7 @@
 
     <!-- CalculateBugHeatJobs -->
     <class class="lp.bugs.model.bugheat.CalculateBugHeatJob">
+        <allow interface="lp.bugs.interfaces.bugjob.IBugJob" />
         <allow interface="lp.bugs.interfaces.bugjob.ICalculateBugHeatJob"/>
     </class>
     <securedutility

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2010-01-25 11:41:18 +0000
+++ lib/lp/bugs/model/bug.py	2010-02-01 08:45:27 +0000
@@ -69,6 +69,7 @@
 from lp.bugs.interfaces.bugactivity import IBugActivitySet
 from lp.bugs.interfaces.bugattachment import (
     BugAttachmentType, IBugAttachmentSet)
+from lp.bugs.interfaces.bugjob import ICalculateBugHeatJobSource
 from lp.bugs.interfaces.bugmessage import IBugMessageSet
 from lp.bugs.interfaces.bugnomination import (
     NominationError, NominationSeriesObsoleteError)
@@ -464,6 +465,9 @@
 
         sub = BugSubscription(
             bug=self, person=person, subscribed_by=subscribed_by)
+
+        getUtility(ICalculateBugHeatJobSource).create(self)
+
         # Ensure that the subscription has been flushed.
         Store.of(sub).flush()
         return sub
@@ -771,6 +775,8 @@
                 notification_data['text'], change.person, recipients,
                 when)
 
+        getUtility(ICalculateBugHeatJobSource).create(self)
+
     def expireNotifications(self):
         """See `IBug`."""
         for notification in BugNotification.selectBy(
@@ -1393,11 +1399,14 @@
             if bap.affected != affected:
                 bap.affected = affected
                 self._flushAndInvalidate()
+
         # Loop over dupes.
         for dupe in self.duplicates:
             if dupe._getAffectedUser(user) is not None:
                 dupe.markUserAffected(user, affected)
 
+        getUtility(ICalculateBugHeatJobSource).create(self)
+
     @property
     def readonly_duplicateof(self):
         """See `IBug`."""
@@ -1414,6 +1423,17 @@
         except LaunchpadValidationError, validation_error:
             raise InvalidDuplicateValue(validation_error)
 
+        if duplicate_of is not None:
+            # Create a job to update the heat of the master bug and set
+            # this bug's heat to 0 (since it's a duplicate, it shouldn't
+            # have any heat at all).
+            getUtility(ICalculateBugHeatJobSource).create(duplicate_of)
+            self.setHeat(0)
+        else:
+            # Otherwise, create a job to recalculate this bug's heat,
+            # since it will be 0 from having been a duplicate.
+            getUtility(ICalculateBugHeatJobSource).create(self)
+
     def setCommentVisibility(self, user, comment_number, visible):
         """See `IBug`."""
         bug_message_set = getUtility(IBugMessageSet)

=== modified file 'lib/lp/bugs/scripts/bugheat.py'
--- lib/lp/bugs/scripts/bugheat.py	2010-01-21 16:56:10 +0000
+++ lib/lp/bugs/scripts/bugheat.py	2010-02-01 08:45:27 +0000
@@ -9,13 +9,6 @@
     ]
 
 
-from zope.component import getUtility
-from zope.interface import implements
-
-from canonical.launchpad.interfaces.looptuner import ITunableLoop
-from canonical.launchpad.utilities.looptuner import DBLoopTuner
-
-
 class BugHeatConstants:
 
     PRIVACY = 150

=== modified file 'lib/lp/bugs/tests/test_bugheat.py'
--- lib/lp/bugs/tests/test_bugheat.py	2010-01-21 17:47:38 +0000
+++ lib/lp/bugs/tests/test_bugheat.py	2010-02-01 08:45:27 +0000
@@ -5,14 +5,17 @@
 
 __metaclass__ = type
 
+import pytz
 import transaction
 import unittest
+from datetime import datetime
 
 from zope.component import getUtility
 
 from canonical.launchpad.scripts.tests import run_script
 from canonical.testing import LaunchpadZopelessLayer
 
+from lp.bugs.adapters.bugchange import BugDescriptionChange
 from lp.bugs.interfaces.bugjob import ICalculateBugHeatJobSource
 from lp.bugs.model.bugheat import CalculateBugHeatJob
 from lp.bugs.scripts.bugheat import BugHeatCalculator
@@ -28,6 +31,26 @@
         super(CalculateBugHeatJobTestCase, self).setUp()
         self.bug = self.factory.makeBug()
 
+        # NB: This looks like it should go in the teardown, however
+        # creating the bug causes a job to be added for it. We clear
+        # this out so that our tests are consistent.
+        self._completeJobsAndAssertQueueEmpty()
+
+    def _completeJobsAndAssertQueueEmpty(self):
+        """Make sure that all the CalculateBugHeatJobs are completed."""
+        for bug_job in getUtility(ICalculateBugHeatJobSource).iterReady():
+            bug_job.job.start()
+            bug_job.job.complete()
+        self.assertEqual(0, self._getJobCount())
+
+    def _getJobCount(self):
+        """Return the number of CalculateBugHeatJobs in the queue."""
+        return len(self._getJobs())
+
+    def _getJobs(self):
+        """Return the pending CalculateBugHeatJobs as a list."""
+        return list(CalculateBugHeatJob.iterReady())
+
     def test_run(self):
         # CalculateBugHeatJob.run() sets calculates and sets the heat
         # for a bug.
@@ -51,8 +74,7 @@
         job = CalculateBugHeatJob.create(self.bug)
 
         # There will now be one job in the queue.
-        self.assertEqual(
-            1, len(list(CalculateBugHeatJob.iterReady())))
+        self.assertEqual(1, self._getJobCount())
 
         new_job = CalculateBugHeatJob.create(self.bug)
 
@@ -60,8 +82,7 @@
         self.assertEqual(job, new_job)
 
         # And the queue will still have a length of 1.
-        self.assertEqual(
-            1, len(list((CalculateBugHeatJob.iterReady()))))
+        self.assertEqual(1, self._getJobCount())
 
     def test_cronscript_succeeds(self):
         # The calculate-bug-heat cronscript will run all pending
@@ -90,6 +111,108 @@
         self.assertIn(('bug_job_id', job.context.id), vars)
         self.assertIn(('bug_job_type', job.context.job_type.title), vars)
 
+    def test_bug_changes_adds_job(self):
+        # Calling addChange() on a Bug will add a CalculateBugHeatJob
+        # for that bug to the queue.
+        self.assertEqual(0, self._getJobCount())
+
+        change = BugDescriptionChange(
+            when=datetime.now().replace(tzinfo=pytz.timezone('UTC')),
+            person=self.bug.owner, what_changed='description',
+            old_value=self.bug.description, new_value='Some text')
+        self.bug.addChange(change)
+
+        # There will now be a job in the queue.
+        self.assertEqual(1, self._getJobCount())
+
+    def test_subscribing_adds_job(self):
+        # Calling Bug.subscribe() will add a CalculateBugHeatJob for the
+        # Bug.
+        self.assertEqual(0, self._getJobCount())
+
+        person = self.factory.makePerson()
+        self.bug.subscribe(person, person)
+        transaction.commit()
+
+        # There will now be a job in the queue.
+        self.assertEqual(1, self._getJobCount())
+
+    def test_unsubscribing_adds_job(self):
+        # Calling Bug.unsubscribe() will add a CalculateBugHeatJob for the
+        # Bug.
+        self.assertEqual(0, self._getJobCount())
+
+        self.bug.unsubscribe(self.bug.owner, self.bug.owner)
+        transaction.commit()
+
+        # There will now be a job in the queue.
+        self.assertEqual(1, self._getJobCount())
+
+    def test_marking_affected_adds_job(self):
+        # Marking a user as affected by a bug adds a CalculateBugHeatJob
+        # for the bug.
+        self.assertEqual(0, self._getJobCount())
+
+        person = self.factory.makePerson()
+        self.bug.markUserAffected(person)
+
+        # There will now be a job in the queue.
+        self.assertEqual(1, self._getJobCount())
+
+    def test_marking_unaffected_adds_job(self):
+        # Marking a user as unaffected by a bug adds a CalculateBugHeatJob
+        # for the bug.
+        self.assertEqual(0, self._getJobCount())
+
+        self.bug.markUserAffected(self.bug.owner, False)
+
+        # There will now be a job in the queue.
+        self.assertEqual(1, self._getJobCount())
+
+    def test_bug_creation_creates_job(self):
+        # Creating a bug adds a CalculateBugHeatJob for the new bug.
+        self.assertEqual(0, self._getJobCount())
+
+        new_bug = self.factory.makeBug()
+
+        # There will now be a job in the queue.
+        self.assertEqual(1, self._getJobCount())
+
+    def test_marking_dupe_creates_job(self):
+        # Marking a bug as a duplicate of another bug creates a job to
+        # update the master bug.
+        new_bug = self.factory.makeBug()
+        new_bug.setHeat(42)
+        self._completeJobsAndAssertQueueEmpty()
+
+        new_bug.markAsDuplicate(self.bug)
+
+        # There will now be a job in the queue.
+        self.assertEqual(1, self._getJobCount())
+
+        # And the job will be for the master bug.
+        bug_job = self._getJobs()[0]
+        self.assertEqual(bug_job.bug, self.bug)
+
+        # Also, the duplicate bug's heat will have been set to zero.
+        self.assertEqual(0, new_bug.heat)
+
+    def test_unmarking_dupe_creates_job(self):
+        # Unmarking a bug as a duplicate will create a
+        # CalculateBugHeatJob for the bug, since its heat will be 0 from
+        # having been marked as a duplicate.
+        new_bug = self.factory.makeBug()
+        new_bug.markAsDuplicate(self.bug)
+        self._completeJobsAndAssertQueueEmpty()
+        new_bug.markAsDuplicate(None)
+
+        # There will now be a job in the queue.
+        self.assertEqual(1, self._getJobCount())
+
+        # And the job will be for the master bug.
+        bug_job = self._getJobs()[0]
+        self.assertEqual(bug_job.bug, new_bug)
+
 
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)