Skip to content

Commit

Permalink
Replace Mockito mocked EmailSender in tests
Browse files Browse the repository at this point in the history
As part of investigating removing the mocking from the tests (for #2534), this switches from using a `mock(EmailSender)` to creating a bean under test which tracks the calls made to it in a simple list.

If the `replace.mailjet.factory` property is set in a test then the standard `MailJetFactory` is replaced by our factory.

This generates EmailSender instances (for HTML and TEXT) which track the calls to their methods in a list of strings.

We can also remove the `setEmailSender` methods which we had to use to force the mock bean into the running services.

I believe this has uncovered a bug in the Guild Service whereby too many emails are being sent.
(see `com.objectcomputing.checkins.services.guild.GuildControllerTest#testEmailSentToGuildLeadWhenGuildMembersAdded`)
I would have expected a single email, but we are sending 2... 🤔
  • Loading branch information
timyates committed Jul 12, 2024
1 parent 8cd58a1 commit 5ed3e5e
Show file tree
Hide file tree
Showing 13 changed files with 494 additions and 313 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ public class EmailServicesImpl implements EmailServices {

private static final Logger LOG = LoggerFactory.getLogger(EmailServicesImpl.class);

private EmailSender htmlEmailSender;
private EmailSender textEmailSender;
private final EmailSender htmlEmailSender;
private final EmailSender textEmailSender;
private final CurrentUserServices currentUserServices;
private final MemberProfileRepository memberProfileRepository;
private final EmailRepository emailRepository;
Expand All @@ -43,14 +43,6 @@ public EmailServicesImpl(@Named(MailJetFactory.HTML_FORMAT) EmailSender htmlEmai
this.emailRepository = emailRepository;
}

public void setHtmlEmailSender(EmailSender emailSender) {
this.htmlEmailSender = emailSender;
}

public void setTextEmailSender(EmailSender emailSender) {
this.textEmailSender = emailSender;
}

@Override
public List<Email> sendAndSaveEmail(String subject, String content, boolean html, String... recipients) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class FeedbackRequestServicesImpl implements FeedbackRequestServices {
private final CurrentUserServices currentUserServices;
private final MemberProfileServices memberProfileServices;
private final ReviewPeriodRepository reviewPeriodRepository;
private EmailSender emailSender;
private final EmailSender emailSender;
private final String notificationSubject;
private final String webURL;

Expand All @@ -62,10 +62,6 @@ public FeedbackRequestServicesImpl(FeedbackRequestRepository feedbackReqReposito
this.webURL = checkInsConfiguration.getWebAddress();
}

public void setEmailSender(EmailSender emailSender) {
this.emailSender = emailSender;
}

private void validateMembers(FeedbackRequest feedbackRequest) {
try {
memberProfileServices.getById(feedbackRequest.getCreatorId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class GuildServicesImpl implements GuildServices {
private final CurrentUserServices currentUserServices;
private final MemberProfileServices memberProfileServices;
private final GuildMemberServices guildMemberServices;
private EmailSender emailSender;
private final EmailSender emailSender;
private final Environment environment;
private final String webAddress;

Expand All @@ -70,10 +70,6 @@ public GuildServicesImpl(GuildRepository guildsRepo,
this.environment = environment;
}

public void setEmailSender (EmailSender emailSender){
this.emailSender = emailSender;
}

public boolean validateLink (String link ) {
try {
new URL(link).toURI();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package com.objectcomputing.checkins.services;

import com.objectcomputing.checkins.notifications.email.EmailSender;
import com.objectcomputing.checkins.notifications.email.MailJetFactory;
import io.micronaut.context.annotation.Factory;
import io.micronaut.context.annotation.Replaces;
import io.micronaut.context.annotation.Requires;
import io.micronaut.core.util.StringUtils;
import jakarta.inject.Named;
import jakarta.inject.Singleton;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

/**
* This class is a replacement for the MailJetFactory class. It provides a mock implementation of the EmailSender.
* The mock implementation records the events that occur when the sendEmail, sendEmailReceivesStatus, and setEmailFormat
* methods are called.
* <p>
* The mock implementation is used when the "replace.mailjet.factory" property is set to true.
* <p>
* This is used instead of Mockito so that it works when running nativeTest under GraalVM.
*/
@Factory
@Replaces(MailJetFactory.class)
@Requires(property = "replace.mailjet.factory", value = StringUtils.TRUE)
public class MailJetFactoryReplacement {

@Singleton
@Named(MailJetFactory.HTML_FORMAT)
@Replaces(value = EmailSender.class, named = MailJetFactory.HTML_FORMAT)
MockEmailSender getHtmlSender() {
return new MockEmailSender();
}

@Singleton
@Named(MailJetFactory.TEXT_FORMAT)
@Replaces(value = EmailSender.class, named = MailJetFactory.TEXT_FORMAT)
MockEmailSender getTextSender() {
return new MockEmailSender();
}

public static class MockEmailSender implements EmailSender {

public final List<List<String>> events = new ArrayList<>();
private RuntimeException exception;

static String nullSafe(String s) {
return s == null ? "null" : s;
}

public void reset() {
exception = null;
events.clear();
}

public void setException(RuntimeException exception) {
this.exception = exception;
}

private void maybeThrow() {
if (exception != null) {
throw exception;
}
}

@Override
public void sendEmail(String fromName, String fromAddress, String subject, String content, String... recipients) {
events.add(List.of(
"SEND_EMAIL",
nullSafe(fromName),
nullSafe(fromAddress),
nullSafe(subject),
nullSafe(content),
Arrays.stream(recipients).map(MockEmailSender::nullSafe).collect(Collectors.joining(","))
));
maybeThrow();
}

@Override
public boolean sendEmailReceivesStatus(String fromName, String fromAddress, String subject, String content, String... recipients) {
events.add(List.of(
"SEND_EMAIL_RECEIVES_STATUS",
nullSafe(fromName),
nullSafe(fromAddress),
nullSafe(subject),
nullSafe(content),
Arrays.stream(recipients).map(MockEmailSender::nullSafe).collect(Collectors.joining(","))
));
maybeThrow();
return true;
}

@Override
public void setEmailFormat(String format) {
events.add(List.of(
"SET_EMAIL_FORMAT",
nullSafe(format)
));
maybeThrow();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,54 +1,55 @@
package com.objectcomputing.checkins.services.email;

import com.objectcomputing.checkins.notifications.email.EmailSender;
import com.objectcomputing.checkins.notifications.email.MailJetFactory;
import com.objectcomputing.checkins.services.MailJetFactoryReplacement;
import com.objectcomputing.checkins.services.TestContainersSuite;
import com.objectcomputing.checkins.services.fixture.MemberProfileFixture;
import com.objectcomputing.checkins.services.fixture.RoleFixture;
import com.objectcomputing.checkins.services.memberprofile.MemberProfile;
import com.objectcomputing.checkins.services.role.RoleType;
import io.micronaut.context.annotation.Property;
import io.micronaut.core.type.Argument;
import io.micronaut.core.util.StringUtils;
import io.micronaut.http.HttpRequest;
import io.micronaut.http.HttpResponse;
import io.micronaut.http.HttpStatus;
import io.micronaut.http.client.HttpClient;
import io.micronaut.http.client.annotation.Client;
import io.micronaut.http.client.exceptions.HttpClientResponseException;
import jakarta.inject.Inject;
import jakarta.inject.Named;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;

import jakarta.inject.Inject;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static com.objectcomputing.checkins.services.validate.PermissionsValidation.NOT_AUTHORIZED_MSG;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

@Property(name = "replace.mailjet.factory", value = StringUtils.TRUE)
class EmailControllerTest extends TestContainersSuite implements MemberProfileFixture, RoleFixture {

@Inject
@Client("/services/email")
private HttpClient client;

@Mock
private final EmailSender htmlEmailSender = mock(EmailSender.class);

@Mock
private final EmailSender textEmailSender = mock(EmailSender.class);
@Inject
@Named(MailJetFactory.HTML_FORMAT)
private MailJetFactoryReplacement.MockEmailSender htmlEmailSender;

@Inject
private EmailServicesImpl emailServicesImpl;
@Named(MailJetFactory.TEXT_FORMAT)
private MailJetFactoryReplacement.MockEmailSender textEmailSender;

@BeforeEach
void resetMocks() {
Mockito.reset(htmlEmailSender);
Mockito.reset(textEmailSender);
emailServicesImpl.setHtmlEmailSender(htmlEmailSender);
emailServicesImpl.setTextEmailSender(textEmailSender);
htmlEmailSender.reset();
textEmailSender.reset();
}

@Test
Expand Down Expand Up @@ -77,8 +78,6 @@ void testSendAndSaveHtmlEmail() {
email.put("html", true);
email.put("recipients", List.of(recipient1.getWorkEmail(), recipient2.getWorkEmail()));

when(htmlEmailSender.sendEmailReceivesStatus(anyString(), anyString(), anyString(), anyString(), anyString(), anyString())).thenReturn(true);

final HttpRequest<?> request = HttpRequest.POST("", email)
.basicAuth(admin.getWorkEmail(), RoleType.Constants.ADMIN_ROLE);
final HttpResponse<List<Email>> response = client.toBlocking()
Expand All @@ -102,8 +101,12 @@ void testSendAndSaveHtmlEmail() {
assertEquals(recipient2.getId(), secondEmailRes.getRecipient());
assertTrue(secondEmailRes.getTransmissionDate().isAfter(secondEmailRes.getSendDate()));

verify(htmlEmailSender).sendEmailReceivesStatus(admin.getFirstName()+" "+admin.getLastName(), admin.getWorkEmail(),"Email Subject", "<p>Email content</p>", recipient1.getWorkEmail(), recipient2.getWorkEmail());
verify(textEmailSender, never()).sendEmailReceivesStatus(anyString(), anyString(), anyString(), anyString(), anyString(), anyString());
assertEquals(1, htmlEmailSender.events.size());
assertEquals(
List.of("SEND_EMAIL_RECEIVES_STATUS", admin.getFirstName()+" "+admin.getLastName(), admin.getWorkEmail(),"Email Subject", "<p>Email content</p>", recipient1.getWorkEmail() + "," + recipient2.getWorkEmail()),
htmlEmailSender.events.getFirst()
);
assertTrue(textEmailSender.events.isEmpty());
}

@Test
Expand All @@ -119,8 +122,6 @@ void testSendAndSaveTextEmail() {
email.put("html", false);
email.put("recipients", List.of(recipient1.getWorkEmail(), recipient2.getWorkEmail()));

when(textEmailSender.sendEmailReceivesStatus(anyString(), anyString(), anyString(), anyString(), anyString(), anyString())).thenReturn(true);

final HttpRequest<?> request = HttpRequest.POST("", email)
.basicAuth(admin.getWorkEmail(), RoleType.Constants.ADMIN_ROLE);
final HttpResponse<List<Email>> response = client.toBlocking()
Expand All @@ -144,8 +145,12 @@ void testSendAndSaveTextEmail() {
assertEquals(recipient2.getId(), secondEmailRes.getRecipient());
assertTrue(secondEmailRes.getTransmissionDate().isAfter(secondEmailRes.getSendDate()));

verify(textEmailSender).sendEmailReceivesStatus(admin.getFirstName()+" "+admin.getLastName(), admin.getWorkEmail(), "Email Subject", "Email content", recipient1.getWorkEmail(), recipient2.getWorkEmail());
verify(htmlEmailSender, never()).sendEmailReceivesStatus(anyString(), anyString(), anyString(), anyString(), anyString(), anyString());
assertEquals(1, textEmailSender.events.size());
assertEquals(
List.of("SEND_EMAIL_RECEIVES_STATUS", admin.getFirstName()+" "+admin.getLastName(), admin.getWorkEmail(),"Email Subject", "Email content", recipient1.getWorkEmail() + "," + recipient2.getWorkEmail()),
textEmailSender.events.getFirst()
);
assertTrue(htmlEmailSender.events.isEmpty());
}

@Test
Expand All @@ -169,6 +174,4 @@ void testSendAndSaveEmailUnauthorized() {
assertEquals(HttpStatus.FORBIDDEN, responseException.getStatus());
assertEquals(NOT_AUTHORIZED_MSG, responseException.getMessage());
}


}
Loading

0 comments on commit 5ed3e5e

Please sign in to comment.