support eventstream traits #110

AllanZhengYP · 2020-01-29T19:44:52Z

This change adds codegen support for eventstream serde. If a member has event stream trait, it will generate specitial event serde functions on top of original ones. Specifically:

Serializer:
- event stream serializer:

//TranscribeStreaming.serializeAws_restJson1_1StartStreamTranscriptionCommand
//...
if (input.AudioStream !== undefined) {
    body =
      context.eventStreamMarshaller.serialize(
        input.AudioStream,
        event => serializeAws_restJson1_1AudioStream_event(event, context)
      );
  }
//build request

if a multi-event event stream, we generate a event dispatcher:

const serializeAws_restJson1_1AudioStream_event = (
  input: any,
  context: __SerdeContext
): __Message => {
  return AudioStream.visit(input, {
    AudioEvent: value => serializeAws_restJson1_1AudioEvent_event(value, context),
    _: value => value as any
  });
}

serializing a single event

const serializeAws_restJson1_1AudioEvent_event = (
  input: AudioEvent,
  context: __SerdeContext
): __Message => {
  const message: __Message = {
    headers: {
      ":event-type": { type: "string", value: "AudioEvent" },
      ":message-type": { type: "string", value: "event" }
    },
    body: new Uint8Array()
  }
  message.body = input.AudioChunk!;
  return message;
}

Deserializer
- event stream deserializer

// call eventStreamMarshaller.deserialize() from command deser
// convert eventname -> event message pair to event name -> event response pair
// after this step, individual event is convert to psuedo HTTP response shape
const data: any =
    context.eventStreamMarshaller.deserialize(
      output.body,
      async event => {
        const eventName = Object.keys(event)[0];
        const eventValue = event[eventName];
        const eventHeader = Object.entries(eventValue.headers).reduce(
          (accummulator, curr) => {accummulator[curr[0]] = curr[1].value; return accummulator; },
          {} as {[key: string]: any}
        );
        const eventMessage = {
          headers: eventHeader,
          body: event[eventName].body
        };
        const parsedEvent = {
          [eventName]: eventMessage
        };
        return await deserializeAws_restJson1_1TranscriptResultStream_event(parsedEvent, context);
      }
    );

  contents.TranscriptResultStream = data;

For multi-event event stream, we generate a event visitor to dispatch individual events

const deserializeAws_restJson1_1TranscriptResultStream_event = async (
  output: any,
  context: __SerdeContext
): Promise<TranscriptResultStream> => {
  if (output['TranscriptEvent'] !== undefined) {
    return {
      TranscriptEvent: await deserializeAws_restJson1_1TranscriptEvent_event(output['TranscriptEvent'], context)
    };
  }
  if(/*other events*/){
  /*other events*/
  }
  return {$unknown: output}
}

Deserialize each event like parsing a response

const deserializeAws_restJson1_1TranscriptEvent_event = async (
  output: any,
  context: __SerdeContext
): Promise<TranscriptEvent> => {
  let contents: TranscriptEvent = {
    __type: "TranscriptEvent",
  } as any;
  const data: any = await parseBody(output.body, context);
  contents = {
    ...contents,
    ...deserializeAws_restJson1_1TranscriptEvent(data, context)
  } as any
  return contents;
}

AllanZhengYP · 2020-01-30T20:34:42Z

I have tested the codegen with kinesis, transcribe-streaming and s3 select, they all work fine. We are still missing the dependencies for browser, but that work will be done on SDK side

AllanZhengYP · 2020-02-04T21:32:22Z

Commit with generated S3 client: AllanZhengYP/aws-sdk-js-v3@1d41801

kstich

First pass of comments since this needs a rebase.

It's hard to detect if changes were made in the reformatted code, can the automatic formatting be undone?
Can you include a sample of the different kinds of generated code in the PR description?

I'll continue on the meatier portion of the code after these adjustments.

kstich · 2020-01-31T19:54:34Z

...rc/main/java/software/amazon/smithy/typescript/codegen/integration/EventStreamGenerator.java

+                TypeScriptDependency.AWS_SDK_TYPES.packageName
+        );
+        writer.writeDocs("The function that provides necessary utilities for generating and signing event stream");
+        writer.write("eventStreamSerdeProvider?: EventStreamSerdeProvider;");


Should have a \n at the end of this line for cleanliness.

kstich · 2020-01-31T20:24:42Z

...rc/main/java/software/amazon/smithy/typescript/codegen/integration/EventStreamGenerator.java

+        }
+        writer.addImport(
+                "EventStreamSerdeProvider",
+                "EventStreamSerdeProvider",


This should have an alias of __EventStreamSerdeProvider and adjust the type of the parameter below.

kstich · 2020-01-31T20:41:46Z

...rc/main/java/software/amazon/smithy/typescript/codegen/integration/EventStreamGenerator.java

+    public List<RuntimeClientPlugin> getClientPlugins() {
+        return ListUtils.of(
+                RuntimeClientPlugin.builder()
+                        .withConventions(


Formatting: The first param should be in the same line as the open param if it fits, the close paren should be on the same line as the last param.

...typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenUtils.java

kstich · 2020-02-04T23:47:27Z

...rc/main/java/software/amazon/smithy/typescript/codegen/integration/EventStreamGenerator.java

+/**
+ * Adds event streams if needed.
+ */
+public class EventStreamGenerator implements TypeScriptIntegration {


This should be more clearly named and match the other integrations, AddEventStreamDependency

kstich · 2020-02-04T23:50:46Z

...rc/main/java/software/amazon/smithy/typescript/codegen/integration/EventStreamGenerator.java

+        return false;
+    }
+
+    public static final boolean operationHasEventStream(


This function should be private.

The operationHasEventStreamInput and operationHasEventStreamOutput can probably also be rolled in to this function.

I marked operationHasEventStream, operationHasEventStreamInput, and operationHasEventStreamOutput public because they are referred outside this class/package. I store all the event stream codegen related util functions here in this class.

Where should I put them to if not here?

...java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java

kstich · 2020-02-05T00:03:23Z

...n/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java

+                    writer.openBlock("if (streamBody instanceof Uint8Array) {", "}", () -> {
+                        writer.write("return Promise.resolve(streamBody);");
+                    });
+                    writer.write("return context.streamCollector(streamBody) || new Uint8Array();");


The OR of this statement won't return a Promise as indicated.

kstich · 2020-02-05T00:03:38Z

...n/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java

+     * @param events  The union of of events bond to an event stream
+     * @return A list of all event structure shapes for the event stream that were dispatched to.
+     */
+    static Set<StructureShape> generateSerializingEvnetUnion(


Typo in name.

kstich · 2020-02-05T00:20:00Z

...rc/main/java/software/amazon/smithy/typescript/codegen/integration/EventStreamGenerator.java

+     * @param shape shape bond to event header
+     * @return string literal for the event message header type
+     */
+    public static String getEventHeaderType(Shape shape) {


This looks like it's only used in one place, which isn't in this class. It can probably be moved there and be made private.

I can. I store all the event stream related util functions in this class, should I move it to already-too-long protocol binding class?

kstich

Let's chat code formatting in person after updates.

...typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenUtils.java

kstich · 2020-02-05T22:58:31Z

...java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java

+                    writer.openBlock("const message: __Message = {", "}", () -> {
+                        writer.openBlock("headers: {", "},", () -> {
+                            //fix headers required by event stream
+                            writer.write("\":event-type\": { type: \"string\", value: \"$L\" },", symbol.getName());


Instead of doing \"$L\" you can do $S to write the value inside quotes. This applies in multiple places.

kstich · 2020-02-06T00:34:58Z

...ain/java/software/amazon/smithy/typescript/codegen/integration/AddEventStreamDependency.java

+        return false;
+    }
+
+    public static final boolean operationHasEventStream(


This method is only used in this class and can be made private.

kstich · 2020-02-06T00:40:04Z

...ain/java/software/amazon/smithy/typescript/codegen/integration/AddEventStreamDependency.java

+        return false;
+    }
+
+    public static final boolean operationHasEventStreamInput(


Given how simple this function and its output variant are, and that they're only used in one place in each Java package supplied here, it's probably good to just inline this check in the other locations. This will mean this class doesn't need to have fully publicly exposed methods for now.

kstich · 2020-02-06T00:41:21Z

...ain/java/software/amazon/smithy/typescript/codegen/integration/AddEventStreamDependency.java

+     * @param shape shape bond to event header
+     * @return string literal for the event message header type
+     */
+    public static String getEventHeaderType(Shape shape) {


This is only used in one location not in this class and can be moved there and made private.

kstich · 2020-02-06T01:11:37Z

...java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java

-        if (hasStreamingComponent) {
+        if (binding.getMember().hasTrait(EventStreamTrait.class)) {
+            // If payload is a event stream, return it after calling event stream deser function.
+            writer.write("const data: any = ").indent();


This can be combined with the openBlock call in the function call that follows, removing the need to do indent/dedent.

...java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java

kstich · 2020-02-06T01:21:45Z

...java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java

+                            writer.write("__type: $S,", event.getId().getName());
+                        }
+                    });
+                    // Parse members from event headers.


Can these chunks of code for handling message headers and bodies be broken down in to other functions like for standard http responses.

kstich · 2020-02-06T01:26:29Z

...n/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java

+                    writer.openBlock("return {", "};", () -> {
+                        // Dispatch to special event deserialize function
+                        Symbol eventSymbol = symbolProvider.toSymbol(target);
+                        String eventDeserMethodName =


This building process can be condensed quite a bit:

Symbol eventSymbol = symbolProvider.toSymbol(target); writer.write("$1L: await $2L(output[$1S], context)", name, ProtocolGenerator.getDeserFunctionName(eventSymbol, protocolName) + "_event");

This kind of clean up can be done in a couple places.

I don't think this would work because context is not passed to each union member's deser function.

kstich · 2020-02-06T01:27:42Z

...n/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java

+     * @param events The union of of events bond to an event stream
+     * @return A list of all event structure shapes for the event stream that were dispatched to.
+     */
+    static Set<StructureShape> generateDeserializingEventUnion(


It's good that these are package private for now, but do they need to exist on this class since they're only used in the HttpBindingProtocolGenerator?

kstich · 2020-02-13T22:44:14Z

...java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java

+            //There's only one event payload member
+            MemberShape payloadMember = payloadMembers.get(0);
+            readEventPayload(context, payloadMember);
+            writer.write("contents.$L = data;", payloadMember.getMemberName());


This line should move in to the readEventPayload method.

kstich · 2020-02-13T22:45:58Z

...java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java

+            // If payload is string, we need to collect body and encode binary to string.
+            writer.write("const data: any = await collectBodyString(output.body, context);");
+        } else {
+            throw new CodegenException(String.format("Unexpected shape type bound to payload: `%s`",


Can this be "to event payload" for clarity?

...java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java

* feat: add event stream serde dependencies([aws#824](aws#824)) * support eventstream traits([smithy-typescript#110](smithy-lang/smithy-typescript#110))

* feat: add event stream serde dependencies([#824](#824)) * support eventstream traits([smithy-typescript#110](smithy-lang/smithy-typescript#110))

* feat: add event stream serde dependencies([aws#824](aws#824)) * support eventstream traits([smithy-typescript#110](smithy-lang/smithy-typescript#110))

This change adds codegen support for eventstream serde. If a member has event stream trait, it will generate specitial event serde functions on top of original ones: * add eventstream serde generator * generate special serde for events and event unions * complete event deserializer and event union deserializer * Add event stream and events unions serializer * formatting code and add necessary docs * add eventstream-serde-browser dependency * address PR feedbacks * use normal error deserializer to deserialize error event in event stream

* Clear up bootstrap warnings * Update type generator to handle zero-length union types * Clear up compiler errors surrounding middlewarestack at later steps * Regenerate SDK packages

AllanZhengYP force-pushed the event-stream-utils-dev branch 3 times, most recently from dfc0930 to cffb837 Compare February 4, 2020 18:53

AllanZhengYP marked this pull request as ready for review February 4, 2020 23:01

kstich requested changes Feb 5, 2020

View reviewed changes

AllanZhengYP force-pushed the event-stream-utils-dev branch 3 times, most recently from 5002daa to 380dcb4 Compare February 5, 2020 08:27

AllanZhengYP requested a review from kstich February 5, 2020 17:07

kstich requested changes Feb 6, 2020

View reviewed changes

AllanZhengYP force-pushed the event-stream-utils-dev branch 3 times, most recently from 968e7c0 to bf83bd8 Compare February 6, 2020 23:15

AllanZhengYP requested a review from kstich February 6, 2020 23:16

AllanZhengYP changed the title ~~[WIP]support eventstream traits~~ support eventstream traits Feb 6, 2020

AllanZhengYP force-pushed the event-stream-utils-dev branch from bf83bd8 to b922d53 Compare February 11, 2020 21:39

AllanZhengYP added 10 commits February 12, 2020 23:23

add eventstream serde generator

a5eb480

generate special serde for events and event unions

5ae94a0

complete event deserializer and event union deserializer

60f6dc6

Add event stream and events unions serializer

d5d34c9

Add event-stream middleware if operation has eventstream input

4eb3e87

formatting code and add necessary docs

7495fab

add eventstream-serde-browser dependency

21125ca

address PR feedbacks

e58e281

re-organize the functions

0009f5e

format java code

e9da31a

AllanZhengYP force-pushed the event-stream-utils-dev branch from b922d53 to e9da31a Compare February 13, 2020 07:27

use normal error deserializer to deserialize error event in event stream

42751c8

kstich requested changes Feb 13, 2020

View reviewed changes

AllanZhengYP force-pushed the event-stream-utils-dev branch from bcfdcb6 to 3eed021 Compare February 14, 2020 00:01

fix readEventPayload()

3eed021

kstich approved these changes Feb 15, 2020

View reviewed changes

AllanZhengYP merged commit 6f44c3b into smithy-lang:master Feb 17, 2020

AllanZhengYP mentioned this pull request Feb 17, 2020

chore: codegen for event stream support aws/aws-sdk-js-v3#937

Merged

AllanZhengYP mentioned this pull request Jul 6, 2020

Eventstream should be supported in HTTP RPC protocols #191

Open

support eventstream traits #110

support eventstream traits #110

Uh oh!

Conversation

AllanZhengYP commented Jan 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AllanZhengYP commented Jan 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AllanZhengYP commented Feb 4, 2020

Uh oh!

kstich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kstich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AllanZhengYP commented Jan 29, 2020 •

edited

Loading

AllanZhengYP commented Jan 30, 2020 •

edited

Loading