Skip to content

Commit 5b1a1d1

Browse files
adamthom-amznmillems
adamthom-amzn
authored and
millems
committed
Fix generation for operations that share an output shape.
Two shapes can have the same C2J name but different intermediate model names. This seems to work fine for request types, but not response types, where the lookups were done by C2J name only.
1 parent 56022f9 commit 5b1a1d1

File tree

9 files changed

+195
-11
lines changed

9 files changed

+195
-11
lines changed

‎.changes/next-release/bugfix-AWSSDKforJavav2-66ea1ac.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "AWS SDK for Java v2",
3+
"type": "bugfix",
4+
"description": "Fix generation for operations that share an output shape."
5+
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/IntermediateModelBuilder.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,9 @@ private void linkOperationsToInputOutputShapes(IntermediateModel model) {
186186

187187
if (operation.getOutput() != null) {
188188
String outputShapeName = operation.getOutput().getShape();
189-
entry.getValue().setOutputShape(model.getShapeByC2jName(outputShapeName));
189+
ShapeModel outputShape =
190+
model.getShapeByNameAndC2jName(entry.getValue().getReturnType().getReturnType(), outputShapeName);
191+
entry.getValue().setOutputShape(outputShape);
190192
}
191193
}
192194
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/IntermediateModel.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import java.util.stream.Collectors;
2828
import software.amazon.awssdk.awscore.AwsResponse;
2929
import software.amazon.awssdk.awscore.AwsResponseMetadata;
30-
import software.amazon.awssdk.codegen.internal.Utils;
3130
import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig;
3231
import software.amazon.awssdk.codegen.model.service.PaginatorDefinition;
3332
import software.amazon.awssdk.codegen.naming.NamingStrategy;
@@ -104,8 +103,21 @@ public Map<String, ShapeModel> getShapes() {
104103
return shapes;
105104
}
106105

107-
public ShapeModel getShapeByC2jName(String c2jName) {
108-
return Utils.findShapeModelByC2jName(this, c2jName);
106+
/**
107+
* Looks up a shape by name and verifies that the expected C2J name matches
108+
* @param shapeName the name of the shape in the intermediate model
109+
* @param shapeC2jName C2J's name for the shape
110+
* @return the ShapeModel
111+
* @throws IllegalArgumentException if no matching shape is found
112+
*/
113+
public ShapeModel getShapeByNameAndC2jName(String shapeName, String shapeC2jName) {
114+
for (ShapeModel sm : getShapes().values()) {
115+
if (shapeName.equals(sm.getShapeName()) && shapeC2jName.equals(sm.getC2jName())) {
116+
return sm;
117+
}
118+
}
119+
throw new IllegalArgumentException("C2J shape " + shapeC2jName + " with shape name " + shapeName + " does not exist in "
120+
+ "the intermediate model.");
109121
}
110122

111123
public CustomizationConfig getCustomizationConfig() {

‎codegen/src/test/java/software/amazon/awssdk/codegen/IntermediateModelBuilderTest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.codegen;
17+
18+
import static org.junit.Assert.assertEquals;
19+
20+
import java.io.File;
21+
import org.junit.Test;
22+
import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig;
23+
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
24+
import software.amazon.awssdk.codegen.model.service.ServiceModel;
25+
import software.amazon.awssdk.codegen.utils.ModelLoaderUtils;
26+
27+
public class IntermediateModelBuilderTest {
28+
29+
@Test
30+
public void sharedOutputShapesLinkCorrectlyToOperationOutputs() {
31+
final File modelFile = new File(IntermediateModelBuilderTest.class
32+
.getResource("poet/client/c2j/shared-output/service-2.json").getFile());
33+
IntermediateModel testModel = new IntermediateModelBuilder(
34+
C2jModels.builder()
35+
.serviceModel(ModelLoaderUtils.loadModel(ServiceModel.class, modelFile))
36+
.customizationConfig(CustomizationConfig.create())
37+
.build())
38+
.build();
39+
40+
assertEquals("PingResponse", testModel.getOperation("Ping").getOutputShape().getShapeName());
41+
assertEquals("SecurePingResponse", testModel.getOperation("SecurePing").getOutputShape().getShapeName());
42+
}
43+
44+
}

‎codegen/src/test/java/software/amazon/awssdk/codegen/model/intermediate/IntermediateModelTest.java

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.codegen.model.intermediate;
17+
18+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
19+
20+
import java.io.File;
21+
import java.util.Collections;
22+
import org.junit.Test;
23+
import software.amazon.awssdk.codegen.C2jModels;
24+
import software.amazon.awssdk.codegen.IntermediateModelBuilder;
25+
import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig;
26+
import software.amazon.awssdk.codegen.model.service.ServiceMetadata;
27+
import software.amazon.awssdk.codegen.model.service.ServiceModel;
28+
import software.amazon.awssdk.codegen.utils.ModelLoaderUtils;
29+
30+
public class IntermediateModelTest {
31+
32+
@Test
33+
public void cannotFindShapeWhenNoShapesExist() {
34+
35+
ServiceMetadata metadata = new ServiceMetadata();
36+
metadata.setProtocol(Protocol.REST_JSON.getValue());
37+
metadata.setServiceId("empty-service");
38+
metadata.setSignatureVersion("V4");
39+
40+
IntermediateModel testModel = new IntermediateModelBuilder(
41+
C2jModels.builder()
42+
.serviceModel(new ServiceModel(metadata,
43+
Collections.emptyMap(),
44+
Collections.emptyMap(),
45+
Collections.emptyMap()))
46+
.customizationConfig(CustomizationConfig.create())
47+
.build())
48+
.build();
49+
50+
assertThatThrownBy(() -> testModel.getShapeByNameAndC2jName("AnyShape", "AnyShape"))
51+
.isInstanceOf(IllegalArgumentException.class)
52+
.hasMessage("C2J shape AnyShape with shape name AnyShape does not exist in the intermediate model.");
53+
}
54+
55+
@Test
56+
public void getShapeByNameAndC2jNameVerifiesC2JName() {
57+
final File modelFile = new File(IntermediateModelTest.class
58+
.getResource("../../poet/client/c2j/shared-output/service-2.json").getFile());
59+
IntermediateModel testModel = new IntermediateModelBuilder(
60+
C2jModels.builder()
61+
.serviceModel(ModelLoaderUtils.loadModel(ServiceModel.class, modelFile))
62+
.customizationConfig(CustomizationConfig.create())
63+
.build())
64+
.build();
65+
66+
67+
68+
assertThatThrownBy(() -> testModel.getShapeByNameAndC2jName("PingResponse", "AnyShape"))
69+
.isInstanceOf(IllegalArgumentException.class)
70+
.hasMessage("C2J shape AnyShape with shape name PingResponse does not exist in the intermediate model.");
71+
}
72+
}

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/shared-output/service-2.json

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
{
2+
"version": "2.0",
3+
"metadata": {
4+
"apiVersion": "2010-05-08",
5+
"endpointPrefix": "shared-output-service",
6+
"globalEndpoint": "shared-output-service.amazonaws.com",
7+
"protocol": "rest-json",
8+
"serviceAbbreviation": "Shared Output Service",
9+
"serviceFullName": "Shared output service",
10+
"serviceId":"SharedOutputService",
11+
"signatureVersion": "v4",
12+
"uid": "shared-output-service-2010-05-08",
13+
"xmlNamespace": "https://shared-output-service.amazonaws.com/doc/2010-05-08/"
14+
},
15+
"operations": {
16+
"Ping": {
17+
"name": "ping",
18+
"http": {
19+
"method": "POST",
20+
"requestUri": "/ping"
21+
},
22+
"errors": [],
23+
"output": {
24+
"shape" : "PingOutput"
25+
},
26+
"documentation": "<p>ping</p>"
27+
},
28+
"SecurePing": {
29+
"name": "sping",
30+
"http": {
31+
"method": "POST",
32+
"requestUri": "/sping"
33+
},
34+
"errors": [],
35+
"output": {
36+
"shape" : "PingOutput"
37+
},
38+
"documentation": "<p>secure ping</p>"
39+
}
40+
},
41+
"shapes": {
42+
"PingOutput": {
43+
"type": "structure",
44+
"required": [],
45+
"members": {}
46+
}
47+
},
48+
"documentation": "A ping service that shared outputs"
49+
}

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-json-client-class.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ public StreamingInputOperationResponse streamingInputOperation(StreamingInputOpe
494494
* The service documentation for the request content is as follows 'This be a stream'
495495
* @param responseTransformer
496496
* Functional interface for processing the streamed response content. The unmarshalled
497-
* StreamingInputOutputOperationRequest and an InputStream to the response content are provided as parameters
497+
* StreamingInputOutputOperationResponse and an InputStream to the response content are provided as parameters
498498
* to the callback. The callback may return a transformed type which will be the return value of this method.
499499
* See {@link software.amazon.awssdk.core.sync.ResponseTransformer} for details on implementing this
500500
* interface and for links to pre-canned implementations for common scenarios like downloading to a file. The
@@ -546,7 +546,7 @@ public <ReturnT> ReturnT streamingInputOutputOperation(
546546
* @param streamingOutputOperationRequest
547547
* @param responseTransformer
548548
* Functional interface for processing the streamed response content. The unmarshalled
549-
* StreamingInputOutputOperationRequest and an InputStream to the response content are provided as parameters
549+
* StreamingOutputOperationResponse and an InputStream to the response content are provided as parameters
550550
* to the callback. The callback may return a transformed type which will be the return value of this method.
551551
* See {@link software.amazon.awssdk.core.sync.ResponseTransformer} for details on implementing this
552552
* interface and for links to pre-canned implementations for common scenarios like downloading to a file. The

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-json-client-interface.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -887,7 +887,7 @@ default StreamingInputOperationResponse streamingInputOperation(
887887
* The service documentation for the request content is as follows 'This be a stream'
888888
* @param responseTransformer
889889
* Functional interface for processing the streamed response content. The unmarshalled
890-
* StreamingInputOutputOperationRequest and an InputStream to the response content are provided as parameters
890+
* StreamingInputOutputOperationResponse and an InputStream to the response content are provided as parameters
891891
* to the callback. The callback may return a transformed type which will be the return value of this method.
892892
* See {@link software.amazon.awssdk.core.sync.ResponseTransformer} for details on implementing this
893893
* interface and for links to pre-canned implementations for common scenarios like downloading to a file. The
@@ -934,7 +934,7 @@ default <ReturnT> ReturnT streamingInputOutputOperation(
934934
* The service documentation for the request content is as follows 'This be a stream'
935935
* @param responseTransformer
936936
* Functional interface for processing the streamed response content. The unmarshalled
937-
* StreamingInputOutputOperationRequest and an InputStream to the response content are provided as parameters
937+
* StreamingInputOutputOperationResponse and an InputStream to the response content are provided as parameters
938938
* to the callback. The callback may return a transformed type which will be the return value of this method.
939939
* See {@link software.amazon.awssdk.core.sync.ResponseTransformer} for details on implementing this
940940
* interface and for links to pre-canned implementations for common scenarios like downloading to a file. The
@@ -1041,7 +1041,7 @@ default StreamingInputOutputOperationResponse streamingInputOutputOperation(
10411041
* @param streamingOutputOperationRequest
10421042
* @param responseTransformer
10431043
* Functional interface for processing the streamed response content. The unmarshalled
1044-
* StreamingInputOutputOperationRequest and an InputStream to the response content are provided as parameters
1044+
* StreamingOutputOperationResponse and an InputStream to the response content are provided as parameters
10451045
* to the callback. The callback may return a transformed type which will be the return value of this method.
10461046
* See {@link software.amazon.awssdk.core.sync.ResponseTransformer} for details on implementing this
10471047
* interface and for links to pre-canned implementations for common scenarios like downloading to a file. The
@@ -1076,7 +1076,7 @@ default <ReturnT> ReturnT streamingOutputOperation(StreamingOutputOperationReque
10761076
* request.
10771077
* @param responseTransformer
10781078
* Functional interface for processing the streamed response content. The unmarshalled
1079-
* StreamingInputOutputOperationRequest and an InputStream to the response content are provided as parameters
1079+
* StreamingOutputOperationResponse and an InputStream to the response content are provided as parameters
10801080
* to the callback. The callback may return a transformed type which will be the return value of this method.
10811081
* See {@link software.amazon.awssdk.core.sync.ResponseTransformer} for details on implementing this
10821082
* interface and for links to pre-canned implementations for common scenarios like downloading to a file. The

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-query-client-class.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ public StreamingInputOperationResponse streamingInputOperation(StreamingInputOpe
182182
* @param streamingOutputOperationRequest
183183
* @param responseTransformer
184184
* Functional interface for processing the streamed response content. The unmarshalled
185-
* StreamingInputOperationRequest and an InputStream to the response content are provided as parameters to
185+
* StreamingOutputOperationResponse and an InputStream to the response content are provided as parameters to
186186
* the callback. The callback may return a transformed type which will be the return value of this method.
187187
* See {@link software.amazon.awssdk.core.sync.ResponseTransformer} for details on implementing this
188188
* interface and for links to pre-canned implementations for common scenarios like downloading to a file. The

0 commit comments

Comments
 (0)