Merge lp:~leonardr/lazr.restful/check_total_size_active_on_call into lp:lazr.restful

[Leonard Richardson]

The 'first_version_with_total_size_link' configuration setting lets you give named operations one behavior in a later version (only usable by versions of lazr.restfulclient designed to handle it) while maintaining backwards compatible behavior in earlier versions. The behavior is switched on or off by the value of a helper function called is_total_size_link_active().

Previously, is_total_size_link_active() was checked when the method adapter was being generated from annotations. There are two problems with this. First, it means that named operations defined manually don't check is_total_size_link_active() unless you write that code yourself. More seriously, it means that named operations inherit the old behavior, even after 'first_version_with_total_size_link', because method adapters aren't generated for every single version.

Here's a specific example. Consider a Launchpad named operation like 'findPeople'. This operation is defined for 'beta' and never changes. Call this class 'GET_findPeople_beta'. In Launchpad, 'first_version_with_total_size_link' is 'devel'. So you think if you invoked findPeople in the 'devel' web service you'd get the old behavior. But the only adapter method is GET_findPeople_beta. When GET_findPeople_beta was being defined, is_total_size_link_active() returned false because back then, the version under consideration was 'beta'.

So invoking findPeople() will give the same results whether it's done in 'beta' or 'devel', because is_total_size_link_active() was called too early.

This branch fixes this problem by checking is_total_size_link_active() as late as possible--at runtime, right at the point where the behavior needs to diverge depending on whether we want the old behavior or the new behavior. I've added tests for both a web service where everything's defined manually (multiversion.txt) and a web service defined with annotations (examples/multiversion).

[Abel Deuring (adeuring)]

review: Approve (code)

=== modified file 'src/lazr/restful/NEWS.txt'
--- src/lazr/restful/NEWS.txt	2010-08-10 12:59:10 +0000
+++ src/lazr/restful/NEWS.txt	2010-08-13 14:46:07 +0000
@@ -2,6 +2,12 @@
 NEWS for lazr.restful
 =====================
 
+0.11.1 (2010-08-13)
+===================
+
+Fixed a bug that prevented first_version_with_total_size_link from
+working properly in a multi-version environment.
+
 0.11.0 (2010-08-10)
 ===================
 

=== modified file 'src/lazr/restful/_operation.py'
--- src/lazr/restful/_operation.py	2010-08-09 19:33:20 +0000
+++ src/lazr/restful/_operation.py	2010-08-13 14:46:07 +0000
@@ -4,7 +4,7 @@
 
 import simplejson
 
-from zope.component import getMultiAdapter, queryMultiAdapter
+from zope.component import getMultiAdapter, getUtility, queryMultiAdapter
 from zope.event import notify
 from zope.interface import Attribute, implements, providedBy
 from zope.interface.interfaces import IInterface
@@ -16,10 +16,12 @@
 from lazr.lifecycle.event import ObjectModifiedEvent
 from lazr.lifecycle.snapshot import Snapshot
 
+from lazr.restful.fields import CollectionField
 from lazr.restful.interfaces import (
     ICollection, IFieldMarshaller, IResourceDELETEOperation,
-    IResourceGETOperation, IResourcePOSTOperation)
+    IResourceGETOperation, IResourcePOSTOperation, IWebServiceConfiguration)
 from lazr.restful.interfaces import ICollectionField, IReference
+from lazr.restful.utils import is_total_size_link_active
 from lazr.restful._resource import (
     BatchingResourceMixin, CollectionResource, ResourceJSONEncoder)
 
@@ -48,7 +50,12 @@
 
     def total_size_link(self, navigator):
         """Return a link to the total size of a collection."""
-        if getattr(self, 'include_total_size', True):
+        # If the version we're being asked for is equal to or later
+        # than the version in which we started exposing
+        # total_size_link, then include it; otherwise include
+        # total_size.
+        config = getUtility(IWebServiceConfiguration)
+        if not is_total_size_link_active(self.request.version, config):
             # This is a named operation that includes the total size
             # inline rather than with a link.
             return None

=== modified file 'src/lazr/restful/declarations.py'
--- src/lazr/restful/declarations.py	2010-08-10 18:36:09 +0000
+++ src/lazr/restful/declarations.py	2010-08-13 14:46:07 +0000
@@ -1245,15 +1245,6 @@
         '_method_name': method.__name__,
         '__doc__': method.__doc__}
 
-    if isinstance(return_type, CollectionField):
-        # If the version we're being asked for is equal to or later than the
-        # version in which we started exposing total_size_link and this is a
-        # read operation, then include it, otherwise include total_size.
-        config = getUtility(IWebServiceConfiguration)
-        class_dict['include_total_size'] = not (
-            is_total_size_link_active(version, config) and
-            operation_type == 'read_operation')
-
     if operation_type == 'write_operation':
         class_dict['send_modification_event'] = True
     factory = type(name, bases, class_dict)

=== modified file 'src/lazr/restful/docs/multiversion.txt'
--- src/lazr/restful/docs/multiversion.txt	2010-08-04 18:25:21 +0000
+++ src/lazr/restful/docs/multiversion.txt	2010-08-13 14:46:07 +0000
@@ -40,6 +40,7 @@
     ...     hostname = 'api.multiversion.dev'
     ...     use_https = False
     ...     active_versions = ['beta', '1.0', 'dev']
+    ...     first_version_with_total_size_link = '1.0'
     ...     code_revision = 'test'
     ...     max_batch_size = 100
     ...     view_permission = None
@@ -52,6 +53,18 @@
     >>> from zope.component import getUtility
     >>> config = getUtility(IWebServiceConfiguration)
 
+Collections previously exposed their total size via a `total_size`
+attribute.  However, newer versions of lazr.restful expose a
+`total_size_link` intead.  To facilitate transitioning from one
+approach to the other the configuration option
+`first_version_with_total_size_link` has been added to
+IWebServiceConfiguration.
+
+In this case, `first_version_with_total_size_link` is '1.0'. This
+means that named operations in versions prior to '1.0' will always
+return a `total_size`, but named operations in '1.0' and later
+versions will return a `total_size_link` when appropriate.
+
 URL generation
 --------------
 
@@ -732,7 +745,9 @@
     >>> print simplejson.loads(field())
     111-2121
 
-We can invoke a named operation.
+We can invoke a named operation, and it returns a total_size (because
+'beta' is an earlier version than the
+first_version_with_total_size_link).
 
     >>> import simplejson
     >>> request_beta = create_web_service_request(
@@ -743,6 +758,9 @@
     >>> [contact['name'] for contact in result['entries']]
     ['Cleo Python']
 
+    >>> result['total_size']
+    1
+
     >>> request_beta = create_web_service_request(
     ...     '/beta/contact_list',
     ...     environ={'QUERY_STRING' : 'ws.op=findContacts&string=111'})
@@ -833,7 +851,9 @@
     NotFound: Object: <Contact object...>, name: u'fax'
 
 We can invoke a named operation. Note that the name of the operation
-is now 'find' (it was 'findContacts' in 'beta').
+is now 'find' (it was 'findContacts' in 'beta'). And note that
+total_size has been replaced by total_size_link, since '1.0' is the
+first_version_with_total_size_link.
 
     >>> request_10 = create_web_service_request(
     ...     '/1.0/contacts',
@@ -843,6 +863,22 @@
     >>> [contact['name'] for contact in result['entries']]
     ['Cleo Python']
 
+    >>> result['total_size']
+    Traceback (most recent call last):
+    ...
+    KeyError: 'total_size'
+
+    >>> print result['total_size_link']
+    http://.../1.0/contacts?string=Cleo&ws.op=find&ws.show=total_size
+    >>> size_request = create_web_service_request(
+    ...     '/1.0/contacts',
+    ...     environ={'QUERY_STRING' :
+    ...         'string=Cleo&ws.op=find&ws.show=total_size'})
+    >>> operation = size_request.traverse(None)
+    >>> result = simplejson.loads(operation())
+    >>> print result
+    1
+
     >>> request_10 = create_web_service_request(
     ...     '/1.0/contacts',
     ...     environ={'QUERY_STRING' : 'ws.op=find&string=111'})
@@ -952,5 +988,5 @@
     ...     environ={'QUERY_STRING' : 'ws.op=find&string=111'})
     >>> operation = request_dev.traverse(None)
     >>> result = simplejson.loads(operation())
-    >>> result['total_size']
-    0
+    >>> [entry for entry in result['entries']]
+    []

=== modified file 'src/lazr/restful/docs/webservice-declarations.txt'
--- src/lazr/restful/docs/webservice-declarations.txt	2010-08-10 11:17:52 +0000
+++ src/lazr/restful/docs/webservice-declarations.txt	2010-08-13 14:46:07 +0000
@@ -1651,51 +1651,6 @@
     AssertionError: 'IMultiVersionCollection' isn't tagged for export
     to web service version 'NoSuchVersion'.
 
-total_size_link
-~~~~~~~~~~~~~~~
-
-Collections previously exposed their total size via a `total_size` attribute.
-However, newer versions of lazr.restful expose a `total_size_link` intead.  To
-facilitate transitioning from one approach to the other the configuration
-option `first_version_with_total_size_link` has been added to
-IWebServiceConfiguration.
-
-By default the first_version_with_total_size_link is set to the earliest
-available web service version, but if you have stable versions of your web
-service you wish to maintain compatability with you can specify the version in
-which you want the new behavior to take effect in the web service
-configuration.
-
-    >>> from zope.component import getUtility
-    >>> config = getUtility(IWebServiceConfiguration)
-    >>> config.last_version_with_mutator_named_operations = '1.0'
-
-    >>> from lazr.restful.declarations import (
-    ...     export_read_operation, operation_returns_collection_of,
-    ...     operation_for_version)
-    >>> class IWithMultiVersionCollection(Interface):
-    ...     export_as_webservice_entry()
-    ...
-    ...     @operation_for_version('2.0')
-    ...     @operation_for_version('1.0')
-    ...     @operation_returns_collection_of(Interface)
-    ...     @export_read_operation()
-    ...     def method():
-    ...         """A method that returns a collection."""
-
-    >>> method = IWithMultiVersionCollection['method']
-    >>> dummy_data = None # this would be an intance that has the method
-    >>> v10 = generate_operation_adapter(method, '1.0')(dummy_data, request)
-    >>> v20 = generate_operation_adapter(method, '2.0')(dummy_data, request)
-
-We can see that version 1.0 includes the total size for backward compatability
-while version 2.0 includes a link to fetch the total size.
-
-    >>> v10.include_total_size
-    True
-    >>> v20.include_total_size
-    False
-
 Entries
 -------
 

=== modified file 'src/lazr/restful/example/multiversion/resources.py'
--- src/lazr/restful/example/multiversion/resources.py	2010-02-08 18:24:57 +0000
+++ src/lazr/restful/example/multiversion/resources.py	2010-08-13 14:46:07 +0000
@@ -14,7 +14,7 @@
     export_as_webservice_entry, export_destructor_operation,
     export_operation_as, export_read_operation, export_write_operation,
     exported, mutator_for, operation_for_version, operation_parameters,
-    operation_removed_in_version)
+    operation_removed_in_version, operation_returns_collection_of)
 
 # Our implementations of these classes can be based on the
 # implementations from the WSGI example.
@@ -84,6 +84,7 @@
     # In 1.0 and 2.0, it's published as 'byValue'
     @export_operation_as('byValue')
     @operation_parameters(value=Text())
+    @operation_returns_collection_of(IKeyValuePair)
     @export_read_operation()
     @operation_for_version('1.0')
     # This operation is not published in versions earlier than 1.0.

=== modified file 'src/lazr/restful/example/multiversion/root.py'
--- src/lazr/restful/example/multiversion/root.py	2010-02-25 17:07:16 +0000
+++ src/lazr/restful/example/multiversion/root.py	2010-08-13 14:46:07 +0000
@@ -35,6 +35,7 @@
 class WebServiceConfiguration(BaseWSGIWebServiceConfiguration):
     code_revision = '1'
     active_versions = ['beta', '1.0', '2.0', '3.0', 'trunk']
+    first_version_with_total_size_link = '2.0'
     last_version_with_mutator_named_operations = '1.0'
     use_https = False
     view_permission = 'zope.Public'

=== modified file 'src/lazr/restful/example/multiversion/tests/introduction.txt'
--- src/lazr/restful/example/multiversion/tests/introduction.txt	2010-02-10 21:44:13 +0000
+++ src/lazr/restful/example/multiversion/tests/introduction.txt	2010-08-13 14:46:07 +0000
@@ -253,7 +253,7 @@
     >>> def show_value(version, op):
     ...     url = '/pairs?ws.op=%s&value=bar' % op
     ...     body = webservice.get(url, api_version=version).jsonBody()
-    ...     return body[0]['key']
+    ...     return body['entries'][0]['key']
 
 The named operation is not published at all in the 'beta' version of
 the web service.

=== modified file 'src/lazr/restful/example/multiversion/tests/operation.txt'
--- src/lazr/restful/example/multiversion/tests/operation.txt	2010-02-25 21:43:10 +0000
+++ src/lazr/restful/example/multiversion/tests/operation.txt	2010-08-13 14:46:07 +0000
@@ -5,6 +5,46 @@
 Named operations have some special features that are too obscure to
 mention in the introductory doctest.
 
+total_size versus total_size_link
+---------------------------------
+
+In old versions of lazr.restful, named operations that return
+collections always send a 'total_size' containing the total size of a
+collection.
+
+    >>> from lazr.restful.testing.webservice import WebServiceCaller
+    >>> webservice = WebServiceCaller(domain='multiversion.dev')
+
+In the example web service, named operations always send 'total_size'
+up to version '2.0'.
+
+    >>> from zope.component import getUtility
+    >>> from lazr.restful.interfaces import IWebServiceConfiguration
+    >>> config = getUtility(IWebServiceConfiguration)
+    >>> print config.first_version_with_total_size_link
+    2.0
+
+When the 'byValue' operation is invoked in version 1.0, it returns a
+total_size.
+
+    >>> def get_collection(version, op='byValue'):
+    ...     url = '/pairs?ws.op=%s&value=bar' % op
+    ...     return webservice.get(url, api_version=version).jsonBody()
+
+    >>> print sorted(get_collection('1.0').keys())
+    [u'entries', u'start', u'total_size']
+
+The operation itself doesn't change between 1.0 and 2.0, but in
+version 2.0, the operation starts returning total_size_link.
+
+    >>> print sorted(get_collection('2.0').keys())
+    [u'entries', u'start', u'total_size_link']
+
+The same happens in 3.0.
+
+    >>> print sorted(get_collection('3.0', 'by_value').keys())
+    [u'entries', u'start', u'total_size_link']
+
 Mutators as named operations
 ----------------------------
 
@@ -12,15 +52,9 @@
 annotated the same way as named operations, and in old versions of
 lazr.restful, they were actually published as named operations.
 
-    >>> from lazr.restful.testing.webservice import WebServiceCaller
-    >>> webservice = WebServiceCaller(domain='multiversion.dev')
-
 In the example web service, mutator methods are published as named
 operations in the 'beta' and '1.0' versions.
 
-    >>> from zope.component import getUtility
-    >>> from lazr.restful.interfaces import IWebServiceConfiguration
-    >>> config = getUtility(IWebServiceConfiguration)
     >>> print config.last_version_with_mutator_named_operations
     1.0
 

=== modified file 'src/lazr/restful/version.txt'
--- src/lazr/restful/version.txt	2010-08-10 12:59:10 +0000
+++ src/lazr/restful/version.txt	2010-08-13 14:46:07 +0000
@@ -1,1 +1,1 @@
-0.11.0
+0.11.1