Skip to content

Commit 7badc6b

Browse files
kuhekuhe
authored
fix(endpoint): fixes to endpoints codegen from user testing (#592)
* fix(endpoint): fixes to endpoints codegen from user testing * feat(endpoint): check clientContextParam required status from endpoint ruleset params * feat(endpoint): exclude contextParams from operation uri
1 parent 911f2af commit 7badc6b

File tree

5 files changed

+122
-56
lines changed

5 files changed

+122
-56
lines changed

‎smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CommandGenerator.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import software.amazon.smithy.model.shapes.MemberShape;
3535
import software.amazon.smithy.model.shapes.OperationShape;
3636
import software.amazon.smithy.model.shapes.ServiceShape;
37+
import software.amazon.smithy.model.shapes.Shape;
3738
import software.amazon.smithy.model.shapes.StructureShape;
3839
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait;
3940
import software.amazon.smithy.typescript.codegen.endpointsV2.EndpointsParamNameMap;
@@ -201,13 +202,14 @@ private void generateEndpointParameterInstructionProvider() {
201202
name, EndpointsParamNameMap.getLocalName(name)
202203
);
203204
});
204-
parameterFinder.getStaticContextParamValues(operation).forEach((name, value) -> {
205+
Shape operationInput = model.getShape(operation.getInputShape()).get();
206+
parameterFinder.getStaticContextParamValues(operationInput).forEach((name, value) -> {
205207
writer.write(
206208
"$L: { type: \"staticContextParams\", value: $L },",
207209
name, value
208210
);
209211
});
210-
parameterFinder.getContextParams(operation).forEach((name, type) -> {
212+
parameterFinder.getContextParams(operationInput).forEach((name, type) -> {
211213
writer.write(
212214
"$L: { type: \"contextParams\", name: \"$L\" },",
213215
name, EndpointsParamNameMap.getLocalName(name)

‎smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,9 @@ private void generateEndpointParameters() {
7272
Map<String, String> clientContextParams =
7373
new RuleSetParameterFinder(service).getClientContextParams();
7474

75-
clientContextParams.forEach((k, v) -> {
76-
writer.write("$L: $L,", k, v);
75+
ObjectNode ruleSet = endpointRuleSetTrait.getRuleSet().expectObjectNode();
76+
ruleSet.getObjectMember("parameters").ifPresent(parameters -> {
77+
parameters.accept(new RuleSetParametersVisitor(writer, clientContextParams));
7778
});
7879
}
7980
);
@@ -115,23 +116,30 @@ private void generateEndpointParameters() {
115116
private void generateEndpointResolver() {
116117
TypeScriptWriter writer = new TypeScriptWriter("");
117118

118-
writer.addImport("EndpointV2", "EndpointV2", "@aws-sdk/types");
119-
writer.addImport("Logger", "Logger", "@aws-sdk/types");
119+
writer.addImport("EndpointV2", null, "@aws-sdk/types");
120+
writer.addImport("Logger", null, "@aws-sdk/types");
120121

121-
writer.addImport("endpointProvider", "endpointProvider", "@aws-sdk/util-endpoints");
122-
writer.addImport("EndpointParameters", "EndpointParameters", "../endpoint/EndpointParameters");
123-
writer.addImport("ruleSet", "ruleSet", "../endpoint/ruleset");
122+
writer.addImport("EndpointParams", null, "@aws-sdk/util-endpoints");
123+
writer.addImport("resolveEndpoint", null, "@aws-sdk/util-endpoints");
124+
writer.addImport("EndpointParameters", null, "../endpoint/EndpointParameters");
125+
writer.addImport("ruleSet", null, "../endpoint/ruleset");
124126

125127
writer.openBlock(
126128
"export const defaultEndpointResolver = ",
127129
"",
128130
() -> {
129131
writer.openBlock(
130-
"(param: EndpointParameters, context: { logger?: Logger } = {}): EndpointV2 => {",
132+
"(endpointParams: EndpointParameters, context: { logger?: Logger } = {}): EndpointV2 => {",
131133
"};",
132134
() -> {
133-
// TODO(endpointsV2) cache
134-
writer.write("return endpointProvider(param, ruleSet, context);");
135+
writer.openBlock(
136+
"return resolveEndpoint(ruleSet, {",
137+
"});",
138+
() -> {
139+
writer.write("endpointParams: endpointParams as EndpointParams,");
140+
writer.write("logger: context.logger,");
141+
}
142+
);
135143
}
136144
);
137145
}
@@ -149,10 +157,10 @@ private void generateEndpointResolver() {
149157
private void generateEndpointRuleset() {
150158
TypeScriptWriter writer = new TypeScriptWriter("");
151159

152-
writer.addImport("RuleSet", "RuleSet", "@aws-sdk/util-endpoints");
160+
writer.addImport("RuleSetObject", null, "@aws-sdk/util-endpoints");
153161

154162
writer.openBlock(
155-
"export const ruleSet: RuleSet = ",
163+
"export const ruleSet: RuleSetObject = ",
156164
"",
157165
() -> {
158166
new RuleSetSerializer(

‎smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParameterFinder.java

Lines changed: 57 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@
2222
import software.amazon.smithy.model.node.NodeVisitor;
2323
import software.amazon.smithy.model.node.ObjectNode;
2424
import software.amazon.smithy.model.node.StringNode;
25-
import software.amazon.smithy.model.shapes.OperationShape;
25+
import software.amazon.smithy.model.shapes.MemberShape;
2626
import software.amazon.smithy.model.shapes.ServiceShape;
27+
import software.amazon.smithy.model.shapes.Shape;
2728
import software.amazon.smithy.rulesengine.traits.ClientContextParamsTrait;
2829
import software.amazon.smithy.rulesengine.traits.ContextParamTrait;
2930
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait;
@@ -65,7 +66,7 @@ public Map<String, String> getClientContextParams() {
6566
clientContextParamsTrait.getParameters().forEach((name, definition) -> {
6667
map.put(
6768
name,
68-
definition.getType().toString() // "boolean" and "string" are directly usable in TS.
69+
definition.getType().toString().toLowerCase() // "boolean" and "string" are directly usable in TS.
6970
);
7071
});
7172
}
@@ -76,46 +77,58 @@ public Map<String, String> getClientContextParams() {
7677
* "The staticContextParams trait defines one or more context parameters that MUST
7778
* be bound to the specified values. This trait MUST target an operation shape."
7879
*/
79-
public Map<String, String> getStaticContextParams(OperationShape operation) {
80+
public Map<String, String> getStaticContextParams(Shape operationInput) {
8081
Map<String, String> map = new HashMap<>();
81-
Optional<StaticContextParamsTrait> trait = operation.getTrait(StaticContextParamsTrait.class);
82-
if (trait.isPresent()) {
83-
StaticContextParamsTrait staticContextParamsTrait = trait.get();
84-
staticContextParamsTrait.getParameters().forEach((name, definition) -> {
85-
map.put(
86-
name,
87-
definition.getValue().getType().toString()
88-
);
82+
83+
if (operationInput.isStructureShape()) {
84+
operationInput.getAllMembers().forEach((String memberName, MemberShape member) -> {
85+
Optional<StaticContextParamsTrait> trait = member.getTrait(StaticContextParamsTrait.class);
86+
if (trait.isPresent()) {
87+
StaticContextParamsTrait staticContextParamsTrait = trait.get();
88+
staticContextParamsTrait.getParameters().forEach((name, definition) -> {
89+
map.put(
90+
name,
91+
definition.getValue().getType().toString()
92+
);
93+
});
94+
}
8995
});
9096
}
97+
9198
return map;
9299
}
93100

94101
/**
95102
* Get map of params to actual values instead of the value type.
96103
*/
97-
public Map<String, String> getStaticContextParamValues(OperationShape operation) {
104+
public Map<String, String> getStaticContextParamValues(Shape operationInput) {
98105
Map<String, String> map = new HashMap<>();
99-
Optional<StaticContextParamsTrait> trait = operation.getTrait(StaticContextParamsTrait.class);
100-
if (trait.isPresent()) {
101-
StaticContextParamsTrait staticContextParamsTrait = trait.get();
102-
staticContextParamsTrait.getParameters().forEach((name, definition) -> {
103-
String value;
104-
if (definition.getValue().isStringNode()) {
105-
value = "`" + definition.getValue().expectStringNode().toString() + "`";
106-
} else if (definition.getValue().isBooleanNode()) {
107-
value = definition.getValue().expectBooleanNode().toString();
108-
} else {
109-
throw new RuntimeException("unexpected type "
110-
+ definition.getValue().getType().toString()
111-
+ " received as staticContextParam.");
106+
107+
if (operationInput.isStructureShape()) {
108+
operationInput.getAllMembers().forEach((String memberName, MemberShape member) -> {
109+
Optional<StaticContextParamsTrait> trait = member.getTrait(StaticContextParamsTrait.class);
110+
if (trait.isPresent()) {
111+
StaticContextParamsTrait staticContextParamsTrait = trait.get();
112+
staticContextParamsTrait.getParameters().forEach((name, definition) -> {
113+
String value;
114+
if (definition.getValue().isStringNode()) {
115+
value = "`" + definition.getValue().expectStringNode().toString() + "`";
116+
} else if (definition.getValue().isBooleanNode()) {
117+
value = definition.getValue().expectBooleanNode().toString();
118+
} else {
119+
throw new RuntimeException("unexpected type "
120+
+ definition.getValue().getType().toString()
121+
+ " received as staticContextParam.");
122+
}
123+
map.put(
124+
name,
125+
value
126+
);
127+
});
112128
}
113-
map.put(
114-
name,
115-
value
116-
);
117129
});
118130
}
131+
119132
return map;
120133
}
121134

@@ -125,17 +138,23 @@ public Map<String, String> getStaticContextParamValues(OperationShape operation)
125138
* The targeted endpoint parameter MUST be a type that is compatible with member’s
126139
* shape targeted by the trait.
127140
*/
128-
public Map<String, String> getContextParams(OperationShape operation) {
141+
public Map<String, String> getContextParams(Shape operationInput) {
129142
Map<String, String> map = new HashMap<>();
130-
Optional<ContextParamTrait> trait = operation.getTrait(ContextParamTrait.class);
131-
if (trait.isPresent()) {
132-
ContextParamTrait staticContextParamsTrait = trait.get();
133-
String name = staticContextParamsTrait.getName();
134-
map.put(
135-
name,
136-
"unknown"
137-
);
143+
144+
if (operationInput.isStructureShape()) {
145+
operationInput.getAllMembers().forEach((String memberName, MemberShape member) -> {
146+
Optional<ContextParamTrait> trait = member.getTrait(ContextParamTrait.class);
147+
if (trait.isPresent()) {
148+
ContextParamTrait contextParamTrait = trait.get();
149+
String name = contextParamTrait.getName();
150+
map.put(
151+
name,
152+
"unknown"
153+
);
154+
}
155+
});
138156
}
157+
139158
return map;
140159
}
141160

‎smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParametersVisitor.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
package software.amazon.smithy.typescript.codegen.endpointsV2;
1717

18+
import java.util.HashMap;
1819
import java.util.Map;
1920
import software.amazon.smithy.model.node.Node;
2021
import software.amazon.smithy.model.node.NodeVisitor;
@@ -25,9 +26,16 @@
2526

2627
public class RuleSetParametersVisitor extends NodeVisitor.Default<Void> {
2728
private final TypeScriptWriter writer;
29+
private final Map<String, String> clientContextParams;
2830

2931
public RuleSetParametersVisitor(TypeScriptWriter writer) {
3032
this.writer = writer;
33+
this.clientContextParams = new HashMap<>();
34+
}
35+
36+
public RuleSetParametersVisitor(TypeScriptWriter writer, Map<String, String> clientContextParams) {
37+
this.writer = writer;
38+
this.clientContextParams = clientContextParams;
3139
}
3240

3341
@Override
@@ -38,7 +46,10 @@ public Void objectNode(ObjectNode node) {
3846
Node param = entry.getValue();
3947

4048
ParameterGenerator parameterGenerator = new ParameterGenerator(key, param);
41-
writer.write(parameterGenerator.toCodeString());
49+
50+
if (clientContextParams.isEmpty() || clientContextParams.containsKey(key)) {
51+
writer.write(parameterGenerator.toCodeString());
52+
}
4253
}
4354
return null;
4455
}

‎smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,13 @@
6666
import software.amazon.smithy.model.traits.MediaTypeTrait;
6767
import software.amazon.smithy.model.traits.StreamingTrait;
6868
import software.amazon.smithy.model.traits.TimestampFormatTrait.Format;
69+
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait;
6970
import software.amazon.smithy.typescript.codegen.ApplicationProtocol;
7071
import software.amazon.smithy.typescript.codegen.CodegenUtils;
7172
import software.amazon.smithy.typescript.codegen.FrameworkErrorModel;
7273
import software.amazon.smithy.typescript.codegen.TypeScriptDependency;
7374
import software.amazon.smithy.typescript.codegen.TypeScriptWriter;
75+
import software.amazon.smithy.typescript.codegen.endpointsV2.RuleSetParameterFinder;
7476
import software.amazon.smithy.utils.ListUtils;
7577
import software.amazon.smithy.utils.OptionalUtils;
7678
import software.amazon.smithy.utils.SetUtils;
@@ -93,8 +95,10 @@ public abstract class HttpBindingProtocolGenerator implements ProtocolGenerator
9395
private final Set<StructureShape> deserializingErrorShapes = new TreeSet<>();
9496
private final boolean isErrorCodeInBody;
9597
private final EventStreamGenerator eventStreamGenerator = new EventStreamGenerator();
96-
9798
private final LinkedHashMap<String, String> headerBuffer = new LinkedHashMap<>();
99+
private final Set<String> contextParamDeduplicationControlSet = SetUtils.of(
100+
"Bucket"
101+
);
98102

99103
/**
100104
* Creates a Http binding protocol generator.
@@ -725,12 +729,34 @@ private void writeResolvedPath(
725729
SymbolProvider symbolProvider = context.getSymbolProvider();
726730
List<HttpBinding> labelBindings = bindingIndex.getRequestBindings(operation, Location.LABEL);
727731

732+
final boolean useEndpointsV2 = context.getService().hasTrait(EndpointRuleSetTrait.class);
733+
final Map<String, String> contextParams = useEndpointsV2
734+
? new RuleSetParameterFinder(context.getService())
735+
.getContextParams(context.getModel().getShape(operation.getInputShape()).get())
736+
: Collections.emptyMap();
737+
728738
// Always write the bound path, but only the actual segments.
729739
writer.write("let resolvedPath = `$L` + $S;",
730740
"${basePath?.endsWith('/') ? basePath.slice(0, -1) : (basePath || '')}",
731741
"/" + trait.getUri().getSegments().stream()
732-
.map(Segment::toString)
733-
.collect(Collectors.joining("/")));
742+
.filter(segment -> {
743+
if (!useEndpointsV2) {
744+
// only applicable in Endpoints 2.0
745+
return true;
746+
}
747+
String content = segment.getContent();
748+
boolean isContextParam = contextParams.containsKey(content);
749+
750+
// If the endpoint also contains the uri segment, e.g. Bucket, we
751+
// do not want to include it in the operation URI to be resolved.
752+
// We use this logic plus a temporary control-list, since it is not yet known
753+
// how many services and param names will have this issue.
754+
755+
return !(isContextParam && contextParamDeduplicationControlSet.contains(content));
756+
})
757+
.map(Segment::toString)
758+
.collect(Collectors.joining("/"))
759+
);
734760

735761
// Handle any label bindings.
736762
if (!labelBindings.isEmpty()) {

0 commit comments

Comments
 (0)