CAMEL-24348: camel-google-bigquery - report unusable bodies and missing jobs properly - #25448
Conversation
…ng jobs properly A message with no body failed with a NullPointerException while building the "Cannot handle body type" message, instead of reporting that there was no body. In the SQL producer a CamelGoogleBigQueryJobId that no longer resolves gave a NullPointerException as well: both getJob and waitFor return null for a job that does not exist any more, and neither was checked. The three raw java.lang.Exception throws in the two producers are now RuntimeCamelException, so routes see a Camel exception rather than a bare one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 9 tested, 29 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
davsclaus
left a comment
There was a problem hiding this comment.
Looks good — three well-scoped fixes for real NPE/exception-hygiene issues.
What's good:
- Null-body guard in
GoogleBigQueryProducercorrectly reports instead of crashing, and the loop +apiRequestRows.isEmpty()handle it gracefully - Null checks on
getJob()/waitFor()inGoogleBigQuerySQLProducercover the realistic failure paths with descriptive messages RuntimeCamelExceptionis the right Camel exception type — propagates cleanly through the routing engine- The insert-ID comment in the
Listbranch is a useful "why" note that prevents someone from re-introducing the dedup bug - New test class follows conventions: AssertJ, package-private, covers both null and non-map/list body cases
Minor observations (non-blocking):
- No unit tests for the SQL producer null-job handling — understandable given the mocking complexity, worth noting as a future improvement
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Three NPE/exception-hygiene fixes.
1. A message with no body failed with a
NullPointerException. The producer built its error outof the body's class:
so the branch meant to report an unusable body threw while reporting it. It now says
Cannot handle body type null.2. An unknown
CamelGoogleBigQueryJobIdgave aNullPointerException. In the SQL producer bothbigquery.getJob(jobId)andjob.waitFor()returnnullfor a job that no longer exists (expired,or never created), and neither was checked before use. Both now fail with a message naming the job.
3. Three raw
throw new Exception(...)inGoogleBigQueryProducerandGoogleBigQuerySQLProducerare now
RuntimeCamelException, so routes get a Camel exception.onException(Exception.class)still matches, since
RuntimeCamelExceptionis one.Withdrawn from the issue description: the audit also claimed the
CamelGoogleBigQueryInsertIdheader is wrongly ignored for
Listbodies. Applying it there would be a bug: BigQuery uses theinsert id for best-effort deduplication, so giving every row of a list the same id makes it keep one
row and drop the rest. The existing behaviour is right —
useAsInsertId(a field name, so one id perrow) is the option for list payloads. This PR only adds a comment recording that, and I commented on
the JIRA issue.
Claude Code on behalf of oscerd
🤖 Generated with Claude Code