OpenFeign doesn't work with kotlin suspend method

XhstormR commented 2 years ago

I have an API interface where the methods are kotlin suspend methods:

@FeignClient(path = "/api/masker", contextId = "MaskerApi", name = Const.SERVICE, configuration = [FeignConfig::class])
interface MaskerApi {

    @PostMapping("/mask")
    suspend fun mask(
        @Valid @RequestBody maskRequest: MaskRequest,
    ): RestResponse<List<CharSequence>>

    @PostMapping("/matches")
    suspend fun matches(
        @Valid @RequestBody maskRequest: MaskRequest,
    ): RestResponse<List<Boolean>>
}

But when I run the program, the program throws an exception:

Caused by: java.lang.IllegalStateException: Method has too many Body parameters: public abstract java.lang.Object io.github.xhstormr.masker.web.api.masker.MaskerApi.matches(io.github.xhstormr.masker.model.masker.MaskRequest,kotlin.coroutines.Continuation)
Warnings:
- 
    at feign.Util.checkState(Util.java:121) ~[feign-core-11.7.jar:na]
    at feign.Contract$BaseContract.parseAndValidateMetadata(Contract.java:142) ~[feign-core-11.7.jar:na]
    at org.springframework.cloud.openfeign.support.SpringMvcContract.parseAndValidateMetadata(SpringMvcContract.java:193) ~[spring-cloud-openfeign-core-3.1.0.jar:3.1.0]
    at feign.Contract$BaseContract.parseAndValidateMetadata(Contract.java:65) ~[feign-core-11.7.jar:na]
    at feign.ReflectiveFeign$ParseHandlersByName.apply(ReflectiveFeign.java:151) ~[feign-core-11.7.jar:na]
    at feign.ReflectiveFeign.newInstance(ReflectiveFeign.java:49) ~[feign-core-11.7.jar:na]
    at feign.Feign$Builder.target(Feign.java:268) ~[feign-core-11.7.jar:na]
    at org.springframework.cloud.openfeign.DefaultTargeter.target(DefaultTargeter.java:30) ~[spring-cloud-openfeign-core-3.1.0.jar:3.1.0]
    at org.springframework.cloud.openfeign.FeignClientFactoryBean.loadBalance(FeignClientFactoryBean.java:373) ~[spring-cloud-openfeign-core-3.1.0.jar:3.1.0]
    at org.springframework.cloud.openfeign.FeignClientFactoryBean.getTarget(FeignClientFactoryBean.java:421) ~[spring-cloud-openfeign-core-3.1.0.jar:3.1.0]
    at org.springframework.cloud.openfeign.FeignClientFactoryBean.getObject(FeignClientFactoryBean.java:396) ~[spring-cloud-openfeign-core-3.1.0.jar:3.1.0]
    at org.springframework.cloud.openfeign.FeignClientsRegistrar.lambda$registerFeignClient$0(FeignClientsRegistrar.java:235) ~[spring-cloud-openfeign-core-3.1.0.jar:3.1.0]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.obtainFromSupplier(AbstractAutowireCapableBeanFactory.java:1249) ~[spring-beans-5.3.14.jar:5.3.14]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1191) ~[spring-beans-5.3.14.jar:5.3.14]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:582) ~[spring-beans-5.3.14.jar:5.3.14]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:542) ~[spring-beans-5.3.14.jar:5.3.14]
    ... 28 common frames omitted

This kotlin.coroutines.Continuation parameter is generated by the kotlin compiler at compile time, so I can't modify this parameter, how can I tell FeignClient to ignore this Continuation parameter?

velo commented 2 years ago

The alternatives I see are:

we would need a feign-kotling module that would know how to handle this; or
create an abstract method without the suspension and a default method that includes suspension.

If the second alternative works, could you please contribute with an example project (like GitHub and Wikipedia examples)?

If doesn't, I would be keen to help on a kotlin module. I just don't know where to get started

XhstormR commented 2 years ago

I tried the second method, but I have a Controller class that inherits this interface and it doesn't work.

I finally customized a class that inherits SpringMvcContract:

class FeignIgnoreParameterContract(
    annotatedParameterProcessors: List<AnnotatedParameterProcessor>,
    conversionService: ConversionService,
    decodeSlash: Boolean
) : SpringMvcContract(
    annotatedParameterProcessors,
    conversionService,
    decodeSlash,
) {

    override fun processAnnotationsOnParameter(
        data: MethodMetadata,
        annotations: Array<out Annotation>,
        paramIndex: Int
    ): Boolean {
        if (data.method().parameterTypes[paramIndex] == Continuation::class.java) {
            data.ignoreParamater(paramIndex)
        }
        return super.processAnnotationsOnParameter(data, annotations, paramIndex)
    }
}

XhstormR commented 2 years ago

When I make a successful call to make an http request, but it fails when converting the http call result json into an object. When I remove the suspend modifier in the method, try again and it will be successful.

java.lang.IllegalStateException: Failed to execute ApplicationRunner
    at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:761) ~[spring-boot-2.6.2.jar:2.6.2]
    at org.springframework.boot.SpringApplication.callRunners(SpringApplication.java:748) ~[spring-boot-2.6.2.jar:2.6.2]
    at org.springframework.boot.SpringApplication.run(SpringApplication.java:309) ~[spring-boot-2.6.2.jar:2.6.2]
    at org.springframework.boot.SpringApplication.run(SpringApplication.java:1301) ~[spring-boot-2.6.2.jar:2.6.2]
    at org.springframework.boot.SpringApplication.run(SpringApplication.java:1290) ~[spring-boot-2.6.2.jar:2.6.2]
    at io.github.xhstormr.masker.ApplicationKt.main(Application.kt:54) ~[main/:na]
Caused by: java.lang.ClassCastException: class java.util.LinkedHashMap cannot be cast to class io.github.xhstormr.masker.model.response.RestResponse (java.util.LinkedHashMap is in module java.base of loader 'bootstrap'; io.github.xhstormr.masker.model.response.RestResponse is in unnamed module of loader 'app')
    at io.github.xhstormr.masker.web.api.masker.MaskerApi.maskSync(MaskerApi.kt:32) ~[main/:na]
    at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:710) ~[na:na]
    at feign.DefaultMethodHandler.invoke(DefaultMethodHandler.java:141) ~[feign-core-11.7.jar:na]
    at feign.ReflectiveFeign$FeignInvocationHandler.invoke(ReflectiveFeign.java:100) ~[feign-core-11.7.jar:na]
    at com.sun.proxy.$Proxy116.maskSync(Unknown Source) ~[na:na]
    at io.github.xhstormr.masker.Application.init$lambda-0(Application.kt:45) ~[main/:na]
    at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:758) ~[spring-boot-2.6.2.jar:2.6.2]
    ... 5 common frames omitted

tmdgusya commented 2 years ago

@XhstormR @velo

Is that correct you want? i did using feign with coroutines using kotlin default library and do not any setting

if correct that below example code, i wish i write example project "feign-with-kotlin-coroutines"

https://github.com/tmdgusya/feign-with-coroutines/blob/master/src/main/kotlin/com/roach/feignwithkotlinexample/TodoController.kt

XhstormR commented 2 years ago

@tmdgusya Thanks for your example. I looked at the code and my thought is https://github.com/tmdgusya/feign-with-coroutines/blob/master/src/main/kotlin/com/roach/feignwithkotlinexample/JsonPlaceHolderClients.kt#L14 method should have suspend modifier, so that TodoController can directly implement JsonPlaceHolderClients.

tmdgusya commented 2 years ago

@XhstormR thanks, i think that it's clients api call executed with coroutine context but I may be wrong. plz see below README.md

https://github.com/tmdgusya/feign-with-coroutines

XhstormR commented 2 years ago

@tmdgusya I see what you mean. The problem with this issue is to make FeignClient annotated classes to resolve suspend method calls.

tmdgusya commented 2 years ago

@XhstormR Thanks to your explanation. I have a one question. What's difference suspend method in interface and my code's(using withContext) I just wonder what the difference is.

tmdgusya commented 2 years ago

@XhstormR i totally knew that. it's bigger difference suspend method in feignClients interfcae and suspend method in own's implements code.

my example code is using coroutines but not suspend function! thanks. i got new knowledge in conversation

tmdgusya commented 2 years ago

Maybe, i think only one way that solving this problem that is implementation to Continuation

velo commented 2 years ago

@XhstormR @velo

Is that correct you want? i did using feign with coroutines using kotlin default library and do not any setting

if correct that below example code, i wish i write example project "feign-with-kotlin-coroutines"

https://github.com/tmdgusya/feign-with-coroutines/blob/master/src/main/kotlin/com/roach/feignwithkotlinexample/TodoController.kt

That would be almost it...

Could it be a non-springboot project and with a unit test for the expected behavior. ideally (not sure if that is possible or not) unit test written in java instead of kotlin.

Also, would be much easier for me to pick up if was a maven project =)

ArtemenkoAndrii commented 2 years ago

Hello guys,

Are there any updates on this, please? I'm also the one who wants to use Feign with coroutines but doesn't see clearway how exactly.

XhstormR commented 2 years ago

Currently I have no good solution.

velo commented 2 years ago

@ArtemenkoAndrii @XhstormR

Hi, I'm willing and able to help, but I have a) ZERO kotlin know-how and b) almost no free time to dedicate to learn it c) I don't have a compelling reason today

If anyone really want to see this fixed I need the following:

A pull request to feign, with a new module feign-kotlin
This module must contain a unit test demonstrating the issue
Must be a maven project
NO springboot, spring, springlite, springish, no "helpful" framework
As much as possible, JAVA code, split testing into multiple classes if needed
CI must fail, as part of test demonstrating the problem
OPTIONAL Cherry on top would be a project that is tested to work with Eclipse =)

As most I appreciate the example provided by @XhstormR , that is too far off my comfort zone

ArtemenkoAndrii commented 2 years ago

Well, not sure if I will have enough time for this. I briefly checked the sources and to me, it doesn't look a simple task. Effectively the method signature should expect to receive one extra parameter with kotlin.coroutines.Continuation type. The resumeWith(Result) should be called when a response is ready.

But I don't think that in itself will give any benefits except syntactic - the calling thread is still to be blocked. As I understand all underlying implementations(okhttp etc) use their own pools so the returning of CompletableFuture will make more sense. The kotlinx-coroutines-jdk has embedded adapters to handle it.

velo commented 2 years ago

Well, not sure if I will have enough time for this

Ok

OpenFeign / feign

OpenFeign doesn't work with kotlin suspend method #1565