Properly handle event payload members of blob and string type#1064
Properly handle event payload members of blob and string type#1064sugmanue wants to merge 1 commit intosmithy-lang:mainfrom
Conversation
| var typeHolder = new AtomicReference<String>(); | ||
| var headers = new HashMap<String, HeaderValue>(); | ||
| var payload = encodeInput(item, typeHolder, headers); | ||
| headers.put(":message-type", HeaderValue.fromString("event")); | ||
| headers.put(":event-type", HeaderValue.fromString(typeHolder.get())); | ||
| headers.put(":content-type", HeaderValue.fromString(payloadMediaType)); | ||
| return new AwsEventFrame(new Message(headers, payload)); | ||
| var contentTypeHolder = new AtomicReference<>(payloadMediaType); |
There was a problem hiding this comment.
why are we using AtomicReference here? this should be synchronous – if this is to emulate out parameters, use a one-element array or something similar to give us a reference we can mutate from within the encodeInput routine. alternatively, have that method return a record class that contains the payload, type, and content-type.
| public void writeStruct(Schema schema, SerializableStruct struct) { | ||
| var memberName = schema.memberName(); | ||
| if (possibleTypes.contains(memberName) && | ||
| typeHolder.compareAndSet(null, memberName)) { |
There was a problem hiding this comment.
is there anything we can do to stop iterating over the struct fields once we set typeHolder? maybe throw an exception to short-circuit the serializer?
| for (var memberSchema : eventSchema.members()) { | ||
| if (memberSchema.hasTrait(TraitKey.ERROR_TRAIT)) { | ||
| if (result.put(memberSchema.memberTarget().id(), memberSchema) != null) { | ||
| throw new IllegalStateException("Duplicate key"); |
There was a problem hiding this comment.
can you include the duplicate in the exception message?
| * Returns true if the schema have any members that ought to be serialized in | ||
| * the event payload. | ||
| */ | ||
| static boolean hasPayloadMembers(Schema struct) { |
There was a problem hiding this comment.
this isn't something to fix in this PR, but this feels like the kind of thing we'd want to attach as a trait on the struct
| private final Codec codec; | ||
| private final EventHeaderSerializer headerSerializer; | ||
| private final ShapeSerializer baseSerializer; | ||
| private final AtomicReference<String> contentTypeHolder; |
There was a problem hiding this comment.
like above, AtomicReference is the wrong thing to use if we don't need atomicity. it's not appropriate to use as a generic mutable reference, unless you commit to only using getPlain and setPlain to interact with it (but even that would be an abuse of its APIs)
| @@ -0,0 +1,18 @@ | |||
| ## Example: Transcribe Streaming Client | |||
| static class EventPayloadSerializer extends InterceptingSerializer { | ||
| private final OutputStream out; | ||
| private final Codec codec; | ||
| private final AtomicReference<String> contentEncodingHolder; |
There was a problem hiding this comment.
this should be contentTypeHolder, not contentEncodingHolder
| public void writeBlob(Schema schema, ByteBuffer value) { | ||
| contentEncodingHolder.set("application/octet-stream"); | ||
| try { | ||
| out.write(ByteBufferUtils.getBytes(value)); |
There was a problem hiding this comment.
ByteBufferUtils.getBytes will trigger a copy if the payload does not exactly fill the underlying array, but we can avoid that with out.write(value.array(), value.arrayOffset() + value.position(), value.remaining())
2 participants
Issue #, if available:
Description of changes:
Properly handle event payload members of blob and string type, those are serialized directly into the event payload using a different content-type header.
Serializes a member with the trait
@eventPayloaddirectly into the payload of the event. The following member types have special handling::content-typeheader is set to "application/octet-stream":content-typeheader is set to "text/plain":content-typeis added as header of the messageBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.