Repository: scrapy/scrapy
Pull request #1086 (2015-03-19 20:02:19)

Add Response.urljoin() helper
Discussed in #548.

Curita: Ouch, I got really distracted with this PR, I'll fix those right away.

Patch:
diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst
index 302ed4f3b..5b4ced992 100644
--- a/docs/topics/request-response.rst
+++ b/docs/topics/request-response.rst
@@ -493,6 +493,18 @@ Response objects
        given new values by whichever keyword arguments are specified. The
        attribute :attr:`Response.meta` is copied by default.
 
+    .. method:: Response.urljoin(url)
+
+        Constructs an absolute url by combining the Response's :attr:`url` with
+        a possible relative url.
+
+        This is a wrapper over `urlparse.urljoin`_, it's merely an alias for
+        making this call::
+
+            urlparse.urljoin(response.url, url)
+
+.. _urlparse.urljoin: https://docs.python.org/2/library/urlparse.html#urlparse.urljoin
+
 .. _topics-request-response-ref-response-subclasses:
 
 Response subclasses
diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py
index 7ff683eb6..5b1d34c6d 100644
--- a/scrapy/http/response/__init__.py
+++ b/scrapy/http/response/__init__.py
@@ -7,6 +7,8 @@ See documentation in docs/topics/request-response.rst
 
 import copy
 
+from six.moves.urllib.parse import urljoin
+
 from scrapy.http.headers import Headers
 from scrapy.utils.trackref import object_ref
 from scrapy.http.common import obsolete_setter
@@ -53,7 +55,7 @@ class Response(object_ref):
         elif body is None:
             self._body = ''
         else:
-            raise TypeError("Response body must either str or unicode. Got: '%s'" \
+            raise TypeError("Response body must either be str or unicode. Got: '%s'" \
                 % type(body).__name__)
 
     body = property(_get_body, obsolete_setter(_set_body, 'body'))
@@ -75,3 +77,8 @@ class Response(object_ref):
             kwargs.setdefault(x, getattr(self, x))
         cls = kwargs.pop('cls', self.__class__)
         return cls(*args, **kwargs)
+
+    def urljoin(self, url):
+        """Join this Response's url with a possible relative url to form an
+        absolute interpretation of the latter."""
+        return urljoin(self.url, url)
diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py
index 14030d8e5..9a435f194 100644
--- a/scrapy/http/response/text.py
+++ b/scrapy/http/response/text.py
@@ -5,9 +5,12 @@ discovering (through HTTP headers) to base Response class.
 See documentation in docs/topics/request-response.rst
 """
 
+from six.moves.urllib.parse import urljoin
+
 from w3lib.encoding import html_to_unicode, resolve_encoding, \
     html_body_declared_encoding, http_content_type_encoding
 from scrapy.http.response import Response
+from scrapy.utils.response import get_base_url
 from scrapy.utils.python import memoizemethod_noargs
 
 
@@ -63,6 +66,11 @@ class TextResponse(Response):
             self._cached_ubody = html_to_unicode(charset, self.body)[1]
         return self._cached_ubody
 
+    def urljoin(self, url):
+        """Join this Response's url with a possible relative url to form an
+        absolute interpretation of the latter."""
+        return urljoin(get_base_url(self), url)
+
     @memoizemethod_noargs
     def _headers_encoding(self):
         content_type = self.headers.get('Content-Type')
diff --git a/scrapy/utils/response.py b/scrapy/utils/response.py
index 61f43535f..1d79ec0e3 100644
--- a/scrapy/utils/response.py
+++ b/scrapy/utils/response.py
@@ -13,7 +13,6 @@ from twisted.web import http
 from twisted.web.http import RESPONSES
 from w3lib import html
 
-from scrapy.http import HtmlResponse, TextResponse
 from scrapy.utils.decorator import deprecated
 
 
@@ -73,6 +72,7 @@ def open_in_browser(response, _openfunc=webbrowser.open):
     """Open the given response in a local web browser, populating the <base>
     tag for external links to work
     """
+    from scrapy.http import HtmlResponse, TextResponse
     # XXX: this implementation is a bit dirty and could be improved
     body = response.body
     if isinstance(response, HtmlResponse):


Test patch:
diff --git a/tests/test_http_response.py b/tests/test_http_response.py
index 26a628182..b0b5b82c4 100644
--- a/tests/test_http_response.py
+++ b/tests/test_http_response.py
@@ -113,6 +113,12 @@ class BaseResponseTest(unittest.TestCase):
         self.assertRaises(AttributeError, setattr, r, 'url', 'http://example2.com')
         self.assertRaises(AttributeError, setattr, r, 'body', 'xxx')
 
+    def test_urljoin(self):
+        """Test urljoin shortcut (only for existence, since behavior equals urljoin)"""
+        joined = self.response_class('http://www.example.com').urljoin('/test')
+        absolute = 'http://www.example.com/test'
+        self.assertEqual(joined, absolute)
+
 
 class ResponseText(BaseResponseTest):
 
@@ -295,6 +301,23 @@ class TextResponseTest(BaseResponseTest):
             response.selector.css("title::text").extract(),
         )
 
+    def test_urljoin_with_base_url(self):
+        """Test urljoin shortcut which also evaluates base-url through get_base_url()."""
+        body = '<html><body><base href="https://example.net"></body></html>'
+        joined = self.response_class('http://www.example.com', body=body).urljoin('/test')
+        absolute = 'https://example.net/test'
+        self.assertEqual(joined, absolute)
+
+        body = '<html><body><base href="/elsewhere"></body></html>'
+        joined = self.response_class('http://www.example.com', body=body).urljoin('test')
+        absolute = 'http://www.example.com/test'
+        self.assertEqual(joined, absolute)
+
+        body = '<html><body><base href="/elsewhere/"></body></html>'
+        joined = self.response_class('http://www.example.com', body=body).urljoin('test')
+        absolute = 'http://www.example.com/elsewhere/test'
+        self.assertEqual(joined, absolute)
+
 
 class HtmlResponseTest(TextResponseTest):