Merge lp:~leonardr/lazr.restful/refactor-tag-request-with-version into lp:lazr.restful

[Leonard Richardson]

[Leonard Richardson (leonardr)]

Ignore the name of this branch; I took care of that feature in another branch.

This branch fixes some edge cases around a recent change to lazr.restful in which mutator methods are no longer published as named operations. The tricky part is that getting rid of the feature altogether would destroy backwards compatibility. I created a configuration variable "last_version_with_mutator_named_operations". The old behavior is maintained in that version and all previous versions. The new behavior holds after that version.

As an example, when I integrate this version of lazr.restful into Launchpad, "last_version_with_mutator_named_operations" will be "beta", and the effect will be that the 'beta' version will not change, but in the '1.0' version, method like 'transitionToStatus' that merely duplicate a mutator will no longer be present.

So, when preparing to do the Launchpad integration I noticed some bugs in edge cases. If you set l_v_w_m_n_o to None, it's supposed to disable the feature altogether, but that wasn't working. And if you set l_v_w_m_n_o to the latest active version, you're supposed to get the old behavior back, but that wasn't working either. I fixed both bugs and added tests for them.

The first changes in webservice-declarations.txt (changing __name__ to __class__.__name__) fix unrelated test failures that I believe are present in the current trunk.

[Gavin Panella (allenap)]

This looks good. Only one comment: the doc test is way too long. I don't expect you to change that, it's just my opinion.

review: Approve (code)

[Leonard Richardson (leonardr)]

I played around with the doctest some more but couldn't find a good way to reduce the size.

=== modified file 'src/lazr/restful/NEWS.txt'
--- src/lazr/restful/NEWS.txt	2010-03-03 13:05:13 +0000
+++ src/lazr/restful/NEWS.txt	2010-03-03 13:51:18 +0000
@@ -15,7 +15,7 @@
 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"
+your web service in the "last_version_with_mutator_named_operations"
 field of your IWebServiceConfiguration implementation.
 
 0.9.21 (2010-02-23)

=== modified file 'src/lazr/restful/docs/webservice-declarations.txt'
--- src/lazr/restful/docs/webservice-declarations.txt	2010-02-25 21:18:49 +0000
+++ src/lazr/restful/docs/webservice-declarations.txt	2010-03-03 13:51:18 +0000
@@ -2722,6 +2722,12 @@
     ...
     ComponentLookupError: ...
 
+Edge cases
+**********
+
+You can't immediately reinstate a mutator operation as a named operation
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
 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
@@ -2764,7 +2770,7 @@
 The mutator is accessible for version 1.0, as you'd expect.
 
     >>> context = MutatorPlusNamedOperation()
-    >>> print operation_for(context, '1.0', 'set_value').__name__
+    >>> print operation_for(context, '1.0', 'set_value').__class__.__name__
     POST_IMutatorPlusNamedOperationEntry_set_value_1_0
 
 But the named operation that replaces the mutator in version 1.0 is
@@ -2778,9 +2784,99 @@
 The named operation of the same name defined in version 3.0 _is_
 accessible.
 
-    >>> print operation_for(context, '3.0', 'set_value').__name__
+    >>> print operation_for(context, '3.0', 'set_value').__class__.__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.
+
+Removing mutator named operations altogether
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+You can remove this behavior altogether (such that mutators are never
+named operations) by setting the value of the configuration variable
+'last_version_with_mutator_named_operations' to None.
+
+    >>> config = getUtility(IWebServiceConfiguration)
+    >>> config.last_version_with_mutator_named_operations = None
+
+Here's a class identical to IBetaMutatorEntry: it defines a mutator in
+the 'beta' version of the web service. (We have to redefine the class
+to avoid conflicting registrations.)
+
+    >>> class IBetaMutatorEntry2(IBetaMutatorEntry):
+    ...     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 BetaMutator2:
+    ...     implements(IBetaMutatorEntry2)
+
+    >>> module = register_test_module(
+    ...     'betamutator2', IBetaMutatorEntry2, BetaMutator2)
+
+    >>> module = register_test_module(
+    ...     'betamutator', IBetaMutatorEntry, BetaMutator)
+
+Back when last_version_with_mutator_named_operations was '1.0', the
+'set_value' named operation on IBetaMutatorEntry was accessible in
+'beta' but not in '1.0' or later versions. Now, IBetaMutatorEntry2's
+'set_value' mutator is not even accessible in 'beta'.
+
+    >>> context = BetaMutator2()
+    >>> operation_for(context, 'beta', 'set_value')
+    Traceback (most recent call last):
+    ...
+    ComponentLookupError: ...
+
+Getting the old behavior back
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+You can bring back the old behavior (in which mutators are always
+named operations) by setting 'last_version_with_mutator_named_operations'
+to the last active version.
+
+    >>> config.last_version_with_mutator_named_operations = (
+    ...     config.active_versions[-1])
+
+Again, we have to publish a new entry class, to avoid conflicting
+registrations.
+
+    >>> class IBetaMutatorEntry3(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 BetaMutator3:
+    ...     implements(IBetaMutatorEntry3)
+
+    >>> module = register_test_module(
+    ...     'betamutator3', IBetaMutatorEntry3, BetaMutator3)
+
+Back when last_version_with_mutator_named_operations was '1.0', the
+'set_value' mutator on IBetaMutatorEntry was not accessible in any
+version past '1.0'. Now, the corresponding IBetaMutatorEntry3 mutator
+is accessible in every version.
+
+    >>> context = BetaMutator3()
+    >>> operation_for(context, 'beta', 'set_value')
+    <...POST_IBetaMutatorEntry3_set_value_beta...>
+    >>> operation_for(context, '1.0', 'set_value')
+    <...POST_IBetaMutatorEntry3_set_value_beta...>
+    >>> operation_for(context, '2.0', 'set_value')
+    <...POST_IBetaMutatorEntry3_set_value_beta...>
+    >>> operation_for(context, '3.0', 'set_value')
+    <...POST_IBetaMutatorEntry3_set_value_beta...>

=== modified file 'src/lazr/restful/metazcml.py'
--- src/lazr/restful/metazcml.py	2010-02-25 19:22:00 +0000
+++ src/lazr/restful/metazcml.py	2010-03-03 13:51:18 +0000
@@ -222,9 +222,11 @@
     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:
+        if len(config.active_versions) > no_mutator_operations_after+1:
             block_mutator_operations_as_of_version = config.active_versions[
                 no_mutator_operations_after+1]
+        else:
+            block_mutator_operations_as_of_version = None
 
     for name, method in interface.namesAndDescriptions(True):
         tag = method.queryTaggedValue(LAZR_WEBSERVICE_EXPORTED)
@@ -309,7 +311,9 @@
                     this_version_index = config.active_versions.index(
                         version)
                 if (tag.get('is_mutator', False)
-                    and no_mutator_operations_after < this_version_index):
+                    and (no_mutator_operations_after is None
+                         or 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
@@ -345,7 +349,8 @@
             previous_operation_name = operation_name
             previous_operation_provides = operation_provides
 
-        if operation_registered_as_mutator:
+        if (operation_registered_as_mutator and
+            block_mutator_operations_as_of_version is not None):
             # The operation was registered as a mutator, and it never
             # got de-registered. De-register it now.
             register_adapter_for_version(