Merge pull request #67 from gadventures/do-not-recache-on-get

Do not recache on `get`

HISTORY.rst

@@ -3,6 +3,14 @@
 History
 =======
 
+Unreleased
+----------
+
+* Fixed `issue 65 <https://github.com/gadventures/gapipy/issues/65>`_: only
+  write data into the local cache after a fetch from the API, do not write data
+  into the local cache when fetching from the local cache.
+
+
 2.5.2 (2017-04-26)
 ------------------
 

gapipy/query.py

@@ -67,7 +67,6 @@ def get(self, resource_id, variation_id=None, cached=True):
                 return None
             raise e
         resource_object = self.resource(data, client=self._client)
-        self._client._cache.set(key, resource_object.to_dict())
         return resource_object
 
     def get_resource_data(self, resource_id, variation_id=None, cached=True):
@@ -82,9 +81,11 @@ def get_resource_data(self, resource_id, variation_id=None, cached=True):
             if resource_data is not None:
                 return resource_data
 
-        # Cache miss; get fresh data from the backend.
+        # Cache miss; get fresh data from the backend, set in cache
         requestor = APIRequestor(self._client, self.resource)
         out = requestor.get(resource_id, variation_id=variation_id)
+        if out is not None:
+            self._client._cache.set(key, out)
         self._filters = {}
         return out
 

tests/test_query.py

@@ -2,7 +2,7 @@
 import sys
 import unittest
 
-from mock import patch
+from mock import MagicMock, patch
 from requests import HTTPError, Response
 
 from gapipy.client import Client
@@ -271,6 +271,29 @@ def test_resources_are_cached(self, mock_request):
         query.get(21346)
         self.assertEqual(len(mock_request.mock_calls), 1)
 
+    def test_cached_get_does_not_set(self):
+        """
+        Regression test https://github.com/gadventures/gapipy/issues/65
+
+        We discovered that when getting a resource, even if it is a hit in our
+        local cache we would set the data back into our cache every time. When
+        using a cache with a TTL on keys (e.g. Redis) this has the effect of
+        resetting the TTL each time that that key is retrieved. This is not the
+        expected behaviour wrt cache key TTLs.
+        """
+        query = Query(self.client, Tour)
+
+        # act like we already have the data in our cache
+        mock_cache_get = MagicMock(return_value=PPP_TOUR_DATA)
+        self.cache.get = mock_cache_get
+
+        mock_cache_set = MagicMock()
+        self.cache.set = mock_cache_set
+
+        query.get(21346)
+        self.assertEqual(len(mock_cache_get.mock_calls), 1)
+        self.assertEqual(len(mock_cache_set.mock_calls), 0)
+
 
 class MockResource(Resource):
     _as_is_fields = ['id', 'first_name', 'last_name']