Merge pull request #1 from Shoobx/master

Update supported Python versions.

.gitignore

@@ -14,4 +14,4 @@ tags
 coverage
 coverage.xml
 htmlcov/
-
+.eggs

CHANGES.txt

@@ -2,10 +2,16 @@
 CHANGES
 =======
 
-2.1.1 (unreleased)
+3.0.0 (unreleased)
 ------------------
 
-- Nothing changed yet.
+- Dropped support for Python 3, since repoze.session is not
+  maintained anymore. At this point, this package is also going into
+  maintainance-only mode.
+
+- Dropped support for Python 2.6.
+
+- Fine-tune logging.
 
 
 2.1.0 (2016-04-18)

buildout.cfg

@@ -1,9 +1,6 @@
 [buildout]
 develop = .
-find-links =
-    ${buildout:directory}/repoze.session-1.0a1.tar.gz
 parts = test coverage coverage-test coverage-report python
-versions = versions
 unzip = true
 
 [python]
@@ -40,13 +37,3 @@ eggs = z3c.coverage
 scripts = coveragereport=coverage-report
 arguments = ('${buildout:directory}/coverage',
              '${buildout:directory}/coverage/report')
-
-
-[versions]
-ZODB = >= 4.0.0dev
-repoze.session = >= 1.0a1
-zope.container = >= 4.0.0a3
-zope.i18n = >= 4.0.0a4
-zope.publisher = >= 4.0.0a3
-zope.security = >= 4.0.0a5
-zope.session = >= 4.0.0a1

setup.py

@@ -37,7 +37,7 @@ def alltests():
 
 setup(
     name='cipher.session',
-    version='2.1.1.dev0',
+    version='2.2.0.dev8',
     url="http://pypi.python.org/pypi/cipher.session/",
     author='Zope Foundation and Contributors',
     author_email='zope-dev@zope.org',
@@ -55,10 +55,7 @@ def alltests():
         'Intended Audience :: Developers',
         'Programming Language :: Python',
         'Programming Language :: Python :: 2',
-        'Programming Language :: Python :: 2.6',
         'Programming Language :: Python :: 2.7',
-        'Programming Language :: Python :: 3',
-        'Programming Language :: Python :: 3.3',
         'Programming Language :: Python :: Implementation :: CPython',
         'Natural Language :: English',
         'Operating System :: OS Independent',
@@ -88,7 +85,6 @@ def alltests():
         'zope.session',
         'zope.location',
         'zope.publisher',
-        'repoze.session',
 
     ],
     tests_require = [

src/cipher/session/session.py

@@ -13,7 +13,7 @@
 ##############################################################################
 """Session handling
 """
-import pprint
+import logging
 
 import zope.interface
 import zope.component
@@ -31,6 +31,14 @@
 from cipher.session import interfaces
 from cipher.session._compat import PY3
 
+LOG = logging.getLogger('cipher.session.session')
+
+
+def formatExtraData(extra, **inData):
+    for name, data in inData.items():
+        extra[name] = repr(data)
+    return extra
+
 
 class AppendOnlyDict(PersistentMapping):
     # taken from Products.faster.appendict by Tres Seaver
@@ -59,17 +67,23 @@ def _p_resolveConflict(self, old, committed, new):
 
         o Raise ConflictError if deltas from old to old->committed collide
           with those from old->new.
+
+        o See ZODB/ConflictResolution.txt for details and dangers
         """
         # _p_resolveConflict is called with persistent state
         # we are operating against the PersistentMapping.__getstate__
         # representation, which aliases '_container' to self.data.
         if not committed['data'] or not new['data']:
+            LOG.error("Can't resolve 'clear'")
             raise ConflictError("Can't resolve 'clear'")
 
+        # save old state, messing with result_data overwrite state
+        extra = {}
+        formatExtraData(extra, old=old, committed=committed, new=new)
+
         result = old.copy()
         c_new = {}
         result_data = result['data']
-        old_data = old['data']
 
         for k, v in committed['data'].items():
             if k not in result_data:
@@ -78,8 +92,22 @@ def _p_resolveConflict(self, old, committed, new):
 
         for k, v in new['data'].items():
             if k in c_new:
-                raise ConflictError("Conflicting insert")
-            if k in old_data:
+                try:
+                    verror = False
+                    rdata_k = result_data[k]
+                    neq = (v != rdata_k)
+                    # value is not the same -> raise ConflictError
+                except ValueError:
+                    # uncomparable PersistentReferences -> raise ConflictError
+                    neq = True
+                    verror = True
+                if neq:
+                    # log everything, debugging ConflictResolution is hard
+                    formatExtraData(extra, k=k, v=v, c_new=c_new, result=result,
+                                    rdata_k=rdata_k, verror=verror)
+                    LOG.error("Conflicting insert", extra=extra)
+                    raise ConflictError("Conflicting insert")
+            if k in result_data:
                 continue
             result_data[k] = v
 
@@ -91,37 +119,31 @@ class SessionData(data.SessionData):
     # ZODB conflict resolution (to prevent write conflicts)
     # parts/inspiration taken from repoze.session
 
-    def _getData(self, state):
-        # happens that PersistentMapping was refactored to 'data' instead
-        # of '_container', be forgiving about which item we use
-        try:
-            return state['_container']
-        except KeyError:
-            return state['data']
+    def _internalResolveConflict(self, resolved, old, committed, new):
+        extra = {}
+        formatExtraData(extra, old=old, committed=committed, new=new)
+        LOG.error("Competing writes to session data:", extra=extra)
+        raise ConflictError("Competing writes to session data:")
 
     def _p_resolveConflict(self, old, committed, new):
         # dict modifiers set '_lm'.
+        resolved = dict(new)
         if committed['_lm'] != new['_lm']:
             # we are operating against the PersistentMapping.__getstate__
-            # representation, which aliases '_container' to self.data.
 
             # for this to work perfectly, you better put comparable items
             # into the session
             # if they don't compare naturally, add a __cmp__ method
-            cd = self._getData(committed)
-            nd = self._getData(new)
-
             try:
-                neq = (cd != nd)
+                neq = (committed['data'] != new['data'])
             except ValueError:
+                # uncomparable PersistentReferences? -> raise ConflictError
                 neq = True
             if neq:
-                msg = "Competing writes to session data: \n%s\n----\n%s" % (
-                        pprint.pformat(cd),
-                        pprint.pformat(nd))
-                raise ConflictError(msg)
+                # if it's a real conflict, raise ConflictError
+                # otherwise update the resolved dict with good values
+                self._internalResolveConflict(resolved, old, committed, new)
 
-        resolved = dict(new)
         invalid = committed.get('_iv') or new.get('_iv')
         if invalid:
             resolved['_iv'] = True

src/cipher/session/tests/test_appendict.py

@@ -38,7 +38,7 @@ def test___setitem__list_value_raises_TypeError(self):
         mutable = ['A', 'B']
         self.assertRaises(TypeError, aod.__setitem__, 'somekey', mutable)
 
-    def test___setitem__non_perssistent_inst_value_raises_TypeError(self):
+    def test___setitem__non_persistent_inst_value_raises_TypeError(self):
         aod = self._makeOne()
         class Mutable:
             pass
@@ -60,7 +60,7 @@ def _call_p_resolveConflict(self, old, committed, new):
         # and the return value is ALSO persistent state
         return resolved
 
-    def test__p_resolveConflict_wo_collistions(self):
+    def test__p_resolveConflict_wo_collistions1(self):
         old = self._makeOne({'a': 'A', 'b': 'B'})
         committed = old.copy()
         committed['c'] = 'C'
@@ -74,6 +74,19 @@ def test__p_resolveConflict_wo_collistions(self):
         resolved = self._call_p_resolveConflict(old, committed, new)
         self.assertEqual(resolved, merged.__getstate__())
 
+    def test__p_resolveConflict_wo_collistions2(self):
+        old = self._makeOne({'a': 'A', 'b': 'B'})
+        committed = old.copy()
+        new = old.copy()
+        new['d'] = 'D'
+
+        merged = old.copy()
+        merged.update(committed)
+        merged.update(new)
+
+        resolved = self._call_p_resolveConflict(old, committed, new)
+        self.assertEqual(resolved, merged.__getstate__())
+
     def test__p_resolveConflict_with_committed_clear(self):
         from ZODB.POSException import ConflictError
 
@@ -111,6 +124,77 @@ def test__p_resolveConflict_with_collisions(self):
         with self.assertRaises(ConflictError):
             self._call_p_resolveConflict(old, committed, new)
 
+    def test__p_resolveConflict_same_inserted(self):
+        old = self._makeOne(
+            {('sid_1', u'app.auth'): 'pers_repr_1',
+            })
+        committed = self._makeOne(
+            {('sid_1', u'app.auth'): 'pers_repr_1',
+             ('sid_2', u'app.auth'): 'pers_repr_2',
+             })
+        new = self._makeOne(
+            {('sid_1', u'app.auth'): 'pers_repr_1',
+             ('sid_2', u'app.auth'): 'pers_repr_2',
+             })
+
+        resolved = self._call_p_resolveConflict(old, committed, new)
+        self.assertEqual(resolved, new.__getstate__())
+
+    def test__p_resolveConflict_different_inserted(self):
+        from ZODB.POSException import ConflictError
+
+        old = self._makeOne(
+            {('sid_1', u'app.auth'): 'pers_repr_1',
+            })
+        committed = self._makeOne(
+            {('sid_1', u'app.auth'): 'pers_repr_1',
+             ('sid_2', u'app.auth'): 'pers_repr_2',
+             })
+        new = self._makeOne(
+            {('sid_1', u'app.auth'): 'pers_repr_1',
+             ('sid_2', u'app.auth'): 'pers_repr_3',
+             })
+
+        with self.assertRaises(ConflictError):
+            self._call_p_resolveConflict(old, committed, new)
+
+    def test__p_resolveConflict_fail_cmp(self):
+        from ZODB.POSException import ConflictError
+
+        old = self._makeOne(
+            {('sid_1', u'app.auth'): PersistentReferenceStub('PR1'),
+            })
+        committed = self._makeOne(
+            {('sid_1', u'app.auth'): PersistentReferenceStub('PR1'),
+             ('sid_2', u'app.auth'): PersistentReferenceStub('PR2'),
+             })
+        new = self._makeOne(
+            {('sid_1', u'app.auth'): PersistentReferenceStub('PR1'),
+             ('sid_2', u'app.auth'): PersistentReferenceStub('PR3'),
+             })
+
+        with self.assertRaises(ConflictError):
+            self._call_p_resolveConflict(old, committed, new)
+
+
+class PersistentReferenceStub(object):
+    def __init__(self, data):
+        self.data = data
+
+    def __ne__(self, other):
+        return self.__cmp__(other) != 0
+
+    def __cmp__(self, other):
+        if self.data == other.data:
+            return 0
+        else:
+            raise ValueError(
+                "can't reliably compare against different "
+                "PersistentReferences")
+
+    def __repr__(self):
+        return "PR(%s %s)" % (id(self), self.data)
+
 
 def test_suite():
     return unittest.TestSuite((

src/cipher/session/tests/test_data.py

@@ -21,33 +21,33 @@ def test_class_implements_ISessionData(self):
         from repoze.session.interfaces import ISessionData
         verifyClass(ISessionData, self._getTargetClass())
 
-    def test_p_resolveConflict_different_lm_different_container(self):
+    def test_p_resolveConflict_different_lm_differentdata(self):
         from ZODB.POSException import ConflictError
         sdo = self._makeOne()
         old = {}
-        committed = {'_lm':1, '_container':1}
-        new       = {'_lm':2, '_container':2}
+        committed = {'_lm':1, 'data':1}
+        new       = {'_lm':2, 'data':2}
         self.assertRaises(ConflictError, sdo._p_resolveConflict, old,
                           committed, new)
 
-    def test_p_resolveConflict_different_lm_same_container(self):
+    def test_p_resolveConflict_different_lm_samedata(self):
         sdo = self._makeOne()
         old = {}
-        committed = {'_lm':1, '_container':1, '_la':1, '_iv':1}
-        new       = {'_lm':2, '_container':1, '_la':1, '_iv':1}
+        committed = {'_lm':1, 'data':1, '_la':1, '_iv':1}
+        new       = {'_lm':2, 'data':1, '_la':1, '_iv':1}
         result = sdo._p_resolveConflict(old, committed, new)
         self.assertEqual(
             result,
-            {'_la': 1, '_container': 1, '_lm': 2, '_iv': True}
+            {'_la': 1, 'data': 1, '_lm': 2, '_iv': True}
             )
 
     def test_p_resolveConflict_same_lm(self):
         sdo = self._makeOne()
         old = {}
-        committed = {'_lm':1, '_container':1}
-        new       = {'_lm':1, '_container':1}
+        committed = {'_lm':1, 'data':1}
+        new       = {'_lm':1, 'data':1}
         result = sdo._p_resolveConflict(old, committed, new)
-        self.assertEqual(result, {'_container': 1, '_lm': 1})
+        self.assertEqual(result, {'data': 1, '_lm': 1})
 
 
     def test_p_resolveConflict_different_lm_different_data(self):

tox.ini

@@ -1,14 +1,12 @@
 [tox]
-envlist =
-    py26,py27,py33
+envlist = py27
 
 [testenv]
 commands =
     python setup.py test -q
 # without explicit deps, setup.py test will download a bunch of eggs into $PWD
 # (and it seems I can't use zope.dottedname[testing] here, so forget DRY)
 deps =
-    {toxinidir}/repoze.session-1.0a1.tar.gz
     ZODB
     zope.component
     zope.interface