REFACT: Tidied validator/profile calls

- profile is now invoked after validation rather than as a branch in CLI;
- added profiling via `ValidationReport` as half-way house before interface refactor;
- added Rule checking via `ValidationReport` and provided a simple method in `AbstractProfile` that calls existing `validate` method;
- improved some variable and method names;and
- made some CLI methods static.

odf-apps/src/main/java/org/openpreservation/odf/apps/CliValidator.java

@@ -9,6 +9,8 @@
 import java.util.Map;
 import java.util.concurrent.Callable;
 
+import javax.xml.parsers.ParserConfigurationException;
+
 import org.openpreservation.messages.Message;
 import org.openpreservation.messages.Message.Severity;
 import org.openpreservation.messages.MessageFactory;
@@ -22,6 +24,7 @@
 import org.openpreservation.odf.validation.ValidationReport;
 import org.openpreservation.odf.validation.Validator;
 import org.openpreservation.odf.validation.rules.Rules;
+import org.xml.sax.SAXException;
 
 import picocli.CommandLine;
 import picocli.CommandLine.Command;
@@ -38,7 +41,6 @@ class CliValidator implements Callable<Integer> {
     @Parameters(paramLabel = "FILE", arity = "1..*", description = "A list of Open Document Format spreadsheet files to validate.")
     private File[] toValidateFiles;
     private final Validator validator = new Validator();
-    private final Profile dnaProfile = Rules.getDnaProfile();
     private MessageLog appMessages = Messages.messageLogInstance();
     private final PackageParser parser = OdfPackages.getPackageParser();
 
@@ -49,15 +51,14 @@ public Integer call() {
             Path toValidate = file.toPath();
             this.appMessages = Messages.messageLogInstance();
             ConsoleFormatter.colourise(FACTORY.getInfo("APP-1", toValidate.toString()));
-            if (this.profileFlag) {
-                final ProfileResult profileResult = profilePath(toValidate);
-                if (profileResult != null) {
-                    retStatus = Math.max(retStatus, results(toValidate, profileResult));
-                }
-            } else {
-                final ValidationReport validationReport = validatePath(toValidate);
-                if (validationReport != null) {
-                    retStatus = Math.max(retStatus, results(toValidate, validationReport));
+            final ValidationReport validationReport = validatePath(toValidate);
+            if (validationReport != null) {
+                retStatus = Math.max(retStatus, results(toValidate, validationReport));
+                if (this.profileFlag) {
+                    final ProfileResult profileResult = profileReport(validationReport, toValidate);
+                    if (profileResult != null) {
+                        retStatus = Math.max(retStatus, results(toValidate, profileResult));
+                    }
                 }
             }
             if (this.appMessages.hasErrors()) {
@@ -80,13 +81,14 @@ private ValidationReport validatePath(final Path toValidate) {
         return null;
     }
 
-    private ProfileResult profilePath(final Path toProfile) {
+    private ProfileResult profileReport(final ValidationReport toProfile, final Path path) {
         try {
-            return dnaProfile.check(parser.parsePackage(toProfile));
-        } catch (IllegalArgumentException | FileNotFoundException e) {
-            this.logMessage(toProfile, Messages.getMessageInstance("APP-2", Severity.ERROR, e.getMessage()));
-        } catch (ParseException e) {
-            this.logMessage(toProfile, Messages.getMessageInstance("SYS-1", Severity.ERROR,
+            final Profile dnaProfile = Rules.getDnaProfile();
+            return dnaProfile.check(toProfile);
+        } catch (IllegalArgumentException e) {
+            this.logMessage(path, Messages.getMessageInstance("APP-2", Severity.ERROR, e.getMessage()));
+        } catch (ParseException | ParserConfigurationException | SAXException e) {
+            this.logMessage(path, Messages.getMessageInstance("SYS-1", Severity.ERROR,
                     "Package could not be parsed, due to an exception.", e.getMessage()));
         }
         return null;
@@ -116,7 +118,7 @@ private Integer processMessageList(final List<Message> messages) {
         return status;
     }
 
-    private Integer results(final Path path, final ValidationReport report) {
+    private static Integer results(final Path path, final ValidationReport report) {
         Integer status = 0;
         ConsoleFormatter.colourise(FACTORY.getInfo("APP-4", path.toString(), "bold"));
         if (report.getMessages().isEmpty()) {
@@ -127,21 +129,21 @@ private Integer results(final Path path, final ValidationReport report) {
         return status;
     }
 
-    private Integer results(final Map<String, List<Message>> messageMap) {
+    private static Integer results(final Map<String, List<Message>> messageMap) {
         Integer status = 0;
         for (Map.Entry<String, List<Message>> entry : messageMap.entrySet()) {
             status = Math.max(status, results(entry.getKey(), entry.getValue()));
         }
         return status;
     }
 
-    private Integer results(final Path path, final ProfileResult report) {
+    private static Integer results(final Path path, final ProfileResult report) {
         Integer status = 0;
-        ConsoleFormatter.colourise(FACTORY.getInfo("APP-5", this.dnaProfile.getName(), path.toString(), "bold"));
+        ConsoleFormatter.colourise(FACTORY.getInfo("APP-5", report.getName(), path.toString(), "bold"));
         if (report.getValidationReport() != null) {
             status = results(report.getValidationReport().documentMessages.getMessages());
         }
-        for (Map.Entry<String, List<Message>> entry : report.getProfileMessages().getMessages().entrySet()) {
+        for (Map.Entry<String, List<Message>> entry : report.getMessageLog().getMessages().entrySet()) {
             status = Math.max(status, results(entry.getKey(), entry.getValue()));
         }
         if (report.getValidationReport() != null && report.getValidationReport().documentMessages.hasErrors()) {
@@ -150,12 +152,12 @@ private Integer results(final Path path, final ProfileResult report) {
         MessageLog profileMessages = (report.getValidationReport() != null)
                 ? report.getValidationReport().documentMessages
                 : Messages.messageLogInstance();
-        profileMessages.add(report.getProfileMessages().getMessages());
+        profileMessages.add(report.getMessageLog().getMessages());
         outputSummary(profileMessages);
         return status;
     }
 
-    private Integer results(final String path, final List<Message> messages) {
+    private static Integer results(final String path, final List<Message> messages) {
         Integer status = 0;
         for (Message message : messages) {
             ConsoleFormatter.colourise(Paths.get(path), message);
@@ -166,7 +168,7 @@ private Integer results(final String path, final List<Message> messages) {
         return status;
     }
 
-    private void outputSummary(final MessageLog messageLog) {
+    private static void outputSummary(final MessageLog messageLog) {
         if (messageLog.hasErrors()) {
             ConsoleFormatter.error(String.format("NOT VALID, %d errors, %d warnings and %d info messages.",
                     messageLog.getErrorCount(), messageLog.getWarningCount(), messageLog.getInfoCount()));

odf-core/src/main/java/org/openpreservation/odf/validation/Profile.java

@@ -8,9 +8,16 @@
 
 public interface Profile {
     public String getId();
+
     public String getName();
+
     public String getDescription();
+
     public ProfileResult check(final OdfXmlDocument document) throws ParseException;
+
+    public ProfileResult check(final ValidationReport report) throws ParseException;
+
     public ProfileResult check(final OdfPackage odfPackage) throws ParseException;
+
     public Set<Rule> getRules();
 }

odf-core/src/main/java/org/openpreservation/odf/validation/ProfileResult.java

@@ -4,7 +4,12 @@
 
 public interface ProfileResult {
     public String getId();
+
+    public String getName();
+
     public ValidationReport getValidationReport();
-    public MessageLog getProfileMessages();
+
+    public MessageLog getMessageLog();
+
     public boolean isValid();
 }

odf-core/src/main/java/org/openpreservation/odf/validation/Rule.java

@@ -8,10 +8,18 @@
 
 public interface Rule {
     public String getId();
+
     public String getName();
+
     public String getDescription();
+
     public Severity getSeverity();
+
     public boolean isPrerequisite();
+
     public MessageLog check(final OdfXmlDocument document) throws ParseException;
+
     public MessageLog check(final OdfPackage odfPackage) throws ParseException;
+
+    public MessageLog check(final ValidationReport report) throws ParseException;
 }

odf-core/src/main/java/org/openpreservation/odf/validation/ValidationReport.java

@@ -11,7 +11,7 @@
 import org.openpreservation.odf.document.OpenDocument;
 
 public final class ValidationReport {
-    final String name;
+    public final String name;
     public final OpenDocument document;
     public final MessageLog documentMessages;
 
@@ -33,6 +33,7 @@ private ValidationReport(final String name, final OpenDocument document, final M
     static final ValidationReport of(final String name) {
         return new ValidationReport(name);
     }
+
     static final ValidationReport of(final String name, final OpenDocument document) {
         return new ValidationReport(name, document);
     }
@@ -41,7 +42,8 @@ static final ValidationReport of(final String name, final MessageLog documentMes
         return new ValidationReport(name, null, documentMessages);
     }
 
-    static final ValidationReport of(final String name, final OpenDocument document, final MessageLog documentMessages) {
+    static final ValidationReport of(final String name, final OpenDocument document,
+            final MessageLog documentMessages) {
         return new ValidationReport(name, document, documentMessages);
     }
 
@@ -69,6 +71,7 @@ public void addAll(final Map<String, List<Message>> messages) {
     public List<Message> getErrors() {
         return documentMessages.getErrors().values().stream().flatMap(List::stream).collect(Collectors.toList());
     }
+
     public List<Message> getMessages() {
         return documentMessages.getMessages().values().stream().flatMap(List::stream).collect(Collectors.toList());
     }

odf-core/src/main/java/org/openpreservation/odf/validation/rules/AbstractRule.java

@@ -3,7 +3,10 @@
 import java.util.Objects;
 
 import org.openpreservation.messages.Message.Severity;
+import org.openpreservation.messages.MessageLog;
+import org.openpreservation.odf.pkg.PackageParser.ParseException;
 import org.openpreservation.odf.validation.Rule;
+import org.openpreservation.odf.validation.ValidationReport;
 
 abstract class AbstractRule implements Rule {
     final String id;
@@ -47,6 +50,11 @@ public boolean isPrerequisite() {
         return this.isPrerequisite.booleanValue();
     }
 
+    @Override
+    public MessageLog check(ValidationReport report) throws ParseException {
+        return report.document.isPackage() ? check(report.document.getPackage()) : check(report.document.getDocument().getXmlDocument());
+    }
+
     @Override
     public final int hashCode() {
         return Objects.hash(id, name, description, severity, isPrerequisite);

odf-core/src/main/java/org/openpreservation/odf/validation/rules/DigitalSignaturesRule.java

@@ -8,6 +8,7 @@
 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 {

odf-core/src/main/java/org/openpreservation/odf/validation/rules/ProfileImpl.java

@@ -1,51 +1,64 @@
 package org.openpreservation.odf.validation.rules;
 
+import java.io.FileNotFoundException;
 import java.util.Collection;
 import java.util.Set;
 import java.util.Map;
 import java.util.List;
 import java.util.stream.Collectors;
 
+import javax.xml.parsers.ParserConfigurationException;
+
 import org.openpreservation.messages.Message;
 import org.openpreservation.messages.MessageLog;
 import org.openpreservation.messages.Messages;
 import org.openpreservation.odf.pkg.OdfPackage;
 import org.openpreservation.odf.pkg.PackageParser.ParseException;
 import org.openpreservation.odf.validation.ProfileResult;
 import org.openpreservation.odf.validation.Rule;
+import org.openpreservation.odf.validation.ValidatingParser;
 import org.openpreservation.odf.validation.ValidationReport;
+import org.openpreservation.odf.validation.Validators;
+import org.xml.sax.SAXException;
 
 final class ProfileImpl extends AbstractProfile {
-    private ValidationReport report = null;
-
-    static final ProfileImpl of(final String id, final String name, final String description, final Set<Rule> rules) {
+    static final ProfileImpl of(final String id, final String name, final String description, final Set<Rule> rules) throws ParserConfigurationException, SAXException {
         return new ProfileImpl(id, name, description, rules);
     }
 
-    private ProfileImpl(final String id, final String name, final String description, final Set<Rule> rules) {
+    private final ValidatingParser validatingParser = Validators.getValidatingParser();
+
+
+    private ProfileImpl(final String id, final String name, final String description, final Set<Rule> rules) throws ParserConfigurationException, SAXException {
         super(id, name, description, rules);
     }
 
     @Override
     public ProfileResult check(final OdfPackage odfPackage) throws ParseException {
+        try {
+            return check(this.validatingParser.validatePackage(odfPackage));
+        } catch (FileNotFoundException e) {
+            throw new ParseException("File not found exception when processing package.", e);
+        }
+    }
+
+    @Override
+    public ProfileResult check(final ValidationReport report) throws ParseException {
         final MessageLog messages = Messages.messageLogInstance();
-        messages.add(getRulesetMessages(odfPackage,
+        messages.add(getRulesetMessages(report.document.getPackage(),
                 this.rules.stream().filter(Rule::isPrerequisite).collect(Collectors.toList())));
         if (!messages.hasErrors()) {
-            messages.add(getRulesetMessages(odfPackage,
+            messages.add(getRulesetMessages(report.document.getPackage(),
                     this.rules.stream().filter(rule -> !rule.isPrerequisite()).collect(Collectors.toList())));
         }
-        return ProfileResultImpl.of(odfPackage.getName(), report, messages);
+        return ProfileResultImpl.of(report.document.getPackage().getName(), this.name, report, messages);
     }
 
     private final Map<String, List<Message>> getRulesetMessages(final OdfPackage odfPackage,
             final Collection<Rule> rules) throws ParseException {
         final MessageLog messages = Messages.messageLogInstance();
         for (final Rule rule : rules) {
             final MessageLog ruleMessages = rule.check(odfPackage);
-            if (rule instanceof ValidPackageRule) {
-                report = ((ValidPackageRule) rule).getValidationReport();
-            }
             messages.add(ruleMessages.getMessages());
         }
         return messages.getMessages();

odf-core/src/main/java/org/openpreservation/odf/validation/rules/ProfileResultImpl.java

@@ -6,37 +6,44 @@
 
 final class ProfileResultImpl implements ProfileResult {
     private final String id;
+    private final String name;
     private final ValidationReport validationReport;
-    private final MessageLog profileMessages;
+    private final MessageLog messageLog;
 
-    private ProfileResultImpl(final String id, final ValidationReport validationReport, final MessageLog profileMessages) {
+    private ProfileResultImpl(final String id, final String name, final ValidationReport validationReport, final MessageLog messageLog) {
         super();
         this.id = id;
+        this.name = name;
         this.validationReport = validationReport;
-        this.profileMessages = profileMessages;
+        this.messageLog = messageLog;
     }
 
     @Override
     public String getId() {
         return this.id;
     }
 
+    @Override
+    public String getName() {
+        return this.name;
+    }
+
     @Override
     public ValidationReport getValidationReport() {
         return this.validationReport;
     }
 
     @Override
-    public MessageLog getProfileMessages() {
-        return this.profileMessages;
+    public MessageLog getMessageLog() {
+        return this.messageLog;
     }
 
     @Override
     public boolean isValid() {
-        return !this.getProfileMessages().hasErrors();
+        return !this.getMessageLog().hasErrors();
     }
 
-    static final ProfileResultImpl of(final String id, final ValidationReport validationReport, final MessageLog profileMessages) {
-        return new ProfileResultImpl(id, validationReport, profileMessages);
+    static final ProfileResultImpl of(final String id, final String name, final ValidationReport validationReport, final MessageLog messageLog) {
+        return new ProfileResultImpl(id, name, validationReport, messageLog);
     }
 }

odf-core/src/main/java/org/openpreservation/odf/validation/rules/Rules.java

@@ -66,7 +66,7 @@ public static final Rule odf10() {
         return SubDocumentRule.getInstance(Severity.WARNING);
     }
 
-    public static final Profile getDnaProfile() {
+    public static final Profile getDnaProfile() throws ParserConfigurationException, SAXException {
         return ProfileImpl.of("DNA", "DNA ODF Spreadsheets Preservation Specification",
                 "Extended validation for OpenDocument spreadsheets.", DNA_RULES);
     }