Aspect + RequestContextResolver: Support response reading in method interceptor

[LoonyRules]

First of, thanks @SentryMan for working hard on the RequestContextResolver functionality I requested on the Discord, but it looks like we forgot one crucial piece of information in our design; reading the computed data out of the Context.

So if you have a method interceptor of whom's purpose is to cache the computed result for X time you will have code like the following in the invoke method:

... hidden code to get the context of the current thread ...

Context context = serverContext.request();
CachedResponse cachedResponse = cache.get("some-key");

// Previously computed, just write to the Context
if (cachedResponse != null) {
  context.statusCode(cacheResponse.statusCode())
         .contentType(cachedResponse.contentType()
         .result(content.result());
  return;
}

// Not previously cached, so lets call the endpoint handler.
// We don't actually use `returnedObject`, we don't need it as we 
// just want the data written to the http Response (Context for Javalin).
Object returnedObject = invocation.invoke();

// Now we need to make a CachedResponse and put it into the cache.
cachedResponse = new CachedResponse(
  context.statusCode(),
  context.contentType(),
  context.result()
);
cache.put("some-key", cachedResponse);

The issue is, the way we currently proxy the call is for the endpoint method itself, so the Context isn't written to until after the intercepted method is completed. Of which is different to how I interpreted such functionality would work. Otherwise, ServerContext#response() is useless anyway as it'd always be overridden outside of the method interceptor.

Currently, the current generated code is:

ApiBuilder.get("/cache/hello-world", ctx -> {
  ctx.status(200);
  var result = resolver.supplyWith(new ServerContext(ctx, ctx), () -> controller.helloWorld());
  stringJsonType.toJson(result, ctx.contentType("application/json").outputStream());
});

but I believe this this specific kind of implementation should actually function as:

ApiBuilder.get("/cache/hello-world", ctx -> {
  resolver.supplyWith(
    new ServerContext(ctx, ctx),
    () -> {
      ctx.status(200);
      var result = controller.helloWorld();
      stringJsonType.toJson(result, ctx.contentType("application/json").outputStream());
    }
  );
});

As this allows the aspect method interceptor to call the original method and its response writing, when Invocation#invoke() is called, not every time the method is being proxied, of which would mean I'd have to provide a Invocation#result(Object) every time, of which is inefficient.

Think about if you stored the CachedResponse in Redis in a HASH. format. You have the raw value data that you just need to write back to the Context using:
context.statusCode(cacheResponse.statusCode()).contentType(cachedResponse.contentType().result(content.result());

You don't want to have to store a JSON payload, just to turn that into a POJO for the jsonb library to convert that back into a JSON payload. It's a waste of computation, memory and time when you already stored the previously computed Response data.

If you still wanted to support Invocation#result(Object) then the new design could just be:

ApiBuilder.get("/cache/hello-world", ctx -> {
  resolver.supplyWith(
    new ServerContext(ctx, ctx),
    // Computed on `Invocation#invoke()` 
    () -> {
      return controller.helloWorld();
    },
    // `result` is either from the above result, or `Invocation#result(Object)` but is 
    // only ran if `Invocation#invoke()` or `Invocation#result(Object)` is called.
    (result) -> {
      ctx.status(200);
      stringJsonType.toJson(result, ctx.contentType("application/json").outputStream());
    }
  );
});

[LoonyRules]

After a bit of extra thought, if avaje as a product doesn't want to steer away from the general AOP principles to support such functionality, how about a HttpResponse class that houses some properties (eg: statusCode, headers, cookies, contentType, body, etc) and then the generated code can be as follows:

ApiBuilder.get("/cache/hello-world", ctx -> {
  ctx.status(200);
  var result = resolver.supplyWith(new ServerContext(ctx, ctx), () -> controller.helloWorld());
  
  if (result instanceof HttpResponse) {
    // Write the properties in HttpResponse to the Context
  } else {
    // Write the value based on what the endpoint method's return type is.
    stringJsonType.toJson(result, ctx.contentType("application/json").outputStream());
  }
});

I would advise against requiring the endpoint method to return a HttpResponse<Object> object as from an end-user's perspective, that's an ugly enforced structure just to support cached responses and in my opinion, would actually steer users away from using such product/feature.

I did think about throwing a HttpResponseException in the method interceptor and then handle reading the Context's response from the exception handler in Javalin, but it other framework implementations (eg: Nima) probably don't have that functionality, but also the Context still isn't going to get the Response written to it as the exception being thrown interrupts the call stack anyway.

I cannot think of any other way that allows such functionality to be supported, but personally I believe avaje steering away from AOP principles for this specific use case, is probably best. As I stated in the issue description, currently ServerContext#response() is unusable as well due to the current implementation.

I don't mind attempting to make the changes to support what I would like, just primarily want the approval that avaje is willing to accept such a strategy firstly.

[SentryMan]

See the problem is still that you can't modify a return value to a different type than the signature with AOP. So no matter what you'd need to modify your controller to return something like HttpResponse<Object>.

It sounds more like that you want a before/after interceptor that'll run before/after a controller method runs. I could create a new construct for that and have the code generate it like this.

// you could implement this to handle your caching logic
interface SeverInterceptor {
// boolean so that the before can skip executing the controller method if desired
  default <T> boolean before(T ctx) {
    return true;
  }
  // will NOT execute if controller throws exception
  default <T> void after(T ctx) {}
}

ApiBuilder.get("/cache/hello-world", ctx -> {
  ctx.status(200);
  if (interceptor.before(ctx)) {
     var result = resolver.supplyWith(new ServerContext(ctx, ctx), () -> controller.helloWorld());
     stringJsonType.toJson(result, ctx.contentType("application/json").outputStream());
  }
  interceptor.after(ctx);
});

Would this help you solve your caching issue?