Added Unittests for cache

fixed Lock & Cache bugs

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;

src/Core/Cache/DatabaseCacheDriver.php

@@ -49,8 +49,12 @@ class DatabaseCacheDriver extends AbstractCacheDriver implements ICacheDriver
 		return dba::delete('cache', ['k' => $key]);
 	}
 
-	public function clear()
+	public function clear($outdated = true)
 	{
+		if ($outdated) {
 			return dba::delete('cache', ['`expires` < NOW()']);
+		} else {
+			return dba::delete('cache', ['`k` IS NOT NULL ']);
+		}
 	}
 }

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);
 }

src/Core/Cache/MemcacheCacheDriver.php

@@ -96,10 +96,14 @@ class MemcacheCacheDriver extends AbstractCacheDriver implements IMemoryCacheDri
 	/**
 	 * (@inheritdoc)
 	 */
-	public function clear()
+	public function clear($outdated = true)
 	{
+		if ($outdated) {
+			return true;
+		} else {
 			return $this->memcache->flush();
 		}
+	}
 
 	/**
 	 * (@inheritdoc)

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,10 +75,14 @@ class MemcachedCacheDriver extends AbstractCacheDriver implements IMemoryCacheDr
 		return $this->memcached->delete($cachekey);
 	}
 
-	public function clear()
+	public function clear($outdated = true)
 	{
+		if ($outdated) {
+			return true;
+		} else {
 			return $this->memcached->flush();
 		}
+	}
 
 	/**
 	 * @brief Sets a value if it's not already stored

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,11 +78,14 @@ class RedisCacheDriver extends AbstractCacheDriver implements IMemoryCacheDriver
 		return ($this->redis->delete($cachekey) > 0);
 	}
 
-	public function clear()
+	public function clear($outdated = true)
 	{
+		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);
 	}
 
 	/**

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);
 				}

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);
 			}

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

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]) {

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;
+	}
+}

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'));
+	}
+}

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();
+	}
+}

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();
+	}
+}

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();
+	}
+}

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;
+	}
 }

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'));
+	}
 }

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;
+	}
 }