DATAMONGO-423 - Fixed handling of negated regular expressions.

When using the not() method combined with the regex(…) methods on Criteria we created an invalid query so far. Fixed the regex(…) method to always transform the regex expressions and options into a Pattern instance and render that according to the $not state.

Author: odrotbohm

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Criteria.java

@@ -20,14 +20,15 @@
 import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.regex.Pattern;
 
+import org.bson.BSON;
 import org.bson.types.BasicBSONList;
 import org.springframework.data.mongodb.InvalidMongoDbApiUsageException;
 import org.springframework.data.mongodb.core.geo.Circle;
 import org.springframework.data.mongodb.core.geo.Point;
 import org.springframework.data.mongodb.core.geo.Shape;
 import org.springframework.util.Assert;
-import org.springframework.util.StringUtils;
 
 import com.mongodb.BasicDBObject;
 import com.mongodb.DBObject;
@@ -97,13 +98,17 @@ public Criteria is(Object o) {
 			throw new InvalidMongoDbApiUsageException(
 					"Multiple 'is' values declared. You need to use 'and' with multiple criteria");
 		}
-		if (this.criteria.size() > 0 && "$not".equals(this.criteria.keySet().toArray()[this.criteria.size() - 1])) {
+		if (lastOperatorWasNot()) {
 			throw new InvalidMongoDbApiUsageException("Invalid query: 'not' can't be used with 'is' - use 'ne' instead.");
 		}
 		this.isValue = o;
 		return this;
 	}
 
+	private boolean lastOperatorWasNot() {
+		return this.criteria.size() > 0 && "$not".equals(this.criteria.keySet().toArray()[this.criteria.size() - 1]);
+	}
+
 	/**
 	 * Creates a criterion using the $ne operator
 	 * 
@@ -269,7 +274,11 @@ public Criteria type(int t) {
 	 * @return
 	 */
 	public Criteria not() {
-		criteria.put("$not", null);
+		return not(null);
+	}
+
+	private Criteria not(Object value) {
+		criteria.put("$not", value);
 		return this;
 	}
 
@@ -280,8 +289,7 @@ public Criteria not() {
 	 * @return
 	 */
 	public Criteria regex(String re) {
-		criteria.put("$regex", re);
-		return this;
+		return regex(re, null);
 	}
 
 	/**
@@ -292,13 +300,32 @@ public Criteria regex(String re) {
 	 * @return
 	 */
 	public Criteria regex(String re, String options) {
-		criteria.put("$regex", re);
-		if (StringUtils.hasText(options)) {
-			criteria.put("$options", options);
+		return regex(toPattern(re, options));
+	}
+
+	/**
+	 * Syntactical sugar for {@link #is(Object)} making obvious that we create a regex predicate.
+	 * 
+	 * @param pattern
+	 * @return
+	 */
+	public Criteria regex(Pattern pattern) {
+
+		Assert.notNull(pattern);
+
+		if (lastOperatorWasNot()) {
+			return not(pattern);
 		}
+
+		this.isValue = pattern;
 		return this;
 	}
 
+	private Pattern toPattern(String regex, String options) {
+		Assert.notNull(regex);
+		return Pattern.compile(regex, options == null ? 0 : BSON.regexFlags(options));
+	}
+
 	/**
 	 * Creates a geospatial criterion using a $within $center operation. This is only available for Mongo 1.7 and higher.
 	 * 
@@ -426,16 +453,17 @@ protected DBObject getSingleCriteriaObject() {
 		DBObject dbo = new BasicDBObject();
 		boolean not = false;
 		for (String k : this.criteria.keySet()) {
+			Object value = this.criteria.get(k);
 			if (not) {
 				DBObject notDbo = new BasicDBObject();
-				notDbo.put(k, this.criteria.get(k));
+				notDbo.put(k, value);
 				dbo.put("$not", notDbo);
 				not = false;
 			} else {
-				if ("$not".equals(k)) {
+				if ("$not".equals(k) && value == null) {
 					not = true;
 				} else {
-					dbo.put(k, this.criteria.get(k));
+					dbo.put(k, value);
 				}
 			}
 		}

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java

@@ -46,9 +46,9 @@
 import org.springframework.data.mongodb.core.convert.CustomConversions;
 import org.springframework.data.mongodb.core.convert.MappingMongoConverter;
 import org.springframework.data.mongodb.core.index.Index;
+import org.springframework.data.mongodb.core.index.Index.Duplicates;
 import org.springframework.data.mongodb.core.index.IndexField;
 import org.springframework.data.mongodb.core.index.IndexInfo;
-import org.springframework.data.mongodb.core.index.Index.Duplicates;
 import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
 import org.springframework.data.mongodb.core.query.Criteria;
 import org.springframework.data.mongodb.core.query.Order;
@@ -131,6 +131,7 @@ protected void cleanDb() {
 		template.dropCollection(template.getCollectionName(PersonWithIdPropertyOfTypeLong.class));
 		template.dropCollection(template.getCollectionName(PersonWithIdPropertyOfPrimitiveLong.class));
 		template.dropCollection(template.getCollectionName(TestClass.class));
+		template.dropCollection(Sample.class);
 	}
 
 	@Test
@@ -1084,10 +1085,33 @@ public void removesEntityWithAnnotatedIdIfIdNeedsMassaging() {
 		assertThat(template.findOne(query(where("id").is(id)), Sample.class), is(nullValue()));
 	}
 
+	/**
+	 * @see DATAMONGO-423
+	 */
+	@Test
+	public void executesQueryWithNegatedRegexCorrectly() {
+
+		Sample first = new Sample();
+		first.field = "Matthews";
+
+		Sample second = new Sample();
+		second.field = "Beauford";
+
+		template.save(first);
+		template.save(second);
+
+		Query query = query(where("field").not().regex("Matthews"));
+		System.out.println(query.getQueryObject());
+		List<Sample> result = template.find(query, Sample.class);
+		assertThat(result.size(), is(1));
+		assertThat(result.get(0).field, is("Beauford"));
+	}
+
 	public static class Sample {
 
 		@Id
 		String id;
+		String field;
 	}
 
 	static class TestClass {

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java

@@ -100,7 +100,7 @@ public void testSimpleQueryWithChainedCriteria() {
 	public void testComplexQueryWithMultipleChainedCriteria() {
 		Query q = new Query(where("name").regex("^T.*").and("age").gt(20).lt(80).and("city")
 				.in("Stockholm", "London", "New York"));
-		String expected = "{ \"name\" : { \"$regex\" : \"^T.*\"} , \"age\" : { \"$gt\" : 20 , \"$lt\" : 80} , "
+		String expected = "{ \"name\" : { \"$regex\" : \"^T.*\" , \"$options\" : \"\"} , \"age\" : { \"$gt\" : 20 , \"$lt\" : 80} , "
 				+ "\"city\" : { \"$in\" : [ \"Stockholm\" , \"London\" , \"New York\"]}}";
 		Assert.assertEquals(expected, q.getQueryObject().toString());
 	}
@@ -134,7 +134,7 @@ public void testQueryWithIn() {
 	@Test
 	public void testQueryWithRegex() {
 		Query q = new Query(where("name").regex("b.*"));
-		String expected = "{ \"name\" : { \"$regex\" : \"b.*\"}}";
+		String expected = "{ \"name\" : { \"$regex\" : \"b.*\" , \"$options\" : \"\"}}";
 		Assert.assertEquals(expected, q.getQueryObject().toString());
 	}