Retire Device#hasMessageDeliveryChannel()

signalapp · Jun 26, 2024 · 73e0aea · 73e0aea
1 parent 1a09f58
commit 73e0aea
Show file tree

Hide file tree

Showing 11 changed files with 41 additions and 96 deletions.
diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Device.java
@@ -14,7 +14,6 @@
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import javax.annotation.Nullable;
-import org.apache.commons.lang3.StringUtils;
 import org.whispersystems.textsecuregcm.auth.SaltedTokenHash;
 import org.whispersystems.textsecuregcm.util.DeviceNameByteArrayAdapter;
 
@@ -187,10 +186,6 @@ public void setCapabilities(DeviceCapabilities capabilities) {
     this.capabilities = capabilities;
   }
 
-  public boolean hasMessageDeliveryChannel() {
-    return fetchesMessages || StringUtils.isNotEmpty(getApnId()) || StringUtils.isNotEmpty(getGcmId());
-  }
-
   public boolean isExpired() {
     return isPrimary()
         ? lastSeen < (System.currentTimeMillis() - ALLOWED_PRIMARY_IDLE_MILLIS)

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java
@@ -22,6 +22,7 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
+import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
@@ -245,7 +246,7 @@ void unlinkLeastActiveDevice(final Account account, byte destinationDeviceId) th
     // its messages) is unlinked
     final Device deviceToDelete = account.getDevices()
         .stream()
-        .filter(d -> !d.isPrimary() && !d.hasMessageDeliveryChannel())
+        .filter(d -> !d.isPrimary() && !deviceHasMessageDeliveryChannel(d))
         .min(Comparator.comparing(Device::getLastSeen))
         .or(() ->
             Flux.fromIterable(account.getDevices())
@@ -288,4 +289,8 @@ void unlinkLeastActiveDevice(final Account account, byte destinationDeviceId) th
           }
         });
   }
+
+  private static boolean deviceHasMessageDeliveryChannel(final Device device) {
+    return device.getFetchesMessages() || StringUtils.isNotEmpty(device.getApnId()) || StringUtils.isNotEmpty(device.getGcmId());
+  }
 }
diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/auth/AccountAuthenticatorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/auth/AccountAuthenticatorTest.java
@@ -162,7 +162,6 @@ void testAuthenticate() {
     when(account.getUuid()).thenReturn(uuid);
     when(account.getDevice(deviceId)).thenReturn(Optional.of(device));
     when(device.getId()).thenReturn(deviceId);
-    when(device.hasMessageDeliveryChannel()).thenReturn(true);
     when(device.getAuthTokenHash()).thenReturn(credentials);
     when(credentials.verify(password)).thenReturn(true);
     when(credentials.getVersion()).thenReturn(SaltedTokenHash.CURRENT_VERSION);
@@ -191,7 +190,6 @@ void testAuthenticateNonDefaultDevice() {
     when(account.getUuid()).thenReturn(uuid);
     when(account.getDevice(deviceId)).thenReturn(Optional.of(device));
     when(device.getId()).thenReturn(deviceId);
-    when(device.hasMessageDeliveryChannel()).thenReturn(true);
     when(device.getAuthTokenHash()).thenReturn(credentials);
     when(credentials.verify(password)).thenReturn(true);
     when(credentials.getVersion()).thenReturn(SaltedTokenHash.CURRENT_VERSION);
@@ -223,7 +221,6 @@ void testAuthenticateEnabled(
     when(account.getUuid()).thenReturn(uuid);
     when(account.getDevice(deviceId)).thenReturn(Optional.of(authenticatedDevice));
     when(authenticatedDevice.getId()).thenReturn(deviceId);
-    when(authenticatedDevice.hasMessageDeliveryChannel()).thenReturn(deviceEnabled);
     when(authenticatedDevice.getAuthTokenHash()).thenReturn(credentials);
     when(credentials.verify(password)).thenReturn(true);
     when(credentials.getVersion()).thenReturn(SaltedTokenHash.CURRENT_VERSION);
@@ -258,7 +255,6 @@ void testAuthenticateV1() {
     when(account.getUuid()).thenReturn(uuid);
     when(account.getDevice(deviceId)).thenReturn(Optional.of(device));
     when(device.getId()).thenReturn(deviceId);
-    when(device.hasMessageDeliveryChannel()).thenReturn(true);
     when(device.getAuthTokenHash()).thenReturn(credentials);
     when(credentials.verify(password)).thenReturn(true);
     when(credentials.getVersion()).thenReturn(SaltedTokenHash.Version.V1);
@@ -294,7 +290,6 @@ void testAuthenticateDeviceNotFound() {
     when(account.getUuid()).thenReturn(uuid);
     when(account.getDevice(deviceId)).thenReturn(Optional.of(device));
     when(device.getId()).thenReturn(deviceId);
-    when(device.hasMessageDeliveryChannel()).thenReturn(true);
     when(device.getAuthTokenHash()).thenReturn(credentials);
     when(credentials.verify(password)).thenReturn(true);
     when(credentials.getVersion()).thenReturn(SaltedTokenHash.CURRENT_VERSION);
@@ -321,7 +316,6 @@ void testAuthenticateIncorrectPassword() {
     when(account.getUuid()).thenReturn(uuid);
     when(account.getDevice(deviceId)).thenReturn(Optional.of(device));
     when(device.getId()).thenReturn(deviceId);
-    when(device.hasMessageDeliveryChannel()).thenReturn(true);
     when(device.getAuthTokenHash()).thenReturn(credentials);
     when(credentials.verify(password)).thenReturn(true);
     when(credentials.getVersion()).thenReturn(SaltedTokenHash.CURRENT_VERSION);

diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java
@@ -221,10 +221,6 @@ void setup() {
     when(sampleDevice3.getRegistrationId()).thenReturn(SAMPLE_REGISTRATION_ID2);
     when(sampleDevice4.getRegistrationId()).thenReturn(SAMPLE_REGISTRATION_ID4);
     when(sampleDevice.getPhoneNumberIdentityRegistrationId()).thenReturn(OptionalInt.of(SAMPLE_PNI_REGISTRATION_ID));
-    when(sampleDevice.hasMessageDeliveryChannel()).thenReturn(true);
-    when(sampleDevice2.hasMessageDeliveryChannel()).thenReturn(true);
-    when(sampleDevice3.hasMessageDeliveryChannel()).thenReturn(false);
-    when(sampleDevice4.hasMessageDeliveryChannel()).thenReturn(true);
     when(sampleDevice.getId()).thenReturn(sampleDeviceId);
     when(sampleDevice2.getId()).thenReturn(sampleDevice2Id);
     when(sampleDevice3.getId()).thenReturn(sampleDevice3Id);

diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/KeysGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/KeysGrpcServiceTest.java
@@ -489,7 +489,6 @@ void getPreKeys(final org.signal.chat.common.IdentityType grpcIdentityType) {
 
       final Device device = mock(Device.class);
       when(device.getId()).thenReturn(deviceId);
-      when(device.hasMessageDeliveryChannel()).thenReturn(true);
 
       devices.put(deviceId, device);
       when(targetAccount.getDevice(deviceId)).thenReturn(Optional.of(device));

diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountTest.java
@@ -50,44 +50,35 @@ class AccountTest {
   @BeforeEach
   void setup() {
     when(oldPrimaryDevice.getLastSeen()).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(366));
-    when(oldPrimaryDevice.hasMessageDeliveryChannel()).thenReturn(true);
     when(oldPrimaryDevice.getId()).thenReturn(Device.PRIMARY_ID);
 
     when(recentPrimaryDevice.getLastSeen()).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(1));
-    when(recentPrimaryDevice.hasMessageDeliveryChannel()).thenReturn(true);
     when(recentPrimaryDevice.getId()).thenReturn(Device.PRIMARY_ID);
 
     when(agingSecondaryDevice.getLastSeen()).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(31));
-    when(agingSecondaryDevice.hasMessageDeliveryChannel()).thenReturn(false);
     final byte deviceId2 = 2;
     when(agingSecondaryDevice.getId()).thenReturn(deviceId2);
 
     when(recentSecondaryDevice.getLastSeen()).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(1));
-    when(recentSecondaryDevice.hasMessageDeliveryChannel()).thenReturn(true);
     when(recentSecondaryDevice.getId()).thenReturn(deviceId2);
 
     when(oldSecondaryDevice.getLastSeen()).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(366));
-    when(oldSecondaryDevice.hasMessageDeliveryChannel()).thenReturn(false);
     when(oldSecondaryDevice.getId()).thenReturn(deviceId2);
 
-    when(paymentActivationCapableDevice.getCapabilities()).thenReturn(
-        new DeviceCapabilities(true, true, true, false));
-    when(paymentActivationCapableDevice.hasMessageDeliveryChannel()).thenReturn(true);
-    when(paymentActivationIncapableDevice.getCapabilities()).thenReturn(
-        new DeviceCapabilities(true, true, false, false));
-    when(paymentActivationIncapableDevice.hasMessageDeliveryChannel()).thenReturn(true);
-    when(paymentActivationIncapableDeviceWithoutDeliveryChannel.getCapabilities()).thenReturn(
-        new DeviceCapabilities(true, true, false, false));
-    when(paymentActivationIncapableDeviceWithoutDeliveryChannel.hasMessageDeliveryChannel()).thenReturn(false);
-
-    when(deleteSyncCapableDevice.getCapabilities()).thenReturn(
-        new DeviceCapabilities(true, true, true, true)
-    );
-    when(deleteSyncCapableDevice.hasMessageDeliveryChannel()).thenReturn(true);
-    when(deleteSyncIncapableDevice.getCapabilities()).thenReturn(
-        new DeviceCapabilities(true, true, true, false)
-    );
-    when(deleteSyncIncapableDevice.hasMessageDeliveryChannel()).thenReturn(true);
+    when(paymentActivationCapableDevice.getCapabilities())
+        .thenReturn(new DeviceCapabilities(true, true, true, false));
+
+    when(paymentActivationIncapableDevice.getCapabilities())
+        .thenReturn(new DeviceCapabilities(true, true, false, false));
+
+    when(paymentActivationIncapableDeviceWithoutDeliveryChannel.getCapabilities())
+        .thenReturn(new DeviceCapabilities(true, true, false, false));
+
+    when(deleteSyncCapableDevice.getCapabilities())
+        .thenReturn(new DeviceCapabilities(true, true, true, true));
+
+    when(deleteSyncIncapableDevice.getCapabilities())
+        .thenReturn(new DeviceCapabilities(true, true, true, false));
 
   }
 

diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java
@@ -134,7 +134,6 @@ void changeNumberSetPrimaryDevicePrekeyAndSendMessages() throws Exception {
     when(account.getPhoneNumberIdentifier()).thenReturn(pni);
 
     final Device d2 = mock(Device.class);
-    when(d2.hasMessageDeliveryChannel()).thenReturn(true);
     final byte deviceId2 = 2;
     when(d2.getId()).thenReturn(deviceId2);
 
@@ -181,7 +180,6 @@ void changeNumberSetPrimaryDevicePrekeyPqAndSendMessages() throws Exception {
     when(account.getPhoneNumberIdentifier()).thenReturn(pni);
 
     final Device d2 = mock(Device.class);
-    when(d2.hasMessageDeliveryChannel()).thenReturn(true);
     final byte deviceId2 = 2;
     when(d2.getId()).thenReturn(deviceId2);
 
@@ -228,7 +226,6 @@ void changeNumberSameNumberSetPrimaryDevicePrekeyAndSendMessages() throws Except
     when(account.getPhoneNumberIdentifier()).thenReturn(pni);
 
     final Device d2 = mock(Device.class);
-    when(d2.hasMessageDeliveryChannel()).thenReturn(true);
     final byte deviceId2 = 2;
     when(d2.getId()).thenReturn(deviceId2);
 
@@ -273,7 +270,6 @@ void updatePniKeysSetPrimaryDevicePrekeyAndSendMessages() throws Exception {
     when(account.getPhoneNumberIdentifier()).thenReturn(pni);
 
     final Device d2 = mock(Device.class);
-    when(d2.hasMessageDeliveryChannel()).thenReturn(true);
     final byte deviceId2 = 2;
     when(d2.getId()).thenReturn(deviceId2);
 
@@ -316,7 +312,6 @@ void updatePniKeysSetPrimaryDevicePrekeyPqAndSendMessages() throws Exception {
     when(account.getPhoneNumberIdentifier()).thenReturn(pni);
 
     final Device d2 = mock(Device.class);
-    when(d2.hasMessageDeliveryChannel()).thenReturn(true);
     final byte deviceId2 = 2;
     when(d2.getId()).thenReturn(deviceId2);
 
@@ -361,7 +356,6 @@ void changeNumberMismatchedRegistrationId() {
     for (byte i = 1; i <= 3; i++) {
       final Device device = mock(Device.class);
       when(device.getId()).thenReturn(i);
-      when(device.hasMessageDeliveryChannel()).thenReturn(true);
       when(device.getRegistrationId()).thenReturn((int) i);
 
       devices.add(device);
@@ -400,7 +394,6 @@ void updatePniKeysMismatchedRegistrationId() {
     for (byte i = 1; i <= 3; i++) {
       final Device device = mock(Device.class);
       when(device.getId()).thenReturn(i);
-      when(device.hasMessageDeliveryChannel()).thenReturn(true);
       when(device.getRegistrationId()).thenReturn((int) i);
 
       devices.add(device);
@@ -439,7 +432,6 @@ void changeNumberMissingData() {
     for (byte i = 1; i <= 3; i++) {
       final Device device = mock(Device.class);
       when(device.getId()).thenReturn(i);
-      when(device.hasMessageDeliveryChannel()).thenReturn(true);
       when(device.getRegistrationId()).thenReturn((int) i);
 
       devices.add(device);

diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/DeviceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/DeviceTest.java
@@ -9,35 +9,11 @@
 
 import java.time.Duration;
 import java.time.Instant;
-import java.util.stream.Stream;
 import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.CsvSource;
-import org.junit.jupiter.params.provider.MethodSource;
 
 class DeviceTest {
 
-  @ParameterizedTest
-  @MethodSource
-  void testHasMessageDeliveryChannel(final boolean fetchesMessages, final String apnId, final String gcmId, final boolean expectEnabled) {
-
-    final Device device = new Device();
-    device.setFetchesMessages(fetchesMessages);
-    device.setApnId(apnId);
-    device.setGcmId(gcmId);
-
-    assertEquals(expectEnabled, device.hasMessageDeliveryChannel());
-  }
-
-  private static Stream<Arguments> testHasMessageDeliveryChannel() {
-    return Stream.of(
-        Arguments.of(false, null, null, false),
-        Arguments.of(false, null, "gcm-id", true),
-        Arguments.of(false, "apn-id", null, true),
-        Arguments.of(true, null, null, true)
-    );
-  }
-
   @ParameterizedTest
   @CsvSource({
       "true, P1D, false",

diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagePersisterTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagePersisterTest.java
@@ -81,8 +81,8 @@ class MessagePersisterTest {
   void setUp() throws Exception {
 
     messagesManager = mock(MessagesManager.class);
-    final DynamicConfigurationManager<DynamicConfiguration> dynamicConfigurationManager = mock(
-        DynamicConfigurationManager.class);
+    @SuppressWarnings("unchecked") final DynamicConfigurationManager<DynamicConfiguration> dynamicConfigurationManager =
+        mock(DynamicConfigurationManager.class);
 
     messagesDynamoDb = mock(MessagesDynamoDb.class);
     accountsManager = mock(AccountsManager.class);
@@ -107,6 +107,9 @@ void setUp() throws Exception {
     messagePersister = new MessagePersister(messagesCache, messagesManager, accountsManager, clientPresenceManager,
         keysManager, dynamicConfigurationManager, PERSIST_DELAY, 1, MoreExecutors.newDirectExecutorService());
 
+    when(messagesManager.clear(any(UUID.class), anyByte())).thenReturn(CompletableFuture.completedFuture(null));
+    when(keysManager.deleteSingleUsePreKeys(any(), anyByte())).thenReturn(CompletableFuture.completedFuture(null));
+
     when(messagesManager.persistMessages(any(UUID.class), any(), any())).thenAnswer(invocation -> {
       final UUID destinationUuid = invocation.getArgument(0);
       final Device destinationDevice = invocation.getArgument(1);
@@ -258,29 +261,29 @@ void testUnlinkFirstInactiveDeviceOnFullQueue() {
     final Device primary = mock(Device.class);
     when(primary.getId()).thenReturn((byte) 1);
     when(primary.isPrimary()).thenReturn(true);
-    when(primary.hasMessageDeliveryChannel()).thenReturn(true);
+    when(primary.getFetchesMessages()).thenReturn(true);
+
     final Device activeA = mock(Device.class);
     when(activeA.getId()).thenReturn((byte) 2);
-    when(activeA.hasMessageDeliveryChannel()).thenReturn(true);
+    when(activeA.getFetchesMessages()).thenReturn(true);
+
     final Device inactiveB = mock(Device.class);
     final byte inactiveId = 3;
     when(inactiveB.getId()).thenReturn(inactiveId);
-    when(inactiveB.hasMessageDeliveryChannel()).thenReturn(false);
+
     final Device inactiveC = mock(Device.class);
     when(inactiveC.getId()).thenReturn((byte) 4);
-    when(inactiveC.hasMessageDeliveryChannel()).thenReturn(false);
+
     final Device activeD = mock(Device.class);
     when(activeD.getId()).thenReturn((byte) 5);
-    when(activeD.hasMessageDeliveryChannel()).thenReturn(true);
+    when(activeD.getFetchesMessages()).thenReturn(true);
+
     final Device destination = mock(Device.class);
     when(destination.getId()).thenReturn(DESTINATION_DEVICE_ID);
-    when(destination.hasMessageDeliveryChannel()).thenReturn(true);
 
     when(destinationAccount.getDevices()).thenReturn(List.of(primary, activeA, inactiveB, inactiveC, activeD, destination));
 
     when(messagesManager.persistMessages(any(UUID.class), any(), anyList())).thenThrow(ItemCollectionSizeLimitExceededException.builder().build());
-    when(messagesManager.clear(any(UUID.class), anyByte())).thenReturn(CompletableFuture.completedFuture(null));
-    when(keysManager.deleteSingleUsePreKeys(any(), eq(inactiveId))).thenReturn(CompletableFuture.completedFuture(null));
 
     assertTimeoutPreemptively(Duration.ofSeconds(1), () ->
         messagePersister.persistQueue(destinationAccount, DESTINATION_DEVICE));
@@ -302,27 +305,27 @@ void testUnlinkActiveDeviceWithOldestMessageOnFullQueueWithNoInactiveDevices() {
     final byte primaryId = 1;
     when(primary.getId()).thenReturn(primaryId);
     when(primary.isPrimary()).thenReturn(true);
-    when(primary.hasMessageDeliveryChannel()).thenReturn(true);
+    when(primary.getFetchesMessages()).thenReturn(true);
     when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(primary)))
         .thenReturn(Mono.just(4L));
 
     final Device deviceA = mock(Device.class);
     final byte deviceIdA = 2;
     when(deviceA.getId()).thenReturn(deviceIdA);
-    when(deviceA.hasMessageDeliveryChannel()).thenReturn(true);
+    when(deviceA.getFetchesMessages()).thenReturn(true);
     when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(deviceA)))
         .thenReturn(Mono.empty());
 
     final Device deviceB = mock(Device.class);
     final byte deviceIdB = 3;
     when(deviceB.getId()).thenReturn(deviceIdB);
-    when(deviceB.hasMessageDeliveryChannel()).thenReturn(true);
+    when(deviceB.getFetchesMessages()).thenReturn(true);
     when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(deviceB)))
         .thenReturn(Mono.just(2L));
 
     final Device destination = mock(Device.class);
     when(destination.getId()).thenReturn(DESTINATION_DEVICE_ID);
-    when(destination.hasMessageDeliveryChannel()).thenReturn(true);
+    when(destination.getFetchesMessages()).thenReturn(true);
     when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(destination)))
         .thenReturn(Mono.just(5L));
 
@@ -352,27 +355,27 @@ void testUnlinkDestinationDevice() {
     final byte primaryId = 1;
     when(primary.getId()).thenReturn(primaryId);
     when(primary.isPrimary()).thenReturn(true);
-    when(primary.hasMessageDeliveryChannel()).thenReturn(true);
+    when(primary.getFetchesMessages()).thenReturn(true);
     when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(primary)))
         .thenReturn(Mono.just(1L));
 
     final Device deviceA = mock(Device.class);
     final byte deviceIdA = 2;
     when(deviceA.getId()).thenReturn(deviceIdA);
-    when(deviceA.hasMessageDeliveryChannel()).thenReturn(true);
+    when(deviceA.getFetchesMessages()).thenReturn(true);
     when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(deviceA)))
         .thenReturn(Mono.just(3L));
 
     final Device deviceB = mock(Device.class);
     final byte deviceIdB = 2;
     when(deviceB.getId()).thenReturn(deviceIdB);
-    when(deviceB.hasMessageDeliveryChannel()).thenReturn(true);
+    when(deviceB.getFetchesMessages()).thenReturn(true);
     when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(deviceB)))
         .thenReturn(Mono.empty());
 
     final Device destination = mock(Device.class);
     when(destination.getId()).thenReturn(DESTINATION_DEVICE_ID);
-    when(destination.hasMessageDeliveryChannel()).thenReturn(true);
+    when(destination.getFetchesMessages()).thenReturn(true);
     when(messagesManager.getEarliestUndeliveredTimestampForDevice(any(), eq(destination)))
         .thenReturn(Mono.just(2L));