davsclaus commented on code in PR #27587:
URL: https://github.com/apache/camel/pull/27587#discussion_r4227329277
##########
components/camel-cloudevents/src/main/java/org/apache/camel/component/cloudevents/transformer/CloudEventJsonDataTypeTransformer.java:
##########
@@ -76,28 +89,40 @@ public void transform(Message message, DataType fromType,
DataType toType) {
cloudEventAttributes.putIfAbsent(cloudEvent.mandatoryAttribute(CloudEvent.CAMEL_CLOUD_EVENT_TIME).json(),
cloudEvent.getEventTime(message.getExchange()));
- String body = MessageHelper.extractBodyAsString(message);
- cloudEventAttributes.putIfAbsent("data", body);
- cloudEventAttributes.putIfAbsent(
-
cloudEvent.mandatoryAttribute(CloudEvent.CAMEL_CLOUD_EVENT_DATA_CONTENT_TYPE).json(),
dataContentType);
+ String dataContentTypeKey =
cloudEvent.mandatoryAttribute(CloudEvent.CAMEL_CLOUD_EVENT_DATA_CONTENT_TYPE).json();
+ cloudEventAttributes.putIfAbsent(dataContentTypeKey,
dataContentType);
+ // the data is written as the datacontenttype of the event says
+ Object eventDataContentType =
cloudEventAttributes.get(dataContentTypeKey);
+ boolean jsonData = isJsonContentType(eventDataContentType);
+ if (message.getBody() instanceof byte[] bytes) {
+ String text = decodeText(bytes,
ExchangeHelper.getCharset(message.getExchange()));
+ if (text != null) {
+ cloudEventAttributes.putIfAbsent("data", text);
+ } else {
+ // decoding these bytes as text would replace the invalid
ones, so the data would be lost
+ cloudEventAttributes.putIfAbsent("data_base64",
Base64.getEncoder().encodeToString(bytes));
+ }
+ } else {
+ cloudEventAttributes.putIfAbsent("data",
MessageHelper.extractBodyAsString(message));
Review Comment:
Not introduced here, so fine as a follow-up: a null body still ends up as
`"data":"null"`, because the `null` returned by `extractBodyAsString` goes
through `String.valueOf`. With the new scalar handling the text `null` now
becomes a JSON `null`, while an actual null body becomes the string `"null"`,
which is the reverse of what a reader would expect. Leaving `data` out for a
null body (it is optional in the spec) would be the cleanest.
##########
components/camel-cloudevents/src/main/java/org/apache/camel/component/cloudevents/transformer/CloudEventJsonDataTypeTransformer.java:
##########
@@ -76,28 +89,40 @@ public void transform(Message message, DataType fromType,
DataType toType) {
cloudEventAttributes.putIfAbsent(cloudEvent.mandatoryAttribute(CloudEvent.CAMEL_CLOUD_EVENT_TIME).json(),
cloudEvent.getEventTime(message.getExchange()));
- String body = MessageHelper.extractBodyAsString(message);
- cloudEventAttributes.putIfAbsent("data", body);
- cloudEventAttributes.putIfAbsent(
-
cloudEvent.mandatoryAttribute(CloudEvent.CAMEL_CLOUD_EVENT_DATA_CONTENT_TYPE).json(),
dataContentType);
+ String dataContentTypeKey =
cloudEvent.mandatoryAttribute(CloudEvent.CAMEL_CLOUD_EVENT_DATA_CONTENT_TYPE).json();
+ cloudEventAttributes.putIfAbsent(dataContentTypeKey,
dataContentType);
+ // the data is written as the datacontenttype of the event says
+ Object eventDataContentType =
cloudEventAttributes.get(dataContentTypeKey);
+ boolean jsonData = isJsonContentType(eventDataContentType);
+ if (message.getBody() instanceof byte[] bytes) {
+ String text = decodeText(bytes,
ExchangeHelper.getCharset(message.getExchange()));
+ if (text != null) {
+ cloudEventAttributes.putIfAbsent("data", text);
+ } else {
+ // decoding these bytes as text would replace the invalid
ones, so the data would be lost
+ cloudEventAttributes.putIfAbsent("data_base64",
Base64.getEncoder().encodeToString(bytes));
Review Comment:
When neither `Content-Type` nor `CamelCloudEventDataContentType` is set,
these bytes are written as `data_base64` together with the defaulted
`datacontenttype: application/json`. Section 3.1.1 of the CloudEvents JSON
format says that, if present, `datacontenttype` MUST reflect the format of the
original binary data. Bytes that are not valid text in the exchange charset
(UTF-8 by default) are not JSON, so a reader that maps the data by its content
type will fail on them. Could you write `application/octet-stream`
(`CloudEvent.APPLICATION_OCTET_STREAM_MIME_TYPE`) in this case, but only when
the content type was defaulted, so that an explicitly declared type stays as it
is? And could you assert the `datacontenttype` for the `null` content-type case
in `shouldWriteBytesThatAreNotValidTextAsBase64WhateverTheContentType`?
##########
components/camel-cloudevents/src/main/java/org/apache/camel/component/cloudevents/transformer/CloudEventJsonDataTypeTransformer.java:
##########
@@ -76,28 +89,40 @@ public void transform(Message message, DataType fromType,
DataType toType) {
cloudEventAttributes.putIfAbsent(cloudEvent.mandatoryAttribute(CloudEvent.CAMEL_CLOUD_EVENT_TIME).json(),
cloudEvent.getEventTime(message.getExchange()));
- String body = MessageHelper.extractBodyAsString(message);
- cloudEventAttributes.putIfAbsent("data", body);
- cloudEventAttributes.putIfAbsent(
-
cloudEvent.mandatoryAttribute(CloudEvent.CAMEL_CLOUD_EVENT_DATA_CONTENT_TYPE).json(),
dataContentType);
+ String dataContentTypeKey =
cloudEvent.mandatoryAttribute(CloudEvent.CAMEL_CLOUD_EVENT_DATA_CONTENT_TYPE).json();
+ cloudEventAttributes.putIfAbsent(dataContentTypeKey,
dataContentType);
+ // the data is written as the datacontenttype of the event says
+ Object eventDataContentType =
cloudEventAttributes.get(dataContentTypeKey);
+ boolean jsonData = isJsonContentType(eventDataContentType);
Review Comment:
Question: without a declared content type the transformer only assumes
`application/json`, and with this change any text that is a JSON scalar is
nested too. For example, an order id `1234567890123456789` becomes a JSON
number, which JavaScript readers can't represent exactly, and the text `null`
becomes a null payload. Would it be safer to nest scalars only when a JSON
content type is actually declared (`Content-Type` or
`CamelCloudEventDataContentType`), and keep the default to objects/arrays as
CAMEL-22339 does? Not decided yet, happy to hear your view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
