MirrorMirror
/
Signal-Server
mirror of https://github.com/signalapp/Signal-Server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Really delete old accounts instead of just removing their push channels.

This commit is contained in:
Jon Chambers 2020-10-01 15:12:58 -04:00 committed by Jon Chambers
parent f0a8aa06bc
commit 2b6811cb1b
3 changed files with 9 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
View File

@ -332,7 +332,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
ActiveUserCounter activeUserCounter = new ActiveUserCounter(config.getMetricsFactory(), cacheCluster); ActiveUserCounter activeUserCounter = new ActiveUserCounter(config.getMetricsFactory(), cacheCluster);
DirectoryReconciler directoryReconciler = new DirectoryReconciler(directoryReconciliationClient, directory); DirectoryReconciler directoryReconciler = new DirectoryReconciler(directoryReconciliationClient, directory);
AccountCleaner accountCleaner = new AccountCleaner(accountsManager, directoryQueue); AccountCleaner accountCleaner = new AccountCleaner(accountsManager);
PushFeedbackProcessor pushFeedbackProcessor = new PushFeedbackProcessor(accountsManager, directoryQueue); PushFeedbackProcessor pushFeedbackProcessor = new PushFeedbackProcessor(accountsManager, directoryQueue);
RegistrationLockVersionCounter registrationLockVersionCounter = new RegistrationLockVersionCounter(metricsCluster, config.getMetricsFactory()); RegistrationLockVersionCounter registrationLockVersionCounter = new RegistrationLockVersionCounter(metricsCluster, config.getMetricsFactory());
List<AccountDatabaseCrawlerListener> accountDatabaseCrawlerListeners = List.of(pushFeedbackProcessor, activeUserCounter, directoryReconciler, accountCleaner, registrationLockVersionCounter); List<AccountDatabaseCrawlerListener> accountDatabaseCrawlerListeners = List.of(pushFeedbackProcessor, activeUserCounter, directoryReconciler, accountCleaner, registrationLockVersionCounter);

14
service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java
View File

@ -20,7 +20,6 @@ import com.codahale.metrics.Meter;
import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.MetricRegistry;
import com.codahale.metrics.SharedMetricRegistries; import com.codahale.metrics.SharedMetricRegistries;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import org.whispersystems.textsecuregcm.sqs.DirectoryQueue;
import org.whispersystems.textsecuregcm.util.Constants; import org.whispersystems.textsecuregcm.util.Constants;
import org.whispersystems.textsecuregcm.util.Util; import org.whispersystems.textsecuregcm.util.Util;
@ -40,11 +39,9 @@ public class AccountCleaner extends AccountDatabaseCrawlerListener {
public static final int MAX_ACCOUNT_UPDATES_PER_CHUNK = 40; public static final int MAX_ACCOUNT_UPDATES_PER_CHUNK = 40;
private final AccountsManager accountsManager; private final AccountsManager accountsManager;
private final DirectoryQueue directoryQueue;
public AccountCleaner(AccountsManager accountsManager, DirectoryQueue directoryQueue) { public AccountCleaner(AccountsManager accountsManager) {
this.accountsManager = accountsManager; this.accountsManager = accountsManager;
this.directoryQueue = directoryQueue;
} }
@Override @Override
@ -63,15 +60,8 @@ public class AccountCleaner extends AccountDatabaseCrawlerListener {
expiredAccountsMeter.mark(); expiredAccountsMeter.mark();
if (accountUpdateCount < MAX_ACCOUNT_UPDATES_PER_CHUNK) { if (accountUpdateCount < MAX_ACCOUNT_UPDATES_PER_CHUNK) {
Device masterDevice = account.getMasterDevice().get(); accountsManager.delete(account);
masterDevice.setFetchesMessages(false);
masterDevice.setApnId(null);
masterDevice.setVoipApnId(null);
masterDevice.setGcmId(null);
accountUpdateCount++; accountUpdateCount++;
accountsManager.update(account);
directoryQueue.refreshRegisteredUser(account);
} }
} }
} }

56
service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountCleanerTest.java
View File

@ -18,7 +18,6 @@ package org.whispersystems.textsecuregcm.tests.storage;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.whispersystems.textsecuregcm.sqs.DirectoryQueue;
import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.AccountCleaner; import org.whispersystems.textsecuregcm.storage.AccountCleaner;
import org.whispersystems.textsecuregcm.storage.AccountDatabaseCrawlerRestartException; import org.whispersystems.textsecuregcm.storage.AccountDatabaseCrawlerRestartException;
@ -32,10 +31,6 @@ import java.util.Optional;
import java.util.UUID; import java.util.UUID;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times; import static org.mockito.Mockito.times;
@ -46,7 +41,6 @@ import static org.mockito.Mockito.when;
public class AccountCleanerTest { public class AccountCleanerTest {
private final AccountsManager accountsManager = mock(AccountsManager.class); private final AccountsManager accountsManager = mock(AccountsManager.class);
private final DirectoryQueue directoryQueue = mock(DirectoryQueue.class);
private final Account deletedDisabledAccount = mock(Account.class); private final Account deletedDisabledAccount = mock(Account.class);
private final Account undeletedDisabledAccount = mock(Account.class); private final Account undeletedDisabledAccount = mock(Account.class);
@ -89,37 +83,16 @@ public class AccountCleanerTest {
@Test @Test
public void testAccounts() throws AccountDatabaseCrawlerRestartException { public void testAccounts() throws AccountDatabaseCrawlerRestartException {
AccountCleaner accountCleaner = new AccountCleaner(accountsManager, directoryQueue); AccountCleaner accountCleaner = new AccountCleaner(accountsManager);
accountCleaner.onCrawlStart(); accountCleaner.onCrawlStart();
accountCleaner.timeAndProcessCrawlChunk(Optional.empty(), Arrays.asList(deletedDisabledAccount, undeletedDisabledAccount, undeletedEnabledAccount)); accountCleaner.timeAndProcessCrawlChunk(Optional.empty(), Arrays.asList(deletedDisabledAccount, undeletedDisabledAccount, undeletedEnabledAccount));
accountCleaner.onCrawlEnd(Optional.empty()); accountCleaner.onCrawlEnd(Optional.empty());
verify(deletedDisabledDevice, never()).setGcmId(any()); verify(accountsManager, never()).delete(deletedDisabledAccount);
verify(deletedDisabledDevice, never()).setApnId(any()); verify(accountsManager).delete(undeletedDisabledAccount);
verify(deletedDisabledDevice, never()).setVoipApnId(any()); verify(accountsManager, never()).delete(undeletedEnabledAccount);
verify(deletedDisabledDevice, never()).setFetchesMessages(anyBoolean());
verify(accountsManager, never()).update(eq(deletedDisabledAccount));
verify(directoryQueue, never()).refreshRegisteredUser(deletedDisabledAccount);
verify(undeletedDisabledDevice, times(1)).setGcmId(isNull());
verify(undeletedDisabledDevice, times(1)).setApnId(isNull());
verify(undeletedDisabledDevice, times(1)).setVoipApnId(isNull());
verify(undeletedDisabledDevice, times(1)).setFetchesMessages(eq(false));
verify(accountsManager, times(1)).update(eq(undeletedDisabledAccount));
verify(directoryQueue, times(1)).refreshRegisteredUser(undeletedDisabledAccount);
verify(undeletedEnabledDevice, never()).setGcmId(any());
verify(undeletedEnabledDevice, never()).setApnId(any());
verify(undeletedEnabledDevice, never()).setVoipApnId(any());
verify(undeletedEnabledDevice, never()).setFetchesMessages(anyBoolean());
verify(accountsManager, never()).update(eq(undeletedEnabledAccount));
verify(directoryQueue, never()).refreshRegisteredUser(undeletedEnabledAccount);
verifyNoMoreInteractions(accountsManager); verifyNoMoreInteractions(accountsManager);
verifyNoMoreInteractions(directoryQueue);
} }
@Test @Test
@ -134,30 +107,13 @@ public class AccountCleanerTest {
accounts.add(deletedDisabledAccount); accounts.add(deletedDisabledAccount);
AccountCleaner accountCleaner = new AccountCleaner(accountsManager, directoryQueue); AccountCleaner accountCleaner = new AccountCleaner(accountsManager);
accountCleaner.onCrawlStart(); accountCleaner.onCrawlStart();
accountCleaner.timeAndProcessCrawlChunk(Optional.empty(), accounts); accountCleaner.timeAndProcessCrawlChunk(Optional.empty(), accounts);
accountCleaner.onCrawlEnd(Optional.empty()); accountCleaner.onCrawlEnd(Optional.empty());
verify(undeletedDisabledDevice, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).setGcmId(isNull()); verify(accountsManager, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).delete(undeletedDisabledAccount);
verify(undeletedDisabledDevice, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).setApnId(isNull());
verify(undeletedDisabledDevice, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).setVoipApnId(isNull());
verify(undeletedDisabledDevice, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).setFetchesMessages(eq(false));
verify(accountsManager, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).update(eq(undeletedDisabledAccount));
verify(directoryQueue, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).refreshRegisteredUser(undeletedDisabledAccount);
verify(deletedDisabledDevice, never()).setGcmId(any());
verify(deletedDisabledDevice, never()).setApnId(any());
verify(deletedDisabledDevice, never()).setFetchesMessages(anyBoolean());
verify(undeletedEnabledDevice, never()).setGcmId(any());
verify(undeletedEnabledDevice, never()).setApnId(any());
verify(undeletedEnabledDevice, never()).setVoipApnId(any());
verify(undeletedEnabledDevice, never()).setFetchesMessages(anyBoolean());
verifyNoMoreInteractions(accountsManager); verifyNoMoreInteractions(accountsManager);
verifyNoMoreInteractions(directoryQueue);
} }
} }