From 27a114d08547dd228c95bf65439c8e64841e7958 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20G=C3=A1bor=20Kis?= Date: Thu, 2 May 2024 16:20:49 +0200 Subject: [PATCH] [Java][Client] (#13968) --- .../Java/libraries/native/api.mustache | 47 ++++++++++--- .../codegen/java/JavaClientCodegenTest.java | 69 +++++++++++++++++++ .../resources/3_0/java/native/issue13968.yaml | 33 +++++++++ 3 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 modules/openapi-generator/src/test/resources/3_0/java/native/issue13968.yaml 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 index 0ade8e9c2bae..0e99f19129e9 100644 --- a/modules/openapi-generator/src/main/resources/Java/libraries/native/api.mustache +++ b/modules/openapi-generator/src/main/resources/Java/libraries/native/api.mustache @@ -1,3 +1,8 @@ +{{! Openapi Generator }} +{{! Copyright (2024) András Gábor Kis, Deutsche Telekom AG }} +{{! This file is made available under the terms of the license Apache-2.0 license }} +{{! SPDX-License-Identifier: Apache-2.0 }} + {{>licenseInfo}} package {{package}}; @@ -271,16 +276,40 @@ 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}} diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java index dc31a2f42a1c..f050ffe78776 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java +++ b/modules/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 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 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() {})" + ); + } + + + /** + * 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 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 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() {})" + ); + } } \ No newline at end of file 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 new file mode 100644 index 000000000000..e44e500d5d29 --- /dev/null +++ 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