From 3603f2d302ac786fd606b17a52d0e91260db8045 Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Thu, 16 Apr 2020 11:49:00 -0700 Subject: [PATCH] Tolerate unknown relationship types This allows us to add new relationships to validators, trait selectors, etc, without breaking older model implementations. For example, we might add a "trait" relationship, "parent", "child", etc. --- docs/source/spec/core/selectors.rst | 6 ++++++ .../emit-each-selector-validator.errors | 3 +-- .../errorfiles/emit-each-selector-validator.json | 9 +-------- .../amazon/smithy/model/selector/Parser.java | 12 +++++++++++- .../model/selector/NeighborSelectorTest.java | 15 +++++++++++++++ .../smithy/model/selector/SelectorTest.java | 9 +++++++++ 6 files changed, 43 insertions(+), 11 deletions(-) diff --git a/docs/source/spec/core/selectors.rst b/docs/source/spec/core/selectors.rst index 8727d0b225e..4fb32d85511 100644 --- a/docs/source/spec/core/selectors.rst +++ b/docs/source/spec/core/selectors.rst @@ -354,6 +354,12 @@ The table below lists the labeled directed relationships from each shape. - The shape targeted by the member. Note that member targets have no relationship name. +.. important:: + + Implementations MUST tolerate parsing unknown relationship types. When + evaluated, the directed traversal of unknown relationship types matches + no shapes. + Functions ========= diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.errors b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.errors index 951f6caecca..92132a1b3f5 100644 --- a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.errors +++ b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.errors @@ -107,7 +107,7 @@ [ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo^foo]": Unable to deserialize Node using fromNode method: Syntax error at character 1 of 9, near `foo^foo]`: Expected one of the following tokens: `id`, `id|member`, `id|name`, `id|namespace`, `service|version`, `trait|`; expression `[foo^foo]` | Model [ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":each(:not(string) > list": Unable to deserialize Node using fromNode method: Syntax error at character 25 of 25, near ``: Expected one of the following tokens: `)`, `,`; expression `:each(:not(string) > list` | Model [ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "foo -[]->": Unable to deserialize Node using fromNode method: Syntax error at character 0 of 9, near `foo -[]->`: Expected one of the following tokens: `*`, `-[`, `:`, `>`, `[`, `bigDecimal`, `bigInteger`, `blob`, `boolean`, `byte`, `collection`, `document`, `double`, `float`, `integer`, `list`, `long`, `map`, `member`, `number`, `operation`, `resource`, `service`, `set`, `short`, `simpleType`, `string`, `structure`, `timestamp`, `union`; expression `foo -[]->` | Model -[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "foo -[foo]->": Unable to deserialize Node using fromNode method: Syntax error at character 0 of 12, near `foo -[foo]->`: Expected one of the following tokens: `*`, `-[`, `:`, `>`, `[`, `bigDecimal`, `bigInteger`, `blob`, `boolean`, `byte`, `collection`, `document`, `double`, `float`, `integer`, `list`, `long`, `map`, `member`, `number`, `operation`, `resource`, `service`, `set`, `short`, `simpleType`, `string`, `structure`, `timestamp`, `union`; expression `foo -[foo]->` | Model +[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "foo -[input]->": Unable to deserialize Node using fromNode method: Syntax error at character 0 of 14, near `foo -[input]->`: Expected one of the following tokens: `*`, `-[`, `:`, `>`, `[`, `bigDecimal`, `bigInteger`, `blob`, `boolean`, `byte`, `collection`, `document`, `double`, `float`, `integer`, `list`, `long`, `map`, `member`, `number`, `operation`, `resource`, `service`, `set`, `short`, `simpleType`, `string`, `structure`, `timestamp`, `union`; expression `foo -[input]->` | Model [ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":not": Unable to deserialize Node using fromNode method: Syntax error at character 4 of 4, near ``: Expected one of the following tokens: `(`; expression `:not` | Model [ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":not(": Unable to deserialize Node using fromNode method: Syntax error at character 5 of 5, near ``: Expected one of the following tokens: `*`, `-[`, `:`, `>`, `[`, `bigDecimal`, `bigInteger`, `blob`, `boolean`, `byte`, `collection`, `document`, `double`, `float`, `integer`, `list`, `long`, `map`, `member`, `number`, `operation`, `resource`, `service`, `set`, `short`, `simpleType`, `string`, `structure`, `timestamp`, `union`; expression `:not(` | Model [ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":not()": Unable to deserialize Node using fromNode method: Syntax error at character 5 of 6, near `)`: Expected one of the following tokens: `*`, `-[`, `:`, `>`, `[`, `bigDecimal`, `bigInteger`, `blob`, `boolean`, `byte`, `collection`, `document`, `double`, `float`, `integer`, `list`, `long`, `map`, `member`, `number`, `operation`, `resource`, `service`, `set`, `short`, `simpleType`, `string`, `structure`, `timestamp`, `union`; expression `:not()` | Model @@ -119,7 +119,6 @@ [ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":each(string, ": Unable to deserialize Node using fromNode method: Syntax error at character 14 of 14, near ``: Expected one of the following tokens: `*`, `-[`, `:`, `>`, `[`, `bigDecimal`, `bigInteger`, `blob`, `boolean`, `byte`, `collection`, `document`, `double`, `float`, `integer`, `list`, `long`, `map`, `member`, `number`, `operation`, `resource`, `service`, `set`, `short`, `simpleType`, `string`, `structure`, `timestamp`, `union`; expression `:each(string, ` | Model [ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":each(string, )": Unable to deserialize Node using fromNode method: Syntax error at character 14 of 15, near `)`: Expected one of the following tokens: `*`, `-[`, `:`, `>`, `[`, `bigDecimal`, `bigInteger`, `blob`, `boolean`, `byte`, `collection`, `document`, `double`, `float`, `integer`, `list`, `long`, `map`, `member`, `number`, `operation`, `resource`, `service`, `set`, `short`, `simpleType`, `string`, `structure`, `timestamp`, `union`; expression `:each(string, )` | Model [ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":each(string, :not())": Unable to deserialize Node using fromNode method: Syntax error at character 19 of 21, near `))`: Expected one of the following tokens: `*`, `-[`, `:`, `>`, `[`, `bigDecimal`, `bigInteger`, `blob`, `boolean`, `byte`, `collection`, `document`, `double`, `float`, `integer`, `list`, `long`, `map`, `member`, `number`, `operation`, `resource`, `service`, `set`, `short`, `simpleType`, `string`, `structure`, `timestamp`, `union`; expression `:each(string, :not())` | Model -[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "service -[foo]-> *": Unable to deserialize Node using fromNode method: Syntax error at character 10 of 18, near `foo]-> *`: Expected one of the following tokens: `bound`, `collectionOperation`, `create`, `delete`, `error`, `identifier`, `input`, `instanceOperation`, `list`, `member`, `operation`, `output`, `put`, `read`, `resource`, `update`; expression `service -[foo]-> *` | Model [ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo]": Unable to deserialize Node using fromNode method: Syntax error at character 1 of 5, near `foo]`: Expected one of the following tokens: `id`, `id|member`, `id|name`, `id|namespace`, `service|version`, `trait|`; expression `[foo]` | Model [ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo=baz]": Unable to deserialize Node using fromNode method: Syntax error at character 1 of 9, near `foo=baz]`: Expected one of the following tokens: `id`, `id|member`, `id|name`, `id|namespace`, `service|version`, `trait|`; expression `[foo=baz]` | Model [NOTE] ns.foo#Long: The long shape is not connected to from any service shape. | UnreferencedShape diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.json b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.json index ea48bdb429d..3596b8e69f2 100644 --- a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.json +++ b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.json @@ -274,7 +274,7 @@ "id": "invalid", "name": "EmitEachSelector", "configuration": { - "selector": "foo -[foo]->" + "selector": "foo -[input]->" } }, { @@ -578,13 +578,6 @@ "selector": "service -[resource]-> resource" } }, - { - "id": "noMatchInvalidRel", - "name": "EmitEachSelector", - "configuration": { - "selector": "service -[foo]-> *" - } - }, { "id": "any", "name": "EmitEachSelector", diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/selector/Parser.java b/smithy-model/src/main/java/software/amazon/smithy/model/selector/Parser.java index 0008c220058..54b9252efb8 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/selector/Parser.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/selector/Parser.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Set; import java.util.function.Function; +import java.util.logging.Logger; import software.amazon.smithy.model.neighbor.RelationshipType; import software.amazon.smithy.model.shapes.CollectionShape; import software.amazon.smithy.model.shapes.NumberShape; @@ -35,6 +36,8 @@ * Parses a selector expression. */ final class Parser { + + private static final Logger LOGGER = Logger.getLogger(Parser.class.getName()); private static final Set BREAK_TOKENS = SetUtils.of(',', ']', ')'); private static final Set REL_TYPES = new HashSet<>(); private static final List FUNCTIONS = ListUtils.of("test", "each", "of", "not"); @@ -155,7 +158,14 @@ private Selector parseMultiEdgeDirectedNeighbor() { String next; do { // Requires at least one relationship type. - relationships.add(expect(REL_TYPES)); + next = parseIdentifier(); + relationships.add(next); + // Tolerate unknown relationships, but log a warning. + if (!REL_TYPES.contains(next)) { + LOGGER.warning(String.format( + "Unknown relationship type '%s' found near %s. Expected one of: %s", + next, position - next.length(), REL_TYPES)); + } next = expect(MULTI_EDGE_NEXT_ARG_TOKEN); } while (!next.equals("]->")); return new NeighborSelector(relationships); diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/selector/NeighborSelectorTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/selector/NeighborSelectorTest.java index cf5a50ebc1a..18ca28438b0 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/selector/NeighborSelectorTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/selector/NeighborSelectorTest.java @@ -18,6 +18,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; import java.util.Set; import java.util.stream.Collectors; @@ -67,4 +68,18 @@ public void directedMultiEdgeTraversal() { assertThat(result, contains("smithy.example#Input", "smithy.example#Output")); } + + @Test + public void skipsUnknownRelationships() { + Set result = selectIds("operation -[input, output, foo]->"); + + assertThat(result, contains("smithy.example#Input", "smithy.example#Output")); + } + + @Test + public void returnsEmptyForUnknownRelationships() { + Set result = selectIds("operation -[foo]->"); + + assertThat(result, empty()); + } } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/selector/SelectorTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/selector/SelectorTest.java index 0242c61bb0c..2339c35b82d 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/selector/SelectorTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/selector/SelectorTest.java @@ -18,6 +18,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import java.util.Set; import org.junit.jupiter.api.Assertions; @@ -112,4 +113,12 @@ public void detectsUnclosedMultiEdgeNeighbor() { public void detectsUnclosedMultiEdgeNeighborTrailingComma() { Assertions.assertThrows(SelectorSyntaxException.class, () -> Selector.parse("operation -[input, ")); } + + @Test + public void toleratesUnexpectedRelationshipTypes() { + String expr = "operation -[foo]-> *"; + Selector selector = Selector.parse(expr); + + assertThat(expr, equalTo(selector.toString())); + } }