Merge pull request #264 from ionrock/ionrock/internal-file-handle-revert

Internal file handle revert

cachecontrol/controller.py

@@ -410,7 +410,6 @@ def update_cached_response(self, request, response):
         cached_response.status = 200
 
         # update our cache
-        body = cached_response.read(decode_content=False)
-        self.cache.set(cache_url, self.serializer.dumps(request, cached_response, body))
+        self.cache.set(cache_url, self.serializer.dumps(request, cached_response))
 
         return cached_response

cachecontrol/serialize.py

@@ -25,10 +25,16 @@ def _b64_decode_str(s):
 
 
 class Serializer(object):
-
-    def dumps(self, request, response, body):
+    def dumps(self, request, response, body=None):
         response_headers = CaseInsensitiveDict(response.headers)
 
+        if body is None:
+            # When a body isn't passed in, we'll read the response. We
+            # also update the response with a new file handler to be
+            # sure it acts as though it was never read.
+            body = response.read(decode_content=False)
+            response._fp = io.BytesIO(body)
+
         # NOTE: This is all a bit weird, but it's really important that on
         #       Python 2.x these objects are unicode and not str, even when
         #       they contain only ascii. The problem here is that msgpack

tests/issue_263.py

@@ -0,0 +1,50 @@
+#!/usr/bin/env python3
+import sys
+
+import cachecontrol
+import requests
+from cachecontrol.cache import DictCache
+from cachecontrol.heuristics import BaseHeuristic
+
+import logging
+
+clogger = logging.getLogger("cachecontrol")
+clogger.addHandler(logging.StreamHandler())
+clogger.setLevel(logging.DEBUG)
+
+
+from pprint import pprint
+
+
+class NoAgeHeuristic(BaseHeuristic):
+    def update_headers(self, response):
+        if "cache-control" in response.headers:
+            del response.headers["cache-control"]
+
+
+cache_adapter = cachecontrol.CacheControlAdapter(
+    DictCache(), cache_etags=True, heuristic=NoAgeHeuristic()
+)
+
+
+session = requests.Session()
+session.mount("https://", cache_adapter)
+
+
+def log_resp(resp):
+    return
+
+    print(f"{resp.status_code} {resp.request.method}")
+    for k, v in response.headers.items():
+        print(f"{k}: {v}")
+
+
+for i in range(2):
+    response = session.get(
+        "https://api.github.com/repos/sigmavirus24/github3.py/pulls/1033"
+    )
+    log_resp(response)
+    print(f"Content length: {len(response.content)}")
+    print(response.from_cache)
+    if len(response.content) == 0:
+        sys.exit(1)

tests/test_serialization.py

@@ -12,7 +12,6 @@
 
 
 class TestSerializer(object):
-
     def setup(self):
         self.serializer = Serializer()
         self.response_data = {
@@ -93,7 +92,9 @@ def test_read_latest_version_streamable(self, url):
         original_resp = requests.get(url, stream=True)
         req = original_resp.request
 
-        resp = self.serializer.loads(req, self.serializer.dumps(req, original_resp.raw, original_resp.content))
+        resp = self.serializer.loads(
+            req, self.serializer.dumps(req, original_resp.raw, original_resp.content)
+        )
 
         assert resp.read()
 
@@ -120,3 +121,17 @@ def test_no_vary_header(self, url):
         assert self.serializer.loads(
             req, self.serializer.dumps(req, original_resp.raw, data)
         )
+
+    def test_no_body_creates_response_file_handle_on_dumps(self, url):
+        original_resp = requests.get(url, stream=True)
+        data = None
+        req = original_resp.request
+
+        assert self.serializer.loads(
+            req, self.serializer.dumps(req, original_resp.raw, data)
+        )
+
+        # By passing in data=None it will force a read of the file
+        # handle. Reading it again proves we're resetting the internal
+        # file handle with a buffer.
+        assert original_resp.raw.read()