Polishing.

Fix nullability annotations for isEqual(…) parameters. Fix generics. Reformat code. Add tests. See #3414 Original pull request: #3615.
spring-projects · Apr 13, 2021 · e403019 · e403019
1 parent 2885c35
commit e403019
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 13 deletions.
diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Criteria.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Criteria.java
@@ -973,7 +973,7 @@ private boolean simpleCriteriaEquals(Criteria left, Criteria right) {
 	 * @param right
 	 * @return
 	 */
-	private boolean isEqual(Object left, Object right) {
+	private boolean isEqual(@Nullable Object left, @Nullable Object right) {
 
 		if (left == null) {
 			return right == null;
@@ -993,42 +993,48 @@ private boolean isEqual(Object left, Object right) {
 		}
 
 		if (left instanceof Document) {
+
 			if (!(right instanceof Document)) {
 				return false;
 			}
+
 			Document leftDocument = (Document) left;
 			Document rightDocument = (Document) right;
-			Iterator leftIterator = leftDocument.entrySet().iterator();
-			Iterator rightIterator = rightDocument.entrySet().iterator();
+			Iterator<Entry<String, Object>> leftIterator = leftDocument.entrySet().iterator();
+			Iterator<Entry<String, Object>> rightIterator = rightDocument.entrySet().iterator();
 
 			while (leftIterator.hasNext() && rightIterator.hasNext()) {
-				Map.Entry leftEntry = (Map.Entry)leftIterator.next();
-				Map.Entry rightEntry = (Map.Entry)rightIterator.next();
-				if (!isEqual(leftEntry.getKey(), rightEntry.getKey())) {
-					return false;
-				}
-				if (!isEqual(leftEntry.getValue(), rightEntry.getValue())) {
+
+				Map.Entry<String, Object> leftEntry = leftIterator.next();
+				Map.Entry<String, Object> rightEntry = rightIterator.next();
+
+				if (!isEqual(leftEntry.getKey(), rightEntry.getKey())
+						|| !isEqual(leftEntry.getValue(), rightEntry.getValue())) {
 					return false;
 				}
 			}
+
 			return !leftIterator.hasNext() && !rightIterator.hasNext();
 		}
 
 		if (Collection.class.isAssignableFrom(left.getClass())) {
+
 			if (!Collection.class.isAssignableFrom(right.getClass())) {
 				return false;
 			}
 
-			Collection leftCollection = (Collection) left;
-			Collection rightCollection = (Collection) right;
-			Iterator leftIterator = leftCollection.iterator();
-			Iterator rightIterator = rightCollection.iterator();
+			Collection<?> leftCollection = (Collection<?>) left;
+			Collection<?> rightCollection = (Collection<?>) right;
+			Iterator<?> leftIterator = leftCollection.iterator();
+			Iterator<?> rightIterator = rightCollection.iterator();
 
 			while (leftIterator.hasNext() && rightIterator.hasNext()) {
+
 				if (!isEqual(leftIterator.next(), rightIterator.next())) {
 					return false;
 				}
 			}
+
 			return !leftIterator.hasNext() && !rightIterator.hasNext();
 		}
 

diff --git a/...-mongodb/src/test/java/org/springframework/data/mongodb/core/query/CriteriaUnitTests.java b/...-mongodb/src/test/java/org/springframework/data/mongodb/core/query/CriteriaUnitTests.java
@@ -39,6 +39,7 @@
  * @author Andreas Zink
  * @author Ziemowit Stolarczyk
  * @author Clément Petit
+ * @author Mark Paluch
  */
 public class CriteriaUnitTests {
 
@@ -361,6 +362,45 @@ public void shouldEqualForSamePattern() {
 	}
 
 	@Test // DATAMONGO-2002
+	public void shouldEqualForDocument() {
+
+		assertThat(new Criteria("field").is(new Document("one", 1).append("two", "two").append("null", null)))
+				.isEqualTo(new Criteria("field").is(new Document("one", 1).append("two", "two").append("null", null)));
+
+		assertThat(new Criteria("field").is(new Document("one", 1).append("two", "two").append("null", null)))
+				.isNotEqualTo(new Criteria("field").is(new Document("one", 1).append("two", "two")));
+
+		assertThat(new Criteria("field").is(new Document("one", 1).append("two", "two")))
+				.isNotEqualTo(new Criteria("field").is(new Document("one", 1).append("two", "two").append("null", null)));
+
+		assertThat(new Criteria("field").is(new Document("one", 1).append("null", null).append("two", "two")))
+				.isNotEqualTo(new Criteria("field").is(new Document("one", 1).append("two", "two").append("null", null)));
+
+		assertThat(new Criteria("field").is(new Document())).isNotEqualTo(new Criteria("field").is("foo"));
+		assertThat(new Criteria("field").is("foo")).isNotEqualTo(new Criteria("field").is(new Document()));
+	}
+
+	@Test // DATAMONGO-2002
+	public void shouldEqualForCollection() {
+
+		assertThat(new Criteria("field").is(Arrays.asList("foo", "bar")))
+				.isEqualTo(new Criteria("field").is(Arrays.asList("foo", "bar")));
+
+		assertThat(new Criteria("field").is(Arrays.asList("foo", 1)))
+				.isNotEqualTo(new Criteria("field").is(Arrays.asList("foo", "bar")));
+
+		assertThat(new Criteria("field").is(Collections.singletonList("foo")))
+				.isNotEqualTo(new Criteria("field").is(Arrays.asList("foo", "bar")));
+
+		assertThat(new Criteria("field").is(Arrays.asList("foo", "bar")))
+				.isNotEqualTo(new Criteria("field").is(Collections.singletonList("foo")));
+
+		assertThat(new Criteria("field").is(Arrays.asList("foo", "bar"))).isNotEqualTo(new Criteria("field").is("foo"));
+
+		assertThat(new Criteria("field").is("foo")).isNotEqualTo(new Criteria("field").is(Arrays.asList("foo", "bar")));
+	}
+
+	@Test // GH-3414
 	public void shouldEqualForSamePatternAndFlags() {
 
 		Criteria left = new Criteria("field").regex("foo", "iu");