CAMEL-24346: camel-google-sheets - number every range and tolerate a range without values - #25400
Conversation
…range without values With splitResults the range counter was allocated inside the per-range loop, so every exchange carried CamelGoogleSheetsRangeIndex=1 no matter which of the configured ranges the value came from, and the increment at the end of the loop was dead. The counter now spans the whole response, like it already did when the results are not split. The values of a range were also dereferenced without a check. The sheets API omits that field for a range that holds nothing, so polling a spreadsheet with an empty range failed with a NullPointerException. The exchange building moved to createExchanges so it can be tested without a Google client, and the branch that reads a whole spreadsheet now marks the consumer as ready too, like the batch-get branch. 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).
|
gnodet
left a comment
There was a problem hiding this comment.
Clean, well-tested fix for two real bugs in GoogleSheetsStreamConsumer:
-
Range index counter bug:
AtomicInteger rangeIndexwas declared inside the for-loop, so it was re-created on every iteration — always reporting1. The fix correctly moves it outside the loop. -
Null values NPE: The Google Sheets API omits the
valuesfield for empty ranges, causinggetValues()to returnnull. The newvaluesOf()helper returnsList.of()as a safe default.
Bonus fix: forceConsumerAsReady() was missing in the non-range code path (the range-based path was covered by CAMEL-20189 but this branch was missed).
Test coverage is thorough, uses AssertJ, and the extracted createExchanges() method enables clean unit testing without a real Google API client. LGTM.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
Two defects in
GoogleSheetsStreamConsumer, both covered by a new test.1.
splitResultsreported range index 1 for every value. The counter was allocated inside theloop over the ranges:
So a consumer configured with
range=A1:B2,C1:D2&splitResults=truesent every exchange withCamelGoogleSheetsRangeIndex=1, and there was no way to tell which range a value came from. Thecounter now spans the whole response, which is what the non-split branch already did.
2. A range without values threw.
valueRange.getValues()was dereferenced directly in bothbranches, but the Sheets API omits the
valuesfield for a range that holds nothing, so polling aspreadsheet with an empty range failed with a
NullPointerExceptionon every cycle.Two smaller things while in there: the exchange building moved into
createExchanges(List<ValueRange>)so it can be tested without a Google client (the API client needs a real transport), and the branch
that reads a whole spreadsheet — used when no
rangeis configured — now callsforceConsumerAsReady()as well, so the consumer health check reports ready on that path too.
Claude Code on behalf of oscerd
🤖 Generated with Claude Code