From f5ad80fc5774934b5f05c3538cd12218f2b17d21 Mon Sep 17 00:00:00 2001
From: Morris Jobke <hey@morrisjobke.de>
Date: Tue, 4 Dec 2018 17:05:52 +0100
Subject: [PATCH 1/2] Add setup check for recommended PHP modules (i.e.
 Imagick, intl)

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
---
 core/js/setupchecks.js                        | 13 +++++++
 core/js/tests/specs/setupchecksSpec.js        | 37 +++++++++++++------
 settings/Controller/CheckSetupController.php  | 22 +++++++++++
 .../Controller/CheckSetupControllerTest.php   |  7 ++++
 4 files changed, 67 insertions(+), 12 deletions(-)

diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js
index c50781f148..97c28a69f6 100644
--- a/core/js/setupchecks.js
+++ b/core/js/setupchecks.js
@@ -324,6 +324,19 @@
 							type: OC.SetupChecks.MESSAGE_TYPE_INFO
 						})
 					}
+					if (data.recommendedPHPModules.length > 0) {
+						var listOfRecommendedPHPModules = "";
+						data.recommendedPHPModules.forEach(function(element){
+							listOfRecommendedPHPModules += "<li>" + element + "</li>";
+						});
+						messages.push({
+							msg: t(
+								'core',
+								'This instance is missing some recommended PHP modules. For improved performance and better compatibility it is highly recommended to install them.'
+							) + "<ul><code>" + listOfRecommendedPHPModules + "</code></ul>",
+							type: OC.SetupChecks.MESSAGE_TYPE_INFO
+						})
+					}
 					if (data.isSqliteUsed) {
 						messages.push({
 							msg: t(
diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js
index b8a2164de5..b6b127bae5 100644
--- a/core/js/tests/specs/setupchecksSpec.js
+++ b/core/js/tests/specs/setupchecksSpec.js
@@ -210,7 +210,8 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: []
+					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: []
 				})
 			);
 
@@ -259,7 +260,8 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: []
+					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: []
 				})
 			);
 
@@ -309,7 +311,8 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: []
+					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: []
 				})
 			);
 
@@ -357,7 +360,8 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: []
+					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: []
 				})
 			);
 
@@ -403,7 +407,8 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: []
+					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: []
 				})
 			);
 
@@ -451,7 +456,8 @@ describe('OC.SetupChecks tests', function() {
 					isMemoryLimitSufficient: true,
 					appDirsWithDifferentOwner: [
 						'/some/path'
-					]
+					],
+					recommendedPHPModules: []
 				})
 			);
 
@@ -497,7 +503,8 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: []
+					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: []
 				})
 			);
 
@@ -543,7 +550,8 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: []
+					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: []
 				})
 			);
 
@@ -589,6 +597,7 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: [],
 					isMemoryLimitSufficient: false
 				})
 			);
@@ -656,7 +665,8 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: []
+					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: []
 				})
 			);
 
@@ -703,7 +713,8 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: []
+					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: []
 				})
 			);
 
@@ -750,7 +761,8 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: []
+					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: []
 				})
 			);
 
@@ -797,7 +809,8 @@ describe('OC.SetupChecks tests', function() {
 						diffInSeconds: 0
 					},
 					isMemoryLimitSufficient: true,
-					appDirsWithDifferentOwner: []
+					appDirsWithDifferentOwner: [],
+					recommendedPHPModules: []
 				})
 			);
 
diff --git a/settings/Controller/CheckSetupController.php b/settings/Controller/CheckSetupController.php
index 5da84eca68..d757c017e1 100644
--- a/settings/Controller/CheckSetupController.php
+++ b/settings/Controller/CheckSetupController.php
@@ -582,6 +582,27 @@ Raw output
 		return $appDirsWithDifferentOwner;
 	}
 
+	/**
+	 * Checks for potential PHP modules that would improve the instance
+	 *
+	 * @return string[] A list of PHP modules that is recommended
+	 */
+	protected function hasRecommendedPHPModules(): array {
+		$recommendedPHPModules = [];
+
+		if (!function_exists('grapheme_strlen')) {
+			$recommendedPHPModules[] = 'intl';
+		}
+
+		if ($this->config->getAppValue('theming', 'enabled', 'no') === 'yes') {
+			if (!extension_loaded('imagick')) {
+				$recommendedPHPModules[] = 'imagick';
+			}
+		}
+
+		return $recommendedPHPModules;
+	}
+
 	/**
 	 * @return DataResponse
 	 */
@@ -621,6 +642,7 @@ Raw output
 				'mailSettingsDocumentation' => $this->urlGenerator->getAbsoluteURL('index.php/settings/admin'),
 				'isMemoryLimitSufficient' => $this->memoryInfo->isMemoryLimitSufficient(),
 				'appDirsWithDifferentOwner' => $this->getAppDirsWithDifferentOwner(),
+				'recommendedPHPModules' => $this->hasRecommendedPHPModules(),
 			]
 		);
 	}
diff --git a/tests/Settings/Controller/CheckSetupControllerTest.php b/tests/Settings/Controller/CheckSetupControllerTest.php
index 7731e08eed..e85750f6b1 100644
--- a/tests/Settings/Controller/CheckSetupControllerTest.php
+++ b/tests/Settings/Controller/CheckSetupControllerTest.php
@@ -158,6 +158,7 @@ class CheckSetupControllerTest extends TestCase {
 				'isPhpMailerUsed',
 				'hasOpcacheLoaded',
 				'getAppDirsWithDifferentOwner',
+				'hasRecommendedPHPModules',
 			])->getMock();
 	}
 
@@ -487,6 +488,11 @@ class CheckSetupControllerTest extends TestCase {
 			->method('getAppDirsWithDifferentOwner')
 			->willReturn([]);
 
+		$this->checkSetupController
+			->expects($this->once())
+			->method('hasRecommendedPHPModules')
+			->willReturn([]);
+
 		$expected = new DataResponse(
 			[
 				'isGetenvServerWorking' => true,
@@ -529,6 +535,7 @@ class CheckSetupControllerTest extends TestCase {
 				'mailSettingsDocumentation' => 'https://server/index.php/settings/admin',
 				'isMemoryLimitSufficient' => true,
 				'appDirsWithDifferentOwner' => [],
+				'recommendedPHPModules' => [],
 			]
 		);
 		$this->assertEquals($expected, $this->checkSetupController->check());

From 5b2222535173312b681ca4ab0de1d01ae28c913b Mon Sep 17 00:00:00 2001
From: Morris Jobke <hey@morrisjobke.de>
Date: Tue, 4 Dec 2018 17:58:35 +0100
Subject: [PATCH 2/2] Update casing of PHP inside method name

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
---
 core/js/setupchecks.js                               |  2 +-
 settings/Controller/CheckSetupController.php         |  4 ++--
 .../Settings/Controller/CheckSetupControllerTest.php | 12 ++++++------
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js
index 97c28a69f6..222b12b8f4 100644
--- a/core/js/setupchecks.js
+++ b/core/js/setupchecks.js
@@ -353,7 +353,7 @@
 							type: OC.SetupChecks.MESSAGE_TYPE_WARNING
 						})
 					}
-					if (data.isPhpMailerUsed) {
+					if (data.isPHPMailerUsed) {
 						messages.push({
 							msg: t(
 								'core',
diff --git a/settings/Controller/CheckSetupController.php b/settings/Controller/CheckSetupController.php
index d757c017e1..45690e986f 100644
--- a/settings/Controller/CheckSetupController.php
+++ b/settings/Controller/CheckSetupController.php
@@ -527,7 +527,7 @@ Raw output
 		return [];
 	}
 
-	protected function isPhpMailerUsed(): bool {
+	protected function isPHPMailerUsed(): bool {
 		return $this->config->getSystemValue('mail_smtpmode', 'smtp') === 'php';
 	}
 
@@ -638,7 +638,7 @@ Raw output
 				'missingIndexes' => $this->hasMissingIndexes(),
 				'isSqliteUsed' => $this->isSqliteUsed(),
 				'databaseConversionDocumentation' => $this->urlGenerator->linkToDocs('admin-db-conversion'),
-				'isPhpMailerUsed' => $this->isPhpMailerUsed(),
+				'isPHPMailerUsed' => $this->isPHPMailerUsed(),
 				'mailSettingsDocumentation' => $this->urlGenerator->getAbsoluteURL('index.php/settings/admin'),
 				'isMemoryLimitSufficient' => $this->memoryInfo->isMemoryLimitSufficient(),
 				'appDirsWithDifferentOwner' => $this->getAppDirsWithDifferentOwner(),
diff --git a/tests/Settings/Controller/CheckSetupControllerTest.php b/tests/Settings/Controller/CheckSetupControllerTest.php
index e85750f6b1..360bd27f25 100644
--- a/tests/Settings/Controller/CheckSetupControllerTest.php
+++ b/tests/Settings/Controller/CheckSetupControllerTest.php
@@ -155,7 +155,7 @@ class CheckSetupControllerTest extends TestCase {
 				'hasFreeTypeSupport',
 				'hasMissingIndexes',
 				'isSqliteUsed',
-				'isPhpMailerUsed',
+				'isPHPMailerUsed',
 				'hasOpcacheLoaded',
 				'getAppDirsWithDifferentOwner',
 				'hasRecommendedPHPModules',
@@ -473,7 +473,7 @@ class CheckSetupControllerTest extends TestCase {
 			]);
 		$this->checkSetupController
 			->expects($this->once())
-			->method('isPhpMailerUsed')
+			->method('isPHPMailerUsed')
 			->willReturn(false);
 		$this->checker
 			->expects($this->once())
@@ -531,7 +531,7 @@ class CheckSetupControllerTest extends TestCase {
 				'isSqliteUsed' => false,
 				'databaseConversionDocumentation' => 'http://docs.example.org/server/go.php?to=admin-db-conversion',
 				'missingIndexes' => [],
-				'isPhpMailerUsed' => false,
+				'isPHPMailerUsed' => false,
 				'mailSettingsDocumentation' => 'https://server/index.php/settings/admin',
 				'isMemoryLimitSufficient' => true,
 				'appDirsWithDifferentOwner' => [],
@@ -541,7 +541,7 @@ class CheckSetupControllerTest extends TestCase {
 		$this->assertEquals($expected, $this->checkSetupController->check());
 	}
 
-	public function testIsPhpMailerUsed() {
+	public function testIsPHPMailerUsed() {
 		$checkSetupController = $this->getMockBuilder('\OC\Settings\Controller\CheckSetupController')
 			->setConstructorArgs([
 				'settings',
@@ -571,8 +571,8 @@ class CheckSetupControllerTest extends TestCase {
 			->with('mail_smtpmode', 'smtp')
 			->will($this->returnValue('not-php'));
 
-		$this->assertTrue($this->invokePrivate($checkSetupController, 'isPhpMailerUsed'));
-		$this->assertFalse($this->invokePrivate($checkSetupController, 'isPhpMailerUsed'));
+		$this->assertTrue($this->invokePrivate($checkSetupController, 'isPHPMailerUsed'));
+		$this->assertFalse($this->invokePrivate($checkSetupController, 'isPHPMailerUsed'));
 	}
 
 	public function testGetCurlVersion() {