ea-async copying annotations to instrument-generated methods

[HaloFour]

I'm testing ea-async with a Spring 5 application I am working on. This is a part of a tech spike/evaluation for implementing asynchronous programming throughout our team. What I've noticed is that the ea-async instrumentation is copying the annotations of the instrumented method to the generated methods and in some cases this is causing problems with Spring.

For example, take the following controller:

@RestController
public class HelloWorldController {

    @Inject
    private AsyncService service;

    @RequestMapping("/hello")
    public CompletionStage<String> get() {
        CompletionStage<String> stage1 = service.getGreetingAsync();
        CompletionStage<String> stage2 = service.getGreetingAsync();
        String result1 = await(stage1);
        String result2 = await(stage2);

        return CompletableFuture.completedFuture(result1 + result2);
    }
}

The instrumentation rewrites this into:

@RestController
public class HelloWorldController {
    @Inject
    private AsyncService service;

    public HelloWorldController() {
    }

    @RequestMapping({"/hello"})
    public CompletionStage<String> get() { /* elided */ }

    @RequestMapping({"/hello"})
    private static CompletableFuture async$get(HelloWorldController var0, CompletionStage stage1, CompletionStage stage2, CompletionStage var3, String result2, int var5, Object var6) { /* elided */ }
}

This causes Spring to fail mapping the controller with the following message:

Caused by: java.lang.IllegalStateException: Ambiguous mapping. Cannot map 'helloWorldController' method 
private static java.util.concurrent.CompletableFuture com.comcast.controller.HelloWorldController.async$get(com.comcast.controller.HelloWorldController,java.util.concurrent.CompletionStage,java.util.concurrent.CompletionStage,java.util.concurrent.CompletionStage,java.lang.String,int,java.lang.Object)
to {[/hello]}: There is already 'helloWorldController' bean method

This can be worked around by having only using Async.await in non-annotated methods, however my concern is that this will make other folks on my team more apprehensive about adopting ea-async.

[JoeHegarty]

This one is interesting because I can see an argument as to why you'd want the behavior both ways depending on the use case. I think I'd err on the side of not duplicating the annotations as in most cases that feels like the right thing to do.

Quite why they are copied isn't immediately obvious from a quick glance at the code so I'd need to take a closer look.

[bvella]

@JoeHegarty you probably need to override visitAnnotation, visitParameterAnnotation and visitTypeAnnotation in MyMethodVisitor and ignore the visit when applying to the continuation method. That way, the continuation method will not have the same annotations.

It might also be a good idea to mark the continuation method as synthetic, so that libraries that scan class methods can skip the continuation method. I'll try to submit a pull request if I find some time

[JoeHegarty]

This should be resolved with #31 as generated methods are now synthetic and will be ignored by most tools.

[DidierLoiseau]

We are looking into using EA Async in our code, but according to our tests, this issue is not fixed by #31. In fact, I couldn't find anything indicating that Spring should ignore synthetic methods (I think Spring only tries to resolve bridge methods, for generics).

@JoeHegarty, could you reopen this issue?

[DidierLoiseau]

Here is a MCVE that reproduces the problem, hope this helps:

src/main/java/eaasync/test/EAAsyncSpringApplication.java:

package eaasync.test;

import java.util.concurrent.CompletableFuture;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;

import com.ea.async.Async;

@SpringBootApplication
public class EAAsyncSpringApplication {
    public static void main(String[] args) {
        Async.init();
        SpringApplication.run(EAAsyncSpringApplication.class, args);
    }
}

@RestController
class SimpleController {
    @GetMapping(params = "/test")
    public CompletableFuture<String> getTest() {
        return CompletableFuture.completedFuture(Async.await(getData()).toString());
    }

    private CompletableFuture<Integer> getData() {
        throw new UnsupportedOperationException("not implemented");
    }
}

pom.xml:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <parent>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-parent</artifactId>
        <version>1.5.14.RELEASE</version>
    </parent>

    <groupId>eaasync.test</groupId>
    <artifactId>tests</artifactId>
    <version>1.0-SNAPSHOT</version>
    <build>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-compiler-plugin</artifactId>
                <version>LATEST</version>
                <configuration>
                    <source>1.8</source>
                    <target>1.8</target>
                </configuration>
            </plugin>
        </plugins>
    </build>
    <dependencies>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-web</artifactId>
        </dependency>
        <dependency>
            <groupId>com.ea.async</groupId>
            <artifactId>ea-async</artifactId>
            <version>1.2.2</version>
        </dependency>
    </dependencies>
</project>