--wip-- review

gravitee-io · Mar 27, 2024 · f47496a · f47496a
1 parent 922d1fb
commit f47496a
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 34 deletions.
diff --git a/...m-rest-api-service/src/main/java/io/gravitee/rest/api/service/EmailRecipientsService.java b/...m-rest-api-service/src/main/java/io/gravitee/rest/api/service/EmailRecipientsService.java
@@ -16,24 +16,26 @@
 package io.gravitee.rest.api.service;
 
 import io.gravitee.rest.api.service.common.ExecutionContext;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 public interface EmailRecipientsService {
     /**
      * Process a list of templated recipients to extract it as a list of unique email addresses
-     * @param templateData is the dateset used to process emails.
+     *
      * @param templatedRecipientsEmail a list of strings representing templated email. Each string can be itself a literal list of recipients separated by ' ' (whitespace) ',' or ';'. If an element contains '$', then it will be processed against templateData parameter with Freemarker processor
+     * @param templateData             is the dateset used to process emails.
      * @return a set of not empty emails.
      */
-    Set<String> processTemplatedRecipients(Map<String, Object> templateData, List<String> templatedRecipientsEmail);
+    Set<String> processTemplatedRecipients(Collection<String> templatedRecipientsEmail, Map<String, Object> templateData);
 
     /**
      * Checks that each email has an opted-in user attached to it. If it is not the case, then the email is excluded from the result.
      * @param executionContext
      * @param recipientsEmail is the list of recipients to verify if attached user has opted-in. This method assumes emails are valids.
      * @return a set of emails attached to an opted-in user.
      */
-    Set<String> filterRegisteredUser(ExecutionContext executionContext, List<String> recipientsEmail);
+    Set<String> filterRegisteredUser(ExecutionContext executionContext, Collection<String> recipientsEmail);
 }
diff --git a/...i-service/src/main/java/io/gravitee/rest/api/service/impl/EmailRecipientsServiceImpl.java b/...i-service/src/main/java/io/gravitee/rest/api/service/impl/EmailRecipientsServiceImpl.java
@@ -24,6 +24,7 @@
 import io.gravitee.rest.api.service.UserService;
 import io.gravitee.rest.api.service.common.ExecutionContext;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -45,7 +46,7 @@ public class EmailRecipientsServiceImpl implements EmailRecipientsService {
     private final UserService userService;
 
     @Override
-    public Set<String> processTemplatedRecipients(final Map<String, Object> templateData, List<String> templatedRecipientsEmail) {
+    public Set<String> processTemplatedRecipients(Collection<String> templatedRecipientsEmail, final Map<String, Object> templateData) {
         return templatedRecipientsEmail
             .stream()
             .flatMap(splittableRecipientsStr ->
@@ -64,19 +65,17 @@ public Set<String> processTemplatedRecipients(final Map<String, Object> template
                         return Optional.of(recipient);
                     })
             )
-            .filter(Optional::isPresent)
-            .map(Optional::get)
+            .flatMap(Optional::stream)
             .filter(not(StringUtils::isEmpty))
             .collect(Collectors.toSet());
     }
 
     @Override
-    public Set<String> filterRegisteredUser(ExecutionContext executionContext, List<String> recipientsEmail) {
+    public Set<String> filterRegisteredUser(ExecutionContext executionContext, Collection<String> recipientsEmail) {
         return recipientsEmail
             .stream()
             .map(email -> userService.findByEmail(executionContext, email))
-            .filter(Optional::isPresent)
-            .map(Optional::get)
+            .flatMap(Optional::stream)
             .filter(UserEntity::optedIn)
             .map(UserEntity::getEmail)
             .collect(Collectors.toSet());

diff --git a/...rest-api-service/src/main/java/io/gravitee/rest/api/service/impl/NotifierServiceImpl.java b/...rest-api-service/src/main/java/io/gravitee/rest/api/service/impl/NotifierServiceImpl.java
@@ -58,7 +58,6 @@
 import java.util.stream.Collectors;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Lazy;
 import org.springframework.scheduling.annotation.Async;
 import org.springframework.stereotype.Component;
@@ -231,17 +230,14 @@ void triggerGenericNotifications(
                             recipients.addAll(emailAdditionalRecipients);
 
                             // extract emails from templated string (eg: ${api.primaryOwner.email})
-                            var processedRecipients = emailRecipientsService
-                                .processTemplatedRecipients(params, recipients)
-                                .stream()
-                                .toList();
+                            var processedRecipients = emailRecipientsService.processTemplatedRecipients(recipients, params);
                             // extract emails of opted-in users if trial instance
                             var validRecipients = parameterService.findAsBoolean(
                                     executionContext,
                                     Key.TRIAL_INSTANCE,
                                     ParameterReferenceType.SYSTEM
                                 )
-                                ? emailRecipientsService.filterRegisteredUser(executionContext, processedRecipients).stream().toList()
+                                ? emailRecipientsService.filterRegisteredUser(executionContext, processedRecipients)
                                 : processedRecipients;
 
                             emailNotifierService.trigger(executionContext, hook, params, validRecipients);

diff --git a/...pi-service/src/main/java/io/gravitee/rest/api/service/notifiers/EmailNotifierService.java b/...pi-service/src/main/java/io/gravitee/rest/api/service/notifiers/EmailNotifierService.java
@@ -18,6 +18,7 @@
 import io.gravitee.repository.management.model.GenericNotificationConfig;
 import io.gravitee.rest.api.service.common.ExecutionContext;
 import io.gravitee.rest.api.service.notification.Hook;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 
@@ -26,5 +27,5 @@
  * @author GraviteeSource Team
  */
 public interface EmailNotifierService {
-    void trigger(ExecutionContext executionContext, final Hook hook, final Map<String, Object> templateData, List<String> recipients);
+    void trigger(ExecutionContext executionContext, final Hook hook, final Map<String, Object> templateData, Collection<String> recipients);
 }
diff --git a/...e/src/main/java/io/gravitee/rest/api/service/notifiers/impl/EmailNotifierServiceImpl.java b/...e/src/main/java/io/gravitee/rest/api/service/notifiers/impl/EmailNotifierServiceImpl.java
@@ -27,6 +27,7 @@
 import io.gravitee.rest.api.service.notification.Hook;
 import io.gravitee.rest.api.service.notifiers.EmailNotifierService;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -57,7 +58,7 @@ public void trigger(
         ExecutionContext executionContext,
         final Hook hook,
         final Map<String, Object> templateData,
-        List<String> recipients
+        Collection<String> recipients
     ) {
         var emailTemplate = getEmailTemplateOptional(hook);
         if (emailTemplate.isEmpty()) {

diff --git a/...rvice/src/test/java/io/gravitee/rest/api/service/impl/EmailRecipientsServiceImplTest.java b/...rvice/src/test/java/io/gravitee/rest/api/service/impl/EmailRecipientsServiceImplTest.java
@@ -70,19 +70,19 @@ void should_transform_one_email() {
                 ApiEntity.builder().primaryOwner(PrimaryOwnerEntity.builder().email("po@gravitee.io").build()).build()
             );
 
-            final Set<String> result = cut.processTemplatedRecipients(templateData, List.of("${api.primaryOwner.email}"));
+            final Set<String> result = cut.processTemplatedRecipients(List.of("${api.primaryOwner.email}"), templateData);
             assertThat(result).hasSize(1).first().isEqualTo("po@gravitee.io");
         }
 
         @Test
         void should_not_transform_one_email_when_not_templated() {
-            final Set<String> result = cut.processTemplatedRecipients(Map.of(), List.of("anEmail@gravitee.io"));
+            final Set<String> result = cut.processTemplatedRecipients(List.of("anEmail@gravitee.io"), Map.of());
             assertThat(result).hasSize(1).first().isEqualTo("anEmail@gravitee.io");
         }
 
         @Test
         void should_return_empty_collection() {
-            final Set<String> result = cut.processTemplatedRecipients(Map.of(), List.of(""));
+            final Set<String> result = cut.processTemplatedRecipients(List.of(""), Map.of());
             assertThat(result).isEmpty();
         }
 
@@ -93,7 +93,7 @@ void should_return_empty_collection_when_invalid_template() {
                 ApiEntity.builder().primaryOwner(PrimaryOwnerEntity.builder().email("po@gravitee.io").build()).build()
             );
 
-            final Set<String> result = cut.processTemplatedRecipients(templateData, List.of("${api.primaryOwner......invalid}"));
+            final Set<String> result = cut.processTemplatedRecipients(List.of("${api.primaryOwner......invalid}"), templateData);
             assertThat(result).isEmpty();
         }
 
@@ -104,7 +104,7 @@ void should_return_empty_collection_when_template_lead_to_unknown_property() {
                 ApiEntity.builder().primaryOwner(PrimaryOwnerEntity.builder().email("").build()).build()
             );
 
-            final Set<String> result = cut.processTemplatedRecipients(templateData, List.of("${api.primaryOwner.email}"));
+            final Set<String> result = cut.processTemplatedRecipients(List.of("${api.primaryOwner.email}"), templateData);
             assertThat(result).isEmpty();
         }
 
@@ -115,7 +115,7 @@ void should_be_able_to_parse_multiple_recipients(List<String> recipients, Set<St
                 "api",
                 ApiEntity.builder().primaryOwner(PrimaryOwnerEntity.builder().email("po@gravitee.io").build()).build()
             );
-            final Set<String> result = cut.processTemplatedRecipients(templateData, recipients);
+            final Set<String> result = cut.processTemplatedRecipients(recipients, templateData);
             assertThat(result).isEqualTo(expectedOutput);
         }
 

diff --git a/...-api-service/src/test/java/io/gravitee/rest/api/service/impl/NotifierServiceImplTest.java b/...-api-service/src/test/java/io/gravitee/rest/api/service/impl/NotifierServiceImplTest.java
@@ -34,13 +34,14 @@
 import io.gravitee.rest.api.model.parameters.Key;
 import io.gravitee.rest.api.model.parameters.ParameterReferenceType;
 import io.gravitee.rest.api.service.EmailRecipientsService;
-import io.gravitee.rest.api.service.NotifierService;
 import io.gravitee.rest.api.service.ParameterService;
 import io.gravitee.rest.api.service.PortalNotificationService;
 import io.gravitee.rest.api.service.common.ExecutionContext;
 import io.gravitee.rest.api.service.notification.ApiHook;
 import io.gravitee.rest.api.service.notifiers.EmailNotifierService;
 import io.gravitee.rest.api.service.notifiers.WebhookNotifierService;
+
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -130,7 +131,7 @@ void should_trigger_email_to_all_recipients() {
                             .build()
                     )
                 );
-            when(emailRecipientsService.processTemplatedRecipients(params, List.of("${(api.primaryOwner.email)!''};second.mail@gio.test")))
+            when(emailRecipientsService.processTemplatedRecipients(List.of("${(api.primaryOwner.email)!''};second.mail@gio.test"), params))
                 .thenReturn(Set.of("recipient@gio.test", "second.mail@gio.test"));
             when(parameterService.findAsBoolean(executionContext, Key.TRIAL_INSTANCE, ParameterReferenceType.SYSTEM)).thenReturn(false);
 
@@ -144,9 +145,9 @@ void should_trigger_email_to_all_recipients() {
             );
 
             verify(emailRecipientsService, never()).filterRegisteredUser(any(), any());
-            final ArgumentCaptor<List<String>> recipientsCaptor = ArgumentCaptor.forClass(List.class);
+            final ArgumentCaptor<Collection<String>> recipientsCaptor = ArgumentCaptor.forClass(Collection.class);
             verify(emailNotifierService).trigger(eq(executionContext), eq(ApiHook.API_STARTED), eq(params), recipientsCaptor.capture());
-            assertThat(recipientsCaptor.getValue()).containsExactly("recipient@gio.test", "second.mail@gio.test");
+            assertThat(recipientsCaptor.getValue()).containsExactlyInAnyOrder("recipient@gio.test", "second.mail@gio.test");
         }
 
         @Test
@@ -174,16 +175,16 @@ void should_trigger_email_to_opted_in_recipients() {
                 );
             when(
                 emailRecipientsService.processTemplatedRecipients(
-                    params,
-                    List.of("${(api.primaryOwner.email)!''};second.mail-not-opted-in@gio.test")
+                    List.of("${(api.primaryOwner.email)!''};second.mail-not-opted-in@gio.test"),
+                    params
                 )
             )
                 .thenReturn(Set.of("recipient@gio.test", "second.mail-not-opted-in@gio.test"));
             when(parameterService.findAsBoolean(executionContext, Key.TRIAL_INSTANCE, ParameterReferenceType.SYSTEM)).thenReturn(true);
             when(
                 emailRecipientsService.filterRegisteredUser(
                     executionContext,
-                    List.of("recipient@gio.test", "second.mail-not-opted-in@gio.test")
+                    Set.of("recipient@gio.test", "second.mail-not-opted-in@gio.test")
                 )
             )
                 .thenReturn(Set.of("recipient@gio.test"));
@@ -197,7 +198,7 @@ void should_trigger_email_to_opted_in_recipients() {
                 List.of()
             );
 
-            final ArgumentCaptor<List<String>> recipientsCaptor = ArgumentCaptor.forClass(List.class);
+            final ArgumentCaptor<Collection<String>> recipientsCaptor = ArgumentCaptor.forClass(Collection.class);
             verify(emailNotifierService).trigger(eq(executionContext), eq(ApiHook.API_STARTED), eq(params), recipientsCaptor.capture());
             assertThat(recipientsCaptor.getValue()).containsExactly("recipient@gio.test");
         }
@@ -227,10 +228,10 @@ void should_trigger_email_to_additional_opted_in_recipients() {
                 );
             when(
                 emailRecipientsService.processTemplatedRecipients(
-                    eq(params),
                     argThat(a ->
                         a.containsAll(List.of("${(api.primaryOwner.email)!''};second.mail-not-opted-in@gio.test", "additional@gio.test"))
-                    )
+                    ),
+                    eq(params)
                 )
             )
                 .thenReturn(Set.of("additional@gio.test", "recipient@gio.test", "second.mail-not-opted-in@gio.test"));
@@ -252,7 +253,7 @@ void should_trigger_email_to_additional_opted_in_recipients() {
                 List.of(new Recipient("default-email", "additional@gio.test"))
             );
 
-            final ArgumentCaptor<List<String>> recipientsCaptor = ArgumentCaptor.forClass(List.class);
+            final ArgumentCaptor<Collection<String>> recipientsCaptor = ArgumentCaptor.forClass(Collection.class);
             verify(emailNotifierService).trigger(eq(executionContext), eq(ApiHook.API_STARTED), eq(params), recipientsCaptor.capture());
             assertThat(recipientsCaptor.getValue()).containsExactlyInAnyOrder("additional@gio.test", "recipient@gio.test");
         }