Improve CompareTool: compare whole catalogs instead of just selected predefined entries

Author: Snipx

kernel/src/main/java/com/itextpdf/kernel/pdf/PdfCatalog.java

@@ -363,6 +363,14 @@ PdfOutline getOutlines(boolean updateOutlines) {
         return outlines;
     }
 
+    /**
+     * Indicates if the catalog has any outlines
+     * @return {@code true}, if there are outlines and {@code false} otherwise.
+     */
+    boolean hasOutlines() {
+        return getPdfObject().containsKey(PdfName.Outlines);
+    }
+
     /**
      * This method removes all outlines associated with a given page
      *
@@ -373,10 +381,12 @@ void removeOutlines(PdfPage page) {
         if (getDocument().getWriter() == null) {
             return;
         }
-        getOutlines(false);
-        if (!pagesWithOutlines.isEmpty()) {
-            for (PdfOutline outline : pagesWithOutlines.get(page.getPdfObject())) {
-                outline.removeOutline();
+        if (hasOutlines()) {
+            getOutlines(false);
+            if (!pagesWithOutlines.isEmpty()) {
+                for (PdfOutline outline : pagesWithOutlines.get(page.getPdfObject())) {
+                    outline.removeOutline();
+                }
             }
         }
     }

kernel/src/main/java/com/itextpdf/kernel/pdf/PdfDocument.java

@@ -1161,11 +1161,11 @@ public String[] getPageLabels() {
     }
 
     /**
-     * indicates if the document has any outlines
-     * @return
+     * Indicates if the document has any outlines
+     * @return {@code true}, if there are outlines and {@code false} otherwise.
      */
     public boolean hasOutlines() {
-        return catalog.getPdfObject().containsKey(PdfName.Outlines);
+        return catalog.hasOutlines();
     }
 
     /**

kernel/src/main/java/com/itextpdf/kernel/utils/CompareTool.java

@@ -26,15 +26,6 @@
 import com.itextpdf.kernel.xmp.XMPUtils;
 import com.itextpdf.kernel.xmp.options.SerializeOptions;
 
-import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.parsers.ParserConfigurationException;
-import javax.xml.transform.OutputKeys;
-import javax.xml.transform.Transformer;
-import javax.xml.transform.TransformerException;
-import javax.xml.transform.TransformerFactory;
-import javax.xml.transform.dom.DOMSource;
-import javax.xml.transform.stream.StreamResult;
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.File;
@@ -49,12 +40,22 @@
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.Stack;
 import java.util.StringTokenizer;
 import java.util.TreeSet;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
 
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -285,7 +286,6 @@ public String compareLinkAnnotations(String outPdf, String cmpPdf) throws IOExce
         return message;
     }
 
-
     public String compareTagStructures(String outPdf, String cmpPdf) throws IOException, ParserConfigurationException, SAXException {
         System.out.print("[itext] INFO  Comparing tag structures......");
 
@@ -573,12 +573,11 @@ private String compareByContent(String outPath, String differenceImagePrefix, Ma
                 equalPages.add(i);
         }
 
-
-        compareCatalogEntry(outDocument, cmpDocument, compareResult, PdfName.StructTreeRoot);
-        compareCatalogEntry(outDocument, cmpDocument, compareResult, PdfName.OCProperties);
-        compareCatalogEntry(outDocument, cmpDocument, compareResult, PdfName.Names);
-        compareCatalogEntry(outDocument, cmpDocument, compareResult, PdfName.AcroForm);
-
+        ObjectPath catalogPath = new ObjectPath(cmpDocument.getCatalog().getPdfObject().getIndirectReference(),
+                outDocument.getCatalog().getPdfObject().getIndirectReference());
+        Set<PdfName> ignoredCatalogEntries = new LinkedHashSet<>(Arrays.asList(PdfName.Pages, PdfName.Metadata));
+        compareDictionariesExtended(outDocument.getCatalog().getPdfObject(), cmpDocument.getCatalog().getPdfObject(),
+                catalogPath, compareResult, ignoredCatalogEntries);
 
         outDocument.close();
         cmpDocument.close();
@@ -606,14 +605,6 @@ private String compareByContent(String outPath, String differenceImagePrefix, Ma
         }
     }
 
-    private void compareCatalogEntry(PdfDocument outDocument, PdfDocument cmpDocument, CompareResult compareResult, PdfName entryName) throws IOException {
-        PdfObject outEntry = outDocument.getCatalog().getPdfObject().get(entryName);
-        PdfObject cmpEntry = cmpDocument.getCatalog().getPdfObject().get(entryName);
-        PdfIndirectReference cmpStructTreeRef = cmpEntry == null ? null : cmpEntry.getIndirectReference();
-        PdfIndirectReference outStructTreeRef = outEntry == null ? null : outEntry.getIndirectReference();
-        compareObjects(outEntry, cmpEntry, new ObjectPath(cmpStructTreeRef, outStructTreeRef), compareResult);
-    }
-
     private void loadPagesFromReader(PdfDocument doc, List<PdfDictionary> pages, List<PdfIndirectReference> pagesRef) {
         int numOfPages = doc.getCatalog().getNumberOfPages();
         for (int i = 0; i < numOfPages; ++i) {
@@ -640,8 +631,11 @@ private boolean compareStreams(InputStream is1, InputStream is2) throws IOExcept
         return true;
     }
 
-
     private boolean compareDictionariesExtended(PdfDictionary outDict, PdfDictionary cmpDict, ObjectPath currentPath, CompareResult compareResult) throws IOException {
+        return compareDictionariesExtended(outDict, cmpDict, currentPath, compareResult, null);
+    }
+
+    private boolean compareDictionariesExtended(PdfDictionary outDict, PdfDictionary cmpDict, ObjectPath currentPath, CompareResult compareResult, Set<PdfName> excludedKeys) throws IOException {
         if (cmpDict != null && outDict == null || outDict != null && cmpDict == null) {
             compareResult.addError(currentPath, "One of the dictionaries is null, the other is not.");
             return false;
@@ -651,6 +645,9 @@ private boolean compareDictionariesExtended(PdfDictionary outDict, PdfDictionary
         Set<PdfName> mergedKeys = new TreeSet<>(cmpDict.keySet());
         mergedKeys.addAll(outDict.keySet());
         for (PdfName key : mergedKeys) {
+            if (excludedKeys != null && excludedKeys.contains(key)) {
+                continue;
+            }
             if (key.equals(PdfName.Parent) || key.equals(PdfName.P) || key.equals(PdfName.ModDate)) continue;
             if (outDict.isStream() && cmpDict.isStream() && (key.equals(PdfName.Filter) || key.equals(PdfName.Length))) continue;
             if (key.equals(PdfName.BaseFont) || key.equals(PdfName.FontName)) {

kernel/src/test/resources/com/itextpdf/kernel/utils/CompareToolTest/cmp_report02.xml

@@ -2,20 +2,20 @@
 <report>
 <errors count="2">
 <error>
-<message>PdfString. Characters differ at position 0. Expected: P (Page 1). Found: W (Wage 1).</message>
-<path>
-<base cmp="12 0 obj" out="12 0 obj"/>
-<dictKey>/T</dictKey>
-<offset>0</offset>
-</path>
-</error>
-<error>
 <message>PdfString. Characters differ at position 6. Expected: r (Courier Std). Found: w (Couriew Std).</message>
 <path>
 <base cmp="3 0 obj" out="3 0 obj"/>
 <dictKey>/FontFamily</dictKey>
 <offset>6</offset>
 </path>
 </error>
+<error>
+<message>PdfString. Characters differ at position 0. Expected: P (Page 1). Found: W (Wage 1).</message>
+<path>
+<base cmp="12 0 obj" out="12 0 obj"/>
+<dictKey>/T</dictKey>
+<offset>0</offset>
+</path>
+</error>
 </errors>
 </report>

layout/src/main/java/com/itextpdf/layout/Document.java

@@ -127,7 +127,6 @@ public void flush() {
      * Do not use when you have set {@link #immediateFlush} to <code>true</code>.
      */
     public void relayout() {
-
         if (immediateFlush) {
             throw new IllegalStateException("Operation not supported with immediate flush");
         }