Skip to content

Commit 38b2ec1

Browse files
christophstroblchristophstrobl
committed
Fix basic update overriding values.
This change makes sure basic update appends values to operations instead of overriding them. This change aligns the behaviour with Update and fixes issues where using the Update annotation with versioned entities can lead to loss of update information.
1 parent f49cd92 commit 38b2ec1

File tree

4 files changed

+257
-7
lines changed

4 files changed

+257
-7
lines changed

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

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
import java.util.Arrays;
1919
import java.util.Collections;
20+
import java.util.LinkedHashMap;
21+
import java.util.Map;
2022

2123
import org.bson.Document;
2224
import org.springframework.lang.Nullable;
@@ -44,7 +46,7 @@ public BasicUpdate(Document updateObject) {
4446

4547
@Override
4648
public Update set(String key, @Nullable Object value) {
47-
updateObject.put("$set", Collections.singletonMap(key, value));
49+
setOperationValue("$set", key, value);
4850
return this;
4951
}
5052

@@ -56,31 +58,44 @@ public Update unset(String key) {
5658

5759
@Override
5860
public Update inc(String key, Number inc) {
59-
updateObject.put("$inc", Collections.singletonMap(key, inc));
61+
setOperationValue("$inc", key, inc);
6062
return this;
6163
}
6264

65+
void setOperationValue(String operator, String key, Object value) {
66+
if (!updateObject.containsKey(operator)) {
67+
updateObject.put(operator, Collections.singletonMap(key, value));
68+
} else {
69+
Object o = updateObject.get(operator);
70+
if (o instanceof Map<?, ?> existing) {
71+
Map<Object, Object> target = new LinkedHashMap<>(existing);
72+
target.put(key, value);
73+
updateObject.put(operator, target);
74+
}
75+
}
76+
}
77+
6378
@Override
6479
public Update push(String key, @Nullable Object value) {
65-
updateObject.put("$push", Collections.singletonMap(key, value));
80+
setOperationValue("$push", key, value);
6681
return this;
6782
}
6883

6984
@Override
7085
public Update addToSet(String key, @Nullable Object value) {
71-
updateObject.put("$addToSet", Collections.singletonMap(key, value));
86+
setOperationValue("$addToSet", key, value);
7287
return this;
7388
}
7489

7590
@Override
7691
public Update pop(String key, Position pos) {
77-
updateObject.put("$pop", Collections.singletonMap(key, (pos == Position.FIRST ? -1 : 1)));
92+
setOperationValue("$pop", key, (pos == Position.FIRST ? -1 : 1));
7893
return this;
7994
}
8095

8196
@Override
8297
public Update pull(String key, @Nullable Object value) {
83-
updateObject.put("$pull", Collections.singletonMap(key, value));
98+
setOperationValue("$pull", key, value);
8499
return this;
85100
}
86101

@@ -94,10 +109,15 @@ public Update pullAll(String key, Object[] values) {
94109

95110
@Override
96111
public Update rename(String oldName, String newName) {
97-
updateObject.put("$rename", Collections.singletonMap(oldName, newName));
112+
setOperationValue("$rename", oldName, newName);
98113
return this;
99114
}
100115

116+
@Override
117+
public boolean modifies(String key) {
118+
return super.modifies(key) || Update.fromDocument(getUpdateObject()).modifies(key);
119+
}
120+
101121
@Override
102122
public Document getUpdateObject() {
103123
return updateObject;

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.springframework.data.mongodb.core.aggregation.SetOperation;
4242
import org.springframework.data.mongodb.core.mapping.Document;
4343
import org.springframework.data.mongodb.core.mapping.Field;
44+
import org.springframework.data.mongodb.core.query.BasicUpdate;
4445
import org.springframework.data.mongodb.core.query.Criteria;
4546
import org.springframework.data.mongodb.core.query.Query;
4647
import org.springframework.data.mongodb.core.query.Update;
@@ -326,6 +327,20 @@ void updateFirstWithSort(Class<?> domainType, Sort sort, UpdateDefinition update
326327
"Science is real!");
327328
}
328329

330+
@Test // GH-4918
331+
void updateShouldHonorVersionProvided() {
332+
333+
Versioned source = template.insert(Versioned.class).one(new Versioned("id-1", "value-0"));
334+
335+
Update update = new BasicUpdate("{ '$set' : { 'value' : 'changed' }, '$inc' : { 'version' : 10 } }");
336+
template.update(Versioned.class).matching(Query.query(Criteria.where("id").is(source.id))).apply(update).first();
337+
338+
assertThat(
339+
collection(Versioned.class).find(new org.bson.Document("_id", source.id)).limit(1).into(new ArrayList<>()))
340+
.containsExactly(new org.bson.Document("_id", source.id).append("version", 10L).append("value", "changed")
341+
.append("_class", "org.springframework.data.mongodb.core.MongoTemplateUpdateTests$Versioned"));
342+
}
343+
329344
private List<org.bson.Document> all(Class<?> type) {
330345
return collection(type).find(new org.bson.Document()).into(new ArrayList<>());
331346
}

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.mongodb.core.query;
17+
18+
import static org.springframework.data.mongodb.test.util.Assertions.assertThat;
19+
20+
import java.util.function.Function;
21+
import java.util.stream.Stream;
22+
23+
import org.bson.Document;
24+
import org.junit.jupiter.params.ParameterizedTest;
25+
import org.junit.jupiter.params.provider.Arguments;
26+
import org.junit.jupiter.params.provider.MethodSource;
27+
import org.springframework.data.mongodb.core.query.Update.Position;
28+
29+
/**
30+
* @author Christoph Strobl
31+
*/
32+
public class BasicUpdateUnitTests {
33+
34+
@ParameterizedTest // GH-4918
35+
@MethodSource("args")
36+
void updateOpsShouldNotOverrideExistingValues(String operator, Function<BasicUpdate, Update> updateFunction) {
37+
38+
Document source = Document.parse("{ '%s' : { 'key-1' : 'value-1' } }".formatted(operator));
39+
Update update = updateFunction.apply(new BasicUpdate(source));
40+
41+
assertThat(update.getUpdateObject()).containsEntry("%s.key-1".formatted(operator), "value-1")
42+
.containsKey("%s.key-2".formatted(operator));
43+
}
44+
45+
static Stream<Arguments> args() {
46+
return Stream.of( //
47+
Arguments.of("$set", (Function<BasicUpdate, Update>) update -> update.set("key-2", "value-2")),
48+
Arguments.of("$inc", (Function<BasicUpdate, Update>) update -> update.inc("key-2", 1)),
49+
Arguments.of("$push", (Function<BasicUpdate, Update>) update -> update.push("key-2", "value-2")),
50+
Arguments.of("$addToSet", (Function<BasicUpdate, Update>) update -> update.addToSet("key-2", "value-2")),
51+
Arguments.of("$pop", (Function<BasicUpdate, Update>) update -> update.pop("key-2", Position.FIRST)),
52+
Arguments.of("$pull", (Function<BasicUpdate, Update>) update -> update.pull("key-2", "value-2")),
53+
Arguments.of("$rename", (Function<BasicUpdate, Update>) update -> update.rename("key-2", "value-2")));
54+
};
55+
}

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/VersionedPersonRepositoryIntegrationTests.java

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.mongodb.repository;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import org.bson.Document;
21+
import org.bson.types.ObjectId;
22+
import org.junit.jupiter.api.BeforeEach;
23+
import org.junit.jupiter.api.Test;
24+
import org.junit.jupiter.api.extension.ExtendWith;
25+
import org.springframework.beans.factory.annotation.Autowired;
26+
import org.springframework.context.annotation.ComponentScan.Filter;
27+
import org.springframework.context.annotation.Configuration;
28+
import org.springframework.context.annotation.FilterType;
29+
import org.springframework.data.mongodb.config.AbstractMongoClientConfiguration;
30+
import org.springframework.data.mongodb.core.MongoTemplate;
31+
import org.springframework.data.mongodb.repository.config.EnableMongoRepositories;
32+
import org.springframework.data.mongodb.test.util.Client;
33+
import org.springframework.data.mongodb.test.util.MongoClientExtension;
34+
import org.springframework.data.mongodb.test.util.MongoTestUtils;
35+
import org.springframework.data.repository.CrudRepository;
36+
import org.springframework.lang.Nullable;
37+
import org.springframework.test.context.ContextConfiguration;
38+
import org.springframework.test.context.junit.jupiter.SpringExtension;
39+
40+
import com.mongodb.client.MongoClient;
41+
42+
/**
43+
* @author Christoph Strobl
44+
* @since 2025/03
45+
*/
46+
@ExtendWith({ MongoClientExtension.class, SpringExtension.class })
47+
@ContextConfiguration
48+
public class VersionedPersonRepositoryIntegrationTests {
49+
50+
static @Client MongoClient mongoClient;
51+
52+
@Autowired VersionedPersonRepository versionedPersonRepository;
53+
@Autowired MongoTemplate template;
54+
55+
@Configuration
56+
@EnableMongoRepositories(considerNestedRepositories = true,
57+
includeFilters = @Filter(type = FilterType.ASSIGNABLE_TYPE, classes = VersionedPersonRepository.class))
58+
static class Config extends AbstractMongoClientConfiguration {
59+
60+
@Override
61+
protected String getDatabaseName() {
62+
return "versioned-person-tests";
63+
}
64+
65+
@Override
66+
public MongoClient mongoClient() {
67+
return mongoClient;
68+
}
69+
}
70+
71+
@BeforeEach
72+
void beforeEach() {
73+
MongoTestUtils.flushCollection("versioned-person-tests", template.getCollectionName(VersionedPersonWithCounter.class),
74+
mongoClient);
75+
}
76+
77+
@Test // GH-4918
78+
void updatesVersionedTypeCorrectly() {
79+
80+
VersionedPerson person = template.insert(VersionedPersonWithCounter.class).one(new VersionedPersonWithCounter("Donald", "Duckling"));
81+
82+
int updateCount = versionedPersonRepository.findAndSetFirstnameToLastnameByLastname(person.getLastname());
83+
84+
assertThat(updateCount).isOne();
85+
86+
Document document = template.execute(VersionedPersonWithCounter.class, collection -> {
87+
return collection.find(new Document("_id", new ObjectId(person.getId()))).first();
88+
});
89+
90+
assertThat(document).containsEntry("firstname", "Duckling").containsEntry("version", 1L);
91+
}
92+
93+
@Test // GH-4918
94+
void updatesVersionedTypeCorrectlyWhenUpdateIsUsingInc() {
95+
96+
VersionedPerson person = template.insert(VersionedPersonWithCounter.class).one(new VersionedPersonWithCounter("Donald", "Duckling"));
97+
98+
int updateCount = versionedPersonRepository.findAndIncCounterByLastname(person.getLastname());
99+
100+
assertThat(updateCount).isOne();
101+
102+
Document document = template.execute(VersionedPersonWithCounter.class, collection -> {
103+
return collection.find(new Document("_id", new ObjectId(person.getId()))).first();
104+
});
105+
106+
assertThat(document).containsEntry("lastname", "Duckling").containsEntry("version", 1L).containsEntry("counter", 42);
107+
}
108+
109+
@Test // GH-4918
110+
void updatesVersionedTypeCorrectlyWhenUpdateCoversVersionBump() {
111+
112+
VersionedPerson person = template.insert(VersionedPersonWithCounter.class).one(new VersionedPersonWithCounter("Donald", "Duckling"));
113+
114+
int updateCount = versionedPersonRepository.findAndSetFirstnameToLastnameIncVersionByLastname(person.getLastname(),
115+
10);
116+
117+
assertThat(updateCount).isOne();
118+
119+
Document document = template.execute(VersionedPersonWithCounter.class, collection -> {
120+
return collection.find(new Document("_id", new ObjectId(person.getId()))).first();
121+
});
122+
123+
assertThat(document).containsEntry("firstname", "Duckling").containsEntry("version", 10L);
124+
}
125+
126+
public interface VersionedPersonRepository extends CrudRepository<VersionedPersonWithCounter, String> {
127+
128+
@Update("{ '$set': { 'firstname' : ?0 } }")
129+
int findAndSetFirstnameToLastnameByLastname(String lastname);
130+
131+
@Update("{ '$inc': { 'counter' : 42 } }")
132+
int findAndIncCounterByLastname(String lastname);
133+
134+
@Update("""
135+
{
136+
'$set': { 'firstname' : ?0 },
137+
'$inc': { 'version' : ?1 }
138+
}""")
139+
int findAndSetFirstnameToLastnameIncVersionByLastname(String lastname, int incVersion);
140+
141+
}
142+
143+
@org.springframework.data.mongodb.core.mapping.Document("versioned-person")
144+
static class VersionedPersonWithCounter extends VersionedPerson {
145+
146+
int counter;
147+
148+
public VersionedPersonWithCounter(String firstname, @Nullable String lastname) {
149+
super(firstname, lastname);
150+
}
151+
152+
public int getCounter() {
153+
return counter;
154+
}
155+
156+
public void setCounter(int counter) {
157+
this.counter = counter;
158+
}
159+
}
160+
}

0 commit comments

Comments
 (0)