I'm very openminded this patch. However, you need to submit it (a) in diff format, so I can see exactly what changed ... and use patch to patch the curent code! (b) upload it to JIRA, not post it to the list
Thanks Gavin |---------+-------------------------------------------> | | Vladimir Dozen | | | <[EMAIL PROTECTED]> | | | Sent by: | | | [EMAIL PROTECTED]| | | ceforge.net | | | | | | | | | 15/08/03 08:45 AM | | | | |---------+-------------------------------------------> >------------------------------------------------------------------------------------------------------------------------------| | | | To: [EMAIL PROTECTED] | | cc: | | Subject: [Hibernate] Patch: access via Field, not via Method if requested so | >------------------------------------------------------------------------------------------------------------------------------| ehlo. Background: As it is known, Hibernate uses accessors (getters/setters) to access object properties. This is noted as a RIGHT THING in documentation, since it raises incapsulation. That's right, but I have a few real-world examples that show that direct field access can be (highly) desirable. Problem scenarios: 1. We're working on a relatively large web application. Hibernate works fine, but there is a problem: There are some legacy modules in the system which sometimes provide not completely correct data to the system database. They have direct access to the database, and it cannot be changed right now. At the same time, we in our application use business objects with setters like public void setEmail(String email) { Assert.validEmail(email); this.email = email.trim(); } What happens when legacy stores invalid email (for example) into the system? We unable not only to load the broken object via Hibernate, but also to load a list of objects via HibernateSession.find(), since it breaks too as soon as it attempts to load the object. 2. Another application has a kind of public forum in it, with search ability. It has also built-in filter that prohibits to post the message if the body contains any of restricted words. The filter itself shall take each single word from the body and compare it with set of restricted words. It takes (quite small) time, but the operation repeats every time we load a message, and it becomes real performance hit when search engine loads a bunch of messages to index them. 3. Third application has a lot of POD (plain old data) objects. They has no any logic associated with them (there are special services instead), and the structure of these objects is stable for a very long time. It looks unconvinient to write getters and setters to satisfy a library. We might have various workarounds in first two cases (like having another set of setters for direct access, or moving fields in superclass), but all of them are ugly and add a portion of mess into code instead of making it cleaner. In short: field access can be beneficial in terms of stability, peformance and code simplicity. Patch: I decided that both access strategies are applicable in various situation, and that there shall be opportunity to choose. At the same time, I understand that there is a lot of written code that relays on setter/getters for Hibernate usage, and having global switch "now we use reflect.Field" is not enough: we need a co-existance, so even existing projects (like ours) may switch new objects to field access while leaving old ones with setters. A best decision would be to make the option class-wide (via *.xml configuration), but I started with a more simple approach: I created new configuration variable hibernate.access.order fields-then-accessors When it absent or has value "accessors-then-fields" the things are very like current state: when accessing property "foo" Hibernate will search for method getFoo (isFoo/setFoo) in the object and it's superclasses (so, all existing project will work as usual). If, thought, it cannot find the method, it will search the field "foo" in the object and it's superclasses. When the variable set to "fields-then-accessors", the order is opposite: the fields are scanned first, and methods second. All this functionality is hidden inside ReflectHelper (Getter/Setter), and completely transparent for the rest of Hibernate. Main changes are in ReflectHelper, with minor changes in AbstractPersister and another class (I've forgot the name ;) ). I also added tests for the functionality. I'm attached the changed source, but it's just for reference (to let to see my coding level etc etc etc). I worked over CVS copy got via anonymous CVS. I can commit my changes back if someone will grant commit right to user tut-framework. I could create a new branch for that. Comments? dozen P.S. There is still unresolved problem with proxy classes that accepts Method instead of Getter; it might be resolved quite easy, as I believe, but it changes the interface ClassPersister. //$Id: ReflectHelper.java,v 1.13 2003/08/03 01:15:28 oneovthafew Exp $ package net.sf.hibernate.util; import java.beans.Introspector; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Member; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import net.sf.cglib.MetaClass; import net.sf.hibernate.AssertionFailure; import net.sf.hibernate.HibernateException; import net.sf.hibernate.MappingException; import net.sf.hibernate.PropertyAccessException; import net.sf.hibernate.PropertyNotFoundException; import net.sf.hibernate.cfg.Environment; import net.sf.hibernate.type.Type; import net.sf.hibernate.type.TypeFactory; public final class ReflectHelper { static final Log log = LogFactory.getLog(ReflectHelper.class); private static final Class[] NO_CLASSES = new Class[0]; private static final Class[] OBJECT = new Class[] { Object.class }; private static final Method OBJECT_EQUALS; static Boolean accessFieldsFirst_ = null; public static boolean tryFieldsFirst() { if( accessFieldsFirst_ == null ) { String val = (String)Environment.getProperties().get(Environment.PROPERTIES_ACCESS_ORDER); log.info(Environment.PROPERTIES_ACCESS_ORDER+" property is "+val); if( val == null || val.trim().equalsIgnoreCase("accessors-then-fields") ) { accessFieldsFirst_ = new Boolean(false); } else { accessFieldsFirst_ = new Boolean(true); } log.info("fields-then-methods="+accessFieldsFirst_); } return accessFieldsFirst_.booleanValue(); } private static class Accessor { protected Class clazz; protected final Method method; protected final Field field; protected final String propertyName; protected Accessor(Class c,Method m,Field f,String name) { clazz = c; method = m; field = f; propertyName = name; if( method != null ) method.setAccessible(true); if( field != null ) field.setAccessible(true); } /** * @deprecated */ public Method getMethod() { return method; } /** * @deprecated */ public String getName() { return method != null? method.getName() : field.getName(); } } public static final class Setter extends Accessor { Setter(Class clazz, Method method, String propertyName) { super(clazz, method, null, propertyName); } Setter(Class clazz, Field field, String propertyName) { super(clazz, null, field, propertyName); } public void set(Object target, Object value) throws HibernateException { try { if( method != null ) { method.invoke( target, new Object[] { value } ); } else { field.set(target,value); } } catch (NullPointerException npe) { if ( value==null && method.getParameterTypes()[0].isPrimitive() ) { throw new PropertyAccessException(npe, "Null value was assigned to a property of primitive type", true, clazz, propertyName); } else { throw new PropertyAccessException(npe, "NullPointerException occurred while calling", true, clazz, propertyName); } } catch (InvocationTargetException ite) { throw new PropertyAccessException(ite, "Exception occurred inside", true, clazz, propertyName); } catch (IllegalAccessException iae) { throw new PropertyAccessException(iae, "IllegalAccessException occurred while calling", true, clazz, propertyName); //cannot occur } catch (IllegalArgumentException iae) { if ( value==null && method.getParameterTypes()[0].isPrimitive() ) { throw new PropertyAccessException(iae, "Null value was assigned to a property of primitive type", true, clazz, propertyName); } else { log.error( "IllegalArgumentException in class: " + clazz.getName() + ", setter method of property: " + propertyName ); log.error( "expected type: " + method.getParameterTypes()[0].getName() + ", actual value: " + ( value==null ? null : value.getClass().getName() ) ); throw new PropertyAccessException(iae, "IllegalArgumentException occurred while calling", true, clazz, propertyName); } } } } public static final class Getter extends Accessor { Getter(Class clazz, Method method, String propertyName) { super(clazz,method,null,propertyName); } Getter(Class clazz, Field field, String propertyName) { super(clazz,null,field,propertyName); } public Object get(Object target) throws HibernateException { try { if( method != null ) { return method.invoke(target, null); } else { return field.get(target); } } catch (InvocationTargetException ite) { throw new PropertyAccessException(ite, "Exception occurred inside", false, clazz, propertyName); } catch (IllegalAccessException iae) { throw new PropertyAccessException(iae, "IllegalAccessException occurred while calling", false, clazz, propertyName); //cannot occur } catch (IllegalArgumentException iae) { log.error( "IllegalArgumentException in class: " + clazz.getName() + ", getter method of property: " + propertyName ); throw new PropertyAccessException(iae, "IllegalArgumentException occurred calling", false, clazz, propertyName); } } public Class getReturnType() { return method != null? method.getReturnType():field.getType(); } } public static Setter getSetter(Class theClass, String propertyName) throws PropertyNotFoundException { Setter result = getSetterOrNull(theClass, propertyName); if (result==null) throw new PropertyNotFoundException( "Could not find a setter for property " + propertyName + " in class " + theClass.getName() ); return result; } public static Getter getGetter(Class theClass, String propertyName) throws PropertyNotFoundException { Getter result = getGetterOrNull(theClass, propertyName); if (result==null) throw new PropertyNotFoundException( "Could not find a getter for " + propertyName + " in class " + theClass.getName() ); return result; } private static Setter getSetterOrNull(Class theClass, String propertyName) { if (theClass==Object.class || theClass==null) return null; if( tryFieldsFirst() ) { // fields-then-accessors try { Field field = theClass.getDeclaredField(propertyName); return new Setter(theClass,field,propertyName); } catch( Exception ex ) { } Method method = setterMethod(theClass, propertyName); if(method!=null) { return new Setter(theClass, method, propertyName); } } else { // accessors-then-fields Method method = setterMethod(theClass, propertyName); if(method!=null) { return new Setter(theClass, method, propertyName); } try { Field field = theClass.getDeclaredField(propertyName); return new Setter(theClass,field,propertyName); } catch( Exception ex ) { } } // scan superclasses and interfaces Setter setter = getSetterOrNull( theClass.getSuperclass(), propertyName ); if (setter==null) { Class[] interfaces = theClass.getInterfaces(); for ( int i=0; setter==null && i<interfaces.length; i++ ) { setter=getSetterOrNull( interfaces[i], propertyName ); } } return setter; } private static Getter getGetterOrNull(Class theClass, String propertyName) { if (theClass==Object.class || theClass==null) return null; if( tryFieldsFirst() ) { // fields-then-accessors try { Field field = theClass.getDeclaredField(propertyName); return new Getter(theClass,field,propertyName); } catch( Exception ex ) { } Method method = getterMethod(theClass, propertyName); if(method!=null) { return new Getter(theClass, method, propertyName); } } else { // accessors-then-fields Method method = getterMethod(theClass, propertyName); if(method!=null) { return new Getter(theClass, method, propertyName); } try { Field field = theClass.getDeclaredField(propertyName); return new Getter(theClass,field,propertyName); } catch( Exception ex ) { } } Getter getter = getGetterOrNull( theClass.getSuperclass(), propertyName ); if (getter==null) { Class[] interfaces = theClass.getInterfaces(); for ( int i=0; getter==null && i<interfaces.length; i++ ) { getter=getGetterOrNull( interfaces[i], propertyName ); } } return getter; } private static Method setterMethod(Class theClass, String propertyName) { Getter getter = getGetterOrNull(theClass, propertyName); Class returnType = (getter==null) ? null : getter.getReturnType(); Method[] methods = theClass.getDeclaredMethods(); Method potentialSetter = null; for (int i=0; i<methods.length; i++) { if( ( methods[i].getName().length() > 3 ) && ( methods[i].getName().startsWith("set") ) ) { String testStdMethod = Introspector.decapitalize( methods[i].getName().substring(3) ); String testOldMethod = methods[i].getName().substring(3); if ( ( testStdMethod.equals(propertyName) || testOldMethod.equals(propertyName) ) && ( methods[i].getParameterTypes().length==1 ) ) { potentialSetter = methods[i]; if ( returnType==null || methods[i].getParameterTypes()[0].equals(returnType) ) return potentialSetter; } } } return potentialSetter; } private static Method getterMethod(Class theClass, String propertyName) { Method[] methods = theClass.getDeclaredMethods(); for (int i=0; i<methods.length; i++) { // only carry on if the method has no parameters if(methods[i].getParameterTypes().length==0) { // try "get" if( (methods[i].getName().length() > 3) && methods[i].getName().startsWith("get") ) { String testStdMethod = Introspector.decapitalize( methods[i].getName().substring(3) ); String testOldMethod = methods[i].getName().substring(3); if( testStdMethod.equals(propertyName) || testOldMethod.equals(propertyName) ) return methods[i]; } // if not "get" then try "is" if( (methods[i].getName().length() > 2) && methods[i].getName().startsWith("is") ) { String testStdMethod = Introspector.decapitalize( methods[i].getName().substring(2) ); String testOldMethod = methods[i].getName().substring(2); if( testStdMethod.equals(propertyName) || testOldMethod.equals(propertyName) ) return methods[i]; } } } return null; } public static Type reflectedPropertyType(Class theClass, String name) throws MappingException { return TypeFactory.hueristicType( getGetter(theClass, name).getReturnType().getName() ); } public static Class classForName(String name) throws ClassNotFoundException { try { return Thread.currentThread().getContextClassLoader().loadClass(name); } catch (Exception e) { return Class.forName(name); } } public static boolean isPublic(Class clazz, Member member) { return Modifier.isPublic( member.getModifiers() ) && Modifier.isPublic( clazz.getModifiers() ); } public static Object getConstantValue(String name) { Class clazz; try { clazz = classForName( StringHelper.qualifier(name) ); } catch(ClassNotFoundException cnfe) { return null; } try { return clazz.getField( StringHelper.unqualify(name) ).get(null); } catch (Exception e) { return null; } } public static Constructor getDefaultConstructor(Class clazz) throws PropertyNotFoundException { if (isAbstractClass(clazz)) return null; try { Constructor constructor = clazz.getDeclaredConstructor(NO_CLASSES); if (!isPublic(clazz, constructor)) { constructor.setAccessible(true); } return constructor; } catch (NoSuchMethodException nme) { throw new PropertyNotFoundException( "Object class " + clazz.getName() + " must declare a default (no-argument) constructor" ); } } public static boolean isAbstractClass(Class clazz) { int modifier = clazz.getModifiers(); return (Modifier.isAbstract(modifier) || Modifier.isInterface(modifier)); } public static MetaClass getMetaClass(Class clazz, String[] getterNames, String[] setterNames, Class[] types) { try { MetaClass optimizer = MetaClass.getInstance( clazz.getClassLoader(), clazz, getterNames, setterNames, types ); if ( !clazz.isInterface() ) { //test out the optimizer: optimizer.setPropertyValues( optimizer.newInstance(), optimizer.getPropertyValues( optimizer.newInstance() ) ); } //if working: return optimizer; } catch (Throwable t) { log.info( "reflection optimizer disabled for: " + clazz.getName() + ", " + StringHelper.unqualify( t.getClass().getName() ) + ": " + t.getMessage() ); return null; } } private ReflectHelper() {} static { Method eq; try { eq = Object.class.getMethod("equals", OBJECT); } catch (Exception e) { throw new AssertionFailure("Could not find Object.equals()", e); } OBJECT_EQUALS = eq; } public static boolean overridesEquals(Class clazz) { Method equals; try { equals = clazz.getMethod("equals", OBJECT); } catch (NoSuchMethodException nsme) { return false; //its an interface so we can't really tell anything... } return !OBJECT_EQUALS.equals(equals); } } ********************************************************************** Any personal or sensitive information contained in this email and attachments must be handled in accordance with the Victorian Information Privacy Act 2000, the Health Records Act 2001 or the Privacy Act 1988 (Commonwealth), as applicable. This email, including all attachments, is confidential. If you are not the intended recipient, you must not disclose, distribute, copy or use the information contained in this email or attachments. Any confidentiality or privilege is not waived or lost because this email has been sent to you in error. If you have received it in error, please let us know by reply email, delete it from your system and destroy any copies. ********************************************************************** ------------------------------------------------------- This SF.Net email sponsored by: Free pre-built ASP.NET sites including Data Reports, E-commerce, Portals, and Forums are available now. Download today and enter to win an XBOX or Visual Studio .NET. http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01 _______________________________________________ hibernate-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/hibernate-devel