consistent-config-caching (#52)

* consistent-config-caching

* fix test

* Use ConfigEntry in tests

* include the `.json` extension in the cache_key

* prepare config JSON may contain '\n' characters

* store fetch time as milliseconds in the cache

* store the response's config_json_string in the cache

* use floor instead of int cast to support python 2.7 long int on 32bit systems

* cache payload test

* python 2.7 test fix

* fix {} default param

configcatclient/configcatclient.py

@@ -2,7 +2,7 @@
 
 from . import utils
 from .configservice import ConfigService
-from .constants import ROLLOUT_RULES, VARIATION_ID, VALUE, ROLLOUT_PERCENTAGE_ITEMS, CONFIG_FILE_NAME
+from .constants import ROLLOUT_RULES, VARIATION_ID, VALUE, ROLLOUT_PERCENTAGE_ITEMS
 from .evaluationdetails import EvaluationDetails
 from .interfaces import ConfigCatClientException
 from .logger import Logger
@@ -12,7 +12,6 @@
 from .overridedatasource import OverrideBehaviour
 from .refreshresult import RefreshResult
 from .rolloutevaluator import RolloutEvaluator
-import hashlib
 from collections import namedtuple
 import copy
 from .utils import method_is_called_from, get_date_time
@@ -349,9 +348,6 @@ def __get_settings(self):
 
         return self._config_service.get_settings()
 
-    def __get_cache_key(self):
-        return hashlib.sha1(('python_' + CONFIG_FILE_NAME + '_' + self._sdk_key).encode('utf-8')).hexdigest()
-
     def __evaluate(self, key, user, default_value, default_variation_id, settings, fetch_time):
         user = user if user is not None else self._default_user
         value, variation_id, rule, percentage_rule, error = self._rollout_evaluator.evaluate(

configcatclient/configentry.py

@@ -1,3 +1,6 @@
+import json
+from math import floor
+
 from . import utils
 
 
@@ -6,31 +9,43 @@ class ConfigEntry(object):
     ETAG = 'etag'
     FETCH_TIME = 'fetch_time'
 
-    def __init__(self, config={}, etag='', fetch_time=utils.distant_past):
-        self.config = config
+    def __init__(self, config=None, etag='', config_json_string='{}', fetch_time=utils.distant_past):
+        self.config = config if config is not None else {}
         self.etag = etag
+        self.config_json_string = config_json_string
         self.fetch_time = fetch_time
 
-    @classmethod
-    def create_from_json(cls, json):
-        if not json:
-            return ConfigEntry.empty
-
-        return ConfigEntry(
-            config=json.get(ConfigEntry.CONFIG, {}),
-            etag=json.get(ConfigEntry.ETAG, ''),
-            fetch_time=json.get(ConfigEntry.FETCH_TIME, utils.distant_past)
-        )
-
     def is_empty(self):
         return self == ConfigEntry.empty
 
-    def to_json(self):
-        return {
-            ConfigEntry.CONFIG: self.config,
-            ConfigEntry.ETAG: self.etag,
-            ConfigEntry.FETCH_TIME: self.fetch_time
-        }
+    def serialize(self):
+        return '{:.0f}\n{}\n{}'.format(floor(self.fetch_time * 1000), self.etag, self.config_json_string)
+
+    @classmethod
+    def create_from_string(cls, string):
+        if not string:
+            return ConfigEntry.empty
+
+        fetch_time_index = string.find('\n')
+        etag_index = string.find('\n', fetch_time_index + 1)
+        if fetch_time_index < 0 or etag_index < 0:
+            raise ValueError('Number of values is fewer than expected.')
+
+        try:
+            fetch_time = float(string[0:fetch_time_index])
+        except ValueError:
+            raise ValueError('Invalid fetch time: {}'.format(string[0:fetch_time_index]))
+
+        etag = string[fetch_time_index + 1:etag_index]
+        if not etag:
+            raise ValueError('Empty eTag value')
+        try:
+            config_json = string[etag_index + 1:]
+            config = json.loads(config_json)
+        except ValueError as e:
+            raise ValueError('Invalid config JSON: {}. {}'.format(config_json, str(e)))
+
+        return ConfigEntry(config=config, etag=etag, config_json_string=config_json, fetch_time=fetch_time / 1000.0)
 
 
 ConfigEntry.empty = ConfigEntry(etag='empty')

configcatclient/configfetcher.py

@@ -169,7 +169,8 @@ def _fetch(self, etag):
                 if response_etag is None:
                     response_etag = ''
                 config = response.json()
-                return FetchResponse.success(ConfigEntry(config, response_etag, get_utc_now_seconds_since_epoch()))
+                return FetchResponse.success(
+                    ConfigEntry(config, response_etag, response.text, get_utc_now_seconds_since_epoch()))
             elif response.status_code == 304:
                 return FetchResponse.not_modified()
             elif response.status_code in [404, 403]:

configcatclient/configservice.py

@@ -1,24 +1,22 @@
 import hashlib
-import json
 from threading import Thread, Event, Lock
 
 from . import utils
 from .configentry import ConfigEntry
-from .constants import CONFIG_FILE_NAME, FEATURE_FLAGS
+from .constants import CONFIG_FILE_NAME, FEATURE_FLAGS, SERIALIZATION_FORMAT_VERSION
 from .pollingmode import AutoPollingMode, LazyLoadingMode
 from .refreshresult import RefreshResult
 
 
 class ConfigService(object):
     def __init__(self, sdk_key, polling_mode, hooks, config_fetcher, log, config_cache, is_offline):
-        self._sdk_key = sdk_key
         self._cached_entry = ConfigEntry.empty
         self._cached_entry_string = ''
         self._polling_mode = polling_mode
         self.log = log
         self._config_cache = config_cache
         self._hooks = hooks
-        self._cache_key = hashlib.sha1(('python_' + CONFIG_FILE_NAME + '_' + self._sdk_key).encode('utf-8')).hexdigest()
+        self._cache_key = ConfigService._get_cache_key(sdk_key)
         self._config_fetcher = config_fetcher
         self._is_offline = is_offline
         self._response_future = None
@@ -172,20 +170,25 @@ def _set_initialized(self):
             self._initialized.set()
             self._hooks.invoke_on_client_ready()
 
+    @staticmethod
+    def _get_cache_key(sdk_key):
+        return hashlib.sha1(
+            (sdk_key + '_' + CONFIG_FILE_NAME + '.json' + '_' + SERIALIZATION_FORMAT_VERSION).encode('utf-8')).hexdigest()
+
     def _read_cache(self):
         try:
             json_string = self._config_cache.get(self._cache_key)
             if not json_string or json_string == self._cached_entry_string:
                 return ConfigEntry.empty
 
             self._cached_entry_string = json_string
-            return ConfigEntry.create_from_json(json.loads(json_string))
+            return ConfigEntry.create_from_string(json_string)
         except Exception:
             self.log.exception('Error occurred while reading the cache.', event_id=2200)
             return ConfigEntry.empty
 
     def _write_cache(self, config_entry):
         try:
-            self._config_cache.set(self._cache_key, json.dumps(config_entry.to_json()))
+            self._config_cache.set(self._cache_key, config_entry.serialize())
         except Exception:
             self.log.exception('Error occurred while writing the cache.', event_id=2201)

configcatclient/constants.py

@@ -1,4 +1,5 @@
 CONFIG_FILE_NAME = 'config_v5'
+SERIALIZATION_FORMAT_VERSION = 'v2'
 
 PREFERENCES = 'p'
 BASE_URL = 'u'

configcatclienttests/mocks.py

@@ -2,7 +2,7 @@
 import time
 
 from configcatclient.configentry import ConfigEntry
-from configcatclient.utils import get_utc_now_seconds_since_epoch
+from configcatclient.utils import get_utc_now_seconds_since_epoch, distant_past
 
 try:
     from unittest.mock import Mock
@@ -74,7 +74,7 @@ def get_configuration(self, etag=''):
         if etag != self._etag:
             self._fetch_count += 1
             return FetchResponse.success(
-                ConfigEntry(json.loads(self._configuration), self._etag, get_utc_now_seconds_since_epoch())
+                ConfigEntry(json.loads(self._configuration), self._etag, self._configuration, get_utc_now_seconds_since_epoch())
             )
         return FetchResponse.not_modified()
 
@@ -106,7 +106,7 @@ def __init__(self, wait_seconds):
 
     def get_configuration(self, etag=''):
         time.sleep(self._wait_seconds)
-        return FetchResponse.success(ConfigEntry(json.loads(TEST_JSON)))
+        return FetchResponse.success(ConfigEntry(json.loads(TEST_JSON), etag, TEST_JSON))
 
 
 class ConfigFetcherCountMock(ConfigFetcher):
@@ -115,13 +115,14 @@ def __init__(self):
 
     def get_configuration(self, etag=''):
         self._value += 1
-        config = json.loads(TEST_JSON_FORMAT.format(value=self._value))
-        return FetchResponse.success(ConfigEntry(config))
+        config_json_string = TEST_JSON_FORMAT.format(value=self._value)
+        config = json.loads(config_json_string)
+        return FetchResponse.success(ConfigEntry(config, etag, config_json_string))
 
 
 class ConfigCacheMock(ConfigCache):
     def get(self, key):
-        return json.dumps({ConfigEntry.CONFIG: TEST_OBJECT, ConfigEntry.ETAG: 'test-etag'})
+        return '\n'.join([str(distant_past), 'test-etag', json.dumps(TEST_OBJECT)])
 
     def set(self, key, value):
         pass
@@ -152,6 +153,7 @@ def get(self, name):
 class MockResponse:
     def __init__(self, json_data, status_code, etag=None):
         self.json_data = json_data
+        self.text = json.dumps(json_data)
         self.status_code = status_code
         self.headers = MockHeader(etag)
 

configcatclienttests/test_autopollingcachepolicy.py

@@ -204,11 +204,12 @@ def test_with_failed_refresh(self):
 
     def test_return_cached_config_when_cache_is_not_expired(self):
         config_fetcher = ConfigFetcherMock()
-        config_cache = SingleValueConfigCache(json.dumps({
-            ConfigEntry.CONFIG: json.loads(TEST_JSON),
-            ConfigEntry.ETAG: 'test-etag',
-            ConfigEntry.FETCH_TIME: get_utc_now_seconds_since_epoch()
-        }))
+        config_cache = SingleValueConfigCache(ConfigEntry(
+            config=json.loads(TEST_JSON),
+            etag='test-etag',
+            config_json_string=TEST_JSON,
+            fetch_time=get_utc_now_seconds_since_epoch()).serialize()
+        )
         poll_interval_seconds = 2
         max_init_wait_time_seconds = 1
 
@@ -240,11 +241,12 @@ def test_fetch_config_when_cache_is_expired(self):
         config_fetcher = ConfigFetcherMock()
         poll_interval_seconds = 2
         max_init_wait_time_seconds = 1
-        config_cache = SingleValueConfigCache(json.dumps({
-            ConfigEntry.CONFIG: json.loads(TEST_JSON),
-            ConfigEntry.ETAG: 'test-etag',
-            ConfigEntry.FETCH_TIME: get_utc_now_seconds_since_epoch() - poll_interval_seconds
-        }))
+        config_cache = SingleValueConfigCache(ConfigEntry(
+            config=json.loads(TEST_JSON),
+            etag='test-etag',
+            config_json_string=TEST_JSON,
+            fetch_time=get_utc_now_seconds_since_epoch() - poll_interval_seconds).serialize()
+        )
         cache_policy = ConfigService('', PollingMode.auto_poll(poll_interval_seconds,
                                                                max_init_wait_time_seconds),
                                      Hooks(), config_fetcher, log, config_cache, False)
@@ -260,11 +262,12 @@ def test_init_wait_time_return_cached(self):
         config_fetcher = ConfigFetcherWaitMock(5)
         poll_interval_seconds = 60
         max_init_wait_time_seconds = 1
-        config_cache = SingleValueConfigCache(json.dumps({
-            ConfigEntry.CONFIG: json.loads(TEST_JSON2),
-            ConfigEntry.ETAG: 'test-etag',
-            ConfigEntry.FETCH_TIME: get_utc_now_seconds_since_epoch() - 2 * poll_interval_seconds
-        }))
+        config_cache = SingleValueConfigCache(ConfigEntry(
+            config=json.loads(TEST_JSON2),
+            etag='test-etag',
+            config_json_string=TEST_JSON2,
+            fetch_time=get_utc_now_seconds_since_epoch() - 2 * poll_interval_seconds).serialize()
+        )
 
         start_time = time.time()
         cache_policy = ConfigService('', PollingMode.auto_poll(poll_interval_seconds,

configcatclienttests/test_configcache.py

@@ -1,8 +1,14 @@
+import json
 import logging
 import unittest
 
+from configcatclient import ConfigCatClient, ConfigCatOptions, PollingMode
 from configcatclient.configcache import InMemoryConfigCache
-from configcatclienttests.mocks import TEST_JSON
+from configcatclient.configcatoptions import Hooks
+from configcatclient.configentry import ConfigEntry
+from configcatclient.configservice import ConfigService
+from configcatclient.utils import get_utc_now_seconds_since_epoch
+from configcatclienttests.mocks import TEST_JSON, SingleValueConfigCache, HookCallbacks, TEST_JSON_FORMAT
 
 logging.basicConfig()
 
@@ -22,6 +28,61 @@ def test_cache(self):
         value2 = config_store.get('key2')
         self.assertEqual(value2, None)
 
+    def test_cache_key(self):
+        self.assertEqual("147c5b4c2b2d7c77e1605b1a4309f0ea6684a0c6", ConfigService._get_cache_key('test1'))
+        self.assertEqual("c09513b1756de9e4bc48815ec7a142b2441ed4d5", ConfigService._get_cache_key('test2'))
+
+    def test_cache_payload(self):
+        now_seconds = 1686756435.8449
+        etag = 'test-etag'
+        entry = ConfigEntry(json.loads(TEST_JSON), etag, TEST_JSON, now_seconds)
+        self.assertEqual('1686756435844' + '\n' + etag + '\n' + TEST_JSON, entry.serialize())
+
+    def test_invalid_cache_content(self):
+        hook_callbacks = HookCallbacks()
+        hooks = Hooks(on_error=hook_callbacks.on_error)
+        config_json_string = TEST_JSON_FORMAT.format(value='"test"')
+        config_cache = SingleValueConfigCache(ConfigEntry(
+            config=json.loads(config_json_string),
+            etag='test-etag',
+            config_json_string=config_json_string,
+            fetch_time=get_utc_now_seconds_since_epoch()).serialize()
+        )
+
+        client = ConfigCatClient.get('test', ConfigCatOptions(polling_mode=PollingMode.manual_poll(),
+                                                              config_cache=config_cache,
+                                                              hooks=hooks))
+
+        self.assertEqual('test', client.get_value('testKey', 'default'))
+        self.assertEqual(0, hook_callbacks.error_call_count)
+
+        # Invalid fetch time in cache
+        config_cache._value = '\n'.join(['text',
+                                         'test-etag',
+                                         TEST_JSON_FORMAT.format(value='"test2"')])
+
+        self.assertEqual('test', client.get_value('testKey', 'default'))
+        self.assertTrue('Error occurred while reading the cache.\nInvalid fetch time: text' in hook_callbacks.error)
+
+        # Number of values is fewer than expected
+        config_cache._value = '\n'.join([str(get_utc_now_seconds_since_epoch()),
+                                         TEST_JSON_FORMAT.format(value='"test2"')])
+
+        self.assertEqual('test', client.get_value('testKey', 'default'))
+        self.assertTrue('Error occurred while reading the cache.\nNumber of values is fewer than expected.'
+                        in hook_callbacks.error)
+
+        # Invalid config JSON
+        config_cache._value = '\n'.join([str(get_utc_now_seconds_since_epoch()),
+                                         'test-etag',
+                                         'wrong-json'])
+
+        self.assertEqual('test', client.get_value('testKey', 'default'))
+        self.assertTrue('Error occurred while reading the cache.\nInvalid config JSON: wrong-json.'
+                        in hook_callbacks.error)
+
+        client.close()
+
 
 if __name__ == '__main__':
     unittest.main()

configcatclienttests/test_configcatclient.py

@@ -177,16 +177,6 @@ def test_get_value_details(self):
 
             client.close()
 
-    def test_cache_key(self):
-        client1 = ConfigCatClient.get('test1', ConfigCatOptions(polling_mode=PollingMode.manual_poll(),
-                                                                config_cache=ConfigCacheMock()))
-        client2 = ConfigCatClient.get('test2', ConfigCatOptions(polling_mode=PollingMode.manual_poll(),
-                                                                config_cache=ConfigCacheMock()))
-        self.assertEqual("5a9acc8437104f46206f6f273c4a5e26dd14715c", client1._ConfigCatClient__get_cache_key())
-        self.assertEqual("ade7f71ba5d52ebd3d9aeef5f5488e6ffe6323b8", client2._ConfigCatClient__get_cache_key())
-        client1.close()
-        client2.close()
-
     def test_default_user_get_value(self):
         client = ConfigCatClient.get('test', ConfigCatOptions(polling_mode=PollingMode.manual_poll(),
                                                               config_cache=ConfigCacheMock()))