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

fix: register jackson modules despite exotic class loaders #219

Merged
merged 2 commits into from Oct 17, 2020

Conversation

iProdigy
Copy link
Member

Prerequisites for Code Changes

  • This pull request follows the code style of the project
  • I have tested this feature

Issues Fixed

  • Folks using Twitch4J in minecraft server plugins would have issues when Instants needed to be deserialized from JSON, despite properly shading their plugin jar to include jackson and the jsr310 module. The custom PluginClassLoader used in this context likely is not compatible with ObjectMapper#findAndRegisterModules

Changes Proposed

Primary

  • Explicitly register ParameterNamesModule and JavaTimeModule (so we are also shifting off of the deprecated JSR310Module with this PR) in TypeConvert, instead of relying upon findAndRegisterModules
  • Use TypeConvert's ObjectMapper wherever possible

Secondary

  • Bump jackson version
  • Bump caffeine version
  • Bump JUnit version
  • Fix typo in pagination of hype train events caused by Twitch's faulty docs
  • Remove NonNull annotation on category search data due to twitch inconsistency

Copy link
Member

@PhilippHeuer PhilippHeuer left a comment

Choose a reason for hiding this comment

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

Good work figuring this out :)

You missed one spot in the tmi module to set the new objectMapper, i added that one.

@PhilippHeuer PhilippHeuer merged commit 7d7574f into develop Oct 17, 2020
@iProdigy iProdigy deleted the fix/jackson-module-register branch October 17, 2020 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants