Skip to content

Commit

Permalink
Merge pull request #1954 from ballerina-platform/status-code-binding-fix
Browse files Browse the repository at this point in the history 
Fix hanging issue in the HTTP client after status code binding feature
  • Loading branch information
@TharmiganK
TharmiganK authored Apr 19, 2024
2 parents c5b7966 + d07fa66 commit 0522caf
Show file tree
Hide file tree
Showing 12 changed files with 937 additions and 357 deletions.
185 changes: 10 additions & 175 deletions ballerina-tests/http-client-tests/tests/sc_res_binding_tests.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,53 +240,17 @@ service /api on new http:Listener(statusCodeBindingPort2) {
}
}

final http:Client albumClient = check new (string `localhost:${statusCodeBindingPort2}/api`);
final http:StatusCodeClient albumClient = check new (string `localhost:${statusCodeBindingPort2}/api`);

@test:Config {}
function testGetSuccessStatusCodeResponse() returns error? {
Album album = check albumClient->/albums/'1;
Album expectedAlbum = albums.get("1");
test:assertEquals(album, expectedAlbum, "Invalid album returned");

AlbumFound albumFound = check albumClient->get("/albums/1");
Album expectedAlbum = albums.get("1");
test:assertEquals(albumFound.body, expectedAlbum, "Invalid album returned");
test:assertEquals(albumFound.headers.user\-id, "user-1", "Invalid user-id header");
test:assertEquals(albumFound.headers.req\-id, 1, "Invalid req-id header");
test:assertEquals(albumFound.mediaType, "application/json", "Invalid media type");

http:Response res = check albumClient->/albums/'1;
test:assertEquals(res.statusCode, 200, "Invalid status code");
json payload = check res.getJsonPayload();
album = check payload.fromJsonWithType();
test:assertEquals(album, expectedAlbum, "Invalid album returned");

Album|AlbumFound res1 = check albumClient->get("/albums/1");
if res1 is AlbumFound {
test:assertEquals(res1.body, expectedAlbum, "Invalid album returned");
test:assertEquals(res1.headers.user\-id, "user-1", "Invalid user-id header");
test:assertEquals(res1.headers.req\-id, 1, "Invalid req-id header");
test:assertEquals(res1.mediaType, "application/json", "Invalid media type");
} else {
test:assertFail("Invalid response type");
}

AlbumFound|Album res2 = check albumClient->/albums/'1;
if res2 is AlbumFound {
test:assertEquals(res2.body, expectedAlbum, "Invalid album returned");
test:assertEquals(res2.headers.user\-id, "user-1", "Invalid user-id header");
test:assertEquals(res2.headers.req\-id, 1, "Invalid req-id header");
test:assertEquals(res2.mediaType, "application/json", "Invalid media type");
} else {
test:assertFail("Invalid response type");
}

Album|AlbumNotFound res3 = check albumClient->get("/albums/1");
if res3 is Album {
test:assertEquals(res3, expectedAlbum, "Invalid album returned");
} else {
test:assertFail("Invalid response type");
}

AlbumFound|AlbumNotFound res4 = check albumClient->/albums/'1;
if res4 is AlbumFound {
test:assertEquals(res4.body, expectedAlbum, "Invalid album returned");
Expand All @@ -297,37 +261,10 @@ function testGetSuccessStatusCodeResponse() returns error? {
test:assertFail("Invalid response type");
}

Album|AlbumFound|AlbumNotFound res5 = check albumClient->get("/albums/1");
if res5 is AlbumFound {
test:assertEquals(res5.body, expectedAlbum, "Invalid album returned");
test:assertEquals(res5.headers.user\-id, "user-1", "Invalid user-id header");
test:assertEquals(res5.headers.req\-id, 1, "Invalid req-id header");
test:assertEquals(res5.mediaType, "application/json", "Invalid media type");
} else {
test:assertFail("Invalid response type");
}

Album|AlbumNotFound|http:Response res6 = check albumClient->/albums/'1;
if res6 is Album {
test:assertEquals(res6, expectedAlbum, "Invalid album returned");
} else {
test:assertFail("Invalid response type");
}

AlbumNotFound|http:Response res7 = check albumClient->get("/albums/1");
if res7 is http:Response {
test:assertEquals(res.statusCode, 200, "Invalid status code");
payload = check res.getJsonPayload();
album = check payload.fromJsonWithType();
test:assertEquals(album, expectedAlbum, "Invalid album returned");
} else {
test:assertFail("Invalid response type");
}

AlbumNotFound|error res8 = albumClient->/albums/'1;
if res8 is error {
test:assertTrue(res8 is http:PayloadBindingError);
test:assertEquals(res8.message(), "incompatible http_client_tests:AlbumNotFound found for response with 200",
test:assertTrue(res8 is http:StatusCodeResponseBindingError);
test:assertEquals(res8.message(), "incompatible AlbumNotFound found for response with 200",
"Invalid error message");
error? cause = res8.cause();
if cause is error {
Expand All @@ -347,129 +284,27 @@ function testGetFailureStatusCodeResponse() returns error? {
test:assertEquals(albumNotFound.headers.req\-id, 1, "Invalid req-id header");
test:assertEquals(albumNotFound.mediaType, "application/json", "Invalid media type");

http:Response res = check albumClient->get("/albums/4");
test:assertEquals(res.statusCode, 404, "Invalid status code");
json payload = check res.getJsonPayload();
ErrorMessage errorMessage = check payload.fromJsonWithType();
test:assertEquals(errorMessage, expectedErrorMessage, "Invalid error message");

Album|AlbumNotFound res1 = check albumClient->/albums/'4;
if res1 is AlbumNotFound {
test:assertEquals(res1.body, expectedErrorMessage, "Invalid error message");
test:assertEquals(res1.headers.user\-id, "user-1", "Invalid user-id header");
test:assertEquals(res1.headers.req\-id, 1, "Invalid req-id header");
test:assertEquals(res1.mediaType, "application/json", "Invalid media type");
} else {
test:assertFail("Invalid response type");
}

AlbumNotFound|http:Response res2 = check albumClient->get("/albums/4");
if res2 is AlbumNotFound {
test:assertEquals(res2.body, expectedErrorMessage, "Invalid error message");
test:assertEquals(res2.headers.user\-id, "user-1", "Invalid user-id header");
test:assertEquals(res2.headers.req\-id, 1, "Invalid req-id header");
test:assertEquals(res2.mediaType, "application/json", "Invalid media type");
} else {
test:assertFail("Invalid response type");
}

Album|http:Response res3 = check albumClient->/albums/'4;
if res3 is http:Response {
test:assertEquals(res3.statusCode, 404, "Invalid status code");
payload = check res3.getJsonPayload();
errorMessage = check payload.fromJsonWithType();
test:assertEquals(errorMessage, expectedErrorMessage, "Invalid error message");
} else {
test:assertFail("Invalid response type");
}

http:Response|AlbumFound res4 = check albumClient->get("/albums/4");
if res4 is http:Response {
test:assertEquals(res4.statusCode, 404, "Invalid status code");
payload = check res4.getJsonPayload();
errorMessage = check payload.fromJsonWithType();
test:assertEquals(errorMessage, expectedErrorMessage, "Invalid error message");
} else {
test:assertFail("Invalid response type");
}

Album|error res5 = albumClient->/albums/'4;
if res5 is error {
test:assertTrue(res5 is http:ClientRequestError);
test:assertEquals(res5.message(), "Not Found", "Invalid error message");
test:assertEquals(res5.detail()["statusCode"], 404, "Invalid status code");
test:assertEquals(res5.detail()["body"], expectedErrorMessage, "Invalid error message");
if res5.detail()["headers"] is map<string[]> {
map<string[]> headers = check res5.detail()["headers"].ensureType();
test:assertEquals(headers.get("user-id")[0], "user-1", "Invalid user-id header");
test:assertEquals(headers.get("req-id")[0], "1", "Invalid req-id header");
}

} else {
test:assertFail("Invalid response type");
}

AlbumFound|error res6 = albumClient->get("/albums/4");
if res6 is error {
test:assertTrue(res6 is http:ClientRequestError);
test:assertEquals(res6.message(), "Not Found", "Invalid error message");
test:assertTrue(res6 is http:StatusCodeResponseBindingError);
test:assertEquals(res6.message(), "incompatible AlbumFound found for response with 404", "Invalid error message");
test:assertEquals(res6.detail()["statusCode"], 404, "Invalid status code");
test:assertEquals(res6.detail()["body"], expectedErrorMessage, "Invalid error message");
if res6.detail()["headers"] is map<string[]> {
map<string[]> headers = check res6.detail()["headers"].ensureType();
test:assertEquals(headers.get("user-id")[0], "user-1", "Invalid user-id header");
test:assertEquals(headers.get("req-id")[0], "1", "Invalid req-id header");
} else {
test:assertFail("Invalid headers type");
}

} else {
test:assertFail("Invalid response type");
}
}

@test:Config {}
function testUnionPayloadBindingWithStatusCodeResponse() returns error? {
Album|AlbumNotFound|map<json>|json res1 = check albumClient->/albums/'1;
if res1 is Album {
test:assertEquals(res1, albums.get("1"), "Invalid album returned");
} else {
test:assertFail("Invalid response type");
}

map<json>|AlbumNotFound|Album|json res2 = check albumClient->get("/albums/1");
if res2 is map<json> {
test:assertEquals(res2, albums.get("1"), "Invalid album returned");
} else {
test:assertFail("Invalid response type");
}

Album|MockAlbum|AlbumNotFound res3 = check albumClient->/albums/'1;
if res3 is Album {
test:assertEquals(res3, albums.get("1"), "Invalid album returned");
} else {
test:assertFail("Invalid response type");
}

MockAlbum|Album|AlbumNotFound res4 = check albumClient->get("/albums/1");
if res4 is MockAlbum {
test:assertEquals(res4, {...albums.get("1"), "type": "mock"}, "Invalid album returned");
} else {
test:assertFail("Invalid response type");
}

AlbumUnion1|AlbumNotFound res5 = check albumClient->/albums/'1;
if res5 is Album {
test:assertEquals(res5, albums.get("1"), "Invalid album returned");
} else {
test:assertFail("Invalid response type");
}

AlbumUnion2|AlbumNotFound res6 = check albumClient->get("/albums/1");
if res6 is MockAlbum {
test:assertEquals(res6, {...albums.get("1"), "type": "mock"}, "Invalid album returned");
} else {
test:assertFail("Invalid response type");
}

AlbumFound|AlbumNotFound|AlbumFoundMock1 res7 = check albumClient->/albums/'1;
if res7 is AlbumFound {
test:assertEquals(res7.body, albums.get("1"), "Invalid album returned");
Expand Down Expand Up @@ -611,7 +446,7 @@ function testStatusCodeBindingWithConstraintsSuccess() returns error? {
test:assertEquals(res1.headers.req\-id, 1, "Invalid req-id header");
test:assertEquals(res1.mediaType, "application/json", "Invalid media type");

AlbumFoundWithConstraints|AlbumFound|Album|http:Response res2 = check albumClient->get("/albums/2");
AlbumFoundWithConstraints|AlbumFound res2 = check albumClient->get("/albums/2");
if res2 is AlbumFoundWithConstraints {
test:assertEquals(res2.body, albums.get("2"), "Invalid album returned");
test:assertEquals(res2.headers.user\-id, "user-1", "Invalid user-id header");
Expand Down Expand Up @@ -652,7 +487,7 @@ function testStatusCodeBindingWithConstraintsFailure() returns error? {
test:assertFail("Invalid response type");
}

AlbumFoundWithInvalidConstraints3|Album|error res3 = albumClient->/albums/'2;
AlbumFoundWithInvalidConstraints3|error res3 = albumClient->/albums/'2;
if res3 is error {
test:assertTrue(res3 is http:MediaTypeValidationError);
test:assertEquals(res3.message(), "media-type binding failed: Validation failed for " +
Expand Down
2 changes: 1 addition & 1 deletion ballerina-tests/http-client-tests/tests/sc_res_binding_with_all_status_codes_tests.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import ballerina/http;
import ballerina/test;

final http:Client statusCodeBindingClient1 = check new (string `localhost:${statusCodeBindingPort1}`);
final http:StatusCodeClient statusCodeBindingClient1 = check new (string `localhost:${statusCodeBindingPort1}`);

service /api on new http:Listener(statusCodeBindingPort1) {

Expand Down
71 changes: 61 additions & 10 deletions ballerina/http_client_endpoint.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public client isolated class Client {
} external;

private isolated function processPost(string path, RequestMessage message, TargetType targetType,
string? mediaType, map<string|string[]>? headers) returns Response|StatusCodeResponse|anydata|ClientError {
string? mediaType, map<string|string[]>? headers) returns Response|anydata|ClientError {
Request req = check buildRequest(message, mediaType);
populateOptions(req, mediaType, headers);
Response|ClientError response = self.httpClient->post(path, req);
Expand Down Expand Up @@ -130,7 +130,7 @@ public client isolated class Client {
} external;

private isolated function processPut(string path, RequestMessage message, TargetType targetType,
string? mediaType, map<string|string[]>? headers) returns Response|StatusCodeResponse|anydata|ClientError {
string? mediaType, map<string|string[]>? headers) returns Response|anydata|ClientError {
Request req = check buildRequest(message, mediaType);
populateOptions(req, mediaType, headers);
Response|ClientError response = self.httpClient->put(path, req);
Expand Down Expand Up @@ -172,7 +172,7 @@ public client isolated class Client {
} external;

private isolated function processPatch(string path, RequestMessage message, TargetType targetType,
string? mediaType, map<string|string[]>? headers) returns Response|StatusCodeResponse|anydata|ClientError {
string? mediaType, map<string|string[]>? headers) returns Response|anydata|ClientError {
Request req = check buildRequest(message, mediaType);
populateOptions(req, mediaType, headers);
Response|ClientError response = self.httpClient->patch(path, req);
Expand Down Expand Up @@ -214,7 +214,7 @@ public client isolated class Client {
} external;

private isolated function processDelete(string path, RequestMessage message, TargetType targetType,
string? mediaType, map<string|string[]>? headers) returns Response|StatusCodeResponse|anydata|ClientError {
string? mediaType, map<string|string[]>? headers) returns Response|anydata|ClientError {
Request req = check buildRequest(message, mediaType);
populateOptions(req, mediaType, headers);
Response|ClientError response = self.httpClient->delete(path, req);
Expand Down Expand Up @@ -277,7 +277,7 @@ public client isolated class Client {
} external;

private isolated function processGet(string path, map<string|string[]>? headers, TargetType targetType)
returns Response|StatusCodeResponse|anydata|ClientError {
returns Response|anydata|ClientError {
Request req = buildRequestWithHeaders(headers);
Response|ClientError response = self.httpClient->get(path, message = req);
if observabilityEnabled && response is Response {
Expand Down Expand Up @@ -313,7 +313,7 @@ public client isolated class Client {
} external;

private isolated function processOptions(string path, map<string|string[]>? headers, TargetType targetType)
returns Response|StatusCodeResponse|anydata|ClientError {
returns Response|anydata|ClientError {
Request req = buildRequestWithHeaders(headers);
Response|ClientError response = self.httpClient->options(path, message = req);
if observabilityEnabled && response is Response {
Expand All @@ -340,7 +340,7 @@ public client isolated class Client {

private isolated function processExecute(string httpVerb, string path, RequestMessage message,
TargetType targetType, string? mediaType, map<string|string[]>? headers)
returns Response|StatusCodeResponse|anydata|ClientError {
returns Response|anydata|ClientError {
Request req = check buildRequest(message, mediaType);
populateOptions(req, mediaType, headers);
Response|ClientError response = self.httpClient->execute(httpVerb, path, req);
Expand All @@ -363,7 +363,7 @@ public client isolated class Client {
} external;

private isolated function processForward(string path, Request request, TargetType targetType)
returns Response|StatusCodeResponse|anydata|ClientError {
returns Response|anydata|ClientError {
Response|ClientError response = self.httpClient->forward(path, request);
if observabilityEnabled && response is Response {
addObservabilityInformation(path, request.method, response.statusCode, self.url);
Expand Down Expand Up @@ -677,16 +677,67 @@ isolated function createResponseError(int statusCode, string reasonPhrase, map<s
}
}

isolated function createStatusCodeResponseBindingError(boolean generalError, int statusCode, string reasonPhrase,
map<string[]> headers, anydata body = ()) returns ClientError {
if generalError {
return error StatusCodeResponseBindingError(reasonPhrase, statusCode = statusCode, headers = headers, body = body);
}
if 100 <= statusCode && statusCode <= 399 {
return error StatusCodeBindingSuccessError(reasonPhrase, statusCode = statusCode, headers = headers, body = body);
} else if 400 <= statusCode && statusCode <= 499 {
return error StatusCodeBindingClientRequestError(reasonPhrase, statusCode = statusCode, headers = headers, body = body);
} else {
return error StatusCodeBindingRemoteServerError(reasonPhrase, statusCode = statusCode, headers = headers, body = body);
}
}

isolated function processResponse(Response|ClientError response, TargetType targetType, boolean requireValidation)
returns Response|anydata|StatusCodeResponse|ClientError {
returns Response|anydata|ClientError {
if targetType is typedesc<Response> || response is ClientError {
return response;
}
int statusCode = response.statusCode;
if 400 <= statusCode && statusCode <= 599 {
string reasonPhrase = response.reasonPhrase;
map<string[]> headers = getHeaders(response);
anydata|error payload = getPayload(response);
if payload is error {
if payload is NoContentError {
return createResponseError(statusCode, reasonPhrase, headers);
}
return error PayloadBindingClientError("http:ApplicationResponseError creation failed: " + statusCode.toString() +
" response payload extraction failed", payload);
} else {
return createResponseError(statusCode, reasonPhrase, headers, payload);
}
}
if targetType is typedesc<anydata> {
anydata payload = check performDataBinding(response, targetType);
if requireValidation {
return performDataValidation(payload, targetType);
}
return payload;
} else {
panic error GenericClientError("invalid payload target type");
}
}

isolated function processResponseNew(Response|ClientError response, typedesc<StatusCodeResponse> targetType, boolean requireValidation)
returns StatusCodeResponse|ClientError {
if response is ClientError {
return response;
}
return externProcessResponse(response, targetType, requireValidation);
return externProcessResponseNew(response, targetType, requireValidation);
}

isolated function externProcessResponse(Response response, TargetType targetType, boolean requireValidation)
returns Response|anydata|StatusCodeResponse|ClientError = @java:Method {
'class: "io.ballerina.stdlib.http.api.nativeimpl.ExternResponseProcessor",
name: "processResponse"
} external;

isolated function externProcessResponseNew(Response response, typedesc<StatusCodeResponse> targetType, boolean requireValidation)
returns StatusCodeResponse|ClientError = @java:Method {
'class: "io.ballerina.stdlib.http.api.nativeimpl.ExternResponseProcessor",
name: "processResponse"
} external;
Loading

0 comments on commit 0522caf

Please sign in to comment.