dnrops
/
smithy
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix 1-2 command, add tests, reduce diff noise

This commit is contained in:
Michael Dowling 2022-09-11 09:05:48 -07:00 committed by Michael Dowling
parent 0bf9630387
commit 1f0da40ff2
26 changed files with 282 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
smithy-cli/src/main/java/software/amazon/smithy/cli/commands/Upgrade1to2Command.java
View File

@ -48,7 +48,6 @@ import software.amazon.smithy.model.SourceLocation;
import software.amazon.smithy.model.loader.ModelAssembler;
import software.amazon.smithy.model.loader.Prelude;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.NumberShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.ShapeType;
import software.amazon.smithy.model.shapes.ShapeVisitor;
@ -272,6 +271,8 @@ public final class Upgrade1to2Command extends SimpleCommand {
protected Void getDefault(Shape shape) {
if (shape.hasTrait(BoxTrait.class)) {
writer.eraseTrait(shape, shape.expectTrait(BoxTrait.class));
} else if (hasSyntheticDefault(shape)) {
addDefault(shape, shape.getType());
}
// Handle members in reverse definition order.
shape.members().stream()
@ -282,23 +283,7 @@ public final class Upgrade1to2Command extends SimpleCommand {
private void handleMemberShape(MemberShape shape) {
if (hasSyntheticDefault(shape)) {
SourceLocation memberLocation = shape.getSourceLocation();
String padding = "";
if (memberLocation.getColumn() > 1) {
padding = StringUtils.repeat(' ', memberLocation.getColumn() - 1);
}
Shape target = completeModel.expectShape(shape.getTarget());
String defaultValue = "";
if (target.isBooleanShape()) {
defaultValue = "false";
} else if (target instanceof NumberShape) {
defaultValue = "0";
} else if (target.isBlobShape() || target.isStringShape()) {
defaultValue = "\"\"";
} else {
throw new UnsupportedOperationException("Unexpected default: " + target);
}
writer.insertLine(shape.getSourceLocation().getLine(), padding + "@default(" + defaultValue + ")");
addDefault(shape, completeModel.expectShape(shape.getTarget()).getType());
}
if (shape.hasTrait(BoxTrait.class)) {
@ -306,7 +291,7 @@ public final class Upgrade1to2Command extends SimpleCommand {
}
}
private boolean hasSyntheticDefault(MemberShape shape) {
private boolean hasSyntheticDefault(Shape shape) {
Optional<SourceLocation> defaultLocation = shape.getTrait(DefaultTrait.class)
.map(Trait::getSourceLocation);
// When Smithy injects the default trait, it sets the source
@ -316,6 +301,40 @@ public final class Upgrade1to2Command extends SimpleCommand {
return defaultLocation.filter(location -> shape.getSourceLocation().equals(location)).isPresent();
}
private void addDefault(Shape shape, ShapeType targetType) {
SourceLocation memberLocation = shape.getSourceLocation();
String padding = "";
if (memberLocation.getColumn() > 1) {
padding = StringUtils.repeat(' ', memberLocation.getColumn() - 1);
}
String defaultValue = "";
// Boxed members get a null default.
if (shape.hasTrait(BoxTrait.class)) {
defaultValue = "null";
} else {
switch (targetType) {
case BOOLEAN:
defaultValue = "false";
break;
case BYTE:
case SHORT:
case INTEGER:
case LONG:
case FLOAT:
case DOUBLE:
defaultValue = "0";
break;
case BLOB:
case STRING:
defaultValue = "\"\"";
break;
default:
throw new UnsupportedOperationException("Unexpected default: " + targetType);
}
}
writer.insertLine(shape.getSourceLocation().getLine(), padding + "@default(" + defaultValue + ")");
}
@Override
public Void memberShape(MemberShape shape) {
// members are handled from their containers so that they can

6
smithy-cli/src/test/java/software/amazon/smithy/cli/commands/Upgrade1to2CommandTest.java
View File

@ -31,6 +31,7 @@ import java.nio.file.attribute.FileTime;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
@ -51,7 +52,10 @@ public class Upgrade1to2CommandTest {
Model model = Model.assembler().addImport(initialPath).assemble().unwrap();
String actual = new Upgrade1to2Command("smithy").upgradeFile(model, initialPath);
String expected = IoUtils.readUtf8File(expectedPath);
assertThat(actual, equalTo(expected));
if (!actual.equals(expected)) {
Assertions.fail("Expected models to be equal:\n\nActual:\n\n" + actual + "\n\nExpected:\n\n" + expected);
}
}
public static Stream<Arguments> source() throws Exception {

39
smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/upgrade/cases/box.v1.smithy Normal file
View File

@ -0,0 +1,39 @@
$version: "1.0"
namespace com.example
@box
integer BoxedInteger
integer NonBoxedInteger
structure StructureWithOptionalString {
boxedTarget: BoxedInteger,
@box
boxedMember: NonBoxedInteger,
}
union BoxyUnion {
boxedTarget: BoxedInteger,
@box
boxedMember: NonBoxedInteger,
}
list BadSparseList {
@box
member: NonBoxedInteger,
}
set BadSparseSet {
@box
member: NonBoxedInteger,
}
map BadSparseMap {
key: String,
@box
value: NonBoxedInteger,
}

35
smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/upgrade/cases/box.v2.smithy Normal file
View File

@ -0,0 +1,35 @@
$version: "2.0"
namespace com.example
integer BoxedInteger
@default(0)
integer NonBoxedInteger
structure StructureWithOptionalString {
boxedTarget: BoxedInteger,
@default(null)
boxedMember: NonBoxedInteger,
}
union BoxyUnion {
boxedTarget: BoxedInteger,
boxedMember: NonBoxedInteger,
}
list BadSparseList {
member: NonBoxedInteger,
}
set BadSparseSet {
member: NonBoxedInteger,
}
map BadSparseMap {
key: String,
value: NonBoxedInteger,
}

1
smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/upgrade/cases/primitive.v2.smithy
View File

@ -25,5 +25,6 @@ structure PrimitiveBearer {
@default(0)
handlesRequired: PrimitiveLong,
@default(null)
handlesBox: PrimitiveByte,
}

2
smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedDefault.java
View File

@ -77,7 +77,7 @@ public final class ChangedDefault extends AbstractDiffEvaluator {
}
}
} else if (!oldTrait.toNode().equals(newTrait.toNode())) {
if (change.getNewShape().asMemberShape().isPresent()) {
if (change.getNewShape().isMemberShape()) {
evaluateChangedTrait(model, change.getNewShape().asMemberShape().get(), oldTrait, newTrait, events);
} else {
events.add(error(change.getNewShape(), newTrait,

69
smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedDefaultTest.java
View File

@ -1,6 +1,7 @@
package software.amazon.smithy.diff.evaluators;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import java.util.List;
@ -114,6 +115,30 @@ public class ChangedDefaultTest {
assertThat(TestHelper.findEvents(events, "ChangedNullability").size(), equalTo(1));
}
@Test
public void updateModelWithAddedDefault() {
String originalModel =
"$version: \"2\"\n"
+ "namespace smithy.example\n"
+ "structure Foo {\n"
+ " @required\n"
+ " bar: Integer\n"
+ "}\n";
String updatedModel =
"$version: \"2\"\n"
+ "namespace smithy.example\n"
+ "structure Foo {\n"
+ " @addedDefault\n"
+ " bar: Integer = 1\n"
+ "}\n";
Model modelA = Model.assembler().addUnparsedModel("test.smithy", originalModel).assemble().unwrap();
Model modelB = Model.assembler().addUnparsedModel("test.smithy", updatedModel).assemble().unwrap();
List<ValidationEvent> events = ModelDiff.compare(modelA, modelB);
assertThat(TestHelper.findEvents(events, "ChangedDefault"), empty());
assertThat(TestHelper.findEvents(events, "ChangedNullability"), empty());
}
@Test
public void errorWhenDefaultChangesFromZeroToNonZeroValue() {
String originalModel =
@ -179,4 +204,48 @@ public class ChangedDefaultTest {
assertThat(TestHelper.findEvents(events, "ChangedDefault").size(), equalTo(0));
assertThat(TestHelper.findEvents(events, "ChangedNullability").size(), equalTo(0));
}
@Test
public void changingFromNullDefaultToOneIsBreaking() {
String originalModel =
"$version: \"2\"\n"
+ "namespace smithy.example\n"
+ "structure Foo {\n"
+ " bar: Integer = null\n"
+ "}\n";
String updatedModel =
"$version: \"2\"\n"
+ "namespace smithy.example\n"
+ "structure Foo {\n"
+ " bar: Integer = 1\n"
+ "}\n";
Model modelA = Model.assembler().addUnparsedModel("test.smithy", originalModel).assemble().unwrap();
Model modelB = Model.assembler().addUnparsedModel("test.smithy", updatedModel).assemble().unwrap();
List<ValidationEvent> events = ModelDiff.compare(modelA, modelB);
assertThat(TestHelper.findEvents(events, "ChangedDefault").size(), equalTo(1));
assertThat(TestHelper.findEvents(events, "ChangedNullability").size(), equalTo(1));
}
@Test
public void changingFromNullDefaultToZeroIsBreaking() {
String originalModel =
"$version: \"2\"\n"
+ "namespace smithy.example\n"
+ "structure Foo {\n"
+ " bar: Integer = null\n"
+ "}\n";
String updatedModel =
"$version: \"2\"\n"
+ "namespace smithy.example\n"
+ "structure Foo {\n"
+ " bar: Integer = 0\n"
+ "}\n";
Model modelA = Model.assembler().addUnparsedModel("test.smithy", originalModel).assemble().unwrap();
Model modelB = Model.assembler().addUnparsedModel("test.smithy", updatedModel).assemble().unwrap();
List<ValidationEvent> events = ModelDiff.compare(modelA, modelB);
assertThat(TestHelper.findEvents(events, "ChangedDefault").size(), equalTo(1));
assertThat(TestHelper.findEvents(events, "ChangedNullability").size(), equalTo(1));
}
}

2
smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderShapeMap.java
View File

@ -363,7 +363,7 @@ final class LoaderShapeMap {
) {
try {
AbstractShapeBuilder<?, ?> builder = defineShape.builder();
ModelUpgrader.patchShapeBeforeBuilding(defineShape, builder, events);
ModelInteropTransformer.patchShapeBeforeBuilding(defineShape, builder, events);
for (MemberShape.Builder memberBuilder : defineShape.memberBuilders().values()) {
for (ShapeModifier modifier : defineShape.modifiers()) {

4
smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java
View File

@ -555,8 +555,8 @@ public final class ModelAssembler {
}
// Do the 1.0 -> 2.0 transform before full-model validation.
ValidatedResult<Model> transformed = new ModelUpgrader(processedModel, events, processor::getShapeVersion)
.transform();
ValidatedResult<Model> transformed = new ModelInteropTransformer(processedModel, events,
processor::getShapeVersion).transform();
if (disableValidation
|| !transformed.getResult().isPresent()

38
smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelUpgrader.java → smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelInteropTransformer.java
View File

@ -19,7 +19,6 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Set;
import java.util.function.Function;
import software.amazon.smithy.model.FromSourceLocation;
import software.amazon.smithy.model.Model;
@ -44,15 +43,21 @@ import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.model.transform.ModelTransformer;
import software.amazon.smithy.model.validation.ValidatedResult;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.utils.SetUtils;
/**
* Upgrades Smithy models from IDL v1 to IDL v2, specifically taking into
* account the removal of the box trait, the change in default value
* semantics of numbers and booleans, and the @default trait.
* Ensures interoperability between IDL 1 and IDL 2 models by adding default traits and synthetic box traits where
* needed to normalize across versions.
*
* <p>For 1 to 2 interop, this class adds default traits. If a root level v1 shape is not boxed and supports the box
* trait, a default trait is added set to the zero value of the type. If a v1 member is marked with the box trait,
* then a default trait is added to the member set to null. If a member is targets a streaming blob and is not
* required, the default trait is added set to an empty string.
*
* <p>For 2 to 1 interop, if a v2 member has a default set to null a synthetic box trait is added. If a root level
* does not have the default trait and the type supports the box trait, a synthetic box trait is added.
*/
@SuppressWarnings("deprecation")
final class ModelUpgrader {
final class ModelInteropTransformer {
/** Shape types in Smithy 1.0 that had a default value. */
private static final EnumSet<ShapeType> HAD_DEFAULT_VALUE_IN_1_0 = EnumSet.of(
@ -64,20 +69,12 @@ final class ModelUpgrader {
ShapeType.DOUBLE,
ShapeType.BOOLEAN);
private static final Set<ShapeType> SUPPORTS_RANGE_TRAIT_AND_ZERO_VALUE = SetUtils.of(
ShapeType.BYTE,
ShapeType.SHORT,
ShapeType.INTEGER,
ShapeType.LONG,
ShapeType.FLOAT,
ShapeType.DOUBLE);
private final Model model;
private final List<ValidationEvent> events;
private final Function<Shape, Version> fileToVersion;
private final List<Shape> shapeUpgrades = new ArrayList<>();
ModelUpgrader(Model model, List<ValidationEvent> events, Function<Shape, Version> fileToVersion) {
ModelInteropTransformer(Model model, List<ValidationEvent> events, Function<Shape, Version> fileToVersion) {
this.model = model;
this.events = events;
this.fileToVersion = fileToVersion;
@ -123,24 +120,23 @@ final class ModelUpgrader {
} else if (member.hasTrait(BoxTrait.class)) {
// Add a default trait to the member set to null to indicate it was boxed in v1.
MemberShape.Builder builder = member.toBuilder();
builder.addTrait(new DefaultTrait(Node.nullNode()));
builder.addTrait(new DefaultTrait(new NullNode(member.getSourceLocation())));
shapeUpgrades.add(builder.build());
}
}
private boolean shouldV1MemberHaveDefaultTrait(MemberShape member, Shape target) {
// Only when the targeted shape had a default value by default in v1 or if
// the member has the http payload trait and targets a streaming blob, which
// implies a default in 2.0
return (HAD_DEFAULT_VALUE_IN_1_0.contains(target.getType()) || isDefaultPayload(member, target))
// the member targets a streaming blob, which implies a default in 2.0
return (HAD_DEFAULT_VALUE_IN_1_0.contains(target.getType()) || streamingBlobNeedsDefault(member, target))
// Don't re-add the @default trait
&& !member.hasTrait(DefaultTrait.ID)
// Don't add a @default trait if the member or target are considered boxed in v1.
&& memberAndTargetAreNotAlreadyExplicitlyBoxed(member, target);
}
private boolean isDefaultPayload(MemberShape member, Shape target) {
// httpPayload requires that the member is required or default.
private boolean streamingBlobNeedsDefault(MemberShape member, Shape target) {
// Streaming blobs require that the member is required or default.
// No need to add the default trait if the member is required.
if (member.isRequired()) {
return false;

11
smithy-model/src/main/java/software/amazon/smithy/model/validation/node/BlobLengthPlugin.java
View File

@ -20,7 +20,6 @@ import software.amazon.smithy.model.node.StringNode;
import software.amazon.smithy.model.shapes.BlobShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.traits.LengthTrait;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.utils.SmithyInternalApi;
/**
@ -40,17 +39,15 @@ final class BlobLengthPlugin extends MemberAndShapeTraitPlugin<BlobShape, String
trait.getMin().ifPresent(min -> {
if (size < min) {
emitter.accept(node, Severity.ERROR,
"Value provided for `" + shape.getId() + "` must have at least "
+ min + " bytes, but the provided value only has " + size + " bytes");
emitter.accept(node, "Value provided for `" + shape.getId() + "` must have at least "
+ min + " bytes, but the provided value only has " + size + " bytes");
}
});
trait.getMax().ifPresent(max -> {
if (value.getBytes(StandardCharsets.UTF_8).length > max) {
emitter.accept(node, Severity.ERROR,
"Value provided for `" + shape.getId() + "` must have no more than "
+ max + " bytes, but the provided value has " + size + " bytes");
emitter.accept(node, "Value provided for `" + shape.getId() + "` must have no more than "
+ max + " bytes, but the provided value has " + size + " bytes");
}
});
}

5
smithy-model/src/main/java/software/amazon/smithy/model/validation/node/CollectionLengthPlugin.java
View File

@ -19,7 +19,6 @@ import software.amazon.smithy.model.node.ArrayNode;
import software.amazon.smithy.model.shapes.CollectionShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.traits.LengthTrait;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.utils.SmithyInternalApi;
/**
@ -37,7 +36,7 @@ final class CollectionLengthPlugin extends MemberAndShapeTraitPlugin<CollectionS
protected void check(Shape shape, LengthTrait trait, ArrayNode node, Context context, Emitter emitter) {
trait.getMin().ifPresent(min -> {
if (node.size() < min) {
emitter.accept(node, Severity.ERROR, String.format(
emitter.accept(node, String.format(
"Value provided for `%s` must have at least %d elements, but the provided value only "
+ "has %d elements", shape.getId(), min, node.size()));
}
@ -45,7 +44,7 @@ final class CollectionLengthPlugin extends MemberAndShapeTraitPlugin<CollectionS
trait.getMax().ifPresent(max -> {
if (node.size() > max) {
emitter.accept(node, Severity.ERROR, String.format(
emitter.accept(node, String.format(
"Value provided for `%s` must have no more than %d elements, but the provided value "
+ "has %d elements", shape.getId(), max, node.size()));
}

5
smithy-model/src/main/java/software/amazon/smithy/model/validation/node/MapLengthPlugin.java
View File

@ -19,7 +19,6 @@ import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.shapes.MapShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.traits.LengthTrait;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.utils.SmithyInternalApi;
/**
@ -36,7 +35,7 @@ final class MapLengthPlugin extends MemberAndShapeTraitPlugin<MapShape, ObjectNo
protected void check(Shape shape, LengthTrait trait, ObjectNode node, Context context, Emitter emitter) {
trait.getMin().ifPresent(min -> {
if (node.size() < min) {
emitter.accept(node, Severity.ERROR, String.format(
emitter.accept(node, String.format(
"Value provided for `%s` must have at least %d entries, but the provided value only "
+ "has %d entries", shape.getId(), min, node.size()));
}
@ -44,7 +43,7 @@ final class MapLengthPlugin extends MemberAndShapeTraitPlugin<MapShape, ObjectNo
trait.getMax().ifPresent(max -> {
if (node.size() > max) {
emitter.accept(node, Severity.ERROR, String.format(
emitter.accept(node, String.format(
"Value provided for `%s` must have no more than %d entries, but the provided value "
+ "has %d entries", shape.getId(), max, node.size()));
}

4
smithy-model/src/main/java/software/amazon/smithy/model/validation/node/NodeValidatorPlugin.java
View File

@ -126,5 +126,9 @@ public interface NodeValidatorPlugin {
@FunctionalInterface
interface Emitter {
void accept(FromSourceLocation sourceLocation, Severity severity, String message);
default void accept(FromSourceLocation sourceLocation, String message) {
accept(sourceLocation, Severity.ERROR, message);
}
}
}

3
smithy-model/src/main/java/software/amazon/smithy/model/validation/node/NonNumericFloatValuesPlugin.java
View File

@ -19,7 +19,6 @@ import java.util.Set;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.node.Node.NonNumericFloat;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.validation.Severity;
/**
* Validates the specific set of non-numeric values allowed for floats and doubles.
@ -34,7 +33,7 @@ final class NonNumericFloatValuesPlugin implements NodeValidatorPlugin {
}
String nodeValue = value.expectStringNode().getValue();
if (!NON_NUMERIC_FLOAT_VALUES.contains(nodeValue)) {
emitter.accept(value, Severity.ERROR, String.format(
emitter.accept(value, String.format(
"Value for `%s` must either be numeric or one of the following strings: [\"%s\"], but was \"%s\"",
shape.getId(), String.join("\", \"", NON_NUMERIC_FLOAT_VALUES), nodeValue
));

3
smithy-model/src/main/java/software/amazon/smithy/model/validation/node/PatternTraitPlugin.java
View File

@ -19,7 +19,6 @@ import software.amazon.smithy.model.node.StringNode;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.StringShape;
import software.amazon.smithy.model.traits.PatternTrait;
import software.amazon.smithy.model.validation.Severity;
/**
* Validates the pattern trait on string shapes or members that target them.
@ -33,7 +32,7 @@ final class PatternTraitPlugin extends MemberAndShapeTraitPlugin<StringShape, St
@Override
protected void check(Shape shape, PatternTrait trait, StringNode node, Context context, Emitter emitter) {
if (!trait.getPattern().matcher(node.getValue()).find()) {
emitter.accept(node, Severity.ERROR, String.format(
emitter.accept(node, String.format(
"String value provided for `%s` must match regular expression: %s",
shape.getId(), trait.getPattern().pattern()));
}

3
smithy-model/src/main/java/software/amazon/smithy/model/validation/node/StringEnumPlugin.java
View File

@ -19,7 +19,6 @@ import java.util.List;
import software.amazon.smithy.model.node.StringNode;
import software.amazon.smithy.model.shapes.StringShape;
import software.amazon.smithy.model.traits.EnumTrait;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.model.validation.ValidationUtils;
/**
@ -36,7 +35,7 @@ final class StringEnumPlugin extends FilteredPlugin<StringShape, StringNode> {
shape.getTrait(EnumTrait.class).ifPresent(trait -> {
List<String> values = trait.getEnumDefinitionValues();
if (!values.contains(node.getValue())) {
emitter.accept(node, Severity.ERROR, String.format(
emitter.accept(node, String.format(
"String value provided for `%s` must be one of the following values: %s",
shape.getId(), ValidationUtils.tickedList(values)));
}

5
smithy-model/src/main/java/software/amazon/smithy/model/validation/node/StringLengthPlugin.java
View File

@ -19,7 +19,6 @@ import software.amazon.smithy.model.node.StringNode;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.StringShape;
import software.amazon.smithy.model.traits.LengthTrait;
import software.amazon.smithy.model.validation.Severity;
/**
* Validates the length trait on string shapes or members that target them.
@ -34,7 +33,7 @@ final class StringLengthPlugin extends MemberAndShapeTraitPlugin<StringShape, St
protected void check(Shape shape, LengthTrait trait, StringNode node, Context context, Emitter emitter) {
trait.getMin().ifPresent(min -> {
if (node.getValue().length() < min) {
emitter.accept(node, Severity.ERROR, String.format(
emitter.accept(node, String.format(
"String value provided for `%s` must be >= %d characters, but the provided value is "
+ "only %d characters.", shape.getId(), min, node.getValue().length()));
}
@ -42,7 +41,7 @@ final class StringLengthPlugin extends MemberAndShapeTraitPlugin<StringShape, St
trait.getMax().ifPresent(max -> {
if (node.getValue().length() > max) {
emitter.accept(node, Severity.ERROR, String.format(
emitter.accept(node, String.format(
"String value provided for `%s` must be <= %d characters, but the provided value is "
+ "%d characters.", shape.getId(), max, node.getValue().length()));
}

20
smithy-model/src/main/java/software/amazon/smithy/model/validation/node/TimestampFormatPlugin.java
View File

@ -80,19 +80,18 @@ final class TimestampFormatPlugin implements NodeValidatorPlugin {
if (value.isStringNode()) {
validateDatetime(shape, value, emitter);
} else {
emitter.accept(value, Severity.ERROR,
"Invalid " + value.getType() + " value provided for timestamp, `"
+ shape.getId() + "`. Expected a number that contains epoch seconds with "
+ "optional millisecond precision, or a string that contains an RFC 3339 "
+ "formatted timestamp (e.g., \"1985-04-12T23:20:50.52Z\")");
emitter.accept(value, "Invalid " + value.getType() + " value provided for timestamp, `"
+ shape.getId() + "`. Expected a number that contains epoch seconds with "
+ "optional millisecond precision, or a string that contains an RFC 3339 "
+ "formatted timestamp (e.g., \"1985-04-12T23:20:50.52Z\")");
}
}
}
private void validateDatetime(Shape shape, Node value, Emitter emitter) {
if (!value.isStringNode()) {
emitter.accept(value, Severity.ERROR, "Expected a string value for a date-time timestamp "
+ "(e.g., \"1985-04-12T23:20:50.52Z\")");
emitter.accept(value, "Expected a string value for a date-time timestamp "
+ "(e.g., \"1985-04-12T23:20:50.52Z\")");
return;
}
@ -101,10 +100,9 @@ final class TimestampFormatPlugin implements NodeValidatorPlugin {
// See: https://bugs.openjdk.java.net/browse/JDK-8166138
// However, Smithy doesn't allow offsets for timestamp shapes.
if (!(timestamp.endsWith("Z") && isValidFormat(timestamp, DATE_TIME_Z))) {
emitter.accept(value, Severity.ERROR,
"Invalid string value, `" + timestamp + "`, provided for timestamp, `"
+ shape.getId() + "`. Expected an RFC 3339 formatted timestamp (e.g., "
+ "\"1985-04-12T23:20:50.52Z\")");
emitter.accept(value, "Invalid string value, `" + timestamp + "`, provided for timestamp, `"
+ shape.getId() + "`. Expected an RFC 3339 formatted timestamp (e.g., "
+ "\"1985-04-12T23:20:50.52Z\")");
}
}

8
smithy-model/src/main/java/software/amazon/smithy/model/validation/node/TimestampValidationStrategy.java
View File

@ -19,7 +19,6 @@ 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.validation.Severity;
/**
* Defines how timestamps are validated.
@ -47,10 +46,9 @@ public enum TimestampValidationStrategy implements NodeValidatorPlugin {
@Override
public void apply(Shape shape, Node value, Context context, Emitter emitter) {
if (isTimestampMember(context.model(), shape) && !value.isNumberNode()) {
emitter.accept(shape, Severity.ERROR,
"Invalid " + value.getType() + " value provided for timestamp, `"
+ shape.getId() + "`. Expected a number that contains epoch "
+ "seconds with optional millisecond precision");
emitter.accept(shape, "Invalid " + value.getType() + " value provided for timestamp, `"
+ shape.getId() + "`. Expected a number that contains epoch "
+ "seconds with optional millisecond precision");
}
}
};

4
smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/DefaultTraitValidator.java
View File

@ -62,7 +62,7 @@ public final class DefaultTraitValidator extends AbstractValidator {
if (memberDefault == null) {
events.add(error(member, String.format(
"Member targets %s, which requires that the member defines the same default "
+ "of `%s`",
+ "of `%s` or `null`",
shape.toShapeId(), Node.printJson(value))));
} else if (!memberDefault.toNode().isNullNode()
&& !value.equals(member.expectTrait(DefaultTrait.class).toNode())) {
@ -101,7 +101,7 @@ public final class DefaultTraitValidator extends AbstractValidator {
return visitor;
}
} else if (value.isNullNode()) {
events.add(error(shape, trait, "The @default trait can only be set to null on members"));
events.add(error(shape, trait, "The @default trait can be set to null only on members"));
return visitor;
}

34
smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java
View File

@ -63,6 +63,7 @@ import software.amazon.smithy.model.shapes.ShapeType;
import software.amazon.smithy.model.shapes.StringShape;
import software.amazon.smithy.model.shapes.StructureShape;
import software.amazon.smithy.model.traits.BoxTrait;
import software.amazon.smithy.model.traits.DefaultTrait;
import software.amazon.smithy.model.traits.DeprecatedTrait;
import software.amazon.smithy.model.traits.DocumentationTrait;
import software.amazon.smithy.model.traits.DynamicTrait;
@ -958,6 +959,8 @@ public class ModelAssemblerTest {
.assemble()
.unwrap();
assertionChecksFor_upgradesAndDowngrades(model1);
// And through unparsed.
String contents = IoUtils.readUtf8Resource(getClass(), "needs-upgrade-document-node.json");
Model model2 = Model.assembler()
@ -965,6 +968,8 @@ public class ModelAssemblerTest {
.assemble()
.unwrap();
assertionChecksFor_upgradesAndDowngrades(model2);
// And through document node.
Node node = Node.parse(contents);
Model model3 = Model.assembler()
@ -972,6 +977,8 @@ public class ModelAssemblerTest {
.assemble()
.unwrap();
assertionChecksFor_upgradesAndDowngrades(model3);
// Pathological case.
Model model4 = Model.assembler()
.addModel(model1)
@ -982,22 +989,37 @@ public class ModelAssemblerTest {
.assemble()
.unwrap();
assertionChecksFor_upgradesAndDowngrades(model4);
assertThat(model1, equalTo(model2));
assertThat(model1, equalTo(model3));
assertThat(model1, equalTo(model4));
}
private void assertionChecksFor_upgradesAndDowngrades(Model model) {
ShapeId boxDouble = ShapeId.from("smithy.example#BoxDouble");
ShapeId primitiveDouble = ShapeId.from("smithy.example#PrimitiveDouble");
ShapeId fooMember = ShapeId.from("smithy.example#Struct$foo");
ShapeId barMember = ShapeId.from("smithy.example#Struct$bar");
ShapeId boxedMember = ShapeId.from("smithy.example#Struct$boxed");
assertThat(model.expectShape(boxDouble).hasTrait(DefaultTrait.class), is(false));
assertThat(model.expectShape(primitiveDouble).hasTrait(DefaultTrait.class), is(true));
assertThat(model.expectShape(fooMember).hasTrait(DefaultTrait.class), is(true));
assertThat(model.expectShape(barMember).hasTrait(DefaultTrait.class), is(false));
assertThat(model.expectShape(boxedMember).hasTrait(DefaultTrait.class), is(true));
assertThat(model.expectShape(boxedMember).expectTrait(DefaultTrait.class).toNode(), equalTo(Node.nullNode()));
assertThat(model.expectShape(boxedMember).hasTrait(BoxTrait.class), is(true));
}
@Test
public void patches2_0_documentNodesToo() {
ShapeId boxDouble = ShapeId.from("smithy.example#BoxDouble");
// Loads fine through import.
Model model1 = Model.assembler()
.addImport(getClass().getResource("needs-downgrade-document-node.json"))
.assemble()
.unwrap();
assertThat(model1.expectShape(boxDouble).hasTrait(BoxTrait.class), is(true));
assertionChecksFor_upgradesAndDowngrades(model1);
// And through unparsed.
String contents = IoUtils.readUtf8Resource(getClass(), "needs-downgrade-document-node.json");
@ -1006,7 +1028,7 @@ public class ModelAssemblerTest {
.assemble()
.unwrap();
assertThat(model2.expectShape(boxDouble).hasTrait(BoxTrait.class), is(true));
assertionChecksFor_upgradesAndDowngrades(model2);
// And through document node.
Node node = Node.parse(contents);
@ -1015,7 +1037,7 @@ public class ModelAssemblerTest {
.assemble()
.unwrap();
assertThat(model3.expectShape(boxDouble).hasTrait(BoxTrait.class), is(true));
assertionChecksFor_upgradesAndDowngrades(model3);
// Pathological case.
Model model4 = Model.assembler()
@ -1027,7 +1049,7 @@ public class ModelAssemblerTest {
.assemble()
.unwrap();
assertThat(model4.expectShape(boxDouble).hasTrait(BoxTrait.class), is(true));
assertionChecksFor_upgradesAndDowngrades(model4);
assertThat(model1, equalTo(model2));
assertThat(model1, equalTo(model3));

4
smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelUpgraderTest.java → smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelInteropTransformerTest.java
View File

@ -38,7 +38,7 @@ import software.amazon.smithy.utils.IoUtils;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.Pair;
public class ModelUpgraderTest {
public class ModelInteropTransformerTest {
@Test
public void upgradesWhenAllModelsUse1_0() {
UpgradeTestCase testCase = UpgradeTestCase.createAndValidate("upgrade/all-1.0");
@ -144,7 +144,7 @@ public class ModelUpgraderTest {
}
private static UpgradeTestCase createFromDirectory(String directory) {
try (Stream<Path> paths = Files.walk(Paths.get(ModelUpgraderTest.class.getResource(directory).toURI()))) {
try (Stream<Path> paths = Files.walk(Paths.get(ModelInteropTransformerTest.class.getResource(directory).toURI()))) {
UpgradeTestCase testCase = new UpgradeTestCase();
paths.filter(Files::isRegularFile).forEach(file -> {
if (file.endsWith("upgraded.smithy") || file.endsWith("upgraded.json")) {

2
smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/defaults/default-with-null.errors
View File

@ -1 +1 @@
[ERROR] smithy.example#NullableInteger: The @default trait can only be set to null on members | DefaultTrait
[ERROR] smithy.example#NullableInteger: The @default trait can be set to null only on members | DefaultTrait

6
smithy-model/src/test/resources/software/amazon/smithy/model/loader/needs-downgrade-document-node.json
View File

@ -21,6 +21,12 @@
},
"bar": {
"target": "smithy.example#BoxDouble"
},
"boxed": {
"target": "smithy.example#PrimitiveDouble",
"traits": {
"smithy.api#default": null
}
}
}
}

6
smithy-model/src/test/resources/software/amazon/smithy/model/loader/needs-upgrade-document-node.json
View File

@ -18,6 +18,12 @@
},
"bar": {
"target": "smithy.example#BoxDouble"
},
"boxed": {
"target": "smithy.example#PrimitiveDouble",
"traits": {
"smithy.api#box": {}
}
}
}
}