Refactor ring.py so that both implementations can be available, and add coverage tests.

persistent/ring.py

@@ -75,8 +75,12 @@ class IRing(Interface):
         """
         Given a sequence of pairs (index, object), remove all of them from
         the ring. This should be equivalent to calling :meth:`delete` for each
-        value, but allows for a more efficient bulk deletion process. Should
-        be at least linear time (not quadratic).
+        value, but allows for a more efficient bulk deletion process.
+
+        If the index and object pairs do not match with the actual state of the
+        ring, this operation is undefined.
+
+        Should be at least linear time (not quadratic).
         """
 
     def __iter__():
@@ -85,10 +89,68 @@ class IRing(Interface):
         recently used to most recently used. Mutating the ring while an iteration
         is in progress has undefined consequences.
         """
+
+from collections import deque
+
+@implementer(IRing)
+class _DequeRing(object):
+    """
+    A ring backed by the :class:`collections.deque` class. Operations
+    are a mix of constant and linear time.
+
+    It is available on all platforms.
+    """
+
+    __slots__ = ('ring', 'ring_oids')
+
+    def __init__(self):
+
+        self.ring = deque()
+        self.ring_oids = set()
+
+    def __len__(self):
+        return len(self.ring)
+
+    def __contains__(self, pobj):
+        return pobj._p_oid in self.ring_oids
+
+    def add(self, pobj):
+        self.ring.append(pobj)
+        self.ring_oids.add(pobj._p_oid)
+
+    def delete(self, pobj):
+        # Note that we do not use self.ring.remove() because that
+        # uses equality semantics and we don't want to call the persistent
+        # object's __eq__ method (which might wake it up just after we
+        # tried to ghost it)
+        i = 0 # Using a manual numeric counter instead of enumerate() is much faster on PyPy
+        for o in self.ring:
+            if o is pobj:
+                del self.ring[i]
+                self.ring_oids.discard(pobj._p_oid)
+                return 1
+            i += 1
+
+    def move_to_head(self, pobj):
+        self.delete(pobj)
+        self.add(pobj)
+
+    def delete_all(self, indexes_and_values):
+        for ix, value in reversed(indexes_and_values):
+            del self.ring[ix]
+            self.ring_oids.discard(value._p_oid)
+
+    def __iter__(self):
+        return iter(self.ring)
+
+
 try:
     from cffi import FFI
-    import os
+except ImportError: #pragma: no cover
+    _CFFIRing = None
+else:
 
+    import os
     this_dir = os.path.dirname(os.path.abspath(__file__))
 
     ffi = FFI()
@@ -97,7 +159,7 @@ try:
 
     ring = ffi.verify("""
     #include "ring.c"
-    """, include_dirs=[os.path.dirname(os.path.abspath(__file__))])
+    """, include_dirs=[this_dir])
 
     _OGA = object.__getattribute__
     _OSA = object.__setattr__
@@ -107,6 +169,8 @@ try:
     class _CFFIRing(object):
         """
         A ring backed by a C implementation. All operations are constant time.
+
+        It is only available on platforms with ``cffi`` installed.
         """
 
         __slots__ = ('ring_home', 'ring_to_obj')
@@ -134,13 +198,11 @@ try:
             _OSA(pobj, '_Persistent__ring', node)
 
         def delete(self, pobj):
-            deleted = 0
-            node = getattr(pobj, '_Persistent__ring', None)
-            if node is not None and node.r_next:
-                ring.ring_del(node)
-                deleted = 1
-            self.ring_to_obj.pop(node, None)
-            return deleted
+            its_node = getattr(pobj, '_Persistent__ring', None)
+            our_obj = self.ring_to_obj.pop(its_node, None)
+            if its_node is not None and our_obj is not None and its_node.r_next:
+                ring.ring_del(its_node)
+                return 1
 
         def move_to_head(self, pobj):
             node = _OGA(pobj, '_Persistent__ring')
@@ -162,59 +224,5 @@ try:
             for node in self.iteritems():
                 yield ring_to_obj[node]
 
-    Ring = _CFFIRing
-
-except ImportError:
-
-    from collections import deque
-
-    @implementer(IRing)
-    class _DequeRing(object):
-        """
-        A ring backed by the :class:`collections.deque` class. Operations
-        are a mix of constant and linear time.
-        """
-
-        __slots__ = ('ring', 'ring_oids')
-
-        def __init__(self):
-
-            self.ring = deque()
-            self.ring_oids = set()
-
-        def __len__(self):
-            return len(self.ring)
-
-        def __contains__(self, pobj):
-            return pobj._p_oid in self.ring_oids
-
-        def add(self, pobj):
-            self.ring.append(pobj)
-            self.ring_oids.add(pobj._p_oid)
-
-        def delete(self, pobj):
-            # Note that we do not use self.ring.remove() because that
-            # uses equality semantics and we don't want to call the persistent
-            # object's __eq__ method (which might wake it up just after we
-            # tried to ghost it)
-            i = 0 # Using a manual numeric counter instead of enumerate() is much faster on PyPy
-            for o in self.ring:
-                if o is pobj:
-                    del self.ring[i]
-                    self.ring_oids.discard(pobj._p_oid)
-                    return 1
-                i += 1
-
-        def move_to_head(self, pobj):
-            self.delete(pobj)
-            self.add(pobj)
-
-        def delete_all(self, indexes_and_values):
-            for ix, value in reversed(indexes_and_values):
-                del self.ring[ix]
-                self.ring_oids.discard(value._p_oid)
-
-        def __iter__(self):
-            return iter(self.ring)
-
-    Ring = _DequeRing
+# Export the best available implementation
+Ring = _CFFIRing if _CFFIRing else _DequeRing

persistent/tests/test_ring.py

+##############################################################################
+#
+# Copyright (c) 2011 Zope Foundation and Contributors.
+# All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.1 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE.
+#
+##############################################################################
+import unittest
+
+from .. import ring
+
+#pylint: disable=R0904,W0212,E1101
+
+class DummyPersistent(object):
+    _p_oid = None
+
+    __next_oid = 0
+
+    @classmethod
+    def _next_oid(cls):
+        cls.__next_oid += 1
+        return cls.__next_oid
+
+    def __init__(self, oid=None):
+        if oid is None:
+            self._p_oid = self._next_oid()
+
+    def __repr__(self):
+        return "<Dummy %r>" % self._p_oid
+
+class _Ring_Base(object):
+
+    def _getTargetClass(self):
+        """Return the type of the ring to test"""
+        raise NotImplementedError()
+
+    def _makeOne(self):
+        return self._getTargetClass()()
+
+    def test_empty_len(self):
+        self.assertEqual(0, len(self._makeOne()))
+
+    def test_empty_contains(self):
+        r = self._makeOne()
+        self.assertFalse(DummyPersistent() in r)
+
+    def test_empty_iter(self):
+        self.assertEqual([], list(self._makeOne()))
+
+    def test_add_one_len1(self):
+        r = self._makeOne()
+        p = DummyPersistent()
+        r.add(p)
+        self.assertEqual(1, len(r))
+
+    def test_add_one_contains(self):
+        r = self._makeOne()
+        p = DummyPersistent()
+        r.add(p)
+        self.assertTrue(p in r)
+
+    def test_delete_one_len0(self):
+        r = self._makeOne()
+        p = DummyPersistent()
+        r.add(p)
+        r.delete(p)
+        self.assertEqual(0, len(r))
+
+    def test_delete_one_multiple(self):
+        r = self._makeOne()
+        p = DummyPersistent()
+        r.add(p)
+        r.delete(p)
+        self.assertEqual(0, len(r))
+        self.assertFalse(p in r)
+
+        r.delete(p)
+        self.assertEqual(0, len(r))
+        self.assertFalse(p in r)
+
+    def test_delete_from_wrong_ring(self):
+        r1 = self._makeOne()
+        r2 = self._makeOne()
+        p1 = DummyPersistent()
+        p2 = DummyPersistent()
+
+        r1.add(p1)
+        r2.add(p2)
+
+        r2.delete(p1)
+
+        self.assertEqual(1, len(r1))
+        self.assertEqual(1, len(r2))
+
+        self.assertEqual([p1], list(r1))
+        self.assertEqual([p2], list(r2))
+
+    def test_move_to_head(self):
+        r = self._makeOne()
+        p1 = DummyPersistent()
+        p2 = DummyPersistent()
+        p3 = DummyPersistent()
+
+        r.add(p1)
+        r.add(p2)
+        r.add(p3)
+
+        self.assertEqual([p1, p2, p3], list(r))
+        self.assertEqual(3, len(r))
+
+        r.move_to_head(p1)
+        self.assertEqual([p2, p3, p1], list(r))
+
+        r.move_to_head(p3)
+        self.assertEqual([p2, p1, p3], list(r))
+
+        r.move_to_head(p3)
+        self.assertEqual([p2, p1, p3], list(r))
+
+    def test_delete_all(self):
+        r = self._makeOne()
+        p1 = DummyPersistent()
+        p2 = DummyPersistent()
+        p3 = DummyPersistent()
+
+        r.add(p1)
+        r.add(p2)
+        r.add(p3)
+        self.assertEqual([p1, p2, p3], list(r))
+
+        r.delete_all([(0, p1), (2, p3)])
+        self.assertEqual([p2], list(r))
+        self.assertEqual(1, len(r))
+
+class DequeRingTests(unittest.TestCase, _Ring_Base):
+
+    def _getTargetClass(self):
+        return ring._DequeRing
+
+_add_to_suite = [DequeRingTests]
+
+if ring._CFFIRing:
+    class CFFIRingTests(unittest.TestCase, _Ring_Base):
+
+        def _getTargetClass(self):
+            return ring._CFFIRing
+
+    _add_to_suite.append(CFFIRingTests)
+
+def test_suite():
+    return unittest.TestSuite([unittest.makeSuite(x) for x in _add_to_suite])
+
+if __name__ == '__main__':
+    unittest.main()

tox.ini

@@ -41,6 +41,8 @@ commands =
 [testenv:coverage]
 basepython =
     python2.6
+setenv =
+    USING_CFFI = 1
 commands =
     nosetests --with-xunit --with-xcoverage
 deps =
@@ -48,6 +50,7 @@ deps =
     nose
     coverage
     nosexcover
+    cffi
 
 [testenv:docs]
 basepython =