Merge pull request #41086 from nextcloud/feat/migrate-memory-limit-setup-check

Migrate memory_limit check to new SetupCheck API

8 files changed, 74 insertions, 46 deletions

diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php
index f4d29d724f7..85767e46585 100644
--- a/apps/settings/composer/composer/autoload_classmap.php
+++ b/apps/settings/composer/composer/autoload_classmap.php
@@ -83,6 +83,7 @@ return array(
     'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => $baseDir . '/../lib/SetupChecks/PhpDefaultCharset.php',
     'OCA\\Settings\\SetupChecks\\PhpFreetypeSupport' => $baseDir . '/../lib/SetupChecks/PhpFreetypeSupport.php',
     'OCA\\Settings\\SetupChecks\\PhpGetEnv' => $baseDir . '/../lib/SetupChecks/PhpGetEnv.php',
+    'OCA\\Settings\\SetupChecks\\PhpMemoryLimit' => $baseDir . '/../lib/SetupChecks/PhpMemoryLimit.php',
     'OCA\\Settings\\SetupChecks\\PhpModules' => $baseDir . '/../lib/SetupChecks/PhpModules.php',
     'OCA\\Settings\\SetupChecks\\PhpOutdated' => $baseDir . '/../lib/SetupChecks/PhpOutdated.php',
     'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => $baseDir . '/../lib/SetupChecks/PhpOutputBuffering.php',
diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php
index f8595eb7c68..ef6390d0efe 100644
--- a/apps/settings/composer/composer/autoload_static.php
+++ b/apps/settings/composer/composer/autoload_static.php
@@ -98,6 +98,7 @@ class ComposerStaticInitSettings
         'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpDefaultCharset.php',
         'OCA\\Settings\\SetupChecks\\PhpFreetypeSupport' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpFreetypeSupport.php',
         'OCA\\Settings\\SetupChecks\\PhpGetEnv' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpGetEnv.php',
+        'OCA\\Settings\\SetupChecks\\PhpMemoryLimit' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpMemoryLimit.php',
         'OCA\\Settings\\SetupChecks\\PhpModules' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpModules.php',
         'OCA\\Settings\\SetupChecks\\PhpOutdated' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutdated.php',
         'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutputBuffering.php',
diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php
index d2539864708..1693ec302e5 100644
--- a/apps/settings/lib/AppInfo/Application.php
+++ b/apps/settings/lib/AppInfo/Application.php
@@ -58,6 +58,7 @@ use OCA\Settings\SetupChecks\MemcacheConfigured;
 use OCA\Settings\SetupChecks\PhpDefaultCharset;
 use OCA\Settings\SetupChecks\PhpFreetypeSupport;
 use OCA\Settings\SetupChecks\PhpGetEnv;
+use OCA\Settings\SetupChecks\PhpMemoryLimit;
 use OCA\Settings\SetupChecks\PhpModules;
 use OCA\Settings\SetupChecks\PhpOutdated;
 use OCA\Settings\SetupChecks\PhpOutputBuffering;
@@ -163,9 +164,10 @@ class Application extends App implements IBootstrap {
 		$context->registerSetupCheck(LegacySSEKeyFormat::class);
 		$context->registerSetupCheck(MemcacheConfigured::class);
 		$context->registerSetupCheck(PhpDefaultCharset::class);
-		$context->registerSetupCheck(PhpModules::class);
 		$context->registerSetupCheck(PhpFreetypeSupport::class);
 		$context->registerSetupCheck(PhpGetEnv::class);
+		$context->registerSetupCheck(PhpMemoryLimit::class);
+		$context->registerSetupCheck(PhpModules::class);
 		$context->registerSetupCheck(PhpOutdated::class);
 		$context->registerSetupCheck(PhpOutputBuffering::class);
 		$context->registerSetupCheck(RandomnessSecure::class);
diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php
index 04efb6b47a7..0d5e94dd047 100644
--- a/apps/settings/lib/Controller/CheckSetupController.php
+++ b/apps/settings/lib/Controller/CheckSetupController.php
@@ -56,7 +56,6 @@ use OC\DB\MissingIndexInformation;
 use OC\DB\MissingPrimaryKeyInformation;
 use OC\DB\SchemaWrapper;
 use OC\IntegrityCheck\Checker;
-use OC\MemoryInfo;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\Attribute\IgnoreOpenAPI;
@@ -105,8 +104,6 @@ class CheckSetupController extends Controller {
 	private $lockingProvider;
 	/** @var IDateTimeFormatter */
 	private $dateTimeFormatter;
-	/** @var MemoryInfo */
-	private $memoryInfo;
 	/** @var IniGetWrapper */
 	private $iniGetWrapper;
 	/** @var IDBConnection */
@@ -135,7 +132,6 @@ class CheckSetupController extends Controller {
 		Connection $db,
 		ILockingProvider $lockingProvider,
 		IDateTimeFormatter $dateTimeFormatter,
-		MemoryInfo $memoryInfo,
 		IniGetWrapper $iniGetWrapper,
 		IDBConnection $connection,
 		IThrottler $throttler,
@@ -157,7 +153,6 @@ class CheckSetupController extends Controller {
 		$this->throttler = $throttler;
 		$this->lockingProvider = $lockingProvider;
 		$this->dateTimeFormatter = $dateTimeFormatter;
-		$this->memoryInfo = $memoryInfo;
 		$this->iniGetWrapper = $iniGetWrapper;
 		$this->connection = $connection;
 		$this->tempManager = $tempManager;
@@ -745,7 +740,6 @@ Raw output
 				'missingColumns' => $this->hasMissingColumns(),
 				'isSqliteUsed' => $this->isSqliteUsed(),
 				'databaseConversionDocumentation' => $this->urlGenerator->linkToDocs('admin-db-conversion'),
-				'isMemoryLimitSufficient' => $this->memoryInfo->isMemoryLimitSufficient(),
 				'appDirsWithDifferentOwner' => $this->getAppDirsWithDifferentOwner(),
 				'isImagickEnabled' => $this->isImagickEnabled(),
 				'areWebauthnExtensionsEnabled' => $this->areWebauthnExtensionsEnabled(),
diff --git a/apps/settings/lib/SetupChecks/PhpMemoryLimit.php b/apps/settings/lib/SetupChecks/PhpMemoryLimit.php
new file mode 100644
index 00000000000..37f8cb8d8de
--- /dev/null
+++ b/apps/settings/lib/SetupChecks/PhpMemoryLimit.php
@@ -0,0 +1,57 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @author Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @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 OCA\Settings\SetupChecks;
+
+use OC\MemoryInfo;
+use OCP\IL10N;
+use OCP\SetupCheck\ISetupCheck;
+use OCP\SetupCheck\SetupResult;
+use OCP\Util;
+
+class PhpMemoryLimit implements ISetupCheck {
+	public function __construct(
+		private IL10N $l10n,
+		private MemoryInfo $memoryInfo,
+	) {
+	}
+
+	public function getCategory(): string {
+		return 'php';
+	}
+
+	public function getName(): string {
+		return $this->l10n->t('PHP memory limit');
+	}
+
+	public function run(): SetupResult {
+		if ($this->memoryInfo->isMemoryLimitSufficient()) {
+			return SetupResult::success(Util::humanFileSize($this->memoryInfo->getMemoryLimit()));
+		} else {
+			return SetupResult::error($this->l10n->t('The PHP memory limit is below the recommended value of %s.'), Util::humanFileSize(MemoryInfo::RECOMMENDED_MEMORY_LIMIT));
+		}
+	}
+}
diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php
index e8087fbd2d1..edb05a1891e 100644
--- a/apps/settings/tests/Controller/CheckSetupControllerTest.php
+++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php
@@ -39,7 +39,6 @@ use Doctrine\DBAL\Platforms\SqlitePlatform;
 use OC;
 use OC\DB\Connection;
 use OC\IntegrityCheck\Checker;
-use OC\MemoryInfo;
 use OCA\Settings\Controller\CheckSetupController;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http;
@@ -97,8 +96,6 @@ class CheckSetupControllerTest extends TestCase {
 	private $lockingProvider;
 	/** @var IDateTimeFormatter|\PHPUnit\Framework\MockObject\MockObject */
 	private $dateTimeFormatter;
-	/** @var MemoryInfo|MockObject */
-	private $memoryInfo;
 	/** @var IniGetWrapper|\PHPUnit\Framework\MockObject\MockObject */
 	private $iniGetWrapper;
 	/** @var IDBConnection|\PHPUnit\Framework\MockObject\MockObject */
@@ -148,9 +145,6 @@ class CheckSetupControllerTest extends TestCase {
 		$this->throttler = $this->createMock(IThrottler::class);
 		$this->lockingProvider = $this->getMockBuilder(ILockingProvider::class)->getMock();
 		$this->dateTimeFormatter = $this->getMockBuilder(IDateTimeFormatter::class)->getMock();
-		$this->memoryInfo = $this->getMockBuilder(MemoryInfo::class)
-			->setMethods(['isMemoryLimitSufficient',])
-			->getMock();
 		$this->iniGetWrapper = $this->getMockBuilder(IniGetWrapper::class)->getMock();
 		$this->connection = $this->getMockBuilder(IDBConnection::class)
 			->disableOriginalConstructor()->getMock();
@@ -173,7 +167,6 @@ class CheckSetupControllerTest extends TestCase {
 				$this->db,
 				$this->lockingProvider,
 				$this->dateTimeFormatter,
-				$this->memoryInfo,
 				$this->iniGetWrapper,
 				$this->connection,
 				$this->throttler,
@@ -351,9 +344,6 @@ class CheckSetupControllerTest extends TestCase {
 			->expects($this->once())
 			->method('hasPassedCheck')
 			->willReturn(true);
-		$this->memoryInfo
-			->method('isMemoryLimitSufficient')
-			->willReturn(true);
 
 		$this->checkSetupController
 			->expects($this->once())
@@ -441,7 +431,6 @@ class CheckSetupControllerTest extends TestCase {
 				'missingIndexes' => [],
 				'missingPrimaryKeys' => [],
 				'missingColumns' => [],
-				'isMemoryLimitSufficient' => true,
 				'appDirsWithDifferentOwner' => [],
 				'isImagickEnabled' => false,
 				'areWebauthnExtensionsEnabled' => false,
@@ -475,7 +464,6 @@ class CheckSetupControllerTest extends TestCase {
 				$this->db,
 				$this->lockingProvider,
 				$this->dateTimeFormatter,
-				$this->memoryInfo,
 				$this->iniGetWrapper,
 				$this->connection,
 				$this->throttler,
@@ -1203,7 +1191,6 @@ Array
 			$this->db,
 			$this->lockingProvider,
 			$this->dateTimeFormatter,
-			$this->memoryInfo,
 			$this->iniGetWrapper,
 			$this->connection,
 			$this->throttler,
@@ -1258,7 +1245,6 @@ Array
 			$this->db,
 			$this->lockingProvider,
 			$this->dateTimeFormatter,
-			$this->memoryInfo,
 			$this->iniGetWrapper,
 			$this->connection,
 			$this->throttler,
diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js
index f5ac8575365..2c487e10997 100644
--- a/core/js/setupchecks.js
+++ b/core/js/setupchecks.js
@@ -355,12 +355,6 @@
 							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_ERROR
-						})
-					}
 
 					if(data.appDirsWithDifferentOwner && data.appDirsWithDifferentOwner.length > 0) {
 						var appDirsWithDifferentOwner = data.appDirsWithDifferentOwner.reduce(
diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js
index 246574653a9..88c455974bd 100644
--- a/core/js/tests/specs/setupchecksSpec.js
+++ b/core/js/tests/specs/setupchecksSpec.js
@@ -237,7 +237,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -292,7 +291,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -347,7 +345,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -402,7 +399,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -455,7 +451,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [
 						'/some/path'
 					],
@@ -511,7 +506,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -567,7 +561,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -621,7 +614,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -675,7 +667,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: false,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -692,6 +683,18 @@ describe('OC.SetupChecks tests', function() {
 								linkToDoc: null
 							}
 						},
+						php: {
+							"Internet connectivity": {
+								severity: "success",
+								description: null,
+								linkToDoc: null
+							},
+							"PHP memory limit": {
+								severity: "error",
+								description: "The PHP memory limit is below the recommended value of 512MB.",
+								linkToDoc: null
+							},
+						},
 					},
 				})
 			);
@@ -748,7 +751,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -808,7 +810,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -861,7 +862,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -918,7 +918,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -972,7 +971,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -1023,7 +1021,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -1077,7 +1074,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: false,
 					areWebauthnExtensionsEnabled: true,
@@ -1131,7 +1127,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: false,
@@ -1184,7 +1179,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,
@@ -1244,7 +1238,6 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [],
 					isImagickEnabled: true,
 					areWebauthnExtensionsEnabled: true,