-
Notifications
You must be signed in to change notification settings - Fork 109
Improve SERIALIZE_IDENTIFIER_FOR_LAZY_NOT_LOADED_OBJECTS
feature
#96
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
Changes from 1 commit
a7098ce
ba677a9
c119de4
a331270
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
package com.fasterxml.jackson.datatype.hibernate4; | ||
|
||
import java.io.IOException; | ||
import java.lang.reflect.Field; | ||
import java.lang.reflect.Method; | ||
import java.util.HashMap; | ||
|
||
import com.fasterxml.jackson.core.*; | ||
|
@@ -18,6 +20,7 @@ | |
import org.hibernate.engine.spi.SessionImplementor; | ||
import org.hibernate.proxy.HibernateProxy; | ||
import org.hibernate.proxy.LazyInitializer; | ||
import org.hibernate.proxy.pojo.BasicLazyInitializer; | ||
|
||
/** | ||
* Serializer to use for values proxied using {@link org.hibernate.proxy.HibernateProxy}. | ||
|
@@ -174,15 +177,18 @@ protected Object findProxied(HibernateProxy proxy) | |
LazyInitializer init = proxy.getHibernateLazyInitializer(); | ||
if (!_forceLazyLoading && init.isUninitialized()) { | ||
if (_serializeIdentifier) { | ||
final String idName; | ||
String idName; | ||
if (_mapping != null) { | ||
idName = _mapping.getIdentifierPropertyName(init.getEntityName()); | ||
} else { | ||
final SessionImplementor session = init.getSession(); | ||
if (session != null) { | ||
idName = session.getFactory().getIdentifierPropertyName(init.getEntityName()); | ||
} else { | ||
idName = init.getEntityName(); | ||
idName = ProxyReader.getIdentifierPropertyName(init); | ||
if (idName == null) { | ||
idName = init.getEntityName(); | ||
} | ||
} | ||
} | ||
final Object idValue = init.getIdentifier(); | ||
|
@@ -194,4 +200,40 @@ protected Object findProxied(HibernateProxy proxy) | |
} | ||
return init.getImplementation(); | ||
} | ||
|
||
// Alas, hibernate offers no public api to access this information, so we must resort to ugly hacks ... | ||
protected static class ProxyReader { | ||
|
||
// static final so the JVM can inline the lookup | ||
private static final Field getIdentifierMethod; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bit confusing to call field a method? Should name be something bit more descriptive? Also, might make sense to explain what the intent is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmh. Maybe it'd be best to just add a Javadoc comment to explain that? |
||
|
||
static { | ||
try { | ||
getIdentifierMethod = BasicLazyInitializer.class.getDeclaredField("getIdentifierMethod"); | ||
getIdentifierMethod.setAccessible(true); | ||
} catch (Exception e) { | ||
// should never happen: the field exists in all versions of hibernate 4 and 5 | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
|
||
/** | ||
* @return the name of the identifier property, or null if the name could not be determined | ||
*/ | ||
static String getIdentifierPropertyName(LazyInitializer init) { | ||
try { | ||
Method idGetter = (Method) getIdentifierMethod.get(init); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So.... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LazyInitializer itself not generated (it is the Javassist MethodHandler the generated proxy class delegates to. The LazyInitializer has special handling for invocations of the getter of the persistent identity, and therefore stores the corresponding I use reflection here because the field is private and has no getter :-( |
||
if (idGetter == null) { | ||
return null; | ||
} | ||
String name = idGetter.getName(); | ||
if (name.startsWith("get")) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably has to remain here; ideally But there is http://docs.oracle.com/javase/7/docs/api/java/beans/Introspector.html#decapitalize(java.lang.String) which is what you can call, so:
if name starts with "get". And the reason to call the method is that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch, thanks :-) |
||
name = Character.toLowerCase(name.charAt(3)) + name.substring(4); | ||
} | ||
return name; | ||
} catch (Exception e) { | ||
throw new RuntimeException(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.
Would be nice to expand just slightly on logic for accessing information.
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.
What to you mean by "expand on logic"? (How can I document the absence of a suitable API? Should I list what I tried, and why that didn't work?)
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.
What I meant was just to explain what is being done; mention that there's no public API is fine, but explain what information, and for what reason. If that is covered in other parts that may be fine.
There's no need (IMO) to explain what else has been tried; just what the intent is.