Skip to content

feat: setup check implementation#7070

Open
guilhermercarvalho wants to merge 10 commits intoLibreSign:mainfrom
guilhermercarvalho:feat/6590-setup-check-implementation
Open

feat: setup check implementation#7070
guilhermercarvalho wants to merge 10 commits intoLibreSign:mainfrom
guilhermercarvalho:feat/6590-setup-check-implementation

Conversation

@guilhermercarvalho
Copy link
Contributor

Related Issue

Issue Number: #6590

Pull Request Type

  • Feature

Pull request checklist

  • Did you explain or provide a way of how can we test your code ?
  • If your pull request is related to frontend modifications provide a print of before and after screen
  • Did you provide a general summary of your changes ?
  • Try to limit your pull request to one type, submit multiple pull requests if needed
  • I implemented tests that cover my contribution

Summary of Changes

This PR refactors the old ConfigureCheckService.php into six specialized classes that implement Nextcloud's ISetupCheck interface. This improves the admin experience by showing detailed setup checks directly in the Nextcloud administration overview and makes them available via occ setupchecks.

The following checks were created:

Check Name Priority Category Logic Source
JavaSetupCheck must-have system ConfigureCheckService::checkJava()
JSignPdfSetupCheck must-have system ConfigureCheckService::checkJSignPdf()
PDFtkSetupCheck must-have system ConfigureCheckService::checkPdftk()
CertificateEngineSetupCheck must-have security ConfigureCheckService::checkCertificate()
PopplerSetupCheck nice-to-have system ConfigureCheckService::checkPoppler()
ImagickSetupCheck nice-to-have system ConfigureCheckService::checkImagick()

Screenshots

localhost_settings_admin_libresign localhost_settings_admin_overview localhost_settings_admin_libresign (1) localhost_settings_admin_overview (6)

All checks are registered in Application.php and the old ConfigureCheckService has been marked as deprecated with a @deprecated annotation pointing to the new classes.

How to Test

  1. Via the web interface:

    • Go to Administration settings → Overview.
    • Scroll down to the "Setup checks" section.
    • Verify that all six checks appear with appropriate statuses (error, warning, info, success) based on your system configuration.
  2. Via CLI:

    occ setupchecks:check

    You should see the results for all LibreSign checks.

  3. Run unit tests:

    composer test:unit -- --filter SetupCheck

    All tests should pass.

@github-project-automation github-project-automation bot moved this to 0. Needs triage in Roadmap Mar 4, 2026
Adds trait with verifyResourceIntegrity and getErrorAndTipFromVerify methods
used by multiple setup checks.

Related issue: LibreSign#6590
Type: Feature

Checklist:
- [x] Add verifyResourceIntegrity method
- [x] Add getErrorAndTipFromVerify method with translated messages
- [x] Declare required properties (signSetupService, urlGenerator, appManager, logger)
- [x] Use IL10N for all user-facing strings

Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds ExecMock, FileSystemMock, SetupCheckFunctions to mock system calls,
and updates bootstrap.php to load them.

Related issue: LibreSign#6590
Type: Test

Checklist:
- [x] Create ExecMock class
- [x] Create FileSystemMock class
- [x] Create SetupCheckFunctions with file_exists and exec overrides
- [x] Update bootstrap.php to include new files

Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds JavaSetupCheck to verify Java installation, path, version and encoding.

Related issue: LibreSign#6590
Type: Feature

Checklist:
- [x] Implement ISetupCheck interface
- [x] Use SetupCheckUtils trait
- [x] Verify Java path exists
- [x] Check Java version matches required version
- [x] Check native.encoding for UTF-8
- [x] Return SetupResult with appropriate severity and translated messages
- [x] Add unit tests covering all scenarios

Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds JSignPdfSetupCheck to verify JSignPdf binary, integrity, Java dependency and version.

Related issue: LibreSign#6590
Type: Feature

Checklist:
- [x] Implement ISetupCheck interface
- [x] Use SetupCheckUtils trait
- [x] Verify JSignPdf path is configured and exists
- [x] Verify Java is available
- [x] Check JSignPdf version against required version
- [x] Return SetupResult with appropriate severity and translated messages
- [x] Add unit tests covering all scenarios

Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds PDFtkSetupCheck to verify PDFtk binary, integrity, Java dependency and version.

Related issue: LibreSign#6590
Type: Feature

Checklist:
- [x] Implement ISetupCheck interface
- [x] Use SetupCheckUtils trait
- [x] Verify PDFtk path is configured and exists
- [x] Verify Java is available
- [x] Check PDFtk version matches required version
- [x] Return SetupResult with appropriate severity and translated messages
- [x] Add unit tests covering all scenarios

Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds PopplerSetupCheck to verify pdfsig and pdfinfo utilities (optional).

Related issue: LibreSign#6590
Type: Feature

Checklist:
- [x] Implement ISetupCheck interface
- [x] Check pdfsig installation and version
- [x] Check pdfinfo installation and version
- [x] Return info severity when missing (optional check)
- [x] Return success when both tools are working
- [x] Add unit tests covering all scenarios

Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds ImagickSetupCheck to verify imagick PHP extension (optional).

Related issue: LibreSign#6590
Type: Feature

Checklist:
- [x] Implement ISetupCheck interface
- [x] Check if imagick extension is loaded
- [x] Return info severity when not loaded (optional check)
- [x] Return success when loaded
- [x] Add unit tests covering both states

Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds CertificateEngineSetupCheck to process certificate engine configuration results.

Related issue: LibreSign#6590
Type: Feature

Checklist:
- [x] Implement ISetupCheck interface
- [x] Use CertificateEngineFactory to get current engine
- [x] Process ConfigureCheckHelper results from engine
- [x] Convert to SetupResult with aggregated messages and tips
- [x] Handle engine not defined (error)
- [x] Add unit tests covering success, warning, error, and mixed results

Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Registers all six setup check classes in Application.php.

Related issue: LibreSign#6590
Type: Feature

Checklist:
- [x] Add registerSetupCheck for JavaSetupCheck
- [x] Add registerSetupCheck for JSignPdfSetupCheck
- [x] Add registerSetupCheck for PDFtkSetupCheck
- [x] Add registerSetupCheck for PopplerSetupCheck
- [x] Add registerSetupCheck for ImagickSetupCheck
- [x] Add registerSetupCheck for CertificateEngineSetupCheck

Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds @deprecated annotation to ConfigureCheckService pointing to the new individual setup checks.

Related issue: LibreSign#6590
Type: Maintenance

Checklist:
- [x] Add @deprecated tag with version 13.0.4
- [x] Mention replacement classes (JavaSetupCheck, JSignPdfSetupCheck, etc.)

Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
@guilhermercarvalho guilhermercarvalho force-pushed the feat/6590-setup-check-implementation branch from 3015ad8 to 2b31973 Compare March 4, 2026 02:22
@guilhermercarvalho guilhermercarvalho changed the title Feat/6590 setup check implementation feat: setup check implementation Mar 4, 2026
Copy link
Member

@vitormattos vitormattos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run the command at LibreSign folder inside container:
composer cs:fix
Some files have a wrong indent pattern.

Comment on lines +23 to +26
/**
* @deprecated 13.0.4 Use the individual SetupCheck classes instead
* (JavaSetupCheck, JSignPdfSetupCheck, etc.).
*/
Copy link
Member

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?

@github-project-automation github-project-automation bot moved this from 0. Needs triage to 1. to do in Roadmap Mar 4, 2026
use OCA\Libresign\Service\Install\InstallService;
use OCA\Libresign\SetupCheck\JavaSetupCheck;
use OCA\Libresign\SetupCheck\FileSystemMock;
use OCA\Libresign\SetupCheck\ExecMock;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that by this way, moving the mock to be an external file of test class, isn't good. Maybe would be best to move the mock files to a new namespace before the row 8 with the mocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 1. to do

Development

Successfully merging this pull request may close these issues.

2 participants