diff --git a/framework/src/play/data/binding/As.java b/framework/src/play/data/binding/As.java index bd892514b8..e0b2dace56 100644 --- a/framework/src/play/data/binding/As.java +++ b/framework/src/play/data/binding/As.java @@ -19,9 +19,9 @@ Class> binder() default DEFAULT.class; Class> unbinder() default DEFAULT.class; - public static final class DEFAULT implements TypeBinder, TypeUnbinder { + final class DEFAULT implements TypeBinder, TypeUnbinder { @Override - public Object bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) throws Exception { + public Object bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) { throw new UnsupportedOperationException("Not supported."); } diff --git a/framework/src/play/data/binding/Binder.java b/framework/src/play/data/binding/Binder.java index eb2652503c..198ecbc04a 100644 --- a/framework/src/play/data/binding/Binder.java +++ b/framework/src/play/data/binding/Binder.java @@ -17,6 +17,7 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.math.BigDecimal; +import java.text.ParseException; import java.util.*; @@ -124,9 +125,9 @@ public static Object bind(RootParamNode parentParamNode, String name, Class c Method method = methodAndParamInfo.method; Method defaultMethod = method.getDeclaringClass().getDeclaredMethod(method.getName() + "$default$" + methodAndParamInfo.parameterIndex); return defaultMethod.invoke(methodAndParamInfo.objectInstance); - } catch (NoSuchMethodException e) { - // + } catch (NoSuchMethodException ignore) { } catch (Exception e) { + logBindingNormalFailure(paramNode, e); throw new UnexpectedException(e); } } @@ -181,7 +182,7 @@ protected static Object internalBind(ParamNode paramNode, Class clazz, Type t } if (Map.class.isAssignableFrom(clazz)) { - return bindMap(clazz, type, paramNode, bindingAnnotations); + return bindMap(type, paramNode, bindingAnnotations); } if (Collection.class.isAssignableFrom(clazz)) { @@ -199,18 +200,38 @@ protected static Object internalBind(ParamNode paramNode, Class clazz, Type t if (clazz.isArray()) { return bindArray(clazz, paramNode, bindingAnnotations); } - - if (!paramNode.getAllChildren().isEmpty()) { - return internalBindBean(clazz, paramNode, bindingAnnotations); - } + + if (!paramNode.getAllChildren().isEmpty()) { + return internalBindBean(clazz, paramNode, bindingAnnotations); + } return null; // give up + } catch (NumberFormatException e) { + logBindingNormalFailure(paramNode, e); + addValidationError(paramNode); + } catch (ParseException e) { + logBindingNormalFailure(paramNode, e); + addValidationError(paramNode); } catch (Exception e) { - Validation.addError(paramNode.getOriginalKey(), "validation.invalid"); + // TODO This is bad catch. I would like to remove it in next version. + logBindingUnexpectedFailure(paramNode, e); + addValidationError(paramNode); } return MISSING; } + private static void addValidationError(ParamNode paramNode) { + Validation.addError(paramNode.getOriginalKey(), "validation.invalid"); + } + + private static void logBindingUnexpectedFailure(ParamNode paramNode, Exception e) { + Logger.error(e, "Failed to bind %s=%s", paramNode.getOriginalKey(), Arrays.toString(paramNode.getValues())); + } + + private static void logBindingNormalFailure(ParamNode paramNode, Exception e) { + Logger.debug("Failed to bind %s=%s: %s", paramNode.getOriginalKey(), Arrays.toString(paramNode.getValues()), e); + } + private static Object bindArray(Class clazz, ParamNode paramNode, BindingAnnotations bindingAnnotations) { Class componentType = clazz.getComponentType(); @@ -238,7 +259,7 @@ private static Object bindArray(Class clazz, ParamNode paramNode, BindingAnno try { Array.set(array, i - invalidItemsCount, directBind(paramNode.getOriginalKey(), bindingAnnotations.annotations, thisValue, componentType, componentType)); } catch (Exception e) { - // bad item.. + Logger.debug("Bad item #%s: %s", i, e); invalidItemsCount++; } } @@ -252,7 +273,7 @@ private static Object bindArray(Class clazz, ParamNode paramNode, BindingAnno try { Array.set(array, i - invalidItemsCount, childValue); } catch (Exception e) { - // bad item.. + Logger.debug("Bad item #%s: %s", i, e); invalidItemsCount++; } } @@ -273,12 +294,24 @@ private static Object bindArray(Class clazz, ParamNode paramNode, BindingAnno return array; } - private static Object internalBindBean(Class clazz, ParamNode paramNode, BindingAnnotations bindingAnnotations) throws Exception { - Object bean = clazz.newInstance(); + private static Object internalBindBean(Class clazz, ParamNode paramNode, BindingAnnotations bindingAnnotations) { + Object bean = createNewInstance(clazz); internalBindBean(paramNode, bean, bindingAnnotations); return bean; } + private static T createNewInstance(Class clazz) { + try { + return clazz.newInstance(); + } catch (InstantiationException e) { + Logger.warn("Failed to create instance of %s: %s", clazz.getName(), e); + throw new UnexpectedException(e); + } catch (IllegalAccessException e) { + Logger.warn("Failed to create instance of %s: %s", clazz.getName(), e); + throw new UnexpectedException(e); + } + } + /** * Invokes the plugins before using the internal bindBean. */ @@ -294,8 +327,9 @@ public static void bindBean(RootParamNode rootParamNode, String name, Object bea try { internalBindBean(paramNode, bean, new BindingAnnotations()); - } catch (Exception e) { - Validation.addError(paramNode.getOriginalKey(), "validation.invalid"); + } catch (NumberFormatException e) { + logBindingNormalFailure(paramNode, e); + addValidationError(paramNode); } } @@ -303,11 +337,11 @@ public static void bindBean(RootParamNode rootParamNode, String name, Object bea /** * Does NOT invoke plugins */ - public static void bindBean(ParamNode paramNode, Object bean, Annotation[] annotations) throws Exception { + public static void bindBean(ParamNode paramNode, Object bean, Annotation[] annotations) { internalBindBean(paramNode, bean, new BindingAnnotations(annotations)); } - private static void internalBindBean(ParamNode paramNode, Object bean, BindingAnnotations bindingAnnotations) throws Exception { + private static void internalBindBean(ParamNode paramNode, Object bean, BindingAnnotations bindingAnnotations) { BeanWrapper bw = getBeanWrapper(bean.getClass()); for (BeanWrapper.Property prop : bw.getWrappers()) { @@ -336,7 +370,7 @@ private static void internalBindBean(ParamNode paramNode, Object bean, BindingAn } @SuppressWarnings("unchecked") - private static Object bindEnum(Class clazz, ParamNode paramNode) throws Exception { + private static Object bindEnum(Class clazz, ParamNode paramNode) { if (paramNode.getValues() == null) { return MISSING; } @@ -349,7 +383,7 @@ private static Object bindEnum(Class clazz, ParamNode paramNode) throws Excep return Enum.valueOf((Class) clazz, value); } - private static Object bindMap(Class clazz, Type type, ParamNode paramNode, BindingAnnotations bindingAnnotations) throws Exception { + private static Object bindMap(Type type, ParamNode paramNode, BindingAnnotations bindingAnnotations) { Class keyClass = String.class; Class valueClass = String.class; if (type instanceof ParameterizedType) { @@ -367,8 +401,15 @@ private static Object bindMap(Class clazz, Type type, ParamNode paramNode, Bi valueObject = null; } r.put(keyObject, valueObject); - } catch (Exception e) { + } catch (ParseException e) { + // Just ignore the exception and continue on the next item + logBindingNormalFailure(paramNode, e); + } catch (NumberFormatException e) { // Just ignore the exception and continue on the next item + logBindingNormalFailure(paramNode, e); + } catch (Exception e) { + // TODO This is bad catch. I would like to remove it in next version. + logBindingUnexpectedFailure(paramNode, e); } } @@ -376,7 +417,7 @@ private static Object bindMap(Class clazz, Type type, ParamNode paramNode, Bi } @SuppressWarnings("unchecked") - private static Object bindCollection(Class clazz, Type type, ParamNode paramNode, BindingAnnotations bindingAnnotations) throws Exception { + private static Object bindCollection(Class clazz, Type type, ParamNode paramNode, BindingAnnotations bindingAnnotations) { if (clazz.isInterface()) { if (clazz.equals(List.class)) { clazz = ArrayList.class; @@ -412,9 +453,9 @@ private static Object bindCollection(Class clazz, Type type, ParamNode paramN for (Annotation annotation : bindingAnnotations.annotations) { if (annotation.annotationType().equals(As.class)) { As as = ((As) annotation); - final String separator = as.value()[0]; - if (separator != null && !separator.isEmpty()){ - values = values[0].split(separator); + String separator = as.value()[0]; + if (separator != null && !separator.isEmpty()) { + values = values[0].split(separator); } } } @@ -424,7 +465,7 @@ private static Object bindCollection(Class clazz, Type type, ParamNode paramN if (clazz.equals(EnumSet.class)) { l = EnumSet.noneOf(componentClass); } else { - l = (Collection) clazz.newInstance(); + l = (Collection) createNewInstance(clazz); } boolean hasMissing = false; for (int i = 0; i < values.length; i++) { @@ -437,6 +478,7 @@ private static Object bindCollection(Class clazz, Type type, ParamNode paramN } } catch (Exception e) { // Just ignore the exception and continue on the next item + logBindingNormalFailure(paramNode, e); // TODO debug or error? } } if(hasMissing && l.size() == 0){ @@ -445,7 +487,7 @@ private static Object bindCollection(Class clazz, Type type, ParamNode paramN return l; } - Collection r = (Collection) clazz.newInstance(); + Collection r = (Collection) createNewInstance(clazz); if (List.class.isAssignableFrom(clazz)) { // Must add items at position resolved from each child's key @@ -502,9 +544,9 @@ public int compare(String arg0, String arg1) { * @param value * @param clazz * @return The binding object - * @throws Exception + * @throws ParseException */ - public static Object directBind(String value, Class clazz) throws Exception { + public static Object directBind(String value, Class clazz) throws ParseException { return directBind(null, value, clazz, null); } @@ -514,9 +556,9 @@ public static Object directBind(String value, Class clazz) throws Exception { * @param value * @param clazz * @return The binding object - * @throws Exception + * @throws ParseException */ - public static Object directBind(String name, Annotation[] annotations, String value, Class clazz) throws Exception { + public static Object directBind(String name, Annotation[] annotations, String value, Class clazz) throws ParseException { return directBind(name, annotations, value, clazz, null); } @@ -526,9 +568,9 @@ public static Object directBind(String name, Annotation[] annotations, String va * @param clazz * @param type * @return The binding object - * @throws Exception + * @throws ParseException */ - public static Object directBind(Annotation[] annotations, String value, Class clazz, Type type) throws Exception { + public static Object directBind(Annotation[] annotations, String value, Class clazz, Type type) throws ParseException { return directBind(null, annotations, value, clazz, type); } @@ -541,9 +583,8 @@ public static Object directBind(Annotation[] annotations, String value, Class * @param clazz * @param type * @return The binding object - * @throws Exception */ - public static Object directBind(String name, Annotation[] annotations, String value, Class clazz, Type type) throws Exception { + public static Object directBind(String name, Annotation[] annotations, String value, Class clazz, Type type) throws ParseException { // calls the direct binding and returns null if no value could be resolved.. Object r = internalDirectBind(name, annotations, value, clazz, type); if ( r == DIRECTBINDING_NO_RESULT) { @@ -555,7 +596,7 @@ public static Object directBind(String name, Annotation[] annotations, String va // If internalDirectBind was not able to bind it, it returns a special variable instance: DIRECTBIND_MISSING // Needs this because sometimes we need to know if no value was returned.. - private static Object internalDirectBind(String name, Annotation[] annotations, String value, Class clazz, Type type) throws Exception { + private static Object internalDirectBind(String name, Annotation[] annotations, String value, Class clazz, Type type) throws ParseException { boolean nullOrEmpty = value == null || value.trim().length() == 0; if (annotations != null) { @@ -564,7 +605,7 @@ private static Object internalDirectBind(String name, Annotation[] annotations, Class> toInstanciate = ((As) annotation).binder(); if (!(toInstanciate.equals(As.DEFAULT.class))) { // Instantiate the binder - TypeBinder myInstance = toInstanciate.newInstance(); + TypeBinder myInstance = createNewInstance(toInstanciate); return myInstance.bind(name, annotations, value, clazz, type); } } @@ -576,7 +617,7 @@ private static Object internalDirectBind(String name, Annotation[] annotations, if (c.isAnnotationPresent(Global.class)) { Class forType = (Class) ((ParameterizedType) c.getGenericInterfaces()[0]).getActualTypeArguments()[0]; if (forType.isAssignableFrom(clazz)) { - Object result = c.newInstance().bind(name, annotations, value, clazz, type); + Object result = createNewInstance(c).bind(name, annotations, value, clazz, type); if (result != null) { return result; } @@ -687,6 +728,4 @@ private static Object internalDirectBind(String name, Annotation[] annotations, return DIRECTBINDING_NO_RESULT; } - - } diff --git a/framework/src/play/data/binding/TypeBinder.java b/framework/src/play/data/binding/TypeBinder.java index ef7111e71f..3546cbd5e7 100644 --- a/framework/src/play/data/binding/TypeBinder.java +++ b/framework/src/play/data/binding/TypeBinder.java @@ -2,6 +2,7 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Type; +import java.text.ParseException; /** * Supported type for binding. This interface is used to implement custom binders. @@ -18,8 +19,8 @@ public interface TypeBinder { * @param actualClass The class of the object you want to associate the value with * @param genericType The generic type associated with the object you want to bound the value to * @return the 'bound' object for example a date object if the value was '12/12/2002' - * @throws Exception + * @throws ParseException if parameter has invalid format (e.g. date) */ - Object bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) throws Exception; + Object bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) throws ParseException; } diff --git a/framework/src/play/data/binding/types/CalendarBinder.java b/framework/src/play/data/binding/types/CalendarBinder.java index 6ad0fd32ac..cfa2abcb68 100644 --- a/framework/src/play/data/binding/types/CalendarBinder.java +++ b/framework/src/play/data/binding/types/CalendarBinder.java @@ -18,24 +18,20 @@ public class CalendarBinder implements TypeBinder { @Override - public Calendar bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) throws Exception { + public Calendar bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) throws ParseException { if (value == null || value.trim().length() == 0) { return null; } Calendar cal = Calendar.getInstance(Lang.getLocale()); - try { - Date date = AnnotationHelper.getDateAs(annotations, value); - if (date != null) { - cal.setTime(date); - } else { - SimpleDateFormat sdf = new SimpleDateFormat(I18N.getDateFormat()); - sdf.setLenient(false); - cal.setTime(sdf.parse(value)); - } - } catch (ParseException e) { - throw new IllegalArgumentException("Cannot convert [" + value + "] to a Calendar: " + e.toString()); - } + Date date = AnnotationHelper.getDateAs(annotations, value); + if (date != null) { + cal.setTime(date); + } else { + SimpleDateFormat sdf = new SimpleDateFormat(I18N.getDateFormat()); + sdf.setLenient(false); + cal.setTime(sdf.parse(value)); + } return cal; } } diff --git a/framework/src/play/data/binding/types/DateBinder.java b/framework/src/play/data/binding/types/DateBinder.java index 5fd10f6083..9595c56dfb 100644 --- a/framework/src/play/data/binding/types/DateBinder.java +++ b/framework/src/play/data/binding/types/DateBinder.java @@ -17,7 +17,7 @@ public class DateBinder implements TypeBinder { public static final String ISO8601 = "'ISO8601:'yyyy-MM-dd'T'HH:mm:ssZ"; @Override - public Date bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) throws Exception { + public Date bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) throws ParseException { if (value == null || value.trim().length() == 0) { return null; } @@ -35,13 +35,8 @@ public Date bind(String name, Annotation[] annotations, String value, Class actu // Ignore } - try { - SimpleDateFormat sdf = new SimpleDateFormat(ISO8601); - sdf.setLenient(false); - return sdf.parse(value); - } catch (Exception e) { - throw new IllegalArgumentException("Cannot convert [" + value + "] to a Date: " + e.toString()); - } - + SimpleDateFormat sdf = new SimpleDateFormat(ISO8601); + sdf.setLenient(false); + return sdf.parse(value); } } diff --git a/framework/src/play/data/binding/types/DateTimeBinder.java b/framework/src/play/data/binding/types/DateTimeBinder.java index 3fb6d1ecea..ea8ab4da80 100644 --- a/framework/src/play/data/binding/types/DateTimeBinder.java +++ b/framework/src/play/data/binding/types/DateTimeBinder.java @@ -2,6 +2,7 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Type; +import java.text.ParseException; import org.joda.time.DateTime; @@ -15,7 +16,7 @@ public class DateTimeBinder implements TypeBinder { private static DateBinder dateBinder = new DateBinder(); @Override - public DateTime bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) throws Exception { + public DateTime bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) throws ParseException { if (value == null || value.trim().length() == 0) { return null; } diff --git a/framework/test-src/play/data/binding/Data3.java b/framework/test-src/play/data/binding/Data3.java index f1c862a9e1..f66f4d126a 100644 --- a/framework/test-src/play/data/binding/Data3.java +++ b/framework/test-src/play/data/binding/Data3.java @@ -23,10 +23,10 @@ public static class GenericType { public static class TestGenericTypeBinder implements TypeBinder> { @Override - public Object bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) throws Exception { + public Object bind(String name, Annotation[] annotations, String value, Class actualClass, Type genericType) { ParameterizedType pt = (ParameterizedType) genericType; if (!Long.class.equals(pt.getActualTypeArguments()[0])) { - throw new IllegalArgumentException("Wrong generic type passed. Does not match class declatarion."); + throw new IllegalArgumentException("Wrong generic type passed. Does not match class declaration."); } Long longValue = Long.valueOf(value); diff --git a/framework/test-src/play/data/binding/types/CalendarBinderTest.java b/framework/test-src/play/data/binding/types/CalendarBinderTest.java new file mode 100644 index 0000000000..4d60054c6b --- /dev/null +++ b/framework/test-src/play/data/binding/types/CalendarBinderTest.java @@ -0,0 +1,46 @@ +package play.data.binding.types; + +import org.junit.Before; +import org.junit.Test; +import play.Play; +import play.PlayBuilder; + +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.Calendar; +import java.util.Date; + +import static org.junit.Assert.*; + +public class CalendarBinderTest { + + private CalendarBinder binder = new CalendarBinder(); + + @Before + public void setup() { + new PlayBuilder().build(); + } + + @Test + public void parses_date_to_calendar() throws ParseException { + Play.configuration.setProperty("date.format", "dd.MM.yyyy"); + Date expected = new SimpleDateFormat("dd.MM.yyyy").parse("31.12.1986"); + Calendar actual = binder.bind("client.birthday", null, "31.12.1986", Calendar.class, null); + assertEquals(expected, actual.getTime()); + } + + @Test + public void parses_null_to_null() throws ParseException { + assertNull(binder.bind("client.birthday", null, null, Calendar.class, null)); + } + + @Test + public void parses_empty_string_to_null() throws ParseException { + assertNull(binder.bind("client.birthday", null, "", Calendar.class, null)); + } + + @Test(expected = ParseException.class) + public void throws_ParseException_for_invalid_value() throws ParseException { + binder.bind("client.birthday", null, "12/31/1986", Calendar.class, null); + } +} \ No newline at end of file diff --git a/framework/test-src/play/data/binding/types/DateBinderTest.java b/framework/test-src/play/data/binding/types/DateBinderTest.java new file mode 100644 index 0000000000..e3822b3381 --- /dev/null +++ b/framework/test-src/play/data/binding/types/DateBinderTest.java @@ -0,0 +1,53 @@ +package play.data.binding.types; + +import org.junit.Before; +import org.junit.Test; +import play.Play; +import play.PlayBuilder; + +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.Date; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +public class DateBinderTest { + private DateBinder binder = new DateBinder(); + + @Before + public void setup() { + new PlayBuilder().build(); + } + + @Test + public void parses_date_in_play_format() throws ParseException { + Play.configuration.setProperty("date.format", "dd.MM.yyyy"); + + Date actual = binder.bind("client.birthday", null, "31.12.1986", Date.class, null); + Date expected = new SimpleDateFormat("MM/dd/yyyy").parse("12/31/1986"); + assertEquals(expected, actual); + } + + @Test + public void parses_date_in_iso_format() throws ParseException { + Date actual = binder.bind("client.birthday", null, "ISO8601:1986-04-12T00:00:00+0500", Date.class, null); + Date expected = new SimpleDateFormat("MM/dd/yyyyZ").parse("04/12/1986+0500"); + assertEquals(expected, actual); + } + + @Test + public void parses_null_to_null() throws ParseException { + assertNull(binder.bind("client.birthday", null, null, Date.class, null)); + } + + @Test + public void parses_empty_string_to_null() throws ParseException { + assertNull(binder.bind("client.birthday", null, "", Date.class, null)); + } + + @Test(expected = ParseException.class) + public void throws_ParseException_for_invalid_value() throws ParseException { + binder.bind("client.birthday", null, "12/31/1986", Date.class, null); + } +} \ No newline at end of file