Optimize array comparison (#783)

* gh-782 add unit test for big json * Fail fast when comparing arrays When comparing arrays, we want to find if the array items are equal, not so much how they differ. --------- Co-authored-by: Veli Döngelci <[email protected]>
lukas-krecan · Jun 26, 2024 · 1a65fae · 1a65fae
1 parent 2b7d0b7
commit 1a65fae
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 13 deletions.
diff --git a/json-unit-core/src/main/java/net/javacrumbs/jsonunit/core/internal/ComparisonMatrix.java b/json-unit-core/src/main/java/net/javacrumbs/jsonunit/core/internal/ComparisonMatrix.java
@@ -18,14 +18,14 @@
 import static java.lang.Math.min;
 import static java.util.Collections.unmodifiableList;
 import static java.util.stream.Collectors.toList;
-import static net.javacrumbs.jsonunit.core.Configuration.dummyDifferenceListener;
 import static net.javacrumbs.jsonunit.core.internal.Diff.DEFAULT_DIFFERENCE_STRING;
 import static net.javacrumbs.jsonunit.core.internal.JsonUnitLogger.NULL_LOGGER;
 
 import java.util.ArrayList;
 import java.util.BitSet;
 import java.util.List;
 import net.javacrumbs.jsonunit.core.Configuration;
+import net.javacrumbs.jsonunit.core.listener.DifferenceListener;
 
 /**
  * Stores comparison result when comparing two arrays.
@@ -81,15 +81,8 @@ private static List<List<Integer>> generateEqualElements(
 
             for (int j = 0; j < expectedElements.size(); j++) {
                 Node expected = expectedElements.get(j);
-                Diff diff = new Diff(
-                        expected,
-                        actual,
-                        Path.create("", path.toElement(i).getFullPath()),
-                        configuration.withDifferenceListener(dummyDifferenceListener()),
-                        NULL_LOGGER,
-                        NULL_LOGGER,
-                        DEFAULT_DIFFERENCE_STRING);
-                if (diff.similar()) {
+                boolean similar = isSimilar(path, configuration, expected, actual, i);
+                if (similar) {
                     actualIsEqualTo.add(j);
                 }
             }
@@ -100,6 +93,22 @@ private static List<List<Integer>> generateEqualElements(
         return equalElements;
     }
 
+    private static boolean isSimilar(Path path, Configuration configuration, Node expected, Node actual, int i) {
+        Diff diff = new Diff(
+                expected,
+                actual,
+                Path.create("", path.toElement(i).getFullPath()),
+                configuration.withDifferenceListener(failFastDifferenceListener),
+                NULL_LOGGER,
+                NULL_LOGGER,
+                DEFAULT_DIFFERENCE_STRING);
+        try {
+            return diff.similar();
+        } catch (DifferenceFoundException e) {
+            return false;
+        }
+    }
+
     ComparisonMatrix compare() {
         doSimpleMatching();
 
@@ -178,15 +187,15 @@ private List<Integer> getEqualToUsedOnlyInEquivalentElements(
     }
 
     private List<Integer> getEquivalentElements(List<Integer> equalTo) {
-        List<Integer> equivalentElments = new ArrayList<>();
+        List<Integer> equivalentElements = new ArrayList<>();
         for (int i = 0; i < equalElements.size(); i++) {
             if (!alreadyMatched.get(i)) {
                 if (equalTo.equals(equalElements.get(i))) {
-                    equivalentElments.add(i);
+                    equivalentElements.add(i);
                 }
             }
         }
-        return equivalentElments;
+        return equivalentElements;
     }
 
     private void addExtra(int index) {
@@ -240,4 +249,15 @@ List<Integer> getMissing() {
     List<Integer> getExtra() {
         return extra;
     }
+
+    private static final DifferenceListener failFastDifferenceListener = (difference, context) -> {
+        throw new DifferenceFoundException();
+    };
+
+    private static final class DifferenceFoundException extends RuntimeException {
+        @Override
+        public synchronized Throwable fillInStackTrace() {
+            return this;
+        }
+    }
 }
diff --git a/json-unit-core/src/test/java/net/javacrumbs/jsonunit/core/internal/DifferenceTest.java b/json-unit-core/src/test/java/net/javacrumbs/jsonunit/core/internal/DifferenceTest.java
@@ -17,16 +17,20 @@
 
 import static java.math.BigDecimal.valueOf;
 import static java.util.Collections.singletonMap;
+import static net.javacrumbs.jsonunit.core.util.ResourceUtils.resource;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.nullValue;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.io.IOException;
 import java.math.BigDecimal;
+import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.List;
 import net.javacrumbs.jsonunit.core.Configuration;
+import net.javacrumbs.jsonunit.core.NumberComparator;
 import net.javacrumbs.jsonunit.core.Option;
 import net.javacrumbs.jsonunit.core.ParametrizedMatcher;
 import net.javacrumbs.jsonunit.core.listener.Difference;
@@ -35,6 +39,7 @@
 import org.hamcrest.BaseMatcher;
 import org.hamcrest.Description;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
 
 public class DifferenceTest {
     private final RecordingDifferenceListener listener = new RecordingDifferenceListener();
@@ -238,6 +243,21 @@ void shouldMatchWithLineSeparatorCustomMatcher() {
         assertThat(listener.getDifferenceList(), hasSize(0));
     }
 
+    @Test
+    @Timeout(1)
+    void shouldRunDiffBeforeTimeout() throws URISyntaxException, IOException {
+        //noinspection DataFlowIssue
+        var actual = resource("big-json-with-common-keys-actual.json");
+        //noinspection DataFlowIssue
+        var expected = resource("big-json-with-common-keys-expected.json");
+        var cfg = commonConfig()
+                .withNumberComparator(new NormalisedNumberComparator())
+                .withOptions(
+                        Option.IGNORING_ARRAY_ORDER, Option.IGNORING_EXTRA_ARRAY_ITEMS, Option.IGNORING_EXTRA_FIELDS);
+        Diff diff = Diff.create(expected, actual, "", "", cfg);
+        diff.similar();
+    }
+
     private Configuration commonConfig() {
         return Configuration.empty().withDifferenceListener(listener);
     }
@@ -285,4 +305,19 @@ public void describeTo(Description description) {
             description.appendText("the same ").appendText(parameter);
         }
     }
+
+    private static class NormalisedNumberComparator implements NumberComparator {
+        @Override
+        public boolean compare(BigDecimal expectedValue, BigDecimal actualValue, BigDecimal tolerance) {
+            var normalisedExpectedValue = expectedValue.stripTrailingZeros();
+            var normalisedActualValue = actualValue.stripTrailingZeros();
+            if (tolerance != null) {
+                var diff =
+                        normalisedExpectedValue.subtract(normalisedActualValue).abs();
+                return diff.compareTo(tolerance) <= 0;
+            } else {
+                return normalisedExpectedValue.equals(normalisedActualValue);
+            }
+        }
+    }
 }
diff --git a/json-unit-core/src/test/resources/big-json-with-common-keys-actual.json b/json-unit-core/src/test/resources/big-json-with-common-keys-actual.json
diff --git a/json-unit-core/src/test/resources/big-json-with-common-keys-expected.json b/json-unit-core/src/test/resources/big-json-with-common-keys-expected.json