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

[java][native] fix empty response body #20334

Merged
merged 4 commits into from
Dec 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
[![Stable releases in Maven Central](https://img.shields.io/maven-metadata/v/https/repo1.maven.org/maven2/org/openapitools/openapi-generator/maven-metadata.xml.svg)](http://search.maven.org/#search%7Cgav%7C1%7Cg%3A%22org.openapitools%22%20AND%20a%3A%22openapi-generator%22)
[![Apache 2.0 License](https://img.shields.io/badge/License-Apache%202.0-orange)](./LICENSE)
[![Open Collective backers](https://img.shields.io/opencollective/backers/openapi_generator?color=orange&label=OpenCollective%20Backers)](https://opencollective.com/openapi_generator)
[![Join the Slack chat room](https://img.shields.io/badge/Slack-Join%20the%20chat%20room-orange)](https://join.slack.com/t/openapi-generator/shared_invite/zt-2uoef5v0g-XGwo8~2oJ3EoziDSO1CmdQ)
[![Join the Slack chat room](https://img.shields.io/badge/Slack-Join%20the%20chat%20room-orange)](https://join.slack.com/t/openapi-generator/shared_invite/zt-2wmkn4s8g-n19PJ99Y6Vei74WMUIehQA)
[![Follow OpenAPI Generator Twitter account to get the latest update](https://img.shields.io/twitter/follow/oas_generator.svg?style=social&label=Follow)](https://twitter.com/oas_generator)
[![Contribute with Gitpod](https://img.shields.io/badge/Contribute%20with-Gitpod-908a85?logo=gitpod)](https://gitpod.io/#https://github.com/OpenAPITools/openapi-generator)
[![Conan Center](https://shields.io/conan/v/openapi-generator)](https://conan.io/center/recipes/openapi-generator)
Expand Down
2 changes: 1 addition & 1 deletion docs/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ title: "FAQ: General"

## Do you have a chat room?

[![Join the Slack chat room](https://img.shields.io/badge/Slack-Join%20the%20chat%20room-orange)](https://join.slack.com/t/openapi-generator/shared_invite/zt-2uoef5v0g-XGwo8~2oJ3EoziDSO1CmdQ)
[![Join the Slack chat room](https://img.shields.io/badge/Slack-Join%20the%20chat%20room-orange)](https://join.slack.com/t/openapi-generator/shared_invite/zt-2wmkn4s8g-n19PJ99Y6Vei74WMUIehQA)

## What is the governance structure of the OpenAPI Generator project?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,22 +271,46 @@ public class {{classname}} {
}
{{/vendorExtensions.x-java-text-plain-string}}
{{^vendorExtensions.x-java-text-plain-string}}
return new ApiResponse<{{{returnType}}}{{^returnType}}Void{{/returnType}}>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
{{#returnType}}
localVarResponse.body() == null ? null : memberVarObjectMapper.readValue(localVarResponse.body(), new TypeReference<{{{returnType}}}>() {}) // closes the InputStream
{{/returnType}}
{{^returnType}}
null
{{/returnType}}
{{#returnType}}
{{! Fix for https://github.com/OpenAPITools/openapi-generator/issues/13968 }}
{{! This part had a bugfix for an empty response in the past, but this part of that PR was reverted because it was not doing anything. }}
{{! Keep this documentation here, because the problem is not obvious. }}
{{! `InputStream.available()` was used, but that only works for inputstreams that are already in memory, it will not give the right result if it is a remote stream. We only work with remote streams here. }}
{{! https://github.com/OpenAPITools/openapi-generator/pull/13993/commits/3e!37411d2acef0311c82e6d941a8e40b3bc0b6da }}
{{! The `available` method would work with a `PushbackInputStream`, because we could read 1 byte to check if it exists then push it back so Jackson can read it again. The issue with that is that it will also insert an ascii character for "head of input" and that will break Jackson as it does not handle special whitespace characters. }}
{{! A fix for that problem is to read it into a string and remove those characters, but if we need to read it before giving it to jackson to fix the string then just reading it into a string as is to do an emptiness check is the cleaner solution. }}
{{! We could also manipulate the inputstream to remove that bad character, but string manipulation is easier to read and this codepath is not asyncronus so we do not gain anything by reading the stream later. }}
{{! This fix does make it unsuitable for large amounts of data because `InputStream.readAllbytes` is not meant for it, but a syncronus client is already not the right tool for that.}}
if (localVarResponse.body() == null) {
return new ApiResponse<{{{returnType}}}>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
null
);
}

String responseBody = new String(localVarResponse.body().readAllBytes());
localVarResponse.body().close();

return new ApiResponse<{{{returnType}}}>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
responseBody.isBlank()? null: memberVarObjectMapper.readValue(responseBody, new TypeReference<{{{returnType}}}>() {})
);
{{/returnType}}
{{^returnType}}
return new ApiResponse<{{{returnType}}}>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
null
);
{{/returnType}}
{{/vendorExtensions.x-java-text-plain-string}}
} finally {
{{^returnType}}
// Drain the InputStream
while (localVarResponse.body().read() != -1) {
// Ignore
// Ignore
}
localVarResponse.body().close();
{{/returnType}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3263,4 +3263,73 @@ public void testGenerateParameterId() {
" getCall(Integer queryParameter, final ApiCallback _callback)"
);
}

@Test
public void callNativeServiceWithEmptyResponseSync() throws IOException {
Map<String, Object> properties = new HashMap<>();
properties.put(CodegenConstants.API_PACKAGE, "xyz.abcdef.api");
properties.put("asyncNative", "false");

File output = Files.createTempDirectory("test").toFile();
output.deleteOnExit();

final CodegenConfigurator configurator = new CodegenConfigurator()
.setGeneratorName("java")
.setLibrary(JavaClientCodegen.NATIVE)
.setAdditionalProperties(properties)
.setInputSpec("src/test/resources/3_0/java/native/issue13968.yaml")
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));

final ClientOptInput clientOptInput = configurator.toClientOptInput();
DefaultGenerator generator = new DefaultGenerator();

Map<String, File> files = generator.opts(clientOptInput).generate().stream()
.collect(Collectors.toMap(File::getName, Function.identity()));

File apiFile = files.get("DefaultApi.java");
assertNotNull(apiFile);

JavaFileAssert.assertThat(apiFile).fileContains(
//reading the body into a string, then checking if it is blank.
"String responseBody = new String(localVarResponse.body().readAllBytes());",
"responseBody.isBlank()? null: memberVarObjectMapper.readValue(responseBody, new TypeReference<LocationData>() {})"
);
}


/**
* This checks that the async client is not affected by this fix.
* See https://github.com/OpenAPITools/openapi-generator/issues/13968
*/
@Test
public void callNativeServiceWithEmptyResponseAsync() throws IOException {
Map<String, Object> properties = new HashMap<>();
properties.put(CodegenConstants.API_PACKAGE, "xyz.abcdef.api");
properties.put("asyncNative", "true");

File output = Files.createTempDirectory("test").toFile();
output.deleteOnExit();

final CodegenConfigurator configurator = new CodegenConfigurator()
.setGeneratorName("java")
.setLibrary(JavaClientCodegen.NATIVE)
.setAdditionalProperties(properties)
.setInputSpec("src/test/resources/3_0/java/native/issue13968.yaml")
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));

final ClientOptInput clientOptInput = configurator.toClientOptInput();
DefaultGenerator generator = new DefaultGenerator();

Map<String, File> files = generator.opts(clientOptInput).generate().stream()
.collect(Collectors.toMap(File::getName, Function.identity()));

File apiFile = files.get("DefaultApi.java");
assertNotNull(apiFile);

JavaFileAssert.assertThat(apiFile).fileDoesNotContain(
//reading the body into a string, then checking if it is blank.
"String responseBody = new String(localVarResponse.body().readAllBytes());",
"responseBody.isBlank()? null: memberVarObjectMapper.readValue(responseBody, new TypeReference<LocationData>() {})"
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
openapi: 3.0.3
info:
title: Example Hello API
description: ''
version: v1
servers:
- url: http://localhost
description: Global Endpoint
paths:
/v1/emptyResponse:
get:
operationId: empty
description: returns an empty response
responses:
200:
description: Successful operation
content:
application/json:
schema:
$ref: '#/components/schemas/LocationData'
204:
description: Empty response
components:
schemas:
LocationData:
type: object
properties:
xPos:
type: integer
format: int32
yPos:
type: integer
format: int32
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,21 @@ public ApiResponse<File> testBinaryGifWithHttpInfo() throws ApiException {
if (localVarResponse.statusCode()/ 100 != 2) {
throw getApiException("testBinaryGif", localVarResponse);
}
if (localVarResponse.body() == null) {
return new ApiResponse<File>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
null
);
}

String responseBody = new String(localVarResponse.body().readAllBytes());
localVarResponse.body().close();

return new ApiResponse<File>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
localVarResponse.body() == null ? null : memberVarObjectMapper.readValue(localVarResponse.body(), new TypeReference<File>() {}) // closes the InputStream
localVarResponse.statusCode(),
localVarResponse.headers().map(),
responseBody.isBlank()? null: memberVarObjectMapper.readValue(responseBody, new TypeReference<File>() {})
);
} finally {
}
Expand Down Expand Up @@ -494,10 +505,21 @@ public ApiResponse<Pet> testEchoBodyAllOfPetWithHttpInfo(Pet pet) throws ApiExce
if (localVarResponse.statusCode()/ 100 != 2) {
throw getApiException("testEchoBodyAllOfPet", localVarResponse);
}
if (localVarResponse.body() == null) {
return new ApiResponse<Pet>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
null
);
}

String responseBody = new String(localVarResponse.body().readAllBytes());
localVarResponse.body().close();

return new ApiResponse<Pet>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
localVarResponse.body() == null ? null : memberVarObjectMapper.readValue(localVarResponse.body(), new TypeReference<Pet>() {}) // closes the InputStream
localVarResponse.statusCode(),
localVarResponse.headers().map(),
responseBody.isBlank()? null: memberVarObjectMapper.readValue(responseBody, new TypeReference<Pet>() {})
);
} finally {
}
Expand Down Expand Up @@ -650,10 +672,21 @@ public ApiResponse<Pet> testEchoBodyPetWithHttpInfo(Pet pet) throws ApiException
if (localVarResponse.statusCode()/ 100 != 2) {
throw getApiException("testEchoBodyPet", localVarResponse);
}
if (localVarResponse.body() == null) {
return new ApiResponse<Pet>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
null
);
}

String responseBody = new String(localVarResponse.body().readAllBytes());
localVarResponse.body().close();

return new ApiResponse<Pet>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
localVarResponse.body() == null ? null : memberVarObjectMapper.readValue(localVarResponse.body(), new TypeReference<Pet>() {}) // closes the InputStream
localVarResponse.statusCode(),
localVarResponse.headers().map(),
responseBody.isBlank()? null: memberVarObjectMapper.readValue(responseBody, new TypeReference<Pet>() {})
);
} finally {
}
Expand Down Expand Up @@ -806,10 +839,21 @@ public ApiResponse<StringEnumRef> testEchoBodyStringEnumWithHttpInfo(String body
if (localVarResponse.statusCode()/ 100 != 2) {
throw getApiException("testEchoBodyStringEnum", localVarResponse);
}
if (localVarResponse.body() == null) {
return new ApiResponse<StringEnumRef>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
null
);
}

String responseBody = new String(localVarResponse.body().readAllBytes());
localVarResponse.body().close();

return new ApiResponse<StringEnumRef>(
localVarResponse.statusCode(),
localVarResponse.headers().map(),
localVarResponse.body() == null ? null : memberVarObjectMapper.readValue(localVarResponse.body(), new TypeReference<StringEnumRef>() {}) // closes the InputStream
localVarResponse.statusCode(),
localVarResponse.headers().map(),
responseBody.isBlank()? null: memberVarObjectMapper.readValue(responseBody, new TypeReference<StringEnumRef>() {})
);
} finally {
}
Expand Down
Loading
Loading