From 57d144900881406279eab9053bf053a995285393 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 19 Feb 2014 17:13:36 +0100 Subject: [PATCH] DATAMONGO-852 - Update keeps track of fields to be modified. Update holds a set of fields that modifications are registered for. This information is used to determine if a modification is registered for the version field of a versioned entity. The change was introduced since the present solution did not correctly find the version property correctly within the DBObject resulting from the mapped update. In case version property is already included in Update automatic version update via $inc is will be skipped. Original pull request: #126 --- .../data/mongodb/core/MongoTemplate.java | 5 +- .../data/mongodb/core/query/Update.java | 44 ++++++++++-- .../data/mongodb/core/MongoTemplateTests.java | 23 ++++++ .../data/mongodb/core/query/UpdateTests.java | 72 +++++++++++++++++++ 4 files changed, 138 insertions(+), 6 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java index 8e761fcf6..9c698d154 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java @@ -1031,8 +1031,9 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { private void increaseVersionForUpdateIfNecessary(MongoPersistentEntity persistentEntity, Update update) { if (persistentEntity != null && persistentEntity.hasVersionProperty()) { - if (!dbObjectContainsVersionProperty(update.getUpdateObject(), persistentEntity)) { - update.inc(persistentEntity.getVersionProperty().getFieldName(), 1L); + String versionFieldName = persistentEntity.getVersionProperty().getFieldName(); + if (!update.modifies(versionFieldName)) { + update.inc(versionFieldName, 1L); } } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Update.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Update.java index 793cbadac..3c3ecd587 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Update.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Update.java @@ -19,11 +19,14 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.util.StringUtils; import com.mongodb.BasicDBObject; import com.mongodb.DBObject; @@ -43,6 +46,7 @@ public class Update { LAST, FIRST } + private Set keysToUpdate = new HashSet(); private Map modifierOps = new LinkedHashMap(); private Map pushCommandBuilders = new LinkedHashMap(1); @@ -78,7 +82,13 @@ public class Update { continue; } - update.modifierOps.put(key, object.get(key)); + Object value = object.get(key); + update.modifierOps.put(key, value); + if (isKeyword(key) && value instanceof DBObject) { + update.keysToUpdate.addAll(((DBObject) value).keySet()); + } else { + update.keysToUpdate.add(key); + } } return update; @@ -222,13 +232,12 @@ public class Update { * @return */ public Update pullAll(String key, Object[] values) { + Object[] convertedValues = new Object[values.length]; for (int i = 0; i < values.length; i++) { convertedValues[i] = values[i]; } - DBObject keyValue = new BasicDBObject(); - keyValue.put(key, convertedValues); - modifierOps.put("$pullAll", keyValue); + addFieldOperation("$pullAll", key, convertedValues); return this; } @@ -252,6 +261,12 @@ public class Update { return dbo; } + protected void addFieldOperation(String operator, String key, Object value) { + + modifierOps.put(operator, new BasicDBObject(key, value)); + this.keysToUpdate.add(key); + } + protected void addMultiFieldOperation(String operator, String key, Object value) { Object existingValue = this.modifierOps.get(operator); @@ -270,6 +285,27 @@ public class Update { } keyValueMap.put(key, value); + this.keysToUpdate.add(key); + } + + /** + * Determine if a given {@code key} will be touched on execution. + * + * @param key + * @return + */ + public boolean modifies(String key) { + return this.keysToUpdate.contains(key); + } + + /** + * Inspects given {@code key} for '$'. + * + * @param key + * @return + */ + private static boolean isKeyword(String key) { + return StringUtils.startsWithIgnoreCase(key, "$"); } /** 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 41a1065ca..0a834dc3f 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 @@ -2398,6 +2398,29 @@ public class MongoTemplateTests { assertThat(result.dbRefAnnotatedList.get(0).id, is((Object) "1")); } + /** + * @see DATAMONGO-852 + */ + @Test + public void updateShouldNotBumpVersionNumberIfVersionPropertyIncludedInUpdate() { + + VersionedPerson person = new VersionedPerson(); + person.firstname = "Dave"; + person.lastname = "Matthews"; + template.save(person); + assertThat(person.id, is(notNullValue())); + + Query qry = query(where("id").is(person.id)); + VersionedPerson personAfterFirstSave = template.findOne(qry, VersionedPerson.class); + assertThat(personAfterFirstSave.version, is(0L)); + + template.updateFirst(qry, Update.update("lastname", "Bubu").set("version", 100L), VersionedPerson.class); + + VersionedPerson personAfterUpdateFirst = template.findOne(qry, VersionedPerson.class); + assertThat(personAfterUpdateFirst.version, is(100L)); + assertThat(personAfterUpdateFirst.lastname, is("Bubu")); + } + static class DocumentWithDBRefCollection { @Id public String id; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/UpdateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/UpdateTests.java index 46ec08b73..0730c6fd2 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/UpdateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/UpdateTests.java @@ -188,4 +188,76 @@ public class UpdateTests { Update u = new Update().setOnInsert("size", 1).setOnInsert("count", 1); assertThat(u.getUpdateObject().toString(), is("{ \"$setOnInsert\" : { \"size\" : 1 , \"count\" : 1}}")); } + + /** + * @see DATAMONGO-852 + */ + @Test + public void testUpdateAffectsFieldShouldReturnTrueWhenMultiFieldOperationAddedForField() { + + Update update = new Update().set("foo", "bar"); + assertThat(update.modifies("foo"), is(true)); + } + + /** + * @see DATAMONGO-852 + */ + @Test + public void testUpdateAffectsFieldShouldReturnFalseWhenMultiFieldOperationAddedForField() { + + Update update = new Update().set("foo", "bar"); + assertThat(update.modifies("oof"), is(false)); + } + + /** + * @see DATAMONGO-852 + */ + @Test + public void testUpdateAffectsFieldShouldReturnTrueWhenSingleFieldOperationAddedForField() { + + Update update = new Update().pullAll("foo", new Object[] { "bar" }); + assertThat(update.modifies("foo"), is(true)); + } + + /** + * @see DATAMONGO-852 + */ + @Test + public void testUpdateAffectsFieldShouldReturnFalseWhenSingleFieldOperationAddedForField() { + + Update update = new Update().pullAll("foo", new Object[] { "bar" }); + assertThat(update.modifies("oof"), is(false)); + } + + /** + * @see DATAMONGO-852 + */ + @Test + public void testUpdateAffectsFieldShouldReturnFalseWhenCalledOnEmptyUpdate() { + assertThat(new Update().modifies("foo"), is(false)); + } + + /** + * @see DATAMONGO-852 + */ + @Test + public void testUpdateAffectsFieldShouldReturnTrueWhenUpdateWithKeyCreatedFromDbObject() { + + Update update = new Update().set("foo", "bar"); + Update clone = Update.fromDBObject(update.getUpdateObject()); + + assertThat(clone.modifies("foo"), is(true)); + } + + /** + * @see DATAMONGO-852 + */ + @Test + public void testUpdateAffectsFieldShouldReturnFalseWhenUpdateWithoutKeyCreatedFromDbObject() { + + Update update = new Update().set("foo", "bar"); + Update clone = Update.fromDBObject(update.getUpdateObject()); + + assertThat(clone.modifies("oof"), is(false)); + } }