Conversation
…f comparison logic
There was a problem hiding this comment.
Pull request overview
This PR updates the Health Report and Report Diff CLI tooling in CLARIN-DSpace to improve comparison correctness when reports contain different check selections and to align CLI option handling/help output.
Changes:
- Remove the legacy
healthcheckCLI command fromlauncher.xmland rely on the Spring script-service basedhealth-report. - Update
report-diffto normalize report JSON to the intersection of check names, add a “Skipped Checks” section, and make key-field mappings independent of check ordering via selector-based paths. - Standardize help flags to
-h/--helpand update tests/fixtures accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
dspace/config/launcher.xml |
Removes legacy healthcheck command entry. |
dspace-api/src/test/java/org/dspace/scripts/ReportDiffIT.java |
Updates help flag usage and adds coverage for skipped checks / name-based comparisons. |
dspace-api/src/main/resources/report-diff-fields.json |
Switches from index-based paths to selector-by-check-name paths. |
dspace-api/src/main/java/org/dspace/app/reportdiff/ReportDiffScriptConfiguration.java |
Changes -i to -h and clarifies -c semantics for filtering comparisons. |
dspace-api/src/main/java/org/dspace/app/reportdiff/ReportDiff.java |
Implements intersection normalization, skipped-check reporting, and selector-based field resolution. |
dspace-api/src/main/java/org/dspace/app/healthreport/HealthReportScriptConfiguration.java |
Changes help flag to -h, supports multiple -c values, and renames output option to -r. |
dspace-api/src/main/java/org/dspace/app/healthreport/HealthReport.java |
Implements multi-check -c parsing, help renaming, and output option rename. |
dspace-api/src/main/java/org/dspace/app/reportdiff/ReportDiff.java
Outdated
Show resolved
Hide resolved
| <class>org.dspace.storage.bitstore.BitStoreMigrate</class> | ||
| </step> | ||
| </command> | ||
| <command> | ||
| <name>healthcheck</name> | ||
| <description>Create health check report</description> | ||
| <step> | ||
| <class>org.dspace.health.Report</class> | ||
| </step> | ||
| </command> | ||
|
|
||
| <command> | ||
| <name>checker</name> | ||
| <description>Run the checksum checker</description> | ||
| <step> | ||
| <class>org.dspace.app.checker.ChecksumChecker</class> |
There was a problem hiding this comment.
launcher.xml no longer exposes the legacy healthcheck command, but the implementation class org.dspace.health.Report still exists in the codebase. If this removal is intentional, consider either removing/deprecating the legacy class as well, or adding a compatibility alias / release note so existing automation using dspace healthcheck doesn’t break unexpectedly.
There was a problem hiding this comment.
@milanmajchrak In dspace/health, there are still many classes from LINDAT (version 5). Which of them do we not want? Does solving this issue also remove the old unused classes (for example, the healthcheck problem that may call a process with the error mentioned by Milan Kuchtiak)?
dspace-api/src/main/java/org/dspace/app/reportdiff/ReportDiff.java
Outdated
Show resolved
Hide resolved
|
Problem description
Followed issues:
https://github.com/dataquest-dev/dspace-customers/issues/430
#1250
Manual Testing (if applicable)
Copilot review