FIX: Missing file error for profile validation

- added missing file error message for profile validation; - tests for the above; - moved file checking and reporting methods to utiltiy class; - bumped version to 0.11.0, including batch files; and - some minor formatting changes.
openpreserve · May 8, 2024 · 97efe12 · 97efe12 · maria-messerschmidt · Jun 7, 2024
1 parent 5f5e5d2
commit 97efe12
Show file tree

Hide file tree

Showing 11 changed files with 102 additions and 37 deletions.
diff --git a/odf-apps/pom.xml b/odf-apps/pom.xml
@@ -5,7 +5,7 @@
   <parent>
     <groupId>org.openpreservation.odf</groupId>
     <artifactId>odf-validator</artifactId>
-    <version>0.10.0</version>
+    <version>0.11.0</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
 

diff --git a/odf-apps/src/main/java/org/openpreservation/odf/apps/CliValidator.java b/odf-apps/src/main/java/org/openpreservation/odf/apps/CliValidator.java
@@ -37,7 +37,7 @@ class CliValidator implements Callable<Integer> {
     private static final MessageFactory FACTORY = Messages
             .getInstance("org.openpreservation.odf.apps.messages.Messages");
 
-    @Option(names = {"-p", "--profile"}, description = "Validate using extended Spreadsheet preservation profile.")
+    @Option(names = { "-p", "--profile" }, description = "Validate using extended Spreadsheet preservation profile.")
     private boolean profileFlag;
     @Parameters(paramLabel = "FILE", arity = "1..*", description = "A list of Open Document Format spreadsheet files to validate.")
     private File[] toValidateFiles;
@@ -77,8 +77,10 @@ private ValidationReport validatePath(final Path toValidate) {
     private ProfileResult profilePath(final Path toProfile) {
         try {
             return dnaProfile.check(parser.parsePackage(toProfile));
-        } catch (IllegalArgumentException | IOException e) {
+        } catch (IllegalArgumentException | FileNotFoundException e) {
             this.logMessage(toProfile, Messages.getMessageInstance("APP-2", Severity.ERROR, e.getMessage()));
+        } catch (IOException e) {
+            e.printStackTrace();
         }
         return null;
     }
@@ -92,15 +94,17 @@ private Integer results() {
         Integer retStatus = 0;
         for (Map.Entry<Path, ValidationReport> entry : validationReports.entrySet()) {
             if (this.appMessages.containsKey(entry.getKey())) {
-                retStatus = Math.max(retStatus, processMessageLists(this.appMessages.get(entry.getKey()).getMessages().values()));
+                retStatus = Math.max(retStatus,
+                        processMessageLists(this.appMessages.get(entry.getKey()).getMessages().values()));
             }
             if (entry.getValue() != null) {
                 retStatus = Math.max(retStatus, results(entry.getKey(), entry.getValue()));
             }
         }
         for (Map.Entry<Path, ProfileResult> entry : profileResults.entrySet()) {
             if (this.appMessages.containsKey(entry.getKey())) {
-                retStatus = Math.max(retStatus, processMessageLists(this.appMessages.get(entry.getKey()).getMessages().values()));
+                retStatus = Math.max(retStatus,
+                        processMessageLists(this.appMessages.get(entry.getKey()).getMessages().values()));
             }
             if (entry.getValue() != null) {
                 retStatus = Math.max(retStatus, results(entry.getKey(), entry.getValue()));
@@ -159,7 +163,9 @@ private Integer results(final Path path, final ProfileResult report) {
         if (report.getValidationReport() != null && report.getValidationReport().documentMessages.hasErrors()) {
             retStatus = 1;
         }
-        MessageLog profileMessages = (report.getValidationReport() != null) ? report.getValidationReport().documentMessages : Messages.messageLogInstance();
+        MessageLog profileMessages = (report.getValidationReport() != null)
+                ? report.getValidationReport().documentMessages
+                : Messages.messageLogInstance();
         profileMessages.add(report.getProfileMessages().getMessages());
         outputSummary(profileMessages);
         return retStatus;
@@ -185,10 +191,12 @@ private void outputSummary(final MessageLog messageLog) {
                     messageLog.getWarningCount(), messageLog.getInfoCount()));
         } else {
             ConsoleFormatter
-                    .info(String.format("VALID, no errors, no warnings and %d info message found.", messageLog.getInfoCount()));
+                    .info(String.format("VALID, no errors, no warnings and %d info message found.",
+                            messageLog.getInfoCount()));
         }
         ConsoleFormatter.newline();
     }
+
     private final void logMessage(final Path path, final Message message) {
         this.appMessages.putIfAbsent(path, Messages.messageLogInstance());
         this.appMessages.get(path).add(path.toString(), message);

diff --git a/odf-core/pom.xml b/odf-core/pom.xml
@@ -5,7 +5,7 @@
   <parent>
     <groupId>org.openpreservation.odf</groupId>
     <artifactId>odf-validator</artifactId>
-    <version>0.10.0</version>
+    <version>0.11.0</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
 

diff --git a/odf-core/src/main/java/org/openpreservation/odf/pkg/PackageParserImpl.java b/odf-core/src/main/java/org/openpreservation/odf/pkg/PackageParserImpl.java
@@ -88,6 +88,7 @@ public OdfPackage parsePackage(final InputStream toParse, final String name) thr
     }
 
     private final OdfPackage parsePackage(final Path toParse, final String name) throws IOException {
+        Checks.existingFileCheck(toParse);
         this.initialise();
         try {
             this.format = sniff(toParse);

diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/Validator.java b/odf-core/src/main/java/org/openpreservation/odf/validation/Validator.java
@@ -1,7 +1,6 @@
 package org.openpreservation.odf.validation;
 
 import java.io.File;
-import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -55,6 +54,7 @@ public ValidationReport validateSingleFormat(final File toValidate, final Format
     public ValidationReport validateSingleFormat(final Path toValidate, final Formats legal) throws ParserConfigurationException, IOException, SAXException {
         Objects.requireNonNull(toValidate, String.format(Checks.NOT_NULL, "Path", TO_VAL_STRING));
         Objects.requireNonNull(legal, String.format(Checks.NOT_NULL, "Formats", "legal"));
+        Checks.existingFileCheck(toValidate);
         ValidationReport report = validate(toValidate);
         if (report.document == null || report.document.getFormat() == null) {
             report.add(toValidate.toString(), FACTORY.getError("DOC-6"));
@@ -77,7 +77,7 @@ public ValidationReport validate(final Path toValidate)
         Objects.requireNonNull(toValidate, String.format(Checks.NOT_NULL, "Path", TO_VAL_STRING));
 
         // Check if the path exists and is not a directory
-        existingFileCheck(toValidate);
+        Checks.existingFileCheck(toValidate);
 
         if (OdfPackages.isZip(toValidate)) {
             return validatePackage(toValidate);
@@ -94,14 +94,6 @@ private static final ValidationReport notOdf(final Path toValidate) {
         return report;
     }
 
-    static final void existingFileCheck(final Path toValidate) throws FileNotFoundException {
-        if (!Files.exists(toValidate)) {
-            throw new FileNotFoundException(errMessage(toValidate.toString(), " does not exist."));
-        } else if (Files.isDirectory(toValidate)) {
-            throw new IllegalArgumentException(errMessage(toValidate.toString(), " is a directory."));
-        }
-    }
-
     private ValidationReport validatePackage(final Path toValidate) throws ParserConfigurationException, SAXException, IOException {
             ValidatingParser parser = Validators.getValidatingParser();
             OdfPackage pckg = parser.parsePackage(toValidate);
@@ -133,8 +125,4 @@ private ValidationReport validateOpenDocumentXml(final Path toValidate) throws P
         report.add(toValidate.toString(), parseResult.getMessages());
         return report;
     }
-
-    private static final String errMessage(final String toValidate, final String subMess) {
-        return String.format("Supplied Path parameter: %s %s", toValidate, subMess);
-    }
 }
diff --git a/odf-core/src/main/java/org/openpreservation/utils/Checks.java b/odf-core/src/main/java/org/openpreservation/utils/Checks.java
@@ -1,5 +1,9 @@
 package org.openpreservation.utils;
 
+import java.io.FileNotFoundException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
 public class Checks {
     public static final String NOT_NULL = "%s parameter: %s must not be null.";
     public static final String NOT_EMPTY = "String parameter: %s must not be empty.";
@@ -8,4 +12,16 @@ public class Checks {
     private Checks() {
         throw new AssertionError("Should not be instantiated.");
     }
+
+    public static final void existingFileCheck(final Path toValidate) throws FileNotFoundException {
+        if (!Files.exists(toValidate)) {
+            throw new FileNotFoundException(errMessage(toValidate.toString(), " does not exist."));
+        } else if (Files.isDirectory(toValidate)) {
+            throw new IllegalArgumentException(errMessage(toValidate.toString(), " is a directory."));
+        }
+    }
+
+    public static final String errMessage(final String toValidate, final String subMess) {
+        return String.format("Supplied Path parameter: %s %s", toValidate, subMess);
+    }
 }
diff --git a/odf-core/src/test/java/org/openpreservation/odf/pkg/PackageParserTest.java b/odf-core/src/test/java/org/openpreservation/odf/pkg/PackageParserTest.java
@@ -7,6 +7,7 @@
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URISyntaxException;
@@ -38,6 +39,28 @@ public void testParseNullFile() throws ParserConfigurationException, SAXExceptio
                 });
     }
 
+    @Test
+    public void testParseMissingFile() throws ParserConfigurationException, SAXException {
+        PackageParser parser = OdfPackages.getPackageParser();
+        File file = new File("missingFileRubbishdhbna");
+        assertThrows("FileNotFoundException expected",
+                FileNotFoundException.class,
+                () -> {
+                    parser.parsePackage(file);
+                });
+    }
+
+    @Test
+    public void testParseDirFile() throws ParserConfigurationException, SAXException {
+        PackageParser parser = OdfPackages.getPackageParser();
+        File file = new File(".");
+        assertThrows("IllegalArgumentException expected",
+                IllegalArgumentException.class,
+                () -> {
+                    parser.parsePackage(file);
+                });
+    }
+
     @Test
     public void testParseNullPath() throws ParserConfigurationException, SAXException {
         PackageParser parser = OdfPackages.getPackageParser();
@@ -49,6 +72,28 @@ public void testParseNullPath() throws ParserConfigurationException, SAXExceptio
                 });
     }
 
+    @Test
+    public void testParseMissingPath() throws ParserConfigurationException, SAXException {
+        PackageParser parser = OdfPackages.getPackageParser();
+        Path path = Path.of(".", "missingFileRubbishdhbna");
+        assertThrows("FileNotFoundException expected",
+                FileNotFoundException.class,
+                () -> {
+                    parser.parsePackage(path);
+                });
+    }
+
+    @Test
+    public void testParseDirPath() throws ParserConfigurationException, SAXException {
+        PackageParser parser = OdfPackages.getPackageParser();
+        Path path = Path.of(".");
+        assertThrows("IllegalArgumentException expected",
+                IllegalArgumentException.class,
+                () -> {
+                    parser.parsePackage(path);
+                });
+    }
+
     @Test
     public void testParseNullStream() throws ParserConfigurationException, SAXException, IOException {
         PackageParser parser = OdfPackages.getPackageParser();
@@ -66,47 +111,54 @@ public void testParseNullStream() throws ParserConfigurationException, SAXExcept
     }
 
     @Test
-    public void testParsePackagePath() throws ParserConfigurationException, SAXException, URISyntaxException, IOException {
+    public void testParsePackagePath()
+            throws ParserConfigurationException, SAXException, URISyntaxException, IOException {
         PackageParser parser = OdfPackages.getPackageParser();
         OdfPackage pkg = parser.parsePackage(Paths.get(new File(TestFiles.EMPTY_ODS.toURI()).getAbsolutePath()));
         assertNotNull("Parsed package should not be null", pkg);
         assertTrue("Package should have a mimetype entry", pkg.hasMimeEntry());
-        assertEquals("Mimetype should be Spreadsheet", "application/vnd.oasis.opendocument.spreadsheet", pkg.getMimeType());
+        assertEquals("Mimetype should be Spreadsheet", "application/vnd.oasis.opendocument.spreadsheet",
+                pkg.getMimeType());
     }
 
     @Test
-    public void testParsePackageFile() throws ParserConfigurationException, SAXException, URISyntaxException, IOException {
+    public void testParsePackageFile()
+            throws ParserConfigurationException, SAXException, URISyntaxException, IOException {
         PackageParser parser = OdfPackages.getPackageParser();
         OdfPackage pkg = parser.parsePackage(new File(TestFiles.EMPTY_ODS.toURI()));
         assertNotNull("Parsed package should not be null", pkg);
         assertTrue("Package should have a mimetype entry", pkg.hasMimeEntry());
-        assertEquals("Mimetype should be Spreadsheet", "application/vnd.oasis.opendocument.spreadsheet", pkg.getMimeType());
+        assertEquals("Mimetype should be Spreadsheet", "application/vnd.oasis.opendocument.spreadsheet",
+                pkg.getMimeType());
     }
 
     @Test
-    public void testParsePackageStream() throws ParserConfigurationException, SAXException, URISyntaxException, IOException {
+    public void testParsePackageStream()
+            throws ParserConfigurationException, SAXException, URISyntaxException, IOException {
         PackageParser parser = OdfPackages.getPackageParser();
         OdfPackage pkg = parser.parsePackage(TestFiles.EMPTY_ODS.openStream(), TestFiles.EMPTY_ODS.toString());
         assertNotNull("Parsed package should not be null", pkg);
         assertTrue("Package should have a mimetype entry", pkg.hasMimeEntry());
-        assertEquals("Mimetype should be Spreadsheet", "application/vnd.oasis.opendocument.spreadsheet", pkg.getMimeType());
+        assertEquals("Mimetype should be Spreadsheet", "application/vnd.oasis.opendocument.spreadsheet",
+                pkg.getMimeType());
     }
 
     @Test
     public void testDsigParsing() throws ParserConfigurationException, SAXException, IOException {
         PackageParser parser = OdfPackages.getPackageParser();
         OdfPackage pkg = parser.parsePackage(TestFiles.DSIG_INVALID.openStream(), TestFiles.DSIG_INVALID.toString());
         ParseResult result = pkg.getEntryXmlParseResult("META-INF/documentsignatures.xml");
-        assertNotNull("Dsig file META-INF/documentsignatures.xml result should not be null" , result);
-        assertTrue("Package should have a well formed dsig for META-INF/documentsignatures.xml" , result.isWellFormed());
+        assertNotNull("Dsig file META-INF/documentsignatures.xml result should not be null", result);
+        assertTrue("Package should have a well formed dsig for META-INF/documentsignatures.xml", result.isWellFormed());
     }
 
     @Test
     public void testManifestNotWF() throws IOException {
         PackageParser parser = OdfPackages.getPackageParser();
-        OdfPackage pkg = parser.parsePackage(TestFiles.MANIFEST_NOT_WF.openStream(), TestFiles.MANIFEST_NOT_WF.toString());
+        OdfPackage pkg = parser.parsePackage(TestFiles.MANIFEST_NOT_WF.openStream(),
+                TestFiles.MANIFEST_NOT_WF.toString());
         ParseResult result = pkg.getEntryXmlParseResult("META-INF/manifest.xml");
-        assertNotNull("Dsig file META-INF/documentsignatures.xml result should not be null" , result);
-        assertFalse("Package should NOT have a well formed META-INF/manifest.xml" , result.isWellFormed());
+        assertNotNull("Dsig file META-INF/documentsignatures.xml result should not be null", result);
+        assertFalse("Package should NOT have a well formed META-INF/manifest.xml", result.isWellFormed());
     }
 }
diff --git a/odf-reporting/pom.xml b/odf-reporting/pom.xml
@@ -5,7 +5,7 @@
   <parent>
     <groupId>org.openpreservation.odf</groupId>
     <artifactId>odf-validator</artifactId>
-    <version>0.10.0</version>
+    <version>0.11.0</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
 

diff --git a/odf-validator b/odf-validator
@@ -78,5 +78,5 @@ fi
 
 exec "$JAVACMD" -Dfile.encoding=UTF8 -XX:+IgnoreUnrecognizedVMOptions \
   -Dapp.name="ODF Validator" \
-  -jar odf-apps/target/odf-apps-0.10.0-jar-with-dependencies.jar \
+  -jar odf-apps/target/odf-apps-0.11.0-jar-with-dependencies.jar \
   "$@"
diff --git a/odf-validator.bat b/odf-validator.bat
@@ -85,7 +85,7 @@ if NOT "%CLASSPATH_PREFIX%" == "" set CLASSPATH=%CLASSPATH_PREFIX%;%CLASSPATH%
 @REM Reaching here means variables are defined and arguments have been captured
 :endInit
 
-"%JAVACMD%" -Dfile.encoding=UTF8 -XX:+IgnoreUnrecognizedVMOptions -Dapp.name="ODF Validator" -jar .\odf-apps\target\odf-apps-0.10.0-jar-with-dependencies.jar %CMD_LINE_ARGS%
+"%JAVACMD%" -Dfile.encoding=UTF8 -XX:+IgnoreUnrecognizedVMOptions -Dapp.name="ODF Validator" -jar .\odf-apps\target\odf-apps-0.11.0-jar-with-dependencies.jar %CMD_LINE_ARGS%
 if %ERRORLEVEL% NEQ 0 goto error
 goto end
 

diff --git a/pom.xml b/pom.xml
@@ -10,7 +10,7 @@
 
   <groupId>org.openpreservation.odf</groupId>
   <artifactId>odf-validator</artifactId>
-  <version>0.10.0</version>
+  <version>0.11.0</version>
   <packaging>pom</packaging>
 
   <modules>