Skip to content

Commit

Permalink
fix(endpoint): fixes to endpoints codegen from user testing (smithy-l…
Browse files Browse the repository at this point in the history 
…ang#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
  • Loading branch information
@kuhe @srchase
kuhe authored and srchase committed Mar 17, 2023
1 parent f6cc359 commit ea065b0
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 56 deletions.
6 changes: 4 additions & 2 deletions ...ipt-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CommandGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.ServiceShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.StructureShape;
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait;
import software.amazon.smithy.typescript.codegen.endpointsV2.EndpointsParamNameMap;
Expand Down Expand Up @@ -201,13 +202,14 @@ private void generateEndpointParameterInstructionProvider() {
name, EndpointsParamNameMap.getLocalName(name)
);
});
parameterFinder.getStaticContextParamValues(operation).forEach((name, value) -> {
Shape operationInput = model.getShape(operation.getInputShape()).get();
parameterFinder.getStaticContextParamValues(operationInput).forEach((name, value) -> {
writer.write(
"$L: { type: \"staticContextParams\", value: $L },",
name, value
);
});
parameterFinder.getContextParams(operation).forEach((name, type) -> {
parameterFinder.getContextParams(operationInput).forEach((name, type) -> {
writer.write(
"$L: { type: \"contextParams\", name: \"$L\" },",
name, EndpointsParamNameMap.getLocalName(name)
Expand Down
32 changes: 20 additions & 12 deletions ...main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ private void generateEndpointParameters() {
Map<String, String> clientContextParams =
new RuleSetParameterFinder(service).getClientContextParams();

clientContextParams.forEach((k, v) -> {
writer.write("$L: $L,", k, v);
ObjectNode ruleSet = endpointRuleSetTrait.getRuleSet().expectObjectNode();
ruleSet.getObjectMember("parameters").ifPresent(parameters -> {
parameters.accept(new RuleSetParametersVisitor(writer, clientContextParams));
});
}
);
Expand Down Expand Up @@ -115,23 +116,30 @@ private void generateEndpointParameters() {
private void generateEndpointResolver() {
TypeScriptWriter writer = new TypeScriptWriter("");

writer.addImport("EndpointV2", "EndpointV2", "@aws-sdk/types");
writer.addImport("Logger", "Logger", "@aws-sdk/types");
writer.addImport("EndpointV2", null, "@aws-sdk/types");
writer.addImport("Logger", null, "@aws-sdk/types");

writer.addImport("endpointProvider", "endpointProvider", "@aws-sdk/util-endpoints");
writer.addImport("EndpointParameters", "EndpointParameters", "../endpoint/EndpointParameters");
writer.addImport("ruleSet", "ruleSet", "../endpoint/ruleset");
writer.addImport("EndpointParams", null, "@aws-sdk/util-endpoints");
writer.addImport("resolveEndpoint", null, "@aws-sdk/util-endpoints");
writer.addImport("EndpointParameters", null, "../endpoint/EndpointParameters");
writer.addImport("ruleSet", null, "../endpoint/ruleset");

writer.openBlock(
"export const defaultEndpointResolver = ",
"",
() -> {
writer.openBlock(
"(param: EndpointParameters, context: { logger?: Logger } = {}): EndpointV2 => {",
"(endpointParams: EndpointParameters, context: { logger?: Logger } = {}): EndpointV2 => {",
"};",
() -> {
// TODO(endpointsV2) cache
writer.write("return endpointProvider(param, ruleSet, context);");
writer.openBlock(
"return resolveEndpoint(ruleSet, {",
"});",
() -> {
writer.write("endpointParams: endpointParams as EndpointParams,");
writer.write("logger: context.logger,");
}
);
}
);
}
Expand All @@ -149,10 +157,10 @@ private void generateEndpointResolver() {
private void generateEndpointRuleset() {
TypeScriptWriter writer = new TypeScriptWriter("");

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

writer.openBlock(
"export const ruleSet: RuleSet = ",
"export const ruleSet: RuleSetObject = ",
"",
() -> {
new RuleSetSerializer(
Expand Down
95 changes: 57 additions & 38 deletions ...in/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParameterFinder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@
import software.amazon.smithy.model.node.NodeVisitor;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.node.StringNode;
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.ServiceShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.rulesengine.traits.ClientContextParamsTrait;
import software.amazon.smithy.rulesengine.traits.ContextParamTrait;
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait;
Expand Down Expand Up @@ -65,7 +66,7 @@ public Map<String, String> getClientContextParams() {
clientContextParamsTrait.getParameters().forEach((name, definition) -> {
map.put(
name,
definition.getType().toString() // "boolean" and "string" are directly usable in TS.
definition.getType().toString().toLowerCase() // "boolean" and "string" are directly usable in TS.
);
});
}
Expand All @@ -76,46 +77,58 @@ public Map<String, String> getClientContextParams() {
* "The staticContextParams trait defines one or more context parameters that MUST
* be bound to the specified values. This trait MUST target an operation shape."
*/
public Map<String, String> getStaticContextParams(OperationShape operation) {
public Map<String, String> getStaticContextParams(Shape operationInput) {
Map<String, String> map = new HashMap<>();
Optional<StaticContextParamsTrait> trait = operation.getTrait(StaticContextParamsTrait.class);
if (trait.isPresent()) {
StaticContextParamsTrait staticContextParamsTrait = trait.get();
staticContextParamsTrait.getParameters().forEach((name, definition) -> {
map.put(
name,
definition.getValue().getType().toString()
);

if (operationInput.isStructureShape()) {
operationInput.getAllMembers().forEach((String memberName, MemberShape member) -> {
Optional<StaticContextParamsTrait> trait = member.getTrait(StaticContextParamsTrait.class);
if (trait.isPresent()) {
StaticContextParamsTrait staticContextParamsTrait = trait.get();
staticContextParamsTrait.getParameters().forEach((name, definition) -> {
map.put(
name,
definition.getValue().getType().toString()
);
});
}
});
}

return map;
}

/**
* Get map of params to actual values instead of the value type.
*/
public Map<String, String> getStaticContextParamValues(OperationShape operation) {
public Map<String, String> getStaticContextParamValues(Shape operationInput) {
Map<String, String> map = new HashMap<>();
Optional<StaticContextParamsTrait> trait = operation.getTrait(StaticContextParamsTrait.class);
if (trait.isPresent()) {
StaticContextParamsTrait staticContextParamsTrait = trait.get();
staticContextParamsTrait.getParameters().forEach((name, definition) -> {
String value;
if (definition.getValue().isStringNode()) {
value = "`" + definition.getValue().expectStringNode().toString() + "`";
} else if (definition.getValue().isBooleanNode()) {
value = definition.getValue().expectBooleanNode().toString();
} else {
throw new RuntimeException("unexpected type "
+ definition.getValue().getType().toString()
+ " received as staticContextParam.");

if (operationInput.isStructureShape()) {
operationInput.getAllMembers().forEach((String memberName, MemberShape member) -> {
Optional<StaticContextParamsTrait> trait = member.getTrait(StaticContextParamsTrait.class);
if (trait.isPresent()) {
StaticContextParamsTrait staticContextParamsTrait = trait.get();
staticContextParamsTrait.getParameters().forEach((name, definition) -> {
String value;
if (definition.getValue().isStringNode()) {
value = "`" + definition.getValue().expectStringNode().toString() + "`";
} else if (definition.getValue().isBooleanNode()) {
value = definition.getValue().expectBooleanNode().toString();
} else {
throw new RuntimeException("unexpected type "
+ definition.getValue().getType().toString()
+ " received as staticContextParam.");
}
map.put(
name,
value
);
});
}
map.put(
name,
value
);
});
}

return map;
}

Expand All @@ -125,17 +138,23 @@ public Map<String, String> getStaticContextParamValues(OperationShape operation)
* The targeted endpoint parameter MUST be a type that is compatible with member’s
* shape targeted by the trait.
*/
public Map<String, String> getContextParams(OperationShape operation) {
public Map<String, String> getContextParams(Shape operationInput) {
Map<String, String> map = new HashMap<>();
Optional<ContextParamTrait> trait = operation.getTrait(ContextParamTrait.class);
if (trait.isPresent()) {
ContextParamTrait staticContextParamsTrait = trait.get();
String name = staticContextParamsTrait.getName();
map.put(
name,
"unknown"
);

if (operationInput.isStructureShape()) {
operationInput.getAllMembers().forEach((String memberName, MemberShape member) -> {
Optional<ContextParamTrait> trait = member.getTrait(ContextParamTrait.class);
if (trait.isPresent()) {
ContextParamTrait contextParamTrait = trait.get();
String name = contextParamTrait.getName();
map.put(
name,
"unknown"
);
}
});
}

return map;
}

Expand Down
13 changes: 12 additions & 1 deletion .../java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParametersVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

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

import java.util.HashMap;
import java.util.Map;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.node.NodeVisitor;
Expand All @@ -25,9 +26,16 @@

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

public RuleSetParametersVisitor(TypeScriptWriter writer) {
this.writer = writer;
this.clientContextParams = new HashMap<>();
}

public RuleSetParametersVisitor(TypeScriptWriter writer, Map<String, String> clientContextParams) {
this.writer = writer;
this.clientContextParams = clientContextParams;
}

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

ParameterGenerator parameterGenerator = new ParameterGenerator(key, param);
writer.write(parameterGenerator.toCodeString());

if (clientContextParams.isEmpty() || clientContextParams.containsKey(key)) {
writer.write(parameterGenerator.toCodeString());
}
}
return null;
}
Expand Down
32 changes: 29 additions & 3 deletions ...a/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,13 @@
import software.amazon.smithy.model.traits.MediaTypeTrait;
import software.amazon.smithy.model.traits.StreamingTrait;
import software.amazon.smithy.model.traits.TimestampFormatTrait.Format;
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait;
import software.amazon.smithy.typescript.codegen.ApplicationProtocol;
import software.amazon.smithy.typescript.codegen.CodegenUtils;
import software.amazon.smithy.typescript.codegen.FrameworkErrorModel;
import software.amazon.smithy.typescript.codegen.TypeScriptDependency;
import software.amazon.smithy.typescript.codegen.TypeScriptWriter;
import software.amazon.smithy.typescript.codegen.endpointsV2.RuleSetParameterFinder;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.OptionalUtils;
import software.amazon.smithy.utils.SetUtils;
Expand All @@ -93,8 +95,10 @@ public abstract class HttpBindingProtocolGenerator implements ProtocolGenerator
private final Set<StructureShape> deserializingErrorShapes = new TreeSet<>();
private final boolean isErrorCodeInBody;
private final EventStreamGenerator eventStreamGenerator = new EventStreamGenerator();

private final LinkedHashMap<String, String> headerBuffer = new LinkedHashMap<>();
private final Set<String> contextParamDeduplicationControlSet = SetUtils.of(
"Bucket"
);

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

final boolean useEndpointsV2 = context.getService().hasTrait(EndpointRuleSetTrait.class);
final Map<String, String> contextParams = useEndpointsV2
? new RuleSetParameterFinder(context.getService())
.getContextParams(context.getModel().getShape(operation.getInputShape()).get())
: Collections.emptyMap();

// Always write the bound path, but only the actual segments.
writer.write("let resolvedPath = `$L` + $S;",
"${basePath?.endsWith('/') ? basePath.slice(0, -1) : (basePath || '')}",
"/" + trait.getUri().getSegments().stream()
.map(Segment::toString)
.collect(Collectors.joining("/")));
.filter(segment -> {
if (!useEndpointsV2) {
// only applicable in Endpoints 2.0
return true;
}
String content = segment.getContent();
boolean isContextParam = contextParams.containsKey(content);

// If the endpoint also contains the uri segment, e.g. Bucket, we
// do not want to include it in the operation URI to be resolved.
// We use this logic plus a temporary control-list, since it is not yet known
// how many services and param names will have this issue.

return !(isContextParam && contextParamDeduplicationControlSet.contains(content));
})
.map(Segment::toString)
.collect(Collectors.joining("/"))
);

// Handle any label bindings.
if (!labelBindings.isEmpty()) {
Expand Down

0 comments on commit ea065b0

Please sign in to comment.