diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java b/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java index bc8bf0217c2..e822cb38ae9 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java @@ -656,4 +656,14 @@ public Model addClientOptional(Model model, boolean applyWhenNoDefaultValue) { public Model downgradeToV1(Model model) { return new DowngradeToV1().transform(this, model); } + + /** + * Remove default traits if the default conflicts with the range trait of the shape. + * + * @param model Model to transform. + * @return Returns the transformed model. + */ + public Model removeInvalidDefaults(Model model) { + return new RemoveInvalidDefaults().transform(this, model); + } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/transform/RemoveInvalidDefaults.java b/smithy-model/src/main/java/software/amazon/smithy/model/transform/RemoveInvalidDefaults.java new file mode 100644 index 00000000000..11077ec95c6 --- /dev/null +++ b/smithy-model/src/main/java/software/amazon/smithy/model/transform/RemoveInvalidDefaults.java @@ -0,0 +1,75 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.model.transform; + +import java.util.HashSet; +import java.util.Set; +import java.util.logging.Logger; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.traits.DefaultTrait; +import software.amazon.smithy.model.traits.RangeTrait; + +/** + * Removes default values from shapes where the default value is incompatible with + * the constraint traits of the shape. + */ +final class RemoveInvalidDefaults { + + private static final Logger LOGGER = Logger.getLogger(RemoveInvalidDefaults.class.getName()); + + Model transform(ModelTransformer transformer, Model model) { + Set invalidDefaults = new HashSet<>(); + Set updates = new HashSet<>(); + + // First collect invalid shapes. Members with invalid defaults either need to remove the default or + // set the default to null if their target shape's default remains intact but the member is invalid. + for (Shape shape : model.getShapesWithTrait(DefaultTrait.class)) { + shape.getMemberTrait(model, RangeTrait.class).ifPresent(rangeTrait -> { + DefaultTrait defaultTrait = shape.expectTrait(DefaultTrait.class); + if (defaultTrait.toNode().isNumberNode()) { + defaultTrait.toNode().expectNumberNode().asBigDecimal().ifPresent(value -> { + if (rangeTrait.getMin().filter(min -> value.compareTo(min) < 0).isPresent() + || rangeTrait.getMin().filter(max -> value.compareTo(max) > 0).isPresent()) { + invalidDefaults.add(shape); + } + }); + } + }); + } + + for (Shape shape : invalidDefaults) { + updates.add(modify(shape, model, invalidDefaults)); + } + + return transformer.replaceShapes(model, updates); + } + + private Shape modify(Shape shape, Model model, Set otherShapes) { + // To show up here, the shape has to have a range trait, or the target has to have one. + RangeTrait rangeTrait = shape.getMemberTrait(model, RangeTrait.class).get(); + LOGGER.info(() -> "Removing default trait from " + shape.getId() + + " because of an incompatible range trait: " + + Node.printJson(rangeTrait.toNode())); + + // Members that target a shape with a default value need to set their default to null to override it. + // Other members and other shapes can simply remove the default trait. + if (shape.isMemberShape()) { + MemberShape member = shape.asMemberShape().get(); + boolean targetHasDefault = model.getShape(member.getTarget()) + // Treat target shapes that will have their default removed as if it doesn't have a default. + .filter(target -> !otherShapes.contains(target) && target.hasTrait(DefaultTrait.class)) + .isPresent(); + if (targetHasDefault) { + return member.toBuilder().addTrait(new DefaultTrait(Node.nullNode())).build(); + } + } + + return Shape.shapeToBuilder(shape).removeTrait(DefaultTrait.ID).build(); + } +} diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/node/RangeTraitPlugin.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/node/RangeTraitPlugin.java index 0dbfc5db9c2..5cf2e63860d 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/node/RangeTraitPlugin.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/node/RangeTraitPlugin.java @@ -15,7 +15,6 @@ package software.amazon.smithy.model.validation.node; -import java.math.BigDecimal; import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.node.Node.NonNumericFloat; import software.amazon.smithy.model.node.NumberNode; @@ -70,27 +69,26 @@ private void checkNonNumeric(Shape shape, RangeTrait trait, StringNode node, Emi } protected void check(Shape shape, Context context, RangeTrait trait, NumberNode node, Emitter emitter) { - Number number = node.getValue(); - BigDecimal decimal = number instanceof BigDecimal - ? (BigDecimal) number - : new BigDecimal(number.toString()); - trait.getMin().ifPresent(min -> { - if (decimal.compareTo(new BigDecimal(min.toString())) < 0) { - emitter.accept(node, getSeverity(node, context), String.format( - "Value provided for `%s` must be greater than or equal to %s, but found %s", - shape.getId(), min, number), - shape.isMemberShape() ? MEMBER : TARGET); - } + node.asBigDecimal().ifPresent(decimal -> { + if (decimal.compareTo(min) < 0) { + emitter.accept(node, getSeverity(node, context), String.format( + "Value provided for `%s` must be greater than or equal to %s, but found %s", + shape.getId(), min, decimal), + shape.isMemberShape() ? MEMBER : TARGET); + } + }); }); trait.getMax().ifPresent(max -> { - if (decimal.compareTo(new BigDecimal(max.toString())) > 0) { - emitter.accept(node, getSeverity(node, context), String.format( - "Value provided for `%s` must be less than or equal to %s, but found %s", - shape.getId(), max, number), - shape.isMemberShape() ? MEMBER : TARGET); - } + node.asBigDecimal().ifPresent(decimal -> { + if (decimal.compareTo(max) > 0) { + emitter.accept(node, getSeverity(node, context), String.format( + "Value provided for `%s` must be less than or equal to %s, but found %s", + shape.getId(), max, decimal), + shape.isMemberShape() ? MEMBER : TARGET); + } + }); }); } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/transform/RemoveInvalidDefaultsTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/transform/RemoveInvalidDefaultsTest.java new file mode 100644 index 00000000000..77f5f539f71 --- /dev/null +++ b/smithy-model/src/test/java/software/amazon/smithy/model/transform/RemoveInvalidDefaultsTest.java @@ -0,0 +1,33 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.model.transform; + +import org.junit.jupiter.api.Test; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.shapes.ModelSerializer; + +public class RemoveInvalidDefaultsTest { + @Test + public void removeInvalidDefaultsBasedOnRangeTrait() { + Model input = Model.assembler() + .addImport(getClass().getResource("bad-defaults-range-trait.smithy")) + .assemble() + .unwrap(); + Model output = Model.assembler() + .addImport(getClass().getResource("bad-defaults-range-trait.fixed.smithy")) + .assemble() + .unwrap(); + + ModelTransformer transformer = ModelTransformer.create(); + + Model result = transformer.removeInvalidDefaults(input); + + Node actual = ModelSerializer.builder().build().serialize(result); + Node expected = ModelSerializer.builder().build().serialize(output); + Node.assertEquals(actual, expected); + } +} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/transform/bad-defaults-range-trait.fixed.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/bad-defaults-range-trait.fixed.smithy new file mode 100644 index 00000000000..1dc37415833 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/bad-defaults-range-trait.fixed.smithy @@ -0,0 +1,39 @@ +$version: "2.0" + +namespace smithy.example + +structure Foo { + // Default was removed. + @range(min: 1) + invalid1: Integer + + // Default was removed. + invalid2: ValueGreaterThanZero + + // The default of the target shape was removed, so it can be removed here too. + invalid3: ValueGreaterThanZeroWithDefault + + // Cancel out the root level default. + @range(min: 1) + invalid4: PrimitiveInteger = null + + valid1: ValueGreaterThanZero = 1 + + valid2: ValueGreaterThanZero + + valid3: Integer + + valid4: Integer = 0 + + @range(min: 1) + valid5: Integer + + @range(min: 1) + valid6: Integer = 1 +} + +@range(min: 1) +integer ValueGreaterThanZero + +@range(min: 1) +integer ValueGreaterThanZeroWithDefault // default was removed diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/transform/bad-defaults-range-trait.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/bad-defaults-range-trait.smithy new file mode 100644 index 00000000000..6f1ce2f190c --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/bad-defaults-range-trait.smithy @@ -0,0 +1,41 @@ +$version: "2.0" + +namespace smithy.example + +structure Foo { + // The member itself creates an invalid combination of the range trait and default value. + @range(min: 1) + invalid1: Integer = 0 + + // The member adds a default value that is incompatible with the target shape. + invalid2: ValueGreaterThanZero = 0 + + // The member targets a shape where the range trait is incompatible with the default of the member. + invalid3: ValueGreaterThanZeroWithDefault = 0 + + // The range trait here makes the default value invalid. This member targets a root shape with a default, so the + // default has to be set to null to cancel out the root level default. + @range(min: 1) + invalid4: PrimitiveInteger = 0 + + valid1: ValueGreaterThanZero = 1 + + valid2: ValueGreaterThanZero + + valid3: Integer + + valid4: Integer = 0 + + @range(min: 1) + valid5: Integer + + @range(min: 1) + valid6: Integer = 1 +} + +@range(min: 1) +integer ValueGreaterThanZero + +@range(min: 1) +@default(0) // bad default and range trait combination. +integer ValueGreaterThanZeroWithDefault