launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/better-createManyTasks into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/better-createManyTasks/+merge/105422

BugTask.createManyTasks was added yesterday to more efficiently approve distroseries nominations. But it's pretty badly duplicated with createTask, and roughly twice as long as it needs to be.

This branch rewrites both of them to share code, be cleaner, and be slightly faster.
-- 
https://code.launchpad.net/~wgrant/launchpad/better-createManyTasks/+merge/105422
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/better-createManyTasks into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2012-05-10 23:14:38 +0000
+++ lib/lp/bugs/model/bugtask.py	2012-05-11 00:10:23 +0000
@@ -1611,109 +1611,62 @@
         params = BugTaskSearchParams(user, **kwargs)
         return self.search(params)
 
-    def _initNewTask(self, bug, owner, target, status, importance, assignee,
-                       milestone):
-        if not status:
+    def createManyTasks(self, bug, owner, targets, status=None,
+                        importance=None, assignee=None, milestone=None):
+        """See `IBugTaskSet`."""
+        if status is None:
             status = IBugTask['status'].default
-        if not importance:
+        if importance is None:
             importance = IBugTask['importance'].default
-        if not assignee:
-            assignee = None
-        if not milestone:
-            milestone = None
-
-        # Make sure there's no task for this bug already filed
-        # against the target.
-        validate_new_target(bug, target)
-
-        target_key = bug_target_to_key(target)
-        if not bug.private and bug.security_related:
-            product = target_key['product']
-            distribution = target_key['distribution']
-            if product and product.security_contact:
-                bug.subscribe(product.security_contact, owner)
-            elif distribution and distribution.security_contact:
-                bug.subscribe(distribution.security_contact, owner)
-
-        non_target_create_params = dict(
-            bug=bug,
-            _status=status,
-            importance=importance,
-            assignee=assignee,
-            owner=owner,
-            milestone=milestone)
-        create_params = non_target_create_params.copy()
-        create_params.update(target_key)
-        return create_params, non_target_create_params
-
-    def createManyTasks(self, bug, owner, targets,
-                        status=IBugTask['status'].default,
-                        importance=IBugTask['importance'].default,
-                        assignee=None, milestone=None):
-        """See `IBugTaskSet`."""
-        params = [self._initNewTask(bug, owner, target, status, importance,
-            assignee, milestone) for target in targets]
-
-        fieldnames = (
-            'assignee',
-            'bug',
-            'distribution',
-            'distroseries',
-            'importance',
-            'milestone',
-            'owner',
-            'product',
-            'productseries',
-            'sourcepackagename',
-            '_status',
-        )
-
-        fields = [getattr(BugTask, field) for field in fieldnames]
-        # Values need to be a list of tuples in the order we're declaring our
-        # fields.
-        values = [[p[0].get(field) for field in fieldnames] for p in params]
-
-        taskset = create(fields, values, get_objects=True)
+        target_keys = []
+        pillars = set()
+        for target in targets:
+            pillars.add(target.pillar)
+            validate_new_target(bug, target)
+            target_keys.append(bug_target_to_key(target))
+        for pillar in pillars:
+            if pillar.security_contact:
+                bug.subscribe(pillar.security_contact, owner)
+
+        values = [
+            (bug, owner, key['product'], key['productseries'],
+             key['distribution'], key['distroseries'],
+             key['sourcepackagename'], status, importance, assignee, milestone)
+            for key in target_keys]
+        tasks = create(
+            (BugTask.bug, BugTask.owner, BugTask.product,
+             BugTask.productseries, BugTask.distribution, BugTask.distroseries,
+             BugTask.sourcepackagename, BugTask._status, BugTask.importance,
+             BugTask.assignee, BugTask.milestone),
+            values, get_objects=True)
+
         del get_property_cache(bug).bugtasks
-        for bugtask in taskset:
+        for bugtask in tasks:
             bugtask.updateTargetNameCache()
             if bugtask.conjoined_slave:
                 bugtask._syncFromConjoinedSlave()
-        return taskset
+        return tasks
 
-    def createTask(self, bug, owner, target,
-                   status=IBugTask['status'].default,
-                   importance=IBugTask['importance'].default,
+    def createTask(self, bug, owner, target, status=None, importance=None,
                    assignee=None, milestone=None):
         """See `IBugTaskSet`."""
-        create_params, non_target_create_params = self._initNewTask(bug,
-            owner, target, status, importance, assignee, milestone)
-        bugtask = BugTask(**create_params)
-
-        if create_params['distribution']:
-            # Create tasks for accepted nominations if this is a source
-            # package addition.
-            accepted_nominations = [
-                nomination for nomination in
-                bug.getNominations(create_params['distribution'])
-                if nomination.isApproved()]
-            for nomination in accepted_nominations:
-                accepted_series_task = BugTask(
-                    distroseries=nomination.distroseries,
-                    sourcepackagename=create_params['sourcepackagename'],
-                    **non_target_create_params)
-                accepted_series_task.updateTargetNameCache()
-
-        if bugtask.conjoined_slave:
-            bugtask._syncFromConjoinedSlave()
-
-        bugtask.updateTargetNameCache()
-        del get_property_cache(bug).bugtasks
-        # Because of block_implicit_flushes, it is possible for a new bugtask
-        # to be queued in appropriately, which leads to Bug.bugtasks not
-        # finding the bugtask.
-        Store.of(bugtask).flush()
-        return bugtask
+        # Create tasks for accepted nominations if this is a source
+        # package addition. Distribution nominations are for all the
+        # tasks.
+        targets = [target]
+        key = bug_target_to_key(target)
+        if key['distribution'] is not None:
+            for nomination in bug.getNominations(key['distribution']):
+                if not nomination.isApproved():
+                    continue
+                targets.append(
+                    nomination.distroseries.getSourcePackage(
+                        key['sourcepackagename']))
+
+        tasks = self.createManyTasks(
+            bug, owner, targets, status=status, importance=importance,
+            assignee=assignee, milestone=milestone)
+        return [task for task in tasks if task.target == target][0]
 
     def getStatusCountsForProductSeries(self, user, product_series):
         """See `IBugTaskSet`."""