From c05f8f056cb036b3c645fb00846bca7f54377fa8 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 21 Sep 2017 14:55:04 +0200 Subject: [PATCH] DATAMONGO-1778 - Fix equals() and hashCode() for Update. We now include the entire update document with its modifiers and the isolation flag when computing the hash code and comparing for object equality. Original pull request: #503. --- .../data/mongodb/core/query/Update.java | 128 +++++++++--------- .../data/mongodb/core/query/UpdateTests.java | 40 ++++++ 2 files changed, 103 insertions(+), 65 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Update.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Update.java index 600bf3c47..4bc450348 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Update.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Update.java @@ -15,8 +15,6 @@ */ package org.springframework.data.mongodb.core.query; -import static org.springframework.util.ObjectUtils.*; - import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -34,6 +32,7 @@ import org.springframework.data.domain.Sort.Direction; import org.springframework.data.domain.Sort.Order; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** @@ -465,7 +464,7 @@ public class Update { */ @Override public int hashCode() { - return getUpdateObject().hashCode(); + return Objects.hash(getUpdateObject(), isolated); } /* @@ -484,6 +483,10 @@ public class Update { } Update that = (Update) obj; + if (this.isolated != that.isolated) { + return false; + } + return Objects.equals(this.getUpdateObject(), that.getUpdateObject()); } @@ -539,7 +542,7 @@ public class Update { */ @Override public int hashCode() { - return nullSafeHashCode(modifiers); + return Objects.hashCode(modifiers); } /* @@ -558,7 +561,6 @@ public class Update { } Modifiers that = (Modifiers) obj; - return Objects.equals(this.modifiers, that.modifiers); } @@ -594,13 +596,63 @@ public class Update { } } + /** + * Abstract {@link Modifier} implementation with defaults for {@link Object#equals(Object)}, {@link Object#hashCode()} + * and {@link Object#toString()}. + * + * @author Christoph Strobl + * @since 2.0 + */ + private static abstract class AbstractModifier implements Modifier { + + /* + * (non-Javadoc) + * @see java.lang.Object#hashCode() + */ + @Override + public int hashCode() { + return ObjectUtils.nullSafeHashCode(getKey()) + ObjectUtils.nullSafeHashCode(getValue()); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#equals(java.lang.Object) + */ + @Override + public boolean equals(Object that) { + + if (this == that) { + return true; + } + + if (that == null || getClass() != that.getClass()) { + return false; + } + + if (!Objects.equals(getKey(), ((Modifier) that).getKey())) { + return false; + } + + return Objects.deepEquals(getValue(), ((Modifier) that).getValue()); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#toString() + */ + @Override + public String toString() { + return toJsonString(); + } + } + /** * Implementation of {@link Modifier} representing {@code $each}. * * @author Christoph Strobl * @author Thomas Darimont */ - private static class Each implements Modifier { + private static class Each extends AbstractModifier { private Object[] values; @@ -638,38 +690,6 @@ public class Update { public Object getValue() { return this.values; } - - /* - * (non-Javadoc) - * @see java.lang.Object#hashCode() - */ - @Override - public int hashCode() { - return nullSafeHashCode(values); - } - - /* - * (non-Javadoc) - * @see java.lang.Object#equals(java.lang.Object) - */ - @Override - public boolean equals(Object that) { - - if (this == that) { - return true; - } - - if (that == null || getClass() != that.getClass()) { - return false; - } - - return nullSafeEquals(values, ((Each) that).values); - } - - @Override - public String toString() { - return toJsonString(); - } } /** @@ -678,7 +698,7 @@ public class Update { * @author Christoph Strobl * @since 1.7 */ - private static class PositionModifier implements Modifier { + private static class PositionModifier extends AbstractModifier { private final int position; @@ -695,11 +715,6 @@ public class Update { public Object getValue() { return position; } - - @Override - public String toString() { - return toJsonString(); - } } /** @@ -708,7 +723,7 @@ public class Update { * @author Mark Paluch * @since 1.10 */ - private static class Slice implements Modifier { + private static class Slice extends AbstractModifier { private int count; @@ -733,11 +748,6 @@ public class Update { public Object getValue() { return this.count; } - - @Override - public String toString() { - return toJsonString(); - } } /** @@ -747,7 +757,7 @@ public class Update { * @author Mark Paluch * @since 1.10 */ - private static class SortModifier implements Modifier { + private static class SortModifier extends AbstractModifier { private final Object sort; @@ -799,11 +809,6 @@ public class Update { public Object getValue() { return this.sort; } - - @Override - public String toString() { - return toJsonString(); - } } /** @@ -940,14 +945,7 @@ public class Update { */ @Override public int hashCode() { - - int result = 17; - - result += 31 * result + getOuterType().hashCode(); - result += 31 * result + nullSafeHashCode(key); - result += 31 * result + nullSafeHashCode(modifiers); - - return result; + return Objects.hash(getOuterType(), key, modifiers); } /* @@ -971,7 +969,7 @@ public class Update { return false; } - return nullSafeEquals(this.key, that.key) && nullSafeEquals(this.modifiers, that.modifiers); + return Objects.equals(this.key, that.key) && Objects.equals(this.modifiers, that.modifiers); } private Update getOuterType() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/UpdateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/UpdateTests.java index b6838cc4c..83e953a8c 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/UpdateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/UpdateTests.java @@ -485,4 +485,44 @@ public class UpdateTests { assertThat(new Update().set("key", "value").isolated().toString(), is(equalTo("{ \"$set\" : { \"key\" : \"value\" }, \"$isolated\" : 1 }"))); } + + @Test // DATAMONGO-1778 + public void equalsShouldConsiderModifiers() { + + Update update1 = new Update().inc("version", 1).push("someField").slice(-10).each("test"); + Update update2 = new Update().inc("version", 1).push("someField").slice(-10).each("test"); + Update update3 = new Update().inc("version", 1).push("someField").slice(10).each("test"); + + assertThat(update1, is(equalTo(update2))); + assertThat(update1, is(not(equalTo(update3)))); + } + + @Test // DATAMONGO-1778 + public void equalsShouldConsiderIsolated() { + + Update update1 = new Update().inc("version", 1).isolated(); + Update update2 = new Update().inc("version", 1).isolated(); + + assertThat(update1, is(equalTo(update2))); + } + + @Test // DATAMONGO-1778 + public void hashCodeShouldConsiderModifiers() { + + Update update1 = new Update().inc("version", 1).push("someField").slice(-10).each("test"); + Update update2 = new Update().inc("version", 1).push("someField").slice(-10).each("test"); + Update update3 = new Update().inc("version", 1).push("someField").slice(10).each("test"); + + assertThat(update1.hashCode(), is(equalTo(update2.hashCode()))); + assertThat(update1.hashCode(), is(not(equalTo(update3.hashCode())))); + } + + @Test // DATAMONGO-1778 + public void hashCodeShouldConsiderIsolated() { + + Update update1 = new Update().inc("version", 1).isolated(); + Update update2 = new Update().inc("version", 1); + + assertThat(update1.hashCode(), is(not(equalTo(update2.hashCode())))); + } }