From a7968ccc3ce29a652ca8eedbd1103590f43169a3 Mon Sep 17 00:00:00 2001
From: Ehren Kret <ehren@signal.org>
Date: Tue, 14 Jul 2020 10:28:03 -0500
Subject: [PATCH] Address code review comments

---
 ...derIdStrategy.java => SenderIdSelector.java} | 17 ++++++-----------
 .../textsecuregcm/sms/TwilioSmsSender.java      |  6 +++---
 2 files changed, 9 insertions(+), 14 deletions(-)
 rename service/src/main/java/org/whispersystems/textsecuregcm/sms/{SenderIdStrategy.java => SenderIdSelector.java} (67%)

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/sms/SenderIdStrategy.java b/service/src/main/java/org/whispersystems/textsecuregcm/sms/SenderIdSelector.java
similarity index 67%
rename from service/src/main/java/org/whispersystems/textsecuregcm/sms/SenderIdStrategy.java
rename to service/src/main/java/org/whispersystems/textsecuregcm/sms/SenderIdSelector.java
index 900ba6e27..53f8bd80d 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/sms/SenderIdStrategy.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/sms/SenderIdSelector.java
@@ -1,6 +1,6 @@
 package org.whispersystems.textsecuregcm.sms;
 
-import com.google.common.base.Strings;
+import org.apache.commons.lang3.StringUtils;
 import org.whispersystems.textsecuregcm.configuration.TwilioCountrySenderIdConfiguration;
 import org.whispersystems.textsecuregcm.configuration.TwilioSenderIdConfiguration;
 import org.whispersystems.textsecuregcm.util.Util;
@@ -12,12 +12,12 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
-public class SenderIdStrategy {
+class SenderIdSelector {
   private final String              defaultSenderId;
   private final Map<String, String> countrySpecificSenderIds;
   private final Set<String>         countryCodesWithoutSenderId;
 
-  public SenderIdStrategy(String defaultSenderId, List<TwilioCountrySenderIdConfiguration> countrySpecificSenderIds, Set<String> countryCodesWithoutSenderId) {
+  SenderIdSelector(String defaultSenderId, List<TwilioCountrySenderIdConfiguration> countrySpecificSenderIds, Set<String> countryCodesWithoutSenderId) {
     this.defaultSenderId = defaultSenderId;
     this.countrySpecificSenderIds = countrySpecificSenderIds.stream().collect(Collectors.toMap(
             TwilioCountrySenderIdConfiguration::getCountryCode,
@@ -25,23 +25,18 @@ public class SenderIdStrategy {
     this.countryCodesWithoutSenderId = countryCodesWithoutSenderId;
   }
 
-  public SenderIdStrategy(TwilioSenderIdConfiguration configuration) {
+  SenderIdSelector(TwilioSenderIdConfiguration configuration) {
     this(configuration.getDefaultSenderId(),
          configuration.getCountrySpecificSenderIds(),
          configuration.getCountryCodesWithoutSenderId());
   }
 
-  public Optional<String> get(@NotNull String destination) {
+  Optional<String> getSenderId(@NotNull String destination) {
     final String countryCode = Util.getCountryCode(destination);
     if (countryCodesWithoutSenderId.contains(countryCode)) {
       return Optional.empty();
     }
 
-    final String countrySpecificSenderId = countrySpecificSenderIds.get(countryCode);
-    if (!Strings.isNullOrEmpty(countrySpecificSenderId)) {
-      return Optional.of(countrySpecificSenderId);
-    }
-
-    return Optional.ofNullable(defaultSenderId);
+    return Optional.ofNullable(StringUtils.stripToNull(countrySpecificSenderIds.getOrDefault(countryCode, defaultSenderId)));
   }
 }
diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java b/service/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java
index a81182558..8f646e7b4 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/sms/TwilioSmsSender.java
@@ -65,8 +65,8 @@ public class TwilioSmsSender {
   private final ArrayList<String> numbers;
   private final String            messagingServicesId;
   private final String            localDomain;
+  private final SenderIdSelector  senderIdSelector;
   private final Random            random;
-  private final SenderIdStrategy  senderIdStrategy;
 
   private final FaultTolerantHttpClient httpClient;
   private final URI                     smsUri;
@@ -81,7 +81,7 @@ public class TwilioSmsSender {
     this.numbers             = new ArrayList<>(twilioConfiguration.getNumbers());
     this.localDomain         = twilioConfiguration.getLocalDomain();
     this.messagingServicesId = twilioConfiguration.getMessagingServicesId();
-    this.senderIdStrategy    = new SenderIdStrategy(twilioConfiguration.getSenderId());
+    this.senderIdSelector    = new SenderIdSelector(twilioConfiguration.getSenderId());
     this.random              = new Random(System.currentTimeMillis());
     this.smsUri              = URI.create(baseUri + "/2010-04-01/Accounts/" + accountId + "/Messages.json");
     this.voxUri              = URI.create(baseUri + "/2010-04-01/Accounts/" + accountId + "/Calls.json"   );
@@ -174,7 +174,7 @@ public class TwilioSmsSender {
   }
 
   private void setOriginationRequestParameter(String destination, Map<String, String> requestParameters) {
-    final Optional<String> senderId = senderIdStrategy.get(destination);
+    final Optional<String> senderId = senderIdSelector.getSenderId(destination);
     if (senderId.isPresent()) {
       requestParameters.put("From", senderId.get());
     } else if (Util.isEmpty(messagingServicesId)) {