Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Jackson Afterburner Module for JWTCreator and JWTParser #669

Closed
wants to merge 4 commits into from

Conversation

adam-gray
Copy link

Changes

Please describe both what is changing and why this is important. Include:

  • Adds dependency com.fasterxml.jackson.module:jackson-module-afterburner with the existing jackson-databind version of 2.14.2
  • Modifies instantiation of ObjectMapper in JWTCreator and JWTParser classes to include the AfterburnerModule

References

Please include relevant links supporting this change such as a:

What is optimized using Jackson Afterburner? (copied from here)
Following things are optimized:

  • For serialization (POJOs to JSON):
    • Accessors for "getting" values (field access, calling getter method) are inlined using generated code instead of reflection
    • Serializers for small number of 'primitive' types (int, long, String) are replaced with direct calls, instead of getting delegated to JsonSerializers
  • For deserialization (JSON to POJOs)
    • Calls to default (no-argument) constructors are byte-generated instead of using reflection
    • Mutators for "setting" values (field access, calling setter method) are inlined using generated code instead of reflection
    • Deserializers for small number of 'primitive' types (int, long, String) are replaced with direct calls, instead of getting delegated to JsonDeserializers

... and what is not?

  • Streaming parser/generator access (although it is possible that some optimizations may be added)
  • Tree Model: there isn't much that can be done at this level

Testing

Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.

  • Additional test coverage - not really sure how to add additional cases for this?
  • This change has been tested on:
    • Java 8
    • Java 11
    • Java 17
  • JMH benchmark added for JWT creation

Checklist

@adam-gray adam-gray requested a review from a team as a code owner August 15, 2023 22:57
@jimmyjames
Copy link
Contributor

Thanks @adam-gray for the PR! Im out of the office until next week, but will take a look when I return. One thing I'll be curious about is the ongoing activity/support for Afterburner, given the notes regarding a potential replacement in the works.

@jimmyjames jimmyjames added the needs investigation An issue that has more questions to answer or otherwise needs work to fully understand the issue label Aug 21, 2023
@jimmyjames
Copy link
Contributor

The JWTDecoderTest is failing with this change, which I've reproduced locally. @adam-gray if you have any insight into the failure and potential fix we can advance this change.

@jimmyjames
Copy link
Contributor

Closing this PR as it has gone inactive, but I do think this (and further potential customization of the ObjectMapper) is something we should consider. It's likely something that will be considered in the new year.

@jimmyjames jimmyjames closed this Dec 4, 2023
@adam-gray
Copy link
Author

sorry for ghosting @jimmyjames - had a real busy period with work and then took some time to travel to see extended family over the holidays. when i get a chance i'll fix the test and resubmit 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation An issue that has more questions to answer or otherwise needs work to fully understand the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants