feat(email connector): implement new email connector, Outbound SMTP, IMAP and POP3, Inbound IMAP #3132

mathias-vandaele · 2024-08-20T09:59:38Z

Description

New implementation of the email connectors.

It has not been completely polished but ready for review.

I would like to come back on the label for the ET, and add new tests

Related issues

related #3108
related #3213
related #3214
related #3215

connectors/email/src/main/java/io/camunda/connector/email/core/JakartaExecutor.java

connectors/email/src/main/java/io/camunda/connector/email/core/JakartaSessionFactory.java

connectors/email/src/main/java/io/camunda/connector/email/core/JakartaExecutor.java

connectors/email/src/main/java/io/camunda/connector/email/core/JakartaSessionFactory.java

connectors/email/src/main/java/io/camunda/connector/email/core/JakartaExecutor.java

connectors/email/src/main/java/io/camunda/connector/email/outbound/core/JakartaExecutor.java

...tors/email/src/main/java/io/camunda/connector/email/outbound/core/JakartaSessionFactory.java

...ctors/email/src/main/java/io/camunda/connector/email/core/jakarta/JakartaActionExecutor.java

...ctors/email/src/main/java/io/camunda/connector/email/core/jakarta/JakartaSessionFactory.java

...ectors/email/src/main/java/io/camunda/connector/email/core/jakarta/JakartaEmailListener.java

bundle/default-bundle/src/test/resources/application.properties

...ectors/email/src/main/java/io/camunda/connector/email/core/jakarta/JakartaEmailListener.java

...ctors/email/src/main/java/io/camunda/connector/email/core/jakarta/JakartaActionExecutor.java

johnBgood

Great work, I left some minor comments

connectors/email/src/main/java/io/camunda/connector/email/outbound/EmailConnectorFunction.java

connectors/email/src/main/java/io/camunda/connector/email/outbound/model/EmailRequest.java

connectors/email/src/main/java/io/camunda/connector/email/inbound/EmailConnectorExecutable.java

connectors/email/src/main/java/io/camunda/connector/email/outbound/protocols/Pop3.java

connectors/email/src/main/java/io/camunda/connector/email/outbound/protocols/Imap.java

connectors/email/src/main/java/io/camunda/connector/email/config/SmtpConfig.java

connectors/email/src/main/java/io/camunda/connector/email/core/ActionExecutor.java

...ctors/email/src/main/java/io/camunda/connector/email/core/jakarta/JakartaActionExecutor.java

connectors/email/src/main/java/io/camunda/connector/email/client/jakarta/Email.java

connectors/email/src/main/java/io/camunda/connector/email/client/ActionExecutor.java

...tors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailListener.java

sbuettner · 2024-09-10T07:45:43Z

...tors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailListener.java

+      Arrays.stream(e.getMessages())
+          .peek(
+              message -> {
+                if (markAsRead) this.jakartaUtils.markAsSeen(message);


Shouldnt it be marked after the correlation? If the correlation fails we have already seen the message and wont process it again.

We create a new process when a new message is received (idle https://en.wikipedia.org/wiki/IMAP_IDLE ), not on every unseen email, so even if it was marked as seen and the correlation fails, It would not be processed again..

But dont we want to reprocess it?

sbuettner · 2024-09-10T07:46:43Z

...tors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailListener.java

+                  connectorContext.correlateWithResult(
+                      new ReadEmailResponse(
+                          email.getMessageId(),
+                          email.getFrom(),
+                          email.getSubject(),
+                          email.getSize(),
+                          email.getBody().getBodyAsPlainText(),
+                          email.getBody().getBodyAsHtml())));


Could be extracted into a separate method and switched to a map(this::correlateEmail)

sbuettner · 2024-09-10T07:48:03Z

...tors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailListener.java

+    }
+  }
+
+  private List<String> createInboxList(Object folderToListen) {


Can we use a proper type like List<Inbox>? or something similar without string handling?

This function is really just to format the object, we dont know if that is a String comma separated or already a List made by the Feel engine

sbuettner · 2024-09-10T07:48:44Z

...tors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailListener.java

+      }
+      this.store.close();
+    } catch (MessagingException e) {
+      throw new RuntimeException(e);


Logging this error would help to debug it.

sbuettner · 2024-09-10T07:50:21Z

connectors/email/src/main/java/io/camunda/connector/email/inbound/model/EmailProperties.java

+import jakarta.validation.Valid;
+import jakarta.validation.constraints.NotNull;
+
+public class EmailProperties {


EmailProperties is quite generic. What about EmailInboundConnectorProperties.

sbuettner · 2024-09-10T07:50:38Z

connectors/email/src/main/java/io/camunda/connector/email/inbound/model/EmailListenerData.java

+import jakarta.validation.Valid;
+import jakarta.validation.constraints.NotNull;
+
+public class EmailListenerData {


Can be a record.

Name is quite generic. What about EmailListenerConfig

sbuettner · 2024-09-10T07:51:15Z

connectors/email/src/main/java/io/camunda/connector/email/inbound/model/EmailListenerData.java

+      optional = true,
+      feel = Property.FeelMode.optional,
+      binding = @TemplateProperty.PropertyBinding(name = "data.folderToListen"))
+  private Object folderToListen;


Lets go with a proper type instead of Object.

It looks like this, on other connector, It is usually managed as an Object, Do you see an other way to do ?
It is the only way I could come up with to make complex search request

Ok, I did not know that it is that complex. Would it be possible to map this with a proper model? Like ImapSearchQuery?

The Object can really depends:

I can have

{ "field":"FROM", "value":"[email protected]" }

or the example given previously, so the root object is not always the same

sbuettner · 2024-09-10T07:52:44Z

Great work @mathias-vandaele. Added some comments.

...mail/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailActionExecutor.java

+    return new JakartaEmailActionExecutor(sessionFactory, objectMapper);
+  }
+
+  public Object execute(EmailRequest emailRequest) {


...tors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailListener.java

+    return new JakartaEmailListener(sessionFactory);
+  }
+
+  public void startListener(InboundConnectorContext context) {


connectors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaUtils.java

...tors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailListener.java

+    switch (correlationResult) {
+      case CorrelationResult.Failure failure -> {
+        switch (failure.handlingStrategy()) {
+          case CorrelationFailureHandlingStrategy.ForwardErrorToUpstream __ -> {}


...tors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailListener.java

+      case CorrelationResult.Failure failure -> {
+        switch (failure.handlingStrategy()) {
+          case CorrelationFailureHandlingStrategy.ForwardErrorToUpstream __ -> {}
+          case CorrelationFailureHandlingStrategy.Ignore __ ->


...tors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailListener.java

+              executePostProcess(message, emailListenerConfig);
+        }
+      }
+      case CorrelationResult.Success __ -> executePostProcess(message, emailListenerConfig);


connectors/email/src/test/java/io/camunda/connector/email/client/jakarta/JakartaUtilsTest.java

+    // Given
+    SmtpConfig smtpConfig = new SmtpConfig("smtp.example.com", 587, CryptographicProtocol.TLS);
+    Smtp smtp = new Smtp(null, smtpConfig);
+    Authentication auth = mock(SimpleAuthentication.class);


connectors/email/src/test/java/io/camunda/connector/email/client/jakarta/JakartaUtilsTest.java

+    // Given
+    SmtpConfig smtpConfig = new SmtpConfig("smtp.example.com", 25, CryptographicProtocol.NONE);
+    Smtp smtp = new Smtp(null, smtpConfig);
+    Authentication auth = mock(SimpleAuthentication.class);


connectors/email/src/test/java/io/camunda/connector/email/client/jakarta/JakartaUtilsTest.java

+  void testCreatePropertiesWithPop3AndNoSecurity() {
+    // Given
+    Pop3Config pop3Config = new Pop3Config("pop3.example.com", 110, CryptographicProtocol.NONE);
+    Authentication auth = mock(SimpleAuthentication.class);


connectors/email/src/test/java/io/camunda/connector/email/client/jakarta/JakartaUtilsTest.java

+    // Given
+    Pop3Config pop3Config = new Pop3Config("pop3.example.com", 995, CryptographicProtocol.TLS);
+
+    Authentication auth = mock(SimpleAuthentication.class);


connectors/email/src/test/java/io/camunda/connector/email/client/jakarta/JakartaUtilsTest.java

+    // Given
+    ImapConfig imapConfig = new ImapConfig("imap.example.com", 143, CryptographicProtocol.NONE);
+
+    Authentication auth = mock(SimpleAuthentication.class);


connectors/email/src/test/java/io/camunda/connector/email/client/jakarta/JakartaUtilsTest.java

+    // Given
+    ImapConfig imapConfig = new ImapConfig("imap.tls-example.com", 993, CryptographicProtocol.TLS);
+    // When
+    Authentication auth = mock(SimpleAuthentication.class);


connectors/email/src/test/java/io/camunda/connector/email/client/jakarta/JakartaUtilsTest.java

+    ImapConfig imapConfig = new ImapConfig("imap.ssl-example.com", 993, CryptographicProtocol.SSL);
+
+    // When
+    Authentication auth = mock(SimpleAuthentication.class);


mathias-vandaele · 2024-09-23T12:55:46Z

@johnBgood Would you be ok for final approvement ?

johnBgood

Nothing blocking, just a few comments 👍

johnBgood · 2024-09-23T13:00:48Z

connectors/email/src/main/java/io/camunda/connector/email/authentication/Authentication.java

+    name = "type",
+    defaultValue = "simple",
+    description = "Specify the Email authentication strategy.")
+public sealed interface Authentication permits SimpleAuthentication {}


Do we still need this interface if there's only a SimpleAuth ? It can be good for the future if we plan to support other types, so I'm just flagging it in case

johnBgood · 2024-09-23T13:02:11Z

connectors/email/src/main/java/io/camunda/connector/email/client/EmailListener.java

+import io.camunda.connector.api.inbound.InboundConnectorContext;
+
+public interface EmailListener {
+  void startListener(InboundConnectorContext context);


maybe start/stop would be sufficient

johnBgood · 2024-09-23T13:04:06Z

...mail/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailActionExecutor.java

+    Action action = protocol.getProtocolAction();
+    Session session = jakartaUtils.createSession(protocol.getConfiguration());
+    return switch (action) {
+      case SmtpSendEmail smtpSendEmail -> smtpSendEmail(smtpSendEmail, authentication, session);


I'd split the methods called here into 3 different classes (imap, smtp, pop3) to avoid adding cognitive load to this class.
This would also enable testability for each of these methods.

johnBgood · 2024-09-23T13:04:49Z

...mail/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailActionExecutor.java

+import jakarta.mail.search.*;
+import java.util.*;
+
+public class JakartaEmailActionExecutor implements EmailActionExecutor {


This class is very important, it should have a Logger and proper logging in place, so that we can debug it some day

johnBgood · 2024-09-23T13:07:48Z

connectors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaUtils.java

+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class JakartaUtils {


This Utils class is huge, not sure if we can/want to refactor it a bit but I feel it touches upon too many things

mathias-vandaele self-assigned this Aug 20, 2024

mathias-vandaele linked an issue Aug 20, 2024 that may be closed by this pull request

[email connector] SMTP outbound connector #3108

Closed

github-advanced-security bot found potential problems Aug 22, 2024

View reviewed changes

github-advanced-security bot found potential problems Aug 23, 2024

View reviewed changes

github-advanced-security bot found potential problems Aug 26, 2024

View reviewed changes

connectors/email/src/main/java/io/camunda/connector/email/core/JakartaExecutor.java Fixed Show fixed Hide fixed

connectors/email/src/main/java/io/camunda/connector/email/core/JakartaExecutor.java Fixed Show fixed Hide fixed

mathias-vandaele force-pushed the 3108-email-connector-smtp-outbound-connector branch from 2b08f94 to baa9aa6 Compare August 26, 2024 11:37

github-advanced-security bot found potential problems Aug 26, 2024

View reviewed changes

github-advanced-security bot found potential problems Aug 27, 2024

View reviewed changes

...ectors/email/src/main/java/io/camunda/connector/email/core/jakarta/JakartaEmailListener.java Fixed Show fixed Hide fixed

johnBgood reviewed Aug 28, 2024

View reviewed changes

bundle/default-bundle/src/test/resources/application.properties Outdated Show resolved Hide resolved

...ectors/email/src/main/java/io/camunda/connector/email/core/jakarta/JakartaEmailListener.java Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems Aug 28, 2024

View reviewed changes

github-advanced-security bot found potential problems Aug 29, 2024

View reviewed changes

...ctors/email/src/main/java/io/camunda/connector/email/core/jakarta/JakartaActionExecutor.java Fixed Show fixed Hide fixed

mathias-vandaele force-pushed the 3108-email-connector-smtp-outbound-connector branch from c0f1f60 to f82fe87 Compare August 30, 2024 08:20

mathias-vandaele changed the title ~~3108 email connector smtp outbound connector~~ feat(email connector): implement new email connector, Outbound SMTP, IMAP and POP3, Inbound IMAP Aug 30, 2024

mathias-vandaele marked this pull request as ready for review August 30, 2024 15:17

mathias-vandaele requested a review from a team as a code owner August 30, 2024 15:17

johnBgood reviewed Sep 5, 2024

View reviewed changes

mathias-vandaele added 13 commits September 9, 2024 15:07

feat(email connector): first checkpoint on email connector

608ec48

feat(email connector): first checkpoint on email connector 2

50e7ebb

feat(email connector): first checkpoint on email connector 3

e62c1a9

feat(email connector): first checkpoint on email connector 4

f8aef94

feat(email connector): first checkpoint on email connector 5

531d50b

feat(email connector): first checkpoint on email connector 5

dfd865b

feat(email connector): first checkpoint on email connector 6

2572bf6

feat(email connector): first checkpoint on email connector 7

8ad463d

feat(email connector): put everything inside a directory

83240b4

feat(email connector): checkpoint on email connector 8

e6383e2

feat(email connector): checkpoint on email connector 9

4c389b8

feat(email connector): checkpoint on email connector 10

f93dc95

feat(email connector): checkpoint on email connector 11

9acc02f

sbuettner reviewed Sep 10, 2024

View reviewed changes

connectors/email/src/main/java/io/camunda/connector/email/client/jakarta/Email.java Outdated Show resolved Hide resolved

sbuettner reviewed Sep 10, 2024

View reviewed changes

connectors/email/src/main/java/io/camunda/connector/email/client/jakarta/Email.java Outdated Show resolved Hide resolved

sbuettner reviewed Sep 10, 2024

View reviewed changes

connectors/email/src/main/java/io/camunda/connector/email/client/jakarta/Email.java Outdated Show resolved Hide resolved

sbuettner reviewed Sep 10, 2024

View reviewed changes

connectors/email/src/main/java/io/camunda/connector/email/client/jakarta/Email.java Outdated Show resolved Hide resolved

sbuettner reviewed Sep 10, 2024

View reviewed changes

connectors/email/src/main/java/io/camunda/connector/email/client/ActionExecutor.java Outdated Show resolved Hide resolved

sbuettner reviewed Sep 10, 2024

View reviewed changes

...tors/email/src/main/java/io/camunda/connector/email/client/jakarta/JakartaEmailListener.java Outdated Show resolved Hide resolved

sbuettner reviewed Sep 10, 2024

View reviewed changes

feat(email connector): applies correction after PR reviews

511b044

github-advanced-security bot found potential problems Sep 11, 2024

View reviewed changes

mathias-vandaele added 4 commits September 11, 2024 16:52

feat(email connector): modify the attachment management

74385e3

feat(email connector): inbound checkpoint

7a418b9

feat(email connector): inbound inbound refactor and labels refinment

5b392e1

feat(email connector): inbound inbound refactor and labels refinment 2

42954e9

github-advanced-security bot found potential problems Sep 16, 2024

View reviewed changes

feat(email connector): inbound inbound refactor and labels refinment 3

d57798c

github-advanced-security bot found potential problems Sep 16, 2024

View reviewed changes

johnBgood approved these changes Sep 23, 2024

View reviewed changes

mathias-vandaele added this to the 8.6.0 milestone Sep 23, 2024

mathias-vandaele added this pull request to the merge queue Sep 23, 2024

Merged via the queue into main with commit 5ab8ecb Sep 23, 2024
10 of 11 checks passed

mathias-vandaele deleted the 3108-email-connector-smtp-outbound-connector branch September 23, 2024 14:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(email connector): implement new email connector, Outbound SMTP, IMAP and POP3, Inbound IMAP #3132

feat(email connector): implement new email connector, Outbound SMTP, IMAP and POP3, Inbound IMAP #3132

mathias-vandaele commented Aug 20, 2024 •

edited

Loading

johnBgood left a comment

sbuettner Sep 10, 2024

mathias-vandaele Sep 10, 2024 •

edited

Loading

sbuettner Sep 10, 2024

sbuettner Sep 10, 2024

mathias-vandaele Sep 10, 2024

sbuettner Sep 10, 2024

mathias-vandaele Sep 11, 2024

sbuettner Sep 10, 2024

sbuettner Sep 10, 2024

sbuettner Sep 10, 2024

sbuettner Sep 10, 2024

sbuettner Sep 10, 2024

mathias-vandaele Sep 10, 2024 •

edited

Loading

sbuettner Sep 10, 2024

mathias-vandaele Sep 11, 2024

sbuettner commented Sep 10, 2024

mathias-vandaele commented Sep 23, 2024

johnBgood left a comment

johnBgood Sep 23, 2024

johnBgood Sep 23, 2024

johnBgood Sep 23, 2024

johnBgood Sep 23, 2024

johnBgood Sep 23, 2024

feat(email connector): implement new email connector, Outbound SMTP, IMAP and POP3, Inbound IMAP #3132

feat(email connector): implement new email connector, Outbound SMTP, IMAP and POP3, Inbound IMAP #3132

Conversation

mathias-vandaele commented Aug 20, 2024 • edited Loading

Description

Related issues

johnBgood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathias-vandaele Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathias-vandaele Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbuettner commented Sep 10, 2024

mathias-vandaele commented Sep 23, 2024

johnBgood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathias-vandaele commented Aug 20, 2024 •

edited

Loading

mathias-vandaele Sep 10, 2024 •

edited

Loading

mathias-vandaele Sep 10, 2024 •

edited

Loading