From e0720fa6f54c87cb9a60c4407c1270dd01fb777f Mon Sep 17 00:00:00 2001 From: Thomas Risberg Date: Thu, 7 Apr 2011 11:52:29 -0400 Subject: [PATCH] DATADOC-85 added mapping from 'id' to '_id' for queries used in remove operation --- .../data/document/mongodb/MongoTemplate.java | 44 ++++++++++++++++--- .../document/mongodb/MongoTemplateTests.java | 29 ++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/MongoTemplate.java index 4bbcfd840..bf98623a6 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/MongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/MongoTemplate.java @@ -843,15 +843,20 @@ public class MongoTemplate implements InitializingBean, MongoOperations, Applica * @see org.springframework.data.document.mongodb.MongoOperations#remove(java.lang.String, com.mongodb.DBObject) */ public void remove(String collectionName, final Query query) { + final DBObject queryObject = query.getQueryObject(); + substituteMappedIdIfNecessary(queryObject); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("remove using query: " + queryObject); + } execute(collectionName, new CollectionCallback() { public Void doInCollection(DBCollection collection) throws MongoException, DataAccessException { WriteResult wr = null; if (writeConcern == null) { - wr = collection.remove(query.getQueryObject()); + wr = collection.remove(queryObject); } else { - wr = collection.remove(query.getQueryObject(), writeConcern); + wr = collection.remove(queryObject, writeConcern); } - handleAnyWriteResultErrors(wr, query.getQueryObject(), "remove"); + handleAnyWriteResultErrors(wr, queryObject, "remove"); return null; } }); @@ -930,7 +935,7 @@ public class MongoTemplate implements InitializingBean, MongoOperations, Applica } substituteMappedIdIfNecessary(query, targetClass, readerToUse); if (LOGGER.isDebugEnabled()) { - LOGGER.debug("findOne using query: " + query.toString()); + LOGGER.debug("findOne using query: " + query + " fields: " + fields + " for class: " + targetClass); } return execute(new FindOneCallback(query, fields), new ReadDbObjectCallback(readerToUse, targetClass), collectionName); @@ -958,7 +963,7 @@ public class MongoTemplate implements InitializingBean, MongoOperations, Applica protected List doFind(String collectionName, DBObject query, DBObject fields, Class targetClass, CursorPreparer preparer) { substituteMappedIdIfNecessary(query, targetClass, mongoConverter); if (LOGGER.isDebugEnabled()) { - LOGGER.debug("find using query: " + query.toString()); + LOGGER.debug("find using query: " + query + " fields: " + fields + " for class: " + targetClass); } return executeEach(new FindCallback(query, fields), preparer, new ReadDbObjectCallback(mongoConverter, targetClass), collectionName); @@ -979,7 +984,7 @@ public class MongoTemplate implements InitializingBean, MongoOperations, Applica protected List doFind(String collectionName, DBObject query, DBObject fields, Class targetClass, MongoReader reader) { substituteMappedIdIfNecessary(query, targetClass, reader); if (LOGGER.isDebugEnabled()) { - LOGGER.debug("find using query: " + query.toString()); + LOGGER.debug("find using query: " + query + " fields: " + fields + " for class: " + targetClass); } return executeEach(new FindCallback(query, fields), null, new ReadDbObjectCallback(reader, targetClass), collectionName); @@ -1020,7 +1025,7 @@ public class MongoTemplate implements InitializingBean, MongoOperations, Applica } substituteMappedIdIfNecessary(query, targetClass, readerToUse); if (LOGGER.isDebugEnabled()) { - LOGGER.debug("findAndRemove using query: " + query.toString()); + LOGGER.debug("findAndRemove using query: " + query + " fields: " + fields + " sort: " + sort + " for class: " + targetClass); } return execute(new FindAndRemoveCallback(query, fields, sort), new ReadDbObjectCallback(readerToUse, targetClass), collectionName); @@ -1130,6 +1135,31 @@ public class MongoTemplate implements InitializingBean, MongoOperations, Applica } } + /** + * Substitutes the id key if it is found in he query. Any 'id' keys will be replaced with '_id'. No conversion + * of the value to an ObjectId is possible since we don't have access to a targetClass or a converter. This + * means the value has to be of the correct form. + * + * @param query + */ + protected void substituteMappedIdIfNecessary(DBObject query) { + String idKey = null; + if (query.containsField("id")) { + idKey = "id"; + } + if (query.containsField("_id")) { + idKey = "_id"; + } + if (idKey == null) { + // no ids in this query + return; + } + if (!idKey.equals(MongoPropertyDescriptor.ID_KEY)) { + Object value = query.get(idKey); + query.removeField(idKey); + query.put(MongoPropertyDescriptor.ID_KEY, value); + } + } private String getRequiredDefaultCollectionName() { String name = getDefaultCollectionName(); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/document/mongodb/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/document/mongodb/MongoTemplateTests.java index 122fbaf84..f622d7be2 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/document/mongodb/MongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/document/mongodb/MongoTemplateTests.java @@ -302,4 +302,33 @@ public class MongoTemplateTests { assertThat(results2.size(), is(2)); assertThat(results3.size(), is(1)); } + + @Test + public void testRemovingDocument() throws Exception { + + PersonWithIdPropertyOfTypeObjectId p1 = new PersonWithIdPropertyOfTypeObjectId(); + p1.setFirstName("Sven_to_be_removed"); + p1.setAge(51); + template.insert(p1); + + Query q1 = new Query(Criteria.where("id").is(p1.getId())); + PersonWithIdPropertyOfTypeObjectId found1 = template.findOne(q1, PersonWithIdPropertyOfTypeObjectId.class); + assertThat(found1, notNullValue()); + Query _q = new Query(Criteria.where("_id").is(p1.getId())); + template.remove(_q); + PersonWithIdPropertyOfTypeObjectId notFound1 = template.findOne(q1, PersonWithIdPropertyOfTypeObjectId.class); + assertThat(notFound1, nullValue()); + + PersonWithIdPropertyOfTypeObjectId p2 = new PersonWithIdPropertyOfTypeObjectId(); + p2.setFirstName("Bubba_to_be_removed"); + p2.setAge(51); + template.insert(p2); + + Query q2 = new Query(Criteria.where("id").is(p2.getId())); + PersonWithIdPropertyOfTypeObjectId found2 = template.findOne(q2, PersonWithIdPropertyOfTypeObjectId.class); + assertThat(found2, notNullValue()); + template.remove(q2); + PersonWithIdPropertyOfTypeObjectId notFound2 = template.findOne(q2, PersonWithIdPropertyOfTypeObjectId.class); + assertThat(notFound2, nullValue()); + } }