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

regression possibly due to AMQP-807 #1093

Closed
fdeguibert opened this issue Sep 13, 2019 · 0 comments · Fixed by #1096
Closed

regression possibly due to AMQP-807 #1093

fdeguibert opened this issue Sep 13, 2019 · 0 comments · Fixed by #1096
Assignees
Milestone

Comments

@fdeguibert
Copy link

fdeguibert commented Sep 13, 2019

As asked by @garyrussell, here is the Github issue related to stack overflow https://stackoverflow.com/questions/57927606/spring-amqp-jackson2jsonmessageconverter-setup-typeid-with-interface-instead

The problem

We have upgraded our Spring-boot version from 2.0.5 to 2.1.8.
As a result, Spring AMQP upgraded from 2.0.6 to 2.1.8 also.

Since then Jackson2JsonMessageConverter is unable to parse answer messages coming from methods annotated with @RabbitListener because they return an interface (actually declared as a generic in the code). And this interface is used to set the message _TypeId_ property .

with version 2.0 it used to set TypeId with the actual concrete class.

I did some digging and here is my understanding of the problem (code will follow)

When the method annotated with @RabbitListener returns, the MessagingMessageListenerAdapter#onMessage is invoked and encapsulates the result in a InvocationResult Object, which contains a genericType property.

This genericType is set up from the return type of the method annotated with @RabbitListener.

Then, it is used by the Jackson2JsonMessageConverter#createMessage method to setup _TypeId_ property.

On the other side, the Jackson2JsonMessageConverter#fromMessage can then parse the Json using this propery to find out the actual Type.

The problem is that, since the introduction of InvocationResult and genericType, our method annotated with @RabbitListener is declared as returning an interface and so the _TypeId_ property is set up with the interface instead of the actual concrete class. Here is the bit of code from Jackson2JsonMessageConverter#fromMessage (actualy from AbstractJackson2MessageConverter)which has changed (among other):

if (getClassMapper() == null) {
			getJavaTypeMapper().fromJavaType(this.objectMapper.constructType(
					genericType == null ? objectToConvert.getClass() : genericType), messageProperties);
		}
		else {
			getClassMapper().fromClass(objectToConvert.getClass(), messageProperties); // NOSONAR never null
		}

Since genericType is not null and contains the interfaceType... you can see our trouble.

Prior to version 2.1, we add no problem since the Jackson2JsonMessageConverter#createMessage() was always directly using objectToConvert.getClass():

		if (getClassMapper() == null) {
			getJavaTypeMapper().fromJavaType(this.jsonObjectMapper.constructType(objectToConvert.getClass()),
					messageProperties);

		}
		else {
			getClassMapper().fromClass(objectToConvert.getClass(),
					messageProperties);

		}

The code

Here is our code:

public abstract class AWorker<I extends RequestDtoInterface, O extends ResponseDtoInterface> {
   
    @RabbitListener(queues = "${rabbit.worker.queue}"
            , errorHandler = "workerErrorHandler"
            , returnExceptions = "true")
    public O receiveMessage(I inputMessage, @Header(LoggerUtil.LOGGER_MDC_ID) String mdcId, @Header(RabbitConstants.CONTEXT_INFO_HEADER_KEY) String contextInfoStr) {

       if(inputMessage instanceof RequestDtoFirstImplementation.class){
           return new ResponseDtoFirstImplementation();
       }else{
           return new ResponseDtoSecondImplementation();
       }
    }

}

Of course, the content of receiveMessage method is simplified, the point is that the actual implementation can return differents concretes types depending of the input concrete type

@fdeguibert fdeguibert changed the title regression possibly due to https://jira.spring.io/browse/AMQP-807 regression possibly due to AMQP-807 Sep 13, 2019
@garyrussell garyrussell added this to the 2.1.11 milestone Sep 16, 2019
@garyrussell garyrussell self-assigned this Sep 16, 2019
garyrussell added a commit to garyrussell/spring-amqp that referenced this issue Sep 16, 2019
Fixes spring-projects#1093

AMQP-807 added support for generic return types; however this broke abstract
return types since the `__TypeId__` header was set to the abstract type.

If the return type is not a container type and represents an abstract class
or interterface use the concrete return type to construct the `JavaType`.
garyrussell added a commit to garyrussell/spring-amqp that referenced this issue Sep 16, 2019
Fixes spring-projects#1093

AMQP-807 added support for generic return types; however this broke abstract
return types since the `__TypeId__` header was set to the abstract type.

If the return type is not a container type and represents an abstract class
or interterface use the concrete return type to construct the `JavaType`.

**cherry-pick to 2.1.x**
artembilan pushed a commit that referenced this issue Sep 16, 2019
Fixes #1093

AMQP-807 added support for generic return types; however this broke abstract
return types since the `__TypeId__` header was set to the abstract type.

If the return type is not a container type and represents an abstract class
or interterface use the concrete return type to construct the `JavaType`.

**cherry-pick to 2.1.x**
artembilan pushed a commit that referenced this issue Sep 16, 2019
Fixes #1093

AMQP-807 added support for generic return types; however this broke abstract
return types since the `__TypeId__` header was set to the abstract type.

If the return type is not a container type and represents an abstract class
or interterface use the concrete return type to construct the `JavaType`.

**cherry-pick to 2.1.x**

# Conflicts:
#	spring-amqp/src/main/java/org/springframework/amqp/support/converter/AbstractJackson2MessageConverter.java
garyrussell added a commit to garyrussell/spring-amqp that referenced this issue Sep 16, 2019
- redundant since interfaces are always abstract
artembilan pushed a commit that referenced this issue Sep 16, 2019
- redundant since interfaces are always abstract
artembilan pushed a commit that referenced this issue Sep 16, 2019
- redundant since interfaces are always abstract
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