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

[fixes #2768] Making the Builder constructor's visibility configurable. #2779

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SebestyenBartha
Copy link

@SebestyenBartha SebestyenBartha commented Mar 19, 2021

This makes the Builder constructor's visibility configurable. Also fixes #2768, however, I just stumbled upon that bug while trying to fix our own problem.
The problem we had in our project is that we need to use Jackson and configure it so that it doesn't override access modifiers (set MapperFeature.CAN_OVERRIDE_ACCESS_MODIFIERS to false) because in our runtime environment (embedded) there is a SecurityManager which doesn't allow that and we have no control over that. In this configuration Jackson cannot work unless the Builder's constructor is public. Making this configurable while keeping the default visibility PACKAGE, existing code is unaffected, but we can add the parameter to our Builder annotations which is more elegant than having to explicitly write the Builder classes.

@rzwitserloot
Copy link
Collaborator

The way it works now was intentional. Reasoning stated in, and awaiting feedback at, linked issue #2768 before proceeding on this PR.

If this is discouraging, that sucks :( - good job on the PR 👍 though; proper styling applied, added name to authors file.

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.

[BUG] Builder Constructors are protected when should be public in 1.18.18
2 participants