Skip to content

Commit

Permalink
DATAMONGO-423 - Fixed handling of negated regular expressions.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
odrotbohm committed Apr 16, 2012
1 parent 3be35cb commit ba5a764
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 13 deletions.
Expand Up @@ -20,14 +20,15 @@
import java.util.Collection; import java.util.Collection;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.regex.Pattern;


import org.bson.BSON;
import org.bson.types.BasicBSONList; import org.bson.types.BasicBSONList;
import org.springframework.data.mongodb.InvalidMongoDbApiUsageException; import org.springframework.data.mongodb.InvalidMongoDbApiUsageException;
import org.springframework.data.mongodb.core.geo.Circle; import org.springframework.data.mongodb.core.geo.Circle;
import org.springframework.data.mongodb.core.geo.Point; import org.springframework.data.mongodb.core.geo.Point;
import org.springframework.data.mongodb.core.geo.Shape; import org.springframework.data.mongodb.core.geo.Shape;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import org.springframework.util.StringUtils;


import com.mongodb.BasicDBObject; import com.mongodb.BasicDBObject;
import com.mongodb.DBObject; import com.mongodb.DBObject;
Expand Down Expand Up @@ -97,13 +98,17 @@ public Criteria is(Object o) {
throw new InvalidMongoDbApiUsageException( throw new InvalidMongoDbApiUsageException(
"Multiple 'is' values declared. You need to use 'and' with multiple criteria"); "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."); throw new InvalidMongoDbApiUsageException("Invalid query: 'not' can't be used with 'is' - use 'ne' instead.");
} }
this.isValue = o; this.isValue = o;
return this; 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 * Creates a criterion using the $ne operator
* *
Expand Down Expand Up @@ -269,7 +274,11 @@ public Criteria type(int t) {
* @return * @return
*/ */
public Criteria not() { public Criteria not() {
criteria.put("$not", null); return not(null);
}

private Criteria not(Object value) {
criteria.put("$not", value);
return this; return this;
} }


Expand All @@ -280,8 +289,7 @@ public Criteria not() {
* @return * @return
*/ */
public Criteria regex(String re) { public Criteria regex(String re) {
criteria.put("$regex", re); return regex(re, null);
return this;
} }


/** /**
Expand All @@ -292,13 +300,32 @@ public Criteria regex(String re) {
* @return * @return
*/ */
public Criteria regex(String re, String options) { public Criteria regex(String re, String options) {
criteria.put("$regex", re); return regex(toPattern(re, options));
if (StringUtils.hasText(options)) { }
criteria.put("$options", 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; 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. * Creates a geospatial criterion using a $within $center operation. This is only available for Mongo 1.7 and higher.
* *
Expand Down Expand Up @@ -426,16 +453,17 @@ protected DBObject getSingleCriteriaObject() {
DBObject dbo = new BasicDBObject(); DBObject dbo = new BasicDBObject();
boolean not = false; boolean not = false;
for (String k : this.criteria.keySet()) { for (String k : this.criteria.keySet()) {
Object value = this.criteria.get(k);
if (not) { if (not) {
DBObject notDbo = new BasicDBObject(); DBObject notDbo = new BasicDBObject();
notDbo.put(k, this.criteria.get(k)); notDbo.put(k, value);
dbo.put("$not", notDbo); dbo.put("$not", notDbo);
not = false; not = false;
} else { } else {
if ("$not".equals(k)) { if ("$not".equals(k) && value == null) {
not = true; not = true;
} else { } else {
dbo.put(k, this.criteria.get(k)); dbo.put(k, value);
} }
} }
} }
Expand Down
Expand Up @@ -130,6 +130,7 @@ protected void cleanDb() {
template.dropCollection(template.getCollectionName(PersonWithIdPropertyOfTypeLong.class)); template.dropCollection(template.getCollectionName(PersonWithIdPropertyOfTypeLong.class));
template.dropCollection(template.getCollectionName(PersonWithIdPropertyOfPrimitiveLong.class)); template.dropCollection(template.getCollectionName(PersonWithIdPropertyOfPrimitiveLong.class));
template.dropCollection(template.getCollectionName(TestClass.class)); template.dropCollection(template.getCollectionName(TestClass.class));
template.dropCollection(Sample.class);
} }


@Test @Test
Expand Down Expand Up @@ -1080,10 +1081,33 @@ public void removesEntityWithAnnotatedIdIfIdNeedsMassaging() {
assertThat(template.findOne(query(where("id").is(id)), Sample.class), is(nullValue())); 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 { public static class Sample {


@Id @Id
String id; String id;
String field;
} }


static class TestClass { static class TestClass {
Expand Down
Expand Up @@ -100,7 +100,7 @@ public void testSimpleQueryWithChainedCriteria() {
public void testComplexQueryWithMultipleChainedCriteria() { public void testComplexQueryWithMultipleChainedCriteria() {
Query q = new Query(where("name").regex("^T.*").and("age").gt(20).lt(80).and("city") Query q = new Query(where("name").regex("^T.*").and("age").gt(20).lt(80).and("city")
.in("Stockholm", "London", "New York")); .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\"]}}"; + "\"city\" : { \"$in\" : [ \"Stockholm\" , \"London\" , \"New York\"]}}";
Assert.assertEquals(expected, q.getQueryObject().toString()); Assert.assertEquals(expected, q.getQueryObject().toString());
} }
Expand Down Expand Up @@ -134,7 +134,7 @@ public void testQueryWithIn() {
@Test @Test
public void testQueryWithRegex() { public void testQueryWithRegex() {
Query q = new Query(where("name").regex("b.*")); 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()); Assert.assertEquals(expected, q.getQueryObject().toString());
} }


Expand Down

0 comments on commit ba5a764

Please sign in to comment.