From 66a04ed7307da6c75f01f5282644a6f8cd96e2b9 Mon Sep 17 00:00:00 2001
From: Jon Chambers <jon@signal.org>
Date: Wed, 9 Sep 2020 19:25:25 -0400
Subject: [PATCH] Don't explicitly notify clients when messages get persisted.

---
 .../textsecuregcm/WhisperServerService.java   |  2 +-
 .../storage/MessagePersister.java             | 47 +------------------
 .../storage/MessagePersisterTest.java         | 14 ++----
 3 files changed, 7 insertions(+), 56 deletions(-)

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
index b32148f7a..eafb07d3f 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
@@ -326,7 +326,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
     TurnTokenGenerator       turnTokenGenerator = new TurnTokenGenerator(config.getTurnConfiguration());
     RecaptchaClient          recaptchaClient    = new RecaptchaClient(config.getRecaptchaConfiguration().getSecret());
 
-    MessagePersister clusterMessagePersister = new MessagePersister(messagesCache, messagesManager, pubSubManager, pushSender, accountsManager, Duration.ofMinutes(config.getMessageCacheConfiguration().getPersistDelayMinutes()));
+    MessagePersister clusterMessagePersister = new MessagePersister(messagesCache, messagesManager, accountsManager, Duration.ofMinutes(config.getMessageCacheConfiguration().getPersistDelayMinutes()));
 
     DirectoryReconciliationClient directoryReconciliationClient = new DirectoryReconciliationClient(config.getDirectoryConfiguration().getDirectoryServerConfiguration());
 
diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java
index 8b10f2dc8..6c44b3e68 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java
@@ -9,11 +9,8 @@ import io.dropwizard.lifecycle.Managed;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.whispersystems.textsecuregcm.entities.MessageProtos;
-import org.whispersystems.textsecuregcm.push.NotPushRegisteredException;
-import org.whispersystems.textsecuregcm.push.PushSender;
 import org.whispersystems.textsecuregcm.util.Constants;
 import org.whispersystems.textsecuregcm.util.Util;
-import org.whispersystems.textsecuregcm.websocket.WebsocketAddress;
 
 import java.time.Duration;
 import java.time.Instant;
@@ -27,8 +24,6 @@ public class MessagePersister implements Managed {
 
     private final MessagesCache   messagesCache;
     private final MessagesManager messagesManager;
-    private final PubSubManager   pubSubManager;
-    private final PushSender      pushSender;
     private final AccountsManager accountsManager;
 
     private final Duration persistDelay;
@@ -39,7 +34,6 @@ public class MessagePersister implements Managed {
     private final MetricRegistry metricRegistry         = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME);
     private final Timer          getQueuesTimer         = metricRegistry.timer(name(MessagePersister.class, "getQueues"));
     private final Timer          persistQueueTimer      = metricRegistry.timer(name(MessagePersister.class, "persistQueue"));
-    private final Timer          notifySubscribersTimer = metricRegistry.timer(name(MessagePersister.class, "notifySubscribers"));
     private final Histogram      queueCountHistogram    = metricRegistry.histogram(name(MessagePersister.class, "queueCount"));
     private final Histogram      queueSizeHistogram     = metricRegistry.histogram(name(MessagePersister.class, "queueSize"));
 
@@ -50,11 +44,9 @@ public class MessagePersister implements Managed {
 
     private static final Logger logger = LoggerFactory.getLogger(MessagePersister.class);
 
-    public MessagePersister(final MessagesCache messagesCache, final MessagesManager messagesManager, final PubSubManager pubSubManager, final PushSender pushSender, final AccountsManager accountsManager, final Duration persistDelay) {
+    public MessagePersister(final MessagesCache messagesCache, final MessagesManager messagesManager, final AccountsManager accountsManager, final Duration persistDelay) {
         this.messagesCache       = messagesCache;
         this.messagesManager     = messagesManager;
-        this.pubSubManager       = pubSubManager;
-        this.pushSender          = pushSender;
         this.accountsManager     = accountsManager;
         this.persistDelay        = persistDelay;
     }
@@ -102,7 +94,6 @@ public class MessagePersister implements Managed {
 
             for (final String queue : queuesToPersist) {
                 persistQueue(queue);
-                notifyClients(MessagesCache.getAccountUuidFromQueueName(queue), MessagesCache.getDeviceIdFromQueueName(queue));
             }
 
             queuesPersisted += queuesToPersist.size();
@@ -149,40 +140,4 @@ public class MessagePersister implements Managed {
             }
         }
     }
-
-    public void notifyClients(final UUID accountUuid, final long deviceId) {
-        try (final Timer.Context ignored = notifySubscribersTimer.time()) {
-            final Optional<Account> maybeAccount = accountsManager.get(accountUuid);
-
-            final String address;
-
-            if (maybeAccount.isPresent()) {
-                address = maybeAccount.get().getNumber();
-            } else {
-                logger.error("No account record found for account {}", accountUuid);
-                return;
-            }
-
-            final boolean notified = pubSubManager.publish(new WebsocketAddress(address, deviceId),
-                    PubSubProtos.PubSubMessage.newBuilder()
-                                              .setType(PubSubProtos.PubSubMessage.Type.QUERY_DB)
-                                              .build());
-
-            if (!notified) {
-                Optional<Account> account = accountsManager.get(address);
-
-                if (account.isPresent()) {
-                    Optional<Device> device = account.get().getDevice(deviceId);
-
-                    if (device.isPresent()) {
-                        try {
-                            pushSender.sendQueuedNotification(account.get(), device.get());
-                        } catch (final NotPushRegisteredException e) {
-                            logger.warn("After message persistence, no longer push registered!");
-                        }
-                    }
-                }
-            }
-        }
-    }
 }
diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagePersisterTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagePersisterTest.java
index 6e39ec8fc..149afe116 100644
--- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagePersisterTest.java
+++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagePersisterTest.java
@@ -6,7 +6,6 @@ import org.apache.commons.lang3.RandomStringUtils;
 import org.junit.Before;
 import org.junit.Test;
 import org.whispersystems.textsecuregcm.entities.MessageProtos;
-import org.whispersystems.textsecuregcm.push.PushSender;
 import org.whispersystems.textsecuregcm.redis.AbstractRedisClusterTest;
 
 import java.nio.charset.StandardCharsets;
@@ -31,12 +30,11 @@ import static org.mockito.Mockito.when;
 
 public class MessagePersisterTest extends AbstractRedisClusterTest {
 
-    private ExecutorService              notificationExecutorService;
-    private MessagesCache messagesCache;
-    private Messages                     messagesDatabase;
-    private PubSubManager                pubSubManager;
+    private ExecutorService  notificationExecutorService;
+    private MessagesCache    messagesCache;
+    private Messages         messagesDatabase;
     private MessagePersister messagePersister;
-    private AccountsManager              accountsManager;
+    private AccountsManager  accountsManager;
 
     private static final UUID   DESTINATION_ACCOUNT_UUID   = UUID.randomUUID();
     private static final String DESTINATION_ACCOUNT_NUMBER = "+18005551234";
@@ -55,7 +53,6 @@ public class MessagePersisterTest extends AbstractRedisClusterTest {
 
         messagesDatabase = mock(Messages.class);
         accountsManager  = mock(AccountsManager.class);
-        pubSubManager    = mock(PubSubManager.class);
 
         final Account account = mock(Account.class);
 
@@ -64,7 +61,7 @@ public class MessagePersisterTest extends AbstractRedisClusterTest {
 
         notificationExecutorService = Executors.newSingleThreadExecutor();
         messagesCache               = new MessagesCache(getRedisCluster(), notificationExecutorService);
-        messagePersister            = new MessagePersister(messagesCache, messagesManager, pubSubManager, mock(PushSender.class), accountsManager, PERSIST_DELAY);
+        messagePersister            = new MessagePersister(messagesCache, messagesManager, accountsManager, PERSIST_DELAY);
 
         doAnswer(invocation -> {
             final String destination             = invocation.getArgument(0, String.class);
@@ -148,7 +145,6 @@ public class MessagePersisterTest extends AbstractRedisClusterTest {
 
         messagePersister.persistNextQueues(now.plus(messagePersister.getPersistDelay()));
 
-        verify(pubSubManager, times(queueCount)).publish(any(), any());
         verify(messagesDatabase, times(queueCount * messagesPerQueue)).store(any(UUID.class), any(MessageProtos.Envelope.class), anyString(), anyLong());
     }