From 80a4e6263fd53f83a710d2a2e6c57baae38cb14b Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Sat, 7 Jul 2018 19:46:16 +0200 Subject: [PATCH] Added Unittests for cache fixed Lock & Cache bugs --- src/Core/Cache/ArrayCache.php | 2 +- src/Core/Cache/DatabaseCacheDriver.php | 10 +- src/Core/Cache/ICacheDriver.php | 3 +- src/Core/Cache/MemcacheCacheDriver.php | 8 +- src/Core/Cache/MemcachedCacheDriver.php | 10 +- src/Core/Cache/RedisCacheDriver.php | 13 ++- src/Core/Lock/CacheLockDriver.php | 5 +- src/Core/Lock/DatabaseLockDriver.php | 9 +- src/Core/Lock/ILockDriver.php | 4 +- src/Core/Lock/SemaphoreLockDriver.php | 5 +- tests/src/Core/Cache/ArrayCacheDriverTest.php | 32 ++++++ tests/src/Core/Cache/CacheTest.php | 106 ++++++++++++++++++ .../Core/Cache/DatabaseCacheDriverTest.php | 25 +++++ .../Core/Cache/MemcachedCacheDriverTest.php | 39 +++++++ tests/src/Core/Cache/RedisCacheDriverTest.php | 39 +++++++ .../Core/Lock/ArrayCacheLockDriverTest.php | 6 + tests/src/Core/Lock/LockTest.php | 20 ++++ .../src/Core/Lock/SemaphoreLockDriverTest.php | 6 + 18 files changed, 317 insertions(+), 25 deletions(-) create mode 100644 tests/src/Core/Cache/ArrayCacheDriverTest.php create mode 100644 tests/src/Core/Cache/CacheTest.php create mode 100644 tests/src/Core/Cache/DatabaseCacheDriverTest.php create mode 100644 tests/src/Core/Cache/MemcachedCacheDriverTest.php create mode 100644 tests/src/Core/Cache/RedisCacheDriverTest.php diff --git a/src/Core/Cache/ArrayCache.php b/src/Core/Cache/ArrayCache.php index ec9f3a2577..b198287148 100644 --- a/src/Core/Cache/ArrayCache.php +++ b/src/Core/Cache/ArrayCache.php @@ -51,7 +51,7 @@ class ArrayCache extends AbstractCacheDriver implements IMemoryCacheDriver /** * (@inheritdoc) */ - public function clear() + public function clear($outdated = true) { $this->cachedData = []; return true; diff --git a/src/Core/Cache/DatabaseCacheDriver.php b/src/Core/Cache/DatabaseCacheDriver.php index 5c71fb1962..059fef3290 100644 --- a/src/Core/Cache/DatabaseCacheDriver.php +++ b/src/Core/Cache/DatabaseCacheDriver.php @@ -37,7 +37,7 @@ class DatabaseCacheDriver extends AbstractCacheDriver implements ICacheDriver { $fields = [ 'v' => serialize($value), - 'expires' => DateTimeFormat::utc('now + ' . $ttl . ' seconds'), + 'expires' => DateTimeFormat::utc('now + ' . $ttl . 'seconds'), 'updated' => DateTimeFormat::utcNow() ]; @@ -49,8 +49,12 @@ class DatabaseCacheDriver extends AbstractCacheDriver implements ICacheDriver return dba::delete('cache', ['k' => $key]); } - public function clear() + public function clear($outdated = true) { - return dba::delete('cache', ['`expires` < NOW()']); + if ($outdated) { + return dba::delete('cache', ['`expires` < NOW()']); + } else { + return dba::delete('cache', ['`k` IS NOT NULL ']); + } } } diff --git a/src/Core/Cache/ICacheDriver.php b/src/Core/Cache/ICacheDriver.php index ced7b4e216..e83b921c6a 100644 --- a/src/Core/Cache/ICacheDriver.php +++ b/src/Core/Cache/ICacheDriver.php @@ -42,8 +42,9 @@ interface ICacheDriver /** * Remove outdated data from the cache + * @param boolean $outdated just remove outdated values * * @return bool */ - public function clear(); + public function clear($outdated = true); } diff --git a/src/Core/Cache/MemcacheCacheDriver.php b/src/Core/Cache/MemcacheCacheDriver.php index af7e5ab0e2..cd12be8ecd 100644 --- a/src/Core/Cache/MemcacheCacheDriver.php +++ b/src/Core/Cache/MemcacheCacheDriver.php @@ -96,9 +96,13 @@ class MemcacheCacheDriver extends AbstractCacheDriver implements IMemoryCacheDri /** * (@inheritdoc) */ - public function clear() + public function clear($outdated = true) { - return $this->memcache->flush(); + if ($outdated) { + return true; + } else { + return $this->memcache->flush(); + } } /** diff --git a/src/Core/Cache/MemcachedCacheDriver.php b/src/Core/Cache/MemcachedCacheDriver.php index dda6411a96..d4aab15c92 100644 --- a/src/Core/Cache/MemcachedCacheDriver.php +++ b/src/Core/Cache/MemcachedCacheDriver.php @@ -58,7 +58,7 @@ class MemcachedCacheDriver extends AbstractCacheDriver implements IMemoryCacheDr return $this->memcached->set( $cachekey, $value, - time() + $ttl + $ttl ); } else { return $this->memcached->set( @@ -75,9 +75,13 @@ class MemcachedCacheDriver extends AbstractCacheDriver implements IMemoryCacheDr return $this->memcached->delete($cachekey); } - public function clear() + public function clear($outdated = true) { - return $this->memcached->flush(); + if ($outdated) { + return true; + } else { + return $this->memcached->flush(); + } } /** diff --git a/src/Core/Cache/RedisCacheDriver.php b/src/Core/Cache/RedisCacheDriver.php index 70412fd215..223c2b8a94 100644 --- a/src/Core/Cache/RedisCacheDriver.php +++ b/src/Core/Cache/RedisCacheDriver.php @@ -61,7 +61,7 @@ class RedisCacheDriver extends AbstractCacheDriver implements IMemoryCacheDriver if ($ttl > 0) { return $this->redis->setex( $cachekey, - time() + $ttl, + $ttl, $cached ); } else { @@ -78,12 +78,15 @@ class RedisCacheDriver extends AbstractCacheDriver implements IMemoryCacheDriver return ($this->redis->delete($cachekey) > 0); } - public function clear() + public function clear($outdated = true) { - return $this->redis->flushAll(); + if ($outdated) { + return true; + } else { + return $this->redis->flushAll(); + } } - /** * (@inheritdoc) */ @@ -92,7 +95,7 @@ class RedisCacheDriver extends AbstractCacheDriver implements IMemoryCacheDriver $cachekey = $this->getCacheKey($key); $cached = json_encode($value); - return $this->redis->setnx($cachekey, $value); + return $this->redis->setnx($cachekey, $cached); } /** diff --git a/src/Core/Lock/CacheLockDriver.php b/src/Core/Lock/CacheLockDriver.php index ccbbb8a350..18d441ffea 100644 --- a/src/Core/Lock/CacheLockDriver.php +++ b/src/Core/Lock/CacheLockDriver.php @@ -2,6 +2,7 @@ namespace Friendica\Core\Lock; +use Friendica\Core\Cache; use Friendica\Core\Cache\IMemoryCacheDriver; class CacheLockDriver extends AbstractLockDriver @@ -24,7 +25,7 @@ class CacheLockDriver extends AbstractLockDriver /** * (@inheritdoc) */ - public function acquireLock($key, $timeout = 120) + public function acquireLock($key, $timeout = 120, $ttl = Cache::FIVE_MINUTES) { $got_lock = false; $start = time(); @@ -43,7 +44,7 @@ class CacheLockDriver extends AbstractLockDriver // At first initialize it with "0" $this->cache->add($cachekey, 0); // Now the value has to be "0" because otherwise the key was used by another process meanwhile - if ($this->cache->compareSet($cachekey, 0, getmypid(), 300)) { + if ($this->cache->compareSet($cachekey, 0, getmypid(), $ttl)) { $got_lock = true; $this->markAcquire($key); } diff --git a/src/Core/Lock/DatabaseLockDriver.php b/src/Core/Lock/DatabaseLockDriver.php index 6f4b942a4d..fc057c6b55 100644 --- a/src/Core/Lock/DatabaseLockDriver.php +++ b/src/Core/Lock/DatabaseLockDriver.php @@ -3,6 +3,7 @@ namespace Friendica\Core\Lock; use dba; +use Friendica\Core\Cache; use Friendica\Database\DBM; use Friendica\Util\DateTimeFormat; @@ -14,7 +15,7 @@ class DatabaseLockDriver extends AbstractLockDriver /** * (@inheritdoc) */ - public function acquireLock($key, $timeout = 120) + public function acquireLock($key, $timeout = 120, $ttl = Cache::FIVE_MINUTES) { $got_lock = false; $start = time(); @@ -28,16 +29,14 @@ class DatabaseLockDriver extends AbstractLockDriver // We want to lock something that was already locked by us? So we got the lock. if ($lock['pid'] == getmypid()) { $got_lock = true; - $this->markAcquire($key); } } if (!$lock['locked']) { - dba::update('locks', ['locked' => true, 'pid' => getmypid(), 'expires' => DateTimeFormat::utc('now + 300seconds')], ['name' => $key]); + dba::update('locks', ['locked' => true, 'pid' => getmypid(), 'expires' => DateTimeFormat::utc('now + ' . $ttl . 'seconds')], ['name' => $key]); $got_lock = true; - $this->markAcquire($key); } } else { - dba::insert('locks', ['name' => $key, 'locked' => true, 'pid' => getmypid(), 'expires' => DateTimeFormat::utc('now + 300seconds')]); + dba::insert('locks', ['name' => $key, 'locked' => true, 'pid' => getmypid(), 'expires' => DateTimeFormat::utc('now + ' . $ttl . 'seconds')]); $got_lock = true; $this->markAcquire($key); } diff --git a/src/Core/Lock/ILockDriver.php b/src/Core/Lock/ILockDriver.php index cd54cc03c3..a255f68345 100644 --- a/src/Core/Lock/ILockDriver.php +++ b/src/Core/Lock/ILockDriver.php @@ -1,6 +1,7 @@ cache = new ArrayCache(); + return $this->cache; + } + + public function tearDown() + { + $this->cache->clear(); + parent::tearDown(); + } + + public function testTTL() + { + // Array Cache doesn't support TTL + return true; + } +} diff --git a/tests/src/Core/Cache/CacheTest.php b/tests/src/Core/Cache/CacheTest.php new file mode 100644 index 0000000000..419e0cd2bd --- /dev/null +++ b/tests/src/Core/Cache/CacheTest.php @@ -0,0 +1,106 @@ +instance = $this->getInstance(); + + // Reusable App object + $this->app = new App(__DIR__.'/../'); + $a = $this->app; + + // Default config + Config::set('config', 'hostname', 'localhost'); + Config::set('system', 'throttle_limit_day', 100); + Config::set('system', 'throttle_limit_week', 100); + Config::set('system', 'throttle_limit_month', 100); + Config::set('system', 'theme', 'system_theme'); + } + + function testSimple() { + $this->assertNull($this->instance->get('value1')); + + $value='foobar'; + $this->instance->set('value1', $value); + $received=$this->instance->get('value1'); + $this->assertEquals($value, $received, 'Value received from cache not equal to the original'); + $value='ipsum lorum'; + $this->instance->set('value1', $value); + $received=$this->instance->get('value1'); + $this->assertEquals($value, $received, 'Value not overwritten by second set'); + + $value2='foobar'; + $this->instance->set('value2', $value2); + $received2=$this->instance->get('value2'); + $this->assertEquals($value, $received, 'Value changed while setting other variable'); + $this->assertEquals($value2, $received2, 'Second value not equal to original'); + + $this->assertNull($this->instance->get('not_set'), 'Unset value not equal to null'); + + $this->assertTrue($this->instance->delete('value1')); + $this->assertNull($this->instance->get('value1')); + } + + function testClear() { + $value='ipsum lorum'; + $this->instance->set('1_value1', $value . '1'); + $this->instance->set('1_value2', $value . '2'); + $this->instance->set('2_value1', $value . '3'); + $this->instance->set('3_value1', $value . '4'); + + $this->assertEquals([ + '1_value1' => 'ipsum lorum1', + '1_value2' => 'ipsum lorum2', + '2_value1' => 'ipsum lorum3', + '3_value1' => 'ipsum lorum4', + ], [ + '1_value1' => $this->instance->get('1_value1'), + '1_value2' => $this->instance->get('1_value2'), + '2_value1' => $this->instance->get('2_value1'), + '3_value1' => $this->instance->get('3_value1'), + ]); + + $this->assertTrue($this->instance->clear(false)); + + $this->assertEquals([ + '1_value1' => null, + '1_value2' => null, + '2_value1' => null, + '3_value1' => null, + ], [ + '1_value1' => $this->instance->get('1_value1'), + '1_value2' => $this->instance->get('1_value2'), + '2_value1' => $this->instance->get('2_value1'), + '3_value1' => $this->instance->get('3_value1'), + ]); + } + + function testTTL() { + $this->assertNull($this->instance->get('value1')); + + $value='foobar'; + $this->instance->set('value1', $value, 1); + $received=$this->instance->get('value1'); + $this->assertEquals($value, $received, 'Value received from cache not equal to the original'); + + sleep(2); + + $this->assertNull($this->instance->get('value1')); + } +} diff --git a/tests/src/Core/Cache/DatabaseCacheDriverTest.php b/tests/src/Core/Cache/DatabaseCacheDriverTest.php new file mode 100644 index 0000000000..ed2e47fb47 --- /dev/null +++ b/tests/src/Core/Cache/DatabaseCacheDriverTest.php @@ -0,0 +1,25 @@ +cache = CacheDriverFactory::create('database'); + return $this->cache; + } + + public function tearDown() + { + $this->cache->clear(); + parent::tearDown(); + } +} diff --git a/tests/src/Core/Cache/MemcachedCacheDriverTest.php b/tests/src/Core/Cache/MemcachedCacheDriverTest.php new file mode 100644 index 0000000000..ff76ddefc4 --- /dev/null +++ b/tests/src/Core/Cache/MemcachedCacheDriverTest.php @@ -0,0 +1,39 @@ +cache = CacheDriverFactory::create('memcached'); + } catch (\Exception $exception) { + print "Memcached - TestCase failed: " . $exception->getMessage(); + throw new \Exception(); + } + return $this->cache; + } else { + $this->markTestSkipped('Memcached driver isn\'t available'); + return null; + } + } + + public function tearDown() + { + if (class_exists('Memcached')) { + $this->cache->clear(); + } + parent::tearDown(); + } +} diff --git a/tests/src/Core/Cache/RedisCacheDriverTest.php b/tests/src/Core/Cache/RedisCacheDriverTest.php new file mode 100644 index 0000000000..44ff0d42b1 --- /dev/null +++ b/tests/src/Core/Cache/RedisCacheDriverTest.php @@ -0,0 +1,39 @@ +cache = CacheDriverFactory::create('redis'); + } catch (\Exception $exception) { + print "Redis - TestCase failed: " . $exception->getMessage(); + throw new \Exception(); + } + return $this->cache; + } else { + $this->markTestSkipped('Redis driver isn\'t available'); + return null; + } + } + + public function tearDown() + { + if (class_exists('Redis')) { + $this->cache->clear(); + } + parent::tearDown(); + } +} diff --git a/tests/src/Core/Lock/ArrayCacheLockDriverTest.php b/tests/src/Core/Lock/ArrayCacheLockDriverTest.php index 0e58bf49e3..dc044f5c5c 100644 --- a/tests/src/Core/Lock/ArrayCacheLockDriverTest.php +++ b/tests/src/Core/Lock/ArrayCacheLockDriverTest.php @@ -24,4 +24,10 @@ class ArrayCacheLockDriverTest extends LockTest $this->cache->clear(); parent::tearDown(); } + + public function testLockTTL() + { + // ArrayCache doesn't support TTL + return true; + } } diff --git a/tests/src/Core/Lock/LockTest.php b/tests/src/Core/Lock/LockTest.php index a120edeb0c..dafbd74a6f 100644 --- a/tests/src/Core/Lock/LockTest.php +++ b/tests/src/Core/Lock/LockTest.php @@ -86,4 +86,24 @@ abstract class LockTest extends DatabaseTest $this->assertFalse($this->instance->isLocked('bar')); $this->assertFalse($this->instance->isLocked('nice')); } + + function testLockTTL() { + + // TODO [nupplaphil] - Because of the Datetime-Utils for the database, we have to wait a FULL second between the checks to invalidate the db-locks/cache + $this->instance->acquireLock('foo', 1, 1); + $this->instance->acquireLock('bar', 1, 3); + + $this->assertTrue($this->instance->isLocked('foo')); + $this->assertTrue($this->instance->isLocked('bar')); + + sleep(2); + + $this->assertFalse($this->instance->isLocked('foo')); + $this->assertTrue($this->instance->isLocked('bar')); + + sleep(2); + + $this->assertFalse($this->instance->isLocked('foo')); + $this->assertFalse($this->instance->isLocked('bar')); + } } diff --git a/tests/src/Core/Lock/SemaphoreLockDriverTest.php b/tests/src/Core/Lock/SemaphoreLockDriverTest.php index 48c2aff4df..cd4b915733 100644 --- a/tests/src/Core/Lock/SemaphoreLockDriverTest.php +++ b/tests/src/Core/Lock/SemaphoreLockDriverTest.php @@ -23,4 +23,10 @@ class SemaphoreLockDriverTest extends LockTest $this->semaphoreLockDriver->releaseAll(); parent::tearDown(); } + + function testLockTTL() + { + // Semaphore doesn't work with TTL + return true; + } }