Repository: smithy-lang/smithy
Pull request #2109 (2024-01-23 21:28:19)

Remove service renames after flattenNamespaces
flattenNamespaces applies all service renames to disambiguate shapes that could be in conflict after flattening. But when consumers try to reload the transformed model, they get an error like:
```
"Service rename for `foo` does not actually change the name from `bar`"
```
because the rename has already been applied. This commit changes FlattenTransformer to also update the service shape by removing all renames, so consumers of a flattened model don't run into validation issues.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/transforms/FlattenNamespaces.java b/smithy-build/src/main/java/software/amazon/smithy/build/transforms/FlattenNamespaces.java
index a8c0a5e76..6c4637795 100644
--- a/smithy-build/src/main/java/software/amazon/smithy/build/transforms/FlattenNamespaces.java
+++ b/smithy-build/src/main/java/software/amazon/smithy/build/transforms/FlattenNamespaces.java
@@ -28,8 +28,8 @@ import software.amazon.smithy.model.neighbor.Walker;
 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.transform.ModelTransformer;
 import software.amazon.smithy.utils.FunctionalUtils;
+import software.amazon.smithy.utils.ListUtils;
 import software.amazon.smithy.utils.Pair;
 
 /**
@@ -120,9 +120,23 @@ public final class FlattenNamespaces extends ConfigurableProjectionTransformer<F
             throw new SmithyBuildException(
                     "'namespace' and 'service' properties must be set on flattenNamespace transformer.");
         }
+
         Model model = context.getModel();
+        if (!model.getShape(config.getService()).isPresent()) {
+            throw new SmithyBuildException("Configured service, " + config.getService()
+                    + ", not found in model when performing flattenNamespaces transform.");
+        }
+
         Map<ShapeId, ShapeId> shapesToRename = getRenamedShapes(config, model);
-        return ModelTransformer.create().renameShapes(model, shapesToRename);
+        model = context.getTransformer().renameShapes(model, shapesToRename);
+
+        // Remove service renames since they've now been applied, so consumers don't fail service shape validation
+        ShapeId updatedServiceId = shapesToRename.get(config.getService());
+        ServiceShape updatedService = model.expectShape(updatedServiceId, ServiceShape.class)
+                .toBuilder()
+                .clearRename()
+                .build();
+        return context.getTransformer().replaceShapes(model, ListUtils.of(updatedService));
     }
 
     @Override
@@ -131,11 +145,6 @@ public final class FlattenNamespaces extends ConfigurableProjectionTransformer<F
     }
 
     private Map<ShapeId, ShapeId> getRenamedShapes(Config config, Model model) {
-        if (!model.getShape(config.getService()).isPresent()) {
-            throw new SmithyBuildException("Configured service, " + config.getService()
-                    + ", not found in model when performing flattenNamespaces transform.");
-        }
-
         Map<ShapeId, ShapeId> shapesToRename = getRenamedShapesConnectedToService(config, model);
         Set<ShapeId> taggedShapesToInclude = getTaggedShapesToInclude(config.getIncludeTagged(), model);
 


Test patch:
diff --git a/smithy-build/src/test/java/software/amazon/smithy/build/transforms/FlattenNamespacesTest.java b/smithy-build/src/test/java/software/amazon/smithy/build/transforms/FlattenNamespacesTest.java
index 681b83fda..4f530e210 100644
--- a/smithy-build/src/test/java/software/amazon/smithy/build/transforms/FlattenNamespacesTest.java
+++ b/smithy-build/src/test/java/software/amazon/smithy/build/transforms/FlattenNamespacesTest.java
@@ -16,11 +16,13 @@
 package software.amazon.smithy.build.transforms;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.anEmptyMap;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.not;
 
 import java.nio.file.Paths;
 import java.util.List;
+import java.util.Optional;
 import java.util.stream.Collectors;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
@@ -31,7 +33,11 @@ import software.amazon.smithy.model.loader.Prelude;
 import software.amazon.smithy.model.node.ExpectationNotMetException;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.ObjectNode;
+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.validation.ValidatedResult;
+import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.utils.FunctionalUtils;
 
 public class FlattenNamespacesTest {
@@ -143,6 +149,52 @@ public class FlattenNamespacesTest {
                 "ns.bar#Widget", "foo.example#Widget")));
     }
 
+    @Test
+    public void removesServiceRenames() {
+        Model model = Model.assembler()
+                .addImport(getClass().getResource("flatten-namespaces-with-renames.json"))
+                .assemble()
+                .unwrap();
+        TransformContext context = TransformContext.builder()
+                .model(model)
+                .settings(Node.objectNode()
+                        .withMember("namespace", "ns.qux")
+                        .withMember("service", "ns.foo#MyService"))
+                .build();
+
+        Model result = new FlattenNamespaces().transform(context);
+        ShapeId transformedServiceId = ShapeId.from("ns.qux#MyService");
+
+        assertThat(result.getShape(transformedServiceId), not(Optional.empty()));
+        assertThat(result.expectShape(transformedServiceId, ServiceShape.class).getRename(), anEmptyMap());
+    }
+
+    @Test
+    public void serviceShapeIsValidAfterTransform() {
+        Model model = Model.assembler()
+                .addImport(getClass().getResource("flatten-namespaces-with-renames.json"))
+                .assemble()
+                .unwrap();
+        TransformContext context = TransformContext.builder()
+                .model(model)
+                .settings(Node.objectNode()
+                        .withMember("namespace", "ns.qux")
+                        .withMember("service", "ns.foo#MyService"))
+                .build();
+
+        Model result = new FlattenNamespaces().transform(context);
+        ValidatedResult<Model> validatedResult = Model.assembler()
+                .addModel(result)
+                .assemble();
+        List<ShapeId> validationEventShapeIds = validatedResult.getValidationEvents().stream()
+                .map(ValidationEvent::getShapeId)
+                .filter(Optional::isPresent)
+                .map(Optional::get)
+                .collect(Collectors.toList());
+
+        assertThat(validationEventShapeIds, not(containsInAnyOrder(ShapeId.from("ns.qux#MyService"))));
+    }
+
     @Test
     public void throwsWhenServiceIsNotConfigured() {
         Model model = Model.assembler()