From 80a4e6263fd53f83a710d2a2e6c57baae38cb14b Mon Sep 17 00:00:00 2001
From: Philipp Holzer <admin@philipp.info>
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 @@
<?php
namespace Friendica\Core\Lock;
+use Friendica\Core\Cache;
/**
* Lock Driver Interface
@@ -23,10 +24,11 @@ interface ILockDriver
*
* @param string $key The Name of the lock
* @param integer $timeout Seconds until we give up
+ * @param integer $ttl Seconds The lock lifespan, must be one of the Cache constants
*
* @return boolean Was the lock successful?
*/
- public function acquireLock($key, $timeout = 120);
+ public function acquireLock($key, $timeout = 120, $ttl = Cache::FIVE_MINUTES);
/**
* Releases a lock if it was set by us
diff --git a/src/Core/Lock/SemaphoreLockDriver.php b/src/Core/Lock/SemaphoreLockDriver.php
index d032d46cc6..cf1ce5a8d8 100644
--- a/src/Core/Lock/SemaphoreLockDriver.php
+++ b/src/Core/Lock/SemaphoreLockDriver.php
@@ -2,6 +2,8 @@
namespace Friendica\Core\Lock;
+use Friendica\Core\Cache;
+
class SemaphoreLockDriver extends AbstractLockDriver
{
private static $semaphore = [];
@@ -30,10 +32,9 @@ class SemaphoreLockDriver extends AbstractLockDriver
}
/**
- *
* (@inheritdoc)
*/
- public function acquireLock($key, $timeout = 120)
+ public function acquireLock($key, $timeout = 120, $ttl = Cache::FIVE_MINUTES)
{
self::$semaphore[$key] = sem_get(self::semaphoreKey($key));
if (self::$semaphore[$key]) {
diff --git a/tests/src/Core/Cache/ArrayCacheDriverTest.php b/tests/src/Core/Cache/ArrayCacheDriverTest.php
new file mode 100644
index 0000000000..860e566d88
--- /dev/null
+++ b/tests/src/Core/Cache/ArrayCacheDriverTest.php
@@ -0,0 +1,32 @@
+<?php
+
+namespace Friendica\Test\src\Core\Cache;
+
+
+use Friendica\Core\Cache\ArrayCache;
+
+class ArrayCacheDriverTest extends CacheTest
+{
+ /**
+ * @var \Friendica\Core\Cache\IMemoryCacheDriver
+ */
+ private $cache;
+
+ protected function getInstance()
+ {
+ $this->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 @@
+<?php
+
+namespace Friendica\Test\src\Core\Cache;
+
+use Friendica\App;
+use Friendica\Core\Config;
+use Friendica\Test\DatabaseTest;
+
+abstract class CacheTest extends DatabaseTest
+{
+ /**
+ * @var \Friendica\Core\Cache\ICacheDriver
+ */
+ protected $instance;
+
+ abstract protected function getInstance();
+
+ protected function setUp()
+ {
+ global $a;
+ parent::setUp();
+ $this->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 @@
+<?php
+
+namespace Friendica\Test\src\Core\Cache;
+
+use Friendica\Core\Cache\CacheDriverFactory;
+
+class DatabaseCacheDriverTest extends CacheTest
+{
+ /**
+ * @var \Friendica\Core\Cache\IMemoryCacheDriver
+ */
+ private $cache;
+
+ protected function getInstance()
+ {
+ $this->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 @@
+<?php
+
+
+namespace Friendica\Test\src\Core\Cache;
+
+
+use Friendica\Core\Cache\CacheDriverFactory;
+
+class MemcachedCacheDriverTest extends CacheTest
+{
+ /**
+ * @var \Friendica\Core\Cache\IMemoryCacheDriver
+ */
+ private $cache;
+
+ protected function getInstance()
+ {
+ if (class_exists('Memcached')) {
+ try {
+ $this->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 @@
+<?php
+
+
+namespace Friendica\Test\src\Core\Cache;
+
+
+use Friendica\Core\Cache\CacheDriverFactory;
+
+class RedisCacheDriverTest extends CacheTest
+{
+ /**
+ * @var \Friendica\Core\Cache\IMemoryCacheDriver
+ */
+ private $cache;
+
+ protected function getInstance()
+ {
+ if (class_exists('Redis')) {
+ try {
+ $this->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;
+ }
}
--
2.39.5