From d6d8a13b0cff10bb2205de99e0ba4d46501463c1 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 2 Apr 2012 17:23:37 +0200 Subject: [PATCH] DATAMONGO-423 - Fixed handling of negated regular expressions. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../data/mongodb/core/query/Criteria.java | 50 +++++++++++++++---- .../data/mongodb/core/MongoTemplateTests.java | 26 +++++++++- .../data/mongodb/core/query/QueryTests.java | 4 +- 3 files changed, 66 insertions(+), 14 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 index 627236b47..4ce58f14a 100644 --- 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 @@ -20,14 +20,15 @@ import java.util.Arrays; 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 class Criteria implements CriteriaDefinition { 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 class Criteria implements CriteriaDefinition { * @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 class Criteria implements CriteriaDefinition { * @return */ public Criteria regex(String re) { - criteria.put("$regex", re); - return this; + return regex(re, null); } /** @@ -292,13 +300,32 @@ public class Criteria implements CriteriaDefinition { * @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 @@ public class Criteria implements CriteriaDefinition { 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); } } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java index 06e59d737..eed2aedfd 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java @@ -46,9 +46,9 @@ import org.springframework.data.mongodb.MongoDbFactory; 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 @@ public class MongoTemplateTests { 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 class MongoTemplateTests { 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 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 { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java index 8c074e6dc..5c9660a57 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java @@ -100,7 +100,7 @@ public class QueryTests { 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 class QueryTests { @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()); }