Merge lp:~robru/friends/quote-urls into lp:friends

[Robert Bruce Park]

Properly quote long URLs when submitting them for shortening, so that they don't break in transit.

Ok, so while I was refactoring the url-shortening logic for trunk-next, I realized that we weren't quoting the URLs to be shortened, which was resulting in some pretty bad breakage in certain cases. For example, if the URL contained a plus-sign (such as the one you're looking at *right now*), the URL shortener would interpret that as a space, and then the shortened URL wouldn't point at the real URL you wanted, but at s/+/ / instead, which obviously breaks the link.

So I fixed that by running the URLs through urllib.parse.quote before submitting them for shortening, and I included a test case to ensure that the strings really are percent-encoded before going out over the wire.

Please merge this in time for raring!

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:172
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~robru/friends/quote-urls/+merge/156100/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/friends-ci/13/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/friends-raring-amd64-ci/13

Click here to trigger a rebuild:
http://s-jenkins:8080/job/friends-ci/13/rebuild

review: Needs Fixing (continuous-integration)

[Ken VanDine (ken-vandine)]

Looks good, but needs a debian/changelog entry

review: Needs Fixing

[lp:~robru/friends/quote-urls]

173. By Robert Bruce Park on 2013-03-29

Rebase on trunk.

[Robert Bruce Park (robru)]

Ok, rebased with changelog, please merge ;-)

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:173
http://jenkins.qa.ubuntu.com/job/friends-ci/15/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/friends-raring-amd64-ci/15

Click here to trigger a rebuild:
http://s-jenkins:8080/job/friends-ci/15/rebuild

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2013-03-29 16:05:24 +0000
+++ debian/changelog	2013-03-29 16:23:20 +0000
@@ -1,6 +1,7 @@
 friends (0.1.3daily13.03.29-0ubuntu1) UNRELEASED; urgency=low
 
   * Don't delete stale avatars, just redownload periodically (LP: #1153896)
+  * Quote URLs properly during shortening.
 
  -- Robert Bruce Park <[e-mail]>  Fri, 29 Mar 2013 08:58:45 -0700
 

=== modified file 'friends/shorteners/base.py'
--- friends/shorteners/base.py	2013-02-13 02:05:37 +0000
+++ friends/shorteners/base.py	2013-03-29 16:23:20 +0000
@@ -20,6 +20,8 @@
     ]
 
 
+from urllib.parse import quote
+
 from friends.utils.http import Downloader
 
 
@@ -29,4 +31,5 @@
     fqdn = None
 
     def shorten(self, url):
-        return Downloader(self.URL_TEMPLATE.format(url)).get_string().rstrip()
+        return Downloader(
+            self.URL_TEMPLATE.format(quote(url, safe=''))).get_string().rstrip()

=== modified file 'friends/tests/test_shortener.py'
--- friends/tests/test_shortener.py	2013-03-14 19:14:03 +0000
+++ friends/tests/test_shortener.py	2013-03-29 16:23:20 +0000
@@ -105,3 +105,11 @@
     def test_is_not_shortened(self):
         # Test a URL that has not been shortened.
         self.assertFalse(lookup.is_shortened('http://www.python.org/bar'))
+
+    @mock.patch('friends.shorteners.base.Downloader')
+    def test_urls_quoted_properly(self, dl_mock):
+        lookup.lookup('tinyurl.com').shorten(
+            'http://example.com/~user/stuff/+things')
+        dl_mock.assert_called_once_with(
+            'http://tinyurl.com/api-create.php?url=http%3A%2F%2Fexample.com'
+            '%2F%7Euser%2Fstuff%2F%2Bthings')