From d6f0f3aa9902ae29cc655fd884e39c10dd110e2a Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Thu, 16 Nov 2023 15:01:03 +0100 Subject: [PATCH] Properly merge non-entity arrays. Fixes #2325. --- .../rest/webmvc/json/DomainObjectReader.java | 8 +-- .../json/DomainObjectReaderUnitTests.java | 69 +++++++++++++++++-- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java index 0c98c7d2e..ae8462079 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java @@ -267,9 +267,8 @@ T doMerge(ObjectNode root, T target, ObjectMapper mapper) throws Exception { if (child.isArray()) { - IntFunction rawValues = index -> readRawCollectionElement(property.getComponentType(), fieldName, index, - root, - mapper); + IntFunction rawValues = index -> readRawCollectionElement(property.getComponentType(), fieldName, + index, root, mapper); if (handleArray(child, it, mapper, property.getTypeInformation(), rawValues)) { i.remove(); @@ -366,7 +365,8 @@ private boolean handleArrayNode(ArrayNode array, Collection collection, if (array.isEmpty() || collection.isEmpty() || ClassUtils.isPrimitiveOrWrapper(componentType.getType()) - || componentType.getType().isEnum()) { + || componentType.getType().isEnum() + || entities.getPersistentEntity(componentType.getType()).isEmpty()) { return false; } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java index db77867e2..f0f4156bf 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java @@ -108,6 +108,7 @@ void setUp() { mappingContext.getPersistentEntity(Pear.class); mappingContext.getPersistentEntity(WithCustomMappedPrimitiveCollection.class); mappingContext.getPersistentEntity(BugModel.class); + mappingContext.getPersistentEntity(ArrayListHolder.class); mappingContext.afterPropertiesSet(); this.entities = new PersistentEntities(Collections.singleton(mappingContext)); @@ -699,6 +700,42 @@ void deserializesNewNestedEntitiesCorrectly() throws Exception { .containsExactly("Foo", "Bar"); } + @Test // #2325 + void arraysCanMutateAndAppendDuringMerge() throws Exception { + + ObjectMapper mapper = new ObjectMapper(); + ArrayHolder target = new ArrayHolder(new String[] { "ancient", "old", "older" }); + JsonNode node = mapper.readTree("{ \"array\" : [ \"new\", \"old\", \"newer\", \"bleeding edge\" ] }"); + + ArrayHolder updated = reader.doMerge((ObjectNode) node, target, mapper); + + assertThat(updated.array).containsExactly("new", "old", "newer", "bleeding edge"); + } + + @Test // #2325 + void arraysCanAppendMoreThanOneElementDuringMerge() throws Exception { + + ObjectMapper mapper = new ObjectMapper(); + ArrayListHolder target = new ArrayListHolder("ancient", "old", "older"); + JsonNode node = mapper.readTree("{ \"values\" : [ \"ancient\", \"old\", \"older\", \"new\", \"newer\" ] }"); + + ArrayListHolder updated = reader.doMerge((ObjectNode) node, target, mapper); + + assertThat(updated.values).containsExactly("ancient", "old", "older", "new", "newer"); + } + + @Test // #2325 + void arraysCanRemoveElementsDuringMerge() throws Exception { + + ObjectMapper mapper = new ObjectMapper(); + ArrayHolder target = new ArrayHolder(new String[] { "ancient", "old", "older" }); + JsonNode node = mapper.readTree("{ \"array\" : [ \"ancient\" ] }"); + + ArrayHolder updated = reader.doMerge((ObjectNode) node, target, mapper); + + assertThat(updated.array).containsExactly("ancient"); + } + @SuppressWarnings("unchecked") private static T as(Object source, Class type) { @@ -837,18 +874,23 @@ public LocalizedValue(String value) { public LocalizedValue() {} + @Override public boolean equals(final Object o) { - if (o == this) + if (o == this) { return true; - if (!(o instanceof LocalizedValue)) + } + if (!(o instanceof LocalizedValue)) { return false; + } final LocalizedValue other = (LocalizedValue) o; - if (!other.canEqual((Object) this)) + if (!other.canEqual(this)) { return false; + } final Object this$value = this.value; final Object other$value = other.value; - if (this$value == null ? other$value != null : !this$value.equals(other$value)) + if (this$value == null ? other$value != null : !this$value.equals(other$value)) { return false; + } return true; } @@ -856,6 +898,7 @@ protected boolean canEqual(final Object other) { return other instanceof LocalizedValue; } + @Override public int hashCode() { final int PRIME = 59; int result = 1; @@ -918,10 +961,12 @@ public List getNested() { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (o == null || getClass() != o.getClass()) + } + if (o == null || getClass() != o.getClass()) { return false; + } SampleWithReference that = (SampleWithReference) o; @@ -1071,4 +1116,16 @@ static class NestedModel { public String value; } } + + static class ArrayListHolder { + Collection values; + + ArrayListHolder(String... values) { + this.values = new ArrayList<>(Arrays.asList(values)); + } + + public void setValues(Collection values) { + this.values = values; + } + } }