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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to