Skip to content

Commit

Permalink
Implement mixins
Browse files Browse the repository at this point in the history
This massive PR implements the mixins proposal.

The idea of redefining inherited members was removed, along with the
special `(inherit)` syntax. Feedback on this syntax was that it was
awkward. Redefining a member is now prohibited. instead, only apply is
allowed. To make it easier to apply multiple traits to a single shape,
particularly a member, a block form of apply was added in a previous
commit.

This change adds:
1. The ability to create mixins using the `@mixin` trait.
2. The ability to restrict which members are inherited by shapes that
   use a mixin using `localTraits` on the `@mixin` trait.
3. The ability to use a mixin with a shape in the IDL using `with`.
4. The ability to use a mixin in the JSON AST using `mixins`.
5. The ability to serialize mixins in the IDL and AST.
6. The ability to query mixin relationships in selectors using the
   `mixin` relationship. This also ensures that mixins are considered
   part of a service closure.
7. The ability to remove mixins from a model, and update all shapes that
   depend on the mixin.
8. The ability to "flatten" mixins out of the model, meaning that the
   mixin is removed, but any members or traits it provides to shapes
   that use it are copied onto those shapes.
9. Validation to ensure no mixin cycles or "with" statements are added
   that point to shapes that don't exist or that aren't marked with
   the `@mixin` trait.
10. Mixins are flattened out of models when converting to JSON Schema
    and to OpenAPI.
11. When a mixin is updated, any shape that uses that mixin is also
    updated.

Implementation notes:
* Mixins were added to the Shape type. This made it far easier to
  implement everything rather than introducing something like a
  `HasMixins` interface implemented on structure, union, and member
  shapes or using visitors. This design decision is similar to what was
  already done with the `members()` method on `Shape`, which just makes
  `Shape` generally easir to use.
* Shapes implicitly hold on to references to their mixins, but the mixin
  shapes are not directly exposed in the shape's API (only ShapeIds).
  This ensures that if we need to do any refactoring later, we can
  without making that a hard design requirement (it isn't so far!).
* Missing validation about member shapes matching their container IDs
  was added because it was needed to ensure mixin members were added
  correctly.
* An updated ListUtils.of() method was added for the case when there are
  two items. This was done because the `members()` method is now used
  each time a shape is created, which previously created an ArrayList
  each time the members of MapShape were requested.
  • Loading branch information
mtdowling committed Sep 22, 2021
1 parent 49b5c5d commit a89500f
Show file tree
Hide file tree
Showing 133 changed files with 4,269 additions and 389 deletions.
169 changes: 49 additions & 120 deletions designs/mixins.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,35 +114,43 @@ structure CityResourceInput {
}
```

The `with` clause of a structure or union statement is used to merge the members
of one or more mixins into a structure or union. Each shape ID in the `with`
clause MUST target a shape marked with the `@mixin` trait. Structure shapes can
only use structure mixins, and union shapes can only use union mixins.
Adding a mixin to a structure or union shape causes the members and traits of
a shape to be copied into the shape. Mixins can be added to a shape using
`with` followed by any number of shape IDs. Each shape ID MUST target a
shape marked with the `@mixin` trait. Structure shapes can only use structure
mixins, and union shapes can only use union mixins.

```
structure GetCityInput with CityResourceInput {}
structure GetCityInput with CityResourceInput {
foo: String
}
```

Multiple mixins can be applied:

```
structure GetCityInput with CityResourceInput, SomeOtherMixin {}
structure GetAnotherCityInput with
CityResourceInput
SomeOtherMixin
{
foo: String
}
```

Mixins can be composed of other mixins:

```
@mixin
structure A {
structure MixinA {
a: String
}
@mixin
structure B with A {
structure MixinB with MixinA {
b: String
}
structure C with B {
structure C with MixinB {
c: String
}
```
Expand Down Expand Up @@ -261,7 +269,9 @@ structure StructB {}
/// C
@threeTrait
@mixin
structure StructC with StructA, StructB {}
structure StructC with
StructA
StructB {}
/// D
@fourTrait
Expand Down Expand Up @@ -339,14 +349,9 @@ can be referred to outside of `smithy.example`.

The members and traits applied to members of a mixin are copied onto the target
shape. It is sometimes necessary to provide a more specific trait value for a
copied member or to add traits only to a specific copy of a member. This can be
achieved in the following ways:

* In the JSON AST using the `apply` type
* In the Smithy IDL using `apply` statements
* In the Smithy IDL by redefining a member and targeting the special
`(inherit)` keyword. This is simply syntactic sugar for using `apply`
statements.
copied member or to add traits only to a specific copy of a member. Traits can
be applied to these members in the JSON AST using the `apply` type and in the
Smithy IDL using `apply` statements.

> Note: using the `apply` type and `apply` statements on members that are
copied from mixin members _do not_ merge the applied trait value with the
Expand Down Expand Up @@ -425,65 +430,6 @@ apply MyStruct$mixinMember @documentation("Specific docs")
```


#### Redefining members in the IDL

As part of this proposal, we will add syntactic sugar in the Smithy IDL for
applying traits to copied mixin members. A mixin member can be redefined in
the Smithy IDL by targeting a special new structure and union member target
keyword named `(inherit)` that indicates the member redefines a mixin member
and inherits its type from a mixin. Redefining a member in this way is simply
syntactic sugar for using `apply` statements and carries no additional
semantic meaning.

For example, the previous example can be defined in the Smithy IDL by
redefining the member instead of using `apply`:

```
$version: "1.1"
namespace smithy.example
@mixin
structure MyMixin {
/// Generic docs
mixinMember: String
}
structure MyStruct with MyMixin {
/// Specific docs
mixinMember: (inherit)
}
```

`MyStruct` is equivalent to the following flattened structure:

```smithy
structure MyStruct {
/// Specific docs
mixinMember: String
}
```

To redefine a mixin member:

1. The redefined member MUST have the exact same name as the mixin member.
2. The redefined member MUST target a special keyword type named `(inherit)`
to indicate the member redefines and inherits its type from a mixin member.
3. Just like replacing traits on the containing shape, any trait applied
to a redefined member completely replaces any resolved traits applied to
the mixin member.
4. Redefining mixin members has no bearing on the ordering of members in
structures that use mixins.

**`(inherit)` keyword**

`(inherit)` is a special keyword for union and structure member targets
available in the Smithy IDL. It can only be used when a structure or union
member is defined by one of the mixins applied to the shape. It is an error
to target this type in any other case. Note that `(inherit)` _is not_ a
shape ID. It is a keyword supported only in the Smithy IDL and _is not_
supported in the JSON AST.


### Mixins are not code generated

Mixins are an implementation detail of models and are only intended to reduce
Expand Down Expand Up @@ -555,7 +501,9 @@ structure A2 {
a: String
}
structure Invalid with A1, A2 {}
structure Invalid with
A1
A2 {}
```

The following model is also invalid, but not specifically because of mixins.
Expand All @@ -573,35 +521,27 @@ structure A2 {
A: String
}
structure Invalid with A1, A2 {}
structure Invalid with
A1
A2 {}
```


### Mixins in the IDL

To support mixins, `structure_statement` and `union_statement` ABNF rules
will be updated to contain an optional `with` clause that comes after the
shape name and before `{`. If present, the `with` clause MUST contain one or
more shape IDs, and each shape MUST target a valid shape marked with the
`@mixin` trait. The `(inherit)` keyword also requires updating the Smithy
IDL grammar for structure and union shapes.
will be updated to contain an optional `mixins` production
that comes after the shape name and before `{`. Each shape ID referenced in
the `mixins` production MUST target a shape of the same type as the
shape being defined and MUST be marked with the `@mixin` trait.

```
structure_statement = "structure" ws `identifier` ws [with_clause] structure_members
structure_members: "{" `ws` *(`structure_members_kvp` `ws`) "}"
structure_members_kvp: `trait_statements` `identifier` `ws`
":" `ws` `structure_and_union_member_target` [`required_member_sugar`]
with_clause = "with" ws 1*shape_id ws
structure_and_union_member_target: `shape_id` / `inherit_keyword`
inherit_keyword: "(inherit)"
union_statement = "union" ws `identifier` ws [with_clause] union_members
union_members: "{" `ws` *(`union_members_kvp` `ws`) "}"
union_members_kvp: `trait_statements` `identifier` `ws`
":" `ws` `structure_and_union_member_target`
structure_statement = "structure" ws identifier ws [mixins ws] structure_members
union_statement = "union" ws identifier ws [mixins ws] union_members
mixins = "with" 1*(ws shape_id)
```


### Mixins in the JSON AST

Mixins are defined in the JSON AST using the `mixins` property of structure and
Expand Down Expand Up @@ -708,13 +648,8 @@ Both yield the following shapes, in any order:

The order of structure and union members is important for languages like C
that require a stable ABI. Mixins provide a deterministic member ordering.
Members are ordered by:

- Members inherited from mixins come before members defined directly in the
shape.
- Members are inherited from mixins in the order in which they are first
applied to a shape. Redefining a member in the Smithy IDL using
`(inherit)` has no bearing on member order.
Members inherited from mixins come before members defined directly in the
shape.

Members are ordered in a kind of depth-first, preorder traversal of mixins
that are applied to a structure or union. To resolve the member order of a
Expand All @@ -741,7 +676,10 @@ structure PaginatedInput {
pageSize: Integer
}
structure ListSomethingInput with PaginatedInput, FilteredByName {
structure ListSomethingInput with
PaginatedInput
FilteredByName
{
sizeFilter: Integer
}
```
Expand Down Expand Up @@ -801,13 +739,6 @@ both mixin members and members local to the shape are returned. This reduces
the complexity of code generators and will prevent issues with code generators
forgetting to traverse mixins to resolve members.

When loading Smithy IDL models that use `(inherit)`, the model loader should
treat the member exactly like an `apply` statement.

When serializing a Smithy model using the IDL, the serialized IDL should
use the redefined member syntax rather than `apply` statements to make the
serialized model more readable.

The reference implementation will contain a model transformation that can
"flatten" mixins out of a model so that they do not need to be accounted for
in code generators and other tooling that performs exogenous model
Expand Down Expand Up @@ -933,15 +864,15 @@ structure FooMixin {
structure ApplicationOfFooMixin with FooMixin {
// Remove the required trait from this member.
@override([omitTraits: [required]])
someMember: (inherit)
someMember: String
}
```

While this could work, it is not strictly required and presents two
tradeoffs: the application of the mixin has significantly more verbose, it
requires introducing an `@override` trait that communicates exactly what
`(inherit)` already communicates, and filtering traits adds more complex
requirements to Smithy implementations that resolve the traits of a structure.
requires introducing an `@override` trait, and filtering traits adds more
complex requirements to Smithy implementations that resolve the traits of a
structure.

Alternatively, multiple levels of mixins can be used in many cases to allow
for reuse with more flexibility. For example:
Expand All @@ -954,10 +885,8 @@ structure FooMixinOptional {
}
@mixin
structure FooMixinRequired with FooMixinOptional {
@required
someMember: (inherit)
}
structure FooMixinRequired with FooMixinOptional {}
apply FooMixinRequired$someMember @required
```


Expand Down
18 changes: 18 additions & 0 deletions docs/source/1.0/guides/style-guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,24 @@ Smithy models SHOULD resemble the following example:
@trait(selector: "string")
structure myTrait {}
// Structures with no members place the braces on the same line.
@mixin
structure MyMixin {}
// When using a single mixin, place "with" and the shape on the same line
structure UsesMixin with MyMixin {
foo: String
}
// When using multiple mixins, place each shape ID on its own line,
// followed by a line that contains the opening brace.
structure UsesMixin with
MyMixin
SomeOtherMixin
{
foo: String
}
* Each statement should appear on its own line.


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ private JsonSchemaConverter(Builder builder) {
mappers.addAll(builder.mappers);
config = SmithyBuilder.requiredState("config", builder.config);
propertyNamingStrategy = SmithyBuilder.requiredState("propertyNamingStrategy", builder.propertyNamingStrategy);
model = SmithyBuilder.requiredState("model", builder.model);

// Flatten mixins out of the model before using the model at all. Mixins are
// not relevant to JSON Schema documents.
Model builderModel = SmithyBuilder.requiredState("model", builder.model);
model = ModelTransformer.create().flattenAndRemoveMixins(builderModel);

shapePredicate = builder.shapePredicate;

LOGGER.fine("Building filtered JSON schema shape index");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.junit.jupiter.api.Test;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.node.NodePointer;
import software.amazon.smithy.model.shapes.BigDecimalShape;
import software.amazon.smithy.model.shapes.BigIntegerShape;
import software.amazon.smithy.model.shapes.BlobShape;
Expand Down Expand Up @@ -571,4 +572,26 @@ public void setBaz(String baz) {
this.baz = baz;
}
}

@Test
public void removesMixins() {
Model model = Model.assembler()
.addImport(getClass().getResource("model-with-mixins.smithy"))
.assemble()
.unwrap();
JsonSchemaConverter converter = JsonSchemaConverter.builder()
.model(model)
.build();

NodePointer mixin = NodePointer.parse("/definitions/Mixin");
NodePointer properties = NodePointer.parse("/definitions/UsesMixin/properties");
SchemaDocument document = converter.convert();

// Mixin isn't there.
assertThat(mixin.getValue(document.toNode()), equalTo(Node.nullNode()));

// The mixin was flattened.
assertThat(properties.getValue(document.toNode()).expectObjectNode().getStringMap().keySet(),
containsInAnyOrder("foo", "baz"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
$version: "1.1"

namespace smithy.example

@mixin
structure Mixin {
foo: String
}

structure UsesMixin with Mixin {
baz: String
}
Loading

0 comments on commit a89500f

Please sign in to comment.