mirror of
https://github.com/signalapp/Signal-Server
synced 2026-04-21 13:58:07 +01:00
Delay processing FCM uninstalled feedback
Check to make sure client is not still active before unregistering, since FCM feedback seems to be often erroneous
This commit is contained in:
@@ -80,13 +80,13 @@ public class MessageControllerTest {
|
||||
@Before
|
||||
public void setup() throws Exception {
|
||||
Set<Device> singleDeviceList = new HashSet<Device>() {{
|
||||
add(new Device(1, null, "foo", "bar", "baz", "isgcm", null, null, false, 111, new SignedPreKey(333, "baz", "boop"), System.currentTimeMillis(), System.currentTimeMillis(), "Test", true));
|
||||
add(new Device(1, null, "foo", "bar", "baz", "isgcm", null, null, false, 111, new SignedPreKey(333, "baz", "boop"), System.currentTimeMillis(), System.currentTimeMillis(), "Test", true, 0));
|
||||
}};
|
||||
|
||||
Set<Device> multiDeviceList = new HashSet<Device>() {{
|
||||
add(new Device(1, null, "foo", "bar", "baz", "isgcm", null, null, false, 222, new SignedPreKey(111, "foo", "bar"), System.currentTimeMillis(), System.currentTimeMillis(), "Test", true));
|
||||
add(new Device(2, null, "foo", "bar", "baz", "isgcm", null, null, false, 333, new SignedPreKey(222, "oof", "rab"), System.currentTimeMillis(), System.currentTimeMillis(), "Test", true));
|
||||
add(new Device(3, null, "foo", "bar", "baz", "isgcm", null, null, false, 444, null, System.currentTimeMillis() - TimeUnit.DAYS.toMillis(31), System.currentTimeMillis(), "Test", true));
|
||||
add(new Device(1, null, "foo", "bar", "baz", "isgcm", null, null, false, 222, new SignedPreKey(111, "foo", "bar"), System.currentTimeMillis(), System.currentTimeMillis(), "Test", true, 0));
|
||||
add(new Device(2, null, "foo", "bar", "baz", "isgcm", null, null, false, 333, new SignedPreKey(222, "oof", "rab"), System.currentTimeMillis(), System.currentTimeMillis(), "Test", true, 0));
|
||||
add(new Device(3, null, "foo", "bar", "baz", "isgcm", null, null, false, 444, null, System.currentTimeMillis() - TimeUnit.DAYS.toMillis(31), System.currentTimeMillis(), "Test", true, 0));
|
||||
}};
|
||||
|
||||
Account singleDeviceAccount = new Account(SINGLE_DEVICE_RECIPIENT, singleDeviceList, "1234".getBytes());
|
||||
|
||||
@@ -1,9 +1,7 @@
|
||||
package org.whispersystems.textsecuregcm.tests.controllers;
|
||||
|
||||
import com.fasterxml.jackson.core.JsonProcessingException;
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import org.glassfish.jersey.test.grizzly.GrizzlyWebTestContainerFactory;
|
||||
import org.hamcrest.MatcherAssert;
|
||||
import org.junit.Before;
|
||||
import org.junit.Rule;
|
||||
import org.junit.Test;
|
||||
@@ -25,7 +23,6 @@ import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
|
||||
import io.dropwizard.auth.AuthValueFactoryProvider;
|
||||
import io.dropwizard.auth.PolymorphicAuthValueFactoryProvider;
|
||||
import io.dropwizard.testing.junit.ResourceTestRule;
|
||||
import static junit.framework.TestCase.*;
|
||||
@@ -55,8 +52,8 @@ public class TransparentDataControllerTest {
|
||||
|
||||
@Before
|
||||
public void setup() {
|
||||
Account accountOne = new Account("+14151231111", Collections.singleton(new Device(1, "foo", "bar", "salt", "keykey", "gcm-id", "apn-id", "voipapn-id", true, 1234, new SignedPreKey(5, "public-signed", "signtture-signed"), 31337, 31336, "CoolClient", true)), new byte[16]);
|
||||
Account accountTwo = new Account("+14151232222", Collections.singleton(new Device(1, "2foo", "2bar", "2salt", "2keykey", "2gcm-id", "2apn-id", "2voipapn-id", true, 1234, new SignedPreKey(5, "public-signed", "signtture-signed"), 31337, 31336, "CoolClient", true)), new byte[16]);
|
||||
Account accountOne = new Account("+14151231111", Collections.singleton(new Device(1, "foo", "bar", "salt", "keykey", "gcm-id", "apn-id", "voipapn-id", true, 1234, new SignedPreKey(5, "public-signed", "signtture-signed"), 31337, 31336, "CoolClient", true, 0)), new byte[16]);
|
||||
Account accountTwo = new Account("+14151232222", Collections.singleton(new Device(1, "2foo", "2bar", "2salt", "2keykey", "2gcm-id", "2apn-id", "2voipapn-id", true, 1234, new SignedPreKey(5, "public-signed", "signtture-signed"), 31337, 31336, "CoolClient", true, 0)), new byte[16]);
|
||||
|
||||
accountOne.setProfileName("OneProfileName");
|
||||
accountOne.setIdentityKey("identity_key_value");
|
||||
|
||||
@@ -13,6 +13,7 @@ import org.whispersystems.textsecuregcm.storage.Account;
|
||||
import org.whispersystems.textsecuregcm.storage.AccountsManager;
|
||||
import org.whispersystems.textsecuregcm.storage.Device;
|
||||
import org.whispersystems.textsecuregcm.tests.util.SynchronousExecutorService;
|
||||
import org.whispersystems.textsecuregcm.util.Util;
|
||||
|
||||
import java.util.Optional;
|
||||
|
||||
@@ -48,45 +49,45 @@ public class GCMSenderTest {
|
||||
verify(sender, times(1)).send(any(Message.class), eq(message));
|
||||
}
|
||||
|
||||
// @Test
|
||||
// public void testSendError() {
|
||||
// String destinationNumber = "+12223334444";
|
||||
// String gcmId = "foo";
|
||||
//
|
||||
// AccountsManager accountsManager = mock(AccountsManager.class);
|
||||
// Sender sender = mock(Sender.class );
|
||||
// Result invalidResult = mock(Result.class );
|
||||
// DirectoryQueue directoryQueue = mock(DirectoryQueue.class );
|
||||
// SynchronousExecutorService executorService = new SynchronousExecutorService();
|
||||
//
|
||||
// Account destinationAccount = mock(Account.class);
|
||||
// Device destinationDevice = mock(Device.class );
|
||||
//
|
||||
// when(destinationAccount.getDevice(1)).thenReturn(Optional.of(destinationDevice));
|
||||
// when(accountsManager.get(destinationNumber)).thenReturn(Optional.of(destinationAccount));
|
||||
// when(destinationDevice.getGcmId()).thenReturn(gcmId);
|
||||
//
|
||||
// when(invalidResult.isInvalidRegistrationId()).thenReturn(true);
|
||||
// when(invalidResult.isUnregistered()).thenReturn(false);
|
||||
// when(invalidResult.hasCanonicalRegistrationId()).thenReturn(false);
|
||||
// when(invalidResult.isSuccess()).thenReturn(true);
|
||||
//
|
||||
// GcmMessage message = new GcmMessage(gcmId, destinationNumber, 1, false);
|
||||
// GCMSender gcmSender = new GCMSender(accountsManager, sender, directoryQueue, executorService);
|
||||
//
|
||||
// SettableFuture<Result> invalidFuture = SettableFuture.create();
|
||||
// invalidFuture.set(invalidResult);
|
||||
//
|
||||
// when(sender.send(any(Message.class), Matchers.anyObject())).thenReturn(invalidFuture);
|
||||
// when(invalidResult.getContext()).thenReturn(message);
|
||||
//
|
||||
// gcmSender.sendMessage(message);
|
||||
//
|
||||
// verify(sender, times(1)).send(any(Message.class), eq(message));
|
||||
// verify(accountsManager, times(1)).get(eq(destinationNumber));
|
||||
// verify(accountsManager, times(1)).update(eq(destinationAccount));
|
||||
// verify(destinationDevice, times(1)).setGcmId(eq((String)null));
|
||||
// }
|
||||
@Test
|
||||
public void testSendUninstalled() {
|
||||
String destinationNumber = "+12223334444";
|
||||
String gcmId = "foo";
|
||||
|
||||
AccountsManager accountsManager = mock(AccountsManager.class);
|
||||
Sender sender = mock(Sender.class );
|
||||
Result invalidResult = mock(Result.class );
|
||||
DirectoryQueue directoryQueue = mock(DirectoryQueue.class );
|
||||
SynchronousExecutorService executorService = new SynchronousExecutorService();
|
||||
|
||||
Account destinationAccount = mock(Account.class);
|
||||
Device destinationDevice = mock(Device.class );
|
||||
|
||||
when(destinationAccount.getDevice(1)).thenReturn(Optional.of(destinationDevice));
|
||||
when(accountsManager.get(destinationNumber)).thenReturn(Optional.of(destinationAccount));
|
||||
when(destinationDevice.getGcmId()).thenReturn(gcmId);
|
||||
|
||||
when(invalidResult.isInvalidRegistrationId()).thenReturn(true);
|
||||
when(invalidResult.isUnregistered()).thenReturn(false);
|
||||
when(invalidResult.hasCanonicalRegistrationId()).thenReturn(false);
|
||||
when(invalidResult.isSuccess()).thenReturn(true);
|
||||
|
||||
GcmMessage message = new GcmMessage(gcmId, destinationNumber, 1, false);
|
||||
GCMSender gcmSender = new GCMSender(accountsManager, sender, directoryQueue, executorService);
|
||||
|
||||
SettableFuture<Result> invalidFuture = SettableFuture.create();
|
||||
invalidFuture.set(invalidResult);
|
||||
|
||||
when(sender.send(any(Message.class), Matchers.anyObject())).thenReturn(invalidFuture);
|
||||
when(invalidResult.getContext()).thenReturn(message);
|
||||
|
||||
gcmSender.sendMessage(message);
|
||||
|
||||
verify(sender, times(1)).send(any(Message.class), eq(message));
|
||||
verify(accountsManager, times(1)).get(eq(destinationNumber));
|
||||
verify(accountsManager, times(1)).update(eq(destinationAccount));
|
||||
verify(destinationDevice, times(1)).setUninstalledFeedbackTimestamp(eq(Util.todayInMillis()));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCanonicalId() {
|
||||
|
||||
@@ -241,7 +241,7 @@ public class AccountsTest {
|
||||
private Device generateDevice(long id) {
|
||||
Random random = new Random(System.currentTimeMillis());
|
||||
SignedPreKey signedPreKey = new SignedPreKey(random.nextInt(), "testPublicKey-" + random.nextInt(), "testSignature-" + random.nextInt());
|
||||
return new Device(id, "testName-" + random.nextInt(), "testAuthToken-" + random.nextInt(), "testSalt-" + random.nextInt(), null, "testGcmId-" + random.nextInt(), "testApnId-" + random.nextInt(), "testVoipApnId-" + random.nextInt(), random.nextBoolean(), random.nextInt(), signedPreKey, random.nextInt(), random.nextInt(), "testUserAgent-" + random.nextInt(), random.nextBoolean());
|
||||
return new Device(id, "testName-" + random.nextInt(), "testAuthToken-" + random.nextInt(), "testSalt-" + random.nextInt(), null, "testGcmId-" + random.nextInt(), "testApnId-" + random.nextInt(), "testVoipApnId-" + random.nextInt(), random.nextBoolean(), random.nextInt(), signedPreKey, random.nextInt(), random.nextInt(), "testUserAgent-" + random.nextInt(), random.nextBoolean(), 0);
|
||||
}
|
||||
|
||||
private Account generateAccount(String number) {
|
||||
|
||||
@@ -21,7 +21,7 @@ public class PublicAccountTest {
|
||||
|
||||
@Test
|
||||
public void testPinSanitation() throws IOException {
|
||||
Set<Device> devices = Collections.singleton(new Device(1, "foo", "bar", "12345", null, "gcm-1234", null, null, true, 1234, new SignedPreKey(1, "public-foo", "signature-foo"), 31337, 31336, "Android4Life", true));
|
||||
Set<Device> devices = Collections.singleton(new Device(1, "foo", "bar", "12345", null, "gcm-1234", null, null, true, 1234, new SignedPreKey(1, "public-foo", "signature-foo"), 31337, 31336, "Android4Life", true, 0));
|
||||
Account account = new Account("+14151231234", devices, new byte[16]);
|
||||
account.setPin("123456");
|
||||
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
package org.whispersystems.textsecuregcm.tests.storage;
|
||||
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.whispersystems.textsecuregcm.sqs.DirectoryQueue;
|
||||
import org.whispersystems.textsecuregcm.storage.Account;
|
||||
import org.whispersystems.textsecuregcm.storage.AccountsManager;
|
||||
import org.whispersystems.textsecuregcm.storage.Device;
|
||||
import org.whispersystems.textsecuregcm.storage.PushFeedbackProcessor;
|
||||
import org.whispersystems.textsecuregcm.util.Util;
|
||||
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import static org.mockito.Mockito.*;
|
||||
|
||||
public class PushFeedbackProcessorTest {
|
||||
|
||||
private AccountsManager accountsManager = mock(AccountsManager.class);
|
||||
private DirectoryQueue directoryQueue = mock(DirectoryQueue.class);
|
||||
|
||||
private Account uninstalledAccount = mock(Account.class);
|
||||
private Account mixedAccount = mock(Account.class);
|
||||
private Account freshAccount = mock(Account.class);
|
||||
private Account cleanAccount = mock(Account.class);
|
||||
private Account stillActiveAccount = mock(Account.class);
|
||||
|
||||
private Device uninstalledDevice = mock(Device.class);
|
||||
private Device uninstalledDeviceTwo = mock(Device.class);
|
||||
private Device installedDevice = mock(Device.class);
|
||||
private Device installedDeviceTwo = mock(Device.class);
|
||||
private Device recentUninstalledDevice = mock(Device.class);
|
||||
private Device stillActiveDevice = mock(Device.class);
|
||||
|
||||
@Before
|
||||
public void setup() {
|
||||
when(uninstalledDevice.getUninstalledFeedbackTimestamp()).thenReturn(Util.todayInMillis() - TimeUnit.DAYS.toMillis(2));
|
||||
when(uninstalledDevice.getLastSeen()).thenReturn(Util.todayInMillis() - TimeUnit.DAYS.toMillis(2));
|
||||
when(uninstalledDeviceTwo.getUninstalledFeedbackTimestamp()).thenReturn(Util.todayInMillis() - TimeUnit.DAYS.toMillis(3));
|
||||
when(uninstalledDeviceTwo.getLastSeen()).thenReturn(Util.todayInMillis() - TimeUnit.DAYS.toMillis(3));
|
||||
|
||||
when(installedDevice.getUninstalledFeedbackTimestamp()).thenReturn(0L);
|
||||
when(installedDeviceTwo.getUninstalledFeedbackTimestamp()).thenReturn(0L);
|
||||
|
||||
when(recentUninstalledDevice.getUninstalledFeedbackTimestamp()).thenReturn(Util.todayInMillis() - TimeUnit.DAYS.toMillis(1));
|
||||
when(recentUninstalledDevice.getLastSeen()).thenReturn(Util.todayInMillis());
|
||||
|
||||
when(stillActiveDevice.getUninstalledFeedbackTimestamp()).thenReturn(Util.todayInMillis() - TimeUnit.DAYS.toMillis(2));
|
||||
when(stillActiveDevice.getLastSeen()).thenReturn(Util.todayInMillis());
|
||||
|
||||
when(uninstalledAccount.getDevices()).thenReturn(Set.of(uninstalledDevice));
|
||||
when(mixedAccount.getDevices()).thenReturn(Set.of(installedDevice, uninstalledDeviceTwo));
|
||||
when(freshAccount.getDevices()).thenReturn(Set.of(recentUninstalledDevice));
|
||||
when(cleanAccount.getDevices()).thenReturn(Set.of(installedDeviceTwo));
|
||||
when(stillActiveAccount.getDevices()).thenReturn(Set.of(stillActiveDevice));
|
||||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void testEmpty() {
|
||||
PushFeedbackProcessor processor = new PushFeedbackProcessor(accountsManager, directoryQueue);
|
||||
processor.onCrawlChunk(Optional.of("+14152222222"), Collections.emptyList());
|
||||
|
||||
verifyZeroInteractions(accountsManager);
|
||||
verifyZeroInteractions(directoryQueue);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testUpdate() {
|
||||
PushFeedbackProcessor processor = new PushFeedbackProcessor(accountsManager, directoryQueue);
|
||||
processor.onCrawlChunk(Optional.of("+14153333333"), List.of(uninstalledAccount, mixedAccount, stillActiveAccount, freshAccount, cleanAccount));
|
||||
|
||||
verify(uninstalledDevice).setApnId(isNull());
|
||||
verify(uninstalledDevice).setGcmId(isNull());
|
||||
verify(uninstalledDevice).setFetchesMessages(eq(false));
|
||||
|
||||
verify(accountsManager).update(eq(uninstalledAccount));
|
||||
|
||||
verify(uninstalledDeviceTwo).setApnId(isNull());
|
||||
verify(uninstalledDeviceTwo).setGcmId(isNull());
|
||||
verify(uninstalledDeviceTwo).setFetchesMessages(eq(false));
|
||||
|
||||
verify(installedDevice, never()).setApnId(any());
|
||||
verify(installedDevice, never()).setGcmId(any());
|
||||
verify(installedDevice, never()).setFetchesMessages(anyBoolean());
|
||||
|
||||
verify(accountsManager).update(eq(mixedAccount));
|
||||
|
||||
verify(recentUninstalledDevice, never()).setApnId(any());
|
||||
verify(recentUninstalledDevice, never()).setGcmId(any());
|
||||
verify(recentUninstalledDevice, never()).setFetchesMessages(anyBoolean());
|
||||
|
||||
verify(accountsManager, never()).update(eq(freshAccount));
|
||||
|
||||
verify(installedDeviceTwo, never()).setApnId(any());
|
||||
verify(installedDeviceTwo, never()).setGcmId(any());
|
||||
verify(installedDeviceTwo, never()).setFetchesMessages(anyBoolean());
|
||||
|
||||
verify(accountsManager, never()).update(eq(cleanAccount));
|
||||
|
||||
verify(stillActiveDevice).setUninstalledFeedbackTimestamp(eq(0L));
|
||||
verify(stillActiveDevice, never()).setApnId(any());
|
||||
verify(stillActiveDevice, never()).setGcmId(any());
|
||||
verify(stillActiveDevice, never()).setFetchesMessages(anyBoolean());
|
||||
|
||||
verify(accountsManager).update(eq(stillActiveAccount));
|
||||
}
|
||||
|
||||
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user