You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2010/03/28 00:05:07 UTC
svn commit: r928300 - in /tapestry/tapestry5/trunk/tapestry-core/src:
main/java/org/apache/tapestry5/internal/services/
main/resources/org/apache/tapestry5/internal/services/
test/java/org/apache/tapestry5/internal/bindings/
test/java/org/apache/tapest...
Author: hlship
Date: Sat Mar 27 23:05:07 2010
New Revision: 928300
URL: http://svn.apache.org/viewvc?rev=928300&view=rev
Log:
TAP5-1035: Use UnknownValueException when a property conduit expression references an unknown property (or public field)
Modified:
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PropertyConduitSourceImpl.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java
tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/bindings/PropBindingFactoryTest.java
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BeanModelSourceImplTest.java
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PropertyConduitSourceImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PropertyConduitSourceImpl.java?rev=928300&r1=928299&r2=928300&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PropertyConduitSourceImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PropertyConduitSourceImpl.java Sat Mar 27 23:05:07 2010
@@ -39,6 +39,7 @@ import java.lang.reflect.Modifier;
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.Set;
import org.antlr.runtime.ANTLRInputStream;
import org.antlr.runtime.CommonTokenStream;
@@ -62,7 +63,9 @@ import org.apache.tapestry5.ioc.services
import org.apache.tapestry5.ioc.services.PropertyAccess;
import org.apache.tapestry5.ioc.services.PropertyAdapter;
import org.apache.tapestry5.ioc.services.TypeCoercer;
+import org.apache.tapestry5.ioc.util.AvailableValues;
import org.apache.tapestry5.ioc.util.BodyBuilder;
+import org.apache.tapestry5.ioc.util.UnknownValueException;
import org.apache.tapestry5.services.ComponentLayer;
import org.apache.tapestry5.services.InvalidationListener;
import org.apache.tapestry5.services.PropertyConduitSource;
@@ -924,8 +927,9 @@ public class PropertyConduitSourceImpl i
Method method = info.getReadMethod();
if (method == null && !info.isField())
- throw new PropertyExpressionException(ServicesMessages.writeOnlyProperty(info.getDescription(),
- activeType, expression), expression, null);
+ throw new RuntimeException(String.format(
+ "Property '%s' of class %s is not readable (it has no read accessor method).", info
+ .getDescription(), activeType.getName()));
// If a primitive type, convert to wrapper type
@@ -994,12 +998,11 @@ public class PropertyConduitSourceImpl i
for (int i = 0; i < expected.length; i++)
tokenNames.add(PropertyExpressionParser.tokenNames[expected[i]]);
- String message = String.format(
- "Node %s (within expression '%s') was type %s, but was expected to be (one of) %s.", node
- .toStringTree(), expression, PropertyExpressionParser.tokenNames[node.getType()],
- InternalUtils.joinSorted(tokenNames));
+ String message = String.format("Node %s was type %s, but was expected to be (one of) %s.", node
+ .toStringTree(), PropertyExpressionParser.tokenNames[node.getType()], InternalUtils
+ .joinSorted(tokenNames));
- return new PropertyExpressionException(message, expression, null);
+ return new RuntimeException(message);
}
private ExpressionTermInfo infoForMember(Class activeType, Tree node)
@@ -1024,8 +1027,18 @@ public class PropertyConduitSourceImpl i
if (fieldInfo != null)
return fieldInfo;
- throw new PropertyExpressionException(ServicesMessages.noSuchProperty(activeType, propertyName,
- expression, classAdapter.getPropertyNames()), expression, null);
+ Set<String> names = CollectionFactory.newSet();
+
+ names.addAll(classAdapter.getPropertyNames());
+
+ for (Field f : activeType.getFields())
+ {
+ names.add(f.getName());
+ }
+
+ throw new UnknownValueException(String.format(
+ "Class %s does not contain a property (or public field) named '%s'.", activeType.getName(),
+ propertyName), new AvailableValues("properties (and public fields)", names));
}
return createExpressionTermInfoForProperty(adapter);
@@ -1142,8 +1155,8 @@ public class PropertyConduitSourceImpl i
final Method method = findMethod(activeType, methodName, parameterCount);
if (method.getReturnType().equals(void.class))
- throw new PropertyExpressionException(ServicesMessages.methodIsVoid(methodName, activeType,
- expression), expression, null);
+ throw new RuntimeException(String.format("Method %s.%s() returns void.", activeType.getName(),
+ methodName));
final Class genericType = GenericsUtils.extractGenericReturnType(activeType, method);
@@ -1192,8 +1205,8 @@ public class PropertyConduitSourceImpl i
}
catch (NoSuchMethodException ex)
{
- throw new PropertyExpressionException(ServicesMessages.methodNotFound(methodName, activeType,
- expression), expression, ex);
+ throw new RuntimeException(String.format("No public method '%s()' in class %s.", methodName, activeType
+ .getName()));
}
}
@@ -1293,66 +1306,74 @@ public class PropertyConduitSourceImpl i
{
Tree tree = parse(expression);
- switch (tree.getType())
+ try
{
- case TRUE:
+ switch (tree.getType())
+ {
+ case TRUE:
- return literalTrue;
+ return literalTrue;
- case FALSE:
+ case FALSE:
- return literalFalse;
+ return literalFalse;
- case NULL:
+ case NULL:
- return literalNull;
+ return literalNull;
- case INTEGER:
+ case INTEGER:
- // Leading '+' may screw this up.
- // TODO: Singleton instance for "0", maybe "1"?
+ // Leading '+' may screw this up.
+ // TODO: Singleton instance for "0", maybe "1"?
- return createLiteralConduit(Long.class, new Long(tree.getText()));
+ return createLiteralConduit(Long.class, new Long(tree.getText()));
- case DECIMAL:
+ case DECIMAL:
- // Leading '+' may screw this up.
- // TODO: Singleton instance for "0.0"?
+ // Leading '+' may screw this up.
+ // TODO: Singleton instance for "0.0"?
- return createLiteralConduit(Double.class, new Double(tree.getText()));
+ return createLiteralConduit(Double.class, new Double(tree.getText()));
- case STRING:
+ case STRING:
- return createLiteralConduit(String.class, tree.getText());
+ return createLiteralConduit(String.class, tree.getText());
- case RANGEOP:
+ case RANGEOP:
- Tree fromNode = tree.getChild(0);
- Tree toNode = tree.getChild(1);
+ Tree fromNode = tree.getChild(0);
+ Tree toNode = tree.getChild(1);
- // If the range is defined as integers (not properties, etc.)
- // then it is possible to calculate the value here, once, and not
- // build a new class.
+ // If the range is defined as integers (not properties, etc.)
+ // then it is possible to calculate the value here, once, and not
+ // build a new class.
- if (fromNode.getType() != INTEGER || toNode.getType() != INTEGER)
- break;
+ if (fromNode.getType() != INTEGER || toNode.getType() != INTEGER)
+ break;
- int from = Integer.parseInt(fromNode.getText());
- int to = Integer.parseInt(toNode.getText());
+ int from = Integer.parseInt(fromNode.getText());
+ int to = Integer.parseInt(toNode.getText());
- IntegerRange ir = new IntegerRange(from, to);
+ IntegerRange ir = new IntegerRange(from, to);
- return createLiteralConduit(IntegerRange.class, ir);
+ return createLiteralConduit(IntegerRange.class, ir);
- case THIS:
+ case THIS:
- return createLiteralThisPropertyConduit(rootClass);
+ return createLiteralThisPropertyConduit(rootClass);
- default:
- break;
- }
+ default:
+ break;
+ }
- return new PropertyConduitBuilder(rootClass, expression, tree).createInstance();
+ return new PropertyConduitBuilder(rootClass, expression, tree).createInstance();
+ }
+ catch (Exception ex)
+ {
+ throw new PropertyExpressionException(String.format("Exception generating conduit for expression '%s': %s",
+ expression, InternalUtils.toMessage(ex)), expression, ex);
+ }
}
private PropertyConduit createLiteralThisPropertyConduit(final Class rootClass)
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java?rev=928300&r1=928299&r2=928300&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java Sat Mar 27 23:05:07 2010
@@ -207,28 +207,6 @@ public class ServicesMessages
.joinSorted(availableNames));
}
- public static String methodIsVoid(String methodName, Class inClass, String propertyExpression)
- {
- return MESSAGES.format("method-is-void", methodName, inClass.getName(), propertyExpression);
- }
-
- public static String methodNotFound(String methodName, Class inClass, String propertyExpression)
- {
- return MESSAGES.format("method-not-found", methodName, inClass.getName(), propertyExpression);
- }
-
- public static String noSuchProperty(Class targetClass, String propertyName, String propertyExpression,
- Collection<String> propertyNames)
- {
- return MESSAGES.format("no-such-property", targetClass.getName(), propertyName, propertyExpression,
- InternalUtils.joinSorted(propertyNames));
- }
-
- public static String writeOnlyProperty(String propertyName, Class clazz, String propertyExpression)
- {
- return MESSAGES.format("write-only-property", propertyName, clazz.getName(), propertyExpression);
- }
-
public static String requestException(Throwable cause)
{
return MESSAGES.format("request-exception", cause);
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties?rev=928300&r1=928299&r2=928300&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties Sat Mar 27 23:05:07 2010
@@ -48,10 +48,6 @@ undefined-tapestry-attribute=Element <%s
attribute-not-allowed=Element <%s> does not support any attributes.
parameter-element-name-required=The name attribute of the <parameter> element must be specified.
missing-application-state-persistence-strategy=No application state persistence strategy is available with name '%s'. Available strategies: %s.
-method-is-void=Method '%s()' returns void (in class %s, within property expression '%s').
-method-not-found=No public method '%s()' in class %s (within property expression '%s').
-no-such-property=Class %s does not contain a property named '%s' (within property expression '%s'). Available properties: %s.
-write-only-property=Property '%s' of class %s (within property expression '%s') is not readable (it has no read accessor method).
request-exception=Processing of request failed with uncaught exception: %s
component-recursion=The template for component %s is recursive (contains another direct or indirect reference to component %<s). \
This is not supported (components may not contain themselves).
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/bindings/PropBindingFactoryTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/bindings/PropBindingFactoryTest.java?rev=928300&r1=928299&r2=928300&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/bindings/PropBindingFactoryTest.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/bindings/PropBindingFactoryTest.java Sat Mar 27 23:05:07 2010
@@ -1,10 +1,10 @@
-// Copyright 2006, 2007, 2008, 2009 The Apache Software Foundation
+// Copyright 2006, 2007, 2008, 2009, 2010 The Apache Software Foundation
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
@@ -140,12 +140,7 @@ public class PropBindingFactoryTest exte
replay();
- Binding binding = factory.newBinding(
- "test binding",
- resources,
- null,
- "stringHolderMethod()",
- l);
+ Binding binding = factory.newBinding("test binding", resources, null, "stringHolderMethod()", l);
assertNotNull(binding.getAnnotation(BeforeRenderBody.class));
@@ -177,12 +172,7 @@ public class PropBindingFactoryTest exte
replay();
- Binding binding = factory.newBinding(
- "test binding",
- resources,
- null,
- "stringHolder.value",
- l);
+ Binding binding = factory.newBinding("test binding", resources, null, "stringHolder.value", l);
assertSame(binding.getBindingType(), String.class);
@@ -194,9 +184,7 @@ public class PropBindingFactoryTest exte
assertEquals(bean.getStringHolder().getValue(), "second");
- assertEquals(
- binding.toString(),
- "PropBinding[test binding foo.Bar:baz(stringHolder.value)]");
+ assertEquals(binding.toString(), "PropBinding[test binding foo.Bar:baz(stringHolder.value)]");
verify();
}
@@ -213,12 +201,7 @@ public class PropBindingFactoryTest exte
replay();
- Binding binding = factory.newBinding(
- "test binding",
- resources,
- null,
- "stringHolderMethod().value",
- l);
+ Binding binding = factory.newBinding("test binding", resources, null, "stringHolderMethod().value", l);
assertSame(binding.getBindingType(), String.class);
@@ -226,9 +209,7 @@ public class PropBindingFactoryTest exte
assertEquals(binding.get(), "first");
- assertEquals(
- binding.toString(),
- "PropBinding[test binding foo.Bar:baz(stringHolderMethod().value)]");
+ assertEquals(binding.toString(), "PropBinding[test binding foo.Bar:baz(stringHolderMethod().value)]");
verify();
}
@@ -242,12 +223,7 @@ public class PropBindingFactoryTest exte
replay();
- Binding binding = factory.newBinding(
- "test binding",
- resources,
- null,
- "stringHolderMethod().stringValue()",
- l);
+ Binding binding = factory.newBinding("test binding", resources, null, "stringHolderMethod().stringValue()", l);
assertSame(binding.getBindingType(), String.class);
@@ -289,9 +265,8 @@ public class PropBindingFactoryTest exte
}
catch (RuntimeException ex)
{
- assertEquals(
- ex.getMessage(),
- "No public method \'isThatRealBlood()\' in class org.apache.tapestry5.internal.bindings.TargetBean (within property expression \'isThatRealBlood().value\').");
+ assertMessageContains(ex,
+ "No public method \'isThatRealBlood()\' in class org.apache.tapestry5.internal.bindings.TargetBean");
}
verify();
@@ -310,19 +285,13 @@ public class PropBindingFactoryTest exte
try
{
- factory.newBinding(
- "test binding",
- resources,
- null,
- "stringHolder.isThatRealBlood()",
- l);
+ factory.newBinding("test binding", resources, null, "stringHolder.isThatRealBlood()", l);
unreachable();
}
catch (RuntimeException ex)
{
- assertEquals(
- ex.getMessage(),
- "No public method \'isThatRealBlood()\' in class org.apache.tapestry5.internal.bindings.StringHolder (within property expression \'stringHolder.isThatRealBlood()\').");
+ assertMessageContains(ex,
+ "No public method \'isThatRealBlood()\' in class org.apache.tapestry5.internal.bindings.StringHolder");
}
verify();
@@ -346,9 +315,8 @@ public class PropBindingFactoryTest exte
}
catch (RuntimeException ex)
{
- assertEquals(
- ex.getMessage(),
- "Method \'voidMethod()\' returns void (in class org.apache.tapestry5.internal.bindings.TargetBean, within property expression \'voidMethod().value\').");
+ assertMessageContains(ex,
+ "Method org.apache.tapestry5.internal.bindings.TargetBean.voidMethod() returns void");
}
verify();
@@ -372,9 +340,8 @@ public class PropBindingFactoryTest exte
}
catch (RuntimeException ex)
{
- assertEquals(
- ex.getMessage(),
- "Method \'voidMethod()\' returns void (in class org.apache.tapestry5.internal.bindings.StringHolder, within property expression \'stringHolder.voidMethod()\').");
+ assertMessageContains(ex,
+ "Method org.apache.tapestry5.internal.bindings.StringHolder.voidMethod() returns void");
}
verify();
@@ -400,10 +367,9 @@ public class PropBindingFactoryTest exte
}
catch (RuntimeException ex)
{
- assertEquals(
- ex.getMessage(),
- "Class org.apache.tapestry5.internal.bindings.StringHolder does not contain a property named \'missingProperty\' "
- + "(within property expression \'stringHolder.missingProperty.terminalProperty\'). Available properties: value.");
+ assertMessageContains(ex,
+ "Class org.apache.tapestry5.internal.bindings.StringHolder does not contain a property",
+ "\'missingProperty\'");
}
verify();
@@ -429,9 +395,9 @@ public class PropBindingFactoryTest exte
}
catch (RuntimeException ex)
{
- assertEquals(
- ex.getMessage(),
- "Property \'writeOnly\' of class org.apache.tapestry5.internal.bindings.TargetBean (within property expression \'writeOnly.terminalProperty\') is not readable (it has no read accessor method).");
+ assertMessageContains(ex,
+ "Property \'writeOnly\' of class org.apache.tapestry5.internal.bindings.TargetBean",
+ "is not readable (it has no read accessor method).");
}
verify();
@@ -481,8 +447,7 @@ public class PropBindingFactoryTest exte
}
catch (TapestryException ex)
{
- assertEquals(
- ex.getMessage(),
+ assertEquals(ex.getMessage(),
"Expression 'readOnly' for class org.apache.tapestry5.internal.bindings.TargetBean is read-only.");
assertEquals(ex.getLocation(), l);
}
@@ -512,8 +477,7 @@ public class PropBindingFactoryTest exte
}
catch (TapestryException ex)
{
- assertEquals(
- ex.getMessage(),
+ assertEquals(ex.getMessage(),
"Expression writeOnly for class org.apache.tapestry5.internal.bindings.TargetBean is write-only.");
assertEquals(ex.getLocation(), l);
}
@@ -539,11 +503,9 @@ public class PropBindingFactoryTest exte
}
catch (RuntimeException ex)
{
- assertEquals(
- ex.getMessage(),
- "Class org.apache.tapestry5.internal.bindings.TargetBean does not contain a property named \'missingProperty\' "
- + "(within property expression \'missingProperty\'). "
- + "Available properties: class, componentResources, intValue, objectValue, readOnly, stringHolder, writeOnly.");
+ assertMessageContains(ex,
+ "Class org.apache.tapestry5.internal.bindings.TargetBean does not contain a property",
+ "\'missingProperty\'");
}
verify();
@@ -577,29 +539,28 @@ public class PropBindingFactoryTest exte
public Object[][] values()
{
return new Object[][]
- {
- { "true", true, },
- { "True", true, },
- { " true ", true, },
- { "false", false },
- { "null", null },
- { "3", 3l },
- { " 37 ", 37l },
- { " -227", -227l },
- { " 5.", 5d },
- { " -100.", -100d },
- { " -0.0 ", -0d },
- { "+50", 50l },
- { "+7..+20", new IntegerRange(7, 20) },
- { "+5.5", 5.5d },
- { "1..10", new IntegerRange(1, 10) },
- { " -20 .. -30 ", new IntegerRange(-20, -30) },
- { "0.", 0d },
- { " 227.75", 227.75d },
- { " -10123.67", -10123.67d },
- { "'Hello World'", "Hello World" },
- { " 'Whitespace Ignored' ", "Whitespace Ignored" },
- { " ' Inside ' ", " Inside " }
- };
+ {
+ { "true", true, },
+ { "True", true, },
+ { " true ", true, },
+ { "false", false },
+ { "null", null },
+ { "3", 3l },
+ { " 37 ", 37l },
+ { " -227", -227l },
+ { " 5.", 5d },
+ { " -100.", -100d },
+ { " -0.0 ", -0d },
+ { "+50", 50l },
+ { "+7..+20", new IntegerRange(7, 20) },
+ { "+5.5", 5.5d },
+ { "1..10", new IntegerRange(1, 10) },
+ { " -20 .. -30 ", new IntegerRange(-20, -30) },
+ { "0.", 0d },
+ { " 227.75", 227.75d },
+ { " -10123.67", -10123.67d },
+ { "'Hello World'", "Hello World" },
+ { " 'Whitespace Ignored' ", "Whitespace Ignored" },
+ { " ' Inside ' ", " Inside " } };
}
}
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BeanModelSourceImplTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BeanModelSourceImplTest.java?rev=928300&r1=928299&r2=928300&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BeanModelSourceImplTest.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BeanModelSourceImplTest.java Sat Mar 27 23:05:07 2010
@@ -579,7 +579,7 @@ public class BeanModelSourceImplTest ext
}
catch (PropertyExpressionException ex)
{
- assertMessageContains(ex, "does not contain a property named 'doesNotExist'");
+ assertMessageContains(ex, "does not contain", "doesNotExist");
}
verify();