Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Special handling of the "data" attribute not supported #33

Closed
joshwlewis opened this issue Aug 22, 2019 · 3 comments
Closed

Special handling of the "data" attribute not supported #33

joshwlewis opened this issue Aug 22, 2019 · 3 comments
Labels
good first issue Good for newcomers

Comments

@joshwlewis
Copy link

joshwlewis commented Aug 22, 2019

In the v0.3 spec for JSON format section about Special Handling of the "data" Attribute, it says:

First, if an implementation determines that the type of the data attribute is Binary or String, it MUST inspect the datacontenttype attribute to determine whether it is indicated that the data value contains JSON data.
If the datacontenttype value is either "application/json" or any media type with a structured +json suffix, the implementation MUST translate the data attribute value into a JSON value, and set the data attribute of the envelope JSON object to this JSON value.

If I'm reading the spec correctly, it looks like the SDK should serialize stringified JSON in the data attribute. In other words, I would expect this to work:

const v03 = require("cloudevents-sdk/v03");
const data = { "so": "scare" };
const ce = v03.event
  .id('4e4cfbd6-d542-4235-9e80-99d2df62abee')
  .source('com.example.whatever')
  .type('something')
  .dataContentType("application/json")
  .data(JSON.stringify(data)) // <-- stringified json data goes in
  .format();

const ceData = ce.getData();
console.log(ceData.so); // <-- this breaks, getData returned the stringified JSON, not an object.

I wrote a sample test (joshwlewis@19cc9f3), if it's more useful than my untested example. It fails with:

  CloudEvents Spec v0.3
    The Constraints check
      'data'
        1) should convert data with stringified json to a json object


  0 passing (8ms)
  1 failing

  1) CloudEvents Spec v0.3
       The Constraints check
         'data'
           should convert data with stringified json to a json object:
     AssertionError: expected '{"much":"wow"}' to equal { much: 'wow' }
      at Context.it (test/spec_0_3_tests.js:209:41)

@fabiojose fabiojose added the good first issue Good for newcomers label Aug 23, 2019
@fabiojose
Copy link
Contributor

fabiojose commented Aug 23, 2019

That reference it's wrong, please, ignore it.

@fabiojose
Copy link
Contributor

fabiojose commented Aug 23, 2019

@joshwlewis, after reading and talking with some fellows, we conclude that's right for incoming events, that needs to be unmarshaled. But, for the outcoming event, created by the v03.event()... the type will be as-is, no parsing at all.

If we parse a string to json in the v03.event().., we will ending to marshall a json object, not a string.

Is that makes sense to you?

ps: even at this moment, the unmarshaller does not parse the string into json object for the incoming events.

@fabiojose
Copy link
Contributor

@joshwlewis, I've some baking on this. Your point of view is right! Because of the dynamic typing of js, we can do that easily.

Lets review it in the PR #35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants