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.

Sign up for GitHub

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

Jump to bottom

JSON format implicit datacontenttype is asymmetrical #880

Closed
jskeet opened this issue Sep 30, 2021 · 5 comments
Closed

JSON format implicit datacontenttype is asymmetrical #880

jskeet opened this issue Sep 30, 2021 · 5 comments
Assignees
@jskeet

Comments

@jskeet
Copy link
Contributor

jskeet commented Sep 30, 2021 •
edited
Loading

Just trying to write tests for the clarifications in #861, and I think there's an asymmetry:

  • When deserializing a CloudEvent, if datacontenttype isn't specified it defaults to application/json
  • When serializing a CloudEvent, there's no such defaulting that I can see

I suspect that we should say that if datacontenttype is missing when serializing - and if data is present - it should default to application/json, so the payload should be serialized as JSON.

I'll proceed along those lines, and we can discuss whether that was actually the intention - and I can put together a PR to clarify the spec if so.
@jskeet
Copy link
Contributor Author

jskeet commented Sep 30, 2021

(I may well have just missed it, of course...)

cc @dazuma

@jskeet
Copy link
Contributor Author

jskeet commented Sep 30, 2021

(Just in case I forget - Clemens pointed out that the core spec does say that an event using JSON format can be implicitly application/json. So I'll just make that absolutely concrete in 3.1.1)

jskeet added a commit to jskeet/spec that referenced this issue Oct 7, 2021
@jskeet
Explicitly state application/json defaulting when serializing
69b3730 
(I've used SHOULD rather than MUST to be consistent with deserialization. If we were starting from scratch, I'd suggest that MUST would be more appropriate, but it's probably too late to do that.)

Fixes cloudevents#880
@jskeet jskeet mentioned this issue Oct 7, 2021
Merged
jskeet added a commit to jskeet/spec that referenced this issue Oct 7, 2021
@jskeet
Explicitly state application/json defaulting when serializing
39c12dc 
(I've used SHOULD rather than MUST to be consistent with deserialization. If we were starting from scratch, I'd suggest that MUST would be more appropriate, but it's probably too late to do that.)

Fixes cloudevents#880

Signed-off-by: Jon Skeet <jonskeet@google.com>
@manuelstein
Copy link
Contributor

regarding that the addition refers to the user-provided datacontenttype plainly as "the datacontenttype", I noticed it's introduced in above (see 3.1) as "the datacontenttype attribute", so referring to it as "the datacontenttype" would be ok for me, but if you want to be clear, could use "the datacontenttype attribute". Would that clear things up? @duglin @jskeet

@duglin
Copy link
Collaborator

duglin commented Nov 4, 2021

@jskeet can we close this now?

@jskeet
Copy link
Contributor Author

jskeet commented Nov 4, 2021

Yup!

@duglin duglin closed this as completed Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jskeet jskeet

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jskeet @duglin @manuelstein