Refactor to define an object of ConfigureCheck

lib/Command/Configure/Check.php

@@ -7,6 +7,8 @@ namespace OCA\Libresign\Command\Configure;
 use OC\Core\Command\Base;
 use OCA\Libresign\Service\ConfigureCheckService;
 use Symfony\Component\Console\Helper\Table;
+use Symfony\Component\Console\Helper\TableCell;
+use Symfony\Component\Console\Helper\TableCellStyle;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -52,31 +54,37 @@ class Check extends Base {
 			$result = $this->configureCheckService->checkAll();
 		} else {
 			if ($preview) {
-				$result = array_merge_recursive($result, $this->configureCheckService->canPreview());
+				$result = array_merge($result, $this->configureCheckService->canPreview());
 			}
 			if ($sign) {
-				$result = array_merge_recursive($result, $this->configureCheckService->checkSign());
+				$result = array_merge($result, $this->configureCheckService->checkSign());
 			}
 			if ($cfssl) {
-				$result = array_merge_recursive($result, $this->configureCheckService->checkCfssl());
+				$result = array_merge($result, $this->configureCheckService->checkCfssl());
 			}
 		}
 
-		$table = new Table($output);
 		if (count($result)) {
-			if (array_key_exists('errors', $result)) {
-				foreach ($result['errors'] as $error) {
-					$table->addRow(['🔴', $error]);
-				}
-			}
-			if (array_key_exists('success', $result)) {
-				foreach ($result['success'] as $success) {
-					$table->addRow(['🟢', $success]);
-				}
+			$table = new Table($output);
+			foreach ($result as $row) {
+				$table->addRow([
+					new TableCell($row->getStatus(), ['style' => new TableCellStyle([
+						'bg' => $row->getStatus() === 'success' ? 'green' : 'red',
+						'align' => 'center',
+					])]),
+					$row->getResource(),
+					$row->getMessage(),
+					$row->getTip(),
+				]);
 			}
 			$table
-				->setHeaders(['Status', 'Description'])
-				->setStyle('compact')
+				->setHeaders([
+					'Status',
+					'Resource',
+					'Message',
+					'Tip',
+				])
+				->setStyle('symfony-style-guide')
 				->render();
 		}
 		return 0;

lib/Handler/CfsslHandler.php

@@ -4,8 +4,10 @@ namespace OCA\Libresign\Handler;
 
 use GuzzleHttp\Client;
 use GuzzleHttp\ClientInterface;
-use GuzzleHttp\Exception\TransferException;
+use GuzzleHttp\Exception\ConnectException;
+use GuzzleHttp\Exception\RequestException;
 use OCA\Libresign\Exception\LibresignException;
+use OCA\Libresign\Helper\MagicGetterSetterTrait;
 
 /**
  * Class FileMapper
@@ -34,6 +36,7 @@ use OCA\Libresign\Exception\LibresignException;
  * @method CfsslHandler setConfigPath()
  */
 class CfsslHandler {
+	use MagicGetterSetterTrait;
 	public const CFSSL_URI = 'http://127.0.0.1:8888/api/v1/cfssl/';
 	private $commonName;
 	private $hosts = [];
@@ -47,20 +50,6 @@ class CfsslHandler {
 	private $binary;
 	/** @var ClientInterface */
 	private $client;
-	public function __call($name, $arguments) {
-		if (!preg_match('/^(?<type>get|set)(?<property>.+)/', $name, $matches)) {
-			throw new \LogicException(sprintf('Cannot set non existing property %s->%s = %s.', \get_class($this), $name, var_export($arguments, true)));
-		}
-		$property = lcfirst($matches['property']);
-		if (!property_exists($this, $property)) {
-			throw new \LogicException(sprintf('Cannot set non existing property %s->%s = %s.', \get_class($this), $name, var_export($arguments, true)));
-		}
-		if ($matches['type'] === 'get') {
-			return $this->$property;
-		}
-		$this->$property = $arguments[0] ?? null;
-		return $this;
-	}
 
 	public function getClient(): ClientInterface {
 		if (!$this->client) {
@@ -121,7 +110,7 @@ class CfsslHandler {
 					$json
 				)
 			;
-		} catch (TransferException $th) {
+		} catch (RequestException | ConnectException $th) {
 			if ($th->getHandlerContext() && $th->getHandlerContext()['error']) {
 				throw new \Exception($th->getHandlerContext()['error'], 1);
 			}
@@ -158,7 +147,7 @@ class CfsslHandler {
 					]
 				)
 			;
-		} catch (TransferException $th) {
+		} catch (RequestException | ConnectException $th) {
 			switch ($th->getCode()) {
 				case 404:
 					throw new \Exception('Endpoint /health of CFSSL server not found. Maybe you are using incompatible version of CFSSL server. Use latests version.', 1);

lib/Helper/ConfigureCheckHelper.php

@@ -0,0 +1,45 @@
+<?php
+
+namespace OCA\Libresign\Helper;
+
+use JsonSerializable;
+
+/**
+ * @method
+ * @method ConfigureCheckHelper setStatus(string $value)
+ * @method string getStatus()
+ * @method ConfigureCheckHelper setMessage(string $value)
+ * @method string getMessage()
+ * @method ConfigureCheckHelper setResource(string $value)
+ * @method string getResource()
+ * @method ConfigureCheckHelper setTip(string $value)
+ * @method string getTip()
+ */
+class ConfigureCheckHelper implements JsonSerializable {
+	use MagicGetterSetterTrait;
+	private string $status = '';
+	private string $message = '';
+	private string $resource = '';
+	private string $tip = '';
+
+	public function setErrorMessage(string $message): ConfigureCheckHelper {
+		$this->setStatus('error');
+		$this->setMessage($message);
+		return $this;
+	}
+
+	public function setSuccessMessage(string $message): ConfigureCheckHelper {
+		$this->setStatus('success');
+		$this->setMessage($message);
+		return $this;
+	}
+
+	public function jsonSerialize(): array {
+		return [
+			'status' => $this->getStatus(),
+			'message' => $this->getMessage(),
+			'resource' => $this->getResource(),
+			'tip' => $this->getTip(),
+		];
+	}
+}

lib/Helper/MagicGetterSetterTrait.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace OCA\Libresign\Helper;
+
+trait MagicGetterSetterTrait {
+	public function __call($name, $arguments) {
+		if (!preg_match('/^(?<type>get|set)(?<property>.+)/', $name, $matches)) {
+			throw new \LogicException(sprintf('Cannot set non existing property %s->%s = %s.', \get_class($this), $name, var_export($arguments, true)));
+		}
+		$property = lcfirst($matches['property']);
+		if (!property_exists($this, $property)) {
+			throw new \LogicException(sprintf('Cannot set non existing property %s->%s = %s.', \get_class($this), $name, var_export($arguments, true)));
+		}
+		if ($matches['type'] === 'get') {
+			return $this->$property;
+		}
+		$this->$property = $arguments[0] ?? null;
+		return $this;
+	}
+}

lib/Service/ConfigureCheckService.php

@@ -6,6 +6,7 @@ use ImagickException;
 use OC\SystemConfig;
 use OCA\Libresign\AppInfo\Application;
 use OCA\Libresign\Handler\JSignPdfHandler;
+use OCA\Libresign\Helper\ConfigureCheckHelper;
 use OCP\IConfig;
 
 class ConfigureCheckService {
@@ -20,18 +21,44 @@ class ConfigureCheckService {
 		$this->systemConfig = $systemConfig;
 	}
 
+	/**
+	 * Get result of all checks
+	 *
+	 * @return ConfigureCheckHelper[]
+	 */
 	public function checkAll(): array {
 		$result = [];
-		$result = array_merge_recursive($result, $this->canPreview());
-		$result = array_merge_recursive($result, $this->checkSign());
-		$result = array_merge_recursive($result, $this->checkCfssl());
+		$result = array_merge($result, $this->checkSign());
+		$result = array_merge($result, $this->canPreview());
+		$result = array_merge($result, $this->checkCfssl());
 		return $result;
 	}
 
+	/**
+	 * Check all requirements to sign
+	 *
+	 * @return ConfigureCheckHelper[]
+	 */
+	public function checkSign(): array {
+		$return = [];
+		$return = array_merge($return, $this->checkJava());
+		$return = array_merge($return, $this->checkJSignPdf());
+		$return = array_merge($return, $this->checkLibresignCli());
+		return $return;
+	}
+
+	/**
+	 * Can preview PDF Files
+	 *
+	 * @return ConfigureCheckHelper[]
+	 */
 	public function canPreview(): array {
 		if (!extension_loaded('imagick')) {
 			return [
-				'errors' => ['Extension Imagick required'],
+				(new ConfigureCheckHelper())
+					->setErrorMessage('Extension Imagick required')
+					->setResource('imagick')
+					->setTip('https://github.com/LibreSign/libresign/issues/829'),
 			];
 		}
 
@@ -43,76 +70,140 @@ class ConfigureCheckService {
 		} catch (ImagickException $ie) {
 			if ($ie->getCode() === 499) {
 				return [
-					'errors' => ['Is necessary to configure the ImageMagick security policy to work with PDF. More information on: https://github.com/LibreSign/libresign/issues/829'],
+					(new ConfigureCheckHelper())
+						->setErrorMessage('Is necessary to configure the ImageMagick security policy to work with PDF.')
+						->setResource('imagick')
+						->setTip('https://github.com/LibreSign/libresign/issues/829'),
 				];
 			}
 		}
 		return [
-			'success' => ['Can generate the preview'],
+			(new ConfigureCheckHelper())
+				->setSuccessMessage('Can generate the preview')
+				->setResource('imagick'),
 		];
 	}
 
-	public function checkSign(): array {
-		$return = [];
-		$return = array_merge_recursive($return, $this->checkJava());
-		$return = array_merge_recursive($return, $this->checkJSignPdf());
-		$return = array_merge_recursive($return, $this->checkLibresignCli());
-		return $return;
-	}
-
+	/**
+	 * Check all requirements to use JSignPdf
+	 *
+	 * @return ConfigureCheckHelper[]
+	 */
 	public function checkJSignPdf(): array {
 		$jsignpdJarPath = $this->config->getAppValue(Application::APP_ID, 'jsignpdf_jar_path');
 		if ($jsignpdJarPath) {
 			if (file_exists($jsignpdJarPath)) {
 				return [
-					'success' => [
-						'JSignPdf version: ' . JSignPdfHandler::VERSION,
-						'JSignPdf path: ' . $jsignpdJarPath,
-					]
+					(new ConfigureCheckHelper())
+						->setSuccessMessage('JSignPdf version: ' . JSignPdfHandler::VERSION)
+						->setResource('jsignpdf'),
+					(new ConfigureCheckHelper())
+						->setSuccessMessage('JSignPdf path: ' . $jsignpdJarPath)
+						->setResource('jsignpdf'),
 				];
 			}
-			return ['errors' => ['JSignPdf binary not found: ' . $jsignpdJarPath . ' run occ libresign:install --jsignpdf']];
+			return [
+				(new ConfigureCheckHelper())
+					->setErrorMessage('JSignPdf binary not found: ' . $jsignpdJarPath)
+					->setResource('jsignpdf')
+					->setTip('run occ libresign:install --jsignpdf'),
+			];
 		}
-		return ['errors' => ['JSignPdf not found. run occ libresign:install --jsignpdf']];
+		return [
+			(new ConfigureCheckHelper())
+				->setErrorMessage('JSignPdf not found')
+				->setResource('jsignpdf')
+				->setTip('run occ libresign:install --jsignpdf'),
+		];
 	}
 
+	/**
+	 * Check all requirements to use Java
+	 *
+	 * @return ConfigureCheckHelper[]
+	 */
 	private function checkJava(): array {
 		$javaPath = $this->config->getAppValue(Application::APP_ID, 'java_path');
 		if ($javaPath) {
 			if (file_exists($javaPath)) {
 				$javaVersion = exec($javaPath . " -version 2>&1");
 				return [
-					'success' => [
-						'Java version: ' . $javaVersion,
-						'Java binary: ' . $javaPath,
-					]
+					(new ConfigureCheckHelper())
+						->setSuccessMessage('Java version: ' . $javaVersion)
+						->setResource('java'),
+					(new ConfigureCheckHelper())
+						->setSuccessMessage('Java binary: ' . $javaPath)
+						->setResource('java'),
 				];
 			}
-			return ['errors' => ['Java binary not found: ' . $javaPath . ' run occ libresign:install --java']];
+			return [
+				(new ConfigureCheckHelper())
+					->setErrorMessage('Java binary not found: ' . $javaPath)
+					->setResource('java')
+					->setTip('run occ libresign:install --java'),
+			];
 		}
 		$javaVersion = exec("java -version 2>&1");
 		$hasJavaVersion = strpos($javaVersion, 'not found') === false;
 		if ($hasJavaVersion) {
-			return ['success' => ['Using java from operational system. Version: ' . $javaVersion]];
+			return [
+				(new ConfigureCheckHelper())
+					->setSuccessMessage('Using java from operational system. Version: ' . $javaVersion)
+					->setResource('java')
+					->setTip('run occ libresign:install --java'),
+			];
 		}
-		return ['errors' => ['Java not installed.']];
+		return [
+			(new ConfigureCheckHelper())
+				->setErrorMessage('Java not installed')
+				->setResource('java')
+				->setTip('run occ libresign:install --java'),
+		];
 	}
 
+	/**
+	 * Check all requirements to use LibreSign CLI tool
+	 *
+	 * @return ConfigureCheckHelper[]
+	 */
 	private function checkLibresignCli(): array {
 		$path = $this->config->getAppValue(Application::APP_ID, 'libresign_cli_path');
 		if (!file_exists($path) || !is_executable($path)) {
-			return ['errors' => ['LibreSign cli tools not found or without execute permission. Run occ libresign:install --cli']];
+			return [
+				(new ConfigureCheckHelper())
+					->setErrorMessage('LibreSign cli tools not found or without execute permission.')
+					->setResource('libresign-cli')
+					->setTip('Run occ libresign:install --cli'),
+			];
 		}
-		return ['success' => ['LibreSign cli tools found in path: ' . $path]];
+		return [
+			(new ConfigureCheckHelper())
+				->setSuccessMessage('LibreSign cli tools found in path: ' . $path)
+				->setResource('libresign-cli'),
+		];
 	}
 
+	/**
+	 * Check all requirements to use CFSSL
+	 *
+	 * @return ConfigureCheckHelper[]
+	 */
 	public function checkCfssl(): array {
 		if (PHP_OS_FAMILY === 'Windows') {
-			return ['errors' => ['CFSSL is incompatible with Windows']];
+			return [
+				(new ConfigureCheckHelper())
+					->setErrorMessage('CFSSL is incompatible with Windows')
+					->setResource('cfssl'),
+			];
 		}
 		$cfsslInstalled = $this->config->getAppValue(Application::APP_ID, 'cfssl_bin');
 		if (!$cfsslInstalled) {
-			return ['errors' => ['CFSSL not installed. Run occ libresign:install --cfssl']];
+			return [
+				(new ConfigureCheckHelper())
+					->setErrorMessage('CFSSL not installed.')
+					->setResource('cfssl')
+					->setTip('Run occ libresign:install --cfssl'),
+			];
 		}
 
 		$instanceId = $this->systemConfig->getValue('instanceid', null);
@@ -121,17 +212,27 @@ class ConfigureCheckService {
 			Application::APP_ID . DIRECTORY_SEPARATOR .
 			'cfssl';
 		if (!file_exists($binary)) {
-			return ['errors' => ['CFSSL not found. Run occ libresign:install --cfssl']];
+			return [
+				(new ConfigureCheckHelper())
+					->setErrorMessage('CFSSL not found.')
+					->setResource('cfssl')
+					->setTip('Run occ libresign:install --cfssl'),
+			];
 		}
-		$return = ['success' => ['CFSSL binary path: ' . $binary]];
+		$return = [];
+		$return[] = (new ConfigureCheckHelper())
+			->setSuccessMessage('CFSSL binary path: ' . $binary)
+			->setResource('cfssl');
 		$version = str_replace("\n", ', ', trim(`$binary version`));
-		$return = ['success' => ['CFSSL: ' . $version]];
+		$return[] = (new ConfigureCheckHelper())
+			->setSuccessMessage('CFSSL: ' . $version)
+			->setResource('cfssl');
 		$configPath = $this->config->getAppValue(Application::APP_ID, 'configPath');
 		if (!is_dir($configPath)) {
-			$return = array_merge_recursive(
-				$return,
-				['errors' => ['CFSSL not configured. Run occ libresign:configure --cfssl']]
-			);
+			$return[] = (new ConfigureCheckHelper())
+				->setErrorMessage('CFSSL not configured.')
+				->setResource('cfssl')
+				->setTip('Run occ libresign:configure --cfssl');
 		}
 		return $return;
 	}

src/views/Settings/ConfigureCheck.vue

@@ -1,23 +1,25 @@
 <template>
 	<SettingsSection :title="title" :description="description">
-		<div class="settings-section">
-			<ul>
-				<li v-if="items.success">
-					<ul class="items">
-						<li v-for="(item, index) in items.success" :key="index" class="success">
-							{{ item }}
-						</li>
-					</ul>
-				</li>
-				<li v-if="items.errors">
-					<ul class="items">
-						<li v-for="(item, index) in items.errors" :key="index" class="error">
-							{{ item }}
-						</li>
-					</ul>
-				</li>
-			</ul>
-		</div>
+		<table class="grid">
+			<tbody>
+				<tr class="group-header">
+					<th>{{ t('libresign', 'Status') }}</th>
+					<th>{{ t('libresign', 'Resource') }}</th>
+					<th>{{ t('libresign', 'Message') }}</th>
+					<th>{{ t('libresign', 'Tip') }}</th>
+				</tr>
+				<tr v-for="(row, index) in items" :key="index">
+					<td
+						:class=row.status
+						>
+						{{row.status}}
+					</td>
+					<td>{{row.message}}</td>
+					<td>{{row.resource}}</td>
+					<td>{{row.tip}}</td>
+				</tr>
+			</tbody>
+		</table>
 	</SettingsSection>
 </template>
 <script>
@@ -51,17 +53,13 @@ export default {
 }
 </script>
 <style lang="scss" scoped>
-.items {
-	li {
-		list-style: initial;
-		list-style-type: initial;
-		list-style-position: inside;
-	}
-	.success {
-		color: green;
-	}
-	.error {
-		color: red;
-	}
+table {
+	white-space: inherit;
+}
+.success {
+	color: green;
+}
+.error {
+	color: red;
 }
 </style>

tests/Unit/Helper/ConfigureCheckHelperTest.php

@@ -0,0 +1,96 @@
+<?php
+
+namespace OCA\Libresign\Tests\Unit\Helper;
+
+use OCA\Libresign\Helper\ConfigureCheckHelper;
+use OCA\Libresign\Tests\Unit\TestCase;
+
+class ConfigureCheckHelperTest extends TestCase {
+	/** @var ConfigureCheckHelper */
+	private $helper;
+
+	public function setUp(): void {
+		parent::setUp();
+		$this->helper = new ConfigureCheckHelper();
+	}
+
+	/**
+	 * @dataProvider providerJsonSerialize()
+	 */
+	public function testJsonSerialize(string $type, string $message, string $resource, string $tip, array $expected): void {
+		if ($type === 'success') {
+			$this->helper->setSuccessMessage($message);
+		} elseif ($type === 'error') {
+			$this->helper->setErrorMessage($message);
+		}
+		$this->helper->setResource($resource);
+		$this->helper->setTip($tip);
+		$actual = $this->helper->jsonSerialize();
+		$this->assertEqualsCanonicalizing($expected, $actual);
+	}
+
+	public function providerJsonSerialize(): array {
+		return [
+			[
+				'success',
+				'message',
+				'resource',
+				'tip',
+				[
+					'status' => 'success',
+					'message' => 'message',
+					'resource' => 'resource',
+					'tip' => 'tip',
+				],
+			],
+			[
+				'error',
+				'message',
+				'resource',
+				'tip',
+				[
+					'status' => 'error',
+					'message' => 'message',
+					'resource' => 'resource',
+					'tip' => 'tip',
+				],
+			],
+			[
+				'error',
+				'message',
+				'resource',
+				'',
+				[
+					'status' => 'error',
+					'message' => 'message',
+					'resource' => 'resource',
+					'tip' => '',
+				],
+			],
+			[
+				'error',
+				'message',
+				'',
+				'tip',
+				[
+					'status' => 'error',
+					'message' => 'message',
+					'resource' => '',
+					'tip' => 'tip',
+				],
+			],
+			[
+				'error',
+				'',
+				'resource',
+				'tip',
+				[
+					'status' => 'error',
+					'message' => '',
+					'resource' => 'resource',
+					'tip' => 'tip',
+				],
+			],
+		];
+	}
+}