-
Notifications
You must be signed in to change notification settings - Fork 165
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
Execute @PostLoad method after fully hydrating all entities #406
Conversation
2717b70
to
cbf9244
Compare
try { | ||
method.invoke(instance); | ||
} catch (IllegalAccessException | InvocationTargetException e) { | ||
logger.error("Failed to execute post load method", e); |
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.
When the PostLoad method does not respect the "contract" (public, no arg method), low level java exceptions are thrown like IllegalArgumentException: wrong number of arguments
Maybe some guidance could be useful like "Cannot call PostLoad annotated method {method fqn}. Make sure it is public and has no arguments"
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.
Changed, I have also changed it to warn as it is not a blocking error.
@@ -190,17 +232,10 @@ private void mapNodes(GraphModel graphModel, Set<Long> nodeIds) { | |||
} | |||
} | |||
|
|||
private void executePostLoad(Object instance) { | |||
private void setIdentity(Object instance, Long id) { |
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.
Isn't it dead code ?
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.
Yes it is. Will delete.
* @param edgeIds edgeIds | ||
*/ | ||
public void executePostLoad(Set<Long> nodeIds, Set<Long> edgeIds) { | ||
Set<Long> seenIds = new HashSet<>(); |
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.
Maybe because it's late but I don't see the purpose of seenIds
as method parameters are Sets. Some old code remaining ?
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.
Good catch!! It also made me discover another but with @PostLoad method, will push fix in another commit.
Fixes #403, performance issue when mapping large number of rows returned.
cbf9244
to
4a9ff55
Compare
Fixes #403, performance issue when mapping large number of rows
returned.
Description
executePostLoad is executed after processing all rows
repeated iteration over accumulating nodeIds and edgeIds is then avoided
Related Issue
See #403
How Has This Been Tested?
All tests pass without modification.
Types of changes
Checklist: