Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a way to automatically deserialize non-OK JSON response #5002

Merged
merged 30 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
476442a
add the poc code of @jrhee17
my4-dev Jun 10, 2023
30bc8aa
Provide a way to automatically deserialize non-OK JSON response
my4-dev Jul 1, 2023
f6c8c80
Merge branch 'main' into issue-4382-new
my4-dev Jul 2, 2023
3182531
move andThenJson methods from BlockingResponseAs to ResponseAs in ord…
my4-dev Jul 5, 2023
a228219
fix to throw InvalidHttpResponseException in case predicate.test is f…
my4-dev Jul 6, 2023
89bd77a
fix to throw InvalidHttpResponseException in case predicate.test is f…
my4-dev Jul 6, 2023
2b9c62c
Merge branch 'main' into issue-4382-new
my4-dev Jul 6, 2023
a800fe9
remove BlockingResponseAs and revert
my4-dev Jul 8, 2023
284d469
apply comments
my4-dev Jul 8, 2023
e2ac43c
delete HttpStatus[Class]Predicate
my4-dev Jul 8, 2023
256acf3
rename andThenJson to json in ResponseAs
my4-dev Jul 10, 2023
8f428cc
revert and format
my4-dev Jul 20, 2023
532526a
apply comments
my4-dev Jul 20, 2023
f6e8c4a
add JavaDoc and header
my4-dev Jul 21, 2023
2b36e1e
Merge branch 'main' into issue-4382-new
my4-dev Jul 22, 2023
7617569
Merge branch 'main' into issue-4382-new
jrhee17 Aug 14, 2023
c6617b8
fix failing test
jrhee17 Aug 14, 2023
9abae16
minor documentation updates
jrhee17 Aug 14, 2023
c2c0f48
lint
my4-dev Aug 16, 2023
89b15a2
Merge branch 'main' into issue-4382-new
my4-dev Oct 14, 2023
8be91f8
apply minor comments
my4-dev Oct 14, 2023
37b809d
rename json methods which return BlockingConditionalResponseAs to blo…
my4-dev Oct 14, 2023
889f5c7
re-defined DefaultConditionalResponseAs as the public class
my4-dev Oct 14, 2023
ff99ed6
add method variants of json
my4-dev Oct 14, 2023
a434b8c
lint
my4-dev Oct 14, 2023
c944bd3
Use orElseJson
Oct 16, 2023
10e7b45
Fix docs
minwoox Oct 17, 2023
a6660c8
More fix
minwoox Oct 17, 2023
6f1cf3a
Merge branch 'main' into issue-4382-new
jrhee17 Apr 4, 2024
146ba11
Merge branch 'main' into issue-4382-new
jrhee17 Apr 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.linecorp.armeria.client;

import java.io.IOException;
import java.util.function.Predicate;

import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -37,27 +38,34 @@ static ResponseAs<AggregatedHttpResponse, ResponseEntity<String>> string() {
return response -> ResponseEntity.of(response.headers(), response.contentUtf8(), response.trailers());
}

static <T> ResponseAs<AggregatedHttpResponse, ResponseEntity<T>> json(Class<? extends T> clazz) {
return response -> newJsonResponseEntity(response, bytes -> JacksonUtil.readValue(bytes, clazz));
static <T> ResponseAs<AggregatedHttpResponse, ResponseEntity<T>> json(
Class<? extends T> clazz, Predicate<AggregatedHttpResponse> predicate) {
return response -> newJsonResponseEntity(
response, bytes -> JacksonUtil.readValue(bytes, clazz), predicate);
}

static <T> ResponseAs<AggregatedHttpResponse, ResponseEntity<T>> json(Class<? extends T> clazz,
ObjectMapper mapper) {
return response -> newJsonResponseEntity(response, bytes -> mapper.readValue(bytes, clazz));
static <T> ResponseAs<AggregatedHttpResponse, ResponseEntity<T>> json(
Class<? extends T> clazz, ObjectMapper mapper, Predicate<AggregatedHttpResponse> predicate) {
return response -> newJsonResponseEntity(response, bytes -> mapper.readValue(bytes, clazz), predicate);
}

static <T> ResponseAs<AggregatedHttpResponse, ResponseEntity<T>> json(TypeReference<? extends T> typeRef) {
return response -> newJsonResponseEntity(response, bytes -> JacksonUtil.readValue(bytes, typeRef));
static <T> ResponseAs<AggregatedHttpResponse, ResponseEntity<T>> json(
TypeReference<? extends T> typeRef, Predicate<AggregatedHttpResponse> predicate) {
return response -> newJsonResponseEntity(
response, bytes -> JacksonUtil.readValue(bytes, typeRef), predicate);
}

static <T> ResponseAs<AggregatedHttpResponse, ResponseEntity<T>> json(TypeReference<? extends T> typeRef,
ObjectMapper mapper) {
return response -> newJsonResponseEntity(response, bytes -> mapper.readValue(bytes, typeRef));
static <T> ResponseAs<AggregatedHttpResponse, ResponseEntity<T>> json(
TypeReference<? extends T> typeRef, ObjectMapper mapper,
Predicate<AggregatedHttpResponse> predicate) {
return response -> newJsonResponseEntity(
response, bytes -> mapper.readValue(bytes, typeRef), predicate);
}

private static <T> ResponseEntity<T> newJsonResponseEntity(AggregatedHttpResponse response,
JsonDecoder<T> decoder) {
if (!response.status().isSuccess()) {
jrhee17 marked this conversation as resolved.
Show resolved Hide resolved
JsonDecoder<T> decoder,
Predicate<AggregatedHttpResponse> predicate) {
if (!predicate.test(response)) {
throw newInvalidHttpResponseException(response);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/*
* Copyright 2023 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package com.linecorp.armeria.client;

import java.util.function.Predicate;

import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;

import com.linecorp.armeria.common.AggregatedHttpResponse;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.ResponseEntity;
import com.linecorp.armeria.common.annotation.UnstableApi;

jrhee17 marked this conversation as resolved.
Show resolved Hide resolved
/**
* Provides a way for users to add {@link ResponseAs} mappings to transform an aggregated response
* given that the corresponding {@link Predicate} is satisfied. Note that the conditionals are
* invoked in the order in which they are added.
*
* <pre>{@code
* RestClient.of(...)
* .get("/")
* .execute(
* ResponseAs.<MyResponse>json(MyMessage.class, res -> res.status().isError())
* .andThenJson(EmptyMessage.class, res -> res.status().isInformational())
* .orElseJson(MyError.class);
* }
*/
@UnstableApi
public final class BlockingConditionalResponseAs<V>
extends DefaultConditionalResponseAs<HttpResponse, AggregatedHttpResponse, ResponseEntity<V>> {
private static final Predicate<AggregatedHttpResponse> TRUE_PREDICATE = unused -> true;

BlockingConditionalResponseAs(ResponseAs<HttpResponse, AggregatedHttpResponse> originalResponseAs,
ResponseAs<AggregatedHttpResponse, ResponseEntity<V>> responseAs,
Predicate<AggregatedHttpResponse> predicate) {
super(originalResponseAs, responseAs, predicate);
}

/**
* Adds a mapping such that the response content will be deserialized
* to the specified {@link Class} if the {@link Predicate} is satisfied.
*/
public BlockingConditionalResponseAs<V> andThenJson(
Copy link
Member

@minwoox minwoox Oct 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should rename this to orElseJson. andThen is used when both instances are used. However, if predicate returns false, then it isn't converted.
Let me send a patch to rename these methods and address comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.
Please send me your suggestion for these method's name.

Copy link
Member

@minwoox minwoox Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @my4-dev! I made a few changes:

  • Do not block the inside of the converter
    • I realized that this chaining converts also can be used by the WebClient. We must not block the inside of the converters.
  • Rename andThenJson to orElseJson
  • Remove ConditionalResponseAs interface because we don't need it at the moment.

PTAL and let me know if there are changes that I need to revert. 😉

Class<? extends V> clazz, Predicate<AggregatedHttpResponse> predicate) {
return andThen(AggregatedResponseAs.json(clazz, predicate), predicate);
}

/**
* Adds a mapping such that the response content will be deserialized
* to the specified {@link Class} using the {@link ObjectMapper} if the {@link Predicate} is satisfied.
*/
public BlockingConditionalResponseAs<V> andThenJson(
Class<? extends V> clazz, ObjectMapper objectMapper, Predicate<AggregatedHttpResponse> predicate) {
return andThen(AggregatedResponseAs.json(clazz, objectMapper, predicate), predicate);
}

/**
* Adds a mapping such that the response content will be deserialized
* with the specified {@link TypeReference} if the {@link Predicate} is satisfied.
*/
public BlockingConditionalResponseAs<V> andThenJson(
TypeReference<? extends V> typeRef, Predicate<AggregatedHttpResponse> predicate) {
return andThen(AggregatedResponseAs.json(typeRef, predicate), predicate);
}

/**
* Adds a mapping such that the response content will be deserialized
* with the specified {@link TypeReference} using the {@link ObjectMapper}
* if the {@link Predicate} is satisfied.
*/
public BlockingConditionalResponseAs<V> andThenJson(
TypeReference<? extends V> typeRef, ObjectMapper objectMapper,
Predicate<AggregatedHttpResponse> predicate) {
return andThen(AggregatedResponseAs.json(typeRef, objectMapper, predicate), predicate);
}

/**
* Returns {@link ResponseAs} based on the configured {@link ResponseAs} to {@link Predicate}
* mappings. If none of the {@link Predicate}s are satisfied, the content will be deserialized
* to the specified {@link Class}.
*/
public ResponseAs<HttpResponse, ResponseEntity<V>> orElseJson(Class<? extends V> clazz) {
return orElse(AggregatedResponseAs.json(clazz, TRUE_PREDICATE));
}

/**
* Returns {@link ResponseAs} based on the configured {@link ResponseAs} to {@link Predicate}
* mappings. If none of the {@link Predicate}s are satisfied, the content will be deserialized
* to the specified {@link Class} using the {@link ObjectMapper}.
*/
public ResponseAs<HttpResponse, ResponseEntity<V>> orElseJson(
Class<? extends V> clazz, ObjectMapper objectMapper) {
return orElse(AggregatedResponseAs.json(clazz, objectMapper, TRUE_PREDICATE));
}

/**
* Returns {@link ResponseAs} based on the configured {@link ResponseAs} to {@link Predicate}
* mappings. If none of the {@link Predicate}s are satisfied, the content will be deserialized
* using the specified {@link TypeReference}.
*/
public ResponseAs<HttpResponse, ResponseEntity<V>> orElseJson(TypeReference<? extends V> typeRef) {
return orElse(AggregatedResponseAs.json(typeRef, TRUE_PREDICATE));
}

/**
* Returns {@link ResponseAs} based on the configured {@link ResponseAs} to {@link Predicate}
* mappings. If none of the {@link Predicate}s are satisfied, the content will be deserialized
* using the specified {@link TypeReference} and {@link ObjectMapper}.
*/
public ResponseAs<HttpResponse, ResponseEntity<V>> orElseJson(
TypeReference<? extends V> typeRef, ObjectMapper objectMapper) {
return orElse(AggregatedResponseAs.json(typeRef, objectMapper, TRUE_PREDICATE));
}

@Override
public BlockingConditionalResponseAs<V> andThen(
ResponseAs<AggregatedHttpResponse, ResponseEntity<V>> responseAs,
Predicate<AggregatedHttpResponse> predicate) {
return (BlockingConditionalResponseAs<V>) super.andThen(responseAs, predicate);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.function.Predicate;

import org.reactivestreams.Publisher;

Expand Down Expand Up @@ -54,6 +55,7 @@ public final class BlockingWebClientRequestPreparation
implements WebRequestPreparationSetters<AggregatedHttpResponse> {

private final WebClientRequestPreparation delegate;
private static final Predicate<AggregatedHttpResponse> SUCCESS_PREDICATE = res -> res.status().isSuccess();

BlockingWebClientRequestPreparation(WebClientRequestPreparation delegate) {
this.delegate = delegate;
Expand Down Expand Up @@ -134,7 +136,7 @@ public TransformingRequestPreparation<AggregatedHttpResponse, ResponseEntity<Str
public <T> TransformingRequestPreparation<AggregatedHttpResponse, ResponseEntity<T>> asJson(
Class<? extends T> clazz) {
requireNonNull(clazz, "clazz");
return as(AggregatedResponseAs.json(clazz));
return as(AggregatedResponseAs.json(clazz, SUCCESS_PREDICATE));
}

/**
Expand Down Expand Up @@ -162,7 +164,7 @@ public <T> TransformingRequestPreparation<AggregatedHttpResponse, ResponseEntity
Class<? extends T> clazz, ObjectMapper mapper) {
requireNonNull(clazz, "clazz");
requireNonNull(mapper, "mapper");
return as(AggregatedResponseAs.json(clazz, mapper));
return as(AggregatedResponseAs.json(clazz, mapper, SUCCESS_PREDICATE));
}

/**
Expand All @@ -188,7 +190,7 @@ public <T> TransformingRequestPreparation<AggregatedHttpResponse, ResponseEntity
public <T> TransformingRequestPreparation<AggregatedHttpResponse, ResponseEntity<T>> asJson(
TypeReference<? extends T> typeRef) {
requireNonNull(typeRef, "typeRef");
return as(AggregatedResponseAs.json(typeRef));
return as(AggregatedResponseAs.json(typeRef, SUCCESS_PREDICATE));
}

/**
Expand All @@ -214,7 +216,7 @@ public <T> TransformingRequestPreparation<AggregatedHttpResponse, ResponseEntity
TypeReference<? extends T> typeRef, ObjectMapper mapper) {
requireNonNull(typeRef, "typeRef");
requireNonNull(mapper, "mapper");
return as(AggregatedResponseAs.json(typeRef, mapper));
return as(AggregatedResponseAs.json(typeRef, mapper, SUCCESS_PREDICATE));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright 2023 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package com.linecorp.armeria.client;

import java.util.function.Predicate;

/**
* Provides a way for users to add {@link ResponseAs} mappings to transform a response
* given that the corresponding {@link Predicate} is satisfied. Note that the conditionals are
* invoked in the order in which they are added.
*
* <pre>{@code
* WebClient.of(...)
* .prepare()
* .get("/server_error")
* .as(ResponseAs.blocking()
* .<MyResponse>andThen(
* res -> new MyError(res.status().codeAsText(), res.contentUtf8()),
* res -> res.status().isError())
* .andThen(
* res -> new EmptyMessage(),
* res -> !res.headers().contains("x-header"))
* .orElse(res -> new MyMessage(res.contentUtf8())))
* }
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
*/
@UnstableApi

public interface ConditionalResponseAs<T, R, V> {

/**
* Adds a mapping such that {@link ResponseAs} will be applied if the {@link Predicate} is satisfied.
*/
ConditionalResponseAs<T, R, V> andThen(ResponseAs<R, V> responseAs, Predicate<R> predicate);

/**
* Returns {@link ResponseAs} based on the configured {@link ResponseAs} to {@link Predicate}
* mappings. If none of the {@link Predicate}s are satisfied, the specified {@link ResponseAs}
* is used as a fallback.
*/
ResponseAs<T, V> orElse(ResponseAs<R, V> responseAs);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright 2023 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package com.linecorp.armeria.client;

import java.util.AbstractMap.SimpleEntry;
import java.util.ArrayList;
import java.util.List;
import java.util.Map.Entry;
import java.util.function.Predicate;

import com.linecorp.armeria.common.annotation.UnstableApi;

/**
* Adds the mapping of {@link ResponseAs} and {@link Predicate} in order to return {@link ResponseAs} whose
* {@link Predicate} is evaluated as true.
*/
@UnstableApi
class DefaultConditionalResponseAs<T, R, V> implements ConditionalResponseAs<T, R, V> {

private final ResponseAs<T, R> originalResponseAs;
private final List<Entry<ResponseAs<R, V>, Predicate<R>>> predicateMappingList = new ArrayList<>();

DefaultConditionalResponseAs(
ResponseAs<T, R> originalResponseAs, ResponseAs<R, V> responseAs, Predicate<R> predicate) {
this.originalResponseAs = originalResponseAs;
andThen(responseAs, predicate);
}

@Override
public DefaultConditionalResponseAs<T, R, V> andThen(ResponseAs<R, V> responseAs, Predicate<R> predicate) {
predicateMappingList.add(new SimpleEntry<>(responseAs, predicate));
return this;
}

@Override
public ResponseAs<T, V> orElse(ResponseAs<R, V> responseAs) {
return new ResponseAs<T, V>() {
@Override
public V as(T response) {
final R r = originalResponseAs.as(response);
for (Entry<ResponseAs<R, V>, Predicate<R>> entry: predicateMappingList) {
if (entry.getValue().test(r)) {
return entry.getKey().as(r);
}
}
return responseAs.as(r);
}
};
}
}