Fix the interface resolution order.

Make ILocatedExternalDict/List extend the more modern interfaces.

Make the implementation classes keep providing the legacy interfaces to the extent possible (everything for Dict, only some for List).

In practice I don't expect this to make a difference to client code; I think registrations for the legacy interfaces that were expected to be hit for instances of ILocatedExternal* would be rare to non-existant.

Fixes #105

.travis.yml

@@ -22,8 +22,7 @@ env:
     - CFLAGS="-Ofast -pipe -fomit-frame-pointer -march=native"
     - PYTHONHASHSEED=random
     - PIP_UPGRADE_STRATEGY=eager
-    # Not Yet. See https://github.com/NextThought/nti.externalization/issues/105
-    # - ZOPE_INTERFACE_STRICT_IRO=1
+    - ZOPE_INTERFACE_STRICT_IRO=1
 
 script:
   - coverage run -m zope.testrunner --test-path=src

CHANGES.rst

@@ -3,10 +3,34 @@
 =========
 
 
-1.1.4 (unreleased)
+2.0.0 (unreleased)
 ==================
 
-- Nothing changed yet.
+- Change ``ILocatedExternalMapping``: Previously it extended the
+  legacy ``zope.interface.common.mapping.IFullMapping``. Now it
+  extends the modern ``zope.interface.common.collections.IMapping``.
+  Note that this does not require mutability unlike the older
+  interface. (The ``LocatedExternalDict`` class provided by this
+  package is fully mutable and implements ``IMutableMapping``. It also
+  continues to implement ``IFullMapping``, but use of that interface
+  is discouraged.)
+
+- Change ``ILocatedExternalSequence``: Previously it extended the
+  legacy ``zope.interface.common.sequence.ISequence``. Now it extends
+  the modern ``zope.interface.common.collections.ISequenceMapping``.
+  Note that this does not require mutability unlike the older
+  interface. (The ``LocatedExternalList`` class provided by this
+  package is fully mutable and implements ``IMutableSequence``.)
+
+- Fix the interface resolution order for ``LocatedExternalList``.
+  Previously, with zope.interface 5, it began implementing both
+  ``IMutableSequence`` (the new interface from
+  ``zope.interface.common.collections``) as well as the older
+  interface ``ISequence`` (from ``zope.interface.common.sequence``);
+  the two have inconsistent resolution orders. Now, it only implements
+  ``IMutableSequence`` and a subset of the legacy interfaces that do
+  not conflict. See `issue 105
+  <https://github.com/NextThought/nti.externalization/issues/105>`_.
 
 
 1.1.3 (2020-06-25)

setup.py

@@ -145,7 +145,7 @@ def _c(m):
 
 setup(
     name='nti.externalization',
-    version='1.1.4.dev0',
+    version='2.0.0.dev0',
     author='Jason Madden',
     author_email='jason@nextthought.com',
     description="NTI Externalization",

src/nti/externalization/_interface_cache.py

@@ -65,6 +65,9 @@ def cache_for(externalizer, ext_self): # type: (object, object) -> InterfaceCach
 
 
 def _cache_cleanUp(instances):
+    # XXX: On PyPy (3 only?) ``list(instances)`` has been
+    # seen to raise "RuntimeError: set changed size during iteration."
+    # We should probably try to run a gc.collect() before iterating it.
     for x in list(instances):
         x.__init__()
 

src/nti/externalization/interfaces.py

@@ -11,8 +11,9 @@
 
 from zope import interface
 from zope.component.interfaces import IFactory
-from zope.interface.common.mapping import IFullMapping
-from zope.interface.common.sequence import ISequence
+from zope.interface.common import collections as icollections
+from zope.interface.common import sequence as legacy_isequence
+from zope.interface.common import mapping as legacy_imapping
 from zope.interface.interfaces import IObjectEvent
 from zope.interface.interfaces import ObjectEvent
 from zope.lifecycleevent import IObjectModifiedEvent
@@ -142,22 +143,24 @@ class IExternalizedObject(interface.Interface):
     """
 
 
-class ILocatedExternalMapping(IExternalizedObject, ILocation, IFullMapping):
+class ILocatedExternalMapping(IExternalizedObject, ILocation, icollections.IMapping):
     """
     The externalization of an object as a dictionary, maintaining its location
     information.
     """
 
 
-class ILocatedExternalSequence(IExternalizedObject, ILocation, ISequence):
+class ILocatedExternalSequence(IExternalizedObject, ILocation, icollections.ISequence):
     """
     The externalization of an object as a sequence, maintaining its location
     information.
     """
 
-
+# This is defined in _base_interfaces for bootstrap reasons.
 interface.classImplements(LocatedExternalDict, ILocatedExternalMapping)
-
+# BWC: Also make the concrete class implement the legacy IFullMapping; the ILocatedExternalMapping
+# used to extend this.
+interface.classImplements(LocatedExternalDict, legacy_imapping.IFullMapping)
 
 @interface.implementer(ILocatedExternalSequence)
 class LocatedExternalList(list):
@@ -176,6 +179,14 @@ class LocatedExternalList(list):
     __acl__ = ()
     mimeType = None
 
+# BWC: Also make the concrete class implement as much of the legacy
+# ISequence as possible (which ILocatedExternalSequence used to
+# extend). We cannot actually implement it, or the legacy IReadSequence,
+# because of interface resolution order conflicts.
+interface.classImplements(LocatedExternalList, legacy_isequence.IWriteSequence)
+interface.classImplements(LocatedExternalList, legacy_isequence.IFiniteSequence)
+
+
 # Representations as strings
 
 

src/nti/externalization/tests/test_externalization.py

@@ -54,7 +54,7 @@
 
 from hamcrest import assert_that
 from hamcrest import calling
-from hamcrest import contains
+from hamcrest import contains_exactly
 from hamcrest import has_entry
 from hamcrest import has_items
 from hamcrest import has_key
@@ -132,7 +132,7 @@ class T(object):
         # Given a plain OID, we return just the plain OID
         oid = b'\x00\x00\x00\x00\x00\x00\x00\x01'
         assert_that(fromExternalOID(oid),
-                    contains(same_instance(oid), '', None))
+                    contains_exactly(same_instance(oid), '', None))
 
     def test_hookable_set_external_identifiers(self):
         assert_that(set_external_identifiers,
@@ -157,7 +157,7 @@ def test_to_external_representation_yaml(self):
         l = LocatedExternalList()
         l.append(LocatedExternalDict(k='v'))
 
-        class SubUnicode(str if bytes is not str else unicode):
+        class SubUnicode(type(u'abc')):
             pass
         l.append(LocatedExternalDict(k2=SubUnicode(u'foo')))
 

src/nti/externalization/tests/test_internalization.py

@@ -24,7 +24,7 @@
 from ..interfaces import IMimeObjectFactory
 
 from hamcrest import assert_that
-from hamcrest import contains
+from hamcrest import contains_exactly
 from hamcrest import contains_string
 from hamcrest import equal_to
 from hamcrest import greater_than_or_equal_to
@@ -665,7 +665,7 @@ def __conform__(self, iface):
         setter, bag = self._callFUT(field, [O()])
 
         setter()
-        assert_that(bag, has_property('field', contains(is_(O))))
+        assert_that(bag, has_property('field', contains_exactly(is_(O))))
 
     def test_wrong_contained_type_object_field_adapts_fails(self):
         from zope.schema.interfaces import WrongContainedType

tox.ini

@@ -12,9 +12,7 @@ deps =
      Cython >= 0.29
 setenv =
     pure: PURE_PYTHON=1
-# Not yet.
-# See https://github.com/NextThought/nti.externalization/issues/105
-#    ZOPE_INTERFACE_STRICT_IRO=1
+    ZOPE_INTERFACE_STRICT_IRO=1
 
 [testenv:coverage]
 usedevelop = true