From 2b9c8f768029a27d632bb3be068fcccdc4da65f8 Mon Sep 17 00:00:00 2001
From: Mars Lan <mars.th.lan@gmail.com>
Date: Tue, 12 May 2020 09:32:05 -0700
Subject: [PATCH] fix: use semantic instead of literal comparison in
 DefaultEqualityTester (#1665)

This fixes https://github.com/linkedin/datahub/issues/1663
Also change the signature of EqualityTester to compare only DataTemplates
---
 .../equality/AlwaysFalseEqualityTester.java   |  7 +++--
 .../dao/equality/DefaultEqualityTester.java   | 13 +++++----
 .../metadata/dao/equality/EqualityTester.java |  3 +-
 .../equality/DefaultEqualityTesterTest.java   | 29 +++++++++++++++++++
 4 files changed, 43 insertions(+), 9 deletions(-)
 create mode 100644 metadata-dao/src/test/java/com/linkedin/metadata/dao/equality/DefaultEqualityTesterTest.java

diff --git a/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/AlwaysFalseEqualityTester.java b/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/AlwaysFalseEqualityTester.java
index 9246757f83..225ee4c171 100644
--- a/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/AlwaysFalseEqualityTester.java
+++ b/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/AlwaysFalseEqualityTester.java
@@ -1,18 +1,19 @@
 package com.linkedin.metadata.dao.equality;
 
+import com.linkedin.data.template.DataTemplate;
 import javax.annotation.Nonnull;
 
 
 /**
  * A {@link EqualityTester} that always returns false.
  */
-public class AlwaysFalseEqualityTester<T> implements EqualityTester<T> {
+public class AlwaysFalseEqualityTester<T extends DataTemplate> implements EqualityTester<T> {
 
   /**
    * Creates a new instance of {@link AlwaysFalseEqualityTester}.
    */
-  public static <CLASS> AlwaysFalseEqualityTester<CLASS> newInstance() {
-    return new AlwaysFalseEqualityTester<CLASS>();
+  public static <CLASS extends DataTemplate> AlwaysFalseEqualityTester<CLASS> newInstance() {
+    return new AlwaysFalseEqualityTester<>();
   }
 
   @Override
diff --git a/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/DefaultEqualityTester.java b/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/DefaultEqualityTester.java
index 9261e2900a..83c0ec86ac 100644
--- a/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/DefaultEqualityTester.java
+++ b/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/DefaultEqualityTester.java
@@ -1,21 +1,24 @@
 package com.linkedin.metadata.dao.equality;
 
+import com.linkedin.data.template.DataTemplate;
+import com.linkedin.data.template.DataTemplateUtil;
 import javax.annotation.Nonnull;
 
 /**
- * A {@link EqualityTester} that relies on the default implementation of {@link Object#equals(Object)}.
+ * A {@link EqualityTester} that uses {@link DataTemplateUtil#areEqual(DataTemplate, DataTemplate)} to check for
+ * semantic equality.
  */
-public class DefaultEqualityTester<T> implements EqualityTester<T> {
+public class DefaultEqualityTester<T extends DataTemplate> implements EqualityTester<T> {
 
   /**
    * Creates a new instance of {@link DefaultEqualityTester}.
    */
-  public static <CLASS> DefaultEqualityTester<CLASS> newInstance() {
-    return new DefaultEqualityTester<CLASS>();
+  public static <CLASS extends DataTemplate> DefaultEqualityTester<CLASS> newInstance() {
+    return new DefaultEqualityTester<>();
   }
 
   @Override
   public boolean equals(@Nonnull T o1, @Nonnull T o2) {
-    return o1.equals(o2);
+    return DataTemplateUtil.areEqual(o1, o2);
   }
 }
diff --git a/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/EqualityTester.java b/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/EqualityTester.java
index 146ca3578d..f09e5f20bf 100644
--- a/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/EqualityTester.java
+++ b/metadata-dao/src/main/java/com/linkedin/metadata/dao/equality/EqualityTester.java
@@ -1,12 +1,13 @@
 package com.linkedin.metadata.dao.equality;
 
+import com.linkedin.data.template.DataTemplate;
 import javax.annotation.Nonnull;
 
 
 /**
  * An interface for testing equality between two objects of the same type.
  */
-public interface EqualityTester<T> {
+public interface EqualityTester<T extends DataTemplate> {
 
   boolean equals(@Nonnull T o1, @Nonnull T o2);
 }
diff --git a/metadata-dao/src/test/java/com/linkedin/metadata/dao/equality/DefaultEqualityTesterTest.java b/metadata-dao/src/test/java/com/linkedin/metadata/dao/equality/DefaultEqualityTesterTest.java
new file mode 100644
index 0000000000..ad4b29f182
--- /dev/null
+++ b/metadata-dao/src/test/java/com/linkedin/metadata/dao/equality/DefaultEqualityTesterTest.java
@@ -0,0 +1,29 @@
+package com.linkedin.metadata.dao.equality;
+
+import com.linkedin.metadata.dao.utils.RecordUtils;
+import com.linkedin.testing.AspectBaz;
+import java.io.IOException;
+import org.testng.annotations.Test;
+
+import static com.linkedin.metadata.utils.TestUtils.*;
+import static com.linkedin.testing.TestUtils.*;
+import static org.testng.Assert.*;
+
+
+public class DefaultEqualityTesterTest {
+
+  @Test
+  public void testSemanticEquality() throws IOException, CloneNotSupportedException {
+    // "longField" is backed by Integer in DataMap when deserialized from JSON
+    AspectBaz fromJson = RecordUtils.toRecordTemplate(AspectBaz.class, loadJsonFromResource("baz.json"));
+    assertEquals(fromJson.data().get("longField").getClass(), Integer.class);
+
+    // Clone and change the value in DataMap to Long
+    AspectBaz cloned = fromJson.clone();
+    cloned.setLongField(1234L);
+    assertEquals(cloned.data().get("longField").getClass(), Long.class);
+
+    // The two should still be semantically equal
+    assertTrue(new DefaultEqualityTester().equals(fromJson, cloned));
+  }
+}