Repository: w3c/epubcheck
Pull request #1499 (2023-04-28 12:58:27)

Fix regressions in the JSON report


Patch:
diff --git a/pom.xml b/pom.xml
index 129298fd..f4aa923b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -160,6 +160,11 @@
             <version>1.1.1</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.jayway.jsonpath</groupId>
+            <artifactId>json-path-assert</artifactId>
+            <version>2.8.0</version>
+        </dependency>
         <dependency>
             <groupId>xmlunit</groupId>
             <artifactId>xmlunit</artifactId>
@@ -190,6 +195,11 @@
             <version>2.1</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-nop</artifactId>
+            <version>1.7.36</version>
+        </dependency>
     </dependencies>
 
     <build>
diff --git a/src/main/java/com/adobe/epubcheck/ocf/OCFChecker.java b/src/main/java/com/adobe/epubcheck/ocf/OCFChecker.java
index 6521d9b7..1e055d61 100755
--- 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.io.InputStream;
 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 @@ public final class OCFChecker extends AbstractChecker
           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 @@ public final class OCFChecker extends AbstractChecker
     }
   }
 
-  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
index ed6ff7f1..c709247f 100755
--- a/src/main/java/com/adobe/epubcheck/opf/OPFHandler.java
+++ b/src/main/java/com/adobe/epubcheck/opf/OPFHandler.java
@@ -287,8 +287,6 @@ public class OPFHandler extends XMLHandler
 
             String mediaOverlay = e.getAttribute("media-overlay");
             itemBuilder.mediaOverlay(mediaOverlay);
-
-            report.info(href, FeatureEnum.UNIQUE_IDENT, id);
           }
         }
       }
@@ -641,6 +639,7 @@ public class OPFHandler extends XMLHandler
    */
   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/main/java/com/adobe/epubcheck/reporting/CheckingReport.java b/src/main/java/com/adobe/epubcheck/reporting/CheckingReport.java
index 8fd74102..c51d61af 100644
--- a/src/main/java/com/adobe/epubcheck/reporting/CheckingReport.java
+++ b/src/main/java/com/adobe/epubcheck/reporting/CheckingReport.java
@@ -1,23 +1,25 @@
 package com.adobe.epubcheck.reporting;
 
+import java.io.File;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.codehaus.jackson.annotate.JsonProperty;
+
+import com.adobe.epubcheck.api.EPUBLocation;
 import com.adobe.epubcheck.api.EpubCheck;
 import com.adobe.epubcheck.api.MasterReport;
-import com.adobe.epubcheck.api.EPUBLocation;
 import com.adobe.epubcheck.messages.Message;
 import com.adobe.epubcheck.util.FeatureEnum;
 import com.adobe.epubcheck.util.JsonWriter;
 import com.adobe.epubcheck.util.PathUtil;
 import com.adobe.epubcheck.util.outWriter;
 
-import org.codehaus.jackson.annotate.JsonProperty;
-
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.OutputStream;
-import java.io.PrintStream;
-import java.util.*;
-
 public class CheckingReport extends MasterReport
 {
   @JsonProperty
@@ -34,19 +36,18 @@ public class CheckingReport extends MasterReport
   @JsonProperty
   final List<CheckMessage> messages = new ArrayList<CheckMessage>();
 
-  final String outputFile;
+  final PrintWriter out;
 
-  public CheckingReport(String epubPath, String outFile)
+  public CheckingReport(PrintWriter out, String epubName)
   {
     this.checker = new CheckerMetadata();
     this.publication = new PublicationMetadata();
-    this.outputFile = outFile;
-    this.setEpubFileName(epubPath);
+    this.out = (out != null) ? out : new PrintWriter(System.out);
+    this.setEpubFileName(PathUtil.removeWorkingDirectory(epubName));
   }
 
   void setParameters()
   {
-    outWriter.println("Epub Name: " + PathUtil.removeWorkingDirectory(this.getEpubFileName()));
     this.checker.setCheckerVersion(EpubCheck.version());
     File f = new File(this.getEpubFileName());
     this.checker.setFileInfo(f);
@@ -95,16 +96,13 @@ public class CheckingReport extends MasterReport
     this.setParameters();
     try
     {
-      this.getJsonReport(this.outputFile);
-    }
-    catch (IOException e)
+      this.getJsonReport();
+    } catch (IOException e)
     {
       outWriter.println("Incorrect path to save JsonFile.");
       return 1;
     }
 
-    long duration = this.getProcessDuration();
-    outWriter.println("Process duration: " + duration + "ms");
     return 0;
   }
 
@@ -113,22 +111,16 @@ public class CheckingReport extends MasterReport
     this.setStartDate();
   }
 
-  void getJsonReport(String path) throws
-      IOException
+  void getJsonReport()
+    throws IOException
   {
     sortCollections();
-    OutputStream out = null;
+
     try
     {
-      if (path == null) {
-          out = new PrintStream(System.out);
-      } else {
-          out = new FileOutputStream(path);
-      }
       JsonWriter jw = JsonWriter.createJsonWriter(true);
       jw.writeJson(this, out);
-    }
-    finally
+    } finally
     {
       if (out != null)
       {
@@ -147,6 +139,7 @@ public class CheckingReport extends MasterReport
     }
 
   }
+
   long getProcessDuration()
   {
     return this.checker.getProcessDuration();
diff --git a/src/main/java/com/adobe/epubcheck/tool/EpubChecker.java b/src/main/java/com/adobe/epubcheck/tool/EpubChecker.java
index c7761977..d92493ca 100644
--- a/src/main/java/com/adobe/epubcheck/tool/EpubChecker.java
+++ b/src/main/java/com/adobe/epubcheck/tool/EpubChecker.java
@@ -430,11 +430,7 @@ public class EpubChecker
     {
       report = new DefaultReportImpl("none");
     }
-    else if (jsonOutput)
-    {
-      report = new CheckingReport(path, (fileOut == null) ? null : fileOut.getPath());
-    }
-    else if (xmlOutput)
+    else if (jsonOutput | xmpOutput | xmlOutput)
     {
       PrintWriter pw = null;
       if (fileOut == null)
@@ -445,20 +441,13 @@ public class EpubChecker
       {
         pw = new PrintWriter(fileOut, "UTF-8");
       }
-      report = new XmlReportImpl(pw, path, EpubCheck.version());
-    }
-    else if (xmpOutput)
-    {
-      PrintWriter pw = null;
-      if (fileOut == null)
-      {
-        pw = new PrintWriter(System.out, true);
-      }
-      else
-      {
-        pw = new PrintWriter(fileOut, "UTF-8");
+      if (xmlOutput) {
+        report = new XmlReportImpl(pw, path, EpubCheck.version());
+      } else if (xmpOutput) {
+        report = new XmpReportImpl(pw, path, EpubCheck.version());
+      } else {
+        report = new CheckingReport(pw, path);
       }
-      report = new XmpReportImpl(pw, path, EpubCheck.version());
     }
     else
     {
diff --git a/src/main/java/com/adobe/epubcheck/util/Archive.java b/src/main/java/com/adobe/epubcheck/util/Archive.java
index 580c2605..b7e08388 100644
--- a/src/main/java/com/adobe/epubcheck/util/Archive.java
+++ b/src/main/java/com/adobe/epubcheck/util/Archive.java
@@ -89,12 +89,6 @@ public class Archive
         names.add(0, name);
         paths.add(0, path);
       }
-      else
-      {
-        // TODO replace system.err
-        System.err.println(
-            "No mimetype file found in expanded publication, output archive will be invalid");
-      }
 
       out = new ZipArchiveOutputStream(epubFile);
       out.setEncoding("UTF-8");
diff --git a/src/main/java/com/adobe/epubcheck/util/JsonWriter.java b/src/main/java/com/adobe/epubcheck/util/JsonWriter.java
index 0196ace6..b2564193 100644
--- a/src/main/java/com/adobe/epubcheck/util/JsonWriter.java
+++ b/src/main/java/com/adobe/epubcheck/util/JsonWriter.java
@@ -1,7 +1,7 @@
 package com.adobe.epubcheck.util;
 
 import java.io.IOException;
-import java.io.OutputStream;
+import java.io.PrintWriter;
 
 import org.codehaus.jackson.JsonFactory;
 import org.codehaus.jackson.JsonGenerator;
@@ -57,10 +57,10 @@ public class JsonWriter
     return new JsonWriter(om);
   }
 
-  public void writeJson(Object content, OutputStream os)
+  public void writeJson(Object content, PrintWriter pw)
       throws
       IOException
   {
-    this.objectMapper.writeValue(os, content);
+    this.objectMapper.writeValue(pw, content);
   }
 }


Test patch:
diff --git a/src/test/java/org/w3c/epubcheck/test/ExecutionSteps.java b/src/test/java/org/w3c/epubcheck/test/ExecutionSteps.java
index a8ebdfd4..b047de69 100644
--- a/src/test/java/org/w3c/epubcheck/test/ExecutionSteps.java
+++ b/src/test/java/org/w3c/epubcheck/test/ExecutionSteps.java
@@ -43,14 +43,24 @@ public class ExecutionSteps
     Locale oldDefaultLocale = Locale.getDefault();
     try
     {
+      // Complete configuration and get the test file 
       Locale.setDefault(configuration.getDefaultLocale());
-      File testFile = getEPUBFile(configuration.getBasepath() + path);
       if (configuration.getMode() == null)
       {
         configuration.setMode(CheckerMode.fromExtension(path));
       }
+      File testFile = getEPUBFile(configuration.getBasepath() + path);
+      
+      // Initialize the report
+      configuration.getReport().setEpubFileName(testFile.getAbsolutePath());
+      configuration.getReport().initialize();
+      
+      // Create the checker and run checks
       Checker checker = getChecker(testFile);
       checker.check();
+      
+      // Finalize the report
+      configuration.getReport().generate();
     } finally
     {
       Locale.setDefault(oldDefaultLocale);
diff --git a/src/test/java/org/w3c/epubcheck/test/JSONReportAssertionSteps.java b/src/test/java/org/w3c/epubcheck/test/JSONReportAssertionSteps.java
new file mode 100644
index 00000000..6064d48f
--- /dev/null
+++ b/src/test/java/org/w3c/epubcheck/test/JSONReportAssertionSteps.java
@@ -0,0 +1,54 @@
+package org.w3c.epubcheck.test;
+
+import static com.jayway.jsonpath.matchers.JsonPathMatchers.hasJsonPath;
+import static com.jayway.jsonpath.matchers.JsonPathMatchers.isJson;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.everyItem;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+
+import io.cucumber.java.en.Then;
+
+public class JSONReportAssertionSteps
+{
+
+  private TestReport report;
+
+  public JSONReportAssertionSteps(TestConfiguration configuration)
+  {
+    this.report = configuration.getReport();
+  }
+
+  @Then("the JSON report is valid")
+  public void jsonIsValid()
+  {
+    assertThat(report.getOutput(), isJson());
+  }
+
+  @Then("JSON at {string} contains {int} items")
+  public void jsonArrayHasSize(String path, int size)
+  {
+    assertThat(report.getOutput(), hasJsonPath(path, hasSize(size)));
+  }
+
+  @Then("JSON at {string} is {string}")
+  public void jsonValueIs(String path, String value)
+  {
+    assertThat(report.getOutput(), hasJsonPath(path, equalTo(value)));
+  }
+
+  @Then("JSON at {string} is not null")
+  public void jsonValueIsNotNull(String path)
+  {
+    assertThat(report.getOutput(), hasJsonPath(path, not(nullValue())));
+  }
+
+  @Then("JSON at {string} has no null values")
+  public void jsonValuesAreNotNull(String path)
+  {
+    assertThat(report.getOutput(), hasJsonPath(path, everyItem(notNullValue())));
+  }
+}
diff --git a/src/test/java/org/w3c/epubcheck/test/TestConfiguration.java b/src/test/java/org/w3c/epubcheck/test/TestConfiguration.java
index 55d834ee..9b13225d 100644
--- a/src/test/java/org/w3c/epubcheck/test/TestConfiguration.java
+++ b/src/test/java/org/w3c/epubcheck/test/TestConfiguration.java
@@ -144,6 +144,12 @@ public class TestConfiguration
   {
     this.profile = profile;
   }
+  
+  @And("(the )reporting format (is )set to {}")
+  public void setReportingFormat(String format)
+  {
+    report.setReportingFormat(format);
+  }
 
   @And("(the )reporting level (is )set to {severity}")
   public void setReportingLevel(Severity severity)
diff --git a/src/test/java/org/w3c/epubcheck/test/TestReport.java b/src/test/java/org/w3c/epubcheck/test/TestReport.java
index d32776e9..a2ff7ca8 100644
--- a/src/test/java/org/w3c/epubcheck/test/TestReport.java
+++ b/src/test/java/org/w3c/epubcheck/test/TestReport.java
@@ -1,5 +1,7 @@
 package org.w3c.epubcheck.test;
 
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.util.EnumSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -8,10 +10,15 @@ import java.util.Map;
 
 import com.adobe.epubcheck.api.EPUBLocation;
 import com.adobe.epubcheck.api.MasterReport;
+import com.adobe.epubcheck.api.QuietReport;
+import com.adobe.epubcheck.api.Report;
 import com.adobe.epubcheck.messages.Message;
 import com.adobe.epubcheck.messages.MessageId;
 import com.adobe.epubcheck.messages.Severity;
+import com.adobe.epubcheck.reporting.CheckingReport;
 import com.adobe.epubcheck.util.FeatureEnum;
+import com.adobe.epubcheck.util.XmlReportImpl;
+import com.adobe.epubcheck.util.XmpReportImpl;
 import com.google.common.base.Function;
 import com.google.common.base.Functions;
 import com.google.common.base.Predicate;
@@ -59,8 +66,16 @@ public class TestReport extends MasterReport
         }
       });
 
+  /* Whether to output messages on System.out */
   private boolean verbose = false;
+  /* Stores the messages to be queried by assertions */
   private List<MessageInfo> messages = new LinkedList<MessageInfo>();
+  /* The output format (can be JSON/XML/XMP/text */
+  private String format = "default";
+  /* A delegate report, used mostly for output formatting */
+  private Report delegate = null;
+  /* a writer storing the report output */
+  private final StringWriter output = new StringWriter();
 
   public TestReport()
   {
@@ -80,6 +95,11 @@ public class TestReport extends MasterReport
         fixMessage(message.getMessage(args)));
     if (verbose) System.out.println(messageInfo);
     messages.add(messageInfo);
+    // delegate to the formatting report
+    if (delegate != null)
+    {
+      delegate.message(message, location, args);
+    }
   }
 
   @Override
@@ -89,17 +109,45 @@ public class TestReport extends MasterReport
         fixMessage("[" + feature + "] " + value));
     if (verbose) System.out.println(messageInfo);
     messages.add(messageInfo);
+    // delegate to the formatting report
+    if (delegate != null)
+    {
+      delegate.info(resource, feature, value);
+    }
   }
 
   @Override
   public int generate()
   {
-    return 0;
+    int result = 0;
+    if (delegate != null)
+    {
+      result = delegate.generate();
+      if (verbose) System.out.println(output);
+    }
+    return result;
   }
 
   @Override
   public void initialize()
   {
+    assert format != null;
+    switch (format)
+    {
+    case "JSON":
+      delegate = new CheckingReport(new PrintWriter(output), getEpubFileName());
+      break;
+    case "XML":
+      delegate = new XmlReportImpl(new PrintWriter(output), getEpubFileName(), "test");
+      break;
+    case "XMP":
+      delegate = new XmpReportImpl(new PrintWriter(output), getEpubFileName(), "test");
+      break;
+    default:
+      delegate = QuietReport.INSTANCE;
+      break;
+    }
+    delegate.initialize();
   }
 
   public Iterable<MessageInfo> getAll(Severity severity)
@@ -144,11 +192,17 @@ public class TestReport extends MasterReport
     messages = partition.get(false);
     return partition.get(true);
   }
-  
-  public List<MessageInfo> getAllMessages() {
+
+  public List<MessageInfo> getAllMessages()
+  {
     return ImmutableList.copyOf(messages);
   }
 
+  public String getOutput()
+  {
+    return output.toString();
+  }
+
   private String fixMessage(String message)
   {
     if (message == null)
@@ -158,4 +212,12 @@ public class TestReport extends MasterReport
     return message.replaceAll("[\\s]+", " ");
   }
 
+  public void setReportingFormat(String format)
+  {
+    if (format != null)
+    {
+      this.format = format;
+    }
+  }
+
 }
diff --git a/src/test/resources/reporting/files/minimal/EPUB/content_001.xhtml b/src/test/resources/reporting/files/minimal/EPUB/content_001.xhtml
new file mode 100644
index 00000000..43a520ea
--- /dev/null
+++ b/src/test/resources/reporting/files/minimal/EPUB/content_001.xhtml
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html xmlns:epub="http://www.idpf.org/2007/ops" xmlns="http://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
new file mode 100644
index 00000000..240745e6
--- /dev/null
+++ b/src/test/resources/reporting/files/minimal/EPUB/nav.xhtml
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml" xmlns:epub="http://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
new file mode 100644
index 00000000..0d1eec6e
--- /dev/null
+++ b/src/test/resources/reporting/files/minimal/EPUB/package.opf
@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<package xmlns="http://www.idpf.org/2007/opf" version="3.0" xml:lang="en" unique-identifier="q">
+<metadata xmlns:dc="http://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>
\ No newline at end of file
diff --git a/src/test/resources/reporting/files/minimal/META-INF/container.xml b/src/test/resources/reporting/files/minimal/META-INF/container.xml
new file mode 100644
index 00000000..31878217
--- /dev/null
+++ 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
new file mode 100644
index 00000000..57ef03f2
--- /dev/null
+++ b/src/test/resources/reporting/files/minimal/mimetype
@@ -0,0 +1,1 @@
+application/epub+zip
\ No newline at end of file
diff --git a/src/test/resources/reporting/json-report.feature b/src/test/resources/reporting/json-report.feature
new file mode 100644
index 00000000..4a86b4e0
--- /dev/null
+++ 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