Move validator and code cleanup (#11)

GeoWerkstatt · Apr 10, 2024 · 71b948b · 71b948b
2 parents 5f3209e + 8b9fcab
commit 71b948b
Show file tree

Hide file tree

Showing 9 changed files with 34 additions and 30 deletions.
diff --git a/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Main.java b/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Main.java
@@ -1,5 +1,6 @@
 package ch.geowerkstatt.interlis.testbed.runner;
 
+import ch.geowerkstatt.interlis.testbed.runner.validation.InterlisValidator;
 import ch.geowerkstatt.interlis.testbed.runner.xtf.XtfFileMerger;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.DefaultParser;
@@ -55,9 +56,9 @@ private static void printUsage(Options options) {
         formatter.printHelp("java -jar interlis-testbed-runner.jar [options] [testbed directory (default: current directory)]", options);
     }
 
-    private static TestOptions getTestOptions(CommandLine commandLine) throws ParseException {
+    private static TestOptions getTestOptions(CommandLine commandLine) {
         var remainingArgs = commandLine.getArgList();
-        var basePath = remainingArgs.isEmpty() ? Path.of(".") : Path.of(remainingArgs.get(0));
+        var basePath = remainingArgs.isEmpty() ? Path.of(".") : Path.of(remainingArgs.getFirst());
         var validatorPath = Path.of(commandLine.getOptionValue(VALIDATOR_PATH_OPTION));
         return new TestOptions(basePath, validatorPath);
     }

diff --git a/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Runner.java b/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Runner.java
@@ -1,5 +1,7 @@
 package ch.geowerkstatt.interlis.testbed.runner;
 
+import ch.geowerkstatt.interlis.testbed.runner.validation.Validator;
+import ch.geowerkstatt.interlis.testbed.runner.validation.ValidatorException;
 import ch.geowerkstatt.interlis.testbed.runner.xtf.XtfMerger;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -35,7 +37,7 @@ public Runner(TestOptions options, Validator validator, XtfMerger xtfMerger) {
      * @return {@code true} if the validation was successful, {@code false} otherwise.
      */
     public boolean run() {
-        LOGGER.info("Starting validation of testbed at " + options.basePath());
+        LOGGER.info("Starting validation of testbed at {}", options.basePath());
 
         try {
             if (!validateBaseData()) {
@@ -66,15 +68,15 @@ private boolean validateBaseData() throws ValidatorException {
             throw new ValidatorException(e);
         }
 
-        LOGGER.info("Validating base data file " + baseFilePath);
+        LOGGER.info("Validating base data file {}", baseFilePath);
         var filenameWithoutExtension = StringUtils.getFilenameWithoutExtension(baseFilePath.getFileName().toString());
         var logFile = options.resolveOutputFilePath(baseFilePath, filenameWithoutExtension + ".log");
 
         var valid = validator.validate(baseFilePath, logFile);
         if (valid) {
-            LOGGER.info("Validation of " + baseFilePath + " completed successfully.");
+            LOGGER.info("Validation of {} completed successfully.", baseFilePath);
         } else {
-            LOGGER.error("Validation of " + baseFilePath + " failed. See " + logFile + " for details.");
+            LOGGER.error("Validation of {} failed. See {} for details.", baseFilePath, logFile);
         }
         return valid;
     }
@@ -113,16 +115,16 @@ private boolean mergeAndValidateTransferFiles() throws ValidatorException {
 
     private boolean validateMergedFile(Path mergedFile, Path logFile, String constraintName) throws ValidatorException {
         if (validator.validate(mergedFile, logFile)) {
-            LOGGER.error("Validation of " + mergedFile + " was expected to fail but completed successfully.");
+            LOGGER.error("Validation of {} was expected to fail but completed successfully.", mergedFile);
             return false;
         }
 
         if (!validator.containsConstraintError(logFile, constraintName)) {
-            LOGGER.error("Could not verify constraint " + constraintName + " for merged file " + mergedFile + ". Check the log file at " + logFile + " for details.");
+            LOGGER.error("Could not verify constraint {} for merged file {}. Check the log file at {} for details.", constraintName, mergedFile, logFile);
             return false;
         }
 
-        LOGGER.info("Validation of " + mergedFile + " failed as expected.");
+        LOGGER.info("Validation of {} failed as expected.", mergedFile);
         return true;
     }
 }
diff --git a/...lis/testbed/runner/InterlisValidator.java → .../runner/validation/InterlisValidator.java b/...lis/testbed/runner/InterlisValidator.java → .../runner/validation/InterlisValidator.java
@@ -1,5 +1,6 @@
-package ch.geowerkstatt.interlis.testbed.runner;
+package ch.geowerkstatt.interlis.testbed.runner.validation;
 
+import ch.geowerkstatt.interlis.testbed.runner.TestOptions;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -21,12 +22,12 @@ public final class InterlisValidator implements Validator {
     public InterlisValidator(TestOptions options) {
         this.options = options;
 
-        LOGGER.info("Using ilivalidator at " + options.ilivalidatorPath());
+        LOGGER.info("Using ilivalidator at {}", options.ilivalidatorPath());
     }
 
     @Override
     public boolean validate(Path filePath, Path logFile) throws ValidatorException {
-        LOGGER.info("Validating " + filePath + " with log file " + logFile);
+        LOGGER.info("Validating {} with log file {}", filePath, logFile);
         try {
             Files.createDirectories(logFile.getParent());
 
@@ -55,7 +56,7 @@ public boolean containsConstraintError(Path logFile, String constraintName) thro
         try (var lines = Files.lines(logFile)) {
             return lines.anyMatch(line -> {
                 if (constraintPattern.matcher(line).find()) {
-                    LOGGER.info("Found expected error for constraint " + constraintName + " in log file: " + line);
+                    LOGGER.info("Found expected error for constraint {} in log file: {}", constraintName, line);
                     return true;
                 }
                 return false;

diff --git a/...tt/interlis/testbed/runner/Validator.java → .../testbed/runner/validation/Validator.java b/...tt/interlis/testbed/runner/Validator.java → .../testbed/runner/validation/Validator.java
@@ -1,4 +1,4 @@
-package ch.geowerkstatt.interlis.testbed.runner;
+package ch.geowerkstatt.interlis.testbed.runner.validation;
 
 import java.nio.file.Path;
 

diff --git a/...is/testbed/runner/ValidatorException.java → ...runner/validation/ValidatorException.java b/...is/testbed/runner/ValidatorException.java → ...runner/validation/ValidatorException.java
@@ -1,4 +1,4 @@
-package ch.geowerkstatt.interlis.testbed.runner;
+package ch.geowerkstatt.interlis.testbed.runner.validation;
 
 /**
  * Exception that is thrown when the validation failed to run.

diff --git a/src/main/java/ch/geowerkstatt/interlis/testbed/runner/xtf/XtfFileMerger.java b/src/main/java/ch/geowerkstatt/interlis/testbed/runner/xtf/XtfFileMerger.java
@@ -6,9 +6,7 @@
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 
-import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.TransformerException;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMSource;
@@ -43,21 +41,21 @@ public XtfFileMerger() {
     @Override
     public boolean merge(Path baseFile, Path patchFile, Path outputFile) {
         try {
-            LOGGER.info("Merging " + baseFile + " with " + patchFile + " into " + outputFile);
-            var documentBuilder = createDocumentBuilder();
+            LOGGER.info("Merging {} with {} into {}", baseFile, patchFile, outputFile);
+            var documentBuilder = factory.newDocumentBuilder();
 
             var baseDocument = documentBuilder.parse(baseFile.toFile());
             var patchDocument = documentBuilder.parse(patchFile.toFile());
 
             var baseBaskets = findBaskets(baseDocument);
             if (baseBaskets.isEmpty()) {
-                LOGGER.error("No baskets found in base file " + baseFile + ".");
+                LOGGER.error("No baskets found in base file {}.", baseFile);
                 return false;
             }
 
             var patchBaskets = findBaskets(patchDocument);
             if (patchBaskets.isEmpty()) {
-                LOGGER.error("No baskets found in patch file " + patchFile + ".");
+                LOGGER.error("No baskets found in patch file {}.", patchFile);
                 return false;
             }
 
@@ -66,18 +64,14 @@ public boolean merge(Path baseFile, Path patchFile, Path outputFile) {
             }
 
             writeMergedFile(baseDocument, outputFile);
-            LOGGER.info("Successfully merged files into " + outputFile);
+            LOGGER.info("Successfully merged files into {}", outputFile);
             return true;
         } catch (Exception e) {
             LOGGER.error("Failed to merge files.", e);
             return false;
         }
     }
 
-    DocumentBuilder createDocumentBuilder() throws ParserConfigurationException {
-        return factory.newDocumentBuilder();
-    }
-
     private static boolean mergeBaskets(Document document, Map<String, Basket> baseBaskets, Map<String, Basket> patchBaskets) {
         var isValid = true;
 
@@ -87,7 +81,7 @@ private static boolean mergeBaskets(Document document, Map<String, Basket> baseB
 
             var originalBasket = baseBaskets.get(basketId);
             if (originalBasket == null) {
-                LOGGER.error("Basket " + basketId + " not found in base file.");
+                LOGGER.error("Basket {} not found in base file.", basketId);
                 isValid = false;
                 continue;
             }
@@ -103,7 +97,7 @@ private static boolean mergeBaskets(Document document, Map<String, Basket> baseB
 
                 if (hasDeleteAttribute(element)) {
                     if (!originalBasket.removeChildNode(entryId)) {
-                        LOGGER.error("Could not remove entry " + entryId + " from basket " + basketId + " as it does not exist.");
+                        LOGGER.error("Could not remove entry {} from basket {} as it does not exist.", entryId, basketId);
                         isValid = false;
                     }
                 } else {
@@ -136,7 +130,7 @@ static Optional<Map<String, Basket>> findBaskets(Document document) {
                 .filter(e -> {
                     var hasId = hasInterlisAttribute(e, BASKET_ID);
                     if (!hasId) {
-                        LOGGER.warn("Basket without " + BASKET_ID + " found.");
+                        LOGGER.warn("Basket without {} found.", BASKET_ID);
                     }
                     return hasId;
                 })
@@ -149,7 +143,7 @@ private static Basket collectBasket(Element basket) {
                 .filter(e -> {
                     var hasId = hasInterlisAttribute(e, OBJECT_ID);
                     if (!hasId) {
-                        LOGGER.warn("Entry without " + OBJECT_ID + " found in basket " + basket.getAttribute(BASKET_ID) + ".");
+                        LOGGER.warn("Entry without {} found in basket {}.", OBJECT_ID, basket.getAttribute(BASKET_ID));
                     }
                     return hasId;
                 })

diff --git a/src/test/java/ch/geowerkstatt/interlis/testbed/runner/RunnerBaseDataTest.java b/src/test/java/ch/geowerkstatt/interlis/testbed/runner/RunnerBaseDataTest.java
@@ -1,5 +1,7 @@
 package ch.geowerkstatt.interlis.testbed.runner;
 
+import ch.geowerkstatt.interlis.testbed.runner.validation.Validator;
+import ch.geowerkstatt.interlis.testbed.runner.validation.ValidatorException;
 import ch.geowerkstatt.interlis.testbed.runner.xtf.XtfMerger;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;

diff --git a/src/test/java/ch/geowerkstatt/interlis/testbed/runner/RunnerWithConstraintsTest.java b/src/test/java/ch/geowerkstatt/interlis/testbed/runner/RunnerWithConstraintsTest.java
@@ -1,5 +1,7 @@
 package ch.geowerkstatt.interlis.testbed.runner;
 
+import ch.geowerkstatt.interlis.testbed.runner.validation.Validator;
+import ch.geowerkstatt.interlis.testbed.runner.validation.ValidatorException;
 import ch.geowerkstatt.interlis.testbed.runner.xtf.XtfMerger;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;

diff --git a/src/test/java/ch/geowerkstatt/interlis/testbed/runner/ValidatorTest.java b/src/test/java/ch/geowerkstatt/interlis/testbed/runner/ValidatorTest.java
@@ -1,5 +1,7 @@
 package ch.geowerkstatt.interlis.testbed.runner;
 
+import ch.geowerkstatt.interlis.testbed.runner.validation.InterlisValidator;
+import ch.geowerkstatt.interlis.testbed.runner.validation.ValidatorException;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;