Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON patch doesn't update single Map's values [DATAREST-1338] #1697

Closed
spring-projects-issues opened this issue Jan 18, 2019 · 4 comments
Closed
Assignees
Labels
type: bug

Comments

@spring-projects-issues
Copy link

spring-projects-issues commented Jan 18, 2019

Luca Cherubin opened DATAREST-1338 and commented

Steps to reproduce:

//Here is my object
public class Book {
    public String author;
    public String ISBN;
    public Map<String, String> characters;
}

// Here I create a simple instance of the object
Book myBook = new Book();
myBook.author = "Me"
myBook.ISBN = "1234567890"
myBook.characters = new HashMap<>();
myBook.characters.put("protagonist", "Pinco");
myBook.characters.put("antagonist", "Pallo");

Here I create JSON patches

// Here the type of operations that work
[
    {"op": "replace", "path": "/author", "value": "NewAuthor"},
    {"op": "replace", "path": "/ISBN", "value": 0987654321 },
]

// I can also modify completely the Map if I want
[
    {"op": "replace", "path": "/characters", "value": {"protagonist": "Pallo", "antagonist": "Pinco"} }
]
  

Here the kind of operations that I would expect to work but do not work

// But I can't update a single value in the map 
[ 
{"op": "replace", "path": "/characters/protagonist", "value": "Pallo" }, {"op": "replace", "path": "/characters/antagonist", "value": "Pinco" } 
] 

// I've also tried weird stuff, but doesn't work 
[ {"op": "replace", "path": "/characters[antagonist]", "value": "Pinco"} ]  

 
Expected result:
I would expect the replace operation on the nested map to work by checking the key.
 
I think the issue is in the path->SpEL conversion, as the nested fields are automatically converted to properties updates.
 


Referenced from: pull request #305, and commits 017b69c, e3b21e7, 0c3092b, 2d76187

Backported to: 3.1.6 (Lovelace SR6)

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Jan 18, 2019

Oliver Drotbohm commented

You're right, it's currently a limitation in our to SpEL translations. It's particularly tricky as we have to but by definition cannot statically verify the paths containing a Map traversal as it needs an object instance to be verified on. Our current verification algorithm can safely identify collection traversals as it can inspect numbers. For maps however it's not clear whether a segment is a map key until we bind the path against a type.

It looks like we need to improve TypedSpelPath.verifyPath(…) to not choke on the map key segment and TypedSpelPath(SpelPath, Class<?>) to rebuild the expression based on the type information so that segments are translated into map keys properly

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Jan 29, 2019

Oliver Drotbohm commented

Should be back-portable to 3.1 and 3.0 (latter optional)

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Feb 6, 2019

mle-enso commented

Wonderful, just verified this for our use-case to PATCH an entity like:

// some Lombok annotations
public class Product {
    private String id;
    private Map<Locale, String> names;
}

with the following change:

[{"op", "remove", "path", "names/fr"}]

…which did not work before

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Mar 5, 2019

Oliver Drotbohm commented

That's now merged for inclusion in Spring Data Moore M2 and Lovelace SR6. Backport to Kay unfortunately didn't work as the implementation was using API implemented in Spring Data Commons in Lovelace only

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug
Projects
None yet
Development

No branches or pull requests

2 participants