From 4879753edfc7380e1e96d882716b60e981a4de53 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Wed, 18 Dec 2013 22:31:23 -0400 Subject: Fix memoize decorator: raise instead of storing None With this fix, we will re-raise an exception that happens while evaluating the callable, instead of storing None as value. --- src/leap/common/decorators.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/leap/common/decorators.py b/src/leap/common/decorators.py index ec6171a..e708fc4 100644 --- a/src/leap/common/decorators.py +++ b/src/leap/common/decorators.py @@ -56,6 +56,15 @@ class _memoized(object): :tyoe args: tuple :type kwargs: dict """ + def ret_or_raise(value): + """ + Returns the value except if it is an exception, + in which case it's raised. + """ + if isinstance(value, Exception): + raise value + return value + if self.is_method: # forget about `self` as key key_args = args[1:] @@ -74,15 +83,16 @@ class _memoized(object): if key in self.cache: logger.debug("Got value from cache...") - return self.cache[key] + value = self.cache[key] + return ret_or_raise(value) else: try: value = self.func(*args, **kwargs) except Exception as exc: logger.error("Exception while calling function: %r" % (exc,)) - value = None + value = exc self.cache[key] = value - return value + return ret_or_raise(value) def __repr__(self): """ -- cgit v1.2.3 From d46207491b9c0c0fc3a79256fe00bfe3f60712a0 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 19 Dec 2013 12:49:22 -0400 Subject: add cache invalidation --- src/leap/common/decorators.py | 61 ++++++++++++++++++++++------ src/leap/common/tests/test_check.py | 1 + src/leap/common/tests/test_memoize.py | 76 +++++++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 src/leap/common/tests/test_memoize.py diff --git a/src/leap/common/decorators.py b/src/leap/common/decorators.py index e708fc4..2ef6711 100644 --- a/src/leap/common/decorators.py +++ b/src/leap/common/decorators.py @@ -18,6 +18,7 @@ Useful decorators. """ import collections +import datetime import functools import logging @@ -32,7 +33,13 @@ class _memoized(object): If called later with the same arguments, the cached value is returned (not reevaluated). """ - def __init__(self, func, ignore_kwargs=None, is_method=False): + + # cache invalidation time, in seconds + CACHE_INVALIDATION_DELTA = 1800 + + def __init__(self, func, ignore_kwargs=None, is_method=False, + invalidation=None): + """ :param ignore_kwargs: If True, ignore all kwargs. If tuple, ignore those kwargs. @@ -45,9 +52,13 @@ class _memoized(object): self.is_method = is_method self.func = func + if invalidation: + self.CACHE_INVALIDATION_DELTA = invalidation + # TODO should put bounds to the cache dict so we do not # consume a huge amount of memory. self.cache = {} + self.cache_ts = {} def __call__(self, *args, **kwargs): """ @@ -68,6 +79,9 @@ class _memoized(object): if self.is_method: # forget about `self` as key key_args = args[1:] + else: + key_args = args + if self.ignore_kwargs is True: key = key_args else: @@ -82,17 +96,40 @@ class _memoized(object): return self.func(*args, **kwargs) if key in self.cache: - logger.debug("Got value from cache...") - value = self.cache[key] - return ret_or_raise(value) - else: - try: - value = self.func(*args, **kwargs) - except Exception as exc: - logger.error("Exception while calling function: %r" % (exc,)) - value = exc - self.cache[key] = value - return ret_or_raise(value) + if self._is_cache_still_valid(key): + value = self.cache[key] + logger.debug("Got value from cache...") + return ret_or_raise(value) + else: + logger.debug("Cache is invalid, evaluating again...") + + # no cache, or cache invalid + try: + value = self.func(*args, **kwargs) + except Exception as exc: + logger.error("Exception while calling function: %r" % (exc,)) + value = exc + self.cache[key] = value + self.cache_ts[key] = datetime.datetime.now() + return ret_or_raise(value) + + def _is_cache_still_valid(self, key, now=datetime.datetime.now): + """ + Returns True if the cache value is still valid, False otherwise. + + For now, this happen if less than CACHE_INVALIDATION_DELTA seconds + have passed from the time in which we recorded the cached value. + + :param key: the key to lookup in the cache + :type key: hashable + :param now: a callable that returns a datetime object. override + for dependency injection during testing. + :type now: callable + :rtype: bool + """ + cached_ts = self.cache_ts[key] + delta = datetime.timedelta(seconds=self.CACHE_INVALIDATION_DELTA) + return (now() - cached_ts) < delta def __repr__(self): """ diff --git a/src/leap/common/tests/test_check.py b/src/leap/common/tests/test_check.py index 6ce8493..cd488ff 100644 --- a/src/leap/common/tests/test_check.py +++ b/src/leap/common/tests/test_check.py @@ -27,6 +27,7 @@ import mock from leap.common import check + class CheckTests(unittest.TestCase): def test_raises_on_false_condition(self): with self.assertRaises(AssertionError): diff --git a/src/leap/common/tests/test_memoize.py b/src/leap/common/tests/test_memoize.py new file mode 100644 index 0000000..c923fc5 --- /dev/null +++ b/src/leap/common/tests/test_memoize.py @@ -0,0 +1,76 @@ +# -*- coding: utf-8 -*- +# test_check.py +# Copyright (C) 2013 LEAP +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +""" +Tests for: + * leap/common/decorators._memoized +""" +try: + import unittest2 as unittest +except ImportError: + import unittest + +from time import sleep + +import mock + +from leap.common.decorators import _memoized + + +class MemoizeTests(unittest.TestCase): + + def test_memoized_call(self): + """ + Test that a memoized function only calls once. + """ + witness = mock.Mock() + + @_memoized + def callmebaby(): + return witness() + + for i in range(10): + callmebaby() + witness.assert_called_once_with() + + def test_cache_invalidation(self): + """ + Test that time makes the cache invalidation expire. + """ + witness = mock.Mock() + + cache_with_alzheimer = _memoized + cache_with_alzheimer.CACHE_INVALIDATION_DELTA = 1 + + @cache_with_alzheimer + def callmebaby(*args): + return witness(*args) + + for i in range(10): + callmebaby() + witness.assert_called_once_with() + + sleep(2) + callmebaby("onemoretime") + + expected = [mock.call(), mock.call("onemoretime")] + self.assertEqual( + witness.call_args_list, + expected) + + +if __name__ == "__main__": + unittest.main() -- cgit v1.2.3