From 05038538e95bc8ba65ac5d2a76b1bf2bd78a5e47 Mon Sep 17 00:00:00 2001 From: RanVaknin <50976344+RanVaknin@users.noreply.github.com> Date: Mon, 20 Jul 2026 14:39:10 -0700 Subject: [PATCH 1/2] Add new codgen processor to throw ModelInvalidException on case colliding shape names instead of NPE --- .../DefaultCustomizationProcessor.java | 1 + .../ShapeNameCollisionValidatorProcessor.java | 63 +++++++++++++++++ .../codegen/IntermediateModelBuilderTest.java | 26 +++++++ ...peNameCollisionValidatorProcessorTest.java | 70 +++++++++++++++++++ .../collision/case-insensitive-service-2.json | 56 +++++++++++++++ .../same-type-service-2.json | 32 +++++++++ .../valid-service-2.json | 42 +++++++++++ 7 files changed, 290 insertions(+) create mode 100644 codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessor.java create mode 100644 codegen/src/test/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessorTest.java create mode 100644 codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/collision/case-insensitive-service-2.json create mode 100644 codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shape-name-collision-validator/same-type-service-2.json create mode 100644 codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shape-name-collision-validator/valid-service-2.json diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/DefaultCustomizationProcessor.java b/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/DefaultCustomizationProcessor.java index 1511bf6be282..ba1e69e55cba 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/DefaultCustomizationProcessor.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/DefaultCustomizationProcessor.java @@ -42,6 +42,7 @@ public static CodegenCustomizationProcessor getProcessorFor( new S3RemoveBucketFromUriProcessor(), new S3ControlRemoveAccountIdHostPrefixProcessor(), new ExplicitStringPayloadQueryProtocolProcessor(), + new ShapeNameCollisionValidatorProcessor(), new LowercaseShapeValidatorProcessor(), new LongPollingOperationProcessor() ); diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessor.java b/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessor.java new file mode 100644 index 000000000000..f00d0a5a5423 --- /dev/null +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessor.java @@ -0,0 +1,63 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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. + */ + +package software.amazon.awssdk.codegen.customization.processors; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import software.amazon.awssdk.codegen.customization.CodegenCustomizationProcessor; +import software.amazon.awssdk.codegen.internal.Utils; +import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; +import software.amazon.awssdk.codegen.model.service.ServiceModel; +import software.amazon.awssdk.codegen.validation.ModelInvalidException; +import software.amazon.awssdk.codegen.validation.ValidationEntry; +import software.amazon.awssdk.codegen.validation.ValidationErrorId; +import software.amazon.awssdk.codegen.validation.ValidationErrorSeverity; + +/** + * Rejects shape names that differ only by first character case, which otherwise collide on their generated Java class name. + */ +public class ShapeNameCollisionValidatorProcessor implements CodegenCustomizationProcessor { + + @Override + public void preprocess(ServiceModel serviceModel) { + Map shapeByClassName = new LinkedHashMap<>(); + List collisions = new ArrayList<>(); + + serviceModel.getShapes().keySet().forEach(shapeName -> { + String className = Utils.capitalize(shapeName); + String existing = shapeByClassName.putIfAbsent(className, shapeName); + if (existing != null) { + String errorMsg = String.format( + "Shape names '%s' and '%s' collide because they map to the same generated class name '%s'. Please rename " + + "one of them in your service model.", + existing, shapeName, className); + collisions.add(ValidationEntry.create(ValidationErrorId.INVALID_IDENTIFIER_NAME, + ValidationErrorSeverity.DANGER, errorMsg)); + } + }); + + if (!collisions.isEmpty()) { + throw ModelInvalidException.builder().validationEntries(collisions).build(); + } + } + + @Override + public void postprocess(IntermediateModel intermediateModel) { + // no-op + } +} diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/IntermediateModelBuilderTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/IntermediateModelBuilderTest.java index abba97770279..2d7b00bcd659 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/IntermediateModelBuilderTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/IntermediateModelBuilderTest.java @@ -16,6 +16,7 @@ package software.amazon.awssdk.codegen; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -28,6 +29,10 @@ import software.amazon.awssdk.codegen.model.intermediate.ShapeModel; import software.amazon.awssdk.codegen.model.service.ServiceModel; import software.amazon.awssdk.codegen.utils.ModelLoaderUtils; +import software.amazon.awssdk.codegen.validation.ModelInvalidException; +import software.amazon.awssdk.codegen.validation.ValidationEntry; +import software.amazon.awssdk.codegen.validation.ValidationErrorId; +import software.amazon.awssdk.codegen.validation.ValidationErrorSeverity; public class IntermediateModelBuilderTest { @@ -47,6 +52,27 @@ public void testServiceAndShapeNameCollisions() throws Exception { .containsExactlyInAnyOrder("DefaultCollisionException", "DefaultCollisionRequest", "DefaultCollisionResponse"); } + @Test + public void build_shapesCollideByCase_throwsModelInvalid() { + final File modelFile = new File(IntermediateModelBuilderTest.class + .getResource("poet/client/c2j/collision/case-insensitive-service-2.json").getFile()); + + assertThatThrownBy(() -> new IntermediateModelBuilder( + C2jModels.builder() + .serviceModel(ModelLoaderUtils.loadModel(ServiceModel.class, modelFile)) + .customizationConfig(CustomizationConfig.create()) + .build()) + .build()) + .isInstanceOf(ModelInvalidException.class) + .hasMessageContaining("ReservationType") + .hasMessageContaining("reservationType") + .matches(e -> { + ValidationEntry entry = ((ModelInvalidException) e).validationEntries().get(0); + return entry.getErrorId() == ValidationErrorId.INVALID_IDENTIFIER_NAME + && entry.getSeverity() == ValidationErrorSeverity.DANGER; + }, "validation entry is INVALID_IDENTIFIER_NAME / DANGER"); + } + @Test public void sharedOutputShapesLinkCorrectlyToOperationOutputs() { final File modelFile = new File(IntermediateModelBuilderTest.class diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessorTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessorTest.java new file mode 100644 index 000000000000..4db0f3ac7c91 --- /dev/null +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessorTest.java @@ -0,0 +1,70 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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. + */ + +package software.amazon.awssdk.codegen.customization.processors; + +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.File; +import java.io.IOException; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.codegen.model.service.ServiceModel; +import software.amazon.awssdk.codegen.utils.ModelLoaderUtils; +import software.amazon.awssdk.codegen.validation.ModelInvalidException; +import software.amazon.awssdk.codegen.validation.ValidationEntry; +import software.amazon.awssdk.codegen.validation.ValidationErrorId; +import software.amazon.awssdk.codegen.validation.ValidationErrorSeverity; + +public class ShapeNameCollisionValidatorProcessorTest { + + private final ShapeNameCollisionValidatorProcessor processor = new ShapeNameCollisionValidatorProcessor(); + + private static ServiceModel loadModel(String resource) throws IOException { + File file = new File(ShapeNameCollisionValidatorProcessorTest.class + .getResource("/software/amazon/awssdk/codegen/poet/client/c2j/" + resource).getFile()); + return ModelLoaderUtils.loadModel(ServiceModel.class, file); + } + + @Test + public void preprocess_shapesCollideByCase_throwsException() throws IOException { + ServiceModel serviceModel = loadModel("collision/case-insensitive-service-2.json"); + assertThatThrownBy(() -> processor.preprocess(serviceModel)) + .isInstanceOf(ModelInvalidException.class) + .hasMessageContaining("ReservationType") + .hasMessageContaining("reservationType") + .matches(e -> { + ModelInvalidException modelInvalid = (ModelInvalidException) e; + ValidationEntry entry = modelInvalid.validationEntries().get(0); + return entry.getErrorId() == ValidationErrorId.INVALID_IDENTIFIER_NAME && + entry.getSeverity() == ValidationErrorSeverity.DANGER; + }, "Validation entry details are correct"); + } + + @Test + public void preprocess_shapesOfSameTypeCollideByCase_throwsException() throws IOException { + ServiceModel serviceModel = loadModel("shape-name-collision-validator/same-type-service-2.json"); + assertThatThrownBy(() -> processor.preprocess(serviceModel)) + .isInstanceOf(ModelInvalidException.class) + .hasMessageContaining("Token") + .hasMessageContaining("token"); + } + + @Test + public void preprocess_noCaseCollision_doesNotThrow() throws IOException { + ServiceModel serviceModel = loadModel("shape-name-collision-validator/valid-service-2.json"); + assertThatCode(() -> processor.preprocess(serviceModel)).doesNotThrowAnyException(); + } +} diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/collision/case-insensitive-service-2.json b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/collision/case-insensitive-service-2.json new file mode 100644 index 000000000000..340849351047 --- /dev/null +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/collision/case-insensitive-service-2.json @@ -0,0 +1,56 @@ +{ + "version": "2.0", + "metadata": { + "apiVersion": "2024-01-01", + "endpointPrefix": "collision", + "jsonVersion": "1.1", + "protocol": "json", + "serviceAbbreviation": "Collision", + "serviceFullName": "Shape Name Collision Build Test Service", + "serviceId": "Collision", + "signatureVersion": "v4", + "targetPrefix": "Collision", + "uid": "collision-2024-01-01" + }, + "operations": { + "GetReservation": { + "name": "GetReservation", + "http": { + "method": "POST", + "requestUri": "/" + }, + "input": {"shape": "GetReservationRequest"}, + "output": {"shape": "GetReservationResponse"} + } + }, + "shapes": { + "GetReservationRequest": { + "type": "structure", + "members": { + "Name": {"shape": "String"} + } + }, + "GetReservationResponse": { + "type": "structure", + "members": { + "ReservationTypes": {"shape": "ReservationTypeList"} + } + }, + "ReservationTypeList": { + "type": "list", + "member": {"shape": "reservationType"} + }, + "ReservationType": { + "type": "structure", + "members": { + "Name": {"shape": "String"} + } + }, + "reservationType": { + "type": "string" + }, + "String": { + "type": "string" + } + } +} diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shape-name-collision-validator/same-type-service-2.json b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shape-name-collision-validator/same-type-service-2.json new file mode 100644 index 000000000000..37cb9644ef45 --- /dev/null +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shape-name-collision-validator/same-type-service-2.json @@ -0,0 +1,32 @@ +{ + "version": "2.0", + "metadata": { + "serviceId": "SameTypeCollision", + "serviceFullName": "Same Type Shape Name Collision Test Service" + }, + "operations": { + "TestOperation": { + "name": "TestOperation", + "http": { + "method": "POST", + "requestUri": "/" + }, + "input": {"shape": "TestOperationRequest"} + } + }, + "shapes": { + "TestOperationRequest": { + "type": "structure", + "members": { + "Token": {"shape": "Token"}, + "OtherToken": {"shape": "token"} + } + }, + "Token": { + "type": "string" + }, + "token": { + "type": "string" + } + } +} diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shape-name-collision-validator/valid-service-2.json b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shape-name-collision-validator/valid-service-2.json new file mode 100644 index 000000000000..f85496b0135c --- /dev/null +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shape-name-collision-validator/valid-service-2.json @@ -0,0 +1,42 @@ +{ + "version": "2.0", + "metadata": { + "serviceId": "ShapeNameNoCollision", + "serviceFullName": "Shape Name No Collision Validation Test Service" + }, + "operations": { + "TestOperation": { + "name": "TestOperation", + "http": { + "method": "POST", + "requestUri": "/" + }, + "input": {"shape": "TestOperationRequest"} + } + }, + "shapes": { + "TestOperationRequest": { + "type": "structure", + "members": { + "IpAddress": { + "shape": "IpAddress" + }, + "IpAddr": { + "shape": "IPAddress" + }, + "Token": { + "shape": "nextToken" + } + } + }, + "IpAddress": { + "type": "string" + }, + "IPAddress": { + "type": "string" + }, + "nextToken": { + "type": "string" + } + } +} From 9e8733f918eb875bf56b94764884f964082b45c9 Mon Sep 17 00:00:00 2001 From: RanVaknin <50976344+RanVaknin@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:40:42 -0700 Subject: [PATCH 2/2] Fix logic to fail only when shapes differ by case AND by type --- .../ShapeNameCollisionValidatorProcessor.java | 14 +++++++++++--- .../codegen/IntermediateModelBuilderTest.java | 2 +- .../ShapeNameCollisionValidatorProcessorTest.java | 11 +++++------ .../case-insensitive-service-2.json | 0 4 files changed, 17 insertions(+), 10 deletions(-) rename codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/{collision => shape-name-collision-validator}/case-insensitive-service-2.json (100%) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessor.java b/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessor.java index f00d0a5a5423..e94d8d74aa9f 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessor.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessor.java @@ -19,17 +19,21 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import software.amazon.awssdk.codegen.customization.CodegenCustomizationProcessor; import software.amazon.awssdk.codegen.internal.Utils; import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; import software.amazon.awssdk.codegen.model.service.ServiceModel; +import software.amazon.awssdk.codegen.model.service.Shape; import software.amazon.awssdk.codegen.validation.ModelInvalidException; import software.amazon.awssdk.codegen.validation.ValidationEntry; import software.amazon.awssdk.codegen.validation.ValidationErrorId; import software.amazon.awssdk.codegen.validation.ValidationErrorSeverity; /** - * Rejects shape names that differ only by first character case, which otherwise collide on their generated Java class name. + * Rejects two shape names that map to the same generated Java class name (they differ only by the case of their first + * character) when the shapes are of different types, since one silently overwrites the other and the lost shape later fails + * codegen. */ public class ShapeNameCollisionValidatorProcessor implements CodegenCustomizationProcessor { @@ -38,10 +42,10 @@ public void preprocess(ServiceModel serviceModel) { Map shapeByClassName = new LinkedHashMap<>(); List collisions = new ArrayList<>(); - serviceModel.getShapes().keySet().forEach(shapeName -> { + serviceModel.getShapes().forEach((shapeName, shape) -> { String className = Utils.capitalize(shapeName); String existing = shapeByClassName.putIfAbsent(className, shapeName); - if (existing != null) { + if (existing != null && !sameType(serviceModel.getShapes().get(existing), shape)) { String errorMsg = String.format( "Shape names '%s' and '%s' collide because they map to the same generated class name '%s'. Please rename " + "one of them in your service model.", @@ -56,6 +60,10 @@ public void preprocess(ServiceModel serviceModel) { } } + private static boolean sameType(Shape a, Shape b) { + return Objects.equals(a.getType(), b.getType()); + } + @Override public void postprocess(IntermediateModel intermediateModel) { // no-op diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/IntermediateModelBuilderTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/IntermediateModelBuilderTest.java index 2d7b00bcd659..6e6ac320b5ae 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/IntermediateModelBuilderTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/IntermediateModelBuilderTest.java @@ -55,7 +55,7 @@ public void testServiceAndShapeNameCollisions() throws Exception { @Test public void build_shapesCollideByCase_throwsModelInvalid() { final File modelFile = new File(IntermediateModelBuilderTest.class - .getResource("poet/client/c2j/collision/case-insensitive-service-2.json").getFile()); + .getResource("poet/client/c2j/shape-name-collision-validator/case-insensitive-service-2.json").getFile()); assertThatThrownBy(() -> new IntermediateModelBuilder( C2jModels.builder() diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessorTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessorTest.java index 4db0f3ac7c91..7efe7bed629b 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessorTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessorTest.java @@ -40,7 +40,7 @@ private static ServiceModel loadModel(String resource) throws IOException { @Test public void preprocess_shapesCollideByCase_throwsException() throws IOException { - ServiceModel serviceModel = loadModel("collision/case-insensitive-service-2.json"); + ServiceModel serviceModel = loadModel("shape-name-collision-validator/case-insensitive-service-2.json"); assertThatThrownBy(() -> processor.preprocess(serviceModel)) .isInstanceOf(ModelInvalidException.class) .hasMessageContaining("ReservationType") @@ -54,12 +54,11 @@ public void preprocess_shapesCollideByCase_throwsException() throws IOException } @Test - public void preprocess_shapesOfSameTypeCollideByCase_throwsException() throws IOException { + public void preprocess_sameTypeShapesCollideByCase_doesNotThrow() throws IOException { + // Token/token both resolve to the same class but are both strings, so one harmlessly overwrites the other with + // equivalent output. Such pairs exist in shipping models (e.g. String/string) and must not be rejected. ServiceModel serviceModel = loadModel("shape-name-collision-validator/same-type-service-2.json"); - assertThatThrownBy(() -> processor.preprocess(serviceModel)) - .isInstanceOf(ModelInvalidException.class) - .hasMessageContaining("Token") - .hasMessageContaining("token"); + assertThatCode(() -> processor.preprocess(serviceModel)).doesNotThrowAnyException(); } @Test diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/collision/case-insensitive-service-2.json b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shape-name-collision-validator/case-insensitive-service-2.json similarity index 100% rename from codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/collision/case-insensitive-service-2.json rename to codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shape-name-collision-validator/case-insensitive-service-2.json