From 3f13a1a47e13cc1c09602c04016c4ff49c3621a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 16 Nov 2023 17:05:24 +0100 Subject: [PATCH 1/5] Migrate opcache check to new SetupCheck API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/settings/lib/AppInfo/Application.php | 2 + .../lib/Controller/CheckSetupController.php | 79 ----------- .../lib/SetupChecks/PhpOpcacheSetup.php | 134 ++++++++++++++++++ .../Controller/CheckSetupControllerTest.php | 14 -- core/js/setupchecks.js | 12 -- core/js/tests/specs/setupchecksSpec.js | 63 -------- 8 files changed, 138 insertions(+), 168 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/PhpOpcacheSetup.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 4ad4e2b652558..b32aab9756955 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -95,6 +95,7 @@ '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\\PhpOpcacheSetup' => $baseDir . '/../lib/SetupChecks/PhpOpcacheSetup.php', 'OCA\\Settings\\SetupChecks\\PhpOutdated' => $baseDir . '/../lib/SetupChecks/PhpOutdated.php', 'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => $baseDir . '/../lib/SetupChecks/PhpOutputBuffering.php', 'OCA\\Settings\\SetupChecks\\RandomnessSecure' => $baseDir . '/../lib/SetupChecks/RandomnessSecure.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index dc4a2b21b3508..09e0d724e4ccb 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -110,6 +110,7 @@ class ComposerStaticInitSettings '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\\PhpOpcacheSetup' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOpcacheSetup.php', 'OCA\\Settings\\SetupChecks\\PhpOutdated' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutdated.php', 'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutputBuffering.php', 'OCA\\Settings\\SetupChecks\\RandomnessSecure' => __DIR__ . '/..' . '/../lib/SetupChecks/RandomnessSecure.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index 0da2485aa00a7..7e963e355930b 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -67,6 +67,7 @@ use OCA\Settings\SetupChecks\PhpGetEnv; use OCA\Settings\SetupChecks\PhpMemoryLimit; use OCA\Settings\SetupChecks\PhpModules; +use OCA\Settings\SetupChecks\PhpOpcacheSetup; use OCA\Settings\SetupChecks\PhpOutdated; use OCA\Settings\SetupChecks\PhpOutputBuffering; use OCA\Settings\SetupChecks\RandomnessSecure; @@ -182,6 +183,7 @@ public function register(IRegistrationContext $context): void { $context->registerSetupCheck(PhpGetEnv::class); $context->registerSetupCheck(PhpMemoryLimit::class); $context->registerSetupCheck(PhpModules::class); + $context->registerSetupCheck(PhpOpcacheSetup::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 14f7068ce59a1..0cb8c50d38728 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -45,7 +45,6 @@ */ namespace OCA\Settings\Controller; -use bantu\IniGetWrapper\IniGetWrapper; use DirectoryIterator; use GuzzleHttp\Exception\ClientException; use OC; @@ -91,8 +90,6 @@ class CheckSetupController extends Controller { private $lockingProvider; /** @var IDateTimeFormatter */ private $dateTimeFormatter; - /** @var IniGetWrapper */ - private $iniGetWrapper; /** @var ITempManager */ private $tempManager; /** @var IManager */ @@ -114,7 +111,6 @@ public function __construct($AppName, IEventDispatcher $dispatcher, ILockingProvider $lockingProvider, IDateTimeFormatter $dateTimeFormatter, - IniGetWrapper $iniGetWrapper, ITempManager $tempManager, IManager $manager, IAppManager $appManager, @@ -131,7 +127,6 @@ public function __construct($AppName, $this->dispatcher = $dispatcher; $this->lockingProvider = $lockingProvider; $this->dateTimeFormatter = $dateTimeFormatter; - $this->iniGetWrapper = $iniGetWrapper; $this->tempManager = $tempManager; $this->manager = $manager; $this->appManager = $appManager; @@ -328,79 +323,6 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse { ); } - /** - * Checks whether a PHP OPcache is properly set up - * @return string[] The list of OPcache setup recommendations - */ - protected function getOpcacheSetupRecommendations(): array { - // If the module is not loaded, return directly to skip inapplicable checks - if (!extension_loaded('Zend OPcache')) { - return [$this->l10n->t('The PHP OPcache module is not loaded. For better performance it is recommended to load it into your PHP installation.')]; - } - - $recommendations = []; - - // Check whether Nextcloud is allowed to use the OPcache API - $isPermitted = true; - $permittedPath = $this->iniGetWrapper->getString('opcache.restrict_api'); - if (isset($permittedPath) && $permittedPath !== '' && !str_starts_with(\OC::$SERVERROOT, rtrim($permittedPath, '/'))) { - $isPermitted = false; - } - - if (!$this->iniGetWrapper->getBool('opcache.enable')) { - $recommendations[] = $this->l10n->t('OPcache is disabled. For better performance, it is recommended to apply opcache.enable=1 to your PHP configuration.'); - - // Check for saved comments only when OPcache is currently disabled. If it was enabled, opcache.save_comments=0 would break Nextcloud in the first place. - if (!$this->iniGetWrapper->getBool('opcache.save_comments')) { - $recommendations[] = $this->l10n->t('OPcache is configured to remove code comments. With OPcache enabled, opcache.save_comments=1 must be set for Nextcloud to function.'); - } - - if (!$isPermitted) { - $recommendations[] = $this->l10n->t('Nextcloud is not allowed to use the OPcache API. With OPcache enabled, it is highly recommended to include all Nextcloud directories with opcache.restrict_api or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.'); - } - } elseif (!$isPermitted) { - $recommendations[] = $this->l10n->t('Nextcloud is not allowed to use the OPcache API. It is highly recommended to include all Nextcloud directories with opcache.restrict_api or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.'); - } elseif ($this->iniGetWrapper->getBool('opcache.file_cache_only')) { - $recommendations[] = $this->l10n->t('The shared memory based OPcache is disabled. For better performance, it is recommended to apply opcache.file_cache_only=0 to your PHP configuration and use the file cache as second level cache only.'); - } else { - // Check whether opcache_get_status has been explicitly disabled an in case skip usage based checks - $disabledFunctions = $this->iniGetWrapper->getString('disable_functions'); - if (isset($disabledFunctions) && str_contains($disabledFunctions, 'opcache_get_status')) { - return []; - } - - $status = opcache_get_status(false); - - // Recommend to raise value, if more than 90% of max value is reached - if ( - empty($status['opcache_statistics']['max_cached_keys']) || - ($status['opcache_statistics']['num_cached_keys'] / $status['opcache_statistics']['max_cached_keys'] > 0.9) - ) { - $recommendations[] = $this->l10n->t('The maximum number of OPcache keys is nearly exceeded. To assure that all scripts can be kept in the cache, it is recommended to apply opcache.max_accelerated_files to your PHP configuration with a value higher than %s.', [($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') ?: 'currently')]); - } - - if ( - empty($status['memory_usage']['free_memory']) || - ($status['memory_usage']['used_memory'] / $status['memory_usage']['free_memory'] > 9) - ) { - $recommendations[] = $this->l10n->t('The OPcache buffer is nearly full. To assure that all scripts can be hold in cache, it is recommended to apply opcache.memory_consumption to your PHP configuration with a value higher than %s.', [($this->iniGetWrapper->getNumeric('opcache.memory_consumption') ?: 'currently')]); - } - - if ( - // Do not recommend to raise the interned strings buffer size above a quarter of the total OPcache size - ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') < $this->iniGetWrapper->getNumeric('opcache.memory_consumption') / 4) && - ( - empty($status['interned_strings_usage']['free_memory']) || - ($status['interned_strings_usage']['used_memory'] / $status['interned_strings_usage']['free_memory'] > 9) - ) - ) { - $recommendations[] = $this->l10n->t('The OPcache interned strings buffer is nearly full. To assure that repeating strings can be effectively cached, it is recommended to apply opcache.interned_strings_buffer to your PHP configuration with a value higher than %s.', [($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') ?: 'currently')]); - } - } - - return $recommendations; - } - protected function getSuggestedOverwriteCliURL(): string { $currentOverwriteCliUrl = $this->config->getSystemValue('overwrite.cli.url', ''); $suggestedOverwriteCliUrl = $this->request->getServerProtocol() . '://' . $this->request->getInsecureServerHost() . \OC::$WEBROOT; @@ -568,7 +490,6 @@ public function check() { 'isCorrectMemcachedPHPModuleInstalled' => $this->isCorrectMemcachedPHPModuleInstalled(), 'hasPassedCodeIntegrityCheck' => $this->checker->hasPassedCheck(), 'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'), - 'OpcacheSetupRecommendations' => $this->getOpcacheSetupRecommendations(), 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), 'appDirsWithDifferentOwner' => $this->getAppDirsWithDifferentOwner(), 'isImagickEnabled' => $this->isImagickEnabled(), diff --git a/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php b/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php new file mode 100644 index 0000000000000..faf686e0dfbb5 --- /dev/null +++ b/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php @@ -0,0 +1,134 @@ + + * + * @author Côme Chilliet + * + * @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 . + * + */ +namespace OCA\Settings\SetupChecks; + +use bantu\IniGetWrapper\IniGetWrapper; +use OCP\IL10N; +use OCP\IURLGenerator; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class PhpOpcacheSetup implements ISetupCheck { + public function __construct( + private IL10N $l10n, + private IURLGenerator $urlGenerator, + private IniGetWrapper $iniGetWrapper, + ) { + } + + public function getName(): string { + return $this->l10n->t('PHP opcache'); + } + + public function getCategory(): string { + return 'php'; + } + + /** + * Checks whether a PHP OPcache is properly set up + * @return string[] The list of OPcache setup recommendations + */ + protected function getOpcacheSetupRecommendations(): array { + // If the module is not loaded, return directly to skip inapplicable checks + if (!extension_loaded('Zend OPcache')) { + return [$this->l10n->t('The PHP OPcache module is not loaded. For better performance it is recommended to load it into your PHP installation.')]; + } + + $recommendations = []; + + // Check whether Nextcloud is allowed to use the OPcache API + $isPermitted = true; + $permittedPath = $this->iniGetWrapper->getString('opcache.restrict_api'); + if (isset($permittedPath) && $permittedPath !== '' && !str_starts_with(\OC::$SERVERROOT, rtrim($permittedPath, '/'))) { + $isPermitted = false; + } + + if (!$this->iniGetWrapper->getBool('opcache.enable')) { + $recommendations[] = $this->l10n->t('OPcache is disabled. For better performance, it is recommended to apply opcache.enable=1 to your PHP configuration.'); + + // Check for saved comments only when OPcache is currently disabled. If it was enabled, opcache.save_comments=0 would break Nextcloud in the first place. + if (!$this->iniGetWrapper->getBool('opcache.save_comments')) { + $recommendations[] = $this->l10n->t('OPcache is configured to remove code comments. With OPcache enabled, opcache.save_comments=1 must be set for Nextcloud to function.'); + } + + if (!$isPermitted) { + $recommendations[] = $this->l10n->t('Nextcloud is not allowed to use the OPcache API. With OPcache enabled, it is highly recommended to include all Nextcloud directories with opcache.restrict_api or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.'); + } + } elseif (!$isPermitted) { + $recommendations[] = $this->l10n->t('Nextcloud is not allowed to use the OPcache API. It is highly recommended to include all Nextcloud directories with opcache.restrict_api or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.'); + } elseif ($this->iniGetWrapper->getBool('opcache.file_cache_only')) { + $recommendations[] = $this->l10n->t('The shared memory based OPcache is disabled. For better performance, it is recommended to apply opcache.file_cache_only=0 to your PHP configuration and use the file cache as second level cache only.'); + } else { + // Check whether opcache_get_status has been explicitly disabled an in case skip usage based checks + $disabledFunctions = $this->iniGetWrapper->getString('disable_functions'); + if (isset($disabledFunctions) && str_contains($disabledFunctions, 'opcache_get_status')) { + return []; + } + + $status = opcache_get_status(false); + + // Recommend to raise value, if more than 90% of max value is reached + if ( + empty($status['opcache_statistics']['max_cached_keys']) || + ($status['opcache_statistics']['num_cached_keys'] / $status['opcache_statistics']['max_cached_keys'] > 0.9) + ) { + $recommendations[] = $this->l10n->t('The maximum number of OPcache keys is nearly exceeded. To assure that all scripts can be kept in the cache, it is recommended to apply opcache.max_accelerated_files to your PHP configuration with a value higher than %s.', [($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') ?: 'currently')]); + } + + if ( + empty($status['memory_usage']['free_memory']) || + ($status['memory_usage']['used_memory'] / $status['memory_usage']['free_memory'] > 9) + ) { + $recommendations[] = $this->l10n->t('The OPcache buffer is nearly full. To assure that all scripts can be hold in cache, it is recommended to apply opcache.memory_consumption to your PHP configuration with a value higher than %s.', [($this->iniGetWrapper->getNumeric('opcache.memory_consumption') ?: 'currently')]); + } + + if ( + // Do not recommend to raise the interned strings buffer size above a quarter of the total OPcache size + ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') < $this->iniGetWrapper->getNumeric('opcache.memory_consumption') / 4) && + ( + empty($status['interned_strings_usage']['free_memory']) || + ($status['interned_strings_usage']['used_memory'] / $status['interned_strings_usage']['free_memory'] > 9) + ) + ) { + $recommendations[] = $this->l10n->t('The OPcache interned strings buffer is nearly full. To assure that repeating strings can be effectively cached, it is recommended to apply opcache.interned_strings_buffer to your PHP configuration with a value higher than %s.', [($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') ?: 'currently')]); + } + } + + return $recommendations; + } + + public function run(): SetupResult { + $recommendations = $this->getOpcacheSetupRecommendations(); + if (!empty($recommendations)) { + return SetupResult::error( + $this->l10n->t('The PHP OPcache module is not properly configured. %s.', implode("\n", $recommendations)), + $this->urlGenerator->linkToDocs('admin-php-opcache') + ); + } else { + return SetupResult::success($this->l10n->t('Correctly configured')); + } + } +} diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index a4cd6bb0a961e..d732b615a0ea7 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -34,7 +34,6 @@ */ namespace OCA\Settings\Tests\Controller; -use bantu\IniGetWrapper\IniGetWrapper; use OC; use OC\IntegrityCheck\Checker; use OCA\Settings\Controller\CheckSetupController; @@ -89,8 +88,6 @@ class CheckSetupControllerTest extends TestCase { private $lockingProvider; /** @var IDateTimeFormatter|\PHPUnit\Framework\MockObject\MockObject */ private $dateTimeFormatter; - /** @var IniGetWrapper|\PHPUnit\Framework\MockObject\MockObject */ - private $iniGetWrapper; /** @var ITempManager|\PHPUnit\Framework\MockObject\MockObject */ private $tempManager; /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ @@ -133,7 +130,6 @@ protected function setUp(): void { $this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); $this->lockingProvider = $this->getMockBuilder(ILockingProvider::class)->getMock(); $this->dateTimeFormatter = $this->getMockBuilder(IDateTimeFormatter::class)->getMock(); - $this->iniGetWrapper = $this->getMockBuilder(IniGetWrapper::class)->getMock(); $this->tempManager = $this->getMockBuilder(ITempManager::class)->getMock(); $this->notificationManager = $this->getMockBuilder(IManager::class)->getMock(); $this->appManager = $this->createMock(IAppManager::class); @@ -152,7 +148,6 @@ protected function setUp(): void { $this->dispatcher, $this->lockingProvider, $this->dateTimeFormatter, - $this->iniGetWrapper, $this->tempManager, $this->notificationManager, $this->appManager, @@ -164,7 +159,6 @@ protected function setUp(): void { 'getSuggestedOverwriteCliURL', 'getCurlVersion', 'isPhpOutdated', - 'getOpcacheSetupRecommendations', 'isPHPMailerUsed', 'getAppDirsWithDifferentOwner', 'isImagickEnabled', @@ -208,10 +202,6 @@ public function testCheck() { ->method('getHeader'); $this->clientService->expects($this->never()) ->method('newClient'); - $this->checkSetupController - ->expects($this->once()) - ->method('getOpcacheSetupRecommendations') - ->willReturn(['recommendation1', 'recommendation2']); $this->checkSetupController ->expects($this->once()) ->method('getSuggestedOverwriteCliURL') @@ -299,7 +289,6 @@ public function testCheck() { 'isCorrectMemcachedPHPModuleInstalled' => true, 'hasPassedCodeIntegrityCheck' => true, 'codeIntegrityCheckerDocumentation' => 'http://docs.example.org/server/go.php?to=admin-code-integrity', - 'OpcacheSetupRecommendations' => ['recommendation1', 'recommendation2'], 'isSettimelimitAvailable' => true, 'appDirsWithDifferentOwner' => [], 'isImagickEnabled' => false, @@ -330,7 +319,6 @@ public function testGetCurlVersion() { $this->dispatcher, $this->lockingProvider, $this->dateTimeFormatter, - $this->iniGetWrapper, $this->tempManager, $this->notificationManager, $this->appManager, @@ -1054,7 +1042,6 @@ public function testIsMysqlUsedWithoutUTF8MB4(string $db, bool $useUTF8MB4, bool $this->dispatcher, $this->lockingProvider, $this->dateTimeFormatter, - $this->iniGetWrapper, $this->tempManager, $this->notificationManager, $this->appManager, @@ -1105,7 +1092,6 @@ public function testIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(string $m $this->dispatcher, $this->lockingProvider, $this->dateTimeFormatter, - $this->iniGetWrapper, $this->tempManager, $this->notificationManager, $this->appManager, diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index e7ad920760eda..e85f32dae4ac6 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -240,18 +240,6 @@ type: OC.SetupChecks.MESSAGE_TYPE_ERROR }); } - if(data.OpcacheSetupRecommendations.length > 0) { - var listOfOPcacheRecommendations = ""; - data.OpcacheSetupRecommendations.forEach(function(element){ - listOfOPcacheRecommendations += '
  • ' + element + '
  • '; - }); - messages.push({ - msg: t('core', 'The PHP OPcache module is not properly configured. See the {linkstart}documentation ↗{linkend} for more information.') - .replace('{linkstart}', '') - .replace('{linkend}', '') + '
      ' + listOfOPcacheRecommendations + '
    ', - type: OC.SetupChecks.MESSAGE_TYPE_INFO - }); - } if(!data.isSettimelimitAvailable) { messages.push({ msg: t('core', 'The PHP function "set_time_limit" is not available. This could result in scripts being halted mid-execution, breaking your installation. Enabling this function is strongly recommended.'), diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 5184b3e8c0d80..bb7d118fc52c5 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -227,7 +227,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -276,7 +275,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -325,7 +323,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -374,7 +371,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: false, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -421,7 +417,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -471,7 +466,6 @@ describe('OC.SetupChecks tests', function() { reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: false, cronErrors: [], cronInfo: { @@ -519,7 +513,6 @@ describe('OC.SetupChecks tests', function() { reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -598,7 +591,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -639,53 +631,6 @@ describe('OC.SetupChecks tests', function() { }); }); - it('should return an info if server has no proper opcache', function(done) { - var async = OC.SetupChecks.checkSetup(); - - suite.server.requests[0].respond( - 200, - { - 'Content-Type': 'application/json' - }, - JSON.stringify({ - suggestedOverwriteCliURL: '', - isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: ['recommendation1', 'recommendation2'], - isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, - appDirsWithDifferentOwner: [], - isImagickEnabled: true, - areWebauthnExtensionsEnabled: true, - isMysqlUsedWithoutUTF8MB4: false, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, - reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, - generic: { - network: { - "Internet connectivity": { - severity: "success", - description: null, - linkToDoc: null - } - }, - }, - }) - ); - - async.done(function( data, s, x ){ - expect(data).toEqual([{ - msg: 'The PHP OPcache module is not properly configured. See the documentation ↗ for more information.
    • recommendation1
    • recommendation2
    ', - type: OC.SetupChecks.MESSAGE_TYPE_INFO - }]); - done(); - }); - }); - it('should return an error if the php version is no longer supported', function(done) { var async = OC.SetupChecks.checkSetup(); @@ -699,7 +644,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -750,7 +694,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -798,7 +741,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -843,7 +785,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -891,7 +832,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -939,7 +879,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -986,7 +925,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -1040,7 +978,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, - OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, cronErrors: [], cronInfo: { From 989b5ffbab5a5c06da36c9c3e9fe1b9047b2ac81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 16 Nov 2023 17:15:21 +0100 Subject: [PATCH 2/5] Remove unused properties from CheckSetupControllerTest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/Controller/CheckSetupController.php | 23 +------------ .../Controller/CheckSetupControllerTest.php | 32 ------------------- 2 files changed, 1 insertion(+), 54 deletions(-) diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 0cb8c50d38728..b251adb0d84f8 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -47,25 +47,20 @@ use DirectoryIterator; use GuzzleHttp\Exception\ClientException; -use OC; use OC\AppFramework\Http; use OC\IntegrityCheck\Checker; -use OCP\App\IAppManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\Attribute\IgnoreOpenAPI; use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; -use OCP\EventDispatcher\IEventDispatcher; use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\IDateTimeFormatter; use OCP\IL10N; use OCP\IRequest; -use OCP\IServerContainer; use OCP\ITempManager; use OCP\IURLGenerator; -use OCP\Lock\ILockingProvider; use OCP\Notification\IManager; use OCP\SetupCheck\ISetupCheckManager; use Psr\Log\LoggerInterface; @@ -84,20 +79,12 @@ class CheckSetupController extends Controller { private $checker; /** @var LoggerInterface */ private $logger; - /** @var IEventDispatcher */ - private $dispatcher; - /** @var ILockingProvider */ - private $lockingProvider; /** @var IDateTimeFormatter */ private $dateTimeFormatter; /** @var ITempManager */ private $tempManager; /** @var IManager */ private $manager; - /** @var IAppManager */ - private $appManager; - /** @var IServerContainer */ - private $serverContainer; private ISetupCheckManager $setupCheckManager; public function __construct($AppName, @@ -108,13 +95,9 @@ public function __construct($AppName, IL10N $l10n, Checker $checker, LoggerInterface $logger, - IEventDispatcher $dispatcher, - ILockingProvider $lockingProvider, IDateTimeFormatter $dateTimeFormatter, ITempManager $tempManager, IManager $manager, - IAppManager $appManager, - IServerContainer $serverContainer, ISetupCheckManager $setupCheckManager, ) { parent::__construct($AppName, $request); @@ -124,13 +107,9 @@ public function __construct($AppName, $this->l10n = $l10n; $this->checker = $checker; $this->logger = $logger; - $this->dispatcher = $dispatcher; - $this->lockingProvider = $lockingProvider; $this->dateTimeFormatter = $dateTimeFormatter; $this->tempManager = $tempManager; $this->manager = $manager; - $this->appManager = $appManager; - $this->serverContainer = $serverContainer; $this->setupCheckManager = $setupCheckManager; } @@ -374,7 +353,7 @@ protected function getAppDirsWithDifferentOwner(): array { $currentUser = posix_getuid(); $appDirsWithDifferentOwner = [[]]; - foreach (OC::$APPSROOTS as $appRoot) { + foreach (\OC::$APPSROOTS as $appRoot) { if ($appRoot['writable'] === true) { $appDirsWithDifferentOwner[] = $this->getAppDirsWithDifferentOwnerForAppRoot($currentUser, $appRoot); } diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index d732b615a0ea7..b9c2ddc8282ff 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -37,21 +37,17 @@ use OC; use OC\IntegrityCheck\Checker; use OCA\Settings\Controller\CheckSetupController; -use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; -use OCP\EventDispatcher\IEventDispatcher; use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\IDateTimeFormatter; use OCP\IL10N; use OCP\IRequest; -use OCP\IServerContainer; use OCP\ITempManager; use OCP\IURLGenerator; -use OCP\Lock\ILockingProvider; use OCP\Notification\IManager; use OCP\SetupCheck\ISetupCheckManager; use PHPUnit\Framework\MockObject\MockObject; @@ -82,20 +78,12 @@ class CheckSetupControllerTest extends TestCase { private $logger; /** @var Checker|\PHPUnit\Framework\MockObject\MockObject */ private $checker; - /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ - private $dispatcher; - /** @var ILockingProvider|\PHPUnit\Framework\MockObject\MockObject */ - private $lockingProvider; /** @var IDateTimeFormatter|\PHPUnit\Framework\MockObject\MockObject */ private $dateTimeFormatter; /** @var ITempManager|\PHPUnit\Framework\MockObject\MockObject */ private $tempManager; /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ private $notificationManager; - /** @var IAppManager|MockObject */ - private $appManager; - /** @var IServerContainer|MockObject */ - private $serverContainer; /** @var ISetupCheckManager|MockObject */ private $setupCheckManager; @@ -124,16 +112,12 @@ protected function setUp(): void { ->willReturnCallback(function ($message, array $replace) { return vsprintf($message, $replace); }); - $this->dispatcher = $this->createMock(IEventDispatcher::class); $this->checker = $this->getMockBuilder('\OC\IntegrityCheck\Checker') ->disableOriginalConstructor()->getMock(); $this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); - $this->lockingProvider = $this->getMockBuilder(ILockingProvider::class)->getMock(); $this->dateTimeFormatter = $this->getMockBuilder(IDateTimeFormatter::class)->getMock(); $this->tempManager = $this->getMockBuilder(ITempManager::class)->getMock(); $this->notificationManager = $this->getMockBuilder(IManager::class)->getMock(); - $this->appManager = $this->createMock(IAppManager::class); - $this->serverContainer = $this->createMock(IServerContainer::class); $this->setupCheckManager = $this->createMock(ISetupCheckManager::class); $this->checkSetupController = $this->getMockBuilder(CheckSetupController::class) ->setConstructorArgs([ @@ -145,13 +129,9 @@ protected function setUp(): void { $this->l10n, $this->checker, $this->logger, - $this->dispatcher, - $this->lockingProvider, $this->dateTimeFormatter, $this->tempManager, $this->notificationManager, - $this->appManager, - $this->serverContainer, $this->setupCheckManager, ]) ->setMethods([ @@ -316,13 +296,9 @@ public function testGetCurlVersion() { $this->l10n, $this->checker, $this->logger, - $this->dispatcher, - $this->lockingProvider, $this->dateTimeFormatter, $this->tempManager, $this->notificationManager, - $this->appManager, - $this->serverContainer, $this->setupCheckManager, ]) ->setMethods(null)->getMock(); @@ -1039,13 +1015,9 @@ public function testIsMysqlUsedWithoutUTF8MB4(string $db, bool $useUTF8MB4, bool $this->l10n, $this->checker, $this->logger, - $this->dispatcher, - $this->lockingProvider, $this->dateTimeFormatter, $this->tempManager, $this->notificationManager, - $this->appManager, - $this->serverContainer, $this->setupCheckManager, ); @@ -1089,13 +1061,9 @@ public function testIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(string $m $this->l10n, $this->checker, $this->logger, - $this->dispatcher, - $this->lockingProvider, $this->dateTimeFormatter, $this->tempManager, $this->notificationManager, - $this->appManager, - $this->serverContainer, $this->setupCheckManager, ); From f6df5dae349c92de41fd1a64d09114a8435eea0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 12 Dec 2023 16:04:05 +0100 Subject: [PATCH 3/5] Remove unsupported tag and use quotes instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../settings/lib/SetupChecks/PhpOpcacheSetup.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php b/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php index faf686e0dfbb5..aa1f2367bf56e 100644 --- a/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php +++ b/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php @@ -67,20 +67,20 @@ protected function getOpcacheSetupRecommendations(): array { } if (!$this->iniGetWrapper->getBool('opcache.enable')) { - $recommendations[] = $this->l10n->t('OPcache is disabled. For better performance, it is recommended to apply opcache.enable=1 to your PHP configuration.'); + $recommendations[] = $this->l10n->t('OPcache is disabled. For better performance, it is recommended to apply "opcache.enable=1" to your PHP configuration.'); // Check for saved comments only when OPcache is currently disabled. If it was enabled, opcache.save_comments=0 would break Nextcloud in the first place. if (!$this->iniGetWrapper->getBool('opcache.save_comments')) { - $recommendations[] = $this->l10n->t('OPcache is configured to remove code comments. With OPcache enabled, opcache.save_comments=1 must be set for Nextcloud to function.'); + $recommendations[] = $this->l10n->t('OPcache is configured to remove code comments. With OPcache enabled, "opcache.save_comments=1" must be set for Nextcloud to function.'); } if (!$isPermitted) { - $recommendations[] = $this->l10n->t('Nextcloud is not allowed to use the OPcache API. With OPcache enabled, it is highly recommended to include all Nextcloud directories with opcache.restrict_api or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.'); + $recommendations[] = $this->l10n->t('Nextcloud is not allowed to use the OPcache API. With OPcache enabled, it is highly recommended to include all Nextcloud directories with "opcache.restrict_api" or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.'); } } elseif (!$isPermitted) { - $recommendations[] = $this->l10n->t('Nextcloud is not allowed to use the OPcache API. It is highly recommended to include all Nextcloud directories with opcache.restrict_api or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.'); + $recommendations[] = $this->l10n->t('Nextcloud is not allowed to use the OPcache API. It is highly recommended to include all Nextcloud directories with "opcache.restrict_api" or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.'); } elseif ($this->iniGetWrapper->getBool('opcache.file_cache_only')) { - $recommendations[] = $this->l10n->t('The shared memory based OPcache is disabled. For better performance, it is recommended to apply opcache.file_cache_only=0 to your PHP configuration and use the file cache as second level cache only.'); + $recommendations[] = $this->l10n->t('The shared memory based OPcache is disabled. For better performance, it is recommended to apply "opcache.file_cache_only=0" to your PHP configuration and use the file cache as second level cache only.'); } else { // Check whether opcache_get_status has been explicitly disabled an in case skip usage based checks $disabledFunctions = $this->iniGetWrapper->getString('disable_functions'); @@ -95,14 +95,14 @@ protected function getOpcacheSetupRecommendations(): array { empty($status['opcache_statistics']['max_cached_keys']) || ($status['opcache_statistics']['num_cached_keys'] / $status['opcache_statistics']['max_cached_keys'] > 0.9) ) { - $recommendations[] = $this->l10n->t('The maximum number of OPcache keys is nearly exceeded. To assure that all scripts can be kept in the cache, it is recommended to apply opcache.max_accelerated_files to your PHP configuration with a value higher than %s.', [($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') ?: 'currently')]); + $recommendations[] = $this->l10n->t('The maximum number of OPcache keys is nearly exceeded. To assure that all scripts can be kept in the cache, it is recommended to apply "opcache.max_accelerated_files" to your PHP configuration with a value higher than "%s".', [($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') ?: 'currently')]); } if ( empty($status['memory_usage']['free_memory']) || ($status['memory_usage']['used_memory'] / $status['memory_usage']['free_memory'] > 9) ) { - $recommendations[] = $this->l10n->t('The OPcache buffer is nearly full. To assure that all scripts can be hold in cache, it is recommended to apply opcache.memory_consumption to your PHP configuration with a value higher than %s.', [($this->iniGetWrapper->getNumeric('opcache.memory_consumption') ?: 'currently')]); + $recommendations[] = $this->l10n->t('The OPcache buffer is nearly full. To assure that all scripts can be hold in cache, it is recommended to apply "opcache.memory_consumption" to your PHP configuration with a value higher than "%s".', [($this->iniGetWrapper->getNumeric('opcache.memory_consumption') ?: 'currently')]); } if ( @@ -113,7 +113,7 @@ protected function getOpcacheSetupRecommendations(): array { ($status['interned_strings_usage']['used_memory'] / $status['interned_strings_usage']['free_memory'] > 9) ) ) { - $recommendations[] = $this->l10n->t('The OPcache interned strings buffer is nearly full. To assure that repeating strings can be effectively cached, it is recommended to apply opcache.interned_strings_buffer to your PHP configuration with a value higher than %s.', [($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') ?: 'currently')]); + $recommendations[] = $this->l10n->t('The OPcache interned strings buffer is nearly full. To assure that repeating strings can be effectively cached, it is recommended to apply "opcache.interned_strings_buffer" to your PHP configuration with a value higher than "%s".', [($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') ?: 'currently')]); } } From c92fbca6e6e7ab3833113f562a98cf09ded518a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 12 Dec 2023 16:14:38 +0100 Subject: [PATCH 4/5] Fix small psalm issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/settings/lib/SetupChecks/PhpOpcacheSetup.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php b/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php index aa1f2367bf56e..796f9c74df171 100644 --- a/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php +++ b/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php @@ -107,7 +107,7 @@ protected function getOpcacheSetupRecommendations(): array { if ( // Do not recommend to raise the interned strings buffer size above a quarter of the total OPcache size - ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') < $this->iniGetWrapper->getNumeric('opcache.memory_consumption') / 4) && + ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') ?? 0 < $this->iniGetWrapper->getNumeric('opcache.memory_consumption') / 4) && ( empty($status['interned_strings_usage']['free_memory']) || ($status['interned_strings_usage']['used_memory'] / $status['interned_strings_usage']['free_memory'] > 9) From 94d039922956c28484d98669caeea97f89e77adb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 8 Jan 2024 15:07:31 +0100 Subject: [PATCH 5/5] Improve PHP opcache setup check and reduce level in some cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/SetupChecks/PhpOpcacheSetup.php | 60 ++++++++++++------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php b/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php index 796f9c74df171..07e4b5134976c 100644 --- a/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php +++ b/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php @@ -49,47 +49,44 @@ public function getCategory(): string { /** * Checks whether a PHP OPcache is properly set up - * @return string[] The list of OPcache setup recommendations + * @return array{'warning'|'error',list} The level and the list of OPcache setup recommendations */ protected function getOpcacheSetupRecommendations(): array { + $level = 'warning'; + // If the module is not loaded, return directly to skip inapplicable checks if (!extension_loaded('Zend OPcache')) { - return [$this->l10n->t('The PHP OPcache module is not loaded. For better performance it is recommended to load it into your PHP installation.')]; + return ['error',[$this->l10n->t('The PHP OPcache module is not loaded. For better performance it is recommended to load it into your PHP installation.')]]; } $recommendations = []; // Check whether Nextcloud is allowed to use the OPcache API $isPermitted = true; - $permittedPath = $this->iniGetWrapper->getString('opcache.restrict_api'); - if (isset($permittedPath) && $permittedPath !== '' && !str_starts_with(\OC::$SERVERROOT, rtrim($permittedPath, '/'))) { + $permittedPath = (string)$this->iniGetWrapper->getString('opcache.restrict_api'); + if ($permittedPath !== '' && !str_starts_with(\OC::$SERVERROOT, rtrim($permittedPath, '/'))) { $isPermitted = false; } if (!$this->iniGetWrapper->getBool('opcache.enable')) { $recommendations[] = $this->l10n->t('OPcache is disabled. For better performance, it is recommended to apply "opcache.enable=1" to your PHP configuration.'); - - // Check for saved comments only when OPcache is currently disabled. If it was enabled, opcache.save_comments=0 would break Nextcloud in the first place. - if (!$this->iniGetWrapper->getBool('opcache.save_comments')) { - $recommendations[] = $this->l10n->t('OPcache is configured to remove code comments. With OPcache enabled, "opcache.save_comments=1" must be set for Nextcloud to function.'); - } - - if (!$isPermitted) { - $recommendations[] = $this->l10n->t('Nextcloud is not allowed to use the OPcache API. With OPcache enabled, it is highly recommended to include all Nextcloud directories with "opcache.restrict_api" or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.'); - } - } elseif (!$isPermitted) { - $recommendations[] = $this->l10n->t('Nextcloud is not allowed to use the OPcache API. It is highly recommended to include all Nextcloud directories with "opcache.restrict_api" or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.'); + $level = 'error'; } elseif ($this->iniGetWrapper->getBool('opcache.file_cache_only')) { $recommendations[] = $this->l10n->t('The shared memory based OPcache is disabled. For better performance, it is recommended to apply "opcache.file_cache_only=0" to your PHP configuration and use the file cache as second level cache only.'); } else { // Check whether opcache_get_status has been explicitly disabled an in case skip usage based checks $disabledFunctions = $this->iniGetWrapper->getString('disable_functions'); if (isset($disabledFunctions) && str_contains($disabledFunctions, 'opcache_get_status')) { - return []; + return [$level, $recommendations]; } $status = opcache_get_status(false); + if ($status === false) { + $recommendations[] = $this->l10n->t('OPcache is not working as it should, opcache_get_status() returns false, please check configuration.'); + $level = 'error'; + } + // Recommend to raise value, if more than 90% of max value is reached if ( empty($status['opcache_statistics']['max_cached_keys']) || @@ -107,7 +104,7 @@ protected function getOpcacheSetupRecommendations(): array { if ( // Do not recommend to raise the interned strings buffer size above a quarter of the total OPcache size - ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') ?? 0 < $this->iniGetWrapper->getNumeric('opcache.memory_consumption') / 4) && + ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') ?? 0 < $this->iniGetWrapper->getNumeric('opcache.memory_consumption') ?? 0 / 4) && ( empty($status['interned_strings_usage']['free_memory']) || ($status['interned_strings_usage']['used_memory'] / $status['interned_strings_usage']['free_memory'] > 9) @@ -117,16 +114,33 @@ protected function getOpcacheSetupRecommendations(): array { } } - return $recommendations; + // Check for saved comments only when OPcache is currently disabled. If it was enabled, opcache.save_comments=0 would break Nextcloud in the first place. + if (!$this->iniGetWrapper->getBool('opcache.save_comments')) { + $recommendations[] = $this->l10n->t('OPcache is configured to remove code comments. With OPcache enabled, "opcache.save_comments=1" must be set for Nextcloud to function.'); + $level = 'error'; + } + + if (!$isPermitted) { + $recommendations[] = $this->l10n->t('Nextcloud is not allowed to use the OPcache API. With OPcache enabled, it is highly recommended to include all Nextcloud directories with "opcache.restrict_api" or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.'); + $level = 'error'; + } + + return [$level, $recommendations]; } public function run(): SetupResult { - $recommendations = $this->getOpcacheSetupRecommendations(); + [$level,$recommendations] = $this->getOpcacheSetupRecommendations(); if (!empty($recommendations)) { - return SetupResult::error( - $this->l10n->t('The PHP OPcache module is not properly configured. %s.', implode("\n", $recommendations)), - $this->urlGenerator->linkToDocs('admin-php-opcache') - ); + return match($level) { + 'error' => SetupResult::error( + $this->l10n->t('The PHP OPcache module is not properly configured. %s.', implode("\n", $recommendations)), + $this->urlGenerator->linkToDocs('admin-php-opcache') + ), + default => SetupResult::warning( + $this->l10n->t('The PHP OPcache module is not properly configured. %s.', implode("\n", $recommendations)), + $this->urlGenerator->linkToDocs('admin-php-opcache') + ), + }; } else { return SetupResult::success($this->l10n->t('Correctly configured')); }