Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mail-connector): check htmlBody and/or body is null and throw an error #3796

Merged
merged 8 commits into from
Dec 18, 2024
Merged

fix(mail-connector): check htmlBody and/or body is null and throw an error #3796

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bf5f932
fix(mail-connector): check htmlBody and/or body is null and throw an …
itsnuyen Dec 13, 2024
f7d9e98
Merge branch 'main' into pham-minh-nguyen-fix-smtp-mailconnector
itsnuyen Dec 16, 2024
3e2815c
chore(mail-connector): build the assertation inside the `SmtpSendMail…
itsnuyen Dec 17, 2024
4baa4d8
Merge branch 'main' into pham-minh-nguyen-fix-smtp-mailconnector
itsnuyen Dec 17, 2024
dab711f
test(mail-connector): adding unit test to validate `isEmailMessageValid`
itsnuyen Dec 18, 2024
7b8a18a
Merge remote-tracking branch 'origin/pham-minh-nguyen-fix-smtp-mailco…
itsnuyen Dec 18, 2024
d3fb393
chore(mail-connector): run mvn spotless to format the `SmtpSendEmailT…
itsnuyen Dec 18, 2024
f356390
chore(mail-connector): use mvn:licence to add the licence to the file
itsnuyen Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions ...n/java/io/camunda/connector/email/client/jakarta/outbound/JakartaEmailActionExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,16 +301,25 @@ private Multipart getMultipart(SmtpSendEmail smtpSendEmail) throws MessagingExce
Multipart multipart = new MimeMultipart();
switch (smtpSendEmail.contentType()) {
case PLAIN -> {
if (smtpSendEmail.body() == null) {
throw new MessagingException("Text Body is null");
}
MimeBodyPart textPart = new MimeBodyPart();
textPart.setText(smtpSendEmail.body(), StandardCharsets.UTF_8.name());
multipart.addBodyPart(textPart);
}
case HTML -> {
if (smtpSendEmail.htmlBody() == null) {
throw new MessagingException("HTML Body is null");
}
MimeBodyPart htmlPart = new MimeBodyPart();
htmlPart.setContent(smtpSendEmail.htmlBody(), JakartaUtils.HTML_CHARSET);
multipart.addBodyPart(htmlPart);
}
case MULTIPART -> {
if (smtpSendEmail.htmlBody() == null || smtpSendEmail.body() == null) {
throw new MessagingException("Text or HTML Body is null");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @itsnuyen, instead of throwing a MessagingException, could you implement a custom validation method in the model that verifies that these values are not null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @sbuettner,
I have one questions. Do you mean adapting inside the SmtpSendEmail record the validation like this?

switch (contentType) {
  case PLAIN:
    if (body == null) {
      throw new IllegalArgumentException("Email body is null");
    }
  case HTML:
    if (htmlBody == null) {
      throw new IllegalArgumentException("Email html body is null");
    }
  case MULTIPART:
    if (body == null || htmlBody == null) {
      throw new IllegalArgumentException("Email body or html body cannot be null");
    }
}

This could also running but as a error message it could be confusing for the end user:
image
As it state that an exception is raised and it is a JSON_PROCESSING_ERROR

Do you have an Idea how I could provide the end user proper feedback?
Or do I even misunderstand your request?

Copy link
Contributor

@sbuettner sbuettner Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hey @itsnuyen I was thinking about something like this as part of the SmtpSendEmail record:

@AssertTrue(message = "Please provide a proper message body")
        public boolean isValidMessageBody() {
                return body != null || htmlBody != null;
        }

MimeBodyPart textPart = new MimeBodyPart();
textPart.setText(smtpSendEmail.body(), StandardCharsets.UTF_8.name());
MimeBodyPart htmlPart = new MimeBodyPart();
Expand Down
Loading