Don't regenerate Schemas of external types

adwsingh · adwsingh · commit abdf7d2bef35 · 2025-05-06T15:01:23.000-07:00
diff --git a/codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/generators/SchemaFieldOrder.java b/codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/generators/SchemaFieldOrder.java
@@ -22,6 +22,7 @@
 import software.amazon.smithy.codegen.core.TopologicalIndex;
 import software.amazon.smithy.codegen.core.directed.Directive;
 import software.amazon.smithy.java.codegen.CodegenUtils;
+import software.amazon.smithy.java.codegen.SymbolProperties;
 import software.amazon.smithy.java.codegen.writer.JavaWriter;
 import software.amazon.smithy.java.core.schema.PreludeSchemas;
 import software.amazon.smithy.model.loader.Prelude;
@@ -77,7 +78,9 @@ public SchemaFieldOrder(Directive<?> directive, long partitionThreshold, SymbolP
                 shapeFieldName = toFullQualifiedSchemaName(shape);
             }
             boolean isShapeRecursive = CodegenUtils.recursiveShape(directive.model(), shape);
-            var shapeField = new SchemaField(shape, shapeFieldName, curClassName, isShapeRecursive);
+            boolean isExternal =
+                    symbolProvider.toSymbol(shape).getProperty(SymbolProperties.EXTERNAL_TYPE).orElse(false);
+            var shapeField = new SchemaField(shape, shapeFieldName, curClassName, isShapeRecursive, isExternal);
             curParition.add(shapeField);
             curFieldNames.add(shapeFieldName);
             if (isShapeRecursive) {
@@ -109,6 +112,8 @@ public String getSchemaFieldName(Shape shape, JavaWriter writer) {
         SchemaField schemaField = this.getSchemaField(shape.getId());
         if (schemaField == null) {
             return getSchemaType(writer, symbolProvider, shape);
+        } else if (schemaField.isExternal()) {
+            return writer.format("$L.$$SCHEMA", symbolProvider.toSymbol(shape));
         }
         return writer.format("$L.$L", schemaField.className(), schemaField.fieldName());
     }
@@ -140,5 +145,5 @@ private static String toFullQualifiedSchemaName(Shape shape) {
                 .toUpperCase(Locale.ENGLISH);
     }
 
-    record SchemaField(Shape shape, String fieldName, String className, boolean isRecursive) {}
+    record SchemaField(Shape shape, String fieldName, String className, boolean isRecursive, boolean isExternal) {}
 }
diff --git a/codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/generators/SchemasGenerator.java b/codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/generators/SchemasGenerator.java
@@ -5,13 +5,16 @@
 
 package software.amazon.smithy.java.codegen.generators;
 
+import static java.util.function.Predicate.not;
+
 import java.util.List;
 import java.util.function.Consumer;
 import software.amazon.smithy.codegen.core.directed.ContextualDirective;
 import software.amazon.smithy.codegen.core.directed.CustomizeDirective;
 import software.amazon.smithy.java.codegen.CodeGenerationContext;
 import software.amazon.smithy.java.codegen.CodegenUtils;
 import software.amazon.smithy.java.codegen.JavaCodegenSettings;
+import software.amazon.smithy.java.codegen.generators.SchemaFieldOrder.SchemaField;
 import software.amazon.smithy.java.codegen.writer.JavaWriter;
 import software.amazon.smithy.java.core.schema.Schema;
 import software.amazon.smithy.java.core.schema.SchemaBuilder;
@@ -70,7 +73,10 @@ final class ${className:L} {
                                         }
                                         """;
                         var recursiveShapes =
-                                shapeOrder.stream().filter(SchemaFieldOrder.SchemaField::isRecursive).toList();
+                                shapeOrder.stream()
+                                        .filter(SchemaField::isRecursive)
+                                        .filter(not(SchemaField::isExternal))
+                                        .toList();
                         var builders = recursiveShapes.stream()
                                 .map(s -> new SchemaBuilderGenerator(writer,
                                         s,
@@ -95,14 +101,14 @@ final class ${className:L} {
 
     private static final class StaticSchemaFieldsGenerator implements Runnable {
         private final JavaWriter writer;
-        private final List<SchemaFieldOrder.SchemaField> schemaFields;
+        private final List<SchemaField> schemaFields;
         private final CodeGenerationContext context;
         private final ContextualDirective<CodeGenerationContext, ?> directive;
         private boolean insideStaticBlock;
 
         private StaticSchemaFieldsGenerator(
                 ContextualDirective<CodeGenerationContext, ?> directive,
-                List<SchemaFieldOrder.SchemaField> schemaFields,
+                List<SchemaField> schemaFields,
                 JavaWriter writer
         ) {
             this.directive = directive;
@@ -115,7 +121,10 @@ private StaticSchemaFieldsGenerator(
         @Override
         public void run() {
             writer.pushState();
-            for (SchemaFieldOrder.SchemaField schemaField : schemaFields) {
+            for (SchemaField schemaField : schemaFields) {
+                if (schemaField.isExternal()) {
+                    continue;
+                }
                 if (schemaField.isRecursive() && !insideStaticBlock) {
                     insideStaticBlock = true;
                     writer.openBlock("\nstatic {");
@@ -142,14 +151,14 @@ public void run() {
         private static final class StaticSchemaFieldGenerator extends ShapeVisitor.Default<Void> {
 
             private final JavaWriter writer;
-            private final SchemaFieldOrder.SchemaField schemaField;
+            private final SchemaField schemaField;
             private final ContextualDirective<CodeGenerationContext, ?> directive;
             private final Model model;
             private final CodeGenerationContext context;
 
             private StaticSchemaFieldGenerator(
                     JavaWriter writer,
-                    SchemaFieldOrder.SchemaField schemaField,
+                    SchemaField schemaField,
                     ContextualDirective<CodeGenerationContext, ?> directive
             ) {
                 this.writer = writer;
@@ -362,11 +371,11 @@ private static final class SchemaBuilderGenerator extends ShapeVisitor.Default<V
         private final Shape shape;
         private final Model model;
         private final CodeGenerationContext context;
-        private final SchemaFieldOrder.SchemaField schemaField;
+        private final SchemaField schemaField;
 
         SchemaBuilderGenerator(
                 JavaWriter writer,
-                SchemaFieldOrder.SchemaField schemaField,
+                SchemaField schemaField,
                 Model model,
                 CodeGenerationContext context
         ) {
@@ -424,7 +433,7 @@ public Void unionShape(UnionShape shape) {
         }
     }
 
-    private record ResolverGenerator(JavaWriter writer, SchemaFieldOrder.SchemaField schemaField) implements Runnable {
+    private record ResolverGenerator(JavaWriter writer, SchemaField schemaField) implements Runnable {
 
         @Override
         public void run() {
diff --git a/codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/integrations/externaltypes/ExternalTypesIntegration.java b/codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/integrations/externaltypes/ExternalTypesIntegration.java
@@ -31,9 +31,9 @@
 @SmithyInternalApi
 public final class ExternalTypesIntegration implements JavaCodegenIntegration {
     private static final String PROPERTY_FILE = "META-INF/smithy-java/type-mappings.properties";
-    private static final Map<ShapeId, Symbol> TYPE_MAPPINGS = getErrorMappings();
+    private static final Map<ShapeId, Symbol> TYPE_MAPPINGS = getTypeMappings();
 
-    private static Map<ShapeId, Symbol> getErrorMappings() {
+    private static Map<ShapeId, Symbol> getTypeMappings() {
         try {
             Map<ShapeId, Symbol> result = new HashMap<>();
             var classLoader = ExternalTypesIntegration.class.getClassLoader();
@@ -49,7 +49,7 @@ private static Map<ShapeId, Symbol> getErrorMappings() {
                     var symbol = CodegenUtils.fromClass(shapeClass)
                             .toBuilder()
                             .putProperty(SymbolProperties.EXTERNAL_TYPE, true)
-                            .build();;
+                            .build();
                     var existing = result.put(shapeId, symbol);
                     if (existing != null) {
                         throw new CodegenException(
diff --git a/examples/standalone-types/build.gradle.kts b/examples/standalone-types/build.gradle.kts
@@ -9,6 +9,7 @@ dependencies {
 
     smithyBuild("software.amazon.smithy.java:plugins:$smithyJavaVersion")
     api("software.amazon.smithy.java:core:$smithyJavaVersion")
+    api("software.amazon.smithy.java:framework-errors:$smithyJavaVersion")
 
     testImplementation("org.hamcrest:hamcrest:3.0")
     testImplementation("org.junit.jupiter:junit-jupiter:5.12.2")
diff --git a/examples/standalone-types/model/person.smithy b/examples/standalone-types/model/person.smithy
@@ -2,6 +2,8 @@ $version: "2"
 
 namespace smithy.example
 
+use smithy.framework#ValidationException
+
 structure Human {
     children: HumanList
 
@@ -29,3 +31,7 @@ structure Parents {
 structure Address {
     city: String
 }
+
+structure HumanValidationException {
+    cause: ValidationException
+}
diff --git a/examples/standalone-types/src/test/java/software/amazon/smithy/java/example/ExternalTypesTest.java b/examples/standalone-types/src/test/java/software/amazon/smithy/java/example/ExternalTypesTest.java
@@ -0,0 +1,22 @@
+package software.amazon.smithy.java.example;
+
+import org.junit.jupiter.api.Test;
+import software.amazon.smithy.java.core.schema.Schema;
+import software.amazon.smithy.java.core.schema.SerializableStruct;
+import software.amazon.smithy.java.core.serde.SpecificShapeSerializer;
+import software.amazon.smithy.java.example.standalone.model.HumanValidationException;
+import software.amazon.smithy.java.framework.model.ValidationException;
+
+class ExternalTypesTest {
+
+    @Test
+    void verifySchemasAreNotRegenerated() {
+        HumanValidationException.builder().cause(ValidationException.builder().message("Test").build()).build().serializeMembers(new SpecificShapeSerializer() {
+
+            @Override
+            public void writeStruct(Schema schema, SerializableStruct struct) {
+               schema.assertMemberTargetIs(ValidationException.$SCHEMA);
+            }
+        });
+    }
+}
diff --git a/jmespath/src/main/java/software/amazon/smithy/java/jmespath/JMESPathFunction.java b/jmespath/src/main/java/software/amazon/smithy/java/jmespath/JMESPathFunction.java
@@ -429,7 +429,7 @@ protected Document applyImpl(List<Document> arguments, ExpressionTypeExpression
                     }
                 }
                 case SHORT, BYTE, INTEGER, INT_ENUM, LONG, FLOAT, DOUBLE, BIG_DECIMAL, BIG_INTEGER ->
-                        Document.ofNumber(argument.asNumber());
+                    Document.ofNumber(argument.asNumber());
                 default -> null;
             };
         }
diff --git a/jmespath/src/test/java/software/amazon/smithy/java/jmespath/ComplianceTestRunner.java b/jmespath/src/test/java/software/amazon/smithy/java/jmespath/ComplianceTestRunner.java
@@ -168,16 +168,21 @@ private static boolean isEqual(Document expected, Document actual) {
         } else if (isNumeric(expected) && isNumeric(actual)) {
             // Normalize all numbers to BigDecimal to make comparisons work.
             return new BigDecimal(expected.asNumber().toString())
-                           .compareTo(new BigDecimal(actual.asNumber().toString())) == 0;
+                    .compareTo(new BigDecimal(actual.asNumber().toString())) == 0;
         }
         return Objects.equals(expected, actual);
     }
 
     private static boolean isNumeric(Document doc) {
         var type = doc.type();
-        return type == ShapeType.BYTE || type == ShapeType.SHORT || type == ShapeType.INTEGER
-                || type == ShapeType.LONG || type == ShapeType.BIG_INTEGER || type == ShapeType.BIG_DECIMAL
-                || type == ShapeType.FLOAT || type == ShapeType.DOUBLE || type == ShapeType.INT_ENUM;
+        return type == ShapeType.BYTE || type == ShapeType.SHORT
+                || type == ShapeType.INTEGER
+                || type == ShapeType.LONG
+                || type == ShapeType.BIG_INTEGER
+                || type == ShapeType.BIG_DECIMAL
+                || type == ShapeType.FLOAT
+                || type == ShapeType.DOUBLE
+                || type == ShapeType.INT_ENUM;
     }
 
     private static final class NodeToDocumentConverter implements NodeVisitor<Document> {

Original file line number	Diff line number	Diff line change
`@@ -429,7 +429,7 @@ protected Document applyImpl(List<Document> arguments, ExpressionTypeExpression`
`429`	`429`	`}`
`430`	`430`	`}`
`431`	`431`	`case SHORT, BYTE, INTEGER, INT_ENUM, LONG, FLOAT, DOUBLE, BIG_DECIMAL, BIG_INTEGER ->`
`432`		`- Document.ofNumber(argument.asNumber());`
	`432`	`+ Document.ofNumber(argument.asNumber());`
`433`	`433`	`default -> null;`
`434`	`434`	`};`
`435`	`435`	`}`