vert-x3 / vertx-service-proxy

EventBus Proxy generation
Apache License 2.0
66 stars 58 forks source link

Interceptors should allow more granularity (per method) #126

Open pmlopes opened 3 years ago

pmlopes commented 3 years ago

Interceptors are defined as List<Function<Message<JsonObject>, Future<Message<JsonObject>>>>. This structure works well if we have to apply global interceptors, however there are situations such as authorization where an interceptor should only be executed for a specific method.

I'd propose to improve this by switching the interceptor signature from:

Function<Message<JsonObject>, Future<Message<JsonObject>>>

To:

BiFunction<String, Message<JsonObject>, Future<Message<JsonObject>>>

Where an extra argument of type string is added. If this argument is null then the behavior should be as now (applied globally), of the value is not null, the interceptor should only be applied if the message header action is equal to the given string.

Something like:

https://github.com/vert-x3/vertx-service-proxy/blob/master/src/main/java/io/vertx/serviceproxy/ProxyHandler.java#L97

if targetMethod == null || msg.headers().get("action").equals(targetMethod) {
  // execute as before
} else {
  // call the next interceptor directly
  prev.handle(msg);
}
Vorimo commented 3 years ago

If I am not mistaken, the only place where msg can get an action header is interceptor.apply() method, so at this https://github.com/vert-x3/vertx-service-proxy/blob/master/src/main/java/io/vertx/serviceproxy/ProxyHandler.java#L97 step no one interceptor will contain that field. Does that make sense?

pmlopes commented 3 years ago

@Vorimo yes, I made a mistake, the interceptor can remain the same signature, now the internal storage should be enhanced, instead of a plain list it needs to be a list of a tuple {action, interceptor}.

say for example:

class InterceptorHolder {
  String action;
  Function<Message<JsonObject>, Future<Message<JsonObject>>> interceptor;
}

if we store the interceptors in List<InterceptorHolder> then we can filter which interceptors should be run based in the previous rules:

if holder.action == null, then run interceptor
else if holder.action == msg.header[action] then run the interceptor
Vorimo commented 3 years ago

@pmlopes right, but how the msg.header[action] can get to the msg before that filter?

pmlopes commented 3 years ago

@Vorimo it should be available at runtime, for example, if we look at the code we're discussing:

handler = msg -> {
  // handler is a java lambda, "msg" is the argument of the lambda,
  // if we would inspect the type of "handler" then we know that it is
  // "Message"

  // so we can now extract the action from it
  String action = msg.headers().get("action");

  // the action cannot be null as it's part of the service proxy agreement, if it's null we need
  // to throw an error, or even better, fail the message "msg.fail(...)"

  Future<Message<JsonObject>> fut = interceptor.apply(msg);
...

I hope this makes it more clear, if not, then I'm not really understanding the question sorry :smiley:

Vorimo commented 3 years ago

@pmlopes OK, got it, thanks!