After doing some exploratory testing this actually is a problem and can
be reproduced with the following:
public class TestPrefixMethod
{
@InjectPage
private TestPrefixMethod2 _otherPage;
@PlusOne
public int getValue() {
// call hashCode() just to access the injected member
int value = _otherPage.hashCode();
return value * 0;
}
}
@PlusOne is a test annotation that simply returns the method value + 1.
When you call getValue() it should return 1 but instead you get an
exception:
java.lang.NoSuchFieldError: _otherPage
I created a ticket for this:
https://issues.apache.org/jira/browse/TAPESTRY-1834
It seems like you really do need to do the field replacement at the end
or else you run the risk of not replacing all the necessary fields.
Renaming the original method and having the new method call the other
may be a good option, especially when you have the situation where you
need to put code at both the beginning and the end of an existing method
(which I've already had to do). Perhaps adding the following method
would be good:
ClassTransformation.replaceMethod(TransformMethodSignature
methodSignature, String newMethodName, String methodBody);
this would rename the existing method which could be called from the
given methodBody. An exception would be thrown if newMethodName already
exists. This way you could do multiple replacements and it should
continue to work. One trick to this is that you would have to remember
to pass in the parameters when calling the old method. However this
still doesn't solve the problem of field replacements as you can't tell
if the method you just replaced is the original or another replacement.
But to back up, why not always do field replacement when
extending/prefixing an existing method? The only fields that will be
replaced are those in the original class, none of which should be
referenced from enhanced code anyway.
If this isn't true, however, another option could be to internally track
1) which methods have been newly created by enhancement and 2) which
methods have already had field replacement done on them but this doesn't
eliminate the problem of not replacing all fields.
Yet another option would be to save the method extensions/prefixes until
the end as small objects so that they could be performed in
finish()after the field replacements. This would make sure they all went
in and happen in the right order but after field replacements.
On Mon, 2007-10-15 at 14:12 -0700, Howard Lewis Ship wrote:
> That does look suspicious and it raises a difficult question, re: how to
> differentiate field accesses in that case?
>
> I think the situation here is that the methods being extended (prior to your
> new @Once annotation) are methods that were introduced new in the class, or
> perhaps its transformed parent class.
>
> It may be that we should NOT defer converting field access until the end,
> but instead, should require that field access conversions occur early
> (before methods are modified). Really, there's a transition point between
> the field access conversions, and the EXTENSION of existing methods.
>
> This stuff can get very, very tricky!
>
> Another approach that occurs to me would be to rename the original method
> (but not mark it as new) and then create an entirely new method that can
> invoke the old method. But that's tricky because you can change the
> original semantics of the method too easily.
>
>
> On 10/15/07, Dan Adams <[EMAIL PROTECTED]> wrote:
> >
> > So IntertalClassTransformationImpl has this:
> >
> > public void extendMethod(TransformMethodSignature methodSignature,
> > String methodBody)
> > {
> > failIfFrozen();
> >
> > CtMethod method = findMethod(methodSignature);
> >
> > try
> > {
> > method.insertAfter(methodBody);
> > }
> > catch (CannotCompileException ex)
> > {
> > throw new
> > MethodCompileException(ServicesMessages.methodCompileError(
> > methodSignature,
> > methodBody,
> > ex), methodBody, ex);
> > }
> >
> > addMethodToDescription("extend", methodSignature, methodBody);
> >
> > _addedMethods.add(method);
> > }
> >
> > The last line there adds the method to the set of added methods which is
> > used later here:
> >
> > private void replaceFieldAccess()
> > {
> > // Provide empty maps here, to make the code in the inner class
> > a tad
> > // easier.
> >
> > if (_fieldReadTransforms == null) _fieldReadTransforms =
> > newMap();
> >
> > if (_fieldWriteTransforms == null) _fieldWriteTransforms =
> > newMap();
> >
> > ExprEditor editor = new ExprEditor()
> > {
> > @Override
> > public void edit(FieldAccess access) throws
> > CannotCompileException
> > {
> > // Ignore any methods to were added as part of the
> > transformation.
> > // If we reference the field there, we really mean the
> > field.
> >
> > if (_addedMethods.contains(access.where())) return;
> >
> > Map<String, String> transformMap = access.isReader() ?
> > _fieldReadTransforms
> > : _fieldWriteTransforms;
> >
> > String body = transformMap.get(access.getFieldName());
> > if (body == null) return;
> >
> > access.replace(body);
> > }
> > };
> >
> > try
> > {
> > _ctClass.instrument(editor);
> > }
> > catch (CannotCompileException ex)
> > {
> > throw new RuntimeException(ex);
> > }
> > }
> >
> > It appears that if you extend a method which already exists with new
> > code and the code that was in the method before it was extended
> > references a field that is now having it's accesses replaced then the
> > accesses to that field won't be replaced. But it appears that this could
> > be a bug such as in the case where it referenced an injected field. Am I
> > correct or am I missing something?
> >
> > --
> > Dan Adams
> > Senior Software Engineer
> > Interactive Factory
> > 617.235.5857
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > For additional commands, e-mail: [EMAIL PROTECTED]
> >
> >
>
>
--
Dan Adams
Senior Software Engineer
Interactive Factory
617.235.5857
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
