FIX: Unimplemented profile checks

- for unimplemented profile checks against a single file, return an empty message log; - added protection against missing package at the start of the profile check; - added some null checks to profile checking methods; and - removed some unused imports and parameters. TODO: Implement schematron checks for a single file.
openpreserve · Nov 14, 2024 · 40ecf86 · 40ecf86
1 parent d5937c4
commit 40ecf86
Show file tree

Hide file tree

Showing 14 changed files with 56 additions and 50 deletions.
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
@@ -16,8 +16,6 @@
 import org.openpreservation.messages.MessageFactory;
 import org.openpreservation.messages.MessageLog;
 import org.openpreservation.messages.Messages;
-import org.openpreservation.odf.pkg.OdfPackages;
-import org.openpreservation.odf.pkg.PackageParser;
 import org.openpreservation.odf.pkg.PackageParser.ParseException;
 import org.openpreservation.odf.validation.Profile;
 import org.openpreservation.odf.validation.ProfileResult;
@@ -42,7 +40,6 @@ class CliValidator implements Callable<Integer> {
     private File[] toValidateFiles;
     private final Validator validator = new Validator();
     private MessageLog appMessages = Messages.messageLogInstance();
-    private final PackageParser parser = OdfPackages.getPackageParser();
 
     @Override
     public Integer call() {

diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/rules/AbstractRule.java b/odf-core/src/main/java/org/openpreservation/odf/validation/rules/AbstractRule.java
@@ -64,11 +64,13 @@ public MessageLog check(ValidationReport report) throws ParseException {
 
     @Override
     public MessageLog check(final OdfXmlDocument document) throws ParseException {
+        Objects.requireNonNull(document, "document must not be null");
         return Messages.messageLogInstance();
     }
 
     @Override
     public MessageLog check(final OdfPackage odfPackage) throws ParseException {
+        Objects.requireNonNull(odfPackage, "odfPackage must not be null");
         return Messages.messageLogInstance();
     }
 

diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/rules/DigitalSignaturesRule.java b/odf-core/src/main/java/org/openpreservation/odf/validation/rules/DigitalSignaturesRule.java
@@ -8,8 +8,6 @@
 import org.openpreservation.odf.pkg.OdfPackage;
 import org.openpreservation.odf.pkg.OdfPackages;
 import org.openpreservation.odf.pkg.PackageParser.ParseException;
-import org.openpreservation.odf.validation.ValidationReport;
-import org.openpreservation.odf.xml.OdfXmlDocument;
 
 final class DigitalSignaturesRule extends AbstractRule {
 
@@ -23,11 +21,6 @@ private DigitalSignaturesRule(final String id, final String name, final String d
         super(id, name, description, severity, isPrerequisite);
     }
 
-    @Override
-    public MessageLog check(final OdfXmlDocument document) {
-        throw new UnsupportedOperationException("Unimplemented method 'check'");
-    }
-
     @Override
     public MessageLog check(final OdfPackage odfPackage) throws ParseException {
         Objects.requireNonNull(odfPackage, "odfPackage must not be null");

diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/rules/EmbeddedObjectsRule.java b/odf-core/src/main/java/org/openpreservation/odf/validation/rules/EmbeddedObjectsRule.java
@@ -28,7 +28,8 @@ private EmbeddedObjectsRule(final String id, final String name, final String des
 
     @Override
     public MessageLog check(final OdfXmlDocument document) throws ParseException {
-        throw new UnsupportedOperationException("Unimplemented method 'check'");
+        Objects.requireNonNull(document, "document must not be null");
+        return this.schematron.check(document);
     }
 
     @Override

diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/rules/MacroRule.java b/odf-core/src/main/java/org/openpreservation/odf/validation/rules/MacroRule.java
@@ -2,6 +2,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Objects;
 
 import javax.xml.parsers.ParserConfigurationException;
 
@@ -37,11 +38,13 @@ private MacroRule(final String id, final String name, final String description,
 
     @Override
     public MessageLog check(final OdfXmlDocument document) throws ParseException {
-        throw new UnsupportedOperationException("Unimplemented method 'check'");
+        Objects.requireNonNull(document, "document must not be null");
+        return this.schematron.check(document);
     }
 
     @Override
     public MessageLog check(final OdfPackage odfPackage) throws ParseException {
+        Objects.requireNonNull(odfPackage, "odfPackage must not be null");
         MessageLog messageLog;
         try {
             messageLog = checkOdfScriptXml(odfPackage);

diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/rules/PackageMimeTypeRule.java b/odf-core/src/main/java/org/openpreservation/odf/validation/rules/PackageMimeTypeRule.java
@@ -8,7 +8,6 @@
 import org.openpreservation.odf.pkg.OdfPackage;
 import org.openpreservation.odf.pkg.OdfPackages;
 import org.openpreservation.odf.pkg.PackageParser.ParseException;
-import org.openpreservation.odf.xml.OdfXmlDocument;
 
 final class PackageMimeTypeRule extends AbstractRule {
 

diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/rules/ProfileImpl.java b/odf-core/src/main/java/org/openpreservation/odf/validation/rules/ProfileImpl.java
@@ -52,7 +52,9 @@ public ProfileResult check(final ValidationReport report) throws ParseException
             messages.add(getRulesetMessages(report,
                     this.rules.stream().filter(rule -> !rule.isPrerequisite()).collect(Collectors.toList())));
         }
-            return ProfileResultImpl.of(report.document == null ? "" : report.document.getPackage().getName(), this.name,
+        final String packageName = report.document == null || report.document.getPackage() == null ? ""
+                : report.document.getPackage().getName();
+        return ProfileResultImpl.of(packageName, this.name,
                 report, messages);
     }
 

diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/rules/SchematronRule.java b/odf-core/src/main/java/org/openpreservation/odf/validation/rules/SchematronRule.java
@@ -14,7 +14,6 @@
 import org.openpreservation.odf.pkg.OdfPackage;
 import org.openpreservation.odf.pkg.OdfPackages;
 import org.openpreservation.odf.pkg.PackageParser.ParseException;
-import org.openpreservation.odf.xml.OdfXmlDocument;
 
 import com.helger.schematron.pure.SchematronResourcePure;
 import com.helger.schematron.svrl.AbstractSVRLMessage;
@@ -36,11 +35,6 @@ private SchematronRule(final String id, final String name, final String descript
         this.schematron = SchematronResourcePure.fromURL(schematronLoc);
     }
 
-    @Override
-    public MessageLog check(final OdfXmlDocument document) throws ParseException {
-        throw new UnsupportedOperationException("Unimplemented method 'check'");
-    }
-
     @Override
     public MessageLog check(final OdfPackage odfPackage) throws ParseException {
         Objects.requireNonNull(odfPackage, "odfPackage must not be null");

diff --git a/odf-core/src/test/java/org/openpreservation/odf/validation/rules/ContentTest.java b/odf-core/src/test/java/org/openpreservation/odf/validation/rules/ContentTest.java
@@ -36,8 +36,8 @@ public void testGetInstance() {
     public void testCheckNullXmlDoc() {
         Rule rule = Rules.odf7();
         OdfXmlDocument nullDoc = null;
-        assertThrows("UnsupportedOperationException expected",
-        UnsupportedOperationException.class,
+        assertThrows("NullPointerException expected",
+        NullPointerException.class,
                 () -> {
                     rule.check(nullDoc);
                 });

diff --git a/...re/src/test/java/org/openpreservation/odf/validation/rules/DigitalSignaturesRuleTest.java b/...re/src/test/java/org/openpreservation/odf/validation/rules/DigitalSignaturesRuleTest.java
@@ -39,8 +39,8 @@ public void testGetInstance() {
     @Test
     public void testCheckNullXmlDoc() {
         OdfXmlDocument nullDoc = null;
-        assertThrows("UnsupportedOperationException expected",
-        UnsupportedOperationException.class,
+        assertThrows("NullPointerException expected",
+                NullPointerException.class,
                 () -> {
                     rule.check(nullDoc);
                 });
@@ -50,7 +50,7 @@ public void testCheckNullXmlDoc() {
     public void testCheckNullPackage() {
         OdfPackage nullPkg = null;
         assertThrows("NullPointerException expected",
-        NullPointerException.class,
+                NullPointerException.class,
                 () -> {
                     rule.check(nullPkg);
                 });
@@ -94,6 +94,7 @@ public void testCheckValidDsigPackage() throws IOException, URISyntaxException,
         OdfPackage pkg = parser.parsePackage(Paths.get(new File(TestFiles.DSIG_VALID.toURI()).getAbsolutePath()));
         MessageLog results = rule.check(pkg);
         assertTrue("File contains valid digital signatures.", results.hasErrors());
-        assertEquals(1, results.getMessages().values().stream().filter(m -> m.stream().filter(e -> e.getId().equals("POL_9")).count() > 0).count());
+        assertEquals(1, results.getMessages().values().stream()
+                .filter(m -> m.stream().filter(e -> e.getId().equals("POL_9")).count() > 0).count());
     }
 }
diff --git a/...core/src/test/java/org/openpreservation/odf/validation/rules/EmbeddedObjectsRuleTest.java b/...core/src/test/java/org/openpreservation/odf/validation/rules/EmbeddedObjectsRuleTest.java
@@ -32,6 +32,7 @@
 
 public class EmbeddedObjectsRuleTest {
     private final Rule rule = Rules.odf6();
+
     @Test
     public void testGetInstance() {
         final SchematronResourcePure schematron = EmbeddedObjectsRule.getInstance(Severity.INFO).schematron.schematron;
@@ -41,8 +42,8 @@ public void testGetInstance() {
     @Test
     public void testCheckNullXmlDoc() {
         OdfXmlDocument nullDoc = null;
-        assertThrows("UnsupportedOperationException expected",
-        UnsupportedOperationException.class,
+        assertThrows("NullPointerException expected",
+                NullPointerException.class,
                 () -> {
                     rule.check(nullDoc);
                 });
@@ -52,7 +53,7 @@ public void testCheckNullXmlDoc() {
     public void testCheckNullPackage() {
         OdfPackage nullPkg = null;
         assertThrows("NullPointerException expected",
-        NullPointerException.class,
+                NullPointerException.class,
                 () -> {
                     rule.check(nullPkg);
                 });
@@ -91,7 +92,8 @@ public void testCheckValidPackage() throws IOException, URISyntaxException, Pars
     @Test
     public void testCheckEmbeddedPackage() throws IOException, URISyntaxException, ParseException {
         PackageParser parser = OdfPackages.getPackageParser();
-        OdfPackage pkg = parser.parsePackage(Paths.get(new File(TestFiles.OLE_EMBEDDED_PACKAGE.toURI()).getAbsolutePath()));
+        OdfPackage pkg = parser
+                .parsePackage(Paths.get(new File(TestFiles.OLE_EMBEDDED_PACKAGE.toURI()).getAbsolutePath()));
         MessageLog results = rule.check(pkg);
         assertFalse("Valid Package should not return errors.", results.hasErrors());
         assertTrue("Valid Package should have info messages.", results.hasInfos());

diff --git a/odf-core/src/test/java/org/openpreservation/odf/validation/rules/ExternalDataTest.java b/odf-core/src/test/java/org/openpreservation/odf/validation/rules/ExternalDataTest.java
@@ -36,8 +36,8 @@ public void testGetInstance() {
     public void testCheckNullXmlDoc() {
         Rule rule = Rules.odf5();
         OdfXmlDocument nullDoc = null;
-        assertThrows("UnsupportedOperationException expected",
-        UnsupportedOperationException.class,
+        assertThrows("NullPointerException expected",
+                NullPointerException.class,
                 () -> {
                     rule.check(nullDoc);
                 });
@@ -48,7 +48,7 @@ public void testCheckNullPackage() {
         Rule rule = Rules.odf5();
         OdfPackage nullPkg = null;
         assertThrows("NullPointerException expected",
-        NullPointerException.class,
+                NullPointerException.class,
                 () -> {
                     rule.check(nullPkg);
                 });
@@ -80,12 +80,14 @@ public void testSchematronExternalDataPass() throws Exception {
     public void testPackageExternalDataFail() throws Exception {
         final Rule odf5 = Rules.odf5();
         PackageParser parser = OdfPackages.getPackageParser();
-        OdfPackage pkg = parser.parsePackage(Paths.get(new File(TestFiles.SCHEMATRON_CHECKER_ODS.toURI()).getAbsolutePath()));
+        OdfPackage pkg = parser
+                .parsePackage(Paths.get(new File(TestFiles.SCHEMATRON_CHECKER_ODS.toURI()).getAbsolutePath()));
         MessageLog messages = odf5.check(pkg);
         assertNotNull(messages);
         assertEquals(0, messages.getErrors().size());
         assertEquals(1, messages.getInfos().size());
-        assertEquals(1, messages.getMessages().values().stream().filter(m -> m.stream().filter(e -> e.getId().equals("POL_5")).count() > 0).count());
+        assertEquals(1, messages.getMessages().values().stream()
+                .filter(m -> m.stream().filter(e -> e.getId().equals("POL_5")).count() > 0).count());
     }
 
     @Test

diff --git a/odf-core/src/test/java/org/openpreservation/odf/validation/rules/MacrosTest.java b/odf-core/src/test/java/org/openpreservation/odf/validation/rules/MacrosTest.java
@@ -28,6 +28,7 @@
 
 public class MacrosTest {
     private final Rule rule = Rules.odf8();
+
     @Test
     public void testGetInstance() {
         final SchematronResourcePure schematron = MacroRule.getInstance(Severity.ERROR).schematron.schematron;
@@ -37,8 +38,8 @@ public void testGetInstance() {
     @Test
     public void testCheckNullXmlDoc() {
         OdfXmlDocument nullDoc = null;
-        assertThrows("UnsupportedOperationException expected",
-        UnsupportedOperationException.class,
+        assertThrows("NullPointerException expected",
+                NullPointerException.class,
                 () -> {
                     rule.check(nullDoc);
                 });
@@ -48,7 +49,7 @@ public void testCheckNullXmlDoc() {
     public void testCheckNullPackage() {
         OdfPackage nullPkg = null;
         assertThrows("NullPointerException expected",
-        NullPointerException.class,
+                NullPointerException.class,
                 () -> {
                     rule.check(nullPkg);
                 });
@@ -83,7 +84,8 @@ public void testPackageMacroFail() throws Exception {
         MessageLog messages = rule.check(pkg);
         assertNotNull(messages);
         assertEquals(2, messages.getErrors().size());
-        assertEquals(2, messages.getMessages().values().stream().filter(m -> m.stream().filter(e -> e.getId().equals("POL_8")).count() > 0).count());
+        assertEquals(2, messages.getMessages().values().stream()
+                .filter(m -> m.stream().filter(e -> e.getId().equals("POL_8")).count() > 0).count());
     }
 
     @Test
@@ -102,6 +104,7 @@ public void testPackageStarBasicFail() throws Exception {
         MessageLog messages = rule.check(pkg);
         assertNotNull(messages);
         assertEquals(1, messages.getErrors().size());
-        assertEquals(1, messages.getMessages().values().stream().filter(m -> m.stream().filter(e -> e.getId().equals("POL_8")).count() > 0).count());
+        assertEquals(1, messages.getMessages().values().stream()
+                .filter(m -> m.stream().filter(e -> e.getId().equals("POL_8")).count() > 0).count());
     }
 }
diff --git a/...core/src/test/java/org/openpreservation/odf/validation/rules/PackageMimeTypeRuleTest.java b/...core/src/test/java/org/openpreservation/odf/validation/rules/PackageMimeTypeRuleTest.java
@@ -25,6 +25,7 @@
 
 public class PackageMimeTypeRuleTest {
     private final Rule rule = Rules.odf3();
+
     @Test
     public void testEqualsContract() {
         EqualsVerifier.forClass(PackageMimeTypeRule.class).verify();
@@ -38,16 +39,18 @@ public void testGetInstance() {
     @Test
     public void testCheckNullXmlDoc() throws ParseException {
         OdfXmlDocument nullDoc = null;
-        MessageLog log = rule.check(nullDoc);
-        assertNotNull(log);
-        assertTrue(log.isEmpty());
+        assertThrows("NullPointerException expected",
+                NullPointerException.class,
+                () -> {
+                    rule.check(nullDoc);
+                });
     }
 
     @Test
     public void testCheckNullPackage() {
         OdfPackage nullPkg = null;
         assertThrows("NullPointerException expected",
-        NullPointerException.class,
+                NullPointerException.class,
                 () -> {
                     rule.check(nullPkg);
                 });
@@ -67,7 +70,8 @@ public void testCheckNotZipPackage() throws IOException, URISyntaxException, Par
         OdfPackage pkg = parser.parsePackage(Paths.get(new File(TestFiles.EMPTY_FODS.toURI()).getAbsolutePath()));
         MessageLog results = rule.check(pkg);
         assertTrue("Document XML should return errors", results.hasErrors());
-        assertEquals(1, results.getMessages().values().stream().filter(m -> m.stream().filter(e -> e.getId().equals("POL_3")).count() > 0).count());
+        assertEquals(1, results.getMessages().values().stream()
+                .filter(m -> m.stream().filter(e -> e.getId().equals("POL_3")).count() > 0).count());
     }
 
     @Test
@@ -76,7 +80,8 @@ public void testCheckNotWellFormedPackage() throws IOException, URISyntaxExcepti
         OdfPackage pkg = parser.parsePackage(Paths.get(new File(TestFiles.BADLY_FORMED_PKG.toURI()).getAbsolutePath()));
         MessageLog results = rule.check(pkg);
         assertTrue("Badly formed package should return errors", results.hasErrors());
-        assertEquals(1, results.getMessages().values().stream().filter(m -> m.stream().filter(e -> e.getId().equals("POL_3")).count() > 0).count());
+        assertEquals(1, results.getMessages().values().stream()
+                .filter(m -> m.stream().filter(e -> e.getId().equals("POL_3")).count() > 0).count());
     }
 
     @Test
@@ -93,16 +98,18 @@ public void testCheckNoMimePackage() throws IOException, URISyntaxException, Par
         OdfPackage pkg = parser.parsePackage(Paths.get(new File(TestFiles.NO_MIME_ROOT_ODS.toURI()).getAbsolutePath()));
         MessageLog results = rule.check(pkg);
         assertTrue("No mimetype with root entry (invalid) return errors", results.hasErrors());
-        assertEquals(1, results.getMessages().values().stream().filter(m -> m.stream().filter(e -> e.getId().equals("POL_3")).count() > 0).count());
+        assertEquals(1, results.getMessages().values().stream()
+                .filter(m -> m.stream().filter(e -> e.getId().equals("POL_3")).count() > 0).count());
     }
 
     @Test
     public void testCheckNoMimeNoRootPackage() throws IOException, URISyntaxException, ParseException {
         PackageParser parser = OdfPackages.getPackageParser();
-        OdfPackage pkg = parser.parsePackage(Paths.get(new File(TestFiles.NO_MIME_NO_ROOT_ODS.toURI()).getAbsolutePath()));
+        OdfPackage pkg = parser
+                .parsePackage(Paths.get(new File(TestFiles.NO_MIME_NO_ROOT_ODS.toURI()).getAbsolutePath()));
         MessageLog results = rule.check(pkg);
         assertTrue("No mimetype with no root entry (valid) should return errors", results.hasErrors());
-        assertEquals(1, results.getMessages().values().stream().filter(m -> m.stream().filter(e -> e.getId().equals("POL_3")).count() > 0).count());
+        assertEquals(1, results.getMessages().values().stream()
+                .filter(m -> m.stream().filter(e -> e.getId().equals("POL_3")).count() > 0).count());
     }
 }
-