From 89c1a3aa6e46b22f84d5c54e8e174ec503ba32f4 Mon Sep 17 00:00:00 2001 From: Jim Schubert Date: Sun, 17 Nov 2019 21:24:18 -0500 Subject: [PATCH 1/2] [java] Improve assumptions about artifactVersion The logic to apply a default artifactVersion was faulty, resulting in generation without an explicit version specified either on the OpenAPI Document or at the CLI/plugin level would result in poms generated with As an example, in any commit made up to 5 weeks before this commit, run: ./bin/java-pkmst-petstore-server.sh The solution is to ensure that artifactVersion isn't overwritten by an "in-process" additonalProperties map, and also to ensure that additionalProperties is synced with the artifactVersion property once it has been modified. As a future task, we'll want to move any modification of additionalProperties outside of preprocessOpenAPI (to processOpts). We're hiding manipulation of the "Opts" at a point where we should really only be applying logic on top of the OpenAPI doc. --- .../languages/AbstractJavaCodegen.java | 6 +-- .../codegen/java/AbstractJavaCodegenTest.java | 50 +++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java index dbbdc762f363..823278ca949a 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java @@ -1040,19 +1040,19 @@ public void preprocessOpenAPI(OpenAPI openAPI) { } } + // TODO: Setting additionalProperties is not the responsibility of this method. These side-effects should be moved elsewhere to prevent unexpected behaviors. if(artifactVersion == null) { // If no artifactVersion is provided in additional properties, version from API specification is used. // If none of them is provided then fallbacks to default version - if (additionalProperties.containsKey(CodegenConstants.ARTIFACT_VERSION)) { + if (additionalProperties.containsKey(CodegenConstants.ARTIFACT_VERSION) && additionalProperties.get(CodegenConstants.ARTIFACT_VERSION) != null) { this.setArtifactVersion((String) additionalProperties.get(CodegenConstants.ARTIFACT_VERSION)); } else if (openAPI.getInfo() != null && openAPI.getInfo().getVersion() != null) { this.setArtifactVersion(openAPI.getInfo().getVersion()); } else { this.setArtifactVersion(ARTIFACT_VERSION_DEFAULT_VALUE); } - } else { - additionalProperties.put(CodegenConstants.ARTIFACT_VERSION, artifactVersion); } + additionalProperties.put(CodegenConstants.ARTIFACT_VERSION, artifactVersion); if (additionalProperties.containsKey(CodegenConstants.SNAPSHOT_VERSION)) { if (convertPropertyToBooleanAndWriteBack(CodegenConstants.SNAPSHOT_VERSION)) { diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java index 7f34d722eb30..73ee84e92659 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java @@ -303,6 +303,7 @@ public void additionalPropertyArtifactSnapShotVersionTest() { @Test(description = "tests if default version is used when neither OpenAPI version nor artifactVersion additional property has been provided") public void defaultVersionTest() { final P_AbstractJavaCodegen codegen = new P_AbstractJavaCodegen(); + codegen.setArtifactVersion(null); OpenAPI api = TestUtils.createOpenAPI(); api.getInfo().setVersion(null); @@ -340,6 +341,55 @@ public void snapshotVersionOpenAPITest() { Assert.assertEquals(codegen.getArtifactVersion(), "2.0-SNAPSHOT"); } + @Test(description = "tests if setting an artifact version programmatically persists to additional properties, when openapi version is null") + public void allowsProgrammaticallySettingArtifactVersionWithNullOpenApiVersion() { + final P_AbstractJavaCodegen codegen = new P_AbstractJavaCodegen(); + final String version = "9.8.7-rc1"; + codegen.setArtifactVersion(version); + + OpenAPI api = TestUtils.createOpenAPI(); + api.getInfo().setVersion(null); + codegen.processOpts(); + codegen.preprocessOpenAPI(api); + + Assert.assertEquals(codegen.getArtifactVersion(), version); + Assert.assertEquals(codegen.additionalProperties().get(CodegenConstants.ARTIFACT_VERSION), version); + } + + @Test(description = "tests if setting an artifact version programmatically persists to additional properties, even when openapi version is specified") + public void allowsProgrammaticallySettingArtifactVersionWithSpecifiedOpenApiVersion() { + final P_AbstractJavaCodegen codegen = new P_AbstractJavaCodegen(); + final String version = "9.8.7-rc1"; + codegen.setArtifactVersion(version); + + OpenAPI api = TestUtils.createOpenAPI(); + api.getInfo().setVersion("1.2.3-SNAPSHOT"); + codegen.processOpts(); + codegen.preprocessOpenAPI(api); + + Assert.assertEquals(codegen.getArtifactVersion(), version); + Assert.assertEquals(codegen.additionalProperties().get(CodegenConstants.ARTIFACT_VERSION), version); + } + + @Test(description = "tests if a null in addition properties artifactVersion results in default version") + public void usesDefaultVersionWhenAdditionalPropertiesVersionIsNull() { + final P_AbstractJavaCodegen codegen = new P_AbstractJavaCodegen(); + final String version = "1.0.0"; + + OpenAPI api = TestUtils.createOpenAPI(); + api.getInfo().setVersion(null); + codegen.setArtifactVersion(null); + codegen.additionalProperties().put(CodegenConstants.ARTIFACT_VERSION, null); + + codegen.processOpts(); + codegen.preprocessOpenAPI(api); + + Assert.assertEquals(codegen.getArtifactVersion(), version); + Assert.assertEquals(codegen.additionalProperties().get(CodegenConstants.ARTIFACT_VERSION), version); + } + + + @Test(description = "tests if default version with snapshot is used when setArtifactVersion is used") public void snapshotVersionAlreadySnapshotTest() { final P_AbstractJavaCodegen codegen = new P_AbstractJavaCodegen(); From 6ca7fb48f87a3bc48c12b09bba0e5e9d6ad958da Mon Sep 17 00:00:00 2001 From: Jim Schubert Date: Sun, 17 Nov 2019 21:36:37 -0500 Subject: [PATCH 2/2] [sample] Regenerate java-pkmst sample --- samples/server/petstore/java-pkmst/.openapi-generator/VERSION | 2 +- samples/server/petstore/java-pkmst/pom.xml | 2 +- .../src/main/java/com/prokarma/pkmst/controller/PetApi.java | 2 +- .../src/main/java/com/prokarma/pkmst/controller/StoreApi.java | 2 +- .../src/main/java/com/prokarma/pkmst/controller/UserApi.java | 2 +- .../java-pkmst/src/main/java/com/prokarma/pkmst/model/Pet.java | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/samples/server/petstore/java-pkmst/.openapi-generator/VERSION b/samples/server/petstore/java-pkmst/.openapi-generator/VERSION index afa636560641..e4955748d3e7 100644 --- a/samples/server/petstore/java-pkmst/.openapi-generator/VERSION +++ b/samples/server/petstore/java-pkmst/.openapi-generator/VERSION @@ -1 +1 @@ -4.0.0-SNAPSHOT \ No newline at end of file +4.2.2-SNAPSHOT \ No newline at end of file diff --git a/samples/server/petstore/java-pkmst/pom.xml b/samples/server/petstore/java-pkmst/pom.xml index 8a62b880bf6c..66150f463153 100644 --- a/samples/server/petstore/java-pkmst/pom.xml +++ b/samples/server/petstore/java-pkmst/pom.xml @@ -232,7 +232,7 @@ org.threeten threetenbp - 1.3.6 + 1.4.0 diff --git a/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/PetApi.java b/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/PetApi.java index 5f3bc7b91155..ec1b2c40b725 100644 --- a/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/PetApi.java +++ b/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/PetApi.java @@ -1,5 +1,5 @@ /** - * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech) (4.0.0-SNAPSHOT). + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech) (4.2.2-SNAPSHOT). * https://openapi-generator.tech * Do not edit the class manually. */ diff --git a/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/StoreApi.java b/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/StoreApi.java index e4786488ec79..c3d12f4e9c1c 100644 --- a/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/StoreApi.java +++ b/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/StoreApi.java @@ -1,5 +1,5 @@ /** - * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech) (4.0.0-SNAPSHOT). + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech) (4.2.2-SNAPSHOT). * https://openapi-generator.tech * Do not edit the class manually. */ diff --git a/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/UserApi.java b/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/UserApi.java index f136720e1970..74634432fb40 100644 --- a/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/UserApi.java +++ b/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/controller/UserApi.java @@ -1,5 +1,5 @@ /** - * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech) (4.0.0-SNAPSHOT). + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech) (4.2.2-SNAPSHOT). * https://openapi-generator.tech * Do not edit the class manually. */ diff --git a/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/model/Pet.java b/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/model/Pet.java index 2d364ea986d5..2d5e1ef4e1bf 100644 --- a/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/model/Pet.java +++ b/samples/server/petstore/java-pkmst/src/main/java/com/prokarma/pkmst/model/Pet.java @@ -26,7 +26,7 @@ public class Pet { private Long id; @JsonProperty("category") - private Category category = null; + private Category category; @JsonProperty("name") private String name;