From a400d760fc966f660cd687d8d773dc1be04bbb85 Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Mon, 8 Apr 2024 16:59:35 +0100 Subject: [PATCH] fix(java-gen): handle multiple colliding enums (#5853) * [java-gen] Handle multiple colliding enums * CHANGELOG.md * sonar * minor --- CHANGELOG.md | 1 + .../fabric8/java/generator/nodes/JEnum.java | 20 ++++++-- .../java/generator/CompilationTest.java | 3 +- .../test/resources/colliding-enums-crd.yml | 48 +++++++++++++++++++ .../it/certmanager/TestSerialization.java | 4 +- .../resources/cert-manager.crds.1.7.1.yaml | 3 ++ .../ser-deser/src/test/resources/sample1.yaml | 2 + 7 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 java-generator/core/src/test/resources/colliding-enums-crd.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index d3b31a8d56b..3be8e2d9c42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### 6.12-SNAPSHOT #### Bugs +* Fix #5853: [java-generator] Gracefully handle colliding enum definitions #### Improvements diff --git a/java-generator/core/src/main/java/io/fabric8/java/generator/nodes/JEnum.java b/java-generator/core/src/main/java/io/fabric8/java/generator/nodes/JEnum.java index f177e2bcddf..6fd39d01b44 100644 --- a/java-generator/core/src/main/java/io/fabric8/java/generator/nodes/JEnum.java +++ b/java-generator/core/src/main/java/io/fabric8/java/generator/nodes/JEnum.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; @@ -96,16 +97,27 @@ public GeneratorResult generateJava() { .setBody(new BlockStmt().addStatement(new ReturnStmt(VALUE))); getValue.addAnnotation("com.fasterxml.jackson.annotation.JsonValue"); - for (String k : this.values) { - String constantName; + Set constantNames = new HashSet<>(values.size()); + for (String k : values) { + StringBuilder constantNameBuilder = new StringBuilder(); try { // If the value can be parsed as an Integer Integer.valueOf(k); // Prepend - constantName = "V_" + sanitizeEnumEntry(sanitizeString(k)); + constantNameBuilder.append("V_" + sanitizeEnumEntry(sanitizeString(k))); } catch (Exception e) { - constantName = sanitizeEnumEntry(sanitizeString(k)); + constantNameBuilder.append(sanitizeEnumEntry(sanitizeString(k))); } + // enums with colliding names are bad practice, we should make sure that the resulting code compiles, + // but we don't need fancy heuristics for the naming let's just prepend an underscore until it works + while (constantNames.contains(constantNameBuilder.toString())) { + String tmp = constantNameBuilder.toString(); + constantNameBuilder.setLength(0); + constantNameBuilder.append("_" + tmp); + } + String constantName = constantNameBuilder.toString(); + constantNames.add(constantName); + String originalName = AbstractJSONSchema2Pojo.escapeQuotes(k); Expression valueArgument = new StringLiteralExpr(originalName); if (!underlyingType.equals(JAVA_LANG_STRING)) { diff --git a/java-generator/core/src/test/java/io/fabric8/java/generator/CompilationTest.java b/java-generator/core/src/test/java/io/fabric8/java/generator/CompilationTest.java index 9b1d5c54d57..80adcfedc86 100644 --- a/java-generator/core/src/test/java/io/fabric8/java/generator/CompilationTest.java +++ b/java-generator/core/src/test/java/io/fabric8/java/generator/CompilationTest.java @@ -70,7 +70,8 @@ private static Stream compilationTestData() { Arguments.of("two-crds.yml", 6), Arguments.of("folder", 6), Arguments.of("calico-ippool-crd.yml", 3), - Arguments.of("emissary-crds.yaml", 242)); + Arguments.of("emissary-crds.yaml", 242), + Arguments.of("colliding-enums-crd.yml", 2)); } @ParameterizedTest(name = "{0} should generate {1} source files and compile OK") diff --git a/java-generator/core/src/test/resources/colliding-enums-crd.yml b/java-generator/core/src/test/resources/colliding-enums-crd.yml new file mode 100644 index 00000000000..78ce1509ae5 --- /dev/null +++ b/java-generator/core/src/test/resources/colliding-enums-crd.yml @@ -0,0 +1,48 @@ +# +# Copyright (C) 2015 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: collidingenums.example.com +spec: + group: example.com + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + properties: + myenum: + items: + enum: + - one + - One + - onE + - OnE + - oNe + type: string + type: array + type: object + scope: Namespaced + names: + plural: collidingenums + singular: collidingenum + kind: Collidingenum diff --git a/java-generator/it/src/it/ser-deser/src/test/java/io/fabric8/it/certmanager/TestSerialization.java b/java-generator/it/src/it/ser-deser/src/test/java/io/fabric8/it/certmanager/TestSerialization.java index e2b728146a3..e3258bc44b0 100644 --- a/java-generator/it/src/it/ser-deser/src/test/java/io/fabric8/it/certmanager/TestSerialization.java +++ b/java-generator/it/src/it/ser-deser/src/test/java/io/fabric8/it/certmanager/TestSerialization.java @@ -50,12 +50,14 @@ void testDeserialization() { List usagesList = sample.getSpec().getUsages(); // Assert - assertEquals(5, usagesList.size()); + assertEquals(7, usagesList.size()); assertEquals(CertificateRequestSpec.Usages._EMPTY, usagesList.get(0)); assertEquals(CertificateRequestSpec.Usages.SIGNING, usagesList.get(1)); assertEquals(CertificateRequestSpec.Usages.DIGITAL_SIGNATURE, usagesList.get(2)); assertEquals(CertificateRequestSpec.Usages.SERVER_AUTH, usagesList.get(3)); assertEquals(CertificateRequestSpec.Usages.S_MIME, usagesList.get(4)); + assertEquals(CertificateRequestSpec.Usages.ANOTHER, usagesList.get(5)); + assertEquals(CertificateRequestSpec.Usages.__ANOTHER, usagesList.get(6)); assertEquals(datetimeValue, sample.getSpec().getDatetime()); } diff --git a/java-generator/it/src/it/ser-deser/src/test/resources/cert-manager.crds.1.7.1.yaml b/java-generator/it/src/it/ser-deser/src/test/resources/cert-manager.crds.1.7.1.yaml index ae6a215b81a..3c08c7aee55 100644 --- a/java-generator/it/src/it/ser-deser/src/test/resources/cert-manager.crds.1.7.1.yaml +++ b/java-generator/it/src/it/ser-deser/src/test/resources/cert-manager.crds.1.7.1.yaml @@ -182,6 +182,9 @@ spec: - ocsp signing - microsoft sgc - netscape sgc + - another + - Another + - anotheR username: description: Username contains the name of the user that created the CertificateRequest. Populated by the cert-manager webhook on creation and immutable. type: string diff --git a/java-generator/it/src/it/ser-deser/src/test/resources/sample1.yaml b/java-generator/it/src/it/ser-deser/src/test/resources/sample1.yaml index b39836ca1f3..0ac3497ccac 100644 --- a/java-generator/it/src/it/ser-deser/src/test/resources/sample1.yaml +++ b/java-generator/it/src/it/ser-deser/src/test/resources/sample1.yaml @@ -28,6 +28,8 @@ spec: - digital signature - server auth - s/mime + - another + - anotheR duration: 2160h issuerRef: name: ca-issuer