Skip to content

Commit

Permalink
For backward compatibility, relax the requirements that transaction
Browse files Browse the repository at this point in the history
  meta data (user or description) be text
  • Loading branch information
jimfulton committed Mar 10, 2017
1 parent 60d862c commit 0398429
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 31 deletions.
9 changes: 8 additions & 1 deletion CHANGES.rst
Expand Up @@ -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)
Expand Down
42 changes: 23 additions & 19 deletions transaction/_transaction.py
Expand Up @@ -14,6 +14,7 @@
import binascii
import logging
import sys
import warnings
import weakref
import traceback

Expand All @@ -22,6 +23,7 @@
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_
Expand Down Expand Up @@ -135,19 +137,17 @@ def user(self):

@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):
return self._description

@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.
Expand Down Expand Up @@ -533,23 +533,17 @@ def abort(self):
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.
Expand Down Expand Up @@ -760,3 +754,13 @@ def __init__(self, datamanager):

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)
93 changes: 82 additions & 11 deletions transaction/tests/test__transaction.py
Expand Up @@ -36,6 +36,8 @@
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


Expand Down Expand Up @@ -992,13 +994,61 @@ def test_note(self):

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()
Expand All @@ -1010,16 +1060,37 @@ def test_setUser_explicit_path(self):
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()
Expand Down

0 comments on commit 0398429

Please sign in to comment.