spotify / github-java-client

A Java client to Github API
Apache License 2.0
129 stars 81 forks source link

Bug: Accessing shaded OkHttpClient properties from Kotlin fails with `NoSuchMethodException` #179

Open vootelerotov opened 7 months ago

vootelerotov commented 7 months ago

Issue

Trying to access dispatcher property of the shaded OkHttpClient from Kotlin results in

 java.lang.NoSuchMethodError: 'com.spotify.githubclient.shade.okhttp3.Dispatcher com.spotify.githubclient.shade.okhttp3.OkHttpClient.getDispatcher()

See reproducing test here.

Note that this also affects other fields in OkHttpClient.

Possible cause

In #157, the okhttp package was bumped from 3.x to 4.x.

In version 4.x of okhttp, the way Kotlin access to fields of OkHttpClient works was reworked for Kotlin, the direct access from Kotlin to the dispatcher method was deprecated:

  @JvmName("-deprecated_dispatcher")
  @Deprecated(
    message = "moved to val",
    replaceWith = ReplaceWith(expression = "dispatcher"),
    level = DeprecationLevel.ERROR,
  )
  fun dispatcher(): Dispatcher = dispatcher

Instead, the getter, that Kotlin generates for property access, was renamed to dispatcher:

  @get:JvmName("dispatcher")
  val dispatcher: Dispatcher = builder.dispatcher

See here for the class.

From Java point of view, nothing functionally changed -- previously dispatch() hit the manually added method, now dispatch() hit the getter generated by Kotlin with the same name.

However, from Kotlin, accessing dispatcher with dispatcher() is now a compile time error and instead one should use the property access syntax (okHttpClient.disptacher). This is exemplified in the reproducing test here.

In this project, the OkHttpClient is shaded into a different package. And I am speculating that the migration of packages does not cover all the metadata that Kotlin compiles into the class file. What seems to go missing is the information that dispatcher should be accessed by a method called dispatcher, instead of using the default name for property getter what would be getDispatcher -- this would explain the exception thrown.

In the reproducing test class, there is also one test that demonstrates that accessing metainfo about the property getter via kotlin-reflect does not succeed. This could also be explained by incomplete migration of packages.

Workaround

A Java class in (otherwise) Kotlin project, with static methods along the lines of:

    public static ExecutorService getExecutorService(Dispatcher dispatcher) {
        return dispatcher.executorService();
    }
vootelerotov commented 6 months ago

As extra context, I do not really expect this to get fixed, unless there is an easy way to do it that I am missing.

But perhaps there is room for documenting this issue, and the possible workaround, somewhere. It was not trivial to figure out, might save some time for someone.

Abhi347 commented 3 months ago

Hey, apologies for delay int he response. We do not support Kotlin at this moment, so I don't think this will get priortised anytime soon.
You are free to create a PR though.