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

Jackson Message Conversion - Allow Abstract Classes #1215

Closed
ryandanielspmc opened this issue Jun 23, 2020 · 4 comments · Fixed by #1216
Closed

Jackson Message Conversion - Allow Abstract Classes #1215

ryandanielspmc opened this issue Jun 23, 2020 · 4 comments · Fixed by #1216

Comments

@ryandanielspmc
Copy link

ryandanielspmc commented Jun 23, 2020

I am using Kotlin, and I have a sealed class as follows:

@JsonDeserialize(using = FooDeserializer::class)
sealed class Foo
data class FooA(val s: String) : Foo()
data class FooB(val l: Long) : Foo()

Then FooDeserializer handles determining whether to instantiate FooA or FooB. The Jackson ObjectMapper handles this situation no problem. But, the message conversion functionality around Jackson2JsonMessageConverter does not allow abstract classes, which is what Foo gets compiled to.

Please allow abstract classes and interfaces in the Jackson message conversion functionality. Let it be the responsibility of the Jackson ObjectMapper to handle this situation instead of preventing it.

@garyrussell
Copy link
Contributor

We will have to give this some thought; simply relaxing the the check could break many applications; currently, if the @RabbitListener parameter is abstract, we fall back to using type information in headers. so we probably need some option.

@garyrussell
Copy link
Contributor

Can you provide more information? I just tested it with this code and it worked fine...

	@Test
	void customAbstractClass() {
		byte[] bytes = "{\"field\" : \"foo\" }".getBytes();
		MessageProperties messageProperties = new MessageProperties();
		messageProperties.setInferredArgumentType(Baz.class);
		Message message = new Message(bytes, messageProperties);
		ObjectMapper mapper = new ObjectMapper();
		mapper.registerModule(new BazModule());
		Jackson2JsonMessageConverter j2Converter = new Jackson2JsonMessageConverter(mapper);
		Baz baz = (Baz) j2Converter.fromMessage(message);
		assertThat(((Qux) baz).getField()).isEqualTo("foo");
	}

	public interface Baz {

	}

	public static class Qux implements Baz {

		private String field;

		public Qux(String field) {
			this.field = field;
		}

		public String getField() {
			return this.field;
		}

		public void setField(String field) {
			this.field = field;
		}

	}

	public static class BazDeserializer extends StdDeserializer {

		public BazDeserializer() {
			super(Baz.class);
		}

		@Override
		public Object deserialize(JsonParser p, DeserializationContext ctxt)
				throws IOException, JsonProcessingException {

			p.nextFieldName();
			return new Qux(p.nextTextValue());

		}

	}

	public static class BazModule extends SimpleModule {

		public BazModule() {
			addDeserializer(Baz.class, new BazDeserializer());
		}

	}

It works because of this logic...

	@Override
	public JavaType toJavaType(MessageProperties properties) {
		JavaType inferredType = getInferredType(properties);
		if (inferredType != null
			 && ((!inferredType.isAbstract() && !inferredType.isInterface()
					|| inferredType.getRawClass().getPackage().getName().startsWith("java.util")))) {
			return inferredType; // we don't return here because Baz is abstract
		}

		String typeIdHeader = retrieveHeaderAsString(properties, getClassIdFieldName());

		if (typeIdHeader != null) {
			return fromTypeHeader(properties, typeIdHeader);
		}

		if (hasInferredTypeHeader(properties)) {
			return fromInferredTypeHeader(properties); // this is the result because there are no type headers
		}

		return TypeFactory.defaultInstance().constructType(Object.class);
	}

I can see it would no work if type headers exist with incorrect information. Is that your case?

@ryandanielspmc
Copy link
Author

That is correct. My situation is that the message comes from a different application. And that application has its own domain classes that are used to serialize the message, then the fully qualified domain class from that application is in the header. Then, on the receiving application, when the @RabbitListener method parameter is an abstract class, it falls back to the type in the header, which is not in the receiving application, and give an error.

@garyrussell
Copy link
Contributor

OK; thanks. I think we can handle it by adding objectMapper.canDeserialize() on the inferred method parameter type.

I just need to figure out the best place to do that test without breaking anything...

@garyrussell garyrussell self-assigned this Jun 23, 2020
garyrussell added a commit to garyrussell/spring-amqp that referenced this issue Jun 23, 2020
Resolves spring-projects#1215

Previously, the message converter would fall back to header type info
if the inferred type was abstract.

Furthermore, we did not examine container type content being abstract.

With a custom deserializer, abstract classes can be deserialized.
artembilan pushed a commit that referenced this issue Jun 23, 2020
Resolves #1215

Previously, the message converter would fall back to header type info
if the inferred type was abstract.

Furthermore, we did not examine container type content being abstract.

With a custom deserializer, abstract classes can be deserialized.
artembilan pushed a commit that referenced this issue Jun 23, 2020
Resolves #1215

Previously, the message converter would fall back to header type info
if the inferred type was abstract.

Furthermore, we did not examine container type content being abstract.

With a custom deserializer, abstract classes can be deserialized.

# Conflicts:
#	src/reference/asciidoc/whats-new.adoc
garyrussell added a commit that referenced this issue Jun 24, 2020
Method complexity.
garyrussell added a commit to garyrussell/spring-amqp that referenced this issue Dec 7, 2020
Resolves spring-projects#1279

Regression: spring-projects#1215

Test for abstract class should not be applied to container types, which can
be abstract.
garyrussell added a commit to garyrussell/spring-amqp that referenced this issue Dec 7, 2020
Resolves spring-projects#1279

Regression: spring-projects#1215

Test for abstract class should not be applied to container types, which can
be abstract.

**cherry-pick to 2.2.x**
artembilan pushed a commit that referenced this issue Dec 7, 2020
Resolves #1279

Regression: #1215

Test for abstract class should not be applied to container types, which can
be abstract.

**cherry-pick to 2.2.x**
artembilan pushed a commit that referenced this issue Dec 7, 2020
Resolves #1279

Regression: #1215

Test for abstract class should not be applied to container types, which can
be abstract.

**cherry-pick to 2.2.x**
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants