From 77d9c3f1f29ad0fc0a5e14130313cf5b00396090 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Tue, 23 Jan 2024 16:18:07 -0500 Subject: [PATCH] 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. --- .../build/transforms/FlattenNamespaces.java | 23 ++++++--- .../transforms/FlattenNamespacesTest.java | 48 +++++++++++++++++++ 2 files changed, 64 insertions(+), 7 deletions(-) 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 a8c0a5e76b6..6c46377959b 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.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 @@ protected Model transformWithConfig(TransformContext context, Config config) { 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 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 String getName() { } private Map 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 shapesToRename = getRenamedShapesConnectedToService(config, model); Set taggedShapesToInclude = getTaggedShapesToInclude(config.getIncludeTagged(), model); 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 681b83fda6b..c491cb364e1 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.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,48 @@ public void supportsRenamesOnService() throws Exception { "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 validatedResult = Model.assembler() + .addModel(result) + .assemble(); + List 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()