apollo-kotlin icon indicating copy to clipboard operation
apollo-kotlin copied to clipboard

Apollo-java runtime explorations

Open BoD opened this issue 3 years ago • 3 comments

In this exploration, an apollo-java module is added.

In this approach the module depends on apollo-runtime (but implementation only so it doesn't "leak")

  • pros: avoid re-writing some amount of code (and keep future changes in sync)
  • cons: we pull kotlin and coroutines dependencies

The code is mostly a bunch of facades around Kotlin code, using callbacks instead of coroutines/Flows. Sometimes a blocking version also exists, e.g. ApolloCall.executeBlocking (naming to be discussed!).

The facades are written in Java when possible:

  • avoids a buch of IntelliJ bugs (e.g. KTIJ-22127) related to usage of generics in Kotlin dependencies
  • nicer for users who navigate to the source to see a familiar language (although they won't see anything interesting, and will see Kotlin when going deeper)

The module also depends on apollo-normalized-cache-sqlite:

  • pros: cache-specific extensions on builders can be used in a "natural" way
  • cons:
    • dependency on cache even when not used
    • less modular (e.g. can't depend on incubating instead)
    • can't use implementation here because we want e.g. FetchPolicy to be visible. So now we expose 2 ApolloStore, you need to import the right one 🙁 (but maybe we could "facade" more things to avoid that)

Lots of things are missing (e.g. Interceptors)!

Misc open questions:

  • for one-off calls (e.g. execute) should we look into a Future based API instead of a specific callback?
  • for Flow like calls (e.g. watch) should we do something with Rx (like we do now with adapters), or are specific callbacks good / preferrable? Should we look into Java's Flow (never used it, didn't even know this existed until recently)

Related to #3694.

BoD avatar Aug 24 '22 14:08 BoD

Deploy Preview for apollo-android-docs canceled.

Name Link
Latest commit 617024202bebbc56d74ccf739203b50b9bae9801
Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/63063a7ac05237000891c154

netlify[bot] avatar Aug 24 '22 14:08 netlify[bot]

A bunch of early comments/thoughts

In this approach the module depends on apollo-runtime (but implementation only so it doesn't "leak") pros: avoid re-writing some amount of code (and keep future changes in sync) cons: we pull kotlin and coroutines dependencies

My gut feeling there is that there is not much logic in apollo-runtime and keeping it around would add a lot of friction (in addition to the added dependency)

Lots of things are missing (e.g. Interceptors)!

That's what I'm talking about. Designing a callback based API for interceptor, passing a non-suspend HttpEngine, having builders work without extension functions, etc... is going to be a lot of working around basic stuff...

Sometimes a blocking version also exists, e.g. ApolloCall.executeBlocking (naming to be discussed!).

Do we need blocking?

for Flow like calls (e.g. watch) should we do something with Rx (like we do now with adapters)

I think RxJava adapters make a lot of sense. Callback are widely compatible, RxJava/WebFlux/Flow/etc... is for everything else (including blocking calls like above) so we'd be better duplicating it.

The facades are written in Java when possible:

Yay for Java 👍

The module also depends on apollo-normalized-cache-sqlite pros: cache-specific extensions on builders can be used in a "natural" way cons: dependency on cache even when not used less modular (e.g. can't depend on incubating instead)

That's a pain but without extension functions, I don't think we can do without this. My 2 cents here would be to design apollo-runtime-java without cache at first. Since it's mostly going to be used on the backend, I'm not sure how much caching is required there.

can't use implementation here because we want e.g. FetchPolicy to be visible. So now we expose 2 ApolloStore, you need to import the right one 🙁 (but maybe we could "facade" more things to avoid that)

If we decide we need a client cache for Java, we duplicate ApolloStore also. The only thing that stays is apollo-api everything else is duplicated. I know this duplicates some code but TBH, it's mostly glue that I don't think contains that much logic.

martinbonnin avatar Aug 24 '22 20:08 martinbonnin

A bit of follow-up: I tend to agree with all of this :). With this "front-end" approach, the only benefit is to avoid rewriting some components (+ having to "backport" future fixes and features). But if the code in question is not that big / complex (especially true if we skip the cache for now) rewriting is not a huge deal. Writing façades/wrappers is also far from ideal, it's a lot of boilerplate, and as a Java developer it's not nice to see Kotlin when browsing the sources. And there's the coroutines dependency + odd stacktraces in exceptions.

BoD avatar Aug 26 '22 13:08 BoD

Closing for now, as we're going with the other approach of rewriting (instead of writing wrappers) for the new runtime

BoD avatar Oct 04 '22 08:10 BoD