Improve memoryFirstWriter

DEVSIX-8582

Author: Eugene Bochilo

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

@@ -25,6 +25,7 @@ This file is part of the iText (R) project.
 import com.itextpdf.commons.actions.contexts.IMetaInfo;
 import com.itextpdf.commons.utils.FileUtil;
 import com.itextpdf.commons.utils.MessageFormatUtil;
+import com.itextpdf.commons.utils.SystemUtil;
 import com.itextpdf.io.font.PdfEncodings;
 import com.itextpdf.io.logs.IoLogMessageConstant;
 import com.itextpdf.io.util.GhostscriptHelper;
@@ -73,6 +74,7 @@ This file is part of the iText (R) project.
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
@@ -119,6 +121,7 @@ public class CompareTool {
     private static final String VERSION_REPLACEMENT = "<version>";
     private static final String COPYRIGHT_REGEXP = "\u00a9\\d+-\\d+ (?:iText Group NV|Apryse Group NV)";
     private static final String COPYRIGHT_REPLACEMENT = "\u00a9<copyright years> Apryse Group NV";
+    private static final boolean MEMORY_FIRST_WRITER_DISABLED;
 
     private static final String NEW_LINES = "\\r|\\n";
 
@@ -147,6 +150,11 @@ public class CompareTool {
     private String gsExec;
     private String compareExec;
 
+    static {
+        MEMORY_FIRST_WRITER_DISABLED = "true".equalsIgnoreCase(
+                SystemUtil.getPropertyOrEnvironmentVariable("DISABLE_MEMORY_FIRST_WRITER"));
+    }
+
     /**
      * Create new {@link CompareTool} instance.
      */
@@ -182,35 +190,39 @@ public static PdfWriter createTestPdfWriter(String filename) throws IOException
      *                               be created, or cannot be opened for any other reason.
      */
     public static PdfWriter createTestPdfWriter(String filename, WriterProperties properties) throws IOException {
-        return new MemoryFirstPdfWriter(filename, properties); // Android-Conversion-Replace return new PdfWriter(filename, properties);
+        if (MEMORY_FIRST_WRITER_DISABLED) {
+            return new PdfWriter(filename, properties);
+        } else {
+            return new MemoryFirstPdfWriter(filename, properties); // Android-Conversion-Replace return new PdfWriter(filename, properties);
+        }
     }
 
     /**
      * Create {@link PdfReader} out of the data created recently or read from disk.
      *
      * @param filename File to read the data from when necessary.
+     * @param properties {@link ReaderProperties} to use.
      * @return {@link PdfReader} to be used in tests.
      * @throws IOException on error
      */
-    public static PdfReader createOutputReader(String filename) throws IOException {
-        return CompareTool.createOutputReader(filename, new ReaderProperties());
+    public static PdfReader createOutputReader(String filename, ReaderProperties properties) throws IOException {
+        MemoryFirstPdfWriter outWriter = MemoryFirstPdfWriter.get(filename);
+        if (outWriter != null) {
+            return new PdfReader(new ByteArrayInputStream(outWriter.getBAOutputStream().toByteArray()), properties);
+        } else {
+            return new PdfReader(filename, properties);
+        }
     }
 
     /**
      * Create {@link PdfReader} out of the data created recently or read from disk.
      *
      * @param filename File to read the data from when necessary.
-     * @param properties {@link ReaderProperties} to use.
      * @return {@link PdfReader} to be used in tests.
      * @throws IOException on error
      */
-    public static PdfReader createOutputReader(String filename, ReaderProperties properties) throws IOException {
-        MemoryFirstPdfWriter outWriter = MemoryFirstPdfWriter.get(filename);
-        if (outWriter != null) {
-            return new PdfReader(new ByteArrayInputStream(outWriter.getBAOutputStream().toByteArray()), properties);
-        } else {
-            return new PdfReader(filename, properties);
-        }
+    public static PdfReader createOutputReader(String filename) throws IOException {
+        return CompareTool.createOutputReader(filename, new ReaderProperties());
     }
 
     /**
@@ -360,39 +372,39 @@ public CompareTool enableEncryptionCompare(boolean kdfSaltCompareEnabled) {
     }
 
     /**
-     * Gets {@link ReaderProperties} to be passed later to the {@link PdfReader} of the output document.
+     * Gets {@link ReaderProperties} to be passed later to the {@link PdfReader} of the cmp document.
      * <p>
      * Documents for comparison are opened in reader mode. This method is intended to alter {@link ReaderProperties}
-     * which are used to open the output document. This is particularly useful for comparison of encrypted documents.
+     * which are used to open the cmp document. This is particularly useful for comparison of encrypted documents.
      * <p>
      * For more explanations about what outDoc and cmpDoc are see last paragraph of the {@link CompareTool}
      * class description.
      *
-     * @return {@link ReaderProperties} instance to be passed later to the {@link PdfReader} of the output document.
+     * @return {@link ReaderProperties} instance to be passed later to the {@link PdfReader} of the cmp document.
      */
-    public ReaderProperties getOutReaderProperties() {
-        if (outProps == null) {
-            outProps = new ReaderProperties();
+    public ReaderProperties getCmpReaderProperties() {
+        if (cmpProps == null) {
+            cmpProps = new ReaderProperties();
         }
-        return outProps;
+        return cmpProps;
     }
 
     /**
-     * Gets {@link ReaderProperties} to be passed later to the {@link PdfReader} of the cmp document.
+     * Gets {@link ReaderProperties} to be passed later to the {@link PdfReader} of the output document.
      * <p>
      * Documents for comparison are opened in reader mode. This method is intended to alter {@link ReaderProperties}
-     * which are used to open the cmp document. This is particularly useful for comparison of encrypted documents.
+     * which are used to open the output document. This is particularly useful for comparison of encrypted documents.
      * <p>
      * For more explanations about what outDoc and cmpDoc are see last paragraph of the {@link CompareTool}
      * class description.
      *
-     * @return {@link ReaderProperties} instance to be passed later to the {@link PdfReader} of the cmp document.
+     * @return {@link ReaderProperties} instance to be passed later to the {@link PdfReader} of the output document.
      */
-    public ReaderProperties getCmpReaderProperties() {
-        if (cmpProps == null) {
-            cmpProps = new ReaderProperties();
+    public ReaderProperties getOutReaderProperties() {
+        if (outProps == null) {
+            outProps = new ReaderProperties();
         }
-        return cmpProps;
+        return outProps;
     }
 
     /**
@@ -1313,6 +1325,10 @@ private String compareByContent(String outPath, String differenceImagePrefix, Ma
                 CompareTool.writeOnDiskIfNotExists(cmpPdf);
                 return compareVisuallyAndCombineReports(compareResult.getReport(), outPath, differenceImagePrefix, ignoredAreas, equalPages);
             }
+        } catch (Exception e) {
+            CompareTool.writeOnDisk(outPdf);
+            CompareTool.writeOnDiskIfNotExists(cmpPdf);
+            throw e;
         }
     }
 

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

@@ -36,17 +36,17 @@ This file is part of the iText (R) project.
 
 /**
  * PdfWriter implementation which allows to create documents in memory and dump them on disk on purpose.
- * Currently it's private and used in automated tests only.
+ * Currently, it's private and used in automated tests only.
  */
 class MemoryFirstPdfWriter extends PdfWriter {
     private static final int MAX_ALLOWED_STREAMS = 100;
 
-    private static Map<String, MemoryFirstPdfWriter> waitingStreams = new ConcurrentHashMap<>();
+    private static final Map<String, MemoryFirstPdfWriter> waitingStreams = new ConcurrentHashMap<>();
 
-    private String filePath;
-    private ByteArrayOutputStream outStream;
+    private final String filePath;
+    private final ByteArrayOutputStream outStream;
 
-    MemoryFirstPdfWriter(String filename, WriterProperties properties) throws FileNotFoundException {
+    MemoryFirstPdfWriter(String filename, WriterProperties properties) {
         this(filename, MemoryFirstPdfWriter.createBAOutputStream(), properties);
     }
 
@@ -81,9 +81,9 @@ static void cleanup(String path) {
     }
 
     void dump() throws IOException {
-        OutputStream fos = FileUtil.getFileOutputStream(filePath);
-        outStream.writeTo(fos);
-        fos.close();
+        try (OutputStream fos = FileUtil.getFileOutputStream(filePath)) {
+            outStream.writeTo(fos);
+        }
     }
 
     ByteArrayOutputStream getBAOutputStream() {

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

@@ -244,15 +244,6 @@ protected PdfWriter getNextPdfWriter(PageRange documentPageRange) {
         return new PdfWriter(new ByteArrayOutputStream());
     }
 
-    private PdfDocument createPdfDocument(PageRange currentPageRange) {
-        PdfDocument newDocument = new PdfDocument(getNextPdfWriter(currentPageRange), new DocumentProperties().setEventCountingMetaInfo(metaInfo));
-        if (pdfDocument.isTagged() && preserveTagged)
-            newDocument.setTagged();
-        if (pdfDocument.hasOutlines() && preserveOutlines)
-            newDocument.initializeOutlines();
-        return newDocument;
-    }
-
     /**
      * The event listener which is called when another document is ready.
      */
@@ -290,8 +281,18 @@ public List<PdfDocument> splitByOutlines(List<String> outlineTitles) {
         return documentList;
     }
 
-    private PdfDocument splitByOutline(String outlineTitle) {
+    private PdfDocument createPdfDocument(PageRange currentPageRange) {
+        PdfDocument newDocument = new PdfDocument(getNextPdfWriter(currentPageRange), new DocumentProperties().setEventCountingMetaInfo(metaInfo));
+        if (pdfDocument.isTagged() && preserveTagged) {
+            newDocument.setTagged();
+        }
+        if (pdfDocument.hasOutlines() && preserveOutlines) {
+            newDocument.initializeOutlines();
+        }
+        return newDocument;
+    }
 
+    private PdfDocument splitByOutline(String outlineTitle) {
         int startPage = -1;
         int endPage = -1;
 
@@ -329,20 +330,33 @@ private PdfDocument splitByOutline(String outlineTitle) {
         return toDocument;
     }
 
-    private PdfPage getPageByOutline(int fromPage, PdfOutline outline) {
-        int size = pdfDocument.getNumberOfPages();
-        for (int i = fromPage; i <= size; i++) {
-            PdfPage pdfPage = pdfDocument.getPage(i);
-            List<PdfOutline> outlineList = pdfPage.getOutlines(false);
-            if (outlineList != null) {
-                for (PdfOutline pdfOutline : outlineList) {
-                    if (pdfOutline.equals(outline)) {
-                        return pdfPage;
-                    }
-                }
+    private PageRange getNextRange(int startPage, int endPage, long size) {
+        PdfResourceCounter counter = new PdfResourceCounter(pdfDocument.getTrailer());
+        Map<Integer, PdfObject> resources = counter.getResources();
+        // initialize with trailer length
+        long lengthWithoutXref = counter.getLength(null);
+        int currentPage = startPage;
+        boolean oversized = false;
+
+        do {
+            PdfPage page = pdfDocument.getPage(currentPage++);
+            counter = new PdfResourceCounter(page.getPdfObject());
+            lengthWithoutXref += counter.getLength(resources);
+            resources.putAll(counter.getResources());
+
+            if (lengthWithoutXref + xrefLength(resources.size()) > size) {
+                oversized = true;
             }
+        } while (currentPage <= endPage && !oversized);
+
+        // true if at least the first page to be copied didn't cause the oversize
+        if (oversized && (currentPage - 1) != startPage) {
+            // we shouldn't copy previous page because it caused
+            // the oversize and it isn't the first page to be copied
+            --currentPage;
         }
-        return null;
+
+        return new PageRange().addPageSequence(startPage, currentPage - 1);
     }
 
     /**
@@ -369,33 +383,20 @@ private PdfOutline getAbsoluteTreeNextOutline(PdfOutline outline) {
         return nextPdfOutline;
     }
 
-    private PageRange getNextRange(int startPage, int endPage, long size) {
-        PdfResourceCounter counter = new PdfResourceCounter(pdfDocument.getTrailer());
-        Map<Integer, PdfObject> resources = counter.getResources();
-        // initialize with trailer length
-        long lengthWithoutXref = counter.getLength(null);
-        int currentPage = startPage;
-        boolean oversized = false;
-
-        do {
-            PdfPage page = pdfDocument.getPage(currentPage++);
-            counter = new PdfResourceCounter(page.getPdfObject());
-            lengthWithoutXref += counter.getLength(resources);
-            resources.putAll(counter.getResources());
-
-            if (lengthWithoutXref + xrefLength(resources.size()) > size) {
-                oversized = true;
+    private PdfPage getPageByOutline(int fromPage, PdfOutline outline) {
+        int size = pdfDocument.getNumberOfPages();
+        for (int i = fromPage; i <= size; i++) {
+            PdfPage pdfPage = pdfDocument.getPage(i);
+            List<PdfOutline> outlineList = pdfPage.getOutlines(false);
+            if (outlineList != null) {
+                for (PdfOutline pdfOutline : outlineList) {
+                    if (pdfOutline.equals(outline)) {
+                        return pdfPage;
+                    }
+                }
             }
-        } while (currentPage <= endPage && !oversized);
-
-        // true if at least the first page to be copied didn't cause the oversize
-        if (oversized && (currentPage - 1) != startPage) {
-            // we shouldn't copy previous page because it caused
-            // the oversize and it isn't the first page to be copied
-            --currentPage;
         }
-
-        return new PageRange().addPageSequence(startPage, currentPage - 1);
+        return null;
     }
 
     private long xrefLength(int size) {

kernel/src/test/java/com/itextpdf/kernel/utils/CompareToolTest.java

@@ -334,6 +334,23 @@ public void memoryFirstWriterNoFileTest() throws InterruptedException, IOExcepti
         Assertions.assertFalse(new File(secondPdf).exists());
     }
 
+    @Test
+    public void memoryFirstWriterCmpMissingTest() throws IOException {
+        String firstPdf = destinationFolder + "memoryFirstWriterCmpMissingTest.pdf";
+        String secondPdf = destinationFolder + "cmp_memoryFirstWriterCmpMissingTest.pdf";
+        PdfDocument firstDocument = new PdfDocument(CompareTool.createTestPdfWriter(firstPdf));
+
+        PdfPage page1FirstDocument = firstDocument.addNewPage();
+        page1FirstDocument.addAnnotation(new PdfLinkAnnotation(new Rectangle(100, 560, 400, 50)).setDestination(
+                PdfExplicitDestination.createFit(page1FirstDocument)).setBorder(new PdfArray(new float[] {0, 0, 1})));
+        page1FirstDocument.flush();
+        firstDocument.close();
+
+        Assertions.assertThrows(IOException.class,
+                () -> new CompareTool().compareByContent(firstPdf, secondPdf, destinationFolder));
+        Assertions.assertTrue(new File(firstPdf).exists());
+    }
+
     @Test
     public void dumpMemoryFirstWriterOnDiskTest() throws InterruptedException, IOException {
         String firstPdf = destinationFolder + "dumpMemoryFirstWriterOnDiskTest.pdf";