Grim
Grim

Reputation: 1976

Spring Aspect for field

I have a bean to report to InfluxDB. The Database have in the table INFLUX_DB_SERVER registred InfluxDBs. If you look at the code you will see that the Method reportMemory do a lot of work, it constructs a Measurement and do call reportAll, all this work is useless when there is no InfluxDB.

So the idea is to skip that work if there is no InfluxDB. Since the public-void-methods do not return a value it has no impact to the surrounding application.

What I could do is I could write a method isWorkPossible and call the method every call. That might follow KISS but that violates DRY. So I like to archivee that using AOP.

But I like to skip the execution of all public void methods if no InfluxDBs are registred.

/**
 * The reporter to notify {@link InfluxDB influxDBs} for changes.
 */
@Named
public class InfluxDBReporter {
    /**
     * Logger for reporting. For security reasons neither the username nor the
     * password should be logged above {@link Level#FINER}.
     */
    private static final Logger LOG = Logger.getLogger(InfluxDBReporter.class.getCanonicalName());

    /**
     * The entitymanager to use, never <code>null</code>.
     */
    @PersistenceContext
    private final EntityManager entityManager = null;

    /**
     * The registred {@link InfluxDBServer} in key and the URL in value.
     */
    @SkipPublicVoidMethodsIfEmpty
    private final Map<InfluxDB, URL> dbs = new LinkedHashMap<>();

    /**
     * Initializes the connections.
     */
    @PostConstruct
    private void connect() {
        for (InfluxDBServer db : FROM(囗InfluxDBServer.class).all(entityManager)) {
            try {
                URL dbUrl = new URL(db.getUrl());
                InfluxDB idb = InfluxDBFactory.connect(db.getUrl(), db.getUsername(), db.getPassword());
                idb.setDatabase(db.getDatabaseName());
                dbs.put(idb, dbUrl);
            } catch (MalformedURLException e) {
                LOG.log(Level.SEVERE, db.getUrl(), e);
            }
        }
    }

    /**
     * Closes all connections to all {@link InfluxDB}.
     */
    @PreDestroy
    private void disconnect() {
        for (InfluxDB influxDB : dbs.keySet()) {
            try {
                influxDB.close();
            } catch (Exception e) {
                // Fault barrier
                LOG.log(Level.WARNING, "InfluxDBServer URL: " + dbs.get(idb), e);
            }
        }
    }

    /**
     * Report memory statistics.
     * 
     * @param availableProcessors Amount of available processors, never negative.
     * @param totalMemory         The total memory, never negative.
     * @param maxMemory           The max memory, never negative.
     * @param freeMemory          The free memory, never negative.
     */
    public void reportMemory(int availableProcessors, long totalMemory, long maxMemory, long freeMemory) {
        reportAll(Point.measurement("jvm").addField("totalMemory", totalMemory).addField("maxMemory", maxMemory)
                .addField("freeMemory", freeMemory));
    }

    /**
     * Report a point to all connected {@link InfluxDBServer}.
     * 
     * @param builder The point to report.
     */
    private void reportAll(Builder builder) {
        Point infoPoint = builder.time(System.currentTimeMillis(), TimeUnit.MILLISECONDS).build();
        for (InfluxDB idb : dbs.keySet()) {
            new Thread(() -> {
                try {
                    idb.write(infoPoint);
                } catch (Exception e) {
                    // Fault barrier
                    LOG.log(Level.WARNING, "InfluxDBServer URL: " + dbs.get(idb), e);
                    throw e;
                }
            }).start();
        }
    }
}

This is my aspect:

@Aspect
public class MethodAnnotations {
    @Pointcut("@annotation(xxx.MethodAnnotations.SkipPublicVoidMethodsIfEmpty)")
    private void anyOldTransfer(JoinPoint jp) {
        System.out.println(jp); <----- never executed.
    }

    public @interface SkipPublicVoidMethodsIfEmpty {
    }
}

I expect the System.out.println to run when the bean is instanticated but it does not.

Any idea why?

Upvotes: 0

Views: 1873

Answers (1)

kriegaex
kriegaex

Reputation: 67297

As JB Nizet already said, @annotation(my.package.MyAnnotation) is designed to capture annotations on methods, not on fields, which explains why your expectation for anything to happen there is wrong.

If you want to find out via AOP if a class has a member with a specific annotation, you need to use a special pointcut like hasfield(@MyAnnotation * *). But that pointcut is unavailable in Spring AOP, you need to switch to AspectJ. The same is true if you want to intercept read/write access to such fields via get(@MyAnnotation MyType *) or set(@MyAnnotation MyType *).

For more details see my other answer here.

AspectJ also provides special pointcuts

  • to intercept static initialisation of a class after class-loading -> staticinitialization()
  • to intercept constructor execution -> MyType.new()

You can use those in order to execute your aspect advice whenever it is the right time to do so. In your example you could also more easily hook into @PostConstruct methods if it is clear that all target classes have one of those.

My answer is quite generic because you do not explain in detail what exactly you want to do. So feel free to ask follow-up questions.


Update: I checked your latest question update. I don't get it, this is a very contrived solution for a very simple problem also not a good case to be solved by AOP. As much as I love AOP, I am failing to see how this situation is a cross-cutting concern:

  • It seems to affect only a single class, InfluxDBReporter.
  • You are using an annotation which exists for the sole purpose of telling an aspect what to do.
  • To make things worse, you put the annotation onto a private field, but expect an external class (an aspect in this case) to react on it. While this is technically possible with AspectJ, it is bad design because you are bleeding private information to the outside.
  • By skipping the public method from your sample class you do not save any expensive DB-related operations because iterating over an empty KeySet means that just nothing will happen, so there also will not be any DB-related errors. The only thing really happening here are the builder calls. They should be cheap.

Even assuming you have many more public methods which should be skipped, I would actually design the AOP solution like this if you do want to stick with the approach:

  • Add a method public boolean isConnectedToDB() { return !dbs.isEmpty(); } to your application class.
  • In your aspect, use an @Around advice and call the boolean method from there, only calling joinPoint.proceed() if there are any connections. Otherwise don't proceed but do nothing instead (for void methods) or return a dummy result like null (for non-void methods).

The exact solution depends on whether you have only this one class or multiple ones with similar requirements, if you only have public void methods or also non-void ones.

Besides, you are mentioning INFLUX_DB_SERVER but I have no idea what this is because I cannot see it anywhere in your code.

Last, but not least: I just noticed that you expect something to happen in a method annotated by @Pointcut. Sorry, even if the pointcut was not wrong sothing would happen there because a pointcut definition is just to be used in an actual advice method such as @Before, @After, @Around. The actions you want to be performed go into the advice, not into the pointcut. I suggest you learn AOP basics first before you try to design AOP-based solutions.

Upvotes: 1

Related Questions