gnodet-bot commented on code in PR #28896:
URL: https://github.com/apache/camel/pull/28896#discussion_r4444415146
##########
components/camel-camunda/src/main/java/org/apache/camel/component/camunda/processor/JobProcessor.java:
##########
@@ -39,24 +38,27 @@ public void process(Exchange exchange) throws Exception {
JobRequest message = null;
Long headerJobKey =
exchange.getMessage().getHeader(CamundaConstants.JOB_KEY, Long.class);
- if (exchange.getMessage().getBody() instanceof JobRequest) {
- message = exchange.getMessage().getBody(JobRequest.class);
- } else if (exchange.getMessage().getBody() instanceof String) {
- try {
- String bodyString =
exchange.getMessage().getBody(String.class);
- message = objectMapper.readValue(bodyString, JobRequest.class);
- } catch (JsonProcessingException jsonProcessingException) {
- throw new IllegalArgumentException("Cannot convert body to
JobRequest", jsonProcessingException);
- }
- } else if (exchange.getMessage().getBody() instanceof
JobWorkerMessage) {
- JobWorkerMessage jobWorkerMessage =
exchange.getMessage().getBody(JobWorkerMessage.class);
+ Object body = exchange.getMessage().getBody();
+ if (body instanceof JobRequest) {
+ message = (JobRequest) body;
+ } else if (body instanceof String) {
+ message = readJson(exchange, JobRequest.class, "Cannot convert
body to JobRequest");
+ } else if (body instanceof JobWorkerMessage) {
+ JobWorkerMessage jobWorkerMessage = (JobWorkerMessage) body;
message = new JobRequest();
message.setJobKey(jobWorkerMessage.getKey());
Review Comment:
⚠️ **Redundant body re-fetch in `body instanceof String` branch**
The `instanceof String` guard already confirmed `body` is a `String`, yet
`readJson` calls `exchange.getMessage().getBody(String.class)` — going through
Camel's type conversion system again. It is safe, but redundant. Consider
passing `body` directly:
```java
message = objectMapper.readValue((String) body, JobRequest.class);
```
Or extract a `readJson(String json, ...)` overload that takes the
already-fetched string, so callers that already have it don't pay for the extra
type-conversion.
##########
components/camel-camunda/src/main/java/org/apache/camel/component/camunda/processor/AbstractBaseProcessor.java:
##########
@@ -44,6 +46,34 @@ protected void setBody(Exchange exchange, CamundaMessage
message) {
}
}
+ /**
+ * Reads the body as JSON. The body is a String, or is converted to one,
such as a byte[] or an InputStream.
+ *
+ * @return the request, or null if the body cannot be converted to a String
+ */
+ protected <T> T readJson(Exchange exchange, Class<T> type, String
errorMessage) {
+ String json = exchange.getMessage().getBody(String.class);
+ if (json == null) {
+ return null;
Review Comment:
⚠️ **Missing `@throws` Javadoc for `readJson`**
The method throws `IllegalArgumentException` on JSON parse failure, but this
is not documented in the Javadoc. Add:
```
* @throws IllegalArgumentException if the body cannot be deserialized as
{@code type}
```
--
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]
