Skip to content

Commit

Permalink
Deprecate HttpResponse.from() methods (#5075)
Browse files Browse the repository at this point in the history
Motivation:
We have decided to deprecate `HttpResponse.from(...)` methods and use `HttpResponse.of(...)` instead for consistency. #4995 (comment)

Modification:
- Deprecate `HttpResponse.from(CompletionStage)` and its variants.
  - `HttpResponse.of(CompletionStage)` and its variants are added.

Result:
- `HttpResponse.from(CompletionStage)` and its variants methods are deprecated.
  - Use `HttpResponse.of(CompletionStage)` and its variants instead.
  • Loading branch information
minwoox committed Jul 31, 2023
1 parent b451e85 commit 4edff55
Show file tree
Hide file tree
Showing 88 changed files with 215 additions and 171 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public void setUp() {
.service("/get", (ctx, req) -> {
return HttpResponse.of("Hello! Armeria");
}).service("/post", (ctx, req) -> {
return HttpResponse.from(req.aggregate().thenApply(agg -> {
return HttpResponse.of(req.aggregate().thenApply(agg -> {
return HttpResponse.of(agg.contentUtf8());
}));
}).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ protected void configure(ServerBuilder sb) {
.responseTimeoutMillis(0);
});

return HttpResponse.from(CompletableFuture.supplyAsync(() -> {
return HttpResponse.of(CompletableFuture.supplyAsync(() -> {
// Make sure the current thread is not context-aware.
assertThat(ServiceRequestContext.currentOrNull()).isNull();
assertThat(currentTraceContext.get()).isNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ protected HttpResponse doGet(ServiceRequestContext ctx, HttpRequest req)
}))).collect(toImmutableList()));

final CompletableFuture<HttpResponse> responseFuture = new CompletableFuture<>();
final HttpResponse res = HttpResponse.from(responseFuture);
final HttpResponse res = HttpResponse.of(responseFuture);
transformAsync(spanAware,
result -> allAsList(IntStream.range(1, 3).mapToObj(
i -> executorService.submit(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ protected void init() {

HttpResponse asyncResponse(Consumer<CompletableFuture<HttpResponse>> completeResponse) {
final CompletableFuture<HttpResponse> responseFuture = new CompletableFuture<>();
final HttpResponse res = HttpResponse.from(responseFuture);
final HttpResponse res = HttpResponse.of(responseFuture);
CommonPools.workerGroup().next().submit(
() -> completeResponse.accept(responseFuture));
return res;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ protected void configure(ServerBuilder sb) {
.thenAcceptAsync(log -> {
serviceMdcContextRef.set(MDC.getCopyOfContextMap());
}, ctx.eventLoop());
return HttpResponse.from(
return HttpResponse.of(
server.webClient(cb -> cb.decorator(BraveClient.newDecorator(tracing)))
.get("/bar").aggregate().thenApply(res -> {
return HttpResponse.of("OK");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,16 +194,16 @@ public static class EchoService extends AbstractHttpService {

@Override
protected final HttpResponse doHead(ServiceRequestContext ctx, HttpRequest req) {
return HttpResponse.from(req.aggregate()
.thenApply(aReq -> HttpResponse.of(HttpStatus.OK))
.exceptionally(CompletionActions::log));
return HttpResponse.of(req.aggregate()
.thenApply(aReq -> HttpResponse.of(HttpStatus.OK))
.exceptionally(CompletionActions::log));
}

@Override
protected final HttpResponse doPost(ServiceRequestContext ctx, HttpRequest req) {
return HttpResponse.from(req.aggregate()
.thenApply(this::echo)
.exceptionally(CompletionActions::log));
return HttpResponse.of(req.aggregate()
.thenApply(this::echo)
.exceptionally(CompletionActions::log));
}

protected HttpResponse echo(AggregatedHttpRequest aReq) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ final class DefaultWebClient extends UserClient<HttpRequest, HttpResponse> imple

DefaultWebClient(ClientBuilderParams params, HttpClient delegate, MeterRegistry meterRegistry) {
super(params, delegate, meterRegistry,
HttpResponse::from, (ctx, cause) -> HttpResponse.ofFailure(cause));
HttpResponse::of, (ctx, cause) -> HttpResponse.ofFailure(cause));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ private static BiPredicate<ClientRequestContext, String> domainFilter(
@Override
public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) throws Exception {
final CompletableFuture<HttpResponse> responseFuture = new CompletableFuture<>();
final HttpResponse res = HttpResponse.from(responseFuture, ctx.eventLoop());
final HttpResponse res = HttpResponse.of(responseFuture, ctx.eventLoop());
final RedirectContext redirectCtx = new RedirectContext(ctx, req, res, responseFuture);
if (ctx.exchangeType().isRequestStreaming()) {
final HttpRequestDuplicator reqDuplicator = req.toDuplicator(ctx.eventLoop().withoutContext(), 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public abstract class UserClient<I extends Request, O extends Response>
* @param delegate the {@link Client} that will process {@link Request}s
* @param meterRegistry the {@link MeterRegistry} that collects various stats
* @param futureConverter the {@link Function} that converts a {@link CompletableFuture} of response
* into a response, e.g. {@link HttpResponse#from(CompletionStage)}
* into a response, e.g. {@link HttpResponse#of(CompletionStage)}
* and {@link RpcResponse#from(CompletionStage)}
* @param errorResponseFactory the {@link BiFunction} that returns a new response failed with
* the given exception
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ private static void completeExceptionally(ClientRequestContext ctx,
/**
* Implement this method to return a new {@link Response} which delegates to the {@link Response}
* the specified {@link CompletionStage} is completed with. For example, you could use
* {@link HttpResponse#from(CompletionStage, EventExecutor)}:
* {@link HttpResponse#of(CompletionStage, EventExecutor)}:
* <pre>{@code
* protected HttpResponse newDeferredResponse(
* ClientRequestContext ctx, CompletionStage<HttpResponse> resFuture) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,6 @@ private ConcurrencyLimitingClient(HttpClient delegate, ConcurrencyLimit concurre
@Override
protected HttpResponse newDeferredResponse(ClientRequestContext ctx,
CompletionStage<HttpResponse> resFuture) throws Exception {
return HttpResponse.from(resFuture, ctx.eventLoop());
return HttpResponse.of(resFuture, ctx.eventLoop());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public static Function<? super HttpClient, RetryingClient> newDecorator(RetryRul
@Override
protected HttpResponse doExecute(ClientRequestContext ctx, HttpRequest req) throws Exception {
final CompletableFuture<HttpResponse> responseFuture = new CompletableFuture<>();
final HttpResponse res = HttpResponse.from(responseFuture, ctx.eventLoop());
final HttpResponse res = HttpResponse.of(responseFuture, ctx.eventLoop());
if (ctx.exchangeType().isRequestStreaming()) {
final HttpRequestDuplicator reqDuplicator = req.toDuplicator(ctx.eventLoop().withoutContext(), 0);
doExecute0(ctx, reqDuplicator, req, res, responseFuture);
Expand Down Expand Up @@ -319,7 +319,7 @@ private void doExecute0(ClientRequestContext ctx, HttpRequestDuplicator rootReqD
ClientPendingThrowableUtil.removePendingThrowable(derivedCtx);
// if the endpoint hasn't been selected, try to initialize the ctx with a new endpoint/event loop
response = initContextAndExecuteWithFallback(
unwrap(), ctxExtension, endpointGroup, HttpResponse::from,
unwrap(), ctxExtension, endpointGroup, HttpResponse::of,
(context, cause) -> HttpResponse.ofFailure(cause));
} else {
response = executeWithFallback(unwrap(), derivedCtx,
Expand Down
102 changes: 73 additions & 29 deletions core/src/main/java/com/linecorp/armeria/common/HttpResponse.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,12 @@ static HttpResponseWriter streaming() {
* closed with the same cause as well.
*
* @param stage the {@link CompletionStage} which will produce the actual {@link HttpResponse}
*
* @deprecated Use {@link #of(CompletionStage)}.
*/
//TODO(minwoox): Rename this to 'of' for consistency.
@Deprecated
static HttpResponse from(CompletionStage<? extends HttpResponse> stage) {
requireNonNull(stage, "stage");

if (stage instanceof CompletableFuture) {
return createHttpResponseFrom((CompletableFuture<? extends HttpResponse>) stage);
} else {
final DeferredHttpResponse res = new DeferredHttpResponse();
res.delegateWhenComplete(stage);
return res;
}
return of(stage);
}

/**
Expand All @@ -106,11 +100,11 @@ static HttpResponse from(CompletionStage<? extends HttpResponse> stage) {
*
* @param future the {@link CompletableFuture} which will produce the actual {@link HttpResponse}
*
* @deprecated Use {@link #from(CompletionStage, EventExecutor)}.
* @deprecated Use {@link #of(CompletionStage)}.
*/
@Deprecated
static HttpResponse from(CompletableFuture<? extends HttpResponse> future) {
return createHttpResponseFrom(future);
return of(future);
}

/**
Expand All @@ -122,35 +116,26 @@ static HttpResponse from(CompletableFuture<? extends HttpResponse> future) {
* @param subscriberExecutor the {@link EventExecutor} which will be used when a user subscribes
* the returned {@link HttpResponse} using {@link #subscribe(Subscriber)}
* or {@link #subscribe(Subscriber, SubscriptionOption...)}.
*
* @deprecated Use {@link #of(CompletionStage, EventExecutor)}.
*/
@Deprecated
static HttpResponse from(CompletionStage<? extends HttpResponse> stage,
EventExecutor subscriberExecutor) {
requireNonNull(stage, "stage");
requireNonNull(subscriberExecutor, "subscriberExecutor");
// Have to use DeferredHttpResponse to use the subscriberExecutor.
final DeferredHttpResponse res = new DeferredHttpResponse(subscriberExecutor);
res.delegateWhenComplete(stage);
return res;
return of(stage, subscriberExecutor);
}

/**
* Creates a new HTTP response that delegates to the {@link HttpResponse} provided by the {@link Supplier}.
*
* @param responseSupplier the {@link Supplier} invokes returning the provided {@link HttpResponse}
* @param executor the {@link Executor} that executes the {@link Supplier}.
*
* @deprecated Use {@link #of(Supplier, Executor)}.
*/
@Deprecated
static HttpResponse from(Supplier<? extends HttpResponse> responseSupplier, Executor executor) {
requireNonNull(responseSupplier, "responseSupplier");
requireNonNull(executor, "executor");
final DeferredHttpResponse res = new DeferredHttpResponse();
executor.execute(() -> {
try {
res.delegate(responseSupplier.get());
} catch (Throwable ex) {
res.abort(ex);
}
});
return res;
return of(responseSupplier, executor);
}

/**
Expand Down Expand Up @@ -503,6 +488,65 @@ static HttpResponse of(ResponseHeaders headers, Publisher<? extends HttpObject>
return PublisherBasedHttpResponse.from(headers, publisher);
}

/**
* Creates a new HTTP response that delegates to the {@link HttpResponse} produced by the specified
* {@link CompletionStage}. If the specified {@link CompletionStage} fails, the returned response will be
* closed with the same cause as well.
*
* @param stage the {@link CompletionStage} which will produce the actual {@link HttpResponse}
*/
static HttpResponse of(CompletionStage<? extends HttpResponse> stage) {
requireNonNull(stage, "stage");

if (stage instanceof CompletableFuture) {
return createHttpResponseFrom((CompletableFuture<? extends HttpResponse>) stage);
} else {
final DeferredHttpResponse res = new DeferredHttpResponse();
res.delegateWhenComplete(stage);
return res;
}
}

/**
* Creates a new HTTP response that delegates to the {@link HttpResponse} produced by the specified
* {@link CompletionStage}. If the specified {@link CompletionStage} fails, the returned response will be
* closed with the same cause as well.
*
* @param stage the {@link CompletionStage} which will produce the actual {@link HttpResponse}
* @param subscriberExecutor the {@link EventExecutor} which will be used when a user subscribes
* the returned {@link HttpResponse} using {@link #subscribe(Subscriber)}
* or {@link #subscribe(Subscriber, SubscriptionOption...)}.
*/
static HttpResponse of(CompletionStage<? extends HttpResponse> stage,
EventExecutor subscriberExecutor) {
requireNonNull(stage, "stage");
requireNonNull(subscriberExecutor, "subscriberExecutor");
// Have to use DeferredHttpResponse to use the subscriberExecutor.
final DeferredHttpResponse res = new DeferredHttpResponse(subscriberExecutor);
res.delegateWhenComplete(stage);
return res;
}

/**
* Creates a new HTTP response that delegates to the {@link HttpResponse} provided by the {@link Supplier}.
*
* @param responseSupplier the {@link Supplier} invokes returning the provided {@link HttpResponse}
* @param executor the {@link Executor} that executes the {@link Supplier}.
*/
static HttpResponse of(Supplier<? extends HttpResponse> responseSupplier, Executor executor) {
requireNonNull(responseSupplier, "responseSupplier");
requireNonNull(executor, "executor");
final DeferredHttpResponse res = new DeferredHttpResponse();
executor.execute(() -> {
try {
res.delegate(responseSupplier.get());
} catch (Throwable ex) {
res.abort(ex);
}
});
return res;
}

/**
* Creates a new HTTP response with the specified {@code content} that is converted into JSON using the
* default {@link ObjectMapper}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public HttpResponse convertResponse(ServiceRequestContext ctx, ResponseHeaders h
return ResponseConverterFunction.fallthrough();
}

return HttpResponse.from(f.thenApply(aggregated -> {
return HttpResponse.of(f.thenApply(aggregated -> {
try {
return responseConverter.convertResponse(ctx, headers, aggregated, trailers);
} catch (Exception ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ private HttpResponse serve0(ServiceRequestContext ctx, HttpRequest req) {
}
}

return HttpResponse.from(serve1(ctx, req, aggregationType));
return HttpResponse.of(serve1(ctx, req, aggregationType));
}

/**
Expand Down Expand Up @@ -415,7 +415,7 @@ private HttpResponse convertResponseInternal(ServiceRequestContext ctx,
HttpHeaders trailers) {
if (result instanceof CompletionStage) {
final CompletionStage<?> future = (CompletionStage<?>) result;
return HttpResponse.from(
return HttpResponse.of(
future.thenApply(object -> convertResponseInternal(ctx, headers, object, trailers)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ public static AuthServiceBuilder builder() {

@Override
public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exception {
return HttpResponse.from(AuthorizerUtil.authorizeAndSupplyHandlers(authorizer, ctx, req)
.handleAsync((result, cause) -> {
return HttpResponse.of(AuthorizerUtil.authorizeAndSupplyHandlers(authorizer, ctx, req)
.handleAsync((result, cause) -> {
try {
final HttpService delegate = (HttpService) unwrap();
if (cause == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ public HttpService asService() {
return HttpResponse.of(HttpStatus.METHOD_NOT_ALLOWED);
}

return HttpResponse.from(readAttributes(ctx.blockingTaskExecutor()).thenApply(attrs -> {
return HttpResponse.of(readAttributes(ctx.blockingTaskExecutor()).thenApply(attrs -> {
if (attrs == null) {
return HttpResponse.of(HttpStatus.NOT_FOUND);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public HttpService asService() {
return delegate.asService();
}

return (ctx, req) -> HttpResponse.from(stage.thenApply(file -> {
return (ctx, req) -> HttpResponse.of(stage.thenApply(file -> {
setDelegate(file);
try {
return file.asService().serve(ctx, req);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ public void serviceAdded(ServiceConfig cfg) throws Exception {

@Override
public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) {
return HttpResponse.from(
return HttpResponse.of(
first.findFile(ctx, req)
.readAttributes(ctx.blockingTaskExecutor())
.thenApply(firstAttrs -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exc
return HttpResponse.of(HttpStatus.METHOD_NOT_ALLOWED);
}

return HttpResponse.from(updateHandler.handle(ctx, req).thenApply(updateResult -> {
return HttpResponse.of(updateHandler.handle(ctx, req).thenApply(updateResult -> {
if (updateResult != null) {
switch (updateResult) {
case HEALTHY:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exc
}
});

return HttpResponse.from(responseFuture);
return HttpResponse.of(responseFuture);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,6 @@ public static ThrottlingServiceBuilder builder(ThrottlingStrategy<HttpRequest> s
ThrottlingService(HttpService delegate, ThrottlingStrategy<HttpRequest> strategy,
ThrottlingAcceptHandler<HttpRequest, HttpResponse> acceptHandler,
ThrottlingRejectHandler<HttpRequest, HttpResponse> rejectHandler) {
super(delegate, strategy, HttpResponse::from, acceptHandler, rejectHandler);
super(delegate, strategy, HttpResponse::of, acceptHandler, rejectHandler);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ private HttpResponse doGetOrPost(HttpRequest req) {
accept);
}

return HttpResponse.from(req.aggregate().handle((aReq, cause) -> {
return HttpResponse.of(req.aggregate().handle((aReq, cause) -> {
if (cause != null) {
return HttpResponse.of(
HttpStatus.INTERNAL_SERVER_ERROR,
Expand Down
Loading

0 comments on commit 4edff55

Please sign in to comment.