Add JwtParserBuilder as the preferred way to create a JwtParser instance by bdemers · Pull Request #486 · jwtk/jjwt

bdemers · 2019-09-16T23:03:22Z

run the build and look at for actual API changes: api/target/japicmp/japicmp.html

This PR still has a few warts and TODOs, but those could change based on feedback, thoughts/ideas welcome.

Added new JwtParserBuilder
Copied mutator methods from JwtParser into new JwtParserBuilder
Marked said methods as deprecated in JwtParser
Copied JwtParserTest and JwtsTest to Deprecated*, as to retain coverage on methods that will be removed in 1.0

Fixes: #473

Related README changes are in #499

lhazlewood

Looking good! Just some comments and requests for changes.

lhazlewood · 2019-09-17T15:00:05Z

api/pom.xml

+        <plugins>
+            <plugin>
+                <groupId>com.github.siom79.japicmp</groupId>
+                <artifactId>japicmp-maven-plugin</artifactId>


How does this work exactly? I found https://siom79.github.io/japicmp/ but it's unclear to me what we're using this plugin for. Does it enforce semantic versioning?

Yup, I've been using it in other projects for Semver (but we are still pre 1.0, so it will not block anything). I'll add a comment to describe it a bit more
Right now it's just configured to break on binary breaking changes (source breaking change are allowed, e.g. adding methods to interfaces)

lhazlewood · 2019-09-17T15:01:51Z

api/src/main/java/io/jsonwebtoken/JwtParser.java

     * @return the parser method for chaining.
     * @see MissingClaimException
     * @see IncorrectClaimException
+     * @deprecated see {@link JwtParserBuilder#requireId(String)}


I think we should make these deprecation notices super clear - why they're deprecated, and how they should be using the builder instead. A link to Jwts#parserBuilder() and some minor explanation should be ok.

Also, when I add a deprecation notice, I like to add it in the JavaDoc paragraph as well, with very clear (bold) indications that this method will be removed from 1.0 final.

api/src/main/java/io/jsonwebtoken/JwtParser.java

lhazlewood · 2019-09-17T15:02:21Z

api/src/main/java/io/jsonwebtoken/JwtParser.java

+     * @return the builder for method chaining.
     * @see MissingClaimException
     * @see IncorrectClaimException
+     * @deprecated see {@link JwtParserBuilder#requireIssuer(String)}


Please see note re: deprecation

api/src/main/java/io/jsonwebtoken/JwtParser.java

api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java

impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java

impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java

lhazlewood · 2019-09-17T15:13:19Z

Oh also, we need to update the documentation to reflect this change everywhere. Also need to update the Changelog.

coveralls · 2019-09-17T17:06:15Z

Coverage remained the same at 100.0% when pulling 499f715 on jwt-parser-builder into 7090bf3 on master.

bdemers · 2019-09-17T20:29:30Z

@lhazlewood take another look, I just added an ImmutableJwtParser (impl) returned from Jwts.parserBuilder() which should protect anyone using the new method once drop the corresponding methods from JwtParser
Thoughts?

bdemers · 2019-09-17T20:31:06Z

NOTE: I did NOT change the README, as to not confuse anyone before the 0.11.0 release

lhazlewood

Just some minor change requests, other than that, looks good!

impl/src/main/java/io/jsonwebtoken/impl/ImmutableJwtParser.java

Doc: #486 Fixes: #494

- Added new JwtParserBuilder - Copied mutator methods from JwtParser into new JwtParserBuilder - Marked said methods as deprecated in JwtParser - Copied JwtParserTest and JwtsTest to Deprecated*, as to retain coverage on methods that will be removed in 1.0 - Added ImmutableJwtParser This is a stop gap until 1.0, all of the mutable methods will now throw a IllegalStateException. NOTE: this only comes into place when using the new Jwts.parserBuilder(), Jwts.parser() is unchanged. Fixes: #473

bdemers · 2019-09-30T22:03:20Z

@lhazlewood I hit some odd merge issues, but everything should be resolved now, take another look when you get a second.

lhazlewood

Awesome! 🚀

Doc: #486 Fixes: #494

…nce (#486) - Added new JwtParserBuilder - Copied mutator methods from JwtParser into new JwtParserBuilder - Marked said methods as deprecated in JwtParser - Copied JwtParserTest and JwtsTest to Deprecated*, as to retain coverage on methods that will be removed in 1.0 - Added ImmutableJwtParser This is a stop gap until 1.0, all of the mutable methods will now throw a IllegalStateException. NOTE: this only comes into place when using the new Jwts.parserBuilder(), Jwts.parser() is unchanged. Fixes: #473

- Docs: Adds section to README covering custom object parsing (#500) - Docs: Add note about JwtParserBuilder creating an immutable JwtParser (#508) Doc: #486 Fixes: #494 Doc: #495 Fixes: #171

- Docs: Adds section to README covering custom object parsing (#500) - Docs: Add note about JwtParserBuilder creating an immutable JwtParser (#508) Doc: #486 Fixes: #494 Doc: #495 Fixes: #171 Updated documentation and changelog to reflect the new Gson extension. Fixes #410. (#476)

…#559) - Docs: Adds section to README covering custom object parsing (#500) - Docs: Add note about JwtParserBuilder creating an immutable JwtParser (#508) Doc: #486 Fixes: #494 Doc: #495 Fixes: #171 Updated documentation and changelog to reflect the new Gson extension. Fixes #410. (#476) Co-authored-by: Brian Demers <brian.demers@gmail.com>

* `signWith()` & `setSigningKey()` now prefer to be handed a `Key` type rather than a base64-encoded String - deprecated in jwtk/jjwt#341 * To obtain a configured `JwtParser`, a builder is now preferred, ie `Jwts.parserBuilder().setSigningKey(key).build()` rather than `Jwts.parser().setSigningKey(key)` - deprecated in jwtk/jjwt#486

* `signWith()` & `setSigningKey()` now prefer to be handed a `Key` type rather than a base64-encoded String - deprecated in jwtk/jjwt#341 * To obtain a configured `JwtParser`, a builder is now preferred, ie `Jwts.parserBuilder().setSigningKey(key).build()` rather than `Jwts.parser().setSigningKey(key)` - deprecated in jwtk/jjwt#486 Note that `play-googleauth` currently supports two versions of Play: * Play v2.9, uses jjwt v0.11.5 * Play v3.0, uses jjwt v0.12.6 ...we need to support both, but thankfully everything we're using is supported in both versions of jjwt.

* `signWith()` & `setSigningKey()` now prefer to be handed a `Key` type rather than a base64-encoded String - deprecated in jwtk/jjwt#341 * To obtain a configured `JwtParser`, a builder is now preferred, ie `Jwts.parserBuilder().setSigningKey(key).build()` rather than `Jwts.parser().setSigningKey(key)` - deprecated in jwtk/jjwt#486 * Use `io.jsonwebtoken.security.SignatureException` rather than `io.jsonwebtoken.SignatureException` - deprecated in jwtk/jjwt#367 Note that `play-googleauth` currently supports two versions of Play: * Play v2.9, uses jjwt v0.11.5 * Play v3.0, uses jjwt v0.12.6 ...we need to support both, but thankfully everything we're using is supported in both versions of jjwt.

bdemers mentioned this pull request Sep 16, 2019

Introduce JwtParserBuilder and migrate JwtParser.* mutator methods #473

Closed

bdemers force-pushed the jwt-parser-builder branch from 35c1a85 to f074bcf Compare September 16, 2019 23:05

bdemers requested review from dogeared and lhazlewood and removed request for dogeared September 16, 2019 23:07

lhazlewood requested changes Sep 17, 2019

View reviewed changes

bdemers force-pushed the jwt-parser-builder branch from 501fa75 to 70db63e Compare September 17, 2019 20:27

lhazlewood requested changes Sep 18, 2019

View reviewed changes

impl/src/main/java/io/jsonwebtoken/impl/ImmutableJwtParser.java Show resolved Hide resolved

bdemers added this to the 0.11.0 milestone Sep 18, 2019

bdemers self-assigned this Sep 18, 2019

bdemers added a commit that referenced this pull request Sep 24, 2019

Update README to use parserBuilder() instead of parser()

3453fca

Doc: #486 Fixes: #494

bdemers mentioned this pull request Sep 24, 2019

Docs: Update README to use parserBuilder() instead of parser() #499

Merged

bdemers force-pushed the jwt-parser-builder branch 3 times, most recently from 727b660 to 0e67ea7 Compare September 30, 2019 21:56

bdemers force-pushed the jwt-parser-builder branch from 0e67ea7 to 499f715 Compare September 30, 2019 22:00

lhazlewood approved these changes Sep 30, 2019

View reviewed changes

bdemers merged commit 94d1511 into master Oct 1, 2019

bdemers deleted the jwt-parser-builder branch October 1, 2019 16:03

bdemers added a commit that referenced this pull request Oct 1, 2019

Update README to use parserBuilder() instead of parser() (#499)

31a0c6c

Doc: #486 Fixes: #494

lhazlewood mentioned this pull request Feb 4, 2020

README edits for 0.11.0 #559

Merged

rtyley mentioned this pull request Jul 4, 2024

Avoid deprecated jjwt methods for signing & parsing guardian/play-googleauth#241

Merged

Conversation

bdemers commented Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lhazlewood left a comment

Choose a reason for hiding this comment

Uh oh!

lhazlewood Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

bdemers Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

lhazlewood Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lhazlewood Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lhazlewood commented Sep 17, 2019

Uh oh!

coveralls commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bdemers commented Sep 17, 2019

Uh oh!

bdemers commented Sep 17, 2019

Uh oh!

lhazlewood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bdemers commented Sep 30, 2019

Uh oh!

lhazlewood left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bdemers commented Sep 16, 2019 •

edited

Loading

coveralls commented Sep 17, 2019 •

edited

Loading