Skip to content

Fix jsonName conversion for OpenAPI examples #2666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,16 @@ abstract Schema createDocumentSchema(
MessageType messageType
);

@Deprecated
Node transformSmithyValueToProtocolValue(Node value) {
return value;
}

/**
* Converts Smithy values in Node form to a data exchange format used by a protocol (e.g., XML).
* Then returns the converted value as a long string (escaping where necessary).
* If data exchange format is JSON (e.g., as in restJson1 protocol),
* method should return values without any modification.
* method should respect the jsonName trait, but otherwise not modify the node.
*
* <p> Used for the value property of OpenAPI example objects.
* For protocols that do not use JSON as data-exchange format,
Expand All @@ -122,10 +127,14 @@ abstract Schema createDocumentSchema(
* E.g., for restXML protocol, values would be converted to a large String of XML value / object,
* escaping where necessary.
*
* @param value value to be converted.
* @param context Conversion context.
* @param shape The shape that the value represents.
* @param value value to be converted.
* @return the long string (escaped where necessary) of values in a data exchange format used by a protocol.
*/
abstract Node transformSmithyValueToProtocolValue(Node value);
Node transformSmithyValueToProtocolValue(Context<T> context, Shape shape, Node value) {
return value;
}

@Override
public Set<String> getProtocolRequestHeaders(Context<T> context, OperationShape operationShape) {
Expand Down Expand Up @@ -204,6 +213,7 @@ private List<ParameterObject> createPathParameters(Context<T> context, Operation
.in("path")
.schema(schema)
.examples(createExamplesForMembersWithHttpTraits(
context,
operation,
binding,
MessageType.REQUEST,
Expand All @@ -219,6 +229,7 @@ private List<ParameterObject> createPathParameters(Context<T> context, Operation
* path parameters, query parameters, header parameters, and payload.
*/
private Map<String, Node> createExamplesForMembersWithHttpTraits(
Context<T> context,
Shape operationOrError,
HttpBinding binding,
MessageType type,
Expand All @@ -229,15 +240,17 @@ private Map<String, Node> createExamplesForMembersWithHttpTraits(
}

if (type == MessageType.ERROR) {
return createErrorExamplesForMembersWithHttpTraits(operationOrError, binding, operation);
return createErrorExamplesForMembersWithHttpTraits(context, operationOrError, binding, operation);
} else {
Map<String, Node> examples = new TreeMap<>();
// unique numbering for unique example names in OpenAPI.
int uniqueNum = 1;

Optional<ExamplesTrait> examplesTrait = operationOrError.getTrait(ExamplesTrait.class);
for (ExamplesTrait.Example example : examplesTrait.map(ExamplesTrait::getExamples)
.orElse(Collections.emptyList())) {
List<ExamplesTrait.Example> modeledExamples = operationOrError.getTrait(ExamplesTrait.class)
.map(ExamplesTrait::getExamples)
.orElse(Collections.emptyList());

for (ExamplesTrait.Example example : modeledExamples) {
ObjectNode inputOrOutput = type == MessageType.REQUEST ? example.getInput()
: example.getOutput().orElse(Node.objectNode());
String name = operationOrError.getId().getName() + "_example" + uniqueNum++;
Expand All @@ -251,7 +264,7 @@ private Map<String, Node> createExamplesForMembersWithHttpTraits(
ExampleObject.builder()
.summary(example.getTitle())
.description(example.getDocumentation().orElse(""))
.value(transformSmithyValueToProtocolValue(values))
.value(transformSmithyValueToProtocolValue(context, binding.getMember(), values))
.build()
.toNode());
}
Expand All @@ -264,6 +277,7 @@ private Map<String, Node> createExamplesForMembersWithHttpTraits(
* Helper method for createExamples() method.
*/
private Map<String, Node> createErrorExamplesForMembersWithHttpTraits(
Context<T> context,
Shape error,
HttpBinding binding,
OperationShape operation
Expand All @@ -290,7 +304,7 @@ private Map<String, Node> createErrorExamplesForMembersWithHttpTraits(
ExampleObject.builder()
.summary(example.getTitle())
.description(example.getDocumentation().orElse(""))
.value(transformSmithyValueToProtocolValue(values))
.value(transformSmithyValueToProtocolValue(context, binding.getMember(), values))
.build()
.toNode());
}
Expand All @@ -302,6 +316,7 @@ private Map<String, Node> createErrorExamplesForMembersWithHttpTraits(
* This method is used for converting the Smithy examples to OpenAPI examples for non-payload HTTP message body.
*/
private Map<String, Node> createBodyExamples(
Context<T> context,
Shape operationOrError,
List<HttpBinding> bindings,
MessageType type,
Expand All @@ -312,7 +327,7 @@ private Map<String, Node> createBodyExamples(
}

if (type == MessageType.ERROR) {
return createErrorBodyExamples(operationOrError, bindings, operation);
return createErrorBodyExamples(context, operationOrError, bindings, operation);
} else {
Map<String, Node> examples = new TreeMap<>();
// unique numbering for unique example names in OpenAPI.
Expand All @@ -321,18 +336,30 @@ private Map<String, Node> createBodyExamples(
Optional<ExamplesTrait> examplesTrait = operationOrError.getTrait(ExamplesTrait.class);
for (ExamplesTrait.Example example : examplesTrait.map(ExamplesTrait::getExamples)
.orElse(Collections.emptyList())) {

Shape structure = operationOrError;
if (operationOrError.isOperationShape()) {
OperationShape op = operationOrError.asOperationShape().get();
if (type == MessageType.REQUEST) {
structure = context.getModel().expectShape(op.getInputShape());
} else {
structure = context.getModel().expectShape(op.getOutputShape());
}
}

// get members included in bindings
ObjectNode values = getMembersWithHttpBindingTrait(bindings,
type == MessageType.REQUEST ? example.getInput()
: example.getOutput().orElse(Node.objectNode()));
String name = operationOrError.getId().getName() + "_example" + uniqueNum++;

// this if condition is needed to avoid errors when converting examples of response.
if (!example.getError().isPresent() || type == MessageType.REQUEST) {
examples.put(name,
ExampleObject.builder()
.summary(example.getTitle())
.description(example.getDocumentation().orElse(""))
.value(transformSmithyValueToProtocolValue(values))
.value(transformSmithyValueToProtocolValue(context, structure, values))
.build()
.toNode());
}
Expand All @@ -342,6 +369,7 @@ private Map<String, Node> createBodyExamples(
}

private Map<String, Node> createErrorBodyExamples(
Context<T> context,
Shape error,
List<HttpBinding> bindings,
OperationShape operation
Expand All @@ -362,7 +390,7 @@ private Map<String, Node> createErrorBodyExamples(
ExampleObject.builder()
.summary(example.getTitle())
.description(example.getDocumentation().orElse(""))
.value(transformSmithyValueToProtocolValue(values))
.value(transformSmithyValueToProtocolValue(context, error, values))
.build()
.toNode());
}
Expand Down Expand Up @@ -456,7 +484,8 @@ private List<ParameterObject> createQueryParameters(Context<T> context, Operatio
}

param.schema(createQuerySchema(context, member, target));
param.examples(createExamplesForMembersWithHttpTraits(operation, binding, MessageType.REQUEST, null));
param.examples(
createExamplesForMembersWithHttpTraits(context, operation, binding, MessageType.REQUEST, null));
result.add(param.build());
}

Expand Down Expand Up @@ -496,7 +525,8 @@ private Map<String, ParameterObject> createHeaderParameters(
param.in(null).name(null);
}

param.examples(createExamplesForMembersWithHttpTraits(operationOrError, binding, messageType, operation));
param.examples(
createExamplesForMembersWithHttpTraits(context, operationOrError, binding, messageType, operation));

// Create the appropriate schema based on the shape type.
Shape target = context.getModel().expectShape(member.getTarget());
Expand Down Expand Up @@ -566,6 +596,7 @@ private Optional<RequestBodyObject> createRequestPayload(
return shapeName + "InputPayload";
}).toBuilder()
.examples(createExamplesForMembersWithHttpTraits(
context,
operation,
binding,
MessageType.REQUEST,
Expand Down Expand Up @@ -598,7 +629,7 @@ private Optional<RequestBodyObject> createRequestDocument(
String pointer = context.putSynthesizedSchema(synthesizedName, schema);
MediaTypeObject mediaTypeObject = MediaTypeObject.builder()
.schema(Schema.builder().ref(pointer).build())
.examples(createBodyExamples(operation, bindings, MessageType.REQUEST, null))
.examples(createBodyExamples(context, operation, bindings, MessageType.REQUEST, null))
.build();

// If any of the top level bindings are required, then the body itself must be required.
Expand Down Expand Up @@ -757,6 +788,7 @@ private void createResponsePayload(
: shapeName + "ErrorPayload";
}).toBuilder()
.examples(createExamplesForMembersWithHttpTraits(
context,
operationOrError,
binding,
type,
Expand Down Expand Up @@ -840,7 +872,7 @@ private void createResponseDocumentIfNeeded(
String pointer = context.putSynthesizedSchema(synthesizedName, schema);
MediaTypeObject mediaTypeObject = MediaTypeObject.builder()
.schema(Schema.builder().ref(pointer).build())
.examples(createBodyExamples(operationOrError, bindings, messageType, operation))
.examples(createBodyExamples(context, operationOrError, bindings, messageType, operation))
.build();

responseBuilder.putContent(mediaType, mediaTypeObject);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ private boolean hasSingleUnionMember(StructureShape shape, Model model) {
}

@Override
Node transformSmithyValueToProtocolValue(Node value) {
return value;
Node transformSmithyValueToProtocolValue(Context<RestJson1Trait> context, Shape shape, Node value) {
return value.accept(new JsonValueNodeTransformer(context, shape));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
package software.amazon.smithy.openapi.fromsmithy.protocols;

import java.util.Map;
import software.amazon.smithy.model.node.ArrayNode;
import software.amazon.smithy.model.node.BooleanNode;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.node.NodeVisitor;
import software.amazon.smithy.model.node.NullNode;
import software.amazon.smithy.model.node.NumberNode;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.node.StringNode;
import software.amazon.smithy.model.shapes.MapShape;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.traits.JsonNameTrait;
import software.amazon.smithy.openapi.fromsmithy.Context;

/**
* Applies the jsonName trait to a node value if applicable.
*/
public class JsonValueNodeTransformer implements NodeVisitor<Node> {
private final Context<?> context;
private final Shape shape;

/**
* Construct a JsonValueNodeTransformer.
*
* @param context Conversion context. Used to determine if jsonName should be used.
* @param shape The shape of the node being converted.
*/
public JsonValueNodeTransformer(Context<?> context, Shape shape) {
this.context = context;
this.shape = shape;
}

@Override
public Node booleanNode(BooleanNode node) {
return node;
}

@Override
public Node nullNode(NullNode node) {
return node;
}

@Override
public Node numberNode(NumberNode node) {
return node;
}

@Override
public Node stringNode(StringNode node) {
return node;
}

@Override
public Node arrayNode(ArrayNode node) {
ArrayNode.Builder resultBuilder = ArrayNode.builder();
Shape listShape = shape.asMemberShape()
.map(m -> context.getModel().expectShape(m.getTarget()))
.orElse(shape);

Shape target = context.getModel().expectShape(listShape.asListShape().get().getMember().getTarget());
JsonValueNodeTransformer elementTransformer = new JsonValueNodeTransformer(context, target);
for (Node element : node.getElements()) {
resultBuilder.withValue(element.accept(elementTransformer));
}
return resultBuilder.build();
}

@Override
public Node objectNode(ObjectNode node) {
Shape actual = shape.asMemberShape()
.map(m -> context.getModel().expectShape(m.getTarget()))
.orElse(shape);

if (shape.isMapShape()) {
return mapNode(actual.asMapShape().get(), node);
}
return structuredNode(actual, node);
}

private Node structuredNode(Shape structure, ObjectNode node) {
ObjectNode.Builder resultBuilder = ObjectNode.builder();
for (Map.Entry<StringNode, Node> entry : node.getMembers().entrySet()) {
String key = entry.getKey().getValue();
if (structure.getMember(key).isPresent()) {
MemberShape member = structure.getMember(key).get();
Shape target = context.getModel().expectShape(member.getTarget());
JsonValueNodeTransformer entryTransformer = new JsonValueNodeTransformer(context, target);
resultBuilder.withMember(getKey(member), entry.getValue().accept(entryTransformer));
} else {
resultBuilder.withMember(key, entry.getValue());
}
}
return resultBuilder.build();
}

private String getKey(MemberShape member) {
if (!context.getJsonSchemaConverter().getConfig().getUseJsonName()) {
return member.getMemberName();
}
return member.getTrait(JsonNameTrait.class)
.map(JsonNameTrait::getValue)
.orElse(member.getMemberName());
}

private Node mapNode(MapShape map, ObjectNode node) {
ObjectNode.Builder resultBuilder = ObjectNode.builder();
Shape target = context.getModel().expectShape(map.getValue().getTarget());
JsonValueNodeTransformer entryTransformer = new JsonValueNodeTransformer(context, target);
for (Map.Entry<StringNode, Node> entry : node.getMembers().entrySet()) {
resultBuilder.withMember(entry.getKey(), entry.getValue().accept(entryTransformer));
}
return resultBuilder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@
"description": "withdrawTestDoc",
"value": {
"location": "Denver",
"bankName": "Chase",
"bank": "Chase",
"atmRecording": "dGVzdHZpZGVv"
}
}
Expand Down Expand Up @@ -470,7 +470,7 @@
"location": {
"type": "string"
},
"bankName": {
"bank": {
"type": "string"
},
"atmRecording": {
Expand Down
Loading
Loading