Skip to content

CAMEL-24344: camel-google-mail - return a body for raw and non-multipart messages - #25357

Open
oscerd wants to merge 2 commits into
apache:mainfrom
oscerd:fix/CAMEL-24344
Open

CAMEL-24344: camel-google-mail - return a body for raw and non-multipart messages#25357
oscerd wants to merge 2 commits into
apache:mainfrom
oscerd:fix/CAMEL-24344

Conversation

@oscerd

@oscerd oscerd commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Three body-related defects in the google-mail-stream consumer.

1. raw=true always produced a null body. The consumer asked for the FULL format:

Message mess = getClient().users().messages().get("me", message.getId()).setFormat("FULL").execute();
...
if (getConfiguration().isRaw()) { message.setBody(mail.getRaw()); }

but Gmail only populates raw for format=RAW
(API reference:
"Returned in messages.get and drafts.get responses when the format=RAW parameter is supplied.").
The requested format now follows the option.

Since the RAW format does not return the parsed payload, a route with raw=true no longer gets
the subject/from/to/cc/bcc/message-id headers — they are part of the RFC 2822 content that is now in
the body. That is covered by an upgrade-guide entry.

2. Messages that are not multipart arrived with no body. The old code only looked at
getPayload().getParts(), which is null for a plain message — its content sits directly on
payload.body.data. Nested multiparts (a multipart/mixed inside a multipart/alternative) were
also skipped, because parts.get(0).getBody().getData() is null for a part that only holds other
parts. The body is now taken from the first part that actually carries data, descending into nested
parts.

3. getPayload() and getPayload().getHeaders() were dereferenced unguarded — both are absent
for some message formats and metadata-only responses.

Also: processRollback re-added the UNREAD label even when markAsRead was never enabled, so a
failed exchange mutated the mailbox for routes that had not asked for it; and its catch block
reported "Error occurred mark as read mail" for a rollback failure.

Claude Code on behalf of oscerd

🤖 Generated with Claude Code

…art messages

The stream consumer always asked the Gmail API for the FULL message format, but
the raw field is only populated for the RAW format, so raw=true always produced
a null body. The consumer now asks for the format matching the option.

The non-raw path only looked at the first element of getParts(), so a message
that is not multipart - which carries its content directly on the payload - was
delivered without a body, and a nested multipart yielded nothing either. The
body is now taken from the first part carrying data, walking into nested parts.

The payload and its header list are also no longer dereferenced without a check,
and a failed exchange no longer marks the mail as unread when markAsRead was not
enabled in the first place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
@oscerd oscerd added the bug Something isn't working label Aug 5, 2026
@oscerd
oscerd requested review from Croway and davsclaus August 5, 2026 11:14
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

protected void processRollback(Exchange exchange, String unreadLabelId) {
if (!getConfiguration().isMarkAsRead()) {
// the mail was never marked as read, so there is nothing to roll back
LOG.warn("Exchange failed: {}", exchange);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we WARN logging here when this method is about marking the mail as un-read, and this is an early exit to not do that, so I would assume this LOG should be removed

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix — the three bugs are well identified and the upgrade guide entry is clear.

One minor finding in the new test; everything else looks good.

This review was generated by an AI agent on behalf of davsclaus and may contain inaccuracies. Please verify all suggestions before applying.

context.start();
GoogleMailStreamEndpoint endpoint = context.getEndpoint(
"google-mail-stream://index?clientId=id&clientSecret=secret&raw=" + raw,
GoogleMailStreamEndpoint.class);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor resource leak: theRawOptionAsksForTheRawFormat calls consumer() twice — the first call creates and starts a DefaultCamelContext, then the second call overwrites this.context without stopping the first one. The orphaned context is never cleaned up by @AfterEach.

Suggestion — stop any previous context before creating a new one:

Suggested change
GoogleMailStreamEndpoint.class);
private GoogleMailStreamConsumer consumer(boolean raw) throws Exception {
if (context != null) {
context.stop();
}
context = new DefaultCamelContext();

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🧪 CI tested the following changed modules:

  • components/camel-google/camel-google-mail
  • docs

🔬 Scalpel shadow comparison — Scalpel: 10 tested, 28 compile-only — current: 9 all tested

Maveniverse Scalpel detected 38 affected modules (current approach: 9).

⚠️ Modules only in Scalpel (29)
  • apache-camel
  • camel-allcomponents
  • camel-catalog
  • camel-catalog-console
  • camel-catalog-lucene
  • camel-catalog-maven
  • camel-catalog-suggest
  • camel-componentdsl
  • camel-csimple-maven-plugin
  • camel-endpointdsl
  • camel-endpointdsl-support
  • camel-itest
  • camel-jbang-core
  • camel-jbang-it
  • camel-jbang-main
  • camel-jbang-plugin-edit
  • camel-jbang-plugin-generate
  • camel-jbang-plugin-kubernetes
  • camel-jbang-plugin-test
  • camel-kamelet-main
  • camel-launcher
  • camel-report-maven-plugin
  • camel-route-parser
  • camel-yaml-dsl
  • camel-yaml-dsl-deserializers
  • camel-yaml-dsl-maven-plugin
  • coverage
  • docs
  • dummy-component

Skip-tests mode would test 10 modules (2 direct + 8 downstream), skip tests for 28 (generated code, meta-modules)

Modules Scalpel would test (10)
  • camel-google-mail
  • camel-jbang-mcp
  • camel-jbang-plugin-mcp
  • camel-jbang-plugin-route-parser
  • camel-jbang-plugin-tui
  • camel-jbang-plugin-validate
  • camel-launcher-container
  • camel-yaml-dsl-validator
  • camel-yaml-dsl-validator-maven-plugin
  • docs
Modules with tests skipped (28)
  • apache-camel
  • camel-allcomponents
  • camel-catalog
  • camel-catalog-console
  • camel-catalog-lucene
  • camel-catalog-maven
  • camel-catalog-suggest
  • camel-componentdsl
  • camel-csimple-maven-plugin
  • camel-endpointdsl
  • camel-endpointdsl-support
  • camel-itest
  • camel-jbang-core
  • camel-jbang-it
  • camel-jbang-main
  • camel-jbang-plugin-edit
  • camel-jbang-plugin-generate
  • camel-jbang-plugin-kubernetes
  • camel-jbang-plugin-test
  • camel-kamelet-main
  • camel-launcher
  • camel-report-maven-plugin
  • camel-route-parser
  • camel-yaml-dsl
  • camel-yaml-dsl-deserializers
  • camel-yaml-dsl-maven-plugin
  • coverage
  • dummy-component

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

All tested modules (38 modules)
  • Camel :: All Components Sync point
  • Camel :: Assembly
  • Camel :: Catalog :: CSimple Maven Plugin (deprecated)
  • Camel :: Catalog :: Camel Catalog
  • Camel :: Catalog :: Camel Report Maven Plugin
  • Camel :: Catalog :: Camel Route Parser
  • Camel :: Catalog :: Console
  • Camel :: Catalog :: Dummy Component
  • Camel :: Catalog :: Lucene (deprecated)
  • Camel :: Catalog :: Maven
  • Camel :: Catalog :: Suggest
  • Camel :: Component DSL
  • Camel :: Coverage
  • Camel :: Docs
  • Camel :: Endpoint DSL
  • Camel :: Endpoint DSL :: Support
  • Camel :: Google :: Mail
  • Camel :: Integration Tests
  • Camel :: JBang :: Core
  • Camel :: JBang :: Integration tests
  • Camel :: JBang :: MCP
  • Camel :: JBang :: Main
  • Camel :: JBang :: Plugin :: Edit
  • Camel :: JBang :: Plugin :: Generate
  • Camel :: JBang :: Plugin :: Kubernetes
  • Camel :: JBang :: Plugin :: MCP
  • Camel :: JBang :: Plugin :: Route Parser
  • Camel :: JBang :: Plugin :: TUI
  • Camel :: JBang :: Plugin :: Testing
  • Camel :: JBang :: Plugin :: Validate
  • Camel :: Kamelet Main
  • Camel :: Launcher
  • Camel :: Launcher :: Container
  • Camel :: YAML DSL
  • Camel :: YAML DSL :: Deserializers
  • Camel :: YAML DSL :: Maven Plugins
  • Camel :: YAML DSL :: Validator
  • Camel :: YAML DSL :: Validator Maven Plugin

⚙️ View full build and test results

…ious test context

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
@oscerd

oscerd commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Both points addressed in the follow-up commit:

  • processRollback — the early exit no longer logs. The failure itself is already reported by the
    route's error handler, and nothing is being rolled back on that path.
  • GoogleMailStreamConsumerBodyTestconsumer(...) now stops the previous context before creating
    a new one, so the context built by the first call in theRawOptionAsksForTheRawFormat is not
    orphaned.

Claude Code on behalf of oscerd

@oscerd
oscerd requested a review from davsclaus August 5, 2026 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working components docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants