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/22 20:12:15 UTC
svn commit: r926257 - in
/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5:
internal/services/ services/
Author: hlship
Date: Mon Mar 22 19:12:15 2010
New Revision: 926257
URL: http://svn.apache.org/viewvc?rev=926257&view=rev
Log:
TAP5-1067: Created component constructor may use too many parameters
Removed:
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ConstructorArg.java
Modified:
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/AbstractInstantiator.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformation.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TransformField.java
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/AbstractInstantiator.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/AbstractInstantiator.java?rev=926257&r1=926256&r2=926257&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/AbstractInstantiator.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/AbstractInstantiator.java Mon Mar 22 19:12:15 2010
@@ -1,10 +1,10 @@
-// Copyright 2008 The Apache Software Foundation
+// Copyright 2008, 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,
@@ -25,10 +25,13 @@ public abstract class AbstractInstantiat
private final String description;
- public AbstractInstantiator(ComponentModel model, String description)
+ protected final Object[] constructorArgs;
+
+ public AbstractInstantiator(ComponentModel model, String description, Object[] constructorArgs)
{
this.model = model;
this.description = description;
+ this.constructorArgs = constructorArgs;
}
@Override
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformation.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformation.java?rev=926257&r1=926256&r2=926257&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformation.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformation.java Mon Mar 22 19:12:15 2010
@@ -61,7 +61,7 @@ public interface InternalClassTransforma
/**
* Returns a copy of the list of constructor arguments for this class.
*/
- List<ConstructorArg> getConstructorArgs();
+ List<Object> getConstructorArgs();
/**
* Searchs for an existing injection of an object, returning the name of the protected field into which the value
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java?rev=926257&r1=926256&r2=926257&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java Mon Mar 22 19:12:15 2010
@@ -685,9 +685,9 @@ public final class InternalClassTransfor
failIfFrozen();
- String argName = addConstructorArg(providerType, provider);
+ String argReference = addConstructorArg(providerType, provider);
- addToConstructor(String.format(" %s = (%s) %s.get(%s);", name, type, argName, resourcesFieldName));
+ addToConstructor(String.format(" %s = (%s) (%s).get(%s);", name, type, argReference, resourcesFieldName));
makeReadOnly(name);
}
@@ -714,7 +714,7 @@ public final class InternalClassTransfor
*/
private StringBuilder constructor = new StringBuilder(INIT_BUFFER_SIZE);
- private final List<ConstructorArg> constructorArgs;
+ private final List<Object> constructorArgs;
private final ComponentModel componentModel;
@@ -771,7 +771,10 @@ public final class InternalClassTransfor
addImplementedInterface(Component.class);
- resourcesFieldName = addInjectedFieldUncached(InternalComponentResources.class, "resources", null);
+ resourcesFieldName = addField(Modifier.PROTECTED | Modifier.FINAL, InternalComponentResources.class.getName(),
+ "resources");
+
+ addToConstructor(String.format(" %s = $1;", resourcesFieldName));
addNewMethod(GET_COMPONENT_RESOURCES_SIGNATURE, "return " + resourcesFieldName + ";");
@@ -804,25 +807,9 @@ public final class InternalClassTransfor
constructorArgs = parentTransformation.getConstructorArgs();
- int count = constructorArgs.size();
-
- // Build the call to the super-constructor.
+ // Re-invoke the constructor, passing the resources and array of values to the super class
- constructor.append("{ super(");
-
- for (int i = 1; i <= count; i++)
- {
- if (i > 1)
- constructor.append(", ");
-
- // $0 is implicitly self, so the 0-index ConstructorArg will be Javassisst
- // pseudeo-variable $1, and so forth.
-
- constructor.append("$");
- constructor.append(i);
- }
-
- constructor.append(");\n");
+ addToConstructor("{\n super($$);");
// The "}" will be added later, inside finish().
}
@@ -1696,14 +1683,14 @@ public final class InternalClassTransfor
TransformField field = createField(Modifier.PRIVATE | Modifier.FINAL, type.getName(), suggestedName);
- String argName = addConstructorArg(providerType, provider);
+ String argReference = addConstructorArg(providerType, provider);
// Inside the constructor,
// pass the resources to the provider's get() method, cast to the
// field type and assign. This will likely not work with
// primitives and arrays, but that's ok for now.
- addToConstructor(String.format(" %s = (%s) %s.get(%s);", field.getName(), type.getName(), argName,
+ addToConstructor(String.format(" %s = (%s) (%s).get(%s);", field.getName(), type.getName(), argReference,
resourcesFieldName));
return field;
@@ -1829,18 +1816,7 @@ public final class InternalClassTransfor
private void addConstructor(String initializer)
{
- int count = constructorArgs.size();
-
- CtClass[] types = new CtClass[count];
-
- for (int i = 0; i < count; i++)
- {
- ConstructorArg arg = constructorArgs.get(i);
-
- types[i] = arg.getType();
- }
-
- // Add a call to the initializer; the method converted fromt the classes default
+ // Add a call to the initializer; the method converted from the class' default
// constructor.
constructor.append(" ");
@@ -1854,6 +1830,9 @@ public final class InternalClassTransfor
try
{
+ CtClass[] types = new CtClass[]
+ { toCtClass(InternalComponentResources.class), toCtClass(Object[].class) };
+
CtConstructor cons = CtNewConstructor.make(types, null, constructorBody, ctClass);
ctClass.addConstructor(cons);
@@ -1863,17 +1842,7 @@ public final class InternalClassTransfor
throw new RuntimeException(ex);
}
- formatter.format("add constructor: %s(", getClassName());
-
- for (int i = 0; i < count; i++)
- {
- if (i > 0)
- description.append(", ");
-
- formatter.format("%s $%d", types[i].getName(), i + 1);
- }
-
- formatter.format(")\n%s\n\n", constructorBody);
+ formatter.format("add constructor: %s(ComponentResources, Object[])\n%s\n\n", getClassName(), constructorBody);
}
private String convertConstructorToMethod()
@@ -1919,81 +1888,23 @@ public final class InternalClassTransfor
ClassFab cf = classFactory.newClass(name, AbstractInstantiator.class);
- BodyBuilder constructor = new BodyBuilder();
-
- // This is really -1 + 2: The first value in constructorArgs is the
- // InternalComponentResources, which doesn't
- // count toward's the Instantiator's constructor ... then we add in the Model and String
- // description.
- // It's tricky because there's the constructor parameters for the Instantiator, most of
- // which are stored
- // in fields and then used as the constructor parameters for the Component.
+ Object[] componentConstructorArgs = constructorArgs.toArray(new Object[constructorArgs.size()]);
- Class[] constructorParameterTypes = new Class[constructorArgs.size() + 1];
- Object[] constructorParameterValues = new Object[constructorArgs.size() + 1];
+ cf.addConstructor(new Class[]
+ { ComponentModel.class, String.class, Object[].class }, null, "super($1, $2, $3);");
- constructorParameterTypes[0] = ComponentModel.class;
- constructorParameterValues[0] = componentModel;
+ // Pass $1 (the InternalComponentResources object) and the constructorArgs (from the AbstractIntantiator
+ // base class) into the new component instance's constructor
- constructorParameterTypes[1] = String.class;
- constructorParameterValues[1] = String.format("Instantiator[%s]", componentClassName);
-
- BodyBuilder newInstance = new BodyBuilder();
-
- newInstance.add("return new %s($1", componentClassName);
-
- constructor.begin();
-
- // Pass the model and description to AbstractInstantiator
-
- constructor.addln("super($1, $2);");
-
- // Again, skip the (implicit) InternalComponentResources field, that's
- // supplied to the Instantiator's newInstance() method.
-
- for (int i = 1; i < constructorArgs.size(); i++)
- {
- ConstructorArg arg = constructorArgs.get(i);
-
- CtClass argCtType = arg.getType();
- Class argType = toClass(argCtType.getName());
-
- boolean primitive = argCtType.isPrimitive();
-
- Class fieldType = primitive ? ClassFabUtils.getPrimitiveType(argType) : argType;
-
- String fieldName = "_param_" + i;
-
- constructorParameterTypes[i + 1] = argType;
- constructorParameterValues[i + 1] = arg.getValue();
-
- cf.addField(fieldName, fieldType);
-
- // $1 is model, $2 is description, to $3 is first dynamic parameter.
-
- // The arguments may be wrapper types, so we cast down to
- // the primitive type.
-
- String parameterReference = "$" + (i + 2);
-
- constructor.addln("%s = %s;", fieldName, ClassFabUtils.castReference(parameterReference, fieldType
- .getName()));
-
- newInstance.add(", %s", fieldName);
- }
-
- constructor.end();
- newInstance.addln(");");
-
- cf.addConstructor(constructorParameterTypes, null, constructor.toString());
-
- cf.addMethod(Modifier.PUBLIC, NEW_INSTANCE_SIGNATURE, newInstance.toString());
+ cf.addMethod(Modifier.PUBLIC, NEW_INSTANCE_SIGNATURE, String.format("return new %s($1, constructorArgs);",
+ componentClassName));
Class instantiatorClass = cf.createClass();
try
{
- Object instance = instantiatorClass.getConstructors()[0].newInstance(constructorParameterValues);
+ Object instance = instantiatorClass.getConstructors()[0].newInstance(componentModel, String.format(
+ "Instantiator[%s]", componentClassName), componentConstructorArgs);
return (Instantiator) instance;
}
@@ -2041,7 +1952,7 @@ public final class InternalClassTransfor
return idAllocator;
}
- public List<ConstructorArg> getConstructorArgs()
+ public List<Object> getConstructorArgs()
{
failIfNotFrozen();
@@ -2295,13 +2206,16 @@ public final class InternalClassTransfor
* type of parameter
* @param value
* value of parameter
- * @return psuedo-name of parameter (i.e., "$2", "$3", etc.)
+ * @return de-referenced argument value
*/
private String addConstructorArg(CtClass parameterType, Object value)
{
- constructorArgs.add(new ConstructorArg(parameterType, value));
- return "$" + constructorArgs.size();
+ int index = constructorArgs.size();
+
+ constructorArgs.add(value);
+
+ return ClassFabUtils.castReference(String.format("$2[%d]", index), parameterType.getName());
}
private static List<TransformMethodSignature> toMethodSignatures(List<TransformMethod> input)
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TransformField.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TransformField.java?rev=926257&r1=926256&r2=926257&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TransformField.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TransformField.java Mon Mar 22 19:12:15 2010
@@ -14,6 +14,8 @@
package org.apache.tapestry5.services;
+import java.lang.reflect.Field;
+
import org.apache.tapestry5.ioc.AnnotationProvider;
import org.apache.tapestry5.ioc.services.FieldValueConduit;
@@ -77,7 +79,11 @@ public interface TransformField extends
*/
void replaceAccess(FieldValueConduit conduit);
- /** Returns the modifiers for the field. */
+ /**
+ * Returns the modifiers for the field.
+ *
+ * @see Field#getModifiers()
+ */
int getModifiers();
/**