Incomplete JavaDoc in JsonWebToken

[keilw]

The JavaDoc of JsonWebToken is incomplete at least in the getSub() method:
"This is the token issuing IDP subject, not the"
is an incomplete sentence, there is nothing after "not the" explaining what it isn't.

[keilw]

@sberyozkin So it seems you don't have resources to fix those smaller issues before a 2.0 release? When would that be expected and should it already use Jakarta EE 9 (in which case much more significant changes will be necessary anyway everywhere, not just the spec)
Btw, what's the difference between the milestones https://github.com/eclipse/microprofile-jwt-auth/milestone/8 and https://github.com/eclipse/microprofile-jwt-auth/milestone/3?

[sberyozkin]

@keilw

First of all, referring to the 3 issues you have opened - thanks for that and for the careful review.

So it seems you don't have resources to fix those smaller issues before a 2.0 release?

The release process is long and requires the coordination from all of the MP community, so trying to do a new minor release to fix the text typos/bugs is not something I'd consider.

When would that be expected and should it already use Jakarta EE 9 (in which case much more significant changes will be necessary anyway everywhere, not just the spec)

I've no idea right now - I suppose we just need to wait and see till the steering team decides on the timeline

Btw, what's the difference between the milestones https://github.com/eclipse/microprofile-jwt-auth/milestone/8 and https://github.com/eclipse/microprofile-jwt-auth/milestone/3?

The former is about the future MP JWT 2.0 release, the latter is outdated and has been closed

[keilw]

@sberyozkin Thanks for the reply. As for the spec, those aren't just typos, they are just plain wrong and should be fixed. IMO it was rushed trying to get LWT out with the others and the same care everyone has to take for e.g. the Jakarta EE specs in order to meet the Jakarta EE 9 goal did not fully apply here ;-/
That is what a MR (Maintenance Release) is meant for in the case of JSRs and there should be a Service Release or similar here, too so the specs are not wrong and polluted for a year or more. The JavaDoc is also odd but that is of lesser concern if it was like that ever since.

[ederks85]

Hi all,

Thanks for bringing up the details for this issue. I would advise though, if changes are really necessary, to pursue these and get that released with a maintenance (hopefully not a major) release of the MP platform. Yes, people and processes need to align here, but with the right focus, it technically should not be a big and long hurdle.

As for the Jakarta EE 9 alignment, I'm planning to bring that up and hopefully kickstart this during the next live hangout.

[keilw]

Hi @ederks85 Thanks for the reply. I will propose a PR for each of those, most came out of my work on Jakarta EE Security with your Fellow "Dutchmen" @arjantijms and @thodorisbais, and it would be nice, if especially the spec may arise in a maintenance or service release because things like JsonWebToken getOtherClaim(String) are just wrong and irritating, so they should be fixed sooner.

[ederks85]

@keilw that would be great. Please notify me of those and I can take them up for discussion when and how to release

[Emily-Jiang]

As mentioned on the google mailing list, MicroProfile platform release does not pick up the maintenance releases of the component specs. MP JWT can go ahead to create a bug release whenever it is necessary.

[Emily-Jiang]

Personally, I think this fix can wait for the next release of MP JWT.

[keilw]

On the spec side I don't really agree. How long has an obvious bug like
JsonWebToken getOtherClaim(String) (a method that was either dropped or never existed) been around btw?

[Emily-Jiang]

well, does the bug stop you from using MicroProfile JWT into production? It will be nice it is bug free, but it is not reality. For some errors we can leave with it for sometime especially for the ones that do not prevent you from using MicroProfile JWT. We continue the release journey and prioritise the issues all the time. We have to judge what issues need to be fixed and released asap and what issues can wait for some time. If MP JWT team wants to do a bug release, it is fine with me.

[keilw]

It confuses the users of MP JWT, e.g. from a user perspective I try to explain how to use it in a Jakarta EE context, which is also made harder by all the outdated spec references like "JSR 375" which no longer apply to version 1.2 but the wrong API reference (for almost 4 years now that is quite embarassing) is the most frustrating part in the spec. The outdated "Java EE" references may bother Oracle's lawyers more than me or the readers of our book but that one is really a pain.

[sberyozkin]

@keilw As I said, I don't think some old spec text bugs should warrant an immediate release. No-one noticed before so I guess it has not been a show-stopper for other users. And indeed when the users will start their work with MP JWT 1.2, I nearly 100% sure no-one will read the PDF in order to figure out how to use JsonWebToken but look at its JavaDocs. So, no, I don't share your major concern about this particular bug.
Also, I do believe we've done a good clean-up job in MP JWT 1.2 after the spec has gone stale. Some parts of that text have not been carefully re-read, but now that you've found those issues I'm sure we can make it a less embarrassing read going forward.

[sberyozkin]

@keilw, more comments

It confuses the users of MP JWT, e.g. from a user perspective I try to explain how to use it in a Jakarta EE context, which is also made harder by all the outdated spec references like "JSR 375" which no longer apply to version 1.2 but the wrong API reference (for almost 4 years now that is quite embarassing) is the most frustrating part in the spec.

You are over-generalizing here. You are the first user who have found this outdated reference so lets not go into asserting how everyone in the world who is reading the specification text is now confused and does not know what to do with JsonWebToken.
Besides, MP JWT 1.2 has not really focused on any Jakarta EE related integration - the focus has been on making the spec live again.
For MP JWT 2.0 we can consider more pervasive changes/updates.

The outdated "Java EE" references may bother Oracle's lawyers more than me

No worries - we will fix it in the next MP platform, whatever it will be

or the readers of our book

Maybe you can avoid copying the reference to the old JSR :-) ?

but that one is really a pain.

Referring to a PDF typo of the JsonWebToken method name as a real pain is over-dramatization IMHO. It is a bug and we will fix it.

Anyway, thank you again for discovering these bugs

[keilw]

@sberyozkin Avoid old references where, in our book, rest assured we fix them there, making it a good reason to buy it ;-D

Glad to help, I'll offer some PRs when I can.

[sberyozkin]

Hey @keilw, sounds like I should get it then, may be even 2 copies :-)

You said you'd open PRs, are you still OK with it ? Can you please open one PR tackling the typo in this PR (just dropping , not the would be fine I guess) and the getOtherClaim() typo ? (or I can do this one)

And another PR dealing with the old JSR refs ? (that one may be more involved).

Then we can discuss again the idea of the 1.2.1 tag - as Emily @Emily-Jiang said it won't be in the MP 4.0 platform, but the next platform release will pick up these fixes again.

[keilw]

@sberyozkin No I created those tickets, no PRs yet. Yes mostly for the old JSRs, as I know pretty well what the replacements are from the Spec Committee. You'd be welcome to propose or fix the getOtherClaim() bug (it's not just a typo ;-) with an appropriate wording. I could only imagine what Scott's blog post did around the same time: https://www.eclipse.org/community/eclipse_newsletter/2017/september/article2.php or maybe you have another idea.

If @thodorisbais and @arjantijms are happy with it, I guess you could be a good expert proof-reader for the book. I can't say if you would get a copy later but you'd certainly get to read a chapter or more that way.

[sberyozkin]

@keilw

You'd be welcome to propose or fix the getOtherClaim() bug (it's not just a typo ;-) with an appropriate wording. I could only imagine what Scott's blog post did around the same time: https://www.eclipse.org/community/eclipse_newsletter/2017/september/article2.php or maybe you have another idea.

Pretty sure this typo (come on, who is going to check page N of the spec PDF to find out how to get the other claim :-) ? ) is originating from that time. OK, np, I'll fix it

If @thodorisbais and @arjantijms are happy with it, I guess you could be a good expert proof-reader for the book. I can't say if you would get a copy later but you'd certainly get to read a chapter or more that way.

Thanks, I won't be taking this opportunity; good luck with the book

P.S I edited your last comment only to fix my alias