launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-629921-packages-empty-filter into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #629921 Archive:+packages with empty name search does like '%%' search.
  https://bugs.launchpad.net/bugs/629921


Passing an empty name filter string into Archive:+packages causes a query asking for packages matching '%%', and PostgreSQL appears to be insufficiently clever to optimise that away to nothing. This can result in very slow query.

The view now treats an empty name filter identically to an omitted one, and the various cases are tested.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-629921-packages-empty-filter/+merge/37339
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-629921-packages-empty-filter into lp:launchpad/devel.

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2010-09-06 20:19:45 +0000
+++ lib/lp/soyuz/browser/archive.py	2010-10-02 07:19:48 +0000
@@ -760,7 +760,8 @@
         requested_name_filter = self.request.query_string_params.get(
             'field.name_filter')
 
-        if requested_name_filter is not None:
+        if (requested_name_filter is not None and
+            len(requested_name_filter[0]) > 0):
             return requested_name_filter[0]
         else:
             return None

=== modified file 'lib/lp/soyuz/browser/tests/test_archive_packages.py'
--- lib/lp/soyuz/browser/tests/test_archive_packages.py	2010-08-21 13:54:20 +0000
+++ lib/lp/soyuz/browser/tests/test_archive_packages.py	2010-10-02 07:19:48 +0000
@@ -89,6 +89,11 @@
 
     layer = LaunchpadFunctionalLayer
 
+    def getPackagesView(self, query_string=None):
+        ppa = self.factory.makeArchive()
+        return create_initialized_view(
+            ppa, "+packages", query_string=query_string)
+
     def test_ppa_packages_menu_is_enabled(self):
         joe = self.factory.makePerson()
         ppa = self.factory.makeArchive()
@@ -96,3 +101,15 @@
         view = create_initialized_view(ppa, "+index")
         menu = ArchiveNavigationMenu(view)
         self.assertTrue(menu.packages().enabled)
+
+    def test_specified_name_filter_works(self):
+        view = self.getPackagesView('field.name_filter=blah')
+        self.assertEquals('blah', view.specified_name_filter)
+
+    def test_specified_name_filter_returns_none_on_omission(self):
+        view = self.getPackagesView()
+        self.assertIs(None, view.specified_name_filter)
+
+    def test_specified_name_filter_returns_none_on_empty_filter(self):
+        view = self.getPackagesView('field.name_filter=')
+        self.assertIs(None, view.specified_name_filter)