Jackson's @JsonTypeInfo is not supported [DATAREST-872]

[spring-projects-issues]

Mathias D opened DATAREST-872 and commented

I am trying to use spring data rest in a scenario with spring-data-jpa and entity inheritance.

Spring data rest seems to be able to serialize entities with inheritance. But as soon as I add @JsonTypeInfo annotations (to be able to deserialize) I get the following error when serializing such entities:

Failed to write HTTP message: org.springframework.http.converter.HttpMessageNotWritableException:
    Could not write content: Type id handling not implemented for type java.lang.Object (by serializer of type org.springframework.data.rest.webmvc.json.PersistentEntityJackson2Module$NestedEntitySerializer)
    (through reference chain: org.springframework.hateoas.PagedResources["_embedded"]->java.util.UnmodifiableMap["parents"]->java.util.ArrayList[0]->org.springframework.data.rest.webmvc.json.["content"]->com.example.Parent["valueHolder"]);

The problem seems to be that the custom serializer used here (PersistentEntityJackson2Module$NestedEntitySerializer) does not implement com.fasterxml.jackson.databind.JsonSerializer#serializeWithType - which throw an Exception by default.

Please see this repo to reproduce the issue:
https://github.com/mduesterhoeft/spring-data-rest-entity-inheritance

---

Affects: 2.5.2 (Hopper SR2)

Reference URL: https://github.com/mduesterhoeft/spring-data-rest-entity-inheritance

Attachments:

• DecimalValue.java (1.14 kB)
• Parent.java (1.28 kB)
• spring-data-rest error stacktrace.txt (11.59 kB)
• StringValue.java (1.10 kB)
• Value.java (1.62 kB)

Referenced from: pull request #221

Backported to: 2.5.3 (Hopper SR3)

2 votes, 4 watchers

[spring-projects-issues]

Mathias D commented

Just read this one - https://twitter.com/olivergierke/status/765812150220759040 - and and I am happy to provide a maven project if that helps ;)

[spring-projects-issues]

Oliver Drotbohm commented

:D The important bit is the last part. If your project builds fine and I can get it imported into Eclipse easily, there's nothing to worry about. :)

[spring-projects-issues]

Mathias D commented

Then I should be on the safe side - the foundation of the project is basically the output of start.spring.io :D

[spring-projects-issues]

Rob Baily commented

I have just been fighting with this issue in the last day as well. The reason this is an issue (for me at least) is if you want to take the same entity (one with an abstract type member) and try to send it through a REST service (I'm using Spring Boot) and you do not define some way to define the type for the abstract class you will get this message: "abstract types either need to be mapped to concrete types, have custom deserializer, or contain additional type information". I am assuming Spring Data Rest has no issue with this because the class name is stored in the document repository. I've tried all sorts of configurations and none of them allow the repository and the REST service to both work. I debugged and got down to the same point that Mathias found. Guessing that at some point the Jackson folks added this serializeWithType method and it was never added to the class in this module.

I may try submitting a pull request for this if no one else plans to work on this soon. BTW https://github.com/spring-projects/spring-data-rest/blob/master/CONTRIBUTING.adoc points to https://github.com/spring-projects/spring-data-build/blob/master/CONTRIBUTING.md which gives a 404 error

[spring-projects-issues]

Rob Baily commented

I went ahead and submitted a pull request that addresses this issue. Oliver, please check it out and let me know what you think. I added an integration test and verified that the error referenced here occurs without the change and does not occur with the change in place.

Mathias, I reformatted your test code to go into the Spring Data Rest project without adding more dependencies and trying to follow some other test setups that they had. I noted you as an author in the code since I copied over what you used before modifying.

One thing to note is that your percentage value test did not work for me so I did not include it. I could not see where percentageValue was referenced anywhere else so not sure whether or not there was something else missing. Below is code that can be inserted into the DataRest872Tests class that compiles and runs, just the assertions fail.

	@Test
	public void testCreatePercentageValue() throws Exception {
		Map<Object, Object> subMap = new HashMap<>();
		subMap.put("type", "DECIMAL");
		subMap.put("value", 0.95);
		Map<Object, Object> jsonMap = new HashMap<>();
		jsonMap.put("valueHolder", subMap);

		Link parentsLink = client.discoverUnique("parents");
		MockHttpServletResponse parent = postAndGet(parentsLink, objectMapper.writeValueAsString(jsonMap),
				MediaType.APPLICATION_JSON);
		assertThat((String) JsonPath.read(parent.getContentAsString(), "$.valueHolder.type"), is("DECIMAL"));
		assertThat((String) JsonPath.read(parent.getContentAsString(), "$.valueHolder.percentageValue"), is(0.95d));
	}

[spring-projects-issues]

Mathias D commented

Rob, looks great.

I think the last line in the test you mentioned is wrong. The test used to fail on on the check for type so I did not realize this error. I think it should be

@Test
	public void testCreatePercentageValue() throws Exception {
		Map<Object, Object> subMap = new HashMap<>();
		subMap.put("type", "DECIMAL");
		subMap.put("value", 0.95);
		Map<Object, Object> jsonMap = new HashMap<>();
		jsonMap.put("valueHolder", subMap);
 
		Link parentsLink = client.discoverUnique("parents");
		MockHttpServletResponse parent = postAndGet(parentsLink, objectMapper.writeValueAsString(jsonMap),
				MediaType.APPLICATION_JSON);
		assertThat((String) JsonPath.read(parent.getContentAsString(), "$.valueHolder.type"), is("DECIMAL"));
		assertThat((String) JsonPath.read(parent.getContentAsString(), "$.valueHolder.value"), is(0.95d));
	}

[spring-projects-issues]

Rob Baily commented

Oliver, as there is a pull request is there any chance this can be slotted for an upcoming release? I am currently using a patched version locally for my project

[spring-projects-issues]

Oliver Drotbohm commented

Sorry, I didn't see the pull request coming in. Feel free to use the Workflow > Submit for Review step going forward.

TBCH, I don't see the PR to fly. It's adding a lot of code but doesn't really make obvious why that code needs to be there. We don't need any repository, JPA or the like to test this as it's about the Jackson serialization, right. The test case provided, doesn't even test anything. I've tried to replicate the fix and see the effect of it on a smaller scale — basically adding a test in PersistentEntitySerializationTests — but failed to actually change anything in the output generated. But maybe that's due to the fact that I don't get the details of the problematic case in the first place.

Would you mind to simply report here: an as tiny as possible set of domain classes, the JSON output it currently generates, describe what is missing or wrong and the JSON output you'd like to see? Happy to then find the most concise way of testing a fix for that :)

[spring-projects-issues]

Rob Baily commented

Ah, did not see the workflow step there. Will do so shortly for this and for the future.

As far as not getting the details I guess you did not see the GitHub project that Mathias submitted when he created the issue? It has code and the error that occurs. Also if you remove the 5 lines of code that I added to the PersistentEntityJackson2Module and run the test in DataRest872Tests you will see this error:

org.springframework.web.util.NestedServletException: Request processing failed; nested exception is org.springframework.http.converter.HttpMessageNotWritableException: Could not write content: Type id handling not implemented for type java.lang.Object (by serializer of type org.springframework.data.rest.webmvc.json.PersistentEntityJackson2Module$NestedEntitySerializer) (through reference chain: org.springframework.hateoas.PagedResources["_embedded"]->java.util.UnmodifiableMap["parents"]->java.util.ArrayList[0]->org.springframework.data.rest.webmvc.json.["content"]->org.springframework.data.rest.webmvc.jpa.Parent["valueHolder"]); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Type id handling not implemented for type java.lang.Object (by serializer of type org.springframework.data.rest.webmvc.json.PersistentEntityJackson2Module$NestedEntitySerializer) (through reference chain: org.springframework.hateoas.PagedResources["_embedded"]->java.util.UnmodifiableMap["parents"]->java.util.ArrayList[0]->org.springframework.data.rest.webmvc.json.["content"]->org.springframework.data.rest.webmvc.jpa.Parent["valueHolder"])

I've attached a file with the full stack trace from this here in case that is helpful.

I agree that it is definitely an issue with Jackson serialization. I've attached the files that were used in the test case I wrote as you should be able to use them. It appears as though you need to have a parent with JsonTypeInfo and then child classes that set up with JsonSubTypes. The trick seems to be is that this also has to be a nested entity in order to hit the NestedEntitySerializer so it does not apply in every case, like when you get a single entity of the parent class.

Please let us know if this does not give you enough info and we will try to provide more. I'll take a look to see if I can do any more on the tests. It seem like the PersistentEntityJackson2ModuleUnitTests.resolvesReferenceToSubtypeCorrectly test should be similar so maybe I can start there.

Not sure why you wouldn't want an additional integration test though. Seems like the more of those you have the better! (y)

[spring-projects-issues]

Rob Baily commented

Pushing to review state for now

[spring-projects-issues]

Rob Baily commented

Ok, when I looked at the stack trace and thought about it a little more it looks like it has to with lists of objects. Sure enough I was able to create the small test method below in PersistentEntityJackson2ModuleUnitTests which throws the exception mentioned previously. If I add my code back into PersistentEntityJackson2Module it throws a different exception about "PersistentEntity must not be null!" which I'm sure because I did not set up other things correctly.

Oliver, please let us know if you can create an appropriate test method from this.

	@Test
	public void handlesTypeInfoInListCorrectly() throws IOException {

		ArrayList<PetOwner> petOwners = new ArrayList<PetOwner>();
		Pet pet = new Cat();
		PetOwner owner = new PetOwner();
		owner.pet = pet;
		petOwners.add(owner);
		String jsonOutput = mapper.writeValueAsString(petOwners);
	}

[spring-projects-issues]

Alex Leigh commented

Using the Parent, Value, and StringValue classes you added, here's a test you can add to PersistentEntitySerializationTests.java to verify the behavior:

	@Test
	public void serializesInheritance() throws Exception {

		StringValue stringValue = new StringValue();
		stringValue.setValue("A string");

		Parent parent = new Parent();
		parent.setValueHolder(stringValue);

		PersistentEntityResource resource = PersistentEntityResource.
				build(parent, repositories.getPersistentEntity(Parent.class)).
				withLink(new Link("/parents/1")).
				build();

		String result = mapper.writeValueAsString(resource);

		assertThat(JsonPath.read(result, "$.valueHolder.type"), equalTo("STRING"));
	}

Unfortunately this test won't pass because the "type" field is missing from the serialized JSON. The fix implemented in the PR does not actually use the TypeSerializers to property serialize the type information that Jackson needs to deserialize. I'm working on improving the fix to actually include the type info but it does not appear to be a simple fix.

[spring-projects-issues]

Rob Baily commented

Alex, yes I knew that might be the case although in this use case does it make sense to add extra fields to the JSON that is returned? I suppose it could but it is not really a representation of what is in the storage

[spring-projects-issues]

Alex Leigh commented

Rob: yes, in this case, adding the extra field is exactly the intention of this annotation:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type")

Let's call the use of this annotation Scenario 1. Here, the annotation tells Jackson that when serializing objects of this type, add a new property to the serialized object, whose name is "type", and whose value is the logical name for this type. In this scenario, the fix in the PR does not behave as expected, because the "type" field is not being written when serializing.

However, there is another scenario worth considering, with a slightly different usage of the annotation:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY, property = "type")

Let's call this Scenario 2. Here the annotation tells Jackson that the logical type name is contained in the "type" property already existing on the object. In this scenario, assuming the class has something like a "getType()" method, the fix in the PR does behave as expected. The polymorphic classes are correctly serialized with the "type" property.

So I think this PR does not fully work as expected with all different usages of Jackson polymorphic annotations, however it is strictly an improvement over the current behavior. Currently, both scenario 1 and scenario 2 causes SDR to throw the "Type id handling not implemented for type java.lang.Object (by serializer of type org.springframework.data.rest.webmvc.json.PersistentEntityJackson2Module$NestedEntitySerializer)" exception. With the fix in the PR, neither scenario 1 nor scenario 2 throw exceptions, and for scenario 2 the behavior is exactly what is desired. Moreover, I tried to work on an improvement that would work for scenario 1 as well, but it turns out to be very difficult. This is because the PersistentEntityResourceSerializer and NestedEntitySerializer wrap objects inside a Resource before invoking the Jackson BeanSerializer to serialize. This wrapping causes the BeanSerializer to pass the wrong type info to the TypeSerializer, and so the wrong type name gets written. This is not too difficult to fix for simple objects, but for collections it is much more difficult. Jackson needs both the type info for each element in the collection as well as the type parameter of the collection to correctly invoke the TypeSerializer, and the way the PersistentEntityResourceSerializer wraps elements and collections in Resource and Resources prevents both kinds of type detection. Thus I've come to the conclusion that attempting a fix that works for scenario 1 would be very time consuming, and so a fix that addresses scenario 2 only is good enough.

I've created a commit that demonstrates how this fix addresses scenario 2 here: alexleigh@9b1c519
I haven't created a PR for this commit because my change differ from this PR only in the tests. Feel free to use my tests without needing to attribute me

[spring-projects-issues]

Oliver Drotbohm commented

I've applied and polished the commit you linked to and would like to resolve this one as fixed as there's now a way to use @JsonTypeInfo in the first place. Would you mind creating a new ticket and (optionally) a failing test case for the second case you outlined? I'll have a look at what we can do then

[spring-projects-issues]

Alex Leigh commented

Thank you for the fix! I created DATAREST-900 for the other use cases

[spring-projects-issues]

Mathias D commented

Thanks for the fix and clarification. I tested against the snapshot and can confirm that @JsonTypeInfo with include=JsonTypeInfo.As.EXISTING_PROPERTY is working now both for serialization and deserialization.

Added a branch to my original example that confirms it is fixed - https://github.com/mduesterhoeft/spring-data-rest-entity-inheritance/tree/fixed-hopper-sr3-snapshot