Skip to content

Commit

Permalink
Migrate memory_limit check to new SetupCheck API
Browse files Browse the repository at this point in the history
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
  • Loading branch information
come-nc committed Nov 13, 2023
1 parent eaaf954 commit 5be166a
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 46 deletions.
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
'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',
Expand Down
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
4 changes: 3 additions & 1 deletion apps/settings/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
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;
Expand Down Expand Up @@ -163,9 +164,10 @@ public function register(IRegistrationContext $context): void {
$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);
Expand Down
6 changes: 0 additions & 6 deletions apps/settings/lib/Controller/CheckSetupController.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
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;
Expand Down Expand Up @@ -105,8 +104,6 @@ class CheckSetupController extends Controller {
private $lockingProvider;
/** @var IDateTimeFormatter */
private $dateTimeFormatter;
/** @var MemoryInfo */
private $memoryInfo;
/** @var IniGetWrapper */
private $iniGetWrapper;
/** @var IDBConnection */
Expand Down Expand Up @@ -135,7 +132,6 @@ public function __construct($AppName,
Connection $db,
ILockingProvider $lockingProvider,
IDateTimeFormatter $dateTimeFormatter,
MemoryInfo $memoryInfo,
IniGetWrapper $iniGetWrapper,
IDBConnection $connection,
IThrottler $throttler,
Expand All @@ -157,7 +153,6 @@ public function __construct($AppName,
$this->throttler = $throttler;
$this->lockingProvider = $lockingProvider;
$this->dateTimeFormatter = $dateTimeFormatter;
$this->memoryInfo = $memoryInfo;
$this->iniGetWrapper = $iniGetWrapper;
$this->connection = $connection;
$this->tempManager = $tempManager;
Expand Down Expand Up @@ -745,7 +740,6 @@ public function check() {
'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(),
Expand Down
58 changes: 58 additions & 0 deletions apps/settings/lib/SetupChecks/PhpMemoryLimit.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?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 {
$value = trim(ini_get('output_buffering'));
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));
}
}
}
14 changes: 0 additions & 14 deletions apps/settings/tests/Controller/CheckSetupControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
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;
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -148,9 +145,6 @@ protected function setUp(): void {
$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();
Expand All @@ -173,7 +167,6 @@ protected function setUp(): void {
$this->db,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->memoryInfo,
$this->iniGetWrapper,
$this->connection,
$this->throttler,
Expand Down Expand Up @@ -351,9 +344,6 @@ public function testCheck() {
->expects($this->once())
->method('hasPassedCheck')
->willReturn(true);
$this->memoryInfo
->method('isMemoryLimitSufficient')
->willReturn(true);

$this->checkSetupController
->expects($this->once())
Expand Down Expand Up @@ -441,7 +431,6 @@ public function testCheck() {
'missingIndexes' => [],
'missingPrimaryKeys' => [],
'missingColumns' => [],
'isMemoryLimitSufficient' => true,
'appDirsWithDifferentOwner' => [],
'isImagickEnabled' => false,
'areWebauthnExtensionsEnabled' => false,
Expand Down Expand Up @@ -475,7 +464,6 @@ public function testGetCurlVersion() {
$this->db,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->memoryInfo,
$this->iniGetWrapper,
$this->connection,
$this->throttler,
Expand Down Expand Up @@ -1203,7 +1191,6 @@ public function testIsMysqlUsedWithoutUTF8MB4(string $db, bool $useUTF8MB4, bool
$this->db,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->memoryInfo,
$this->iniGetWrapper,
$this->connection,
$this->throttler,
Expand Down Expand Up @@ -1258,7 +1245,6 @@ public function testIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(string $m
$this->db,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->memoryInfo,
$this->iniGetWrapper,
$this->connection,
$this->throttler,
Expand Down
6 changes: 0 additions & 6 deletions core/js/setupchecks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
31 changes: 12 additions & 19 deletions core/js/tests/specs/setupchecksSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -292,7 +291,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -347,7 +345,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -402,7 +399,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -455,7 +451,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [
'/some/path'
],
Expand Down Expand Up @@ -511,7 +506,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -567,7 +561,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -621,7 +614,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -675,7 +667,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: false,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand All @@ -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
},
},
},
})
);
Expand Down Expand Up @@ -748,7 +751,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -808,7 +810,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -861,7 +862,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -918,7 +918,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -972,7 +971,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -1023,7 +1021,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -1077,7 +1074,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: false,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -1131,7 +1127,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: false,
Expand Down Expand Up @@ -1184,7 +1179,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down Expand Up @@ -1244,7 +1238,6 @@ describe('OC.SetupChecks tests', function() {
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
Expand Down

0 comments on commit 5be166a

Please sign in to comment.