CAMEL-23258: Add google-mail:draft DataType transformer#22389
CAMEL-23258: Add google-mail:draft DataType transformer#22389zbendhiba wants to merge 3 commits intoapache:mainfrom
Conversation
This code was created with the help of Claude code Fixes #CAMEL-23258
| It works with `google-mail-stream`, which sets headers like `threadId`, `messageId`, `from`, `to`, `cc`, `bcc`, and `subject` automatically when consuming messages. | ||
| You can also set these headers manually to create drafts from scratch. | ||
|
|
||
| === Headers |
There was a problem hiding this comment.
aren't the headers already (automatically) printed?
There was a problem hiding this comment.
@Croway In the reply-to use case, we reuse the existing headers from the Camel exchange.
When creating a draft from scratch (a new email, not a reply-to), those headers need to be populated explicitly. However, I didn't make them mandatory. A user might want to send a message to an agent about an idea for an email via WhatsApp for example. They may not know the proper email address at that moment.
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
apupier
left a comment
apupier
left a comment
There was a problem hiding this comment.
one comment to address and otherwise optional thing: a lot of places where using AssertJ assertions will allow less code and better error message in case of test failure (not marked all)
| assertTrue(decodedMessage.contains("This is a test draft message")); | ||
| assertTrue(decodedMessage.contains("Content-Type: text/plain; charset=UTF-8")); |
There was a problem hiding this comment.
optional: using AssertJ assertions allows to have a better error message in case of failure. Not just only true/false but the expected text and what searched inside.
| assertTrue(decodedMessage.contains("From: sender@example.com")); | ||
| assertTrue(decodedMessage.contains("Subject: Re: Test Subject")); | ||
| assertTrue(decodedMessage.contains("In-Reply-To: <msg-456@mail.gmail.com>")); | ||
| assertTrue(decodedMessage.contains("References: <msg-456@mail.gmail.com>")); | ||
| assertTrue(decodedMessage.contains("Reply message")); |
There was a problem hiding this comment.
optional: using AssertJ assertions allows to have a better error message in case of failure. Not just only true/false but the expected text and what searched inside.
These assetiosn can also be chained reducing slightly the amount of code written
| assertTrue(decodedMessage.contains("From: from@example.com")); | ||
| assertTrue(decodedMessage.contains("To: to@example.com")); | ||
| assertTrue(decodedMessage.contains("Cc: cc@example.com")); | ||
| assertTrue(decodedMessage.contains("Bcc: bcc@example.com")); | ||
| assertTrue(decodedMessage.contains("Subject: Complete Test")); | ||
| assertTrue(decodedMessage.contains("Complete message")); |
There was a problem hiding this comment.
optional: using AssertJ assertions allows to have a better error message in case of failure. Not just only true/false but the expected text and what searched inside.
These assetiosn can also be chained reducing slightly the amount of code written
| assertNotNull(draft.getMessage().getRaw()); | ||
|
|
||
| String decodedMessage = decodeMessage(draft.getMessage().getRaw()); | ||
| assertTrue(decodedMessage.contains("Subject: Empty Body Test")); |
There was a problem hiding this comment.
optional: using AssertJ assertions allows to have a better error message in case of failure. Not just only true/false but the expected text and what searched inside.
|
|
||
| Draft draft = exchange.getMessage().getBody(Draft.class); | ||
| assertNotNull(draft); | ||
| assertNotNull(draft.getMessage().getRaw()); |
There was a problem hiding this comment.
Is it normal that we do not have a null body if we set a null body?
There was a problem hiding this comment.
@apupier Good point. I'll make the body mandatory. A draft without a body doesn't make much sense.
|
|
||
| String decodedMessage = decodeMessage(draft.getMessage().getRaw()); | ||
| // Without explicit Content-Type header, defaults to text/plain even for HTML-like body | ||
| assertTrue(decodedMessage.contains("text/plain")); |
There was a problem hiding this comment.
optional: using AssertJ assertions allows to have a better error message in case of failure. Not just only true/false but the expected text and what searched inside.
| assertNotNull(draft); | ||
|
|
||
| String decodedMessage = decodeMessage(draft.getMessage().getRaw()); | ||
| // Without explicit Content-Type header, defaults to text/plain even for HTML-like body |
There was a problem hiding this comment.
I think this comment will be better placed directly in the error message of the assertion so that when it fails we directly understand what was looked for and why.
Also better chance that it is kept in sync with the code if it is not a comment
| assertTrue(decodedMessage.contains("text/html")); | ||
| assertTrue(decodedMessage.contains(htmlBody)); |
There was a problem hiding this comment.
optional: using AssertJ assertions allows to have a better error message in case of failure. Not just only true/false but the expected text and what searched inside.
| // Verify that the non-ASCII characters are preserved after decoding | ||
| String decodedSubject = MimeUtility.decodeText( | ||
| decodedMessage.lines().filter(l -> l.startsWith("Subject:")).findFirst().orElse("")); | ||
| assertNotEquals("", decodedSubject); |
There was a problem hiding this comment.
this check is not useful given that the next assertions is checking more precisely and will cover the case of empty string
|
@apupier I've addressed most of your comments in my new commits |
3 participants
This code was created with the help of Claude code Fixes #CAMEL-23258
Description
Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.