Repository: scrapy/scrapy
Pull request #1680 (2016-01-15 12:53:22)

[MRG+1] Py3: port downloader cache and compression middlewares
This port downloader cache middleware to py3, and is a continuation of #1678. Together with #1637 they port all downloader middlewares to py3, I think.

Here most fixes are about headers parsing (in bytes), and fixing leveldb (adding it to tox.ini and using bytes as keys).

One change that is maybe not necessary is changing ``headers.get('foo')`` to ``headers.get(b'foo')`` - both work, because keys are converted to bytes, but using bytes feels more natural as values are also bytes.


Patch:
diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py
index 719507396..7ab304c17 100644
--- a/scrapy/downloadermiddlewares/httpcompression.py
+++ b/scrapy/downloadermiddlewares/httpcompression.py
@@ -9,13 +9,13 @@ from scrapy.exceptions import NotConfigured
 class HttpCompressionMiddleware(object):
     """This middleware allows compressed (gzip, deflate) traffic to be
     sent/received from web sites"""
-    
+
     @classmethod
     def from_crawler(cls, crawler):
         if not crawler.settings.getbool('COMPRESSION_ENABLED'):
             raise NotConfigured
         return cls()
-    
+
     def process_request(self, request, spider):
         request.headers.setdefault('Accept-Encoding', 'gzip,deflate')
 
@@ -39,10 +39,10 @@ class HttpCompressionMiddleware(object):
         return response
 
     def _decode(self, body, encoding):
-        if encoding == 'gzip' or encoding == 'x-gzip':
+        if encoding == b'gzip' or encoding == b'x-gzip':
             body = gunzip(body)
 
-        if encoding == 'deflate':
+        if encoding == b'deflate':
             try:
                 body = zlib.decompress(body)
             except zlib.error:
diff --git a/scrapy/extensions/httpcache.py b/scrapy/extensions/httpcache.py
index 2911dd6bc..a871cc895 100644
--- a/scrapy/extensions/httpcache.py
+++ b/scrapy/extensions/httpcache.py
@@ -12,6 +12,7 @@ from scrapy.responsetypes import responsetypes
 from scrapy.utils.request import request_fingerprint
 from scrapy.utils.project import data_path
 from scrapy.utils.httpobj import urlparse_cached
+from scrapy.utils.python import to_bytes, to_unicode
 
 
 class DummyPolicy(object):
@@ -40,12 +41,13 @@ class RFC2616Policy(object):
     def __init__(self, settings):
         self.always_store = settings.getbool('HTTPCACHE_ALWAYS_STORE')
         self.ignore_schemes = settings.getlist('HTTPCACHE_IGNORE_SCHEMES')
-        self.ignore_response_cache_controls = settings.getlist('HTTPCACHE_IGNORE_RESPONSE_CACHE_CONTROLS')
+        self.ignore_response_cache_controls = [to_bytes(cc) for cc in
+            settings.getlist('HTTPCACHE_IGNORE_RESPONSE_CACHE_CONTROLS')]
         self._cc_parsed = WeakKeyDictionary()
 
     def _parse_cachecontrol(self, r):
         if r not in self._cc_parsed:
-            cch = r.headers.get('Cache-Control', '')
+            cch = r.headers.get(b'Cache-Control', b'')
             parsed = parse_cachecontrol(cch)
             if isinstance(r, Response):
                 for key in self.ignore_response_cache_controls:
@@ -58,7 +60,7 @@ class RFC2616Policy(object):
             return False
         cc = self._parse_cachecontrol(request)
         # obey user-agent directive "Cache-Control: no-store"
-        if 'no-store' in cc:
+        if b'no-store' in cc:
             return False
         # Any other is eligible for caching
         return True
@@ -69,7 +71,7 @@ class RFC2616Policy(object):
         # Status code 206 is not included because cache can not deal with partial contents
         cc = self._parse_cachecontrol(response)
         # obey directive "Cache-Control: no-store"
-        if 'no-store' in cc:
+        if b'no-store' in cc:
             return False
         # Never cache 304 (Not Modified) responses
         elif response.status == 304:
@@ -78,14 +80,14 @@ class RFC2616Policy(object):
         elif self.always_store:
             return True
         # Any hint on response expiration is good
-        elif 'max-age' in cc or 'Expires' in response.headers:
+        elif b'max-age' in cc or b'Expires' in response.headers:
             return True
         # Firefox fallbacks this statuses to one year expiration if none is set
         elif response.status in (300, 301, 308):
             return True
         # Other statuses without expiration requires at least one validator
         elif response.status in (200, 203, 401):
-            return 'Last-Modified' in response.headers or 'ETag' in response.headers
+            return b'Last-Modified' in response.headers or b'ETag' in response.headers
         # Any other is probably not eligible for caching
         # Makes no sense to cache responses that does not contain expiration
         # info and can not be revalidated
@@ -95,7 +97,7 @@ class RFC2616Policy(object):
     def is_cached_response_fresh(self, cachedresponse, request):
         cc = self._parse_cachecontrol(cachedresponse)
         ccreq = self._parse_cachecontrol(request)
-        if 'no-cache' in cc or 'no-cache' in ccreq:
+        if b'no-cache' in cc or b'no-cache' in ccreq:
             return False
 
         now = time()
@@ -109,7 +111,7 @@ class RFC2616Policy(object):
         if currentage < freshnesslifetime:
             return True
 
-        if 'max-stale' in ccreq and 'must-revalidate' not in cc:
+        if b'max-stale' in ccreq and b'must-revalidate' not in cc:
             # From RFC2616: "Indicates that the client is willing to
             # accept a response that has exceeded its expiration time.
             # If max-stale is assigned a value, then the client is
@@ -117,7 +119,7 @@ class RFC2616Policy(object):
             # expiration time by no more than the specified number of
             # seconds. If no value is assigned to max-stale, then the
             # client is willing to accept a stale response of any age."
-            staleage = ccreq['max-stale']
+            staleage = ccreq[b'max-stale']
             if staleage is None:
                 return True
 
@@ -136,22 +138,22 @@ class RFC2616Policy(object):
         # as long as the old response didn't specify must-revalidate.
         if response.status >= 500:
             cc = self._parse_cachecontrol(cachedresponse)
-            if 'must-revalidate' not in cc:
+            if b'must-revalidate' not in cc:
                 return True
 
         # Use the cached response if the server says it hasn't changed.
         return response.status == 304
 
     def _set_conditional_validators(self, request, cachedresponse):
-        if 'Last-Modified' in cachedresponse.headers:
-            request.headers['If-Modified-Since'] = cachedresponse.headers['Last-Modified']
+        if b'Last-Modified' in cachedresponse.headers:
+            request.headers[b'If-Modified-Since'] = cachedresponse.headers[b'Last-Modified']
 
-        if 'ETag' in cachedresponse.headers:
-            request.headers['If-None-Match'] = cachedresponse.headers['ETag']
+        if b'ETag' in cachedresponse.headers:
+            request.headers[b'If-None-Match'] = cachedresponse.headers[b'ETag']
 
     def _get_max_age(self, cc):
         try:
-            return max(0, int(cc['max-age']))
+            return max(0, int(cc[b'max-age']))
         except (KeyError, ValueError):
             return None
 
@@ -164,18 +166,18 @@ class RFC2616Policy(object):
             return maxage
 
         # Parse date header or synthesize it if none exists
-        date = rfc1123_to_epoch(response.headers.get('Date')) or now
+        date = rfc1123_to_epoch(response.headers.get(b'Date')) or now
 
         # Try HTTP/1.0 Expires header
-        if 'Expires' in response.headers:
-            expires = rfc1123_to_epoch(response.headers['Expires'])
+        if b'Expires' in response.headers:
+            expires = rfc1123_to_epoch(response.headers[b'Expires'])
             # When parsing Expires header fails RFC 2616 section 14.21 says we
             # should treat this as an expiration time in the past.
             return max(0, expires - date) if expires else 0
 
         # Fallback to heuristic using last-modified header
         # This is not in RFC but on Firefox caching implementation
-        lastmodified = rfc1123_to_epoch(response.headers.get('Last-Modified'))
+        lastmodified = rfc1123_to_epoch(response.headers.get(b'Last-Modified'))
         if lastmodified and lastmodified <= date:
             return (date - lastmodified) / 10
 
@@ -192,13 +194,13 @@ class RFC2616Policy(object):
         currentage = 0
         # If Date header is not set we assume it is a fast connection, and
         # clock is in sync with the server
-        date = rfc1123_to_epoch(response.headers.get('Date')) or now
+        date = rfc1123_to_epoch(response.headers.get(b'Date')) or now
         if now > date:
             currentage = now - date
 
-        if 'Age' in response.headers:
+        if b'Age' in response.headers:
             try:
-                age = int(response.headers['Age'])
+                age = int(response.headers[b'Age'])
                 currentage = max(currentage, age)
             except ValueError:
                 pass
@@ -305,7 +307,7 @@ class FilesystemCacheStorage(object):
             'timestamp': time(),
         }
         with self._open(os.path.join(rpath, 'meta'), 'wb') as f:
-            f.write(repr(metadata))
+            f.write(to_bytes(repr(metadata)))
         with self._open(os.path.join(rpath, 'pickled_meta'), 'wb') as f:
             pickle.dump(metadata, f, protocol=2)
         with self._open(os.path.join(rpath, 'response_headers'), 'wb') as f:
@@ -373,14 +375,14 @@ class LeveldbCacheStorage(object):
             'body': response.body,
         }
         batch = self._leveldb.WriteBatch()
-        batch.Put('%s_data' % key, pickle.dumps(data, protocol=2))
-        batch.Put('%s_time' % key, str(time()))
+        batch.Put(key + b'_data', pickle.dumps(data, protocol=2))
+        batch.Put(key + b'_time', to_bytes(str(time())))
         self.db.Write(batch)
 
     def _read_data(self, spider, request):
         key = self._request_key(request)
         try:
-            ts = self.db.Get('%s_time' % key)
+            ts = self.db.Get(key + b'_time')
         except KeyError:
             return  # not found or invalid entry
 
@@ -388,14 +390,14 @@ class LeveldbCacheStorage(object):
             return  # expired
 
         try:
-            data = self.db.Get('%s_data' % key)
+            data = self.db.Get(key + b'_data')
         except KeyError:
             return  # invalid entry
         else:
             return pickle.loads(data)
 
     def _request_key(self, request):
-        return request_fingerprint(request)
+        return to_bytes(request_fingerprint(request))
 
 
 
@@ -404,16 +406,16 @@ def parse_cachecontrol(header):
 
     http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9
 
-    >>> parse_cachecontrol('public, max-age=3600') == {'public': None,
-    ...                                                'max-age': '3600'}
+    >>> parse_cachecontrol(b'public, max-age=3600') == {b'public': None,
+    ...                                                 b'max-age': b'3600'}
     True
-    >>> parse_cachecontrol('') == {}
+    >>> parse_cachecontrol(b'') == {}
     True
 
     """
     directives = {}
-    for directive in header.split(','):
-        key, sep, val = directive.strip().partition('=')
+    for directive in header.split(b','):
+        key, sep, val = directive.strip().partition(b'=')
         if key:
             directives[key.lower()] = val if sep else None
     return directives
@@ -421,6 +423,7 @@ def parse_cachecontrol(header):
 
 def rfc1123_to_epoch(date_str):
     try:
+        date_str = to_unicode(date_str, encoding='ascii')
         return mktime_tz(parsedate_tz(date_str))
     except Exception:
         return None
diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py
index 8435b0354..b151933b6 100644
--- a/scrapy/settings/default_settings.py
+++ b/scrapy/settings/default_settings.py
@@ -18,6 +18,8 @@ import sys
 from importlib import import_module
 from os.path import join, abspath, dirname
 
+import six
+
 AJAXCRAWL_ENABLED = False
 
 AUTOTHROTTLE_ENABLED = False
@@ -163,7 +165,7 @@ HTTPCACHE_ALWAYS_STORE = False
 HTTPCACHE_IGNORE_HTTP_CODES = []
 HTTPCACHE_IGNORE_SCHEMES = ['file']
 HTTPCACHE_IGNORE_RESPONSE_CACHE_CONTROLS = []
-HTTPCACHE_DBM_MODULE = 'anydbm'
+HTTPCACHE_DBM_MODULE = 'anydbm' if six.PY2 else 'dbm'
 HTTPCACHE_POLICY = 'scrapy.extensions.httpcache.DummyPolicy'
 HTTPCACHE_GZIP = False
 


Test patch:
diff --git a/tests/py3-ignores.txt b/tests/py3-ignores.txt
index 57e80f590..1f7d85ef7 100644
--- a/tests/py3-ignores.txt
+++ b/tests/py3-ignores.txt
@@ -4,10 +4,7 @@ tests/test_command_shell.py
 tests/test_exporters.py
 tests/test_linkextractors_deprecated.py
 tests/test_crawl.py
-tests/test_downloadermiddleware_httpcache.py
-tests/test_downloadermiddleware_httpcompression.py
 tests/test_downloadermiddleware_httpproxy.py
-tests/test_downloadermiddleware.py
 tests/test_downloadermiddleware_retry.py
 tests/test_engine.py
 tests/test_mail.py
@@ -31,7 +28,6 @@ scrapy/linkextractors/sgml.py
 scrapy/linkextractors/regex.py
 scrapy/linkextractors/htmlparser.py
 scrapy/downloadermiddlewares/retry.py
-scrapy/downloadermiddlewares/httpcache.py
 scrapy/downloadermiddlewares/httpproxy.py
 scrapy/downloadermiddlewares/cookies.py
 scrapy/extensions/statsmailer.py
diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt
index a709a734e..5cf786a89 100644
--- a/tests/requirements-py3.txt
+++ b/tests/requirements-py3.txt
@@ -3,6 +3,7 @@ pytest-twisted
 pytest-cov
 testfixtures
 jmespath
+leveldb
 # optional for shell wrapper tests
 bpython
 ipython
diff --git a/tests/test_downloadermiddleware.py b/tests/test_downloadermiddleware.py
index 13f35b92a..fb51392b2 100644
--- a/tests/test_downloadermiddleware.py
+++ b/tests/test_downloadermiddleware.py
@@ -5,6 +5,7 @@ from scrapy.http import Request, Response
 from scrapy.spiders import Spider
 from scrapy.core.downloader.middleware import DownloaderMiddlewareManager
 from scrapy.utils.test import get_crawler
+from scrapy.utils.python import to_bytes
 from tests import mock
 
 
@@ -68,7 +69,7 @@ class DefaultsTest(ManagerTestCase):
 
         """
         req = Request('http://example.com')
-        body = '<p>You are being redirected</p>'
+        body = b'<p>You are being redirected</p>'
         resp = Response(req.url, status=302, body=body, headers={
             'Content-Length': str(len(body)),
             'Content-Type': 'text/html',
@@ -78,12 +79,12 @@ class DefaultsTest(ManagerTestCase):
         ret = self._download(request=req, response=resp)
         self.assertTrue(isinstance(ret, Request),
                         "Not redirected: {0!r}".format(ret))
-        self.assertEqual(ret.url, resp.headers['Location'],
+        self.assertEqual(to_bytes(ret.url), resp.headers['Location'],
                          "Not redirected to location header")
 
     def test_200_and_invalid_gzipped_body_must_fail(self):
         req = Request('http://example.com')
-        body = '<p>You are being redirected</p>'
+        body = b'<p>You are being redirected</p>'
         resp = Response(req.url, status=200, body=body, headers={
             'Content-Length': str(len(body)),
             'Content-Type': 'text/html',
diff --git a/tests/test_downloadermiddleware_httpcache.py b/tests/test_downloadermiddleware_httpcache.py
index 53389ae3b..12b69860a 100644
--- a/tests/test_downloadermiddleware_httpcache.py
+++ b/tests/test_downloadermiddleware_httpcache.py
@@ -31,7 +31,7 @@ class _BaseTest(unittest.TestCase):
                                headers={'User-Agent': 'test'})
         self.response = Response('http://www.example.com',
                                  headers={'Content-Type': 'text/html'},
-                                 body='test body',
+                                 body=b'test body',
                                  status=202)
         self.crawler.stats.open_spider(self.spider)
 
@@ -84,9 +84,9 @@ class _BaseTest(unittest.TestCase):
 
     def assertEqualRequestButWithCacheValidators(self, request1, request2):
         self.assertEqual(request1.url, request2.url)
-        assert not 'If-None-Match' in request1.headers
-        assert not 'If-Modified-Since' in request1.headers
-        assert any(h in request2.headers for h in ('If-None-Match', 'If-Modified-Since'))
+        assert not b'If-None-Match' in request1.headers
+        assert not b'If-Modified-Since' in request1.headers
+        assert any(h in request2.headers for h in (b'If-None-Match', b'If-Modified-Since'))
         self.assertEqual(request1.body, request2.body)
 
     def test_dont_cache(self):
@@ -257,6 +257,7 @@ class RFC2616PolicyTest(DefaultStorageTest):
     policy_class = 'scrapy.extensions.httpcache.RFC2616Policy'
 
     def _process_requestresponse(self, mw, request, response):
+        result = None
         try:
             result = mw.process_request(request, self.spider)
             if result:
@@ -291,7 +292,7 @@ class RFC2616PolicyTest(DefaultStorageTest):
             self.assertEqualResponse(res2, res3)
             # request with no-cache directive must not return cached response
             # but it allows new response to be stored
-            res0b = res0.replace(body='foo')
+            res0b = res0.replace(body=b'foo')
             res4 = self._process_requestresponse(mw, req2, res0b)
             self.assertEqualResponse(res4, res0b)
             assert 'cached' not in res4.flags
@@ -435,7 +436,7 @@ class RFC2616PolicyTest(DefaultStorageTest):
                 assert 'cached' not in res1.flags
                 # Same request but as cached response is stale a new response must
                 # be returned
-                res0b = res0a.replace(body='bar')
+                res0b = res0a.replace(body=b'bar')
                 res2 = self._process_requestresponse(mw, req0, res0b)
                 self.assertEqualResponse(res2, res0b)
                 assert 'cached' not in res2.flags
diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py
index a18994ef3..2e6e47fef 100644
--- a/tests/test_downloadermiddleware_httpcompression.py
+++ b/tests/test_downloadermiddleware_httpcompression.py
@@ -50,46 +50,46 @@ class HttpCompressionTest(TestCase):
         request = Request('http://scrapytest.org')
         assert 'Accept-Encoding' not in request.headers
         self.mw.process_request(request, self.spider)
-        self.assertEqual(request.headers.get('Accept-Encoding'), 'gzip,deflate')
+        self.assertEqual(request.headers.get('Accept-Encoding'), b'gzip,deflate')
 
     def test_process_response_gzip(self):
         response = self._getresponse('gzip')
         request = response.request
 
-        self.assertEqual(response.headers['Content-Encoding'], 'gzip')
+        self.assertEqual(response.headers['Content-Encoding'], b'gzip')
         newresponse = self.mw.process_response(request, response, self.spider)
         assert newresponse is not response
-        assert newresponse.body.startswith('<!DOCTYPE')
+        assert newresponse.body.startswith(b'<!DOCTYPE')
         assert 'Content-Encoding' not in newresponse.headers
 
     def test_process_response_rawdeflate(self):
         response = self._getresponse('rawdeflate')
         request = response.request
 
-        self.assertEqual(response.headers['Content-Encoding'], 'deflate')
+        self.assertEqual(response.headers['Content-Encoding'], b'deflate')
         newresponse = self.mw.process_response(request, response, self.spider)
         assert newresponse is not response
-        assert newresponse.body.startswith('<!DOCTYPE')
+        assert newresponse.body.startswith(b'<!DOCTYPE')
         assert 'Content-Encoding' not in newresponse.headers
 
     def test_process_response_zlibdelate(self):
         response = self._getresponse('zlibdeflate')
         request = response.request
 
-        self.assertEqual(response.headers['Content-Encoding'], 'deflate')
+        self.assertEqual(response.headers['Content-Encoding'], b'deflate')
         newresponse = self.mw.process_response(request, response, self.spider)
         assert newresponse is not response
-        assert newresponse.body.startswith('<!DOCTYPE')
+        assert newresponse.body.startswith(b'<!DOCTYPE')
         assert 'Content-Encoding' not in newresponse.headers
 
     def test_process_response_plain(self):
-        response = Response('http://scrapytest.org', body='<!DOCTYPE...')
+        response = Response('http://scrapytest.org', body=b'<!DOCTYPE...')
         request = Request('http://scrapytest.org')
 
         assert not response.headers.get('Content-Encoding')
         newresponse = self.mw.process_response(request, response, self.spider)
         assert newresponse is response
-        assert newresponse.body.startswith('<!DOCTYPE')
+        assert newresponse.body.startswith(b'<!DOCTYPE')
 
     def test_multipleencodings(self):
         response = self._getresponse('gzip')
@@ -97,7 +97,7 @@ class HttpCompressionTest(TestCase):
         request = response.request
         newresponse = self.mw.process_response(request, response, self.spider)
         assert newresponse is not response
-        self.assertEqual(newresponse.headers.getlist('Content-Encoding'), ['uuencode'])
+        self.assertEqual(newresponse.headers.getlist('Content-Encoding'), [b'uuencode'])
 
     def test_process_response_encoding_inside_body(self):
         headers = {
@@ -142,5 +142,5 @@ class HttpCompressionTest(TestCase):
 
         newresponse = self.mw.process_response(request, response, self.spider)
         self.assertIs(newresponse, response)
-        self.assertEqual(response.headers['Content-Encoding'], 'gzip')
-        self.assertEqual(response.headers['Content-Type'], 'application/gzip')
+        self.assertEqual(response.headers['Content-Encoding'], b'gzip')
+        self.assertEqual(response.headers['Content-Type'], b'application/gzip')