From 3a47f192d7a9ae00726948a9fe4acdbbe98e09cb Mon Sep 17 00:00:00 2001 From: Thomas Risberg Date: Fri, 4 Feb 2011 21:02:14 -0500 Subject: [PATCH] update methods now return WriteResult; added configurable WriteResultChecking for update methods --- .../document/mongodb/MongoOperations.java | 9 +- .../data/document/mongodb/MongoTemplate.java | 109 +++++++--- .../document/mongodb/WriteResultChecking.java | 5 + .../data/document/mongodb/builder/Update.java | 4 + .../document/mongodb/MongoTemplateTests.java | 31 +++ .../data/document/mongodb/Person.java | 198 +++++++++--------- 6 files changed, 230 insertions(+), 126 deletions(-) create mode 100644 spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/WriteResultChecking.java diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/MongoOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/MongoOperations.java index d69fd2e27..0245e89d2 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/MongoOperations.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/MongoOperations.java @@ -24,6 +24,7 @@ import org.springframework.data.document.mongodb.builder.UpdateDefinition; import com.mongodb.CommandResult; import com.mongodb.DBCollection; import com.mongodb.DBObject; +import com.mongodb.WriteResult; /** * Interface that specifies a basic set of MongoDB operations. Implemented by {@link MongoTemplate}. @@ -433,7 +434,7 @@ public interface MongoOperations { * @param updateDoc the update document that contains the updated object or $ operators to manipulate the * existing object. */ - void updateFirst(QueryDefinition query, UpdateDefinition update); + WriteResult updateFirst(QueryDefinition query, UpdateDefinition update); /** * Updates the first object that is found in the specified collection that matches the query document criteria @@ -444,7 +445,7 @@ public interface MongoOperations { * @param updateDoc the update document that contains the updated object or $ operators to manipulate the * existing object. */ - void updateFirst(String collectionName, QueryDefinition query, + WriteResult updateFirst(String collectionName, QueryDefinition query, UpdateDefinition update); /** @@ -455,7 +456,7 @@ public interface MongoOperations { * @param updateDoc the update document that contains the updated object or $ operators to manipulate the * existing object. */ - void updateMulti(QueryDefinition query, UpdateDefinition update); + WriteResult updateMulti(QueryDefinition query, UpdateDefinition update); /** * Updates all objects that are found in the specified collection that matches the query document criteria @@ -466,7 +467,7 @@ public interface MongoOperations { * @param updateDoc the update document that contains the updated object or $ operators to manipulate the * existing object. */ - void updateMulti(String collectionName, QueryDefinition query, + WriteResult updateMulti(String collectionName, QueryDefinition query, UpdateDefinition update); /** 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 2618853b2..44c73886e 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 @@ -28,6 +28,7 @@ import org.springframework.beans.ConfigurablePropertyAccessor; import org.springframework.beans.PropertyAccessorFactory; import org.springframework.beans.factory.InitializingBean; import org.springframework.dao.DataAccessException; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.data.document.mongodb.MongoPropertyDescriptors.MongoPropertyDescriptor; import org.springframework.data.document.mongodb.builder.QueryDefinition; import org.springframework.data.document.mongodb.builder.UpdateDefinition; @@ -43,6 +44,7 @@ import com.mongodb.DBObject; import com.mongodb.Mongo; import com.mongodb.MongoException; import com.mongodb.WriteConcern; +import com.mongodb.WriteResult; import com.mongodb.util.JSON; /** @@ -65,6 +67,12 @@ public class MongoTemplate implements InitializingBean, MongoOperations { */ private WriteConcern writeConcern = null; + /* + * WriteResultChecking to be used for write operations if it has been specified. Otherwise + * we should not do any checking. + */ + private WriteResultChecking writeResultChecking = WriteResultChecking.NONE; + private final MongoConverter mongoConverter; private final Mongo mongo; private final MongoExceptionTranslator exceptionTranslator = new MongoExceptionTranslator(); @@ -81,7 +89,7 @@ public class MongoTemplate implements InitializingBean, MongoOperations { * @param databaseName */ public MongoTemplate(Mongo mongo, String databaseName) { - this(mongo, databaseName, null, null, null); + this(mongo, databaseName, null, null, null, null); } /** @@ -91,8 +99,8 @@ public class MongoTemplate implements InitializingBean, MongoOperations { * @param databaseName * @param writeConcern */ - public MongoTemplate(Mongo mongo, String databaseName, WriteConcern writeConcern) { - this(mongo, databaseName, null, null, writeConcern); + public MongoTemplate(Mongo mongo, String databaseName, WriteConcern writeConcern, WriteResultChecking writeResultChecking) { + this(mongo, databaseName, null, null, writeConcern, writeResultChecking); } /** @@ -102,7 +110,7 @@ public class MongoTemplate implements InitializingBean, MongoOperations { * @param defaultCollectionName */ public MongoTemplate(Mongo mongo, String databaseName, String defaultCollectionName) { - this(mongo, databaseName, defaultCollectionName, null, null); + this(mongo, databaseName, defaultCollectionName, null, null, null); } /** @@ -113,8 +121,8 @@ public class MongoTemplate implements InitializingBean, MongoOperations { * @param defaultCollectionName * @param writeConcern */ - public MongoTemplate(Mongo mongo, String databaseName, String defaultCollectionName, WriteConcern writeConcern) { - this(mongo, databaseName, defaultCollectionName, null, writeConcern); + public MongoTemplate(Mongo mongo, String databaseName, String defaultCollectionName, WriteConcern writeConcern, WriteResultChecking writeResultChecking) { + this(mongo, databaseName, defaultCollectionName, null, writeConcern, writeResultChecking); } /** @@ -125,7 +133,7 @@ public class MongoTemplate implements InitializingBean, MongoOperations { * @param mongoConverter */ public MongoTemplate(Mongo mongo, String databaseName, String defaultCollectionName, MongoConverter mongoConverter) { - this(mongo, databaseName, defaultCollectionName, mongoConverter, null); + this(mongo, databaseName, defaultCollectionName, mongoConverter, null, null); } /** @@ -136,8 +144,9 @@ public class MongoTemplate implements InitializingBean, MongoOperations { * @param defaultCollectionName * @param mongoConverter * @param writeConcern + * @param writeResultChecking */ - public MongoTemplate(Mongo mongo, String databaseName, String defaultCollectionName, MongoConverter mongoConverter, WriteConcern writeConcern) { + public MongoTemplate(Mongo mongo, String databaseName, String defaultCollectionName, MongoConverter mongoConverter, WriteConcern writeConcern, WriteResultChecking writeResultChecking) { Assert.notNull(mongo); Assert.notNull(databaseName); @@ -147,6 +156,9 @@ public class MongoTemplate implements InitializingBean, MongoOperations { this.mongo = mongo; this.databaseName = databaseName; this.writeConcern = writeConcern; + if (writeResultChecking != null) { + this.writeResultChecking = writeResultChecking; + } } @@ -601,23 +613,25 @@ public class MongoTemplate implements InitializingBean, MongoOperations { /* (non-Javadoc) * @see org.springframework.data.document.mongodb.MongoOperations#updateFirst(com.mongodb.DBObject, com.mongodb.DBObject) */ - public void updateFirst(QueryDefinition query, UpdateDefinition update) { - updateFirst(getRequiredDefaultCollectionName(), query, update); + public WriteResult updateFirst(QueryDefinition query, UpdateDefinition update) { + return updateFirst(getRequiredDefaultCollectionName(), query, update); } /* (non-Javadoc) * @see org.springframework.data.document.mongodb.MongoOperations#updateFirst(java.lang.String, com.mongodb.DBObject, com.mongodb.DBObject) */ - public void updateFirst(String collectionName, final QueryDefinition query, final UpdateDefinition update) { - execute(new CollectionCallback() { - public Void doInCollection(DBCollection collection) throws MongoException, DataAccessException { + public WriteResult updateFirst(String collectionName, final QueryDefinition query, final UpdateDefinition update) { + return execute(new CollectionCallback() { + public WriteResult doInCollection(DBCollection collection) throws MongoException, DataAccessException { + WriteResult wr; if (writeConcern == null) { - collection.update(query.getQueryObject(), update.getUpdateObject()); + wr = collection.update(query.getQueryObject(), update.getUpdateObject()); } else { - collection.update(query.getQueryObject(), update.getUpdateObject(), false, false, writeConcern); + wr = collection.update(query.getQueryObject(), update.getUpdateObject(), false, false, writeConcern); } - return null; + handleAnyWriteResultErrors(wr, query.getQueryObject(), "update with '" +update.getUpdateObject() + "'"); + return wr; } }, collectionName); } @@ -625,23 +639,25 @@ public class MongoTemplate implements InitializingBean, MongoOperations { /* (non-Javadoc) * @see org.springframework.data.document.mongodb.MongoOperations#updateMulti(com.mongodb.DBObject, com.mongodb.DBObject) */ - public void updateMulti(QueryDefinition query, UpdateDefinition update) { - updateMulti(getRequiredDefaultCollectionName(), query, update); + public WriteResult updateMulti(QueryDefinition query, UpdateDefinition update) { + return updateMulti(getRequiredDefaultCollectionName(), query, update); } /* (non-Javadoc) * @see org.springframework.data.document.mongodb.MongoOperations#updateMulti(java.lang.String, com.mongodb.DBObject, com.mongodb.DBObject) */ - public void updateMulti(String collectionName, final QueryDefinition query, final UpdateDefinition update) { - execute(new CollectionCallback() { - public Void doInCollection(DBCollection collection) throws MongoException, DataAccessException { + public WriteResult updateMulti(String collectionName, final QueryDefinition query, final UpdateDefinition update) { + return execute(new CollectionCallback() { + public WriteResult doInCollection(DBCollection collection) throws MongoException, DataAccessException { + WriteResult wr = null; if (writeConcern == null) { - collection.updateMulti(query.getQueryObject(), update.getUpdateObject()); + wr = collection.updateMulti(query.getQueryObject(), update.getUpdateObject()); } else { - collection.update(query.getQueryObject(), update.getUpdateObject(), false, true, writeConcern); + wr = collection.update(query.getQueryObject(), update.getUpdateObject(), false, true, writeConcern); } - return null; + handleAnyWriteResultErrors(wr, query.getQueryObject(), "update with '" +update.getUpdateObject() + "'"); + return wr; } }, collectionName); } @@ -659,12 +675,14 @@ public class MongoTemplate implements InitializingBean, MongoOperations { public void remove(String collectionName, final QueryDefinition query) { execute(new CollectionCallback() { public Void doInCollection(DBCollection collection) throws MongoException, DataAccessException { + WriteResult wr = null; if (writeConcern == null) { - collection.remove(query.getQueryObject()); + wr = collection.remove(query.getQueryObject()); } else { - collection.remove(query.getQueryObject(), writeConcern); + wr = collection.remove(query.getQueryObject(), writeConcern); } + handleAnyWriteResultErrors(wr, query.getQueryObject(), "remove"); return null; } }, collectionName); @@ -794,7 +812,43 @@ public class MongoTemplate implements InitializingBean, MongoOperations { return name; } - /** + /** + * Checks and handles any errors. + * + * TODO: current implementation logs errors - will be configurable to log warning, errors or + * throw exception in later versions + * + */ + private void handleAnyWriteResultErrors(WriteResult wr, DBObject query, String operation) { + if (WriteResultChecking.NONE == this.writeResultChecking) { + return; + } + String error = wr.getError(); + int n = wr.getN(); + if (error != null) { + String message = "Execution of '" + operation + + (query == null ? "" : "' using '" + query.toString() + "' query") + " failed: " + error; + if (WriteResultChecking.EXCEPTION == this.writeResultChecking) { + throw new DataIntegrityViolationException(message); + } + else { + LOGGER.error(message); + } + } + else if(n == 0) { + String message = "Execution of '" + operation + + (query == null ? "" : "' using '" + query.toString() + "' query") + " did not succeed: 0 documents updated"; + if (WriteResultChecking.EXCEPTION == this.writeResultChecking) { + throw new DataIntegrityViolationException(message); + } + else { + LOGGER.warn(message); + } + } + + } + + /** * Tries to convert the given {@link RuntimeException} into a {@link DataAccessException} but returns the original * exception if the conversation failed. Thus allows safe rethrowing of the return value. * @@ -802,7 +856,6 @@ public class MongoTemplate implements InitializingBean, MongoOperations { * @return */ private RuntimeException potentiallyConvertRuntimeException(RuntimeException ex) { - RuntimeException resolved = this.exceptionTranslator.translateExceptionIfPossible(ex); return resolved == null ? ex : resolved; } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/WriteResultChecking.java b/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/WriteResultChecking.java new file mode 100644 index 000000000..d2bb144f7 --- /dev/null +++ b/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/WriteResultChecking.java @@ -0,0 +1,5 @@ +package org.springframework.data.document.mongodb; + +public enum WriteResultChecking { + NONE, LOG, EXCEPTION +} diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/builder/Update.java b/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/builder/Update.java index 73b6a452b..1afb4291d 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/builder/Update.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/document/mongodb/builder/Update.java @@ -29,6 +29,10 @@ public class Update implements UpdateDefinition { } private HashMap criteria = new LinkedHashMap(); + + public static Update startUpdate() { + return new Update(); + } public Update set(String key, Object value) { criteria.put("$set", Collections.singletonMap(key, value)); 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 492d74878..ab4322e96 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 @@ -17,22 +17,30 @@ package org.springframework.data.document.mongodb; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.endsWith; import static org.junit.Assert.assertThat; import java.util.List; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.data.document.mongodb.builder.Query; +import org.springframework.data.document.mongodb.builder.Update; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import com.mongodb.WriteConcern; + /** * Integration test for {@link MongoTemplate}. * * @author Oliver Gierke + * @author Thomas Risberg */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("classpath:infrastructure.xml") @@ -41,6 +49,9 @@ public class MongoTemplateTests { @Autowired MongoTemplate template; + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Before public void setUp() { template.dropCollection(template.getDefaultCollectionName()); @@ -50,6 +61,7 @@ public class MongoTemplateTests { public void insertsSimpleEntityCorrectly() throws Exception { Person person = new Person("Oliver"); + person.setAge(25); template.insert(person); MongoConverter converter = template.getConverter(); @@ -59,8 +71,27 @@ public class MongoTemplateTests { assertThat(result, hasItem(person)); } + @Test + public void updateFailure() throws Exception { + + MongoTemplate mongoTemplate = new MongoTemplate(template.getDb().getMongo(), "test", "people", + new WriteConcern(), WriteResultChecking.EXCEPTION); + + Person person = new Person("Oliver2"); + person.setAge(25); + mongoTemplate.insert(person); + + Query q = Query.startQueryWithCriteria("BOGUS").gt(22).end(); + Update u = Update.startUpdate().set("firstName", "Sven"); + thrown.expect(DataIntegrityViolationException.class); + thrown.expectMessage( endsWith("0 documents updated") ); + mongoTemplate.updateFirst(q, u); + + } + @Test public void simpleQuery() throws Exception { Query.startQueryWithCriteria("name").is("Mary").and("age").lt(33).gt(22).end().skip(22).limit(20); + // TODO: more tests } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/document/mongodb/Person.java b/spring-data-mongodb/src/test/java/org/springframework/data/document/mongodb/Person.java index 387918e4c..78bddbef2 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/document/mongodb/Person.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/document/mongodb/Person.java @@ -1,94 +1,104 @@ -/* - * 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.document.mongodb; - -import org.bson.types.ObjectId; - -public class Person { - - private final ObjectId id; - - private String firstName; - - private Person friend; - - public Person() { - this.id = new ObjectId(); - } - - public Person(ObjectId id, String firstname) { - this.id = id; - this.firstName = firstname; - } - - public Person(String firstname) { - this(); - this.firstName = firstname; - } - - public ObjectId getId() { - return id; - } - - public String getFirstName() { - return firstName; - } - - public void setFirstName(String firstName) { - this.firstName = firstName; - } - - public Person getFriend() { - return friend; - } - - public void setFriend(Person friend) { - this.friend = friend; - } - - /* - * (non-Javadoc) - * - * @see java.lang.Object#equals(java.lang.Object) - */ - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; - } - - if (obj == null) { - return false; - } - - if (!(getClass().equals(obj.getClass()))) { - return false; - } - - Person that = (Person) obj; - - return this.id == null ? false : this.id.equals(that.id); - } - - - /* (non-Javadoc) - * @see java.lang.Object#hashCode() - */ - @Override - public int hashCode() { - return id.hashCode(); - } -} +/* + * 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.document.mongodb; + +import org.bson.types.ObjectId; + +public class Person { + + private final ObjectId id; + + private String firstName; + + private int age; + + private Person friend; + + public Person() { + this.id = new ObjectId(); + } + + public Person(ObjectId id, String firstname) { + this.id = id; + this.firstName = firstname; + } + + public Person(String firstname) { + this(); + this.firstName = firstname; + } + + public ObjectId getId() { + return id; + } + + public String getFirstName() { + return firstName; + } + + public void setFirstName(String firstName) { + this.firstName = firstName; + } + + public int getAge() { + return age; + } + + public void setAge(int age) { + this.age = age; + } + + public Person getFriend() { + return friend; + } + + public void setFriend(Person friend) { + this.friend = friend; + } + + /* + * (non-Javadoc) + * + * @see java.lang.Object#equals(java.lang.Object) + */ + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + + if (obj == null) { + return false; + } + + if (!(getClass().equals(obj.getClass()))) { + return false; + } + + Person that = (Person) obj; + + return this.id == null ? false : this.id.equals(that.id); + } + + + /* (non-Javadoc) + * @see java.lang.Object#hashCode() + */ + @Override + public int hashCode() { + return id.hashCode(); + } +}