Merge pull request #10539 from nextcloud/feature-8642-memory-check

Adds a setup and cli check for the recommended memory limit

console.php

@@ -85,7 +85,13 @@ try {
 		echo "The process control (PCNTL) extensions are required in case you want to interrupt long running commands - see http://php.net/manual/en/book.pcntl.php" . PHP_EOL;
 	}
 
-	$application = new Application(\OC::$server->getConfig(), \OC::$server->getEventDispatcher(), \OC::$server->getRequest(), \OC::$server->getLogger());
+	$application = new Application(
+		\OC::$server->getConfig(),
+		\OC::$server->getEventDispatcher(),
+		\OC::$server->getRequest(),
+		\OC::$server->getLogger(),
+		\OC::$server->query(\OC\MemoryInfo::class)
+	);
 	$application->loadCommands(new ArgvInput(), new ConsoleOutput());
 	$application->run();
 } catch (Exception $ex) {

core/js/setupchecks.js

@@ -316,8 +316,17 @@
 							type: OC.SetupChecks.MESSAGE_TYPE_WARNING
 						});
 					}
+					if (!data.isMemoryLimitSufficient) {
+						messages.push({
+							msg: t(
+								'core',
+								'The PHP memory limit is below the recommended value of 512MB.'
+							),
+							type: OC.SetupChecks.MESSAGE_TYPE_WARNING
+						})
+					}
 
-					if(data.appDirsWithDifferentOwner.length > 0) {
+					if(data.appDirsWithDifferentOwner && data.appDirsWithDifferentOwner.length > 0) {
 						var appDirsWithDifferentOwner = data.appDirsWithDifferentOwner.reduce(
 							function(appDirsWithDifferentOwner, directory) {
 								return appDirsWithDifferentOwner + '<li>' + directory + '</li>';

core/js/tests/specs/setupchecksSpec.js

@@ -171,6 +171,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: []
 				})
 			);
@@ -219,6 +220,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: []
 				})
 			);
@@ -268,6 +270,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: []
 				})
 			);
@@ -315,6 +318,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: []
 				})
 			);
@@ -360,6 +364,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: []
 				})
 			);
@@ -405,6 +410,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [
 						'/some/path'
 					]
@@ -452,6 +458,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: []
 				})
 			);
@@ -497,6 +504,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: []
 				})
 			);
@@ -510,6 +518,52 @@ describe('OC.SetupChecks tests', function() {
 			});
 		});
 
+		it('should return a warning if the memory limit is below the recommended value', function(done) {
+			var async = OC.SetupChecks.checkSetup();
+
+			suite.server.requests[0].respond(
+				200,
+				{
+					'Content-Type': 'application/json',
+				},
+				JSON.stringify({
+					hasFileinfoInstalled: true,
+					isGetenvServerWorking: true,
+					isReadOnlyConfig: false,
+					hasWorkingFileLocking: true,
+					hasValidTransactionIsolationLevel: true,
+					suggestedOverwriteCliURL: '',
+					isUrandomAvailable: true,
+					serverHasInternetConnection: true,
+					isMemcacheConfigured: true,
+					forwardedForHeadersWorking: true,
+					reverseProxyDocs: 'https://docs.owncloud.org/foo/bar.html',
+					isCorrectMemcachedPHPModuleInstalled: true,
+					hasPassedCodeIntegrityCheck: true,
+					isOpcacheProperlySetup: true,
+					hasOpcacheLoaded: true,
+					isSettimelimitAvailable: true,
+					hasFreeTypeSupport: true,
+					missingIndexes: [],
+					outdatedCaches: [],
+					cronErrors: [],
+					cronInfo: {
+						diffInSeconds: 0
+					},
+					appDirsWithDifferentOwner: [],
+					isMemoryLimitSufficient: false
+				})
+			);
+
+			async.done(function( data, s, x ){
+				expect(data).toEqual([{
+					msg: 'The PHP memory limit is below the recommended value of 512MB.',
+					type: OC.SetupChecks.MESSAGE_TYPE_WARNING
+				}]);
+				done();
+			});
+		});
+
 		it('should return an error if the response has no statuscode 200', function(done) {
 			var async = OC.SetupChecks.checkSetup();
 
@@ -563,6 +617,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: []
 				})
 			);
@@ -609,6 +664,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: []
 				})
 			);
@@ -655,6 +711,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: []
 				})
 			);
@@ -701,6 +758,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: []
 				})
 			);

lib/composer/composer/autoload_classmap.php

@@ -830,6 +830,7 @@ return array(
     'OC\\Memcache\\Memcached' => $baseDir . '/lib/private/Memcache/Memcached.php',
     'OC\\Memcache\\NullCache' => $baseDir . '/lib/private/Memcache/NullCache.php',
     'OC\\Memcache\\Redis' => $baseDir . '/lib/private/Memcache/Redis.php',
+    'OC\\MemoryInfo' => $baseDir . '/lib/private/MemoryInfo.php',
     'OC\\Migration\\BackgroundRepair' => $baseDir . '/lib/private/Migration/BackgroundRepair.php',
     'OC\\Migration\\ConsoleOutput' => $baseDir . '/lib/private/Migration/ConsoleOutput.php',
     'OC\\Migration\\SimpleOutput' => $baseDir . '/lib/private/Migration/SimpleOutput.php',

lib/composer/composer/autoload_static.php

@@ -860,6 +860,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Memcache\\Memcached' => __DIR__ . '/../../..' . '/lib/private/Memcache/Memcached.php',
         'OC\\Memcache\\NullCache' => __DIR__ . '/../../..' . '/lib/private/Memcache/NullCache.php',
         'OC\\Memcache\\Redis' => __DIR__ . '/../../..' . '/lib/private/Memcache/Redis.php',
+        'OC\\MemoryInfo' => __DIR__ . '/../../..' . '/lib/private/MemoryInfo.php',
         'OC\\Migration\\BackgroundRepair' => __DIR__ . '/../../..' . '/lib/private/Migration/BackgroundRepair.php',
         'OC\\Migration\\ConsoleOutput' => __DIR__ . '/../../..' . '/lib/private/Migration/ConsoleOutput.php',
         'OC\\Migration\\SimpleOutput' => __DIR__ . '/../../..' . '/lib/private/Migration/SimpleOutput.php',

lib/private/Console/Application.php

@@ -29,6 +29,7 @@
  */
 namespace OC\Console;
 
+use OC\MemoryInfo;
 use OC\NeedsUpdateException;
 use OC_App;
 use OCP\AppFramework\QueryException;
@@ -52,20 +53,28 @@ class Application {
 	private $request;
 	/** @var ILogger  */
 	private $logger;
+	/** @var MemoryInfo */
+	private $memoryInfo;
 
 	/**
 	 * @param IConfig $config
 	 * @param EventDispatcherInterface $dispatcher
 	 * @param IRequest $request
 	 * @param ILogger $logger
+	 * @param MemoryInfo $memoryInfo
 	 */
-	public function __construct(IConfig $config, EventDispatcherInterface $dispatcher, IRequest $request, ILogger $logger) {
+	public function __construct(IConfig $config,
+								EventDispatcherInterface $dispatcher,
+								IRequest $request,
+								ILogger $logger,
+								MemoryInfo $memoryInfo) {
 		$defaults = \OC::$server->getThemingDefaults();
 		$this->config = $config;
 		$this->application = new SymfonyApplication($defaults->getName(), \OC_Util::getVersionString());
 		$this->dispatcher = $dispatcher;
 		$this->request = $request;
 		$this->logger = $logger;
+		$this->memoryInfo = $memoryInfo;
 	}
 
 	/**
@@ -97,6 +106,14 @@ class Application {
 		if ($input->getOption('no-warnings')) {
 			$output->setVerbosity(OutputInterface::VERBOSITY_QUIET);
 		}
+
+		if ($this->memoryInfo->isMemoryLimitSufficient() === false) {
+			$output->getErrorOutput()->writeln(
+				'<comment>The current PHP memory limit ' .
+				'is below the recommended value of 512MB.</comment>'
+			);
+		}
+
 		try {
 			require_once __DIR__ . '/../../../core/register_command.php';
 			if ($this->config->getSystemValue('installed', false)) {

lib/private/MemoryInfo.php

@@ -0,0 +1,81 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2018, Michael Weimann (<mail@michael-weimann.eu>)
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * 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
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC;
+
+/**
+ * Helper class that covers memory info.
+ */
+class MemoryInfo {
+
+	const RECOMMENDED_MEMORY_LIMIT = 512 * 1024 * 1024;
+
+	/**
+	 * Tests if the memory limit is greater or equal the recommended value.
+	 *
+	 * @return bool
+	 */
+	public function isMemoryLimitSufficient(): bool {
+		$memoryLimit = $this->getMemoryLimit();
+		return $memoryLimit === -1 || $memoryLimit >= self::RECOMMENDED_MEMORY_LIMIT;
+	}
+
+	/**
+	 * Returns the php memory limit.
+	 *
+	 * @return int The memory limit in bytes.
+	 */
+	public function getMemoryLimit(): int {
+		$iniValue = trim(ini_get('memory_limit'));
+		if ($iniValue === '-1') {
+			return -1;
+		} else if (is_numeric($iniValue) === true) {
+			return (int)$iniValue;
+		} else {
+			return $this->memoryLimitToBytes($iniValue);
+		}
+	}
+
+	/**
+	 * Converts the ini memory limit to bytes.
+	 *
+	 * @param string $memoryLimit The "memory_limit" ini value
+	 * @return int
+	 */
+	private function memoryLimitToBytes(string $memoryLimit): int {
+		$last = strtolower(substr($memoryLimit, -1));
+		$memoryLimit = (int)substr($memoryLimit, 0, -1);
+
+		// intended fall trough
+		switch($last) {
+			case 'g':
+				$memoryLimit *= 1024;
+			case 'm':
+				$memoryLimit *= 1024;
+			case 'k':
+				$memoryLimit *= 1024;
+		}
+
+		return $memoryLimit;
+	}
+}

settings/Controller/CheckSetupController.php

@@ -41,6 +41,7 @@ use OC\DB\Connection;
 use OC\DB\MissingIndexInformation;
 use OC\IntegrityCheck\Checker;
 use OC\Lock\NoopLockingProvider;
+use OC\MemoryInfo;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Http\DataResponse;
@@ -83,6 +84,8 @@ class CheckSetupController extends Controller {
 	private $lockingProvider;
 	/** @var IDateTimeFormatter */
 	private $dateTimeFormatter;
+	/** @var MemoryInfo */
+	private $memoryInfo;
 
 	public function __construct($AppName,
 								IRequest $request,
@@ -96,7 +99,8 @@ class CheckSetupController extends Controller {
 								EventDispatcherInterface $dispatcher,
 								IDBConnection $db,
 								ILockingProvider $lockingProvider,
-								IDateTimeFormatter $dateTimeFormatter) {
+								IDateTimeFormatter $dateTimeFormatter,
+								MemoryInfo $memoryInfo) {
 		parent::__construct($AppName, $request);
 		$this->config = $config;
 		$this->clientService = $clientService;
@@ -109,6 +113,7 @@ class CheckSetupController extends Controller {
 		$this->db = $db;
 		$this->lockingProvider = $lockingProvider;
 		$this->dateTimeFormatter = $dateTimeFormatter;
+		$this->memoryInfo = $memoryInfo;
 	}
 
 	/**
@@ -616,6 +621,7 @@ Raw output
 				'databaseConversionDocumentation' => $this->urlGenerator->linkToDocs('admin-db-conversion'),
 				'isPhpMailerUsed' => $this->isPhpMailerUsed(),
 				'mailSettingsDocumentation' => $this->urlGenerator->getAbsoluteURL('index.php/settings/admin'),
+				'isMemoryLimitSufficient' => $this->memoryInfo->isMemoryLimitSufficient(),
 				'appDirsWithDifferentOwner' => $this->getAppDirsWithDifferentOwner(),
 			]
 		);

tests/Settings/Controller/CheckSetupControllerTest.php

@@ -23,6 +23,7 @@ namespace Tests\Settings\Controller;
 
 use OC;
 use OC\DB\Connection;
+use OC\MemoryInfo;
 use OC\Settings\Controller\CheckSetupController;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataDisplayResponse;
@@ -37,6 +38,7 @@ use OCP\IRequest;
 use OCP\IURLGenerator;
 use OC_Util;
 use OCP\Lock\ILockingProvider;
+use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Http\Message\ResponseInterface;
 use Symfony\Component\EventDispatcher\EventDispatcher;
 use Test\TestCase;
@@ -75,6 +77,8 @@ class CheckSetupControllerTest extends TestCase {
 	private $lockingProvider;
 	/** @var IDateTimeFormatter|\PHPUnit_Framework_MockObject_MockObject */
 	private $dateTimeFormatter;
+	/** @var MemoryInfo|MockObject */
+	private $memoryInfo;
 
 	/**
 	 * Holds a list of directories created during tests.
@@ -112,6 +116,9 @@ class CheckSetupControllerTest extends TestCase {
 			->disableOriginalConstructor()->getMock();
 		$this->lockingProvider = $this->getMockBuilder(ILockingProvider::class)->getMock();
 		$this->dateTimeFormatter = $this->getMockBuilder(IDateTimeFormatter::class)->getMock();
+		$this->memoryInfo = $this->getMockBuilder(MemoryInfo::class)
+			->setMethods(['isMemoryLimitSufficient',])
+			->getMock();
 		$this->checkSetupController = $this->getMockBuilder('\OC\Settings\Controller\CheckSetupController')
 			->setConstructorArgs([
 				'settings',
@@ -127,6 +134,7 @@ class CheckSetupControllerTest extends TestCase {
 				$this->db,
 				$this->lockingProvider,
 				$this->dateTimeFormatter,
+				$this->memoryInfo,
 				])
 			->setMethods([
 				'isReadOnlyConfig',
@@ -447,6 +455,9 @@ class CheckSetupControllerTest extends TestCase {
 			->expects($this->once())
 			->method('hasPassedCheck')
 			->willReturn(true);
+		$this->memoryInfo
+			->method('isMemoryLimitSufficient')
+			->willReturn(true);
 
 		$this->checkSetupController
 			->expects($this->once())
@@ -493,6 +504,7 @@ class CheckSetupControllerTest extends TestCase {
 				'missingIndexes' => [],
 				'isPhpMailerUsed' => false,
 				'mailSettingsDocumentation' => 'https://server/index.php/settings/admin',
+				'isMemoryLimitSufficient' => true,
 				'appDirsWithDifferentOwner' => [],
 			]
 		);
@@ -515,6 +527,7 @@ class CheckSetupControllerTest extends TestCase {
 				$this->db,
 				$this->lockingProvider,
 				$this->dateTimeFormatter,
+				$this->memoryInfo,
 			])
 			->setMethods(null)->getMock();
 

tests/lib/MemoryInfoTest.php

@@ -0,0 +1,118 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2018, Michael Weimann (<mail@michael-weimann.eu>)
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * 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
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Test;
+
+use OC\MemoryInfo;
+use PHPUnit\Framework\MockObject\MockObject;
+
+/**
+ * This class provides tests for the MemoryInfo class.
+ */
+class MemoryInfoTest extends TestCase {
+	/**
+	 * The "memory_limit" value before tests.
+	 *
+	 * @var string
+	 */
+	private $iniSettingBeforeTest;
+
+	/**
+	 * @beforeClass
+	 */
+	public function backupMemoryInfoIniSetting() {
+		$this->iniSettingBeforeTest = ini_get('memory_limit');
+	}
+
+	/**
+	 * @afterClass
+	 */
+	public function restoreMemoryInfoIniSetting() {
+		ini_set('memory_limit', $this->iniSettingBeforeTest);
+	}
+
+	/**
+	 * Provides test data.
+	 *
+	 * @return array
+	 */
+	public function getMemoryLimitTestData(): array {
+		return [
+			'unlimited' => ['-1', -1,],
+			'0' => ['0', 0,],
+			'134217728 bytes' => ['134217728', 134217728,],
+			'128M' => ['128M', 134217728,],
+			'131072K' => ['131072K', 134217728,],
+			'2G' => ['2G', 2147483648,],
+		];
+	}
+
+	/**
+	 * Tests that getMemoryLimit works as expected.
+	 *
+	 * @param string $iniValue The "memory_limit" ini data.
+	 * @param int $expected The expected detected memory limit.
+	 * @dataProvider getMemoryLimitTestData
+	 */
+	public function testMemoryLimit($iniValue, int $expected) {
+		ini_set('memory_limit', $iniValue);
+		$memoryInfo = new MemoryInfo();
+		self::assertEquals($expected, $memoryInfo->getMemoryLimit());
+	}
+
+	/**
+	 * Provides sufficient memory limit test data.
+	 *
+	 * @return array
+	 */
+	public function getSufficientMemoryTestData(): array {
+		return [
+			'unlimited' => [-1, true,],
+			'512M' => [512 * 1024 * 1024, true,],
+			'1G' => [1024 * 1024 * 1024, true,],
+			'256M' => [256 * 1024 * 1024, false,],
+		];
+	}
+
+	/**
+	 * Tests that isMemoryLimitSufficient returns the correct values.
+	 *
+	 * @param int $memoryLimit The memory limit
+	 * @param bool $expected If the memory limit is sufficient.
+	 * @dataProvider getSufficientMemoryTestData
+	 * @return void
+	 */
+	public function testIsMemoryLimitSufficient(int $memoryLimit, bool $expected) {
+		/* @var MemoryInfo|MockObject $memoryInfo */
+		$memoryInfo = $this->getMockBuilder(MemoryInfo::class)
+			->setMethods(['getMemoryLimit',])
+			->getMock();
+
+		$memoryInfo
+			->method('getMemoryLimit')
+			->willReturn($memoryLimit);
+
+		$isMemoryLimitSufficient = $memoryInfo->isMemoryLimitSufficient();
+		self::assertEquals($expected, $isMemoryLimitSufficient);
+	}
+}