From d5fc3a22d42429c7a1261baebe9f628f5a699368 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Mon, 29 Apr 2024 15:46:42 -0700 Subject: [PATCH 01/10] Add support for StringArray RulesEngine Parameters --- .../language/evaluation/type/Type.java | 3 ++ .../language/evaluation/value/ArrayValue.java | 19 +++++++- .../syntax/parameters/ParameterType.java | 46 +++++++++++++++++-- 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/Type.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/Type.java index 244a1c9ab5a..97a02c0e72d 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/Type.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/Type.java @@ -45,6 +45,9 @@ static Type fromParameterType(ParameterType parameterType) { if (parameterType == ParameterType.BOOLEAN) { return booleanType(); } + if (parameterType == ParameterType.STRING_ARRAY) { + return arrayType(optionalType(stringType())); + } throw new IllegalArgumentException("Unexpected parameter type: " + parameterType); } diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java index 7f736179bc3..1b869d098b6 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java @@ -54,11 +54,26 @@ public Type getType() { return Type.arrayType(Type.emptyType()); } else { Type first = values.get(0).getType(); + boolean hasEmpties = false; + if (first.isA(Type.emptyType())) { + hasEmpties = true; + first = null; + } for (Value value : values) { - if (value.getType() != first) { - throw new SourceException("An array cannot contain different types", this); + if (first == null && !value.getType().isA(Type.emptyType())) { + first = value.getType(); + } else if (!value.getType().isA(Type.emptyType()) && !value.getType().isA(first)) { + throw new SourceException("An array cannot contain different types. Expected: " + + first + " found: " + value.getType(), this); } } + if (first == null) { + // all empties + return Type.arrayType(Type.emptyType()); + } + if (hasEmpties) { + return Type.arrayType(Type.optionalType(first)); + } return Type.arrayType(first); } } diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java index 14ddcdc1d76..e39e1d7a093 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java @@ -10,6 +10,7 @@ import software.amazon.smithy.model.node.StringNode; import software.amazon.smithy.model.shapes.ShapeType; import software.amazon.smithy.rulesengine.language.error.RuleError; +import software.amazon.smithy.rulesengine.language.evaluation.type.ArrayType; import software.amazon.smithy.rulesengine.language.evaluation.type.BooleanType; import software.amazon.smithy.rulesengine.language.evaluation.type.StringType; import software.amazon.smithy.rulesengine.language.evaluation.type.Type; @@ -28,7 +29,12 @@ public enum ParameterType { /** * A "boolean" parameter type. */ - BOOLEAN; + BOOLEAN, + + /** + * An array (list) of strings parameter type. + */ + STRING_ARRAY; /** * Creates a {@link ParameterType} of a specific type from the given Node information. @@ -45,6 +51,9 @@ public static ParameterType fromNode(StringNode node) throws RuleError { if (value.equalsIgnoreCase("Boolean")) { return BOOLEAN; } + if (value.equalsIgnoreCase("StringArray")) { + return STRING_ARRAY; + } throw new RuleError(new SourceException( String.format("Unexpected parameter type `%s`. Expected `String` or `Boolean`.", value), node)); } @@ -63,6 +72,17 @@ public static ParameterType fromNode(Node node) throws RuleError { if (node.isBooleanNode()) { return BOOLEAN; } + if (node.isArrayNode()) { + // confirm all elements are Strings + node.expectArrayNode().getElements().forEach(memberNode -> { + if (!memberNode.isStringNode()) { + throw new RuleError(new SourceException( + String.format("Unexpected array member parameter type `%s`. Expected a string.", + memberNode.getType()), memberNode)); + } + }); + return STRING_ARRAY; + } throw new RuleError(new SourceException( String.format("Unexpected parameter type `%s`. Expected a string or boolean.", node.getType()), node)); } @@ -81,8 +101,14 @@ public static ParameterType fromType(Type type) { if (type instanceof BooleanType) { return BOOLEAN; } + if (type instanceof ArrayType) { + ArrayType arrayType = (ArrayType) type; + if (arrayType.getMember().isA(Type.stringType()) || arrayType.getMember().isA(Type.emptyType())) { + return STRING_ARRAY; + } + } throw new RuntimeException( - String.format("Unexpected parameter type `%s`. Expected a string or boolean.", type)); + String.format("Unexpected parameter type `%s`. Expected a string, boolean, or array.", type)); } /** @@ -99,12 +125,24 @@ public static ParameterType fromShapeType(ShapeType type) { if (type == ShapeType.BOOLEAN) { return BOOLEAN; } + if (type == ShapeType.LIST) { + return STRING_ARRAY; + } throw new RuntimeException( - String.format("Unexpected parameter type `%s`. Expected string or boolean.", type)); + String.format("Unexpected parameter type `%s`. Expected string or boolean or list.", type)); } @Override public String toString() { - return this == STRING ? "String" : "Boolean"; + switch (this) { + case STRING: + return "String"; + case BOOLEAN: + return "Boolean"; + case STRING_ARRAY: + return "StringArray"; + default: + return "Unknown Type"; + } } } From 69494ca54a76fc80435e9f34835713a27a334854 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Mon, 29 Apr 2024 16:07:46 -0700 Subject: [PATCH 02/10] Add docs --- .../additional-specs/rules-engine/parameters.rst | 9 ++++++--- .../additional-specs/rules-engine/specification.rst | 4 ++-- 2 files changed, 8 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 3385dc5f201..502995b637c 100644 --- a/docs/source-2.0/additional-specs/rules-engine/parameters.rst +++ b/docs/source-2.0/additional-specs/rules-engine/parameters.rst @@ -23,9 +23,9 @@ This following is the :rfc:`ABNF <5234>` grammar for rule set parameter names: .. productionlist:: smithy identifier = ALPHA *(ALPHA / DIGIT) -Parameters declare their respective type using the ``type`` key. There are two -supported rule set parameter types: ``string`` and ``boolean``. The following -table provides the description of these types, and their Smithy compatible +Parameters declare their respective type using the ``type`` key. There are three +supported rule set parameter types: ``string``, ``boolean``, and ``stringArray```. +The following table provides the description of these types, and their Smithy compatible types whose values can be bound to these parameters. Rule set parameters are always considered nullable and have no default value associated with them. @@ -42,6 +42,9 @@ always considered nullable and have no default value associated with them. * - ``boolean`` - ``boolean`` - Boolean value type. + * - ``stringArray`` + - ``List`` + - A list with ``String`` members. .. _rules-engine-parameters-implementation: diff --git a/docs/source-2.0/additional-specs/rules-engine/specification.rst b/docs/source-2.0/additional-specs/rules-engine/specification.rst index 148170f6619..c92443accaf 100644 --- a/docs/source-2.0/additional-specs/rules-engine/specification.rst +++ b/docs/source-2.0/additional-specs/rules-engine/specification.rst @@ -91,13 +91,13 @@ A parameter object contains the following properties: - Description * - type - ``string`` - - **Required**. MUST be one of ``string`` or ``boolean``. + - **Required**. MUST be one of ``string``, ``boolean``, or ``stringArray``. * - builtIn - ``string`` - Specifies a named built-in value that is sourced and provided to the endpoint provider by a caller. * - default - - ``string`` or ``boolean`` + - ``string``, ``boolean`` or a list of ``string``. - Specifies the default value for the parameter if not set. Parameters with defaults MUST also be marked as ``required``. The type of the provided default MUST match ``type``. From 29b50a3a667a7b7cef6b6a5a5b4b447c6aec0f5b Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Mon, 29 Apr 2024 16:19:07 -0700 Subject: [PATCH 03/10] Fix docs --- docs/source-2.0/additional-specs/rules-engine/parameters.rst | 2 +- 1 file changed, 1 insertion(+), 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 502995b637c..857c123d771 100644 --- a/docs/source-2.0/additional-specs/rules-engine/parameters.rst +++ b/docs/source-2.0/additional-specs/rules-engine/parameters.rst @@ -24,7 +24,7 @@ This following is the :rfc:`ABNF <5234>` grammar for rule set parameter names: identifier = ALPHA *(ALPHA / DIGIT) Parameters declare their respective type using the ``type`` key. There are three -supported rule set parameter types: ``string``, ``boolean``, and ``stringArray```. +supported rule set parameter types: ``string``, ``boolean``, and ``stringArray``. The following table provides the description of these types, and their Smithy compatible types whose values can be bound to these parameters. Rule set parameters are always considered nullable and have no default value associated with them. From 31cbf7bc7cf1cff8b7b386998ab1822650bb3b02 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Tue, 30 Apr 2024 11:30:18 -0700 Subject: [PATCH 04/10] Add tests --- .../rules-engine/parameters.rst | 7 ++- .../language/evaluation/value/ArrayValue.java | 7 +-- .../syntax/expressions/functions/GetAttr.java | 4 +- .../StaticContextParamsTraitValidator.java | 15 +++++- .../errorfiles/valid/default-values.errors | 1 + .../errorfiles/valid/default-values.smithy | 52 ++++++++++++++++++- .../errorfiles/valid/valid-model.smithy | 7 ++- 7 files changed, 81 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 857c123d771..6087e83bd42 100644 --- a/docs/source-2.0/additional-specs/rules-engine/parameters.rst +++ b/docs/source-2.0/additional-specs/rules-engine/parameters.rst @@ -221,13 +221,13 @@ The ``staticContextParam`` structure has the following properties: * - value - ``document`` - **Required**. The static value to be set for the parameter. The type - of the value MUST be either a ``string`` or ``boolean``. + of the value MUST be either a ``string``, ``boolean`` or a list of ``string``. Each parameter is identified using it’s name as specified in the rule set. The type of a ``staticContextParam`` MUST be compatible with the parameter type specified in the rule set. -The following example specifies two parameters to statically set for an +The following example specifies three parameters to statically set for an operation: .. code-block:: smithy @@ -238,6 +238,9 @@ operation: } previewEndpoint: { value: true + }, + supportedPrefixes: { + value: ["host", "id", "resourceId"] } ) operation GetThing {} diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java index 1b869d098b6..ec156295d8a 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java @@ -54,9 +54,7 @@ public Type getType() { return Type.arrayType(Type.emptyType()); } else { Type first = values.get(0).getType(); - boolean hasEmpties = false; if (first.isA(Type.emptyType())) { - hasEmpties = true; first = null; } for (Value value : values) { @@ -71,10 +69,7 @@ public Type getType() { // all empties return Type.arrayType(Type.emptyType()); } - if (hasEmpties) { - return Type.arrayType(Type.optionalType(first)); - } - return Type.arrayType(first); + return Type.arrayType(Type.optionalType(first)); } } diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/GetAttr.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/GetAttr.java index 6dd674ba344..97e1d83ea08 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/GetAttr.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/GetAttr.java @@ -105,7 +105,9 @@ private static List parse(String path, FromSourceLocation sourceLocation) throw new InvalidRulesException("Invalid path component: slice index must be >= 0", sourceLocation); } - result.add(Part.Key.of(component.substring(0, slicePartIndex))); + if (slicePartIndex > 0) { + result.add(Part.Key.of(component.substring(0, slicePartIndex))); + } result.add(new Part.Index(slice)); } catch (NumberFormatException ex) { throw new InvalidRulesException(String.format("%s could not be parsed as a number", slicePart), diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/StaticContextParamsTraitValidator.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/StaticContextParamsTraitValidator.java index b5aa8bfe260..976a65b91f5 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/StaticContextParamsTraitValidator.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/StaticContextParamsTraitValidator.java @@ -31,7 +31,7 @@ public List validate(Model model) { .orElse(Collections.emptyMap()); for (Map.Entry entry : definitionMap.entrySet()) { Node node = entry.getValue().getValue(); - if (node.isStringNode() || node.isBooleanNode()) { + if (supportedType(node)) { continue; } events.add(error(operationShape, @@ -45,4 +45,17 @@ public List validate(Model model) { } return events; } + + private static boolean supportedType(Node node) { + if (node.isStringNode() || node.isBooleanNode()) { + return true; + } + + if (node.isArrayNode()) { + // all elements must be strings + return node.expectArrayNode().getElements().stream().allMatch( e -> e.isStringNode()); + } + + return false; + } } diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.errors b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.errors index be1d71040e0..e6eceff66b2 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.errors +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.errors @@ -1,3 +1,4 @@ [WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#clientContextParams | UnstableTrait [WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#endpointRuleSet | UnstableTrait [WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#endpointTests | UnstableTrait +[WARNING] example#GetThing: This shape applies a trait that is unstable: smithy.rules#staticContextParams | UnstableTrait.smithy.rules#staticContextParams \ No newline at end of file diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.smithy index d63c041fe41..57a747654a4 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.smithy +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.smithy @@ -5,6 +5,7 @@ namespace example use smithy.rules#clientContextParams use smithy.rules#endpointRuleSet use smithy.rules#endpointTests +use smithy.rules#staticContextParams @clientContextParams( bar: {type: "string", documentation: "a client string parameter"} @@ -30,10 +31,16 @@ use smithy.rules#endpointTests default: "asdf" documentation: "docs" }, + stringArrayParam: { + type: "stringArray", + required: true, + default: ["a", "b", "c"], + documentation: "docs" + } }, rules: [ { - "documentation": "Template the region into the URI when FIPS is enabled", + "documentation": "Template baz into URI when bar is set", "conditions": [ { "fn": "isSet", @@ -49,6 +56,33 @@ use smithy.rules#endpointTests }, "type": "endpoint" }, + { + "documentation": "Template first array value into URI", + "conditions": [ + { + "fn": "getAttr", + "argv": [ + { + "ref": "stringArrayParam" + }, + "[0]" + ], + "assign": "arrayValue" + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "arrayValue" + } + ] + } + ], + "endpoint": { + "url": "https://example.com/{arrayValue}" + }, + "type": "endpoint" + }, { "conditions": [], "documentation": "error fallthrough", @@ -94,6 +128,19 @@ use smithy.rules#endpointTests } }, { + "documentation": "Default array values used" + "params": { + } + "expect": { + "endpoint": { + "url": "https://example.com/a" + } + } + }, + { + "params": { + "stringArrayParam": [] + } "documentation": "a documentation string", "expect": { "error": "endpoint error" @@ -106,6 +153,9 @@ service FizzBuzz { operations: [GetThing] } +@staticContextParams( + "stringArrayParam": {value: []} +) operation GetThing { input := {} } diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/valid-model.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/valid-model.smithy index de5b5bc87de..9d2e80212e3 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/valid-model.smithy +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/valid-model.smithy @@ -25,6 +25,10 @@ use smithy.rules#staticContextParams "ExtraParameter": { "type": "string", "documentation": "docs" + }, + "StringArrayParameter": { + "type": "stringArray", + documentation: "docs" } }, "rules": [] @@ -38,7 +42,8 @@ service FizzBuzz { @staticContextParams( "ParameterFoo": {value: "foo"}, - "ExtraParameter": {value: "someValue"} + "ExtraParameter": {value: "someValue"}, + "StringArrayParameter": {value: ["a", "b", "c"]} ) operation GetResource { input: GetResourceInput From 796f14a391cd35bec49143970821d3b6bfb42177 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Tue, 30 Apr 2024 11:45:25 -0700 Subject: [PATCH 05/10] Fix tests and style --- .../rulesengine/traits/StaticContextParamsTraitValidator.java | 2 +- .../traits/errorfiles/invalid-static-array-param-value.smithy | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/StaticContextParamsTraitValidator.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/StaticContextParamsTraitValidator.java index 976a65b91f5..6197aa3fafc 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/StaticContextParamsTraitValidator.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/StaticContextParamsTraitValidator.java @@ -53,7 +53,7 @@ private static boolean supportedType(Node node) { if (node.isArrayNode()) { // all elements must be strings - return node.expectArrayNode().getElements().stream().allMatch( e -> e.isStringNode()); + return node.expectArrayNode().getElements().stream().allMatch(e -> e.isStringNode()); } return false; diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/invalid-static-array-param-value.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/invalid-static-array-param-value.smithy index 88a5d894427..de5db297118 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/invalid-static-array-param-value.smithy +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/invalid-static-array-param-value.smithy @@ -4,5 +4,5 @@ namespace smithy.example use smithy.rules#staticContextParams -@staticContextParams(arrayParam: {value: ["foo", "bar"]}) +@staticContextParams(arrayParam: {value: ["foo", 3]}) operation OperationArray {} From c3b96f499e1837820a29b33ffdeca389546e1ac2 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Wed, 1 May 2024 14:46:17 -0700 Subject: [PATCH 06/10] Support only dense (non-sparse) arrays --- .../language/evaluation/type/Type.java | 2 +- .../language/evaluation/value/ArrayValue.java | 16 +++------------- .../syntax/parameters/ParameterType.java | 5 +---- .../errorfiles/valid/default-values.errors | 2 +- .../errorfiles/valid/default-values.smithy | 8 -------- 5 files changed, 6 insertions(+), 27 deletions(-) diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/Type.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/Type.java index 97a02c0e72d..d4a709ec3b6 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/Type.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/type/Type.java @@ -46,7 +46,7 @@ static Type fromParameterType(ParameterType parameterType) { return booleanType(); } if (parameterType == ParameterType.STRING_ARRAY) { - return arrayType(optionalType(stringType())); + return arrayType(stringType()); } throw new IllegalArgumentException("Unexpected parameter type: " + parameterType); } diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java index ec156295d8a..2a76ac8dedb 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/ArrayValue.java @@ -54,22 +54,12 @@ public Type getType() { return Type.arrayType(Type.emptyType()); } else { Type first = values.get(0).getType(); - if (first.isA(Type.emptyType())) { - first = null; - } for (Value value : values) { - if (first == null && !value.getType().isA(Type.emptyType())) { - first = value.getType(); - } else if (!value.getType().isA(Type.emptyType()) && !value.getType().isA(first)) { - throw new SourceException("An array cannot contain different types. Expected: " - + first + " found: " + value.getType(), this); + if (!value.getType().isA(first)) { + throw new SourceException("An array cannot contain different types", this); } } - if (first == null) { - // all empties - return Type.arrayType(Type.emptyType()); - } - return Type.arrayType(Type.optionalType(first)); + return Type.arrayType(first); } } diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java index e39e1d7a093..73d1f3c2cc3 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java @@ -125,11 +125,8 @@ public static ParameterType fromShapeType(ShapeType type) { if (type == ShapeType.BOOLEAN) { return BOOLEAN; } - if (type == ShapeType.LIST) { - return STRING_ARRAY; - } throw new RuntimeException( - String.format("Unexpected parameter type `%s`. Expected string or boolean or list.", type)); + String.format("Unexpected parameter type `%s`. Expected string or boolean.", type)); } @Override diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.errors b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.errors index e6eceff66b2..d40f70809a0 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.errors +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.errors @@ -1,4 +1,4 @@ [WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#clientContextParams | UnstableTrait [WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#endpointRuleSet | UnstableTrait [WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#endpointTests | UnstableTrait -[WARNING] example#GetThing: This shape applies a trait that is unstable: smithy.rules#staticContextParams | UnstableTrait.smithy.rules#staticContextParams \ No newline at end of file +[WARNING] example#GetThing: This shape applies a trait that is unstable: smithy.rules#staticContextParams | UnstableTrait.smithy.rules#staticContextParams diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.smithy index 57a747654a4..f137a187240 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.smithy +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/valid/default-values.smithy @@ -68,14 +68,6 @@ use smithy.rules#staticContextParams "[0]" ], "assign": "arrayValue" - }, - { - "fn": "isSet", - "argv": [ - { - "ref": "arrayValue" - } - ] } ], "endpoint": { From 48fdd24eed125b1535c5f010615db150b434c015 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Wed, 1 May 2024 14:50:23 -0700 Subject: [PATCH 07/10] Update docs --- .../additional-specs/rules-engine/parameters.rst | 7 ++++--- .../additional-specs/rules-engine/specification.rst | 2 +- 2 files changed, 5 insertions(+), 4 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 6087e83bd42..776b3c91807 100644 --- a/docs/source-2.0/additional-specs/rules-engine/parameters.rst +++ b/docs/source-2.0/additional-specs/rules-engine/parameters.rst @@ -23,8 +23,8 @@ This following is the :rfc:`ABNF <5234>` grammar for rule set parameter names: .. productionlist:: smithy identifier = ALPHA *(ALPHA / DIGIT) -Parameters declare their respective type using the ``type`` key. There are three -supported rule set parameter types: ``string``, ``boolean``, and ``stringArray``. +Parameters declare their respective type using the ``type`` key. The following +parameter types are supported: ``string``, ``boolean``, and ``stringArray``. The following table provides the description of these types, and their Smithy compatible types whose values can be bound to these parameters. Rule set parameters are always considered nullable and have no default value associated with them. @@ -221,7 +221,8 @@ The ``staticContextParam`` structure has the following properties: * - value - ``document`` - **Required**. The static value to be set for the parameter. The type - of the value MUST be either a ``string``, ``boolean`` or a list of ``string``. + of the value MUST be either a ``string``, ``boolean`` or a + json array of ``string``. Each parameter is identified using it’s name as specified in the rule set. The type of a ``staticContextParam`` MUST be compatible with the parameter type diff --git a/docs/source-2.0/additional-specs/rules-engine/specification.rst b/docs/source-2.0/additional-specs/rules-engine/specification.rst index c92443accaf..1c9547c5cde 100644 --- a/docs/source-2.0/additional-specs/rules-engine/specification.rst +++ b/docs/source-2.0/additional-specs/rules-engine/specification.rst @@ -97,7 +97,7 @@ A parameter object contains the following properties: - Specifies a named built-in value that is sourced and provided to the endpoint provider by a caller. * - default - - ``string``, ``boolean`` or a list of ``string``. + - ``string``, ``boolean`` or a json array of ``string``. - Specifies the default value for the parameter if not set. Parameters with defaults MUST also be marked as ``required``. The type of the provided default MUST match ``type``. From 07c1dfdc60a479bca3fa558a2ecdcfd9049f7feb Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Thu, 2 May 2024 08:34:35 -0700 Subject: [PATCH 08/10] Updates from PR --- .../additional-specs/rules-engine/parameters.rst | 8 ++++---- .../language/syntax/parameters/ParameterType.java | 15 ++++++++------- .../invalid-rules/invalid-param-type.json5 | 2 +- .../rulesengine/language/minimal-ruleset.json | 2 +- 4 files changed, 14 insertions(+), 13 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 776b3c91807..94f73acc7d3 100644 --- a/docs/source-2.0/additional-specs/rules-engine/parameters.rst +++ b/docs/source-2.0/additional-specs/rules-engine/parameters.rst @@ -43,8 +43,8 @@ always considered nullable and have no default value associated with them. - ``boolean`` - Boolean value type. * - ``stringArray`` - - ``List`` - - A list with ``String`` members. + - ``list`` + - A list with ``string`` members. .. _rules-engine-parameters-implementation: @@ -221,8 +221,8 @@ The ``staticContextParam`` structure has the following properties: * - value - ``document`` - **Required**. The static value to be set for the parameter. The type - of the value MUST be either a ``string``, ``boolean`` or a - json array of ``string``. + of the value MUST be either a ``string``, ``boolean`` or an + array of ``string``. Each parameter is identified using it’s name as specified in the rule set. The type of a ``staticContextParam`` MUST be compatible with the parameter type diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java index 73d1f3c2cc3..56b0973aab8 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java @@ -45,17 +45,18 @@ public enum ParameterType { */ public static ParameterType fromNode(StringNode node) throws RuleError { String value = node.getValue(); - if (value.equalsIgnoreCase("String")) { + if (value.equalsIgnoreCase("string")) { return STRING; } - if (value.equalsIgnoreCase("Boolean")) { + if (value.equalsIgnoreCase("boolean")) { return BOOLEAN; } - if (value.equalsIgnoreCase("StringArray")) { + if (value.equalsIgnoreCase("stringArray")) { return STRING_ARRAY; } throw new RuleError(new SourceException( - String.format("Unexpected parameter type `%s`. Expected `String` or `Boolean`.", value), node)); + String.format("Unexpected parameter type `%s`. Expected `string`, `boolean`, or `stringArray`.", + value), node)); } /** @@ -133,11 +134,11 @@ public static ParameterType fromShapeType(ShapeType type) { public String toString() { switch (this) { case STRING: - return "String"; + return "string"; case BOOLEAN: - return "Boolean"; + return "boolean"; case STRING_ARRAY: - return "StringArray"; + return "stringArray"; default: return "Unknown Type"; } diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/invalid-rules/invalid-param-type.json5 b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/invalid-rules/invalid-param-type.json5 index a2432b5cabe..b4c642acabc 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/invalid-rules/invalid-param-type.json5 +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/invalid-rules/invalid-param-type.json5 @@ -2,7 +2,7 @@ // while parsing the parameter `RegionName` // at invalid-rules/invalid-param-type.json5:10 // while parsing the parameter type -// Unexpected parameter type `notastring`. Expected `String` or `Boolean`. +// Unexpected parameter type `notastring`. Expected `string`, `boolean`, or `stringArray`. // at invalid-rules/invalid-param-type.json5:11 { "version": "1.2", diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/minimal-ruleset.json b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/minimal-ruleset.json index 92b6d8292e5..e6ab3e810d6 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/minimal-ruleset.json +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/minimal-ruleset.json @@ -4,7 +4,7 @@ "Region": { "builtIn": "AWS::Region", "required": true, - "type": "String" + "type": "string" } }, "rules": [ From f2edf4d750733e4fef1798ac38f70ca647bc364e Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Thu, 2 May 2024 10:14:01 -0700 Subject: [PATCH 09/10] Revert changes to serialization of boolean/string parameter types --- .../language/syntax/parameters/ParameterType.java | 7 ++++--- .../smithy/rulesengine/language/minimal-ruleset.json | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java index 56b0973aab8..6882d02b8c1 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/ParameterType.java @@ -51,7 +51,7 @@ public static ParameterType fromNode(StringNode node) throws RuleError { if (value.equalsIgnoreCase("boolean")) { return BOOLEAN; } - if (value.equalsIgnoreCase("stringArray")) { + if (value.equals("stringArray")) { return STRING_ARRAY; } throw new RuleError(new SourceException( @@ -132,11 +132,12 @@ public static ParameterType fromShapeType(ShapeType type) { @Override public String toString() { + // Inconsistent casing on string/boolean to preserve backwards compatibility in serialization switch (this) { case STRING: - return "string"; + return "String"; case BOOLEAN: - return "boolean"; + return "Boolean"; case STRING_ARRAY: return "stringArray"; default: diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/minimal-ruleset.json b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/minimal-ruleset.json index e6ab3e810d6..92b6d8292e5 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/minimal-ruleset.json +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/minimal-ruleset.json @@ -4,7 +4,7 @@ "Region": { "builtIn": "AWS::Region", "required": true, - "type": "string" + "type": "String" } }, "rules": [ From 60ee43125d9ff5b20862b9cea8617fd5f05f328d Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Thu, 2 May 2024 10:23:56 -0700 Subject: [PATCH 10/10] Remove json from doc --- docs/source-2.0/additional-specs/rules-engine/specification.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source-2.0/additional-specs/rules-engine/specification.rst b/docs/source-2.0/additional-specs/rules-engine/specification.rst index 1c9547c5cde..bd934d9d304 100644 --- a/docs/source-2.0/additional-specs/rules-engine/specification.rst +++ b/docs/source-2.0/additional-specs/rules-engine/specification.rst @@ -97,7 +97,7 @@ A parameter object contains the following properties: - Specifies a named built-in value that is sourced and provided to the endpoint provider by a caller. * - default - - ``string``, ``boolean`` or a json array of ``string``. + - ``string``, ``boolean`` or an array of ``string``. - Specifies the default value for the parameter if not set. Parameters with defaults MUST also be marked as ``required``. The type of the provided default MUST match ``type``.