craigmcc 2002/12/24 10:49:52
Modified: src/share/org/apache/struts/config FormPropertyConfig.java
src/test/org/apache/struts/action TestDynaActionForm.java
TestDynaActionFormClass.java
src/test/org/apache/struts/mock MockPrincipal.java
TestMockBase.java
src/test/org/apache/struts/util TestRequestUtils.java
Log:
The second half of the fix for 14800. The problem with mutable property
values is *not* limited to array elements -- it will also occur when a
scalar property is of a mutable type. As a solution to this, initial()
now calculates and returns a new value every time, rather than caching
the old one. This is a relatively minor performance hit, but not (in
principle at least) any worse than evaluating the initialization expression
for an instance variable in a JavaBean every time one is created.
As an added benefit, the initial() method will also attempt to create
instances of properties (via a zero-args constructor) if you did not specify
an "initial" value to pass in to ConvertUtils.convert(). This is true both
for scalar properties and for elements of arrays constructed with the
"size" attribute. In general, DynaActionForm beans now act very much more
like standard ActionForm beans with respect to preinitialized property
values than they did before these changes.
PR: Bugzilla #14800
Submitted by: James Turner <turner at blackbear.com>
Revision Changes Path
1.9 +62 -49
jakarta-struts/src/share/org/apache/struts/config/FormPropertyConfig.java
Index: FormPropertyConfig.java
===================================================================
RCS file:
/home/cvs/jakarta-struts/src/share/org/apache/struts/config/FormPropertyConfig.java,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -r1.8 -r1.9
--- FormPropertyConfig.java 23 Dec 2002 22:00:24 -0000 1.8
+++ FormPropertyConfig.java 24 Dec 2002 18:49:52 -0000 1.9
@@ -138,18 +138,6 @@
protected boolean configured = false;
- /**
- * Have we calculated the initial value object yet?
- */
- protected transient boolean initialized = false;
-
-
- /**
- * The calculated initial value for this property.
- */
- protected transient Object initialValue = null;
-
-
// ------------------------------------------------------------- Properties
@@ -189,7 +177,8 @@
/**
* <p>The size of the array to be created if this property is an array
- * type and there is no specified <code>initial</code> value.</p>
+ * type and there is no specified <code>initial</code> value. This
+ * value must be non-negative.</p>
*
* @since Struts 1.1-b3
*/
@@ -203,6 +192,9 @@
if (configured) {
throw new IllegalStateException("Configuration is frozen");
}
+ if (size < 0) {
+ throw new IllegalArgumentException("size < 0");
+ }
this.size = size;
}
@@ -288,46 +280,67 @@
/**
- * Return an object representing the initial value of this property.
+ * <p>Return an object representing the initial value of this property.
+ * This is calculated according to the following algorithm:</p>
+ * <ul>
+ * <li>If the value you have specified for the <code>type</code>
+ * property represents an array (i.e. it ends with "[]"):
+ * <ul>
+ * <li>If you have specified a value for the <code>initial</code>
+ * property, <code>ConvertUtils.convert()</code> will be
+ * called to convert it into an instance of the specified
+ * array type.</li>
+ * <li>If you have not specified a value for the <code>initial</code>
+ * property, an array of the length specified by the
+ * <code>size</code> property will be created. Each element
+ * of the array will be instantiated via the zero-args constructor
+ * on the specified class (if any). Otherwise, <code>null</code>
+ * will be returned.</li>
+ * </ul></li>
+ * <li>If the value you have specified for the <code>type</code>
+ * property does not represent an array:
+ * <ul>
+ * <li>If you have specified a value for the <code>initial</code>
+ * property, <code>ConvertUtils.convert()</code>
+ * will be called to convert it into an object instance.</li>
+ * <li>If you have not specified a value for the <code>initial</code>
+ * attribute, Struts will instantiate an instance via the
+ * zero-args constructor on the specified class (if any).
+ * Otherwise, <code>null</code> will be returned.</li>
+ * </ul></li>
+ * </ul>
*/
public Object initial() {
- // Compute our initial value the first time it is requested
- // Don't bother synchronizing, a race is basically harmless
- if (!initialized) {
- try {
- Class clazz = getTypeClass();
- if (clazz.isArray()) {
- if (initial != null) {
- initialValue =
- ConvertUtils.convert(initial, clazz);
- } else {
- initialValue =
- Array.newInstance(clazz.getComponentType(), size);
- }
+ Object initialValue = null;
+ try {
+ Class clazz = getTypeClass();
+ if (clazz.isArray()) {
+ if (initial != null) {
+ initialValue =
+ ConvertUtils.convert(initial, clazz);
} else {
+ initialValue =
+ Array.newInstance(clazz.getComponentType(), size);
+ for (int i = 0; i < size; i++) {
+ try {
+ Array.set(initialValue, i,
+ clazz.getComponentType().newInstance());
+ } catch (Throwable t) {
+ ; // Probably does not have a zero-args constructor
+ }
+ }
+ }
+ } else {
+ if (initial != null) {
initialValue = ConvertUtils.convert(initial, clazz);
+ } else {
+ initialValue = clazz.newInstance();
}
- } catch (Throwable t) {
- initialValue = null;
}
- initialized = true;
+ } catch (Throwable t) {
+ initialValue = null;
}
-
- // Clone if the initial value is an array
- if ((initialValue != null) &&
- (initialValue.getClass().isArray())) {
- int n = Array.getLength(initialValue);
- Class componentType =
- initialValue.getClass().getComponentType();
- Object newValue = Array.newInstance(componentType, n);
- for (int j = 0; j < n; j++) {
- Array.set(newValue, j, Array.get(initialValue, j));
- }
- return (newValue);
- }
-
- // Return the calculated value
return (initialValue);
}
1.9 +3 -3
jakarta-struts/src/test/org/apache/struts/action/TestDynaActionForm.java
Index: TestDynaActionForm.java
===================================================================
RCS file:
/home/cvs/jakarta-struts/src/test/org/apache/struts/action/TestDynaActionForm.java,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -r1.8 -r1.9
--- TestDynaActionForm.java 23 Dec 2002 22:00:24 -0000 1.8
+++ TestDynaActionForm.java 24 Dec 2002 18:49:52 -0000 1.9
@@ -149,7 +149,7 @@
"longProperty",
"mappedProperty",
"mappedIntProperty",
- "nullProperty",
+ // "nullProperty",
"shortProperty",
"stringArray",
"stringIndexed",
@@ -209,8 +209,8 @@
(Long) dynaForm.get("longProperty"));
// FIXME - mappedProperty
// FIXME - mappedIntProperty
- assertEquals("nullProperty", (String) null,
- (String) dynaForm.get("nullProperty"));
+ // assertEquals("nullProperty", (String) null,
+ // (String) dynaForm.get("nullProperty"));
assertEquals("shortProperty", new Short((short) 987),
(Short) dynaForm.get("shortProperty"));
assertEquals("stringProperty", "This is a string",
1.7 +3 -3
jakarta-struts/src/test/org/apache/struts/action/TestDynaActionFormClass.java
Index: TestDynaActionFormClass.java
===================================================================
RCS file:
/home/cvs/jakarta-struts/src/test/org/apache/struts/action/TestDynaActionFormClass.java,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- TestDynaActionFormClass.java 16 Nov 2002 04:58:47 -0000 1.6
+++ TestDynaActionFormClass.java 24 Dec 2002 18:49:52 -0000 1.7
@@ -135,7 +135,7 @@
new FormPropertyConfig("longProperty", "long", "321"),
new FormPropertyConfig("mappedProperty", "java.util.Map", null),
new FormPropertyConfig("mappedIntProperty", "java.util.Map", null),
- new FormPropertyConfig("nullProperty", "java.lang.String", null),
+ // new FormPropertyConfig("nullProperty", "java.lang.String", null),
new FormPropertyConfig("shortProperty", "short", "987"),
new FormPropertyConfig("stringArray", "java.lang.String[]",
"{ 'String 0', 'String 1', 'String 2', 'String 3',
'String 4'}"),
@@ -262,8 +262,8 @@
beanConfig.findFormPropertyConfig("longProperty").initial());
// FIXME - mappedProperty
// FIXME - mappedIntProperty
- assertNull("nullProperty value",
- beanConfig.findFormPropertyConfig("nullProperty").initial());
+ // assertNull("nullProperty value",
+ //
beanConfig.findFormPropertyConfig("nullProperty").initial());
assertEquals("shortProperty value",
new Short((short) 987),
beanConfig.findFormPropertyConfig("shortProperty").initial());
1.2 +36 -4
jakarta-struts/src/test/org/apache/struts/mock/MockPrincipal.java
Index: MockPrincipal.java
===================================================================
RCS file:
/home/cvs/jakarta-struts/src/test/org/apache/struts/mock/MockPrincipal.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- MockPrincipal.java 1 Jul 2002 22:10:35 -0000 1.1
+++ MockPrincipal.java 24 Dec 2002 18:49:52 -0000 1.2
@@ -87,6 +87,13 @@
public class MockPrincipal implements Principal {
+ public MockPrincipal() {
+ super();
+ this.name = "";
+ this.roles = new String[0];
+ }
+
+
public MockPrincipal(String name) {
super();
this.name = name;
@@ -119,6 +126,31 @@
}
}
return (false);
+ }
+
+
+ public boolean equals(Object o) {
+ if (o == null) {
+ return (false);
+ }
+ if (!(o instanceof Principal)) {
+ return (false);
+ }
+ Principal p = (Principal) o;
+ if (name == null) {
+ return (p.getName() == null);
+ } else {
+ return (name.equals(p.getName()));
+ }
+ }
+
+
+ public int hashCode() {
+ if (name == null) {
+ return ("".hashCode());
+ } else {
+ return (name.hashCode());
+ }
}
1.9 +8 -4 jakarta-struts/src/test/org/apache/struts/mock/TestMockBase.java
Index: TestMockBase.java
===================================================================
RCS file:
/home/cvs/jakarta-struts/src/test/org/apache/struts/mock/TestMockBase.java,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -r1.8 -r1.9
--- TestMockBase.java 23 Dec 2002 22:00:24 -0000 1.8
+++ TestMockBase.java 24 Dec 2002 18:49:52 -0000 1.9
@@ -231,6 +231,10 @@
(new FormPropertyConfig("intArray2", "int[]",
null, 5)); // 5 should be respected
formBean.addFormPropertyConfig
+ (new FormPropertyConfig("principal",
+ "org.apache.struts.mock.MockPrincipal",
+ null));
+ formBean.addFormPropertyConfig
(new FormPropertyConfig("stringArray1", "java.lang.String[]",
"{aaa,bbb,ccc}", 2)); // 2 should be ignored
formBean.addFormPropertyConfig
1.15 +36 -7
jakarta-struts/src/test/org/apache/struts/util/TestRequestUtils.java
Index: TestRequestUtils.java
===================================================================
RCS file:
/home/cvs/jakarta-struts/src/test/org/apache/struts/util/TestRequestUtils.java,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -r1.14 -r1.15
--- TestRequestUtils.java 23 Dec 2002 22:00:24 -0000 1.14
+++ TestRequestUtils.java 24 Dec 2002 18:49:52 -0000 1.15
@@ -79,6 +79,7 @@
import org.apache.struts.action.RequestProcessor;
import org.apache.struts.config.ApplicationConfig;
import org.apache.struts.mock.MockFormBean;
+import org.apache.struts.mock.MockPrincipal;
import org.apache.struts.mock.TestMockBase;
import org.apache.struts.taglib.html.Constants;
@@ -1147,6 +1148,11 @@
assertEquals("intArray2[3] value is correct", 0, intArray2[3]);
assertEquals("intArray2[4] value is correct", 0, intArray2[4]);
+ value = dform.get("principal");
+ assertNotNull("principal exists", value);
+ assertTrue("principal is MockPrincipal",
+ value instanceof MockPrincipal);
+
value = dform.get("stringArray1");
assertNotNull("stringArray1 exists", value);
assertTrue("stringArray1 is int[]", value instanceof String[]);
@@ -1165,11 +1171,34 @@
String stringArray2[] = (String[]) value;
assertEquals("stringArray2 length is correct", 3, stringArray2.length);
assertEquals("stringArray2[0] value is correct",
- (String) null, stringArray2[0]);
+ new String(), stringArray2[0]);
assertEquals("stringArray2[1] value is correct",
- (String) null, stringArray2[1]);
+ new String(), stringArray2[1]);
assertEquals("stringArray2[2] value is correct",
- (String) null, stringArray2[2]);
+ new String(), stringArray2[2]);
+
+ // Different form beans should get different property value instances
+ Object value1 = null;
+ DynaActionForm dform1 = (DynaActionForm)
+ RequestUtils.createActionForm(request, mapping, appConfig, null);
+
+ value = dform.get("principal");
+ value1 = dform1.get("principal");
+ assertEquals("Different form beans get equal instance values",
+ value, value1);
+ assertTrue("Different form beans get different instances 1",
+ value != value1);
+
+ value = dform.get("stringArray1");
+ value1 = dform1.get("stringArray1");
+ assertTrue("Different form beans get different instances 2",
+ value != value1);
+
+ dform1.set("stringProperty", "Different stringProperty value");
+ value = dform.get("stringProperty");
+ value1 = dform1.get("stringProperty");
+ assertTrue("Different form beans get different instances 3",
+ value != value1);
}
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
