Add check for activated local memcache

Also used the opportunity to refactor it into an AppFramework controller so that we can unit test it.

Fixes https://github.com/owncloud/core/issues/14956

core/js/setupchecks.js

@@ -59,6 +59,11 @@
 							t('core', 'Your data directory and your files are probably accessible from the Internet. The .htaccess file is not working. We strongly suggest that you configure your web server in a way that the data directory is no longer accessible or you move the data directory outside the web server document root.')
 						);
 					}
+					if(!data.isMemcacheConfigured) {
+						messages.push(
+							t('core', 'No memory cache has been configured. To enhance your performance please configure a memcache if available. Further information can be found in our <a href="https://doc.owncloud.org/server/8.0/admin_manual/configuration_server/performance_tuning.html">documentation</a>.')
+						);
+					}
 				} else {
 					messages.push(t('core', 'Error occurred while checking server setup'));
 				}

core/js/tests/specs/setupchecksSpec.js

@@ -66,11 +66,11 @@ describe('OC.SetupChecks tests', function() {
 				{
 					'Content-Type': 'application/json'
 				},
-				JSON.stringify({data: {serverHasInternetConnection: false}})
+				JSON.stringify({serverHasInternetConnection: false})
 			);
 
 			async.done(function( data, s, x ){
-				expect(data).toEqual(['This server has no working Internet connection. This means that some of the features like mounting external storage, notifications about updates or installation of third-party apps will not work. Accessing files remotely and sending of notification emails might not work, either. We suggest to enable Internet connection for this server if you want to have all features.', 'Your data directory and your files are probably accessible from the Internet. The .htaccess file is not working. We strongly suggest that you configure your web server in a way that the data directory is no longer accessible or you move the data directory outside the web server document root.']);
+				expect(data).toEqual(['This server has no working Internet connection. This means that some of the features like mounting external storage, notifications about updates or installation of third-party apps will not work. Accessing files remotely and sending of notification emails might not work, either. We suggest to enable Internet connection for this server if you want to have all features.', 'Your data directory and your files are probably accessible from the Internet. The .htaccess file is not working. We strongly suggest that you configure your web server in a way that the data directory is no longer accessible or you move the data directory outside the web server document root.', 'No memory cache has been configured. To enhance your performance please configure a memcache if available. Further information can be found in our <a href="https://doc.owncloud.org/server/8.0/admin_manual/configuration_server/performance_tuning.html">documentation</a>.']);
 				done();
 			});
 		});
@@ -83,7 +83,24 @@ describe('OC.SetupChecks tests', function() {
 				{
 					'Content-Type': 'application/json'
 				},
-				JSON.stringify({data: {serverHasInternetConnection: false, dataDirectoryProtected: false}})
+				JSON.stringify({serverHasInternetConnection: false, dataDirectoryProtected: false})
+			);
+
+			async.done(function( data, s, x ){
+				expect(data).toEqual(['This server has no working Internet connection. This means that some of the features like mounting external storage, notifications about updates or installation of third-party apps will not work. Accessing files remotely and sending of notification emails might not work, either. We suggest to enable Internet connection for this server if you want to have all features.', 'Your data directory and your files are probably accessible from the Internet. The .htaccess file is not working. We strongly suggest that you configure your web server in a way that the data directory is no longer accessible or you move the data directory outside the web server document root.', 'No memory cache has been configured. To enhance your performance please configure a memcache if available. Further information can be found in our <a href="https://doc.owncloud.org/server/8.0/admin_manual/configuration_server/performance_tuning.html">documentation</a>.']);
+				done();
+			});
+		});
+
+		it('should return an error if server has no internet connection and data directory is not protected and memcache is available', function(done) {
+			var async = OC.SetupChecks.checkSetup();
+
+			suite.server.requests[0].respond(
+				200,
+				{
+					'Content-Type': 'application/json',
+				},
+				JSON.stringify({serverHasInternetConnection: false, dataDirectoryProtected: false, isMemcacheConfigured: true})
 			);
 
 			async.done(function( data, s, x ){
@@ -92,6 +109,7 @@ describe('OC.SetupChecks tests', function() {
 			});
 		});
 
+
 		it('should return an error if the response has no statuscode 200', function(done) {
 			var async = OC.SetupChecks.checkSetup();
 

lib/private/setup.php

@@ -181,7 +181,8 @@ class Setup {
 			\OC\Setup::protectDataDirectory();
 
 			try {
-				$htAccessWorking = \OC_Util::isHtaccessWorking();
+				$util = new \OC_Util();
+				$htAccessWorking = $util->isHtaccessWorking(\OC::$server->getConfig());
 			} catch (\OC\HintException $e) {
 				$errors[] = array(
 					'error' => $e->getMessage(),

lib/private/util.php

@@ -1155,14 +1155,13 @@ class OC_Util {
 
 	/**
 	 * Check if the .htaccess file is working
-	 *
+	 * @param \OCP\IConfig $config
-	 * @throws OC\HintException If the testfile can't get written.
 	 * @return bool
-	 * @description Check if the .htaccess file is working by creating a test
+	 * @throws Exception
-	 * file in the data directory and trying to access via http
+	 * @throws \OC\HintException If the test file can't get written.
 	 */
-	public static function isHtaccessWorking() {
+	public function isHtaccessWorking(\OCP\IConfig $config) {
-		if (\OC::$CLI || !OC::$server->getConfig()->getSystemValue('check_for_working_htaccess', true)) {
+		if (\OC::$CLI || !$config->getSystemValue('check_for_working_htaccess', true)) {
 			return true;
 		}
 
@@ -1176,7 +1175,7 @@ class OC_Util {
 		$testContent = 'testcontent';
 
 		// creating a test file
-		$testFile = OC::$server->getConfig()->getSystemValue('datadirectory', OC::$SERVERROOT . '/data') . '/' . $fileName;
+		$testFile = $config->getSystemValue('datadirectory', OC::$SERVERROOT . '/data') . '/' . $fileName;
 
 		if (file_exists($testFile)) {// already running this test, possible recursive call
 			return false;
@@ -1244,36 +1243,6 @@ class OC_Util {
 		return function_exists('finfo_open');
 	}
 
-	/**
-	 * Check if the ownCloud server can connect to the internet
-	 *
-	 * @param \OCP\Http\Client\IClientService $clientService
-	 * @return bool
-	 */
-	public static function isInternetConnectionWorking(\OCP\Http\Client\IClientService $clientService) {
-		// in case there is no internet connection on purpose return false
-		if (self::isInternetConnectionEnabled() === false) {
-			return false;
-		}
-
-		try {
-			$client = $clientService->newClient();
-			$response = $client->get('https://www.owncloud.org/');
-			return $response->getStatusCode() === 200;
-		} catch (\Exception $e) {
-			return false;
-		}
-	}
-
-	/**
-	 * Check if the connection to the internet is disabled on purpose
-	 *
-	 * @return string
-	 */
-	public static function isInternetConnectionEnabled() {
-		return \OC_Config::getValue("has_internet_connection", true);
-	}
-
 	/**
 	 * clear all levels of output buffering
 	 *

settings/ajax/checksetup.php

@@ -1,40 +0,0 @@
-<?php
-/**
- * @author Lukas Reschke <lukas@owncloud.com>
- * @author Morris Jobke <hey@morrisjobke.de>
- * @author Vincent Petry <pvince81@owncloud.com>
- *
- * @copyright Copyright (c) 2015, ownCloud, Inc.
- * @license AGPL-3.0
- *
- * This code is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License, version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License, version 3,
- * along with this program.  If not, see <http://www.gnu.org/licenses/>
- *
- */
-
-OCP\JSON::checkAdminUser();
-OCP\JSON::callCheck();
-
-\OC::$server->getSession()->close();
-
-// no warning when has_internet_connection is false in the config
-$hasInternet = true;
-if (OC_Util::isInternetConnectionEnabled()) {
-	$hasInternet = OC_Util::isInternetConnectionWorking(\OC::$server->getHTTPClientService());
-}
-
-OCP\JSON::success(
-	array (
-		'serverHasInternetConnection' => $hasInternet,
-		'dataDirectoryProtected' => OC_Util::isHtaccessWorking()
-	)
-);

settings/application.php

@@ -24,6 +24,7 @@
 namespace OC\Settings;
 
 use OC\Settings\Controller\AppSettingsController;
+use OC\Settings\Controller\CheckSetupController;
 use OC\Settings\Controller\GroupsController;
 use OC\Settings\Controller\LogSettingsController;
 use OC\Settings\Controller\MailSettingsController;
@@ -44,7 +45,7 @@ class Application extends App {
 	/**
 	 * @param array $urlParams
 	 */
-	public function __construct(array $urlParams=array()){
+	public function __construct(array $urlParams=[]){
 		parent::__construct('settings', $urlParams);
 
 		$container = $this->getContainer();
@@ -117,6 +118,15 @@ class Application extends App {
 				$c->query('L10N')
 			);
 		});
+		$container->registerService('CheckSetupController', function(IContainer $c) {
+			return new CheckSetupController(
+				$c->query('AppName'),
+				$c->query('Request'),
+				$c->query('Config'),
+				$c->query('ClientService'),
+				$c->query('Util')
+			);
+		});
 
 		/**
 		 * Middleware
@@ -178,5 +188,11 @@ class Application extends App {
 		$container->registerService('URLGenerator', function(IContainer $c) {
 			return $c->query('ServerContainer')->getURLGenerator();
 		});
+		$container->registerService('ClientService', function(IContainer $c) {
+			return $c->query('ServerContainer')->getHTTPClientService();
+		});
+		$container->registerService('Util', function(IContainer $c) {
+			return new \OC_Util();
+		});
 	}
 }

settings/controller/checksetupcontroller.php

@@ -0,0 +1,99 @@
+<?php
+/**
+ * @author Lukas Reschke <lukas@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OC\Settings\Controller;
+
+use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http\DataResponse;
+use OCP\Http\Client\IClientService;
+use OCP\IConfig;
+use OCP\IRequest;
+use OC_Util;
+
+/**
+ * @package OC\Settings\Controller
+ */
+class CheckSetupController extends Controller {
+	/** @var IConfig */
+	private $config;
+	/** @var IClientService */
+	private $clientService;
+	/** @var \OC_Util */
+	private $util;
+
+	/**
+	 * @param string $AppName
+	 * @param IRequest $request
+	 * @param IConfig $config
+	 * @param IClientService $clientService
+	 * @param \OC_Util $util
+	 */
+	public function __construct($AppName,
+								IRequest $request,
+								IConfig $config,
+								IClientService $clientService,
+								\OC_Util $util) {
+		parent::__construct($AppName, $request);
+		$this->config = $config;
+		$this->clientService = $clientService;
+		$this->util = $util;
+	}
+
+	/**
+	 * Checks if the ownCloud server can connect to the internet using HTTPS and HTTP
+	 * @return bool
+	 */
+	private function isInternetConnectionWorking() {
+		if ($this->config->getSystemValue('has_internet_connection', true) === false) {
+			return false;
+		}
+
+		try {
+			$client = $this->clientService->newClient();
+			$client->get('https://www.owncloud.org/');
+			$client->get('http://www.owncloud.org/');
+			return true;
+		} catch (\Exception $e) {
+			return false;
+		}
+	}
+
+	/**
+	 * Checks whether a local memcache is installed or not
+	 * @return bool
+	 */
+	private function isMemcacheConfigured() {
+		return $this->config->getSystemValue('memcache.local', null) !== null;
+	}
+
+	/**
+	 * @return DataResponse
+	 */
+	public function check() {
+		return new DataResponse(
+			[
+				'serverHasInternetConnection' => $this->isInternetConnectionWorking(),
+				'dataDirectoryProtected' => $this->util->isHtaccessWorking($this->config),
+				'isMemcacheConfigured' => $this->isMemcacheConfigured(),
+			]
+		);
+	}
+}

settings/routes.php

@@ -49,6 +49,7 @@ $application->registerRoutes($this, array(
 		array('name' => 'LogSettings#setLogLevel', 'url' => '/settings/admin/log/level', 'verb' => 'POST'),
 		array('name' => 'LogSettings#getEntries', 'url' => '/settings/admin/log/entries', 'verb' => 'GET'),
 		array('name' => 'LogSettings#download', 'url' => '/settings/admin/log/download', 'verb' => 'GET'),
+		['name' => 'CheckSetup#check', 'url' => '/settings/ajax/checksetup', 'verb' => 'GET'],
 	)
 ));
 
@@ -112,5 +113,3 @@ $this->create('settings_ajax_navigationdetect', '/settings/ajax/navigationdetect
 // admin
 $this->create('settings_ajax_excludegroups', '/settings/ajax/excludegroups.php')
 	->actionInclude('settings/ajax/excludegroups.php');
-$this->create('settings_ajax_checksetup', '/settings/ajax/checksetup')
-	->actionInclude('settings/ajax/checksetup.php');

tests/lib/util.php

@@ -129,14 +129,6 @@ class Test_Util extends \Test\TestCase {
 		$this->assertEquals($expected, \OC_Util::fileInfoLoaded());
 	}
 
-	public function testIsInternetConnectionEnabled() {
-		\OC_Config::setValue("has_internet_connection", false);
-		$this->assertFalse(\OC_Util::isInternetConnectionEnabled());
-
-		\OC_Config::setValue("has_internet_connection", true);
-		$this->assertTrue(\OC_Util::isInternetConnectionEnabled());
-	}
-
 	function testGenerateRandomBytes() {
 		$result = strlen(OC_Util::generateRandomBytes(59));
 		$this->assertEquals(59, $result);

tests/settings/controller/CheckSetupControllerTest.php

@@ -0,0 +1,231 @@
+<?php
+/**
+ * @author Lukas Reschke <lukas@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OC\Settings\Controller;
+
+use OCP\AppFramework\Http\DataResponse;
+use Test\TestCase;
+use OCP\IRequest;
+use OCP\IConfig;
+use OCP\Http\Client\IClientService;
+use OC_Util;
+
+/**
+ * Class CheckSetupControllerTest
+ *
+ * @package OC\Settings\Controller
+ */
+class CheckSetupControllerTest extends TestCase {
+	/** @var CheckSetupController */
+	private $checkSetupController;
+	/** @var IRequest */
+	private $request;
+	/** @var IConfig */
+	private $config;
+	/** @var IClientService */
+	private $clientService;
+	/** @var OC_Util */
+	private $util;
+
+	public function setUp() {
+		parent::setUp();
+
+		$this->request = $this->getMockBuilder('\OCP\IRequest')
+			->disableOriginalConstructor()->getMock();
+		$this->config = $this->getMockBuilder('\OCP\IConfig')
+			->disableOriginalConstructor()->getMock();
+		$this->config = $this->getMockBuilder('\OCP\IConfig')
+			->disableOriginalConstructor()->getMock();
+		$this->clientService = $this->getMockBuilder('\OCP\Http\Client\IClientService')
+			->disableOriginalConstructor()->getMock();
+		$this->util = $this->getMockBuilder('\OC_Util')
+			->disableOriginalConstructor()->getMock();
+
+		$this->checkSetupController = new CheckSetupController(
+			'settings',
+			$this->request,
+			$this->config,
+			$this->clientService,
+			$this->util
+		);
+	}
+
+	public function testIsInternetConnectionWorkingDisabledViaConfig() {
+		$this->config->expects($this->once())
+			->method('getSystemValue')
+			->with('has_internet_connection', true)
+			->will($this->returnValue(false));
+
+		$this->assertFalse(
+			\Test_Helper::invokePrivate(
+				$this->checkSetupController,
+				'isInternetConnectionWorking'
+			)
+		);
+	}
+
+	public function testIsInternetConnectionWorkingCorrectly() {
+		$this->config->expects($this->once())
+			->method('getSystemValue')
+			->with('has_internet_connection', true)
+			->will($this->returnValue(true));
+
+		$client = $this->getMockBuilder('\OCP\Http\Client\IClient')
+			->disableOriginalConstructor()->getMock();
+		$client->expects($this->at(0))
+			->method('get')
+			->with('https://www.owncloud.org/', []);
+		$client->expects($this->at(1))
+			->method('get')
+			->with('http://www.owncloud.org/', []);
+
+		$this->clientService->expects($this->once())
+			->method('newClient')
+			->will($this->returnValue($client));
+
+
+		$this->assertTrue(
+			\Test_Helper::invokePrivate(
+				$this->checkSetupController,
+				'isInternetConnectionWorking'
+			)
+		);
+	}
+
+	public function testIsInternetConnectionHttpsFail() {
+		$this->config->expects($this->once())
+			->method('getSystemValue')
+			->with('has_internet_connection', true)
+			->will($this->returnValue(true));
+
+		$client = $this->getMockBuilder('\OCP\Http\Client\IClient')
+			->disableOriginalConstructor()->getMock();
+		$client->expects($this->at(0))
+			->method('get')
+			->with('https://www.owncloud.org/', [])
+			->will($this->throwException(new \Exception()));
+
+		$this->clientService->expects($this->once())
+			->method('newClient')
+			->will($this->returnValue($client));
+
+		$this->assertFalse(
+			\Test_Helper::invokePrivate(
+				$this->checkSetupController,
+				'isInternetConnectionWorking'
+			)
+		);
+	}
+
+	public function testIsInternetConnectionHttpFail() {
+		$this->config->expects($this->once())
+			->method('getSystemValue')
+			->with('has_internet_connection', true)
+			->will($this->returnValue(true));
+
+		$client = $this->getMockBuilder('\OCP\Http\Client\IClient')
+			->disableOriginalConstructor()->getMock();
+		$client->expects($this->at(0))
+			->method('get')
+			->with('https://www.owncloud.org/', []);
+		$client->expects($this->at(1))
+			->method('get')
+			->with('http://www.owncloud.org/', [])
+			->will($this->throwException(new \Exception()));
+
+		$this->clientService->expects($this->once())
+			->method('newClient')
+			->will($this->returnValue($client));
+
+		$this->assertFalse(
+			\Test_Helper::invokePrivate(
+				$this->checkSetupController,
+				'isInternetConnectionWorking'
+			)
+		);
+	}
+
+	public function testIsMemcacheConfiguredFalse() {
+		$this->config->expects($this->once())
+			->method('getSystemValue')
+			->with('memcache.local', null)
+			->will($this->returnValue(null));
+
+		$this->assertFalse(
+			\Test_Helper::invokePrivate(
+				$this->checkSetupController,
+				'isMemcacheConfigured'
+			)
+		);
+	}
+
+	public function testIsMemcacheConfiguredTrue() {
+		$this->config->expects($this->once())
+			->method('getSystemValue')
+			->with('memcache.local', null)
+			->will($this->returnValue('SomeProvider'));
+
+		$this->assertTrue(
+			\Test_Helper::invokePrivate(
+				$this->checkSetupController,
+				'isMemcacheConfigured'
+			)
+		);
+	}
+
+	public function testCheck() {
+		$this->config->expects($this->at(0))
+			->method('getSystemValue')
+			->with('has_internet_connection', true)
+			->will($this->returnValue(true));
+		$this->config->expects($this->at(1))
+			->method('getSystemValue')
+			->with('memcache.local', null)
+			->will($this->returnValue('SomeProvider'));
+
+		$client = $this->getMockBuilder('\OCP\Http\Client\IClient')
+			->disableOriginalConstructor()->getMock();
+		$client->expects($this->at(0))
+			->method('get')
+			->with('https://www.owncloud.org/', []);
+		$client->expects($this->at(1))
+			->method('get')
+			->with('http://www.owncloud.org/', [])
+			->will($this->throwException(new \Exception()));
+
+		$this->clientService->expects($this->once())
+			->method('newClient')
+			->will($this->returnValue($client));
+
+		$this->util->expects($this->once())
+			->method('isHtaccessWorking')
+			->will($this->returnValue(true));
+
+		$expected = new DataResponse(
+			[
+				'serverHasInternetConnection' => false,
+				'dataDirectoryProtected' => true,
+				'isMemcacheConfigured' => true,
+			]
+		);
+		$this->assertEquals($expected, $this->checkSetupController->check());
+	}
+}