Add merging for dirty annotation in checkboxes and fix AddIndexStrategy bug

DEVSIX-8806

Author: Eugene Bochilo

forms/src/main/java/com/itextpdf/forms/fields/PdfFormFieldMergeUtil.java

@@ -136,18 +136,15 @@ public static void processDirtyAnnotations(PdfFormField parentField, boolean thr
         for (PdfFormField field : parentField.getChildFormFields()) {
             PdfDictionary formDict = field.getPdfObject();
             // Process form fields without PdfName.Widget having only annotations as children
-            if (field.getChildFields().size() > 0 && field.getChildFormFields().size() == 0) {
+            if (!field.getChildFields().isEmpty() && field.getChildFormFields().isEmpty()) {
                 boolean shouldBeMerged = true;
 
-                // If parent is radio button or signature we don't care about field related keys, always merge
+                // If parent is radio button, checkbox or signature we don't care about field related keys, always merge
                 // If not - go over all fields to compare with parent's fields
-                if (!(PdfName.Btn.equals(parentField.getFormType()) &&
-                        parentField.getFieldFlag(PdfButtonFormField.FF_RADIO)) &&
-                        !PdfName.Sig.equals(parentField.getFormType())) {
-                    if (formDict.containsKey(PdfName.T)) {
-                        // We only want to perform the merge if field doesn't contain any name (even empty one)
-                        continue;
-                    }
+                boolean isRadioOrCheckbox = PdfName.Btn.equals(parentField.getFormType()) &&
+                        !parentField.getFieldFlag(PdfButtonFormField.FF_PUSH_BUTTON);
+                boolean isSignature = PdfName.Sig.equals(parentField.getFormType());
+                if (!isRadioOrCheckbox && !isSignature) {
                     for (final PdfName key : formDict.keySet()) {
                         // Everything except Parent and Kids must be identical to allow the merge
                         if (!PdfName.Parent.equals(key) && !PdfName.Kids.equals(key) &&
@@ -158,6 +155,13 @@ public static void processDirtyAnnotations(PdfFormField parentField, boolean thr
                     }
                 }
 
+                boolean isRadioButton = PdfName.Btn.equals(parentField.getFormType()) &&
+                        parentField.getFieldFlag(PdfButtonFormField.FF_RADIO);
+                if (formDict.containsKey(PdfName.T) && !isRadioButton && !isSignature) {
+                    // We only want to perform the merge if field doesn't contain any name (even empty one).
+                    // The only exceptions are radio buttons and signatures.
+                    continue;
+                }
                 if (shouldBeMerged) {
                     parentField.removeChild(field);
                     formDict.remove(PdfName.Parent);

forms/src/main/java/com/itextpdf/forms/fields/merging/AddIndexStrategy.java

@@ -68,20 +68,17 @@ public AddIndexStrategy() {
      * @param secondField           the second field
      * @param throwExceptionOnError if true, an exception will be thrown
      *
-     * @return true if the second field was renamed successfully, false otherwise
+     * @return returns {@code false} value, since {@link AddIndexStrategy} never merges fields.
      */
     @Override
     public boolean execute(PdfFormField firstField, PdfFormField secondField, boolean throwExceptionOnError) {
-        if (firstField == null || secondField == null) {
-            return false;
+        if (firstField != null && secondField != null &&
+                firstField.getFieldName() != null && secondField.getFieldName() != null) {
+            String originalFieldName = firstField.getFieldName().toUnicodeString();
+            String fieldToAddNewName = originalFieldName + separator + getNextIndex(originalFieldName);
+            secondField.setFieldName(fieldToAddNewName);
         }
-        if (firstField.getFieldName() == null || secondField.getFieldName() == null) {
-            return true;
-        }
-        String originalFieldName = firstField.getFieldName().toUnicodeString();
-        String fieldToAddNewName = originalFieldName + separator + getNextIndex(originalFieldName);
-        secondField.setFieldName(fieldToAddNewName);
-        return true;
+        return false;
     }
 
     int getNextIndex(String name) {

forms/src/main/java/com/itextpdf/forms/fields/merging/OnDuplicateFormFieldNameStrategy.java

@@ -37,7 +37,7 @@ public interface OnDuplicateFormFieldNameStrategy {
      * @param secondField           the second field
      * @param throwExceptionOnError if true, an exception will be thrown
      *
-     * @return true if the second field was renamed successfully, false otherwise
+     * @return true if the second field was merged successfully, false otherwise
      */
     boolean execute(PdfFormField firstField, PdfFormField secondField, boolean throwExceptionOnError);
 }

forms/src/test/java/com/itextpdf/forms/PdfFormFieldTest.java

@@ -1964,6 +1964,19 @@ public void pdfWithSignatureAndFontInBuilderFieldTest() throws IOException, Inte
                 "diff_"));
     }
 
+    @Test
+    public void dirtyCheckBoxAnnotationMergedTest() throws IOException, InterruptedException {
+        String outputFileName = destinationFolder + "dirtyCheckBoxAnnotationMergedTest.pdf";
+        String inputFileName = sourceFolder + "dirtyCheckBoxAnnotationMergedTest.pdf";
+        String cmpFileName = sourceFolder + "cmp_dirtyCheckBoxAnnotationMergedTest.pdf";
+        try (PdfDocument pdf = new PdfDocument(new PdfReader(inputFileName), new PdfWriter(outputFileName))) {
+            PdfFormCreator.getAcroForm(pdf, false);
+            // Do nothing.
+        }
+
+        Assertions.assertNull(new CompareTool().compareByContent(outputFileName, cmpFileName, destinationFolder, "diff_"));
+    }
+
     @Test
     @LogMessages(messages = @LogMessage(messageTemplate = FormsLogMessageConstants.FORM_FIELD_HAS_CYCLED_PARENT_STRUCTURE,
             ignore = true))

forms/src/test/java/com/itextpdf/forms/fields/merging/OnDuplicateFormFieldNameStrategyTest.java

@@ -260,6 +260,20 @@ public void flattenReadOnlyAddIndexTo() throws IOException, InterruptedException
                 SOURCE_FOLDER + "cmp_flattenReadOnlyAddIndexTo.pdf", DESTINATION_FOLDER, "diff_"));
     }
 
+    @Test
+    public void fieldsWithSameNamesTest() throws IOException {
+        String source = SOURCE_FOLDER + "fieldsWithSameNamesTest.pdf";
+        try (PdfDocument pdfDocument = new PdfDocument(new PdfReader(source), new PdfWriter(new ByteArrayOutputStream()))) {
+            PdfAcroForm form = PdfAcroForm.getAcroForm(pdfDocument, false, new AddIndexStrategy());
+
+            Assertions.assertNotNull(form.getField("test"));
+            Assertions.assertNotNull(form.getField("test_1"));
+            Assertions.assertNotNull(form.getField("test_2"));
+            Assertions.assertNotNull(form.getField("bingbong"));
+            Assertions.assertNotNull(form.getField("bingbong_1"));
+            Assertions.assertNotNull(form.getField("bingbong_2"));
+        }
+    }
 
     @Test
     public void addIndexStrategySeparatesTheFields() throws IOException, InterruptedException {