Skip to content

Commit

Permalink
Add ability to elide targets for mixins/resources
Browse files Browse the repository at this point in the history
This adds in syntactic sugar that allows modelers to elide targets
for members that are inherited from mixins or defined in resources.

This necessarily also adds the ability for a structure to indicate
a resource to pull potential targets from.

This is all syntactic sugar for the IDL, so the JSON AST isn't
impacted.

This is accomplished by refactoring the code used for mixins to
instead be a generic system for applying modifications to a shape
after some set of dependencies has been resolved.
  • Loading branch information
JordonPhillips committed May 20, 2022
1 parent 1922f41 commit 430ef61
Show file tree
Hide file tree
Showing 20 changed files with 966 additions and 89 deletions.
77 changes: 60 additions & 17 deletions docs/source/1.0/guides/migrating-idl-1-to-2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,6 @@ having to copy and paste them. The following model:
namespace smithy.example
resource User {
identifiers: {
email: String,
id: String,
},
read: GetUser
}
operation GetUser {
input: GetUserInput,
output: GetUserOutput
Expand Down Expand Up @@ -240,15 +232,6 @@ Can be updated to:
namespace smithy.example
resource User {
identifiers: {
email: String
id: String
username: String
},
read: GetUser
}
@mixin
structure UserIdentifiers {
@required
Expand All @@ -274,6 +257,66 @@ that otherwise have to be copied and pasted.
work.


Use the target elision syntax sugar to reduce boilerplate
---------------------------------------------------------

Resource shapes contain a set of identifiers, but when writing structures that
contain those identifiers you have to duplicate those definitions entirely. In
IDL 2.0, you can use the target elision syntax with a structure bound to a
resource. For example:

.. code-block:: smithy
$version: "2.0"
namespace smithy.example
resource User {
identifiers: {
id: String
email: String
}
}
// The `for` syntax here determines which resource should be checked.
structure UserDetails for User {
// With this syntax, the target is automatically inferred from the
// resource.
$id
// Uncomment this to include an email member. Unlike with mixins, you
// must opt in to the members that you want to include. This allows you
// to have partial views of a resource, such as in a create operation
// that does not bind all of the identifiers.
// $email
address: String
}
This syntax can also be used with mixins to more succinctly add additional
traits to included members.

.. code-block:: smithy
$version: "2.0"
namespace smithy.example
@mixin
structure UserIdentifiers {
id: String
email: String
}
structure UserDetails with [UserIdentifiers] {
@required
$id
@required
$email
}
Remove unsightly commas
-----------------------

Expand Down
109 changes: 106 additions & 3 deletions docs/source/1.0/spec/core/idl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,14 @@ The Smithy IDL is defined by the following ABNF:
enum_shape_statement :`enum_type_name` `ws` `identifier` [`mixins`] `ws` `enum_shape_members`
enum_type_name :"enum" / "intEnum"
enum_shape_members :"{" `ws` 1*(`trait_statements` `identifier` `ws`) "}"
shape_members :"{" `ws` *(`shape_member_kvp` `ws`) "}"
shape_member_kvp :`trait_statements` `identifier` `ws` ":" `ws` `shape_id`
shape_members :"{" `ws` *(`trait_statements` `shape_member` `ws`) "}"
shape_member :`shape_member_kvp` / `shape_member_elided`
shape_member_kvp :`identifier` `ws` ":" `ws` `shape_id`
shape_member_elided :"$" `identifier`
list_statement :"list" `ws` `identifier` [`mixins`] `ws` `shape_members`
set_statement :"set" `ws` `identifier` [`mixins`] `ws` `shape_members`
map_statement :"map" `ws` `identifier` [`mixins`] `ws` `shape_members`
structure_statement :"structure" `ws` `identifier` [`mixins`] `ws` `shape_members`
structure_statement :"structure" `ws` `identifier` ["for" `shape_id`] [`mixins`] `ws` `shape_members`
union_statement :"union" `ws` `identifier` [`mixins`] `ws` `shape_members`
service_statement :"service" `ws` `identifier` [`mixins`] `ws` `node_object`
operation_statement :"operation" `ws` `identifier` [`mixins`] `ws` `inlineable_properties`
Expand Down Expand Up @@ -1408,6 +1410,11 @@ and defines a :ref:`read <read-lifecycle>` operation:
}
}

.. seealso::

The :ref:`target elision syntax <idl-target-elision>` for an easy way to
define structures that reference resource identifiers without having to
repeat the target definition.

.. _idl-mixins:

Expand Down Expand Up @@ -1438,6 +1445,102 @@ For example:
string SensitiveText with [SensitiveString]
.. _idl-target-elision:

Target Elision
--------------

Having to completely redefine a :ref:`resource identifier <resource-identifiers>`
to use it in a structure or redefine a member from a :ref:`mixin` to add
additional traits can be cumbersome and potentially error-prone. The
:token:`type elision syntax <smithy:shape_member_elided>` can be used to cut
down on that repetition by prefixing the member name with a ``$``. If a member
is prefixed this way, its target will automatically be set to the target of a
mixin member with the same name. The following example shows how to elide the
target for a member inherited from a mixin:

.. code-block:: smithy
$version: "2.0"
namespace smithy.example
@mixin
structure IdBearer {
id: String
}
structure IdRequired with [IdBearer] {
@required
$id
}
Additionally, structure shapes can reference a :ref:`resource <idl-resource>`
shape to define members representing the resource's identifiers without having
to redefine the target shape. In addition to prefixing a member with ``$``, the
structure must also add ``for`` followed by the resource being referenced in
the shape's definition before any mixins are specified. When using a resource
reference, any member prefixed with ``$`` will first check for an identifier
from that resource whose name matches the member name and use its target. If no
identifier matches, mixins will then be checked. The following example shows a
structure re-using an identifier definition from a resource:

.. code-block:: smithy
$version: "2.0"
namespace smithy.example
resource User {
identifiers: {
name: String
uuid: String
}
}
structure UserSummary for User {
$name
age: Short
}
Note that the ``UserSummary`` structure does not attempt to define the
``uuid`` identifier. When referencing a resource in this way, only the
identifiers that are explicitly referenced are added to the structure. This
allows structures to define subsets of identifiers, which can be useful for
operations like create operations where some of those identifiers may be
generated by the service.

Structures may only reference one resource shape in this way.

When using both mixins and a resource reference, the referenced resource will
be checked first. This means that the following example would be invalid:

.. code-block:: smithy
$version: "2.0"
namespace smithy.example
resource User {
identifiers: {
uuid: String
}
}
@mixin
structure UserIdentifiers {
uuid: Blob
}
// This is invalid because the `uuid` member's target is set to
// String, which then conflicts with the UserIdentifiers mixin.
structure UserSummary for User with [UserIdentifiers] {
$uuid
}
.. _documentation-comment:

Documentation comment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ abstract class AbstractMutableModelFile implements ModelFile {
private final Set<ShapeId> allShapeIds = new HashSet<>();
private final Map<ShapeId, AbstractShapeBuilder<?, ?>> shapes = new LinkedHashMap<>();
private final Map<ShapeId, Map<String, MemberShape.Builder>> members = new HashMap<>();
private final Map<ShapeId, Set<ShapeId>> pendingShapes = new HashMap<>();
private final Map<ShapeId, List<PendingShapeModifier>> pendingModifications = new HashMap<>();
private final Map<ShapeId, Set<ShapeId>> pendingDependencies = new HashMap<>();
private final List<ValidationEvent> events = new ArrayList<>();
private final MetadataContainer metadata = new MetadataContainer(events);
private final TraitFactory traitFactory;
Expand Down Expand Up @@ -106,7 +107,13 @@ void onShape(AbstractShapeBuilder<?, ?> builder) {
}

void addPendingMixin(ShapeId shape, ShapeId mixin) {
pendingShapes.computeIfAbsent(shape, id -> new LinkedHashSet<>()).add(mixin);
addPendingModification(shape, new ApplyMixin(mixin, events));
}

void addPendingModification(ShapeId shape, PendingShapeModifier pendingModification) {
pendingDependencies.computeIfAbsent(shape, id -> new LinkedHashSet<>())
.addAll(pendingModification.getDependencies());
pendingModifications.computeIfAbsent(shape, id -> new ArrayList<>()).add(pendingModification);
}

private SourceException onConflict(AbstractShapeBuilder<?, ?> builder, AbstractShapeBuilder<?, ?> previous) {
Expand Down Expand Up @@ -181,13 +188,15 @@ public final CreatedShapes createShapes(TraitContainer resolvedTraits) {
List<Shape> resolvedShapes = new ArrayList<>(shapes.size());
List<PendingShape> pendingMixins = new ArrayList<>();

for (Map.Entry<ShapeId, Set<ShapeId>> entry : pendingShapes.entrySet()) {
for (Map.Entry<ShapeId, List<PendingShapeModifier>> entry : this.pendingModifications.entrySet()) {
ShapeId subject = entry.getKey();
Set<ShapeId> mixins = entry.getValue();
List<PendingShapeModifier> pendingModifications = entry.getValue();
Set<ShapeId> dependencies = pendingDependencies.getOrDefault(subject, Collections.emptySet());
AbstractShapeBuilder<?, ?> builder = shapes.get(entry.getKey());
Map<String, MemberShape.Builder> builderMembers = claimMembersOfContainer(builder.getId());
shapes.remove(entry.getKey());
pendingMixins.add(createPendingShape(subject, builder, builderMembers, mixins, traitContainer));
pendingMixins.add(createPendingShape(
subject, builder, builderMembers, dependencies, pendingModifications, traitContainer));
}

// Build members and add them to top-level shapes.
Expand Down Expand Up @@ -223,55 +232,21 @@ private PendingShape createPendingShape(
AbstractShapeBuilder<?, ?> builder,
Map<String, MemberShape.Builder> builderMembers,
Set<ShapeId> mixins,
List<PendingShapeModifier> pendingModifications,
TraitContainer resolvedTraits
) {
return PendingShape.create(subject, builder, mixins, shapeMap -> {
// Build normal members first.
for (MemberShape.Builder memberBuilder : builderMembers.values()) {
for (PendingShapeModifier pendingModification : pendingModifications) {
pendingModification.modifyMember(builder, memberBuilder, resolvedTraits, shapeMap);
}
buildShape(memberBuilder, resolvedTraits).ifPresent(builder::addMember);
}
// Add each mixin and ensure there are no member conflicts.
for (ShapeId mixin : mixins) {
Shape mixinShape = shapeMap.get(mixin);
for (MemberShape member : mixinShape.members()) {
ShapeId targetId = builder.getId().withMember(member.getMemberName());
Map<ShapeId, Trait> introducedTraits = traitContainer.getTraitsForShape(targetId);

MemberShape introducedMember = null;
if (builderMembers.containsKey(member.getMemberName())) {
introducedMember = builderMembers.get(member.getMemberName())
.addMixin(member)
.build();

if (!introducedMember.getTarget().equals(member.getTarget())) {
// Members cannot be redefined if their targets conflict.
MemberShape.Builder conflict = builderMembers.get(member.getMemberName());
events.add(ValidationEvent.builder()
.severity(Severity.ERROR)
.id(Validator.MODEL_ERROR)
.shapeId(conflict.getId())
.sourceLocation(conflict.getSourceLocation())
.message("Member conflicts with an inherited mixin member: " + member.getId())
.build());
}
} else if (!introducedTraits.isEmpty()) {
// Build local member copies before adding mixins if traits
// were introduced to inherited mixin members.
introducedMember = MemberShape.builder()
.id(targetId)
.target(member.getTarget())
.source(member.getSourceLocation())
.addTraits(introducedTraits.values())
.addMixin(member)
.build();
}

if (introducedMember != null) {
builder.addMember(introducedMember);
}
}
builder.addMixin(mixinShape);

for (PendingShapeModifier pendingModification : pendingModifications) {
pendingModification.modifyShape(builder, builderMembers, resolvedTraits, shapeMap);
}

buildShape(builder, resolvedTraits).ifPresent(result -> shapeMap.put(result.getId(), result));
});
}
Expand All @@ -285,10 +260,24 @@ private <S extends Shape, B extends AbstractShapeBuilder<? extends B, S>> Option
builder.addTrait(trait);
}
return Optional.of(builder.build());
} catch (IllegalStateException e) {
if (builder.getShapeType() == ShapeType.MEMBER && ((MemberShape.Builder) builder).getTarget() == null) {
events.add(ValidationEvent.builder()
.severity(Severity.ERROR)
.id(Validator.MODEL_ERROR)
.shapeId(builder.getId())
.sourceLocation(builder.getSourceLocation())
.message("Member target was elided, but no bound resource or mixin contained a matching "
+ "identifier or member name.")
.build());
return Optional.empty();
}
throw e;
} catch (SourceException e) {
events.add(ValidationEvent.fromSourceException(e, "", builder.getId()));
resolvedTraits.clearTraitsForShape(builder.getId());
return Optional.empty();
}
}

}
Loading

0 comments on commit 430ef61

Please sign in to comment.