Unable to upgrade smallrye-jwt 1.1.0 to 1.1.1 due to constructor change in JWTCallerPrincipal

[rsvoboda]

Unable to upgrade smallrye-jwt 1.1.0 to 1.1.1 due to constructor change in JWTCallerPrincipal

https://github.com/quarkusio/quarkus/blob/master/extensions/smallrye-jwt/runtime/src/main/java/io/quarkus/smallrye/jwt/runtime/auth/ElytronJwtCallerPrincipal.java#L38 calls JWTCallerPrincipal constructor but it was changed

Changed happened in smallrye/smallrye-jwt@488d438#diff-6873cfacfbc8acd082a03ea8ae79e8b3 by @sberyozkin

@sberyozkin, please change the code in quarkus to reflect your changes from smallrye

Another aspect is that such changes probably shouldn't happen in micro version releases.

[sberyozkin]

@rsvoboda should be a straightforward update, where is the factory which creates this Principal, would like to have look first

[sberyozkin]

By the way, 1.1.1 is all about minimizing the duplication in classes like ElytronJWTCallerPrincipal. I actually thought at a time Thorntail was the only consumer of that code as I was not really aware of where the quarkus source was at a time :-).

[sberyozkin]

Got it, it is the line I needed

[sberyozkin]

FYI, smallrye/smallrye-jwt#29 (not relevant to this issue but I'm seeing a lot of duplication)

[sberyozkin]

A lot of duplication between Quarkus smallrye-jwt and smallrye-jwt, PR is nearly ready but unfortunately the Quarkus code diverged in the way the conversion from claim values to JSON objects is done, so I'll have to think it would be simpler to do 1.2.0. I should've proposed to do 1.2.0 instead of 1.1.1 after my changes went in, my mistake.
However I'm positive the end result will be good for Quarkus - better have the jose4j and claim manipulation code centralized in the smallrye-jwt.
Thanks

[sberyozkin]

@rsvoboda , #1542 has been submitted; my apologies for getting the relevant changes into 1.1.1, though I was under the impression the first 2 version digits were somehow linked to the MP spec releases. I'll ask Scott what the versioning policy should be. Though I guess in this case since it is already in 1.1.1 we will probably go 1.1.2... thanks

[rsvoboda]

I was under the impression the first 2 version digits were somehow linked to the MP spec releases

Relation to MP spec version is a nice feature. Micro versions on the other hand should preserve compatibility and include mainly fixes. So it's tough situation, we can go away from this problem saying that end user api wasn't changed, but it's alibistic :)

With increasing number of projects (TT, WF, Quarkus, Liberty?) which are using SR bits I think there should be bigger push on compatibility in micro versions. For now I think we are fine (thanks for quick PR!).

[sberyozkin]

Thanks. I was just a bit careless, 1.1.1 did not mean to be incompatible, I was simply a bit naive assuming only Thorntail was integrating at the level I was applying my changes to (ex, I needed a cleaner code to have a KC factory/principal).
I'll ask on the smallrye list what the versioning policy we should follow

Cheers