From a19227188bea1cc6f8197853b13ef0f08bfe0fd0 Mon Sep 17 00:00:00 2001 From: j2blake Date: Wed, 5 Jun 2013 15:37:05 -0400 Subject: [PATCH] VIVO-120 improve the error messages from OperationUtils.cloneBean() Add unit tests for the method and restructure the logic so a failure to create the clone is treated as seriously as a failure to load it with values. Previously a failure to create resulted in a null return (with a log message) while a failure to load resulted in an exception (with a log message). --- .../mannlib/vedit/util/OperationUtils.java | 280 ++++++------ .../util/OperationUtils_CloneBeanTest.java | 431 ++++++++++++++++++ 2 files changed, 574 insertions(+), 137 deletions(-) create mode 100644 webapp/test/edu/cornell/mannlib/vedit/util/OperationUtils_CloneBeanTest.java diff --git a/webapp/src/edu/cornell/mannlib/vedit/util/OperationUtils.java b/webapp/src/edu/cornell/mannlib/vedit/util/OperationUtils.java index e0fbd4c21..809d41eda 100644 --- a/webapp/src/edu/cornell/mannlib/vedit/util/OperationUtils.java +++ b/webapp/src/edu/cornell/mannlib/vedit/util/OperationUtils.java @@ -1,7 +1,7 @@ /* $This file is distributed under the terms of the license in /doc/license.txt$ */ -package edu.cornell.mannlib.vedit.util; - +package edu.cornell.mannlib.vedit.util; + import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -9,139 +9,145 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import edu.cornell.mannlib.vedit.beans.EditProcessObject; - - -public class OperationUtils{ - - private static final Log log = LogFactory.getLog(OperationUtils.class.getName()); - - public static void beanSetAndValidate(Object newObj, String field, String value, EditProcessObject epo){ - Class cls = (epo.getBeanClass() != null) ? epo.getBeanClass() : newObj.getClass(); - Class[] paramList = new Class[1]; - paramList[0] = String.class; - boolean isInt = false; - boolean isBoolean = false; - Method setterMethod = null; - try { - setterMethod = cls.getMethod("set"+field,paramList); - } catch (NoSuchMethodException e) { - //let's try int - paramList[0] = int.class; - try { - setterMethod = cls.getMethod("set"+field,paramList); - isInt = true; - } catch (NoSuchMethodException f) { - //let's try boolean - paramList[0]=boolean.class; - try { - setterMethod = cls.getMethod("set"+field,paramList); - isBoolean = true; - log.debug("found boolean field "+field); - } catch (NoSuchMethodException g) { - log.error("beanSet could not find an appropriate String, int, or boolean setter method for "+field); - } - - } - } - Object[] arglist = new Object[1]; - if (isInt) - arglist[0] = Integer.decode(value); - else if (isBoolean) - arglist[0] = (value.equalsIgnoreCase("TRUE")); - else - arglist[0] = value; - try { - setterMethod.invoke(newObj,arglist); - } catch (Exception e) { - log.error("Couldn't invoke method"); - log.error(e.getMessage()); - log.error(field+" "+arglist[0]); - } - } - - /** - * Takes a bean and clones it using reflection. - * Any fields without standard getter/setter methods will not be copied. - * @param bean - * @return - */ - public static Object cloneBean (Object bean) { - return cloneBean(bean, bean.getClass(), bean.getClass()); - } - - /** - * Takes a bean and clones it using reflection. - * Any fields without standard getter/setter methods will not be copied. - * @param bean - * @return - */ - public static Object cloneBean (Object bean, Class beanClass, Class iface){ - Object newBean = null; - try { - newBean = beanClass.newInstance(); - Method[] beanMeths = iface.getMethods(); - for (int i=0; i cls = (epo.getBeanClass() != null) ? epo.getBeanClass() : newObj + .getClass(); + Class[] paramList = new Class[1]; + paramList[0] = String.class; + boolean isInt = false; + boolean isBoolean = false; + Method setterMethod = null; + try { + setterMethod = cls.getMethod("set" + field, paramList); + } catch (NoSuchMethodException e) { + // let's try int + paramList[0] = int.class; + try { + setterMethod = cls.getMethod("set" + field, paramList); + isInt = true; + } catch (NoSuchMethodException f) { + // let's try boolean + paramList[0] = boolean.class; + try { + setterMethod = cls.getMethod("set" + field, paramList); + isBoolean = true; + log.debug("found boolean field " + field); + } catch (NoSuchMethodException g) { + log.error("beanSet could not find an appropriate String, int, or boolean setter method for " + + field); + } + + } + } + Object[] arglist = new Object[1]; + if (isInt) + arglist[0] = Integer.decode(value); + else if (isBoolean) + arglist[0] = (value.equalsIgnoreCase("TRUE")); + else + arglist[0] = value; + try { + setterMethod.invoke(newObj, arglist); + } catch (Exception e) { + log.error("Couldn't invoke method"); + log.error(e.getMessage()); + log.error(field + " " + arglist[0]); + } + } + + /** + * Takes a bean and clones it using reflection. Any fields without standard + * getter/setter methods will not be copied. + */ + public static Object cloneBean(Object bean) { + if (bean == null) { + throw new NullPointerException("bean may not be null."); + } + return cloneBean(bean, bean.getClass(), bean.getClass()); + } + + /** + * Takes a bean and clones it using reflection. Any fields without standard + * getter/setter methods will not be copied. + */ + public static Object cloneBean(final Object bean, final Class beanClass, + final Class iface) { + if (bean == null) { + throw new NullPointerException("bean may not be null."); + } + if (beanClass == null) { + throw new NullPointerException("beanClass may not be null."); + } + if (iface == null) { + throw new NullPointerException("iface may not be null."); + } + + class CloneBeanException extends RuntimeException { + public CloneBeanException(String message, Throwable cause) { + super(message + " <" + cause.getClass().getSimpleName() + + ">: bean=" + bean + ", beanClass=" + + beanClass.getName() + ", iface=" + iface.getName(), + cause); + } + } + + Object newBean; + try { + newBean = beanClass.getConstructor().newInstance(); + } catch (NoSuchMethodException e) { + throw new CloneBeanException("bean has no 'nullary' constructor.", e); + } catch (InstantiationException e) { + throw new CloneBeanException("tried to create instance of an abstract class.", e); + } catch (IllegalAccessException e) { + throw new CloneBeanException("bean constructor is not accessible.", e); + } catch (InvocationTargetException e) { + throw new CloneBeanException("bean constructor threw an exception.", e); + } catch (Exception e) { + throw new CloneBeanException("failed to instantiate a new bean.", e); + } + + for (Method beanMeth : iface.getMethods()) { + String methName = beanMeth.getName(); + if (!methName.startsWith("get")) { + continue; + } + if (beanMeth.getParameterTypes().length != 0) { + continue; + } + String fieldName = methName.substring(3, methName.length()); + Class returnType = beanMeth.getReturnType(); + + Method setterMethod; + try { + setterMethod = iface.getMethod("set" + fieldName, returnType); + } catch (NoSuchMethodException nsme) { + continue; + } + + Object fieldVal; + try { + fieldVal = beanMeth.invoke(bean, (Object[]) null); + } catch (Exception e) { + throw new CloneBeanException("failed to invoke " + beanMeth, e); + } + + try { + Object[] setArgs = new Object[1]; + setArgs[0] = fieldVal; + setterMethod.invoke(newBean, setArgs); + } catch (Exception e) { + throw new CloneBeanException( + "failed to invoke " + setterMethod, e); + } + } + return newBean; + } + } \ No newline at end of file diff --git a/webapp/test/edu/cornell/mannlib/vedit/util/OperationUtils_CloneBeanTest.java b/webapp/test/edu/cornell/mannlib/vedit/util/OperationUtils_CloneBeanTest.java new file mode 100644 index 000000000..83f93278c --- /dev/null +++ b/webapp/test/edu/cornell/mannlib/vedit/util/OperationUtils_CloneBeanTest.java @@ -0,0 +1,431 @@ +/* $This file is distributed under the terms of the license in /doc/license.txt$ */ + +package edu.cornell.mannlib.vedit.util; + +import static org.junit.Assert.assertEquals; + +import java.lang.reflect.InvocationTargetException; +import java.util.HashMap; +import java.util.Map; + +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.hamcrest.Matcher; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import edu.cornell.mannlib.vitro.testing.AbstractTestClass; + +/** + * All of these tests are for OperationUtils.cloneBean() + */ +public class OperationUtils_CloneBeanTest extends AbstractTestClass { + + // ---------------------------------------------------------------------- + // Allow the tests to expect a RuntimeException with a particular cause. + // ---------------------------------------------------------------------- + + @Rule + public ExpectedException thrown = ExpectedException.none(); + + Matcher causedBy(Class causeClass) { + return new CausedByMatcher(causeClass); + } + + class CausedByMatcher extends BaseMatcher { + private final Class causeClass; + + public CausedByMatcher(Class causeClass) { + this.causeClass = causeClass; + } + + @Override + public boolean matches(Object actualThrowable) { + if (!(actualThrowable instanceof RuntimeException)) { + return false; + } + Throwable cause = ((RuntimeException) actualThrowable).getCause(); + return causeClass.isInstance(cause); + } + + @Override + public void describeTo(Description d) { + d.appendText("RuntimeException caused by " + causeClass.getName()); + } + } + + // ---------------------------------------------------------------------- + // Test for invalid classes + // ---------------------------------------------------------------------- + + @Test(expected = NullPointerException.class) + public void nullArgument() { + OperationUtils.cloneBean(null); + } + + @Test(expected = NullPointerException.class) + public void nullBean() { + OperationUtils + .cloneBean(null, SimpleSuccess.class, SimpleSuccess.class); + } + + @Test(expected = NullPointerException.class) + public void nullBeanClass() { + OperationUtils + .cloneBean(new SimpleSuccess(), null, SimpleSuccess.class); + } + + @Test(expected = NullPointerException.class) + public void nullInterfaceClass() { + OperationUtils + .cloneBean(new SimpleSuccess(), SimpleSuccess.class, null); + } + + @Test(expected = IllegalAccessException.class) + @Ignore("Why doesn't this throw an exception?") + public void privateClass() { + OperationUtils.cloneBean(new PrivateClass()); + } + + @Test + public void privateConstructor() { + thrown.expect(causedBy(NoSuchMethodException.class)); + OperationUtils.cloneBean(new PrivateConstructor()); + } + + @Test + public void abstractClass() { + thrown.expect(causedBy(InstantiationException.class)); + OperationUtils.cloneBean(new ConcreteOfAbstractClass(), + AbstractClass.class, AbstractClass.class); + } + + @Test + public void interfaceClass() { + thrown.expect(causedBy(InstantiationException.class)); + OperationUtils.cloneBean(new ConcreteOfInterfaceClass(), + InterfaceClass.class, InterfaceClass.class); + } + + @Test + public void arrayClass() { + thrown.expect(causedBy(NoSuchMethodException.class)); + OperationUtils.cloneBean(new String[0]); + } + + @Test + public void primitiveTypeClass() { + thrown.expect(causedBy(NoSuchMethodException.class)); + OperationUtils.cloneBean(1, Integer.TYPE, Integer.TYPE); + } + + @Test + public void voidClass() { + thrown.expect(causedBy(NoSuchMethodException.class)); + OperationUtils.cloneBean(new Object(), Void.TYPE, Void.TYPE); + } + + @Test + public void noNullaryConstructor() { + thrown.expect(causedBy(NoSuchMethodException.class)); + OperationUtils.cloneBean(new NoNullaryConstructor(1)); + } + + @Test(expected = ExceptionInInitializerError.class) + public void classThrowsExceptionWhenLoaded() { + OperationUtils.cloneBean(new ThrowsExceptionWhenLoaded()); + } + + @Test + public void initializerThrowsException() { + thrown.expect(causedBy(InvocationTargetException.class)); + OperationUtils.cloneBean("random object", + InitializerThrowsException.class, + InitializerThrowsException.class); + } + + @Test + public void wrongInterfaceClass() { + thrown.expect(causedBy(IllegalArgumentException.class)); + OperationUtils.cloneBean(new WrongConcreteClass(), + WrongConcreteClass.class, WrongInterface.class); + } + + @Test + public void getThrowsException() { + thrown.expect(causedBy(InvocationTargetException.class)); + OperationUtils.cloneBean(new GetMethodThrowsException()); + } + + @Test + public void setThrowsException() { + thrown.expect(causedBy(InvocationTargetException.class)); + OperationUtils.cloneBean(new SetMethodThrowsException()); + } + + private static class PrivateClass { + public PrivateClass() { + } + } + + private static class PrivateConstructor { + private PrivateConstructor() { + } + } + + public abstract static class AbstractClass { + public AbstractClass() { + } + } + + public static class ConcreteOfAbstractClass extends AbstractClass { + public ConcreteOfAbstractClass() { + } + } + + public abstract static class InterfaceClass { + public InterfaceClass() { + } + } + + public static class ConcreteOfInterfaceClass extends InterfaceClass { + public ConcreteOfInterfaceClass() { + } + } + + public static class NoNullaryConstructor { + @SuppressWarnings("unused") + public NoNullaryConstructor(int i) { + // nothing to do + } + } + + public static class ThrowsExceptionWhenLoaded { + static { + if (true) + throw new IllegalArgumentException(); + } + } + + public static class InitializerThrowsException { + { + if (true) + throw new IllegalStateException("Initializer throws exception"); + } + } + + public static class WrongConcreteClass { + private String junk = "junk"; + + public String getJunk() { + return this.junk; + } + + @SuppressWarnings("unused") + private void setJunk(String junk) { + this.junk = junk; + } + } + + public static interface WrongInterface { + String getJunk(); + + void setJunk(String junk); + } + + public static class GetMethodThrowsException { + @SuppressWarnings("unused") + private String junk = "junk"; + + public String getJunk() { + throw new UnsupportedOperationException(); + } + + public void setJunk(String junk) { + this.junk = junk; + } + } + + public static class SetMethodThrowsException { + private String junk = "junk"; + + public String getJunk() { + return this.junk; + } + + @SuppressWarnings("unused") + public void setJunk(String junk) { + throw new UnsupportedOperationException(); + } + } + + // ---------------------------------------------------------------------- + // Test simple success and innocuous variations + // ---------------------------------------------------------------------- + + @Test + public void simpleSuccess() { + expectSuccess(new SimpleSuccess().insertField("label", "a prize")); + } + + @Test + public void getButNoSet() { + expectSuccess(new GetButNoSet().insertField("label", "shouldBeEqual")); + } + + @Test + public void getTakesParameters() { + expectSuccess(new GetTakesParameters().insertField("label", "fine")); + } + + @Test + public void getReturnsVoid() { + expectSuccess(new GetReturnsVoid().insertField("label", "fine")); + } + + @Test + public void getAndSetDontMatch() { + expectSuccess(new GetAndSetDontMatch().insertField("label", "fine")); + } + + @Test + public void getIsStatic() { + expectSuccess(new GetIsStatic().insertField("label", "fine") + .insertField("instanceJunk", "the junk")); + } + + @Test + public void getMethodIsPrivate() { + expectSuccess(new GetMethodIsPrivate().insertField("label", "fine")); + } + + @Test + public void setMethodIsPrivate() { + expectSuccess(new SetMethodIsPrivate().insertField("label", "fine")); + } + + private void expectSuccess(BeanBase original) { + BeanBase cloned = (BeanBase) OperationUtils.cloneBean(original); + assertEquals("Simple success", original, cloned); + } + + public static abstract class BeanBase { + protected final Map fields = new HashMap<>(); + + public BeanBase insertField(String key, Object value) { + if (value != null) { + fields.put(key, value); + } + return this; + } + + @Override + public int hashCode() { + return fields.hashCode(); + } + + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } + if (o == null) { + return false; + } + if (!this.getClass().equals(o.getClass())) { + return false; + } + return this.fields.equals(((BeanBase) o).fields); + } + + @Override + public String toString() { + return this.getClass().getSimpleName() + fields; + } + } + + public static class SimpleSuccess extends BeanBase { + public String getLabel() { + return (String) fields.get("label"); + } + + public void setLabel(String label) { + insertField("label", label); + } + } + + public static class GetButNoSet extends SimpleSuccess { + public String getJunk() { + throw new UnsupportedOperationException(); + } + } + + public static class GetTakesParameters extends SimpleSuccess { + @SuppressWarnings("unused") + public String getJunk(String why) { + throw new UnsupportedOperationException(); + } + + @SuppressWarnings("unused") + public void setJunk(String junk) { + throw new UnsupportedOperationException(); + } + } + + public static class GetReturnsVoid extends SimpleSuccess { + public void getJunk() { + throw new UnsupportedOperationException(); + } + + @SuppressWarnings("unused") + public void setJunk(String junk) { + throw new UnsupportedOperationException(); + } + } + + public static class GetAndSetDontMatch extends SimpleSuccess { + public String getJunk() { + throw new UnsupportedOperationException(); + } + + @SuppressWarnings("unused") + public void setJunk(Integer junk) { + throw new UnsupportedOperationException(); + } + } + + public static class GetIsStatic extends SimpleSuccess { + public static String getJunk() { + return ("the junk"); + } + + public void setJunk(String junk) { + insertField("instanceJunk", junk); + } + } + + public static class GetMethodIsPrivate extends SimpleSuccess { + @SuppressWarnings("unused") + private String getJunk() { + return ("the junk"); + } + + public void setJunk(String junk) { + insertField("instanceJunk", junk); + } + } + + public static class SetMethodIsPrivate extends SimpleSuccess { + public String getJunk() { + return ("the junk"); + } + + @SuppressWarnings("unused") + private void setJunk(String junk) { + insertField("instanceJunk", junk); + } + } + +}