Skip to content

Commit a2db161

Browse files
m3k0813mp911de
authored andcommitted
Support multiple Update operations for the same column name.
Previously, the `Update` class used a `Map` to store its operations, keyed by column name. This prevented multiple operations on the same column, such as setting different keys in a map-type column, as later operations would overwrite earlier ones. This commit changes the internal data structure to a `List`, allowing multiple operations to be recorded. To align with the 'last-wins' semantics for conflicting operations (e.g., setting the same column twice), a conflict resolution mechanism has been added. New operations now replace existing, conflicting ones. Adds comprehensive unit tests to verify both the coexistence of non-conflicting operations and the replacement of conflicting ones. Signed-off-by: Jeongjun Min <m3k0813@gmail.com> See #1525 Original pull request: #1596
1 parent 43063c3 commit a2db161

File tree

3 files changed

+117
-16
lines changed

3 files changed

+117
-16
lines changed

spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/query/Update.java

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@
1717

1818
import static org.springframework.data.cassandra.core.query.SerializationUtils.*;
1919

20+
import java.util.ArrayList;
2021
import java.util.Arrays;
2122
import java.util.Collection;
2223
import java.util.Collections;
23-
import java.util.LinkedHashMap;
24+
import java.util.List;
2425
import java.util.Map;
2526

2627
import org.springframework.data.cassandra.core.query.Update.AddToOp.Mode;
@@ -40,13 +41,14 @@
4041
*
4142
* @author Mark Paluch
4243
* @author Chema Vinacua
44+
* @author Jeongjun Min
4345
* @since 2.0
4446
*/
4547
public class Update {
4648

47-
private static final Update EMPTY = new Update(Collections.emptyMap());
49+
private static final Update EMPTY = new Update(Collections.emptyList());
4850

49-
private final Map<ColumnName, AssignmentOp> updateOperations;
51+
private final List<AssignmentOp> updateOperations;
5052

5153
/**
5254
* Create an empty {@link Update} object.
@@ -66,11 +68,9 @@ public static Update of(Iterable<AssignmentOp> assignmentOps) {
6668

6769
Assert.notNull(assignmentOps, "Update operations must not be null");
6870

69-
Map<ColumnName, AssignmentOp> updateOperations = assignmentOps instanceof Collection
70-
? new LinkedHashMap<>(((Collection<?>) assignmentOps).size())
71-
: new LinkedHashMap<>();
71+
List<AssignmentOp> updateOperations = new ArrayList<>();
7272

73-
assignmentOps.forEach(assignmentOp -> updateOperations.put(assignmentOp.getColumnName(), assignmentOp));
73+
assignmentOps.forEach(updateOperations::add);
7474

7575
return new Update(updateOperations);
7676
}
@@ -84,7 +84,7 @@ public static Update update(String columnName, @Nullable Object value) {
8484
return empty().set(columnName, value);
8585
}
8686

87-
private Update(Map<ColumnName, AssignmentOp> updateOperations) {
87+
private Update(List<AssignmentOp> updateOperations) {
8888
this.updateOperations = updateOperations;
8989
}
9090

@@ -215,24 +215,56 @@ public Update decrement(String columnName, Number delta) {
215215
* @return {@link Collection} of update operations.
216216
*/
217217
public Collection<AssignmentOp> getUpdateOperations() {
218-
return Collections.unmodifiableCollection(updateOperations.values());
218+
return Collections.unmodifiableCollection(updateOperations);
219219
}
220220

221221
private Update add(AssignmentOp assignmentOp) {
222222

223-
Map<ColumnName, AssignmentOp> map = new LinkedHashMap<>(this.updateOperations.size() + 1);
223+
List<AssignmentOp> list = new ArrayList<>(this.updateOperations.size() + 1);
224224

225-
map.putAll(this.updateOperations);
226-
map.put(assignmentOp.getColumnName(), assignmentOp);
225+
for (AssignmentOp existing : this.updateOperations) {
226+
if (!conflicts(existing, assignmentOp)) {
227+
list.add(existing);
228+
}
229+
}
227230

228-
return new Update(map);
231+
list.add(assignmentOp);
232+
233+
return new Update(list);
234+
}
235+
236+
/**
237+
* Determine whether two assignment operations conflict and should not co-exist in a single {@link Update}.
238+
* Conflicts are defined as whole-column operations on the same column or element-level operations targeting
239+
* the same element (same map key or same list index) on the same column. In case of conflict, last-wins semantics
240+
* apply and the incoming operation replaces the existing one.
241+
*/
242+
private static boolean conflicts(AssignmentOp existing, AssignmentOp incoming) {
243+
244+
if (!existing.getColumnName().equals(incoming.getColumnName())) {
245+
return false;
246+
}
247+
248+
if (existing instanceof SetAtKeyOp e && incoming instanceof SetAtKeyOp i) {
249+
return equalsNullSafe(e.getKey(), i.getKey());
250+
}
251+
252+
if (existing instanceof SetAtIndexOp e && incoming instanceof SetAtIndexOp i) {
253+
return e.getIndex() == i.getIndex();
254+
}
255+
256+
return true;
229257
}
230258

231-
@Override
232-
public String toString() {
233-
return StringUtils.collectionToDelimitedString(updateOperations.values(), ", ");
259+
private static boolean equalsNullSafe(@Nullable Object a, @Nullable Object b) {
260+
return a == b || (a != null && a.equals(b));
234261
}
235262

263+
@Override
264+
public String toString() {
265+
return StringUtils.collectionToDelimitedString(updateOperations, ", ");
266+
}
267+
236268
/**
237269
* Builder to add a single element/multiple elements to a collection associated with a {@link ColumnName}.
238270
*

spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/StatementFactoryUnitTests.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
*
6868
* @author Mark Paluch
6969
* @author Sam Lightfoot
70+
* @author Jeongjun Min
7071
*/
7172
class StatementFactoryUnitTests {
7273

@@ -964,6 +965,19 @@ void shouldRenderSimilaritySelector() {
964965
assertThat(statement.getNamedValues()).isEmpty();
965966
}
966967

968+
@Test // GH-1525
969+
void shouldCreateUpdateWithMultipleOperationsOnSameColumnDifferentKeys() {
970+
971+
Update update = Update.empty().set("map").atKey("key1").to("value1").set("map").atKey("key2").to("value2");
972+
973+
StatementBuilder<com.datastax.oss.driver.api.querybuilder.update.Update> updateStatementBuilder = statementFactory
974+
.update(Query.empty(), update, personEntity);
975+
976+
String cql = updateStatementBuilder.build(ParameterHandling.INLINE).getQuery();
977+
978+
assertThat(cql).isEqualTo("UPDATE person SET map['key1']='value1', map['key2']='value2'");
979+
}
980+
967981
@SuppressWarnings("unused")
968982
static class Person {
969983

spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/query/UpdateUnitTests.java

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
*
2626
* @author Mark Paluch
2727
* @author Chema Vinacua
28+
* @author Jeongjun Min
2829
*/
2930
class UpdateUnitTests {
3031

@@ -153,4 +154,58 @@ void shouldCreateDecrementLongUpdate() {
153154
assertThat(update.getUpdateOperations()).hasSize(1);
154155
assertThat(update).hasToString("foo = foo - 2400000000");
155156
}
157+
158+
@Test // GH-1525
159+
void shouldAllowMultipleSetAtKeyOperationsOnSameColumn() {
160+
161+
Update update = Update.empty().set("foo").atKey("k1").to("v1").set("foo").atKey("k2").to("v2");
162+
163+
assertThat(update.getUpdateOperations()).hasSize(2);
164+
assertThat(update).hasToString("foo['k1'] = 'v1', foo['k2'] = 'v2'");
165+
}
166+
167+
@Test // GH-1525
168+
void shouldUseLastWinsForDuplicateSetAtKeyOnSameKey() {
169+
170+
Update update = Update.empty().set("foo").atKey("k1").to("v1").set("foo").atKey("k1").to("v2");
171+
172+
assertThat(update.getUpdateOperations()).hasSize(1);
173+
assertThat(update).hasToString("foo['k1'] = 'v2'");
174+
}
175+
176+
@Test // GH-1525
177+
void shouldAllowMultipleSetAtIndexOperationsOnSameColumn() {
178+
179+
Update update = Update.empty().set("foo").atIndex(1).to("A").set("foo").atIndex(2).to("B");
180+
181+
assertThat(update.getUpdateOperations()).hasSize(2);
182+
assertThat(update).hasToString("foo[1] = 'A', foo[2] = 'B'");
183+
}
184+
185+
@Test // GH-1525
186+
void shouldUseLastWinsForDuplicateSetAtIndexOnSameIndex() {
187+
188+
Update update = Update.empty().set("foo").atIndex(1).to("A").set("foo").atIndex(1).to("B");
189+
190+
assertThat(update.getUpdateOperations()).hasSize(1);
191+
assertThat(update).hasToString("foo[1] = 'B'");
192+
}
193+
194+
@Test // GH-1525
195+
void wholeColumnAndElementLevelShouldConflict_LastWinsWholeColumn() {
196+
197+
Update update = Update.empty().set("foo").atKey("k1").to("v1").set("foo", "ALL");
198+
199+
assertThat(update.getUpdateOperations()).hasSize(1);
200+
assertThat(update).hasToString("foo = 'ALL'");
201+
}
202+
203+
@Test // GH-1525
204+
void wholeColumnAndElementLevelShouldConflict_LastWinsElementLevel() {
205+
206+
Update update = Update.empty().set("foo", "ALL").set("foo").atKey("k1").to("v1");
207+
208+
assertThat(update.getUpdateOperations()).hasSize(1);
209+
assertThat(update).hasToString("foo['k1'] = 'v1'");
210+
}
156211
}

0 commit comments

Comments
 (0)