Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access to original field value for set() #154

Open
kriegaex opened this issue Apr 10, 2022 · 3 comments
Open

Access to original field value for set() #154

kriegaex opened this issue Apr 10, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@kriegaex
Copy link
Contributor

kriegaex commented Apr 10, 2022

The original enhancement request Bugzilla #95716 exists since 2005. I think it should be implemented. I needed this many times myself, and e.g. it also came up on Stack Overflow lately and last year.

package de.scrum_master.app;

public class Application {
  int number;
  private boolean isActive;
  private boolean isNice;

  public static void main(String[] args) {
    Application app = new Application();

    // Value change -> trigger logging aspect
    app.number = 11;
    // Value change -> trigger logging aspect
    app.isActive = true;
    // No value change
    app.isNice = false;

    // Value change -> trigger logging aspect
    app.number = 22;
    // No value change
    app.isActive = true;
    // Value change -> trigger logging aspect
    app.isNice = true;

    // No value change
    app.number = 22;
    // No value change
    app.isActive = true;
    // No value change
    app.isNice = true;
  }
}

Now we want to create an aspect logging something if the write access changes the field value, i.e. we need access to the old value. Another example might be to use the old value in order to calculate something from it for validation, say the new value must be no more than 25% higher than the old one.

The current workaround for around() looks like this:

package de.scrum_master.aspect;

import java.lang.reflect.Field;

import org.aspectj.lang.SoftException;
import org.aspectj.lang.reflect.FieldSignature;

public privileged aspect MyAspect {
  void around(Object instance, Object newValue) :
    set(* *) && target(instance) && args(newValue)
  {
    Object oldValue;
    try {
      oldValue = ((FieldSignature) thisJoinPointStaticPart.getSignature()).getField().get(instance);
    } catch (IllegalArgumentException | IllegalAccessException e) {
      throw new SoftException(e);
    }
    if (oldValue == null && newValue != null || oldValue != null && !oldValue.equals(newValue))
      System.out.println(thisJoinPoint + ": " + oldValue + " -> " + newValue);
    proceed(instance, newValue);
  }
}

For before(), we need an additional field.setAccessible(true), because the code accessing the field is generated in the aspect rather than in the target class:

  before(Object instance, Object newValue) :
    set(* *) && target(instance) && args(newValue)
  {
    Object oldValue;
    try {
      Field field = ((FieldSignature) thisJoinPointStaticPart.getSignature()).getField();
      field.setAccessible(true);
      oldValue = field.get(instance);
    } catch (IllegalArgumentException | IllegalAccessException e) {
      throw new SoftException(e);
    }
    if (oldValue == null && newValue != null || oldValue != null && !oldValue.equals(newValue))
      System.out.println(thisJoinPoint + ": " + oldValue + " -> " + newValue);
  }

Of course, in an AspectJ implementation there would be no reflection from the user's perspective, and a privileged aspect would be able to access non-public fields.

Syntax-wise I would keep it simple and not use something new like oldargs(oldValue), but instead simply an optional second argument, i.e. set(* *) && args(newValue, oldValue), which would, if present, bind the old value.

BTW, a reminder to myself and whoever else might need it: The way to get the field value in get() is to

  • either use around(), get the current value via proceed() and then either pass it through or return something else,
  • or use after() returning(Object value).
@kriegaex kriegaex added the enhancement New feature or request label Apr 10, 2022
@FedericoBruzzone
Copy link

Hi @kriegaex, I am a student attending the master's degree in computer science. I am studying and using aspectJ on a daily basis, the idea behind it is brilliant. I have a background in writing reflective code and bytecode engineering as well.

I was wondering if this part was implemented? Taking time away from other things, I wanted to start studying the repo, I saw that you are currently the maintainer, is it right?

@kriegaex
Copy link
Contributor Author

I was wondering if this part was implemented?

@FedericoBruzzone: This has not been implemented yet, hence the open issue.

I saw that you are currently the maintainer, is it right?

Yes, this is basically correct. While not being the official project lead - that would be @aclement - I am currently the most active committer. Do you have any specific questions?

@FedericoBruzzone
Copy link

FedericoBruzzone commented Jan 14, 2023

Yes, this is basically correct. While not being the official project lead - that would be @aclement - I am currently the most active committer. Do you have any specific questions?

Thanks for your time. @kriegaex
I have not run into the problem of this issue. But in the next few days I will study the repo and maybe I'll ask you some question if I'm allowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants