-
-
Notifications
You must be signed in to change notification settings - Fork 111
feat: setup check implementation #7070
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
guilhermercarvalho
wants to merge
23
commits into
LibreSign:main
Choose a base branch
from
guilhermercarvalho:feat/6590-setup-check-implementation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 10 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
ba29270
feat(utils): add SetupCheckUtils trait
guilhermercarvalho 3ab2f05
test: add test infrastructure for setup checks
guilhermercarvalho 71abe0c
feat(setup-check): add JavaSetupCheck
guilhermercarvalho 59ce235
feat(setup-check): add JSignPdfSetupCheck
guilhermercarvalho 1ab95c9
feat(setup-check): add PDFtkSetupCheck
guilhermercarvalho 39cef8b
feat(setup-check): add PopplerSetupCheck
guilhermercarvalho ef6695f
feat(setup-check): add ImagickSetupCheck
guilhermercarvalho 67c627b
feat(setup-check): add CertificateEngineSetupCheck
guilhermercarvalho 350323e
feat(application): register new setup checks
guilhermercarvalho 2b31973
refactor: deprecate ConfigureCheckService
guilhermercarvalho 3766c42
refactor(setup-check): reorganize mocks and apply code standards
guilhermercarvalho 1a56510
style(setup-check): apply code style fixes and improve Psalm annotations
guilhermercarvalho ec70886
fix(setup-check): address Psalm issues and improve robustness
guilhermercarvalho 453c15a
test(setup-check): update PDFtkSetupCheckTest to match new error message
guilhermercarvalho 037da11
test(setupcheck): reorganize test mocks into dedicated namespace
guilhermercarvalho c06327f
feat(command): update configure:check to use ISetupCheckManager
guilhermercarvalho 934b7ac
refactor: replace deprecated ConfigureCheckService with SetupCheckRes…
guilhermercarvalho 03283ef
fix(api): ensure OU field is always returned as string in certificate…
guilhermercarvalho c640d0f
refactor(service): remove deprecated ConfigureCheckService
guilhermercarvalho db325bd
test(service): add unit tests for SetupCheckResultService
guilhermercarvalho 0ac2fed
chore(service): remove redundant comment in SetupCheckResultService
guilhermercarvalho ebb17c5
test(handler): update OU filtering test to expect string
guilhermercarvalho 502124e
chore(service): remove redundant comments
guilhermercarvalho File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
| /** | ||
| * SPDX-FileCopyrightText: 2026 LibreCode coop and contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| namespace OCA\Libresign\SetupCheck; | ||
|
|
||
| use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; | ||
| use OCA\Libresign\Helper\ConfigureCheckHelper; | ||
| use OCP\IL10N; | ||
| use OCP\SetupCheck\ISetupCheck; | ||
| use OCP\SetupCheck\SetupResult; | ||
|
|
||
| class CertificateEngineSetupCheck implements ISetupCheck | ||
| { | ||
| private IL10N $l10n; | ||
| private CertificateEngineFactory $certificateEngineFactory; | ||
|
|
||
| public function __construct( | ||
| IL10N $l10n, | ||
| CertificateEngineFactory $certificateEngineFactory | ||
| ) { | ||
| $this->l10n = $l10n; | ||
| $this->certificateEngineFactory = $certificateEngineFactory; | ||
| } | ||
|
|
||
| public function getName(): string | ||
| { | ||
| return $this->l10n->t('Certificate engine'); | ||
| } | ||
|
|
||
| public function getCategory(): string | ||
| { | ||
| return 'security'; | ||
| } | ||
|
|
||
| public function run(): SetupResult | ||
| { | ||
| try { | ||
| $engine = $this->certificateEngineFactory->getEngine(); | ||
| $checkResults = $engine->configureCheck(); | ||
| } catch (\Throwable $e) { | ||
| $engineName = $this->certificateEngineFactory->getEngine()->getName() ?? 'unknown'; | ||
| return SetupResult::error( | ||
| $this->l10n->t('Define the certificate engine to use'), | ||
| $this->l10n->t('Run occ libresign:configure:%s --help', [$engineName]) | ||
| ); | ||
| } | ||
|
|
||
| // Process results: if any error, return error; else if any warning, return warning; else success | ||
| $hasError = false; | ||
| $hasWarning = false; | ||
| $messages = []; | ||
| $tips = []; | ||
|
|
||
| foreach ($checkResults as $result) { | ||
| if ($result instanceof ConfigureCheckHelper) { | ||
| $status = $result->getStatus(); | ||
| $msg = $result->getMessage(); | ||
| $tip = $result->getTip(); | ||
|
|
||
| if ($status === 'error') { | ||
| $hasError = true; | ||
| $messages[] = "[ERROR] $msg"; | ||
| } elseif ($status === 'warning') { | ||
| $hasWarning = true; | ||
| $messages[] = "[WARNING] $msg"; | ||
| } else { | ||
| $messages[] = $msg; | ||
| } | ||
|
|
||
| if (!empty($tip)) { | ||
| $tips[] = $tip; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| $tip = ''; | ||
| if (!empty($tips)) { | ||
| $tip = implode("\n", array_unique($tips)); | ||
| } | ||
|
|
||
| if ($hasError) { | ||
| return SetupResult::error(implode("\n", $messages), $tip); | ||
| } elseif ($hasWarning) { | ||
| return SetupResult::warning(implode("\n", $messages), $tip); | ||
| } else { | ||
| return SetupResult::success(implode("\n", $messages) ?: $this->l10n->t('Certificate engine is configured correctly')); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
| /** | ||
| * SPDX-FileCopyrightText: 2026 LibreCode coop and contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| namespace OCA\Libresign\SetupCheck; | ||
|
|
||
| use OCP\IL10N; | ||
| use OCP\SetupCheck\ISetupCheck; | ||
| use OCP\SetupCheck\SetupResult; | ||
|
|
||
| class ImagickSetupCheck implements ISetupCheck { | ||
| private IL10N $l10n; | ||
|
|
||
| public function __construct(IL10N $l10n) { | ||
| $this->l10n = $l10n; | ||
| } | ||
|
|
||
| public function getName(): string { | ||
| return $this->l10n->t('Imagick PHP extension'); | ||
| } | ||
|
|
||
| public function getCategory(): string { | ||
| return 'system'; | ||
| } | ||
|
|
||
| public function run(): SetupResult { | ||
| if (!extension_loaded('imagick')) { | ||
| return SetupResult::info( | ||
| $this->l10n->t('Imagick extension is not loaded'), | ||
| $this->l10n->t('Install php-imagick to enable visible signatures, background images, and signature element rendering.') | ||
| ); | ||
| } | ||
| return SetupResult::success($this->l10n->t('Imagick extension is loaded')); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
| /** | ||
| * SPDX-FileCopyrightText: 2026 LibreCode coop and contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| namespace OCA\Libresign\SetupCheck; | ||
|
|
||
| use OCA\Libresign\Helper\JavaHelper; | ||
| use OCA\Libresign\AppInfo\Application; | ||
| use OCA\Libresign\Handler\SignEngine\JSignPdfHandler; | ||
| use OCA\Libresign\Service\Install\SignSetupService; | ||
| use OCA\Libresign\Service\Install\InstallService; | ||
| use OCP\IL10N; | ||
| use OCP\IAppConfig; | ||
| use OCP\IURLGenerator; | ||
| use OCP\App\IAppManager; | ||
| use OCP\SetupCheck\ISetupCheck; | ||
| use OCP\SetupCheck\SetupResult; | ||
| use OCP\IConfig; | ||
| use Psr\Log\LoggerInterface; | ||
|
|
||
| class JSignPdfSetupCheck implements ISetupCheck | ||
| { | ||
| use SetupCheckUtils; | ||
|
|
||
| private IL10N $l10n; | ||
| private IAppConfig $appConfig; | ||
| private JSignPdfHandler $jSignPdfHandler; | ||
| private IConfig $systemConfig; | ||
| private JavaHelper $javaHelper; | ||
|
|
||
| private SignSetupService $signSetupService; | ||
| private IURLGenerator $urlGenerator; | ||
| private IAppManager $appManager; | ||
| private LoggerInterface $logger; | ||
|
|
||
| public function __construct( | ||
| IL10N $l10n, | ||
| IAppConfig $appConfig, | ||
| JSignPdfHandler $jSignPdfHandler, | ||
| SignSetupService $signSetupService, | ||
| IURLGenerator $urlGenerator, | ||
| IAppManager $appManager, | ||
| LoggerInterface $logger, | ||
| IConfig $systemConfig, | ||
| JavaHelper $javaHelper | ||
| ) { | ||
| $this->l10n = $l10n; | ||
| $this->appConfig = $appConfig; | ||
| $this->jSignPdfHandler = $jSignPdfHandler; | ||
| $this->signSetupService = $signSetupService; | ||
| $this->urlGenerator = $urlGenerator; | ||
| $this->appManager = $appManager; | ||
| $this->logger = $logger; | ||
| $this->systemConfig = $systemConfig; | ||
| $this->javaHelper = $javaHelper; | ||
| } | ||
|
|
||
| public function getName(): string | ||
| { | ||
| return $this->l10n->t('JSignPdf'); | ||
| } | ||
|
|
||
| public function getCategory(): string | ||
| { | ||
| return 'system'; | ||
| } | ||
|
|
||
| public function run(): SetupResult | ||
| { | ||
| $debugEnabled = $this->systemConfig->getSystemValueBool('debug', false); | ||
| $jsignpdfJarPath = $this->appConfig->getValueString(Application::APP_ID, 'jsignpdf_jar_path'); | ||
|
|
||
| if (!$jsignpdfJarPath) { | ||
| return SetupResult::error( | ||
| $this->l10n->t('JSignPdf not found'), | ||
| $this->l10n->t('Run occ libresign:install --jsignpdf') | ||
| ); | ||
| } | ||
|
|
||
| $verifyResult = $this->verifyResourceIntegrity('jsignpdf', $debugEnabled); | ||
| if (!empty($verifyResult)) { | ||
| [$errorMsg, $tip] = $this->getErrorAndTipFromVerify($verifyResult, 'jsignpdf', $debugEnabled, $this->l10n); | ||
| return SetupResult::error($errorMsg, $tip); | ||
| } | ||
|
|
||
| if (!file_exists($jsignpdfJarPath)) { | ||
| return SetupResult::error( | ||
| $this->l10n->t('JSignPdf binary not found: %s', [$jsignpdfJarPath]), | ||
| $this->l10n->t('Run occ libresign:install --jsignpdf') | ||
| ); | ||
| } | ||
|
|
||
| $javaPath = $this->javaHelper->getJavaPath(); | ||
| if (!$javaPath || !file_exists($javaPath)) { | ||
| return SetupResult::error( | ||
| $this->l10n->t('Necessary Java to run JSignPdf'), | ||
| $this->l10n->t('Run occ libresign:install --java') | ||
| ); | ||
| } | ||
|
|
||
| $jsignPdf = $this->jSignPdfHandler->getJSignPdf(); | ||
| $jsignPdf->setParam($this->jSignPdfHandler->getJSignParam()); | ||
| $currentVersion = $jsignPdf->getVersion(); | ||
|
|
||
| if (!$currentVersion) { | ||
| $msg = $this->l10n->t('Necessary install the version %s', [InstallService::JSIGNPDF_VERSION]); | ||
| return SetupResult::error($msg, $this->l10n->t('Run occ libresign:install --jsignpdf')); | ||
| } | ||
|
|
||
| if (version_compare($currentVersion, InstallService::JSIGNPDF_VERSION, '<')) { | ||
| $msg = $this->l10n->t('Necessary bump JSignPdf version from %s to %s', [$currentVersion, InstallService::JSIGNPDF_VERSION]); | ||
| return SetupResult::error($msg, $this->l10n->t('Run occ libresign:install --jsignpdf')); | ||
| } | ||
|
|
||
| if (version_compare($currentVersion, InstallService::JSIGNPDF_VERSION, '>')) { | ||
| return SetupResult::error( | ||
| $this->l10n->t('Necessary downgrade JSignPdf version from %s to %s', [$currentVersion, InstallService::JSIGNPDF_VERSION]), | ||
| $this->l10n->t('Run occ libresign:install --jsignpdf') | ||
| ); | ||
| } | ||
|
|
||
| $messages = [ | ||
| $this->l10n->t('JSignPdf version: %s', [$currentVersion]), | ||
| $this->l10n->t('JSignPdf path: %s', [$jsignpdfJarPath]) | ||
| ]; | ||
|
|
||
| return SetupResult::success(implode("\n", $messages)); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this, and why not solve?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
@deprecatedannotation was originally added to maintain backward compatibility as suggested in the issue discussion. However, after further analysis and the reviewer's comment, I realized that fully removing the class is cleaner and more aligned with the goal of this refactor. The oldConfigureCheckServiceis now completely replaced by the newSetupCheckResultServiceand the individualISetupCheckimplementations. There are no remaining usages of the old service in the codebase, so keeping a deprecated stub would only add unnecessary maintenance burden. Therefore, I removed the class entirely. This should resolve the concern – the service is gone, not just deprecated.