StefanOltmann commented on code in PR #366:
URL: https://github.com/apache/commons-imaging/pull/366#discussion_r1537688118


##########
src/test/java/org/apache/commons/imaging/formats/jpeg/exif/ExifRewriterRoundtripTest.java:
##########
@@ -56,124 +62,160 @@ public static Stream<File> data() throws Exception {
 
     private File duplicateFile;
 
-    private void assertTiffEquals(final TiffOutputSet tiffOutputSet, final 
TiffOutputSet tiffOutputSet1) {
-        final List<TiffOutputDirectory> directories = 
tiffOutputSet.getDirectories();
-        final List<TiffOutputDirectory> directories1 = 
tiffOutputSet1.getDirectories();
-        assertEquals(directories.size(), directories1.size(), "The 
TiffOutputSets have different numbers of directories.");
-
-        for (int i = 0; i < directories.size(); i++) {
-            final List<TiffOutputField> fields = 
directories.get(i).getFields();
-            final List<TiffOutputField> fields1 = 
directories1.get(i).getFields();
-            assertEquals(fields.size(), fields1.size(), "The 
TiffOutputDirectories have different numbers of fields.");
-
-            for (int j = 0; j < fields.size(); j++) {
-                final TiffOutputField field = fields.get(j);
-                final TiffOutputField field1 = fields1.get(j);
-                assertEquals(field.tag, field1.tag, "TiffOutputField tag 
mismatch.");
-                assertEquals(field.tagInfo, field1.tagInfo, "TiffOutputField 
tagInfo mismatch.");
-                assertEquals(field.abstractFieldType, 
field1.abstractFieldType, "TiffOutputField fieldType mismatch.");
-                assertEquals(field.count, field1.count, "TiffOutputField count 
mismatch.");
+    private void assertTiffEquals(final TiffOutputSet sourceTiffOutputSet, 
final TiffOutputSet duplicateTiffOutputSet) {
+
+        final List<TiffOutputDirectory> sourceDirectories = 
sourceTiffOutputSet.getDirectories();
+        final List<TiffOutputDirectory> duplicatedDirectories = 
duplicateTiffOutputSet.getDirectories();
+
+        assertEquals(sourceDirectories.size(), duplicatedDirectories.size(), 
"The TiffOutputSets have different numbers of directories.");
+
+        for (int i = 0; i < sourceDirectories.size(); i++) {
+
+            final TiffOutputDirectory sourceDirectory = 
sourceDirectories.get(i);
+            final TiffOutputDirectory duplicateDirectory = 
duplicatedDirectories.get(i);
+
+            assertEquals(sourceDirectory.getType(), 
duplicateDirectory.getType(), "Directory type mismatch.");
+
+            final List<TiffOutputField> sourceFields = 
sourceDirectory.getFields();
+            final List<TiffOutputField> duplicateFields = 
duplicateDirectory.getFields();
+
+            final boolean fieldCountMatches = sourceFields.size() == 
duplicateFields.size();
+
+            if (!fieldCountMatches) {
+
+                /*
+                 * Note that offset fields are not written again if a re-order 
makes
+                 * them unnecessary. The TiffWriter does not write in the same 
order,
+                 * but tries to optimize it.
+                 */
+
+                final List<Integer> sourceTags = new ArrayList<>();
+                final List<Integer> duplicatedTags = new ArrayList<>();
+
+                for (TiffOutputField field : sourceFields)
+                    sourceTags.add(field.tag);
+
+                for (TiffOutputField field : duplicateFields)
+                    duplicatedTags.add(field.tag);
+
+                final List<Integer> missingTags = new ArrayList<>(sourceTags);
+                missingTags.removeAll(duplicatedTags);
+                missingTags.remove((Integer) 
ExifTagConstants.EXIF_TAG_EXIF_OFFSET.tag);
+                missingTags.remove((Integer) 
ExifTagConstants.EXIF_TAG_GPSINFO.tag);
+                missingTags.remove((Integer) 
ExifTagConstants.EXIF_TAG_INTEROP_OFFSET.tag);
+                missingTags.remove((Integer) 
TiffTagConstants.TIFF_TAG_JPEG_INTERCHANGE_FORMAT.tag);
+
+                assertTrue(missingTags.isEmpty(), "Missing tags: " + 
missingTags);
+            }
+
+            for (TiffOutputField sourceField : sourceFields) {
+
+                final boolean isOffsetField =
+                        sourceField.tag == 
ExifTagConstants.EXIF_TAG_EXIF_OFFSET.tag ||
+                                sourceField.tag == 
ExifTagConstants.EXIF_TAG_GPSINFO.tag ||
+                                sourceField.tag == 
ExifTagConstants.EXIF_TAG_INTEROP_OFFSET.tag ||
+                                sourceField.tag == 
TiffTagConstants.TIFF_TAG_JPEG_INTERCHANGE_FORMAT.tag;
+
+                /*
+                 * Ignore offset fields. They may not be needed after rewrite
+                 * and their value changes anyway.
+                 */
+                if (isOffsetField)
+                    continue;
+
+                TiffOutputField duplicateField = 
duplicateDirectory.findField(sourceField.tag);
+
+                assertNotNull(duplicateField, "Field is missing: " + 
sourceField.tagInfo);
+
+                assertEquals(sourceField.tag, duplicateField.tag, 
"TiffOutputField tag mismatch.");
+                assertEquals(sourceField.abstractFieldType, 
duplicateField.abstractFieldType, "TiffOutputField fieldType mismatch.");
+                assertEquals(sourceField.count, duplicateField.count, 
"TiffOutputField count mismatch.");
+
+                boolean sameValue = 
TiffOutputFieldTestUtil.bytesEqual(sourceField, duplicateField);
+
+                assertTrue(sameValue, "Bytes are different for field: " + 
sourceField);
             }
         }
     }
 
     private void copyToDuplicateFile(final File sourceFile, final 
TiffOutputSet duplicateTiffOutputSet) throws IOException, ImagingException, 
ImagingException {
+

Review Comment:
   It's a really small PR and this empty line makes the code easier to read.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to