Merge pull request #28 from zopefoundation/meta-data-26-27

Transaction meta data cleanup

CHANGES.rst

 Changes
 =======
 
+2.0.0 (unreleased)
+------------------
+
+- The transaction ``user`` and ``description`` attributes are now
+  defined to be text (unicode) as apposed to Python the ``str`` type.
+
+- Added the ``extended_info`` transaction attribute which contains
+  transaction meta data.  (The ``_extension`` attribute is retained as
+  an alias for backward compatibility.)
+
+  The transaction interface, ``ITransaction``, now requires
+  ``extended_info`` keys to be text (unicode) and values to be
+  JSON-serializable.
+
+- Removed setUser from ITransaction.  We'll keep the method
+  undefinately, but it's unseemly in ITransaction. :)
+
+The main purpose of these changes is to tighten up the text
+specification of user, description and extended_info keys, and to give
+us more flexibility in the future for serializing extended info.  It's
+possible that these changes will be breaking, so we're also increasing
+the major version number.
+
 1.7.0 (2016-11-08)
 ------------------
 

transaction/_transaction.py

@@ -76,10 +76,10 @@ class Transaction(object):
     # savepoint to its index (see above).
     _savepoint2index = None
 
-    # Meta data.  ._extension is also metadata, but is initialized to an
+    # Meta data. extended_info is also metadata, but is initialized to an
     # emtpy dict in __init__.
-    user = ""
-    description = ""
+    _user = u""
+    _description = u""
 
     def __init__(self, synchronizers=None, manager=None):
         self.status = Status.ACTIVE
@@ -100,9 +100,9 @@ class Transaction(object):
         # manager as a key, because we can't guess whether the actual
         # resource managers will be safe to use as dict keys.
 
-        # The user, description, and _extension attributes are accessed
+        # The user, description, and extended_info attributes are accessed
         # directly by storages, leading underscore notwithstanding.
-        self._extension = {}
+        self.extended_info = {}
 
         self.log = _makeLogger()
         self.log.debug("new transaction")
@@ -118,6 +118,28 @@ class Transaction(object):
         # List of (hook, args, kws) tuples added by addAfterCommitHook().
         self._after_commit = []
 
+    @property
+    def _extension(self):
+        # for backward compatibility, since most clients used this
+        # absent any formal API.
+        return self.extended_info
+
+    @property
+    def user(self):
+        return self._user
+
+    @user.setter
+    def user(self, v):
+        self._user = v + u'' # + u'' to make sure it's unicode
+
+    @property
+    def description(self):
+        return self._description
+
+    @description.setter
+    def description(self, v):
+        self._description = v + u'' # + u'' to make sure it's unicode
+
     def isDoomed(self):
         """ See ITransaction.
         """
@@ -504,19 +526,19 @@ class Transaction(object):
         """
         text = text.strip()
         if self.description:
-            self.description += "\n" + text
+            self.description += u"\n" + text
         else:
             self.description = text
 
     def setUser(self, user_name, path="/"):
         """ See ITransaction.
         """
-        self.user = "%s %s" % (path, user_name)
+        self.user = u"%s %s" % (path, user_name)
 
     def setExtendedInfo(self, name, value):
         """ See ITransaction.
         """
-        self._extension[name] = value
+        self.extended_info[name + u''] = value # + u'' to make sure it's unicode
 
 
 # TODO: We need a better name for the adapters.

transaction/interfaces.py

@@ -105,7 +105,7 @@ class ITransaction(Interface):
         """A user name associated with the transaction.
 
         The format of the user name is defined by the application.  The value
-        is of Python type str.  Storages record the user value, as meta-data,
+        is text (unicode).  Storages record the user value, as meta-data,
         when a transaction commits.
 
         A storage may impose a limit on the size of the value; behavior is
@@ -116,7 +116,7 @@ class ITransaction(Interface):
     description = Attribute(
         """A textual description of the transaction.
 
-        The value is of Python type str.  Method note() is the intended
+        The value is text (unicode).  Method note() is the intended
         way to set the value.  Storages record the description, as meta-data,
         when a transaction commits.
 
@@ -125,6 +125,13 @@ class ITransaction(Interface):
         raise an exception, or truncate the value).
         """)
 
+    extended_info = Attribute(
+        """A dictionary containing application-defined metadata.
+
+        Keys must be text (unicode).  Values must be simple values
+        serializable with json or pickle (not instances).
+        """)
+
     def commit():
         """Finalize the transaction.
 
@@ -167,7 +174,7 @@ class ITransaction(Interface):
         """
 
     def note(text):
-        """Add text to the transaction description.
+        """Add text (unicode) to the transaction description.
 
         This modifies the `.description` attribute; see its docs for more
         detail.  First surrounding whitespace is stripped from `text`.  If
@@ -176,21 +183,17 @@ class ITransaction(Interface):
         appended to `.description`.
         """
 
-    def setUser(user_name, path="/"):
-        """Set the user name.
-
-        path should be provided if needed to further qualify the
-        identified user.  This is a convenience method used by Zope.
-        It sets the .user attribute to str(path) + " " + str(user_name).
-        This sets the `.user` attribute; see its docs for more detail.
-        """
-
     def setExtendedInfo(name, value):
         """Add extension data to the transaction.
 
-        name is the name of the extension property to set, of Python type
-        str; value must be picklable.  Multiple calls may be made to set
-        multiple extension properties, provided the names are distinct.
+        name
+          is the text (unicode) name of the extension property to set
+
+        value
+          must be picklable and json serializable (not an instance).
+
+        Multiple calls may be made to set multiple extension
+        properties, provided the names are distinct.
 
         Storages record the extension data, as meta-data, when a transaction
         commits.

transaction/tests/test__transaction.py

@@ -69,14 +69,15 @@ class TransactionTests(unittest.TestCase):
         self.assertTrue(isinstance(txn._synchronizers, WeakSet))
         self.assertEqual(len(txn._synchronizers), 0)
         self.assertTrue(txn._manager is None)
-        self.assertEqual(txn.user, "")
-        self.assertEqual(txn.description, "")
+        self.assertEqual(txn.user, u"")
+        self.assertEqual(txn.description, u"")
         self.assertTrue(txn._savepoint2index is None)
         self.assertEqual(txn._savepoint_index, 0)
         self.assertEqual(txn._resources, [])
         self.assertEqual(txn._adapters, {})
         self.assertEqual(txn._voted, {})
-        self.assertEqual(txn._extension, {})
+        self.assertEqual(txn.extended_info, {})
+        self.assertTrue(txn._extension is txn.extended_info) # legacy
         self.assertTrue(txn.log is logger)
         self.assertEqual(len(logger._log), 1)
         self.assertEqual(logger._log[0][0], 'debug')
@@ -983,33 +984,45 @@ class TransactionTests(unittest.TestCase):
         txn = self._makeOne()
         try:
             txn.note('This is a note.')
-            self.assertEqual(txn.description, 'This is a note.')
+            self.assertEqual(txn.description, u'This is a note.')
             txn.note('Another.')
-            self.assertEqual(txn.description, 'This is a note.\nAnother.')
+            self.assertEqual(txn.description, u'This is a note.\nAnother.')
         finally:
             txn.abort()
 
+    def test_description_nonascii_bytes(self):
+        txn = self._makeOne()
+        with self.assertRaises((UnicodeDecodeError, TypeError)):
+            txn.description = b'\xc2\x80'
+
     def test_setUser_default_path(self):
         txn = self._makeOne()
         txn.setUser('phreddy')
-        self.assertEqual(txn.user, '/ phreddy')
+        self.assertEqual(txn.user, u'/ phreddy')
 
     def test_setUser_explicit_path(self):
         txn = self._makeOne()
         txn.setUser('phreddy', '/bedrock')
-        self.assertEqual(txn.user, '/bedrock phreddy')
+        self.assertEqual(txn.user, u'/bedrock phreddy')
+
+    def test_user_nonascii_bytes(self):
+        txn = self._makeOne()
+        with self.assertRaises((UnicodeDecodeError, TypeError)):
+            txn.user = b'\xc2\x80'
 
     def test_setExtendedInfo_single(self):
         txn = self._makeOne()
         txn.setExtendedInfo('frob', 'qux')
-        self.assertEqual(txn._extension, {'frob': 'qux'})
+        self.assertEqual(txn.extended_info, {u'frob': 'qux'})
+        self.assertTrue(txn._extension is txn._extension) # legacy
 
     def test_setExtendedInfo_multiple(self):
         txn = self._makeOne()
         txn.setExtendedInfo('frob', 'qux')
         txn.setExtendedInfo('baz', 'spam')
         txn.setExtendedInfo('frob', 'quxxxx')
-        self.assertEqual(txn._extension, {'frob': 'quxxxx', 'baz': 'spam'})
+        self.assertEqual(txn._extension, {u'frob': 'quxxxx', u'baz': 'spam'})
+        self.assertTrue(txn._extension is txn._extension) # legacy
 
     def test_data(self):
         txn = self._makeOne()