Skip to content

AsyncExecution fails to detect the return type of an annotated method from an interface with a generic - #33957

Closed
PaulNgo-BlueOC wants to merge 2 commits into
spring-projects:mainfrom
PaulNgo-BlueOC:fix-async-wrong-return-type-in-case-of-abstraction
Closed

AsyncExecution fails to detect the return type of an annotated method from an interface with a generic#33957
PaulNgo-BlueOC wants to merge 2 commits into
spring-projects:mainfrom
PaulNgo-BlueOC:fix-async-wrong-return-type-in-case-of-abstraction

Conversation

@PaulNgo-BlueOC

@PaulNgo-BlueOC PaulNgo-BlueOC commented Nov 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

The last param of AsyncExecutionAspectSupport.doSubmit is a return type, which can be get from the invocation. But in case of interface with paramter type like:

interface Service<O> {
  O doSomething();
}

class DefaultAsyncService implements Service<Future<String>> {
  @Override
  @Async
  public Future<String> doSomething() {
   return CompletableFuture.abc();
  }
}

@RestController
class Controller {
  @Autowire
  Service<Future<String>> service;

  @GetMapping
  public Future<String> api() {
    return service.doSomething();
  }
}

Return type retuned by invocation.getMethod().getReturnType() will be Object because we invoke doSomething by the interface, not by the impl.
My PR try to fix that

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 25, 2024
@snicoll

snicoll commented Nov 26, 2024

Copy link
Copy Markdown
Member

Can you please add a unit test that exercise the scenario you've described?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Nov 26, 2024
@PaulNgo-BlueOC

PaulNgo-BlueOC commented Nov 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Can you please add a unit test that exercise the scenario you've described?

Hi @snicoll , you need a unit test which cover the fix/changes, or the unit test to reproduce the issue?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 27, 2024
@snicoll

snicoll commented Nov 27, 2024

Copy link
Copy Markdown
Member

Isn't that the same thing? I'd like that we see the change covered by an actual test that shows the behavior you've described is now working.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Nov 27, 2024
@PaulNgo-BlueOC

PaulNgo-BlueOC commented Nov 27, 2024

Copy link
Copy Markdown
Contributor Author

Isn't that the same thing? I'd like that we see the change covered by an actual test that shows the behavior you've described is now working.

Ok I will add it soon. Thank you

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 27, 2024
@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Nov 27, 2024
@PaulNgo-BlueOC PaulNgo-BlueOC changed the title Fix wrong return type when invoking Async-annotated Fix wrong return type when invoking Async-annotated method on interface Nov 27, 2024
@PaulNgo-BlueOC

PaulNgo-BlueOC commented Nov 28, 2024

Copy link
Copy Markdown
Contributor Author

I added unit test

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 28, 2024
@snicoll
snicoll requested a review from jhoeller December 4, 2024 11:51
@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 3, 2025
@jhoeller

jhoeller commented Feb 4, 2025

Copy link
Copy Markdown
Contributor

Could we simply always use userMethod.getReturnType() instead of invocation.getMethod().getReturnType()?

@snicoll snicoll self-assigned this Feb 4, 2025
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Feb 4, 2025
@snicoll snicoll added this to the 6.2.3 milestone Feb 4, 2025
@snicoll snicoll changed the title Fix wrong return type when invoking Async-annotated method on interface AsyncExecution fails to detect the return type of an annotated method from an interface with a generic Feb 4, 2025
snicoll pushed a commit that referenced this pull request Feb 4, 2025
@baongoaeris @snicoll
Use proper return type in AsyncExecutionInterceptor
8eb2445 
See gh-33957
snicoll added a commit that referenced this pull request Feb 4, 2025
@snicoll
Polish "Use proper return type in AsyncExecutionInterceptor"
3923150 
See gh-33957
@snicoll snicoll closed this in a4d99d6 Feb 4, 2025
@snicoll

snicoll commented Feb 4, 2025

Copy link
Copy Markdown
Member

@anaconda874 thanks very much for making your first contribution to Spring Framework.

@PaulNgo-BlueOC

This comment was marked as resolved.

Sign in to view
@snicoll

This comment was marked as resolved.

Sign in to view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhoeller jhoeller Awaiting requested review from jhoeller

Assignees

@snicoll snicoll

Labels

in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug

Projects

None yet

Milestone

6.2.3

Development

Successfully merging this pull request may close these issues.

6 participants

@PaulNgo-BlueOC @snicoll @jhoeller @rstoyanchev @spring-projects-issues @baongoaeris