Enhance and complement OPcache setup checks

The current OPcache recommendations match the PHP defaults, but the values are much higher than required to run Nextcloud, even with a high number of installed apps. On the other hand, when other applications use the same OPcache instance, the recommended values might not be sufficient. Accurate recommendations need to take into account actual OPcache usage.

With this commit, recommendations are shown to raise the config value if more than 90% of max cache size or number of keys is used.

The checks whether the module is loaded and whether the OPcache is properly configured have been merged into a single function. This allowed to reduce the overhead of OPcache configuration checks when the module is not loaded.

A check has been added whether Nextcloud is permitted to use the OPcache API. Without this, inconsistencies during core or app upgrades may cause errors and OPcache usage cannot be determined for the new usage based checks.

OPcache usage based checks are skipped when Nextcloud is not permitted to use the API.

Signed-off-by: MichaIng <micha@dietpi.com>

apps/settings/lib/Controller/CheckSetupController.php

@@ -25,6 +25,7 @@
  * @author timm2k <timm2k@gmx.de>
  * @author Timo Förster <tfoerster@webfoersterei.de>
  * @author Valdnet <47037905+Valdnet@users.noreply.github.com>
+ * @author MichaIng <micha@dietpi.com>
  *
  * @license AGPL-3.0
  *
@@ -238,7 +239,7 @@ class CheckSetupController extends Controller {
 	}
 
 	/**
-	 * Check if the used  SSL lib is outdated. Older OpenSSL and NSS versions do
+	 * Check if the used SSL lib is outdated. Older OpenSSL and NSS versions do
 	 * have multiple bugs which likely lead to problems in combination with
 	 * functionality required by ownCloud such as SNI.
 	 *
@@ -450,31 +451,61 @@ Raw output
 	}
 
 	/**
-	 * Checks whether a PHP opcache is properly set up
-	 * @return bool
+	 * Checks whether a PHP OPcache is properly set up
+	 * @return string[] The list of OPcache setup recommendations
 	 */
-	protected function isOpcacheProperlySetup() {
-		if (!$this->iniGetWrapper->getBool('opcache.enable')) {
-			return false;
+	protected function getOpcacheSetupRecommendations(): array {
+		// If the module is not loaded, return directly to skip inapplicable checks
+		if (!extension_loaded('Zend OPcache')) {
+			return ['The PHP OPcache module is not loaded. <a target="_blank" rel="noreferrer noopener" class="external" href="' . $this->urlGenerator->linkToDocs('admin-php-opcache') . '">For better performance it is recommended</a> to load it into your PHP installation.'];
 		}
 
-		if (!$this->iniGetWrapper->getBool('opcache.save_comments')) {
-			return false;
-		}
+		$recommendations = [];
 
-		if ($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') < 10000) {
-			return false;
+		// 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, $permittedPath)) {
+			$isPermitted = false;
 		}
 
-		if ($this->iniGetWrapper->getNumeric('opcache.memory_consumption') < 128) {
-			return false;
-		}
+		if (!$this->iniGetWrapper->getBool('opcache.enable')) {
+			$recommendations[] = 'OPcache is disabled. For better performance, it is recommended to apply <code>opcache.enable=1</code> to your PHP configuration.';
 
-		if ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') < 8) {
-			return false;
+			// 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[] = '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[] = '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[] = '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.';
+		} 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 ($status['opcache_statistics']['num_cached_keys'] / $status['opcache_statistics']['max_cached_keys'] > 0.9) {
+				$recommendations[] = 'The maximum number of OPcache keys is nearly exceeded. To assure that all scripts can be hold in cache, it is recommended to apply <code>opcache.max_accelerated_files</code> to your PHP configuration with a value higher than <code>' . ($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') ?: 'currently') . '</code>.';
+			}
+
+			if ($status['memory_usage']['used_memory'] / $status['memory_usage']['free_memory'] > 9) {
+				$recommendations[] = '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>' . ($this->iniGetWrapper->getNumeric('opcache.memory_consumption') ?: 'currently') . '</code>.';
+			}
+
+			if ($status['interned_strings_usage']['used_memory'] / $status['interned_strings_usage']['free_memory'] > 9) {
+				$recommendations[] = '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>' . ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') ?: 'currently') . '</code>.';
+			}
 		}
 
-		return true;
+		return $recommendations;
 	}
 
 	/**
@@ -574,10 +605,6 @@ Raw output
 		return [];
 	}
 
-	protected function hasOpcacheLoaded(): bool {
-		return extension_loaded('Zend OPcache');
-	}
-
 	private function isTemporaryDirectoryWritable(): bool {
 		try {
 			if (!empty($this->tempManager->getTempBaseDir())) {
@@ -791,9 +818,7 @@ Raw output
 				'isCorrectMemcachedPHPModuleInstalled' => $this->isCorrectMemcachedPHPModuleInstalled(),
 				'hasPassedCodeIntegrityCheck' => $this->checker->hasPassedCheck(),
 				'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'),
-				'isOpcacheProperlySetup' => $this->isOpcacheProperlySetup(),
-				'hasOpcacheLoaded' => $this->hasOpcacheLoaded(),
-				'phpOpcacheDocumentation' => $this->urlGenerator->linkToDocs('admin-php-opcache'),
+				'OpcacheSetupRecommendations' => $this->getOpcacheSetupRecommendations(),
 				'isSettimelimitAvailable' => $this->isSettimelimitAvailable(),
 				'hasFreeTypeSupport' => $this->hasFreeTypeSupport(),
 				'missingPrimaryKeys' => $this->hasMissingPrimaryKeys(),

apps/settings/tests/Controller/CheckSetupControllerTest.php

@@ -179,13 +179,12 @@ class CheckSetupControllerTest extends TestCase {
 				'getSuggestedOverwriteCliURL',
 				'getCurlVersion',
 				'isPhpOutdated',
-				'isOpcacheProperlySetup',
+				'getOpcacheSetupRecommendations',
 				'hasFreeTypeSupport',
 				'hasMissingIndexes',
 				'hasMissingPrimaryKeys',
 				'isSqliteUsed',
 				'isPHPMailerUsed',
-				'hasOpcacheLoaded',
 				'getAppDirsWithDifferentOwner',
 				'hasRecommendedPHPModules',
 				'hasBigIntConversionPendingColumns',
@@ -479,8 +478,8 @@ class CheckSetupControllerTest extends TestCase {
 			->willReturn(true);
 		$this->checkSetupController
 			->expects($this->once())
-			->method('isOpcacheProperlySetup')
-			->willReturn(false);
+			->method('getOpcacheSetupRecommendations')
+			->willReturn(['recommendation1', 'recommendation2']);
 		$this->checkSetupController
 			->method('hasFreeTypeSupport')
 			->willReturn(false);
@@ -505,10 +504,6 @@ class CheckSetupControllerTest extends TestCase {
 			->expects($this->once())
 			->method('hasFileinfoInstalled')
 			->willReturn(true);
-		$this->checkSetupController
-			->expects($this->once())
-			->method('hasOpcacheLoaded')
-			->willReturn(true);
 		$this->checkSetupController
 			->expects($this->once())
 			->method('hasWorkingFileLocking')
@@ -624,9 +619,7 @@ class CheckSetupControllerTest extends TestCase {
 				'isCorrectMemcachedPHPModuleInstalled' => true,
 				'hasPassedCodeIntegrityCheck' => true,
 				'codeIntegrityCheckerDocumentation' => 'http://docs.example.org/server/go.php?to=admin-code-integrity',
-				'isOpcacheProperlySetup' => false,
-				'hasOpcacheLoaded' => true,
-				'phpOpcacheDocumentation' => 'http://docs.example.org/server/go.php?to=admin-php-opcache',
+				'OpcacheSetupRecommendations' => ['recommendation1', 'recommendation2'],
 				'isSettimelimitAvailable' => true,
 				'hasFreeTypeSupport' => false,
 				'isSqliteUsed' => false,

core/js/setupchecks.js

@@ -328,18 +328,16 @@
 							type: OC.SetupChecks.MESSAGE_TYPE_ERROR
 						});
 					}
-					if(!data.hasOpcacheLoaded) {
-						messages.push({
-							msg: t('core', 'The PHP OPcache module is not loaded. {linkstart}For better performance it is recommended ↗{linkend} to load it into your PHP installation.')
-								.replace('{linkstart}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + data.phpOpcacheDocumentation + '">')
-								.replace('{linkend}', '</a>'),
-							type: OC.SetupChecks.MESSAGE_TYPE_INFO
+					if(data.OpcacheSetupRecommendations.length > 0) {
+						var listOfOPcacheRecommendations = "";
+						data.OpcacheSetupRecommendations.forEach(function(element){
+							listOfOPcacheRecommendations += "<li>" + element + "</li>";
 						});
-					} else if(!data.isOpcacheProperlySetup) {
 						messages.push({
-							msg: t('core', 'The PHP OPcache module is not properly configured. {linkstart}For better performance it is recommended ↗{linkend} to use the following settings in the <code>php.ini</code>:')
-								.replace('{linkstart}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + data.phpOpcacheDocumentation + '">')
-								.replace('{linkend}', '</a>') + "<pre><code>opcache.enable=1\nopcache.interned_strings_buffer=8\nopcache.max_accelerated_files=10000\nopcache.memory_consumption=128\nopcache.save_comments=1\nopcache.revalidate_freq=1</code></pre>",
+							msg: t(
+								'core',
+								'The PHP OPcache module is not properly configured:'
+							) + "<ul>" + listOfOPcacheRecommendations + "</ul>",
 							type: OC.SetupChecks.MESSAGE_TYPE_INFO
 						});
 					}

core/js/tests/specs/setupchecksSpec.js

@@ -236,8 +236,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -294,8 +293,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -353,8 +351,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -410,8 +407,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -465,8 +461,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: false,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -520,8 +515,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -577,8 +571,7 @@ describe('OC.SetupChecks tests', function() {
 					reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html',
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -632,8 +625,7 @@ describe('OC.SetupChecks tests', function() {
 					reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html',
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: false,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -687,8 +679,7 @@ describe('OC.SetupChecks tests', function() {
 					reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html',
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -763,8 +754,7 @@ describe('OC.SetupChecks tests', function() {
 					phpSupported: {eol: true, version: '5.4.0'},
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -818,8 +808,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: false,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: ['recommendation1', 'recommendation2'],
 					phpOpcacheDocumentation: 'https://example.org/link/to/doc',
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
@@ -844,63 +833,7 @@ describe('OC.SetupChecks tests', function() {
 
 			async.done(function( data, s, x ){
 				expect(data).toEqual([{
-						msg: 'The PHP OPcache module is not properly configured. <a target="_blank" rel="noreferrer noopener" class="external" href="https://example.org/link/to/doc">For better performance it is recommended ↗</a> to use the following settings in the <code>php.ini</code>:' + "<pre><code>opcache.enable=1\nopcache.interned_strings_buffer=8\nopcache.max_accelerated_files=10000\nopcache.memory_consumption=128\nopcache.save_comments=1\nopcache.revalidate_freq=1</code></pre>",
-						type: OC.SetupChecks.MESSAGE_TYPE_INFO
-					}]);
-				done();
-			});
-		});
-
-		it('should return an info if server has no opcache at all', function(done) {
-			var async = OC.SetupChecks.checkSetup();
-
-			suite.server.requests[0].respond(
-				200,
-				{
-					'Content-Type': 'application/json'
-				},
-				JSON.stringify({
-					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
-					isReadOnlyConfig: false,
-					hasWorkingFileLocking: true,
-					hasValidTransactionIsolationLevel: true,
-					suggestedOverwriteCliURL: '',
-					isRandomnessSecure: true,
-					securityDocs: 'https://docs.nextcloud.com/myDocs.html',
-					isFairUseOfFreePushService: true,
-					serverHasInternetConnectionProblems: false,
-					isMemcacheConfigured: true,
-					forwardedForHeadersWorking: true,
-					isCorrectMemcachedPHPModuleInstalled: true,
-					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: false,
-					phpOpcacheDocumentation: 'https://example.org/link/to/doc',
-					isSettimelimitAvailable: true,
-					hasFreeTypeSupport: true,
-					missingIndexes: [],
-					missingPrimaryKeys: [],
-					missingColumns: [],
-					cronErrors: [],
-					cronInfo: {
-						diffInSeconds: 0
-					},
-					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: [],
-					recommendedPHPModules: [],
-					pendingBigIntConversionColumns: [],
-					isMysqlUsedWithoutUTF8MB4: false,
-					isDefaultPhoneRegionSet: true,
-					isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true,
-					reverseProxyGeneratedURL: 'https://server',
-					temporaryDirectoryWritable: true,
-				})
-			);
-
-			async.done(function( data, s, x ){
-				expect(data).toEqual([{
-						msg: 'The PHP OPcache module is not loaded. <a target="_blank" rel="noreferrer noopener" class="external" href="https://example.org/link/to/doc">For better performance it is recommended ↗</a> to load it into your PHP installation.',
+						msg: 'The PHP OPcache module is not properly configured:<ul><li>recommendation1</li><li>recommendation2</li></ul>',
 						type: OC.SetupChecks.MESSAGE_TYPE_INFO
 					}]);
 				done();
@@ -930,8 +863,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					phpOpcacheDocumentation: 'https://example.org/link/to/doc',
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: false,
@@ -986,8 +918,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -1045,8 +976,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -1101,8 +1031,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -1154,8 +1083,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -1209,8 +1137,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],
@@ -1264,8 +1191,7 @@ describe('OC.SetupChecks tests', function() {
 					forwardedForHeadersWorking: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
 					hasPassedCodeIntegrityCheck: true,
-					isOpcacheProperlySetup: true,
-					hasOpcacheLoaded: true,
+					OpcacheSetupRecommendations: [],
 					isSettimelimitAvailable: true,
 					hasFreeTypeSupport: true,
 					missingIndexes: [],