Provide a way to automatically deserialize non-OK JSON response using WebClient and RestClient

[ikhoon]

WebClient or RestClient is able to deserialize a JSON response into an object using:

WebClient client = WebClient.of(...);
ComletableFuture<ResponseEntity<MyItems>> response = 
  client.prepare()
        .get("/api/items/...")
        .asJson(MyItems.class)
        .execute();

The client tries to convert the JSON response into an object when the class of response status is a success.

armeria/core/src/main/java/com/linecorp/armeria/client/AggregatedResponseAs.java

Lines 58 to 62 in ccf630c

	private static <T> ResponseEntity<T> newJsonResponseEntity(AggregatedHttpResponse response,
	JsonDecoder<T> decoder) {
	if (!response.status().isSuccess()) {
	throw newInvalidHttpResponseException(response);
	}

I believe that most REST clients expect a 2xx status for normal situations.
However, when it comes to tests, it would be inconvenient. Users might want to test an error response.

So it could be useful to add some variant methods of .asJson(...) of WebClient and .execute(...) of RestClient

ComletableFuture<ResponseEntity<MyError>> response = 
  client.prepare()
        .get("/api/items/...?bad=parameter")
        .asJson(MyError.class, HttpStatus.INTERNAL_SERVER_ERROR)
        .execute();

In addition to HttpStatus, we can also add Predicate<HttpStatus> and HttpStatusClass as a parameter.

[ahnyujin]

Hi @ikhoon, I understand the intent of this issue is to deserialize error response for the tests.
But I wonder why status is given as parameter for .asJson(MyError.class, HttpStatus.INTERNAL_SERVER_ERROR).
Is it to throw an error in the same way as before if the given status does not match?

[ikhoon]

But I wonder why status is given as parameter for .asJson(MyError.class, HttpStatus.INTERNAL_SERVER_ERROR).

The response format between a normal response and an error response may be different in most cases.
The deserialized type of response is decided when a client sends a request.
HttpStatus.isSuccess() is used whether the expected response data format is received.
If a specific HttpStatus or predicate is given, we may override the default predicate.

Is it to throw an error in the same way as before if the given status does not match?

We need to update the error message below. If a Predicate is set, we can skip the (expect: ... ) part.

armeria/core/src/main/java/com/linecorp/armeria/client/AggregatedResponseAs.java

Line 76 in ccf630c

" (expect: the success class (2xx). response: " + response, null);

[ikhoon]

For gentle error messages, we can internally create HttpStatusPredicate.

class HttpStatusPredicate<T> extends Predicate<T> {

    // Use the following fields to generate error messages
    @Nullable
    private HttpStatus status;
    @Nullable
    private HttpStatusClass statusClass;
    ...
}

// WebClientRequestPreparation
public <T> ... asJson(Class<? extends T> clazz, HttpStatus status) {
    return asJson(clazz, new HttpStatusPredicate(status));
}

public <T> ... asJson(Class<? extends T> clazz, Predicate<HttpStatus> predicate) {
    ...
}

[ahnyujin]

Thanks a lot🙇 The part I was wondering about has been solved.
I'll try it!

[my4-dev]

Hi @ikhoon! I'm trying to this issue.
But, I can't understand how HttpStatusPredicate which you show upper would be used.
Please let me know at detail.

[ikhoon]

We can implement HttpStatusPredicate like the following.

final class HttpStatusPredicate extends Predicate<HttpStatus> {

    @Nullable
    private HttpStatus status;
    @Nullable
    private HttpStatusClass statusClass;
     
    ...    

    @Override
    boolean test(HttpStatus status) {
        if (this.status != null) {
            return this.status.equals(status);
        }
        assert statusClass != null;
        return status.codeClass() == statusClass;
    }

    @Nullable
    HttpStatus status() {
        return status;
    }

    @Nullable
    HttpStatusClass statusClass() {
        return statusClass;
    }
}

The custom HttpStatusPredicate will carry the desired status information,
and we can refactor AggregatedResponseAs to use HttpStatusPredicate for a simple condition.

// In AggregatedResponseAs
private static final HttpStatusPredicate SUCCESS_PREDICATE = new HttpStatusPredicate(HttpStatusClass.SUCCESS);

private static <T> ResponseEntity<T> 
    newJsonResponseEntity(AggregatedHttpResponse response, 
                          JsonDecoder<T> decoder) {
     return newJsonResponseEntity(response, decoder, SUCCESS_PREDICATE);
}

private static <T> ResponseEntity<T> 
    newJsonResponseEntity(AggregatedHttpResponse response, 
                          JsonDecoder<T> decoder, HttpStatus status) {
     return newJsonResponseEntity(response, decoder, new HttpStatusPredicate(status));
}

private static <T> ResponseEntity<T> 
    newJsonResponseEntity(AggregatedHttpResponse response, 
                          JsonDecoder<T> decoder, Predicate<HttpStatus> status) {
    if (status instanceof HttpStatusPredicate) {
       ... // Extract either HttpStatus or HttpStatusClass to provide the expected status.
    }
}

[my4-dev]

Previous pull request #4411 has a little problem, so I submit pull request again as #4412 .
Please review pull request #4412 🙏

[ikhoon]

Thanks for the pull request. I will take a look at it soon.