Merge pull request #10197 from nextcloud/feature/9410/improve-opcache-feedback

Improve opcache feedback

4 files changed, 103 insertions, 11 deletions

diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js
index 75d335043ac..93072981e99 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. <a target="_blank" rel="noreferrer noopener" href="{docLink}">For better performance it is recommended</a> 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 900b9f8fc66..894099bee32 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. <a target="_blank" rel="noreferrer noopener" href="https://example.org/link/to/doc">For better performance it is recommended</a> 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 a301ecb1f66..c706d6e7350 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;
 	}
 
 	/**
@@ -527,6 +525,10 @@ Raw output
 		return $this->config->getSystemValue('mail_smtpmode', 'php') === 'php';
 	}
 
+	protected function hasOpcacheLoaded(): bool {
+		return function_exists('opcache_get_status');
+	}
+
 	/**
 	 * @return DataResponse
 	 */
@@ -555,6 +557,7 @@ Raw output
 				'hasPassedCodeIntegrityCheck' => $this->checker->hasPassedCheck(),
 				'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'),
 				'isOpcacheProperlySetup' => $this->isOpcacheProperlySetup(),
+				'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 057774a45ba..305e2ba22ce 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();
 	}
 
@@ -397,6 +398,10 @@ class CheckSetupControllerTest extends TestCase {
 			->willReturn(true);
 		$this->checkSetupController
 			->expects($this->once())
+			->method('hasOpcacheLoaded')
+			->willReturn(true);
+		$this->checkSetupController
+			->expects($this->once())
 			->method('hasWorkingFileLocking')
 			->willReturn(true);
 		$this->checkSetupController
@@ -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,