Merge lp:~lifeless/launchpad/batchnav into lp:launchpad

[Robert Collins]

[r=lifeless][bug=809719] Update to lazr.batchnavigator 1.2.5.

This is https://code.launchpad.net/~stub/launchpad/batch-navigator/+merge/67233 updated to grab 1.2.5 and fix the regression we found in it (discussed on that bug). The upstream change is trivial so I'm just self-reviewing this and shooting to ec2. I've made a placeholder bug to permit a QA step.

[Robert Collins (lifeless)]

review: Approve

=== modified file 'lib/canonical/launchpad/webapp/batching.py'
--- lib/canonical/launchpad/webapp/batching.py	2010-10-14 23:03:41 +0000
+++ lib/canonical/launchpad/webapp/batching.py	2011-07-13 06:27:29 +0000
@@ -98,19 +98,17 @@
 class ActiveBatchNavigator(BatchNavigator):
     """A paginator for active items.
 
-    Used when a view needs to display more than one BatchNavigator of items.
+    Used when a view needs to display two BatchNavigators.
     """
-    start_variable_name = 'active_start'
-    batch_variable_name = 'active_batch'
+    variable_name_prefix = 'active'
 
 
 class InactiveBatchNavigator(BatchNavigator):
     """A paginator for inactive items.
 
-    Used when a view needs to display more than one BatchNavigator of items.
+    Used when a view needs to display two Batchavigators.
     """
-    start_variable_name = 'inactive_start'
-    batch_variable_name = 'inactive_batch'
+    variable_name_prefix = 'inactive'
 
 
 class TableBatchNavigator(BatchNavigator):

=== modified file 'lib/lp/app/browser/root.py'
--- lib/lp/app/browser/root.py	2011-07-12 01:42:11 +0000
+++ lib/lp/app/browser/root.py	2011-07-13 06:27:29 +0000
@@ -13,6 +13,7 @@
 import time
 
 import feedparser
+from lazr.batchnavigator import ListRangeFactory
 from lazr.batchnavigator.z3batching import batch
 from zope.component import getUtility
 from zope.interface import Interface
@@ -566,6 +567,7 @@
 class GoogleBatchNavigator(BatchNavigator):
     """A batch navigator with a fixed size of 20 items per batch."""
 
+    _batch_factory = WindowedListBatch
     # Searches generally don't show the 'Last' link when there is a
     # good chance of getting over 100,000 results.
     show_last_link = False
@@ -573,7 +575,9 @@
     singular_heading = 'page'
     plural_heading = 'pages'
 
-    def __init__(self, results, request, start=0, size=20, callback=None):
+    def __init__(self, results, request, start=0, size=20, callback=None,
+                 transient_parameters=None, force_start=False,
+                 range_factory=None):
         """See `BatchNavigator`.
 
         :param results: A `PageMatches` object that contains the matching
@@ -584,25 +588,13 @@
         :param size: The batch size is fixed to 20, The param is not used.
         :param callback: Not used.
         """
-        # We do not want to call super() because it will use the batch
-        # size from the URL.
-        # pylint: disable-msg=W0231
         results = WindowedList(results, start, results.total)
-        self.request = request
-        request_start = request.get(self.start_variable_name, None)
-        if request_start is None:
-            self.start = start
-        else:
-            try:
-                self.start = int(request_start)
-            except (ValueError, TypeError):
-                self.start = start
+        super(GoogleBatchNavigator, self).__init__(results, request,
+            start=start, size=size, callback=callback,
+            transient_parameters=transient_parameters, force_start=force_start,
+            range_factory=range_factory)
 
+    def determineSize(self, size, batch_params_source):
+        # Force the default and users requested sizes to 20.
         self.default_size = 20
-
-        self.transient_parameters = [self.start_variable_name]
-
-        self.batch = WindowedListBatch(
-            results, start=self.start, size=self.default_size)
-        self.setHeadings(
-            self.default_singular_heading, self.default_plural_heading)
+        return 20

=== modified file 'lib/lp/app/browser/tests/launchpad-search-pages.txt'
--- lib/lp/app/browser/tests/launchpad-search-pages.txt	2011-07-12 01:42:11 +0000
+++ lib/lp/app/browser/tests/launchpad-search-pages.txt	2011-07-13 06:27:29 +0000
@@ -444,8 +444,6 @@
 provides a link to the next batch, which also happens to be the last
 batch.
 
-    >>> len(search_view.pages.getBatches())
-    2
     >>> search_view.pages.nextBatchURL()
     '...start=20'
     >>> search_view.pages.lastBatchURL()

=== modified file 'lib/lp/app/doc/batch-navigation.txt'
--- lib/lp/app/doc/batch-navigation.txt	2011-07-12 01:42:11 +0000
+++ lib/lp/app/doc/batch-navigation.txt	2011-07-13 06:27:29 +0000
@@ -47,88 +47,18 @@
   ...     'Cupid', 'Donner', 'Blitzen', 'Rudolph']
 
 
-Performance with SQLObject
-==========================
-
-This section demonstrates that batching generates sensible SQL queries when
-used with SQLObject, i.e. that it puts appropriate LIMIT clauses on queries.
-
-Imports and initialization:
-
-  >>> from lp.testing.pgsql import CursorWrapper
-  >>> from canonical.launchpad.database.emailaddress import EmailAddress
-  >>> from canonical.launchpad.interfaces.lpstorm import IStore
-  >>> ignore = IStore(EmailAddress) # Prime the database connection.
-  ... # (Priming is important if this test is run in isolation).
-  >>> CursorWrapper.record_sql = True
-
-Prepare a query, and create a batch of the results:
-
-  >>> select_results = EmailAddress.select(orderBy='id')
-  >>> batch_nav = BatchNavigator(select_results, build_request(), size=10)
-  >>> email_batch = batch_nav.currentBatch()
-  >>> batch_items = list(email_batch)
-
-Because we're only looking at the first batch, the database is only
-asked for the first 11 rows. (lazr.batchnavigator asks for 11 instead
-of 10 so that it can reliably detect the end of the dataset).
-
-  >>> len(CursorWrapper.last_executed_sql)
-  1
-  >>> print CursorWrapper.last_executed_sql[0]
-  SELECT ... FROM EmailAddress ... LIMIT 11...
-
-Get the next 10.  The database is only asked for the next 11 rows:
-
-  >>> CursorWrapper.last_executed_sql = []
-  >>> email_batch2 = email_batch.nextBatch()
-  >>> batch_items = list(email_batch2)
-  >>> len(CursorWrapper.last_executed_sql)
-  1
-  >>> CursorWrapper.last_executed_sql[0].endswith('LIMIT 11 OFFSET 10')
-  True
-
-As seen above, simply accessing the batch doesn't trigger a SQL query
-asking for the length of the entire resultset. But explicitly asking
-for the length will trigger a SQL query in most circumstances.
-
-  >>> CursorWrapper.last_executed_sql = []
-  >>> ignored = email_batch.total()
-  >>> print CursorWrapper.last_executed_sql[0]
-  SELECT COUNT(*) FROM EmailAddress
-
-There are exceptions. When the current batch is the last one in the
-list, it's possible to get the length of the entire resultset without
-triggering a COUNT query.
-
-  >>> CursorWrapper.last_executed_sql = []
-  >>> batch_nav = BatchNavigator(select_results, build_request(), size=50)
-  >>> final_batch = batch_nav.currentBatch().nextBatch()
-  >>> batch_items = list(final_batch)
-  >>> ignored = final_batch.total()
-  >>> print "\n".join(CursorWrapper.last_executed_sql)
-  SELECT ... FROM EmailAddress ... OFFSET 0
-  SELECT ... FROM EmailAddress ... OFFSET 50
-
-When the current batch contains the entire resultset, it's possible to
-get the length of the resultset without triggering a COUNT query.
-
-  >>> CursorWrapper.last_executed_sql = []
-  >>> one_page_nav = BatchNavigator(select_results, build_request(), size=200)
-  >>> only_batch = one_page_nav.currentBatch()
-  >>> batch_items = list(only_batch)
-  >>> ignored = only_batch.total()
-  >>> print "\n".join(CursorWrapper.last_executed_sql)
-  SELECT ... FROM EmailAddress ... OFFSET 0
-
 Multiple pages
 ==============
 
 The batch navigator tells us whether multiple pages will be used.
 
+    >>> from canonical.launchpad.database.emailaddress import EmailAddress
+    >>> select_results = EmailAddress.select(orderBy='id')
+    >>> batch_nav = BatchNavigator(select_results, build_request(), size=50)
     >>> batch_nav.has_multiple_pages
     True
 
+    >>> one_page_nav = BatchNavigator(select_results, build_request(), size=200)
     >>> one_page_nav.has_multiple_pages
     False
 

=== modified file 'lib/lp/bugs/stories/webservice/xx-bug-tracker.txt'
--- lib/lp/bugs/stories/webservice/xx-bug-tracker.txt	2011-07-12 01:42:11 +0000
+++ lib/lp/bugs/stories/webservice/xx-bug-tracker.txt	2011-07-13 06:27:29 +0000
@@ -10,7 +10,7 @@
     >>> bug_tracker_collection = anon_webservice.get(
     ...     '/bugs/bugtrackers').jsonBody()
     >>> pprint_collection(bug_tracker_collection)
-    next_collection_link: u'http://.../bugs/bugtrackers?ws.start=5&ws.size=5'
+    next_collection_link: u'http://.../bugs/bugtrackers?ws.size=5&memo=5&ws.start=5'
     resource_type_link: u'http://.../#bug_trackers'
     start: 0
     total_size: 8

=== modified file 'lib/lp/bugs/stories/webservice/xx-bug.txt'
--- lib/lp/bugs/stories/webservice/xx-bug.txt	2011-07-12 01:42:11 +0000
+++ lib/lp/bugs/stories/webservice/xx-bug.txt	2011-07-13 06:27:29 +0000
@@ -1841,7 +1841,7 @@
 
     >>> cves = webservice.get("/bugs/cve").jsonBody()
     >>> pprint_collection(cves)
-    next_collection_link: u'http://.../bugs/cve?ws.start=5&ws.size=5'
+    next_collection_link: u'http://.../bugs/cve?ws.size=5&memo=5&ws.start=5'
     resource_type_link: u'http://.../#cves'
     start: 0
     total_size: 10
@@ -2130,7 +2130,7 @@
     >>> activity = webservice.get(
     ...     bug_one['activity_collection_link']).jsonBody()
     >>> pprint_collection(activity)
-    next_collection_link: u'http://.../bugs/1/activity?ws.start=5&ws.size=5'
+    next_collection_link: u'http://.../bugs/1/activity?ws.size=5&memo=5&ws.start=5'
     resource_type_link: u'http://.../#bug_activity-page-resource'
     start: 0
     total_size: 25

=== modified file 'lib/lp/services/features/browser/tests/test_changelog.py'
--- lib/lp/services/features/browser/tests/test_changelog.py	2011-07-12 01:42:11 +0000
+++ lib/lp/services/features/browser/tests/test_changelog.py	2011-07-13 06:27:29 +0000
@@ -80,7 +80,7 @@
         self.assertEqual('change', batch._singular_heading)
         self.assertEqual('changes', batch._plural_heading)
         self.assertEqual(10, batch.default_size)
-        self.assertEqual(2, len(batch.getBatches()))
+        self.assertEqual(None, batch.currentBatch().nextBatch().nextBatch())
 
     def test_page_batched_changes(self):
         self.makeFeatureFlagChanges()

=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-navigation.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-navigation.txt	2011-07-12 01:42:11 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-navigation.txt	2011-07-13 06:27:29 +0000
@@ -179,8 +179,8 @@
 The 'First' and 'Previous' links, however, are now active.
 
     >>> anon_browser.getLink('First').url
-    'http://launchpad.dev/%7Ecprov/+archive/ppa/+index?start=0&batch=1'
+    'http://launchpad.dev/%7Ecprov/+archive/ppa/+index?batch=1'
 
     >>> anon_browser.getLink('Previous').url
-    'http://launchpad.dev/%7Ecprov/+archive/ppa/+index?start=1&batch=1'
+    'http://launchpad.dev/%7Ecprov/+archive/ppa/+index?batch=1&direction=backwards&memo=2&start=1'
 

=== modified file 'lib/lp/translations/browser/tests/pofile-views.txt'
--- lib/lp/translations/browser/tests/pofile-views.txt	2011-07-12 01:42:11 +0000
+++ lib/lp/translations/browser/tests/pofile-views.txt	2011-07-13 06:27:29 +0000
@@ -241,7 +241,7 @@
 Also, we get redirected to the next batch.
 
     >>> pofile_view.request.response.getHeader('Location')
-    'http://127.0.0.1?start=10'
+    'http://127.0.0.1?memo=10&start=10'
 
 The messsage's sequence is the position of that message in latest imported
 template. We are going to test now what happens when we submit a potmsgset

=== modified file 'lib/lp/translations/browser/translationmessage.py'
--- lib/lp/translations/browser/translationmessage.py	2011-07-12 01:42:11 +0000
+++ lib/lp/translations/browser/translationmessage.py	2011-07-13 06:27:29 +0000
@@ -128,6 +128,9 @@
         results is an iterable of results. request is the web request
         being processed. size is a default batch size which the callsite
         can choose to provide.
+
+        Why a custom BatchNavigator is required is a great mystery and
+        should be documented here.
         """
         schema, netloc, path, parameters, query, fragment = (
             urlparse(str(request.URL)))
@@ -164,7 +167,7 @@
 
         BatchNavigator.__init__(self, results, request, start_value, size)
 
-    def generateBatchURL(self, batch):
+    def generateBatchURL(self, batch, backwards=False):
         """Return a custom batch URL for `ITranslationMessage`'s views."""
         url = ""
         if batch is None:

=== modified file 'lib/lp/translations/stories/importqueue/xx-translation-import-queue-filtering.txt'
--- lib/lp/translations/stories/importqueue/xx-translation-import-queue-filtering.txt	2011-07-12 01:42:11 +0000
+++ lib/lp/translations/stories/importqueue/xx-translation-import-queue-filtering.txt	2011-07-13 06:27:29 +0000
@@ -56,7 +56,7 @@
     >>> browser.getLink('Next')
     <Link
       text='Next'
-      url='http://.../+imports/+index?start=5&batch=5'>
+      url='http://.../+imports/+index?batch=5&memo=5&start=5'>
 
 
 == Target filter ==

=== modified file 'lib/lp/translations/stories/standalone/xx-pofile-translate-empty-strings-without-validation.txt'
--- lib/lp/translations/stories/standalone/xx-pofile-translate-empty-strings-without-validation.txt	2011-07-12 01:42:11 +0000
+++ lib/lp/translations/stories/standalone/xx-pofile-translate-empty-strings-without-validation.txt	2011-07-13 06:27:29 +0000
@@ -27,5 +27,5 @@
 it as an error.
 
   >>> print browser.url
-  http://translations.launchpad.dev/ubuntu/hoary/+source/evolution/+pots/evolution-2.2/es/+translate?batch=1&start=13
+  http://translations.launchpad.dev/ubuntu/hoary/+source/evolution/+pots/evolution-2.2/es/+translate?batch=1&memo=13&start=13
 

=== modified file 'lib/lp/translations/stories/standalone/xx-pofile-translate-html-tags-escape.txt'
--- lib/lp/translations/stories/standalone/xx-pofile-translate-html-tags-escape.txt	2011-07-12 01:42:11 +0000
+++ lib/lp/translations/stories/standalone/xx-pofile-translate-html-tags-escape.txt	2011-07-13 06:27:29 +0000
@@ -20,7 +20,7 @@
 We are in next form page.
 
   >>> print user_browser.url
-  http://translations.launchpad.dev/ubuntu/hoary/+source/pmount/+pots/pmount/hr/+translate?start=10
+  http://translations.launchpad.dev/ubuntu/hoary/+source/pmount/+pots/pmount/hr/+translate?memo=10&start=10
 
 Let's go back to the modified message.
 

=== modified file 'lib/lp/translations/stories/standalone/xx-pofile-translate-newlines-check.txt'
--- lib/lp/translations/stories/standalone/xx-pofile-translate-newlines-check.txt	2011-07-12 01:42:11 +0000
+++ lib/lp/translations/stories/standalone/xx-pofile-translate-newlines-check.txt	2011-07-13 06:27:29 +0000
@@ -82,7 +82,7 @@
 We were redirected to the next form, the translation was accepted.
 
     >>> print browser.url
-    http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/es/+translate?batch=1&start=0
+    http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/es/+translate?batch=1
 
 Get previous page to check that the save translation is the right one.
 
@@ -112,7 +112,7 @@
 We were redirected to the next form, the translation was accepted.
 
     >>> print browser.url
-    http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/es/+translate?batch=1&start=0
+    http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/es/+translate?batch=1
 
 Get previous page to check that the save translation is the right one.
 
@@ -142,7 +142,7 @@
 We were redirected to the next form, the translation was accepted.
 
     >>> print browser.url
-    http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/es/+translate?batch=1&start=0
+    http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/es/+translate?batch=1
 
 Get previous page to check that the save translation is the right one.
 

=== modified file 'versions.cfg'
--- versions.cfg	2011-07-13 01:37:43 +0000
+++ versions.cfg	2011-07-13 06:27:29 +0000
@@ -30,7 +30,7 @@
 launchpadlib = 1.9.3
 lazr.amqp = 0.1
 lazr.authentication = 0.1.1
-lazr.batchnavigator = 1.2.2
+lazr.batchnavigator = 1.2.5
 lazr.config = 1.1.3
 lazr.delegates = 1.2.0
 lazr.enum = 1.1.3