From 7da79b90fe79582f567b11f93444ac446eb15970 Mon Sep 17 00:00:00 2001 From: Pablo Hoffman Date: Wed, 8 Sep 2010 00:15:11 -0300 Subject: [PATCH] Make url/body attributes of Request/Response objects read-only - use replace() to change them. Deprecation warning left for backwards compatibilty. --- docs/topics/request-response.rst | 14 +++++++++++++- scrapy/contrib_exp/crawlspider/reqext.py | 9 ++------- scrapy/contrib_exp/crawlspider/reqproc.py | 8 ++------ scrapy/http/common.py | 9 +++++++++ scrapy/http/request/__init__.py | 6 ++++-- scrapy/http/request/form.py | 2 +- scrapy/http/response/__init__.py | 5 +++-- scrapy/http/response/text.py | 5 ----- scrapy/tests/test_http_request.py | 13 ------------- scrapy/tests/test_http_response.py | 3 --- scrapy/tests/test_utils_request.py | 3 +-- 11 files changed, 35 insertions(+), 42 deletions(-) create mode 100644 scrapy/http/common.py diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 6959070b7..c35668f10 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -108,6 +108,9 @@ Request objects attribute contains the escaped URL, so it can differ from the URL passed in the constructor. + This attribute is read-only. To change the URL of a Request use + :meth:`replace`. + .. attribute:: Request.method A string representing the HTTP method in the request. This is guaranteed to @@ -119,7 +122,10 @@ Request objects .. attribute:: Request.body - A str that contains the request body + A str that contains the request body. + + This attribute is read-only. To change the body of a Request use + :meth:`replace`. .. attribute:: Request.meta @@ -355,6 +361,9 @@ Response objects A string containing the URL of the response. + This attribute is read-only. To change the URL of a Response use + :meth:`replace`. + .. attribute:: Response.status An integer representing the HTTP status of the response. Example: ``200``, @@ -371,6 +380,9 @@ Response objects :meth:`TextResponse.body_as_unicode` (only available in :class:`TextResponse` and subclasses). + This attribute is read-only. To change the body of a Response use + :meth:`replace`. + .. attribute:: Response.request The :class:`Request` object that generated this response. This attribute is diff --git a/scrapy/contrib_exp/crawlspider/reqext.py b/scrapy/contrib_exp/crawlspider/reqext.py index e23e78082..98210fa11 100644 --- a/scrapy/contrib_exp/crawlspider/reqext.py +++ b/scrapy/contrib_exp/crawlspider/reqext.py @@ -38,13 +38,8 @@ class BaseSgmlRequestExtractor(FixedSGMLParser): def _make_absolute_urls(self, base_url, encoding): """Makes all request's urls absolute""" - for req in self.requests: - url = req.url - # make absolute url - url = urljoin_rfc(base_url, url, encoding) - url = safe_url_string(url, encoding) - # replace in-place request's url - req.url = url + self.requests = [x.replace(url=safe_url_string(urljoin_rfc(base_url, \ + x.url, encoding), encoding)) for x in self.requests] def _fix_link_text_encoding(self, encoding): """Convert link_text to unicode for each request""" diff --git a/scrapy/contrib_exp/crawlspider/reqproc.py b/scrapy/contrib_exp/crawlspider/reqproc.py index d39399b20..adc07005a 100644 --- a/scrapy/contrib_exp/crawlspider/reqproc.py +++ b/scrapy/contrib_exp/crawlspider/reqproc.py @@ -2,20 +2,16 @@ from scrapy.utils.misc import arg_to_iter from scrapy.utils.url import canonicalize_url, url_is_from_any_domain -from itertools import ifilter, imap +from itertools import ifilter import re class Canonicalize(object): """Canonicalize Request Processor""" - def _replace_url(self, req): - # replace in-place - req.url = canonicalize_url(req.url) - return req def __call__(self, requests): """Canonicalize all requests' urls""" - return imap(self._replace_url, requests) + return (x.replace(url=canonicalize_url(x.url)) for x in requests) class FilterDupes(object): diff --git a/scrapy/http/common.py b/scrapy/http/common.py new file mode 100644 index 000000000..63d264264 --- /dev/null +++ b/scrapy/http/common.py @@ -0,0 +1,9 @@ +import warnings + +def deprecated_setter(setter, attrname): + def newsetter(self, value): + c = self.__class__.__name__ + warnings.warn("Don't modify %s.%s attribute, use %s.replace() instead" % \ + (c, attrname, c), DeprecationWarning, stacklevel=2) + return setter(self, value) + return newsetter diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 9c669a1d8..f791aded5 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -10,6 +10,8 @@ import copy from scrapy.http.headers import Headers from scrapy.utils.url import safe_url_string from scrapy.utils.trackref import object_ref +from scrapy.utils.decorator import deprecated +from scrapy.http.common import deprecated_setter class Request(object_ref): @@ -58,7 +60,7 @@ class Request(object_ref): else: raise TypeError('Request url must be str or unicode, got %s:' % type(url).__name__) - url = property(_get_url, _set_url) + url = property(_get_url, deprecated_setter(_set_url, 'url')) def _get_body(self): return self._body @@ -76,7 +78,7 @@ class Request(object_ref): else: raise TypeError("Request body must either str or unicode. Got: '%s'" % type(body).__name__) - body = property(_get_body, _set_body) + body = property(_get_body, deprecated_setter(_set_body, 'body')) @property def encoding(self): diff --git a/scrapy/http/request/form.py b/scrapy/http/request/form.py index 01219de1e..1356cd27f 100644 --- a/scrapy/http/request/form.py +++ b/scrapy/http/request/form.py @@ -33,7 +33,7 @@ class FormRequest(Request): query = [(unicode_to_str(k, self.encoding), _unicode_to_str(v, self.encoding)) for k, v in items] self.method = 'POST' - self.body = urllib.urlencode(query, doseq=1) + self._set_body(urllib.urlencode(query, doseq=1)) self.headers['Content-Type'] = 'application/x-www-form-urlencoded' @classmethod diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index 251e77ad7..20f6f1773 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -9,6 +9,7 @@ import copy from scrapy.http.headers import Headers from scrapy.utils.trackref import object_ref +from scrapy.http.common import deprecated_setter class Response(object_ref): @@ -40,7 +41,7 @@ class Response(object_ref): raise TypeError('%s url must be str, got %s:' % (type(self).__name__, \ type(url).__name__)) - url = property(_get_url, _set_url) + url = property(_get_url, deprecated_setter(_set_url, 'url')) def _get_body(self): return self._body @@ -57,7 +58,7 @@ class Response(object_ref): raise TypeError("Response body must either str or unicode. Got: '%s'" \ % type(body).__name__) - body = property(_get_body, _set_body) + body = property(_get_body, deprecated_setter(_set_body, 'body')) def __repr__(self): attrs = ['url', 'status', 'body', 'headers', 'meta', 'flags'] diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index 59f44fb56..b54c3e279 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -33,9 +33,6 @@ class TextResponse(Response): self._cached_ubody = None super(TextResponse, self).__init__(url, status, headers, body, meta, flags) - def _get_url(self): - return self._url - def _set_url(self, url): if isinstance(url, unicode): if self.encoding is None: @@ -45,8 +42,6 @@ class TextResponse(Response): else: super(TextResponse, self)._set_url(url) - url = property(_get_url, _set_url) - def _set_body(self, body): self._body = '' if isinstance(body, unicode): diff --git a/scrapy/tests/test_http_request.py b/scrapy/tests/test_http_request.py index 9248de464..cc8c0b412 100644 --- a/scrapy/tests/test_http_request.py +++ b/scrapy/tests/test_http_request.py @@ -29,9 +29,6 @@ class RequestTest(unittest.TestCase): self.assertEqual(r.url, "http://www.example.com") self.assertEqual(r.method, self.default_method) - r.url = "http://www.example.com/other" - assert isinstance(r.url, str) - assert isinstance(r.headers, Headers) self.assertEqual(r.headers, self.default_headers) self.assertEqual(r.meta, self.default_meta) @@ -80,22 +77,12 @@ class RequestTest(unittest.TestCase): r = self.request_class(url="http://www.scrapy.org/path") self.assertEqual(r.url, "http://www.scrapy.org/path") - # url quoting on attribute assign - r.url = "http://www.scrapy.org/blank%20space" - self.assertEqual(r.url, "http://www.scrapy.org/blank%20space") - r.url = "http://www.scrapy.org/blank space" - self.assertEqual(r.url, "http://www.scrapy.org/blank%20space") - # url quoting on creation r = self.request_class(url="http://www.scrapy.org/blank%20space") self.assertEqual(r.url, "http://www.scrapy.org/blank%20space") r = self.request_class(url="http://www.scrapy.org/blank space") self.assertEqual(r.url, "http://www.scrapy.org/blank%20space") - # url coercion to string - r.url = u"http://www.scrapy.org/test" - self.assert_(isinstance(r.url, str)) - # url encoding r1 = self.request_class(url=u"http://www.scrapy.org/price/\xa3", encoding="utf-8") r2 = self.request_class(url=u"http://www.scrapy.org/price/\xa3", encoding="latin1") diff --git a/scrapy/tests/test_http_response.py b/scrapy/tests/test_http_response.py index 437d75b98..eb48d0a86 100644 --- a/scrapy/tests/test_http_response.py +++ b/scrapy/tests/test_http_response.py @@ -154,9 +154,6 @@ class TextResponseTest(BaseResponseTest): self.assertEqual(resp.url, 'http://www.example.com/price/\xc2\xa3') resp = self.response_class(url=u"http://www.example.com/price/\xa3", encoding='latin-1') self.assertEqual(resp.url, 'http://www.example.com/price/\xa3') - resp = self.response_class(url="http://www.example.com/price/", encoding='utf-8') - resp.url = u'http://www.example.com/price/\xa3' - self.assertEqual(resp.url, 'http://www.example.com/price/\xc2\xa3') resp = self.response_class(u"http://www.example.com/price/\xa3", headers={"Content-type": ["text/html; charset=utf-8"]}) self.assertEqual(resp.url, 'http://www.example.com/price/\xc2\xa3') resp = self.response_class(u"http://www.example.com/price/\xa3", headers={"Content-type": ["text/html; charset=iso-8859-1"]}) diff --git a/scrapy/tests/test_utils_request.py b/scrapy/tests/test_utils_request.py index f554c615c..82116ac7b 100644 --- a/scrapy/tests/test_utils_request.py +++ b/scrapy/tests/test_utils_request.py @@ -51,8 +51,7 @@ class UtilsRequestTest(unittest.TestCase): # cached fingerprint must be cleared on request copy r1 = Request("http://www.example.com") fp1 = request_fingerprint(r1) - r2 = r1.copy() - r2.url = "http://www.example.com/other" + r2 = r1.replace(url = "http://www.example.com/other") fp2 = request_fingerprint(r2) self.assertNotEqual(fp1, fp2)