feat: allow non backward compatible required member (smithy-lang#566)

* feat: allow non backward-compatible required member
---------

Co-authored-by: Eduardo Rodrigues <[email protected]>
Co-authored-by: Jaykumar Gosar <[email protected]>

Author: 2 people

smithy-typescript-codegen-test/smithy-build.json

@@ -8,7 +8,8 @@
             "packageVersion": "0.0.1",
             "packageJson": {
                 "license": "Apache-2.0"
-            }
+            },
+            "requiredMemberMode": "strict"
         }
     }
 }

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/DirectedTypeScriptCodegen.java

@@ -331,7 +331,8 @@ public void generateStructure(GenerateStructureDirective<TypeScriptCodegenContex
                     directive.symbolProvider(),
                     writer,
                     directive.shape(),
-                    directive.settings().generateServerSdk()
+                    directive.settings().generateServerSdk(),
+                    directive.settings().getRequiredMemberMode()
             );
             generator.run();
         });
@@ -345,7 +346,8 @@ public void generateError(GenerateErrorDirective<TypeScriptCodegenContext, TypeS
                     directive.symbolProvider(),
                     writer,
                     directive.shape(),
-                    directive.settings().generateServerSdk()
+                    directive.settings().generateServerSdk(),
+                    directive.settings().getRequiredMemberMode()
             );
             generator.run();
         });

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructureGenerator.java

@@ -27,6 +27,7 @@
 import software.amazon.smithy.model.shapes.MemberShape;
 import software.amazon.smithy.model.shapes.StructureShape;
 import software.amazon.smithy.model.traits.ErrorTrait;
+import software.amazon.smithy.typescript.codegen.TypeScriptSettings.RequiredMemberMode;
 import software.amazon.smithy.typescript.codegen.integration.HttpProtocolGeneratorUtils;
 import software.amazon.smithy.utils.SmithyInternalApi;
 
@@ -67,24 +68,29 @@ final class StructureGenerator implements Runnable {
     private final TypeScriptWriter writer;
     private final StructureShape shape;
     private final boolean includeValidation;
+    private final RequiredMemberMode requiredMemberMode;
 
     /**
-     * sets 'includeValidation' to 'false' for backwards compatibility.
+     * sets 'includeValidation' to 'false' and requiredMemberMode
+     * to {@link RequiredMemberMode#NULLABLE}.
      */
     StructureGenerator(Model model, SymbolProvider symbolProvider, TypeScriptWriter writer, StructureShape shape) {
-        this(model, symbolProvider, writer, shape, false);
+        this(model, symbolProvider, writer, shape, false,
+            RequiredMemberMode.NULLABLE);
     }
 
     StructureGenerator(Model model,
                        SymbolProvider symbolProvider,
                        TypeScriptWriter writer,
                        StructureShape shape,
-                       boolean includeValidation) {
+                       boolean includeValidation,
+                       RequiredMemberMode requiredMemberMode) {
         this.model = model;
         this.symbolProvider = symbolProvider;
         this.writer = writer;
         this.shape = shape;
         this.includeValidation = includeValidation;
+        this.requiredMemberMode = requiredMemberMode;
     }
 
     @Override
@@ -158,7 +164,7 @@ private void renderNonErrorStructure() {
         }
 
         StructuredMemberWriter config = new StructuredMemberWriter(
-                model, symbolProvider, shape.getAllMembers().values());
+                model, symbolProvider, shape.getAllMembers().values(), this.requiredMemberMode);
         config.writeMembers(writer, shape);
         writer.closeBlock("}");
         writer.write("");
@@ -255,7 +261,7 @@ private void renderErrorStructure() {
             HttpProtocolGeneratorUtils.writeRetryableTrait(writer, shape, ";");
         }
         StructuredMemberWriter structuredMemberWriter = new StructuredMemberWriter(model, symbolProvider,
-                shape.getAllMembers().values());
+                shape.getAllMembers().values(), this.requiredMemberMode);
         // since any error interface must extend from JavaScript Error interface, message member is already
         // required in the JavaScript Error interface
         structuredMemberWriter.skipMembers.add("message");

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java

@@ -46,6 +46,7 @@
 import software.amazon.smithy.model.traits.StreamingTrait;
 import software.amazon.smithy.model.traits.Trait;
 import software.amazon.smithy.model.traits.UniqueItemsTrait;
+import software.amazon.smithy.typescript.codegen.TypeScriptSettings.RequiredMemberMode;
 import software.amazon.smithy.utils.SmithyInternalApi;
 
 /**
@@ -61,12 +62,19 @@ final class StructuredMemberWriter {
     Collection<MemberShape> members;
     String memberPrefix = "";
     boolean noDocs;
+    RequiredMemberMode requiredMemberMode;
     final Set<String> skipMembers = new HashSet<>();
 
     StructuredMemberWriter(Model model, SymbolProvider symbolProvider, Collection<MemberShape> members) {
+        this(model, symbolProvider, members, RequiredMemberMode.NULLABLE);
+    }
+
+    StructuredMemberWriter(Model model, SymbolProvider symbolProvider, Collection<MemberShape> members,
+            RequiredMemberMode requiredMemberMode) {
         this.model = model;
         this.symbolProvider = symbolProvider;
         this.members = new LinkedHashSet<>(members);
+        this.requiredMemberMode = requiredMemberMode;
     }
 
     void writeMembers(TypeScriptWriter writer, Shape shape) {
@@ -80,7 +88,8 @@ void writeMembers(TypeScriptWriter writer, Shape shape) {
             boolean wroteDocs = !noDocs && writer.writeMemberDocs(model, member);
             String memberName = getSanitizedMemberName(member);
             String optionalSuffix = shape.isUnionShape() || !isRequiredMember(member) ? "?" : "";
-            String typeSuffix = isRequiredMember(member) ? " | undefined" : "";
+            String typeSuffix = requiredMemberMode == RequiredMemberMode.NULLABLE
+                         && isRequiredMember(member) ? " | undefined" : "";
             writer.write("${L}${L}${L}: ${T}${L};", memberPrefix, memberName, optionalSuffix,
                          symbolProvider.toSymbol(member), typeSuffix);
 

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/TypeScriptSettings.java

@@ -34,6 +34,8 @@
 import software.amazon.smithy.model.shapes.ServiceShape;
 import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.shapes.ShapeId;
+import software.amazon.smithy.model.traits.DefaultTrait;
+import software.amazon.smithy.model.traits.RequiredTrait;
 import software.amazon.smithy.utils.SmithyUnstableApi;
 
 /**
@@ -43,6 +45,7 @@
 public final class TypeScriptSettings {
 
     static final String DISABLE_DEFAULT_VALIDATION = "disableDefaultValidation";
+    static final String REQUIRED_MEMBER_MODE = "requiredMemberMode";
     static final String TARGET_NAMESPACE = "targetNamespace";
     private static final Logger LOGGER = Logger.getLogger(TypeScriptSettings.class.getName());
 
@@ -66,6 +69,8 @@ public final class TypeScriptSettings {
     private boolean isPrivate;
     private ArtifactType artifactType = ArtifactType.CLIENT;
     private boolean disableDefaultValidation = false;
+    private RequiredMemberMode requiredMemberMode =
+        RequiredMemberMode.NULLABLE;
     private PackageManager packageManager = PackageManager.YARN;
 
     @Deprecated
@@ -106,6 +111,10 @@ public static TypeScriptSettings from(Model model, ObjectNode config, ArtifactTy
         if (artifactType == ArtifactType.SSDK) {
             settings.setDisableDefaultValidation(config.getBooleanMemberOrDefault(DISABLE_DEFAULT_VALIDATION));
         }
+        settings.setRequiredMemberMode(
+            config.getStringMember(REQUIRED_MEMBER_MODE)
+                .map(s -> RequiredMemberMode.fromString(s.getValue()))
+                .orElse(RequiredMemberMode.NULLABLE));
 
         settings.setPluginSettings(config);
         return settings;
@@ -296,6 +305,28 @@ public void setDisableDefaultValidation(boolean disableDefaultValidation) {
         this.disableDefaultValidation = disableDefaultValidation;
     }
 
+    /**
+     * Returns the code generation mode for required members.
+     *
+     * @return the configured mode for required members.
+     * Defaults to {@link RequiredMemberMode#NULLABLE}
+     */
+    public RequiredMemberMode getRequiredMemberMode() {
+        return requiredMemberMode;
+    }
+
+    public void setRequiredMemberMode(
+        RequiredMemberMode requiredMemberMode) {
+            if (requiredMemberMode != RequiredMemberMode.NULLABLE) {
+                LOGGER.warning(String.format("By setting the required member mode to '%s', a"
+                    + " member that has the '@required' trait applied CANNOT be 'undefined'."
+                    + " It will be considered a BACKWARDS INCOMPATIBLE change for"
+                    + " Smithy services even when the required constraint is dropped from a member.",
+                    requiredMemberMode.mode, RequiredMemberMode.NULLABLE.mode));
+            }
+            this.requiredMemberMode = requiredMemberMode;
+    }
+
     /**
      * Returns the package manager used by the generated package.
      *
@@ -396,10 +427,11 @@ public String getDefaultSigningName() {
     public enum ArtifactType {
         CLIENT(SymbolVisitor::new,
                 Arrays.asList(PACKAGE, PACKAGE_DESCRIPTION, PACKAGE_JSON, PACKAGE_VERSION, PACKAGE_MANAGER,
-                              SERVICE, PROTOCOL, TARGET_NAMESPACE, PRIVATE)),
+                              SERVICE, PROTOCOL, TARGET_NAMESPACE, PRIVATE, REQUIRED_MEMBER_MODE)),
         SSDK((m, s) -> new ServerSymbolVisitor(m, new SymbolVisitor(m, s)),
                 Arrays.asList(PACKAGE, PACKAGE_DESCRIPTION, PACKAGE_JSON, PACKAGE_VERSION, PACKAGE_MANAGER,
-                              SERVICE, PROTOCOL, TARGET_NAMESPACE, PRIVATE, DISABLE_DEFAULT_VALIDATION));
+                              SERVICE, PROTOCOL, TARGET_NAMESPACE, PRIVATE, REQUIRED_MEMBER_MODE,
+                              DISABLE_DEFAULT_VALIDATION));
 
         private final BiFunction<Model, TypeScriptSettings, SymbolProvider> symbolProviderFactory;
         private final List<String> configProperties;
@@ -422,6 +454,52 @@ public SymbolProvider createSymbolProvider(Model model, TypeScriptSettings setti
         }
     }
 
+    /**
+     * An enum indicating the code generation mode for required members.
+     */
+    public enum RequiredMemberMode {
+        /**
+         * This is the current behavior and it will be the default. When set,
+         * it allows a member that has the {@link RequiredTrait} applied to be {@code undefined}.
+         * By doing so it can still be considered a backwards compatible change fo
+         * Smithy services even when the required constraint is dropped from a member.
+         */
+        NULLABLE("nullable"),
+
+        /**
+         * This will dissallow members marked as {@link RequiredTrait} to be {@code undefined}.
+         * Use this mode with CAUTION because it comes with certain risks. When a server drops
+         * {@link RequiredTrait} from an output shape (and it is replaced with {@link DefaultTrait}
+         * as defined by the spec), if the server does not always serialize a value,
+         * customer code consuming the client and trying to access this member, may get a
+         * NullPointerException. Smithy spec says: "Authoritative model consumers like servers
+         * SHOULD always serialize default values to remove any ambiguity about the value of
+         * the most up to default value." So one should use this mode on the client, only if
+         * the server is following the approach proposed by the spec.
+         */
+        STRICT("strict");
+
+        private final String mode;
+
+        RequiredMemberMode(String mode) {
+            this.mode = mode;
+        }
+
+        public String getMode() {
+            return mode;
+        }
+
+        public static RequiredMemberMode fromString(String s) {
+            if ("nullable".equals(s)) {
+                return NULLABLE;
+            }
+            if ("strict".equals(s)) {
+                return STRICT;
+            }
+            throw new CodegenException(String.format("Unsupported required member mode: %s", s));
+        }
+    }
+
     public enum PackageManager {
         YARN("yarn"),
         NPM("npm");

smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java

@@ -6,13 +6,13 @@
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.build.MockManifest;
 import software.amazon.smithy.build.PluginContext;
-import software.amazon.smithy.codegen.core.SymbolProvider;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.loader.ModelAssembler;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.shapes.MemberShape;
 import software.amazon.smithy.model.shapes.OperationShape;
 import software.amazon.smithy.model.shapes.StructureShape;
+import software.amazon.smithy.typescript.codegen.TypeScriptSettings.RequiredMemberMode;
 
 public class StructureGeneratorTest {
     @Test
@@ -508,7 +508,19 @@ public void skipsFilterOnEncounteringRecursiveShapes() {
                                 + "})");
     }
 
+    @Test
+    public void properlyGeneratesRequiredMessageMemberNotBackwardCompatible() {
+        testStructureCodegenBase("test-required-member.smithy",
+                                "export interface GetFooOutput {\n"
+                                + "  someRequiredMember: string;\n"
+                                + "}\n", RequiredMemberMode.STRICT);
+    }
+
     private String testStructureCodegen(String file, String expectedType) {
+        return testStructureCodegenBase(file, expectedType, RequiredMemberMode.NULLABLE);
+    }
+
+    private String testStructureCodegenBase(String file, String expectedType, RequiredMemberMode requiredMemberMode) {
         Model model = Model.assembler()
                 .addImport(getClass().getResource(file))
                 .assemble()
@@ -521,6 +533,7 @@ private String testStructureCodegen(String file, String expectedType) {
                         .withMember("service", Node.from("smithy.example#Example"))
                         .withMember("package", Node.from("example"))
                         .withMember("packageVersion", Node.from("1.0.0"))
+                        .withMember("requiredMemberMode", Node.from(requiredMemberMode.getMode()))
                         .build())
                 .build();
 

smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-required-member.smithy

@@ -0,0 +1,17 @@
+namespace smithy.example
+
+service Example {
+    version: "1.0.0",
+    operations: [GetFoo]
+}
+
+operation GetFoo {
+    input: GetFooInput,
+    output: GetFooOutput
+}
+
+structure GetFooInput {}
+structure GetFooOutput {
+    @required
+    someRequiredMember: String
+}