You are viewing a plain text version of this content. The canonical link for it is here.
Posted to ojb-dev@db.apache.org by Lance Eason <la...@whisperwire.com> on 2003/03/15 21:16:05 UTC
Some small suggestions...
I have three minor suggestions for improvements that would make my life easier in using OJB. All the code is based off of a pull from CVS yesterday...
1. I was very happy to see that thanks to the logic in ConstructorHelper my class doesn't have to provide a public no argument constructor, I can keep it protected or private and still have everything work. The problem is that using a non-public constructor currently causes an exception to be thrown and the constructor definition to be looked up for each instance that is created which is a significant performance hit. I modified ClassDescriptor to provide a getZeroArgumentConstructor() method that looks up the constructor once and remembers:
private static final Class[] NO_PARAMS = {};
/**
* the zero argument constructor for this class
*/
private Constructor zeroArgumentConstructor = null;
/**
* whether we have already tried to look up the zero argument constructor
*/
private boolean alreadyLookedupZeroArguments = false;
/**
* returns the zero argument constructor for the class represented by this class descriptor
* or null if a zero argument constructor does not exist. If the zero argument constructor
* for this class is not public it is made accessible before being returned.
*/
public Constructor getZeroArgumentConstructor()
{
if (zeroArgumentConstructor == null && !alreadyLookedupZeroArguments)
{
try
{
zeroArgumentConstructor = getClassOfObject().getConstructor(NO_PARAMS);
}
catch (NoSuchMethodException e)
{
//no public zero argument constructor available let's try for a private/protected one
try
{
zeroArgumentConstructor = getClassOfObject().getDeclaredConstructor(NO_PARAMS);
//we found one, now let's make it accessible
zeroArgumentConstructor.setAccessible(true);
}
catch (NoSuchMethodException e2)
{
//out of options, log the fact and let the method return null
LoggerFactory.getDefaultLogger().warn(this.getClass().getName() + ": " +
"No zero argument constructor defined for "
+ this.getClassOfObject());
}
}
alreadyLookedupZeroArguments = true;
}
return zeroArgumentConstructor;
}
This pretty much mirrors the as yet unused getConstructor() that is already on ClassDescriptor for returning a multi-argument constructor. I then added a signature to ConstructorHelper that takes a Constructor:
public static final Object[] NO_ARGS = {};
/**
* create a new instance of the class represented by the no-argument constructor provided
* @param constructor the zero argument constructor for the class
* @return a new instance of the class
* @throws InstantiationException
* @throws ClassNotPersistenceCapableException if the constructor is null or there is an
* exception while trying to create a new instance
*/
public static Object instantiate(Constructor constructor) throws InstantiationException
{
if (constructor == null)
{
throw new ClassNotPersistenceCapableException(
"A zero argument constructor was not provided!");
}
Object result = null;
try
{
result = constructor.newInstance(NO_ARGS);
}
catch (Throwable t)
{
throw new ClassNotPersistenceCapableException(
"Could not instantiate "
+ constructor.getDeclaringClass().getName()
+ ": "
+ t.getMessage() + ")");
}
return result;
}
And modified RowReaderDefaultImpl.buildWithReflection() and PkEnumeration.getIdentityFromResultSet() to use the new signature:
protected Object buildWithReflection(Map row, ClassDescriptor targetClassDescriptor)
{
Object result;
FieldDescriptor fmd;
try
{
// 1. create an empty Object (persistent classes need a public default constructor)
Constructor c = targetClassDescriptor.getZeroArgumentConstructor();
result = ConstructorHelper.instantiate(c);
private Identity getIdentityFromResultSet()
{
try
{
// 1. get an empty instance of the target class
Constructor c = classDescriptor.getZeroArgumentConstructor();
Object obj = ConstructorHelper.instantiate(c);
2. RowReader gives me an interception point in the class mapping process which is awesome. But if I want to generically hook it I have to set the row-reader attribute of every class-descriptor individually. I was surprised that I couldn't set the global default in OJB.properties like I can with so many other classes. I modified ClassDescriptor.getRowReader() to read the name of the default RowReader from OJB.properties if it exists:
/**
* Returns the {@link org.apache.ojb.broker.accesslayer.RowReader}
* for this descriptor.
*/
public synchronized RowReader getRowReader()
{
if (rowReader == null)
{
Configurator configurator = OjbConfigurator.getInstance();
Configuration config = configurator.getConfigurationFor(null);
Class rrClass = config.getClass("RowReaderDefaultClass",
RowReaderDefaultImpl.class);
setRowReader(rrClass.getName());
}
return this.rowReader;
}
3. On several of my mapped classes it isn't sufficient to just invoke a no-arg constructor and then set all the field values, there's some additional initialization that needs to happen. I can certainly accomplish this by making my classes PersistenceBrokerAware and implementing afterLookup but this means coupling my classes to OJB and goes against OJB's spirit of minimal intrusion. What I'd really like to do is make this part of the mapping instead. So I added an optional attribute to class-descriptor, "initialization-method", that specifies a zero argument method to be invoked after all the fields are set. I then modified RowReaderDefaultImpl to invoke this method (if specified) after all field values are set on a newly created instance. This let's me have a class like:
public class Article
{
int id;
String name;
private Article()
{
}
private init()
{
// invoked after the state of the object is fully restored
// for example maybe add this Article to a global index or do some validation of the state
}
}
and map it in the repository such that init() gets called:
<class-descriptor class="com.foo.Article" table="Article" initialization-method="init">
To do this, I modified ClassDescriptor to have get/setInitializationMethod (as well as adding it to the toXML method):
/**
* optional method to be invoked after instance fields are initialized
*/
private Method initializationMethod;
/**
* sets the initialization method for this descriptor
*/
public void setInitializationMethod(Method newMethod)
{
if (newMethod != null)
{
// make sure it's a no argument method
if (newMethod.getParameterTypes().length > 0)
{
throw new MetadataException("Initialization methods must be zero argument methods: "
+ newMethod.getClass().getName() + "." + newMethod.getName());
}
// make it accessible if it's not already
if (!newMethod.isAccessible())
{
newMethod.setAccessible(true);
}
}
this.initializationMethod = newMethod;
}
/**
* sets the initialization method for this descriptor by name
*/
public void setInitializationMethod(String newMethodName)
{
Method newMethod = null;
if (newMethodName != null)
{
try
{
// see if we have a publicly accessible method by the name
newMethod = getClassOfObject().getMethod(newMethodName, NO_PARAMS);
}
catch (NoSuchMethodException e)
{
try
{
// no publicly accessible method, see if there is a private/protected one
newMethod = getClassOfObject().getDeclaredMethod(newMethodName, NO_PARAMS);
}
catch (NoSuchMethodException e2)
{
// there is no such method available
throw new MetadataException("Invalid initialization method, there is not"
+ " a zero argument method named " + newMethodName
+ " on class " + getClassOfObject().getName() + ".");
}
}
}
setInitializationMethod(newMethod);
}
/**
* Returns the initialization method for this descriptor or null if no
* initialization method is defined.
*/
public synchronized Method getInitializationMethod()
{
return this.initializationMethod;
}
modified RepositoryXmlHandler to read it from the class-descriptor and set it (and added it to RepositoryElements and RepositoryTags):
//set initializationMethod attribute
String initializationMethod = atts.getValue(tags.getTagById(INITIALIZATION_METHOD));
if (isDebug) logger.debug(" " + tags.getTagById(INITIALIZATION_METHOD) + ": " + initializationMethod);
if (initializationMethod != null)
{
m_CurrentCLD.setInitializationMethod(initializationMethod);
}
and modified RowReaderDefaultImpl to invoke it appropriately:
protected Object buildWithReflection(Map row, ClassDescriptor targetClassDescriptor)
{
Object result;
FieldDescriptor fmd;
try
{
// 1. create an empty Object (persistent classes need a public default constructor)
Constructor c = targetClassDescriptor.getZeroArgumentConstructor();
result = ConstructorHelper.instantiate(c);
// 2. fill all scalar attributes of the new object
FieldDescriptor[] fields = targetClassDescriptor.getFieldDescriptions();
for (int i = 0; i < fields.length; i++)
{
fmd = fields[i];
fmd.getPersistentField().set(result, row.get(fmd.getColumnName()));
}
// 3. invoke the initialization method for the class if one is provided
Method initializationMethod = targetClassDescriptor.getInitializationMethod();
if (initializationMethod != null)
{
initializationMethod.invoke(result, NO_ARGS);
}
return result;
}
catch (Exception ex)
{
throw new PersistenceBrokerException("Unable to build object instance (MAYBE you don't have a constructor available):" + cld.getClassOfObject(), ex);
}
}
I've attached my modified files
Re: Some small suggestions...
Posted by Thomas Mahler <th...@web.de>.
Hi Lance,
I like all 3 suggestions!
I reviewed them and changed the code base accordingly.
It's already in CVS!
thanks for your useful improvements,
Thomas
Lance Eason wrote:
> I have three minor suggestions for improvements that would make my life easier in using OJB. All the code is based off of a pull from CVS yesterday...
>
> 1. I was very happy to see that thanks to the logic in ConstructorHelper my class doesn't have to provide a public no argument constructor, I can keep it protected or private and still have everything work. The problem is that using a non-public constructor currently causes an exception to be thrown and the constructor definition to be looked up for each instance that is created which is a significant performance hit. I modified ClassDescriptor to provide a getZeroArgumentConstructor() method that looks up the constructor once and remembers:
>
> private static final Class[] NO_PARAMS = {};
>
> /**
> * the zero argument constructor for this class
> */
> private Constructor zeroArgumentConstructor = null;
> /**
> * whether we have already tried to look up the zero argument constructor
> */
> private boolean alreadyLookedupZeroArguments = false;
>
> /**
> * returns the zero argument constructor for the class represented by this class descriptor
> * or null if a zero argument constructor does not exist. If the zero argument constructor
> * for this class is not public it is made accessible before being returned.
> */
> public Constructor getZeroArgumentConstructor()
> {
> if (zeroArgumentConstructor == null && !alreadyLookedupZeroArguments)
> {
> try
> {
> zeroArgumentConstructor = getClassOfObject().getConstructor(NO_PARAMS);
> }
> catch (NoSuchMethodException e)
> {
> //no public zero argument constructor available let's try for a private/protected one
> try
> {
> zeroArgumentConstructor = getClassOfObject().getDeclaredConstructor(NO_PARAMS);
>
> //we found one, now let's make it accessible
> zeroArgumentConstructor.setAccessible(true);
> }
> catch (NoSuchMethodException e2)
> {
> //out of options, log the fact and let the method return null
>
> LoggerFactory.getDefaultLogger().warn(this.getClass().getName() + ": " +
> "No zero argument constructor defined for "
> + this.getClassOfObject());
> }
> }
>
> alreadyLookedupZeroArguments = true;
> }
>
> return zeroArgumentConstructor;
> }
>
> This pretty much mirrors the as yet unused getConstructor() that is already on ClassDescriptor for returning a multi-argument constructor. I then added a signature to ConstructorHelper that takes a Constructor:
>
> public static final Object[] NO_ARGS = {};
>
> /**
> * create a new instance of the class represented by the no-argument constructor provided
> * @param constructor the zero argument constructor for the class
> * @return a new instance of the class
> * @throws InstantiationException
> * @throws ClassNotPersistenceCapableException if the constructor is null or there is an
> * exception while trying to create a new instance
> */
> public static Object instantiate(Constructor constructor) throws InstantiationException
> {
> if (constructor == null)
> {
> throw new ClassNotPersistenceCapableException(
> "A zero argument constructor was not provided!");
> }
>
> Object result = null;
>
> try
> {
> result = constructor.newInstance(NO_ARGS);
> }
> catch (Throwable t)
> {
> throw new ClassNotPersistenceCapableException(
> "Could not instantiate "
> + constructor.getDeclaringClass().getName()
> + ": "
> + t.getMessage() + ")");
> }
>
> return result;
> }
>
> And modified RowReaderDefaultImpl.buildWithReflection() and PkEnumeration.getIdentityFromResultSet() to use the new signature:
>
> protected Object buildWithReflection(Map row, ClassDescriptor targetClassDescriptor)
> {
> Object result;
> FieldDescriptor fmd;
> try
> {
> // 1. create an empty Object (persistent classes need a public default constructor)
> Constructor c = targetClassDescriptor.getZeroArgumentConstructor();
> result = ConstructorHelper.instantiate(c);
>
> private Identity getIdentityFromResultSet()
> {
> try
> {
> // 1. get an empty instance of the target class
> Constructor c = classDescriptor.getZeroArgumentConstructor();
> Object obj = ConstructorHelper.instantiate(c);
>
>
>
> 2. RowReader gives me an interception point in the class mapping process which is awesome. But if I want to generically hook it I have to set the row-reader attribute of every class-descriptor individually. I was surprised that I couldn't set the global default in OJB.properties like I can with so many other classes. I modified ClassDescriptor.getRowReader() to read the name of the default RowReader from OJB.properties if it exists:
>
> /**
> * Returns the {@link org.apache.ojb.broker.accesslayer.RowReader}
> * for this descriptor.
> */
> public synchronized RowReader getRowReader()
> {
> if (rowReader == null)
> {
> Configurator configurator = OjbConfigurator.getInstance();
> Configuration config = configurator.getConfigurationFor(null);
> Class rrClass = config.getClass("RowReaderDefaultClass",
> RowReaderDefaultImpl.class);
>
> setRowReader(rrClass.getName());
> }
> return this.rowReader;
> }
>
>
>
> 3. On several of my mapped classes it isn't sufficient to just invoke a no-arg constructor and then set all the field values, there's some additional initialization that needs to happen. I can certainly accomplish this by making my classes PersistenceBrokerAware and implementing afterLookup but this means coupling my classes to OJB and goes against OJB's spirit of minimal intrusion. What I'd really like to do is make this part of the mapping instead. So I added an optional attribute to class-descriptor, "initialization-method", that specifies a zero argument method to be invoked after all the fields are set. I then modified RowReaderDefaultImpl to invoke this method (if specified) after all field values are set on a newly created instance. This let's me have a class like:
>
> public class Article
> {
> int id;
> String name;
>
> private Article()
> {
> }
>
> private init()
> {
> // invoked after the state of the object is fully restored
> // for example maybe add this Article to a global index or do some validation of the state
> }
> }
>
> and map it in the repository such that init() gets called:
>
> <class-descriptor class="com.foo.Article" table="Article" initialization-method="init">
>
> To do this, I modified ClassDescriptor to have get/setInitializationMethod (as well as adding it to the toXML method):
>
> /**
> * optional method to be invoked after instance fields are initialized
> */
> private Method initializationMethod;
>
> /**
> * sets the initialization method for this descriptor
> */
> public void setInitializationMethod(Method newMethod)
> {
> if (newMethod != null)
> {
> // make sure it's a no argument method
> if (newMethod.getParameterTypes().length > 0)
> {
> throw new MetadataException("Initialization methods must be zero argument methods: "
> + newMethod.getClass().getName() + "." + newMethod.getName());
> }
>
> // make it accessible if it's not already
> if (!newMethod.isAccessible())
> {
> newMethod.setAccessible(true);
> }
> }
>
> this.initializationMethod = newMethod;
> }
>
> /**
> * sets the initialization method for this descriptor by name
> */
> public void setInitializationMethod(String newMethodName)
> {
> Method newMethod = null;
>
> if (newMethodName != null)
> {
> try
> {
> // see if we have a publicly accessible method by the name
> newMethod = getClassOfObject().getMethod(newMethodName, NO_PARAMS);
> }
> catch (NoSuchMethodException e)
> {
> try
> {
> // no publicly accessible method, see if there is a private/protected one
> newMethod = getClassOfObject().getDeclaredMethod(newMethodName, NO_PARAMS);
> }
> catch (NoSuchMethodException e2)
> {
> // there is no such method available
> throw new MetadataException("Invalid initialization method, there is not"
> + " a zero argument method named " + newMethodName
> + " on class " + getClassOfObject().getName() + ".");
> }
> }
> }
>
> setInitializationMethod(newMethod);
> }
>
> /**
> * Returns the initialization method for this descriptor or null if no
> * initialization method is defined.
> */
> public synchronized Method getInitializationMethod()
> {
> return this.initializationMethod;
> }
>
> modified RepositoryXmlHandler to read it from the class-descriptor and set it (and added it to RepositoryElements and RepositoryTags):
>
> //set initializationMethod attribute
> String initializationMethod = atts.getValue(tags.getTagById(INITIALIZATION_METHOD));
> if (isDebug) logger.debug(" " + tags.getTagById(INITIALIZATION_METHOD) + ": " + initializationMethod);
> if (initializationMethod != null)
> {
> m_CurrentCLD.setInitializationMethod(initializationMethod);
> }
>
> and modified RowReaderDefaultImpl to invoke it appropriately:
>
> protected Object buildWithReflection(Map row, ClassDescriptor targetClassDescriptor)
> {
> Object result;
> FieldDescriptor fmd;
> try
> {
>
> // 1. create an empty Object (persistent classes need a public default constructor)
> Constructor c = targetClassDescriptor.getZeroArgumentConstructor();
> result = ConstructorHelper.instantiate(c);
>
> // 2. fill all scalar attributes of the new object
> FieldDescriptor[] fields = targetClassDescriptor.getFieldDescriptions();
> for (int i = 0; i < fields.length; i++)
> {
> fmd = fields[i];
> fmd.getPersistentField().set(result, row.get(fmd.getColumnName()));
> }
>
> // 3. invoke the initialization method for the class if one is provided
> Method initializationMethod = targetClassDescriptor.getInitializationMethod();
> if (initializationMethod != null)
> {
> initializationMethod.invoke(result, NO_ARGS);
> }
>
> return result;
> }
> catch (Exception ex)
> {
> throw new PersistenceBrokerException("Unable to build object instance (MAYBE you don't have a constructor available):" + cld.getClassOfObject(), ex);
> }
> }
>
>
>
> I've attached my modified files
>
>
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-dev-help@db.apache.org