For backward compatibility, relax the requirements that transaction

  meta data (user or description) be text

CHANGES.rst

@@ -4,7 +4,14 @@ Changes
 2.1.1 (unreleased)
 ------------------
 
-- Nothing changed yet.
+- For backward compatibility, relax the requirements that transaction
+  meta data (user or description) be text:
+
+  - If None is assigned, the assignment is ignored.
+
+  - If a non-text value is assigned, a warning is issued and the value
+    is converted to text. If the value is a binary string, it will be
+    decoded with the UTF-8 encoding the ``replace`` error policy.
 
 
 2.1.0 (2017-02-08)

transaction/_transaction.py

@@ -14,6 +14,7 @@
 import binascii
 import logging
 import sys
+import warnings
 import weakref
 import traceback
 
@@ -22,6 +23,7 @@ from zope.interface import implementer
 from transaction.weakset import WeakSet
 from transaction.interfaces import TransactionFailedError
 from transaction import interfaces
+from transaction._compat import binary_type
 from transaction._compat import reraise
 from transaction._compat import get_thread_ident
 from transaction._compat import native_
@@ -135,9 +137,8 @@ class Transaction(object):
 
     @user.setter
     def user(self, v):
-        if not isinstance(v, text_type):
-            raise TypeError("User must be text (unicode)")
-        self._user = v
+        if v is not None:
+            self._user = text_or_warn(v)
 
     @property
     def description(self):
@@ -145,9 +146,8 @@ class Transaction(object):
 
     @description.setter
     def description(self, v):
-        if not isinstance(v, text_type):
-            raise TypeError("Description must be text (unicode)")
-        self._description = v
+        if v is not None:
+            self._description = text_or_warn(v)
 
     def isDoomed(self):
         """ See ITransaction.
@@ -533,23 +533,17 @@ class Transaction(object):
     def note(self, text):
         """ See ITransaction.
         """
-        if not isinstance(text, text_type):
-            raise TypeError("Note must be text (unicode)")
-
-        text = text.strip()
-        if self.description:
-            self.description += u"\n" + text
-        else:
-            self.description = text
+        if text is not None:
+            text = text_or_warn(text).strip()
+            if self.description:
+                self.description += u"\n" + text
+            else:
+                self.description = text
 
     def setUser(self, user_name, path=u"/"):
         """ See ITransaction.
         """
-        if not isinstance(user_name, text_type):
-            raise TypeError("User name must be text (unicode)")
-        if not isinstance(path, text_type):
-            raise TypeError("Path must be text (unicode)")
-        self.user = u"%s %s" % (path, user_name)
+        self.user = u"%s %s" % (text_or_warn(path), text_or_warn(user_name))
 
     def setExtendedInfo(self, name, value):
         """ See ITransaction.
@@ -760,3 +754,13 @@ class NoRollbackSavepoint:
 
     def rollback(self):
         raise TypeError("Savepoints unsupported", self.datamanager)
+
+def text_or_warn(s):
+    if isinstance(s, text_type):
+        return s
+
+    warnings.warn("Expected text", DeprecationWarning, stacklevel=3)
+    if isinstance(s, binary_type):
+        return s.decode('utf-8', 'replace')
+    else:
+        return text_type(s)

transaction/tests/test__transaction.py

@@ -36,6 +36,8 @@ TODO
     add in tests for objects which are modified multiple times,
     for example an object that gets modified in multiple sub txns.
 """
+import os
+import warnings
 import unittest
 
 
@@ -992,13 +994,61 @@ class TransactionTests(unittest.TestCase):
 
     def test_note_bytes(self):
         txn = self._makeOne()
-        with self.assertRaises(TypeError):
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
             txn.note(b'haha')
+            self.assertNonTextDeprecationWarning(w)
+            self.assertEqual(txn.description, u'haha')
+
+    def test_note_None(self):
+        txn = self._makeOne()
+        self.assertEqual(u'', txn.description)
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+            txn.note(None)
+            self.assertFalse(w)
+        self.assertEqual(txn.description, u'')
+
+    def test_note_42(self):
+        txn = self._makeOne()
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+            txn.note(42)
+            self.assertNonTextDeprecationWarning(w)
+            self.assertEqual(txn.description, u'42')
+
+    def assertNonTextDeprecationWarning(self, w):
+        [w] = w
+        self.assertEqual(
+            (DeprecationWarning, "Expected text",
+             os.path.splitext(__file__)[0]),
+            (w.category, str(w.message), os.path.splitext(w.filename)[0]),
+            )
 
     def test_description_bytes(self):
         txn = self._makeOne()
-        with self.assertRaises(TypeError):
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
             txn.description = b'haha'
+            self.assertNonTextDeprecationWarning(w)
+            self.assertEqual(txn.description, u'haha')
+
+    def test_description_42(self):
+        txn = self._makeOne()
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+            txn.description = 42
+            self.assertNonTextDeprecationWarning(w)
+            self.assertEqual(txn.description, u'42')
+
+    def test_description_None(self):
+        txn = self._makeOne()
+        self.assertEqual(u'', txn.description)
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+            txn.description = None
+            self.assertFalse(w)
+        self.assertEqual(txn.description, u'')
 
     def test_setUser_default_path(self):
         txn = self._makeOne()
@@ -1010,16 +1060,37 @@ class TransactionTests(unittest.TestCase):
         txn.setUser(u'phreddy', u'/bedrock')
         self.assertEqual(txn.user, u'/bedrock phreddy')
 
-    def test_user_bytes(self):
+    def _test_user_non_text(self, user, path, expect, both=False):
         txn = self._makeOne()
-        with self.assertRaises(TypeError):
-            txn.user = b'phreddy'
-        with self.assertRaises(TypeError):
-            txn.setUser(b'phreddy', u'/bedrock')
-        with self.assertRaises(TypeError):
-            txn.setUser(u'phreddy', b'/bedrock')
-        with self.assertRaises(TypeError):
-            txn.setUser(b'phreddy')
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+            if path:
+                txn.setUser(user, path)
+            else:
+                if path is None:
+                    txn.setUser(user)
+                else:
+                    txn.user = user
+
+            if both:
+                self.assertNonTextDeprecationWarning(w[:1])
+                self.assertNonTextDeprecationWarning(w[1:])
+            else:
+                self.assertNonTextDeprecationWarning(w)
+
+        self.assertEqual(expect, txn.user)
+
+    def test_user_non_text(self, user=b'phreddy', path=b'/bedrock',
+                        expect=u"/bedrock phreddy", both=True):
+        self._test_user_non_text(b'phreddy', b'/bedrock',
+                                 u"/bedrock phreddy", True)
+        self._test_user_non_text(b'phreddy', None, u'/ phreddy')
+        self._test_user_non_text(b'phreddy', False, u'phreddy')
+        self._test_user_non_text(b'phreddy', u'/bedrock', u'/bedrock phreddy')
+        self._test_user_non_text(u'phreddy', b'/bedrock', u'/bedrock phreddy')
+        self._test_user_non_text(u'phreddy', 2, u'2 phreddy')
+        self._test_user_non_text(1, u'/bedrock', u'/bedrock 1')
+        self._test_user_non_text(1, 2, u'2 1', True)
 
     def test_setExtendedInfo_single(self):
         txn = self._makeOne()