Skip to content

Commit 56b9090

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 a2db161 commit 56b9090

File tree

3 files changed

+74
-45
lines changed

3 files changed

+74
-45
lines changed

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

Lines changed: 56 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,14 @@
2727
import org.springframework.data.cassandra.core.query.Update.AddToOp.Mode;
2828
import org.springframework.lang.Nullable;
2929
import org.springframework.util.Assert;
30+
import org.springframework.util.ObjectUtils;
3031
import org.springframework.util.StringUtils;
3132

3233
import com.datastax.oss.driver.api.core.CqlIdentifier;
3334

3435
/**
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.
36+
* Update object representing a set of update operations. {@link Update} objects can be created in a fluent style. Each
37+
* construction operation creates a new immutable {@link Update} object.
3738
*
3839
* <pre class="code">
3940
* Update update = Update.empty().set("foo", "bar").addTo("baz").prependAll(listOfValues);
@@ -68,7 +69,7 @@ public static Update of(Iterable<AssignmentOp> assignmentOps) {
6869

6970
Assert.notNull(assignmentOps, "Update operations must not be null");
7071

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

7374
assignmentOps.forEach(updateOperations::add);
7475

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

221222
private Update add(AssignmentOp assignmentOp) {
222223

223-
List<AssignmentOp> list = new ArrayList<>(this.updateOperations.size() + 1);
224+
List<AssignmentOp> list = new ArrayList<>(this.updateOperations.size() + 1);
224225

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-
}
247-
248-
if (existing instanceof SetAtKeyOp e && incoming instanceof SetAtKeyOp i) {
249-
return equalsNullSafe(e.getKey(), i.getKey());
226+
for (AssignmentOp existing : this.updateOperations) {
227+
if (!assignmentOp.overrides(existing)) {
228+
list.add(existing);
229+
}
250230
}
251231

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

256-
return true;
234+
return new Update(list);
257235
}
258236

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

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

268243
/**
269244
* Builder to add a single element/multiple elements to a collection associated with a {@link ColumnName}.
@@ -574,6 +549,15 @@ public ColumnName getColumnName() {
574549
public CqlIdentifier toCqlIdentifier() {
575550
return this.columnName.getCqlIdentifier().orElseGet(() -> CqlIdentifier.fromCql(this.columnName.toCql()));
576551
}
552+
553+
/**
554+
* Determine whether the {@code other} assignment overrides this assignment.
555+
*
556+
* @since 4.5.3
557+
*/
558+
boolean overrides(AssignmentOp other) {
559+
return ObjectUtils.nullSafeEquals(this.columnName, other.columnName);
560+
}
577561
}
578562

579563
/**
@@ -680,6 +664,20 @@ public int getIndex() {
680664
return index;
681665
}
682666

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+
683681
@Override
684682
public String toString() {
685683
return String.format("%s[%d] = %s", getColumnName(), index, serializeToCqlSafely(getValue()));
@@ -713,6 +711,20 @@ public Object getValue() {
713711
return value;
714712
}
715713

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+
716728
@Override
717729
public String toString() {
718730
return String.format("%s[%s] = %s", getColumnName(), serializeToCqlSafely(key), serializeToCqlSafely(getValue()));
@@ -762,6 +774,11 @@ public Object getValue() {
762774
return value;
763775
}
764776

777+
@Override
778+
boolean overrides(AssignmentOp other) {
779+
return false;
780+
}
781+
765782
@Override
766783
public String toString() {
767784
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)