Skip to content

Commit

Permalink
GH-516 - Treat @PostLoad methods the same way as JSR-250 treats @Post…
Browse files Browse the repository at this point in the history
…Construct.

Forcing the PostLoad method to be public and non final leads to poor domain classes:
- often the postload method is also used from the constructor, so they should be at least final so that the class can be safely overwritten
- also postload should usually not be an exposed method to calling code, it’s used to be only internal

Steps taken:
- Include „final“ methods in MethodsInfo.
- When postload is executed, check if the annotated method is accessible. If not, try to make it accessible.
  • Loading branch information
michael-simons committed Sep 3, 2018
1 parent 64e35ac commit 73a5ac3
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 4 deletions.
5 changes: 4 additions & 1 deletion CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ o Improve determination of labels in class hierarchy scenarios. Labels are now c
abstract classes are now considered as labels as long as the class contributes to the index. If a hierarchy lead to a
situation where multiple different labels can be applied, the topmost one is used. #437
o Don't run the auto index manager if auto index mode is NONE (prevents eagerly opening a session). #437
o Deprecated #getIndexes and #build in AutoIndexManager. #437
o Deprecate #getIndexes and #build in AutoIndexManager. #437
o Convert array correct in delete operations. #509
o Treat @PostLoad methods the same way as JSR-250 treats @PostConstruct. #516
o Recognize overwritten @PostLoad methods in a class hierarchy. #414, #516
o Deprecate default constructor and mutating put-method in ObjectAnnotations

3.1.2
o Improve documentation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
/**
* @author Vince Bickers
* @author Luanne Misquitta
* @author Michael J. Simons
*/
public class GraphEntityMapper implements ResponseMapper<GraphModel> {

Expand Down Expand Up @@ -187,7 +188,13 @@ private void executePostLoad(Object instance) {
if (postLoadMethod != null) {
final Method method = classInfo.getMethod(postLoadMethod);
try {
if(!method.isAccessible()) {
method.setAccessible(true);
}
method.invoke(instance);
} catch(SecurityException e) {
logger.warn("Cannot call PostLoad annotated method {} on class {}, "
+ "security manager denied access.", method.getName(), classInfo.name(), e);
} catch (IllegalAccessException | InvocationTargetException e) {
logger.warn("Cannot call PostLoad annotated method {} on class {}. "
+ "Make sure it is public and has no arguments", method.getName(), classInfo.name(), e);
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/neo4j/ogm/metadata/MethodsInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public MethodsInfo(Class<?> cls) {

for (Method method : cls.getDeclaredMethods()) {
final int modifiers = method.getModifiers();
if (!Modifier.isTransient(modifiers) && !Modifier.isFinal(modifiers) && !Modifier.isStatic(modifiers)) {
if (!Modifier.isTransient(modifiers) && !Modifier.isStatic(modifiers)) {
ObjectAnnotations objectAnnotations = new ObjectAnnotations();
final Annotation[] declaredAnnotations = method.getDeclaredAnnotations();
for (Annotation annotation : declaredAnnotations) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,33 @@
* code for these subcomponents is subject to the terms and
* conditions of the subcomponent's license, as noted in the LICENSE file.
*/
package org.neo4j.ogm.domain.generic_hierarchy;
package org.neo4j.ogm.domain.postload;

import java.util.UUID;

import org.neo4j.ogm.annotation.PostLoad;

/**
* @author Michael J. Simons
*/
public class User extends AbstractBaseUser {
public class UserWithBetterPostLoadMethod {
private Long id;

private transient String randomName;

public UserWithBetterPostLoadMethod() {
this.randomName = "n/a";
}

public Long getId() {
return id;
}

public String getRandomName() {
return randomName;
}

@PostLoad final void postLoad() {
this.randomName = UUID.randomUUID().toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.junit.BeforeClass;
import org.junit.Test;
import org.neo4j.ogm.domain.postload.User;
import org.neo4j.ogm.domain.postload.UserWithBetterPostLoadMethod;
import org.neo4j.ogm.session.Session;
import org.neo4j.ogm.session.SessionFactory;
import org.neo4j.ogm.testutil.MultiDriverTestClass;
Expand Down Expand Up @@ -90,4 +91,16 @@ public void shouldCallPostLoadForEachEntityOnce() throws Exception {

assertThat(User.getPostLoadCount()).isEqualTo(3);
}

@Test // #516
public void shouldCallNonPublicFinalPostLoad() throws Exception {
UserWithBetterPostLoadMethod user = new UserWithBetterPostLoadMethod();
session.save(user);

session.clear();

UserWithBetterPostLoadMethod loaded = session.load(UserWithBetterPostLoadMethod.class, user.getId());
assertThat(loaded).isNotNull();
assertThat(loaded.getRandomName()).isNotEqualTo(user.getRandomName());
}
}

0 comments on commit 73a5ac3

Please sign in to comment.