Merge lp:~leonardr/lazr.restful/promote-write-operation-to-mutator into lp:lazr.restful

[Leonard Richardson]

Let me explain this in terms of the class I used to test it.

    >>> class IOperationPromotedToMutator(Interface):
    ... export_as_webservice_entry()
    ...
    ... field = exported(TextLine(readonly=True))
    ...
    ... @mutator_for(field)
    ... @operation_for_version('3.0')
    ... @operation_parameters(text=TextLine())
    ... @export_write_operation()
    ... @operation_for_version('1.0')
    ... def set_value(text):
    ... pass

The example web service has four versions: beta, 1.0, 2.0, and 3.0. In 'beta', mutator methods were also published as named operations (just like in Launchpad 'beta'), but that behavior stopped in 1.0. Call 1.0 the 'cutoff version'.

The 'set_value' method is introduced in 1.0, and it's a normal write operation in 2.0, but in 3.0 it's promoted to a mutator method.

Here's the problem: this method was not available in 1.0 or 2.0. lazr.restful saw that set_value was defined as a mutator, and added a mask registration to stop it from showing up as of the 'cutoff version', 1.0. This would have been the correct behavior if set_value was defined as a mutator *before* the cutoff version, but actually it was defined as a mutator *after* the cutoff version.

lazr.restful was masking the write operation registration in the cutoff version if the method was defined as a mutator in *any* version. This branch changes it so that the operation is only masked in the cutoff version if it was defined as a mutator *before* the cutoff version. If a method is promoted to a mutator after the cutoff version, a lookup for that method will fail starting in the promotion version.

This branch also changes the versioned request marker interfaces in webservice-declarations.txt so that version n+1 is a subclass of version n. This more closely reproduces the behavior of a real lazr.restful web service, and lets me do a more realistic test.

[Данило Шеган (danilo)]

FTR:

<danilos> leonardr, I assume there are tests for mutators working correctly otherwise? (since your test only "proves" that it stops providing a method)
<leonardr> danilos: yes, there are. a mutator is just a write operation that doesn't show up in lookups
<leonardr> i could add a test that the mutator works in 3.0
<danilos> leonardr, right, thanks
 leonardr, if there are already tests for that elsewhere, no need to imho
 leonardr, just one more question: is it still possible to keep a method and make it a mutator? (i.e. have set_value keep working in 3.0)
<leonardr> danilos: no, because there's no point in having a named operation that duplicates the effects of a mutator
<leonardr> the situation we have here is where we made a mistake--this should have been a mutator method all along
 but we can't change an old version for backwards compatibility reasons
<danilos> leonardr, well, I can imagine someone wanting to keep both so you don't have to port client code, no?
 leonardr, maybe that'd be hidden by things like launchpadlib instead, so I am just wondering
<leonardr> danilos: this falls into the category of 'if you don't want to port code, don't use the new version'
<leonardr> it's an easy change to make
<danilos> leonardr, ok, as long as it's recognized as being a use-case (or not), I think it's fine; some explicit documentation might be useful, if you agree
 leonardr, anyway, with that considered, r=me

review: Approve

=== modified file 'src/lazr/restful/NEWS.txt'
--- src/lazr/restful/NEWS.txt	2011-02-02 17:55:01 +0000
+++ src/lazr/restful/NEWS.txt	2011-02-16 14:39:11 +0000
@@ -2,9 +2,15 @@
 NEWS for lazr.restful
 =====================
 
-0.16.0 (Unreleased)
+0.16.1 (2011-02-16)
 ===================
 
+Fixed a bug that prevented a write operation from being promoted to a
+mutator operation.
+
+0.16.0 (No official release)
+============================
+
 If each entry in the web service corresponds to some object on a
 website, and there's a way of converting a web service request into a
 website request, the web service will now provide website links for
@@ -16,13 +22,13 @@
 Validation errors for named operations will be properly sent to the
 client even if they contain Unicode characters. (Launchpad bug 619180.)
 
-0.15.4 (2010-01-26)
+0.15.4 (2011-01-26)
 ===================
 
 Fixed inconsistent handling of custom HTML field renderings. An
 IFieldHTMLRenderer can now return either Unicode or UTF-8.
 
-0.15.3 (2010-01-21)
+0.15.3 (2011-01-21)
 ===================
 
 lazr.restful will now complain if you try to export an IObject, as
@@ -32,7 +38,7 @@
 use IObject.
 
 
-0.15.2 (2010-01-20)
+0.15.2 (2011-01-20)
 ===================
 
 lazr.restful gives a more helpful error message when a published
@@ -41,7 +47,7 @@
 
 lazr.restful's tests now pass in Python 2.7. (Launchpad bug 691841)
 
-0.15.1 (2010-01-19)
+0.15.1 (2011-01-19)
 ===================
 
 Fixed a redirect bug when a web browser requests a representation
 
=== modified file 'src/lazr/restful/docs/webservice-declarations.txt'
--- src/lazr/restful/docs/webservice-declarations.txt	2011-01-24 22:00:56 +0000
+++ src/lazr/restful/docs/webservice-declarations.txt	2011-02-16 14:39:11 +0000
@@ -989,15 +989,19 @@
 need a marker interface for every version, registered as a utility
 under the name of the version.
 
+Each version interface subclasses the previous version's
+interface. This lets a request use a resource definition for the
+previous version if it hasn't changed since then.
+
     >>> from zope.component import getSiteManager
     >>> from lazr.restful.interfaces import IWebServiceVersion
     >>> class ITestServiceRequestBeta(IWebServiceVersion):
     ...     pass
-    >>> class ITestServiceRequest10(IWebServiceVersion):
-    ...     pass
-    >>> class ITestServiceRequest20(IWebServiceVersion):
-    ...     pass
-    >>> class ITestServiceRequest30(IWebServiceVersion):
+    >>> class ITestServiceRequest10(ITestServiceRequestBeta):
+    ...     pass
+    >>> class ITestServiceRequest20(ITestServiceRequest10):
+    ...     pass
+    >>> class ITestServiceRequest30(ITestServiceRequest20):
     ...     pass
     >>> sm = getSiteManager()
     >>> for marker, name in [(ITestServiceRequestBeta, 'beta'),
@@ -2804,6 +2808,62 @@
 Edge cases
 **********
 
+You can promote a named operation to a mutator operation
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Here's a named operation that was defined in '1.0' and promoted to a
+mutator in '3.0'.
+
+    >>> class IOperationPromotedToMutator(Interface):
+    ...     export_as_webservice_entry()
+    ...
+    ...     field = exported(TextLine(readonly=True))
+    ...
+    ...     @mutator_for(field)
+    ...     @operation_for_version('3.0')
+    ...     @operation_parameters(text=TextLine())
+    ...     @export_write_operation()
+    ...     @operation_for_version('1.0')
+    ...     def set_value(text):
+    ...         pass
+
+    >>> class OperationPromotedToMutator:
+    ...     implements(IOperationPromotedToMutator)
+
+    >>> module = register_test_module(
+    ...     'mutatorpromotion', IOperationPromotedToMutator,
+    ...     OperationPromotedToMutator)
+
+    >>> context = OperationPromotedToMutator()
+
+The operation is not available in 'beta', because it hasn't been
+defined yet.
+
+    >>> print operation_for(context, 'beta', 'set_value').__class__.__name__
+    Traceback (most recent call last):
+    ...
+    ComponentLookupError: ...
+
+The operation is available in both '1.0', and '2.0', even though
+mutator operations aren't published as named operations after
+1.0. This is because the operation doesn't become a mutator operation
+until 3.0.
+
+    >>> print operation_for(context, '1.0', 'set_value').__class__.__name__
+    POST_IOperationPromotedToMutator_set_value_1_0
+
+    >>> print operation_for(context, '2.0', 'set_value').__class__.__name__
+    POST_IOperationPromotedToMutator_set_value_1_0
+
+The operation is not available in 3.0, the version in which it becomes
+a mutator.
+
+    >>> operation_for(context, '3.0', 'set_value')
+    Traceback (most recent call last):
+    ...
+    ComponentLookupError: ...
+
+
 You can't immediately reinstate a mutator operation as a named operation
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
=== modified file 'src/lazr/restful/metazcml.py'
--- src/lazr/restful/metazcml.py	2011-01-12 05:30:51 +0000
+++ src/lazr/restful/metazcml.py	2011-02-16 14:39:11 +0000
@@ -336,8 +336,15 @@
         # version.
         previous_operation_name = None
         previous_operation_provides = None
-        operation_registered_as_mutator = False
+        mutator_operation_needs_to_be_blocked = False
+        master_stack = tag
         for version, tag in tag.stack:
+            if version is None:
+                this_version_index = 0
+            else:
+                this_version_index = config.active_versions.index(
+                    version)
+
             if tag['type'] == REMOVED_OPERATION_TYPE:
                 # This operation is not present in this version.
                 # We'll represent this by setting the operation_name
@@ -389,15 +396,9 @@
                 if tag['type'] in ['destructor']:
                     operation_name = ''
 
-                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 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
@@ -406,7 +407,7 @@
                     # This will save us from having to do another
                     # de-registration later.
                     factory = _mask_adapter_registration
-                    operation_registered_as_mutator = False
+                    mutator_operation_needs_to_be_blocked = False
                 else:
                     factory = generate_operation_adapter(method, version)
 
@@ -428,15 +429,25 @@
                 register_adapter_for_version(
                     factory, interface, version, operation_provides,
                     operation_name, context.info)
-                if tag.get('is_mutator'):
-                    operation_registered_as_mutator = True
+                if (tag.get('is_mutator')
+                    and block_mutator_operations_as_of_version != None):
+                    defined_in = this_version_index
+                    blocked_as_of = config.active_versions.index(
+                        block_mutator_operations_as_of_version)
+                    if defined_in < blocked_as_of:
+                        # In this version, this mutator is also a
+                        # named operation. But in a later version
+                        # mutators stop being named operations, and
+                        # the operation registration for this mutator
+                        # will need to be blocked.
+                        mutator_operation_needs_to_be_blocked = True
             previous_operation_name = operation_name
             previous_operation_provides = operation_provides
 
-        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.
+        if mutator_operation_needs_to_be_blocked:
+            # The operation was registered as a mutator, back in the
+            # days when mutator operations were also named operations,
+            # and it never got de-registered. De-register it now.
             register_adapter_for_version(
                 _mask_adapter_registration, interface,
                 block_mutator_operations_as_of_version,
 
=== modified file 'src/lazr/restful/version.txt'
--- src/lazr/restful/version.txt	2011-01-26 16:50:13 +0000
+++ src/lazr/restful/version.txt	2011-02-16 14:39:11 +0000
@@ -1,1 +1,1 @@
-0.16.0
+0.16.1