diff --git a/.kiro/steering/analyzer.md b/.kiro/steering/analyzer.md index 9de71e6..a3ffce4 100644 --- a/.kiro/steering/analyzer.md +++ b/.kiro/steering/analyzer.md @@ -24,11 +24,10 @@ Java 11+, Maven 3.8+. SNAPSHOT dependencies on `eforms-core-java` and `efx-toolk ```bash java -jar target/eforms-sdk-analyzer-*-all.jar java -jar target/eforms-sdk-analyzer-*-all.jar --skip-efx # skip slow EFX pass -java -jar target/eforms-sdk-analyzer-*-all.jar --verbose # full findings list java -jar target/eforms-sdk-analyzer-*-all.jar benchmark # schematron perf ``` -Framework logs go to stderr; the report goes to stdout. A full INFO log is written to `analyzer.log` in the working directory. +Framework logs go to stderr; the summary goes to stdout. The summary and the full per-finding detail are also written to `analyzer-summary.txt` and `analyzer-report.txt`, and a full INFO log to `analyzer.log` — all in the working directory. ## Architecture @@ -82,7 +81,7 @@ when ... ### Report output -`SummaryReportRenderer` groups findings by SDK section and by problem statement. `DetailReportRenderer` adds the full list under `--verbose`. +Both the console and the files open with a title banner (SDK version, analyser version, run time in UTC). `SummaryReportRenderer` groups findings by SDK section and by problem statement; this summary goes to the console and `analyzer-summary.txt`. `DetailReportRenderer` writes the full per-finding list to `analyzer-report.txt`. ## Adding a new rule diff --git a/README.md b/README.md index e4c807e..80be82e 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ java -jar target/eforms-sdk-analyzer-*-all.jar path/to/eforms-sdk This will return the exit code 0 if no errors are found, and 1 otherwise. -The analysis writes a report to the standard output: a summary (findings grouped by SDK section and by problem statement) followed by the actionable items. Add `--verbose` to also print the full, unaggregated list of every finding. Add `--skip-efx` to skip the EFX translation pass, which is by far the slowest. Framework logs go to the standard error, and a full INFO log is written to `analyzer.log` in the working directory. +Each report — on the console and in both files — opens with a title banner showing the SDK version, the analyser's own version, and the run time (UTC). The analysis writes a summary to the standard output: findings grouped by SDK section and by problem statement, followed by the actionable items. The same summary is also written to `analyzer-summary.txt`, and the full, unaggregated list of every finding to `analyzer-report.txt`, both in the working directory. Add `--skip-efx` to skip the EFX translation pass, which is by far the slowest. Framework logs go to the standard error, and a full INFO log is written to `analyzer.log` in the working directory. ### Schematron rules benchmark diff --git a/src/main/java/eu/europa/ted/eforms/sdk/analysis/CliCommand.java b/src/main/java/eu/europa/ted/eforms/sdk/analysis/CliCommand.java index 551efcd..4e994dd 100644 --- a/src/main/java/eu/europa/ted/eforms/sdk/analysis/CliCommand.java +++ b/src/main/java/eu/europa/ted/eforms/sdk/analysis/CliCommand.java @@ -26,12 +26,6 @@ class CliCommand implements Callable { @Parameters(index = "0", description = "SDK resources root folder.") private Path sdkRoot; - @Option(names = {"-v", "--verbose"}, - description = "List every individual finding and show framework logging (INFO) on the console." - + " A full INFO run log is written to analyzer.log in the working directory when it is" - + " writable, regardless of this flag.") - private boolean verbose; - @Option(names = {"--skip-efx"}, description = "Skip the EFX expression validation of view templates. It is the slowest" + " validator and is independent of the rule engine, so skipping it greatly speeds up" @@ -40,14 +34,14 @@ class CliCommand implements Callable { @Override public Integer call() throws Exception { - new LoggingConfigurator().configure(this.verbose); - return SdkAnalyzer.analyze(this.sdkRoot, this.verbose, this.skipEfx); + new LoggingConfigurator().configure(); + return SdkAnalyzer.analyze(this.sdkRoot, this.skipEfx); } @Command(name = "benchmark", mixinStandardHelpOptions = true, description = "Run benchmark of Schematron rules") public int runBenchmark() throws Exception { - new LoggingConfigurator().configure(this.verbose); + new LoggingConfigurator().configure(); return SchematronBenchmark.run(this.sdkRoot); } diff --git a/src/main/java/eu/europa/ted/eforms/sdk/analysis/LoggingConfigurator.java b/src/main/java/eu/europa/ted/eforms/sdk/analysis/LoggingConfigurator.java index 9eae030..7793686 100644 --- a/src/main/java/eu/europa/ted/eforms/sdk/analysis/LoggingConfigurator.java +++ b/src/main/java/eu/europa/ted/eforms/sdk/analysis/LoggingConfigurator.java @@ -14,13 +14,11 @@ import ch.qos.logback.core.FileAppender; /** - * Configures logging for the analyzer CLI. Framework logging is kept off the console by default - * (only warnings and errors show) and raised to INFO with {@code --verbose}, while a full INFO run - * log is written to {@value #LOG_FILE} in the working directory (when it is writable) for later - * inspection. The validation report itself is written separately to stdout by - * {@link eu.europa.ted.eforms.sdk.analysis.report.SummaryReportRenderer} (the summary and actionable - * items) and {@link eu.europa.ted.eforms.sdk.analysis.report.DetailReportRenderer} (the full list - * under {@code --verbose}). + * Configures logging for the analyzer CLI. Framework logging is kept off stdout — only warnings and + * errors show, on stderr — while a full INFO run log is written to {@value #LOG_FILE} in the working + * directory (when it is writable) for later inspection. The validation report itself goes to stdout + * (the summary, via {@link eu.europa.ted.eforms.sdk.analysis.report.SummaryReportRenderer}) and to the + * {@code analyzer-summary.txt} / {@code analyzer-report.txt} files, separate from logging. * *

Applied programmatically by the CLI only, so applications that use the analyzer as a library * keep their own logging configuration; the library jar ships no {@code logback.xml}. @@ -30,7 +28,7 @@ public class LoggingConfigurator { private static final String LOG_FILE = "analyzer.log"; private static final String PATTERN = "%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n"; - public void configure(final boolean verbose) { + public void configure() { final LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory(); context.reset(); @@ -39,7 +37,7 @@ public void configure(final boolean verbose) { // Framework logs go to stderr so stdout stays dedicated to the report. console.setTarget("System.err"); console.setEncoder(encoder(context)); - console.addFilter(thresholdFilter(context, verbose ? Level.INFO : Level.WARN)); + console.addFilter(thresholdFilter(context, Level.WARN)); console.start(); final Logger root = context.getLogger(ROOT_LOGGER); diff --git a/src/main/java/eu/europa/ted/eforms/sdk/analysis/SdkAnalyzer.java b/src/main/java/eu/europa/ted/eforms/sdk/analysis/SdkAnalyzer.java index 20fca25..e708fe6 100644 --- a/src/main/java/eu/europa/ted/eforms/sdk/analysis/SdkAnalyzer.java +++ b/src/main/java/eu/europa/ted/eforms/sdk/analysis/SdkAnalyzer.java @@ -1,20 +1,27 @@ package eu.europa.ted.eforms.sdk.analysis; import java.io.IOException; +import java.io.InputStream; import java.io.PrintStream; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Properties; +import java.util.function.Consumer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import eu.europa.ted.eforms.sdk.analysis.enums.ValidationStatusEnum; import eu.europa.ted.eforms.sdk.analysis.fact.ValidatorFact; import eu.europa.ted.eforms.sdk.analysis.report.DetailReportRenderer; import eu.europa.ted.eforms.sdk.analysis.report.SummaryReportRenderer; +import eu.europa.ted.eforms.sdk.analysis.util.SdkMetadataParser; import eu.europa.ted.eforms.sdk.analysis.validator.EfxValidator; import eu.europa.ted.eforms.sdk.analysis.validator.SchematronValidator; import eu.europa.ted.eforms.sdk.analysis.validator.SdkValidator; @@ -27,24 +34,32 @@ public class SdkAnalyzer { private static final Logger logger = LoggerFactory.getLogger(SdkAnalyzer.class); - /** The console summary is also written here (in the working directory), alongside analyzer.log. */ + /** The consumable summary is always written here (in the working directory), alongside analyzer.log. */ private static final String SUMMARY_FILE = "analyzer-summary.txt"; - /** The full report is always written here (in the working directory), alongside analyzer.log. */ + /** The full per-finding detail is always written here (in the working directory). */ private static final String REPORT_FILE = "analyzer-report.txt"; + /** Maven stamps the build version here in every jar; read for the analyser version on the banner. */ + private static final String POM_PROPERTIES = + "/META-INF/maven/eu.europa.ted.eforms/eforms-sdk-analyzer/pom.properties"; + private SdkAnalyzer() {} + /** Runs the full analysis of the SDK at {@code sdkRoot} — every validator, including EFX. */ public static int analyze(final Path sdkRoot) throws Exception { return analyze(sdkRoot, false); } - public static int analyze(final Path sdkRoot, final boolean verbose) throws Exception { - return analyze(sdkRoot, verbose, false); - } - - public static int analyze(final Path sdkRoot, final boolean verbose, final boolean skipEfx) - throws Exception { + /** + * Runs the analysis of the SDK at {@code sdkRoot}, optionally skipping EFX. Package-private: the + * public entry point is {@link #analyze(Path)} (a full analysis); skipping EFX is an internal + * debugging option, exposed on the command line via {@code --skip-efx}. + * + * @param skipEfx when {@code true}, skips the slow EFX translation pass; the other validators are + * unaffected. + */ + static int analyze(final Path sdkRoot, final boolean skipEfx) throws Exception { logger.info("Analyzing SDK under folder [{}]", sdkRoot); // Each validator is built lazily inside the guarded run loop, so a constructor failure (e.g. a @@ -70,34 +85,108 @@ public static int analyze(final Path sdkRoot, final boolean verbose, final boole } final AnalysisResults results = AnalysisResults.of(runValidators(validators)); - // Lead with the summary and the actionable items; the full, unaggregated list of findings - // follows only under --verbose. DetailReportRenderer carries that closing detail and the - // headline total, so the summary is always what the reader sees first. - new SummaryReportRenderer().render(results); - new DetailReportRenderer().render(results, verbose); - - // Always write the report to fixed files in the working directory — like analyzer.log — so CI can - // upload them as artifacts while the console keeps the concise summary: the summary (as shown on - // the console) and the full report (with every finding). A write failure is logged, not fatal. - writeReport(results, Path.of(SUMMARY_FILE), false); - writeReport(results, Path.of(REPORT_FILE), true); + // The banner is built once so its timestamp is identical on the console and in both files: the SDK + // being analysed on the title line, the analyser's own version and the run time on the subtitle. + final String title = reportTitle(sdkRoot); + final String subtitle = reportSubtitle(); + + // The console shows the consumable summary (per-section/per-problem counts and actionable items). + // The same summary and the full per-finding detail are written to separate fixed files in the + // working directory — like analyzer.log — so CI can upload them as separate artifacts: open + // whichever you need. A write failure is logged, not fatal. + renderSummary(System.out, results, title, subtitle); + writeFile(Path.of(SUMMARY_FILE), out -> renderSummary(out, results, title, subtitle)); + writeFile(Path.of(REPORT_FILE), out -> renderDetail(out, results, title, subtitle)); return results.exitCode(); } - /** - * Writes the report to {@code file}: the summary and actionable items, then the per-finding list when - * {@code verbose}. The summary file uses {@code verbose=false} (the console view); the full report - * file uses {@code verbose=true} (every finding). - */ - private static void writeReport(final AnalysisResults results, final Path file, - final boolean verbose) { + /** Writes {@code render}'s output to {@code file}; a write failure is logged, not fatal. */ + private static void writeFile(final Path file, final Consumer render) { try (PrintStream out = new PrintStream(Files.newOutputStream(file), false, StandardCharsets.UTF_8)) { - new SummaryReportRenderer(out).render(results); - new DetailReportRenderer(out).render(results, verbose); + render.accept(out); + } catch (final IOException e) { + logger.warn("Could not write [{}]: {}", file, e.getMessage()); + } + } + + /** + * The consumable summary: the title banner, the per-section and per-problem counts and the actionable + * items, then a pointer to the detail file (so a reader who needs every finding knows where it is). + * Package-private so the detail-report pointer can be exercised in isolation. + */ + static void renderSummary(final PrintStream out, final AnalysisResults results, + final String title, final String subtitle) { + renderTitle(out, title, subtitle); + new SummaryReportRenderer(out).render(results); + new DetailReportRenderer(out).render(results, false); + // Point to the detail report whenever it has something to show — warning-only runs (no errors, + // so isClean() is true) still list those warnings in the detail file. + if (!results.isClean() || results.warningCount() > 0) { + out.println("The full list of findings is in " + REPORT_FILE + "."); + } + } + + /** The full detail: the title banner and every individual finding — no summary (that is its own file). */ + private static void renderDetail(final PrintStream out, final AnalysisResults results, + final String title, final String subtitle) { + renderTitle(out, title, subtitle); + new DetailReportRenderer(out).render(results, true); + } + + /** + * The title banner: a rule, the title line, a rule, then the subtitle beneath. The rules are sized to + * the wider of the two lines so both sit within the banner. + */ + private static void renderTitle(final PrintStream out, final String title, final String subtitle) { + final String rule = "-".repeat(Math.max(title.length(), subtitle.length())); + out.println(rule); + out.println(title); + out.println(rule); + out.println(subtitle); + } + + /** The banner's title line: the SDK being analysed (version from its metadata) and the report name. */ + private static String reportTitle(final Path sdkRoot) { + return "eForms SDK " + sdkVersion(sdkRoot) + " - SDK Analyser Report"; + } + + /** The banner's subtitle line: the analyser's own version and the run time, in UTC. */ + private static String reportSubtitle() { + final String when = ZonedDateTime.now(ZoneOffset.UTC) + .format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm 'UTC'")); + return "Analyser " + analyzerVersion() + " - " + when; + } + + /** + * The analyser's own build version, read from the Maven {@code pom.properties} bundled in the jar and + * formatted as {@code vX.Y.Z}. Falls back to a placeholder when it cannot be read (e.g. running from + * unpackaged classes in the IDE or tests, where {@code pom.properties} is absent). + */ + private static String analyzerVersion() { + try (InputStream in = SdkAnalyzer.class.getResourceAsStream(POM_PROPERTIES)) { + if (in != null) { + final Properties properties = new Properties(); + properties.load(in); + final String version = properties.getProperty("version"); + if (version != null && !version.isBlank()) { + return "v" + version; + } + } } catch (final IOException e) { - logger.warn("Could not write the report to [{}]: {}", file, e.getMessage()); + logger.debug("Could not read the analyser version: {}", e.getMessage()); + } + return "(version unknown)"; + } + + /** The SDK version from its metadata, or a placeholder when it cannot be read. */ + private static String sdkVersion(final Path sdkRoot) { + try { + return SdkMetadataParser.loadSdkMetadata(sdkRoot).getVersion(); + } catch (final Exception e) { + logger.debug("Could not read the SDK version: {}", e.getMessage()); + return "(unknown version)"; } } diff --git a/src/main/java/eu/europa/ted/eforms/sdk/analysis/fact/FieldFact.java b/src/main/java/eu/europa/ted/eforms/sdk/analysis/fact/FieldFact.java index fc8684b..e913a2b 100644 --- a/src/main/java/eu/europa/ted/eforms/sdk/analysis/fact/FieldFact.java +++ b/src/main/java/eu/europa/ted/eforms/sdk/analysis/fact/FieldFact.java @@ -322,6 +322,11 @@ public String getId() { return field.getId(); } + /** The id of the business term this field belongs to (its {@code btId}), or {@code null}. */ + public String getBtId() { + return field.getBtId(); + } + @Override public String getTypeName() { return "field"; diff --git a/src/main/java/eu/europa/ted/eforms/sdk/analysis/report/DetailReportRenderer.java b/src/main/java/eu/europa/ted/eforms/sdk/analysis/report/DetailReportRenderer.java index dfd0b1b..098d2c9 100644 --- a/src/main/java/eu/europa/ted/eforms/sdk/analysis/report/DetailReportRenderer.java +++ b/src/main/java/eu/europa/ted/eforms/sdk/analysis/report/DetailReportRenderer.java @@ -5,13 +5,11 @@ import eu.europa.ted.eforms.sdk.analysis.vo.AnalysisResults; /** - * Renders the report's headline and, under {@code --verbose}, the full unaggregated list of findings. - * - *

The summary (per-section and per-problem counts) and the actionable items are produced by - * {@link SummaryReportRenderer} and printed first. This renderer closes the report: the grand total of - * errors (or, on a clean run, the all-clear message) and — only when verbose — every individual - * finding listed in full after the summary. Writing the report here rather than through the logger - * keeps it free of log decoration and separate from framework logging. + * Renders the closing headline — the grand total of errors (or, on a clean run, the all-clear + * message) — and, when {@code full}, the complete list of every individual finding. The summary view + * uses it without the list (just the headline); the detail file uses it with the full list. Writing + * this here rather than through the logger keeps it free of log decoration and separate from framework + * logging. */ public class DetailReportRenderer { private final PrintStream out; @@ -24,15 +22,14 @@ public DetailReportRenderer(final PrintStream out) { this.out = out; } - public void render(final AnalysisResults results, final boolean verbose) { + public void render(final AnalysisResults results, final boolean full) { if (results.isClean() && results.warningCount() == 0) { this.out.println("No validation errors or warnings found."); return; } - // The default run stops at the summary and actionable items (rendered by SummaryReportRenderer); - // verbose adds the full, unaggregated list of every individual finding underneath them. - if (verbose) { + // The summary view shows only the headline; the detail view (full) lists every individual finding. + if (full) { listEveryFinding(results); } @@ -41,9 +38,6 @@ public void render(final AnalysisResults results, final boolean verbose) { if (!results.isClean()) { this.out.println("Total number of validation errors: " + results.errorCount()); } - if (!verbose) { - this.out.println("Re-run the analyzer with --verbose to see every individual finding."); - } } /** Every finding in full, errors before warnings — matching the order of the summary above. */ diff --git a/src/main/java/eu/europa/ted/eforms/sdk/analysis/report/JsonReportRenderer.java b/src/main/java/eu/europa/ted/eforms/sdk/analysis/report/JsonReportRenderer.java new file mode 100644 index 0000000..76ee575 --- /dev/null +++ b/src/main/java/eu/europa/ted/eforms/sdk/analysis/report/JsonReportRenderer.java @@ -0,0 +1,120 @@ +package eu.europa.ted.eforms.sdk.analysis.report; + +import java.util.Comparator; +import java.util.List; +import java.util.Locale; +import java.util.Map; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; + +import eu.europa.ted.eforms.sdk.analysis.enums.ValidationStatusEnum; +import eu.europa.ted.eforms.sdk.analysis.vo.AnalysisResults; +import eu.europa.ted.eforms.sdk.analysis.vo.AssetRef; +import eu.europa.ted.eforms.sdk.analysis.vo.Finding; +import eu.europa.ted.eforms.sdk.analysis.vo.SdkSection; +import eu.europa.ted.eforms.sdk.analysis.vo.ValidationResult; + +/** + * Renders an {@link AnalysisResults} as a JSON string, for programmatic consumers — primarily the + * Metadata Manager, which displays the findings in a table so a user can review the analysis before + * exporting an SDK. The shape is a small {@code summary} (counts, plus per-section counts) and a flat + * {@code findings} array — one self-describing object per finding (section, severity, rule, problem, + * subject, referenced assets, message) so a UI can bind, sort and filter (including by section) without + * re-deriving anything. + * + *

A deliberately simple starting contract; consumers are free to ask for more fields as the UI takes + * shape. Within each section the findings are ordered by message, so the output is stable across runs. + */ +public class JsonReportRenderer { + private final ObjectMapper mapper = new ObjectMapper(); + + /** The analysis as a pretty-printed JSON string: {@code { summary, findings }}. */ + public String render(final AnalysisResults results) { + final ObjectNode root = this.mapper.createObjectNode(); + root.set("summary", summary(results)); + root.set("findings", findings(results)); + try { + return this.mapper.writerWithDefaultPrettyPrinter().writeValueAsString(root); + } catch (final JsonProcessingException e) { + throw new IllegalStateException("Could not render the analysis as JSON", e); + } + } + + /** Headline counts plus a per-section breakdown (only sections that carry a finding, in section order). */ + private ObjectNode summary(final AnalysisResults results) { + final ObjectNode summary = this.mapper.createObjectNode(); + summary.put("errors", results.errorCount()); + summary.put("warnings", results.warningCount()); + summary.put("clean", results.isClean()); + + final Map> errors = results.errorFindingsBySection(); + final Map> warnings = results.warningFindingsBySection(); + final ArrayNode sections = summary.putArray("sections"); + for (final SdkSection section : SdkSection.values()) { + final int sectionErrors = sizeOf(errors.get(section)); + final int sectionWarnings = sizeOf(warnings.get(section)); + if (sectionErrors > 0 || sectionWarnings > 0) { + final ObjectNode node = sections.addObject(); + node.put("section", section.getTitle()); + node.put("errors", sectionErrors); + node.put("warnings", sectionWarnings); + } + } + return summary; + } + + /** Every finding as a flat array — errors first (by section), then warnings (by section). */ + private ArrayNode findings(final AnalysisResults results) { + final ArrayNode findings = this.mapper.createArrayNode(); + appendFindings(findings, results.errorFindingsBySection()); + appendFindings(findings, results.warningFindingsBySection()); + return findings; + } + + private void appendFindings(final ArrayNode array, + final Map> bySection) { + for (final Map.Entry> entry : bySection.entrySet()) { + entry.getValue().stream() + .sorted(Comparator.comparing(finding -> String.valueOf(finding.getResult().getMessage()))) + .forEach(finding -> array.add(findingNode(entry.getKey(), finding))); + } + } + + private ObjectNode findingNode(final SdkSection section, final Finding finding) { + final ValidationResult result = finding.getResult(); + final ObjectNode node = this.mapper.createObjectNode(); + node.put("section", section.getTitle()); + node.put("severity", severity(result.getStatus())); + node.put("rule", finding.getKind()); + node.put("problem", problemOf(finding)); + node.set("subject", assetNode(result.getSubject())); + final ArrayNode references = node.putArray("references"); + result.getReferences().forEach(reference -> references.add(assetNode(reference))); + node.put("message", result.getMessage()); + return node; + } + + private ObjectNode assetNode(final AssetRef asset) { + final ObjectNode node = this.mapper.createObjectNode(); + node.put("type", asset.getType()); + node.put("id", asset.getId()); + return node; + } + + /** The display problem statement, never null (so the UI always has a column value to show). */ + private static String problemOf(final Finding finding) { + final String problem = finding.getProblemOrKind(); + return problem != null ? problem : "(unknown)"; + } + + private static String severity(final ValidationStatusEnum status) { + return status.name().toLowerCase(Locale.ROOT); + } + + private static int sizeOf(final List findings) { + return findings == null ? 0 : findings.size(); + } +} diff --git a/src/main/java/eu/europa/ted/eforms/sdk/analysis/report/SummaryReportRenderer.java b/src/main/java/eu/europa/ted/eforms/sdk/analysis/report/SummaryReportRenderer.java index 627168f..444f591 100644 --- a/src/main/java/eu/europa/ted/eforms/sdk/analysis/report/SummaryReportRenderer.java +++ b/src/main/java/eu/europa/ted/eforms/sdk/analysis/report/SummaryReportRenderer.java @@ -17,9 +17,9 @@ * The report's lead: a per-section summary, then actionable items grouped by SDK section and, within * each section, by the problem statement (the rule's {@code @problem}, falling back * to its name). Grouping by the problem — not the rule — lets rules that report the same thing (e.g. a - * field and a business entity both referencing a missing label) fuse into one group. The full, - * unaggregated list of every finding follows this (under {@code --verbose}) via - * {@link DetailReportRenderer}, which also prints the closing headline total. + * field and a business entity both referencing a missing label) fuse into one group. The closing + * headline total follows this via {@link DetailReportRenderer}; the full, unaggregated list of every + * finding is written separately to {@code analyzer-report.txt}. * *

Each group is a one-line header (problem + the count), then one line per asset to act on: the * referenced asset (the thing to add or fix), shown once, with the facts that reference it. When a diff --git a/src/main/java/eu/europa/ted/eforms/sdk/analysis/validator/EfxValidator.java b/src/main/java/eu/europa/ted/eforms/sdk/analysis/validator/EfxValidator.java index 5cb10e3..91533ad 100644 --- a/src/main/java/eu/europa/ted/eforms/sdk/analysis/validator/EfxValidator.java +++ b/src/main/java/eu/europa/ted/eforms/sdk/analysis/validator/EfxValidator.java @@ -166,7 +166,8 @@ private void validateFieldExpressions(final Field field) { /** * Records an EFX failure: the full message goes on the {@link ValidationResult} (shown verbatim only - * in {@code --verbose}), while the {@link Finding}'s problem is the failure's category, + * in the detail report, {@code analyzer-report.txt}), while the {@link Finding}'s problem is the + * failure's category, * derived from the exception type — so the summary and actionable items group EFX errors by kind * rather than by their unique per-instance text. */ @@ -237,7 +238,7 @@ private static String symbolErrorType(final SymbolResolutionException error) { /** * Applies {@code action} to every item — across {@link #threadCount()} worker threads when * {@code parallel}, otherwise serially — logging progress at INFO so a run can be watched live (via - * {@code analyzer.log} or {@code --verbose}). Each action handles its own errors (recorded as + * {@code analyzer.log}). Each action handles its own errors (recorded as * findings), so no exception escapes the workers; only an unexpected framework failure surfaces here. * The progress counter is the key debugging aid: a climbing count means it is working; a frozen count * pinpoints (to within one report step) where it is stuck. diff --git a/src/main/java/eu/europa/ted/eforms/sdk/analysis/validator/SchematronValidator.java b/src/main/java/eu/europa/ted/eforms/sdk/analysis/validator/SchematronValidator.java index e5e1bfb..1d3aa61 100644 --- a/src/main/java/eu/europa/ted/eforms/sdk/analysis/validator/SchematronValidator.java +++ b/src/main/java/eu/europa/ted/eforms/sdk/analysis/validator/SchematronValidator.java @@ -191,7 +191,8 @@ private String describeCollectedErrors(final CollectingPSErrorHandler errorHandl /** * Records a Schematron failure: the full, detailed message (which file, which line, which XPath) - * goes on the {@link ValidationResult}, shown verbatim only under {@code --verbose}; the + * goes on the {@link ValidationResult}, shown verbatim only in the detail report + * ({@code analyzer-report.txt}); the * {@link Finding}'s problem is the stable {@link Problem} category, so the summary and actionable * items group failures by category rather than by their unique per-file text. */ diff --git a/src/main/resources/eu/europa/ted/eforms/sdk/analysis/drools/fieldsAndNodesRules.drl b/src/main/resources/eu/europa/ted/eforms/sdk/analysis/drools/fieldsAndNodesRules.drl index 55b8f45..f1c0cfa 100644 --- a/src/main/resources/eu/europa/ted/eforms/sdk/analysis/drools/fieldsAndNodesRules.drl +++ b/src/main/resources/eu/europa/ted/eforms/sdk/analysis/drools/fieldsAndNodesRules.drl @@ -206,6 +206,28 @@ then results.add(new ValidationResult($f, "Referenced label " + $labelId + " does not exist", ValidationStatusEnum.ERROR, AssetRef.label($labelId))); end +// TEDEMD-90: a field shown in a notice type definition should have a description label — on the field +// itself, or failing that on its business term. Database-only: a content-completeness check for the +// metadata editor, not an SDK packaging invariant, so it does not run against an exported SDK. +// +// Checks the "description" label, as the original MDM implementation did. The ticket spoke of the +// field "tooltip", which is the "hint" label rather than "description" — so whether this should check +// description or hint is still to be confirmed against the original user request. +rule "Fields used in notice types have a description label" + @source(DATABASE) + @problem("Field used in a notice type has no description label") +when + $field : /fields[ $fieldId: id ] + exists /noticeTypes[ contentFieldIds contains $fieldId ] + $fieldLabelId : String() from ("field|description|" + $fieldId) + $btLabelId : String() from ("business-term|description|" + $field.btId) + not (exists /labels[ id == $fieldLabelId ]) + not (exists /labels[ id == $btLabelId ]) +then + results.add(new ValidationResult($field, "Field " + $fieldId + + " is used in a notice type but has no description label", ValidationStatusEnum.ERROR)); +end + rule "Fields corresponding to XML attributes have the expected information" @problem("Attribute field is missing attributeName or attributeOf") when diff --git a/src/test/java/eu/europa/ted/eforms/sdk/analysis/SdkAnalyzerTest.java b/src/test/java/eu/europa/ted/eforms/sdk/analysis/SdkAnalyzerTest.java index 2213962..f7eb49e 100644 --- a/src/test/java/eu/europa/ted/eforms/sdk/analysis/SdkAnalyzerTest.java +++ b/src/test/java/eu/europa/ted/eforms/sdk/analysis/SdkAnalyzerTest.java @@ -1,8 +1,12 @@ package eu.europa.ted.eforms.sdk.analysis; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.nio.charset.StandardCharsets; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -15,6 +19,7 @@ import eu.europa.ted.eforms.sdk.analysis.enums.ValidationStatusEnum; import eu.europa.ted.eforms.sdk.analysis.fact.LabelFact; import eu.europa.ted.eforms.sdk.analysis.validator.Validator; +import eu.europa.ted.eforms.sdk.analysis.vo.AnalysisResults; import eu.europa.ted.eforms.sdk.analysis.vo.Finding; import eu.europa.ted.eforms.sdk.analysis.vo.SdkSection; import eu.europa.ted.eforms.sdk.analysis.vo.ValidationResult; @@ -95,7 +100,7 @@ void aCrashingValidatorBecomesAFindingAndDoesNotAbortTheRun() { assertEquals(SdkSection.ANALYZER, SdkSection.forType(crash.getResult().getSubject().getType())); assertEquals("ThrowingValidator", crash.getResult().getSubject().getId()); - // The reason (exception type and message) is preserved on the result for the verbose dump. + // The reason (exception type and message) is preserved on the result for the detail report. final String message = crash.getResult().getMessage(); assertTrue(message.contains("ThrowingValidator did not complete"), message); assertTrue(message.contains("IllegalStateException"), message); @@ -136,4 +141,32 @@ void findingsCollectedBeforeACrashAreNotDiscarded() { assertTrue(findings.stream().anyMatch(f -> "A validator failed to run".equals(f.getProblem())), findings.toString()); } + + @Test + void theSummaryPointsToTheDetailReportWhenThereAreFindings() { + final LabelFact fact = new LabelFact(new Label("code|name|x")); + final String pointer = "The full list of findings is in analyzer-report.txt"; + + // A warning-only run has no errors (isClean() is true), but the detail report still lists those + // warnings — so the summary must point to it. + final String warningOnly = summaryOf(new AnalysisResults( + List.of(new ValidationResult(fact, "a warning", ValidationStatusEnum.WARNING)))); + assertTrue(warningOnly.contains(pointer), warningOnly); + + final String withError = summaryOf(new AnalysisResults( + List.of(new ValidationResult(fact, "an error", ValidationStatusEnum.ERROR)))); + assertTrue(withError.contains(pointer), withError); + + // A fully clean run has nothing to investigate, so it gives no pointer. + final String clean = summaryOf(new AnalysisResults(List.of())); + assertFalse(clean.contains(pointer), clean); + } + + /** Renders the console summary to a string, so the detail-report pointer can be asserted. */ + private static String summaryOf(final AnalysisResults results) { + final ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + SdkAnalyzer.renderSummary(new PrintStream(buffer, true, StandardCharsets.UTF_8), results, + "title", "subtitle"); + return buffer.toString(StandardCharsets.UTF_8); + } } diff --git a/src/test/java/eu/europa/ted/eforms/sdk/analysis/report/DetailReportRendererTest.java b/src/test/java/eu/europa/ted/eforms/sdk/analysis/report/DetailReportRendererTest.java index 6b28cbd..3ebe38e 100644 --- a/src/test/java/eu/europa/ted/eforms/sdk/analysis/report/DetailReportRendererTest.java +++ b/src/test/java/eu/europa/ted/eforms/sdk/analysis/report/DetailReportRendererTest.java @@ -35,21 +35,21 @@ private AnalysisResults sampleResults() { return new AnalysisResults(List.of(found, assumed, other, warning)); } - private String render(final boolean verbose) { + private String render(final boolean full) { final ByteArrayOutputStream buffer = new ByteArrayOutputStream(); new DetailReportRenderer(new PrintStream(buffer, true, StandardCharsets.UTF_8)) - .render(sampleResults(), verbose); + .render(sampleResults(), full); return buffer.toString(StandardCharsets.UTF_8); } @Test - void defaultReportShowsOnlyTheHeadlineAndVerboseHint() { + void defaultReportShowsOnlyTheHeadline() { final String output = render(false); // The default run leads with the summary and actionable items (rendered by SummaryReportRenderer); - // this renderer adds only the closing headline, so no individual finding text appears here. + // this renderer adds only the closing headline — no individual finding text, and no pointer to the + // full report (that is added by SdkAnalyzer, which owns the report file). assertTrue(output.contains("Total number of validation errors: 3"), output); - assertTrue(output.contains("--verbose"), output); // No per-finding list and no per-message grouping in the default console output. assertFalse(output.contains("All validation errors"), output); assertFalse(output.contains("in sdkVersion is incorrect"), output); @@ -58,7 +58,7 @@ void defaultReportShowsOnlyTheHeadlineAndVerboseHint() { } @Test - void verboseReportListsEveryFinding() { + void fullReportListsEveryFinding() { final String output = render(true); assertTrue(output.contains("All validation errors (3)"), output); @@ -68,6 +68,5 @@ void verboseReportListsEveryFinding() { // Warnings are listed in full too, after the errors. assertTrue(output.contains("All validation warnings (1)"), output); assertTrue(output.contains("a warning"), output); - assertFalse(output.contains("--verbose"), output); } } diff --git a/src/test/java/eu/europa/ted/eforms/sdk/analysis/report/JsonReportRendererTest.java b/src/test/java/eu/europa/ted/eforms/sdk/analysis/report/JsonReportRendererTest.java new file mode 100644 index 0000000..2bc4938 --- /dev/null +++ b/src/test/java/eu/europa/ted/eforms/sdk/analysis/report/JsonReportRendererTest.java @@ -0,0 +1,83 @@ +package eu.europa.ted.eforms.sdk.analysis.report; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + +import eu.europa.ted.eforms.sdk.analysis.domain.label.Label; +import eu.europa.ted.eforms.sdk.analysis.enums.ValidationStatusEnum; +import eu.europa.ted.eforms.sdk.analysis.fact.LabelFact; +import eu.europa.ted.eforms.sdk.analysis.vo.AnalysisResults; +import eu.europa.ted.eforms.sdk.analysis.vo.AssetRef; +import eu.europa.ted.eforms.sdk.analysis.vo.Finding; +import eu.europa.ted.eforms.sdk.analysis.vo.ValidationResult; + +class JsonReportRendererTest { + + private final ObjectMapper mapper = new ObjectMapper(); + private final LabelFact fact = new LabelFact(new Label("code|name|x")); + + private AnalysisResults sampleResults() { + final Finding missingLabel = new Finding("Field references an existing label", + "Label is referenced but is missing", + new ValidationResult(this.fact, "Referenced label business-entity|name|UBO does not exist", + ValidationStatusEnum.ERROR, List.of(AssetRef.label("business-entity|name|UBO")))); + final Finding warning = new Finding("Some advisory rule", "An advisory problem", + new ValidationResult(this.fact, "a warning", ValidationStatusEnum.WARNING)); + return AnalysisResults.of(List.of(missingLabel, warning)); + } + + private JsonNode render() throws Exception { + return this.mapper.readTree(new JsonReportRenderer().render(sampleResults())); + } + + @Test + void summaryCarriesCountsAndPerSectionBreakdown() throws Exception { + final JsonNode summary = render().path("summary"); + + assertEquals(1, summary.path("errors").asInt()); + assertEquals(1, summary.path("warnings").asInt()); + assertFalse(summary.path("clean").asBoolean()); + + // Both findings have a label subject, so they fall under one section with one error and one warning. + final JsonNode sections = summary.path("sections"); + assertEquals(1, sections.size()); + assertEquals("Translations", sections.get(0).path("section").asText()); + assertEquals(1, sections.get(0).path("errors").asInt()); + assertEquals(1, sections.get(0).path("warnings").asInt()); + } + + @Test + void findingsAreAFlatSelfDescribingArrayErrorsFirst() throws Exception { + final JsonNode findings = render().path("findings"); + assertEquals(2, findings.size()); + + // Errors are listed before warnings. + final JsonNode error = findings.get(0); + assertEquals("Translations", error.path("section").asText()); + assertEquals("error", error.path("severity").asText()); + assertEquals("Field references an existing label", error.path("rule").asText()); + assertEquals("Label is referenced but is missing", error.path("problem").asText()); + assertEquals("label", error.path("subject").path("type").asText()); + assertTrue(error.path("subject").hasNonNull("id"), error.toString()); + assertTrue(error.path("message").asText().contains("does not exist"), error.toString()); + + // The implicated asset (the thing to add/fix) is carried as a typed reference. + final JsonNode reference = error.path("references").get(0); + assertEquals("label", reference.path("type").asText()); + assertEquals("business-entity|name|UBO", reference.path("id").asText()); + + final JsonNode warning = findings.get(1); + assertEquals("warning", warning.path("severity").asText()); + // An intrinsic finding (no implicated asset) still has the field, as an empty array. + assertTrue(warning.path("references").isArray(), warning.toString()); + assertEquals(0, warning.path("references").size()); + } +} diff --git a/src/test/resources/eforms-sdk-tests/field-description/invalid/fields/fields.json b/src/test/resources/eforms-sdk-tests/field-description/invalid/fields/fields.json new file mode 100644 index 0000000..212faf2 --- /dev/null +++ b/src/test/resources/eforms-sdk-tests/field-description/invalid/fields/fields.json @@ -0,0 +1,23 @@ +{ + "ublVersion" : "2.3", + "sdkVersion" : "eforms-sdk-1.6.0", + "metadataDatabase" : { + "version" : "1.0.0", + "createdOn" : "2024-01-01T00:00:00" + }, + "xmlStructure" : [ { + "id" : "ND-Root", + "xpathAbsolute" : "/*", + "xpathRelative" : "/*", + "repeatable" : false + } ], + "fields" : [ { + "id" : "BT-X-Field", + "parentNodeId" : "ND-Root", + "name" : "Test field", + "btId" : "BT-Y", + "xpathAbsolute" : "/*/cbc:Test", + "xpathRelative" : "cbc:Test", + "type" : "text" + } ] +} diff --git a/src/test/resources/eforms-sdk-tests/field-description/invalid/notice-types/1.json b/src/test/resources/eforms-sdk-tests/field-description/invalid/notice-types/1.json new file mode 100644 index 0000000..6af0c6f --- /dev/null +++ b/src/test/resources/eforms-sdk-tests/field-description/invalid/notice-types/1.json @@ -0,0 +1,16 @@ +{ + "ublVersion" : "2.3", + "sdkVersion" : "1.6.0", + "metadataDatabase" : { + "version" : "1.0.0", + "createdOn" : "2024-01-01T00:00:00" + }, + "noticeId" : "1", + "metadata" : [ ], + "content" : [ { + "id" : "BT-X-Field", + "contentType" : "field", + "displayType" : "TEXTBOX", + "_label" : "field|name|BT-X-Field" + } ] +} diff --git a/src/test/resources/eforms-sdk-tests/field-description/invalid/notice-types/notice-types.json b/src/test/resources/eforms-sdk-tests/field-description/invalid/notice-types/notice-types.json new file mode 100644 index 0000000..8ffda54 --- /dev/null +++ b/src/test/resources/eforms-sdk-tests/field-description/invalid/notice-types/notice-types.json @@ -0,0 +1,13 @@ +{ + "ublVersion" : "2.3", + "sdkVersion" : "1.6.0", + "metadataDatabase" : { + "version" : "1.0.0", + "createdOn" : "2024-01-01T00:00:00" + }, + "noticeSubTypes" : [ { + "subTypeId" : "1", + "documentType" : "BRIN", + "_label" : "notice|name|1" + } ] +} diff --git a/src/test/resources/eforms-sdk-tests/field-description/invalid/translations/field_en.xml b/src/test/resources/eforms-sdk-tests/field-description/invalid/translations/field_en.xml new file mode 100644 index 0000000..51ecaf5 --- /dev/null +++ b/src/test/resources/eforms-sdk-tests/field-description/invalid/translations/field_en.xml @@ -0,0 +1,6 @@ + + + +field in English. File generated from metadata database. +Test field + diff --git a/src/test/resources/eforms-sdk-tests/field-description/invalid/translations/translations.json b/src/test/resources/eforms-sdk-tests/field-description/invalid/translations/translations.json new file mode 100644 index 0000000..c85769c --- /dev/null +++ b/src/test/resources/eforms-sdk-tests/field-description/invalid/translations/translations.json @@ -0,0 +1,13 @@ +{ + "files" : [ { + "assetType" : "field", + "twoLetterCode" : "en", + "threeLetterCode" : "eng", + "filename" : "field_en.xml" + } ], + "languages" : [ { + "description" : "English", + "twoLetterCode" : "en", + "threeLetterCode" : "eng" + } ] +} diff --git a/src/test/resources/eforms-sdk-tests/field-description/valid/fields/fields.json b/src/test/resources/eforms-sdk-tests/field-description/valid/fields/fields.json new file mode 100644 index 0000000..212faf2 --- /dev/null +++ b/src/test/resources/eforms-sdk-tests/field-description/valid/fields/fields.json @@ -0,0 +1,23 @@ +{ + "ublVersion" : "2.3", + "sdkVersion" : "eforms-sdk-1.6.0", + "metadataDatabase" : { + "version" : "1.0.0", + "createdOn" : "2024-01-01T00:00:00" + }, + "xmlStructure" : [ { + "id" : "ND-Root", + "xpathAbsolute" : "/*", + "xpathRelative" : "/*", + "repeatable" : false + } ], + "fields" : [ { + "id" : "BT-X-Field", + "parentNodeId" : "ND-Root", + "name" : "Test field", + "btId" : "BT-Y", + "xpathAbsolute" : "/*/cbc:Test", + "xpathRelative" : "cbc:Test", + "type" : "text" + } ] +} diff --git a/src/test/resources/eforms-sdk-tests/field-description/valid/notice-types/1.json b/src/test/resources/eforms-sdk-tests/field-description/valid/notice-types/1.json new file mode 100644 index 0000000..6af0c6f --- /dev/null +++ b/src/test/resources/eforms-sdk-tests/field-description/valid/notice-types/1.json @@ -0,0 +1,16 @@ +{ + "ublVersion" : "2.3", + "sdkVersion" : "1.6.0", + "metadataDatabase" : { + "version" : "1.0.0", + "createdOn" : "2024-01-01T00:00:00" + }, + "noticeId" : "1", + "metadata" : [ ], + "content" : [ { + "id" : "BT-X-Field", + "contentType" : "field", + "displayType" : "TEXTBOX", + "_label" : "field|name|BT-X-Field" + } ] +} diff --git a/src/test/resources/eforms-sdk-tests/field-description/valid/notice-types/notice-types.json b/src/test/resources/eforms-sdk-tests/field-description/valid/notice-types/notice-types.json new file mode 100644 index 0000000..8ffda54 --- /dev/null +++ b/src/test/resources/eforms-sdk-tests/field-description/valid/notice-types/notice-types.json @@ -0,0 +1,13 @@ +{ + "ublVersion" : "2.3", + "sdkVersion" : "1.6.0", + "metadataDatabase" : { + "version" : "1.0.0", + "createdOn" : "2024-01-01T00:00:00" + }, + "noticeSubTypes" : [ { + "subTypeId" : "1", + "documentType" : "BRIN", + "_label" : "notice|name|1" + } ] +} diff --git a/src/test/resources/eforms-sdk-tests/field-description/valid/translations/field_en.xml b/src/test/resources/eforms-sdk-tests/field-description/valid/translations/field_en.xml new file mode 100644 index 0000000..93ad5a4 --- /dev/null +++ b/src/test/resources/eforms-sdk-tests/field-description/valid/translations/field_en.xml @@ -0,0 +1,7 @@ + + + +field in English. File generated from metadata database. +Test field +A tooltip for the test field. + diff --git a/src/test/resources/eforms-sdk-tests/field-description/valid/translations/translations.json b/src/test/resources/eforms-sdk-tests/field-description/valid/translations/translations.json new file mode 100644 index 0000000..c85769c --- /dev/null +++ b/src/test/resources/eforms-sdk-tests/field-description/valid/translations/translations.json @@ -0,0 +1,13 @@ +{ + "files" : [ { + "assetType" : "field", + "twoLetterCode" : "en", + "threeLetterCode" : "eng", + "filename" : "field_en.xml" + } ], + "languages" : [ { + "description" : "English", + "twoLetterCode" : "en", + "threeLetterCode" : "eng" + } ] +} diff --git a/src/test/resources/eu/europa/ted/eforms/sdk/analysis/cucumber/field-description.feature b/src/test/resources/eu/europa/ted/eforms/sdk/analysis/cucumber/field-description.feature new file mode 100644 index 0000000..cba6dd5 --- /dev/null +++ b/src/test/resources/eu/europa/ted/eforms/sdk/analysis/cucumber/field-description.feature @@ -0,0 +1,28 @@ +@field-description +Feature: Notice Types - field description labels + TEDEMD-90: A field shown in a notice type definition should have a description label, on the field + itself or — failing that — on its business term. + (The ticket called this the field "tooltip", which is actually the "hint" label, not "description"; + the label type to check is still to be confirmed against the original request.) + Test files under "src/test/resources/eforms-sdk-tests/field-description" + + Background: + Given The following rules + | Fields used in notice types have a description label | + + Scenario: Fields used in notice types have a description label + Given A "field-description" folder with "valid" files + When I load all fields + And I load all notice types + And I load all labels + And I execute validation + Then I should get 0 SDK validation errors + + Scenario: A field used in a notice type has no description label + Given A "field-description" folder with "invalid" files + When I load all fields + And I load all notice types + And I load all labels + And I execute validation + Then Rule "Fields used in notice types have a description label" should have been fired + Then I should get 1 SDK validation errors