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

Revisiting JSON format's data member #934

Closed
jskeet opened this issue Jan 17, 2022 · 6 comments
Closed

Revisiting JSON format's data member #934

jskeet opened this issue Jan 17, 2022 · 6 comments

Comments

@jskeet
Copy link
Contributor

jskeet commented Jan 17, 2022

I'm back to implementing #861, and I've run into this:

If the datacontenttype declares the data to contain JSON-formatted content, a
JSON serializer MUST translate the data value to a [JSON value][json-value], and
use the member name data to store it inside the JSON representation. The data
value MUST be stored directly as a JSON value, rather than as an encoded JSON
document represented as a string.

I have a memory that the value of data must be a JSON string or object, not an array, number or Boolean value. However, I don't see any indication of that within the spec. The JSON value reference includes numbers, Boolean values, arrays and indeed null.

So if an SDK is provided CloudEvent data of "the number 1.5" and a content type of application/json (explicitly or implicitly), is it reasonable for the structured event representation to be:

{
  "id": "...",
  "type": "...",
  "datacontenttype": "application/json",
  "data": 1.5
}

? If that's valid, then my life will be easier but my memory is incorrect. If it's invalid, then is that already covered somewhere in the spec and I just haven't seen it, or do we need to change the spec?

@duglin
Copy link
Collaborator

duglin commented Jan 17, 2022

From the spec, I see this example:

{
    "specversion" : "1.0",
    "type" : "com.example.someevent",
    "source" : "/mycontext",
    "subject": null,
    "id" : "D234-1234-1234",
    "time" : "2018-04-05T17:31:00Z",
    "comexampleextension1" : "value",
    "comexampleothervalue" : 5,
    "data" : "I'm just a string"
}

And from that I would conclude that if data was a number then it would look like: "data": 1.5 since the switch from string to number is "I'm just a string" to 1.5. No extra json magic/wrapping should be needed. Also, note that comexampleothervalue does the same thing.

@jskeet
Copy link
Contributor Author

jskeet commented Jan 17, 2022

comexampleothervalue is just an extension attribute - that's fine. If we could talk about this on Thursday, someone else at remember the same bit as me about "only a string or object for data". If I'm just misremembering, that would make my life easier :)

@duglin
Copy link
Collaborator

duglin commented Jan 17, 2022

Another way I look at it is this: any valid/stand-alone JSON should be able to be a child of data. If we require some kind of wrapper, how can we tell if the wrapper is there just for CE or there because it's part of the original JSON. That would then mean that: "data": 1.5 must be valid because there would be ambiguity if the 1.5 had to be wrapped/modified.

@JemDay
Copy link
Contributor

JemDay commented Feb 17, 2022

I still think the spec is pretty clear ...

When we're talking about data then any JSONValue is valid .. so all of these are good:
"data": 6
"data": 10.7
"data": []
etc

When we're talking about CE attributes then only a subset of types are available..

Appropriate
"intextension": 55
"txtextension": "I'm good"

Not Appropriate
"decimalextension": 3.142

The above is NOT valid according to the JSON Format specification, or indeed the general CloudEvent specification. My argument is that this should either be ignored by SDKs, or coerced into an extension attribute with a type of string.

@jskeet
Copy link
Contributor Author

jskeet commented Feb 17, 2022

I still think the spec is pretty clear ...

I agree, although I still think an example makes it even clearer. (I seem to remember I have an action item to add another example in #950.)

[decimal extension value]

The above is NOT valid according to the JSON Format specification, or indeed the general CloudEvent specification. My argument is that this should either be ignored by SDKs, or coerced into an extension attribute with a type of string.

I'd suggest that if something isn't valid, it should cause an error rather than being ignored or coerced. (I think that's what the C# SDK does, but I now want to add a test for it...) Would welcome talking about that more.

As a corner case, what about

"intextension": 10.00

? I think I'd want to interpret that as a value of 10... but I could probably be persuaded that the ".00" suggests it's expected to be handled as a non-integer value, and should behave the same was as 3.142...

@duglin
Copy link
Collaborator

duglin commented Feb 24, 2022

We agreed to close this on the 2/17 meeting. @jskeet reopen if I got that wrong

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

No branches or pull requests

3 participants