launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/custom-result-decorator-for-bugtask-search into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/custom-result-decorator-for-bugtask-search/+merge/51876

This branch adds the ability to specify a user defined resultset decorator to use when calling search() on IBugTaskSet. This is required as a pre-requisite for some other functionality currently under development. It is required to perform a bug task search and access from the resulting query more than just the BugTask and Bug instances which is all that is catered for now.

Tests

Add new test_search_with_user_decorator test to test_bugtask_search.py

Lint

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/model/bugtask.py
  lib/lp/bugs/tests/test_bugtask_search.py

./lib/lp/bugs/model/bugtask.py
    2099: E501 line too long (81 characters)
    2133: E301 expected 1 blank line, found 0
    2099: Line exceeds 78 characters.
./lib/lp/bugs/tests/test_bugtask_search.py
     636: Line exceeds 78 characters.

-- 
https://code.launchpad.net/~wallyworld/launchpad/custom-result-decorator-for-bugtask-search/+merge/51876
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/custom-result-decorator-for-bugtask-search into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-02-27 19:45:44 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-03-02 11:15:25 +0000
@@ -2417,12 +2417,15 @@
                 origin.append(table)
         return origin
 
-    def _search(self, resultrow, prejoins, pre_iter_hook, params, *args):
+    def _search(self, resultrow, prejoins, user_resultset_decorator,
+                pre_iter_hook, params, *args):
         """Return a Storm result set for the given search parameters.
 
         :param resultrow: The type of data returned by the query.
         :param prejoins: A sequence of Storm SQL row instances which are
             pre-joined.
+        :param user_resultset_decorator: An optional decorator used to extract
+            the required data from the result set.
         :param pre_iter_hook: An optional pre-iteration hook used for eager
             loading bug targets for list views.
         :param params: A BugTaskSearchParams instance.
@@ -2441,13 +2444,16 @@
             else:
                 origin = self.buildOrigin(join_tables, prejoins, clauseTables)
                 resultset = store.using(*origin).find(resultrow, query)
-            if prejoins:
-                decorator = lambda row: bugtask_decorator(row[0])
-            else:
-                decorator = bugtask_decorator
+            default_decorator = user_resultset_decorator
+            if not default_decorator:
+                if prejoins:
+                    default_decorator = lambda row: bugtask_decorator(row[0])
+                else:
+                    default_decorator = bugtask_decorator
 
             resultset.order_by(orderby)
-            return DecoratedResultSet(resultset, result_decorator=decorator,
+            return DecoratedResultSet(resultset,
+                result_decorator=default_decorator,
                 pre_iter_hook=pre_iter_hook)
 
         bugtask_fti = SQL('BugTask.fti')
@@ -2455,7 +2461,10 @@
         origin = self.buildOrigin(join_tables, [], clauseTables)
         resultset = store.using(*origin).find(inner_resultrow, query)
 
-        decorators = [bugtask_decorator]
+        default_decorator = user_resultset_decorator
+        if not default_decorator:
+            default_decorator = bugtask_decorator
+        decorators = [default_decorator]
         for arg in args:
             [query, clauseTables, ignore, decorator, join_tables,
              has_duplicate_results] = self.buildQuery(arg)
@@ -2504,6 +2513,8 @@
         from lp.registry.model.product import Product
         from lp.bugs.model.bug import Bug
         _noprejoins = kwargs.get('_noprejoins', False)
+        user_resultset_decorator = kwargs.get(
+            'user_resultset_decorator', None)
         if _noprejoins:
             prejoins = []
             resultrow = BugTask
@@ -2534,17 +2545,19 @@
                 table for table, join in requested_joins
                 if table not in resultrow]
             resultrow = resultrow + tuple(additional_result_objects)
-        return self._search(resultrow, prejoins, eager_load, params, *args)
+        return self._search(
+            resultrow, prejoins, user_resultset_decorator, eager_load,
+            params, *args)
 
     def searchBugIds(self, params):
         """See `IBugTaskSet`."""
-        return self._search(BugTask.bugID, [], None, params).result_set
+        return self._search(BugTask.bugID, [], None, None, params).result_set
 
     def countBugs(self, params, group_on):
         """See `IBugTaskSet`."""
         resultset = self._search(
             group_on + (SQL("COUNT(Distinct BugTask.bug)"),),
-            [], None, params).result_set
+            [], None, None, params).result_set
         # We group on the related field:
         resultset.group_by(*group_on)
         resultset.order_by()

=== modified file 'lib/lp/bugs/tests/test_bugtask_search.py'
--- lib/lp/bugs/tests/test_bugtask_search.py	2011-02-22 22:23:20 +0000
+++ lib/lp/bugs/tests/test_bugtask_search.py	2011-03-02 11:15:25 +0000
@@ -533,6 +533,27 @@
         self.assertSearchFinds(params, self.bugtasks[:1])
 
 
+class CustomisedSearchTestCase(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_search_with_user_decorator(self):
+    # Test that a custom resultset decorator is called as expected.
+        expected_tasks = []
+
+        def resultset_decorator(row):
+            expected_tasks.append(row[0])
+            return row[0]
+
+        bugtask_set = getUtility(IBugTaskSet)
+        bug = self.factory.makeBug()
+        params = BugTaskSearchParams(user=None, bug=bug)
+        found_tasks = list(bugtask_set.search(
+            params, user_resultset_decorator=resultset_decorator))
+        self.assertEqual(found_tasks, [bug.default_bugtask])
+        self.assertEqual(expected_tasks, [bug.default_bugtask])
+
+
 class DeactivatedProductBugTaskTestCase(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
@@ -607,7 +628,7 @@
 
 class BugTargetTestBase:
     """A base class for the bug target mixin classes.
-    
+
     :ivar searchtarget: A bug context to search within.
     :ivar searchtarget2: A sibling bug context for testing cross-context
         searches. Created on demand when