Author: hlship
Date: Fri Dec 23 18:54:17 2011
New Revision: 1222786
URL: http://svn.apache.org/viewvc?rev=1222786&view=rev
Log:
TAP5-1801: Allow non-public fields in instrumented classes
Added:
tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/PublicInstanceField.java
- copied, changed from r1221952,
tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/NonPrivateInstanceField.java
Removed:
tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/NonPrivateInstanceField.java
Modified:
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java
tapestry/tapestry5/trunk/plastic/src/test/groovy/org/apache/tapestry5/plastic/ObtainPlasticClass.groovy
Modified:
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java?rev=1222786&r1=1222785&r2=1222786&view=diff
==============================================================================
---
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
(original)
+++
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
Fri Dec 23 18:54:17 2011
@@ -222,7 +222,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 +271,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));
}
Modified:
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java?rev=1222786&r1=1222785&r2=1222786&view=diff
==============================================================================
---
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java
(original)
+++
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java
Fri Dec 23 18:54:17 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,22 +334,26 @@ 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();
+
+ String setAccessName =
plasticClass.makeUnique(plasticClass.methodNames, "conduit_set_" + node.name);
setAccess = new MethodNode(Opcodes.ACC_SYNTHETIC | Opcodes.ACC_FINAL,
setAccessName, "(" + node.desc + ")V", null, null);
@@ -372,9 +386,11 @@ class PlasticFieldImpl extends PlasticMe
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);
@@ -441,7 +457,9 @@ 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);
@@ -462,7 +480,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 +500,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/trunk/plastic/src/test/groovy/org/apache/tapestry5/plastic/ObtainPlasticClass.groovy
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/test/groovy/org/apache/tapestry5/plastic/ObtainPlasticClass.groovy?rev=1222786&r1=1222785&r2=1222786&view=diff
==============================================================================
---
tapestry/tapestry5/trunk/plastic/src/test/groovy/org/apache/tapestry5/plastic/ObtainPlasticClass.groovy
(original)
+++
tapestry/tapestry5/trunk/plastic/src/test/groovy/org/apache/tapestry5/plastic/ObtainPlasticClass.groovy
Fri Dec 23 18:54:17 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"() {
Copied:
tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/PublicInstanceField.java
(from r1221952,
tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/NonPrivateInstanceField.java)
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/PublicInstanceField.java?p2=tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/PublicInstanceField.java&p1=tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/NonPrivateInstanceField.java&r1=1221952&r2=1222786&rev=1222786&view=diff
==============================================================================
---
tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/NonPrivateInstanceField.java
(original)
+++
tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/PublicInstanceField.java
Fri Dec 23 18:54:17 2011
@@ -14,7 +14,7 @@
package testsubjects;
-public class NonPrivateInstanceField
+public class PublicInstanceField
{
- String shouldBePrivate;
+ public String publicNotAllowed;
}
