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..e94d8d74aa9f --- /dev/null +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessor.java @@ -0,0 +1,71 @@ +/* + * 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 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 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 { + + @Override + public void preprocess(ServiceModel serviceModel) { + Map shapeByClassName = new LinkedHashMap<>(); + List collisions = new ArrayList<>(); + + serviceModel.getShapes().forEach((shapeName, shape) -> { + String className = Utils.capitalize(shapeName); + String existing = shapeByClassName.putIfAbsent(className, shapeName); + 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.", + existing, shapeName, className); + collisions.add(ValidationEntry.create(ValidationErrorId.INVALID_IDENTIFIER_NAME, + ValidationErrorSeverity.DANGER, errorMsg)); + } + }); + + if (!collisions.isEmpty()) { + throw ModelInvalidException.builder().validationEntries(collisions).build(); + } + } + + 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 abba97770279..6e6ac320b5ae 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/shape-name-collision-validator/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..7efe7bed629b --- /dev/null +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/customization/processors/ShapeNameCollisionValidatorProcessorTest.java @@ -0,0 +1,69 @@ +/* + * 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("shape-name-collision-validator/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_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"); + assertThatCode(() -> processor.preprocess(serviceModel)).doesNotThrowAnyException(); + } + + @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/shape-name-collision-validator/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 new file mode 100644 index 000000000000..340849351047 --- /dev/null +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shape-name-collision-validator/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" + } + } +}