From 4da9ee5071fee85fd19396e2e8d6a340e767e262 Mon Sep 17 00:00:00 2001 From: Edik Amperyan Date: Mon, 5 Aug 2024 04:51:43 +0300 Subject: [PATCH 1/2] Fix updating nested collection fields using JSON Patch Fix updating nested collection fields using JSON Patch Closes #2401 Related tickets #2401 --- .../data/rest/webmvc/json/patch/JsonPointerMapping.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonPointerMapping.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonPointerMapping.java index e81cb3bdd..63b58221f 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonPointerMapping.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonPointerMapping.java @@ -74,7 +74,6 @@ private String verify(String pointer, Class type, BiFunction return pointer; } - PropertyPath base = null; StringBuilder result = new StringBuilder(); TypeInformation currentType = ClassTypeInformation.from(type); @@ -108,13 +107,11 @@ private String verify(String pointer, Class type, BiFunction .orElseThrow(() -> reject(segment, rejectType, pointer, qualifier)); try { - base = base == null ? PropertyPath.from(property, type) : base.nested(segment); + currentType = PropertyPath.from(property, currentType).getTypeInformation(); } catch (PropertyReferenceException o_O) { throw reject(segment, rejectType, pointer, qualifier); } - currentType = base.getTypeInformation(); - result.append("/").append(property); } From f66e097a67d59b2de1cf9e2a496ae3bb6a1cbc8d Mon Sep 17 00:00:00 2001 From: Edik Amperyan Date: Wed, 10 Jun 2026 15:54:31 +0300 Subject: [PATCH 2/2] Fix path verification for append to collections nested below indexed collection elements SkippedPropertyPath.nested() used path.getTypeInformation(), which returns the type of the first segment of the PropertyPath chain instead of the leaf property. For pointers like /catalog/books/0/reviews/- the index segment was therefore not skipped and was resolved as a property name, failing with PropertyReferenceException ("No property '0' found..."). Use path.getLeafProperty().getTypeInformation(), mirroring what SpelExpressionBuilder.next() already does. Related tickets #2401 Co-Authored-By: Claude Fable 5 --- .../data/rest/webmvc/json/patch/SpelPath.java | 2 +- .../webmvc/json/patch/SpelPathUnitTests.java | 78 +++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java index cbddf8cf5..3faf16918 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java @@ -521,7 +521,7 @@ public SkippedPropertyPath nested(String segment) { return SkippedPropertyPath.of(path.nested(segment), false); } - TypeInformation typeInformation = path.getTypeInformation(); + TypeInformation typeInformation = path.getLeafProperty().getTypeInformation(); return typeInformation.isMap() || typeInformation.isCollectionLike() // ? SkippedPropertyPath.of(path, true) // diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java index d4a77ab87..de012b2f9 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java @@ -54,6 +54,7 @@ void setUp() { context.getPersistentEntity(MapWrapper.class); context.getPersistentEntity(Todo.class); context.getPersistentEntity(Person.class); + context.getPersistentEntity(Library.class); PersistentEntities entities = new PersistentEntities(Arrays.asList(context)); BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities, @@ -167,6 +168,29 @@ void mapsRenamedProperty() { assertThat(path.getExpressionString()).isEqualTo("renamed"); } + @Test // #2401 + void getsLeafTypeForAppendToCollectionBelowIndexedCollectionElement() { + + WritingOperations path = SpelPath.untyped("/catalog/books/0/reviews/-") // + .bindForWrite(Library.class, context); + + assertThat(path.getExpressionString()).isEqualTo("catalog.books[0].reviews$[true]"); + assertThat(path.getLeafType()).isEqualTo(Review.class); + } + + @Test // #2401 + void appendsToCollectionBelowIndexedCollectionElement() { + + Library library = new Library(); + library.catalog.books.add(new Book()); + + Review review = new Review(); + AddOperation.of("/catalog/books/0/reviews/-", review) // + .perform(library, Library.class, context); + + assertThat(library.catalog.books.get(0).reviews).containsExactly(review); + } + @Test // #2233 void bindsDatesProperly() { @@ -252,4 +276,58 @@ public void setPeopleByInt(Map peopleByInt) { this.peopleByInt = peopleByInt; } } + + // #2401 + + static class Library { + + Catalog catalog = new Catalog(); + + public Catalog getCatalog() { + return this.catalog; + } + + public void setCatalog(Catalog catalog) { + this.catalog = catalog; + } + } + + static class Catalog { + + List books = new ArrayList<>(); + + public List getBooks() { + return this.books; + } + + public void setBooks(List books) { + this.books = books; + } + } + + static class Book { + + List reviews = new ArrayList<>(); + + public List getReviews() { + return this.reviews; + } + + public void setReviews(List reviews) { + this.reviews = reviews; + } + } + + static class Review { + + String text; + + public String getText() { + return this.text; + } + + public void setText(String text) { + this.text = text; + } + } }