Skip to content
This repository has been archived by the owner on May 11, 2021. It is now read-only.

Commit

Permalink
Make local_cache threadsafe
Browse files Browse the repository at this point in the history
  • Loading branch information
Jaidan authored and Adam Chainz committed Oct 26, 2016
1 parent 44b7a9c commit 7491f4a
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 20 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
test:
flake8 --exclude=migrations --ignore=E501,E225,E121,E123,E124,E125,E127,E128 --exit-zero modeldict || exit 1
python setup.py test
python runtests.py
31 changes: 16 additions & 15 deletions modeldict/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class CachedDict(object):
def __init__(self, cache=cache, timeout=30):
cls_name = type(self).__name__

self._local_cache = None
self._local_cache = {}
self._local_last_updated = None

self._last_checked_for_remote_changes = 0.0
Expand All @@ -26,6 +26,9 @@ def __getitem__(self, key):
try:
return self._local_cache[key]
except KeyError:
if self._local_last_updated is None:
# Another thread reset the cache
return self[key]
value = self.get_default(key)

if value is NoValue:
Expand All @@ -40,9 +43,7 @@ def __delitem__(self, key):
raise NotImplementedError

def __len__(self):
if self._local_cache is None:
self._populate()

self._populate()
return len(self._local_cache)

def __contains__(self, key):
Expand Down Expand Up @@ -115,8 +116,8 @@ def local_cache_is_invalid(self):
A return value of ``None`` signifies that no data was available.
"""
# If the local_cache is empty, avoid hitting memcache entirely
if self._local_cache is None:
# If the local_cache hasn't been set up, avoid hitting memcache entirely
if self._local_last_updated is None:
return True

remote_last_updated = self.remote_cache.get(
Expand Down Expand Up @@ -144,9 +145,9 @@ def clear_cache(self):
"""
Clears the in-process cache.
"""
self._local_cache = None
self._local_last_updated = None
self._last_checked_for_remote_changes = 0.0
self._local_cache.clear()

def _populate(self, reset=False):
"""
Expand All @@ -163,9 +164,8 @@ def _populate(self, reset=False):
"""
now = time.time()

# If asked to reset, then simply set local cache to None
if reset:
self._local_cache = None
self.clear_cache()
# Otherwise, if the local cache has expired, we need to go check with
# our remote last_updated value to see if the dict values have changed.
elif self.local_cache_has_expired():
Expand All @@ -181,17 +181,18 @@ def _populate(self, reset=False):
# pull in the values from the remote cache and set it to the
# local_cache
if local_cache_is_invalid or local_cache_is_invalid is None:
self._local_cache = self.remote_cache.get(self.remote_cache_key)

# We've updated from remote, so mark ourselves as
# such so that we won't expire until the next timeout
self._local_last_updated = now
remote_value = self.remote_cache.get(self.remote_cache_key)
if remote_value is not None:
self._local_cache = remote_value
# We've updated from remote, so mark ourselves as
# such so that we won't expire until the next timeout
self._local_last_updated = now

# We last checked for remote changes just now
self._last_checked_for_remote_changes = now

# Update from cache if local_cache is still empty
if self._local_cache is None:
if self._local_last_updated is None:
self._update_cache_data()

return self._local_cache
Expand Down
14 changes: 10 additions & 4 deletions tests/testapp/test_modeldict.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def test_modeldict_expirey(self):

mydict = ModelDict(ModelDictModel, key='key', value='value')

assert mydict._local_cache is None
assert mydict._local_cache == {}

mydict['test_modeldict_expirey'] = 'hello'

Expand Down Expand Up @@ -318,7 +318,8 @@ def test_switch_access(self):

def test_switch_access_without_local_cache(self):
self.mydict['hello'] = 'foo'
self.mydict._local_cache = None
self.mydict._local_cache = {}
self.mydict._local_last_updated = None
self.mydict._last_checked_for_remote_changes = 0.0
self.cache.reset_mock()
foo = self.mydict['hello']
Expand Down Expand Up @@ -373,6 +374,7 @@ def setUp(self):
@mock.patch('modeldict.base.CachedDict.local_cache_is_invalid', mock.Mock(return_value=False))
def test_expired_does_update_data(self, _update_cache_data):
self.mydict._local_cache = {}
self.mydict._local_last_updated = time.time()
self.mydict._last_checked_for_remote_changes = time.time()
self.mydict._populate()

Expand All @@ -383,6 +385,7 @@ def test_expired_does_update_data(self, _update_cache_data):
@mock.patch('modeldict.base.CachedDict.local_cache_is_invalid', mock.Mock(return_value=True))
def test_reset_does_expire(self, _update_cache_data):
self.mydict._local_cache = {}
self.mydict._local_last_updated = time.time()
self.mydict._last_checked_for_remote_changes = time.time()
self.mydict._populate(reset=True)

Expand All @@ -393,6 +396,7 @@ def test_reset_does_expire(self, _update_cache_data):
@mock.patch('modeldict.base.CachedDict.local_cache_is_invalid', mock.Mock(return_value=True))
def test_does_not_expire_by_default(self, _update_cache_data):
self.mydict._local_cache = {}
self.mydict._local_last_updated = time.time()
self.mydict._last_checked_for_remote_changes = time.time()
self.mydict._populate()

Expand Down Expand Up @@ -424,15 +428,17 @@ def test_is_not_expired_if_remote_cache_is_old(self):
def test_is_expired_if_remote_cache_is_new(self):
# set it to an expired time, but with a local cache
self.mydict._local_cache = dict(a=1)
self.mydict._last_checked_for_remote_changes = time.time() - 101
last_update = time.time() - 101
self.mydict._local_last_updated = last_update
self.mydict._last_checked_for_remote_changes = last_update
self.cache.get.return_value = time.time()

result = self.mydict.local_cache_is_invalid()

assert result
self.cache.get.assert_called_once_with(
self.mydict.remote_cache_last_updated_key
)
assert result

def test_is_invalid_if_local_cache_is_none(self):
self.mydict._local_cache = None
Expand Down

0 comments on commit 7491f4a

Please sign in to comment.