Allow to get a Claim as Map by lbalmaceda · Pull Request #152 · auth0/java-jwt · GitHub
Skip to content

Allow to get a Claim as Map#152

Merged
lbalmaceda merged 2 commits into
masterfrom
add-claim-as-map
Apr 12, 2017
Merged

Allow to get a Claim as Map#152
lbalmaceda merged 2 commits into
masterfrom
add-claim-as-map

Conversation

@lbalmaceda

@lbalmaceda lbalmaceda commented Mar 14, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR introduces the option to get a Claim as Map<String, Object>.

example usage:

DecodedJWT jwt = // decode or verify the jwt
Map<String, Object> userDataMap = jwt.getClaim("user-data").asMap();

Fixes #146

@lbalmaceda lbalmaceda requested a review from hzalaz March 14, 2017 22:25
@lbalmaceda lbalmaceda force-pushed the add-claim-as-map branch 2 times, most recently from 5640336 to 24518a9 Compare March 15, 2017 17:51
@lbalmaceda lbalmaceda added this to the v3-Next milestone Mar 15, 2017
@lbalmaceda lbalmaceda mentioned this pull request Mar 22, 2017
Closed
@lbalmaceda lbalmaceda requested a review from nikolaseu April 7, 2017 17:59
nikolaseu
nikolaseu reviewed Apr 11, 2017
View reviewed changes
Comment thread lib/src/main/java/com/auth0/jwt/impl/JsonNodeClaim.java

@nikolaseu nikolaseu Apr 11, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

not really this PR, but why isn't there just one ObjectMapper (even static) instead of creating a new one every time?
the same can be said about the TypeReference

@lbalmaceda lbalmaceda Apr 11, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's on our backlog to enhance this. We're thinking on moving the implementation to use Jackson's Reader and Writer.

Comment thread lib/src/test/java/com/auth0/jwt/impl/JsonNodeClaimTest.java

@nikolaseu nikolaseu Apr 11, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

we need more "high level" tests here IMHO, we're just using the same implementation to encode & decode.
isn't somewhere tests that use a plain json string? I think we should add there one example claim of this type and test that everything works as expected

@lbalmaceda lbalmaceda Apr 11, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I added the tests you're asking for, considering this PR adds only the ability to "get as map" and not the verify/create.

@lbalmaceda lbalmaceda merged commit bf0a030 into master Apr 12, 2017
@lbalmaceda lbalmaceda deleted the add-claim-as-map branch April 12, 2017 17:56
@lbalmaceda lbalmaceda modified the milestones: v3-Next, 3.2.0 May 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hzalaz hzalaz Awaiting requested review from hzalaz
1 more reviewer
@nikolaseu nikolaseu nikolaseu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CH: Added

Projects

None yet

Milestone

3.2.0

Development

Successfully merging this pull request may close these issues.

2 participants

@lbalmaceda @nikolaseu