-
Notifications
You must be signed in to change notification settings - Fork 12
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
FMWK-467 Issue using refs with yaml file #161
Conversation
src/main/java/com/aerospike/mapper/tools/configuration/ReferenceConfig.java
Show resolved
Hide resolved
typeMapper = new ObjectReferenceMapper(ClassCache.getInstance().loadClass(clazz, mapper), ref.getLazy(), ref.getBatchLoad(), ref.getType(), mapper); | ||
typeMapper = new ObjectReferenceMapper( | ||
ClassCache.getInstance().loadClass(clazz, mapper), | ||
ref.getLazy() == null ? false : ref.getLazy(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we assign defaults for lazy
and batchLoad
in ReferenceConfig
class? that way if another place requires defaults it will be taken from a single point of truth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After consulting with Tim, we can improve it but at some point we will need a refactor of yaml/annotation config reading which will affect this place as well - I'm ok with the scope of this PR just because its only a single place of null check but later we will probably want to refactor/improve/clean some areas of the Java Object Mapper including this specific flow
# Conflicts: # src/main/java/com/aerospike/mapper/tools/configuration/ClassConfig.java
Tim: "Fixed a null pointer exception when using refs via the YAML file, caused by BatchLoad being null and reading code incorrectly assuming a valid value."