Skip to content

Commit ad4dfc4

Browse files
committed
Polishing.
Reformat code. Rename conflicts method to overrides to indicate actual semantics and use type hierarchy for checks. Replace spaces with tab indents. Extend test. See #1525 Original pull request: #1596
1 parent 205cd1b commit ad4dfc4

File tree

3 files changed

+75
-45
lines changed

3 files changed

+75
-45
lines changed

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

Lines changed: 57 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,17 @@
2525
import java.util.Map;
2626

2727
import org.jspecify.annotations.Nullable;
28+
2829
import org.springframework.data.cassandra.core.query.Update.AddToOp.Mode;
2930
import org.springframework.util.Assert;
31+
import org.springframework.util.ObjectUtils;
3032
import org.springframework.util.StringUtils;
3133

3234
import com.datastax.oss.driver.api.core.CqlIdentifier;
3335

3436
/**
35-
* Update object representing a set of update operations. {@link Update} objects can be created in a fluent
36-
* style. Each construction operation creates a new immutable {@link Update} object.
37+
* Update object representing a set of update operations. {@link Update} objects can be created in a fluent style. Each
38+
* construction operation creates a new immutable {@link Update} object.
3739
*
3840
* <pre class="code">
3941
* Update update = Update.empty().set("foo", "bar").addTo("baz").prependAll(listOfValues);
@@ -68,7 +70,7 @@ public static Update of(Iterable<AssignmentOp> assignmentOps) {
6870

6971
Assert.notNull(assignmentOps, "Update operations must not be null");
7072

71-
List<AssignmentOp> updateOperations = new ArrayList<>();
73+
List<AssignmentOp> updateOperations = new ArrayList<>(assignmentOps instanceof Collection<?> c ? c.size() : 16);
7274

7375
assignmentOps.forEach(updateOperations::add);
7476

@@ -220,50 +222,24 @@ public Collection<AssignmentOp> getUpdateOperations() {
220222

221223
private Update add(AssignmentOp assignmentOp) {
222224

223-
List<AssignmentOp> list = new ArrayList<>(this.updateOperations.size() + 1);
224-
225-
for (AssignmentOp existing : this.updateOperations) {
226-
if (!conflicts(existing, assignmentOp)) {
227-
list.add(existing);
228-
}
229-
}
230-
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-
}
225+
List<AssignmentOp> list = new ArrayList<>(this.updateOperations.size() + 1);
247226

248-
if (existing instanceof SetAtKeyOp e && incoming instanceof SetAtKeyOp i) {
249-
return equalsNullSafe(e.getKey(), i.getKey());
227+
for (AssignmentOp existing : this.updateOperations) {
228+
if (!assignmentOp.overrides(existing)) {
229+
list.add(existing);
230+
}
250231
}
251232

252-
if (existing instanceof SetAtIndexOp e && incoming instanceof SetAtIndexOp i) {
253-
return e.getIndex() == i.getIndex();
254-
}
233+
list.add(assignmentOp);
255234

256-
return true;
235+
return new Update(list);
257236
}
258237

259-
private static boolean equalsNullSafe(@Nullable Object a, @Nullable Object b) {
260-
return a == b || (a != null && a.equals(b));
261-
}
262238

263239
@Override
264-
public String toString() {
265-
return StringUtils.collectionToDelimitedString(updateOperations, ", ");
266-
}
240+
public String toString() {
241+
return StringUtils.collectionToDelimitedString(updateOperations, ", ");
242+
}
267243

268244
/**
269245
* Builder to add a single element/multiple elements to a collection associated with a {@link ColumnName}.
@@ -574,6 +550,15 @@ public ColumnName getColumnName() {
574550
public CqlIdentifier toCqlIdentifier() {
575551
return this.columnName.getCqlIdentifier().orElseGet(() -> CqlIdentifier.fromCql(this.columnName.toCql()));
576552
}
553+
554+
/**
555+
* Determine whether the {@code other} assignment overrides this assignment.
556+
*
557+
* @since 5.0
558+
*/
559+
boolean overrides(AssignmentOp other) {
560+
return ObjectUtils.nullSafeEquals(this.columnName, other.columnName);
561+
}
577562
}
578563

579564
/**
@@ -679,6 +664,20 @@ public int getIndex() {
679664
return index;
680665
}
681666

667+
@Override
668+
boolean overrides(AssignmentOp other) {
669+
670+
if (super.overrides(other)) {
671+
if (other instanceof SetAtIndexOp i) {
672+
return getIndex() == i.getIndex();
673+
} else {
674+
return true;
675+
}
676+
}
677+
678+
return false;
679+
}
680+
682681
@Override
683682
public String toString() {
684683
return String.format("%s[%d] = %s", getColumnName(), index, serializeToCqlSafely(getValue()));
@@ -712,6 +711,20 @@ public Object getValue() {
712711
return value;
713712
}
714713

714+
@Override
715+
boolean overrides(AssignmentOp other) {
716+
717+
if (super.overrides(other)) {
718+
if (other instanceof SetAtKeyOp i) {
719+
return ObjectUtils.nullSafeEquals(getKey(), i.getKey());
720+
} else {
721+
return true;
722+
}
723+
}
724+
725+
return false;
726+
}
727+
715728
@Override
716729
public String toString() {
717730
return String.format("%s[%s] = %s", getColumnName(), serializeToCqlSafely(key), serializeToCqlSafely(getValue()));
@@ -761,6 +774,11 @@ public Object getValue() {
761774
return value;
762775
}
763776

777+
@Override
778+
boolean overrides(AssignmentOp other) {
779+
return false;
780+
}
781+
764782
@Override
765783
public String toString() {
766784
return String.format("%s = %s - %s", getColumnName(), getColumnName(), serializeToCqlSafely(getValue()));

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ void shouldSaveAndReadNullEmbeddedUDTCorrectly() {
783783
assertThat(target).isEqualTo(entity);
784784
}
785785

786-
@Test // DATACASS-829
786+
@Test // DATACASS-829, GH-1525
787787
void shouldPartiallyUpdateListOfMappedUdt() {
788788

789789
WithMappedUdtList entity = new WithMappedUdtList();
@@ -792,12 +792,14 @@ void shouldPartiallyUpdateListOfMappedUdt() {
792792

793793
template.insert(entity);
794794

795-
Update update = Update.empty().set("mappedUdts").atIndex(1).to(new MappedUdt("replacement"));
795+
Update update = Update.empty().set("mappedUdts").atIndex(1).to(new MappedUdt("replacement")).set("mappedUdts")
796+
.atIndex(2).to(new MappedUdt("replacement2"));
796797

797798
template.update(Query.query(where("id").is("id-1")), update, WithMappedUdtList.class);
798799

799800
WithMappedUdtList updated = template.selectOne(Query.query(where("id").is("id-1")), WithMappedUdtList.class);
800-
assertThat(updated.getMappedUdts()).extracting(MappedUdt::name).containsExactly("one", "replacement", "three");
801+
assertThat(updated.getMappedUdts()).extracting(MappedUdt::name).containsExactly("one", "replacement",
802+
"replacement2");
801803
}
802804

803805
@UserDefinedType

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,15 @@ void shouldRemoveFromList() {
9292
assertThat(update).hasToString("foo = foo - ['bar']");
9393
}
9494

95+
@Test // DATACASS-343
96+
void multipleRemovalsAreRetained() {
97+
98+
Update update = Update.empty().remove("foo", "bar").remove("foo", "baz");
99+
100+
assertThat(update.getUpdateOperations()).hasSize(2);
101+
assertThat(update).hasToString("foo = foo - ['bar'], foo = foo - ['baz']");
102+
}
103+
95104
@Test // DATACASS-343
96105
void shouldClearCollection() {
97106

@@ -185,10 +194,11 @@ void shouldAllowMultipleSetAtIndexOperationsOnSameColumn() {
185194
@Test // GH-1525
186195
void shouldUseLastWinsForDuplicateSetAtIndexOnSameIndex() {
187196

188-
Update update = Update.empty().set("foo").atIndex(1).to("A").set("foo").atIndex(1).to("B");
197+
Update update = Update.empty().set("foo").atIndex(1).to("A").set("bar").atIndex(1).to("B").set("foo").atIndex(1)
198+
.to("B");
189199

190-
assertThat(update.getUpdateOperations()).hasSize(1);
191-
assertThat(update).hasToString("foo[1] = 'B'");
200+
assertThat(update.getUpdateOperations()).hasSize(2);
201+
assertThat(update).hasToString("bar[1] = 'B', foo[1] = 'B'");
192202
}
193203

194204
@Test // GH-1525

0 commit comments

Comments
 (0)