Skip to content

Commit 9747bef

Browse files
sugmanuekstich
sugmanue
and
kstich
authored
Change the transform to add new shapes before removing old ones (#3001)
* Change the transform to add new shapes before removing old ones If the shapes are removed first, when the operation has the `clientDiscoveredEndpoint` the transform `CleanClientDiscoveryTraitTransformer` that handles the removal of shapes throws an exception because the operation that's expecting is not longer in the model. To avoid this issue, we change the order of the transformation operations, first we add the new shapes, and after that we remove the old ones. This ensures that when the `CleanClientDiscoveryTraitTransformer` is run the expected shapes are present. * Improve the logic and remove the extra loops * Update smithy-model/src/main/java/software/amazon/smithy/model/transform/CreateDedicatedInputAndOutput.java Co-authored-by: Kevin Stich <kevin@kstich.com> --------- Co-authored-by: Kevin Stich <kevin@kstich.com>
1 parent 4491592 commit 9747bef

File tree

4 files changed

+231
-7
lines changed

4 files changed

+231
-7
lines changed

‎gradle/wrapper/gradle-wrapper.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
distributionBase=GRADLE_USER_HOME
22
distributionPath=wrapper/dists
3-
distributionUrl=https\://services.gradle.org/distributions/gradle-9.4.0-bin.zip
3+
distributionUrl=https\://services.gradle.org/distributions/gradle-9.1.0-bin.zip
44
networkTimeout=10000
55
validateDistributionUrl=true
66
zipStoreBase=GRADLE_USER_HOME

‎smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/clientendpointdiscovery/CleanClientDiscoveryTraitTransformerTest.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99

1010
import org.junit.jupiter.api.Test;
1111
import software.amazon.smithy.model.Model;
12+
import software.amazon.smithy.model.node.Node;
1213
import software.amazon.smithy.model.shapes.MemberShape;
14+
import software.amazon.smithy.model.shapes.ModelSerializer;
1315
import software.amazon.smithy.model.shapes.OperationShape;
1416
import software.amazon.smithy.model.shapes.ServiceShape;
1517
import software.amazon.smithy.model.shapes.Shape;
@@ -139,6 +141,25 @@ public void keepsDiscoveryIdTraitIfStillBound() {
139141
.get();
140142

141143
assertTrue(id.hasTrait(ClientEndpointDiscoveryIdTrait.ID));
144+
}
145+
146+
@Test
147+
public void clientDiscoveryTraitIsPreservedAfterCreateDedicatedInputAndOutputTransform() {
148+
Model model = Model.assembler()
149+
.discoverModels(getClass().getClassLoader())
150+
.addImport(getClass().getResource("test-model.json"))
151+
.assemble()
152+
.unwrap();
153+
Model actualResult = ModelTransformer.create().createDedicatedInputAndOutput(model, "Request", "Response");
154+
Model expectedResult = Model.assembler()
155+
.discoverModels(getClass().getClassLoader())
156+
.addImport(getClass().getResource("test-model-dedicated-input-and-output.json"))
157+
.assemble()
158+
.unwrap();
159+
160+
Node actualNode = ModelSerializer.builder().build().serialize(actualResult);
161+
Node expectedNode = ModelSerializer.builder().build().serialize(expectedResult);
142162

163+
Node.assertEquals(actualNode, expectedNode);
143164
}
144165
}

‎smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/clientendpointdiscovery/test-model-dedicated-input-and-output.json‎

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
{
2+
"smithy": "2.0",
3+
"shapes": {
4+
"ns.foo#FooService": {
5+
"type": "service",
6+
"version": "2019-09-10",
7+
"operations": [
8+
{
9+
"target": "ns.foo#DescribeEndpoints"
10+
},
11+
{
12+
"target": "ns.foo#GetObject"
13+
},
14+
{
15+
"target": "ns.foo#PutObject"
16+
}
17+
],
18+
"traits": {
19+
"aws.api#clientEndpointDiscovery": {
20+
"operation": "ns.foo#DescribeEndpoints",
21+
"error": "ns.foo#InvalidEndpointError"
22+
}
23+
}
24+
},
25+
"ns.foo#BarService": {
26+
"type": "service",
27+
"version": "2019-09-10",
28+
"operations": [
29+
{
30+
"target": "ns.foo#DescribeEndpoints"
31+
}
32+
]
33+
},
34+
"ns.foo#DescribeEndpoints": {
35+
"type": "operation",
36+
"input": {
37+
"target": "ns.foo#DescribeEndpointsRequest"
38+
},
39+
"output": {
40+
"target": "ns.foo#DescribeEndpointsResponse"
41+
}
42+
},
43+
"ns.foo#DescribeEndpointsRequest": {
44+
"type": "structure",
45+
"members": {
46+
"Operation": {
47+
"target": "smithy.api#String"
48+
},
49+
"Identifiers": {
50+
"target": "ns.foo#Identifiers"
51+
}
52+
},
53+
"traits": {
54+
"smithy.api#input": {}
55+
}
56+
},
57+
"ns.foo#Identifiers": {
58+
"type": "map",
59+
"key": {
60+
"target": "smithy.api#String"
61+
},
62+
"value": {
63+
"target": "smithy.api#String"
64+
}
65+
},
66+
"ns.foo#DescribeEndpointsResponse": {
67+
"type": "structure",
68+
"members": {
69+
"Endpoints": {
70+
"target": "ns.foo#Endpoints"
71+
}
72+
},
73+
"traits": {
74+
"smithy.api#output": {}
75+
}
76+
},
77+
"ns.foo#Endpoints": {
78+
"type": "list",
79+
"member": {
80+
"target": "ns.foo#Endpoint"
81+
}
82+
},
83+
"ns.foo#Endpoint": {
84+
"type": "structure",
85+
"members": {
86+
"Address": {
87+
"target": "smithy.api#String"
88+
},
89+
"CachePeriodInMinutes": {
90+
"target": "smithy.api#Long"
91+
}
92+
}
93+
},
94+
"ns.foo#GetObject": {
95+
"type": "operation",
96+
"input": {
97+
"target": "ns.foo#GetObjectRequest"
98+
},
99+
"output": {
100+
"target": "ns.foo#GetObjectResponse"
101+
},
102+
"errors": [
103+
{
104+
"target": "ns.foo#InvalidEndpointError"
105+
}
106+
],
107+
"traits": {
108+
"aws.api#clientDiscoveredEndpoint": {
109+
"required": true
110+
}
111+
}
112+
},
113+
"ns.foo#GetObjectRequest": {
114+
"type": "structure",
115+
"members": {
116+
"Id": {
117+
"target": "smithy.api#String",
118+
"traits": {
119+
"aws.api#clientEndpointDiscoveryId": {},
120+
"smithy.api#required": {}
121+
}
122+
}
123+
},
124+
"traits": {
125+
"smithy.api#input": {}
126+
}
127+
},
128+
"ns.foo#GetObjectResponse": {
129+
"type": "structure",
130+
"members": {
131+
"Object": {
132+
"target": "smithy.api#Blob"
133+
}
134+
},
135+
"traits": {
136+
"smithy.api#output": {}
137+
}
138+
},
139+
"ns.foo#PutObject": {
140+
"type": "operation",
141+
"input": {
142+
"target": "ns.foo#PutObjectRequest"
143+
},
144+
"output": {
145+
"target": "ns.foo#PutObjectResponse"
146+
},
147+
"errors": [
148+
{
149+
"target": "ns.foo#InvalidEndpointError"
150+
}
151+
],
152+
"traits": {
153+
"aws.api#clientDiscoveredEndpoint": {
154+
"required": false
155+
}
156+
}
157+
},
158+
"ns.foo#PutObjectRequest": {
159+
"type": "structure",
160+
"members": {
161+
"Id": {
162+
"target": "smithy.api#String",
163+
"traits": {
164+
"aws.api#clientEndpointDiscoveryId": {},
165+
"smithy.api#required": {}
166+
}
167+
},
168+
"Object": {
169+
"target": "smithy.api#Blob"
170+
}
171+
},
172+
"traits": {
173+
"smithy.api#input": {}
174+
}
175+
},
176+
"ns.foo#PutObjectResponse": {
177+
"type": "structure",
178+
"traits": {
179+
"smithy.api#output": {}
180+
}
181+
},
182+
"ns.foo#InvalidEndpointError": {
183+
"type": "structure",
184+
"traits": {
185+
"smithy.api#error": "client",
186+
"smithy.api#httpError": 421
187+
}
188+
}
189+
}
190+
}

‎smithy-model/src/main/java/software/amazon/smithy/model/transform/CreateDedicatedInputAndOutput.java‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ Model transform(ModelTransformer transformer, Model model) {
5555
}
5656

5757
OperationShape.Builder builder = operation.toBuilder();
58+
boolean removeInput = false;
59+
boolean removeOutput = false;
5860
if (inputChanged) {
5961
LOGGER.fine(() -> String.format("Updating operation input of %s from %s to %s",
6062
operation.getId(),
@@ -65,7 +67,7 @@ Model transform(ModelTransformer transformer, Model model) {
6567
// If the ID changed and the original is no longer referenced, then remove it.
6668
boolean idChanged = !input.getId().equals(updatedInput.getId());
6769
if (idChanged && isSingularReference(reverse, input, operation)) {
68-
toRemove.add(input);
70+
removeInput = true;
6971
LOGGER.fine("Removing now unused input shape " + input.getId());
7072
}
7173
}
@@ -79,18 +81,29 @@ Model transform(ModelTransformer transformer, Model model) {
7981
// If the ID changed and the original is no longer referenced, then remove it.
8082
boolean idChanged = !output.getId().equals(updatedOutput.getId());
8183
if (idChanged && isSingularReference(reverse, output, operation)) {
82-
toRemove.add(output);
84+
removeOutput = true;
8385
LOGGER.fine("Removing now unused output shape " + output.getId());
8486
}
8587
}
88+
// Handle a corner case when the operation uses the same structure for input and output and
89+
// it is named as expected for one of them, e.g., GetFoo with input and output GetFooInput
90+
// In that case we don't want to delete it or the newly introduced input will be deleted as well.
91+
if (removeInput && removeOutput) {
92+
toRemove.add(input);
93+
toRemove.add(output);
94+
} else if (removeInput && !input.getId().equals(updatedOutput.getId())) {
95+
toRemove.add(input);
96+
} else if (removeOutput && !output.getId().equals(updatedInput.getId())) {
97+
toRemove.add(output);
98+
}
8699
updates.add(builder.build());
87100
}
88101

89-
// Remove no longer referenced shapes.
90-
Model result = transformer.removeShapes(model, toRemove);
91-
92102
// Replace the operations and add new shapes.
93-
return transformer.replaceShapes(result, updates);
103+
Model result = transformer.replaceShapes(model, updates);
104+
105+
// Remove no longer referenced shapes.
106+
return transformer.removeShapes(result, toRemove);
94107
}
95108

96109
private StructureShape createdUpdatedInput(

0 commit comments

Comments
 (0)