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: TypeManager initialization fails, if Jackson is not on class path #3149

Closed
wants to merge 2 commits into from

Conversation

rPraml
Copy link
Contributor

@rPraml rPraml commented Aug 8, 2023

If ebean-jackson-mapper but no jackson is on classpath, it can happen, that the service-loader finds the ScalarJsonJacksonMapper class, and ebean thinks, there is a jsonMapper, but when trying to call jsonMapper.markerAnnotation() you'll get a NoClassDefFoundError

This PR checks on initialization, if the markerAnnotation is accessible.

if (mapper.markerAnnotation() != null) {
return mapper;
} else {
log.log(System.Logger.Level.WARNING, "Not using {0}, because markerAnnotation is null", mapper.getClass().getName());

Choose a reason for hiding this comment

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

I think a better message would be to simply say that Jackson is missing on class path. Nobody knows what "markerAnnotation is null" actually means.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jnehlmeier thanks for feedback. Do you think this message is OK now? While there isn't an alternative to jackson at the moment, I didn't want to refer to it too directly. See #2564

Choose a reason for hiding this comment

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

I think I would keep it specific as long as only a single implementation is supported. Alternative would be a link to documentation but I think there is no page that describes json configuration for ebean.

rPraml added a commit to FOCONIS/ebean that referenced this pull request Aug 10, 2023
@rbygrave
Copy link
Member

but no jackson is on classpath

The better fix is to include Jackson on the class path as a dependency of ebean-jackson-mapper. Folks can also specify the Jackson dependency to control the version of the Jackson dependency included in the class path.

@rPraml
Copy link
Contributor Author

rPraml commented Oct 17, 2023

@rbygrave yes, adding jackson to ebean-jackson-mapper would also be an acceptable solution for me.
Can you do this little thing on the side? Or would you prefer that I provide a PR?

@rbygrave
Copy link
Member

This PR - #3250 ... includes Jackson-core and Jackson-databind as transitive dependencies.

@rbygrave
Copy link
Member

I think that means we should close this issue. I'll close it, re-open if needed.

@rbygrave rbygrave closed this Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants