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.

Sign up for GitHub

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

Rest proxy remove block from Schedulers #5376

Merged
merged 9 commits into from
Sep 16, 2019
Merged

Rest proxy remove block from Schedulers #5376

Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a0e7584
Rest proxy remove block from Schedulers
sima-zhu Sep 15, 2019
911a83d
Checkstyle fix
sima-zhu Sep 15, 2019
41df2f3
Fixed the case when deserialized header is null
sima-zhu Sep 16, 2019
8115e12
Use the reactor way of handling null case
sima-zhu Sep 16, 2019
025c9de
Use the reactor way of handling null case
sima-zhu Sep 16, 2019
8fb56a6
Revert some changes back to origin
sima-zhu Sep 16, 2019
d0f1649
Clean up some try/catch block
sima-zhu Sep 16, 2019
495a831
Convert helper to method
sima-zhu Sep 16, 2019
89b0885
Fix linting
sima-zhu Sep 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 39 additions & 10 deletions sdk/core/azure-core/src/main/java/com/azure/core/implementation/RestProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,13 +459,12 @@ private Mono<?> handleRestResponseReturnType(HttpDecodedResponse response, Swagg

if (TypeUtil.isTypeOrSubTypeOf(bodyType, Void.class)) {
asyncResult = response.getSourceResponse().getBody().ignoreElements()
.then(Mono.just(createResponse(response, entityType, null)));
.then(createResponse(response, entityType, null));
} else {
asyncResult = handleBodyReturnType(response, methodParser, bodyType)
.map((Function<Object, Response<?>>) bodyAsObject -> createResponse(response, entityType,
.flatMap((Function<Object, Mono<?>>) bodyAsObject -> createResponse(response, entityType,
bodyAsObject))
.switchIfEmpty(Mono.defer((Supplier<Mono<Response<?>>>) () -> Mono.just(createResponse(response,
entityType, null))));
.switchIfEmpty(createResponse(response, entityType, null));
}
} else {
// For now we're just throwing if the Maybe didn't emit a value.
Expand All @@ -475,8 +474,14 @@ private Mono<?> handleRestResponseReturnType(HttpDecodedResponse response, Swagg
return asyncResult;
}

private static final class NullObject {
private NullObject() {
}
public static final NullObject INSTANCE = new NullObject();
}

@SuppressWarnings("unchecked")
private Response<?> createResponse(HttpDecodedResponse response, Type entityType, Object bodyAsObject) {
private Mono<Response<?>> createResponse(HttpDecodedResponse response, Type entityType, Object bodyAsObject) {
final HttpResponse httpResponse = response.getSourceResponse();
final HttpRequest httpRequest = httpResponse.getRequest();
final int responseStatusCode = httpResponse.getStatusCode();
Expand Down Expand Up @@ -511,7 +516,6 @@ private Response<?> createResponse(HttpDecodedResponse response, Type entityType
if (constructors.isEmpty()) {
throw logger.logExceptionAsError(new RuntimeException("Cannot find suitable constructor for class " + cls));
}

// try to create an instance using our list of potential candidates
for (Constructor<?> constructor : constructors) {
final Constructor<? extends Response<?>> ctor = (Constructor<? extends Response<?>>) constructor;
Expand All @@ -521,12 +525,37 @@ private Response<?> createResponse(HttpDecodedResponse response, Type entityType

switch (paramCount) {
case 3:
return ctor.newInstance(httpRequest, responseStatusCode, responseHeaders);
return Mono.just(ctor.newInstance(httpRequest, responseStatusCode, responseHeaders));
case 4:
return ctor.newInstance(httpRequest, responseStatusCode, responseHeaders, bodyAsObject);
return Mono.just(ctor.newInstance(httpRequest, responseStatusCode, responseHeaders,
bodyAsObject));
case 5:
return ctor.newInstance(httpRequest, responseStatusCode, responseHeaders, bodyAsObject,
response.getDecodedHeaders().block());
return response.getDecodedHeaders()
.defaultIfEmpty(NullObject.INSTANCE)
.map(headers -> {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be switched to a flatMap and follow a similar pattern as handleRestResponseReturnType which you changed in the PR as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

map works well in this case and make code less overhead.
I would prefer to keep using map here.

try {
if (headers instanceof NullObject) {
return ctor.newInstance(httpRequest, responseStatusCode,
responseHeaders, bodyAsObject, null);
}
return ctor.newInstance(httpRequest, responseStatusCode,
responseHeaders, bodyAsObject, headers);
} catch (IllegalAccessException | InvocationTargetException
| InstantiationException e) {
throw logger.logExceptionAsError(Exceptions.propagate(e));
}
});
// Function<Object, Response<?>> headersToResponse = (deserializedHeaders) -> {
// try {
// return ctor.newInstance(httpRequest, responseStatusCode,
// responseHeaders, bodyAsObject, deserializedHeaders);
// }catch (IllegalAccessException | InvocationTargetException
// | InstantiationException e) {
// throw logger.logExceptionAsError(Exceptions.propagate(e));
// } };
// return response.getDecodedHeaders()
// .map(headersToResponse)
// .defaultIfEmpty(headersToResponse.apply(null));
default:
throw logger.logExceptionAsError(new IllegalStateException(
"Response constructor with expected parameters not found."));
Expand Down