Support kotlin coroutines

[wplong11]

Resolves: #1565

!! This is working PoC

Inspired by https://github.com/PlaytikaOSS/feign-reactive/pull/486

TODO

• [ ] Separate Kotlin support module
• [ ] Enhance test case
• [ ] Refactoring
• [ ] Clean up pom.xml

[velo]

This looks really nice. It makes total sense to make this available on the AsyncFeign only, really good plan!

snyk is asking to "Upgrade org.jetbrains.kotlin:kotlin-stdlib to version 1.6.0 or higher"

Sounds like a solid plan, once you are ready to move to a new module, fee free to ping me for any assistance needed

[wplong11]

@velo I pushed some changes

1. Refactor some code
   • Use FutureKt.await instead of KotlinExtensions.awaitResponse
   • Renamed KotlinExtensions to MethodKt
2. Update the Kotlin version
   • If the Feign library is using the latest version, and the Kotlin version of the project using the Feign library is lower, is it safe? I'm going to do some research on this, but I'm asking to check if you have any knowledge to share with me.

[wplong11]

@velo ~I'm looking into other libraries to isolate the Kotlin support module so that the core module doesn't reference the Kotlin library, but Spring is using Kotlin in the core module. How important do you think the necessity is with regard to module separation?~

I came up with a good idea. Looking at the Spring code a little more, it seems that I am handling it smarter than I thought. I'll look into it a bit more and update.

[kdavisk6]

I’m so happy someone picked this up. @velo could I have a day or two to review this as well?

[velo]

I’m so happy someone picked this up. @velo could I have a day or two to review this as well?

You absolutely can sir!

But, until this is moved to a module, I wouldn't be comfortable to merge, as core needs to be lean and have no dependencies.

[kdavisk6]

I’m so happy someone picked this up. @velo could I have a day or two to review this as well?

You absolutely can sir!

But, until this is moved to a module, I wouldn't be comfortable to merge, as core needs to be lean and have no dependencies.

Exactly why I wanted to review. Like minds

[wplong11]

@velo

But, until this is moved to a module, I wouldn't be comfortable to merge, as core needs to be lean and have no dependencies.

I'm considering splitting modules using Optional Dependencies. What do you think about this approach? After a brief investigation, it seems that Spring Core also supports Kotlin using Optional Dependencies. I'm going to try implementing it today using optional dependencies, and investigate further if there are other approaches. (I live in the KST time zone (UTC+9), and I can write code after work.)

Optional Dependencies

Optional dependencies are used when it's not possible (for whatever reason) to split a project into sub-modules. The idea is that some of the dependencies are only used for certain features in the project and will not be needed if that feature isn't used. Ideally, such a feature would be split into a sub-module that depends on the core functionality project. This new subproject would have only non-optional dependencies, since you'd need them all if you decided to use the subproject's functionality.

However, since the project cannot be split up (again, for whatever reason), these dependencies are declared optional. If a user wants to use functionality related to an optional dependency, they have to redeclare that optional dependency in their own project. This is not the clearest way to handle this situation, but both optional dependencies and dependency exclusions are stop-gap solutions.

[velo]

I don't think optional is going to cut. But tell you what, get your changes to a place where you are happy with, and I can jump in and help refactoring the core to make a module possible

[wplong11]

@velo I pushed some changes to share idea.

• Because the optional dependency is used, Kotlin support related libraries will not be referenced in a Java-only project. 407ab47
• Assert.java / ClasUtils.java / KotlinDetector.java is from spring core project -> I will refactor when the implementation approach is agreed upon.
• As an additional idea, I thought of separating the code related to Kotlin support into a separate module and referencing that module as an Optional Dependency from the core module. 883f036

[velo]

Hi @wplong11 , so, as is, we can't merge, as we (Myself and @kdavisk6 ) won't approve any changes that introduce dependencies to feign core.... optional=>true is still a dependency, and that is not going to fly. Core needs ZERO dependencies.

My plan is to refactor feign-core to allow for the new bits you created to be attached to core like we do with everything else. Like, feign supports "jackson" but there is no jackson dependency on core. Could you imagine the caos it would bring to everybody if overnight feign-core included jackson, json, http commons, apache http5 and more and more

So, once you get your changes to a point that you are happy with, ping me and I will help you out.

[wplong11]

@velo Looking at the final result, feign-core depends on feign-kotlin as an optional dependency. I think it's a bit better than relying on kotlin-related dependencies directly, but as you said, I'd also like to have zero dependencies if possible. But I have no idea how to implement it. Do you have any ideas?

Considerations

• People using feign only depend on feign-kotlin and do not require additional code.

[velo]

Do you have any ideas?

Not yet. But, I will get something done

• People using feign only depend on feign-kotlin and do not require additional code.

Not really, it will bring in kotlin and whatever else it depends on. As is, you can't use feign if kotlin library is not included.

[wplong11]

@velo Looking at the Github Example project, it seems that feign-core is used with zero dependency. Am I misunderstanding something?

[velo]

either that dependency tree wrong, or a runtime exception will happen when performing an async operation

[wplong11]

@velo

either that dependency tree wrong, or a runtime exception will happen when performing an async operation

I checked that Github Example in Feign project runs correctly without kotlin related modules.

According to Maven document,

Project-A -> Project-B The diagram above says that Project-A depends on Project-B. When A declares B as an optional dependency in its POM, this relationship remains unchanged. It's just like a normal build where Project-B will be added in Project-A's classpath.

Project-X -> Project-A When another project (Project-X) declares Project-A as a dependency in its POM, the optional nature of the dependency takes effect. Project-B is not included in the classpath of Project-X. You need to declare it directly in the POM of Project X for B to be included in X's classpath.

Kotlin related modules is not included in the classpath of Github Example. Is there a difference between not being included on the classpath and excluded from the dependency tree?

+) Even though Spring Core uses kotlin related modules as optional dependency, It seems that all java only projects also run correctly without any kotlin related dependency

[velo]

@wplong11 I moved kotlin to a module, but couldn't get the tests working.... could you take a look, I must admit I don't know where to start

[wplong11]

Thanks for the additional work. I've been too busy lately. I'll be able to start working again in two weeks. I'll finish this PR soon.

[velo]

Awesome, and thanks for this massive contribution.

I have o my mind now, that I need to find a way to have simpler Feign#builder classes..

[wplong11]

Branch updated to latest state

$ git rebase master && git push --force

[wplong11]

1. This Branch updated to latest master branch

   $ git rebase master && git push --force
   

2. Add a single commit 78589d5