diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 9c6e3ade7..7891e938c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -56,8 +56,8 @@ import org.glassfish.jersey.server.ServerProperties; import org.signal.event.AdminEventLogger; import org.signal.event.GoogleCloudAdminEventLogger; import org.signal.i18n.HeaderControlledResourceBundleLookup; -import org.signal.libsignal.zkgroup.ServerSecretParams; import org.signal.libsignal.zkgroup.GenericServerSecretParams; +import org.signal.libsignal.zkgroup.ServerSecretParams; import org.signal.libsignal.zkgroup.auth.ServerZkAuthOperations; import org.signal.libsignal.zkgroup.profiles.ServerZkProfileOperations; import org.signal.libsignal.zkgroup.receipts.ReceiptCredentialPresentation; @@ -172,7 +172,6 @@ import org.whispersystems.textsecuregcm.storage.AccountDatabaseCrawlerListener; import org.whispersystems.textsecuregcm.storage.Accounts; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.ChangeNumberManager; -import org.whispersystems.textsecuregcm.storage.ContactDiscoveryWriter; import org.whispersystems.textsecuregcm.storage.DeletedAccounts; import org.whispersystems.textsecuregcm.storage.DeletedAccountsManager; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; @@ -571,7 +570,6 @@ public class WhisperServerService extends Application accountDatabaseCrawlerListeners = List.of( new NonNormalizedAccountCrawlerListener(accountsManager, metricsCluster), - new ContactDiscoveryWriter(accountsManager), // PushFeedbackProcessor may update device properties new PushFeedbackProcessor(accountsManager)); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java deleted file mode 100644 index 56d4d3adf..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright 2013-2021 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.storage; - -import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name; - -import com.google.common.annotations.VisibleForTesting; -import io.micrometer.core.instrument.Counter; -import io.micrometer.core.instrument.Metrics; -import java.util.List; -import java.util.Optional; -import java.util.UUID; -import java.util.function.Consumer; - -public class ContactDiscoveryWriter extends AccountDatabaseCrawlerListener { - private final AccountsManager accounts; - - private final Counter updatedAccountsCounter; - - public ContactDiscoveryWriter(final AccountsManager accounts) { - this.accounts = accounts; - this.updatedAccountsCounter = Metrics.counter(name(ContactDiscoveryWriter.class, "updatedAccounts")); - } - - @Override - public void onCrawlStart() { - // nothing - } - - @Override - public void onCrawlEnd(final Optional fromUuid) { - // nothing - } - - // We "update" by doing nothing, since everything about the account is already accurate except for a temporal - // change in the 'shouldBeVisible' trait. This update forces a new write of the underlying DB to reflect - // that temporal change persistently. - @VisibleForTesting - static final Consumer NOOP_UPDATER = a -> {}; - - @Override - protected void onCrawlChunk(final Optional fromUuid, final List chunkAccounts) - throws AccountDatabaseCrawlerRestartException { - for (Account account : chunkAccounts) { - if (account.isCanonicallyDiscoverable() != account.shouldBeVisibleInDirectory()) { - // It’s less than ideal, but crawler listeners currently must not call update() - // with the accounts from the chunk, because updates cause the account instance to become stale. Instead, they - // must get a new copy, which they are free to update. - accounts.getByAccountIdentifier(account.getUuid()).ifPresent(a -> accounts.update(a, NOOP_UPDATER)); - updatedAccountsCounter.increment(); - } - } - } -} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriterTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriterTest.java deleted file mode 100644 index 5fd24dfc0..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriterTest.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright 2013-2021 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.storage; - -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; - -import java.util.List; -import java.util.Optional; -import java.util.UUID; -import java.util.stream.Stream; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -public class ContactDiscoveryWriterTest { - - @ParameterizedTest - @MethodSource - void testUpdatesOnChange(boolean canonicallyDiscoverable, boolean shouldBeVisible, boolean updateCalled) throws AccountDatabaseCrawlerRestartException { - AccountsManager mgr = mock(AccountsManager.class); - UUID uuid = UUID.randomUUID(); - Account acct = mock(Account.class); - when(acct.getUuid()).thenReturn(uuid); - when(acct.isCanonicallyDiscoverable()).thenReturn(canonicallyDiscoverable); - when(acct.shouldBeVisibleInDirectory()).thenReturn(shouldBeVisible); - when(mgr.getByAccountIdentifier(uuid)).thenReturn(Optional.of(acct)); - ContactDiscoveryWriter writer = new ContactDiscoveryWriter(mgr); - writer.onCrawlChunk(Optional.empty(), List.of(acct)); - verify(mgr, times(updateCalled ? 1 : 0)).update(acct, ContactDiscoveryWriter.NOOP_UPDATER); - } - - static Stream testUpdatesOnChange() { - return Stream.of( - Arguments.of(true, true, false), - Arguments.of(false, false, false), - Arguments.of(true, false, true), - Arguments.of(false, true, true)); - } -}