javalin / javalin-ssl

Straightforward SSL Configuration for Javalin!
https://javalin.io/plugins/ssl-helpers
10 stars 1 forks source link

Add @JvmField to TlsConfig constants #132

Closed bluemods closed 7 months ago

bluemods commented 7 months ago

From java side without JvmField you have to access it like this:

ssl.tlsConfig = TlsConfig.Companion.getINTERMEDIATE();

This allows you to access it the expected way:

ssl.tlsConfig = TlsConfig.INTERMEDIATE;
zugazagoitia commented 7 months ago

Actually this is a breaking change for Java users, it'll have to wait until the next major release. But let's keep the PR open.

bluemods commented 7 months ago

Actually this is a breaking change for Java users, it'll have to wait until the next major release. But let's keep the PR open.

My perspective was as someone coming from 5.6.3 to 6.0.0, it was a breaking change that required changing code to access it using the companion class. For that case, this PR would allow for upgrading without changing those lines. But yes for existing users that have migrated they would have to change it back to how it was coded on 5.x.

Not a huge deal, just something I came across when migrating to 6.0.0.

zugazagoitia commented 7 months ago

Of course I understand your point, and I'd prefer it to be that way. It's just something I overlooked. But following strict semver I can't push breaking changes to code on a minor version bump.

dzikoysk commented 7 months ago

You should be able to cover backwards compatibility for Java with something like this:

class Test {
    companion object {
        @JvmField
        val MODERN = "abc"
        fun getMODERN(): String = MODERN 
    }
}
Playacem commented 7 months ago

We previously considered such java compatibility issues as bugs which would be fixed in a patch release. I would consider @dzikoysk approach only with marking the getter method as deprecated as it is definitely not the expected java api.