Adding DataContentType to the Cloudevent builder. #10
Conversation
Adding DataContentType to the cloudevent builder.
rjalander
requested review from
afrittoli,
e-backmark-ericsson and
m-linner-ericsson
| .withId(UUID.randomUUID().toString()) | ||
| .withSource(URI.create("cdevents-sdk-java")) | ||
| .withType(eventType) | ||
| .withDataContentType("application/json; charset=UTF-8") |
There was a problem hiding this comment.
Would it make sense to add a test that asserts all CDEvents created with the builder have a content type set?
There was a problem hiding this comment.
"Soon" there will be json schemas published for the CDEvents protocol, se cdevents/spec#61. When that is done, and when the Java SDK is lifted to the latest CDEvents spec version, we should have tests in all SDKs that verify that the produced events validate towards the json schemas for the protocol. If/when we have that, is there still a reason to assert the content type?
There was a problem hiding this comment.
Excuse my ignorance, I wasn't aware of that pull request. To answer your question: probably not, if the the json schemas enforce the content type to be specified.
There was a problem hiding this comment.
Oups, now you must excuse me :) I was too quick in my reply previously. The data content type will not be part of the protocol itself, and therefore not part of the protocol schema. There will be "supported bindings", of which json is the main one, but that is on higher level than the schema itself.
So, it might be wise to actually assert that the content type is set within the Java SDK already now.
|
See also the work on setting content type for the new customData field in the CDEvents spec here: cdevents/spec#63 (comment) |
|
@rjalander - given the recent refactor, is his PR still needed? |
|
We don't need this change now, as we are giving an option to set only the CustomDataContent type and CloudEvent DataType id defaulted to |
Adding DataContentType to the cloudevent builder.