Merge lp:~leonardr/lazr.restful/mutators-are-not-named-operations into lp:lazr.restful

[Leonard Richardson]

[Leonard Richardson (leonardr)]

This branch stops mutator methods from being published as named operations. To preserve backwards compatibility, users are allowed to specify in which version of the web service the change takes effect.

The tests in webservice-declarations.txt and the new example/multiversion/tests/operation.txt should explain the feature. webservice-declarations.txt also walks you through one place where the feature might cause someone pain (it's very unlikely, and difficult to fix, so I left it alone).

[Eleanor Berger (intellectronica)]

review: Approve (code)

=== modified file 'src/lazr/restful/NEWS.txt'
--- src/lazr/restful/NEWS.txt	2010-02-23 13:26:11 +0000
+++ src/lazr/restful/NEWS.txt	2010-02-25 21:46:13 +0000
@@ -2,6 +2,19 @@
 NEWS for lazr.restful
 =====================
 
+Development
+===========
+
+Special note: this version will break backwards compatibility in your
+web service unless you take a special step. See
+"last_version_with_named_mutator_operations" below.
+
+By default, mutator methods are no longer separately published as
+named operations. To maintain backwards compatibility (or if you just
+want this feature back), put the name of the most recent version of
+your web service in the "last_version_with_named_mutator_operations"
+field of your IWebServiceConfiguration implementation.
+
 0.9.21 (2010-02-23)
 ===================
 

=== modified file 'src/lazr/restful/declarations.py'
--- src/lazr/restful/declarations.py	2010-02-23 17:26:37 +0000
+++ src/lazr/restful/declarations.py	2010-02-25 21:46:13 +0000
@@ -561,6 +561,7 @@
                 "A field can only have one mutator method for version %s; "
                 "%s makes two." % (_version_name(version), method.__name__ ))
         mutator_annotations[version] = (method, dict(method_annotations))
+        method_annotations['is_mutator'] = True
 
 
 def _free_parameters(method, annotations):

=== modified file 'src/lazr/restful/docs/absoluteurl.txt'
--- src/lazr/restful/docs/absoluteurl.txt	2010-02-11 17:57:16 +0000
+++ src/lazr/restful/docs/absoluteurl.txt	2010-02-25 21:46:13 +0000
@@ -30,6 +30,7 @@
     ...     hostname = "hostname"
     ...     service_root_uri_prefix = "root_uri_prefix/"
     ...     active_versions = ['active_version', 'latest_version']
+    ...     last_version_with_mutator_named_operations = None
     ...     port = 1000
     ...     use_https = True
 

=== modified file 'src/lazr/restful/docs/webservice-declarations.txt'
--- src/lazr/restful/docs/webservice-declarations.txt	2010-02-23 17:26:37 +0000
+++ src/lazr/restful/docs/webservice-declarations.txt	2010-02-25 21:46:13 +0000
@@ -903,6 +903,7 @@
     ...     implements(IWebServiceConfiguration)
     ...     view_permission = "lazr.View"
     ...     active_versions = ["beta", "1.0", "2.0", "3.0"]
+    ...     last_version_with_mutator_named_operations = "1.0"
     ...     code_revision = "1.0b"
     ...     default_batch_size = 50
     ...
@@ -1259,7 +1260,9 @@
     DELETE_IBookOnSteroids_destroy_beta
 
 
-=== Destructor ===
+
+Destructor
+==========
 
 A method can be designated as a destructor for the entry. Here, the
 destroy() method is designated as the destructor for IHasText.
@@ -2265,8 +2268,8 @@
     >>> attrs['return_type']
     <lazr.restful.fields.CollectionField object...>
 
-Mutator operations
-******************
+Mutators
+********
 
 Different versions can define different mutator methods for the same field.
 
@@ -2617,3 +2620,167 @@
     annotations for version "2.0", even though it's not published in
     that version. The bad annotations are: "params", "as".
     ...
+
+Mutators as named operations
+----------------------------
+
+In earlier versions of lazr.restful, mutator methods were published as
+named operations. This behavior is now deprecated and will eventually
+be removed. But to maintain backwards compatibility, mutator methods
+are still published as named operations up to a certain point. The
+MyWebServiceConfiguration class (above) defines
+last_version_with_mutator_named_operations as '1.0', meaning that in
+'beta' and '1.0', mutator methods will be published as named
+operations, and in '2.0' and '3.0' they will not.
+
+Let's consider an entry that defines a mutator in the very first
+version of the web service and never removes it.
+
+    >>> class IBetaMutatorEntry(Interface):
+    ...     export_as_webservice_entry()
+    ...
+    ...     field = exported(TextLine(readonly=True))
+    ...
+    ...     @mutator_for(field)
+    ...     @export_write_operation()
+    ...     @operation_parameters(new_value=TextLine())
+    ...     def set_value(new_value):
+    ...         pass
+
+    >>> class BetaMutator:
+    ...     implements(IBetaMutatorEntry)
+
+    >>> module = register_test_module(
+    ...     'betamutator', IBetaMutatorEntry, BetaMutator)
+
+Here's a helper method that will look up named operation for a given
+version.
+
+    >>> from zope.interface import alsoProvides
+    >>> from lazr.restful.interfaces import IResourcePOSTOperation
+    >>> def operation_for(context, version, name):
+    ...     request = FakeRequest(version=version)
+    ...     marker = getUtility(IWebServiceVersion, name=version)
+    ...     alsoProvides(request, marker)
+    ...     return getMultiAdapter(
+    ...         (context, request), IResourcePOSTOperation, name)
+
+In the 'beta' and '1.0' versions, the lookup succeeds and returns the
+generated adapter class defined for 'beta'. These two versions publish
+"set_value" as a named POST operation.
+
+    >>> context = BetaMutator()
+    >>> operation_for(context, 'beta', 'set_value')
+    <lazr.restful.declarations.POST_IBetaMutatorEntry_set_value_beta ...>
+    >>> operation_for(context, '1.0', 'set_value')
+    <lazr.restful.declarations.POST_IBetaMutatorEntry_set_value_beta ...>
+
+In '2.0', the lookup fails, not because of anything in the definition
+of IBetaMutatorEntry, but because the web service configuration
+defines 1.0 as the last version in which mutators are published as
+named operations.
+
+    >>> operation_for(context, '2.0', 'set_value')
+    Traceback (most recent call last):
+    ...
+    ComponentLookupError: ...
+
+Here's an entry that defines a mutator method in version 2.0, after
+the cutoff point.
+
+    >>> class I20MutatorEntry(Interface):
+    ...     export_as_webservice_entry()
+    ...
+    ...     field = exported(TextLine(readonly=True))
+    ...
+    ...     @mutator_for(field)
+    ...     @export_write_operation()
+    ...     @operation_parameters(new_value=TextLine())
+    ...     @operation_for_version('2.0')
+    ...     def set_value(new_value):
+    ...         pass
+
+    >>> class Mutator20:
+    ...     implements(I20MutatorEntry)
+
+    >>> module = register_test_module(
+    ...     'mutator20', I20MutatorEntry, Mutator20)
+
+The named operation lookup never succeeds. In '1.0' it fails because
+the mutator hasn't been published yet. In '2.0' it fails because that
+version comes after the last one to publish mutators as named
+operations ('1.0').
+
+    >>> context = Mutator20()
+    >>> operation_for(context, '1.0', 'set_value')
+    Traceback (most recent call last):
+    ...
+    ComponentLookupError: ...
+
+    >>> operation_for(context, '2.0', 'set_value')
+    Traceback (most recent call last):
+    ...
+    ComponentLookupError: ...
+
+Here's one that shows a limitation of the software. This method
+defines a mutator 'set_value' for version 1.0, which will be removed
+in version 2.0. It *also* defines a named operation to be published
+as 'set_value' in version 2.0, and a third operation to be published
+as 'set_value' in version 3.0.
+
+    >>> class IMutatorPlusNamedOperationEntry(Interface):
+    ...     export_as_webservice_entry()
+    ...
+    ...     field = exported(TextLine(readonly=True))
+    ...
+    ...     @mutator_for(field)
+    ...     @export_write_operation()
+    ...     @operation_parameters(new_value=TextLine())
+    ...     @operation_for_version('1.0')
+    ...     def set_value(new_value):
+    ...         pass
+    ...
+    ...     @export_write_operation()
+    ...     @operation_parameters(new_value=TextLine())
+    ...     @export_operation_as('set_value')
+    ...     @operation_for_version('2.0')
+    ...     def not_a_mutator(new_value):
+    ...         pass
+    ...
+    ...     @export_write_operation()
+    ...     @operation_parameters(new_value=TextLine())
+    ...     @export_operation_as('set_value')
+    ...     @operation_for_version('3.0')
+    ...     def also_not_a_mutator(new_value):
+    ...         pass
+
+    >>> class MutatorPlusNamedOperation:
+    ...     implements(IMutatorPlusNamedOperationEntry)
+
+    >>> module = register_test_module(
+    ...     'multimutator', IMutatorPlusNamedOperationEntry,
+    ...     MutatorPlusNamedOperation)
+
+The mutator is accessible for version 1.0, as you'd expect.
+
+    >>> context = MutatorPlusNamedOperation()
+    >>> print operation_for(context, '1.0', 'set_value').__name__
+    POST_IMutatorPlusNamedOperationEntry_set_value_1_0
+
+But the named operation that replaces the mutator in version 1.0 is
+not accessible.
+
+    >>> operation_for(context, '2.0', 'set_value')
+    Traceback (most recent call last):
+    ...
+    ComponentLookupError: ...
+
+The named operation of the same name defined in version 3.0 _is_
+accessible.
+
+    >>> print operation_for(context, '3.0', 'set_value').__name__
+    POST_IMutatorPlusNamedOperationEntry_set_value_3_0
+
+So, in the version that gets rid of named operations for mutator
+methods, you can't define a named operation with the same name as one
+of the outgoing mutator methods.

=== modified file 'src/lazr/restful/docs/webservice-error.txt'
--- src/lazr/restful/docs/webservice-error.txt	2010-02-11 17:57:16 +0000
+++ src/lazr/restful/docs/webservice-error.txt	2010-02-25 21:46:13 +0000
@@ -17,6 +17,7 @@
     ...     implements(IWebServiceConfiguration)
     ...     show_tracebacks = False
     ...     active_versions = ['trunk']
+    ...     last_version_with_mutator_named_operations = None
     >>> webservice_configuration = SimpleWebServiceConfiguration()
     >>> sm.registerUtility(webservice_configuration)
 

=== modified file 'src/lazr/restful/example/base/root.py'
--- src/lazr/restful/example/base/root.py	2010-02-18 15:02:10 +0000
+++ src/lazr/restful/example/base/root.py	2010-02-25 21:46:13 +0000
@@ -390,6 +390,7 @@
     hostname='cookbooks.dev'
     match_batch_size=50
     active_versions=['1.0', 'devel']
+    last_version_with_mutator_named_operations=None
     use_https=False
     view_permission='lazr.restful.example.base.View'
 

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

=== modified file 'src/lazr/restful/example/wsgi/root.py'
--- src/lazr/restful/example/wsgi/root.py	2009-11-12 19:08:10 +0000
+++ src/lazr/restful/example/wsgi/root.py	2010-02-25 21:46:13 +0000
@@ -36,6 +36,7 @@
     code_revision = '1'
     active_versions = ['1.0']
     use_https = False
+    last_version_with_mutator_named_operations = None
     view_permission = 'zope.Public'
 
 

=== modified file 'src/lazr/restful/interfaces/_rest.py'
--- src/lazr/restful/interfaces/_rest.py	2010-02-11 17:57:16 +0000
+++ src/lazr/restful/interfaces/_rest.py	2010-02-25 21:46:13 +0000
@@ -448,6 +448,18 @@
 
         This list must contain at least one version name.""")
 
+    last_version_with_mutator_named_operations = TextLine(
+        default=None,
+        description=u"""In earlier versions of lazr.restful, mutator methods
+        were also published as named operations. This redundant
+        behavior is no longer enabled by default, but this setting
+        allows for backwards compatibility.
+
+        Mutator methods will also be published as named operations in
+        the version you specify here, and in any previous versions. In
+        all subsequent versions, they will not be published as named
+        operations.""")
+
     code_revision = TextLine(
         default=u"",
         description=u"""A string designating the current revision

=== modified file 'src/lazr/restful/metazcml.py'
--- src/lazr/restful/metazcml.py	2010-02-23 17:26:37 +0000
+++ src/lazr/restful/metazcml.py	2010-02-25 21:46:13 +0000
@@ -213,6 +213,19 @@
     Different versions of the web service may publish the same
     operation differently or under different names.
     """
+    # First of all, figure out when to stop publishing field mutators
+    # as named operations.
+    config = getUtility(IWebServiceConfiguration)
+    if config.last_version_with_mutator_named_operations is None:
+        no_mutator_operations_after = None
+        block_mutator_operations_as_of_version = None
+    else:
+        no_mutator_operations_after = config.active_versions.index(
+            config.last_version_with_mutator_named_operations)
+        if len(config.active_versions) > no_mutator_operations_after:
+            block_mutator_operations_as_of_version = config.active_versions[
+                no_mutator_operations_after+1]
+
     for name, method in interface.namesAndDescriptions(True):
         tag = method.queryTaggedValue(LAZR_WEBSERVICE_EXPORTED)
         if tag is None or tag['type'] not in OPERATION_TYPES:
@@ -237,6 +250,7 @@
         # version.
         previous_operation_name = None
         previous_operation_provides = None
+        operation_registered_as_mutator = False
         for version, tag in tag.stack:
             if tag['type'] == REMOVED_OPERATION_TYPE:
                 # This operation is not present in this version.
@@ -284,10 +298,29 @@
                     # that case is handled above.
                     raise AssertionError(
                         'Unknown operation type: %s' % tag['type'])
+
                 operation_name = tag.get('as')
                 if tag['type'] in ['destructor']:
                     operation_name = ''
-                factory = generate_operation_adapter(method, version)
+
+                if version is None:
+                    this_version_index = 0
+                else:
+                    this_version_index = config.active_versions.index(
+                        version)
+                if (tag.get('is_mutator', False)
+                    and no_mutator_operations_after < this_version_index):
+                    # This is a mutator method, and in this version,
+                    # mutator methods are not published as named
+                    # operations at all. Block any lookup of the named
+                    # operation from succeeding.
+                    #
+                    # This will save us from having to do another
+                    # de-registration later.
+                    factory = _mask_adapter_registration
+                    operation_registered_as_mutator = False
+                else:
+                    factory = generate_operation_adapter(method, version)
 
             # Operations are looked up by name. If the operation's
             # name has changed from the previous version to this
@@ -307,9 +340,19 @@
                 register_adapter_for_version(
                     factory, interface, version, operation_provides,
                     operation_name, context.info)
+                if tag.get('is_mutator'):
+                    operation_registered_as_mutator = True
             previous_operation_name = operation_name
             previous_operation_provides = operation_provides
 
+        if operation_registered_as_mutator:
+            # The operation was registered as a mutator, and it never
+            # got de-registered. De-register it now.
+            register_adapter_for_version(
+                _mask_adapter_registration, interface,
+                block_mutator_operations_as_of_version,
+                previous_operation_provides, previous_operation_name,
+                context.info)
 
 def _mask_adapter_registration(*args):
     """A factory function that stops an adapter lookup from succeeding.

=== modified file 'src/lazr/restful/tests/test_webservice.py'
--- src/lazr/restful/tests/test_webservice.py	2010-02-18 17:33:04 +0000
+++ src/lazr/restful/tests/test_webservice.py	2010-02-25 21:46:13 +0000
@@ -114,6 +114,7 @@
      show_tracebacks = False
      active_versions = ['1.0', '2.0']
      hostname = "webservice_test"
+     last_version_with_mutator_named_operations = None
 
      def createRequest(self, body_instream, environ):
          request = Request(body_instream, environ)