More Request/Response cleanup:

* made status attribute an int
 * made engine use __str__ to display crawled requests
 * HTTP cache now inherits Response class to change __str__
 * added tests to check that the class is preserved on .copy() (for both Requests and Responses)
 * removed custom cached attribute (and passed to a Response.meta item)
 * removed some custom (and seldom used) methods from Response class: version(), info()
 * reinforced the privacy of the ResponseBody class, by renaming it to _ResponseBody and added a warning that it may be removed in the future
 * added tests for Request & Response to_string() methods
 * fixed minor (and harmless) bug in to_string() methods

--HG--
extra : convert_revision : svn%3Ab85faa78-f9eb-468e-a121-7cced6da292c%40737

scrapy/trunk/scrapy/contrib/downloadermiddleware/cache.py

@@ -15,6 +15,14 @@ from scrapy.core.exceptions import NotConfigured, HttpException, IgnoreRequest
 from scrapy.utils.request import request_fingerprint
 from scrapy.conf import settings
 
+class CachedResponse(Response):
+
+    def __init__(self, *args, **kwargs):
+        Response.__init__(self, *args, **kwargs)
+        self.meta['cached'] = True
+
+    def __str__(self):
+        return "(cached) " + Response.__str__(self)
 
 class CacheMiddleware(object):
     def __init__(self):
@@ -39,7 +47,6 @@ class CacheMiddleware(object):
             log.msg("Corrupt cache for %s" % request.url, log.WARNING)
             response = False
         if response:
-            response.cached = True
             if not 200 <= int(response.status) < 300:
                 raise HttpException(response.status, None, response)
             return response
@@ -50,7 +57,7 @@ class CacheMiddleware(object):
         if not is_cacheable(request):
             return response
 
-        if isinstance(response, Response) and not response.cached:
+        if isinstance(response, Response) and not response.meta.get('cached'):
             key = request_fingerprint(request)
             domain = spider.domain_name
             self.cache.store(domain, key, request, response)
@@ -153,8 +160,7 @@ class Cache(object):
         headers = Headers(responseheaders)
         status = metadata['status']
 
-        response = Response(domain=domain, url=url, headers=headers, status=status, body=responsebody)
-        response.cached = True
+        response = CachedResponse(domain=domain, url=url, headers=headers, status=status, body=responsebody)
         return response
 
     def store(self, domain, key, request, response):

scrapy/trunk/scrapy/contrib/downloadermiddleware/debug.py

@@ -1,20 +1,19 @@
 from scrapy import log
+from scrapy.core.exceptions import NotConfigured
 from scrapy.conf import settings
 
 class CrawlDebug(object):
+
     def __init__(self):
-        self.enabled = settings.getbool('CRAWL_DEBUG')
+        raise NotConfigured
     
     def process_request(self, request, spider):
-        if self.enabled:
-            log.msg("Crawling %s" % repr(request), domain=spider.domain_name, level=log.DEBUG)
+        log.msg("Crawling %s" % repr(request), domain=spider.domain_name, level=log.DEBUG)
 
     def process_exception(self, request, exception, spider):
-        if self.enabled:
-            log.msg("Crawl exception %s in %s" % (exception, repr(request)), domain=spider.domain_name, level=log.DEBUG)
+        log.msg("Crawl exception %s in %s" % (exception, repr(request)), domain=spider.domain_name, level=log.DEBUG)
 
     def process_response(self, request, response, spider):
-        if self.enabled:
-            log.msg("Fetched %s from %s" % (response.info(), repr(request)), domain=spider.domain_name, level=log.DEBUG)
+        log.msg("Fetched %s from %s" % (response, repr(request)), domain=spider.domain_name, level=log.DEBUG)
         return response
 

scrapy/trunk/scrapy/contrib/history/middleware.py

@@ -35,7 +35,7 @@ class HistoryMiddleware(object):
 
     def process_response(self, request, response, spider):
         version = request.context.get('history_response_version')
-        if version == response.version():
+        if version == self.get_version(response):
             del request.content['history_response_version']
             hist = self.historydata.version_info(domain, version)
             if hist:
@@ -66,11 +66,13 @@ class HistoryMiddleware(object):
         if response:
             redirect_url = response.url
             parentkey = urlkey(response.request.headers.get('referer')) if response.request else None
-            version = response.version()
+            version = self.get_version(response)
         else:
             redirect_url, parentkey, version = url, None, None
         self.historydata.store(domain, key, url, parentkey, version, post_version)
 
+    def get_version(self, response):
+        key = hashlib.sha1(response.body.to_string()).hexdigest()
 
 def urlkey(url):
     """Generate a 'key' for a given url

scrapy/trunk/scrapy/contrib/history/scheduler.py

@@ -2,6 +2,7 @@
 WARNING: This Scheduler code is obsolete and needs to be rewritten 
 """
 
+import hashlib
 from datetime import datetime
 
 from twisted.internet import defer
@@ -72,7 +73,8 @@ class RulesScheduler(Scheduler):
 
         def callback(pagedata):
             """process other callback if we pass the checks"""
-            if version == pagedata.version():
+            
+            if version == self.get_version(pagedata):
                 hist = self.historydata.version_info(domain, version)
                 if hist:
                     versionkey, created = hist
@@ -83,7 +85,7 @@ class RulesScheduler(Scheduler):
                         message = "skipping %s: unchanged for %s" % (pagedata.url, delta)
                         raise IgnoreRequest(message)
             self.record_visit(domain, request.url, pagedata.url,
-                              pagedata.parent,  pagedata.version(),
+                              pagedata.parent,  self.get(pagedata),
                               post_version)
             return pagedata
 
@@ -98,3 +100,6 @@ class RulesScheduler(Scheduler):
         #request.prepend_callback(d)
 
         return request
+
+    def get_version(self, response):
+        key = hashlib.sha1(response.body.to_string()).hexdigest()

scrapy/trunk/scrapy/core/downloader/handlers.py

@@ -47,7 +47,7 @@ def download_http(request, spider):
 
     def _response(body):
         body = body or ''
-        status = factory.status
+        status = int(factory.status)
         headers = Headers(factory.response_headers)
         r = Response(domain=spider.domain_name, url=request.url, status=status, headers=headers, body=body)
         signals.send_catch_log(signal=signals.request_uploaded, sender='download_http', request=request, spider=spider)

scrapy/trunk/scrapy/core/engine.py

@@ -353,7 +353,7 @@ class ExecutionEngine(object):
             if isinstance(response, Response):
                 response.request = request # tie request to obtained response
                 cached = 'cached' if response.cached else 'live'
-                log.msg("Crawled %s <%s> from <%s>" % (cached, response.url, request.headers.get('referer')), level=log.DEBUG, domain=domain)
+                log.msg("Crawled %s from <%s>" % (response, request.headers.get('referer')), level=log.DEBUG, domain=domain)
                 return response
             elif isinstance(response, Request):
                 redirected = response # proper alias

scrapy/trunk/scrapy/http/__init__.py

@@ -2,10 +2,10 @@
 Module containing all HTTP related classes
 
 Use this module (instead of the more specific ones) when importing Headers,
-Request, Response, ResponseBody and Url outside this module.
+Request, Response and Url outside this module.
 """
 
 from scrapy.http.url import Url
 from scrapy.http.headers import Headers
 from scrapy.http.request import Request
-from scrapy.http.response import Response, ResponseBody
+from scrapy.http.response import Response

scrapy/trunk/scrapy/http/request.py

@@ -63,9 +63,10 @@ class Request(object):
     url = property(lambda x: x._url, set_url)
 
     def __str__(self):
-        if self.method != 'GET':
-            return "<(%s) %s>" % (self.method, self.url)
-        return "<%s>" % self.url
+        if self.method == 'GET':
+            return "<%s>" % self.url
+        else:
+            return "<%s %s>" % (self.method, self.url)
 
     def __len__(self):
         """Return raw HTTP request size"""
@@ -103,7 +104,8 @@ class Request(object):
 
         s  = "%s %s HTTP/1.1\r\n" % (self.method, self.url)
         s += "Host: %s\r\n" % self.url.hostname
-        s += self.headers.to_string() + "\r\n"
+        if self.headers:
+            s += self.headers.to_string() + "\r\n"
         s += "\r\n"
         if self.body:
             s += self.body

scrapy/trunk/scrapy/http/response.py

@@ -6,17 +6,15 @@ See documentation in docs/ref/request-response.rst
 """
 
 import re
-import hashlib
 import copy
+from types import NoneType
 
+from twisted.web.http import RESPONSES
 from BeautifulSoup import UnicodeDammit
 
 from scrapy.http.url import Url
 from scrapy.http.headers import Headers
 
-from twisted.web import http
-reason_phrases = http.RESPONSES 
-
 class Response(object):
 
     _ENCODING_RE = re.compile(r'charset=([\w-]+)', re.I)
@@ -26,20 +24,17 @@ class Response(object):
         self.url = Url(url)
         self.headers = Headers(headers or {})
         self.status = status
-        # ResponseBody is not meant to be used directly (use .replace instead)
-        assert(isinstance(body, basestring) or body is None)
-        self.body = ResponseBody(body, self.headers_encoding())
+        if body is not None:
+            assert isinstance(body, basestring), \
+                "body must be basestring, got %s" % type(body).__name__
+            self.body = _ResponseBody(body, self.headers_encoding())
+        else:
+            self.body = None
         self.cached = False
-        self.request = None # request which originated this response
+        self.request = None
         self.meta = {}
         self.cache = {}
 
-    def version(self):
-        """A hash of the contents of this response"""
-        if not hasattr(self, '_version'):
-            self._version = hashlib.sha1(self.body.to_string()).hexdigest()
-        return self._version
-
     def headers_encoding(self):
         content_type = self.headers.get('Content-Type')
         if content_type:
@@ -52,39 +47,36 @@ class Response(object):
                 (repr(self.domain), repr(self.url), repr(self.headers), repr(self.status), repr(self.body))
 
     def __str__(self):
-        version = '%s..%s' % (self.version()[:4], self.version()[-4:])
-        return "<Response: %s %s (%s)>" % (self.status, self.url, version)
+        if self.status == 200:
+            return "<%s>" % (self.url)
+        else:
+            return "<%d %s>" % (self.status, self.url)
 
     def __len__(self):
         """Return raw HTTP response size"""
         return len(self.to_string())
 
-    def info(self):
-        return "<Response status=%s domain=%s url=%s headers=%s" % (self.status, self.domain, self.url, self.headers)
-
     def copy(self):
         """Create a new Response based on the current one"""
         return self.replace()
 
-    def replace(self, **kw):
-        """Create a new Response with the same attributes except for those given new values.
+    def replace(self, domain=None, url=None, status=None, headers=None, body=None):
+        """Create a new Response with the same attributes except for those
+        given new values.
 
-        Example: newresp = oldresp.replace(body="New body")
+        Example:
+
+        >>> newresp = oldresp.replace(body="New body")
         """
-        def sameheaders():
-            return copy.deepcopy(self.headers)
-        def samebody():
-            return copy.deepcopy(self.body)
-        newresp = Response(kw.get('domain', self.domain),
-                           kw.get('url', self.url),
-                           headers=kw.get('headers', sameheaders()),
-                           status=kw.get('status', self.status),
-                           body=kw.get('body'))
-        # Response.__init__ forbids the use of ResponseBody instances
-        if 'body' not in kw:
-            newresp.body = samebody()
-        newresp.meta = self.meta.copy()
-        return newresp
+        new = self.__class__(domain=domain or self.domain,
+                             url=url or self.url,
+                             status=status or self.status,
+                             headers=headers or copy.deepcopy(self.headers),
+                             body=body)
+        if body is None:
+            new.body = copy.deepcopy(self.body)
+        new.meta = self.meta.copy()
+        return new
 
     def to_string(self):
         """
@@ -93,18 +85,24 @@ class Response(object):
         received (that's not exposed by Twisted).
         """
 
-        s  = "HTTP/1.1 %s %s\r\n" % (self.status, reason_phrases[int(self.status)])
-        s += self.headers.to_string() + "\r\n"
+        s  = "HTTP/1.1 %s %s\r\n" % (self.status, RESPONSES[self.status])
+        if self.headers:
+            s += self.headers.to_string() + "\r\n"
         s += "\r\n"
         if self.body:
             s += self.body.to_string()
             s += "\r\n"
         return s
 
-class ResponseBody(object):
-    """The body of an HTTP response
+class _ResponseBody(object):
+    """The body of an HTTP response. 
+    
+    WARNING: This is a private class and could be removed in the future without
+    previous notice. Do not use it this class from outside this module, use
+    the Response class instead.
 
-    This handles conversion to unicode and various character encodings.
+    Currently, the main purpose of this class is to handle conversion to
+    unicode and various character encodings.
     """
 
     _template = r'''%s\s*=\s*["']?\s*%s\s*["']?'''
@@ -182,7 +180,7 @@ class ResponseBody(object):
         return proposed
 
     def __repr__(self):
-        return "ResponseBody(content=%s, declared_encoding=%s)" % (repr(self._content), repr(self.declared_encoding))
+        return "_ResponseBody(content=%s, declared_encoding=%s)" % (repr(self._content), repr(self.declared_encoding))
 
     def __str__(self):
         return self.to_string()

scrapy/trunk/scrapy/replay/__init__.py

@@ -6,6 +6,7 @@ import shutil
 import tempfile
 import tarfile
 import copy
+import hashlib
 
 from pydispatch import dispatcher
 
@@ -132,7 +133,7 @@ class Replay(object):
             self.passed_new[str(item.guid)] = item
 
     def response_received(self, response, spider):
-        key = response.version()
+        key = hashlib.sha1(response.body.to_string()).hexdigest()
         if (self.recording or self.updating) and key:
             self.responses_old[key] = response.copy()
         elif key:

scrapy/trunk/scrapy/tests/test_adaptors.py

@@ -42,7 +42,7 @@ class AdaptorsTestCase(unittest.TestCase):
     def get_selector(self, domain, url, sample_filename, headers=None, selector=HtmlXPathSelector):
         sample_filename = os.path.join(self.samplesdir, sample_filename)
         body = file(sample_filename).read()
-        response = Response(domain=domain, url=url, headers=Headers(headers), status='200', body=body)
+        response = Response(domain=domain, url=url, headers=Headers(headers), status=200, body=body)
         return selector(response)
 
     def test_extract(self):

scrapy/trunk/scrapy/tests/test_engine.py

@@ -149,9 +149,9 @@ class EngineTest(unittest.TestCase):
 
         for response, spider in session.respplug:
             if session.getpath(response.url) == '/item999.html':
-                self.assertEqual('404', response.status)
+                self.assertEqual(404, response.status)
             if session.getpath(response.url) == '/redirect':
-                self.assertEqual('302', response.status)
+                self.assertEqual(302, response.status)
             self.assertEqual(response.domain, spider.domain_name)
 
     def test_item_data(self):

scrapy/trunk/scrapy/tests/test_http_request.py

@@ -94,5 +94,23 @@ class RequestTest(unittest.TestCase):
         assert r1.meta is not r2.meta, "meta must be a shallow copy, not identical"
         self.assertEqual(r1.meta, r2.meta)
 
+    def test_copy_inherited_classes(self):
+        """Test Request children copies preserve their class"""
+
+        class CustomRequest(Request):
+            pass
+
+        r1 = CustomRequest('example.com', 'http://www.example.com')
+        r2 = r1.copy()
+
+        assert type(r2) is CustomRequest
+
+    def test_to_string(self):
+        r1 = Request("http://www.example.com")
+        self.assertEqual(r1.to_string(), 'GET http://www.example.com HTTP/1.1\r\nHost: www.example.com\r\n\r\n')
+
+        r1 = Request("http://www.example.com", method='POST', headers={"Content-type": "text/html"}, body="Some body")
+        self.assertEqual(r1.to_string(), 'POST http://www.example.com HTTP/1.1\r\nHost: www.example.com\r\nContent-Type: text/html\r\n\r\nSome body\r\n')
+
 if __name__ == "__main__":
     unittest.main()

scrapy/trunk/scrapy/tests/test_http_response.py

@@ -1,7 +1,9 @@
 import unittest
-from scrapy.http import Response, ResponseBody
+from scrapy.http import Response
+from scrapy.http.response import _ResponseBody
 
 class ResponseTest(unittest.TestCase):
+
     def test_init(self):
         # Response requires domain and url
         self.assertRaises(Exception, Response)
@@ -10,7 +12,6 @@ class ResponseTest(unittest.TestCase):
         # body can be str or None but not ResponseBody
         self.assertTrue(isinstance(Response('example.com', 'http://example.com/', body=None), Response))
         self.assertTrue(isinstance(Response('example.com', 'http://example.com/', body='body'), Response))
-        self.assertRaises(AssertionError, Response, 'example.com', 'http://example.com/', body=ResponseBody('body', 'utf-8'))
         # test presence of all optional parameters
         self.assertTrue(isinstance(Response('example.com', 'http://example.com/', headers={}, status=200, body=None), Response))
 
@@ -29,12 +30,30 @@ class ResponseTest(unittest.TestCase):
         assert r1.meta is not r2.meta, "meta must be a shallow copy, not identical"
         self.assertEqual(r1.meta, r2.meta)
 
+    def test_copy_inherited_classes(self):
+        """Test Response children copies preserve their class"""
+
+        class CustomResponse(Response):
+            pass
+
+        r1 = CustomResponse('example.com', 'http://www.example.com')
+        r2 = r1.copy()
+
+        assert type(r2) is CustomResponse
+
+    def test_to_string(self):
+        r1 = Response('example.com', "http://www.example.com")
+        self.assertEqual(r1.to_string(), 'HTTP/1.1 200 OK\r\n\r\n')
+
+        r1 = Response('example.com', "http://www.example.com", status=404, headers={"Content-type": "text/html"}, body="Some body")
+        self.assertEqual(r1.to_string(), 'HTTP/1.1 404 Not Found\r\nContent-Type: text/html\r\n\r\nSome body\r\n')
+        
 class ResponseBodyTest(unittest.TestCase):
     unicode_string = u'\u043a\u0438\u0440\u0438\u043b\u043b\u0438\u0447\u0435\u0441\u043a\u0438\u0439 \u0442\u0435\u043a\u0441\u0442'
 
     def test_encoding(self):
         original_string = self.unicode_string.encode('cp1251')
-        cp1251_body     = ResponseBody(original_string, 'cp1251')
+        cp1251_body     = _ResponseBody(original_string, 'cp1251')
 
         # check to_unicode
         self.assertTrue(isinstance(cp1251_body.to_unicode(), unicode))