Skip to content

Commit 2a68067

Browse files
MichaIngMichaIng
committed
Enhance OPcache checks
Merge the checks whether the module is loaded and whether the OPcache is properly configured. Reduce the overhead of OPcache configuration checks when the module is not loaded. Add a check 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. Skip OPcache usage based checks when Nextcloud is not permitted to use the API.
1 parent 32245d2 commit 2a68067

3 files changed

Lines changed: 45 additions & 61 deletions

File tree

‎apps/settings/lib/Controller/CheckSetupController.php‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -435,20 +435,36 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse {
435435
* Checks whether a PHP OPcache is properly set up
436436
* @return string[] The list of OPcache setup recommendations
437437
*/
438-
protected function isOpcacheProperlySetup(): array {
438+
protected function getOpcacheSetupRecommendations(): array {
439+
// If the module is not loaded, return directly to skip inapplicable checks
440+
if (!extension_loaded('Zend OPcache')) {
441+
return array('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.');
442+
}
443+
439444
$recommendations = [];
440445

446+
// Check whether Nextcloud is allowed to use the OPcache API
447+
$isPermitted = true;
448+
$permittedPath = $this->iniGetWrapper->getBool('opcache.restrict_api');
449+
if ($permittedPath) && !str_starts_with(\OC::$SERVERROOT, $permittedPath)) {
450+
$isPermitted = false;
451+
}
452+
441453
if (!$this->iniGetWrapper->getBool('opcache.enable')) {
442454
$recommendations[] = 'OPcache is disabled. For better performance, it is recommended to apply <code>opcache.enable=1</code> to your PHP configuration.';
443455

444456
// 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.
445457
if (!$this->iniGetWrapper->getBool('opcache.save_comments')) {
446458
$recommendations[] = 'OPcache is configured to remove code comments. With OPcache enabled, <code>opcache.save_comments=1</code> must be set for Nextcloud to function.';
447459
}
460+
461+
if (!$isPermitted) {
462+
$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.';
463+
}
464+
} elseif (!$isPermitted) {
465+
$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.';
448466
} else {
449-
// Mute error when opcache.restrict_api is set and does not permit Nextcloud to use opcache_get_status(). All below conditions will then return false.
450-
// ToDo: Add a check for opcache.restrict_api, since it can cause issues when Nextcloud cannot evict cached scripts: https://github.com/nextcloud/server/pull/8188
451-
$status = @opcache_get_status(false);
467+
$status = opcache_get_status(false);
452468

453469
// Recommend to raise value, if more than 90% of max value is reached
454470
if ($status['opcache_statistics']['num_cached_keys'] / $status['opcache_statistics']['max_cached_keys'] > 0.9) {
@@ -564,10 +580,6 @@ protected function getCronErrors() {
564580
return [];
565581
}
566582

567-
protected function hasOpcacheLoaded(): bool {
568-
return extension_loaded('Zend OPcache');
569-
}
570-
571583
private function isTemporaryDirectoryWritable(): bool {
572584
try {
573585
if (!empty($this->tempManager->getTempBaseDir())) {
@@ -779,9 +791,7 @@ public function check() {
779791
'isCorrectMemcachedPHPModuleInstalled' => $this->isCorrectMemcachedPHPModuleInstalled(),
780792
'hasPassedCodeIntegrityCheck' => $this->checker->hasPassedCheck(),
781793
'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'),
782-
'isOpcacheProperlySetup' => $this->isOpcacheProperlySetup(),
783-
'hasOpcacheLoaded' => $this->hasOpcacheLoaded(),
784-
'phpOpcacheDocumentation' => $this->urlGenerator->linkToDocs('admin-php-opcache'),
794+
'OpcacheSetupRecommendations' => $this->getOpcacheSetupRecommendations(),
785795
'isSettimelimitAvailable' => $this->isSettimelimitAvailable(),
786796
'hasFreeTypeSupport' => $this->hasFreeTypeSupport(),
787797
'missingPrimaryKeys' => $this->hasMissingPrimaryKeys(),

‎core/js/setupchecks.js‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -322,23 +322,16 @@
322322
type: OC.SetupChecks.MESSAGE_TYPE_ERROR
323323
});
324324
}
325-
if(!data.hasOpcacheLoaded) {
326-
messages.push({
327-
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.')
328-
.replace('{linkstart}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + data.phpOpcacheDocumentation + '">')
329-
.replace('{linkend}', '</a>'),
330-
type: OC.SetupChecks.MESSAGE_TYPE_INFO
331-
});
332-
} else if(data.isOpcacheProperlySetup.length > 0) {
333-
var listOfOPcacheRecommends = "";
334-
data.isOpcacheProperlySetup.forEach(function(element){
335-
listOfOPcacheRecommends += "<li>" + element + "</li>";
325+
if(data.OpcacheSetupRecommendations.length > 0) {
326+
var listOfOPcacheRecommendations = "";
327+
data.OpcacheSetupRecommendations.forEach(function(element){
328+
listOfOPcacheRecommendations += "<li>" + element + "</li>";
336329
});
337330
messages.push({
338331
msg: t(
339332
'core',
340333
'The PHP OPcache module is not properly configured:'
341-
) + "<ul>" + listOfOPcacheRecommends + "</ul>",
334+
) + "<ul>" + listOfOPcacheRecommendations + "</ul>",
342335
type: OC.SetupChecks.MESSAGE_TYPE_INFO
343336
});
344337
}

‎core/js/tests/specs/setupchecksSpec.js‎

Lines changed: 19 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,7 @@ describe('OC.SetupChecks tests', function() {
235235
forwardedForHeadersWorking: true,
236236
isCorrectMemcachedPHPModuleInstalled: true,
237237
hasPassedCodeIntegrityCheck: true,
238-
isOpcacheProperlySetup: [],
239-
hasOpcacheLoaded: true,
238+
OpcacheSetupRecommendations: [],
240239
isSettimelimitAvailable: true,
241240
hasFreeTypeSupport: true,
242241
missingIndexes: [],
@@ -292,8 +291,7 @@ describe('OC.SetupChecks tests', function() {
292291
forwardedForHeadersWorking: true,
293292
isCorrectMemcachedPHPModuleInstalled: true,
294293
hasPassedCodeIntegrityCheck: true,
295-
isOpcacheProperlySetup: [],
296-
hasOpcacheLoaded: true,
294+
OpcacheSetupRecommendations: [],
297295
isSettimelimitAvailable: true,
298296
hasFreeTypeSupport: true,
299297
missingIndexes: [],
@@ -350,8 +348,7 @@ describe('OC.SetupChecks tests', function() {
350348
forwardedForHeadersWorking: true,
351349
isCorrectMemcachedPHPModuleInstalled: true,
352350
hasPassedCodeIntegrityCheck: true,
353-
isOpcacheProperlySetup: [],
354-
hasOpcacheLoaded: true,
351+
OpcacheSetupRecommendations: [],
355352
isSettimelimitAvailable: true,
356353
hasFreeTypeSupport: true,
357354
missingIndexes: [],
@@ -406,8 +403,7 @@ describe('OC.SetupChecks tests', function() {
406403
forwardedForHeadersWorking: true,
407404
isCorrectMemcachedPHPModuleInstalled: true,
408405
hasPassedCodeIntegrityCheck: true,
409-
isOpcacheProperlySetup: [],
410-
hasOpcacheLoaded: true,
406+
OpcacheSetupRecommendations: [],
411407
isSettimelimitAvailable: true,
412408
hasFreeTypeSupport: true,
413409
missingIndexes: [],
@@ -460,8 +456,7 @@ describe('OC.SetupChecks tests', function() {
460456
forwardedForHeadersWorking: true,
461457
isCorrectMemcachedPHPModuleInstalled: false,
462458
hasPassedCodeIntegrityCheck: true,
463-
isOpcacheProperlySetup: [],
464-
hasOpcacheLoaded: true,
459+
OpcacheSetupRecommendations: [],
465460
isSettimelimitAvailable: true,
466461
hasFreeTypeSupport: true,
467462
missingIndexes: [],
@@ -514,8 +509,7 @@ describe('OC.SetupChecks tests', function() {
514509
forwardedForHeadersWorking: true,
515510
isCorrectMemcachedPHPModuleInstalled: true,
516511
hasPassedCodeIntegrityCheck: true,
517-
isOpcacheProperlySetup: [],
518-
hasOpcacheLoaded: true,
512+
OpcacheSetupRecommendations: [],
519513
isSettimelimitAvailable: true,
520514
hasFreeTypeSupport: true,
521515
missingIndexes: [],
@@ -570,8 +564,7 @@ describe('OC.SetupChecks tests', function() {
570564
reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html',
571565
isCorrectMemcachedPHPModuleInstalled: true,
572566
hasPassedCodeIntegrityCheck: true,
573-
isOpcacheProperlySetup: [],
574-
hasOpcacheLoaded: true,
567+
OpcacheSetupRecommendations: [],
575568
isSettimelimitAvailable: true,
576569
hasFreeTypeSupport: true,
577570
missingIndexes: [],
@@ -624,8 +617,7 @@ describe('OC.SetupChecks tests', function() {
624617
reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html',
625618
isCorrectMemcachedPHPModuleInstalled: true,
626619
hasPassedCodeIntegrityCheck: true,
627-
isOpcacheProperlySetup: [],
628-
hasOpcacheLoaded: true,
620+
OpcacheSetupRecommendations: [],
629621
isSettimelimitAvailable: false,
630622
hasFreeTypeSupport: true,
631623
missingIndexes: [],
@@ -678,8 +670,7 @@ describe('OC.SetupChecks tests', function() {
678670
reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html',
679671
isCorrectMemcachedPHPModuleInstalled: true,
680672
hasPassedCodeIntegrityCheck: true,
681-
isOpcacheProperlySetup: [],
682-
hasOpcacheLoaded: true,
673+
OpcacheSetupRecommendations: [],
683674
isSettimelimitAvailable: true,
684675
hasFreeTypeSupport: true,
685676
missingIndexes: [],
@@ -753,8 +744,7 @@ describe('OC.SetupChecks tests', function() {
753744
phpSupported: {eol: true, version: '5.4.0'},
754745
isCorrectMemcachedPHPModuleInstalled: true,
755746
hasPassedCodeIntegrityCheck: true,
756-
isOpcacheProperlySetup: [],
757-
hasOpcacheLoaded: true,
747+
OpcacheSetupRecommendations: [],
758748
isSettimelimitAvailable: true,
759749
hasFreeTypeSupport: true,
760750
missingIndexes: [],
@@ -807,8 +797,7 @@ describe('OC.SetupChecks tests', function() {
807797
forwardedForHeadersWorking: true,
808798
isCorrectMemcachedPHPModuleInstalled: true,
809799
hasPassedCodeIntegrityCheck: true,
810-
isOpcacheProperlySetup: ['recommendation1', 'recommendation2'],
811-
hasOpcacheLoaded: true,
800+
OpcacheSetupRecommendations: ['recommendation1', 'recommendation2'],
812801
phpOpcacheDocumentation: 'https://example.org/link/to/doc',
813802
isSettimelimitAvailable: true,
814803
hasFreeTypeSupport: true,
@@ -862,8 +851,7 @@ describe('OC.SetupChecks tests', function() {
862851
forwardedForHeadersWorking: true,
863852
isCorrectMemcachedPHPModuleInstalled: true,
864853
hasPassedCodeIntegrityCheck: true,
865-
isOpcacheProperlySetup: true,
866-
hasOpcacheLoaded: false,
854+
OpcacheSetupRecommendations: [],
867855
phpOpcacheDocumentation: 'https://example.org/link/to/doc',
868856
isSettimelimitAvailable: true,
869857
hasFreeTypeSupport: true,
@@ -917,8 +905,7 @@ describe('OC.SetupChecks tests', function() {
917905
forwardedForHeadersWorking: true,
918906
isCorrectMemcachedPHPModuleInstalled: true,
919907
hasPassedCodeIntegrityCheck: true,
920-
isOpcacheProperlySetup: true,
921-
hasOpcacheLoaded: true,
908+
OpcacheSetupRecommendations: [],
922909
phpOpcacheDocumentation: 'https://example.org/link/to/doc',
923910
isSettimelimitAvailable: true,
924911
hasFreeTypeSupport: false,
@@ -972,8 +959,7 @@ describe('OC.SetupChecks tests', function() {
972959
forwardedForHeadersWorking: true,
973960
isCorrectMemcachedPHPModuleInstalled: true,
974961
hasPassedCodeIntegrityCheck: true,
975-
isOpcacheProperlySetup: true,
976-
hasOpcacheLoaded: true,
962+
OpcacheSetupRecommendations: [],
977963
isSettimelimitAvailable: true,
978964
hasFreeTypeSupport: true,
979965
missingIndexes: [],
@@ -1030,8 +1016,7 @@ describe('OC.SetupChecks tests', function() {
10301016
forwardedForHeadersWorking: true,
10311017
isCorrectMemcachedPHPModuleInstalled: true,
10321018
hasPassedCodeIntegrityCheck: true,
1033-
isOpcacheProperlySetup: true,
1034-
hasOpcacheLoaded: true,
1019+
OpcacheSetupRecommendations: [],
10351020
isSettimelimitAvailable: true,
10361021
hasFreeTypeSupport: true,
10371022
missingIndexes: [],
@@ -1085,8 +1070,7 @@ describe('OC.SetupChecks tests', function() {
10851070
forwardedForHeadersWorking: true,
10861071
isCorrectMemcachedPHPModuleInstalled: true,
10871072
hasPassedCodeIntegrityCheck: true,
1088-
isOpcacheProperlySetup: true,
1089-
hasOpcacheLoaded: true,
1073+
OpcacheSetupRecommendations: [],
10901074
isSettimelimitAvailable: true,
10911075
hasFreeTypeSupport: true,
10921076
missingIndexes: [],
@@ -1137,8 +1121,7 @@ describe('OC.SetupChecks tests', function() {
11371121
forwardedForHeadersWorking: true,
11381122
isCorrectMemcachedPHPModuleInstalled: true,
11391123
hasPassedCodeIntegrityCheck: true,
1140-
isOpcacheProperlySetup: true,
1141-
hasOpcacheLoaded: true,
1124+
OpcacheSetupRecommendations: [],
11421125
isSettimelimitAvailable: true,
11431126
hasFreeTypeSupport: true,
11441127
missingIndexes: [],
@@ -1191,8 +1174,7 @@ describe('OC.SetupChecks tests', function() {
11911174
forwardedForHeadersWorking: true,
11921175
isCorrectMemcachedPHPModuleInstalled: true,
11931176
hasPassedCodeIntegrityCheck: true,
1194-
isOpcacheProperlySetup: true,
1195-
hasOpcacheLoaded: true,
1177+
OpcacheSetupRecommendations: [],
11961178
isSettimelimitAvailable: true,
11971179
hasFreeTypeSupport: true,
11981180
missingIndexes: [],
@@ -1245,8 +1227,7 @@ describe('OC.SetupChecks tests', function() {
12451227
forwardedForHeadersWorking: true,
12461228
isCorrectMemcachedPHPModuleInstalled: true,
12471229
hasPassedCodeIntegrityCheck: true,
1248-
isOpcacheProperlySetup: true,
1249-
hasOpcacheLoaded: true,
1230+
OpcacheSetupRecommendations: [],
12501231
isSettimelimitAvailable: true,
12511232
hasFreeTypeSupport: true,
12521233
missingIndexes: [],

0 commit comments

Comments
 (0)