2
@Component("taskCreateListener")
public class TaskCreateListener implements FlowableEventListener { 

@LogMethod
    @DetermineCaseTypeOfWork
    @Override
    public void onEvent(FlowableEvent event) {

///Do stuff that results in variables I want to pass to aspect
//For example, get ids and details and set to variables that I want to pass to insertThing() method once onEvent(FlowableEvent event) is finished executing
//ex:
//String procInstId = "abc1234";
//String type = "case1";
}

I need the onEvent to fully complete and then get local variables set in onEvent(FlowableEvent event) passed into my aspect insertThing() method:

@Aspect
@Component
public class DetermineCaseTypeOfWork {

@Transactional
@After(@annotation(path goes here))
public void insertThing() {
    //Do something here with the variables passed in from doSomething method
//if(procInstId.equals("abc1234") && type.equals("case1")) {
//do something
} else if(//other id and type) {
//do something else
} else {
//do something else
}
}

I can't modify the onEvent(FlowableEvent event) method to return something and the onEvent(FlowableEvent event) method has to fully complete first, so how would I go about passing parameters into insertThing()?

moesyzlack23
  • 105
  • 1
  • 11
  • Please edit your question and rephrase it. You said you have a method A (do you mean class?) with a method B and need a method go get something from your aspect method. You use the word "method" 4x in quite ambiguous ways. I suggest you let code speak. Please be advised to learn what an [MCVE](https://stackoverflow.com/help/mcve) is, then edit your question to be one. As soon as I understand (not speculate) what you want to do, I am pretty sure I can answer your question. :-) – kriegaex Feb 15 '20 at 02:07
  • There are multiple ways to achieve this . Your requirement is `@annotation` specific is it ? . More details on the `public void doSomething(Argument arg) { }` will let us help you better. – R.G Feb 15 '20 at 03:08
  • Thanks for the feedback and links, hope the extra details help – moesyzlack23 Feb 15 '20 at 16:56
  • More details will be appreciated, as pointed out already. The closest I can think of is something like this: @AfterReturning( pointcut = "execution(* com.foo.bar.MethodThatReturnsValue(..))", returning= "returnedVal") – z atef Feb 15 '20 at 18:26
  • @zee Thanks, I added a little more details. I can't change the TaskCreateListener onEvent(FlowableEvent event) method to return something (it's just a void method). Is there any other way to go about it? – moesyzlack23 Feb 15 '20 at 22:21
  • @moesyzlack23 using Spring AOP one can intercept before and after the method call . This means in this case we can get the `event ` argument in the advice method and process the same , but not the `local variables set in onEvent(FlowableEvent event)` which is within the method execution ( not before or after ) – R.G Feb 16 '20 at 03:03

2 Answers2

8

Preface / Rationale

I would not suggest to use Daniil's solution because

  • it adds the complexity of handling a thread-local variable to both the aspect (OK) and the core business code (not OK in my opinion),
  • the context holder, being a map, is not particularly type-safe,
  • the core business code does not work anymore without the aspect (it relies on the aspect initialising the context holder) and is inextricably linked to it, which is a bad design decision.

The main problem is in the OP's (moesyzlack23) way of thinking: He said, he wants to "pass parameters to the aspect". This is violating the basic AOP principle that the aspect should know how to add cross-cutting behaviour but the application code should be agnostic of the aspect.

AspectJ solution

I would suggest to

  • just add a method responsible for calculating the results to the TaskCreateListener class and calling it from onEvent(..),
  • switch from Spring AOP to AspectJ as described in the Spring manual and use features like cflowbelow pointcut and percflow aspect instantiation, thus getting rid of thread-locals and making the core code agnostic of AOP again,
  • optionally converting the Map<String, Object> into a more type-safe data object with regular getter methods. This would only be difficult if the aspect applies to many annotated methods with a very diverse set of data to be handled. But to me it looks like this aspect is pretty specific, though.

Here is an example in plain AspectJ (no Spring) which you can easily integrate into your Spring application after enabling full AspectJ:

Helper classes:

package de.scrum_master.app;

public class FlowableEvent {}
package de.scrum_master.app;

import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import java.lang.annotation.Retention;
import java.lang.annotation.Target;

@Retention(RUNTIME)
@Target(METHOD)
public @interface DetermineCaseTypeOfWork {}

Aspect target class:

package de.scrum_master.app;

public class TaskCreateListener {
  @DetermineCaseTypeOfWork
  public void onEvent(FlowableEvent event) {
    // Calculate values which might be dependent on 'event' or not
    Data data = calculateData(event);
    System.out.println("[" + Thread.currentThread().getId() + "] onEvent: " + data);
  }

  public Data calculateData(FlowableEvent event) {
    return new Data("thread-" + Thread.currentThread().getId(), "case1");
  }

  public static class Data {
    private String procInstId;
    private String type;

    public Data(String procInstId, String type) {
      this.procInstId = procInstId;
      this.type = type;
    }

    public String getProcInstId() {
      return procInstId;
    }

    public String getType() {
      return type;
    }

    @Override
    public String toString() {
      return "Data[procInstId=" + procInstId + ", type=" + type + "]";
    }
  }
}

The inner Data class is optional, you could just continue to use a Map<String, Object> and refactor the class and the aspect (see below) so as to use the map instead.

Driver application starting several threads:

package de.scrum_master.app;

public class Application {
  public static void main(String[] args) {
    for (int taskCount = 0; taskCount < 5; taskCount++) {
      new Thread(() -> new TaskCreateListener().onEvent(new FlowableEvent())).start();
    }
  }
}

Console log without aspect:

[11] onEvent: Data[procInstId=thread-11, type=case1]
[12] onEvent: Data[procInstId=thread-12, type=case1]
[13] onEvent: Data[procInstId=thread-13, type=case1]
[10] onEvent: Data[procInstId=thread-10, type=case1]
[14] onEvent: Data[procInstId=thread-14, type=case1]

So far, so simple.

Aspect:

package de.scrum_master.aspect;

import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.AfterReturning;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Pointcut;

import de.scrum_master.app.TaskCreateListener.Data;

@Aspect("percflow(myPointcut())")
public class DetermineTypeOfWorkAspect {
  private Data data;

  @Pointcut("execution(* *(..)) && @annotation(de.scrum_master.app.DetermineCaseTypeOfWork)")
  private void myPointcut() {}

  @Around("myPointcut()")
  public void insertThing(ProceedingJoinPoint joinPoint) throws Throwable {
    System.out.println("[" + Thread.currentThread().getId() + "] " + joinPoint);
    joinPoint.proceed();
    System.out.println("[" + Thread.currentThread().getId() + "] " + "insertThing: " + data);
  }

  @AfterReturning(pointcut = "execution(* *(..)) && cflowbelow(myPointcut())", returning = "result")
  public void saveData(JoinPoint joinPoint, Data result) throws Throwable {
    System.out.println("[" + Thread.currentThread().getId() + "] " + joinPoint);
    data = result;
  }
}

Please note:

  • @Aspect("percflow(myPointcut())") makes sure that the aspect is not a singleton, which would be the default. Instead, one aspect instance is created each time the application enters the control flow of myPointcut(), i.e. each time a method annotated by DetermineCaseTypeOfWork is being executed.
  • The @Around advice insertThing relies on the @AfterReturning advice saveData being executed during its waiting for joinPoint.proceed() to return.
  • The @AfterReturning advice saveData gets triggered for every method execution below the control flow of myPointcut() and returning a Data object. Methods returning something else or being executed outside the specified control flow will be ignored. The advice makes sure the result of the intercepted method call is being assigned to a private Data variable which can later be accessed by the insertThing advice.
  • I am adding execution(* *(..)) && to the pointcuts because in AspectJ there are other joinpoints such as method call() in addition method execution being the only supported type in Spring AOP. So there you don't need to be so specific, in AspectJ you ought to.
  • If you remove the percflow(myPointcut()) instantiation from the @Aspect annotation, you will have to make the private Data member a ThreadLocal<Data> instead in order to make the aspect thread-safe again. This also works and still keeps the core application free from thread-local handling, but the aspect itself would have to deal with it.

Console log with active aspect:

[10] execution(void de.scrum_master.app.TaskCreateListener.onEvent(FlowableEvent))
[14] execution(void de.scrum_master.app.TaskCreateListener.onEvent(FlowableEvent))
[12] execution(void de.scrum_master.app.TaskCreateListener.onEvent(FlowableEvent))
[13] execution(void de.scrum_master.app.TaskCreateListener.onEvent(FlowableEvent))
[11] execution(void de.scrum_master.app.TaskCreateListener.onEvent(FlowableEvent))
[14] execution(TaskCreateListener.Data de.scrum_master.app.TaskCreateListener.calculateData(FlowableEvent))
[14] onEvent: Data[procInstId=thread-14, type=case1]
[14] insertThing: Data[procInstId=thread-14, type=case1]
[11] execution(TaskCreateListener.Data de.scrum_master.app.TaskCreateListener.calculateData(FlowableEvent))
[11] onEvent: Data[procInstId=thread-11, type=case1]
[10] execution(TaskCreateListener.Data de.scrum_master.app.TaskCreateListener.calculateData(FlowableEvent))
[12] execution(TaskCreateListener.Data de.scrum_master.app.TaskCreateListener.calculateData(FlowableEvent))
[12] onEvent: Data[procInstId=thread-12, type=case1]
[10] onEvent: Data[procInstId=thread-10, type=case1]
[10] insertThing: Data[procInstId=thread-10, type=case1]
[12] insertThing: Data[procInstId=thread-12, type=case1]
[11] insertThing: Data[procInstId=thread-11, type=case1]
[13] execution(TaskCreateListener.Data de.scrum_master.app.TaskCreateListener.calculateData(FlowableEvent))
[13] onEvent: Data[procInstId=thread-13, type=case1]
[13] insertThing: Data[procInstId=thread-13, type=case1]

Please note how the thread IDs at the beginning of each log line correspond to the value of procInstId. This proves that it actually works without thread-locals due to the percflow aspect instantiation model.

Spring AOP solution

Alternative with Spring AOP: If you want to stick with Spring AOP, you can use neither percflow instantiation nor cflowbelow pointcuts because Spring AOP just does not support those features. So instead of the former you could still use a ThreadLocal inside the aspect and instead of the latter you could factor the calculation out into a separate Spring component/bean and make sure the saveData advice intercepts that one instead. So probably the cost of not having to use AspectJ (if you are so inclined to avoid it) would still be acceptable: one thread-local plus one new component. Please let me know if you are interested in that approach.


Update:

I would also be interested in seeing your approach using Spring AOP if you wouldn't mind sharing.

Fine. I will post the complete MCVE again with different package names in order to disambiguate all classes (some with minor or major changes) from the AspectJ sample code.

Helper classes:

package de.scrum_master.spring.q60234800;

public class FlowableEvent {}
package de.scrum_master.spring.q60234800;

import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import java.lang.annotation.Retention;
import java.lang.annotation.Target;

@Retention(RUNTIME)
@Target(METHOD)
public @interface DetermineCaseTypeOfWork {}

Data provider component:

This is the new component/bean I was talking about. Again, instead of the inner Data class you could use a Map but that would be less type-safe. The decision depends on how specific or generic you need your solution to be. What is important here is that the provider itself is a singleton bean but provides a new Data instance on each calculateData(..) call. So you need to make sure that method only depends on its input parameters and not on class fields in order to be thread-safe.

package de.scrum_master.spring.q60234800;

import org.springframework.stereotype.Component;

@Component
public class DataProvider {
  public Data calculateData(FlowableEvent event) {
    return new Data("thread-" + Thread.currentThread().getId(), "event-" + event.hashCode());
  }

  public static class Data {
    private String procInstId;
    private String type;

    public Data(String procInstId, String type) {
      this.procInstId = procInstId;
      this.type = type;
    }

    @Override
    public String toString() {
      return "Data[procInstId=" + procInstId + ", type=" + type + "]";
    }
  }
}

Listener component:

This is also a normal singleton bean which gets the data provider auto-injected.

package de.scrum_master.spring.q60234800;

import de.scrum_master.spring.q60234800.DataProvider.Data;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

@Component
public class TaskCreateListener {
  @Autowired
  DataProvider dataProvider;

  @DetermineCaseTypeOfWork
  public void onEvent(FlowableEvent event) {
    // Calculate values which might be dependent on 'event' or not
    Data data = dataProvider.calculateData(event);
    System.out.println("[" + Thread.currentThread().getId() + "] onEvent: " + data);
  }
}

Driver application:

Again, the application creates multiple threads and fires TaskCreateListener.onEvent(..) for each of them.

package de.scrum_master.spring.q60234800;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.EnableAspectJAutoProxy;

import java.util.stream.IntStream;

@SpringBootApplication
@Configuration
@EnableAspectJAutoProxy//(proxyTargetClass = true)
public class Application {
  public static void main(String[] args) {
    try (ConfigurableApplicationContext appContext = SpringApplication.run(Application.class, args)) {
      TaskCreateListener taskCreateListener = appContext.getBean(TaskCreateListener.class);
      IntStream.range(0, 5).forEach(i ->
        new Thread(() -> taskCreateListener.onEvent(new FlowableEvent())).start()
      );
    }
  }
}

Spring AOP aspect:

As described earlier, we need a ThreadLocal<Data> field for thread-safety because also the aspect is a singleton bean. The combination of two pointcut/advice pairs targeting two different components makes sure that first we gather and save the right Data and then use them in the other advice.

package de.scrum_master.spring.q60234800;

import de.scrum_master.spring.q60234800.DataProvider.Data;
import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.AfterReturning;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.springframework.stereotype.Component;

@Aspect
@Component
public class DetermineTypeOfWorkAspect {
  private ThreadLocal<Data> data = new ThreadLocal<>();

  @Around("@annotation(de.scrum_master.spring.q60234800.DetermineCaseTypeOfWork)")
  public void insertThing(ProceedingJoinPoint joinPoint) throws Throwable {
    System.out.println("[" + Thread.currentThread().getId() + "] " + joinPoint);
    joinPoint.proceed();
    System.out.println("[" + Thread.currentThread().getId() + "] " + "insertThing: " + data.get());
  }

  @AfterReturning(pointcut = "execution(* calculateData(..))", returning = "result")
  public void saveData(JoinPoint joinPoint, Data result) throws Throwable {
    System.out.println("[" + Thread.currentThread().getId() + "] " + joinPoint);
    data.set(result);
  }
}

Console log:

Just like in the AspectJ solution the thread IDs at the beggining of the log lines correspond with the ones captured in the Data objects.

  .   ____          _            __ _ _
 /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
 \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
  '  |____| .__|_| |_|_| |_\__, | / / / /
 =========|_|==============|___/=/_/_/_/
 :: Spring Boot ::        (v1.5.2.RELEASE)

(...)
2020-02-20 08:03:47.494  INFO 12864 --- [           main] s.b.c.e.t.TomcatEmbeddedServletContainer : Tomcat started on port(s): 8080 (http)
2020-02-20 08:03:47.498  INFO 12864 --- [           main] d.s.spring.q60234800.Application         : Started Application in 4.429 seconds (JVM running for 5.986)
[33] execution(void de.scrum_master.spring.q60234800.TaskCreateListener.onEvent(FlowableEvent))
[32] execution(void de.scrum_master.spring.q60234800.TaskCreateListener.onEvent(FlowableEvent))
[35] execution(void de.scrum_master.spring.q60234800.TaskCreateListener.onEvent(FlowableEvent))
[34] execution(void de.scrum_master.spring.q60234800.TaskCreateListener.onEvent(FlowableEvent))
[36] execution(void de.scrum_master.spring.q60234800.TaskCreateListener.onEvent(FlowableEvent))
[33] execution(Data de.scrum_master.spring.q60234800.DataProvider.calculateData(FlowableEvent))
[35] execution(Data de.scrum_master.spring.q60234800.DataProvider.calculateData(FlowableEvent))
[34] execution(Data de.scrum_master.spring.q60234800.DataProvider.calculateData(FlowableEvent))
[33] onEvent: Data[procInstId=thread-33, type=event-932577999]
[33] insertThing: Data[procInstId=thread-33, type=event-932577999]
[34] onEvent: Data[procInstId=thread-34, type=event-1335128372]
[34] insertThing: Data[procInstId=thread-34, type=event-1335128372]
[36] execution(Data de.scrum_master.spring.q60234800.DataProvider.calculateData(FlowableEvent))
[36] onEvent: Data[procInstId=thread-36, type=event-130476008]
[32] execution(Data de.scrum_master.spring.q60234800.DataProvider.calculateData(FlowableEvent))
[36] insertThing: Data[procInstId=thread-36, type=event-130476008]
[35] onEvent: Data[procInstId=thread-35, type=event-987686114]
[35] insertThing: Data[procInstId=thread-35, type=event-987686114]
[32] onEvent: Data[procInstId=thread-32, type=event-1849439251]
[32] insertThing: Data[procInstId=thread-32, type=event-1849439251]
kriegaex
  • 63,017
  • 15
  • 111
  • 202
  • Thanks for detailed explanation - I will try the first approach but I would also be interested in seeing your approach using Spring AOP if you wouldn't mind sharing. Thanks again! – moesyzlack23 Feb 19 '20 at 18:48
  • See my rather lengthy answer update for the Spring AOP solution. Just like with AspectJ, the goal was to not just make it thread-safe but also to keep any dependencies on aspects and/or direct handling of thread-locals out of the application classes. The aspect deals with that as it should. If you delete the aspect, the application will still run. – kriegaex Feb 20 '20 at 01:23
  • In case of Spring solution, is there a need to `.remove()` from `ThreadLocal` before returning from the `@Around` advise? Or is this done automatically when a thread finishes execution? – Airwavezx Jun 27 '21 at 10:27
  • Why are you asking? Did you have any problems with my solution? If so, please describe them. In case the description is too complex for a comment, please ask a new question, include an [MCVE](https://stackoverflow.com/help/mcve) and explain how the actual result deviates from your expectation. – kriegaex Jun 27 '21 at 16:29
3

According your question, there is no possibility to change signature of the onEvent() method, which should be handled by an aspect. You could try to create a ThreadLocal based container class which is initialized in aspect before calling onEvent() and assessed after finishing onEvent(). But that approach requires you to be able to edit onEvent() code (but do not requires change its returning type). Here are some details:

public class VariableContextHolder {

/**
 * ThreadLocal with map storing variables
 */
private final ThreadLocal<Map<String, Object>> threadlocal = new ThreadLocal<>();

private static VariableContextHolder instance;

private VariableContextHolder () {

}

public final static VariableContextHolder getInstance() {
    if (instance == null) {
        instance = new VariableContextHolder ();
    }
    return instance;
}

public Map<String, Object>get() {
    return threadlocal.get();
}

public void set(Map<String, Object>map) {
    threadlocal.set(map);
}

public void clear() {
    threadlocal.remove();
}
}

Aspect class:

@Aspect()
public class DetermineCaseTypeOfWork {

@Transactional
@Around(@annotation("path goes here"))
public void insertThing(ProceedingJoinPoint joinPoint) throws Throwable {

// save initialized map to threadlocal    
VariableContextHolder.getInstance().set(new HashMap<>());

// method onEvent() will be called
joinPoint.proceed();

// retrieve map from threadlocal
Map<String, Object> variablesMap = VariableContextHolder.getInstance().get();

// get variables by names and handle them
String procInstId = variablesMap.get("procInstId");

// clear threadlocal after using it
VariableContextHolder.getInstance().clear();
}

}

Changes needed to be done in onEvent() method:

public void onEvent(FlowableEvent event) {

    // retrieve map from threadlocal
    Map<String, Object> variablesMap = VariableContextHolder.getInstance().get();
    String procInstId = "abc1234";
    String type = "case1";
    // save variables to map
    variablesMap.put("procInstId", procInstId);
    variablesMap.put("type", type);
}
Daniil
  • 913
  • 8
  • 19