From e3d00b6f25a29df612614fc11976132adbf17a8c Mon Sep 17 00:00:00 2001 From: Michael Ritter Date: Tue, 22 Mar 2022 11:33:23 -0600 Subject: [PATCH 1/5] Add failure only reporting option --- .../org/fcrepo/migration/validator/Driver.java | 5 +++++ .../impl/ApplicationConfigurationHelper.java | 2 +- .../validator/impl/Fedora3ValidationConfig.java | 13 +++++++++++++ .../impl/FileSystemValidationResultWriter.java | 15 +++++++++++---- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/fcrepo/migration/validator/Driver.java b/src/main/java/org/fcrepo/migration/validator/Driver.java index 13ea533..60275a4 100644 --- a/src/main/java/org/fcrepo/migration/validator/Driver.java +++ b/src/main/java/org/fcrepo/migration/validator/Driver.java @@ -104,6 +104,10 @@ public class Driver implements Callable { description = "Validate objects in the Inactive state as deleted.") private boolean deleteInactive; + @CommandLine.Option(names = {"--failure-only"}, order = 19, + description = "Report only objects which have failed validations.") + private boolean failureOnly; + @CommandLine.Option(names = {"--debug"}, order = 30, description = "Enables debug logging") private boolean debug; @@ -125,6 +129,7 @@ public Integer call() { config.setOcflRepositoryRootDirectory(ocflRootDirectory); config.setObjectsToValidate(objectsToValidate); config.setDeleteInactive(deleteInactive); + config.setFailureOnly(failureOnly); LOGGER.info("Configuration created: {}", config); LOGGER.info("Preparing to execute validation run..."); diff --git a/src/main/java/org/fcrepo/migration/validator/impl/ApplicationConfigurationHelper.java b/src/main/java/org/fcrepo/migration/validator/impl/ApplicationConfigurationHelper.java index 2d7b6cd..9c64166 100644 --- a/src/main/java/org/fcrepo/migration/validator/impl/ApplicationConfigurationHelper.java +++ b/src/main/java/org/fcrepo/migration/validator/impl/ApplicationConfigurationHelper.java @@ -64,7 +64,7 @@ public ApplicationConfigurationHelper(final Fedora3ValidationConfig config) { } public ValidationResultWriter validationResultWriter() { - return new FileSystemValidationResultWriter(config.getJsonOutputDirectory()); + return new FileSystemValidationResultWriter(config.getJsonOutputDirectory(), config.isFailureOnly()); } public ObjectSource objectSource() { diff --git a/src/main/java/org/fcrepo/migration/validator/impl/Fedora3ValidationConfig.java b/src/main/java/org/fcrepo/migration/validator/impl/Fedora3ValidationConfig.java index 6ab1e7e..dd223b2 100644 --- a/src/main/java/org/fcrepo/migration/validator/impl/Fedora3ValidationConfig.java +++ b/src/main/java/org/fcrepo/migration/validator/impl/Fedora3ValidationConfig.java @@ -17,6 +17,7 @@ public class Fedora3ValidationConfig extends ValidationConfig { private boolean checksum; + private boolean failureOnly; private boolean deleteInactive; private boolean validateHeadOnly; private boolean checkNumObjects; @@ -183,4 +184,16 @@ public Fedora3ValidationConfig setDeleteInactive(final boolean deleteInactive) { this.deleteInactive = deleteInactive; return this; } + + public boolean isFailureOnly() { + return failureOnly; + } + + /** + * @param failureOnly + */ + public Fedora3ValidationConfig setFailureOnly(final boolean failureOnly) { + this.failureOnly = failureOnly; + return this; + } } diff --git a/src/main/java/org/fcrepo/migration/validator/impl/FileSystemValidationResultWriter.java b/src/main/java/org/fcrepo/migration/validator/impl/FileSystemValidationResultWriter.java index ca8ed0d..d791c34 100644 --- a/src/main/java/org/fcrepo/migration/validator/impl/FileSystemValidationResultWriter.java +++ b/src/main/java/org/fcrepo/migration/validator/impl/FileSystemValidationResultWriter.java @@ -14,6 +14,7 @@ import java.nio.file.Path; import java.util.List; +import static org.fcrepo.migration.validator.api.ValidationResult.Status.OK; import static org.fcrepo.migration.validator.impl.ValidationResultUtils.resolvePathToJsonResult; import static org.slf4j.LoggerFactory.getLogger; @@ -26,23 +27,29 @@ public class FileSystemValidationResultWriter implements ValidationResultWriter private static final Logger LOGGER = getLogger(FileSystemValidationResultWriter.class); - private Path validationRoot; - + private final Path validationRoot; + private final boolean writeFailureOnly; /** * Constructor * * @param validationRoot The root of validation report associated with the run + * @param writeFailureOnly Flag to indicate if we should write only failed validations or all */ - public FileSystemValidationResultWriter(final Path validationRoot) { + public FileSystemValidationResultWriter(final Path validationRoot, final boolean writeFailureOnly) { this.validationRoot = validationRoot; - this.validationRoot.toFile().mkdirs(); + this.writeFailureOnly = writeFailureOnly; + validationRoot.toFile().mkdirs(); } @Override public void write(final List results) { final var objectMapper = new ObjectMapper(); for (final var result : results) { + if (result.getStatus() == OK && writeFailureOnly) { + continue; + } + LOGGER.info("Writing of results here: {}", result); final var jsonFilePath = this.validationRoot.resolve(resolvePathToJsonResult(result)); final var file = jsonFilePath.toFile(); From e34c3a5115ef43ccbd6a18330cec2e94f4249e2d Mon Sep 17 00:00:00 2001 From: Michael Ritter Date: Thu, 24 Mar 2022 15:26:20 -0600 Subject: [PATCH 2/5] Create tests for basic report generation coverage --- .../validator/ReportGeneratorIT.java | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 src/test/java/org/fcrepo/migration/validator/ReportGeneratorIT.java diff --git a/src/test/java/org/fcrepo/migration/validator/ReportGeneratorIT.java b/src/test/java/org/fcrepo/migration/validator/ReportGeneratorIT.java new file mode 100644 index 0000000..1e684bb --- /dev/null +++ b/src/test/java/org/fcrepo/migration/validator/ReportGeneratorIT.java @@ -0,0 +1,125 @@ +/* + * 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. + */ +package org.fcrepo.migration.validator; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.function.Predicate; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +import edu.wisc.library.ocfl.api.OcflRepository; +import org.fcrepo.migration.validator.impl.ApplicationConfigurationHelper; +import org.fcrepo.migration.validator.impl.Fedora3ValidationExecutionManager; +import org.fcrepo.migration.validator.report.CsvReportHandler; +import org.fcrepo.migration.validator.report.HtmlReportHandler; +import org.fcrepo.migration.validator.report.ReportGeneratorImpl; +import org.fcrepo.migration.validator.report.ReportType; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +/** + * + * @author mikejritter + */ +@RunWith(Parameterized.class) +public class ReportGeneratorIT extends AbstractValidationIT { + + @Parameterized.Parameters + public static Collection data() { + return Arrays.asList(new Object[][] { + { ReportType.html }, { ReportType.csv } + }); + } + + /** + * Regex to ignore non-object reports + */ + private static final Predicate IGNORE = Pattern.compile("index|repository|migration-validation-summary") + .asPredicate() + .negate(); + + private final ReportType reportType; + + public ReportGeneratorIT(final ReportType reportType) { + this.reportType = reportType; + } + + @Test + public void testAllPassHTML() throws Exception { + final var f3ObjectsDir = new File(FIXTURES_BASE_DIR, "valid/f3/objects"); + final var f3DatastreamsDir = new File(FIXTURES_BASE_DIR, "valid/f3/datastreams"); + final var f6OcflRootDir = new File(FIXTURES_BASE_DIR, "valid/f6/data/ocfl-root"); + + final var config = getConfig(f3DatastreamsDir, f3ObjectsDir, f6OcflRootDir); + final var reportDir = config.getReportDirectory(reportType); + + final var configHelper = new ApplicationConfigurationHelper(config); + final var executionManager = new Fedora3ValidationExecutionManager(configHelper); + executionManager.doValidation(); + + final var handler = reportType == ReportType.html ? new HtmlReportHandler(reportDir) + : new CsvReportHandler(reportDir, reportType); + final var generator = new ReportGeneratorImpl(config.getJsonOutputDirectory(), handler); + generator.generate(); + + final var ocflRepository = configHelper.ocflRepository(); + final var objectIds = getExpectedObjectReports(ocflRepository, reportType); + + // get object reports and validate + final var objectReports = getObjectReports(reportDir, IGNORE); + assertThat(objectReports) + .hasSize(objectIds.size()) + .allMatch(objectIds::contains); + } + + @Test + public void testOnlyWriteFailureAllPassHTML() throws IOException { + final var f3ObjectsDir = new File(FIXTURES_BASE_DIR, "valid/f3/objects"); + final var f3DatastreamsDir = new File(FIXTURES_BASE_DIR, "valid/f3/datastreams"); + final var f6OcflRootDir = new File(FIXTURES_BASE_DIR, "valid/f6/data/ocfl-root"); + + final var config = getConfig(f3DatastreamsDir, f3ObjectsDir, f6OcflRootDir); + config.setFailureOnly(true); + final var reportDir = config.getReportDirectory(reportType); + + final var configHelper = new ApplicationConfigurationHelper(config); + final var executionManager = new Fedora3ValidationExecutionManager(configHelper); + executionManager.doValidation(); + + final var handler = reportType == ReportType.html ? new HtmlReportHandler(reportDir) + : new CsvReportHandler(reportDir, reportType); + final var generator = new ReportGeneratorImpl(config.getJsonOutputDirectory(), handler); + generator.generate(); + + // filter out non-object reports + final var objectReports = getObjectReports(reportDir, IGNORE); + assertThat(objectReports).isEmpty(); + } + + public List getExpectedObjectReports(final OcflRepository repository, final ReportType reportType) { + return repository.listObjectIds() + .map(objectId -> objectId.substring("info:fedora/".length())) + .map(objectId -> objectId + reportType.getExtension()) + .collect(Collectors.toList()); + } + + public List getObjectReports(final Path reportDir, final Predicate filter) throws IOException { + return Files.list(reportDir) + .map(Path::getFileName) + .map(Path::toString) + .filter(filter) + .collect(Collectors.toList()); + } +} From 7bebc147dc12dad1b25e6f09d55c4b17d0b75377 Mon Sep 17 00:00:00 2001 From: Michael Ritter Date: Thu, 24 Mar 2022 15:52:58 -0600 Subject: [PATCH 3/5] Update logging to be less verbose and include failed validations --- .../migration/validator/impl/F3ObjectValidationTask.java | 2 +- .../validator/impl/FileSystemValidationResultWriter.java | 2 +- .../migration/validator/impl/ValidatingObjectHandler.java | 7 ++++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/fcrepo/migration/validator/impl/F3ObjectValidationTask.java b/src/main/java/org/fcrepo/migration/validator/impl/F3ObjectValidationTask.java index 3fe0a67..b1e1858 100644 --- a/src/main/java/org/fcrepo/migration/validator/impl/F3ObjectValidationTask.java +++ b/src/main/java/org/fcrepo/migration/validator/impl/F3ObjectValidationTask.java @@ -49,7 +49,7 @@ public F3ObjectValidationTask(final FedoraObjectProcessor processor, @Override public void run() { - LOGGER.info("starting to process {} ", processor.getObjectInfo().getPid()); + LOGGER.info("Processing {} ", processor.getObjectInfo().getPid()); final var validator = new Fedora3ObjectValidator(ocflObjectSessionFactory, objectValidationConfig); final var results = validator.validate(processor); writer.write(results); diff --git a/src/main/java/org/fcrepo/migration/validator/impl/FileSystemValidationResultWriter.java b/src/main/java/org/fcrepo/migration/validator/impl/FileSystemValidationResultWriter.java index d791c34..4d00715 100644 --- a/src/main/java/org/fcrepo/migration/validator/impl/FileSystemValidationResultWriter.java +++ b/src/main/java/org/fcrepo/migration/validator/impl/FileSystemValidationResultWriter.java @@ -50,7 +50,7 @@ public void write(final List results) { continue; } - LOGGER.info("Writing of results here: {}", result); + LOGGER.debug("Writing of results here: {}", result); final var jsonFilePath = this.validationRoot.resolve(resolvePathToJsonResult(result)); final var file = jsonFilePath.toFile(); file.getParentFile().mkdirs(); diff --git a/src/main/java/org/fcrepo/migration/validator/impl/ValidatingObjectHandler.java b/src/main/java/org/fcrepo/migration/validator/impl/ValidatingObjectHandler.java index 9619958..2a9b9cd 100644 --- a/src/main/java/org/fcrepo/migration/validator/impl/ValidatingObjectHandler.java +++ b/src/main/java/org/fcrepo/migration/validator/impl/ValidatingObjectHandler.java @@ -289,7 +289,7 @@ private void validateObjectProperty(final ObjectProperty op, final ResourceHeade final var property = op.getName(); final var sourceValue = op.getValue(); - LOGGER.info("PID = {}, object property: name = {}, value = {}", pid, property, sourceValue); + LOGGER.debug("PID = {}, object property: name = {}, value = {}", pid, property, sourceValue); final var resolver = OCFL_PROPERTY_RESOLVERS.get(property); if (resolver != null) { final var success = "pid: %s -> properties match: f3 prop name=%s, source=%s, target=%s"; @@ -303,7 +303,7 @@ private void validateObjectProperty(final ObjectProperty op, final ResourceHeade .map(targetVal -> resolver.equals(sourceValue, targetVal) ? builder.ok(METADATA, format(success, pid, property, sourceValue, targetVal)) : builder.fail(METADATA, format(error, pid, property, sourceValue, targetVal))) - .orElse(builder.fail(METADATA, format(notFound, pid, property, sourceValue))); + .orElseGet(() -> builder.fail(METADATA, format(notFound, pid, property, sourceValue))); validationResults.add(result); } } @@ -584,7 +584,7 @@ private void validateSize(final DatastreamVersion dsVersion, return builder.ok(BINARY_SIZE, format(success, version, sourceBytes)); } return builder.fail(BINARY_SIZE, format(error, version, sourceBytes, targetBytes)); - }).orElse(builder.fail(BINARY_SIZE, format(notFound, version, "source"))); + }).orElseGet(() -> builder.fail(BINARY_SIZE, format(notFound, version, "source"))); validationResults.add(result); } } @@ -636,6 +636,7 @@ public ValidationResult ok(final ValidationType type, final String details) { } public ValidationResult fail(final ValidationType type, final String details) { + LOGGER.info("[{}] {} validation failed: {}", sourceObjectId, type, details); return new ValidationResult(indexCounter++, FAIL, validationLevel, type, sourceObjectId, targetObjectId, sourceResource, targetResource, details); } From c531e415f02d4c266de27fa05ad4cb8eddd22a6c Mon Sep 17 00:00:00 2001 From: Michael Ritter Date: Fri, 25 Mar 2022 13:57:21 -0600 Subject: [PATCH 4/5] Rename tests --- .../org/fcrepo/migration/validator/ReportGeneratorIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/fcrepo/migration/validator/ReportGeneratorIT.java b/src/test/java/org/fcrepo/migration/validator/ReportGeneratorIT.java index 1e684bb..403e831 100644 --- a/src/test/java/org/fcrepo/migration/validator/ReportGeneratorIT.java +++ b/src/test/java/org/fcrepo/migration/validator/ReportGeneratorIT.java @@ -57,7 +57,7 @@ public ReportGeneratorIT(final ReportType reportType) { } @Test - public void testAllPassHTML() throws Exception { + public void testAllPass() throws Exception { final var f3ObjectsDir = new File(FIXTURES_BASE_DIR, "valid/f3/objects"); final var f3DatastreamsDir = new File(FIXTURES_BASE_DIR, "valid/f3/datastreams"); final var f6OcflRootDir = new File(FIXTURES_BASE_DIR, "valid/f6/data/ocfl-root"); @@ -85,7 +85,7 @@ public void testAllPassHTML() throws Exception { } @Test - public void testOnlyWriteFailureAllPassHTML() throws IOException { + public void testOnlyWriteFailureAllPass() throws IOException { final var f3ObjectsDir = new File(FIXTURES_BASE_DIR, "valid/f3/objects"); final var f3DatastreamsDir = new File(FIXTURES_BASE_DIR, "valid/f3/datastreams"); final var f6OcflRootDir = new File(FIXTURES_BASE_DIR, "valid/f6/data/ocfl-root"); From 9ebec18c48283009d99346c220b038a6cd914c9a Mon Sep 17 00:00:00 2001 From: Michael Ritter Date: Fri, 25 Mar 2022 13:57:45 -0600 Subject: [PATCH 5/5] Omit object results if none are present --- src/main/resources/templates/summary.ftl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/resources/templates/summary.ftl b/src/main/resources/templates/summary.ftl index 6ec4d53..7a4124d 100644 --- a/src/main/resources/templates/summary.ftl +++ b/src/main/resources/templates/summary.ftl @@ -49,7 +49,7 @@

Summary

- + @@ -70,6 +70,7 @@ + <#if objectCount gt 0>

Object result details

    @@ -78,5 +79,6 @@
+
Total objects:Total object reports: ${objectCount}