Skip to content

Commit

Permalink
Properly fix multi-mixin members in shape build
Browse files Browse the repository at this point in the history
Shape builders have a saftey feature to fix members whose mixin list
doesn't match the expected list based on the container's mixin list.
This fixes a bug in that where it would drop all but the last mixin
from that "fixed" list.
  • Loading branch information
JordonPhillips committed Feb 22, 2024
1 parent d0d4e07 commit 1a8241c
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ static Map<String, MemberShape> computeMixinMembers(
// but omits the mixin members of parent shapes. Arguably, that's way too
// nuanced and error-prone to _not_ try to smooth over.
if (localMember.getMixins().isEmpty() || !localMember.getMixins().contains(member.getId())) {
localMember = localMember.toBuilder().clearMixins().addMixin(member).build();
localMember = rebuildMemberMixins(localMember, mixins.values());
}
computedMembers.put(name, localMember);
} else {
Expand Down Expand Up @@ -87,6 +87,14 @@ static Map<String, MemberShape> computeMixinMembers(
return Collections.unmodifiableMap(computedMembers);
}

private static MemberShape rebuildMemberMixins(MemberShape member, Collection<Shape> mixins) {
MemberShape.Builder builder = member.toBuilder().clearMixins();
for (Shape mixin : mixins) {
mixin.getMember(member.getMemberName()).ifPresent(builder::addMixin);
}
return builder.build();
}

static Set<MemberShape> flattenMixins(
Map<String, MemberShape> members,
Map<ShapeId, Shape> mixins,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,4 +310,30 @@ public void redefiningMembersPreservesOrder() {

assertThat(concrete.getMemberNames(), contains("a", "b", "c", "d"));
}

@Test
public void fixesMissingMemberMixins() {
StringShape string = StringShape.builder().id("smithy.example#String").build();
StructureShape mixin1 = StructureShape.builder()
.id("smithy.example#Mixin1")
.addTrait(MixinTrait.builder().build())
.addMember("a", string.getId())
.build();
StructureShape mixin2 = StructureShape.builder()
.id("smithy.example#Mixin2")
.addTrait(MixinTrait.builder().build())
.addMember("a", string.getId())
.build();
StructureShape concrete = StructureShape.builder()
.id("smithy.example#Concrete")
.addMember("a", string.getId())
.addMixin(mixin1)
.addMixin(mixin2)
.build();

assertThat(concrete.getMember("a").get().getMixins(), contains(
ShapeId.from("smithy.example#Mixin1$a"),
ShapeId.from("smithy.example#Mixin2$a")
));
}
}

0 comments on commit 1a8241c

Please sign in to comment.