From f875b3ca229770eaef9ffda8c8b4fc9f9ad2929d Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Mon, 29 Apr 2024 10:25:54 -0700 Subject: [PATCH 1/8] Add operation context params trait + validator --- smithy-rules-engine/build.gradle | 1 + .../rulesengine/traits/ContextIndex.java | 11 + .../OperationContextParamDefinition.java | 69 ++++ .../traits/OperationContextParamsTrait.java | 110 ++++++ .../OperationContextParamsTraitValidator.java | 313 ++++++++++++++++++ ...re.amazon.smithy.model.traits.TraitService | 1 + ...e.amazon.smithy.model.validation.Validator | 1 + .../META-INF/smithy/smithy.rules.smithy | 21 ++ 8 files changed, 527 insertions(+) create mode 100644 smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamDefinition.java create mode 100644 smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTrait.java create mode 100644 smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java diff --git a/smithy-rules-engine/build.gradle b/smithy-rules-engine/build.gradle index ea5067f6441..2b8df453d97 100644 --- a/smithy-rules-engine/build.gradle +++ b/smithy-rules-engine/build.gradle @@ -13,4 +13,5 @@ ext { dependencies { api project(":smithy-model") api project(":smithy-utils") + api project(":smithy-jmespath") } diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/ContextIndex.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/ContextIndex.java index 4ddddcdc370..9f55985fc65 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/ContextIndex.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/ContextIndex.java @@ -56,6 +56,17 @@ public Optional getStaticContextParams(Shape operation return operation.getTrait(StaticContextParamsTrait.class); } + /** + * Gets the operation context parameter names and their {@link OperationContextParamDefinition} for the given + * operation. + * + * @param operation The operation shape. + * @return The mapping of context parameter names to the JMESPath expression to bind to input elements. + */ + public Optional getOperationContextParams(Shape operation) { + return operation.getTrait(OperationContextParamsTrait.class); + } + /** * Gets the mapping of {@link MemberShape} to {@link ContextParamTrait} for the operation. * diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamDefinition.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamDefinition.java new file mode 100644 index 00000000000..8c99ed5992b --- /dev/null +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamDefinition.java @@ -0,0 +1,69 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rulesengine.traits; + +import java.util.Objects; +import software.amazon.smithy.utils.SmithyBuilder; +import software.amazon.smithy.utils.SmithyUnstableApi; +import software.amazon.smithy.utils.ToSmithyBuilder; + +/** + * An operation context parameter definition. + */ +@SmithyUnstableApi +public final class OperationContextParamDefinition implements ToSmithyBuilder { + private final String path; + + private OperationContextParamDefinition(Builder builder) { + this.path = SmithyBuilder.requiredState("path", builder.path); + } + + public static Builder builder() { + return new Builder(); + } + + public String getPath() { + return path; + } + + @Override + public Builder toBuilder() { + return builder().path(path); + } + + @Override + public int hashCode() { + return Objects.hash(getPath()); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + OperationContextParamDefinition that = (OperationContextParamDefinition) o; + return getPath().equals(that.getPath()); + } + + public static final class Builder implements SmithyBuilder { + private String path; + + private Builder() { + } + + public Builder path(String path) { + this.path = path; + return this; + } + + public OperationContextParamDefinition build() { + return new OperationContextParamDefinition(this); + } + } +} diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTrait.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTrait.java new file mode 100644 index 00000000000..5bf6d68a599 --- /dev/null +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTrait.java @@ -0,0 +1,110 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rulesengine.traits; + +import java.util.LinkedHashMap; +import java.util.Map; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.node.NodeMapper; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.traits.AbstractTrait; +import software.amazon.smithy.model.traits.AbstractTraitBuilder; +import software.amazon.smithy.model.traits.Trait; +import software.amazon.smithy.utils.BuilderRef; +import software.amazon.smithy.utils.SmithyUnstableApi; +import software.amazon.smithy.utils.ToSmithyBuilder; + +/** + * Binds elements of a target operation's input to rule-set parameters. + */ +@SmithyUnstableApi +public final class OperationContextParamsTrait extends AbstractTrait + implements ToSmithyBuilder { + public static final ShapeId ID = ShapeId.from("smithy.rules#operationContextParams"); + + private final Map parameters; + + private OperationContextParamsTrait(Builder builder) { + super(ID, builder.getSourceLocation()); + this.parameters = builder.parameters.copy(); + } + + public static Builder builder() { + return new Builder(); + } + + public Map getParameters() { + return parameters; + } + + @Override + protected Node createNode() { + NodeMapper mapper = new NodeMapper(); + mapper.setOmitEmptyValues(true); + return mapper.serialize(getParameters()).expectObjectNode(); + } + + @Override + public Builder toBuilder() { + return new Builder() + .sourceLocation(getSourceLocation()) + .parameters(parameters); + } + + public static final class Provider extends AbstractTrait.Provider { + public Provider() { + super(ID); + } + + @Override + public Trait createTrait(ShapeId target, Node value) { + NodeMapper mapper = new NodeMapper(); + Map parameters = new LinkedHashMap<>(); + value.expectObjectNode().getMembers().forEach((stringNode, node) -> { + parameters.put(stringNode.getValue(), mapper.deserialize(node, OperationContextParamDefinition.class)); + }); + OperationContextParamsTrait trait = builder() + .sourceLocation(value) + .parameters(parameters) + .build(); + trait.setNodeCache(value); + return trait; + } + } + + public static final class Builder extends AbstractTraitBuilder { + private final BuilderRef> parameters = BuilderRef.forOrderedMap(); + + private Builder() { + } + + public Builder parameters(Map parameters) { + this.parameters.clear(); + this.parameters.get().putAll(parameters); + return this; + } + + public Builder putParameter(String name, OperationContextParamDefinition definition) { + this.parameters.get().put(name, definition); + return this; + } + + public Builder removeParameter(String name) { + this.parameters.get().remove(name); + return this; + } + + public Builder clearParameters() { + this.parameters.clear(); + return this; + } + + @Override + public OperationContextParamsTrait build() { + return new OperationContextParamsTrait(this); + } + } +} diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java new file mode 100644 index 00000000000..cbac0d07b68 --- /dev/null +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java @@ -0,0 +1,313 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rulesengine.traits; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import software.amazon.smithy.jmespath.JmespathException; +import software.amazon.smithy.jmespath.JmespathExpression; +import software.amazon.smithy.jmespath.LinterResult; +import software.amazon.smithy.jmespath.ast.LiteralExpression; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.OperationIndex; +import software.amazon.smithy.model.shapes.BigDecimalShape; +import software.amazon.smithy.model.shapes.BigIntegerShape; +import software.amazon.smithy.model.shapes.BlobShape; +import software.amazon.smithy.model.shapes.BooleanShape; +import software.amazon.smithy.model.shapes.ByteShape; +import software.amazon.smithy.model.shapes.DocumentShape; +import software.amazon.smithy.model.shapes.DoubleShape; +import software.amazon.smithy.model.shapes.FloatShape; +import software.amazon.smithy.model.shapes.IntegerShape; +import software.amazon.smithy.model.shapes.ListShape; +import software.amazon.smithy.model.shapes.LongShape; +import software.amazon.smithy.model.shapes.MapShape; +import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.ResourceShape; +import software.amazon.smithy.model.shapes.ServiceShape; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.shapes.ShapeVisitor; +import software.amazon.smithy.model.shapes.ShortShape; +import software.amazon.smithy.model.shapes.StringShape; +import software.amazon.smithy.model.shapes.StructureShape; +import software.amazon.smithy.model.shapes.TimestampShape; +import software.amazon.smithy.model.shapes.UnionShape; +import software.amazon.smithy.model.traits.LengthTrait; +import software.amazon.smithy.model.traits.RangeTrait; +import software.amazon.smithy.model.validation.AbstractValidator; +import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.utils.SmithyUnstableApi; + +/** + * Validates {@link OperationContextParamsTrait} traits. + */ +@SmithyUnstableApi +public final class OperationContextParamsTraitValidator extends AbstractValidator { + @Override + public List validate(Model model) { + ContextIndex index = ContextIndex.of(model); + List events = new ArrayList<>(); + for (OperationShape operationShape : model.getOperationShapes()) { + Map definitionMap = index.getOperationContextParams(operationShape) + .map(OperationContextParamsTrait::getParameters) + .orElse(Collections.emptyMap()); + for (Map.Entry entry : definitionMap.entrySet()) { + + try { + JmespathExpression path = JmespathExpression.parse(entry.getValue().getPath()); + StructureShape input = OperationIndex.of(model).expectInputShape(operationShape); + LinterResult linterResult = path.lint(createCurrentNodeFromShape(input, model)); + + if (!linterResult.getProblems().isEmpty()) { + events.add(error(operationShape, + String.format("The operation `%s` is marked with `%s` which contains a " + + "key `%s` with an invalid JMESPath path `%s`: %s.", + operationShape.getId(), + OperationContextParamsTrait.ID.toString(), + entry.getKey(), + entry.getValue().getPath(), + String.join(", ", + linterResult.getProblems().stream() + .map(p -> "'" + p.toString() + "'") + .collect(Collectors.toList())) + ))); + } + } catch (JmespathException e) { + events.add(error(operationShape, + String.format("The operation `%s` is marked with `%s` which contains a " + + "key `%s` with an unparseable JMESPath path `%s`: %s.", + operationShape.getId(), + OperationContextParamsTrait.ID.toString(), + entry.getKey(), + entry.getValue().getPath(), + e.getMessage() + ))); + } + } + } + return events; + } + + private LiteralExpression createCurrentNodeFromShape(Shape shape, Model model) { + return shape == null + ? LiteralExpression.ANY + : new LiteralExpression(shape.accept(new ModelRuntimeTypeGenerator(model))); + } + + private static final class ModelRuntimeTypeGenerator implements ShapeVisitor { + + private final Model model; + private Set visited = new HashSet<>(); + + ModelRuntimeTypeGenerator(Model model) { + this.model = model; + } + + @Override + public Object blobShape(BlobShape shape) { + return "blob"; + } + + @Override + public Object booleanShape(BooleanShape shape) { + return true; + } + + @Override + public Object byteShape(ByteShape shape) { + return computeRange(shape); + } + + @Override + public Object shortShape(ShortShape shape) { + return computeRange(shape); + } + + @Override + public Object integerShape(IntegerShape shape) { + return computeRange(shape); + } + + @Override + public Object longShape(LongShape shape) { + return computeRange(shape); + } + + @Override + public Object floatShape(FloatShape shape) { + return computeRange(shape); + } + + @Override + public Object doubleShape(DoubleShape shape) { + return computeRange(shape); + } + + @Override + public Object bigIntegerShape(BigIntegerShape shape) { + return computeRange(shape); + } + + @Override + public Object bigDecimalShape(BigDecimalShape shape) { + return computeRange(shape); + } + + @Override + public Object documentShape(DocumentShape shape) { + return LiteralExpression.ANY; + } + + @Override + public Object stringShape(StringShape shape) { + // Create a random string that does not exceed or go under the length trait. + int chars = computeLength(shape); + + // Fill a string with "a"'s up to chars. + return new String(new char[chars]).replace("\0", "a"); + } + + @Override + public Object listShape(ListShape shape) { + return withCopiedVisitors(() -> { + int size = computeLength(shape); + List result = new ArrayList<>(size); + Object memberValue = shape.getMember().accept(this); + if (memberValue != null) { + for (int i = 0; i < size; i++) { + result.add(memberValue); + } + } + return result; + }); + } + + // Visits members and mutates a copy of the current set of visited + // shapes rather than a shared set. This allows a shape to be used + // multiple times in the closure of a single shape without causing the + // reuse of the shape to always be assumed to be a recursive type. + private Object withCopiedVisitors(Supplier supplier) { + // Account for recursive shapes at the current + Set visitedCopy = new HashSet<>(visited); + Object result = supplier.get(); + visited = visitedCopy; + return result; + } + + @Override + public Object mapShape(MapShape shape) { + return withCopiedVisitors(() -> { + int size = computeLength(shape); + Map result = new HashMap<>(); + String key = (String) shape.getKey().accept(this); + Object memberValue = shape.getValue().accept(this); + for (int i = 0; i < size; i++) { + result.put(key + i, memberValue); + } + return result; + }); + } + + @Override + public Object structureShape(StructureShape shape) { + return structureOrUnion(shape); + } + + @Override + public Object unionShape(UnionShape shape) { + return structureOrUnion(shape); + } + + private Object structureOrUnion(Shape shape) { + return withCopiedVisitors(() -> { + Map result = new LinkedHashMap<>(); + for (MemberShape member : shape.members()) { + Object memberValue = member.accept(this); + result.put(member.getMemberName(), memberValue); + } + return result; + }); + } + + @Override + public Object memberShape(MemberShape shape) { + // Account for recursive shapes. + // A false return value means it was in the set. + if (!visited.add(shape)) { + return LiteralExpression.ANY; + } + + return model.getShape(shape.getTarget()) + .map(target -> target.accept(this)) + // Rather than fail on broken models during waiter validation, + // return an ANY to get *some* validation. + .orElse(LiteralExpression.ANY); + } + + @Override + public Object timestampShape(TimestampShape shape) { + return LiteralExpression.NUMBER; + } + + @Override + public Object operationShape(OperationShape shape) { + throw new UnsupportedOperationException(shape.toString()); + } + + @Override + public Object resourceShape(ResourceShape shape) { + throw new UnsupportedOperationException(shape.toString()); + } + + @Override + public Object serviceShape(ServiceShape shape) { + throw new UnsupportedOperationException(shape.toString()); + } + + private int computeLength(Shape shape) { + // Create a random string that does not exceed or go under the length trait. + int chars = 2; + + if (shape.hasTrait(LengthTrait.class)) { + LengthTrait trait = shape.expectTrait(LengthTrait.class); + if (trait.getMin().isPresent()) { + chars = Math.max(chars, trait.getMin().get().intValue()); + } + if (trait.getMax().isPresent()) { + chars = Math.min(chars, trait.getMax().get().intValue()); + } + } + + return chars; + } + + private double computeRange(Shape shape) { + // Create a random string that does not exceed or go under the range trait. + double i = 8; + + if (shape.hasTrait(RangeTrait.class)) { + RangeTrait trait = shape.expectTrait(RangeTrait.class); + if (trait.getMin().isPresent()) { + i = Math.max(i, trait.getMin().get().doubleValue()); + } + if (trait.getMax().isPresent()) { + i = Math.min(i, trait.getMax().get().doubleValue()); + } + } + + return i; + } + } +} diff --git a/smithy-rules-engine/src/main/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService b/smithy-rules-engine/src/main/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService index ab79eaaa5e5..353f3b95e8e 100644 --- a/smithy-rules-engine/src/main/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService +++ b/smithy-rules-engine/src/main/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService @@ -1,5 +1,6 @@ software.amazon.smithy.rulesengine.traits.ClientContextParamsTrait$Provider software.amazon.smithy.rulesengine.traits.ContextParamTrait$Provider software.amazon.smithy.rulesengine.traits.StaticContextParamsTrait$Provider +software.amazon.smithy.rulesengine.traits.OperationContextParamsTrait$Provider software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait$Provider software.amazon.smithy.rulesengine.traits.EndpointTestsTrait$Provider diff --git a/smithy-rules-engine/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator b/smithy-rules-engine/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator index af9a50287f4..2f24c94156b 100644 --- a/smithy-rules-engine/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator +++ b/smithy-rules-engine/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator @@ -1,5 +1,6 @@ software.amazon.smithy.rulesengine.traits.EndpointTestsTraitValidator software.amazon.smithy.rulesengine.traits.StaticContextParamsTraitValidator +software.amazon.smithy.rulesengine.traits.OperationContextParamsTraitValidator software.amazon.smithy.rulesengine.validators.RuleSetAuthSchemesValidator software.amazon.smithy.rulesengine.validators.RuleSetBuiltInValidator software.amazon.smithy.rulesengine.validators.RuleSetUriValidator diff --git a/smithy-rules-engine/src/main/resources/META-INF/smithy/smithy.rules.smithy b/smithy-rules-engine/src/main/resources/META-INF/smithy/smithy.rules.smithy index de26a37bb85..2fadb68a80b 100644 --- a/smithy-rules-engine/src/main/resources/META-INF/smithy/smithy.rules.smithy +++ b/smithy-rules-engine/src/main/resources/META-INF/smithy/smithy.rules.smithy @@ -41,6 +41,18 @@ map staticContextParams { value: StaticContextParamDefinition } +/// Binds one or more named rule-set parameters to elements contained in the operation's input structure. +/// The type of the shapes targeted by the trait MUST match the parameter types defined in the rule-set. +@unstable +@trait(selector: "operation") +map operationContextParams { + /// The rule-set parameter name. + key: String, + + /// The static parameter definition. + value: OperationContextParamDefinition +} + /// Defines one or more named rule-set parameters to be generated as configurable client parameters. /// The type specified for the client parameter MUST match the parameter type defined in the rule-set. @unstable @@ -62,6 +74,15 @@ structure StaticContextParamDefinition { value: Document } +/// An operation context parameter definition. +@unstable +@private +structure OperationContextParamDefinition { + /// a JMESPath expression to select the input element to bind to. + @required + path: String +} + /// A client context parameter definition. @unstable @private From 80ed962f92bb2ced17399447416284a4b3b2006d Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Mon, 29 Apr 2024 15:04:43 -0700 Subject: [PATCH 2/8] Add documentation --- .../rules-engine/parameters.rst | 76 +++++++++++++++++++ .../META-INF/smithy/smithy.rules.smithy | 2 +- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/docs/source-2.0/additional-specs/rules-engine/parameters.rst b/docs/source-2.0/additional-specs/rules-engine/parameters.rst index 3385dc5f201..f7743ed2524 100644 --- a/docs/source-2.0/additional-specs/rules-engine/parameters.rst +++ b/docs/source-2.0/additional-specs/rules-engine/parameters.rst @@ -94,6 +94,7 @@ value must be selected and provided to the implementation. The following is the order of the most specific to least specific value locations: #. `smithy.rules#staticContextParams trait`_ +#. `smithy.rules#operationContextParams trait`_ #. `smithy.rules#contextParam trait`_ #. `smithy.rules#clientContextParams trait`_ #. Built-in bindings @@ -240,6 +241,77 @@ operation: operation GetThing {} +.. smithy-trait:: smithy.rules#operationContextParams +.. _smithy.rules#operationContextParams-trait: + +``smithy.rules#operationContextParams`` trait +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Summary + Defines one or more rule set parameters that MUST be bound to values + specified in the operation input. +Trait selector + ``operation`` +Value type + ``map`` of ``string`` containing a rule set parameter name to a + ``operationContextParam`` structure. + +The ``operationContextParam`` structure has the following properties: + +.. list-table:: + :header-rows: 1 + :widths: 10 23 67 + + * - Property + - Type + - Description + * - path + - ``string`` + - **Required**. A JMESPath expression to select element(s) from the operation input to bind to. + +Each parameter is identified using it’s name as specified in the rule set. The +type of a ``operationContextParam`` MUST be compatible with the parameter type +specified in the rule set. + +The following example specifies a parameter bound to an array of object keys in the +operation input using a JMESPath expression: + +.. code-block:: smithy + + @operationContextParams( + ObjectKeys: { + path: "Delete.Objects[*].Key" + } + ) + operation DeleteObjects { + input: DeleteObjectsRequest + } + + structure DeleteObjectsRequest { + Delete: Delete + } + + structure Delete { + Objects: ObjectIdentifierList + } + + list ObjectIdentifierList { + member: ObjectIdentifier + } + + structure ObjectIdentifier { + Key: String + } + +`paths` specified in :ref:`OperationContextParams ` are limited +to a subset of JMESPath: + +* `Identifiers`_ - the most basic expression and can be used to extract a single element from a JSON document. The return value for an identifier is the value associated with the identifier. If the identifier does not exist in the JSON document, than a null value is returned. +* `Sub Expressions`_ - a combination of two expressions separated by the `.` char. Example: `grandparent.parent.child` +* `Wildcard Expressions`_ - Creates a projection over the values in an array or map. Remaining expressions are evaluated against each returned element. +* `Keys function`_ - return a list of the keys in a map. This is the only supported function but is required for binding to key values. + + .. smithy-trait:: smithy.rules#contextParam .. _smithy.rules#contextParam-trait: @@ -343,3 +415,7 @@ The rules engine is highly extensible through .. _Javadocs: https://smithy.io/javadoc/__smithy_version__/software/amazon/smithy/rulesengine/language/EndpointRuleSetExtension.html .. _service providers: https://docs.oracle.com/javase/tutorial/sound/SPI-intro.html +.. _Identifiers: https://jmespath.org/specification.html#identifiers +.. _Sub expressions: https://jmespath.org/specification.html#subexpressions +.. _Wildcard expressions: https://jmespath.org/specification.html#wildcard-expressions +.. _Keys function: https://jmespath.org/specification.html#keys \ No newline at end of file diff --git a/smithy-rules-engine/src/main/resources/META-INF/smithy/smithy.rules.smithy b/smithy-rules-engine/src/main/resources/META-INF/smithy/smithy.rules.smithy index 2fadb68a80b..9555434d36a 100644 --- a/smithy-rules-engine/src/main/resources/META-INF/smithy/smithy.rules.smithy +++ b/smithy-rules-engine/src/main/resources/META-INF/smithy/smithy.rules.smithy @@ -78,7 +78,7 @@ structure StaticContextParamDefinition { @unstable @private structure OperationContextParamDefinition { - /// a JMESPath expression to select the input element to bind to. + /// a JMESPath expression to select element(s) from the operation input to bind to. @required path: String } From 7875b365ba7caedf648a58a48a26efa4818a496e Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Mon, 29 Apr 2024 15:31:17 -0700 Subject: [PATCH 3/8] Validate supported jmespath in operation context params paths --- .../OperationContextParamsTraitValidator.java | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java index cbac0d07b68..915b0dae9aa 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java @@ -15,10 +15,28 @@ import java.util.Set; import java.util.function.Supplier; import java.util.stream.Collectors; +import software.amazon.smithy.jmespath.ExpressionVisitor; import software.amazon.smithy.jmespath.JmespathException; import software.amazon.smithy.jmespath.JmespathExpression; import software.amazon.smithy.jmespath.LinterResult; +import software.amazon.smithy.jmespath.ast.AndExpression; +import software.amazon.smithy.jmespath.ast.ComparatorExpression; +import software.amazon.smithy.jmespath.ast.CurrentExpression; +import software.amazon.smithy.jmespath.ast.ExpressionTypeExpression; +import software.amazon.smithy.jmespath.ast.FieldExpression; +import software.amazon.smithy.jmespath.ast.FilterProjectionExpression; +import software.amazon.smithy.jmespath.ast.FlattenExpression; +import software.amazon.smithy.jmespath.ast.FunctionExpression; +import software.amazon.smithy.jmespath.ast.IndexExpression; import software.amazon.smithy.jmespath.ast.LiteralExpression; +import software.amazon.smithy.jmespath.ast.MultiSelectHashExpression; +import software.amazon.smithy.jmespath.ast.MultiSelectListExpression; +import software.amazon.smithy.jmespath.ast.NotExpression; +import software.amazon.smithy.jmespath.ast.ObjectProjectionExpression; +import software.amazon.smithy.jmespath.ast.OrExpression; +import software.amazon.smithy.jmespath.ast.ProjectionExpression; +import software.amazon.smithy.jmespath.ast.SliceExpression; +import software.amazon.smithy.jmespath.ast.Subexpression; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.knowledge.OperationIndex; import software.amazon.smithy.model.shapes.BigDecimalShape; @@ -48,6 +66,7 @@ import software.amazon.smithy.model.traits.RangeTrait; import software.amazon.smithy.model.validation.AbstractValidator; import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.utils.ListUtils; import software.amazon.smithy.utils.SmithyUnstableApi; /** @@ -84,6 +103,23 @@ public List validate(Model model) { .collect(Collectors.toList())) ))); } + + List unsupportedExpressions = path.accept(new UnsupportedJmesPathVisitor()); + if (!unsupportedExpressions.isEmpty()) { + events.add(error(operationShape, + String.format("The operation `%s` is marked with `%s` which contains a " + + "key `%s` with a JMESPath path `%s` with " + + "unsupported expressions: %s.", + operationShape.getId(), + OperationContextParamsTrait.ID.toString(), + entry.getKey(), + entry.getValue().getPath(), + String.join(", ", + unsupportedExpressions.stream() + .map(e -> "'" + e + "'") + .collect(Collectors.toList())) + ))); + } } catch (JmespathException e) { events.add(error(operationShape, String.format("The operation `%s` is marked with `%s` which contains a " @@ -310,4 +346,107 @@ private double computeRange(Shape shape) { return i; } } + + private static final class UnsupportedJmesPathVisitor implements ExpressionVisitor> { + + @Override + public List visitComparator(ComparatorExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitCurrentNode(CurrentExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitExpressionType(ExpressionTypeExpression expression) { + return expression.getExpression().accept(this); + } + + @Override + public List visitFlatten(FlattenExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitFunction(FunctionExpression expression) { + if (expression.getName().equals("keys")) { + return Collections.emptyList(); + } else { + return ListUtils.of(expression.toString()); + } + } + + @Override + public List visitField(FieldExpression expression) { + return Collections.emptyList(); + } + + @Override + public List visitIndex(IndexExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitLiteral(LiteralExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitMultiSelectList(MultiSelectListExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitMultiSelectHash(MultiSelectHashExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitAnd(AndExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitOr(OrExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitNot(NotExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitProjection(ProjectionExpression expression) { + return null; + } + + @Override + public List visitFilterProjection(FilterProjectionExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitObjectProjection(ObjectProjectionExpression expression) { + List unsupported = new ArrayList<>(); + unsupported.addAll(expression.getLeft().accept(this)); + unsupported.addAll(expression.getRight().accept(this)); + return Collections.unmodifiableList(unsupported); + } + + @Override + public List visitSlice(SliceExpression expression) { + return ListUtils.of(expression.toString()); + } + + @Override + public List visitSubexpression(Subexpression expression) { + List unsupported = new ArrayList<>(); + unsupported.addAll(expression.getLeft().accept(this)); + unsupported.addAll(expression.getRight().accept(this)); + return Collections.unmodifiableList(unsupported); + } + } } From 98e53efeffeda4da99bd5353b2a0eb5fc33d87e3 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Mon, 29 Apr 2024 16:23:34 -0700 Subject: [PATCH 4/8] Fix docs --- .../additional-specs/rules-engine/parameters.rst | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/source-2.0/additional-specs/rules-engine/parameters.rst b/docs/source-2.0/additional-specs/rules-engine/parameters.rst index f7743ed2524..b2dc6910a24 100644 --- a/docs/source-2.0/additional-specs/rules-engine/parameters.rst +++ b/docs/source-2.0/additional-specs/rules-engine/parameters.rst @@ -94,8 +94,8 @@ value must be selected and provided to the implementation. The following is the order of the most specific to least specific value locations: #. `smithy.rules#staticContextParams trait`_ -#. `smithy.rules#operationContextParams trait`_ #. `smithy.rules#contextParam trait`_ +#. `smithy.rules#operationContextParams trait`_ #. `smithy.rules#clientContextParams trait`_ #. Built-in bindings #. Built-in binding default values @@ -306,10 +306,15 @@ operation input using a JMESPath expression: `paths` specified in :ref:`OperationContextParams ` are limited to a subset of JMESPath: -* `Identifiers`_ - the most basic expression and can be used to extract a single element from a JSON document. The return value for an identifier is the value associated with the identifier. If the identifier does not exist in the JSON document, than a null value is returned. -* `Sub Expressions`_ - a combination of two expressions separated by the `.` char. Example: `grandparent.parent.child` -* `Wildcard Expressions`_ - Creates a projection over the values in an array or map. Remaining expressions are evaluated against each returned element. -* `Keys function`_ - return a list of the keys in a map. This is the only supported function but is required for binding to key values. +* `Identifiers`_ - the most basic expression and can be used to extract a single element from a JSON document. + The return value for an identifier is the value associated with the identifier. If the identifier does not + exist in the JSON document, than a null value is returned. +* `Sub Expressions`_ - a combination of two expressions separated by the ``.`` char. + Example: ``grandparent.parent.child`` +* `Wildcard Expressions`_ - Creates a projection over the values in an array or map. + Remaining expressions are evaluated against each returned element. +* `Keys function`_ - return a list of the keys in a map. This is the only supported function but is required + for binding to key values. .. smithy-trait:: smithy.rules#contextParam From a34a5f85bfc61fb164a497b7634de3eea02f2a02 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Tue, 30 Apr 2024 09:27:58 -0700 Subject: [PATCH 5/8] Add tests --- .../OperationContextParamsTraitValidator.java | 37 ++++++++-------- .../invalid-operation-context-params.errors | 7 ++++ .../invalid-operation-context-params.smithy | 35 ++++++++++++++++ .../valid/operation-context-params.errors | 1 + .../valid/operation-context-params.smithy | 42 +++++++++++++++++++ 5 files changed, 105 insertions(+), 17 deletions(-) create mode 100644 smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.errors create mode 100644 smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy create mode 100644 smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.errors create mode 100644 smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java index 915b0dae9aa..675cf98e873 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java @@ -92,14 +92,14 @@ public List validate(Model model) { if (!linterResult.getProblems().isEmpty()) { events.add(error(operationShape, String.format("The operation `%s` is marked with `%s` which contains a " - + "key `%s` with an invalid JMESPath path `%s`: %s.", + + "path `%s` with an invalid JMESPath path `%s`: %s.", operationShape.getId(), OperationContextParamsTrait.ID.toString(), entry.getKey(), entry.getValue().getPath(), String.join(", ", linterResult.getProblems().stream() - .map(p -> "'" + p.toString() + "'") + .map(p -> "'" + p.message + "'") .collect(Collectors.toList())) ))); } @@ -351,12 +351,12 @@ private static final class UnsupportedJmesPathVisitor implements ExpressionVisit @Override public List visitComparator(ComparatorExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("comparator"); } @Override public List visitCurrentNode(CurrentExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("current node"); } @Override @@ -366,15 +366,18 @@ public List visitExpressionType(ExpressionTypeExpression expression) { @Override public List visitFlatten(FlattenExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("flatten"); } @Override public List visitFunction(FunctionExpression expression) { if (expression.getName().equals("keys")) { - return Collections.emptyList(); + return expression.getArguments().stream() + .map(e -> e.accept(this)) + .flatMap(List::stream) + .collect(Collectors.toList()); } else { - return ListUtils.of(expression.toString()); + return ListUtils.of("`" + expression.getName() + "` function"); } } @@ -385,47 +388,47 @@ public List visitField(FieldExpression expression) { @Override public List visitIndex(IndexExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("index"); } @Override public List visitLiteral(LiteralExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("literal"); } @Override public List visitMultiSelectList(MultiSelectListExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("multiselect list"); } @Override public List visitMultiSelectHash(MultiSelectHashExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("multiselect hash"); } @Override public List visitAnd(AndExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("and"); } @Override public List visitOr(OrExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("or"); } @Override public List visitNot(NotExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("not"); } @Override public List visitProjection(ProjectionExpression expression) { - return null; + return Collections.emptyList(); } @Override public List visitFilterProjection(FilterProjectionExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("filter projection"); } @Override @@ -438,7 +441,7 @@ public List visitObjectProjection(ObjectProjectionExpression expression) @Override public List visitSlice(SliceExpression expression) { - return ListUtils.of(expression.toString()); + return ListUtils.of("slice"); } @Override diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.errors b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.errors new file mode 100644 index 00000000000..65f61bb3905 --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.errors @@ -0,0 +1,7 @@ +[WARNING] example#Bar: This shape applies a trait that is unstable: smithy.rules#operationContextParams | UnstableTrait.smithy.rules#operationContextParams +[ERROR] example#Bar: The operation `example#Bar` is marked with `smithy.rules#operationContextParams` which contains a key `invalidJmesPath` with an unparseable JMESPath path `...`: Syntax error at line 1 column 1: Expected ['@', 'A-Z|a-z|_', '`', '*', '{', '[', '[]', '&', '!', '[?', '('], but found '.'. | OperationContextParamsTrait +[ERROR] example#Bar: The operation `example#Bar` is marked with `smithy.rules#operationContextParams` which contains a path `failsLint` with an invalid JMESPath path `listOfObjects.notAKey`: 'Object field 'notAKey' extraction performed on array'. | OperationContextParamsTrait +[ERROR] example#Bar: The operation `example#Bar` is marked with `smithy.rules#operationContextParams` which contains a key `unsupportedFunction` with a JMESPath path `length(listOfObjects)` with unsupported expressions: '`length` function'. | OperationContextParamsTrait +[ERROR] example#Bar: The operation `example#Bar` is marked with `smithy.rules#operationContextParams` which contains a key `unsupportedExpression` with a JMESPath path `listOfObjects[1]` with unsupported expressions: 'index'. | OperationContextParamsTrait + + diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy new file mode 100644 index 00000000000..1e3bf607f69 --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy @@ -0,0 +1,35 @@ +$version: "1.0" + +namespace example + +use smithy.rules#operationContextParams + +service FizzBuzz { + operations: [Bar] +} + +@operationContextParams( + invalidJmesPath: {path: "..." }, + failsLint: {path: "listOfObjects.notAKey"}, + unsupportedFunction: {path: "length(listOfObjects)"}, + unsupportedExpression: {path: "listOfObjects[1]"} +) +operation Bar { + input: BarInput +} + +structure BarInput { + resourceId: ResourceId, + listOfObjects: ListOfObjects +} + +list ListOfObjects { + member: ObjectMember +} + +structure ObjectMember { + key: Key, +} + +string Key +string ResourceId diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.errors b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.errors new file mode 100644 index 00000000000..e5619e6a771 --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.errors @@ -0,0 +1 @@ +[WARNING] example#Bar: This shape applies a trait that is unstable: smithy.rules#operationContextParams | UnstableTrait.smithy.rules#operationContextParams diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy new file mode 100644 index 00000000000..fa1673224e4 --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy @@ -0,0 +1,42 @@ +$version: "1.0" + +namespace example + +use smithy.rules#operationContextParams + +service FizzBuzz { + operations: [Bar] +} + +@operationContextParams( + toplLevelMember: {path: "resourceId" }, + projection: {path: "listOfObjects[*].key"}, + subExpression: {path: "nestedObject.key"}, + keysFunction: {path: "keys(map)"} +) +operation Bar { + input: BarInput +} + +structure BarInput { + resourceId: ResourceId, + nestedObject: ObjectMember, + listOfObjects: ListOfObjects, + map: Map +} + +list ListOfObjects { + member: ObjectMember +} + +structure ObjectMember { + key: Key, +} + +map Map { + key: Key, + value: ResourceId +} + +string Key +string ResourceId From 517b0c0152b04414ff23ffb9c1309d42b78033bf Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Wed, 1 May 2024 08:56:20 -0700 Subject: [PATCH 6/8] Fixes from PR --- .../additional-specs/rules-engine/parameters.rst | 4 ++-- .../OperationContextParamsTraitValidator.java | 14 ++++++-------- .../invalid-operation-context-params.smithy | 2 +- .../valid/operation-context-params.smithy | 2 +- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/docs/source-2.0/additional-specs/rules-engine/parameters.rst b/docs/source-2.0/additional-specs/rules-engine/parameters.rst index b2dc6910a24..d1ee3e931b1 100644 --- a/docs/source-2.0/additional-specs/rules-engine/parameters.rst +++ b/docs/source-2.0/additional-specs/rules-engine/parameters.rst @@ -296,11 +296,11 @@ operation input using a JMESPath expression: } list ObjectIdentifierList { - member: ObjectIdentifier + member: ObjectIdentifier } structure ObjectIdentifier { - Key: String + Key: String } `paths` specified in :ref:`OperationContextParams ` are limited diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java index 675cf98e873..22937f0f7e7 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java @@ -97,10 +97,9 @@ public List validate(Model model) { OperationContextParamsTrait.ID.toString(), entry.getKey(), entry.getValue().getPath(), - String.join(", ", - linterResult.getProblems().stream() - .map(p -> "'" + p.message + "'") - .collect(Collectors.toList())) + linterResult.getProblems().stream() + .map(p -> "'" + p.message + "'") + .collect(Collectors.joining(", ")) ))); } @@ -114,10 +113,9 @@ public List validate(Model model) { OperationContextParamsTrait.ID.toString(), entry.getKey(), entry.getValue().getPath(), - String.join(", ", - unsupportedExpressions.stream() - .map(e -> "'" + e + "'") - .collect(Collectors.toList())) + unsupportedExpressions.stream() + .map(e -> "'" + e + "'") + .collect(Collectors.joining(", ")) ))); } } catch (JmespathException e) { diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy index 1e3bf607f69..4296ac8b4c8 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy @@ -1,4 +1,4 @@ -$version: "1.0" +$version: "2.0" namespace example diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy index fa1673224e4..770abfbc72e 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy @@ -1,4 +1,4 @@ -$version: "1.0" +$version: "2.0" namespace example From c3a90394a969da0b711ba20a26fb63bd9a43e658 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Wed, 1 May 2024 09:20:04 -0700 Subject: [PATCH 7/8] Add comment about duplication of ModelRuntimeTypeGenerator --- .../OperationContextParamsTraitValidator.java | 218 +++++++++--------- 1 file changed, 112 insertions(+), 106 deletions(-) diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java index 22937f0f7e7..c652fc1c15a 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/OperationContextParamsTraitValidator.java @@ -140,6 +140,118 @@ private LiteralExpression createCurrentNodeFromShape(Shape shape, Model model) { : new LiteralExpression(shape.accept(new ModelRuntimeTypeGenerator(model))); } + private static final class UnsupportedJmesPathVisitor implements ExpressionVisitor> { + + @Override + public List visitComparator(ComparatorExpression expression) { + return ListUtils.of("comparator"); + } + + @Override + public List visitCurrentNode(CurrentExpression expression) { + return ListUtils.of("current node"); + } + + @Override + public List visitExpressionType(ExpressionTypeExpression expression) { + return expression.getExpression().accept(this); + } + + @Override + public List visitFlatten(FlattenExpression expression) { + return ListUtils.of("flatten"); + } + + @Override + public List visitFunction(FunctionExpression expression) { + if (expression.getName().equals("keys")) { + return expression.getArguments().stream() + .map(e -> e.accept(this)) + .flatMap(List::stream) + .collect(Collectors.toList()); + } else { + return ListUtils.of("`" + expression.getName() + "` function"); + } + } + + @Override + public List visitField(FieldExpression expression) { + return Collections.emptyList(); + } + + @Override + public List visitIndex(IndexExpression expression) { + return ListUtils.of("index"); + } + + @Override + public List visitLiteral(LiteralExpression expression) { + return ListUtils.of("literal"); + } + + @Override + public List visitMultiSelectList(MultiSelectListExpression expression) { + return ListUtils.of("multiselect list"); + } + + @Override + public List visitMultiSelectHash(MultiSelectHashExpression expression) { + return ListUtils.of("multiselect hash"); + } + + @Override + public List visitAnd(AndExpression expression) { + return ListUtils.of("and"); + } + + @Override + public List visitOr(OrExpression expression) { + return ListUtils.of("or"); + } + + @Override + public List visitNot(NotExpression expression) { + return ListUtils.of("not"); + } + + @Override + public List visitProjection(ProjectionExpression expression) { + return Collections.emptyList(); + } + + @Override + public List visitFilterProjection(FilterProjectionExpression expression) { + return ListUtils.of("filter projection"); + } + + @Override + public List visitObjectProjection(ObjectProjectionExpression expression) { + List unsupported = new ArrayList<>(); + unsupported.addAll(expression.getLeft().accept(this)); + unsupported.addAll(expression.getRight().accept(this)); + return Collections.unmodifiableList(unsupported); + } + + @Override + public List visitSlice(SliceExpression expression) { + return ListUtils.of("slice"); + } + + @Override + public List visitSubexpression(Subexpression expression) { + List unsupported = new ArrayList<>(); + unsupported.addAll(expression.getLeft().accept(this)); + unsupported.addAll(expression.getRight().accept(this)); + return Collections.unmodifiableList(unsupported); + } + } + + /** + * This class is duplicated from + * smithy-waiters/src/main/java/software/amazon/smithy/waiters/ModelRuntimeTypeGenerator.java + * + * It is duplicated here to avoid taking a dependency on smithy-waiters. + */ private static final class ModelRuntimeTypeGenerator implements ShapeVisitor { private final Model model; @@ -344,110 +456,4 @@ private double computeRange(Shape shape) { return i; } } - - private static final class UnsupportedJmesPathVisitor implements ExpressionVisitor> { - - @Override - public List visitComparator(ComparatorExpression expression) { - return ListUtils.of("comparator"); - } - - @Override - public List visitCurrentNode(CurrentExpression expression) { - return ListUtils.of("current node"); - } - - @Override - public List visitExpressionType(ExpressionTypeExpression expression) { - return expression.getExpression().accept(this); - } - - @Override - public List visitFlatten(FlattenExpression expression) { - return ListUtils.of("flatten"); - } - - @Override - public List visitFunction(FunctionExpression expression) { - if (expression.getName().equals("keys")) { - return expression.getArguments().stream() - .map(e -> e.accept(this)) - .flatMap(List::stream) - .collect(Collectors.toList()); - } else { - return ListUtils.of("`" + expression.getName() + "` function"); - } - } - - @Override - public List visitField(FieldExpression expression) { - return Collections.emptyList(); - } - - @Override - public List visitIndex(IndexExpression expression) { - return ListUtils.of("index"); - } - - @Override - public List visitLiteral(LiteralExpression expression) { - return ListUtils.of("literal"); - } - - @Override - public List visitMultiSelectList(MultiSelectListExpression expression) { - return ListUtils.of("multiselect list"); - } - - @Override - public List visitMultiSelectHash(MultiSelectHashExpression expression) { - return ListUtils.of("multiselect hash"); - } - - @Override - public List visitAnd(AndExpression expression) { - return ListUtils.of("and"); - } - - @Override - public List visitOr(OrExpression expression) { - return ListUtils.of("or"); - } - - @Override - public List visitNot(NotExpression expression) { - return ListUtils.of("not"); - } - - @Override - public List visitProjection(ProjectionExpression expression) { - return Collections.emptyList(); - } - - @Override - public List visitFilterProjection(FilterProjectionExpression expression) { - return ListUtils.of("filter projection"); - } - - @Override - public List visitObjectProjection(ObjectProjectionExpression expression) { - List unsupported = new ArrayList<>(); - unsupported.addAll(expression.getLeft().accept(this)); - unsupported.addAll(expression.getRight().accept(this)); - return Collections.unmodifiableList(unsupported); - } - - @Override - public List visitSlice(SliceExpression expression) { - return ListUtils.of("slice"); - } - - @Override - public List visitSubexpression(Subexpression expression) { - List unsupported = new ArrayList<>(); - unsupported.addAll(expression.getLeft().accept(this)); - unsupported.addAll(expression.getRight().accept(this)); - return Collections.unmodifiableList(unsupported); - } - } } From 7903b09c101b5e896378aa9e36c9cb7dbc6cc6cf Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Wed, 1 May 2024 09:41:40 -0700 Subject: [PATCH 8/8] Add more tests --- .../invalid-operation-context-params.errors | 4 ++- .../invalid-operation-context-params.smithy | 11 +++++--- .../valid/operation-context-params.smithy | 25 +++++++++++++------ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.errors b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.errors index 65f61bb3905..f499df1f5fb 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.errors +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.errors @@ -1,6 +1,8 @@ [WARNING] example#Bar: This shape applies a trait that is unstable: smithy.rules#operationContextParams | UnstableTrait.smithy.rules#operationContextParams [ERROR] example#Bar: The operation `example#Bar` is marked with `smithy.rules#operationContextParams` which contains a key `invalidJmesPath` with an unparseable JMESPath path `...`: Syntax error at line 1 column 1: Expected ['@', 'A-Z|a-z|_', '`', '*', '{', '[', '[]', '&', '!', '[?', '('], but found '.'. | OperationContextParamsTrait -[ERROR] example#Bar: The operation `example#Bar` is marked with `smithy.rules#operationContextParams` which contains a path `failsLint` with an invalid JMESPath path `listOfObjects.notAKey`: 'Object field 'notAKey' extraction performed on array'. | OperationContextParamsTrait +[ERROR] example#Bar: The operation `example#Bar` is marked with `smithy.rules#operationContextParams` which contains a path `arrayAsObject` with an invalid JMESPath path `listOfObjects.notAKey`: 'Object field 'notAKey' extraction performed on array'. | OperationContextParamsTrait +[ERROR] example#Bar: The operation `example#Bar` is marked with `smithy.rules#operationContextParams` which contains a path `incorrectKey` with an invalid JMESPath path `nested.bar`: 'Object field 'bar' does not exist in object with properties [foo]'. | OperationContextParamsTrait +[ERROR] example#Bar: The operation `example#Bar` is marked with `smithy.rules#operationContextParams` which contains a path `projectionOnScalar` with an invalid JMESPath path `nested.foo[*]`: 'Array projection performed on string'. | OperationContextParamsTrait [ERROR] example#Bar: The operation `example#Bar` is marked with `smithy.rules#operationContextParams` which contains a key `unsupportedFunction` with a JMESPath path `length(listOfObjects)` with unsupported expressions: '`length` function'. | OperationContextParamsTrait [ERROR] example#Bar: The operation `example#Bar` is marked with `smithy.rules#operationContextParams` which contains a key `unsupportedExpression` with a JMESPath path `listOfObjects[1]` with unsupported expressions: 'index'. | OperationContextParamsTrait diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy index 4296ac8b4c8..687e1148f0a 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-operation-context-params.smithy @@ -10,7 +10,9 @@ service FizzBuzz { @operationContextParams( invalidJmesPath: {path: "..." }, - failsLint: {path: "listOfObjects.notAKey"}, + arrayAsObject: {path: "listOfObjects.notAKey"}, + incorrectKey: {path: "nested.bar"}, + projectionOnScalar: {path: "nested.foo[*]"}, unsupportedFunction: {path: "length(listOfObjects)"}, unsupportedExpression: {path: "listOfObjects[1]"} ) @@ -19,7 +21,7 @@ operation Bar { } structure BarInput { - resourceId: ResourceId, + nested: Nested, listOfObjects: ListOfObjects } @@ -31,5 +33,8 @@ structure ObjectMember { key: Key, } +structure Nested { + foo: String, +} + string Key -string ResourceId diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy index 770abfbc72e..1a257d9e537 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/operation-context-params.smithy @@ -11,7 +11,8 @@ service FizzBuzz { @operationContextParams( toplLevelMember: {path: "resourceId" }, projection: {path: "listOfObjects[*].key"}, - subExpression: {path: "nestedObject.key"}, + subExpression: {path: "nested.nested.bar"}, + recursive: {path: "nested.nested.recursiveMember.foo"} keysFunction: {path: "keys(map)"} ) operation Bar { @@ -19,8 +20,8 @@ operation Bar { } structure BarInput { - resourceId: ResourceId, - nestedObject: ObjectMember, + resourceId: String, + nested: Nested1, listOfObjects: ListOfObjects, map: Map } @@ -30,13 +31,21 @@ list ListOfObjects { } structure ObjectMember { - key: Key, + key: String, } map Map { - key: Key, - value: ResourceId + key: String, + value: String +} + +structure Nested1 { + foo: String, + nested: Nested2 +} + +structure Nested2 { + bar: String, + recursiveMember: Nested1 } -string Key -string ResourceId