From 4201eb8058e3b9ccd045aa217070131fb19477b1 Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Thu, 10 Sep 2026 19:18:04 +0200 Subject: [PATCH] Restore the fork health checks taken wholesale from vanilla (23 of 26 report-diff mappings had no emitter) InfoCheck, ItemCheck and UserCheck were byte-identical with vanilla dspace-9.3 on the v9 base although the fork changes all three. The upgrade took them from upstream whole and the fork hunks never landed, so their setReportJson calls were gone: only LicenseCheck and (since BE-13) MetadataCheck still emitted report JSON, and 23 of the 26 mappings in report-diff-fields.json addressed a value nothing produced - Item summary 17, User summary 4, General Information 2. report-diff printed those columns empty and nothing failed. The three checks now build the same JSON the fork does, alongside the unchanged human-readable report. Two deliberate deviations from the fork, both recorded in the PR: getObjectSizesInfo keeps the v9 direct calls instead of the fork's wrapSql wrapper, which converts the checked SQLException into a RuntimeException that run()'s catch cannot see and that would abort the whole report; and ReportInfo returns LocalDate on v9, so the dates are formatted directly instead of via Date.toInstant().atZone(). UserCheck also carries the fork's fix of the "Self registered" counter, which tested getNetid() twice. Report.java goes with them: the fork deleted it in 3dc9cecf7d when the health-report script replaced it, and launcher.xml still pointed the healthcheck command at it, so `dspace healthcheck` ran the old class that writes no JSON and no ReportResult at all. ReportDiffFieldMappingIT walks every mapping in report-diff-fields.json against a real health report, so a mapping that loses its emitter fails instead of silently printing nothing. Card X-08. Co-Authored-By: Claude Opus 5 (1M context) --- .../java/org/dspace/health/InfoCheck.java | 57 ++-- .../java/org/dspace/health/ItemCheck.java | 161 +++++++---- .../main/java/org/dspace/health/Report.java | 227 --------------- .../java/org/dspace/health/UserCheck.java | 104 +++++-- .../health/ReportDiffFieldMappingIT.java | 267 ++++++++++++++++++ dspace/config/launcher.xml | 7 - 6 files changed, 494 insertions(+), 329 deletions(-) delete mode 100644 dspace-api/src/main/java/org/dspace/health/Report.java create mode 100644 dspace-api/src/test/java/org/dspace/health/ReportDiffFieldMappingIT.java diff --git a/dspace-api/src/main/java/org/dspace/health/InfoCheck.java b/dspace-api/src/main/java/org/dspace/health/InfoCheck.java index 9ecc75458772..1c7011c86620 100644 --- a/dspace-api/src/main/java/org/dspace/health/InfoCheck.java +++ b/dspace-api/src/main/java/org/dspace/health/InfoCheck.java @@ -8,13 +8,14 @@ package org.dspace.health; import java.io.File; -import java.time.Instant; -import java.time.format.DateTimeFormatter; +import java.time.LocalDateTime; import org.apache.commons.io.FileUtils; import org.dspace.services.ConfigurationService; import org.dspace.storage.bitstore.DSBitStoreService; import org.dspace.utils.DSpace; +import org.json.JSONArray; +import org.json.JSONObject; /** * @author LINDAT/CLARIN dev team @@ -26,24 +27,31 @@ public String run(ReportInfo ri) { ConfigurationService configurationService = new DSpace().getConfigurationService(); StringBuilder sb = new StringBuilder(); - sb.append("Generated: ").append( - Instant.now().toString() - ).append("\n"); + JSONObject root = new JSONObject(); - sb.append("From - Till: ").append( - DateTimeFormatter.ISO_LOCAL_DATE.format(ri.from()) - ).append(" - ").append( - DateTimeFormatter.ISO_LOCAL_DATE.format(ri.till()) - ).append("\n"); + String generatedStr = LocalDateTime.now().format(DateFormatConstants.DATETIME_FORMATTER); + sb.append("Generated: ").append(generatedStr).append("\n"); + root.put("generated", generatedStr); - sb.append("Url: ").append( - configurationService.getProperty("dspace.ui.url") - ).append("\n"); + String fromTill = "From - Till: " + + ri.from().format(DateFormatConstants.DATE_FORMATTER) + + " - " + ri.till().format(DateFormatConstants.DATE_FORMATTER); + sb.append(fromTill).append("\n"); + root.put("fromTill", fromTill); + + String urlValue = configurationService.getProperty("dspace.ui.url"); + sb.append("Url: ").append(urlValue).append("\n"); sb.append("\n"); + root.put("url", urlValue); DSBitStoreService localStore = new DSpace().getServiceManager() .getServicesByType(DSBitStoreService.class) .get(0); + + // Build the directory stats array. report-diff-fields.json addresses these positionally + // (directoryStats/0 = assetstore, directoryStats/1 = log dir), so an entry is appended for + // every directory even when it cannot be read - dropping one would shift the other's index. + JSONArray dirStatsArray = new JSONArray(); for (String[] ss : new String[][] { new String[] { localStore.getBaseDir().toString(), @@ -51,26 +59,39 @@ public String run(ReportInfo ri) { new String[] { configurationService.getProperty("log.report.dir"), "Log dir size",},}) { + JSONObject oneStat = new JSONObject(); + oneStat.put("label", ss[1]); + if (ss[0] != null) { try { File dir = new File(ss[0]); if (dir.exists()) { long dir_size = FileUtils.sizeOfDirectory(dir); - sb.append(String.format("%-20s: %s\n", ss[1], - FileUtils.byteCountToDisplaySize(dir_size)) + String displaySize = FileUtils.byteCountToDisplaySize(dir_size); + sb.append(String.format("%-20s: %s\n", ss[1], displaySize) ); + oneStat.put("path", ss[0]); + oneStat.put("size_bytes", dir_size); + oneStat.put("size_display", displaySize); } else { - sb.append(String.format("Directory [%s] does not exist!\n", ss[0])); + String msg = String.format("Directory %s does not exist!", ss[0]); + oneStat.put("path", ss[0]); + oneStat.put("notExist", msg); + sb.append(msg).append("\n"); } } catch (Exception e) { error(e, "directory - " + ss[0]); } } else { // cannot read property for some reason - sb.append(String.format("Could not get information for %s!\n", ss[1])); + String msg = String.format("Could not get information for %s!\n", ss[1]); + sb.append(msg); + oneStat.put("warning", msg); } + dirStatsArray.put(oneStat); } + root.put("directoryStats", dirStatsArray); + this.setReportJson(root); return sb.toString(); } - } diff --git a/dspace-api/src/main/java/org/dspace/health/ItemCheck.java b/dspace-api/src/main/java/org/dspace/health/ItemCheck.java index d67be523a984..0a47c2087278 100644 --- a/dspace-api/src/main/java/org/dspace/health/ItemCheck.java +++ b/dspace-api/src/main/java/org/dspace/health/ItemCheck.java @@ -35,6 +35,8 @@ import org.dspace.handle.service.HandleService; import org.dspace.xmlworkflow.factory.XmlWorkflowServiceFactory; import org.dspace.xmlworkflow.storedcomponents.service.XmlWorkflowItemService; +import org.json.JSONArray; +import org.json.JSONObject; /** * @author LINDAT/CLARIN dev team @@ -57,88 +59,124 @@ public class ItemCheck extends Check { @Override public String run(ReportInfo ri) { - String ret = ""; + StringBuilder sb = new StringBuilder(); + JSONObject root = new JSONObject(); int tot_cnt = 0; Context context = new Context(); try { + JSONArray communitiesArray = new JSONArray(); for (Map.Entry name_count : getCommunities(context)) { - ret += String.format("Community [%s]: %d\n", - name_count.getKey(), name_count.getValue()); - tot_cnt += name_count.getValue(); + String comName = name_count.getKey(); + int comSize = name_count.getValue(); + sb.append(String.format("Community [%s]: %d\n", comName, comSize)); + tot_cnt += comSize; + JSONObject oneCommunity = new JSONObject(); + oneCommunity.put("name", comName); + oneCommunity.put("size", comSize); + communitiesArray.put(oneCommunity); } + root.put("communities", communitiesArray); } catch (SQLException e) { error(e); } try { - ret += "\nCollection sizes:\n"; - ret += getCollectionSizesInfo(context); + JSONObject colSizesInfo = new JSONObject(); + sb.append("\nCollection sizes:\n"); + sb.append(getCollectionSizesInfo(context, colSizesInfo)); + root.put("collectionsSizesInfo", colSizesInfo); } catch (SQLException e) { error(e); } - ret += String.format( - "\nPublished items (archived, not withdrawn): %d\n", tot_cnt); + sb.append(String.format("\nPublished items (archived, not withdrawn): %d\n", tot_cnt)); + root.put("publishedItems", tot_cnt); try { - ret += String.format( - "Withdrawn items: %d\n", itemService.countWithdrawnItems(context)); - ret += String.format( - "Not published items (in workspace or workflow mode): %d\n", - itemService.countNotArchivedItems(context)); + int withdrawnItems = itemService.countWithdrawnItems(context); + sb.append(String.format("Withdrawn items: %d\n", withdrawnItems)); + root.put("withdrawnItems", withdrawnItems); + + int notPublishedItems = itemService.countNotArchivedItems(context); + sb.append(String.format("Not published items (in workspace or workflow mode): %d\n", notPublishedItems)); + root.put("notPublishedItems", notPublishedItems); + JSONArray stagesCountArray = new JSONArray(); for (Map.Entry row : workspaceItemService.getStageReachedCounts(context)) { - ret += String.format("\tIn Stage %s: %s\n", - row.getKey(), //"stage_reached" - row.getValue() //"cnt" - ); + sb.append(String.format("\tIn Stage %s: %s\n", + row.getKey(), //"stage_reached" + row.getValue() //"cnt" + )); + JSONObject oneStage = new JSONObject(); + oneStage.put("stage", row.getKey()); + oneStage.put("count", row.getValue()); + stagesCountArray.put(oneStage); } + root.put("stagesCounts", stagesCountArray); - ret += String.format( - "\tWaiting for approval (workflow items): %d\n", - workflowItemService.countAll(context)); + int waitingForApprovalCount = workflowItemService.countAll(context); + sb.append(String.format("\tWaiting for approval (workflow items): %d\n", waitingForApprovalCount)); + root.put("waitingForApproval", waitingForApprovalCount); } catch (SQLException e) { error(e); } try { - ret += getObjectSizesInfo(context); + sb.append(getObjectSizesInfo(context, root)); context.complete(); } catch (SQLException e) { error(e); } - return ret; + + this.setReportJson(root); + return sb.toString(); } - public String getObjectSizesInfo(Context context) throws SQLException { + /** + * Appends the entity counts to the human-readable report and puts each of them into {@code jo} under + * the key {@code report-diff-fields.json} addresses it by. + * + * @param context current DSpace session + * @param jo the check's JSON report, extended in place + * @return the human-readable section of the report + * @throws SQLException passed through so that {@link #run(ReportInfo)} records it with + * {@link Check#error(Throwable)} and still finishes the rest of the report + */ + public String getObjectSizesInfo(Context context, JSONObject jo) throws SQLException { StringBuilder sb = new StringBuilder(); - sb.append(String.format("Count %-14s: %s\n", "Bitstream", - String.valueOf(bitstreamService.countTotal(context)))); - sb.append(String.format("Count %-14s: %s\n", "Bundle", - String.valueOf(bundleService.countTotal(context)))); - sb.append(String.format("Count %-14s: %s\n", "Collection", - String.valueOf(collectionService.countTotal(context)))); - sb.append(String.format("Count %-14s: %s\n", "Community", - String.valueOf(communityService.countTotal(context)))); - sb.append(String.format("Count %-14s: %s\n", "MetadataValue", - String.valueOf(metadataValueService.countTotal(context)))); - sb.append(String.format("Count %-14s: %s\n", "EPerson", - String.valueOf(ePersonService.countTotal(context)))); - sb.append(String.format("Count %-14s: %s\n", "Item", - String.valueOf(itemService.countTotal(context)))); - sb.append(String.format("Count %-14s: %s\n", "Handle", - String.valueOf(handleService.countTotal(context)))); - sb.append(String.format("Count %-14s: %s\n", "Group", - String.valueOf(groupService.countTotal(context)))); - sb.append(String.format("Count %-14s: %s\n", "BasicWorkflowItem", - String.valueOf(workflowItemService.countAll(context)))); - sb.append(String.format("Count %-14s: %s\n", "WorkspaceItem", - String.valueOf(workspaceItemService.countTotal(context)))); + sb.append(countLine(jo, "Bitstream", "bitstreamsCount", bitstreamService.countTotal(context))); + sb.append(countLine(jo, "Bundle", "bundlesCount", bundleService.countTotal(context))); + sb.append(countLine(jo, "Collection", "collectionsCount", collectionService.countTotal(context))); + sb.append(countLine(jo, "Community", "communitiesCount", communityService.countTotal(context))); + sb.append(countLine(jo, "MetadataValue", "metadataValuesCount", metadataValueService.countTotal(context))); + sb.append(countLine(jo, "EPerson", "ePersonsCount", ePersonService.countTotal(context))); + sb.append(countLine(jo, "Item", "itemsCount", itemService.countTotal(context))); + sb.append(countLine(jo, "Handle", "handlesCount", handleService.countTotal(context))); + sb.append(countLine(jo, "Group", "groupsCount", groupService.countTotal(context))); + sb.append(countLine(jo, "BasicWorkflowItem", "basicWorkflowItemsCount", workflowItemService.countAll(context))); + sb.append(countLine(jo, "WorkspaceItem", "workspaceItemsCount", workspaceItemService.countTotal(context))); return sb.toString(); } - public String getCollectionSizesInfo(final Context context) throws SQLException { + /** + * Emits one count into both halves of the report: the formatted text line and the JSON key. + */ + private String countLine(JSONObject jo, String displayName, String jsonKey, int count) { + jo.put(jsonKey, count); + return String.format("Count %-20s: %s\n", displayName, String.valueOf(count)); + } + + /** + * Appends the per-collection sizes to the human-readable report and puts the aggregate numbers into + * {@code jo}, which {@link #run(ReportInfo)} stores under {@code collectionsSizesInfo}. + * + * @param context current DSpace session + * @param jo the {@code collectionsSizesInfo} object, extended in place + * @return the human-readable section of the report + * @throws SQLException passed through, see {@link #getObjectSizesInfo(Context, JSONObject)} + */ + public String getCollectionSizesInfo(final Context context, JSONObject jo) throws SQLException { final StringBuffer ret = new StringBuffer(); List> colBitSizes = collectionService .getCollectionsWithBitstreamSizesTotal(context); @@ -157,31 +195,52 @@ public int compare(Map.Entry o1, Map.Entry o return 0; } }); + + JSONArray collectionsSizesArray = new JSONArray(); for (Map.Entry row : colBitSizes) { Long size = row.getValue(); total_size += size; Collection col = row.getKey(); + String colPath = CollectionDropDown.collectionPath(context, col); + String colSize = FileUtils.byteCountToDisplaySize((long) size); ret.append(String.format( - "\t%s: %s\n", CollectionDropDown.collectionPath(context, col), - FileUtils.byteCountToDisplaySize((long) size))); + "\t%s: %s\n", colPath, colSize)); + JSONObject oneColSize = new JSONObject(); + oneColSize.put("path", colPath); + oneColSize.put("size", colSize); + collectionsSizesArray.put(oneColSize); } + jo.put("collectionSizes", collectionsSizesArray); + + String totalSizeToDisplay = FileUtils.byteCountToDisplaySize(total_size); ret.append(String.format( - "Total size: %s\n", FileUtils.byteCountToDisplaySize(total_size))); + "Total size: %s\n", totalSizeToDisplay)); + jo.put("totalSize", totalSizeToDisplay); + int resourceWOPolicyCount = bitstreamService.countBitstreamsWithoutPolicy(context); ret.append(String.format( - "Resource without policy: %d\n", bitstreamService.countBitstreamsWithoutPolicy(context))); + "Resource without policy: %d\n", resourceWOPolicyCount)); + jo.put("resourceWOPolicy", resourceWOPolicyCount); + int deletedBitstreamsCount = bitstreamService.countDeletedBitstreams(context); ret.append(String.format( - "Deleted bitstreams: %d\n", bitstreamService.countDeletedBitstreams(context))); + "Deleted bitstreams: %d\n", deletedBitstreamsCount)); + jo.put("deletedBitstreams", deletedBitstreamsCount); String list_str = ""; + JSONArray orphanBitstreamsArray = new JSONArray(); List bitstreamOrphans = bitstreamService.getNotReferencedBitstreams(context); for (Bitstream orphan : bitstreamOrphans) { UUID id = orphan.getID(); + JSONObject oneOrphanBitstream = new JSONObject(); + oneOrphanBitstream.put("uuid", id.toString()); + orphanBitstreamsArray.put(oneOrphanBitstream); list_str += String.format("%s, ", id); } ret.append(String.format( "Orphan bitstreams: %d [%s]\n", bitstreamOrphans.size(), list_str)); + jo.put("orphanBitstreamsCount", bitstreamOrphans.size()); + jo.put("orphanBitstreams", orphanBitstreamsArray); return ret.toString(); } diff --git a/dspace-api/src/main/java/org/dspace/health/Report.java b/dspace-api/src/main/java/org/dspace/health/Report.java deleted file mode 100644 index 2418f19a8499..000000000000 --- a/dspace-api/src/main/java/org/dspace/health/Report.java +++ /dev/null @@ -1,227 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.health; - -import java.io.IOException; -import java.time.Instant; -import java.util.ArrayList; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map.Entry; -import java.util.StringTokenizer; - -import jakarta.mail.MessagingException; -import org.apache.commons.cli.CommandLine; -import org.apache.commons.cli.DefaultParser; -import org.apache.commons.cli.HelpFormatter; -import org.apache.commons.cli.Options; -import org.apache.commons.cli.ParseException; -import org.apache.commons.lang3.exception.ExceptionUtils; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.dspace.core.Email; -import org.dspace.core.factory.CoreServiceFactory; -import org.dspace.core.service.PluginService; -import org.dspace.services.ConfigurationService; -import org.dspace.services.factory.DSpaceServicesFactory; - -/** - * @author LINDAT/CLARIN dev team - */ -public class Report { - - private static final Logger log = LogManager.getLogger(Report.class); - public static final String EMAIL_PATH = "config/emails/healthcheck"; - // store the individual check reports - private final StringBuilder summary_; - - // ctor - // - public Report() { - summary_ = new StringBuilder(); - } - - // run checks - // - public void run(List to_perform, ReportInfo ri) { - - int pos = -1; - for (Entry check_entry : checks().entrySet()) { - ++pos; - if (null != to_perform && !to_perform.contains(pos)) { - continue; - } - String check_name = check_entry.getKey(); - Check check = check_entry.getValue(); - - log.info(String.format("#%d. Processing [%s] at [%s]", - pos, check_name, Instant.now().toString())); - - try { - // do the stuff - check.report(ri); - store(check_name, check.took_, check.report_); - - } catch (Exception e) { - store( - check_name, - -1, - "Exception occurred when processing report - " + ExceptionUtils.getStackTrace(e) - ); - } - } - } - - // create check list - public static LinkedHashMap checks() { - LinkedHashMap checks = new LinkedHashMap<>(); - String check_names[] = DSpaceServicesFactory.getInstance().getConfigurationService() - .getArrayProperty("healthcheck.checks"); - PluginService pluginService = CoreServiceFactory.getInstance().getPluginService(); - for (String check_name : check_names) { - Check check = (Check) pluginService.getNamedPlugin( - Check.class, check_name); - if (null != check) { - checks.put(check_name, check); - } else { - log.warn(String.format( - "Could not find implementation for [%s]", check_name)); - } - } - return checks; - } - - @Override - public String toString() { - return summary_.toString(); - } - - // - private void store(String name, long took, String report) { - name += String.format(" [took: %ds] [# lines: %d]", - took / 1000, - new StringTokenizer(report, "\r\n").countTokens() - ); - - String one_summary = String.format( - "\n#### %s\n%s\n\n###############################\n", - name, - report.replaceAll("\\s+$", "") - ); - summary_.append(one_summary); - - // output it - System.out.println(one_summary); - - } - - // main - // - - public static void main(String[] args) { - log.info("Starting healthcheck report..."); - - final String option_help = "h"; - final String option_email = "e"; - final String option_check = "c"; - final String option_last_n = "f"; - final String option_verbose = "v"; - - // command line options - Options options = new Options(); - options.addOption(option_help, "help", false, - "Show available checks and their index."); - options.addOption(option_email, "email", true, - "Send report to this email address."); - options.addOption(option_check, "check", true, - "Perform only specific check (use index starting from 0)."); - options.addOption(option_last_n, "for", true, - "For last N days."); - options.addOption(option_verbose, "verbose", false, - "Verbose report."); - - CommandLine cmdline = null; - try { - cmdline = new DefaultParser().parse(options, args); - } catch (ParseException e) { - log.fatal("Invalid command line " + e.toString(), e); - System.exit(1); - } - - if (cmdline.hasOption(option_help)) { - String checks_summary = ""; - int pos = 0; - for (String check_name : checks().keySet()) { - checks_summary += String.format("%d. %s\n", pos++, check_name); - } - HelpFormatter formatter = new HelpFormatter(); - formatter.printHelp("dspace healthcheck", options); - System.out.println("\nAvailable checks:\n" + checks_summary); - return; - } - - // what to perform - List to_perform = null; - if (null != cmdline.getOptionValues(option_check)) { - to_perform = new ArrayList<>(); - for (String s : cmdline.getOptionValues('c')) { - to_perform.add(Integer.valueOf(s)); - } - } - - try { - ConfigurationService configurationService - = DSpaceServicesFactory.getInstance().getConfigurationService(); - // last n days - int for_last_n_days = configurationService.getIntProperty( - "healthcheck.last_n_days"); - if (cmdline.hasOption(option_last_n)) { - for_last_n_days = Integer.getInteger( - cmdline.getOptionValue(option_last_n)); - } - ReportInfo ri = new ReportInfo(for_last_n_days); - if (cmdline.hasOption(option_verbose)) { - ri.verbose(true); - } - - // run report - Report r = new Report(); - r.run(to_perform, ri); - log.info("reports generated..."); - - // send/output the report - if (cmdline.hasOption(option_email)) { - String to = cmdline.getOptionValue(option_email); - if (!to.contains("@")) { - to = configurationService.getProperty(to); - } - try { - String dspace_dir = configurationService.getProperty("dspace.dir"); - String email_path = dspace_dir.endsWith("/") ? dspace_dir - : dspace_dir + "/"; - email_path += Report.EMAIL_PATH; - log.info(String.format( - "Looking for email template at [%s]", email_path)); - Email email = Email.getEmail(email_path); - email.addRecipient(to); - email.addArgument(r.toString()); - email.send(); - } catch (IOException | MessagingException e) { - log.fatal("Error sending email:", e); - System.err.println("Error sending email:\n" + e.getMessage()); - System.exit(1); - } - } - - } catch (Exception e) { - log.fatal(e); - e.printStackTrace(); - } - } - -} diff --git a/dspace-api/src/main/java/org/dspace/health/UserCheck.java b/dspace-api/src/main/java/org/dspace/health/UserCheck.java index 875bb1bb0b60..27f19bb1c598 100644 --- a/dspace-api/src/main/java/org/dspace/health/UserCheck.java +++ b/dspace-api/src/main/java/org/dspace/health/UserCheck.java @@ -12,6 +12,7 @@ import java.util.List; import java.util.Map; +import com.google.common.base.CaseFormat; import org.dspace.content.Collection; import org.dspace.content.DSpaceObject; import org.dspace.content.factory.ContentServiceFactory; @@ -22,6 +23,8 @@ import org.dspace.eperson.factory.EPersonServiceFactory; import org.dspace.eperson.service.EPersonService; import org.dspace.eperson.service.GroupService; +import org.json.JSONArray; +import org.json.JSONObject; /** * @author LINDAT/CLARIN dev team @@ -32,17 +35,20 @@ public class UserCheck extends Check { private static final GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); private static final CollectionService collectionService = ContentServiceFactory.getInstance() .getCollectionService(); + private static final String HAVE_EMAIL = "Have email"; + private static final String COUNT = "Count"; @Override public String run(ReportInfo ri) { Context context = new Context(); - String ret = ""; + StringBuilder sb = new StringBuilder(); + JSONObject root = new JSONObject(); Map info = new HashMap(); try { List epersons = ePersonService.findAll(context, EPerson.LASTNAME); - info.put("Count", epersons.size()); + info.put(COUNT, epersons.size()); info.put("Can log in (password)", 0); - info.put("Have email", 0); + info.put(HAVE_EMAIL, 0); info.put("Have 1st name", 0); info.put("Have 2nd name", 0); info.put("Have lang", 0); @@ -51,7 +57,7 @@ public String run(ReportInfo ri) { for (EPerson e : epersons) { if (e.getEmail() != null && !e.getEmail().isEmpty()) { - info.put("Have email", info.get("Have email") + 1); + info.put(HAVE_EMAIL, info.get(HAVE_EMAIL) + 1); } if (e.canLogIn()) { info.put("Can log in (password)", @@ -69,7 +75,9 @@ public String run(ReportInfo ri) { if (e.getNetid() != null && !e.getNetid().isEmpty()) { info.put("Have netid", info.get("Have netid") + 1); } - if (e.getNetid() != null && !e.getNetid().isEmpty()) { + // Counted from the self-registration flag, not from netid: the netid test was a + // copy-paste of the line above it and made this number a duplicate of "Have netid". + if (e.getSelfRegistered()) { info.put("Self registered", info.get("Self registered") + 1); } } @@ -78,38 +86,54 @@ public String run(ReportInfo ri) { error(e); } - ret += String.format( - "%-20s: %d\n", "Users", info.get("Count")); - ret += String.format( - "%-20s: %d\n", "Have email", info.get("Have email")); + sb.append(String.format("%-22s: %d\n", "Users", info.get(COUNT))); + root.put("users", info.get(COUNT)); + sb.append(String.format("%-22s: %d\n", HAVE_EMAIL, info.get(HAVE_EMAIL))); + root.put("haveEmail", info.get(HAVE_EMAIL)); for (Map.Entry e : info.entrySet()) { - if (!e.getKey().equals("Count") && !e.getKey().equals("Have email")) { - ret += String.format("%-21s: %s\n", e.getKey(), - String.valueOf(e.getValue())); + if (!e.getKey().equals(COUNT) && !e.getKey().equals(HAVE_EMAIL)) { + String key = e.getKey(); + int value = e.getValue(); + sb.append(String.format("%-22s: %s\n", key, value)); + + key = toCamelCase(key); + root.put(key, value); } } - ret += "\n"; + sb.append("\n"); try { // empty group List emptyGroups = groupService.getEmptyGroups(context); - ret += String.format("Empty groups: #%d\n ", emptyGroups.size()); + sb.append(String.format("Empty groups: #%d\n ", emptyGroups.size())); + JSONArray emptyGroupsArray = new JSONArray(); for (Group group : emptyGroups) { - ret += String.format("id=%s;name=%s,\n ", group.getID(), group.getName()); + JSONObject oneEmptyGroup = new JSONObject(); + sb.append(String.format("id=%s;name=%s,\n ", group.getID(), group.getName())); + oneEmptyGroup.put("id", group.getID()); + oneEmptyGroup.put("name", group.getName()); + emptyGroupsArray.put(oneEmptyGroup); } + root.put("emptyGroups", emptyGroupsArray); + + sb.append("\n"); //subscribers List subscribers = ePersonService.findEPeopleWithSubscription(context); - ret += String.format( - "Subscribers: #%d [%s]\n", - subscribers.size(), formatIds(subscribers)); + JSONArray subsIdsArray = new JSONArray(); + sb.append(String.format("Subscribers: #%d ", subscribers.size())); + formatIds(subscribers, subsIdsArray, sb); + sb.append("\n"); + root.put("subscribers", subsIdsArray); //subscribed collections List subscribedCols = collectionService.findCollectionsWithSubscribers(context); - ret += String.format( - "Subscribed cols.: #%d [%s]\n", - subscribedCols.size(), formatIds(subscribedCols)); + JSONArray subsColsArray = new JSONArray(); + sb.append(String.format("Subscribed cols.: #%d ", subscribedCols.size())); + formatIds(subscribedCols, subsColsArray, sb); + sb.append("\n"); + root.put("subscribedCollections", subsColsArray); context.complete(); @@ -117,14 +141,42 @@ public String run(ReportInfo ri) { error(e); } - return ret; + this.setReportJson(root); + return sb.toString(); } - private String formatIds(List objects) { - StringBuilder ids = new StringBuilder(); + /** + * Formats a list of DSpace objects' IDs into both a JSON array and a human-readable string representation. + *

+ * This method takes a list of DSpace objects and extracts their IDs, adding them to: + *

    + *
  • A JSON array for programmatic access
    • + *
  • A StringBuilder in the format "[id1, id2, id3]" for logging or display purposes
    • + *
+ * + * @param objects The list of DSpace objects whose IDs should be formatted + * @param jsonOut The JSON array to which the object IDs will be added + * @param strOut The StringBuilder that will be populated with the formatted string representation of IDs + * in the format "[id1, id2, id3]" + */ + private void formatIds(List objects, JSONArray jsonOut, StringBuilder strOut) { + strOut.append("["); for (DSpaceObject o : objects) { - ids.append(o.getID()).append(", "); + strOut.append(o.getID()).append(", "); + jsonOut.put(o.getID()); + } + + //deleting last delimiter (comma and space) + if (!objects.isEmpty() && strOut.length() > 1) { + strOut.delete(strOut.length() - 2, strOut.length()); } - return ids.toString(); + + strOut.append("]"); + } + + private String toCamelCase(String str) { + str = str.toLowerCase().replace(" ", "_"); + str = CaseFormat.LOWER_UNDERSCORE.to(CaseFormat.LOWER_CAMEL, str); + return str; } } diff --git a/dspace-api/src/test/java/org/dspace/health/ReportDiffFieldMappingIT.java b/dspace-api/src/test/java/org/dspace/health/ReportDiffFieldMappingIT.java new file mode 100644 index 000000000000..768a9de2c7d7 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/health/ReportDiffFieldMappingIT.java @@ -0,0 +1,267 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.health; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.dspace.AbstractIntegrationTestWithDatabase; +import org.dspace.app.launcher.ScriptLauncher; +import org.dspace.app.scripts.handler.impl.TestDSpaceRunnableHandler; +import org.dspace.builder.CollectionBuilder; +import org.dspace.builder.CommunityBuilder; +import org.dspace.builder.ItemBuilder; +import org.dspace.content.Collection; +import org.dspace.content.Community; +import org.dspace.content.ReportResult; +import org.dspace.content.factory.ContentServiceFactory; +import org.dspace.content.service.ReportResultService; +import org.dspace.core.factory.CoreServiceFactory; +import org.dspace.services.ConfigurationService; +import org.dspace.services.factory.DSpaceServicesFactory; +import org.dspace.storage.bitstore.DSBitStoreService; +import org.dspace.utils.DSpace; +import org.junit.Before; +import org.junit.Test; + +/** + * Guards the contract between {@code report-diff-fields.json} and the health checks that are supposed to + * fill it. + *

+ * {@code report-diff} does not read the checks; it reads the JSON a health-report run stored, addressing + * every value by a path such as + * {@code /checks/[name=Item summary]/report/collectionsSizesInfo/totalSize}. Nothing links that path back + * to the class that is meant to emit it, so a check can stop emitting - or never start - and the only + * symptom is a column that is quietly always empty. That is exactly what the v9 upgrade did: it took + * {@code InfoCheck}, {@code ItemCheck} and {@code UserCheck} wholesale from vanilla, which has no + * {@code setReportJson} at all, and 23 of the 26 mappings lost their emitter without a single test + * turning red. + *

+ * These tests close that gap from both ends: every mapped path must resolve in a real health report, every + * mapped check name must resolve to a {@link Check} on the classpath, and every mapping must also appear in + * {@code fieldOrder} - {@code ReportDiff} iterates {@code fieldOrder}, so a mapping missing from it is + * never printed either. + */ +public class ReportDiffFieldMappingIT extends AbstractIntegrationTestWithDatabase { + + /** The mapping file, read from the classpath exactly as {@code ReportDiff} reads it. */ + private static final String MAPPINGS_RESOURCE = "/report-diff-fields.json"; + + private static final String FIELD_MAPPINGS = "fieldMappings"; + private static final String FIELD_ORDER = "fieldOrder"; + + /** {@code /checks/[name=]/report/} */ + private static final Pattern MAPPING_PATH = + Pattern.compile("^/?checks/\\[name=([^]]+)]/report/(.+)$"); + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + @Before + public void setUpFixture() throws Exception { + context.turnOffAuthorisationSystem(); + Community community = CommunityBuilder.createCommunity(context) + .withName("Report diff mapping community") + .build(); + Collection collection = CollectionBuilder.createCollection(context, community) + .withName("Report diff mapping collection") + .build(); + // Deliberately no bitstream: a full health report runs the Checksum check, which records a + // most_recent_checksum row for every bitstream it sees, and the foreign key from that row then + // blocks the builder teardown. Nothing this test asserts needs one. + ItemBuilder.createItem(context, collection) + .withTitle("Report diff mapping item") + .withIssueDate("2026-09-10") + .build(); + context.restoreAuthSystemState(); + context.commit(); + + // General Information reports one entry per directory and report-diff addresses them by position + // (directoryStats/0 = assetstore, directoryStats/1 = log dir). A directory that does not exist + // yields no size, so make sure both are there before the report runs. + ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); + DSBitStoreService localStore = new DSpace().getServiceManager() + .getServicesByType(DSBitStoreService.class).get(0); + ensureDirectory(localStore.getBaseDir().toString()); + ensureDirectory(configurationService.getProperty("log.report.dir")); + } + + private void ensureDirectory(String path) { + if (path == null) { + return; + } + File dir = new File(path); + if (!dir.exists()) { + assertTrue("Could not create the directory the health report reads: " + path, dir.mkdirs()); + } + } + + /** + * The point of this class: run a real health report and resolve every mapped path in what it produced. + * A check that emits no JSON, or emits it under a different key, fails here with the path named. + */ + @Test + public void everyMappedFieldIsEmittedByItsCheck() throws Exception { + JsonNode mappings = loadMappingFile().get(FIELD_MAPPINGS); + assertNotNull(MAPPINGS_RESOURCE + " has no " + FIELD_MAPPINGS, mappings); + assertTrue(MAPPINGS_RESOURCE + " maps no fields at all", mappings.size() > 0); + + JsonNode report = runHealthReport(); + + List unresolved = new ArrayList<>(); + mappings.fieldNames().forEachRemaining(path -> { + if (resolve(report, path) == null) { + unresolved.add(path + " (displayed as \"" + mappings.get(path).asText() + "\")"); + } + }); + + assertThat("report-diff-fields.json maps " + mappings.size() + " fields, but the health report does" + + " not contain " + unresolved.size() + " of them - the check that should emit them" + + " either does not call setReportJson or uses a different key:\n " + + String.join("\n ", unresolved), + unresolved, empty()); + } + + /** + * The other end of the same contract: every {@code [name=...]} in the mapping file must be a check that + * actually exists. A renamed or dropped check class leaves the mapping addressing nothing. + */ + @Test + public void everyMappedCheckNameResolvesToACheckOnTheClasspath() throws Exception { + JsonNode mappings = loadMappingFile().get(FIELD_MAPPINGS); + + Set checkNames = new LinkedHashSet<>(); + mappings.fieldNames().forEachRemaining(path -> { + Matcher matcher = MAPPING_PATH.matcher(path); + assertTrue("Mapped path is not addressable by report-diff: " + path, matcher.matches()); + checkNames.add(matcher.group(1)); + }); + assertTrue("No check names found in " + MAPPINGS_RESOURCE, checkNames.size() > 0); + + List missing = new ArrayList<>(); + for (String checkName : checkNames) { + Object plugin = CoreServiceFactory.getInstance().getPluginService() + .getNamedPlugin(Check.class, checkName); + if (!(plugin instanceof Check)) { + missing.add(checkName); + } + } + + assertThat("report-diff-fields.json addresses checks that no Check class is registered for in" + + " config/modules/healthcheck.cfg: " + missing, + missing, empty()); + } + + /** + * {@code ReportDiff} iterates {@code fieldOrder} and only looks the display name up in + * {@code fieldMappings}, so a field present in one and absent from the other is silently dropped. + */ + @Test + public void fieldMappingsAndFieldOrderDescribeTheSameFields() throws Exception { + JsonNode root = loadMappingFile(); + + Set mapped = new LinkedHashSet<>(); + root.get(FIELD_MAPPINGS).fieldNames().forEachRemaining(mapped::add); + + Set ordered = new LinkedHashSet<>(); + root.get(FIELD_ORDER).forEach(node -> ordered.add(node.asText())); + + List mappedNotOrdered = new ArrayList<>(mapped); + mappedNotOrdered.removeAll(ordered); + List orderedNotMapped = new ArrayList<>(ordered); + orderedNotMapped.removeAll(mapped); + + assertThat("Mapped but never printed, because report-diff iterates fieldOrder: " + mappedNotOrdered, + mappedNotOrdered, empty()); + assertThat("Ordered but unmapped, so report-diff prints the raw path as the column name: " + + orderedNotMapped, orderedNotMapped, empty()); + assertEquals("fieldMappings and fieldOrder must describe the same fields", + mapped.size(), ordered.size()); + } + + private JsonNode loadMappingFile() throws Exception { + try (InputStream is = ReportDiffFieldMappingIT.class.getResourceAsStream(MAPPINGS_RESOURCE)) { + assertNotNull(MAPPINGS_RESOURCE + " is not on the classpath", is); + return MAPPER.readTree(is); + } + } + + /** + * Runs the health-report script and returns the JSON it stored, i.e. the very document report-diff + * later reads. + */ + private JsonNode runHealthReport() throws Exception { + TestDSpaceRunnableHandler handler = new TestDSpaceRunnableHandler(); + String[] args = new String[] { "health-report" }; + ScriptLauncher.handleScript(args, ScriptLauncher.getConfig(kernelImpl), handler, kernelImpl); + assertThat("The health report itself failed, so nothing can be said about the mappings", + handler.getErrorMessages(), empty()); + + ReportResultService reportResultService = ContentServiceFactory.getInstance().getReportResultService(); + context.reloadEntity(eperson); + List allReports = reportResultService.findAll(context); + assertTrue("The health-report run stored no ReportResult", !allReports.isEmpty()); + // findAll() does not guarantee ordering; sort by lastModified so the newest report is last. + allReports.sort(Comparator.comparing(ReportResult::getLastModified)); + String stored = allReports.get(allReports.size() - 1).getValue(); + assertNotNull("The stored health report is empty", stored); + return MAPPER.readTree(stored); + } + + /** + * Resolves one report-diff field path against a health report, the way + * {@code ReportDiff} does: {@code /} separates segments, {@code [name=X]} selects the element of an + * array whose {@code name} is {@code X}, and a numeric segment is an array index. + * + * @param report the stored health report + * @param path a key of {@code fieldMappings} + * @return the addressed node, or {@code null} if any segment does not resolve + */ + private JsonNode resolve(JsonNode report, String path) { + JsonNode current = report; + for (String segment : path.split("/")) { + if (segment.isEmpty()) { + continue; + } + if (current == null) { + return null; + } + if (segment.startsWith("[name=") && segment.endsWith("]")) { + String wanted = segment.substring("[name=".length(), segment.length() - 1); + JsonNode found = null; + for (JsonNode candidate : current) { + JsonNode name = candidate.get("name"); + if (name != null && wanted.equals(name.asText())) { + found = candidate; + break; + } + } + current = found; + } else if (current.isArray() && segment.matches("\\d+")) { + current = current.get(Integer.parseInt(segment)); + } else { + current = current.get(segment); + } + } + return current; + } +} diff --git a/dspace/config/launcher.xml b/dspace/config/launcher.xml index 9cfda286b54d..6474c20338bd 100644 --- a/dspace/config/launcher.xml +++ b/dspace/config/launcher.xml @@ -7,13 +7,6 @@ org.dspace.storage.bitstore.BitStoreMigrate - - healthcheck - Create health check report - - org.dspace.health.Report - - checker Run the checksum checker