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

Ignore non public members for base java classes #177

Merged
merged 3 commits into from
Jan 25, 2022

Conversation

nicky9door
Copy link
Contributor

@nicky9door nicky9door commented Jan 21, 2022

This change causes genson to ignore non-public members for classes in java/javax packages. Public members are still checked for other modifiers specified by the filter. This should fix the reflection issues specified in #173

An alternative fix is provided in #178

return false;
}
}

return isVisible(member.getModifiers());
Copy link
Contributor

Choose a reason for hiding this comment

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

You forgot to include what I mentioned in the other PR, you want to leverage the same filter as isVisible. Otherwise this will include all fields/methods that are public. Some might be static, transient, etc, these shouldn't be included.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code is slightly different from the other PR. The main difference being the ! in front of Modifiers.isPublic. In this PR, any non-public field in a base class is excluded. Otherwise, we perform the additional call to isVisible (line 81)

The previous PR was automatically including all public fields in base classes without considering the additional filters. I can see how this might not be the clearest code however. I will modify the PR to be a bit more clear

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops my bad! Thanks for the explanation.

@nicky9door
Copy link
Contributor Author

I added a second commit which should hopefully make everything a little more clear

@EugenCepoi EugenCepoi merged commit e00ef31 into owlike:master Jan 25, 2022
@armujahid armujahid mentioned this pull request Feb 6, 2024
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.

2 participants