[lance]

Proposed Changes

• Change CloudEvent so that it never modifies the data attribute
• If an event's datacontenttype property is a content type that we don't support, just set the data attribute - don't throw

Description

When setting an event's data attribute we were trying to be really clever and this is problematic. Instead, keep the data attribute unchanged. Per the 1.0 specification, the data attribute is still inspected to determine if it is binary data, and if so, a data_base64 attribute is added with the contents of the data property encoded as base64.

Fixes: #343

Signed-off-by: Lance Ball lball@redhat.com

[lance]

Do not merge. I have found a bug in this implementation that I need to fix. Per the spec, this test is incorrect.

The spec reads:

When a CloudEvents is deserialized from JSON, the presence of the data_base64 member clearly indicates that the value is a Base64 encoded binary data, which the serializer MUST decode into a binary runtime data type.

// Instead of this
      expect(eventDeserialized.data).to.equal({ foo: "bar" });
// It should be
      expect(eventDeserialized.data).to.equal(dataBinary);

[lance]

@cloudevents/sdk-javascript-maintainers there is an interesting discussion in Slack around this question of binary data: https://cloud-native.slack.com/archives/C9DB5ABAA/p1601416717013400

[lance]

@cloudevents/sdk-javascript-maintainers PTAL - thanks

[grant]

Not modifying the data is great.
I think we'll talk about base64 data a bit more in the weekly SDK call too.

[lance]

Based on the discussion in today's CNCF call, I am going to remove this test and the associated functionality.

sdk-javascript/test/integration/message_test.ts

Lines 190 to 195 in 5255a19

	it("Parses binary data from binary messages with content type application/json", () => {
	const message = HTTP.binary(fixture.cloneWith({ data: dataBinary }));
	const eventDeserialized = HTTP.toEvent(message);
	expect(eventDeserialized.data).to.deep.equal({ foo: "bar" });
	expect(eventDeserialized.data_base64).to.be.undefined;
	});

Edit: I did not remove the test. Instead, I changed it to assert that we do NOT parse binary data even if the datacontenttype is application/json.