From 3d334bdebdea6bbc00018d74853b287a1124bc44 Mon Sep 17 00:00:00 2001 From: Hunter Mellema Date: Tue, 21 Feb 2023 08:02:38 -0700 Subject: [PATCH 1/4] fix smithy validate throwing warning on operation mixins --- .../smithy/model/loader/AstModelLoader.java | 2 +- .../model/loader/AstModelLoaderTest.java | 10 +++ .../model/loader/mixins/operation-mixins.json | 77 +++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/operation-mixins.json diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java index 3f0f70c1168..5010b2dc484 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java @@ -83,7 +83,7 @@ final class AstModelLoader { private static final Set MEMBER_PROPERTIES = SetUtils.of(TARGET, TRAITS); private static final Set REFERENCE_PROPERTIES = SetUtils.of(TARGET); private static final Set OPERATION_PROPERTY_NAMES = SetUtils.of( - TYPE, "input", "output", ERRORS, TRAITS); + TYPE, "input", "output", ERRORS, TRAITS, MIXINS); private static final Set RESOURCE_PROPERTIES = SetUtils.of( TYPE, "create", "read", "update", "delete", "list", "put", "identifiers", "resources", "operations", "collectionOperations", "properties", TRAITS); diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java index 561f0238072..d71ff9144f0 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java @@ -43,4 +43,14 @@ public void doesNotFailOnEmptyApply() { .assemble() .unwrap(); } + + @Test + public void allowsMixinsOnOperationsWithoutWarningOrError() { + ValidatedResult model = Model.assembler() + .addImport(getClass().getResource("mixins/operation-mixins.json")) + .assemble(); + assertEquals(0, model.getValidationEvents(Severity.WARNING).size()); + assertEquals(0, model.getValidationEvents(Severity.ERROR).size()); + } + } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/operation-mixins.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/operation-mixins.json new file mode 100644 index 00000000000..cdb781b55cc --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/operation-mixins.json @@ -0,0 +1,77 @@ +{ + "smithy": "2.0", + "shapes": { + "example.weather.errors#ThrottlingException": { + "type": "structure", + "members": {}, + "traits": { + "smithy.api#error": "server" + } + }, + "example.weather.errors#ValidationException": { + "type": "structure", + "members": {}, + "traits": { + "smithy.api#error": "client" + } + }, + "example.weather.mixins#OperationMixin": { + "type": "operation", + "input": { + "target": "smithy.api#Unit" + }, + "output": { + "target": "smithy.api#Unit" + }, + "errors": [ + { + "target": "example.weather.errors#ThrottlingException" + }, + { + "target": "example.weather.errors#ValidationException" + } + ], + "traits": { + "smithy.api#mixin": {} + } + }, + "example.weather.operations#OperationWithMixin": { + "type": "operation", + "mixins": [ + { + "target": "example.weather.mixins#OperationMixin" + } + ], + "input": { + "target": "example.weather.operations#OperationWithMixinInput" + }, + "output": { + "target": "smithy.api#Unit" + }, + "traits": { + "smithy.api#http": { + "method": "POST", + "uri": "/my/resource/uri/{myInputField}" + } + } + }, + "example.weather.operations#OperationWithMixinInput": { + "type": "structure", + "members": { + "myInputField": { + "target": "smithy.api#String", + "traits": { + "smithy.api#httpLabel": {}, + "smithy.api#required": {} + } + }, + "other": { + "target": "smithy.api#String" + } + }, + "traits": { + "smithy.api#input": {} + } + } + } +} \ No newline at end of file From 1767293a670d18e784c57d993d4924b574e6975f Mon Sep 17 00:00:00 2001 From: Hunter Mellema Date: Tue, 21 Feb 2023 08:28:26 -0700 Subject: [PATCH 2/4] fix validation for resource shapes with mixin to no longer throw warning --- .../smithy/model/loader/AstModelLoader.java | 2 +- .../model/loader/AstModelLoaderTest.java | 9 ++++ .../model/loader/mixins/resource-mixins.json | 46 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/resource-mixins.json diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java index 5010b2dc484..176158500ff 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java @@ -86,7 +86,7 @@ final class AstModelLoader { TYPE, "input", "output", ERRORS, TRAITS, MIXINS); private static final Set RESOURCE_PROPERTIES = SetUtils.of( TYPE, "create", "read", "update", "delete", "list", "put", - "identifiers", "resources", "operations", "collectionOperations", "properties", TRAITS); + "identifiers", "resources", "operations", "collectionOperations", "properties", TRAITS, MIXINS); private static final Set SERVICE_PROPERTIES = SetUtils.of( TYPE, "version", "operations", "resources", "rename", ERRORS, TRAITS); diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java index d71ff9144f0..8ea5c513470 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java @@ -53,4 +53,13 @@ public void allowsMixinsOnOperationsWithoutWarningOrError() { assertEquals(0, model.getValidationEvents(Severity.ERROR).size()); } + @Test + public void allowsMixinsOnResourcesWithoutWarningOrError() { + ValidatedResult model = Model.assembler() + .addImport(getClass().getResource("mixins/resource-mixins.json")) + .assemble(); + assertEquals(0, model.getValidationEvents(Severity.WARNING).size()); + assertEquals(0, model.getValidationEvents(Severity.ERROR).size()); + } + } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/resource-mixins.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/resource-mixins.json new file mode 100644 index 00000000000..c567ff296ff --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/resource-mixins.json @@ -0,0 +1,46 @@ +{ + "smithy": "2.0", + "shapes": { + "example.weather.resources#DummyOperation": { + "type": "operation", + "input": { + "target": "example.weather.resources#DummyOperationInput" + }, + "output": { + "target": "smithy.api#Unit" + }, + "traits": { + } + }, + "example.weather.resources#DummyOperationInput": { + "type": "structure", + "members": { + "dummyInput": { + "target": "smithy.api#String" + } + }, + "traits": { + "smithy.api#input": {} + } + }, + "example.weather.mixins#ResourceMixin": { + "type": "resource", + "traits": { + "smithy.api#mixin": {} + } + }, + "example.weather.resources#ResourceWithMixin": { + "type": "resource", + "mixins": [ + { + "target": "example.weather.mixins#ResourceMixin" + } + ], + "operations": [ + { + "target": "example.weather.resources#DummyOperation" + } + ] + } + } +} \ No newline at end of file From a91bae579bfbcde7986dd54be1cc15f473fe449f Mon Sep 17 00:00:00 2001 From: Hunter Mellema Date: Wed, 22 Feb 2023 09:43:50 -0700 Subject: [PATCH 3/4] also fix service, resource, collection, and simple shapes --- .../smithy/model/loader/AstModelLoader.java | 8 +-- .../model/loader/AstModelLoaderTest.java | 58 ++++++++++++++++--- .../loader/mixins/collection-mixins.json | 50 ++++++++++++++++ .../model/loader/mixins/service-mixins.json | 44 ++++++++++++++ .../loader/mixins/simple-shape-mixins.json | 40 +++++++++++++ .../model/loader/mixins/structure-mixins.json | 29 ++++++++++ .../model/loader/mixins/union-mixins.json | 32 ++++++++++ 7 files changed, 250 insertions(+), 11 deletions(-) create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/collection-mixins.json create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/service-mixins.json create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/simple-shape-mixins.json create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/structure-mixins.json create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/union-mixins.json diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java index 176158500ff..98d4ce476b3 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java @@ -76,10 +76,10 @@ final class AstModelLoader { private static final List TOP_LEVEL_PROPERTIES = ListUtils.of("smithy", SHAPES, METADATA); private static final List APPLY_PROPERTIES = ListUtils.of(TYPE, TRAITS); - private static final List SIMPLE_PROPERTY_NAMES = ListUtils.of(TYPE, TRAITS); + private static final List SIMPLE_PROPERTY_NAMES = ListUtils.of(TYPE, TRAITS, MIXINS); private static final List NAMED_MEMBER_SHAPE_PROPERTY_NAMES = ListUtils.of(TYPE, MEMBERS, TRAITS, MIXINS); - private static final List COLLECTION_PROPERTY_NAMES = ListUtils.of(TYPE, "member", TRAITS); - private static final List MAP_PROPERTY_NAMES = ListUtils.of(TYPE, "key", "value", TRAITS); + private static final List COLLECTION_PROPERTY_NAMES = ListUtils.of(TYPE, "member", TRAITS, MIXINS); + private static final List MAP_PROPERTY_NAMES = ListUtils.of(TYPE, "key", "value", TRAITS, MIXINS); private static final Set MEMBER_PROPERTIES = SetUtils.of(TARGET, TRAITS); private static final Set REFERENCE_PROPERTIES = SetUtils.of(TARGET); private static final Set OPERATION_PROPERTY_NAMES = SetUtils.of( @@ -88,7 +88,7 @@ final class AstModelLoader { TYPE, "create", "read", "update", "delete", "list", "put", "identifiers", "resources", "operations", "collectionOperations", "properties", TRAITS, MIXINS); private static final Set SERVICE_PROPERTIES = SetUtils.of( - TYPE, "version", "operations", "resources", "rename", ERRORS, TRAITS); + TYPE, "version", "operations", "resources", "rename", ERRORS, TRAITS, MIXINS); private final Version modelVersion; private final ObjectNode model; diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java index 8ea5c513470..51d1d29a792 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/AstModelLoaderTest.java @@ -27,21 +27,21 @@ public class AstModelLoaderTest { @Test public void failsToLoadPropertiesFromV1() { ValidatedResult model = Model.assembler() - .addImport(getClass().getResource("invalid/properties-v2-only.json")) - .assemble(); + .addImport(getClass().getResource("invalid/properties-v2-only.json")) + .assemble(); assertEquals(1, model.getValidationEvents(Severity.ERROR).size()); assertTrue(model.getValidationEvents(Severity.ERROR).get(0).getMessage() - .contains("Resource properties can only be used with Smithy version 2 or later.")); + .contains("Resource properties can only be used with Smithy version 2 or later.")); } @Test public void doesNotFailOnEmptyApply() { // Empty apply statements are pointless but shouldn't break the loader. Model.assembler() - .addImport(getClass().getResource("ast-empty-apply-1.json")) - .addImport(getClass().getResource("ast-empty-apply-2.json")) - .assemble() - .unwrap(); + .addImport(getClass().getResource("ast-empty-apply-1.json")) + .addImport(getClass().getResource("ast-empty-apply-2.json")) + .assemble() + .unwrap(); } @Test @@ -62,4 +62,48 @@ public void allowsMixinsOnResourcesWithoutWarningOrError() { assertEquals(0, model.getValidationEvents(Severity.ERROR).size()); } + @Test + public void allowsMixinsOnServiceWithoutWarningOrError() { + ValidatedResult model = Model.assembler() + .addImport(getClass().getResource("mixins/service-mixins.json")) + .assemble(); + assertEquals(0, model.getValidationEvents(Severity.WARNING).size()); + assertEquals(0, model.getValidationEvents(Severity.ERROR).size()); + } + + @Test + public void allowsMixinsOnStructuresWithoutWarningOrError() { + ValidatedResult model = Model.assembler() + .addImport(getClass().getResource("mixins/structure-mixins.json")) + .assemble(); + assertEquals(0, model.getValidationEvents(Severity.WARNING).size()); + assertEquals(0, model.getValidationEvents(Severity.ERROR).size()); + } + + @Test + public void allowsMixinsOnUnionsWithoutWarningOrError() { + ValidatedResult model = Model.assembler() + .addImport(getClass().getResource("mixins/union-mixins.json")) + .assemble(); + assertEquals(0, model.getValidationEvents(Severity.WARNING).size()); + assertEquals(0, model.getValidationEvents(Severity.ERROR).size()); + } + + @Test + public void allowsMixinsOnSimpleShapesWithoutWarningOrError() { + ValidatedResult model = Model.assembler() + .addImport(getClass().getResource("mixins/simple-shape-mixins.json")) + .assemble(); + assertEquals(0, model.getValidationEvents(Severity.WARNING).size()); + assertEquals(0, model.getValidationEvents(Severity.ERROR).size()); + } + + @Test + public void allowsMixinsOnCollectionShapesWithoutWarningOrError() { + ValidatedResult model = Model.assembler() + .addImport(getClass().getResource("mixins/collection-mixins.json")) + .assemble(); + assertEquals(0, model.getValidationEvents(Severity.WARNING).size()); + assertEquals(0, model.getValidationEvents(Severity.ERROR).size()); + } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/collection-mixins.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/collection-mixins.json new file mode 100644 index 00000000000..faad2450d79 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/collection-mixins.json @@ -0,0 +1,50 @@ +{ + "smithy": "2.0", + "shapes": { + "smithy.example#MyList": { + "type": "list", + "mixins": [ + { + "target": "smithy.example#MyListMixin" + } + ] + }, + "smithy.example#MyListMixin": { + "type": "list", + "member": { + "target": "smithy.api#String" + }, + "traits": { + "smithy.api#length": { + "min": 2, + "max": 4 + }, + "smithy.api#mixin": {} + } + }, + "smithy.example#MyMap": { + "type": "map", + "mixins": [ + { + "target": "smithy.example#MyMapMixin" + } + ] + }, + "smithy.example#MyMapMixin": { + "type": "map", + "key": { + "target": "smithy.api#String" + }, + "value": { + "target": "smithy.api#String" + }, + "traits": { + "smithy.api#length": { + "min": 1, + "max": 4 + }, + "smithy.api#mixin": {} + } + } + } +} \ No newline at end of file diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/service-mixins.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/service-mixins.json new file mode 100644 index 00000000000..fde05b02050 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/service-mixins.json @@ -0,0 +1,44 @@ +{ + "smithy": "2.0", + "shapes": { + "smithy.example#MyDummyOperation": { + "type": "operation", + "input": { + "target": "smithy.example#MyDummyOperationInput" + }, + "output": { + "target": "smithy.api#Unit" + } + }, + "smithy.example#MyDummyOperationInput": { + "type": "structure", + "members": { + "foo": { + "target": "smithy.api#String" + } + }, + "traits": { + "smithy.api#input": {} + } + }, + "smithy.example#MyMixin": { + "type": "service", + "operations": [ + { + "target": "smithy.example#MyDummyOperation" + } + ], + "traits": { + "smithy.api#mixin": {} + } + }, + "smithy.example#MyService": { + "type": "service", + "mixins": [ + { + "target": "smithy.example#MyMixin" + } + ] + } + } +} \ No newline at end of file diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/simple-shape-mixins.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/simple-shape-mixins.json new file mode 100644 index 00000000000..3043d5e93d4 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/simple-shape-mixins.json @@ -0,0 +1,40 @@ +{ + "smithy": "2.0", + "shapes": { + "smithy.example#MyIntMixin": { + "type": "integer", + "traits": { + "smithy.api#mixin": {}, + "smithy.api#range": { + "min": 1 + } + } + }, + "smithy.example#MyStringMixin": { + "type": "string", + "traits": { + "smithy.api#length": { + "min": 2, + "max": 4 + }, + "smithy.api#mixin": {} + } + }, + "smithy.example#MyTestInt": { + "type": "integer", + "mixins": [ + { + "target": "smithy.example#MyIntMixin" + } + ] + }, + "smithy.example#MyTestString": { + "type": "string", + "mixins": [ + { + "target": "smithy.example#MyStringMixin" + } + ] + } + } +} \ No newline at end of file diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/structure-mixins.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/structure-mixins.json new file mode 100644 index 00000000000..fc001548850 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/structure-mixins.json @@ -0,0 +1,29 @@ +{ + "smithy": "2.0", + "shapes": { + "smithy.example#Baz": { + "type": "structure", + "mixins": [ + { + "target": "smithy.example#MyMixin" + } + ], + "members": { + "foo": { + "target": "smithy.api#String" + } + } + }, + "smithy.example#MyMixin": { + "type": "structure", + "members": { + "bar": { + "target": "smithy.api#String" + } + }, + "traits": { + "smithy.api#mixin": {} + } + } + } +} \ No newline at end of file diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/union-mixins.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/union-mixins.json new file mode 100644 index 00000000000..c4c7e186025 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/union-mixins.json @@ -0,0 +1,32 @@ +{ + "smithy": "2.0", + "shapes": { + "smithy.example#MyMixin": { + "type": "union", + "members": { + "bar": { + "target": "smithy.api#String" + } + }, + "traits": { + "smithy.api#mixin": {} + } + }, + "smithy.example#MyUnion": { + "type": "union", + "mixins": [ + { + "target": "smithy.example#MyMixin" + } + ], + "members": { + "baz": { + "target": "smithy.api#String" + } + }, + "traits": { + "smithy.api#mixin": {} + } + } + } +} \ No newline at end of file From 76e730dad57b78a8d3e9e98ebb62e3f1e272e024 Mon Sep 17 00:00:00 2001 From: Hunter Mellema Date: Wed, 22 Feb 2023 11:04:06 -0700 Subject: [PATCH 4/4] Update smithy-model error files to reflect updated warning message --- .../errorfiles/loader/map-warns-additional-properties.errors | 2 +- .../errorfiles/loader/shapes-warn-additional-properties.errors | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/map-warns-additional-properties.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/map-warns-additional-properties.errors index b430ae3b444..9650851199a 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/map-warns-additional-properties.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/map-warns-additional-properties.errors @@ -1 +1 @@ -[WARNING] smithy.example#Hi: Expected an object with possible properties of `key`, `traits`, `type`, `value`, but found additional properties: `foo` | Model +[WARNING] smithy.example#Hi: Expected an object with possible properties of `key`, `mixins`, `traits`, `type`, `value`, but found additional properties: `foo` | Model diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/shapes-warn-additional-properties.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/shapes-warn-additional-properties.errors index f3626829572..8ada31b1540 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/shapes-warn-additional-properties.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/shapes-warn-additional-properties.errors @@ -1 +1 @@ -[WARNING] smithy.example#Hi: Expected an object with possible properties of `traits`, `type`, but found additional properties: `invalid` | Model +[WARNING] smithy.example#Hi: Expected an object with possible properties of `mixins`, `traits`, `type`, but found additional properties: `invalid` | Model