From bfdf065f15952df41a5cf6ef65eb348b6120a184 Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Fri, 5 Aug 2022 20:18:56 -0700 Subject: [PATCH] Add CLIENT_CAREFUL mode to NullableIndex --- .../smithy/model/knowledge/NullableIndex.java | 53 ++++++++++++++----- .../model/knowledge/NullableIndexTest.java | 23 ++++++++ 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/NullableIndex.java b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/NullableIndex.java index f9605b274..439b95537 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/NullableIndex.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/NullableIndex.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -20,7 +20,9 @@ import java.util.Objects; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.shapes.StructureShape; import software.amazon.smithy.model.shapes.ToShapeId; +import software.amazon.smithy.model.shapes.UnionShape; import software.amazon.smithy.model.traits.BoxTrait; import software.amazon.smithy.model.traits.ClientOptionalTrait; import software.amazon.smithy.model.traits.DefaultTrait; @@ -52,7 +54,32 @@ public class NullableIndex implements KnowledgeIndex { * A client, or any other kind of non-authoritative model consumer * that must honor the {@link InputTrait} and {@link ClientOptionalTrait}. */ - CLIENT, + CLIENT { + @Override + boolean isStructureMemberOptional(StructureShape container, MemberShape member, Shape target) { + if (member.hasTrait(ClientOptionalTrait.class) || container.hasTrait(InputTrait.class)) { + return true; + } + + return SERVER.isStructureMemberOptional(container, member, target); + } + }, + + /** + * Like {@link #CLIENT} mode, but will treat all members that target + * structures and unions as optional because these members can never + * transition to optional using a default trait. + */ + CLIENT_CAREFUL { + @Override + boolean isStructureMemberOptional(StructureShape container, MemberShape member, Shape target) { + if (target instanceof StructureShape || target instanceof UnionShape) { + return true; + } + + return CLIENT.isStructureMemberOptional(container, member, target); + } + }, /** * A server, or any other kind of authoritative model consumer @@ -66,7 +93,15 @@ public class NullableIndex implements KnowledgeIndex { * server is required to be updated in order to implement newly added * model components. */ - SERVER + SERVER { + @Override + boolean isStructureMemberOptional(StructureShape container, MemberShape member, Shape target) { + // Structure members that are @required or @default are not nullable. + return !(member.hasTrait(DefaultTrait.class) || member.hasTrait(RequiredTrait.class)); + } + }; + + abstract boolean isStructureMemberOptional(StructureShape container, MemberShape member, Shape target); } /** @@ -74,7 +109,7 @@ public class NullableIndex implements KnowledgeIndex { * * @param member Member to check. * @return Returns true if the member is optional in - * non-authoritative consumers of the model like clients. + * non-authoritative consumers of the model like clients. * @see #isMemberNullable(MemberShape, CheckMode) */ public boolean isMemberNullable(MemberShape member) { @@ -97,17 +132,11 @@ public class NullableIndex implements KnowledgeIndex { public boolean isMemberNullable(MemberShape member, CheckMode checkMode) { Model m = Objects.requireNonNull(model.get()); Shape container = m.expectShape(member.getContainer()); + Shape target = m.expectShape(member.getTarget()); switch (container.getType()) { case STRUCTURE: - // Client mode honors the nullable and input trait. - if (checkMode == CheckMode.CLIENT - && (member.hasTrait(ClientOptionalTrait.class) || container.hasTrait(InputTrait.class))) { - return true; - } - - // Structure members that are @required or @default are not nullable. - return !member.hasTrait(DefaultTrait.class) && !member.hasTrait(RequiredTrait.class); + return checkMode.isStructureMemberOptional(container.asStructureShape().get(), member, target); case UNION: case SET: // Union and set members are never null. diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/NullableIndexTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/NullableIndexTest.java index 3275c1bfe..98bb90ed2 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/NullableIndexTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/NullableIndexTest.java @@ -387,4 +387,27 @@ public class NullableIndexTest { assertThat(index.isNullable(list), is(true)); assertThat(index.isNullable(list.getMember()), is(true)); } + + @Test + public void carefulModeTreatsStructureAndUnionAsOptional() { + StructureShape struct = StructureShape.builder() + .id("smithy.example#Struct") + .build(); + UnionShape union = UnionShape.builder() + .id("smithy.example#Union") + .addMember("a", ShapeId.from("smithy.api#String")) + .build(); + StructureShape outer = StructureShape.builder() + .id("smithy.example#Outer") + .addMember("a", struct.getId(), m -> m.addTrait(new RequiredTrait())) + .addMember("b", union.getId(), m -> m.addTrait(new RequiredTrait())) + .build(); + Model model = Model.assembler().addShapes(struct, union, outer).assemble().unwrap(); + NullableIndex index = NullableIndex.of(model); + + assertThat(index.isMemberNullable(outer.getMember("a").get(), NullableIndex.CheckMode.CLIENT_CAREFUL), + is(true)); + assertThat(index.isMemberNullable(outer.getMember("b").get(), NullableIndex.CheckMode.CLIENT_CAREFUL), + is(true)); + } }