launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24190
[Merge] ~cjwatson/launchpad:scripts-utilities-js-future-imports into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:scripts-utilities-js-future-imports into launchpad:master.
Commit message:
Convert lp.scripts.utilities.js to preferred __future__ imports
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/377136
This requires care because of WSGI's requirement for native strings in some places.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:scripts-utilities-js-future-imports into launchpad:master.
diff --git a/lib/lp/scripts/utilities/js/combinecss.py b/lib/lp/scripts/utilities/js/combinecss.py
index 2047d25..1aa1072 100755
--- a/lib/lp/scripts/utilities/js/combinecss.py
+++ b/lib/lp/scripts/utilities/js/combinecss.py
@@ -1,6 +1,8 @@
# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+from __future__ import absolute_import, print_function, unicode_literals
+
import os
import scandir
diff --git a/lib/lp/scripts/utilities/js/combo.py b/lib/lp/scripts/utilities/js/combo.py
index 4d27293..96fcbfa 100644
--- a/lib/lp/scripts/utilities/js/combo.py
+++ b/lib/lp/scripts/utilities/js/combo.py
@@ -1,6 +1,8 @@
# Copyright 2011-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+from __future__ import absolute_import, print_function, unicode_literals
+
__metaclass__ = type
import os
@@ -8,10 +10,11 @@ import urlparse
from six.moves.urllib.parse import parse_qsl
-from jsbuild import (
+from lp.scripts.utilities.js.jsbuild import (
CSSComboFile,
JSComboFile,
)
+from lp.services.encoding import wsgi_native_string
def parse_url(url):
@@ -32,7 +35,7 @@ def parse_qs(query):
return tuple([param for param, value in params])
-def combine_files(fnames, root, resource_prefix="",
+def combine_files(fnames, root, resource_prefix=b"",
minify_css=True, rewrite_urls=True):
"""Combine many files into one.
@@ -56,13 +59,12 @@ def combine_files(fnames, root, resource_prefix="",
if not full.startswith(root) or not os.path.exists(full):
yield combo.get_comment("[missing]")
else:
- f = open(full, "r")
- content = f.read()
- f.close()
+ with open(full, "rb") as f:
+ content = f.read()
yield combo.filter_file_content(content, full)
-def combo_app(root, resource_prefix="", minify_css=True, rewrite_urls=True):
+def combo_app(root, resource_prefix=b"", minify_css=True, rewrite_urls=True):
"""A simple YUI Combo Service WSGI app.
Serves any files under C{root}, setting an appropriate
@@ -78,11 +80,14 @@ def combo_app(root, resource_prefix="", minify_css=True, rewrite_urls=True):
content_type = "text/javascript"
elif fnames[0].endswith(".css"):
content_type = "text/css"
+ status = "200 OK"
+ body = combine_files(
+ fnames, root, resource_prefix, minify_css, rewrite_urls)
else:
- start_response("404 Not Found", [("Content-Type", content_type)])
- return ("Not Found",)
- start_response("200 OK", [("Content-Type", content_type)])
- return combine_files(fnames, root, resource_prefix,
- minify_css, rewrite_urls)
+ status = "404 Not Found"
+ body = (b"Not Found",)
+ response_headers = [("Content-Type", wsgi_native_string(content_type))]
+ start_response(wsgi_native_string(status), response_headers)
+ return body
return app
diff --git a/lib/lp/scripts/utilities/js/jsbuild.py b/lib/lp/scripts/utilities/js/jsbuild.py
index 5499ea7..14841c0 100644
--- a/lib/lp/scripts/utilities/js/jsbuild.py
+++ b/lib/lp/scripts/utilities/js/jsbuild.py
@@ -3,6 +3,8 @@
"""build.py - Minifies and creates the JS build directory."""
+from __future__ import absolute_import, print_function, unicode_literals
+
__metaclass__ = type
__all__ = [
'CSSComboFile',
@@ -26,9 +28,9 @@ BUILD_DIR = os.path.normpath(os.path.join(
DEFAULT_SRC_DIR = os.path.normpath(os.path.join(
HERE, os.pardir, os.pardir, os.pardir, 'app', 'javascript'))
-ESCAPE_STAR_PROPERTY_RE = re.compile(r'\*([a-zA-Z0-9_-]+):')
-UNESCAPE_STAR_PROPERTY_RE = re.compile(r'([a-zA-Z0-9_-]+)_ie_star_hack:')
-URL_RE = re.compile("url\([ \"\']*([^ \"\']+)[ \"\']*\)")
+ESCAPE_STAR_PROPERTY_RE = re.compile(br'\*([a-zA-Z0-9_-]+):')
+UNESCAPE_STAR_PROPERTY_RE = re.compile(br'([a-zA-Z0-9_-]+)_ie_star_hack:')
+URL_RE = re.compile(br"url\([ \"\']*([^ \"\']+)[ \"\']*\)")
def relative_path(from_file, to_file):
@@ -77,37 +79,33 @@ class ComboFile:
def update(self):
"""Update the file from its source files."""
- target_fh = open(self.target_file, 'w')
- try:
+ with open(self.target_file, 'wb') as target_fh:
for src_file in self.src_files:
self.log("Processing '%s'" % os.path.basename(src_file))
target_fh.write(self.get_file_header(src_file))
- fh = open(src_file)
- content = fh.read()
- fh.close()
+ with open(src_file, 'rb') as fh:
+ content = fh.read()
try:
target_fh.write(
self.filter_file_content(content, src_file))
except Exception:
os.remove(self.target_file)
raise
- finally:
- target_fh.close()
def get_comment(self, msg):
- """Return a string wrapped in a comment to be include in the output.
+ """Return a byte string wrapped in a comment to include in the output.
Can be used to help annotate the output file.
"""
- return ''
+ return b''
def get_file_header(self, path):
- """Return a string to include before outputting a file.
+ """Return a byte string to include before outputting a file.
Can be used by subclasses to output a file reference in the combined
file. Default implementation returns nothing.
"""
- return ''
+ return b''
def filter_file_content(self, file_content, path):
"""Hook to process the file content before being combined."""
@@ -122,13 +120,13 @@ class JSComboFile(ComboFile):
"""
def get_comment(self, msg):
- return "// %s\n" % msg
+ return b"// %s\n" % msg.encode("UTF-8")
def get_file_header(self, path):
return self.get_comment(relative_path(self.target_file, path))
def filter_file_content(self, file_content, path):
- return file_content + '\n'
+ return file_content + b'\n'
class CSSComboFile(ComboFile):
@@ -138,15 +136,15 @@ class CSSComboFile(ComboFile):
to the new location, and minify the result.
"""
- def __init__(self, src_files, target_file, resource_prefix="",
+ def __init__(self, src_files, target_file, resource_prefix=b"",
minify=True, rewrite_urls=True):
super(CSSComboFile, self).__init__(src_files, target_file)
- self.resource_prefix = resource_prefix.rstrip("/")
+ self.resource_prefix = resource_prefix.rstrip(b"/")
self.minify = minify
self.rewrite_urls = rewrite_urls
def get_comment(self, msg):
- return "/* %s */\n" % msg
+ return b"/* %s */\n" % msg.encode("UTF-8")
def get_file_header(self, path):
return self.get_comment(relative_path(self.target_file, path))
@@ -164,18 +162,18 @@ class CSSComboFile(ComboFile):
def fix_relative_url(match):
url = match.group(1)
# Don't modify absolute URLs or 'data:' urls.
- if (url.startswith("http") or
- url.startswith("/") or
- url.startswith("data:")):
+ if (url.startswith(b"http") or
+ url.startswith(b"/") or
+ url.startswith(b"data:")):
return match.group(0)
parts = relative_parts + url.split("/")
result = []
for part in parts:
- if part == ".." and result and result[-1] != "..":
+ if part == b".." and result and result[-1] != b"..":
result.pop(-1)
continue
result.append(part)
- return "url(%s)" % "/".join(
+ return b"url(%s)" % b"/".join(
filter(None, [self.resource_prefix] + result))
file_content = URL_RE.sub(fix_relative_url, file_content)
@@ -186,16 +184,16 @@ class CSSComboFile(ComboFile):
cssutils.ser.prefs.useMinified()
stylesheet = ESCAPE_STAR_PROPERTY_RE.sub(
- r'\1_ie_star_hack:', file_content)
+ br'\1_ie_star_hack:', file_content)
settings.set('DXImageTransform.Microsoft', True)
parser = cssutils.CSSParser(raiseExceptions=True)
css = parser.parseString(stylesheet)
stylesheet = UNESCAPE_STAR_PROPERTY_RE.sub(
- r'*\1:', css.cssText)
- return stylesheet + "\n"
+ br'*\1:', css.cssText)
+ return stylesheet + b"\n"
finally:
cssutils.setSerializer(old_serializer)
- return file_content + "\n"
+ return file_content + b"\n"
class Builder:
diff --git a/lib/lp/scripts/utilities/js/tests/test_combo.py b/lib/lp/scripts/utilities/js/tests/test_combo.py
index b1c8bb7..600e1b9 100644
--- a/lib/lp/scripts/utilities/js/tests/test_combo.py
+++ b/lib/lp/scripts/utilities/js/tests/test_combo.py
@@ -1,6 +1,8 @@
# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+from __future__ import absolute_import, print_function, unicode_literals
+
__metaclass__ = type
import os
@@ -14,6 +16,7 @@ from lp.scripts.utilities.js.combo import (
combo_app,
parse_url,
)
+from lp.services.encoding import wsgi_native_string
from lp.testing import TestCase
@@ -435,7 +438,7 @@ class TestCombo(ComboTestBase):
"".join(combine_files(["widget/assets/skins/sam/widget.css",
"editor/assets/skins/sam/editor.css"],
root=test_dir,
- resource_prefix="/static/")).strip(),
+ resource_prefix=b"/static/")).strip(),
expected)
def test_missing_file_is_ignored(self):
@@ -537,18 +540,21 @@ class TestWSGICombo(ComboTestBase):
os.path.join("event-custom", "event-custom-min.js"),
"** event-custom-min **")
- expected = "\n".join(("// yui/yui-min.js",
- "** yui-min **",
- "// oop/oop-min.js",
- "** oop-min **",
- "// event-custom/event-custom-min.js",
- "** event-custom-min **"))
+ expected = wsgi_native_string(
+ "\n".join(("// yui/yui-min.js",
+ "** yui-min **",
+ "// oop/oop-min.js",
+ "** oop-min **",
+ "// event-custom/event-custom-min.js",
+ "** event-custom-min **")))
res = self.app.get("/?" + "&".join(
["yui/yui-min.js",
"oop/oop-min.js",
"event-custom/event-custom-min.js"]), status=200)
- self.assertEqual(res.headers, [("Content-Type", "text/javascript")])
+ self.assertEqual(
+ res.headers,
+ [("Content-Type", wsgi_native_string("text/javascript"))])
self.assertEqual(res.body.strip(), expected)
def test_combo_app_sets_content_type_for_css(self):
@@ -558,15 +564,17 @@ class TestWSGICombo(ComboTestBase):
os.path.join("widget", "skin", "sam", "widget.css"),
"/* widget-skin-sam */")
- expected = "/* widget/skin/sam/widget.css */"
+ expected = wsgi_native_string("/* widget/skin/sam/widget.css */")
res = self.app.get("/?" + "&".join(
["widget/skin/sam/widget.css"]), status=200)
- self.assertEqual(res.headers, [("Content-Type", "text/css")])
+ self.assertEqual(
+ res.headers, [("Content-Type", wsgi_native_string("text/css"))])
self.assertEqual(res.body.strip(), expected)
def test_no_filename_gives_404(self):
"""If no filename is included, a 404 should be returned."""
res = self.app.get("/", status=404)
- self.assertEqual(res.headers, [("Content-Type", "text/plain")])
- self.assertEqual(res.body, "Not Found")
+ self.assertEqual(
+ res.headers, [("Content-Type", wsgi_native_string("text/plain"))])
+ self.assertEqual(res.body, wsgi_native_string("Not Found"))
diff --git a/lib/lp/services/encoding.py b/lib/lp/services/encoding.py
index 2483176..7feb308 100644
--- a/lib/lp/services/encoding.py
+++ b/lib/lp/services/encoding.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Character encoding utilities"""
@@ -8,11 +8,14 @@ __all__ = [
'escape_nonascii_uniquely',
'guess',
'is_ascii_only',
+ 'wsgi_native_string',
]
import codecs
import re
+import six
+
_boms = [
(codecs.BOM_UTF16_BE, 'utf_16_be'),
@@ -212,3 +215,18 @@ def is_ascii_only(string):
return False
else:
return True
+
+
+def wsgi_native_string(s):
+ """Make a native string suitable for use in WSGI.
+
+ PEP 3333 requires environment variables to be native strings that
+ contain only code points representable in ISO-8859-1. To support
+ porting to Python 3 via an intermediate stage of Unicode literals in
+ Python 2, we enforce this here.
+ """
+ result = six.ensure_str(s, encoding='ISO-8859-1')
+ if six.PY3 and isinstance(s, six.text_type):
+ # Ensure we're limited to ISO-8859-1.
+ result.encode('ISO-8859-1')
+ return result
diff --git a/lib/lp/services/tests/test_encoding.py b/lib/lp/services/tests/test_encoding.py
index 1d91552..4a0e235 100644
--- a/lib/lp/services/tests/test_encoding.py
+++ b/lib/lp/services/tests/test_encoding.py
@@ -1,14 +1,44 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
from doctest import (
DocTestSuite,
ELLIPSIS,
)
+import unittest
+
+import six
import lp.services.encoding
+from lp.services.encoding import wsgi_native_string
+from lp.testing import TestCase
+
+
+class TestWSGINativeString(TestCase):
+
+ def _toNative(self, s):
+ if six.PY3:
+ return s
+ else:
+ return s.encode('ISO-8859-1')
+
+ def test_not_bytes_or_unicode(self):
+ self.assertRaises(TypeError, wsgi_native_string, object())
+
+ def test_bytes_iso_8859_1(self):
+ self.assertEqual(
+ self._toNative(u'foo\xfe'), wsgi_native_string(b'foo\xfe'))
+
+ def test_unicode_iso_8859_1(self):
+ self.assertEqual(
+ self._toNative(u'foo\xfe'), wsgi_native_string(u'foo\xfe'))
+
+ def test_unicode_not_iso_8859_1(self):
+ self.assertRaises(UnicodeEncodeError, wsgi_native_string, u'foo\u2014')
def test_suite():
- suite = DocTestSuite(lp.services.encoding, optionflags=ELLIPSIS)
- return suite
+ return unittest.TestSuite((
+ unittest.TestLoader().loadTestsFromName(__name__),
+ DocTestSuite(lp.services.encoding, optionflags=ELLIPSIS),
+ ))
diff --git a/lib/lp/services/webapp/servers.py b/lib/lp/services/webapp/servers.py
index c9fc0e6..e99fb05 100644
--- a/lib/lp/services/webapp/servers.py
+++ b/lib/lp/services/webapp/servers.py
@@ -64,6 +64,7 @@ from lp.app import versioninfo
from lp.app.errors import UnexpectedFormData
import lp.layers
from lp.services.config import config
+from lp.services.encoding import wsgi_native_string
from lp.services.features import get_relevant_feature_controller
from lp.services.features.flags import NullFeatureController
from lp.services.feeds.interfaces.application import IFeedsApplication
@@ -534,21 +535,6 @@ def get_query_string_params(request):
return decoded_qs
-def wsgi_native_string(s):
- """Make a native string suitable for use in WSGI.
-
- PEP 3333 requires environment variables to be native strings that
- contain only code points representable in ISO-8859-1. To support
- porting to Python 3 via an intermediate stage of Unicode literals in
- Python 2, we enforce this here.
- """
- result = six.ensure_str(s, encoding='ISO-8859-1')
- if six.PY3 and isinstance(s, six.text_type):
- # Ensure we're limited to ISO-8859-1.
- result.encode('ISO-8859-1')
- return result
-
-
class LaunchpadBrowserRequestMixin:
"""Provides methods used for both API and web browser requests."""
diff --git a/lib/lp/services/webapp/tests/test_servers.py b/lib/lp/services/webapp/tests/test_servers.py
index 79595e6..02afdb7 100644
--- a/lib/lp/services/webapp/tests/test_servers.py
+++ b/lib/lp/services/webapp/tests/test_servers.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -21,7 +21,6 @@ from lazr.restful.testing.webservice import (
IGenericEntry,
WebServiceTestCase,
)
-import six
from zope.component import (
getGlobalSiteManager,
getUtility,
@@ -48,7 +47,6 @@ from lp.services.webapp.servers import (
WebServicePublication,
WebServiceRequestPublicationFactory,
WebServiceTestRequest,
- wsgi_native_string,
)
from lp.testing import (
EventRecorder,
@@ -355,29 +353,6 @@ class TestWebServiceRequest(WebServiceTestCase):
request.response.getHeader('Vary'), 'Accept')
-class TestWSGINativeString(TestCase):
-
- def _toNative(self, s):
- if six.PY3:
- return s
- else:
- return s.encode('ISO-8859-1')
-
- def test_not_bytes_or_unicode(self):
- self.assertRaises(TypeError, wsgi_native_string, object())
-
- def test_bytes_iso_8859_1(self):
- self.assertEqual(
- self._toNative(u'foo\xfe'), wsgi_native_string(b'foo\xfe'))
-
- def test_unicode_iso_8859_1(self):
- self.assertEqual(
- self._toNative(u'foo\xfe'), wsgi_native_string(u'foo\xfe'))
-
- def test_unicode_not_iso_8859_1(self):
- self.assertRaises(UnicodeEncodeError, wsgi_native_string, u'foo\u2014')
-
-
class TestBasicLaunchpadRequest(TestCase):
"""Tests for the base request class"""
diff --git a/lib/lp/testing/layers.py b/lib/lp/testing/layers.py
index 59617f5..65fb63e 100644
--- a/lib/lp/testing/layers.py
+++ b/lib/lp/testing/layers.py
@@ -121,6 +121,7 @@ from lp.services.config.fixture import (
)
from lp.services.database.interfaces import IStore
from lp.services.database.sqlbase import session_store
+from lp.services.encoding import wsgi_native_string
from lp.services.job.tests import celery_worker
from lp.services.librarian.model import LibraryFileAlias
from lp.services.librarianserver.testing.server import LibrarianServerFixture
@@ -147,7 +148,6 @@ from lp.services.webapp.interfaces import IOpenLaunchBag
from lp.services.webapp.servers import (
LaunchpadAccessLogger,
register_launchpad_request_publication_factories,
- wsgi_native_string,
)
import lp.services.webapp.session
from lp.testing import (
diff --git a/lib/lp/testing/pages.py b/lib/lp/testing/pages.py
index 79bfba5..1c23980 100644
--- a/lib/lp/testing/pages.py
+++ b/lib/lp/testing/pages.py
@@ -79,6 +79,7 @@ from lp.services.beautifulsoup import (
SoupStrainer,
)
from lp.services.config import config
+from lp.services.encoding import wsgi_native_string
from lp.services.oauth.interfaces import (
IOAuthConsumerSet,
OAUTH_REALM,
@@ -86,10 +87,7 @@ from lp.services.oauth.interfaces import (
from lp.services.webapp import canonical_url
from lp.services.webapp.authorization import LaunchpadPermissiveSecurityPolicy
from lp.services.webapp.interfaces import OAuthPermission
-from lp.services.webapp.servers import (
- LaunchpadTestRequest,
- wsgi_native_string,
- )
+from lp.services.webapp.servers import LaunchpadTestRequest
from lp.services.webapp.url import urlsplit
from lp.testing import (
ANONYMOUS,
Follow ups
