Skip to content

Commit b34dd8a

Browse files
author
Matus Kasak
committed
Sorting reports and updating tests, plus enable multiple -c in report-diff
1 parent 903c321 commit b34dd8a

3 files changed

Lines changed: 36 additions & 21 deletions

File tree

dspace-api/src/main/java/org/dspace/app/reportdiff/ReportDiff.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,11 @@ public void internalRun() throws Exception {
262262

263263
defaultReportIds(context);
264264

265+
if (sourceReportId == null || targetReportId == null) {
266+
handler.logInfo("Need at least 2 reports in the database to perform a comparison. Aborting.");
267+
return;
268+
}
269+
265270
if (!validateReportIdSelection()) {
266271
return;
267272
}
@@ -353,6 +358,9 @@ private void defaultReportIds(Context context) {
353358
return;
354359
}
355360

361+
// findAll() does not guarantee ordering; sort by lastModified ascending so the
362+
// newest reports are at the end of the list.
363+
allReports.sort(Comparator.comparing(ReportResult::getLastModified));
356364
int size = allReports.size();
357365

358366
if (Objects.isNull(targetReportId) && size > 0) {
@@ -376,6 +384,9 @@ private void displayReportDates() {
376384
try (Context context = new Context()) {
377385
context.setCurrentUser(ePersonService.find(context, getEpersonIdentifier()));
378386
List<ReportResult> allReports = reportResultService.findAll(context);
387+
// findAll() does not guarantee ordering; sort by lastModified ascending so the
388+
// newest reports are at the end of the list.
389+
allReports.sort(Comparator.comparing(ReportResult::getLastModified));
379390
// Determine how many reports to process, respecting maxEntries if it's within valid range
380391
long limitCount = (maxEntries > 0 && maxEntries < allReports.size()) ? maxEntries : allReports.size();
381392
Map<String, List<DateWithArgs>> reportDatesMap = new HashMap<>();

dspace-api/src/main/java/org/dspace/app/reportdiff/ReportDiffScriptConfiguration.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88
package org.dspace.app.reportdiff;
99

10+
import org.apache.commons.cli.Option;
1011
import org.apache.commons.cli.Options;
1112
import org.dspace.app.healthreport.HealthReport;
1213
import org.dspace.scripts.configuration.ScriptConfiguration;
@@ -38,11 +39,13 @@ public Options getOptions() {
3839
options.addOption("e", "email", true,
3940
"Send report to this email address.");
4041
options.getOption("e").setType(String.class);
41-
options.addOption("c", "check", true,
42-
String.format("Filter comparison to a specific check by index (0 to %d). " +
43-
"Only the specified check will be compared from both reports.",
44-
HealthReport.getNumberOfChecks() - 1));
45-
options.getOption("c").setType(String.class);
42+
Option checkOption = Option.builder("c").longOpt("check").hasArgs()
43+
.desc(String.format("Filter comparison to one or more specific checks by index (0 to %d). " +
44+
"Repeat the flag (e.g. -c 1 -c 3) to compare multiple checks from both reports.",
45+
HealthReport.getNumberOfChecks() - 1))
46+
.type(String.class)
47+
.build();
48+
options.addOption(checkOption);
4649

4750
options.addOption("l", "list", false,
4851
"List available reports (ID, timestamp, args). Use to find report IDs.");

dspace-api/src/test/java/org/dspace/scripts/HealthReportIT.java

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -255,20 +255,21 @@ public void testReportFileSaved() throws Exception {
255255
String content = Files.readString(tempFile.toPath());
256256
assertThat("Report file must contain health report header", content, containsString("HEALTH REPORT:"));
257257
}
258-
@Test
259-
public void testStoredArgsContainAllCheckOptions() throws Exception {
260-
ReportResultService reportResultService = ContentServiceFactory.getInstance().getReportResultService();
261-
262-
TestDSpaceRunnableHandler handler = new TestDSpaceRunnableHandler();
263-
String[] args = new String[] { "health-report", "-c", "2", "-c", "3" };
264-
ScriptLauncher.handleScript(args, ScriptLauncher.getConfig(kernelImpl), handler, kernelImpl);
265-
266-
context.reloadEntity(eperson);
267-
List<ReportResult> allReports = reportResultService.findAll(context);
268-
ReportResult latest = allReports.get(allReports.size() - 1);
269-
270-
assertThat(handler.getErrorMessages(), empty());
271-
assertThat(latest.getArgs(), containsString("-c: 2"));
272-
assertThat(latest.getArgs(), containsString("-c: 3"));
273-
}
258+
259+
@Test
260+
public void testStoredArgsContainAllCheckOptions() throws Exception {
261+
ReportResultService reportResultService = ContentServiceFactory.getInstance().getReportResultService();
262+
263+
TestDSpaceRunnableHandler handler = new TestDSpaceRunnableHandler();
264+
String[] args = new String[] { "health-report", "-c", "2", "-c", "3" };
265+
ScriptLauncher.handleScript(args, ScriptLauncher.getConfig(kernelImpl), handler, kernelImpl);
266+
267+
context.reloadEntity(eperson);
268+
List<ReportResult> allReports = reportResultService.findAll(context);
269+
ReportResult latest = allReports.get(allReports.size() - 1);
270+
271+
assertThat(handler.getErrorMessages(), empty());
272+
assertThat(latest.getArgs(), containsString("-c: 2"));
273+
assertThat(latest.getArgs(), containsString("-c: 3"));
274+
}
274275
}

0 commit comments

Comments
 (0)