From d94263ebe882595d02666bbd3ca4c4f393578b4c Mon Sep 17 00:00:00 2001 From: Ravi Khadiwala Date: Wed, 6 Dec 2023 17:14:02 -0600 Subject: [PATCH] update messagebird with delivery success/failure --- .../MessageBirdClassicConversionReporter.java | 110 ++++++++++++++++++ .../classic/MessageBirdVoiceSender.java | 4 +- ...sageBirdClassicConversionReporterTest.java | 79 +++++++++++++ 3 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/signal/registration/sender/messagebird/classic/MessageBirdClassicConversionReporter.java create mode 100644 src/test/java/org/signal/registration/sender/messagebird/classic/MessageBirdClassicConversionReporterTest.java diff --git a/src/main/java/org/signal/registration/sender/messagebird/classic/MessageBirdClassicConversionReporter.java b/src/main/java/org/signal/registration/sender/messagebird/classic/MessageBirdClassicConversionReporter.java new file mode 100644 index 00000000..a34a12a9 --- /dev/null +++ b/src/main/java/org/signal/registration/sender/messagebird/classic/MessageBirdClassicConversionReporter.java @@ -0,0 +1,110 @@ +package org.signal.registration.sender.messagebird.classic; + +import com.google.common.annotations.VisibleForTesting; +import com.messagebird.MessageBirdService; +import com.messagebird.MessageBirdServiceImpl; +import com.messagebird.exceptions.GeneralException; +import com.messagebird.exceptions.UnauthorizedException; +import io.micrometer.core.instrument.MeterRegistry; +import io.micronaut.context.event.ApplicationEventListener; +import io.micronaut.scheduling.TaskExecutors; +import jakarta.inject.Named; +import jakarta.inject.Singleton; +import java.util.Optional; +import java.util.concurrent.CompletionException; +import java.util.concurrent.Executor; +import org.apache.commons.lang3.StringUtils; +import org.signal.registration.messagebird.MessageBirdClientConfiguration; +import org.signal.registration.metrics.MetricsUtil; +import org.signal.registration.session.RegistrationAttempt; +import org.signal.registration.session.RegistrationSession; +import org.signal.registration.session.SessionCompletedEvent; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Report success/failure outcomes of messagebird sms/voice messages to messagebird. Does not include messagebird-verify + * outcomes, because those are delivered as part of checking verification codes. + */ +@Singleton +public class MessageBirdClassicConversionReporter implements ApplicationEventListener { + + private static final Logger logger = LoggerFactory.getLogger(MessageBirdClassicConversionReporter.class); + + private static final String COUNTER_NAME = MetricsUtil.name(MessageBirdClassicConversionReporter.class, + "conversionPosted"); + + private final MessageBirdService messageBirdService; + private final Executor executor; + private final MeterRegistry meterRegistry; + + public MessageBirdClassicConversionReporter( + final MessageBirdClientConfiguration clientConfiguration, + final MeterRegistry meterRegistry, + final @Named(TaskExecutors.IO) Executor executor) { + this(new MessageBirdServiceImpl(clientConfiguration.accessKey()), meterRegistry, executor); + } + + @VisibleForTesting + MessageBirdClassicConversionReporter( + final MessageBirdService messageBirdService, + final MeterRegistry meterRegistry, + final Executor executor) { + this.messageBirdService = messageBirdService; + this.meterRegistry = meterRegistry; + this.executor = executor; + } + + + /** + * Request body for the messagebird /conversion endpoint + * + * @param service "voice" or "sms" + * @param id the message id + * @param success whether the attempt was successfully converted + */ + @VisibleForTesting + record ConversionRequest(String service, String id, boolean success) { + + } + + @Override + public void onApplicationEvent(final SessionCompletedEvent event) { + final RegistrationSession session = event.session(); + + for (int i = 0; i < session.getRegistrationAttemptsCount(); i++) { + final RegistrationAttempt attempt = session.getRegistrationAttempts(i); + + // Assume that all verification attempts before the last one were not successfully verified + final boolean attemptVerified = StringUtils.isNotBlank(session.getVerifiedCode()) && + i == session.getRegistrationAttemptsCount() - 1; + + messageBirdServiceType(attempt.getSenderName()).ifPresent(serviceType -> executor.execute(() -> { + final ConversionRequest request = new ConversionRequest(serviceType, attempt.getRemoteId(), attemptVerified); + try { + messageBirdService.sendPayLoad("/conversions", request, null); + meterRegistry.counter(COUNTER_NAME, MetricsUtil.SUCCESS_TAG_NAME, String.valueOf(true)).increment(); + } catch (UnauthorizedException | GeneralException e) { + logger.warn("Failed to post conversion", e); + meterRegistry.counter(COUNTER_NAME, MetricsUtil.SUCCESS_TAG_NAME, String.valueOf(false)).increment(); + throw new CompletionException(e); + } + })); + } + } + + /** + * Get the service type for an attempt to provide in the conversion request + * + * @param senderName The senderName of the sender that processed the attempt + * @return The serviceType to use in the request, or empty if this is not message bird sender that requires conversion + * reporting + */ + private static Optional messageBirdServiceType(final String senderName) { + return switch (senderName) { + case MessageBirdSmsSender.SENDER_NAME -> Optional.of("sms"); + case MessageBirdVoiceSender.SENDER_NAME -> Optional.of("voice"); + default -> Optional.empty(); + }; + } +} diff --git a/src/main/java/org/signal/registration/sender/messagebird/classic/MessageBirdVoiceSender.java b/src/main/java/org/signal/registration/sender/messagebird/classic/MessageBirdVoiceSender.java index e48e382e..3ac20d53 100644 --- a/src/main/java/org/signal/registration/sender/messagebird/classic/MessageBirdVoiceSender.java +++ b/src/main/java/org/signal/registration/sender/messagebird/classic/MessageBirdVoiceSender.java @@ -53,6 +53,8 @@ public class MessageBirdVoiceSender implements VerificationCodeSender { private static final Logger logger = LoggerFactory.getLogger(MessageBirdVoiceSender.class); + public static final String SENDER_NAME = "messagebird-voice"; + private final MessageBirdVoiceConfiguration configuration; private final Map supportedLanguages; private final Executor executor; @@ -82,7 +84,7 @@ public MessageBirdVoiceSender( @Override public String getName() { - return "messagebird-voice"; + return SENDER_NAME; } @Override diff --git a/src/test/java/org/signal/registration/sender/messagebird/classic/MessageBirdClassicConversionReporterTest.java b/src/test/java/org/signal/registration/sender/messagebird/classic/MessageBirdClassicConversionReporterTest.java new file mode 100644 index 00000000..245416d9 --- /dev/null +++ b/src/test/java/org/signal/registration/sender/messagebird/classic/MessageBirdClassicConversionReporterTest.java @@ -0,0 +1,79 @@ +/* + * Copyright 2024 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ +package org.signal.registration.sender.messagebird.classic; + +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import com.messagebird.MessageBirdService; +import com.messagebird.exceptions.GeneralException; +import com.messagebird.exceptions.UnauthorizedException; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.signal.registration.sender.messagebird.verify.MessageBirdVerifySender; +import org.signal.registration.session.RegistrationAttempt; +import org.signal.registration.session.RegistrationSession; +import org.signal.registration.session.SessionCompletedEvent; + +class MessageBirdClassicConversionReporterTest { + private final MessageBirdService mockService = mock(MessageBirdService.class); + private final MessageBirdClassicConversionReporter reporter = new MessageBirdClassicConversionReporter(mockService, + new SimpleMeterRegistry(), Runnable::run); + + @BeforeEach + public void setup() { + reset(mockService); + } + + @Test + public void reportSingleAttempt() throws GeneralException, UnauthorizedException { + reporter.onApplicationEvent(new SessionCompletedEvent(RegistrationSession.newBuilder() + .setVerifiedCode("abc") + .addRegistrationAttempts(RegistrationAttempt.newBuilder() + .setRemoteId("a") + .setSenderName(MessageBirdSmsSender.SENDER_NAME) + .build()) + .build())); + + verify(mockService, times(1)) + .sendPayLoad(eq("/conversions"), eq(new MessageBirdClassicConversionReporter.ConversionRequest("sms", "a", true)), isNull()); + verifyNoMoreInteractions(mockService); + } + + @Test + public void reportMultiAttempt() throws GeneralException, UnauthorizedException { + reporter.onApplicationEvent(new SessionCompletedEvent(RegistrationSession.newBuilder() + .setVerifiedCode("abc") + // attempt failed + .addRegistrationAttempts(RegistrationAttempt.newBuilder() + .setRemoteId("a") + .setSenderName(MessageBirdSmsSender.SENDER_NAME) + .build()) + // attempt with non-classic sender + .addRegistrationAttempts(RegistrationAttempt.newBuilder() + .setRemoteId("b") + .setSenderName(MessageBirdVerifySender.SENDER_NAME) + .build()) + // success with voice sender + .addRegistrationAttempts(RegistrationAttempt.newBuilder() + .setRemoteId("c") + .setSenderName(MessageBirdVoiceSender.SENDER_NAME) + .build()) + .build())); + + verify(mockService, times(1)) + .sendPayLoad(eq("/conversions"), eq(new MessageBirdClassicConversionReporter.ConversionRequest("sms", "a", false)), isNull()); + verify(mockService, times(1)) + .sendPayLoad(eq("/conversions"), eq(new MessageBirdClassicConversionReporter.ConversionRequest("voice", "c", true)), isNull()); + verifyNoMoreInteractions(mockService); + } + +}