feat: JSON format to assume JSON content-type where possible#604
feat: JSON format to assume JSON content-type where possible#604pierDipi merged 1 commit intocloudevents:mainfrom
Conversation
|
/cc @pierDipi |
|
|
||
| // Parse datacontenttype if any | ||
| String contentType = getOptionalStringNode(this.node, this.p, "datacontenttype"); | ||
| if (contentType == null && this.node.has("data")) { |
There was a problem hiding this comment.
I think this is aligned w/ the JSON-format.md file, section 3.1:
https://github.com/cloudevents/spec/blob/main/cloudevents/formats/json-format.md#31-handling-of-data
As for the non-binary / none base64 case it states:
If the
datacontenttypeis unspecified, processing SHOULD proceed as if the
datacontenttypehad been specified explicitly asapplication/json.
However the spec says should not must.
Perhaps that's part of the issue?
There was a problem hiding this comment.
Yes, it can introduce dubious questioning as the one we are having in the Knative community. @duglin can you chime in?
There was a problem hiding this comment.
I'm not sure I'm understanding the question. Is it: why is it SHOULD and not a MUST in that part of the spec? If so, I can't remember for sure, but I suspect it was done to allow for some out of band knowledge built into the event producer that allowed for it to "just know" what the real type is and take the appropriate serialization action. But w/o any extra knowledge things should assume/default to JSON. Make any sense?
pierDipi
left a comment
pierDipi
left a comment
There was a problem hiding this comment.
Should this be done behind a configurable flag as it is a "should" vs "must" in the spec? and perhaps as opt-out?
That may be a good idea. How would we call it? |
matejvasek
force-pushed
the
implied-datacontenttype
branch
from
8a58df9 to
a2bb831
Compare
formats/json-jackson/src/main/java/io/cloudevents/jackson/JsonFormatOptions.java
Outdated
Show resolved
Hide resolved
|
One small style nit |
Signed-off-by: Matej Vašek <mvasek@redhat.com>
matejvasek
force-pushed
the
implied-datacontenttype
branch
from
7c37753 to
df80e19
Compare
fixed @pierDipi |
pierDipi
changed the title
5 participants
For JSON structured CE we may assume JSON
datacontenttypeas long asdatais present int the CE.