CAMEL-24342: camel-google-storage - fix wrong blob metadata and missing object handling - #25338
Conversation
…ng object handling processFile assigned the content type to the Content-Encoding and Cache-Control fields of the blob, so an upload carrying those headers stored the content type in them, or null when no content type was sent. getObject dereferenced the blob returned by the storage client without checking it, so asking for an object that is not in the bucket failed with a NullPointerException instead of naming the missing object. The consumer had the same problem when polling a configured objectName that does not exist: the poll threw on every cycle, and that path never marked the consumer as ready. Also drops the unreachable else branch in createDownloadLink (the header lookup already supplies the default) and replaces the java.util wildcard import. 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.
Excellent bug-fix PR that addresses three real issues:
-
Copy-paste variable error in blob metadata handling —
contentEncodingandcacheControlwere being set to the content-type value instead of their respective header values. The root cause traces back to CAMEL-18300 (2022), wherectwas used instead ofce/ccfor two of the four metadata fields. CAMEL-22273 fixedcontentDispositionlast year; this PR completes the fix for the remaining two. -
NPE path when a configured
objectNamepoints to a missing object — now correctly throws a descriptive exception with the object name. -
createDownloadLinkdead code cleanup —getHeader(key, defaultValue, type)with a non-null default of300000Lcan never returnnull, so theelsebranch was unreachable. Simplifying to primitivelongis the right call.
Test coverage is good, with two dedicated test classes covering all three fixes.
Minor convention note: ProducerBlobMetadataTest uses public class/method visibility and JUnit assertions (assertEquals, assertNotNull), while the other new test file (GoogleCloudStorageConsumerMissingObjectTest) correctly uses package-private visibility and AssertJ. Per project conventions, new test code should prefer package-private visibility and AssertJ assertions. Not blocking — just a consistency note.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
…(package-private + AssertJ) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
Three defects in
camel-google-storage, all covered by new tests.1. Uploads stored the content type in two other blob fields.
processFileread theContent-EncodingandCache-Controlentries from the metadata map but then passed the contenttype variable to the builder:
So an upload sending those headers ended up with the content type in both fields — or
null, whenno
Content-Typeheader was sent at all.2.
getObjectNPE'd on a missing object.storage.get(BlobId)returnsnullwhen the objectis not there, and the result was dereferenced straight away. It now fails with a message naming the
object and the bucket.
3. The consumer NPE'd on every poll when the configured
objectNamewas missing. Same nullblob, this time handed to
createExchange. The poll now logs and yields no exchange. That path alsonever called
forceConsumerAsReady(), so the consumer health check stayed not-ready whenobjectNamewas configured; it is now marked ready as soon as the storage client answers, like onthe list path.
Two cleanups in the touched file: the
elsebranch increateDownloadLinkwas unreachable (theheader lookup already supplies the
300000Ldefault), and thejava.util.*wildcard import isreplaced with explicit ones.
Not changed: the audit that produced this issue also flagged that the
objectNameendpointoption wins over the
CamelGoogleCloudStorageObjectNameheader. That is deliberate — CAMEL-20998made it so, because a consumer in the same route sets that header and would otherwise hijack the
producer destination. Only a comment recording the reason was added, and the item was withdrawn in a
comment on the JIRA issue.
Claude Code on behalf of oscerd
🤖 Generated with Claude Code