Added Lock Unittests & Bugfixings

Added Redis Lock Unittests
Added Memcached Lock Unittests

Fixed a bug in dba
Fixed a bug in RedisLock

.travis.yml

@@ -9,6 +9,8 @@ php:
 
 services:
  - mysql
+ - redis-server
+ - memcached
 env:
  - MYSQL_HOST=localhost MYSQL_PORT=3306 MYSQL_USERNAME=travis MYSQL_PASSWORD= MYSQL_DATABASE=test
 
@@ -17,3 +19,5 @@ install:
 before_script:
  - mysql -e 'CREATE DATABASE IF NOT EXISTS test;'
  - mysql -utravis test < database.sql
+ - echo "extension=redis.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini
+ - echo "extension=memcached.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini

include/dba.php

@@ -1109,7 +1109,7 @@ class dba {
 
 					// Split the SQL queries in chunks of 100 values
 					// We do the $i stuff here to make the code better readable
-					$i = $counter[$key_table][$key_condition];
+					$i = isset($counter[$key_table][$key_condition]) ? $counter[$key_table][$key_condition] : 0;
 					if (isset($compacted[$key_table][$key_condition][$i]) && count($compacted[$key_table][$key_condition][$i]) > 100) {
 						++$i;
 					}

src/Core/Cache/AbstractCacheDriver.php

@@ -18,6 +18,7 @@ abstract class AbstractCacheDriver extends BaseObject
 	 * @return string		The cache key used for the cache
 	 */
 	protected function getCacheKey($key) {
+		// We fetch with the hostname as key to avoid problems with other applications
 		return self::getApp()->get_hostname() . ":" . $key;
 	}
 }

src/Core/Cache/RedisCacheDriver.php

@@ -35,15 +35,12 @@ class RedisCacheDriver extends AbstractCacheDriver implements IMemoryCacheDriver
 		$return = null;
 		$cachekey = $this->getCacheKey($key);
 
-		// We fetch with the hostname as key to avoid problems with other applications
 		$cached = $this->redis->get($cachekey);
-
-		// @see http://php.net/manual/en/redis.get.php#84275
-		if (is_bool($cached) || is_double($cached) || is_long($cached)) {
-			return $return;
+		if ($cached === false && !$this->redis->exists($cachekey)) {
+			return null;
 		}
 
-		$value = @unserialize($cached);
+		$value = json_decode($cached);
 
 		// Only return a value if the serialized value is valid.
 		// We also check if the db entry is a serialized
@@ -59,17 +56,18 @@ class RedisCacheDriver extends AbstractCacheDriver implements IMemoryCacheDriver
 	{
 		$cachekey = $this->getCacheKey($key);
 
-		// We store with the hostname as key to avoid problems with other applications
+		$cached = json_encode($value);
+
 		if ($ttl > 0) {
 			return $this->redis->setex(
 				$cachekey,
 				time() + $ttl,
-				serialize($value)
+				$cached
 			);
 		} else {
 			return $this->redis->set(
 				$cachekey,
-				serialize($value)
+				$cached
 			);
 		}
 	}
@@ -81,7 +79,7 @@ class RedisCacheDriver extends AbstractCacheDriver implements IMemoryCacheDriver
 
 	public function clear()
 	{
-		return true;
+		return $this->redis->flushAll();
 	}
 
 
@@ -91,10 +89,7 @@ class RedisCacheDriver extends AbstractCacheDriver implements IMemoryCacheDriver
 	public function add($key, $value, $ttl = Cache::FIVE_MINUTES)
 	{
 		$cachekey = $this->getCacheKey($key);
-
-		if (!is_int($value)) {
-			$value = serialize($value);
-		}
+		$cached = json_encode($value);
 
 		return $this->redis->setnx($cachekey, $value);
 	}
@@ -106,16 +101,14 @@ class RedisCacheDriver extends AbstractCacheDriver implements IMemoryCacheDriver
 	{
 		$cachekey = $this->getCacheKey($key);
 
-		if (!is_int($newValue)) {
-			$newValue = serialize($newValue);
-		}
+		$newCached = json_encode($newValue);
 
 		$this->redis->watch($cachekey);
 		// If the old value isn't what we expected, somebody else changed the key meanwhile
-		if ($this->get($cachekey) === $oldValue) {
+		if ($this->get($key) === $oldValue) {
 			if ($ttl > 0) {
 				$result = $this->redis->multi()
-					->setex($cachekey, $ttl, $newValue)
+					->setex($cachekey, $ttl, $newCached)
 					->exec();
 			} else {
 				$result = $this->redis->multi()

tests/src/Core/Lock/DatabaseLockDriverTest.php

@@ -11,49 +11,6 @@ use PHPUnit_Extensions_Database_DB_IDatabaseConnection;
 
 class DatabaseLockDriverTest extends LockTest
 {
-	use TestCaseTrait;
-
-	/**
-	 * Get database connection.
-	 *
-	 * This function is executed before each test in order to get a database connection that can be used by tests.
-	 * If no prior connection is available, it tries to create one using the USER, PASS and DB environment variables.
-	 *
-	 * If it could not connect to the database, the test is skipped.
-	 *
-	 * @return PHPUnit_Extensions_Database_DB_IDatabaseConnection
-	 * @see https://phpunit.de/manual/5.7/en/database.html
-	 */
-	protected function getConnection()
-	{
-		if (!dba::$connected) {
-			dba::connect('localhost', getenv('USER'), getenv('PASS'), getenv('DB'));
-
-			if (dba::$connected) {
-				$app = get_app();
-				// We need to do this in order to disable logging
-				$app->module = 'install';
-
-				// Create database structure
-				DBStructure::update(false, true, true);
-			} else {
-				$this->markTestSkipped('Could not connect to the database.');
-			}
-		}
-
-		return $this->createDefaultDBConnection(dba::get_db(), getenv('DB'));
-	}
-
-	/**
-	 * Get dataset to populate the database with.
-	 * @return YamlDataSet
-	 * @see https://phpunit.de/manual/5.7/en/database.html
-	 */
-	protected function getDataSet()
-	{
-		return new YamlDataSet(__DIR__ . '/../../../datasets/api.yml');
-	}
-
 	protected function getInstance()
 	{
 		return new DatabaseLockDriver();
@@ -64,4 +21,4 @@ class DatabaseLockDriverTest extends LockTest
 		dba::delete('locks', [ 'id > 0']);
 		parent::tearDown();
 	}
-}
+}

tests/src/Core/Lock/LockTest.php

@@ -4,9 +4,10 @@ namespace Friendica\Test\src\Core\Lock;
 
 use Friendica\App;
 use Friendica\Core\Config;
+use Friendica\Test\DatabaseTest;
 use PHPUnit\Framework\TestCase;
 
-abstract class LockTest extends TestCase
+abstract class LockTest extends DatabaseTest
 {
 	/**
 	 * @var \Friendica\Core\Lock\ILockDriver
@@ -58,6 +59,10 @@ abstract class LockTest extends TestCase
 		$this->instance->acquireLock('bar', 1);
 		$this->instance->acquireLock('nice', 1);
 
+		$this->assertTrue($this->instance->isLocked('foo'));
+		$this->assertTrue($this->instance->isLocked('bar'));
+		$this->assertTrue($this->instance->isLocked('nice'));
+
 		$this->instance->releaseAll();
 
 		$this->assertFalse($this->instance->isLocked('foo'));
@@ -72,9 +77,13 @@ abstract class LockTest extends TestCase
 
 		$this->instance->releaseLock('foo');
 
+		$this->assertFalse($this->instance->isLocked('foo'));
+		$this->assertTrue($this->instance->isLocked('bar'));
+		$this->assertTrue($this->instance->isLocked('nice'));
+
 		$this->instance->releaseAll();
 
 		$this->assertFalse($this->instance->isLocked('bar'));
-		$this->assertFalse($this->instance->isLocked('#/$%§'));
+		$this->assertFalse($this->instance->isLocked('nice'));
 	}
-}
+}

tests/src/Core/Lock/MemcachedCacheLockDriverTest.php

@@ -0,0 +1,40 @@
+<?php
+
+
+namespace Friendica\Test\src\Core\Lock;
+
+
+use Friendica\Core\Cache\CacheDriverFactory;
+use Friendica\Core\Lock\CacheLockDriver;
+
+class MemcachedCacheLockDriverTest extends LockTest
+{
+	/**
+	 * @var \Friendica\Core\Cache\IMemoryCacheDriver
+	 */
+	private $cache;
+
+	protected function getInstance()
+	{
+		if (class_exists('Memcached')) {
+			try {
+				$this->cache = CacheDriverFactory::create('memcached');
+			} catch (\Exception $exception) {
+				print "Redis - TestCase failed: " . $exception->getMessage();
+				throw new \Exception();
+			}
+			return new CacheLockDriver($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/RedisCacheLockDriverTest.php

@@ -0,0 +1,40 @@
+<?php
+
+
+namespace Friendica\Test\src\Core\Lock;
+
+
+use Friendica\Core\Cache\CacheDriverFactory;
+use Friendica\Core\Lock\CacheLockDriver;
+
+class RedisCacheLockDriverTest extends LockTest
+{
+	/**
+	 * @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 new CacheLockDriver($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/SemaphoreLockDriverTest.php

@@ -23,4 +23,4 @@ class SemaphoreLockDriverTest extends LockTest
 		$this->semaphoreLockDriver->releaseAll();
 		parent::tearDown();
 	}
-}
+}