From f6f49c77f7d013e5713c964d4d7e3d4b4eedc636 Mon Sep 17 00:00:00 2001 From: Cthulhux Date: Mon, 7 May 2018 20:46:19 +0200 Subject: [PATCH 1/3] opcache module check Improved the speed of isOpcacheProperlySetup() (instant return instead of continuing when we're already failed), added a check for the opcache extension itself. Potentially fixes #9410 Signed-off-by: Morris Jobke --- core/js/setupchecks.js | 13 +++- core/js/tests/specs/setupchecksSpec.js | 74 +++++++++++++++++++- settings/Controller/CheckSetupController.php | 17 +++-- 3 files changed, 93 insertions(+), 11 deletions(-) diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 75d335043a..93072981e9 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -233,7 +233,18 @@ type: OC.SetupChecks.MESSAGE_TYPE_ERROR }); } - if(!data.isOpcacheProperlySetup) { + if(!data.hasOpcacheLoaded) { + messages.push({ + msg: t( + 'core', + 'The PHP OPcache module is not loaded. For better performance it is recommended to load it into your PHP installation.', + { + docLink: data.phpOpcacheDocumentation, + } + ), + type: OC.SetupChecks.MESSAGE_TYPE_INFO + }); + } else if(!data.isOpcacheProperlySetup) { messages.push({ msg: t( 'core', diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 900b9f8fc6..894099bee3 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -103,7 +103,36 @@ describe('OC.SetupChecks tests', function() { it('should return an error if data directory is not protected', function(done) { var async = OC.SetupChecks.checkDataProtected(); - suite.server.requests[0].respond(200, {'Content-Type': 'text/plain'}, ''); + suite.server.requests[0].respond( + 200, + { + 'Content-Type': 'application/json' + }, + JSON.stringify({ + hasFileinfoInstalled: true, + isGetenvServerWorking: true, + isReadOnlyConfig: false, + hasWorkingFileLocking: true, + hasValidTransactionIsolationLevel: true, + suggestedOverwriteCliURL: '', + isUrandomAvailable: true, + serverHasInternetConnection: false, + memcacheDocs: 'https://docs.nextcloud.com/server/go.php?to=admin-performance', + forwardedForHeadersWorking: true, + isCorrectMemcachedPHPModuleInstalled: true, + hasPassedCodeIntegrityCheck: true, + isOpcacheProperlySetup: true, + hasOpcacheLoaded: false, + isSettimelimitAvailable: true, + hasFreeTypeSupport: true, + missingIndexes: [], + outdatedCaches: [], + cronErrors: [], + cronInfo: { + diffInSeconds: 0 + } + }) + ); async.done(function( data, s, x ){ expect(data).toEqual([ @@ -162,6 +191,7 @@ describe('OC.SetupChecks tests', function() { isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, isOpcacheProperlySetup: true, + hasOpcacheLoaded: true, isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], @@ -208,6 +238,7 @@ describe('OC.SetupChecks tests', function() { isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, isOpcacheProperlySetup: true, + hasOpcacheLoaded: true, isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], @@ -255,6 +286,7 @@ describe('OC.SetupChecks tests', function() { isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, isOpcacheProperlySetup: true, + hasOpcacheLoaded: true, isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], @@ -300,6 +332,7 @@ describe('OC.SetupChecks tests', function() { isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, isOpcacheProperlySetup: true, + hasOpcacheLoaded: true, isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], @@ -343,6 +376,7 @@ describe('OC.SetupChecks tests', function() { isCorrectMemcachedPHPModuleInstalled: false, hasPassedCodeIntegrityCheck: true, isOpcacheProperlySetup: true, + hasOpcacheLoaded: true, isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], @@ -386,6 +420,7 @@ describe('OC.SetupChecks tests', function() { isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, isOpcacheProperlySetup: true, + hasOpcacheLoaded: true, isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], @@ -429,6 +464,7 @@ describe('OC.SetupChecks tests', function() { isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, isOpcacheProperlySetup: true, + hasOpcacheLoaded: true, isSettimelimitAvailable: false, hasFreeTypeSupport: true, missingIndexes: [], @@ -493,6 +529,7 @@ describe('OC.SetupChecks tests', function() { isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, isOpcacheProperlySetup: true, + hasOpcacheLoaded: true, isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], @@ -536,6 +573,7 @@ describe('OC.SetupChecks tests', function() { isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, isOpcacheProperlySetup: false, + hasOpcacheLoaded: true, phpOpcacheDocumentation: 'https://example.org/link/to/doc', isSettimelimitAvailable: true, hasFreeTypeSupport: true, @@ -557,6 +595,39 @@ describe('OC.SetupChecks tests', function() { }); }); + 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({ + isUrandomAvailable: true, + securityDocs: 'https://docs.owncloud.org/myDocs.html', + serverHasInternetConnection: true, + isMemcacheConfigured: true, + forwardedForHeadersWorking: true, + isCorrectMemcachedPHPModuleInstalled: true, + hasPassedCodeIntegrityCheck: true, + isOpcacheProperlySetup: true, + hasOpcacheLoaded: false, + phpOpcacheDocumentation: 'https://example.org/link/to/doc', + isSettimelimitAvailable: true, + hasFreeTypeSupport: true + }) + ); + + async.done(function( data, s, x ){ + expect(data).toEqual([{ + msg: 'The PHP OPcache module is not loaded. For better performance it is recommended to load it into your PHP installation.', + type: OC.SetupChecks.MESSAGE_TYPE_INFO + }]); + done(); + }); + }); + it('should return an info if server has no FreeType support', function(done) { var async = OC.SetupChecks.checkSetup(); @@ -580,6 +651,7 @@ describe('OC.SetupChecks tests', function() { isCorrectMemcachedPHPModuleInstalled: true, hasPassedCodeIntegrityCheck: true, isOpcacheProperlySetup: true, + hasOpcacheLoaded: true, phpOpcacheDocumentation: 'https://example.org/link/to/doc', isSettimelimitAvailable: true, hasFreeTypeSupport: false, diff --git a/settings/Controller/CheckSetupController.php b/settings/Controller/CheckSetupController.php index a301ecb1f6..bd493641bf 100644 --- a/settings/Controller/CheckSetupController.php +++ b/settings/Controller/CheckSetupController.php @@ -396,33 +396,31 @@ Raw output protected function isOpcacheProperlySetup() { $iniWrapper = new IniGetWrapper(); - $isOpcacheProperlySetUp = true; - if(!$iniWrapper->getBool('opcache.enable')) { - $isOpcacheProperlySetUp = false; + return false; } if(!$iniWrapper->getBool('opcache.save_comments')) { - $isOpcacheProperlySetUp = false; + return false; } if(!$iniWrapper->getBool('opcache.enable_cli')) { - $isOpcacheProperlySetUp = false; + return false; } if($iniWrapper->getNumeric('opcache.max_accelerated_files') < 10000) { - $isOpcacheProperlySetUp = false; + return false; } if($iniWrapper->getNumeric('opcache.memory_consumption') < 128) { - $isOpcacheProperlySetUp = false; + return false; } if($iniWrapper->getNumeric('opcache.interned_strings_buffer') < 8) { - $isOpcacheProperlySetUp = false; + return false; } - return $isOpcacheProperlySetUp; + return true; } /** @@ -555,6 +553,7 @@ Raw output 'hasPassedCodeIntegrityCheck' => $this->checker->hasPassedCheck(), 'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'), 'isOpcacheProperlySetup' => $this->isOpcacheProperlySetup(), + 'hasOpcacheLoaded' => extension_loaded("opcache"), 'phpOpcacheDocumentation' => $this->urlGenerator->linkToDocs('admin-php-opcache'), 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), 'hasFreeTypeSupport' => $this->hasFreeTypeSupport(), From 7c6c3d0d761f8634d5fa9637d022451b5f17f7bd Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Wed, 11 Jul 2018 15:31:33 +0200 Subject: [PATCH 2/3] Fix tests Signed-off-by: Morris Jobke --- settings/Controller/CheckSetupController.php | 6 +++++- tests/Settings/Controller/CheckSetupControllerTest.php | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/settings/Controller/CheckSetupController.php b/settings/Controller/CheckSetupController.php index bd493641bf..b377e36944 100644 --- a/settings/Controller/CheckSetupController.php +++ b/settings/Controller/CheckSetupController.php @@ -525,6 +525,10 @@ Raw output return $this->config->getSystemValue('mail_smtpmode', 'php') === 'php'; } + protected function hasOpcacheLoaded(): bool { + return extension_loaded('opcache'); + } + /** * @return DataResponse */ @@ -553,7 +557,7 @@ Raw output 'hasPassedCodeIntegrityCheck' => $this->checker->hasPassedCheck(), 'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'), 'isOpcacheProperlySetup' => $this->isOpcacheProperlySetup(), - 'hasOpcacheLoaded' => extension_loaded("opcache"), + 'hasOpcacheLoaded' => $this->hasOpcacheLoaded(), 'phpOpcacheDocumentation' => $this->urlGenerator->linkToDocs('admin-php-opcache'), 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), 'hasFreeTypeSupport' => $this->hasFreeTypeSupport(), diff --git a/tests/Settings/Controller/CheckSetupControllerTest.php b/tests/Settings/Controller/CheckSetupControllerTest.php index 057774a45b..305e2ba22c 100644 --- a/tests/Settings/Controller/CheckSetupControllerTest.php +++ b/tests/Settings/Controller/CheckSetupControllerTest.php @@ -134,6 +134,7 @@ class CheckSetupControllerTest extends TestCase { 'hasMissingIndexes', 'isSqliteUsed', 'isPhpMailerUsed', + 'hasOpcacheLoaded', ])->getMock(); } @@ -395,6 +396,10 @@ 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') @@ -451,6 +456,7 @@ class CheckSetupControllerTest extends TestCase { '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', 'isSettimelimitAvailable' => true, 'hasFreeTypeSupport' => false, From 882aab9d70b877ec69dbc59c5fbbc6eb01266f5b Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Wed, 11 Jul 2018 16:05:31 +0200 Subject: [PATCH 3/3] Fix detection if opcache extension is loaded Signed-off-by: Morris Jobke --- settings/Controller/CheckSetupController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/settings/Controller/CheckSetupController.php b/settings/Controller/CheckSetupController.php index b377e36944..c706d6e735 100644 --- a/settings/Controller/CheckSetupController.php +++ b/settings/Controller/CheckSetupController.php @@ -526,7 +526,7 @@ Raw output } protected function hasOpcacheLoaded(): bool { - return extension_loaded('opcache'); + return function_exists('opcache_get_status'); } /**