Skip to content

Commit

Permalink
Fix member timestamp format node validation
Browse files Browse the repository at this point in the history
The NodeValidationVisitor vists shapes, members, and then the targets
of members. When visiting the target of a member, the context of the
referring member was lost, which meant a member with a timestampFormat
trait like http-date isn't something the target shape has access to,
which will cause validation to fail. We now pass in the referring
member as part of the validation context.

Closes #1946
  • Loading branch information
mtdowling committed Aug 24, 2023
1 parent 0f1c378 commit 302ace8
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,13 @@ public List<ValidationEvent> unionShape(UnionShape shape) {
@Override
public List<ValidationEvent> memberShape(MemberShape shape) {
List<ValidationEvent> events = applyPlugins(shape);
events.addAll(model.getShape(shape.getTarget())
.map(member -> member.accept(this))
.orElse(ListUtils.of()));
model.getShape(shape.getTarget()).ifPresent(target -> {
// We only need to keep track of a single referring member, so a stack of members or anything like that
// isn't needed here.
validationContext.setReferringMember(shape);
events.addAll(target.accept(this));
validationContext.setReferringMember(null);
});
return events;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import software.amazon.smithy.model.FromSourceLocation;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.selector.Selector;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.validation.NodeValidationVisitor;
import software.amazon.smithy.model.validation.Severity;
Expand Down Expand Up @@ -72,6 +74,7 @@ static List<NodeValidatorPlugin> getBuiltins() {
final class Context {
private final Model model;
private final Set<NodeValidationVisitor.Feature> features;
private MemberShape referringMember;

// Use an LRU cache to ensure the Selector cache doesn't grow too large
// when given bad inputs.
Expand Down Expand Up @@ -121,6 +124,14 @@ public Set<Shape> select(Selector selector) {
public boolean hasFeature(NodeValidationVisitor.Feature feature) {
return features.contains(feature);
}

public void setReferringMember(MemberShape referringMember) {
this.referringMember = referringMember;
}

public Optional<MemberShape> getReferringMember() {
return Optional.ofNullable(referringMember);
}
}

@SmithyInternalApi
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ final class TimestampFormatPlugin implements NodeValidatorPlugin {
@Override
public void apply(Shape shape, Node value, Context context, Emitter emitter) {
if (shape instanceof TimestampShape) {
validate(shape, shape.getTrait(TimestampFormatTrait.class).orElse(null), value, emitter);
// Don't validate the timestamp target if a referring member had the timestampFormat trait.
boolean fromMemberWithTrait = context.getReferringMember()
.filter(member -> member.hasTrait(TimestampFormatTrait.class))
.isPresent();
if (!fromMemberWithTrait) {
validate(shape, shape.getTrait(TimestampFormatTrait.class).orElse(null), value, emitter);
}
} else if (shape instanceof MemberShape && shape.getTrait(TimestampFormatTrait.class).isPresent()) {
// Only perform timestamp format validation on a member when it references
// a timestamp shape and the member has an explicit timestampFormat trait.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public class ValidationEventDecoratorTest {
static final String UNREFERENCED_SHAPE_EVENT_ID = "UnreferencedShape";
static final Set<ShapeId> STRUCT_SHAPE_IDS = SetUtils.of(ShapeId.from("ns.foo#Structure"),
ShapeId.from("ns.foo#Structure2"),
ShapeId.from("ns.foo#Structure3"));
ShapeId.from("ns.foo#Structure3"),
ShapeId.from("ns.foo#Structure4"));

@Test
public void canDecorateValidationEvents() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,11 @@ public static Collection<Object[]> data() {
// timestamp member with format.
{"ns.foo#TimestampList", "[\"1985-04-12T23:20:50.52Z\"]", null},
{"ns.foo#TimestampList", "[\"1985-04-12T23:20:50.52-07:00\"]", new String[] {
"0: Invalid string value, `1985-04-12T23:20:50.52-07:00`, provided for timestamp, `smithy.api#Timestamp`. Expected an RFC 3339 formatted timestamp (e.g., \"1985-04-12T23:20:50.52Z\")",
"0: Invalid string value, `1985-04-12T23:20:50.52-07:00`, provided for timestamp, `ns.foo#TimestampList$member`. Expected an RFC 3339 formatted timestamp (e.g., \"1985-04-12T23:20:50.52Z\")"
}},
{"ns.foo#TimestampList", "[123]", new String[] {"0: Expected a string value for a date-time timestamp (e.g., \"1985-04-12T23:20:50.52Z\")"}},
{"ns.foo#Structure4", "{\"httpDate\": 1234}", new String[] {"httpDate: Invalid value provided for http-date formatted timestamp. Expected a string value that matches the IMF-fixdate production of RFC 7231 section-7.1.1.1. Found: number"}},
{"ns.foo#Structure4", "{\"httpDateTarget\": 1234}", new String[] {"httpDateTarget: Invalid value provided for http-date formatted timestamp. Expected a string value that matches the IMF-fixdate production of RFC 7231 section-7.1.1.1. Found: number"}},

// timestamp member with no format.
{"ns.foo#TimestampListNoFormatTrait", "[123]", null},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,20 @@
}
}
},
"ns.foo#Structure4": {
"type": "structure",
"members": {
"httpDate": {
"target": "smithy.api#Timestamp",
"traits": {
"smithy.api#timestampFormat": "http-date"
}
},
"httpDateTarget": {
"target": "ns.foo#HttpDate"
}
}
},
"ns.foo#Service": {
"type": "service",
"version": "2017-17-01",
Expand Down

0 comments on commit 302ace8

Please sign in to comment.