Skip to content

feat!: Kotlin Multiplatform support #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

bencehornak
Copy link

@bencehornak bencehornak commented May 17, 2025

I was curious how much effort it would be to extend this SDK to multiple KMP platforms. It was surprisingly straight-forward, there are only a few references to the JDK in the project.

This PR

  • Converts the project from an Android project to a Kotlin Multiplatform one with the following targets:
    • android
    • jvm
    • linuxX64 (probably this is probably quite niche, we can remove this to not overcommit to new platforms)
    • js (browser & nodejs)
    • ... (iOS?)
  • Replace dependency on java.util.Date with kotlin.time.Instant and rename class accordingly from Value.Date to Value.Instant (breaking change)
  • Changes the Maven artifact ID from dev.openfeature:android-sdk to dev.openfeature:kotlin-sdk (breaking change)
  • Changes the base package from dev.openfeature.sdk to dev.openfeature.kotlin.sdk in order to eliminate the conflict with the Java SDK (breaking change)

Related Issues

Fixes #147

To do

  • Discuss which platforms should be supported -> I'd start with a few platforms, later more can be added, see my recommendation for supported platforms
  • CI infrastructure for each supported platform
  • Some providers should be available for them to make this library useful -> feat!: Bootstrap project and implement env-var provider kotlin-sdk-contrib#1
  • Change artifact id from android-sdk to kotlin-sdk
  • Change docs page title from Android to Kotlin (the URL already says /kotlin)
  • Document supported platforms
  • Rename base package from dev.openfeature.sdk to dev.openfeature.kotlin.sdk
  • Add a multiplatform example app -> not that simple as I thought, would dedicate a separate PR for this

How to test

Prerequisite for browser tests: Install Google Chrome (or Google Chrome headless)

To run tests on all supported platforms:

./gradlew allTests

@bencehornak bencehornak changed the title Kotlin Multiplatform PoC feat: Kotlin Multiplatform support May 17, 2025
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
@bencehornak bencehornak force-pushed the kotlin-multiplatform branch from 62a5911 to eb1c6b1 Compare May 17, 2025 15:24
@thomaspoignant
Copy link
Member

Hey thanks for this, I will look at it as soon as I have a bit of time.
When it comes to server, the dynamic paradigm is really different and it requires a different sdk.

I have played myself in kotlin with the java server sdk and it works well.

Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Copy link

@realdadfish realdadfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nicklasl nicklasl changed the title feat: Kotlin Multiplatform support feat!: Kotlin Multiplatform support May 21, 2025
@nicklasl
Copy link
Member

Some providers should be available for them to make this library useful

Trying to wrap my brains around this... This wouldn't just work with any js provider, right? It has to be a provider that is written to work with this SDK, a provider that is also multiplatform.

@bencehornak
Copy link
Author

Some providers should be available for them to make this library useful

Trying to wrap my brains around this... This wouldn't just work with any js provider, right? It has to be a provider that is written to work with this SDK, a provider that is also multiplatform.

Actually, it is technically possible to create wrappers around the java and js providers thanks to Kotlin's interop capabilities. The wrapper would proxy calls to the underlying implementation, and would convert back the return values and exceptions to KMP values/exceptions. I have seen something similar in action for OpenTelemetry in the Embrace repo I referred to on Slack in the thread we talked about this topic.

https://cloud-native.slack.com/archives/C080WHYDG5V/p1747593332038039?thread_ts=1747593332.038039&cid=C080WHYDG5V

As I mentioned on Slack, the current roadblock for such Java wrapper is a) the collision of package names and b) the static vs dynamic context paradigm issue you mentioned on Slack.

a) we can easily solve in this PR even, if it's not too many breaking changes for a single PR. Regarding b) my thoughts are that it's incorrect to categorize Java as a server language and Kotlin as a client one, given that both can be used for both purposes. I'd much rather have Providers for both languages that support both modes of operation.

But this is maybe a bit off-topic for this PR, I suggest that we switch to the Slack thread to discuss the question of the wrapper.

@lukas-reining
Copy link
Member

lukas-reining commented May 22, 2025

This is a cool PR! :)

To me, it makes sense to make clear that the Kotlin and also the Swift are implemented as client (static context) SDKs.
With Javascript we decided, that it makes more sense to have two SDKs for the two paradigms sharing core logic.
Also because they are working so differently, we have providers for each context there, sometimes sharing core logic like mappings of evaluation details.

If the rewrite is really that straight forward and the KMP version works for all platforms without additional effort, I think this would be great to get in.

KMP would be only targeting static context (aka client) SDKs anyways right?
It would be great to have a SDK working for client apps on all these environments.

I would only want to make sure that the effort maintaining the SDK does not dramatically increase if the SDK targets all KMP platforms. I guess we need solid test runs on all of the platforms then right?

@bencehornak
Copy link
Author

bencehornak commented May 23, 2025

KMP would be only targeting static context (aka client) SDKs anyways right? It would be great to have a SDK working for client apps on all these environments.

@lukas-reining I'm with you, the static-context (aka client) paradigm sounds very reasonable to me as the current focus for the Kotlin SDK. If the community around this repo grows, I could imagine an implementation for the dynamic-context paradigm as well, pretty much like for JS.

If the rewrite is really that straight forward and the KMP version works for all platforms without additional effort, I think this would be great to get in.

I would only want to make sure that the effort maintaining the SDK does not dramatically increase if the SDK targets all KMP platforms. I guess we need solid test runs on all of the platforms then right?

To JetBrains's credit so far it's been light sailing, I haven't needed to write a single line of platform-specific code 🎉 I believe it's not gonna get particularly complicated for maintaining this SDK, given that it contains more interfaces than actual implementations. The test infra will definitely need to be there, I think the current tests will make it for all platform given that I haven't added any platform-specific stuff. The browser test needs to be fixed by installing some browser in the CI, but that's not a big deal either.

For the Providers, on the other hand, it might be challenging to support multiple platforms based on how they are implemented:

  • If they are just wrapping the Android or iOS feature flag SDK of their backend (similar to the Flagsmith Java Provider), then bunch of platform-specific code and abstractions over that will be needed, requiring more work and maintenance effort for each additional targeted platform.
  • If they are depending directly on some REST/gRPC/... API (similar to the flagd Java Provider), then it's possible to have an actual multiplatform implementation based on pure Kotlin technologies (such as Ktor or Okio) keeping the overhead of each targeted platform to a minimum.

Ultimately, the authors of the Providers can decide, which platform they are targeting, it's not a requirement by any means for them to target all platforms supported by the SDK. The best thing we can do on the SDK side is to support as many platforms as we reasonably can, allowing the Provider authors to target all the platforms they like.

@bencehornak
Copy link
Author

bencehornak commented May 23, 2025

I'll be AFK for the rest of the week, but I'm planning to come back to this topic next week to make sure that all parts are up to speed before merging the PR.

Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
@bencehornak bencehornak force-pushed the kotlin-multiplatform branch from a9e2723 to 1a76c44 Compare May 31, 2025 13:11
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
@nicklasl
Copy link
Member

nicklasl commented Jun 2, 2025

While I support this PR I am having problems building this PR with Java SDK 11 while it is working for Java SDK 17.
Haven't looked into why but I assume it is due to AGP?

I would also like us to think if we are willing to keep go to some extra length to provide (publishing to maven central) the android-sdk as well as kotlin-sdk?

@realdadfish
Copy link

Yes, since AGP 8.0 JDK 17 is a build time requirement.

@bencehornak
Copy link
Author

bencehornak commented Jun 2, 2025

Yes, since AGP 8.0 JDK 17 is a build time requirement.

@realdadfish that requirement is related only to the Gradle JVM, and not to the Toolchain JDK as far as I understand.

image

I have checked some popular Android libraries regarding the JVM settings, all the ones I checked are targeting JDK 8:

Library jvmTarget sourceCompatibility targetCompatibility
Dagger 1.8 1.8 1.8
Kotlin Coroutines 1.8 1.8 1.8
Koin 1.8 1.8 1.8
OpenTelemetry Android 1.8 1.8 1.8

So I'd not increase the jvmTarget from 11. I'd also not go below, because Oracle is already done with the 'premier' support of JDK 8. As far as the Gradle JVM is concerned, the usage of JDK 17 seems to be inevitable as @realdadfish points out, so long as the project depends on a recent version of the AGP.

@bencehornak
Copy link
Author

I would also like us to think if we are willing to keep go to some extra length to provide (publishing to maven central) the android-sdk as well as kotlin-sdk?

@nicklasl since this PR is already bringing in some breaking changes (renaming of the base package, the replacement of java.util.Date with kotlin.time.Instant) the users must unfortunately perform some steps when upgrading anyways. I'd argue that it is better to push through with these breaking changes in a single version than interrupting the library users multiple times (additionally having two up-to-date versions is confusing).

So instead, I'd suggest relocating the artifact on Maven Central like this:
image

@bencehornak bencehornak marked this pull request as ready for review June 2, 2025 20:17
@nicklasl
Copy link
Member

nicklasl commented Jun 3, 2025

I would also like us to think if we are willing to keep go to some extra length to provide (publishing to maven central) the android-sdk as well as kotlin-sdk?

@nicklasl since this PR is already bringing in some breaking changes (renaming of the base package, the replacement of java.util.Date with kotlin.time.Instant) the users must unfortunately perform some steps when upgrading anyways. I'd argue that it is better to push through with these breaking changes in a single version than interrupting the library users multiple times (additionally having two up-to-date versions is confusing).

So instead, I'd suggest relocating the artifact on Maven Central

Yes, I agree.
I didn't know about the relocating feature.... I asked the AI about it and it claims that as long as you produce the correct pom (you would then produce an extra pom for the old version with the relocation) it "should work".... Then again, that was just an AI...

I tried it out using the code below inside `publishing` and did `publishToMavenLocal` and it seems to be legit.
// --- Relocation publication for the OLD 0.4.1 artifact ---
// This will publish a POM for dev.openfeature:android-sdk:0.4.1 that redirects.
create<MavenPublication>("relocationForOldAndroidSdk0_4_1") {
    // Define the OLD coordinates for this publication
    groupId = "dev.openfeature"   // The old groupId
    artifactId = "android-sdk"    // The old artifactId
    version = "0.4.1"             // The specific old version being relocated

    pom.withXml {
        asNode().appendNode("distributionManagement").appendNode("relocation").apply {
            // Point to the NEW coordinates
            appendNode("groupId", "dev.openfeature")
            appendNode("artifactId", "kotlin-sdk")
            // Point to the specific version of the new artifact you want users to use
            // when they try to resolve the old 0.4.1. In your case, this is 0.5.0.
            appendNode("version", "0.5.0")
            appendNode("message", "This artifact has been relocated. Please update your dependency to dev.openfeature:kotlin-sdk:0.5.0.")
        }
    }
}

Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
@bencehornak
Copy link
Author

If there is interest, I'm happy to finalize these changes, as it is a dependency for the newly set up kotlin-sdk-provider project. @nicklasl / @beeme1mr can you help with the review in the next days to have this PR merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kotlin Multiplatform Support
6 participants