From 3b08b2658954b95932af8eec7cfaac17b1da7873 Mon Sep 17 00:00:00 2001
From: Vincent Petry <pvince81@owncloud.com>
Date: Tue, 9 Jun 2015 15:22:33 +0200
Subject: [PATCH] Throw exception if memcache misconfigured or missing

Instead of falling back to null memcache, throw exceptions.
Also throw file locking specific exceptions in case the class is not
available.
---
 lib/private/memcache/factory.php | 17 +++++++-
 lib/private/server.php           |  4 ++
 tests/lib/memcache/factory.php   | 70 ++++++++++++++++++++------------
 3 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/lib/private/memcache/factory.php b/lib/private/memcache/factory.php
index 5e3d857006..320657a71a 100644
--- a/lib/private/memcache/factory.php
+++ b/lib/private/memcache/factory.php
@@ -62,12 +62,25 @@ class Factory implements ICacheFactory {
 	{
 		$this->globalPrefix = $globalPrefix;
 
-		if (!($localCacheClass && $localCacheClass::isAvailable())) {
+		if (!$localCacheClass) {
 			$localCacheClass = self::NULL_CACHE;
 		}
-		if (!($distributedCacheClass && $distributedCacheClass::isAvailable())) {
+		if (!$distributedCacheClass) {
 			$distributedCacheClass = $localCacheClass;
 		}
+
+		if (!$localCacheClass::isAvailable()) {
+			throw new \OC\HintException(
+				'Missing memcache class ' . $localCacheClass . ' for local cache',
+				'Is the matching PHP module installed and enabled ?'
+			);
+		}
+		if (!$distributedCacheClass::isAvailable()) {
+			throw new \OC\HintException(
+				'Missing memcache class ' . $distributedCacheClass . ' for distributed cache',
+				'Is the matching PHP module installed and enabled ?'
+			);
+		}
 		if (!($lockingCacheClass && $lockingCacheClass::isAvailable())) {
 			// dont fallback since the fallback might not be suitable for storing lock
 			$lockingCacheClass = '\OC\Memcache\NullCache';
diff --git a/lib/private/server.php b/lib/private/server.php
index 5ed9d78f41..ef033eb389 100644
--- a/lib/private/server.php
+++ b/lib/private/server.php
@@ -431,6 +431,10 @@ class Server extends SimpleContainer implements IServerContainer {
 				if (!($memcache instanceof \OC\Memcache\NullCache)) {
 					return new MemcacheLockingProvider($memcache);
 				}
+				throw new HintException(
+					'File locking is enabled but the locking cache class was not found',
+					'Please check the "memcache.locking" setting and make sure the matching PHP module is installed and enabled'
+				);
 			}
 			return new NoopLockingProvider();
 		});
diff --git a/tests/lib/memcache/factory.php b/tests/lib/memcache/factory.php
index 4ce032abbe..c25e5937c1 100644
--- a/tests/lib/memcache/factory.php
+++ b/tests/lib/memcache/factory.php
@@ -66,45 +66,65 @@ class Test_Factory extends \Test\TestCase {
 		return [
 			[
 				// local and distributed available
-				self::AVAILABLE1, self::AVAILABLE2,
-				self::AVAILABLE1, self::AVAILABLE2
-			],
-			[
-				// local available, distributed unavailable
-				self::AVAILABLE1, self::UNAVAILABLE1,
-				self::AVAILABLE1, self::AVAILABLE1
-			],
-			[
-				// local unavailable, distributed available
-				self::UNAVAILABLE1, self::AVAILABLE1,
-				\OC\Memcache\Factory::NULL_CACHE, self::AVAILABLE1
-			],
-			[
-				// local and distributed unavailable
-				self::UNAVAILABLE1, self::UNAVAILABLE2,
-				\OC\Memcache\Factory::NULL_CACHE, \OC\Memcache\Factory::NULL_CACHE
+				self::AVAILABLE1, self::AVAILABLE2, null,
+				self::AVAILABLE1, self::AVAILABLE2, \OC\Memcache\Factory::NULL_CACHE
 			],
 			[
 				// local and distributed null
-				null, null,
-				\OC\Memcache\Factory::NULL_CACHE, \OC\Memcache\Factory::NULL_CACHE
+				null, null, null,
+				\OC\Memcache\Factory::NULL_CACHE, \OC\Memcache\Factory::NULL_CACHE, \OC\Memcache\Factory::NULL_CACHE
 			],
 			[
 				// local available, distributed null (most common scenario)
-				self::AVAILABLE1, null,
-				self::AVAILABLE1, self::AVAILABLE1
+				self::AVAILABLE1, null, null,
+				self::AVAILABLE1, self::AVAILABLE1, \OC\Memcache\Factory::NULL_CACHE
+			],
+			[
+				// locking cache available
+				null, null, self::AVAILABLE1,
+				\OC\Memcache\Factory::NULL_CACHE, \OC\Memcache\Factory::NULL_CACHE, self::AVAILABLE1
+			],
+			[
+				// locking cache unavailable: no exception here in the factory
+				null, null, self::UNAVAILABLE1,
+				\OC\Memcache\Factory::NULL_CACHE, \OC\Memcache\Factory::NULL_CACHE, \OC\Memcache\Factory::NULL_CACHE
 			]
 		];
 	}
 
+	public function cacheUnavailableProvider() {
+		return [
+			[
+				// local available, distributed unavailable
+				self::AVAILABLE1, self::UNAVAILABLE1
+			],
+			[
+				// local unavailable, distributed available
+				self::UNAVAILABLE1, self::AVAILABLE1
+			],
+			[
+				// local and distributed unavailable
+				self::UNAVAILABLE1, self::UNAVAILABLE2
+			],
+		];
+	}
+
 	/**
 	 * @dataProvider cacheAvailabilityProvider
 	 */
-	public function testCacheAvailability($localCache, $distributedCache,
-		$expectedLocalCache, $expectedDistributedCache)
-	{
-		$factory = new \OC\Memcache\Factory('abc', $localCache, $distributedCache);
+	public function testCacheAvailability($localCache, $distributedCache, $lockingCache,
+		$expectedLocalCache, $expectedDistributedCache, $expectedLockingCache) {
+		$factory = new \OC\Memcache\Factory('abc', $localCache, $distributedCache, $lockingCache);
 		$this->assertTrue(is_a($factory->createLocal(), $expectedLocalCache));
 		$this->assertTrue(is_a($factory->createDistributed(), $expectedDistributedCache));
+		$this->assertTrue(is_a($factory->createLocking(), $expectedLockingCache));
+	}
+
+	/**
+	 * @dataProvider cacheUnavailableProvider
+	 * @expectedException \OC\HintException
+	 */
+	public function testCacheNotAvailableException($localCache, $distributedCache) {
+		new \OC\Memcache\Factory('abc', $localCache, $distributedCache);
 	}
 }