launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13154
[Merge] lp:~allenap/maas/apiclient-urlencode-from-maascli into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/apiclient-urlencode-from-maascli into lp:maas with lp:~allenap/maas/apiclient-httplib2 as a prerequisite.
Commit message:
Move the non-insane urlencode from maascli into apiclient, and use it.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/maas/apiclient-urlencode-from-maascli/+merge/128586
--
https://code.launchpad.net/~allenap/maas/apiclient-urlencode-from-maascli/+merge/128586
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/apiclient-urlencode-from-maascli into lp:maas.
=== modified file 'src/apiclient/maas_client.py'
--- src/apiclient/maas_client.py 2012-10-08 21:00:31 +0000
+++ src/apiclient/maas_client.py 2012-10-08 21:00:31 +0000
@@ -16,9 +16,10 @@
'MAASOAuth',
]
-from urllib import urlencode
+from collections import Mapping
from apiclient.multipart import encode_multipart_data
+from apiclient.utils import urlencode
import httplib2
import oauth.oauth as oauth
@@ -138,7 +139,7 @@
"""
url = self._make_url(path)
if params is not None and len(params) > 0:
- url += "?" + urlencode(params)
+ url += "?" + urlencode(params.items())
headers = {}
self.auth.sign_request(url, headers)
return url, headers
@@ -157,7 +158,7 @@
if 'op' in params:
params = dict(params)
op = params.pop('op')
- url += '?' + urlencode({'op': op})
+ url += '?' + urlencode([('op', op)])
body, headers = encode_multipart_data(params, {})
self.auth.sign_request(url, headers)
return url, headers, body
=== modified file 'src/apiclient/tests/test_utils.py'
--- src/apiclient/tests/test_utils.py 2012-09-12 19:56:23 +0000
+++ src/apiclient/tests/test_utils.py 2012-10-08 21:00:31 +0000
@@ -12,8 +12,19 @@
__metaclass__ = type
__all__ = []
-from apiclient.utils import ascii_url
+from urllib import unquote
+
+from apiclient.utils import (
+ ascii_url,
+ urlencode,
+ )
+from django.utils.encoding import smart_unicode
from maastesting.testcase import TestCase
+from testtools.matchers import (
+ Equals,
+ IsInstance,
+ MatchesAll,
+ )
class TestHelpers(TestCase):
@@ -27,3 +38,23 @@
self.assertEqual(
b'http://example.com/', ascii_url('http://example.com/'))
self.assertIsInstance(ascii_url('http://example.com'), bytes)
+
+ def test_urlencode_encodes_utf8_and_quotes(self):
+ # urlencode UTF-8 encodes unicode strings and applies standard query
+ # string quoting rules, and always returns a byte string.
+ data = [("=\u1234", "&\u4321")]
+ query = urlencode(data)
+ self.assertThat(
+ query, MatchesAll(
+ Equals(b"%3D%E1%88%B4=%26%E4%8C%A1"),
+ IsInstance(bytes)))
+
+ def test_urlencode_roundtrip_through_django(self):
+ # Check that urlencode's approach works with Django, as described on
+ # https://docs.djangoproject.com/en/dev/ref/unicode/.
+ data = [("=\u1234", "&\u4321")]
+ query = urlencode(data)
+ name, value = query.split(b"=")
+ name, value = unquote(name), unquote(value)
+ name, value = smart_unicode(name), smart_unicode(value)
+ self.assertEqual(data, [(name, value)])
=== modified file 'src/apiclient/utils.py'
--- src/apiclient/utils.py 2012-09-12 19:56:23 +0000
+++ src/apiclient/utils.py 2012-10-08 21:00:31 +0000
@@ -12,9 +12,11 @@
__metaclass__ = type
__all__ = [
"ascii_url",
+ "urlencode",
]
+from urllib import quote_plus
from urlparse import urlparse
@@ -26,3 +28,19 @@
netloc=urlparts.netloc.encode("idna"))
url = urlparts.geturl()
return url.encode("ascii")
+
+
+def urlencode(data):
+ """A version of `urllib.urlencode` that isn't insane.
+
+ This only cares that `data` is an iterable of iterables. Each sub-iterable
+ must be of overall length 2, i.e. a name/value pair.
+
+ Unicode strings will be encoded to UTF-8. This is what Django expects; see
+ `smart_text` in the Django documentation.
+ """
+ enc = lambda string: quote_plus(
+ string.encode("utf-8") if isinstance(string, unicode) else string)
+ return b"&".join(
+ b"%s=%s" % (enc(name), enc(value))
+ for name, value in data)
=== modified file 'src/maascli/api.py'
--- src/maascli/api.py 2012-10-08 11:18:45 +0000
+++ src/maascli/api.py 2012-10-08 21:00:31 +0000
@@ -28,7 +28,10 @@
from apiclient.maas_client import MAASOAuth
from apiclient.multipart import encode_multipart_data
-from apiclient.utils import ascii_url
+from apiclient.utils import (
+ ascii_url,
+ urlencode,
+ )
import httplib2
from maascli.command import (
Command,
@@ -39,7 +42,6 @@
handler_command_name,
parse_docstring,
safe_name,
- urlencode,
)
=== modified file 'src/maascli/tests/test_utils.py'
--- src/maascli/tests/test_utils.py 2012-10-08 15:10:58 +0000
+++ src/maascli/tests/test_utils.py 2012-10-08 21:00:31 +0000
@@ -12,16 +12,11 @@
__metaclass__ = type
__all__ = []
-from urllib import unquote
-
-from django.utils.encoding import smart_unicode
from maascli import utils
from maastesting.testcase import TestCase
from testtools.matchers import (
AfterPreprocessing,
Equals,
- IsInstance,
- MatchesAll,
MatchesListwise,
)
@@ -177,26 +172,6 @@
self.assertIsInstance(utils.ensure_trailing_slash(u"fred"), unicode)
self.assertIsInstance(utils.ensure_trailing_slash(b"fred"), bytes)
- def test_urlencode_encodes_utf8_and_quotes(self):
- # urlencode UTF-8 encodes unicode strings and applies standard query
- # string quoting rules, and always returns a byte string.
- data = [("=\u1234", "&\u4321")]
- query = utils.urlencode(data)
- self.assertThat(
- query, MatchesAll(
- Equals(b"%3D%E1%88%B4=%26%E4%8C%A1"),
- IsInstance(bytes)))
-
- def test_urlencode_roundtrip_through_django(self):
- # Check that urlencode's approach works with Django, as described on
- # https://docs.djangoproject.com/en/dev/ref/unicode/.
- data = [("=\u1234", "&\u4321")]
- query = utils.urlencode(data)
- name, value = query.split(b"=")
- name, value = unquote(name), unquote(value)
- name, value = smart_unicode(name), smart_unicode(value)
- self.assertEqual(data, [(name, value)])
-
def test_api_url(self):
transformations = {
"http://example.com/":
=== modified file 'src/maascli/utils.py'
--- src/maascli/utils.py 2012-10-08 13:34:40 +0000
+++ src/maascli/utils.py 2012-10-08 21:00:31 +0000
@@ -15,7 +15,6 @@
"handler_command_name",
"parse_docstring",
"safe_name",
- "urlencode",
]
from functools import partial
@@ -24,7 +23,6 @@
getdoc,
)
import re
-from urllib import quote_plus
from urlparse import urlparse
@@ -100,19 +98,3 @@
if re.search("/api/[0-9.]+/?$", url.path) is None:
url = url._replace(path=url.path + "api/1.0/")
return url.geturl()
-
-
-def urlencode(data):
- """A version of `urllib.urlencode` that isn't insane.
-
- This only cares that `data` is an iterable of iterables. Each sub-iterable
- must be of overall length 2, i.e. a name/value pair.
-
- Unicode strings will be encoded to UTF-8. This is what Django expects; see
- `smart_text` in the Django documentation.
- """
- enc = lambda string: quote_plus(
- string.encode("utf-8") if isinstance(string, unicode) else string)
- return b"&".join(
- b"%s=%s" % (enc(name), enc(value))
- for name, value in data)