Merge lp:~ricardokirkner/convoy/open-non-ascii-files into lp:convoy

[Ricardo Kirkner]

handle non-ascii data in files to be combined

[Fabián Ezequiel Gallina (fgallina)]

LGTM

review: Approve

[James Tait (jamestait)]

Yep, seems sensible.

review: Approve

[lp:~ricardokirkner/convoy/open-non-ascii-files]

26. By Ricardo Kirkner on 2014-06-27

handle non-ascii data in files to be combined

27. By Ricardo Kirkner on 2014-06-27

proper fix for handled non-ascii data in files to combine

as data is decoded into unicode as soon as it's read, data should be encoded into bytes
just before returning from the function and everything should be treated internally as unicode

28. By Ricardo Kirkner on 2014-06-27

modified tests to ensure regex code is being involved

29. By Ricardo Kirkner on 2014-06-27

removed lingering pdb

Unmerged revisions

29. By Ricardo Kirkner on 2014-06-27

removed lingering pdb

28. By Ricardo Kirkner on 2014-06-27

modified tests to ensure regex code is being involved

27. By Ricardo Kirkner on 2014-06-27

proper fix for handled non-ascii data in files to combine

as data is decoded into unicode as soon as it's read, data should be encoded into bytes
just before returning from the function and everything should be treated internally as unicode

26. By Ricardo Kirkner on 2014-06-27

handle non-ascii data in files to be combined

=== modified file 'convoy/combo.py'
--- convoy/combo.py	2013-08-20 14:48:24 +0000
+++ convoy/combo.py	2014-06-27 19:30:26 +0000
@@ -16,6 +16,7 @@
 
 
 import cgi
+import codecs
 import os
 import re
 import urlparse
@@ -54,7 +55,8 @@
     return tuple([param for param, value in params])
 
 
-def combine_files(fnames, root, resource_prefix="", rewrite_urls=True):
+def combine_files(fnames, root, resource_prefix="", rewrite_urls=True,
+                  encoding="utf-8"):
     """Combine many files into one.
 
     Returns an iterator with the combined content of all the
@@ -71,7 +73,7 @@
         if not full.startswith(root) or not os.path.exists(full):
             yield "/* [missing] */\n"
         else:
-            with open(full, "r") as f:
+            with codecs.open(full, "r", encoding=encoding) as f:
                 if file_ext == ".css" and rewrite_urls:
                     file_content = f.read()
                     src_dir = os.path.dirname(full)
@@ -96,7 +98,7 @@
                         return "url(%s)" % "/".join(
                             filter(None, [resource_prefix] + result))
                     file_content = URL_RE.sub(fix_relative_url, file_content)
-                    yield file_content
+                    yield file_content.encode(encoding)
                     yield "\n"
                 else:
                     while True:
@@ -104,7 +106,7 @@
                         if not chunk:
                             yield "\n"
                             break
-                        yield chunk
+                        yield chunk.encode(encoding)
 
 
 def combo_app(root, resource_prefix="", rewrite_urls=True):

=== modified file 'convoy/tests/test_combo.py'
--- convoy/tests/test_combo.py	2013-08-20 14:48:24 +0000
+++ convoy/tests/test_combo.py	2014-06-27 19:30:26 +0000
@@ -1,3 +1,4 @@
+# coding=utf-8
 #   Convoy is a WSGI app for loading multiple files in the same request.
 #   Copyright (C) 2010-2012  Canonical, Ltd.
 #
@@ -30,8 +31,8 @@
 
 class ComboTestBase(object):
 
-    def makeSampleFile(self, root, fname, content):
-        content = textwrap.dedent(content).strip()
+    def makeSampleFile(self, root, fname, content, encoding="utf-8"):
+        content = textwrap.dedent(content).strip().encode(encoding)
         full = os.path.join(root, fname)
         parent = os.path.dirname(full)
         if not os.path.exists(parent):
@@ -373,6 +374,68 @@
                                   root=test_dir)).strip(),
             expected)
 
+    def test_open_non_ascii_file(self):
+        """Reading files with non-ascii characters should be handled properly.
+
+        Expect files to be encoded in a utf-8 compatible encoding.
+        """
+        # include some non-ascii data
+        data = u"ŸÛÌ"
+        content = ('.foo{background-image: url("some/url");'
+                   'content: "%s";}' % data)
+        test_dir = self.makeDir()
+        files = [
+            self.makeSampleFile(
+                test_dir,
+                os.path.join("yui", "base", "base.css"),
+                content),
+        ]
+
+        expected_content = (
+            '.foo{background-image: url(yui/base/some/url);'
+            'content: "%s";}' % data.encode("utf-8"))
+        expected = """
+            /* yui/base/base.css */
+            %s
+        """ % expected_content
+        self.assertTextEquals(
+            "".join(combine_files(["yui/base/base.css"],
+                                  root=test_dir)).strip(),
+            expected)
+
+    def test_open_with_custom_encoding(self):
+        """Reading files with non-ascii characters should be handled properly.
+
+        Handle explicit encoding when provided.
+        """
+        encoding = 'iso-8859-1'
+        # include some non-ascii data
+        data = u"fóöbâr"
+        content = ('.foo{background-image: url("some/url");'
+                   'content: "%s";}' % data)
+        test_dir = self.makeDir()
+        files = [
+            self.makeSampleFile(
+                test_dir,
+                os.path.join("yui", "base", "base.css"),
+                content,
+                encoding=encoding),
+        ]
+
+        expected_content = (
+            '.foo{background-image: url(yui/base/some/url);'
+            'content: "%s";}' % data.encode(encoding))
+        expected = """
+            /* yui/base/base.css */
+            %s
+        """ % expected_content
+        self.assertTextEquals(
+            "".join(combine_files(["yui/base/base.css"],
+                                  root=test_dir,
+                                  encoding=encoding)).strip(),
+            expected)
+
+
 
 class WSGIComboTest(ComboTestBase, mocker.MockerTestCase):