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

* [Java][Client] (#13968) * update * update * update slack url --------- Co-authored-by: András Gábor Kis <[email protected]>
OpenAPITools · Dec 15, 2024 · ea2ba0c · ea2ba0c
1 parent 9b3484c
commit ea2ba0c
Show file tree

Hide file tree

Showing 17 changed files with 812 additions and 279 deletions.
diff --git a/README.md b/README.md
@@ -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)

diff --git a/docs/faq.md b/docs/faq.md
@@ -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?
 

diff --git a/modules/openapi-generator/src/main/resources/Java/libraries/native/api.mustache b/modules/openapi-generator/src/main/resources/Java/libraries/native/api.mustache
@@ -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}}

diff --git a/.../openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java b/.../openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java
@@ -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>() {})"
+        );
+    }
 }
diff --git a/modules/openapi-generator/src/test/resources/3_0/java/native/issue13968.yaml b/modules/openapi-generator/src/test/resources/3_0/java/native/issue13968.yaml
@@ -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
diff --git a/samples/client/echo_api/java/native/src/main/java/org/openapitools/client/api/BodyApi.java b/samples/client/echo_api/java/native/src/main/java/org/openapitools/client/api/BodyApi.java
@@ -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 {
       }
@@ -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 {
       }
@@ -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 {
       }
@@ -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 {
       }