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

readerForUpdating in 2.9.0.pr3 merge list instead of replace #1625

Closed
simon998yang opened this issue May 12, 2017 · 9 comments
Closed

readerForUpdating in 2.9.0.pr3 merge list instead of replace #1625

simon998yang opened this issue May 12, 2017 · 9 comments

Comments

@simon998yang
Copy link

I'm using code objectMapper.readerForUpdating(variables).readValue(jsonData)

variables is an existing HashMap, If there is list in the Map and jsonData, the lists will get merged, instead of replacing the existing list in the map.

@cowtowncoder
Copy link
Member

Could you please provide piece of sample code to show exactly what you are doing, so I can reproduce the issue.

@simon998yang
Copy link
Author

Thanks cowtowncoder for your response.

HashMap variables = new HashMap();
List list = new ArrayList();
list.add("a");
variables.put("list", list);
objectMapper.readerForUpdating(variables).readValue("{"list":["b"]}");
list = (List) variables.get("list");

I expect list = ["b"], but it is ["a","b"]

@cowtowncoder
Copy link
Member

Actually, this is the way merging should work as per #1399. I'll see whether default merge settings take effect here, will add notes.
Also, may want to consider whether List/array merging (which is done by appending) should be something that can be toggled on/off via new DeserializationFeature.

@cowtowncoder
Copy link
Member

Ok, so, yes, merging is occurring here. Part of the problem is that this is root-level value, and since call via readerForUpdating() enables merging, there is nothing preventing merge of values as well. This is a feature, new in 2.9.

Unfortunately settings that would otherwise be applicable (such as disabling mergeability of types like Lists) are not available here as nominal type is Map<?,?>. Although in theory specifying non-mergeability of Object would do the trick...

I'll have to see what could be done.

cowtowncoder added a commit that referenced this issue Jun 13, 2017
@cowtowncoder cowtowncoder added this to the 2.9,0.pr4 milestone Jun 14, 2017
@cowtowncoder
Copy link
Member

Ok, so... I added support for disabling merging for root-value Maps, when nominal value is Object.
If so, disabling mergability in general, or for type Object.class will make merge shallow:

mapper.setDefaultMergeable(false); // global default, no merging except where annotated
mapper.configOverride(Object.class) // prevent merging of values with type `Object`
            .setMergeable(false);

This is somewhat limited, and similar handling could be added for other types.
I will have to think whether something more generic could be done for Collection types (and/or arrays); supporting both shallow and deep merge is tricky to handle.

@SAJLinders
Copy link

SAJLinders commented Dec 8, 2022

Has there been an update since this was initially implemented? We are dealing with a JSON list that has many requirements for unique properties, so ideally we'd just be able to replace an entire list instead of having to worry about merging it correctly.

This (pseudo-) code is my current situation:

ObjectMapper mapper = new ObjectMapper();

JsonNode listA = mapper.readTree("{\"list\": [ \"A\" ] }");
JsonNode listB = mapper.readTree("{\"list\": [ \"B\" ] }");

JsonNode result = mapper.readerForUpdating(listA).readValue(listB);

System.out.println(result.toPrettyString()); // prints {"list":["A","B"]}

But ideally, I would only have a single element in the list (being B). I've tried adding the mapper options from the previous comment, but without any luck. Any suggestions? We're currently using Jackson 2.13.3.

@cowtowncoder
Copy link
Member

@SAJLinders No, the default behavior is addition. For overrides you could try applying those to JsonNode or ArrayNode and see if either (esp. latter) works (if you have not tried those targets yet). Specifying Object.class would not apply to JsonNode (it must be exact target, not a super type).

If that does not work I don't think there are other options at this point.

@SAJLinders
Copy link

@cowtowncoder thanks a lot, I wasn't aware that it had to be exact type, not super type. Since yesterday I was able to figure out what was going on though. This behaviour was only patched in Jackson 2.14 as per #3338, so I just had to use that version. For anyone reading this in the future, the following test will pass:

@Test
public void testMergeArrays_shouldReplace() throws IOException {
    ObjectMapper mapper = new ObjectMapper();
    mapper.configOverride(ArrayNode.class).setMergeable(false);

    JsonNode listA = mapper.readTree("{\"list\": [ \"A\" ] }");
    JsonNode listB = mapper.readTree("{\"list\": [ \"B\" ] }");

    JsonNode result = mapper.readerForUpdating(listA).readValue(listB);

    assertEquals(listB, result);
}

@cowtowncoder
Copy link
Member

@SAJLinders Ok great! Thank you for adding an update, that should help others who might have hit the issue.

And yes, the requirement for exact type match is non-obvious.

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

No branches or pull requests

3 participants