-
Notifications
You must be signed in to change notification settings - Fork 584
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
remove dataencoding and intro data_base64 in JSON #492
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,9 +92,12 @@ respective CloudEvents type when the mapping rules are fulfilled. | |
### 2.3. Mapping Data | ||
|
||
If an implementation determines that the actual type of `data` is a `String`, | ||
the value MUST be represented as [JSON string][json-string] expression; for | ||
`Binary`, the value MUST represented as [JSON string][json-string] expression | ||
containing the [Base64][base64] encoded binary value. | ||
the value MUST be represented as [JSON string][json-string] expression and the | ||
member name inside the JSON object MUST be `data`. For `Binary`, the value MUST | ||
be represented as a [JSON string][json-string] expression containing the | ||
[Base64][base64] encoded binary value, and the member name inside the JSON | ||
object MUST be `data_base64`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While it's implied, it doesn't actually ban someone from having both
Someone may try to be "smart" and fill in both (when possible, like when it's a string) and that's a situation I think we need to avoid. Also, could someone misinterpret the For cases where |
||
|
||
|
||
### 2.4. Examples | ||
|
||
|
@@ -155,12 +158,6 @@ the [type-system mapping](#22-type-system-mapping), the resulting `data` member | |
[JSON value][json-value] is unrestricted, and MAY also contain numeric and | ||
logical JSON types. | ||
|
||
Second, whether a Base64-encoded string in `data` is treated as | ||
`Binary` or as a `String` is also determined by the `datacontenttype` value. If | ||
the `datacontenttype` media type is known to contain text, the data attribute | ||
value is not further interpreted and treated as a text string. Otherwise, it is | ||
decoded and treated as a binary value. | ||
|
||
### 3.2. Examples | ||
|
||
Example event with `String`-valued `data`: | ||
|
@@ -195,7 +192,7 @@ Example event with `Binary`-valued data | |
"otherValue": 5 | ||
}, | ||
"datacontenttype" : "application/vnd.apache.thrift.binary", | ||
"data" : "... base64 encoded string ..." | ||
"data_base64" : "... base64 encoded string ..." | ||
} | ||
``` | ||
|
||
|
@@ -264,7 +261,7 @@ second with JSON data. | |
"otherValue": 5 | ||
}, | ||
"datacontenttype" : "application/vnd.apache.thrift.binary", | ||
"data" : "... base64 encoded string ..." | ||
"data_base64" : "... base64 encoded string ..." | ||
}, | ||
{ | ||
"specversion" : "0.4-wip", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Terrible nit, but I think this should be in section 3 now, because we've removed
data
from the attributes, and section 2 is "Attributes".There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would also put this text closer to the text in section 3.1 'Special Handling of "data"'.