From 927c01e985c9ce9c3dfe4713373963bbca80caaf Mon Sep 17 00:00:00 2001 From: Mikel Alejo Date: Wed, 15 Nov 2023 14:01:36 +0100 Subject: [PATCH] RHCLOUD-29047 | feature: specify sender and default recipient (#2328) Co-authored-by: Gwenneg Lepage --- .../email/EmailCloudEventDataExtractor.java | 1 + .../connector/email/EmailRouteBuilder.java | 4 +- .../email/constants/ExchangeProperty.java | 16 +++++--- .../email/model/bop/SendEmailsRequest.java | 10 ++++- .../processors/bop/BOPRequestPreparer.java | 8 +++- .../EmailCloudEventDataExtractorTest.java | 5 +++ .../bop/BOPRequestPreparerTest.java | 6 +-- .../processors/email/EmailActorsResolver.java | 25 ++++++++++++ .../processors/email/EmailProcessor.java | 5 ++- .../email/EmailSubscriptionTypeProcessor.java | 7 +++- .../connector/dto/EmailNotification.java | 40 +++++++++++-------- .../email/EmailActorsResolverTest.java | 21 ++++++++++ .../processors/email/EmailProcessorTest.java | 9 +++++ 13 files changed, 123 insertions(+), 34 deletions(-) create mode 100644 engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailActorsResolver.java create mode 100644 engine/src/test/java/com/redhat/cloud/notifications/processors/email/EmailActorsResolverTest.java diff --git a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailCloudEventDataExtractor.java b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailCloudEventDataExtractor.java index ecdfcdc2df..bfeff003b4 100644 --- a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailCloudEventDataExtractor.java +++ b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailCloudEventDataExtractor.java @@ -52,5 +52,6 @@ public void extract(final Exchange exchange, final JsonObject cloudEventData) { exchange.setProperty(ExchangeProperty.SUBSCRIBERS, subscribers); exchange.setProperty(ExchangeProperty.UNSUBSCRIBERS, unsubscribers); exchange.setProperty(ExchangeProperty.EMAIL_RECIPIENTS, emails); + exchange.setProperty(ExchangeProperty.EMAIL_SENDER, cloudEventData.getString("email_sender")); } } diff --git a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilder.java b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilder.java index c9fe7b353f..b202cd123a 100644 --- a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilder.java +++ b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/EmailRouteBuilder.java @@ -399,8 +399,8 @@ public void configureRoute() throws Exception { private Predicate shouldSkipEmail() { return exchange -> exchange.getProperty(FILTERED_USERS, Set.class).isEmpty() && - (!emailConnectorConfig.isSkipBopUsersResolution() || - exchange.getProperty(EMAIL_RECIPIENTS, Set.class).isEmpty()); + (!emailConnectorConfig.isSkipBopUsersResolution() || + exchange.getProperty(EMAIL_RECIPIENTS, Set.class).isEmpty()); } /** diff --git a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/constants/ExchangeProperty.java b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/constants/ExchangeProperty.java index 99bbd9f602..eaed6da733 100644 --- a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/constants/ExchangeProperty.java +++ b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/constants/ExchangeProperty.java @@ -11,6 +11,16 @@ public class ExchangeProperty { * retrieval page. */ public static final String ELEMENTS_COUNT = "elements_count"; + /** + * Email recipients initially included in the payload received by notifications-engine. + * The subscriptions of these recipients are not checked. + * They are simply added to the list of recipients retrieved from notifications-recipients-resolver. + */ + public static final String EMAIL_RECIPIENTS = "email_recipients"; + /** + * Holds the email's sender that will be specified for sending the email. + */ + public static final String EMAIL_SENDER = "email_sender"; /** * Holds the filtered users. It is used in order to avoid the set of * cached users from being modified. @@ -60,10 +70,4 @@ public class ExchangeProperty { * notification through email. */ public static final String USERS = "users"; - /** - * Email recipients initially included in the payload received by notifications-engine. - * The subscriptions of these recipients are not checked. - * They are simply added to the list of recipients retrieved from notifications-recipients-resolver. - */ - public static final String EMAIL_RECIPIENTS = "email_recipients"; } diff --git a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/model/bop/SendEmailsRequest.java b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/model/bop/SendEmailsRequest.java index b1a60fd3db..cb31d2f06c 100644 --- a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/model/bop/SendEmailsRequest.java +++ b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/model/bop/SendEmailsRequest.java @@ -1,6 +1,7 @@ package com.redhat.cloud.notifications.connector.email.model.bop; import com.fasterxml.jackson.annotation.JsonAutoDetect; +import com.fasterxml.jackson.annotation.JsonInclude; import java.util.HashSet; import java.util.Set; @@ -8,12 +9,17 @@ import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY; @JsonAutoDetect(fieldVisibility = ANY) +@JsonInclude(JsonInclude.Include.NON_NULL) public class SendEmailsRequest { private final Set emails = new HashSet<>(); private final boolean skipUsersResolution = true; + private final String emailSender; + private final String defaultRecipient; - public void addEmail(Email email) { - emails.add(email); + public SendEmailsRequest(final Set emails, final String emailSender, final String defaultRecipient) { + this.emails.addAll(emails); + this.emailSender = emailSender; + this.defaultRecipient = defaultRecipient; } } diff --git a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/processors/bop/BOPRequestPreparer.java b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/processors/bop/BOPRequestPreparer.java index 37b72e9798..9227de2845 100644 --- a/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/processors/bop/BOPRequestPreparer.java +++ b/connector-email/src/main/java/com/redhat/cloud/notifications/connector/email/processors/bop/BOPRequestPreparer.java @@ -42,6 +42,7 @@ public void process(final Exchange exchange) { recipients = users.stream().map(User::getUsername).collect(toSet()); } + // Prepare the email to be sent. final Email email = new Email( subject, body, @@ -50,8 +51,11 @@ public void process(final Exchange exchange) { JsonObject bopBody; if (emailConnectorConfig.isSkipBopUsersResolution()) { - final SendEmailsRequest request = new SendEmailsRequest(); - request.addEmail(email); + final SendEmailsRequest request = new SendEmailsRequest( + Set.of(email), + exchange.getProperty(ExchangeProperty.EMAIL_SENDER, String.class), + exchange.getProperty(ExchangeProperty.EMAIL_SENDER, String.class) + ); bopBody = JsonObject.mapFrom(request); } else { final Emails emails = new Emails(); diff --git a/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/EmailCloudEventDataExtractorTest.java b/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/EmailCloudEventDataExtractorTest.java index 294bfab256..7dbd6aa1ee 100644 --- a/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/EmailCloudEventDataExtractorTest.java +++ b/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/EmailCloudEventDataExtractorTest.java @@ -14,6 +14,7 @@ import java.util.UUID; import static com.redhat.cloud.notifications.connector.email.constants.ExchangeProperty.EMAIL_RECIPIENTS; +import static com.redhat.cloud.notifications.connector.email.constants.ExchangeProperty.EMAIL_SENDER; import static com.redhat.cloud.notifications.connector.email.constants.ExchangeProperty.RECIPIENT_SETTINGS; import static com.redhat.cloud.notifications.connector.email.constants.ExchangeProperty.RENDERED_BODY; import static com.redhat.cloud.notifications.connector.email.constants.ExchangeProperty.RENDERED_SUBJECT; @@ -73,6 +74,8 @@ void testExtract() { final String emailBody = "fake email body"; final String emailSubject = "fake email subject"; + final String emailSender = "\"Red Hat Insights\" noreply@redhat.com"; + // Prepare the JSON object. final JsonObject payload = new JsonObject(); payload.put("recipient_settings", recipientSettingsList); @@ -80,6 +83,7 @@ void testExtract() { payload.put("unsubscribers", unsubscribers); payload.put("email_body", emailBody); payload.put("email_subject", emailSubject); + payload.put("email_sender", emailSender); payload.put("subscribed_by_default", true); final Exchange exchange = this.createExchangeWithBody(""); @@ -96,6 +100,7 @@ void testExtract() { assertEquals(subscribers, exchange.getProperty(SUBSCRIBERS, Set.class)); assertEquals(unsubscribers, exchange.getProperty(UNSUBSCRIBERS, Set.class)); assertEquals(Set.of("foo@bar.com", "bar@foo.com", "john@doe.com"), exchange.getProperty(EMAIL_RECIPIENTS, Set.class)); + assertEquals(emailSender, exchange.getProperty(EMAIL_SENDER, String.class)); assertTrue(exchange.getProperty(SUBSCRIBED_BY_DEFAULT, boolean.class)); } } diff --git a/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/processors/bop/BOPRequestPreparerTest.java b/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/processors/bop/BOPRequestPreparerTest.java index 097cc032e5..7aef480953 100644 --- a/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/processors/bop/BOPRequestPreparerTest.java +++ b/connector-email/src/test/java/com/redhat/cloud/notifications/connector/email/processors/bop/BOPRequestPreparerTest.java @@ -77,13 +77,13 @@ void testProcess(boolean skipBopUsersResolution) { if (skipBopUsersResolution) { assertEquals(5, actualEmail.getJsonArray("bccList").size()); assertTrue(Set.of("a-email", "b-email", "c-email", "foo@bar.com", "bar@foo.com").stream() - .allMatch(bcc -> actualEmail.getJsonArray("bccList").contains(bcc))); + .allMatch(bcc -> actualEmail.getJsonArray("bccList").contains(bcc))); } else { assertEquals(3, actualEmail.getJsonArray("bccList").size()); assertTrue(Set.of("a", "b", "c").stream() - .allMatch(bcc -> actualEmail.getJsonArray("bccList").contains(bcc))); + .allMatch(bcc -> actualEmail.getJsonArray("bccList").contains(bcc))); assertTrue(Set.of("foo@bar.com", "bar@foo.com").stream() - .noneMatch(bcc -> actualEmail.getJsonArray("bccList").contains(bcc))); + .noneMatch(bcc -> actualEmail.getJsonArray("bccList").contains(bcc))); } assertEquals("html", actualEmail.getString("bodyType")); if (skipBopUsersResolution) { diff --git a/engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailActorsResolver.java b/engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailActorsResolver.java new file mode 100644 index 0000000000..9834ba92f8 --- /dev/null +++ b/engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailActorsResolver.java @@ -0,0 +1,25 @@ +package com.redhat.cloud.notifications.processors.email; + +import com.redhat.cloud.notifications.models.Event; +import jakarta.enterprise.context.ApplicationScoped; + +@ApplicationScoped +public class EmailActorsResolver { + /** + * Standard "Red Hat Insights" sender that the vast majority of the + * ConsoleDot applications will use. + */ + public static final String RH_INSIGHTS_SENDER = "\"Red Hat Insights\" noreply@redhat.com"; + + /** + * Determines which sender should be set in the email from the given event. + * When sending emails we will use the sender for both the sender itself + * and the default recipient —the one that appears in the "to" field—. + * @param event the event to determine the sender and the default + * recipients from. + * @return the sender that should be used for the given event. + */ + public String getEmailSender(final Event event) { + return RH_INSIGHTS_SENDER; + } +} diff --git a/engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailProcessor.java b/engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailProcessor.java index e3f6a97bbf..2431e42885 100644 --- a/engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailProcessor.java +++ b/engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailProcessor.java @@ -33,6 +33,9 @@ public class EmailProcessor extends SystemEndpointTypeProcessor { @Inject EndpointRepository endpointRepository; + @Inject + EmailActorsResolver emailActorsResolver; + @Inject EmailSubscriptionTypeProcessor emailSubscriptionTypeProcessor; @@ -47,7 +50,6 @@ public class EmailProcessor extends SystemEndpointTypeProcessor { @Override public void process(final Event event, final List endpoints) { - // Generate an aggregation if the event supports it. this.emailSubscriptionTypeProcessor.generateAggregationWhereDue(event); @@ -101,6 +103,7 @@ public void process(final Event event, final List endpoints) { final EmailNotification emailNotification = new EmailNotification( body, subject, + this.emailActorsResolver.getEmailSender(event), event.getOrgId(), recipientSettings, subscribers, diff --git a/engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailSubscriptionTypeProcessor.java b/engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailSubscriptionTypeProcessor.java index 3a946e400b..ba9d87c73c 100644 --- a/engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailSubscriptionTypeProcessor.java +++ b/engine/src/main/java/com/redhat/cloud/notifications/processors/email/EmailSubscriptionTypeProcessor.java @@ -76,6 +76,9 @@ public class EmailSubscriptionTypeProcessor extends SystemEndpointTypeProcessor @Inject BaseTransformer baseTransformer; + @Inject + EmailActorsResolver emailActorsResolver; + @Inject EmailSender emailSender; @@ -325,7 +328,9 @@ private void processAggregateEmailsByAggregationKey(AggregationCommand aggregati // Prepare all the data to be sent to the connector. final EmailNotification emailNotification = new EmailNotification( - bodyStr, subjectStr, + bodyStr, + subjectStr, + this.emailActorsResolver.getEmailSender(event), event.getOrgId(), recipientSettings, /* diff --git a/engine/src/main/java/com/redhat/cloud/notifications/processors/email/connector/dto/EmailNotification.java b/engine/src/main/java/com/redhat/cloud/notifications/processors/email/connector/dto/EmailNotification.java index b569e4c72a..ded000bbdf 100644 --- a/engine/src/main/java/com/redhat/cloud/notifications/processors/email/connector/dto/EmailNotification.java +++ b/engine/src/main/java/com/redhat/cloud/notifications/processors/email/connector/dto/EmailNotification.java @@ -6,23 +6,29 @@ /** * Represents the data structure that the email connector is expecting. - * @param emailBody the rendered body of the email to be sent. - * @param emailSubject the rendered subject of the email to be sent. - * @param orgId the organization ID associated with the triggered - * event. - * @param recipientSettings the collection of recipient settings extracted from - * both the event and the related endpoints to the - * event. - * @param subscribers the list of usernames who subscribed to the event type. - * @param unsubscribers the list of usernames who unsubscribed from the event type. - * @param subscribedByDefault true if the event type is subscribed by default. + * @param emailBody the rendered body of the email to be sent. + * @param emailSubject the rendered subject of the email to be sent. + * @param emailSender the sender that will appear in the email when + * the user receives it. + * @param orgId the organization ID associated with the + * triggered event. + * @param recipientSettings the collection of recipient settings extracted + * from both the event and the related endpoints + * to the event. + * @param subscribers the list of usernames who subscribed to the + * event type. + * @param unsubscribers the list of usernames who unsubscribed from the + * event type. + * @param subscribedByDefault true if the event type is subscribed by + * default. */ public record EmailNotification( - @JsonProperty("email_body") String emailBody, - @JsonProperty("email_subject") String emailSubject, - @JsonProperty("orgId") String orgId, - @JsonProperty("recipient_settings") Collection recipientSettings, - @JsonProperty("subscribers") Collection subscribers, - @JsonProperty("unsubscribers") Collection unsubscribers, - @JsonProperty("subscribed_by_default") boolean subscribedByDefault + @JsonProperty("email_body") String emailBody, + @JsonProperty("email_subject") String emailSubject, + @JsonProperty("email_sender") String emailSender, + @JsonProperty("orgId") String orgId, + @JsonProperty("recipient_settings") Collection recipientSettings, + @JsonProperty("subscribers") Collection subscribers, + @JsonProperty("unsubscribers") Collection unsubscribers, + @JsonProperty("subscribed_by_default") boolean subscribedByDefault ) { } diff --git a/engine/src/test/java/com/redhat/cloud/notifications/processors/email/EmailActorsResolverTest.java b/engine/src/test/java/com/redhat/cloud/notifications/processors/email/EmailActorsResolverTest.java new file mode 100644 index 0000000000..633f6dc115 --- /dev/null +++ b/engine/src/test/java/com/redhat/cloud/notifications/processors/email/EmailActorsResolverTest.java @@ -0,0 +1,21 @@ +package com.redhat.cloud.notifications.processors.email; + +import com.redhat.cloud.notifications.models.Event; +import io.quarkus.test.junit.QuarkusTest; +import jakarta.inject.Inject; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +@QuarkusTest +public class EmailActorsResolverTest { + @Inject + EmailActorsResolver emailActorsResolver; + + /** + * Tests that the default "RH Insights" sender is returned for every event. + */ + @Test + void testGetEmailSender() { + Assertions.assertEquals(EmailActorsResolver.RH_INSIGHTS_SENDER, this.emailActorsResolver.getEmailSender(new Event()), "unexpected email sender returned from the function under test"); + } +} diff --git a/engine/src/test/java/com/redhat/cloud/notifications/processors/email/EmailProcessorTest.java b/engine/src/test/java/com/redhat/cloud/notifications/processors/email/EmailProcessorTest.java index 9ab1cb7e3e..8fac71666f 100644 --- a/engine/src/test/java/com/redhat/cloud/notifications/processors/email/EmailProcessorTest.java +++ b/engine/src/test/java/com/redhat/cloud/notifications/processors/email/EmailProcessorTest.java @@ -44,6 +44,9 @@ public class EmailProcessorTest { @InjectMock ConnectorSender connectorSender; + @InjectMock + EmailActorsResolver emailActorsResolver; + @Inject EmailProcessor emailProcessor; @@ -340,6 +343,10 @@ void testSuccess() { endpoint.setId(UUID.randomUUID()); Mockito.when(this.endpointRepository.getOrCreateDefaultSystemSubscription(event.getAccountId(), event.getOrgId(), EndpointType.EMAIL_SUBSCRIPTION)).thenReturn(endpoint); + // Mock the sender and the default recipients of the email + final String stubbedSender = "Red Hat Insights noreply@redhat.com"; + Mockito.when(this.emailActorsResolver.getEmailSender(Mockito.any())).thenReturn(stubbedSender); + // Call the processor under test. this.emailProcessor.process(event, endpoints); @@ -368,6 +375,7 @@ void testSuccess() { final String resultEmailBody = payload.getString("email_body"); final String resultEmailSubject = payload.getString("email_subject"); final String resultOrgId = payload.getString("orgId"); + final String resultEmailSender = payload.getString("email_sender"); final Set resultSubscribers = payload.getJsonArray("subscribers").stream().map(String.class::cast).collect(toSet()); final Set resultRecipientSettings = payload.getJsonArray("recipient_settings") .stream() @@ -386,6 +394,7 @@ void testSuccess() { Assertions.assertEquals(stubbedRenderedBody, resultEmailBody, "the rendered email's body from the email notification does not match the stubbed email body"); Assertions.assertEquals(stubbedRenderedSubject, resultEmailSubject, "the rendered email's subject from the email notification does not match the stubbed email subject"); + Assertions.assertEquals(stubbedSender, resultEmailSender, "the rendered email's sender from the email notification does not match the stubbed sender"); Assertions.assertEquals(event.getOrgId(), resultOrgId, "the organization ID from the email notification does not match the one set in the stubbed event"); Assertions.assertEquals(Set.copyOf(subscribers), resultSubscribers, "the subscribers set in the email notification do not match the stubbed ones");