You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "allthingssecurity (via GitHub)" <gi...@apache.org> on 2026/10/17 00:12:48 UTC
[PR] CAMEL-25710: camel-mail - remove the idempotent keys of mails that a poll does not route [camel]
allthingssecurity opened a new pull request, #27990:
URL: https://github.com/apache/camel/pull/27990
# Description
[CAMEL-25710](https://issues.apache.org/jira/browse/CAMEL-25710)
With an `idempotentRepository` (documented as the way to let several consumers share one mailbox), `MailConsumer.retrieveMessages` adds the key of every mail of a poll to the repository before any of them is routed; the key is removed or confirmed when the exchange of the mail completes. Mails that the poll did not route kept their key, so every later poll, also on other consumers sharing the repository, skipped them as duplicates. They stayed unread on the server and were never routed, without a log line:
- the rest of the batch when the consumer stops in the middle of it (graceful route or CamelContext stop, default `ShutdownRunningTask.CompleteCurrentTaskOnly`); those exchanges were not released either;
- the mails retrieved before a mail whose exchange cannot be created (for example its attachments cannot be read), which fails the poll, and that mail itself: after the bad mail is removed, the others are still skipped.
Changes (`MailConsumer` only):
- `processBatch` gives back the exchanges it did not route: removes their key from the idempotent repository and releases them. The aws2-s3 consumer does the same with its in-progress keys since CAMEL-25521.
- `retrieveMessages` does the same for the exchanges created so far, and removes the key of the failing mail, when creating an exchange throws; the exception is rethrown as before.
What users can notice (upgrade guide note added, 4.24 guide, `camel-mail` section): with an `idempotentRepository` and the defaults `skipFailedMessage=false` / `handleFailedMessage=false`, a mail whose exchange cannot be created now fails every poll until it is removed, as it already does without an `idempotentRepository` and as the documentation of these options describes. Before, it failed only the first poll and was then skipped, together with the mails retrieved before it. `skipFailedMessage` or `handleFailedMessage` skip such a mail.
Tests:
- New `MailIdempotentRepositoryNotRoutedTest` (2 tests, GreenMail IMAP, `MemoryIdempotentRepository`): (1) 5 mails, the route is stopped while it routes the first one, then started again: all 5 must be routed; (2) 3 mails and a multipart mail without its boundary: the poll fails, the bad mail is deleted, the route is started again: the 3 mails must be routed. Bounded waits only, no sleeps.
- Without the change both tests fail: `The keys of the mails that were not routed must be removed ==> expected: <0> but was: <4>`.
- With the change: the camel-mail tests pass: 217 run, 0 failures, 0 errors, 3 skipped.
What is not changed:
- A mail skipped by `skipFailedMessage` / `handleFailedMessage`, or flagged DELETED, keeps its key, as before.
- No option or default changes.
What was not run: a persistent or shared idempotent repository, POP3 (the tests use IMAP).
# Target
- [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch)
# Tracking
- [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it): [CAMEL-25710](https://issues.apache.org/jira/browse/CAMEL-25710).
# Apache Camel coding standards and style
- [x] I checked that each commit in the pull request has a meaningful subject line and body.
- [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes.
(I built camel-mail and the modules it depends on, including the formatter and import-sort plugins, and ran the camel-mail tests. I did not run the full root build.)
# AI-assisted contributions
- [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used.
This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer.
_Claude Code on behalf of allthingssecurity_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25710: camel-mail - remove the idempotent keys of mails that a poll does not route [camel]
Posted by "allthingssecurity (via GitHub)" <gi...@apache.org>.
allthingssecurity commented on code in PR #27990:
URL: https://github.com/apache/camel/pull/27990#discussion_r4302274445
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -266,8 +278,32 @@ public String toString() {
// process the exchange
processExchange(exchange);
}
+ }
- return total;
+ /**
+ * Gives back exchanges that are not routed: removes the key of each mail from the idempotent repository (where
+ * {@link #isValidMessage(String, Message)} added it when the mail was retrieved), so the mail is not skipped as a
+ * duplicate later, and releases the exchange.
+ */
+ private void releaseNotRouted(Queue<Exchange> notRouted) {
+ Exchange exchange;
+ while ((exchange = notRouted.poll()) != null) {
+ removeIdempotentKey(exchange.getProperty(MAIL_MESSAGE_UID, String.class));
+ releaseExchange(exchange, true);
Review Comment:
Done in f4c7892748f2: `releaseExchange` in the give-back loop is now guarded like `removeIdempotentKey` (WARN with the exception, then the loop goes on), as gnodet-bot suggested. New test `keysOfTheRestOfTheBatchAreRemovedWhenReleasingAnExchangeThrows`: routing the second of 5 mails throws and releasing the first given-back exchange throws; the keys of all three given-back mails are still removed. Without the change two keys stay. camel-mail tests: 245 run, 0 failures. The PR description is updated to match.
The key of the exchange that `processExchange` throws on stays as a follow-up, as you said.
_Claude Code on behalf of allthingssecurity_
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -266,8 +278,32 @@ public String toString() {
// process the exchange
processExchange(exchange);
Review Comment:
Agreed, that is the remaining case (only an unchecked exception escaping `processExchange` reaches it). As Claus suggested, I'll do it as a follow-up with its own JIRA and test, so this PR stays as reviewed.
_Claude Code on behalf of allthingssecurity_
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25710: camel-mail - remove the idempotent keys of mails that a poll does not route [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #27990:
URL: https://github.com/apache/camel/pull/27990#discussion_r4302642500
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -266,8 +278,32 @@ public String toString() {
// process the exchange
processExchange(exchange);
Review Comment:
Tracked as the follow-up with its own JIRA and test that @allthingssecurity offered; resolving so this PR can go in.
_Claude Code on behalf of Claus Ibsen_
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25710: camel-mail - remove the idempotent keys of mails that a poll does not route [camel]
Posted by "allthingssecurity (via GitHub)" <gi...@apache.org>.
allthingssecurity commented on code in PR #27990:
URL: https://github.com/apache/camel/pull/27990#discussion_r4301873321
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -266,10 +266,32 @@ public String toString() {
// process the exchange
processExchange(exchange);
}
+ // the remaining exchanges are not routed as the consumer is stopping: their mails must be consumed by a
+ // later poll (or by another consumer sharing the idempotent repository)
+ releaseNotRouted(CastUtils.cast(exchanges));
Review Comment:
Thanks, fixed in d699316e2543. `processBatch` now routes the batch in a `try` and gives back the rest in a `finally`, so the keys of the exchanges that are not routed are also removed when `processExchange` throws.
New test `MailIdempotentRepositoryPollErrorTest.keysOfTheRestOfTheBatchAreRemovedWhenRoutingAnExchangeThrows`: 5 mails, routing the second throws a `RejectedExecutionException`. Afterwards the repository holds only the keys of the two mails that went into routing. Without the change, all 5 keys stay. camel-mail tests: 244 run, 0 failures.
_Claude Code on behalf of allthingssecurity_
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -330,7 +352,15 @@ private Queue<Exchange> retrieveMessages() throws MessagingException {
if (getEndpoint().getConfiguration().isPeek()) {
peekMessage(message);
}
- Exchange exchange = createExchange(new KeyValueHolder<>(key, message));
+ Exchange exchange;
+ try {
+ exchange = createExchange(new KeyValueHolder<>(key, message));
+ } catch (Exception e) {
Review Comment:
Thanks, fixed in d699316e2543. The `try`/`catch` in `retrieveMessages` now covers the whole loop body. The key of the current mail is removed only once `isValidMessage` has added it, so a failing `add(key)` removes only the keys of the mails before it. I also made removing a key robust: a repository that also fails in `remove` is logged at WARN, and the other mails are still given back, and the failure of the poll is not hidden.
New test `MailIdempotentRepositoryPollErrorTest.keysOfTheMailsRetrievedSoFarAreRemovedWhenAddingTheKeyOfALaterMailFails`: the repository fails the third `add`, the poll fails, and the repository is empty afterwards. Without the change, the first two keys stay.
_Claude Code on behalf of allthingssecurity_
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25710: camel-mail - remove the idempotent keys of mails that a poll does not route [camel]
Posted by "allthingssecurity (via GitHub)" <gi...@apache.org>.
allthingssecurity commented on code in PR #27990:
URL: https://github.com/apache/camel/pull/27990#discussion_r4302723909
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -266,8 +278,32 @@ public String toString() {
// process the exchange
processExchange(exchange);
Review Comment:
Filed as CAMEL-25733.
_Claude Code on behalf of allthingssecurity_
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25710: camel-mail - remove the idempotent keys of mails that a poll does not route [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #27990:
URL: https://github.com/apache/camel/pull/27990#discussion_r4301786297
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -266,10 +266,32 @@ public String toString() {
// process the exchange
processExchange(exchange);
}
+ // the remaining exchanges are not routed as the consumer is stopping: their mails must be consumed by a
+ // later poll (or by another consumer sharing the idempotent repository)
+ releaseNotRouted(CastUtils.cast(exchanges));
Review Comment:
Minor: the give-back runs only when the loop ends normally. If `processExchange` throws (rare, since routing errors normally land on the exchange, but for example a `RejectedExecutionException`), the rest of the batch keeps its keys again. Putting the loop in `try { ... } finally { releaseNotRouted(CastUtils.cast(exchanges)); }` would cover that path too.
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -330,7 +352,15 @@ private Queue<Exchange> retrieveMessages() throws MessagingException {
if (getEndpoint().getConfiguration().isPeek()) {
peekMessage(message);
}
- Exchange exchange = createExchange(new KeyValueHolder<>(key, message));
+ Exchange exchange;
+ try {
+ exchange = createExchange(new KeyValueHolder<>(key, message));
+ } catch (Exception e) {
Review Comment:
Minor: only `createExchange` is guarded. If `getIdempotentRepository().add(key)` in `isValidMessage` throws for a later mail (a JDBC/Infinispan/Redis repository that is briefly unavailable), the keys of the mails already in `answer` stay in the repository, which is the same leak. Putting the try/catch around the whole `for` loop body (and removing the current key only once it has been added) would cover that path too.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25710: camel-mail - remove the idempotent keys of mails that a poll does not route [camel]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #27990:
URL: https://github.com/apache/camel/pull/27990#discussion_r4301865352
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -266,8 +278,32 @@ public String toString() {
// process the exchange
processExchange(exchange);
}
+ }
- return total;
+ /**
+ * Gives back exchanges that are not routed: removes the key of each mail from the idempotent repository (where
+ * {@link #isValidMessage(String, Message)} added it when the mail was retrieved), so the mail is not skipped as a
+ * duplicate later, and releases the exchange.
+ */
+ private void releaseNotRouted(Queue<Exchange> notRouted) {
+ Exchange exchange;
+ while ((exchange = notRouted.poll()) != null) {
+ removeIdempotentKey(exchange.getProperty(MAIL_MESSAGE_UID, String.class));
+ releaseExchange(exchange, true);
Review Comment:
⚠️ **`releaseExchange` not guarded in cleanup loop** — if it throws a `RuntimeException` (e.g. a pooled exchange factory error), the `while` loop terminates and all remaining exchanges in the queue are skipped: their idempotent keys leak, exactly the bug this PR is fixing. `removeIdempotentKey` is already guarded with `try/catch`; `releaseExchange` should be too:
```suggestion
removeIdempotentKey(exchange.getProperty(MAIL_MESSAGE_UID, String.class));
try {
releaseExchange(exchange, true);
} catch (Exception e) {
LOG.warn("Cannot release an exchange that is not routed: {}.", e.toString());
}
```
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -266,8 +278,32 @@ public String toString() {
// process the exchange
processExchange(exchange);
Review Comment:
💡 **Residual key leak for the exchange that `processExchange` throws on** — that exchange is already polled from the queue (two lines above), so `releaseNotRouted` in the `finally` block won't see it. Its idempotent key stays in the repository until the repository is cleared externally. Test 3 (`keysOfTheRestOfTheBatchAreRemovedWhenRoutingAnExchangeThrows`) explicitly asserts this: `assertEquals(new TreeSet<>(routed), repo.keys())` — the key of the throwing exchange stays. This is the same gap @davsclaus flagged, and the PR closes the *rest-of-batch* part of it. The *current-exchange* part could be addressed by catching the exception in `routeBatch` itself, removing the key, and re-throwing — but that is a separate, minor case (only unchecked exceptions escaping `processExchange` reach here).
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25710: camel-mail - remove the idempotent keys of mails that a poll does not route [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27990:
URL: https://github.com/apache/camel/pull/27990#issuecomment-6230981852
<!-- ci-tested-modules -->
:test_tube: **CI tested the following changed modules:**
- `components/camel-mail`
- `docs`
:repeat: **1 test passed only after a retry on JDK 25** (1 retried attempt)
<details><summary>Recovered flaky tests on JDK 25 (1)</summary>
| Module | Test | Failed attempts | First failure |
| --- | --- | --- | --- |
| `components/camel-mail` | `MailMoveToTest.testMoveToWithDelete` | 1 | expected: <5> but was: <4> |
> :information_source: These tests did **not** fail the build. Surefire retried them and they passed.
> Retries are enabled project-wide by `surefire.rerunFailingTestsCount` in `parent/pom.xml`.
</details>
---
<details><summary>:microscope: Scalpel shadow comparison — Scalpel: 9 of 706 tested, 26 compile-only — current: 9 all tested</summary>
[Maveniverse Scalpel](https://github.com/maveniverse/scalpel) detected **9 affected modules** (current approach: 9).
**Skip-tests mode would test 9 modules** (2 direct + 8 downstream), **skip tests for 26** (generated code, meta-modules)
<details><summary>Modules Scalpel would test (9)</summary>
- `camel-jbang-mcp` ← downstream of org.apache.camel:camel-catalog
- `camel-jbang-plugin-mcp` ← downstream of org.apache.camel:camel-jbang-core
- `camel-jbang-plugin-tui` ← downstream of org.apache.camel:camel-catalog
- `camel-jbang-plugin-validate` ← downstream of org.apache.camel:camel-yaml-dsl-validator
- `camel-launcher-container` ← downstream of org.apache.camel:camel-launcher
- `camel-mail` ← components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java, components/camel-mail/src/test/java/org/apache/camel/component/mail/MailIdempotentRepositoryNotRoutedTest.java
- `camel-mail-microsoft-oauth` ← downstream of org.apache.camel:camel-mail
- `camel-yaml-dsl-validator` ← downstream of org.apache.camel:camel-catalog
- `camel-yaml-dsl-validator-maven-plugin` ← downstream of org.apache.camel:camel-yaml-dsl-validator
</details>
<details><summary>Modules with tests skipped (26)</summary>
- `apache-camel`
- `camel-allcomponents`
- `camel-catalog`
- `camel-catalog-console`
- `camel-catalog-maven`
- `camel-catalog-suggest`
- `camel-componentdsl`
- `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`
</details>
> :information_source: Shadow mode — Scalpel observes but does not affect test execution. [Learn more](https://github.com/maveniverse/scalpel)
</details>
<details><summary>All tested modules (36 modules, 5m 32s total)</summary>
**Total reactor time:** 5m 32s
| Module | Duration | Status |
| --- | --- | --- |
| Camel :: Launcher | 45.8s | SUCCESS |
| Camel :: JBang :: MCP | 40.5s | SUCCESS |
| Camel :: JBang :: Plugin :: Validate | 38.4s | SUCCESS |
| Camel :: Component DSL | 26.2s | SUCCESS |
| Camel :: Catalog :: Camel Catalog | 21.4s | SUCCESS |
| Camel :: YAML DSL | 19.8s | SUCCESS |
| Camel :: YAML DSL :: Validator | 19.8s | SUCCESS |
| Camel :: JBang :: Plugin :: Kubernetes | 19.0s | SUCCESS |
| Camel :: JBang :: Plugin :: Testing | 15.7s | SUCCESS |
| Camel :: Kamelet Main | 14.8s | SUCCESS |
| Camel :: Docs | 14.3s | SUCCESS |
| Camel :: YAML DSL :: Deserializers | 9.0s | SUCCESS |
| Camel :: Catalog :: Camel Route Parser | 7.9s | SUCCESS |
| Camel :: Catalog :: Camel Report Maven Plugin | 7.1s | SUCCESS |
| Camel :: All Components Sync point | 5.4s | SUCCESS |
| Camel :: YAML DSL :: Validator Maven Plugin | 4.1s | SUCCESS |
| Camel :: YAML DSL :: Maven Plugins | 3.1s | SUCCESS |
| Camel :: Catalog :: Maven | 2.9s | SUCCESS |
| Camel :: Catalog :: Suggest (deprecated) | 2.6s | SUCCESS |
| Camel :: JBang :: Plugin :: MCP | 1.8s | SUCCESS |
| Camel :: Assembly | 1.8s | SUCCESS |
| Camel :: JBang :: Integration tests | 1.7s | SUCCESS |
| Camel :: JBang :: Plugin :: Edit | 1.4s | SUCCESS |
| Camel :: JBang :: Plugin :: Generate | 1.3s | SUCCESS |
| Camel :: Coverage | 1.2s | SUCCESS |
| Camel :: Catalog :: Console | 1.0s | SUCCESS |
| Camel :: Mail :: Microsoft OAuth | 0.9s | SUCCESS |
| Camel :: JBang :: Main | 0.8s | SUCCESS |
| Camel :: Endpoint DSL :: Support | 0.8s | SUCCESS |
| Camel :: Catalog :: Dummy Component | 0.7s | SUCCESS |
| Camel :: Launcher :: Container | 0.5s | SUCCESS |
| Camel :: Endpoint DSL | n/a | |
| Camel :: Integration Tests | n/a | |
| Camel :: JBang :: Core | n/a | |
| Camel :: JBang :: Plugin :: TUI | n/a | |
| Camel :: Mail | n/a | |
**Top 20 slowest modules:**
- `Camel :: Launcher` (45.8s)
- `Camel :: JBang :: MCP` (40.5s)
- `Camel :: JBang :: Plugin :: Validate` (38.4s)
- `Camel :: Component DSL` (26.2s)
- `Camel :: Catalog :: Camel Catalog` (21.4s)
- `Camel :: YAML DSL` (19.8s)
- `Camel :: YAML DSL :: Validator` (19.8s)
- `Camel :: JBang :: Plugin :: Kubernetes` (19.0s)
- `Camel :: JBang :: Plugin :: Testing` (15.7s)
- `Camel :: Kamelet Main` (14.8s)
- `Camel :: Docs` (14.3s)
- `Camel :: YAML DSL :: Deserializers` (9.0s)
- `Camel :: Catalog :: Camel Route Parser` (7.9s)
- `Camel :: Catalog :: Camel Report Maven Plugin` (7.1s)
- `Camel :: All Components Sync point` (5.4s)
- `Camel :: YAML DSL :: Validator Maven Plugin` (4.1s)
- `Camel :: YAML DSL :: Maven Plugins` (3.1s)
- `Camel :: Catalog :: Maven` (2.9s)
- `Camel :: Catalog :: Suggest (deprecated)` (2.6s)
- `Camel :: JBang :: Plugin :: MCP` (1.8s)
</details>
---
:gear: [View full build and test results](https://github.com/apache/camel/actions/runs/38985313909)
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25710: camel-mail - remove the idempotent keys of mails that a poll does not route [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus merged PR #27990:
URL: https://github.com/apache/camel/pull/27990
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25710: camel-mail - remove the idempotent keys of mails that a poll does not route [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27990:
URL: https://github.com/apache/camel/pull/27990#issuecomment-6230664542
:star2: Thank you for your contribution to the Apache Camel project! :star2:
:robot: CI automation will test this PR automatically.
:camel: 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](https://camel.apache.org/community/team/#committers) have access to the summary.
:warning: Be careful when sharing logs. Review their contents before sharing them publicly.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25710: camel-mail - remove the idempotent keys of mails that a poll does not route [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #27990:
URL: https://github.com/apache/camel/pull/27990#discussion_r4302184994
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -266,8 +278,32 @@ public String toString() {
// process the exchange
processExchange(exchange);
}
+ }
- return total;
+ /**
+ * Gives back exchanges that are not routed: removes the key of each mail from the idempotent repository (where
+ * {@link #isValidMessage(String, Message)} added it when the mail was retrieved), so the mail is not skipped as a
+ * duplicate later, and releases the exchange.
+ */
+ private void releaseNotRouted(Queue<Exchange> notRouted) {
+ Exchange exchange;
+ while ((exchange = notRouted.poll()) != null) {
+ removeIdempotentKey(exchange.getProperty(MAIL_MESSAGE_UID, String.class));
+ releaseExchange(exchange, true);
Review Comment:
@allthingssecurity, could you apply this suggestion before we merge? It is the same give-back path the PR fixes: if `releaseExchange` throws, the `while` loop ends and the remaining mails of the batch keep their idempotent keys, which is the leak this PR removes. `removeIdempotentKey` just above is already guarded the same way. The other note (the key of the exchange that `processExchange` throws on) can be a follow-up.
_Claude Code on behalf of Claus Ibsen_
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org