Skip to content

Commit def6db8

Browse files
adamthom-amznadamthom-amzn
committed
Revert "Parse timestamps strictly"
This reverts commit 885fec0.
1 parent 885fec0 commit def6db8

File tree

5 files changed

+41
-120
lines changed

5 files changed

+41
-120
lines changed

‎smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberDeserVisitor.java

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -89,21 +89,6 @@ public DocumentMemberDeserVisitor(
8989
this.defaultTimestampFormat = defaultTimestampFormat;
9090
}
9191

92-
/**
93-
* @return the member this visitor is being run against. Used to discover member-applied
94-
* traits, such as @timestampFormat. Can be, and defaults, to, null.
95-
*/
96-
protected MemberShape getMemberShape() {
97-
return null;
98-
}
99-
100-
/**
101-
* @return true if string-formatted epoch seconds in payloads are disallowed. Defaults to false.
102-
*/
103-
protected boolean requiresNumericEpochSecondsInPayload() {
104-
return false;
105-
}
106-
10792
/**
10893
* Gets the generation context.
10994
*
@@ -223,25 +208,8 @@ public final String memberShape(MemberShape shape) {
223208
@Override
224209
public String timestampShape(TimestampShape shape) {
225210
HttpBindingIndex httpIndex = HttpBindingIndex.of(context.getModel());
226-
Format format;
227-
if (getMemberShape() == null) {
228-
format = httpIndex.determineTimestampFormat(shape, Location.DOCUMENT, defaultTimestampFormat);
229-
} else {
230-
if (!shape.getId().equals(getMemberShape().getTarget())) {
231-
throw new IllegalArgumentException(
232-
String.format("Encountered timestamp shape %s that was not the target of member shape %s",
233-
shape.getId(), getMemberShape().getId()));
234-
}
235-
format = httpIndex.determineTimestampFormat(getMemberShape(), Location.DOCUMENT, defaultTimestampFormat);
236-
}
237-
238-
return HttpProtocolGeneratorUtils.getTimestampOutputParam(
239-
context.getWriter(),
240-
dataSource,
241-
Location.DOCUMENT,
242-
shape,
243-
format,
244-
requiresNumericEpochSecondsInPayload());
211+
Format format = httpIndex.determineTimestampFormat(shape, Location.DOCUMENT, defaultTimestampFormat);
212+
return HttpProtocolGeneratorUtils.getTimestampOutputParam(dataSource, Location.DOCUMENT, shape, format);
245213
}
246214

247215
@Override

‎smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2579,9 +2579,7 @@ private String getOutputValue(
25792579
} else if (target instanceof TimestampShape) {
25802580
HttpBindingIndex httpIndex = HttpBindingIndex.of(context.getModel());
25812581
Format format = httpIndex.determineTimestampFormat(member, bindingType, getDocumentTimestampFormat());
2582-
return HttpProtocolGeneratorUtils.getTimestampOutputParam(
2583-
context.getWriter(), dataSource, bindingType, member, format,
2584-
requiresNumericEpochSecondsInPayload());
2582+
return HttpProtocolGeneratorUtils.getTimestampOutputParam(dataSource, bindingType, member, format);
25852583
} else if (target instanceof BlobShape) {
25862584
return getBlobOutputParam(bindingType, dataSource);
25872585
} else if (target instanceof CollectionShape) {
@@ -2935,9 +2933,4 @@ protected abstract void deserializeErrorDocumentBody(
29352933
StructureShape error,
29362934
List<HttpBinding> documentBindings
29372935
);
2938-
2939-
/**
2940-
* @return true if this protocol disallows string epoch timestamps in payloads.
2941-
*/
2942-
protected abstract boolean requiresNumericEpochSecondsInPayload();
29432936
}

‎smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -88,44 +88,35 @@ public static String getTimestampInputParam(
8888
* Given a format and a source of data, generate an output value provider for the
8989
* timestamp.
9090
*
91-
* @param writer The current writer (so that imports may be added)
9291
* @param dataSource The in-code location of the data to provide an output of
9392
* ({@code output.foo}, {@code entry}, etc.)
9493
* @param bindingType How this value is bound to the operation output.
9594
* @param shape The shape that represents the value being received.
9695
* @param format The timestamp format to provide.
97-
* @param requireNumericEpochSecondsInPayload if true, paylaod epoch seconds are not allowed to be coerced
98-
* from strings.
9996
* @return Returns a value or expression of the output timestamp.
10097
*/
101-
public static String getTimestampOutputParam(TypeScriptWriter writer,
102-
String dataSource,
103-
Location bindingType,
104-
Shape shape,
105-
Format format,
106-
boolean requireNumericEpochSecondsInPayload) {
107-
// This has always explicitly wrapped the dataSource in "new Date(..)", so it could never generate
108-
// an expression that evaluates to null. Codegen relies on this.
109-
writer.addImport("expectNonNull", "__expectNonNull", "@aws-sdk/smithy-client");
98+
public static String getTimestampOutputParam(String dataSource, Location bindingType, Shape shape, Format format) {
99+
String modifiedSource;
110100
switch (format) {
111101
case DATE_TIME:
112-
writer.addImport("parseRfc3339DateTime", "__parseRfc3339DateTime", "@aws-sdk/smithy-client");
113-
return String.format("__expectNonNull(__parseRfc3339DateTime(%s))", dataSource);
114102
case HTTP_DATE:
115-
writer.addImport("parseRfc7231DateTime", "__parseRfc7231DateTime", "@aws-sdk/smithy-client");
116-
return String.format("__expectNonNull(__parseRfc7231DateTime(%s))", dataSource);
103+
modifiedSource = dataSource;
104+
break;
117105
case EPOCH_SECONDS:
118-
writer.addImport("parseEpochTimestamp", "__parseEpochTimestamp", "@aws-sdk/smithy-client");
119-
String modifiedDataSource = dataSource;
120-
if (requireNumericEpochSecondsInPayload
121-
&& (bindingType == Location.DOCUMENT || bindingType == Location.PAYLOAD)) {
122-
writer.addImport("expectNumber", "__expectNumber", "@aws-sdk/smithy-client");
123-
modifiedDataSource = String.format("__expectNumber(%s)", dataSource);
106+
modifiedSource = dataSource;
107+
// Make sure we turn a header, label, or query's forced string into a number.
108+
if (bindingType.equals(Location.HEADER) || bindingType.equals(Location.QUERY)
109+
|| bindingType.equals(Location.LABEL)) {
110+
modifiedSource = "parseInt(" + modifiedSource + ", 10)";
124111
}
125-
return String.format("__expectNonNull(__parseEpochTimestamp(%s))", modifiedDataSource);
112+
// Convert whole and decimal numbers to milliseconds.
113+
modifiedSource = "Math.round(" + modifiedSource + " * 1000)";
114+
break;
126115
default:
127116
throw new CodegenException("Unexpected timestamp format `" + format.toString() + "` on " + shape);
128117
}
118+
119+
return "new Date(" + modifiedSource + ")";
129120
}
130121

131122
/**

‎smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberDeserVisitorTest.java

Lines changed: 18 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import software.amazon.smithy.codegen.core.CodegenException;
1212
import software.amazon.smithy.codegen.core.Symbol;
1313
import software.amazon.smithy.codegen.core.SymbolProvider;
14-
import software.amazon.smithy.model.Model;
1514
import software.amazon.smithy.model.shapes.BlobShape;
1615
import software.amazon.smithy.model.shapes.BooleanShape;
1716
import software.amazon.smithy.model.shapes.ByteShape;
@@ -32,10 +31,8 @@
3231
import software.amazon.smithy.model.shapes.ShortShape;
3332
import software.amazon.smithy.model.shapes.StringShape;
3433
import software.amazon.smithy.model.shapes.StructureShape;
35-
import software.amazon.smithy.model.shapes.TimestampShape;
3634
import software.amazon.smithy.model.shapes.UnionShape;
3735
import software.amazon.smithy.model.traits.MediaTypeTrait;
38-
import software.amazon.smithy.model.traits.TimestampFormatTrait;
3936
import software.amazon.smithy.model.traits.TimestampFormatTrait.Format;
4037
import software.amazon.smithy.typescript.codegen.TypeScriptWriter;
4138
import software.amazon.smithy.typescript.codegen.integration.ProtocolGenerator.GenerationContext;
@@ -56,65 +53,40 @@ public class DocumentMemberDeserVisitorTest {
5653

5754
@ParameterizedTest
5855
@MethodSource("validMemberTargetTypes")
59-
public void providesExpectedDefaults(Shape shape, String expected, MemberShape memberShape) {
60-
Shape fakeStruct = StructureShape.builder().id("com.smithy.example#Enclosing").addMember(memberShape).build();
61-
mockContext.setModel(Model.builder().addShapes(shape, fakeStruct).build());
62-
DocumentMemberDeserVisitor visitor =
63-
new DocumentMemberDeserVisitor(mockContext, DATA_SOURCE, FORMAT) {
64-
@Override
65-
protected MemberShape getMemberShape() {
66-
return memberShape;
67-
}
68-
};
56+
public void providesExpectedDefaults(Shape shape, String expected) {
57+
DocumentMemberDeserVisitor visitor = new DocumentMemberDeserVisitor(mockContext, DATA_SOURCE, FORMAT);
6958
assertThat(expected, equalTo(shape.accept(visitor)));
7059
}
7160

7261
public static Collection<Object[]> validMemberTargetTypes() {
7362
String id = "com.smithy.example#Foo";
7463
String targetId = id + "Target";
75-
MemberShape source = MemberShape.builder().id("com.smithy.example#Enclosing$sourceMember").target(id).build();
7664
MemberShape member = MemberShape.builder().id(id + "$member").target(targetId).build();
7765
MemberShape key = MemberShape.builder().id(id + "$key").target(targetId).build();
7866
MemberShape value = MemberShape.builder().id(id + "$value").target(targetId).build();
7967
String delegate = "deserialize" + ProtocolGenerator.getSanitizedName(PROTOCOL) + "Foo"
8068
+ "(" + DATA_SOURCE + ", context)";
8169

8270
return ListUtils.of(new Object[][]{
83-
{BooleanShape.builder().id(id).build(), "__expectBoolean(" + DATA_SOURCE + ")", source},
84-
{ByteShape.builder().id(id).build(), "__expectByte(" + DATA_SOURCE + ")", source},
85-
{DoubleShape.builder().id(id).build(), "__limitedParseDouble(" + DATA_SOURCE + ")", source},
86-
{FloatShape.builder().id(id).build(), "__limitedParseFloat32(" + DATA_SOURCE + ")", source},
87-
{IntegerShape.builder().id(id).build(), "__expectInt32(" + DATA_SOURCE + ")", source},
88-
{LongShape.builder().id(id).build(), "__expectLong(" + DATA_SOURCE + ")", source},
89-
{ShortShape.builder().id(id).build(), "__expectShort(" + DATA_SOURCE + ")", source},
90-
{StringShape.builder().id(id).build(), "__expectString(" + DATA_SOURCE + ")", source},
71+
{BooleanShape.builder().id(id).build(), "__expectBoolean(" + DATA_SOURCE + ")"},
72+
{ByteShape.builder().id(id).build(), "__expectByte(" + DATA_SOURCE + ")"},
73+
{DoubleShape.builder().id(id).build(), "__limitedParseDouble(" + DATA_SOURCE + ")"},
74+
{FloatShape.builder().id(id).build(), "__limitedParseFloat32(" + DATA_SOURCE + ")"},
75+
{IntegerShape.builder().id(id).build(), "__expectInt32(" + DATA_SOURCE + ")"},
76+
{LongShape.builder().id(id).build(), "__expectLong(" + DATA_SOURCE + ")"},
77+
{ShortShape.builder().id(id).build(), "__expectShort(" + DATA_SOURCE + ")"},
78+
{StringShape.builder().id(id).build(), "__expectString(" + DATA_SOURCE + ")"},
9179
{
9280
StringShape.builder().id(id).addTrait(new MediaTypeTrait("foo+json")).build(),
93-
"new __LazyJsonString(" + DATA_SOURCE + ")",
94-
source
95-
},
96-
{BlobShape.builder().id(id).build(), "context.base64Decoder(" + DATA_SOURCE + ")", source},
97-
{DocumentShape.builder().id(id).build(), delegate, source},
98-
{ListShape.builder().id(id).member(member).build(), delegate, source},
99-
{SetShape.builder().id(id).member(member).build(), delegate, source},
100-
{MapShape.builder().id(id).key(key).value(value).build(), delegate, source},
101-
{StructureShape.builder().id(id).build(), delegate, source},
102-
{UnionShape.builder().id(id).addMember(member).build(), delegate, source},
103-
{
104-
TimestampShape.builder().id(id).build(),
105-
"__expectNonNull(__parseEpochTimestamp(" + DATA_SOURCE + "))",
106-
source
107-
},
108-
{
109-
TimestampShape.builder().id(id).build(),
110-
"__expectNonNull(__parseRfc3339DateTime(" + DATA_SOURCE + "))",
111-
source.toBuilder().addTrait(new TimestampFormatTrait(TimestampFormatTrait.DATE_TIME)).build()
112-
},
113-
{
114-
TimestampShape.builder().id(id).build(),
115-
"__expectNonNull(__parseRfc7231DateTime(" + DATA_SOURCE + "))",
116-
source.toBuilder().addTrait(new TimestampFormatTrait(TimestampFormatTrait.HTTP_DATE)).build()
81+
"new __LazyJsonString(" + DATA_SOURCE + ")"
11782
},
83+
{BlobShape.builder().id(id).build(), "context.base64Decoder(" + DATA_SOURCE + ")"},
84+
{DocumentShape.builder().id(id).build(), delegate},
85+
{ListShape.builder().id(id).member(member).build(), delegate},
86+
{SetShape.builder().id(id).member(member).build(), delegate},
87+
{MapShape.builder().id(id).key(key).value(value).build(), delegate},
88+
{StructureShape.builder().id(id).build(), delegate},
89+
{UnionShape.builder().id(id).addMember(member).build(), delegate},
11890
});
11991
}
12092

‎smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtilsTest.java

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,13 @@ public void givesCorrectTimestampSerialization() {
3939
@Test
4040
public void givesCorrectTimestampDeserialization() {
4141
TimestampShape shape = TimestampShape.builder().id("com.smithy.example#Foo").build();
42-
TypeScriptWriter writer = new TypeScriptWriter("foo");
4342

44-
assertThat("__expectNonNull(__parseRfc3339DateTime(" + DATA_SOURCE + "))",
45-
equalTo(HttpProtocolGeneratorUtils.getTimestampOutputParam(writer, DATA_SOURCE, Location.DOCUMENT, shape, Format.DATE_TIME, false)));
46-
assertThat("__expectNonNull(__parseEpochTimestamp(__expectNumber(" + DATA_SOURCE + ")))",
47-
equalTo(HttpProtocolGeneratorUtils.getTimestampOutputParam(writer, DATA_SOURCE, Location.DOCUMENT, shape, Format.EPOCH_SECONDS, true)));
48-
assertThat("__expectNonNull(__parseEpochTimestamp(" + DATA_SOURCE + "))",
49-
equalTo(HttpProtocolGeneratorUtils.getTimestampOutputParam(writer, DATA_SOURCE, Location.DOCUMENT, shape, Format.EPOCH_SECONDS, false)));
50-
assertThat("__expectNonNull(__parseRfc7231DateTime(" + DATA_SOURCE + "))",
51-
equalTo(HttpProtocolGeneratorUtils.getTimestampOutputParam(writer, DATA_SOURCE, Location.DOCUMENT, shape, Format.HTTP_DATE, false)));
43+
assertThat("new Date(" + DATA_SOURCE + ")",
44+
equalTo(HttpProtocolGeneratorUtils.getTimestampOutputParam(DATA_SOURCE, Location.DOCUMENT, shape, Format.DATE_TIME)));
45+
assertThat("new Date(Math.round(" + DATA_SOURCE + " * 1000))",
46+
equalTo(HttpProtocolGeneratorUtils.getTimestampOutputParam(DATA_SOURCE, Location.DOCUMENT, shape, Format.EPOCH_SECONDS)));
47+
assertThat("new Date(" + DATA_SOURCE + ")",
48+
equalTo(HttpProtocolGeneratorUtils.getTimestampOutputParam(DATA_SOURCE, Location.DOCUMENT, shape, Format.HTTP_DATE)));
5249
}
5350

5451
@Test

0 commit comments

Comments
 (0)