eclipse-vertx / vert.x

Vert.x is a tool-kit for building reactive applications on the JVM
http://vertx.io
Other
14.26k stars 2.07k forks source link

Contravariant method parameter types declarations for Future #5314

Closed vietj closed 2 weeks ago

vietj commented 2 weeks ago

Motivation

io.vertx.core.Future does not use variance in its declarations preventing reuse or forcing to introduce adapter functions, e.g.

public void flatMap(Future<String> fut, Function<CharSequence, Future<Integer>> fn) {
  fut.flatMap(fn::apply); // produce a new function adapting "fn", it could be "fn" instead
}

public void onComplete(Future<String> fut, Promise<CharSequence> promise) {
  // That
  fut.onComplete(event -> promise.handle(event.map(s -> s))); // we would like to use "promise" instead

  // Or
  fut.map(s -> (CharSequence)s).onComplete(promise);
}

Changes

Three changes are necessary to solve this problem at the expense of small breaking changes

Use variance on existing methods

// After
<U> flatMap(Function<T, Future<U>> fn);
// After
<U> flatMap(Function<? super T, Future<U>> fn);

This changes remains source compatible (for user), there are breaking changes but those are for implementation of the Future type, which are acceptable.

It does not apply to Handler<AsyncResult<T>> arguments. In practice Handler<AsyncResult<T>> is solvable but not realistic, it should be Handler<? extends AsyncResult<? super T>> but this one exhibit issues with lambdas, e.g. future.onComplete(ar -> ar.result() /* Object and not T */).

Overloading handler of async results methods

There are two possible solutions

The former does not seem possible to use as overload because onComplete(ar -> ...) will be ambiguous. It could be solved by adding new methods (aliases) though.

The later does not introduce issues since we are adding method overloads

Future<T> onComplete(Handler<AsyncResult<T>> handler);
Future<T> onComplete(Completable<? super T> handler); // Overload

Likewise

<U> Future<U> transform(Function<AsyncResult<T>, Future<U>> fn);
<U> Future<U> transform(BiFunction<? super T, ? super Throwable, Future<U>> fn); // Overload

with

@FunctionalInterface
public interface Completable<T> {
  default void succeed(T value) {
    complete(value, null);
  }
  default void fail(Throwable cause) {
    complete(null, cause);
  }
  void complete(T value, Throwable err);
}

There are a few breaking changes though, when null is an argument, the compiler cannot decide with overload to use. We consider those are marginal, we found some of these in the vertx test suite to check that null arguments produces errors, e.g. future.onComplete(null).

Retrofit Promise as Completable instead of async result handler

Since now we have variant part with a Completable, the need Promise to extend Completable instead of Handler<AsyncResult<T>>

Promise<T> extends Completable<T> {
  ...
}

This creates breaking changes when Promise<T> was used at the place of Handler<AsyncResult<T>>, e.g. internally in vertx we still have a few of those, and promise::handle should be used to fix the issues. Since Vert.x 5 does not anymore exhibit Handler<AsyncResult<T>> methods, this should not be an issue for users.

This will require adaptation in Vert.x code base because there are many remaining usage of Handler<AsyncResult<T>> idiom: here is a recap of the necessary changes https://gist.github.com/vietj/02ce9dc89c8a1c11dabe8828f760f973 . This list is actually quite long, however it mostly solves internal issues of the vertx codebase still using Vert.x 3 idioms and was never migrated to use futures, e.g. recent vertx components like the new vertx-grpc or vertx-service-resolver do not need any changes.