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

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);
 			}
 		}
 	}

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<String> keysToUpdate = new HashSet<String>();
 	private Map<String, Object> modifierOps = new LinkedHashMap<String, Object>();
 	private Map<String, PushOperatorBuilder> pushCommandBuilders = new LinkedHashMap<String, PushOperatorBuilder>(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, "$");
 	}
 
 	/**

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;

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));
+	}
 }