Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
RE: Add argument to field set joinpoint ? Was:Re:[aspectj-users]Question re: declare error

There's an additional problem with doing this in a pointcut.
 
In AspectJ's dynamic join point model, the join points are nodes in the dynamic calling graph through which control passes 'on the way in' and 'on the way out'. One property of that kind JPM is that poincuts should be true of join points both on the way in and on the way out. (Since its the same join point, and we want the pointcuts to be stable predicates.) [1]
 
So one problem with putting the feature you want into a pointcut is getting comfortable with what it should mean on the way back out of the join point.
 
If we were all comfortable with the idea that the 'old value' was the same on the way in and the way out, then, wrt the issue of pointcut stability such a pointcut could make sense.
 
There may of course be some other lurking reason we decided not to do this that isn't coming back to me right now though. 
 
----
[1] As an aside, this is one thing that's so striking about proposals that put before and after into the pointcut language -- they are proposing a significantly different JPM. Which has its advantages, things like finite state machine pointcuts [see Walker's work] are much easier to write in such a JPM.
 

From: aspectj-users-bounces@xxxxxxxxxxx [mailto:aspectj-users-bounces@xxxxxxxxxxx] On Behalf Of Peter Murray
Sent: Monday, September 18, 2006 11:29 AM
To: aspectj-users@xxxxxxxxxxx
Subject: Re: Add argument to field set joinpoint ? Was:Re:[aspectj-users]Question re: declare error

Rob,

Thanks for those thoughts.  I understand your point on expanding the args and an willing to abandon that approach to solving the problem.  However, I still think we should be able to support access to the field's current state from within the joinpoint without the use of reflection.  I'd be perfectly happy to have another pointcut designator that is only valid in the context of a set joinpoint. 

Would JoinPoint need to have a method called getFieldVal()?  Is that what you are referring to by "expose this value through thisJoinPoint"? 

What would be involved in implemeting such a change?  I'd be happy to take a stab at it if there was someone willing to act as a high-level guide.

The obfuscators that I'm familiar with operate post compilation and munge the names of packages, classes and members according to a set of rules defined by the user.  Typically you would except 3rd party libraries from obfuscation so their methods / fields wouldn't be touched.  If they use reflection, that shouldn't pose a problem.

Cheers,

pete
peter m. murray
pete@xxxxxxxxxxxx


On 9/18/06, Ron Bodkin < rbodkin@xxxxxxxxxxxxxx> wrote:

Hi Peter,

 

It's true that there aren't method arguments at a field set join point, but I'd argue that the value being set has the semantics of being an argument in a field set operation, whereas the current value does not. Likewise the use of args in a handler join point in capturing the handled throwable is semantically an argument.

 

Another way of stating my point about matching for args is that currently in AspectJ args(S) and args(S, T) will NEVER match the same join point. The first one will match only join points with a single argument of type S and the second only join points with two arguments of types S and T. I think it's highly desirable to preserve a definition of args that is an ordered list without optional members.

 

You're right that exposing the java.lang.reflect.Field with thisJoinPoint.getField() could break with obfuscated code. If a field() primitive pointcut designator returned the Field in question it also would have that problem. Indeed, you could pretty easily calculate the Field associated with a join point static part, so providing this would be a minor optimization.

 

It would be nice from a developer's view point if AspectJ had a primitive pointcut designator fieldval(X) that would be defined as the current value of the related field and which would only be defined at field get and field set join points. Perhaps this is what Matthew was suggesting with field(). However, this would have the odd behavior of being mutable at a join point, i.e., the value for fieldval could change during the join point. I also suspect it would be hard to expose this value through thisJoinPoint (to do so without using reflection would require updating the value of fieldval multiple times).

 

You do make a good argument for providing the feature. In many use cases, use of reflection is quite performant so in past when people have asked for optimized access to the current field value, the response has been to demonstrate a compelling example where the performance is unacceptable for the use. With respect to allowing reweaving, the AspectJ approach to reweaving statically woven code at load-time is to reweave unwoven code with all relevant aspects included, i.e., it doesn't try to weave woven field access code in a second stage. With respect to obfuscation, is it really true that the obfuscator just breaks when reflection is used? That seems like a big limitation given how much reflection is used in libraries.

 

From: aspectj-users-bounces@xxxxxxxxxxx [mailto: aspectj-users-bounces@xxxxxxxxxxx] On Behalf Of Peter Murray
Sent: Saturday, September 16, 2006 4:28 AM

Subject: Re: Add argument to field set joinpoint ? Was: Re:[aspectj-users]Question re: declare error

 

Ron,

Please excuse me if I'm not making sense - AspectJ is still a bit fresh to me.

Isn't it the case that "arguments" at a set join point are already arbitrary as there are really no official arguments as there are in a method joinpoint.  Therefore it's up to the framework to define what the arguments are at that point.  I do agree that calling proceed with three parameters in this case would 'feel' a bit odd.  I'm not sure I understand your point on the positional matching of args.

It would be best to avoid using reflection for the following reasons:

  • for performance reasons, at some level
  • to allow woven classes to be obfuscated
  • to allow woven classes to be further woven at run-time if desired

Once we resort to reflection, the woven code is 'disconnected' from the structure of the class and it's field accesses and invocations cannot be intercepted by examining the bytecode.  Sometimes this may be desireable, but it should be the decision of the developer which way to go.

It seems to me that it would be possible to modify AspectJ to allow for non-reflective access to existing field values during a set operation.  The question is - what would be the most consistent way to do this?

Cheers,

pete
peter m. murray
pete@xxxxxxxxxxxx


On 9/15/06, Ron Bodkin < rbodkin@xxxxxxxxxxxxxx> wrote:

I think it would be inconsistent because args is really about arguments at a join point whereas this value of old state isn't an argument. And allowing args(x) or args(x,y) to match is inconsistent because args matches positionally (e.g., args(x, *)), so the optional extra parameter would be surprising. I certainly agree that retaining backward compatibility would be important.

 

What about having a getField() method on thisJoinPoint? You could imagine having a subclass of JoinPoint for FieldAccessJoinPoint (much as Signature has subtypes). Failing that, it could be null if there isn't a java.lang.reflect.Field associated with the given join point. At least in this way you preserve pointcut orthogonality and the kinded-specific join point information is accessed through the API.

 

From: aspectj-users-bounces@xxxxxxxxxxx [mailto: aspectj-users-bounces@xxxxxxxxxxx] On Behalf Of Peter Murray
Sent: Friday, September 15, 2006 5:26 AM

Subject: Add argument to field set joinpoint ? Was: Re: [aspectj-users]Question re: declare error

 

Matthew,

What about the idea of adding a second "argument" to the set joinpoint that would be optionally bound (to be backwards compatible) and, if present, would pass in the current value of the field?  The only slightly weird thing about that woudl be that the proceed() would also contain that argument which would essentially be ignored.

What do you think about that idea?

Cheers,

pete
peter m. murray
pete@xxxxxxxxxxxx

On 9/15/06, Matthew Webster <matthew_webster@xxxxxxxxxx > wrote:


Pete,

When used with get/set the pointcut this & target match the object accessing the field and the one that defines it respectively.There is no pointcut to match the field itself although you could image one: "field()" for example. But there are problems. Like this & target it must be typed but you can't just use Object because of primitive fields. But it gets worse because in AspectJ any pointcut combination is possible - they are orthogonal - although certain combinations are not very useful because they match nothing e.g. call & execution. The "field()" pointcut would be _undefined_ at certain join points which is messy.

Matthew Webster
AOSD Project
Java Technology Centre, MP146
IBM Hursley Park, Winchester,  SO21 2JN, England
Telephone: +44 196 2816139 (external) 246139 (internal)
Email: Matthew Webster/UK/IBM @ IBMGB, matthew_webster@xxxxxxxxxx

"Peter Murray" < pete@xxxxxxxxxxxx>

Sent by: aspectj-users-bounces@xxxxxxxxxxx

14/09/2006 23:17

Please respond to
aspectj-users@xxxxxxxxxxx

To

aspectj-users@xxxxxxxxxxx

cc

 

 

 

Subject

Re: [aspectj-users] Question re: declare error

 

 

 




Yes, but that solution requires specific knowledge of the field you are pointcutting on.   In general, I need to write one pointcut that services many objects with many different fields.  Otherwise I might as well manually inline my undo / transaction rollback facility.

-pete

On 9/14/06, Vincent Jorrand <vjorrand@xxxxxxxxx > wrote:
Matthew Webster's post actually mentions 2 solutions, one of which uses a priviledged aspect and no reflection.

The following is copied from his post:
public class SomeClass {
     private int intField;

     public void setInt (int i) {
           intField = i;
     }
}


public privileged aspect PrivilegedAspect {

     pointcut intFieldSet(SomeClass obj, int newValue) :
           set(* intField) && target(obj) && args(newValue)
           && withincode(* SomeClass+.set*(..))
           ;


     before (SomeClass obj, int newValue) : intFieldSet (obj,newValue) {
           int oldValue = obj.intField;
           System.out.println("?
beforeIntFieldSet() oldVaue=" + oldValue
+ ", newValue=" + newValue);

     }
}
Vincent

----- Original Message ----
From: Peter Murray <pete@xxxxxxxxxxxx >
To: aspectj-users@xxxxxxxxxxx
Sent: Thursday, September 14, 2006 5:12:00 PM
Subject: Re: [aspectj-users] Question re: declare error

I wonder if there would be support in the AspectJ community for adding an additional "argument" to the set property that could optionally be bound using syntax like

... set(* *) && args(newValue, oldValue) ...

which would send in the old value?

Any thoughts?  Am I off in the weeds - or is this technically possible if we expand the definition of the set joinpoint?

-pete
peter m. murray
pete@xxxxxxxxxxxx

On 9/14/06, Ron Bodkin < rbodkin@xxxxxxxxxxxxxx> wrote:
Hi Peter: the topic of getting old values of a field during set has come up before on the mailing list and as you've determined the only way to do it where you advise different fields is via reflection (e.g., see Matthew Webster's post at http://dev.eclipse.org/mhonarc/lists/aspectj-users/msg04126.html ).  I see how the obfuscator might break with reflective code (though I'd hope that an obfuscator wouldn't: lots of libraries now rely on reflection). One idea might be to track the old value of the field also in advice so you can compare the previous value to the current value (e.g., using your map that associates static parts with state).

Hope that helps,

Ron

From: aspectj-users-bounces@xxxxxxxxxxx [mailto: aspectj-users-bounces@xxxxxxxxxxx] On Behalf Of Peter Murray
Sent: Thursday, September 14, 2006 1:23 PM


To: aspectj-users@xxxxxxxxxxx
Subject: Re: [aspectj-users] Question re: declare error

 

Ron,

Thanks a bunch!  That does the trick.  I was suspicious that there might be an operator for "subtypes".

Now, the real question is - how can I get ahold of the original value of a field during around set advice so I can use AspectJ to create the ValueChange objects and post them based on the @Transactional annotation.  It seems the only way is through reflection which will break once we obfuscate...

Any thoughts on that?

Cheers,

pete
peter m. murray
pete@xxxxxxxxxxxx

On 9/14/06, Ron Bodkin < rbodkin@xxxxxxxxxxxxxx> wrote:

Hi Peter,

I would use within(Change+) instead of this(Change). That isn't the exact same semantics as using this (*) but in most cases it would capture your intent: it makes it an error to set the field outside of code that's in a subtype of Change.

(*) it's based on the join points being lexically located inside Change. So it can differ because it won't allow setting the fields in a base class that might be extended by a type that extends Change. It also doesn't allow for setting inside of inter-type declared methods (you could extend the rule to allow those if that matters to you).

From: aspectj-users-bounces@xxxxxxxxxxx [mailto: aspectj-users-bounces@xxxxxxxxxxx] On Behalf Of Peter Murray
Sent: Thursday, September 14, 2006 10:41 AM

Subject: Re: [aspectj-users] Question re: declare error

 

Thanks for your reply, Elizabeth.

That makes sense - do you see a way to accomplish what I am trying to do?

Cheers,

pete
peter m. murray
pete@xxxxxxxxxxxx

On 9/14/06, Echlin Harmer, Elizabeth <echline@xxxxxxx > wrote:

Peter

As I understand it, declare is a compile time error or warning; this(), target() and args() are all run time checks. Except in the simplest of cases, it is not possible to tell at compile time what the type of this, target or args will be.

Elizabeth
-----Original Message-----
From: aspectj-users-bounces@xxxxxxxxxxx [mailto: aspectj-users-bounces@xxxxxxxxxxx] On Behalf Of Peter Murray
Sent: September 14, 2006 1:12 PM
To: aspectj-users@xxxxxxxxxxx
Subject: [aspectj-users] Question re: declare error


I'd like to declare an error if my @Transactional attributed instance variables are being set outside of the context of a Change object.  In other words, I'd like to have an error in this case:

public class Foo
{
   @Transactional
    String name

     public void setName(String newName)
     {
           name = newName;
     }
}

But not in this case:

public class Foo
{
   @Transactional
    String name

     public void setName(String newName)
     {
           new ValueChange<String>(name, newName)
           {
                public void set(String value)
                 {
                       name = value;
                 }
            }.post();
     }
}

It seems like this kind of aspect should do that:

public aspect FieldChangeAspect
{
   declare error :
       set(@Transactional * *) &&
       !this(Change)  : "Set of @Transactional variable not in Change object";
}

But this gives me an error saying "this() pointcut designator cannot be used in declare statements."  BTW,  target(), and args() also give the same error for declare statements i guess, so you can't filter on types of this, target, or args in declares.

Am I missing something?  Is there a reason for this or could these be enhancements?

Cheers,
--
-pete
peter m. murray
pete@xxxxxxxxxxxx

 


CONFIDENTIAL AND PRIVILEGED INFORMATION NOTICE

This e-mail, and any attachments, may contain information that
is confidential, subject to copyright, or exempt from disclosure.
Any unauthorized review, disclosure, retransmission,
dissemination or other use of or reliance on this information
may be unlawful and is strictly prohibited.  

AVIS D'INFORMATION CONFIDENTIELLE ET PRIVIL�GI�E

Le pr�sent courriel, et toute pi�ce jointe, peut contenir de
l'information qui est confidentielle, r�gie par les droits


d'auteur, ou interdite de divulgation. Tout examen,
divulgation, retransmission, diffusion ou autres utilisations

non autoris�es de l'information ou d�pendance non autoris�e
envers celle-ci peut �tre ill�gale et est strictement interdite.


_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users




--
-pete
peter m. murray
pete@xxxxxxxxxxxx


_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users




--
-pete
peter m. murray
pete@xxxxxxxxxxxx

_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users





--
-pete
peter m. murray
pete@xxxxxxxxxxxx
_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users


_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users





--
-pete
peter m. murray


pete@xxxxxxxxxxxx _______________________________________________

 


_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users




--
-pete
peter m. murray
pete@xxxxxxxxxxxx


_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users




--
-pete
peter m. murray
pete@xxxxxxxxxxxx


_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users





--
-pete
peter m. murray
pete@xxxxxxxxxxxx

Back to the top