fix: items were listed twice and/or with null values in the JSON report

rdeltour · rdeltour · commit b2c5d8ce33d0 · 2023-04-21T13:57:15.000+02:00
This was caused by a regression after the refactoring of the parsing of the container structure and package documents. This commit adds a new feature file to write tests for the JSON report. The first test scenarios check basic assertions: - JSON is well-formed - the items array has the correct number of items (no items duplication) - the checksum field Fix #1475, Fix #1490
diff --git a/src/main/java/com/adobe/epubcheck/ocf/OCFChecker.java b/src/main/java/com/adobe/epubcheck/ocf/OCFChecker.java
@@ -28,7 +28,6 @@
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 
 import org.w3c.epubcheck.constants.MIMEType;
@@ -319,7 +318,7 @@ private boolean checkContainerStructure(OCFCheckerState state)
           new OCFFilenameChecker(resource.getPath(), state.context().build()).check();
 
           // Report entry metadata
-          reportFeatures(resource.getProperties());
+          reportFeatures(resource);
 
           // Add the resource to the container model
           state.addResource(resource);
@@ -553,11 +552,12 @@ private EPUBVersion checkPublicationVersion(OCFCheckerState state)
     }
   }
 
-  private void reportFeatures(Map<FeatureEnum, String> features)
+  private void reportFeatures(OCFResource resource)
   {
-    for (FeatureEnum feature : features.keySet())
+    for (FeatureEnum feature : resource.getProperties().keySet())
     {
-      report.info(context.path, feature, features.get(feature));
+//      report.info(context.path, feature, resource.getProperties().get(feature));
+      report.info(resource.getPath(), feature, resource.getProperties().get(feature));
     }
   }
 
diff --git a/src/main/java/com/adobe/epubcheck/opf/OPFHandler.java b/src/main/java/com/adobe/epubcheck/opf/OPFHandler.java
@@ -287,8 +287,6 @@ else if (name.equals("item"))
 
             String mediaOverlay = e.getAttribute("media-overlay");
             itemBuilder.mediaOverlay(mediaOverlay);
-
-            report.info(href, FeatureEnum.UNIQUE_IDENT, id);
           }
         }
       }
@@ -641,6 +639,7 @@ private void buildItems()
    */
   protected void reportItem(OPFItem item)
   {
+    report.info(item.getPath(), FeatureEnum.UNIQUE_IDENT, item.getId());
     if (item.isInSpine())
     {
       report.info(item.getPath(), FeatureEnum.IS_SPINEITEM, "true");
diff --git a/src/test/resources/reporting/files/minimal/EPUB/content_001.xhtml b/src/test/resources/reporting/files/minimal/EPUB/content_001.xhtml
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html xmlns:epub="https://www.idpf.org/2007/ops" xmlns="https://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+	<head>
+		<meta charset="utf-8"/>
+		<title>Minimal EPUB</title>
+	</head>
+	<body>
+		<h1>Loomings</h1>
+		<p>Call me Ishmael.</p>
+	</body>
+</html>
diff --git a/src/test/resources/reporting/files/minimal/EPUB/nav.xhtml b/src/test/resources/reporting/files/minimal/EPUB/nav.xhtml
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html xmlns="https://www.w3.org/1999/xhtml" xmlns:epub="https://www.idpf.org/2007/ops" xml:lang="en" lang="en">
+	<head>
+		<meta charset="utf-8"/>
+		<title>Minimal Nav</title>
+	</head>
+	<body>
+		<nav epub:type="toc">
+			<ol>
+				<li><a href="content_001.xhtml">content 001</a></li>
+			</ol>
+		</nav>
+	</body>
+</html>
diff --git a/src/test/resources/reporting/files/minimal/EPUB/package.opf b/src/test/resources/reporting/files/minimal/EPUB/package.opf
@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<package xmlns="https://www.idpf.org/2007/opf" version="3.0" xml:lang="en" unique-identifier="q">
+<metadata xmlns:dc="https://purl.org/dc/elements/1.1/">
+  <dc:title id="title">Minimal EPUB 3.0</dc:title>
+  <dc:language>en</dc:language>
+  <dc:identifier id="q">NOID</dc:identifier>
+  <meta property="dcterms:modified">2017-06-14T00:00:01Z</meta>
+</metadata>
+<manifest>
+  <item id="content_001"  href="content_001.xhtml" media-type="application/xhtml+xml"/>
+  <item id="nav"  href="nav.xhtml" media-type="application/xhtml+xml" properties="nav"/>
+</manifest>
+<spine>
+  <itemref idref="content_001" />
+</spine>
+</package>
diff --git a/src/test/resources/reporting/files/minimal/META-INF/container.xml b/src/test/resources/reporting/files/minimal/META-INF/container.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<container version="1.0" xmlns="urn:oasis:names:tc:opendocument:xmlns:container">
+	<rootfiles>
+		<rootfile full-path="EPUB/package.opf" media-type="application/oebps-package+xml"/>
+	</rootfiles>
+</container>
diff --git a/src/test/resources/reporting/files/minimal/mimetype b/src/test/resources/reporting/files/minimal/mimetype
@@ -0,0 +1 @@
+application/epub+zip
diff --git a/src/test/resources/reporting/json-report.feature b/src/test/resources/reporting/json-report.feature
@@ -0,0 +1,15 @@
+ Feature: EPUBCheck - JSON Report tests
+  
+  Checks the JSON report format
+
+
+  Background: 
+    Given EPUB test files located at '/reporting/files/'
+    And the reporting format is set to JSON
+    And EPUBCheck with default settings
+
+  Scenario: Basic well-formedness checks
+    When checking EPUB 'minimal'
+    Then the JSON report is valid
+    And JSON at '$.items' contains 5 items
+    And JSON at '$..checkSum' has no null values 

Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,6 @@`
`28`	`28`	`import java.util.HashSet;`
`29`	`29`	`import java.util.LinkedList;`
`30`	`30`	`import java.util.List;`
`31`		`-import java.util.Map;`
`32`	`31`	`import java.util.Set;`
`33`	`32`
`34`	`33`	`import org.w3c.epubcheck.constants.MIMEType;`
`@@ -319,7 +318,7 @@ private boolean checkContainerStructure(OCFCheckerState state)`
`319`	`318`	`new OCFFilenameChecker(resource.getPath(), state.context().build()).check();`
`320`	`319`
`321`	`320`	`// Report entry metadata`
`322`		`- reportFeatures(resource.getProperties());`
	`321`	`+ reportFeatures(resource);`
`323`	`322`
`324`	`323`	`// Add the resource to the container model`
`325`	`324`	`state.addResource(resource);`
`@@ -553,11 +552,12 @@ private EPUBVersion checkPublicationVersion(OCFCheckerState state)`
`553`	`552`	`}`
`554`	`553`	`}`
`555`	`554`
`556`		`- private void reportFeatures(Map<FeatureEnum, String> features)`
	`555`	`+ private void reportFeatures(OCFResource resource)`
`557`	`556`	`{`
`558`		`- for (FeatureEnum feature : features.keySet())`
	`557`	`+ for (FeatureEnum feature : resource.getProperties().keySet())`
`559`	`558`	`{`
`560`		`- report.info(context.path, feature, features.get(feature));`
	`559`	`+// report.info(context.path, feature, resource.getProperties().get(feature));`
	`560`	`+ report.info(resource.getPath(), feature, resource.getProperties().get(feature));`
`561`	`561`	`}`
`562`	`562`	`}`
`563`	`563`
Original file line number	Diff line number	Diff line change
`@@ -287,8 +287,6 @@ else if (name.equals("item"))`
`287`	`287`
`288`	`288`	`String mediaOverlay = e.getAttribute("media-overlay");`
`289`	`289`	`itemBuilder.mediaOverlay(mediaOverlay);`
`290`		`-`
`291`		`- report.info(href, FeatureEnum.UNIQUE_IDENT, id);`
`292`	`290`	`}`
`293`	`291`	`}`
`294`	`292`	`}`
`@@ -641,6 +639,7 @@ private void buildItems()`
`641`	`639`	`*/`
`642`	`640`	`protected void reportItem(OPFItem item)`
`643`	`641`	`{`
	`642`	`+ report.info(item.getPath(), FeatureEnum.UNIQUE_IDENT, item.getId());`
`644`	`643`	`if (item.isInSpine())`
`645`	`644`	`{`
`646`	`645`	`report.info(item.getPath(), FeatureEnum.IS_SPINEITEM, "true");`