You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/03/01 16:22:02 UTC
[GitHub] [camel] essobedo opened a new pull request #7073: CAMEL-17720: camel-jackson - Add option to allow jackson to support null body
essobedo opened a new pull request #7073:
URL: https://github.com/apache/camel/pull/7073
Fix for https://issues.apache.org/jira/browse/CAMEL-17720
## Motivation
It is common to unmarshal data to json via Jackson, and the DataFormat we have will then fail if the message body is empty. We should add an option that can be turned on to allow null body.
## Modifications
* Add the new method `allowNullBody` to the interface `DataFormat.java`
* Skip the call to the method `unmarshal` if `allowNullBody()` and the body is `null`
* Add `allowNullBody` to the `json` builders
* Add `allowNullBody` to the yaml DSL
* Add `allowNullBody` to `JsonDataFormat`
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] essobedo commented on a change in pull request #7073: CAMEL-17720: camel-jackson - Add option to allow jackson to support null body
Posted by GitBox <gi...@apache.org>.
essobedo commented on a change in pull request #7073:
URL: https://github.com/apache/camel/pull/7073#discussion_r817008481
##########
File path: core/camel-api/src/main/java/org/apache/camel/spi/DataFormat.java
##########
@@ -54,4 +54,13 @@
* @throws Exception can be thrown
*/
Object unmarshal(Exchange exchange, InputStream stream) throws Exception;
+
+ /**
+ * Indicates whether {@code null} is allowed as value of a body to unmarshall.
+ *
+ * @return {@code true} if {@code null} is allowed, {@code false} otherwise.
+ */
+ default boolean allowNullBody() {
Review comment:
That's a good remark, actually, the failure doesn't happen in the Jackson `DataFormat` itself but in the `UnmarshalProcessor`. Indeed, The failure is due to the fact that it cannot find a converter to convert a `null` value into an `InputStream` which occurs when calling `in.getMandatoryBody(InputStream.class)`. That is why I had to manage it at a higher level, moreover @davsclaus proposed in the same ticket to eventually have it for all the existing DataFormat which could give more sense to this approach. Do you see a better/simpler approach? If I need to apply the same logic to the other DataFormats, I had better to have the best approach.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a change in pull request #7073: CAMEL-17720: camel-jackson - Add option to allow jackson to support null body
Posted by GitBox <gi...@apache.org>.
davsclaus commented on a change in pull request #7073:
URL: https://github.com/apache/camel/pull/7073#discussion_r817102082
##########
File path: core/camel-support/src/main/java/org/apache/camel/support/processor/UnmarshalProcessor.java
##########
@@ -55,13 +55,16 @@ public boolean process(Exchange exchange, AsyncCallback callback) {
InputStream stream = null;
Object result = null;
try {
- stream = exchange.getIn().getMandatoryBody(InputStream.class);
+ final Message in = exchange.getIn();
+ final Message out = exchange.getOut();
Review comment:
Do not call getOut() to early as it will create an empty message, and cause the exchange to have this empty message as the response, even if the message body is null and allow null enabled. So move this out to later where its first in use
##########
File path: core/camel-api/src/main/java/org/apache/camel/spi/DataFormat.java
##########
@@ -54,4 +54,13 @@
* @throws Exception can be thrown
*/
Object unmarshal(Exchange exchange, InputStream stream) throws Exception;
+
+ /**
+ * Indicates whether {@code null} is allowed as value of a body to unmarshall.
+ *
+ * @return {@code true} if {@code null} is allowed, {@code false} otherwise.
+ */
+ default boolean allowNullBody() {
Review comment:
Yeah I can't think of a better way as there is no common DataFormatConfiguration class that they all extend etc where we could have common options.
But this is really a useful feature to let the data format not fail on null bodies.
Especially for use-cases with json where we would often use jackson.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] essobedo commented on pull request #7073: CAMEL-17720: Add option to allow unmarshalling null body
Posted by GitBox <gi...@apache.org>.
essobedo commented on pull request #7073:
URL: https://github.com/apache/camel/pull/7073#issuecomment-1056780269
@davsclaus @oscerd What do you think of this new approach? As it is not related to DataFormat but rather to the way to Unmarshal, I propose to add an option directly in the Unmarshal EIP, WDYT? Is that acceptable?
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] essobedo edited a comment on pull request #7073: CAMEL-17720: Add option to allow unmarshalling null body
Posted by GitBox <gi...@apache.org>.
essobedo edited a comment on pull request #7073:
URL: https://github.com/apache/camel/pull/7073#issuecomment-1056780269
@davsclaus @oscerd What do you think of this new approach? As it is not related to DataFormat but rather to the way to unmarshal the body of a message, I propose to add an option directly in the Unmarshal EIP, WDYT? Is that acceptable?
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] essobedo commented on pull request #7073: CAMEL-17720: Add option to allow unmarshalling null body
Posted by GitBox <gi...@apache.org>.
essobedo commented on pull request #7073:
URL: https://github.com/apache/camel/pull/7073#issuecomment-1056802702
@davsclaus Good remark, done
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] essobedo commented on pull request #7073: CAMEL-17720: camel-jackson - Add option to allow jackson to support null body
Posted by GitBox <gi...@apache.org>.
essobedo commented on pull request #7073:
URL: https://github.com/apache/camel/pull/7073#issuecomment-1055618720
@davsclaus Is-it more or less what you have in mind?
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] oscerd commented on pull request #7073: CAMEL-17720: Add option to allow unmarshalling null body
Posted by GitBox <gi...@apache.org>.
oscerd commented on pull request #7073:
URL: https://github.com/apache/camel/pull/7073#issuecomment-1056933901
Sorry, late to the party. Thanks for this @essobedo, keep up the good work!
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] oscerd commented on a change in pull request #7073: CAMEL-17720: camel-jackson - Add option to allow jackson to support null body
Posted by GitBox <gi...@apache.org>.
oscerd commented on a change in pull request #7073:
URL: https://github.com/apache/camel/pull/7073#discussion_r816934109
##########
File path: core/camel-api/src/main/java/org/apache/camel/spi/DataFormat.java
##########
@@ -54,4 +54,13 @@
* @throws Exception can be thrown
*/
Object unmarshal(Exchange exchange, InputStream stream) throws Exception;
+
+ /**
+ * Indicates whether {@code null} is allowed as value of a body to unmarshall.
+ *
+ * @return {@code true} if {@code null} is allowed, {@code false} otherwise.
+ */
+ default boolean allowNullBody() {
Review comment:
Is this correct? It should be specific to Jackson and not generic to data format.
I'm from mobile so I might losing something
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] oscerd commented on a change in pull request #7073: CAMEL-17720: camel-jackson - Add option to allow jackson to support null body
Posted by GitBox <gi...@apache.org>.
oscerd commented on a change in pull request #7073:
URL: https://github.com/apache/camel/pull/7073#discussion_r817009890
##########
File path: core/camel-api/src/main/java/org/apache/camel/spi/DataFormat.java
##########
@@ -54,4 +54,13 @@
* @throws Exception can be thrown
*/
Object unmarshal(Exchange exchange, InputStream stream) throws Exception;
+
+ /**
+ * Indicates whether {@code null} is allowed as value of a body to unmarshall.
+ *
+ * @return {@code true} if {@code null} is allowed, {@code false} otherwise.
+ */
+ default boolean allowNullBody() {
Review comment:
Well, I didn't read the suggestion from @davsclaus, in that case it makes completely sense as a basic step to then provide the feature on all the dataformat. So fine with me. Thanks for the explanation.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on pull request #7073: CAMEL-17720: Add option to allow unmarshalling null body
Posted by GitBox <gi...@apache.org>.
davsclaus commented on pull request #7073:
URL: https://github.com/apache/camel/pull/7073#issuecomment-1056789821
Yeah this is better.
Would be good to have an test that the yaml-dsl works too
https://github.com/apache/camel/blob/main/dsl/camel-yaml-dsl/camel-yaml-dsl/src/test/groovy/org/apache/camel/dsl/yaml/UnmarshalTest.groovy
You could maybe add a new test method (and keep existing as-is)
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] essobedo edited a comment on pull request #7073: CAMEL-17720: Add option to allow unmarshalling null body
Posted by GitBox <gi...@apache.org>.
essobedo edited a comment on pull request #7073:
URL: https://github.com/apache/camel/pull/7073#issuecomment-1056802702
@davsclaus Good remark, done [here](https://github.com/apache/camel/pull/7073/files#diff-65535169d653ed6957d2488a787bd695c1ff1d10005a0a12b28392f88fe84359R79-R121)
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org