From c38d6e2ff4b662d8fcdd9a580e1739277f896c4f Mon Sep 17 00:00:00 2001 From: Carl Wilson Date: Mon, 13 Nov 2023 16:08:27 +0100 Subject: [PATCH] TEST: Message and valdiation classes - better and tested instantiation for `MessageLogImpl`; - replaced naive `List` matches with `Stream` implementations; - tests and fixes for `MessageImpl` and `ValidationReport` `hashCode` and `equals`; and - improvements to OdfDocument tests. --- .../messages/MessageImpl.java | 16 ++ .../messages/MessageLogImpl.java | 43 ++--- .../openpreservation/messages/Messages.java | 7 +- .../odf/document/OpenDocument.java | 6 + .../odf/validation/ValidationReport.java | 8 +- .../messages/MessageImplTest.java | 67 ++++++++ .../odf/document/DocumentsTest.java | 156 ++++++++++++++++++ .../odf/document/OdfDocumentTest.java | 25 +-- .../openpreservation/odf/document/Utils.java | 33 ++++ .../odf/validation/ValidationReportTest.java | 13 ++ 10 files changed, 331 insertions(+), 43 deletions(-) create mode 100644 odf-core/src/test/java/org/openpreservation/messages/MessageImplTest.java create mode 100644 odf-core/src/test/java/org/openpreservation/odf/document/DocumentsTest.java create mode 100644 odf-core/src/test/java/org/openpreservation/odf/document/Utils.java create mode 100644 odf-core/src/test/java/org/openpreservation/odf/validation/ValidationReportTest.java diff --git a/odf-core/src/main/java/org/openpreservation/messages/MessageImpl.java b/odf-core/src/main/java/org/openpreservation/messages/MessageImpl.java index 3d8c123b..5c71a853 100644 --- a/odf-core/src/main/java/org/openpreservation/messages/MessageImpl.java +++ b/odf-core/src/main/java/org/openpreservation/messages/MessageImpl.java @@ -97,10 +97,12 @@ public int hashCode() { final int prime = 31; int result = 1; result = prime * result + ((this.id == null) ? 0 : this.id.hashCode()); + result = prime * result + ((this.severity == null) ? 0 : this.severity.hashCode()); result = prime * result + ((this.message == null) ? 0 : this.message.hashCode()); result = prime * result + ((this.subMessage == null) ? 0 : this.subMessage.hashCode()); + result = prime * result + ((this.timestamp == null) ? 0 : this.timestamp.hashCode()); return result; } @@ -126,6 +128,13 @@ public boolean equals(final Object obj) { } else if (!this.id.equals(other.id)) { return false; } + if (this.severity == null) { + if (other.severity != null) { + return false; + } + } else if (!this.severity.equals(other.severity)) { + return false; + } if (this.message == null) { if (other.message != null) { return false; @@ -140,6 +149,13 @@ public boolean equals(final Object obj) { } else if (!this.subMessage.equals(other.subMessage)) { return false; } + if (this.timestamp == null) { + if (other.timestamp != null) { + return false; + } + } else if (!this.timestamp.equals(other.timestamp)) { + return false; + } return true; } } diff --git a/odf-core/src/main/java/org/openpreservation/messages/MessageLogImpl.java b/odf-core/src/main/java/org/openpreservation/messages/MessageLogImpl.java index 1ae5f253..38b58c37 100644 --- a/odf-core/src/main/java/org/openpreservation/messages/MessageLogImpl.java +++ b/odf-core/src/main/java/org/openpreservation/messages/MessageLogImpl.java @@ -4,10 +4,28 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.stream.Collectors; final class MessageLogImpl implements MessageLog { - private final List messages = new ArrayList<>(); + private final List messages; + private MessageLogImpl() { + this(new ArrayList<>()); + } + + private MessageLogImpl(final List messages) { + super(); + this.messages = new ArrayList<>(messages); + } + + static final MessageLog of() { + return new MessageLogImpl(); + } + + static final MessageLog of(final List messages) { + return new MessageLogImpl(messages); + } + @Override public int size() { return this.messages.size(); @@ -47,13 +65,7 @@ public List getInfos() { @Override public List getMessages(final Message.Severity severity) { - final List filteredList = new ArrayList<>(); - for (final Message message : this.messages) { - if (message.getSeverity() == severity) { - filteredList.add(message); - } - } - return Collections.unmodifiableList(filteredList); + return this.messages.stream().filter(m -> m.getSeverity() == severity).collect(Collectors.toUnmodifiableList()); } @Override @@ -63,13 +75,7 @@ public List getMessages() { @Override public List getMessages(final String id) { - final List filteredList = new ArrayList<>(); - for (final Message message : this.messages) { - if (message.getId().equals(id)) { - filteredList.add(message); - } - } - return Collections.unmodifiableList(filteredList); + return this.messages.stream().filter(m -> m.getId().equals(id)).collect(Collectors.toUnmodifiableList()); } @Override @@ -88,11 +94,6 @@ public boolean hasInfos() { } private boolean containsSeverity(final Message.Severity severity) { - for (final Message message : this.messages) { - if (message.getSeverity() == severity) { - return true; - } - } - return false; + return this.messages.stream().anyMatch((m -> m.getSeverity() == severity)); } } diff --git a/odf-core/src/main/java/org/openpreservation/messages/Messages.java b/odf-core/src/main/java/org/openpreservation/messages/Messages.java index c0969670..3479e993 100644 --- a/odf-core/src/main/java/org/openpreservation/messages/Messages.java +++ b/odf-core/src/main/java/org/openpreservation/messages/Messages.java @@ -1,5 +1,6 @@ package org.openpreservation.messages; +import java.util.List; import java.util.Locale; import java.util.ResourceBundle; @@ -147,6 +148,10 @@ public static MessageFactory getInstance(final String bundleName, } public static MessageLog messageLogInstance() { - return new MessageLogImpl(); + return MessageLogImpl.of(); + } + + public static MessageLog messageLogInstance(final List messages) { + return MessageLogImpl.of(messages); } } diff --git a/odf-core/src/main/java/org/openpreservation/odf/document/OpenDocument.java b/odf-core/src/main/java/org/openpreservation/odf/document/OpenDocument.java index 983b007f..af654c91 100644 --- a/odf-core/src/main/java/org/openpreservation/odf/document/OpenDocument.java +++ b/odf-core/src/main/java/org/openpreservation/odf/document/OpenDocument.java @@ -45,5 +45,11 @@ public interface OpenDocument { */ public OdfPackage getPackage(); + /** + * Get the format of the OpenDocument, this will be the declared format of the package + * or the parsed format of a single document. + * + * @return the format of the OpenDocument + */ public Formats getFormat(); } diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/ValidationReport.java b/odf-core/src/main/java/org/openpreservation/odf/validation/ValidationReport.java index 70091173..a5b85beb 100644 --- a/odf-core/src/main/java/org/openpreservation/odf/validation/ValidationReport.java +++ b/odf-core/src/main/java/org/openpreservation/odf/validation/ValidationReport.java @@ -11,7 +11,7 @@ import org.openpreservation.messages.Messages; import org.openpreservation.odf.document.OpenDocument; -public class ValidationReport { +public final class ValidationReport { final String name; public final OpenDocument document; public final Map documentMessages; @@ -139,6 +139,7 @@ public int hashCode() { int result = 1; result = prime * result + ((name == null) ? 0 : name.hashCode()); result = prime * result + ((documentMessages == null) ? 0 : documentMessages.hashCode()); + result = prime * result + ((document == null) ? 0 : document.hashCode()); return result; } @@ -161,6 +162,11 @@ public boolean equals(final Object obj) { return false; } else if (!documentMessages.equals(other.documentMessages)) return false; + if (document == null) { + if (other.document != null) + return false; + } else if (!document.equals(other.document)) + return false; return true; } } diff --git a/odf-core/src/test/java/org/openpreservation/messages/MessageImplTest.java b/odf-core/src/test/java/org/openpreservation/messages/MessageImplTest.java new file mode 100644 index 00000000..a6eb068b --- /dev/null +++ b/odf-core/src/test/java/org/openpreservation/messages/MessageImplTest.java @@ -0,0 +1,67 @@ +package org.openpreservation.messages; + +import org.junit.Test; + +import nl.jqno.equalsverifier.EqualsVerifier; + +public class MessageImplTest { + @Test + public void testEqualsContract() { + EqualsVerifier.forClass(MessageImpl.class).verify(); + } + + @Test + public void testGetId() { + + } + + @Test + public void testGetInstance() { + + } + + @Test + public void testGetMessage() { + + } + + @Test + public void testGetSeverity() { + + } + + @Test + public void testGetSubMessage() { + + } + + @Test + public void testGetTimestamp() { + + } + + @Test + public void testHasSubMessage() { + + } + + @Test + public void testIsError() { + + } + + @Test + public void testIsFatal() { + + } + + @Test + public void testIsInfo() { + + } + + @Test + public void testIsWarning() { + + } +} diff --git a/odf-core/src/test/java/org/openpreservation/odf/document/DocumentsTest.java b/odf-core/src/test/java/org/openpreservation/odf/document/DocumentsTest.java new file mode 100644 index 00000000..342cfedb --- /dev/null +++ b/odf-core/src/test/java/org/openpreservation/odf/document/DocumentsTest.java @@ -0,0 +1,156 @@ +package org.openpreservation.odf.document; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.net.URISyntaxException; + +import javax.xml.parsers.ParserConfigurationException; + +import org.junit.Test; +import org.openpreservation.format.xml.ParseResult; +import org.openpreservation.format.xml.XmlParser; +import org.openpreservation.odf.fmt.Formats; +import org.openpreservation.odf.fmt.TestFiles; +import org.openpreservation.odf.pkg.OdfPackage; +import org.openpreservation.odf.pkg.OdfPackages; +import org.openpreservation.odf.pkg.PackageParser; +import org.openpreservation.odf.xml.DocumentType; +import org.openpreservation.odf.xml.OdfXmlDocument; +import org.openpreservation.odf.xml.OdfXmlDocuments; +import org.xml.sax.SAXException; + +public class DocumentsTest { + private Utils utils = new Utils(); + + public DocumentsTest() throws IOException, ParserConfigurationException, SAXException { + } + + @Test + public void testOpenDocInstantiationOfNullDocument() { + OdfDocument nullDoc = null; + assertThrows("NullPointerException expected", + NullPointerException.class, + () -> { + Documents.openDocumentOf(nullDoc); + }); + } + + @Test + public void testOpenDocInstantiationOfDocument() throws IOException, ParserConfigurationException, SAXException { + OdfDocument doc = OdfDocumentImpl.from(TestFiles.EMPTY_FODS.openStream()); + OpenDocument openDoc = Documents.openDocumentOf(doc); + assertNotNull("Parsed OpenDocument should not be null.", openDoc); + assertFalse("Parsed document should be a package", openDoc.isPackage()); + assertEquals("Document should be a spreadsheet.", Formats.ODS, openDoc.getFormat()); + } + + @Test + public void testOpenDocInstantiationOfNullPackage() { + OdfPackage nullPkg = null; + assertThrows("NullPointerException expected", + NullPointerException.class, + () -> { + Documents.openDocumentOf(nullPkg); + }); + } + + @Test + public void testOpenDocInstantiationOfPackage() + throws IOException, ParserConfigurationException, SAXException, URISyntaxException { + PackageParser parser = OdfPackages.getPackageParser(); + OdfPackage pkg = parser.parsePackage(new File(TestFiles.EMPTY_ODS.toURI())); + OpenDocument openDoc = Documents.openDocumentOf(pkg); + assertNotNull("Parsed OpenDocument should not be null.", openDoc); + assertTrue("Parsed document should be a package", openDoc.isPackage()); + assertEquals("Document should be a spreadsheet.", Formats.ODS, openDoc.getFormat()); + } + + @Test + public void testOdfDocInstantiationOfNullXmlDocument() { + OdfXmlDocument nullDoc = null; + assertThrows("NullPointerException expected", + NullPointerException.class, + () -> { + Documents.odfDocumentOf(nullDoc, utils.metadata); + }); + } + + @Test + public void testOdfDocInstantiationOfNullXmlDocMetadata() + throws IOException, ParserConfigurationException, SAXException { + OdfXmlDocument xmlDoc = OdfXmlDocuments.xmlDocumentFrom(TestFiles.EMPTY_FODS.openStream()); + assertThrows("NullPointerException expected", + NullPointerException.class, + () -> { + Documents.odfDocumentOf(xmlDoc, null); + }); + } + + @Test + public void testOdfDocInstantiationOfXmlDocument() throws IOException, ParserConfigurationException, SAXException { + OdfXmlDocument xmlDoc = OdfXmlDocuments.xmlDocumentFrom(TestFiles.EMPTY_FODS.openStream()); + OdfDocument odfDoc = Documents.odfDocumentOf(xmlDoc, utils.metadata); + assertNotNull("Parsed OdfDocument should not be null.", odfDoc); + assertEquals("Parsed document should same as original", xmlDoc, odfDoc.getXmlDocument()); + assertEquals("Document should be a spreadsheet.", DocumentType.SPREADSHEET, odfDoc.getDocumentType()); + } + + @Test + public void testOdfDocInstantiationOfNullParseResultWithMetadata() { + ParseResult nullResult = null; + assertThrows("NullPointerException expected", + NullPointerException.class, + () -> { + Documents.odfDocumentOf(nullResult, utils.metadata); + }); + } + + @Test + public void testOdfDocInstantiationOfNullParseResultMetadata() + throws IOException, ParserConfigurationException, SAXException { + XmlParser parser = new XmlParser(); + ParseResult result = parser.parse(TestFiles.EMPTY_FODS.openStream()); + assertThrows("NullPointerException expected", + NullPointerException.class, + () -> { + Documents.odfDocumentOf(result, null); + }); + } + + @Test + public void testOdfDocInstantiationOfParseResultWithMetadata() throws IOException, ParserConfigurationException, SAXException { + XmlParser parser = new XmlParser(); + ParseResult result = parser.parse(TestFiles.EMPTY_FODS.openStream()); + OdfDocument odfDoc = Documents.odfDocumentOf(result, utils.metadata); + assertNotNull("Parsed OdfDocument should not be null.", odfDoc); + assertEquals("Parse result of document should same as original", result, odfDoc.getXmlDocument().getParseResult()); + assertEquals("Document should be a spreadsheet.", DocumentType.SPREADSHEET, odfDoc.getDocumentType()); + } + + @Test + public void testOdfDocInstantiationOfNullParseResult() + throws IOException, ParserConfigurationException, SAXException { + ParseResult nullResult = null; + assertThrows("NullPointerException expected", + NullPointerException.class, + () -> { + Documents.odfDocumentOf(nullResult); + }); + } + + @Test + public void testOdfDocInstantiationOfParseResult() throws IOException, ParserConfigurationException, SAXException { + XmlParser parser = new XmlParser(); + ParseResult result = parser.parse(TestFiles.EMPTY_FODS.openStream()); + OdfDocument odfDoc = Documents.odfDocumentOf(result); + assertNotNull("Parsed OdfDocument should not be null.", odfDoc); + assertEquals("Parse result of document should same as original", result, odfDoc.getXmlDocument().getParseResult()); + assertEquals("Document should be a spreadsheet.", DocumentType.SPREADSHEET, odfDoc.getDocumentType()); + } +} diff --git a/odf-core/src/test/java/org/openpreservation/odf/document/OdfDocumentTest.java b/odf-core/src/test/java/org/openpreservation/odf/document/OdfDocumentTest.java index 6430e29d..8c1d7b04 100644 --- a/odf-core/src/test/java/org/openpreservation/odf/document/OdfDocumentTest.java +++ b/odf-core/src/test/java/org/openpreservation/odf/document/OdfDocumentTest.java @@ -4,21 +4,14 @@ import static org.junit.Assert.assertThrows; import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; import javax.xml.parsers.ParserConfigurationException; import org.junit.Test; import org.openpreservation.format.xml.ParseResult; -import org.openpreservation.format.xml.ValidationResults; import org.openpreservation.format.xml.XmlParser; -import org.openpreservation.format.xml.XmlTestUtils; import org.openpreservation.odf.fmt.TestFiles; import org.openpreservation.odf.xml.Metadata; -import org.openpreservation.odf.xml.Metadata.UserDefinedField; import org.openpreservation.odf.xml.OdfXmlDocument; import org.openpreservation.odf.xml.OdfXmlDocuments; import org.xml.sax.SAXException; @@ -26,15 +19,7 @@ import nl.jqno.equalsverifier.EqualsVerifier; public class OdfDocumentTest { - private static ParseResult parseResult = ValidationResults.parseResultOf(true, XmlTestUtils.exampleNamespace, - new ArrayList<>(), - "prefix", "name", new ArrayList<>(), new ArrayList<>()); - Map stringValues = new HashMap<>(); - List userDefinedFields = new ArrayList<>(); - Metadata metadata = OdfXmlDocuments.metadataOf("", stringValues, userDefinedFields); - OdfXmlDocument odfDocument = OdfXmlDocuments - .xmlDocumentFrom(TestFiles.EMPTY_FODS.openStream()); - + private Utils utils = new Utils(); public OdfDocumentTest() throws IOException, ParserConfigurationException, SAXException { } @@ -45,22 +30,22 @@ public void testInstantiation() { assertThrows("NullPointerException expected", NullPointerException.class, () -> { - OdfDocumentImpl.of(nullDoc, metadata); + OdfDocumentImpl.of(nullDoc, utils.metadata); }); assertThrows("NullPointerException expected", NullPointerException.class, () -> { - OdfDocumentImpl.of(odfDocument, null); + OdfDocumentImpl.of(utils.odfDocument, null); }); assertThrows("NullPointerException expected", NullPointerException.class, () -> { - OdfDocumentImpl.of(nullResult, metadata); + OdfDocumentImpl.of(nullResult, utils.metadata); }); assertThrows("NullPointerException expected", NullPointerException.class, () -> { - OdfDocumentImpl.of(parseResult, null); + OdfDocumentImpl.of(utils.parseResult, null); }); } diff --git a/odf-core/src/test/java/org/openpreservation/odf/document/Utils.java b/odf-core/src/test/java/org/openpreservation/odf/document/Utils.java new file mode 100644 index 00000000..282a30ed --- /dev/null +++ b/odf-core/src/test/java/org/openpreservation/odf/document/Utils.java @@ -0,0 +1,33 @@ +package org.openpreservation.odf.document; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import javax.xml.parsers.ParserConfigurationException; + +import org.openpreservation.format.xml.ParseResult; +import org.openpreservation.format.xml.ValidationResults; +import org.openpreservation.format.xml.XmlTestUtils; +import org.openpreservation.odf.fmt.TestFiles; +import org.openpreservation.odf.xml.Metadata; +import org.openpreservation.odf.xml.Metadata.UserDefinedField; +import org.openpreservation.odf.xml.OdfXmlDocument; +import org.openpreservation.odf.xml.OdfXmlDocuments; +import org.xml.sax.SAXException; + +final class Utils { + ParseResult parseResult = ValidationResults.parseResultOf(true, XmlTestUtils.exampleNamespace, + new ArrayList<>(), + "prefix", "name", new ArrayList<>(), new ArrayList<>()); + Map stringValues = new HashMap<>(); + List userDefinedFields = new ArrayList<>(); + Metadata metadata = OdfXmlDocuments.metadataOf("", stringValues, userDefinedFields); + OdfXmlDocument odfDocument = OdfXmlDocuments + .xmlDocumentFrom(TestFiles.EMPTY_FODS.openStream()); + + public Utils() throws IOException, ParserConfigurationException, SAXException { + } +} diff --git a/odf-core/src/test/java/org/openpreservation/odf/validation/ValidationReportTest.java b/odf-core/src/test/java/org/openpreservation/odf/validation/ValidationReportTest.java new file mode 100644 index 00000000..070ecc5b --- /dev/null +++ b/odf-core/src/test/java/org/openpreservation/odf/validation/ValidationReportTest.java @@ -0,0 +1,13 @@ +package org.openpreservation.odf.validation; + +import org.junit.Test; + +import nl.jqno.equalsverifier.EqualsVerifier; + +public class ValidationReportTest { + + @Test + public void testEqualsContract() { + EqualsVerifier.forClass(ValidationReport.class).verify(); + } +}