Feature/json builder #11
Conversation
packages/json-builder/index.ts
Outdated
|
|
||
| type JsonValue = Json|Scalar|JsonArray; | ||
|
|
||
| // TODO move to @aws/types |
packages/json-builder/index.ts
Outdated
| private readonly utf8Decoder: Decoder | ||
| ) {} | ||
|
|
||
| public build<T extends object>(shape: SerializationModel, input: T): string { |
There was a problem hiding this comment.
Generally we should have docstrings for the public interfaces.
packages/json-builder/index.ts
Outdated
| private format(shape: SerializationModel, input: any): JsonValue { | ||
| if (shape.type === 'structure') { | ||
| const data: Json = {}; | ||
| // The validator should have ensured that this input is an object |
There was a problem hiding this comment.
It seems per discussion that we mean this is the responsibility of the caller, which is not clear from this comment. Let's clarify that here.
There was a problem hiding this comment.
My reading of "The validator" was that it was elsewhere in this package.
There was a problem hiding this comment.
In line with how the responsibilities between serializers and validators are separated in other SDKs (e.g., ruby and php), the JSON builder is not expected to perform validation but just trusts that the data it has been handed fulfills the contract of the supplied serialization model. Adding a comment kind of makes this trust official, whereas other SDKs take it for granted.
However, I take your point and will throw an exception here. TypeScript definitions can mostly take the place of runtime validation, so I would expect many TS users to disable validation, and I wouldn't be surprised if we make it an opt-in rather than an opt-out part of the v3 SDK.
packages/json-parser/index.ts
Outdated
|
|
||
| type JsonValue = Json|Scalar|JsonArray; | ||
|
|
||
| // TODO move to @aws/types |
packages/json-parser/index.ts
Outdated
| export class JsonParser { | ||
| constructor(private readonly base64Decoder: Decoder) {} | ||
|
|
||
| public parse<T extends object>( |
| return undefined; | ||
| } | ||
|
|
||
| return toDate(input); |
There was a problem hiding this comment.
Is this date conversion method always proper?
There was a problem hiding this comment.
Yes. All date formats returned by AWS services (to wit, RFC-822, ISO-8601, and unix epoch timestamps) can be handled by the toDate function.
|
@chrisradek This has been rebased against master after merging in #35. Since the commit history's usefulness was wiped out by the rebase, I've squashed this PR down to a single commit. |
| Timestamp, | ||
| } from "@aws/types"; | ||
|
|
||
| describe('JsonBuilder', () => { |
There was a problem hiding this comment.
s/JsonBuilder/JsonParser/
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |
This PR adds a JSON builder and parser for use with the JSON-RPC and REST-JSON marshallers and unmarshallers. It includes and depends on #14