diff --git a/app/__init__.py b/app/__init__.py index b70b850..404f0cf 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -17,7 +17,7 @@ login_manager = LoginManager() -def create_app(): +def create_app(config=None): from app.lib.screen import Screen, ScreenLoadError app = Flask(__name__) @@ -40,6 +40,7 @@ def create_app(): app.config['ENABLE_USERS'] = bool(app.config.get('ENABLE_USERS', False)) app.config['ENABLE_DISPLAY_APPROVAL'] = bool(app.config.get('ENABLE_DISPLAY_APPROVAL', False)) app.config['ENABLE_DISPLAY_AUTH'] = bool(app.config.get('ENABLE_DISPLAY_AUTH', False)) + app.config.update(config or {}) Bootstrap(app) db.init_app(app) diff --git a/app/lib/cache.py b/app/lib/cache.py index 0b12ed1..00c7f5f 100644 --- a/app/lib/cache.py +++ b/app/lib/cache.py @@ -5,6 +5,8 @@ import pickle import random +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm.exc import StaleDataError from flask import Flask import arrow @@ -89,11 +91,11 @@ def __init__(self, app: Flask): raise RuntimeError("Filesystem cache dir does not exist or is not a directory: " + self.cache_dir) def _get_path(self, key: str) -> str: - return os.path.join(self.cache_dir, hashlib.new('sha256', key).hexdigest()) + return os.path.join(self.cache_dir, hashlib.new('sha256', key.encode('utf-8')).hexdigest()) def _load_key(self, key: str) -> Optional[Any]: filename = self._get_path(key) - if os.is_file(filename): + if os.path.isfile(filename): try: with open(filename, 'rb') as fp: res = pickle.load(fp) @@ -102,7 +104,10 @@ def _load_key(self, key: str) -> Optional[Any]: except: pass - os.unlink(filename) + try: + os.unlink(filename) + except: + pass def get(self, key: str) -> Optional[Any]: res = self._load_key(key) @@ -119,8 +124,11 @@ def set(self, key: str, expiry: int, data: Any) -> bool: def delete(self, key: str) -> bool: filename = self._get_path(key) - if os.is_file(filename): - os.unlink(filename) + try: + if os.path.isfile(filename): + os.unlink(filename) + return True + except FileNotFoundError: return True return False @@ -134,12 +142,17 @@ def __init__(self, app: Flask): self.db = db self.CacheModel = CacheModel - def get(self, key: str) -> Optional[Any]: + def _maybe_cleanup(self): # 1% chance to clean up if random.random() <= 0.01: - self.CacheModel.query.filter(self.CacheModel.expires <= arrow.utcnow()).delete() - self.db.session.commit() + try: + self.CacheModel.query.filter(self.CacheModel.expires <= arrow.utcnow()).delete() + self.db.session.commit() + except IntegrityError: + self.db.session.rollback() + def get(self, key: str) -> Optional[Any]: + self._maybe_cleanup() obj = self.CacheModel.query.get(key) if obj: if obj.expires > arrow.utcnow(): @@ -147,23 +160,35 @@ def get(self, key: str) -> Optional[Any]: return pickle.loads(obj.data) except: pass - self.db.session.delete(obj) - self.db.session.commit() + try: + self.db.session.delete(obj) + self.db.session.commit() + except IntegrityError: + self.db.session.rollback() def set(self, key: str, expiry: int, data: Any) -> bool: - obj = self.CacheModel.query.get(key) - if not obj: - obj = self.CacheModel(key=key) - self.db.session.add(obj) - obj.expires = arrow.utcnow().shift(seconds=expiry) - obj.data = pickle.dumps(data) - self.db.session.commit() + while True: + try: + obj = self.CacheModel.query.get(key) + if not obj: + obj = self.CacheModel(key=key) + self.db.session.add(obj) + obj.expires = arrow.utcnow().shift(seconds=expiry) + obj.data = pickle.dumps(data) + self.db.session.commit() + break + except (IntegrityError, StaleDataError): + self.db.session.rollback() return True def delete(self, key: str) -> bool: obj = self.CacheModel.query.get(key) if obj: - self.db.session.delete(obj) - self.db.session.commit() - return True + try: + self.db.session.delete(obj) + self.db.session.commit() + return True + except IntegrityError: + self.db.session.rollback() + return True return False diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/lib/test_cache.py b/tests/lib/test_cache.py new file mode 100644 index 0000000..ee05953 --- /dev/null +++ b/tests/lib/test_cache.py @@ -0,0 +1,401 @@ +from unittest import TestCase +from unittest.mock import patch, MagicMock +import tempfile +import shutil +import os +import time +import threading + +from app.lib.cache import ( + make_key_with_args, + Cache, + FilesystemDriver, + DatabaseDriver, +) +from app import create_app, db + + +@patch('app.lib.cache.hashlib') +class TestMakeKeyWithArgs(TestCase): + def test_no_args(self, mock_hl): + mock_hl.new.return_value.hexdigest.return_value = 'testhash' + res = make_key_with_args('foo') + self.assertEqual(res, 'foo-testhash') + mock_hl.new.assert_called_once_with('sha256', b'{}') + + def test_only_args(self, mock_hl): + mock_hl.new.return_value.hexdigest.return_value = 'testhash' + res = make_key_with_args('foo', 'bar', 3, 'baz') + self.assertEqual(res, 'foo-testhash') + mock_hl.new.assert_called_once_with('sha256', b'bar3baz{}') + + def test_only_callback(self, mock_hl): + mock_hl.new.return_value.hexdigest.return_value = 'testhash' + res = make_key_with_args('foo', callback='asdf') + self.assertEqual(res, 'foo-asdf-testhash') + mock_hl.new.assert_called_once_with('sha256', b'{}') + + def test_only_kwargs(self, mock_hl): + mock_hl.new.return_value.hexdigest.return_value = 'testhash' + res = make_key_with_args('foo', b='bar', a='baz', c=3) + self.assertEqual(res, 'foo-testhash') + mock_hl.new.assert_called_once_with('sha256', b"{'a': 'baz', 'b': 'bar', 'c': '3'}") + + def test_all_args(self, mock_hl): + mock_hl.new.return_value.hexdigest.return_value = 'testhash' + res = make_key_with_args('foo', 'bar', 'baz', callback='asdf', b='bar', a='baz') + self.assertEqual(res, 'foo-asdf-testhash') + mock_hl.new.assert_called_once_with('sha256', b"barbaz{'a': 'baz', 'b': 'bar'}") + + +@patch('app.lib.cache.Cache.init_app') +class TestCache__Init(TestCase): + def test_no_app(self, mock_init_app): + c = Cache() + self.assertIsNone(c.driver) + mock_init_app.assert_not_called() + + def test_with_app(self, mock_init_app): + mockapp = MagicMock() + c = Cache(app=mockapp) + self.assertIsNone(c.driver) + mock_init_app.assert_called_once_with(mockapp) + + +@patch('app.lib.cache.CacheDriver._get_driver') +class TestCache__InitApp(TestCase): + def test_no_configured_driver(self, mock_get_driver): + mockapp = MagicMock(config={}) + c = Cache() + c.init_app(mockapp) + self.assertEqual(c.driver, mock_get_driver.return_value.return_value) + mock_get_driver.assert_called_once_with(None) + mock_get_driver.return_value.assert_called_once_with(mockapp) + + def test_with_configured_driver(self, mock_get_driver): + mockapp = MagicMock(config={'CACHE_DRIVER': 'foo'}) + c = Cache() + c.init_app(mockapp) + self.assertEqual(c.driver, mock_get_driver.return_value.return_value) + mock_get_driver.assert_called_once_with('foo') + mock_get_driver.return_value.assert_called_once_with(mockapp) + + +@patch('app.lib.cache.make_key_with_args', return_value='testkey') +@patch('app.lib.cache.Cache.get', return_value=None) +@patch('app.lib.cache.Cache.set') +class TestCache__GetOrFetch(TestCase): + def test_key_in_cache(self, mock_set, mock_get, mock_mk_key): + mock_get.return_value = 'testval' + mock_callback = MagicMock() + mock_callback.__name__ = 'testcbname' + mock_callback.return_value = None + c = Cache() + res = c.get_or_fetch('foo', 3, mock_callback, 'bar', baz='quux') + self.assertEqual(res, 'testval') + mock_mk_key.assert_called_once_with('foo', 'bar', callback='testcbname', baz='quux') + mock_get.assert_called_once_with('testkey') + mock_callback.assert_not_called() + mock_set.assert_not_called() + + def test_key_not_in_cache__nothing_fetched(self, mock_set, mock_get, mock_mk_key): + mock_callback = MagicMock() + mock_callback.__name__ = 'testcbname' + mock_callback.return_value = None + c = Cache() + res = c.get_or_fetch('foo', 3, mock_callback, 'bar', baz='quux') + self.assertIsNone(res) + mock_mk_key.assert_called_once_with('foo', 'bar', callback='testcbname', baz='quux') + mock_get.assert_called_once_with('testkey') + mock_callback.assert_called_once_with('bar', baz='quux') + mock_set.assert_not_called() + + def test_key_not_in_cache__value_fetched(self, mock_set, mock_get, mock_mk_key): + mock_callback = MagicMock() + mock_callback.__name__ = 'testcbname' + mock_callback.return_value = 'testval' + c = Cache() + res = c.get_or_fetch('foo', 3, mock_callback, 'bar', baz='quux') + self.assertEqual(res, 'testval') + mock_mk_key.assert_called_once_with('foo', 'bar', callback='testcbname', baz='quux') + mock_get.assert_called_once_with('testkey') + mock_callback.assert_called_once_with('bar', baz='quux') + mock_set.assert_called_once_with('testkey', 3, 'testval') + + + +class TestCache__Get(TestCase): + def test_no_driver(self): + c = Cache() + res = c.get('foo') + self.assertIsNone(res) + + def test_with_driver(self): + mock_driver = MagicMock() + c = Cache() + c.driver = mock_driver + res = c.get('foo') + self.assertEqual(res, mock_driver.get.return_value) + mock_driver.get.assert_called_once_with('foo') + + +class TestCache__Set(TestCase): + def test_no_driver(self): + c = Cache() + res = c.set('foo', 3, 'bar') + self.assertTrue(res) + + def test_with_driver(self): + mock_driver = MagicMock() + c = Cache() + c.driver = mock_driver + res = c.set('foo', 3, 'bar') + self.assertEqual(res, mock_driver.set.return_value) + mock_driver.set.assert_called_once_with('foo', 3, 'bar') + + +class TestCache__Delete(TestCase): + def test_no_driver(self): + c = Cache() + res = c.delete('foo') + self.assertTrue(res) + + def test_with_driver(self): + mock_driver = MagicMock() + c = Cache() + c.driver = mock_driver + res = c.delete('foo') + self.assertEqual(res, mock_driver.delete.return_value) + mock_driver.delete.assert_called_once_with('foo') + + +class BaseCacheDriverTest: + def setUp(self): + self.base_tempdir = os.path.join(tempfile.gettempdir(), 'fruitstand-tests') + self.cache_tempdir = os.path.join(self.base_tempdir, 'cache') + self.db_tempdir = os.path.join(self.base_tempdir, 'database') + os.makedirs(self.cache_tempdir, exist_ok=True) + os.makedirs(self.db_tempdir, exist_ok=True) + config = { + 'FILESYSTEM_CACHE_DIR': self.cache_tempdir, + 'SQLALCHEMY_DATABASE_URI': 'sqlite:///{}'.format( + os.path.abspath(os.path.join(self.db_tempdir, 'test.sqlite3')) + ), + } + self.app = create_app(config=config) + self.driver = self.DriverClass(self.app) + with self.app.app_context(): + db.create_all() + super().setUp() + + def tearDown(self): + super().tearDown() + del self.driver + del self.app + shutil.rmtree(self.base_tempdir) + del self.cache_tempdir + del self.db_tempdir + del self.base_tempdir + + def test_get_set(self): + with self.app.app_context(): + d = self.driver + self.assertIsNone(d.get('testkey')) + self.assertTrue(d.set('testkey', 1, 'testval')) + self.assertEqual(d.get('testkey'), 'testval') + d.delete('testkey') + self.assertIsNone(d.get('testkey')) + + def test_expire(self): + with self.app.app_context(): + d = self.driver + d.set('testkey', 1, 'testval') + self.assertEqual(d.get('testkey'), 'testval') + time.sleep(1) + self.assertIsNone(d.get('testkey')) + + def test_concurrent_wait(self): + n_threads = 10 + r_threads = [] + errors = [] + err_lock = threading.Lock() + def err(part, msg, *a, exc=None, **ka): + msg = msg.format(*a, **ka) + if exc: + msg += f': {exc.__class__.__name__}:{exc}' + msg = f'{threading.current_thread().name}:{part} - {msg}' + with err_lock: + errors.append(msg) + + def runner(bs, n): + with self.app.app_context(): + d = self.DriverClass(self.app) + r = list(range(n_threads)) + bs[0].wait() + try: + res = d.get('testkey') + if res is not None: + err('get_1_nx', 'value should be none, is {}', res) + except Exception as e: + err('get_1_nx', 'exception', exc=e) + + bs[1].wait() + try: + res = d.set('testkey', 1, n) + if res is not True: + err('set_1', 'failed to set') + except Exception as e: + err('set_1', 'exception', exc=e) + + bs[2].wait() + try: + res = d.get('testkey') + if res not in r: + err('get_1', 'value {} not in {}', res, r) + except Exception as e: + err('get_1', 'exception', exc=e) + + bs[3].wait() + try: + d.delete('testkey') + except Exception as e: + err('del_1', 'exception', exc=e) + + bs[4].wait() + try: + res = d.get('testkey') + if res is not None: + err('get_2_nx', 'deleted key present, is {}', res) + except Exception as e: + err('get_2_nx', 'exception', exc=e) + + bs[5].wait() + try: + res = d.set('testkey', 1, n) + if res is not True: + err('set_2', 'failed to set') + except Exception as e: + err('set_2', 'exception', exc=e) + + bs[6].wait() + try: + res = d.get('testkey') + if res not in r: + err('get_2', 'value {} not in {}', res, r) + except Exception as e: + err('get_2', 'exception', exc=e) + + time.sleep(1) + bs[7].wait() + try: + res = d.get('testkey') + if res is not None: + err('get_2_exp', 'expired key present, is {}', res) + except Exception as e: + err('get_2_exp', 'exception', exc=e) + + with err_lock: + r_threads.append(1) + + bs = [threading.Barrier(n_threads) for _ in range(8)] + threads = [threading.Thread(target=runner, args=(bs, i)) for i in range(n_threads)] + for t in threads: + t.start() + for t in threads: + t.join() + + for e in errors: + print(e) + self.assertEqual(errors, []) + self.assertEqual(len(r_threads), n_threads) + + def test_concurrent_nowait(self): + n_threads = 10 + r_threads = [] + errors = [] + err_lock = threading.Lock() + def err(part, msg, *a, exc=None, **ka): + msg = msg.format(*a, **ka) + if exc: + msg += f': {exc.__class__.__name__}:{exc}' + msg = f'{threading.current_thread().name}:{part} - {msg}' + with err_lock: + errors.append(msg) + + def runner(n): + with self.app.app_context(): + d = self.DriverClass(self.app) + r = list(range(n_threads)) + error_key = 'get_1_nx' + try: + res = d.get('testkey') + except Exception as e: + err('get_1_nx', 'exception', exc=e) + + try: + res = d.set('testkey', 1, n) + if res is not True: + err('set_1', 'failed to set') + except Exception as e: + err('set_1', 'exception', exc=e) + + try: + res = d.get('testkey') + if res is not None: + if res not in r: + err('get_1', 'value {} not in {}', res, r) + except Exception as e: + err('get_1', 'exception', exc=e) + + try: + d.delete('testkey') + except Exception as e: + err('del_1', 'exception', exc=e) + + try: + res = d.get('testkey') + except Exception as e: + err('get_2_nx', 'exception', exc=e) + + try: + res = d.set('testkey', 1, n) + if res is not True: + err('set_2', 'failed to set') + except Exception as e: + err('set_2', 'exception', exc=e) + + try: + res = d.get('testkey') + if res is not None: + if res not in r: + err('get_2', 'value {} not in {}', res, r) + except Exception as e: + err('get_2', 'exception', exc=e) + + time.sleep(1) + try: + res = d.get('testkey') + except Exception as e: + err('get_2_exp', 'exception', exc=e) + + with err_lock: + r_threads.append(1) + + threads = [threading.Thread(target=runner, args=(i,)) for i in range(n_threads)] + for t in threads: + t.start() + for t in threads: + t.join() + + for e in errors: + print(e) + self.assertEqual(errors, []) + self.assertEqual(len(r_threads), n_threads) + + +class TestFilesystemDriver(BaseCacheDriverTest, TestCase): + DriverClass = FilesystemDriver + + +class TestDatabaseDriver(BaseCacheDriverTest, TestCase): + DriverClass = DatabaseDriver