Skip to content

Commit

Permalink
Comment out equals() and hashcode() (#275)
Browse files Browse the repository at this point in the history 
<fix> : remove .equals() and .hashCode() overrides that go against encouraged Best Practices for Ebean

This may have confounding effects on Ebean's ability to .find() records, introducing duplicity into the job-gms DB.

---------

Co-authored-by: Jonathan Hui <jhui@jhui-ld2.linkedin.biz>
  • Loading branch information
@jphui
jphui and Jonathan Hui authored Jun 6, 2023
1 parent 6273d6e commit c29b606
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 38 deletions.
71 changes: 36 additions & 35 deletions dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanMetadataAspect.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
import lombok.NoArgsConstructor;
import lombok.NonNull;
import lombok.Setter;
import lombok.SneakyThrows;
import org.json.simple.JSONObject;
import org.json.simple.parser.JSONParser;
// import lombok.SneakyThrows;
// import org.json.simple.JSONObject;
// import org.json.simple.parser.JSONParser;


/**
Expand Down Expand Up @@ -88,38 +88,39 @@ public static class PrimaryKey {
@Column(name = CREATED_FOR_COLUMN, nullable = true)
private String createdFor;

@SneakyThrows
@Override
public boolean equals(Object o) {
if (o == null) {
return false;
}
if (o.getClass() != this.getClass()) {
return false;
}
EbeanMetadataAspect other = (EbeanMetadataAspect) o;

boolean primitiveEqualityCheck = this.key.equals(other.key)
// either both metadata fields are null or both are equal (will check non-null equality after)
&& ((this.metadata == null && other.metadata == null) || (this.metadata != null && other.metadata != null))
&& Math.abs(this.createdOn.getTime() - other.getCreatedOn().getTime()) < 1000 // timestamps are considered equal if within 1s of each other
&& this.createdBy.equals(other.getCreatedBy())
// either both createdFor fields are null or both are equal (need to check this.createdFor != null to avoid NPE)
&& ((this.createdFor == null && other.getCreatedFor() == null) || (this.createdFor != null && this.createdFor.equals(other.getCreatedFor())));
if (!primitiveEqualityCheck) {
return false;
}

JSONParser parser = new JSONParser();
JSONObject thisMetadata = (JSONObject) parser.parse(this.metadata);
JSONObject otherMetadata = (JSONObject) parser.parse(other.metadata);
return thisMetadata.equals(otherMetadata);
}

@Override
public int hashCode() {
return super.hashCode();
}
// TODO (@jphui) META-18962 De-deduplicity investigation
// @SneakyThrows
// @Override
// public boolean equals(Object o) {
// if (o == null) {
// return false;
// }
// if (o.getClass() != this.getClass()) {
// return false;
// }
// EbeanMetadataAspect other = (EbeanMetadataAspect) o;

// boolean primitiveEqualityCheck = this.key.equals(other.key)
// // either both metadata fields are null or both are equal (will check non-null equality after)
// && ((this.metadata == null && other.metadata == null) || (this.metadata != null && other.metadata != null))
// && Math.abs(this.createdOn.getTime() - other.getCreatedOn().getTime()) < 1000 // timestamps are considered equal if within 1s of each other
// && this.createdBy.equals(other.getCreatedBy())
// // either both createdFor fields are null or both are equal (need to check this.createdFor != null to avoid NPE)
// && ((this.createdFor == null && other.getCreatedFor() == null) || (this.createdFor != null && this.createdFor.equals(other.getCreatedFor())));
// if (!primitiveEqualityCheck) {
// return false;
// }

// JSONParser parser = new JSONParser();
// JSONObject thisMetadata = (JSONObject) parser.parse(this.metadata);
// JSONObject otherMetadata = (JSONObject) parser.parse(other.metadata);
// return thisMetadata.equals(otherMetadata);
// }

// @Override
// public int hashCode() {
// return super.hashCode();
// }

@Override
public String toString() {
Expand Down
12 changes: 9 additions & 3 deletions dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ public void testCompareResultsListEbeanMetadataAspectSingleton() {
ema4.setCreatedFor("tester");
ema4.setCreatedOn(new Timestamp(1234567890L));

assertFalse(EBeanDAOUtils.compareResults(Collections.singletonList(ema1), Collections.singletonList(ema4), "testMethod"));
// TODO (@jphui) META-18962 De-deduplicity investigation
// This test case is only used for new schema migration dual-mode validation
// assertFalse(EBeanDAOUtils.compareResults(Collections.singletonList(ema1), Collections.singletonList(ema4), "testMethod"));

// same metadata, different order
EbeanMetadataAspect ema4Different = new EbeanMetadataAspect();
Expand All @@ -106,13 +108,17 @@ public void testCompareResultsListEbeanMetadataAspectSingleton() {

assertTrue(EBeanDAOUtils.compareResults(Collections.singletonList(ema1), Collections.singletonList(ema5), "testMethod"));

// TODO (@jphui) META-18962 De-deduplicity investigation
// This test case is only used for new schema migration dual-mode validation
// different createdon
ema5.setCreatedOn(new Timestamp(1234560000L)); // 7890 ms different than Timestamp(1234567890L)
assertFalse(EBeanDAOUtils.compareResults(Collections.singletonList(ema1), Collections.singletonList(ema5), "testMethod"));
// assertFalse(EBeanDAOUtils.compareResults(Collections.singletonList(ema1), Collections.singletonList(ema5), "testMethod"));

// TODO (@jphui) META-18962 De-deduplicity investigation
// This test case is only used for new schema migration dual-mode validation
// createdFor is nullable, set one EbeanMetadataAspect's createdFor field to null
ema1.setCreatedFor(null);
assertFalse(EBeanDAOUtils.compareResults(Collections.singletonList(ema1), Collections.singletonList(ema2), "testMethod"));
// assertFalse(EBeanDAOUtils.compareResults(Collections.singletonList(ema1), Collections.singletonList(ema2), "testMethod"));

// both createdFor fields being null should return true
ema2.setCreatedFor(null);
Expand Down

0 comments on commit c29b606

Please sign in to comment.