Merge pull request #139 from ClusterHQ/unicode-files-135

Make Unicode files work.

Fixes #135.

docs/source/news.rst

@@ -15,6 +15,10 @@ Features:
 * :ref:`eliot.add_global_fields <add_global_fields>` allows adding fields with specific values to all Eliot messages logged by your program.
   This can be used to e.g. distinguish between log messages from different processes by including relevant identifying information.
 
+Bug fixes:
+
+* On Python 3 files that accept unicode (e.g. ``sys.stdout``) should now work.
+
 
 0.5.0
 ^^^^^

docs/source/output.rst

@@ -11,7 +11,7 @@ For example, if we want each message to be encoded in JSON and written on a new
     from eliot import add_destination
 
     def stdout(message):
-        sys.stdout.write(json.dumps(message) + b"\n")
+        sys.stdout.write(json.dumps(message) + "\n")
     add_destination(stdout)
 
 

eliot/_output.py

@@ -4,11 +4,13 @@
 
 from __future__ import unicode_literals, absolute_import
 
+import json as pyjson
+
 from characteristic import attributes
 from six import text_type as unicode, PY3
 if PY3:
-    from . import _py3json as json
-    pyjson = json
+    from . import _py3json as fast_json
+    slow_json = fast_json
 else:
     try:
         # ujson has some issues, in particular it is far too lenient on bad
@@ -17,11 +19,11 @@
         # better. We import built-in module for use by the validation code
         # path, since we want to validate messages encode in all JSON
         # encoders.
-        import ujson as json
-        import json as pyjson
+        import ujson as fast_json
+        import json as slow_json
     except ImportError:
-        import json
-        pyjson = json
+        import json as fast_json
+        slow_json = fast_json
 
 
 
@@ -32,6 +34,7 @@
 from ._util import saferepr
 
 
+
 class Destinations(object):
     """
     Manage a list of destinations for message dictionaries.
@@ -263,8 +266,8 @@ def validate(self):
 
             # Make sure we can be encoded with different JSON encoder, since
             # ujson has different behavior in some cases:
-            json.dumps(dictionary)
-            pyjson.dumps(dictionary)
+            fast_json.dumps(dictionary)
+            slow_json.dumps(dictionary)
 
 
     def serialize(self):
@@ -300,17 +303,43 @@ def reset(self):
 
 
 @attributes(["file"])
-class _FileDestination(object):
+class FileDestination(object):
     """
     Callable that writes JSON messages to a file.
 
+    On Python 3 the file may support either C{bytes} or C{unicode}.  On
+    Python 2 only C{bytes} are supported since that is what all files expect
+    in practice.
+
     @ivar file: The file to which messages will be written.
+
+    @ivar _dumps: Function that serializes an object to JSON.
+
+    @ivar _linebreak: C{"\n"} as either bytes or unicode.
     """
+
+    def __init__(self):
+        unicodeFile = False
+        if PY3:
+            try:
+                self.file.write(b"")
+            except TypeError:
+                unicodeFile = True
+
+        if unicodeFile:
+            # On Python 3 native json module outputs unicode:
+            self._dumps = pyjson.dumps
+            self._linebreak = u"\n"
+        else:
+            self._dumps = fast_json.dumps
+            self._linebreak = b"\n"
+
+
     def __call__(self, message):
         """
         @param message: A message dictionary.
         """
-        self.file.write(json.dumps(message) + b"\n")
+        self.file.write(self._dumps(message) + self._linebreak)
 
 
 
@@ -320,4 +349,4 @@ def to_file(output_file):
 
     @param output_file: A file-like object.
     """
-    Logger._destinations.add(_FileDestination(file=output_file))
+    Logger._destinations.add(FileDestination(file=output_file))

eliot/logwriter.py

@@ -17,7 +17,7 @@
     from twisted.internet.selectreactor import SelectReactor as Reactor
 
 from . import addDestination, removeDestination
-from ._output import json
+from ._output import FileDestination
 
 
 class ThreadedFileWriter(Service):
@@ -51,6 +51,7 @@ def __init__(self, logFile, reactor):
         @param reactor: The main reactor.
         """
         self._logFile = logFile
+        self._destination = FileDestination(file=logFile)
         self._reactor = Reactor()
         # Ick. See https://twistedmatrix.com/trac/ticket/6982 for real solution.
         self._reactor._registerAsIOThread = False
@@ -82,13 +83,12 @@ def stopService(self):
 
     def __call__(self, data):
         """
-        Write given bytes to the queue, to be written by the writer thread with a
-        newline added.
+        Add the data to the queue, to be serialized to JSON and written by the
+        writer thread with a newline added.
 
         @param data: C{bytes} to write to disk.
         """
-        self._reactor.callFromThread(self._logFile.write,
-                                     json.dumps(data) + b'\n')
+        self._reactor.callFromThread(self._destination, data)
 
 
     def _writer(self):

eliot/tests/test_logwriter.py

@@ -6,7 +6,11 @@
 
 import time
 import threading
-from io import BytesIO
+# Make sure to use StringIO that only accepts unicode:
+from io import BytesIO, StringIO
+from unittest import skipIf
+
+from six import PY2
 
 try:
     from zope.interface.verify import verifyClass
@@ -74,6 +78,10 @@ def close(self):
 class ThreadedFileWriterTests(TestCase):
     """
     Tests for L{ThreadedFileWriter}.
+
+    Many of these tests involve interactions across threads, so they
+    arbitrarily wait for up to 5 seconds to reduce chances of slow thread
+    switching causing the test to fail.
     """
     def test_interface(self):
         """
@@ -172,6 +180,24 @@ def test_write(self):
         self.assertEqual(f.getvalue(), b'{"hello": 123}\n')
 
 
+    @skipIf(PY2, "Python 2 files always accept bytes")
+    def test_write_unicode(self):
+        """
+        Messages passed to L{ThreadedFileWriter.write} are then written by the
+        writer thread with a newline added to files that accept unicode.
+        """
+        f = StringIO()
+        writer = ThreadedFileWriter(f, reactor)
+        writer.startService()
+        self.addCleanup(writer.stopService)
+
+        writer({"hello\u1234": 123})
+        start = time.time()
+        while not f.getvalue() and time.time() - start < 5:
+            time.sleep(0.0001)
+        self.assertEqual(f.getvalue(), '{"hello\u1234": 123}\n')
+
+
     def test_stopServiceFinishesWriting(self):
         """
         L{ThreadedFileWriter.stopService} stops the writer thread, but only after

eliot/tests/test_output.py

@@ -4,16 +4,19 @@
 
 from __future__ import unicode_literals
 
+from sys import stdout
 from unittest import TestCase, skipIf
-from io import BytesIO
+# Make sure to use StringIO that only accepts unicode:
+from io import BytesIO, StringIO
+import json as pyjson
 
-from six import PY3
+from six import PY3, PY2
 
 from zope.interface.verify import verifyClass
 
 from .._output import (
-    MemoryLogger, ILogger, Destinations, Logger, json, to_file,
-    _FileDestination,
+    MemoryLogger, ILogger, Destinations, Logger, fast_json as json, to_file,
+    FileDestination,
     )
 from .._validation import ValidationError, Field, _MessageSerializer
 from .._traceback import writeTraceback
@@ -104,7 +107,8 @@ def test_failedValidation(self):
 
     def test_JSON(self):
         """
-        L{MemoryLogger.validate} will encode the output of serialization to JSON.
+        L{MemoryLogger.validate} will encode the output of serialization to
+        JSON.
         """
         serializer = _MessageSerializer(
             [Field.forValue("message_type", "type", u"The type"),
@@ -536,25 +540,39 @@ class ToFileTests(TestCase):
     """
     def test_to_file_adds_destination(self):
         """
-        L{to_file} adds a L{_FileDestination} destination with the given file.
+        L{to_file} adds a L{FileDestination} destination with the given file.
         """
-        f = object()
+        f = stdout
         to_file(f)
-        expected = _FileDestination(file=f)
+        expected = FileDestination(file=f)
         self.addCleanup(Logger._destinations.remove, expected)
         self.assertIn(expected, Logger._destinations._destinations)
 
 
-    def test_filedestination_writes_json(self):
+    def test_filedestination_writes_json_bytes(self):
         """
-        L{_FileDestination} writes JSON-encoded messages.
+        L{FileDestination} writes JSON-encoded messages to a file that accepts
+        bytes.
         """
         message1 = {"x": 123}
         message2 = {"y": None, "x": "abc"}
-        f = BytesIO()
-        destination = _FileDestination(file=f)
+        bytes_f = BytesIO()
+        destination = FileDestination(file=bytes_f)
         destination(message1)
         destination(message2)
         self.assertEqual(
-            [json.loads(line) for line in f.getvalue().splitlines()],
+            [json.loads(line) for line in bytes_f.getvalue().splitlines()],
             [message1, message2])
+
+
+    @skipIf(PY2, "Python 2 files always accept bytes")
+    def test_filedestination_writes_json_unicode(self):
+        """
+        L{FileDestination} writes JSON-encoded messages to file that only
+        accepts Unicode.
+        """
+        message = {"x": "\u1234"}
+        unicode_f = StringIO()
+        destination = FileDestination(file=unicode_f)
+        destination(message)
+        self.assertEqual(pyjson.loads(unicode_f.getvalue()), message)

tox.ini

@@ -15,6 +15,7 @@ commands =
 basepython = pypy
 
 [testenv:py27]
+deps = ujson # Optional dependency - pypy env tests the case where it's missing
 basepython = python2.7
 
 [testenv:py33]