DATADOC-283 DATADOC-300 Refatoring the QueryCriteria implementations to better support $and, $or and $nor queries

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

@@ -1,51 +0,0 @@
-/*
- * Copyright 2010-2011 the original author or authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.springframework.data.mongodb.core.query;
-
-import com.mongodb.BasicDBObject;
-import com.mongodb.DBObject;
-import org.bson.types.BasicBSONList;
-
-public class AndCriteria implements CriteriaDefinition {
-
-	Query[] queries = null;
-
-	public AndCriteria(Query[] queries) {
-		super();
-		this.queries = queries == null ? null : queries.clone();
-	}
-
-	/*
-	 * (non-Javadoc)
-	 * 
-	 * @see org.springframework.datastore.document.mongodb.query.Criteria#
-	 * getCriteriaObject(java.lang.String)
-	 */
-	public DBObject getCriteriaObject() {
-		DBObject dbo = new BasicDBObject();
-		BasicBSONList l = new BasicBSONList();
-		for (Query q : queries) {
-			l.add(q.getQueryObject());
-		}
-		dbo.put(getOperator(), l);
-		return dbo;
-	}
-
-	protected String getOperator() {
-		return "$and";
-	}
-
-}

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

@@ -1,24 +0,0 @@
-/*
- * Copyright 2010-2011 the original author or authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.springframework.data.mongodb.core.query;
-
-public class AndQuery extends Query {
-
-	public AndQuery(Query... q) {
-		super.and(q);
-	}
-
-}

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

@@ -20,6 +20,8 @@ import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.List;
 
+import org.bson.types.BasicBSONList;
+import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.mongodb.InvalidMongoDbApiUsageException;
 import org.springframework.data.mongodb.core.geo.Circle;
 import org.springframework.data.mongodb.core.geo.Point;
@@ -30,6 +32,10 @@ import org.springframework.util.StringUtils;
 import com.mongodb.BasicDBObject;
 import com.mongodb.DBObject;
 
+/**
+ * Central class for creating queries. It follows a fluent API style so that you can easily chain together multiple
+ * criteria. Static import of the 'Criteria.where' method will improve readability.
+ */
 public class Criteria implements CriteriaDefinition {
 
 	/**
@@ -45,6 +51,10 @@ public class Criteria implements CriteriaDefinition {
 
 	private Object isValue = NOT_SET;
 
+	public Criteria() {
+		this.criteriaChain = new ArrayList<Criteria>();
+	}
+
 	public Criteria(String key) {
 		this.criteriaChain = new ArrayList<Criteria>();
 		this.criteriaChain.add(this);
@@ -70,7 +80,6 @@ public class Criteria implements CriteriaDefinition {
 	/**
 	 * Static factory method to create a Criteria using the provided key
 	 *
-	 * @param key
 	 * @return
 	 */
 	public Criteria and(String key) {
@@ -356,6 +365,25 @@ public class Criteria implements CriteriaDefinition {
 		criteria.put("$or", queries);
 	}
 
+	public Criteria orOperator(Criteria... criteria) {
+		BasicBSONList bsonList = createCriteriaList(criteria);
+		criteriaChain.add(new Criteria("$or").is(bsonList));
+		return this;
+	}
+
+	public Criteria norOperator(Criteria... criteria) {
+		BasicBSONList bsonList = createCriteriaList(criteria);
+		criteriaChain.add(new Criteria("$nor").is(bsonList));
+		return this;
+	}
+
+	public Criteria andOperator(Criteria... criteria) {
+		BasicBSONList bsonList = createCriteriaList(criteria);
+		criteriaChain.add(new Criteria("$and").is(bsonList));
+		return this;
+	}
+
+
 	public String getKey() {
 		return this.key;
 	}
@@ -372,7 +400,10 @@ public class Criteria implements CriteriaDefinition {
 		} else {
 			DBObject criteriaObject = new BasicDBObject();
 			for (Criteria c : this.criteriaChain) {
-				criteriaObject.putAll(c.getSingleCriteriaObject());
+				DBObject dbo = c.getSingleCriteriaObject();
+				for (String k : dbo.keySet()) {
+					setValue(criteriaObject, k, dbo.get(k));
+				}
 			}
 			return criteriaObject;
 		}
@@ -405,4 +436,24 @@ public class Criteria implements CriteriaDefinition {
 		return queryCriteria;
 	}
 
+	private BasicBSONList createCriteriaList(Criteria[] criteria) {
+		BasicBSONList bsonList = new BasicBSONList();
+		for (Criteria c : criteria) {
+			bsonList.add(c.getCriteriaObject());
+		}
+		return bsonList;
+	}
+
+	private void setValue(DBObject dbo, String key, Object value) {
+		Object existing = dbo.get(key);
+		if (existing == null) {
+			dbo.put(key, value);
+		}
+		else {
+			throw new InvalidDataAccessApiUsageException("Due to limitations of the com.mongodb.BasicDBObject, " +
+					"you can't add a second '" + key + "' expression specified as '" + key + " : " + value + "'. " +
+					"Criteria already contains '" + key + " : " + existing + "'.");
+		}
+	}
+
 }

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

@@ -1,28 +0,0 @@
-/*
- * Copyright 2010-2011 the original author or authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.springframework.data.mongodb.core.query;
-
-public class NorCriteria extends OrCriteria {
-
-	public NorCriteria(Query[] queries) {
-		super(queries);
-	}
-
-	@Override
-	protected String getOperator() {
-		return "$nor";
-	}
-}

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

@@ -1,9 +0,0 @@
-package org.springframework.data.mongodb.core.query;
-
-public class NorQuery extends Query {
-
-	public NorQuery(Query... q) {
-		super.nor(q);
-	}
-
-}

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

@@ -1,51 +0,0 @@
-/*
- * Copyright 2010-2011 the original author or authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.springframework.data.mongodb.core.query;
-
-import com.mongodb.BasicDBObject;
-import com.mongodb.DBObject;
-import org.bson.types.BasicBSONList;
-
-public class OrCriteria implements CriteriaDefinition {
-
-	Query[] queries = null;
-
-	public OrCriteria(Query[] queries) {
-		super();
-		this.queries = queries == null ? null : queries.clone();
-	}
-
-	/*
-	 * (non-Javadoc)
-	 * 
-	 * @see org.springframework.datastore.document.mongodb.query.Criteria#
-	 * getCriteriaObject(java.lang.String)
-	 */
-	public DBObject getCriteriaObject() {
-		DBObject dbo = new BasicDBObject();
-		BasicBSONList l = new BasicBSONList();
-		for (Query q : queries) {
-			l.add(q.getQueryObject());
-		}
-		dbo.put(getOperator(), l);
-		return dbo;
-	}
-
-	protected String getOperator() {
-		return "$or";
-	}
-
-}

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

@@ -15,10 +15,21 @@
  */
 package org.springframework.data.mongodb.core.query;
 
+import java.util.ArrayList;
+import java.util.List;
+
 public class OrQuery extends Query {
 
 	public OrQuery(Query... q) {
-		super.or(q);
+		super(getOrCriteria(q));
+	}
+
+	private static Criteria getOrCriteria(Query[] queries) {
+		List<Criteria> criteriaList = new ArrayList<Criteria>();
+		for (Query q : queries) {
+			criteriaList.addAll(q.getCriteria());
+		}
+		return new Criteria(criteriaList, "$or");
 	}
 
 }

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

@@ -15,14 +15,18 @@
  */
 package org.springframework.data.mongodb.core.query;
 
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.LinkedHashMap;
+import java.util.List;
 
 import com.mongodb.BasicDBObject;
 import com.mongodb.DBObject;
+import org.springframework.dao.InvalidDataAccessApiUsageException;
 
 public class Query {
 
-	private LinkedHashMap<String, CriteriaDefinition> criteria = new LinkedHashMap<String, CriteriaDefinition>();
+	private LinkedHashMap<String, Criteria> criteria = new LinkedHashMap<String, Criteria>();
 	private Field fieldSpec;
 	private Sort sort;
 	private int skip;
@@ -46,22 +50,16 @@ public class Query {
 	}
 
 	public Query addCriteria(Criteria criteria) {
-		this.criteria.put(criteria.getKey(), criteria);
-		return this;
+		CriteriaDefinition existing = this.criteria.get(criteria.getKey());
+		String key = criteria.getKey();
+		if (existing == null) {
+			this.criteria.put(key, criteria);
 		}
-
-	public Query and(Query... queries) {
-		this.criteria.put("$and", new AndCriteria(queries));
-		return this;
+		else {
+			throw new InvalidDataAccessApiUsageException("Due to limitations of the com.mongodb.BasicDBObject, " +
+					"you can't add a second '" + key + "' criteria. " +
+					"Query already contains '" + existing.getCriteriaObject() + "'.");
 		}
-
-	public Query or(Query... queries) {
-		this.criteria.put("$or", new OrCriteria(queries));
-		return this;
-	}
-
-	public Query nor(Query... queries) {
-		this.criteria.put("$nor", new NorCriteria(queries));
 		return this;
 	}
 
@@ -124,4 +122,8 @@ public class Query {
 	public int getLimit() {
 		return this.limit;
 	}
+
+	protected List<Criteria> getCriteria() {
+		return new ArrayList<Criteria>(this.criteria.values());
+	}
 }

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoQueryCreator.java

@@ -31,6 +31,7 @@ import org.springframework.data.mongodb.core.geo.Shape;
 import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
 import org.springframework.data.mongodb.core.query.Criteria;
 import org.springframework.data.mongodb.core.query.CriteriaDefinition;
+import org.springframework.data.mongodb.core.query.OrQuery;
 import org.springframework.data.mongodb.core.query.Query;
 import org.springframework.data.mongodb.repository.query.ConvertingParameterAccessor.PotentiallyConvertingIterator;
 import org.springframework.data.repository.query.parser.AbstractQueryCreator;
@@ -133,8 +134,7 @@ class MongoQueryCreator extends AbstractQueryCreator<Query, Query> {
 	*/
 	@Override
 	protected Query or(Query base, Query query) {
-
-		return new Query().or(base, query);
+		return new OrQuery(new Query[] {base, query});
 	}
 
 	/*

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepository.java

@@ -220,14 +220,19 @@ public class SimpleMongoRepository<T, ID extends Serializable> implements Paging
 
 		Query query = null;
 
+		//TODO: verify intent
+//		for (ID id : ids) {
+//			if (query == null) {
+//				query = getIdQuery(id);
+//			} else {
+//				query = new Query().or(getIdQuery(id));
+//			}
+//		}
+		List<ID> idList = new ArrayList<ID>();
 		for (ID id : ids) {
-			if (query == null) {
-				query = getIdQuery(id);
-			} else {
-				query = new Query().or(getIdQuery(id));
-			}
+			idList.add(id);
 		}
-
+		query = new Query(Criteria.where(entityInformation.getIdAttribute()).in(idList));
 		return findAll(query);
 	}
 

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

@@ -641,9 +641,7 @@ public class MongoTemplateTests {
 		p4.setAge(41);
 		template.insert(p4);
 
-		Query q1 = new Query(Criteria.where("age").in(11, 21));
-		Query q2 = new Query(Criteria.where("age").is(31));
-		Query orQuery = new Query().or(q1, q2);
+		Query orQuery = new Query(new Criteria().orOperator(where("age").in(11, 21), where("age").is(31)));
 		List<PersonWithIdPropertyOfTypeObjectId> results = template.find(orQuery, PersonWithIdPropertyOfTypeObjectId.class);
 		assertThat(results.size(), is(3));
 		for (PersonWithIdPropertyOfTypeObjectId p : results) {

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

@@ -363,7 +363,8 @@ public class MappingTests {
 
 		Query one = query(where("ssn").is(1));
 		Query two = query(where("ssn").is(2));
-		List<PersonWithObjectId> results = template.find(new Query().or(one, two), PersonWithObjectId.class);
+		List<PersonWithObjectId> results = template.find(new Query(
+				new Criteria().orOperator(where("ssn").is(1), where("ssn").is(2))), PersonWithObjectId.class);
 
 		assertNotNull(results);
 		assertThat(results.size(), is(2));

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

@@ -20,10 +20,6 @@ import static org.springframework.data.mongodb.core.query.Criteria.*;
 import org.junit.Assert;
 import org.junit.Test;
 import org.springframework.data.mongodb.InvalidMongoDbApiUsageException;
-import org.springframework.data.mongodb.core.query.BasicQuery;
-import org.springframework.data.mongodb.core.query.NorQuery;
-import org.springframework.data.mongodb.core.query.OrQuery;
-import org.springframework.data.mongodb.core.query.Query;
 
 public class QueryTests {
 
@@ -52,23 +48,21 @@ public class QueryTests {
 
 	@Test
 	public void testOrQuery() {
-		Query q = new OrQuery(new Query(where("name").is("Sven").and("age").lt(50)), new Query(where("age").lt(50)),
-				new BasicQuery("{'name' : 'Thomas'}"));
+		Query q = new Query(new Criteria().orOperator(where("name").is("Sven").and("age").lt(50), where("age").lt(50), where("name").is("Thomas")));
 		String expected = "{ \"$or\" : [ { \"name\" : \"Sven\" , \"age\" : { \"$lt\" : 50}} , { \"age\" : { \"$lt\" : 50}} , { \"name\" : \"Thomas\"}]}";
 		Assert.assertEquals(expected, q.getQueryObject().toString());
 	}
 
 	@Test
 	public void testAndQuery() {
-		Query q = new AndQuery(new Query(where("name").is("Sven")), new Query(where("age").lt(50)));
+		Query q = new Query(new Criteria().andOperator(where("name").is("Sven"), where("age").lt(50)));
 		String expected = "{ \"$and\" : [ { \"name\" : \"Sven\"} , { \"age\" : { \"$lt\" : 50}}]}";
 		Assert.assertEquals(expected, q.getQueryObject().toString());
 	}
 
 	@Test
 	public void testNorQuery() {
-		Query q = new NorQuery(new Query(where("name").is("Sven")), new Query(where("age").lt(50)), new BasicQuery(
-				"{'name' : 'Thomas'}"));
+		Query q = new Query(new Criteria().norOperator(where("name").is("Sven"), where("age").lt(50), where("name").is("Thomas")));
 		String expected = "{ \"$nor\" : [ { \"name\" : \"Sven\"} , { \"age\" : { \"$lt\" : 50}} , { \"name\" : \"Thomas\"}]}";
 		Assert.assertEquals(expected, q.getQueryObject().toString());
 	}