launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/bug-607960 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~lifeless/launchpad/bug-607960/+merge/49588

LIKE searching in targetnamecache is very expensive, this allows it to be disabled so we can see the true impact and assess its usability impacts.
-- 
https://code.launchpad.net/~lifeless/launchpad/bug-607960/+merge/49588
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-607960 into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-02-13 23:02:49 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-02-14 04:46:15 +0000
@@ -163,6 +163,7 @@
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.registry.model.pillar import pillar_sort_key
 from lp.registry.model.sourcepackagename import SourcePackageName
+from lp.services import features
 from lp.services.propertycache import get_property_cache
 from lp.soyuz.enums import PackagePublishingStatus
 from lp.soyuz.model.publishing import SourcePackagePublishingHistory
@@ -2205,9 +2206,14 @@
                 AND MessageChunk.fti @@ ftq(%s))""" % searchtext_quoted
         text_search_clauses = [
             "Bug.fti @@ ftq(%s)" % searchtext_quoted,
-            "BugTask.fti @@ ftq(%s)" % searchtext_quoted,
-            "BugTask.targetnamecache ILIKE '%%' || %s || '%%'" % (
-                searchtext_like_quoted)]
+            "BugTask.fti @@ ftq(%s)" % searchtext_quoted
+            ]
+        no_targetnamesearch = bool(features.getFeatureFlag(
+            'malone.disable_targetnamesearch'))
+        if not no_targetnamesearch:
+            text_search_clauses.append(
+                "BugTask.targetnamecache ILIKE '%%' || %s || '%%'" % (
+                searchtext_like_quoted))
         # Due to performance problems, whether to search in comments is
         # controlled by a config option.
         if config.malone.search_comments:

=== modified file 'lib/lp/bugs/tests/test_bugtask_search.py'
--- lib/lp/bugs/tests/test_bugtask_search.py	2011-02-06 21:58:31 +0000
+++ lib/lp/bugs/tests/test_bugtask_search.py	2011-02-14 04:46:15 +0000
@@ -44,6 +44,7 @@
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.sourcepackage import ISourcePackage
 from lp.registry.model.person import Person
+from lp.services.features.testing import FeatureFixture
 from lp.testing import (
     person_logged_in,
     StormStatementRecorder,
@@ -755,6 +756,25 @@
                 subscriber, subscribed_by=subscriber)
         return subscriber
 
+    def test_disable_targetnames_search(self):
+        # searching in the target name is contentious and arguably a bug. To
+        # permit incremental changes we allow it to be disabled via a feature
+        # flag.
+        with person_logged_in(self.owner):
+            product1 = self.factory.makeProduct(name='product-foo',
+                owner=self.owner, project=self.searchtarget)
+            product2 = self.factory.makeProduct(name='product-bar',
+                owner=self.owner, project=self.searchtarget)
+            bug1 = self.factory.makeBug(product=product1)
+            bug1.default_bugtask.updateTargetNameCache()
+            bug2 = self.factory.makeBug(product=product2)
+        params = self.getBugTaskSearchParams(user=None, searchtext='uct-fo')
+        # With no flag, we find the first bug.
+        self.assertSearchFinds(params, [bug1.default_bugtask])
+        with FeatureFixture({'malone.disable_targetnamesearch': u'on'}):
+            # With a flag set, no bugs are found.
+            self.assertSearchFinds(params, [])
+
 
 class MilestoneTarget(BugTargetTestBase):
     """Use a milestone as the bug target."""

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2011-02-07 07:10:54 +0000
+++ lib/lp/services/features/flags.py	2011-02-14 04:46:15 +0000
@@ -32,6 +32,10 @@
      '[on|off]',
      'enables advanced subscriptions features',
      'off'),
+    ('malone.disable_targetnamesearch',
+     '[empty|nonempty]',
+     'If nonempty targetnames are not consulted during text search.'
+     ''),
     ('memcache',
      '[enabled|disabled]',
      'enables/disables memcache',