Skip to content

Commit

Permalink
Migrate opcache 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 Jan 15, 2024
1 parent ca63726 commit 265f6dc
Show file tree
Hide file tree
Showing 8 changed files with 138 additions and 168 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 @@ -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',
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 @@ -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',
Expand Down
2 changes: 2 additions & 0 deletions apps/settings/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
79 changes: 0 additions & 79 deletions apps/settings/lib/Controller/CheckSetupController.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
*/
namespace OCA\Settings\Controller;

use bantu\IniGetWrapper\IniGetWrapper;
use DirectoryIterator;
use GuzzleHttp\Exception\ClientException;
use OC;
Expand Down Expand Up @@ -91,8 +90,6 @@ class CheckSetupController extends Controller {
private $lockingProvider;
/** @var IDateTimeFormatter */
private $dateTimeFormatter;
/** @var IniGetWrapper */
private $iniGetWrapper;
/** @var ITempManager */
private $tempManager;
/** @var IManager */
Expand All @@ -114,7 +111,6 @@ public function __construct($AppName,
IEventDispatcher $dispatcher,
ILockingProvider $lockingProvider,
IDateTimeFormatter $dateTimeFormatter,
IniGetWrapper $iniGetWrapper,
ITempManager $tempManager,
IManager $manager,
IAppManager $appManager,
Expand All @@ -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;
Expand Down Expand Up @@ -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 <code>opcache.enable=1</code> 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, <code>opcache.save_comments=1</code> 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 <code>opcache.restrict_api</code> 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 <code>opcache.restrict_api</code> 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 <code>opcache.file_cache_only=0</code> 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 <code>opcache.max_accelerated_files</code> to your PHP configuration with a value higher than <code>%s</code>.', [($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 <code>opcache.memory_consumption</code> to your PHP configuration with a value higher than <code>%s</code>.', [($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 <code>opcache.interned_strings_buffer</code> to your PHP configuration with a value higher than <code>%s</code>.', [($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;
Expand Down Expand Up @@ -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(),
Expand Down
134 changes: 134 additions & 0 deletions apps/settings/lib/SetupChecks/PhpOpcacheSetup.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
<?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 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 <code>opcache.enable=1</code> 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, <code>opcache.save_comments=1</code> 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 <code>opcache.restrict_api</code> 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 <code>opcache.restrict_api</code> 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 <code>opcache.file_cache_only=0</code> 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 <code>opcache.max_accelerated_files</code> to your PHP configuration with a value higher than <code>%s</code>.', [($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 <code>opcache.memory_consumption</code> to your PHP configuration with a value higher than <code>%s</code>.', [($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 <code>opcache.interned_strings_buffer</code> to your PHP configuration with a value higher than <code>%s</code>.', [($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'));
}
}
}
14 changes: 0 additions & 14 deletions apps/settings/tests/Controller/CheckSetupControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
*/
namespace OCA\Settings\Tests\Controller;

use bantu\IniGetWrapper\IniGetWrapper;
use OC;
use OC\IntegrityCheck\Checker;
use OCA\Settings\Controller\CheckSetupController;
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand All @@ -152,7 +148,6 @@ protected function setUp(): void {
$this->dispatcher,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->iniGetWrapper,
$this->tempManager,
$this->notificationManager,
$this->appManager,
Expand All @@ -164,7 +159,6 @@ protected function setUp(): void {
'getSuggestedOverwriteCliURL',
'getCurlVersion',
'isPhpOutdated',
'getOpcacheSetupRecommendations',
'isPHPMailerUsed',
'getAppDirsWithDifferentOwner',
'isImagickEnabled',
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -330,7 +319,6 @@ public function testGetCurlVersion() {
$this->dispatcher,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->iniGetWrapper,
$this->tempManager,
$this->notificationManager,
$this->appManager,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1105,7 +1092,6 @@ public function testIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(string $m
$this->dispatcher,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->iniGetWrapper,
$this->tempManager,
$this->notificationManager,
$this->appManager,
Expand Down

0 comments on commit 265f6dc

Please sign in to comment.