Author: hlship
Date: Fri Dec 23 22:34:24 2011
New Revision: 1222877
URL: http://svn.apache.org/viewvc?rev=1222877&view=rev
Log:
TAP5-1801: Allow non-public fields in instrumented classes
Modified:
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/plastic/ClassType.java
tapestry/tapestry5/branches/5.3/plastic/src/test/groovy/org/apache/tapestry5/plastic/ObtainPlasticClass.groovy
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/symbolparam/GridSymbolDemoTests.groovy
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Datum.java
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ProtectedFields.java
Modified:
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java?rev=1222877&r1=1222876&r2=1222877&view=diff
==============================================================================
---
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
(original)
+++
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
Fri Dec 23 22:34:24 2011
@@ -85,6 +85,8 @@ public class PlasticClassImpl extends Lo
final PlasticClassPool pool;
+ private final boolean proxy;
+
final String className;
private final String superClassName;
@@ -131,17 +133,12 @@ public class PlasticClassImpl extends Lo
// such methods are not optimized away incorrectly.
final Set<MethodNode> shimInvokedMethods = PlasticInternalUtils.newSet();
- /**
- * Maps a field name to a replacement method that should be invoked
instead of reading the
- * field.
- */
- final Map<String, MethodNode> fieldToReadMethod =
PlasticInternalUtils.newMap();
/**
- * Maps a field name to a replacement method that should be invoked
instead of writing the
- * field.
+ * Tracks instrumentations of fields of this class, including private
fields which are not published into the
+ * {@link PlasticClassPool}.
*/
- final Map<String, MethodNode> fieldToWriteMethod =
PlasticInternalUtils.newMap();
+ private final FieldInstrumentations fieldInstrumentations = new
FieldInstrumentations();
/**
* This normal no-arguments constructor, or null. By the end of the
transformation
@@ -175,12 +172,14 @@ public class PlasticClassImpl extends Lo
* @param pool
* @param parentInheritanceData
* @param parentStaticContext
+ * @param proxy
*/
public PlasticClassImpl(ClassNode classNode, PlasticClassPool pool,
InheritanceData parentInheritanceData,
- StaticContext parentStaticContext)
+ StaticContext parentStaticContext, boolean proxy)
{
this.classNode = classNode;
this.pool = pool;
+ this.proxy = proxy;
staticContext = parentStaticContext.dupe();
@@ -222,7 +221,7 @@ public class PlasticClassImpl extends Lo
/**
* Static methods are not visible to the main API methods, but
they must still be transformed,
- * incase they directly access fields. In addition, track their
names to avoid collisions.
+ * in case they directly access fields. In addition, track their
names to avoid collisions.
*/
if (Modifier.isStatic(node.access))
{
@@ -271,16 +270,7 @@ public class PlasticClassImpl extends Lo
if (Modifier.isStatic(node.access))
continue;
- // TODO: Perhaps we should defer this private check until it is
needed,
- // i.e., when we do some work on the field that requires it to be
private?
- // However, given class loading issues, public fields are likely
to cause
- // their own problems when passing the ClassLoader boundary.
-
- if (!Modifier.isPrivate(node.access))
- throw new IllegalArgumentException(
- String.format(
- "Field %s of class %s is not private. Class
transformation requires that all instance fields be private.",
- node.name, className));
+ // When we instrument the field such that it must be private,
we'll get an exception.
fields.add(new PlasticFieldImpl(this, node));
}
@@ -717,27 +707,13 @@ public class PlasticClassImpl extends Lo
*/
private void interceptFieldAccess()
{
- Set<MethodNode> unusedAccessMethods = PlasticInternalUtils.newSet();
-
- // Track all the access methods created for the fields.
-
- unusedAccessMethods.addAll(fieldToReadMethod.values());
- unusedAccessMethods.addAll(fieldToWriteMethod.values());
-
- // Keep any field access methods invoked from the shim
- unusedAccessMethods.removeAll(shimInvokedMethods);
-
for (MethodNode node : fieldTransformMethods)
{
// Intercept field access inside the method, tracking which access
methods
// are actually used by removing them from accessMethods
- interceptFieldAccess(node, unusedAccessMethods);
+ interceptFieldAccess(node);
}
-
- // Remove all added methods that aren't actually used.
-
- classNode.methods.removeAll(unusedAccessMethods);
}
/**
@@ -914,7 +890,7 @@ public class PlasticClassImpl extends Lo
}
}
- private void interceptFieldAccess(MethodNode methodNode, Set<MethodNode>
unusedAccessMethods)
+ private void interceptFieldAccess(MethodNode methodNode)
{
InsnList insns = methodNode.instructions;
@@ -927,33 +903,33 @@ public class PlasticClassImpl extends Lo
int opcode = node.getOpcode();
if (opcode != GETFIELD && opcode != PUTFIELD)
+ {
continue;
-
- // Make sure we're talking about access to a field of this class,
not some other
- // visible field of another class.
+ }
FieldInsnNode fnode = (FieldInsnNode) node;
- if (!fnode.owner.equals(classNode.name))
- continue;
-
- Map<String, MethodNode> fieldToMethod = opcode == GETFIELD ?
fieldToReadMethod : fieldToWriteMethod;
+ FieldInstrumentation instrumentation =
findInstrumentations(fnode).get(fnode.name, opcode == GETFIELD);
- MethodNode mn = fieldToMethod.get(fnode.name);
-
- if (mn == null)
- continue;
-
- String methodDescription = opcode == GETFIELD ? "()" + fnode.desc
: "(" + fnode.desc + ")V";
+ if (instrumentation == null) { continue; }
// Replace the field access node with the appropriate method
invocation.
- insns.insertBefore(fnode, new MethodInsnNode(INVOKEVIRTUAL,
fnode.owner, mn.name, methodDescription));
+ insns.insertBefore(fnode, new MethodInsnNode(INVOKEVIRTUAL,
fnode.owner, instrumentation.methodName, instrumentation.methodDescription));
it.remove();
+ }
+ }
- unusedAccessMethods.remove(mn);
+ private FieldInstrumentations findInstrumentations(FieldInsnNode node)
+ {
+ if (node.owner.equals(classNode.name))
+ {
+ return fieldInstrumentations;
}
+
+ return pool.getFieldInstrumentations(node.owner);
+
}
String getInstanceContextFieldName()
@@ -1141,4 +1117,27 @@ public class PlasticClassImpl extends Lo
return this;
}
+ void redirectFieldWrite(String fieldName, boolean privateField, MethodNode
method)
+ {
+ FieldInstrumentation fi = new FieldInstrumentation(method.name,
method.desc);
+
+ fieldInstrumentations.write.put(fieldName, fi);
+
+ if (!(proxy || privateField))
+ {
+ pool.setFieldWriteInstrumentation(classNode.name, fieldName, fi);
+ }
+ }
+
+ void redirectFieldRead(String fieldName, boolean privateField, MethodNode
method)
+ {
+ FieldInstrumentation fi = new FieldInstrumentation(method.name,
method.desc);
+
+ fieldInstrumentations.read.put(fieldName, fi);
+
+ if (!(proxy || privateField))
+ {
+ pool.setFieldReadInstrumentation(classNode.name, fieldName, fi);
+ }
+ }
}
Modified:
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java?rev=1222877&r1=1222876&r2=1222877&view=diff
==============================================================================
---
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
(original)
+++
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
Fri Dec 23 22:34:24 2011
@@ -17,8 +17,7 @@ package org.apache.tapestry5.internal.pl
import org.apache.tapestry5.internal.plastic.asm.ClassReader;
import org.apache.tapestry5.internal.plastic.asm.ClassWriter;
import org.apache.tapestry5.internal.plastic.asm.Opcodes;
-import org.apache.tapestry5.internal.plastic.asm.tree.AnnotationNode;
-import org.apache.tapestry5.internal.plastic.asm.tree.ClassNode;
+import org.apache.tapestry5.internal.plastic.asm.tree.*;
import org.apache.tapestry5.plastic.*;
import java.lang.annotation.Annotation;
@@ -39,6 +38,11 @@ public class PlasticClassPool implements
private final Set<String> controlledPackages;
+
+ // Would use Deque, but that's added in 1.6 and we're still striving for
1.5 code compatibility.
+
+ private final Stack<String> activeInstrumentClassNames = new
Stack<String>();
+
/**
* Maps class names to instantiators for that class name.
* Synchronized on the loader.
@@ -53,10 +57,9 @@ public class PlasticClassPool implements
private final Cache<String, TypeCategory> typeName2Category = new
Cache<String, TypeCategory>()
{
-
protected TypeCategory convert(String typeName)
{
- ClassNode cn = constructClassNode(typeName);
+ ClassNode cn = constructClassNodeFromBytecode(typeName);
return Modifier.isInterface(cn.access) ? TypeCategory.INTERFACE :
TypeCategory.CLASS;
}
@@ -78,7 +81,13 @@ public class PlasticClassPool implements
/**
* Map from FQCN to BaseClassDef. Synchronized on the loader.
*/
- private final Map<String, BaseClassDef> baseClassDefs = new
HashMap<String, PlasticClassPool.BaseClassDef>();
+ private final Map<String, BaseClassDef> baseClassDefs =
PlasticInternalUtils.newMap();
+
+
+ private final Map<String, FieldInstrumentations> instrumentations =
PlasticInternalUtils.newMap();
+
+ private final FieldInstrumentations placeholder = new
FieldInstrumentations();
+
private final Set<TransformationOption> options;
@@ -119,7 +128,7 @@ public class PlasticClassPool implements
}
- public Class realize(String primaryClassName, ClassType classType, final
ClassNode classNode)
+ public Class realize(String primaryClassName, ClassType classType,
ClassNode classNode)
{
synchronized (loader)
{
@@ -298,35 +307,110 @@ public class PlasticClassPool implements
return false;
}
- public Class<?> loadAndTransformClass(String className) throws
ClassNotFoundException
+ // Hopefully the synchronized will not cause a deadlock
+
+ public synchronized Class<?> loadAndTransformClass(String className)
throws ClassNotFoundException
{
// Inner classes are not transformed, but they are loaded by the same
class loader.
if (className.contains("$"))
+ {
return loadInnerClass(className);
+ }
// TODO: What about interfaces, enums, annotations, etc. ... they
shouldn't be in the package, but
// we should generate a reasonable error message.
- InternalPlasticClassTransformation transformation =
getPlasticClassTransformation(className);
+ if (activeInstrumentClassNames.contains(className))
+ {
+ StringBuilder builder = new StringBuilder("");
+ String sep = "";
+
+ for (String name : activeInstrumentClassNames)
+ {
+ builder.append(sep);
+ builder.append(name);
+
+ sep = ", ";
+ }
- delegate.transform(transformation.getPlasticClass());
+ throw new IllegalStateException(String.format("Unable to transform
class %s as it is already in the process of being transformed; there is a cycle
among the following classes: %s.",
+ className, builder));
+ }
+
+ activeInstrumentClassNames.push(className);
+
+ try
+ {
- ClassInstantiator createInstantiator =
transformation.createInstantiator();
- ClassInstantiator configuredInstantiator =
delegate.configureInstantiator(className, createInstantiator);
+ InternalPlasticClassTransformation transformation =
getPlasticClassTransformation(className);
- instantiators.put(className, configuredInstantiator);
+ delegate.transform(transformation.getPlasticClass());
- return transformation.getTransformedClass();
+ ClassInstantiator createInstantiator =
transformation.createInstantiator();
+ ClassInstantiator configuredInstantiator =
delegate.configureInstantiator(className, createInstantiator);
+
+ instantiators.put(className, configuredInstantiator);
+
+ return transformation.getTransformedClass();
+ } finally
+ {
+ activeInstrumentClassNames.pop();
+ }
}
private Class loadInnerClass(String className)
{
- byte[] bytecode = readBytecode(className);
+ ClassNode classNode = constructClassNodeFromBytecode(className);
+
+ interceptFieldAccess(classNode);
+
+ return realize(className, ClassType.INNER, classNode);
+ }
+
+ private void interceptFieldAccess(ClassNode classNode)
+ {
+ for (MethodNode method : (List<MethodNode>) classNode.methods) {
+ interceptFieldAccess(classNode.name, method);
+ }
+ }
+
+ private void interceptFieldAccess(String classInternalName, MethodNode
method)
+ {
+ InsnList insns = method.instructions;
+
+ ListIterator it = insns.iterator();
+
+ while (it.hasNext())
+ {
+ AbstractInsnNode node = (AbstractInsnNode) it.next();
+
+ int opcode = node.getOpcode();
+
+ if (opcode != GETFIELD && opcode != PUTFIELD)
+ {
+ continue;
+ }
+
+ FieldInsnNode fnode = (FieldInsnNode) node;
+
+ String ownerInternalName = fnode.owner;
+
+ if (ownerInternalName.equals(classInternalName)) { continue; }
+
+ FieldInstrumentation instrumentation =
getFieldInstrumentations(ownerInternalName).get(fnode.name, opcode == GETFIELD);
+
+ if (instrumentation == null) { continue; }
- return loader.defineClassWithBytecode(className, bytecode);
+ // Replace the field access node with the appropriate method
invocation.
+
+ insns.insertBefore(fnode, new MethodInsnNode(INVOKEVIRTUAL,
ownerInternalName, instrumentation.methodName,
instrumentation.methodDescription));
+
+ it.remove();
+ }
}
+
/**
* For a fully-qualified class name of an <em>existing</em> class, loads
the bytes for the class
* and returns a PlasticClass instance.
@@ -338,14 +422,24 @@ public class PlasticClassPool implements
{
assert PlasticInternalUtils.isNonBlank(className);
- ClassNode classNode = constructClassNode(className);
+ ClassNode classNode = constructClassNodeFromBytecode(className);
String baseClassName =
PlasticInternalUtils.toClassName(classNode.superName);
- return createTransformation(baseClassName, classNode);
+ instrumentations.put(classNode.name, new FieldInstrumentations());
+
+ return createTransformation(baseClassName, classNode, false);
}
- private InternalPlasticClassTransformation createTransformation(String
baseClassName, ClassNode classNode)
+ /**
+ *
+ * @param baseClassName class from which the transformed class extends
+ * @param classNode node for the class
+ * @param proxy if true, the class is a new empty class; if false
an existing class that's being transformed
+ * @return
+ * @throws ClassNotFoundException
+ */
+ private InternalPlasticClassTransformation createTransformation(String
baseClassName, ClassNode classNode, boolean proxy)
throws ClassNotFoundException
{
if (shouldInterceptClassLoading(baseClassName))
@@ -356,12 +450,12 @@ public class PlasticClassPool implements
assert def != null;
- return new PlasticClassImpl(classNode, this, def.inheritanceData,
def.staticContext);
+ return new PlasticClassImpl(classNode, this, def.inheritanceData,
def.staticContext, proxy);
}
// When the base class is Object, or otherwise not in a transformed
package,
// then start with the empty
- return new PlasticClassImpl(classNode, this, emptyInheritanceData,
emptyStaticContext);
+ return new PlasticClassImpl(classNode, this, emptyInheritanceData,
emptyStaticContext, proxy);
}
/**
@@ -371,7 +465,7 @@ public class PlasticClassPool implements
* @param className fully qualified class name
* @return corresponding ClassNode
*/
- public ClassNode constructClassNode(String className)
+ public ClassNode constructClassNodeFromBytecode(String className)
{
byte[] bytecode = readBytecode(className);
@@ -397,7 +491,7 @@ public class PlasticClassPool implements
newClassNode.visit(V1_5, ACC_PUBLIC,
PlasticInternalUtils.toInternalName(newClassName), null,
PlasticInternalUtils.toInternalName(baseClassName), null);
- return createTransformation(baseClassName, newClassNode);
+ return createTransformation(baseClassName, newClassNode, true);
} catch (ClassNotFoundException ex)
{
throw new RuntimeException(String.format("Unable to create class
%s as sub-class of %s: %s", newClassName,
@@ -481,4 +575,54 @@ public class PlasticClassPool implements
{
return options.contains(option);
}
+
+
+ void setFieldReadInstrumentation(String classInternalName, String
fieldName, FieldInstrumentation fi)
+ {
+ instrumentations.get(classInternalName).read.put(fieldName, fi);
+ }
+
+ FieldInstrumentations getFieldInstrumentations(String classInternalName)
+ {
+ FieldInstrumentations result = instrumentations.get(classInternalName);
+
+ if (result != null)
+ {
+ return result;
+ }
+
+ String className = PlasticInternalUtils.toClassName(classInternalName);
+
+ // If it is a top-level (not inner) class in a controlled package,
then we
+ // will recursively load the class, to identify any field
instrumentations
+ // in it.
+ if (!className.contains("$") && shouldInterceptClassLoading(className))
+ {
+ try
+ {
+ loadAndTransformClass(className);
+
+ // The key is written into the instrumentations map as a
side-effect
+ // of loading the class.
+ return instrumentations.get(classInternalName);
+ } catch (Exception ex)
+ {
+ throw new RuntimeException(PlasticInternalUtils.toMessage(ex),
ex);
+ }
+ }
+
+ // Either a class outside of controlled packages, or an inner class.
Use a placeholder
+ // that contains empty maps.
+
+ result = placeholder;
+ instrumentations.put(classInternalName, result);
+
+ return result;
+ }
+
+ void setFieldWriteInstrumentation(String classInternalName, String
fieldName, FieldInstrumentation fi)
+ {
+ instrumentations.get(classInternalName).write.put(fieldName, fi);
+ }
+
}
Modified:
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java?rev=1222877&r1=1222876&r2=1222877&view=diff
==============================================================================
---
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java
(original)
+++
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java
Fri Dec 23 22:34:24 2011
@@ -14,8 +14,8 @@
package org.apache.tapestry5.internal.plastic;
-import org.apache.tapestry5.internal.plastic.asm.*;
import org.apache.tapestry5.internal.plastic.asm.Opcodes;
+import org.apache.tapestry5.internal.plastic.asm.Type;
import org.apache.tapestry5.internal.plastic.asm.tree.FieldNode;
import org.apache.tapestry5.internal.plastic.asm.tree.MethodNode;
import org.apache.tapestry5.plastic.*;
@@ -104,9 +104,11 @@ class PlasticFieldImpl extends PlasticMe
plasticClass.check();
if (this.tag != null)
+ {
throw new IllegalStateException(String.format(
"Field %s of class %s can not be claimed by %s as it is
already claimed by %s.", node.name,
plasticClass.className, tag, this.tag));
+ }
this.tag = tag;
@@ -141,8 +143,18 @@ class PlasticFieldImpl extends PlasticMe
private void verifyInitialState(String operation)
{
if (state != FieldState.INITIAL)
+ {
throw new IllegalStateException(String.format("Unable to %s field
%s of class %s, as it already %s.",
operation, node.name, plasticClass.className,
state.description));
+ }
+ }
+
+ private void ensureNotPublic()
+ {
+ if (Modifier.isPublic(node.access))
+ {
+ throw new IllegalArgumentException(String.format("Field %s of
class %s must be instrumented, and may not be public.", node.name,
plasticClass.className));
+ }
}
public PlasticField inject(Object value)
@@ -265,8 +277,6 @@ class PlasticFieldImpl extends PlasticMe
replaceFieldReadAccess(conduitField.getName());
replaceFieldWriteAccess(conduitField.getName());
- // TODO: Do we keep the field or not? It will now always be
null/0/false.
-
state = FieldState.CONDUIT;
return this;
@@ -324,24 +334,28 @@ class PlasticFieldImpl extends PlasticMe
private void introduceAccessorMethod(String returnType, String name,
String[] parameterTypes, String signature,
InstructionBuilderCallback callback)
{
- MethodDescription description = new
MethodDescription(org.apache.tapestry5.internal.plastic.asm.Opcodes.ACC_PUBLIC,
returnType, name, parameterTypes,
+ MethodDescription description = new
MethodDescription(Opcodes.ACC_PUBLIC, returnType, name, parameterTypes,
signature, null);
String desc = plasticClass.nameCache.toDesc(description);
if (plasticClass.inheritanceData.isImplemented(name, desc))
+ {
throw new IllegalArgumentException(String.format(
"Unable to create new accessor method %s on class %s as
the method is already implemented.",
description.toString(), plasticClass.className));
+ }
plasticClass.introduceMethod(description, callback);
}
private void replaceFieldWriteAccess(String conduitFieldName)
{
- String setAccessName =
plasticClass.makeUnique(plasticClass.methodNames, "set_" + node.name);
+ ensureNotPublic();
- setAccess = new MethodNode(Opcodes.ACC_SYNTHETIC | Opcodes.ACC_FINAL,
setAccessName, "(" + node.desc + ")V", null, null);
+ String setAccessName =
plasticClass.makeUnique(plasticClass.methodNames, "conduit_set_" + node.name);
+
+ setAccess = new MethodNode(accessForMethod(), setAccessName, "(" +
node.desc + ")V", null, null);
InstructionBuilder builder = plasticClass.newBuilder(setAccess);
@@ -367,16 +381,33 @@ class PlasticFieldImpl extends PlasticMe
plasticClass.addMethod(setAccess);
- plasticClass.fieldToWriteMethod.put(node.name, setAccess);
+ plasticClass.redirectFieldWrite(node.name, isPrivate(), setAccess);
+ }
+
+ /**
+ * Determines the access for a method that takes the place of reading from
or writing to the field. Escalates
+ * private fields to package private visibility, but leaves protected and
package private alone (public fields
+ * can not be instrumented).
+ */
+ private int accessForMethod()
+ {
+ return (Opcodes.ACC_SYNTHETIC | Opcodes.ACC_FINAL | node.access) &
~Opcodes.ACC_PRIVATE;
+ }
+
+ private boolean isPrivate()
+ {
+ return Modifier.isPrivate(node.access);
}
private void replaceFieldReadAccess(String conduitFieldName)
{
+ ensureNotPublic();
+
boolean writeBehindEnabled = isWriteBehindEnabled();
- String getAccessName =
plasticClass.makeUnique(plasticClass.methodNames, "getfieldvalue_" + node.name);
+ String getAccessName =
plasticClass.makeUnique(plasticClass.methodNames, "conduit_get_" + node.name);
- getAccess = new MethodNode(Opcodes.ACC_SYNTHETIC | Opcodes.ACC_FINAL,
getAccessName, "()" + node.desc, null, null);
+ getAccess = new MethodNode(accessForMethod(), getAccessName, "()" +
node.desc, null, null);
InstructionBuilder builder = plasticClass.newBuilder(getAccess);
@@ -418,7 +449,7 @@ class PlasticFieldImpl extends PlasticMe
plasticClass.addMethod(getAccess);
- plasticClass.fieldToReadMethod.put(node.name, getAccess);
+ plasticClass.redirectFieldRead(node.name, isPrivate(), getAccess);
}
private boolean isWriteBehindEnabled()
@@ -441,9 +472,11 @@ class PlasticFieldImpl extends PlasticMe
private void makeReadOnly()
{
- String setAccessName =
plasticClass.makeUnique(plasticClass.methodNames, "setfieldvalue_" + node.name);
+ ensureNotPublic();
+
+ String setAccessName =
plasticClass.makeUnique(plasticClass.methodNames, "reject_field_change_" +
node.name);
- setAccess = new MethodNode(Opcodes.ACC_SYNTHETIC | Opcodes.ACC_FINAL,
setAccessName, "(" + node.desc + ")V", null, null);
+ setAccess = new MethodNode(accessForMethod(), setAccessName, "(" +
node.desc + ")V", null, null);
String message = String.format("Field %s of class %s is read-only.",
node.name, plasticClass.className);
@@ -451,7 +484,7 @@ class PlasticFieldImpl extends PlasticMe
plasticClass.addMethod(setAccess);
- plasticClass.fieldToWriteMethod.put(node.name, setAccess);
+ plasticClass.redirectFieldWrite(node.name, isPrivate(), setAccess);
node.access |= Opcodes.ACC_FINAL;
}
@@ -462,7 +495,7 @@ class PlasticFieldImpl extends PlasticMe
*/
private MethodNode addShimSetAccessMethod()
{
- String name = plasticClass.makeUnique(plasticClass.methodNames,
"shimset_" + node.name);
+ String name = plasticClass.makeUnique(plasticClass.methodNames,
"shim_set_" + node.name);
// Takes two Object parameters (instance, and value) and returns void.
@@ -482,7 +515,7 @@ class PlasticFieldImpl extends PlasticMe
private MethodNode addShimGetAccessMethod()
{
- String name = plasticClass.makeUnique(plasticClass.methodNames,
"shimget_" + node.name);
+ String name = plasticClass.makeUnique(plasticClass.methodNames,
"shim_get_" + node.name);
MethodNode mn = new MethodNode(Opcodes.ACC_SYNTHETIC |
Opcodes.ACC_FINAL, name, "()" + node.desc, null, null);
Modified:
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/plastic/ClassType.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/plastic/ClassType.java?rev=1222877&r1=1222876&r2=1222877&view=diff
==============================================================================
---
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/plastic/ClassType.java
(original)
+++
tapestry/tapestry5/branches/5.3/plastic/src/main/java/org/apache/tapestry5/plastic/ClassType.java
Fri Dec 23 22:34:24 2011
@@ -33,5 +33,13 @@ public enum ClassType
* An implementation of {@link MethodInvocation}, needed to handle advice
added to
* a method of a primary class.
*/
- METHOD_INVOCATION
+ METHOD_INVOCATION,
+
+ /**
+ * An inner class within a controlled package, which may have field
accesses (to non-private
+ * fields visible to it) instrumented.
+ *
+ * @since 5.4
+ */
+ INNER;
}
Modified:
tapestry/tapestry5/branches/5.3/plastic/src/test/groovy/org/apache/tapestry5/plastic/ObtainPlasticClass.groovy
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/plastic/src/test/groovy/org/apache/tapestry5/plastic/ObtainPlasticClass.groovy?rev=1222877&r1=1222876&r2=1222877&view=diff
==============================================================================
---
tapestry/tapestry5/branches/5.3/plastic/src/test/groovy/org/apache/tapestry5/plastic/ObtainPlasticClass.groovy
(original)
+++
tapestry/tapestry5/branches/5.3/plastic/src/test/groovy/org/apache/tapestry5/plastic/ObtainPlasticClass.groovy
Fri Dec 23 22:34:24 2011
@@ -107,13 +107,16 @@ class ObtainPlasticClass extends Abstrac
pc.allFields == []
}
- def "instance fields must be private"() {
+ def "instrumented instance fields must be private"() {
when:
- mgr.getPlasticClass("testsubjects.NonPrivateInstanceField")
+
+ PlasticClass pc =
mgr.getPlasticClass("testsubjects.PublicInstanceField")
+
+ pc.getAllFields()[0].inject("fixed string value")
then:
def e = thrown(IllegalArgumentException)
- e.message == "Field shouldBePrivate of class
testsubjects.NonPrivateInstanceField is not private. Class transformation
requires that all instance fields be private."
+ e.message == "Field publicNotAllowed of class
testsubjects.PublicInstanceField must be instrumented, and may not be public."
}
def "original constructors now throw exceptions"() {
Modified:
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/symbolparam/GridSymbolDemoTests.groovy
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/symbolparam/GridSymbolDemoTests.groovy?rev=1222877&r1=1222876&r2=1222877&view=diff
==============================================================================
---
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/symbolparam/GridSymbolDemoTests.groovy
(original)
+++
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/symbolparam/GridSymbolDemoTests.groovy
Fri Dec 23 22:34:24 2011
@@ -29,9 +29,10 @@ class GridSymbolDemoTests extends Tapest
clickAndWait "link=4"
- assertText("//tr[1]/td[3]", "6");
- assertText("//tr[1]/td[4]", "false");
- assertText("//tr[2]/td[3]", "7");
- assertText("//tr[2]/td[4]", "true");
+ // Using the XPath selectors was a bit flakey, so maybe css is better.
+ assertText("css=tr.t-first td.me", "6");
+ assertText("css=tr.t-first td.odd", "false");
+ assertText("css=tr.t-last td.me", "7");
+ assertText("css=tr.t-first td.odd", "false");
}
}
Modified:
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java?rev=1222877&r1=1222876&r2=1222877&view=diff
==============================================================================
---
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java
(original)
+++
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java
Fri Dec 23 22:34:24 2011
@@ -936,13 +936,13 @@ public class CoreBehaviorsTests extends
* TAPESTRY-2196
*/
@Test
- public void protected_field_in_page_class()
+ public void public_field_in_page_class()
{
openLinks("Protected Fields Demo", "Trigger the Exception");
assertTextPresent(
"An unexpected application exception has occurred.",
- "Field _field of class
org.apache.tapestry5.integration.app1.pages.ProtectedFields is not private.
Class transformation requires that all instance fields be private.");
+ "Field _field of class
org.apache.tapestry5.integration.app1.pages.ProtectedFields must be
instrumented, and may not be public.");
}
/**
@@ -955,7 +955,7 @@ public class CoreBehaviorsTests extends
{
openLinks("Class Transformation Exception Demo");
- assertTextPresent("Field _value of class
org.apache.tapestry5.integration.app1.pages.Datum is not private. Class
transformation requires that all instance fields be private.");
+ assertTextPresent("Field _value of class
org.apache.tapestry5.integration.app1.pages.Datum must be instrumented, and may
not be public.");
}
@Test
Modified:
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Datum.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Datum.java?rev=1222877&r1=1222876&r2=1222877&view=diff
==============================================================================
---
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Datum.java
(original)
+++
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Datum.java
Fri Dec 23 22:34:24 2011
@@ -19,7 +19,9 @@ package org.apache.tapestry5.integration
*/
public class Datum
{
- protected int _value;
+ // This will be instrumented as with any Tapestry component field, and
that will fail as the field may not
+ // be public.
+ public int _value;
public int getValue()
{
Modified:
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ProtectedFields.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ProtectedFields.java?rev=1222877&r1=1222876&r2=1222877&view=diff
==============================================================================
---
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ProtectedFields.java
(original)
+++
tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ProtectedFields.java
Fri Dec 23 22:34:24 2011
@@ -19,7 +19,8 @@ package org.apache.tapestry5.integration
*/
public class ProtectedFields
{
- protected String _field;
+ // Until Tapestry 5.4, component fields had to be private. Starting in
5.4, they merely must not be public.
+ public String _field;
public String getField()
{
