launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-931235 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #931235 in Launchpad itself: "IndexError on Distribution:+builds when filtering by name and using a memo"
  https://bugs.launchpad.net/launchpad/+bug/931235

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-931235/+merge/92711

This branch fixes a Distribution:+builds OOPS (in circumstances described by the bug).

BinaryPackageBuildSet.handleOptionalParamsForBuildQueries was single-escaping the '%'s as '%%' in its LIKE clause, then interpolating it itself, yielding a string with unescaped '%'s. Now, this was all well and good until the StormRangeFactory came along, introducing Storm-side interpolation into queries that use that clause. Storm sees the two unescaped '%'s, and crashes with an IndexError because there aren't enough parameters.

This branch fixes this by double-escaping '%' to '%%%%', as is already done in various places around the codebase. While this does mean '%%' will get through to the DB sometimes, this proves to have no effect.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-931235/+merge/92711
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-931235 into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/tests/test_build_views.py'
--- lib/lp/soyuz/browser/tests/test_build_views.py	2012-01-01 02:58:52 +0000
+++ lib/lp/soyuz/browser/tests/test_build_views.py	2012-02-13 04:27:33 +0000
@@ -424,3 +424,15 @@
             self.assertThat(
                 test_range_factory,
                 Not(Raises(MatchesException(StormRangeFactoryError))))
+
+    def test_name_filter_with_storm_range_factory(self):
+        distroseries = self.factory.makeDistroSeries()
+        self.factory.makeDistroArchSeries(distroseries=distroseries)
+        view = create_initialized_view(
+            distroseries.distribution, name="+builds",
+            form={
+                'build_state': 'built',
+                'build_text': 'foo',
+                'start': 75,
+                'memo': '["2012-01-01T01:01:01", 0]'})
+        view.setupBuildList()

=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py	2012-01-06 11:08:30 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py	2012-02-13 04:27:33 +0000
@@ -982,7 +982,7 @@
                         SourcePackageRelease.id AND
                     SourcePackageRelease.sourcepackagename =
                         SourcePackageName.id
-                    AND SourcepackageName.name LIKE '%%' || %s || '%%'
+                    AND SourcepackageName.name LIKE '%%%%' || %s || '%%%%'
                 ''' % quote_like(name))
             else:
                 queries.append('''