Merge lp:~cjwatson/lazr.restful/mutator-replacement into lp:lazr.restful

[Colin Watson]

Remove limitation on immediately reinstating a mutator as a named operation.

This previously only worked if the registration code happened to encounter the replacing named operation before it encountered the replaced mutator, which ultimately depended on dict iteration order. I considered making it enforce the limitation consistently, but it was just as easy to remove the limitation.

[Ioana Lasc (ilasc)]

review: Approve

=== modified file 'src/lazr/restful/NEWS.txt'
--- src/lazr/restful/NEWS.txt	2018-02-21 15:18:34 +0000
+++ src/lazr/restful/NEWS.txt	2018-09-28 14:44:56 +0000
@@ -2,6 +2,15 @@
 NEWS for lazr.restful
 =====================
 
+0.20.2
+======
+
+Remove limitation on immediately reinstating a named operation with the same
+name as a mutator in the webservice version that gets rid of named
+operations for mutator methods.  (This was previously only unreliably
+enforced in any case, as it depended on the order of methods returned by
+zope.interface.Interface.namesAndDescriptions.)
+
 0.20.1 (2018-02-21)
 ===================
 
 
=== modified file 'src/lazr/restful/docs/webservice-declarations.txt'
--- src/lazr/restful/docs/webservice-declarations.txt	2016-02-17 01:07:21 +0000
+++ src/lazr/restful/docs/webservice-declarations.txt	2018-09-28 14:44:56 +0000
@@ -2901,14 +2901,13 @@
     >>> print entry.field
     !foo!
 
-You can't immediately reinstate a mutator operation as a named operation
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+You can 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
-as 'set_value' in version 2.0, and a third operation to be published
-as 'set_value' in version 3.0.
+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()
@@ -2950,23 +2949,17 @@
     >>> 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
-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.
-
+The named operations that replace the mutator in versions 2.0 and 3.0 are
+also accessible.
+
+    >>> print operation_for(context, '2.0', 'set_value').__class__.__name__
+    POST_IMutatorPlusNamedOperationEntry_set_value_2_0
     >>> 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.
+So, in the version that gets rid of named operations for mutator methods,
+you can immediately define a named operation with the same name as one of
+the outgoing mutator methods.
 
 Removing mutator named operations altogether
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
=== modified file 'src/lazr/restful/metazcml.py'
--- src/lazr/restful/metazcml.py	2016-11-26 00:54:04 +0000
+++ src/lazr/restful/metazcml.py	2018-09-28 14:44:56 +0000
@@ -476,6 +476,8 @@
                 no_mutator_operations_after+1]
         else:
             block_mutator_operations_as_of_version = None
+    registered_adapters = set()
+    block_mutator_operations = set()
 
     methods = interface.namesAndDescriptions(True)
     for iface in REGISTERED_CONTRIBUTORS[interface]:
@@ -522,7 +524,6 @@
         # version.
         previous_operation_name = None
         previous_operation_provides = None
-        mutator_operation_needs_to_be_blocked = False
         for version, tag in tag.stack:
             if version is None:
                 this_version_index = 0
@@ -592,7 +593,6 @@
                     # This will save us from having to do another
                     # de-registration later.
                     factory = _mask_adapter_registration
-                    mutator_operation_needs_to_be_blocked = False
                 else:
                     factory = generate_operation_adapter(method, version)
 
@@ -607,6 +607,9 @@
                     _mask_adapter_registration, interface, version,
                     previous_operation_provides, previous_operation_name,
                     context.info)
+                registered_adapters.add(
+                    (previous_operation_name, version,
+                     previous_operation_provides))
 
             # If the operation exists in this version (ie. its name is
             # not None), register it using this version's name.
@@ -614,6 +617,8 @@
                 register_adapter_for_version(
                     factory, interface, version, operation_provides,
                     operation_name, context.info)
+                registered_adapters.add(
+                    (operation_name, version, operation_provides))
                 if (tag.get('is_mutator')
                     and block_mutator_operations_as_of_version != None):
                     defined_in = this_version_index
@@ -625,21 +630,25 @@
                         # mutators stop being named operations, and
                         # the operation registration for this mutator
                         # will need to be blocked.
-                        mutator_operation_needs_to_be_blocked = True
+                        block_mutator_operations.add(
+                            (operation_name,
+                             block_mutator_operations_as_of_version,
+                             operation_provides))
                 if repository is not None:
                     repository.append(VersionedObject(version, method))
             previous_operation_name = operation_name
             previous_operation_provides = operation_provides
 
-        if mutator_operation_needs_to_be_blocked:
+    for operation_key in block_mutator_operations:
+        if operation_key not in registered_adapters:
+            operation_name, version, operation_provides = operation_key
             # 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,
-                previous_operation_provides, previous_operation_name,
-                context.info)
+                _mask_adapter_registration, interface, version,
+                operation_provides, operation_name, context.info)
+
 
 def _mask_adapter_registration(*args):
     """A factory function that stops an adapter lookup from succeeding.