diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PushFeedbackProcessor.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PushFeedbackProcessor.java index c522999d4..0565c5623 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/PushFeedbackProcessor.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/PushFeedbackProcessor.java @@ -60,30 +60,33 @@ public class PushFeedbackProcessor extends AccountDatabaseCrawlerListener { } if (update) { - account = accountsManager.update(account, a -> { - for (Device device: a.getDevices()) { - if (deviceNeedsUpdate(device)) { - if (deviceExpired(device)) { - if (!Util.isEmpty(device.getApnId())) { - if (device.getId() == 1) { - device.setUserAgent("OWI"); - } else { - device.setUserAgent("OWP"); + // fetch a new version, since the chunk is shared and implicitly read-only + accountsManager.get(account.getUuid()).ifPresent(accountToUpdate -> { + Account updatedAccount = accountsManager.update(accountToUpdate, a -> { + for (Device device : a.getDevices()) { + if (deviceNeedsUpdate(device)) { + if (deviceExpired(device)) { + if (!Util.isEmpty(device.getApnId())) { + if (device.getId() == 1) { + device.setUserAgent("OWI"); + } else { + device.setUserAgent("OWP"); + } + } else if (!Util.isEmpty(device.getGcmId())) { + device.setUserAgent("OWA"); } - } else if (!Util.isEmpty(device.getGcmId())) { - device.setUserAgent("OWA"); + device.setGcmId(null); + device.setApnId(null); + device.setVoipApnId(null); + device.setFetchesMessages(false); + } else { + device.setUninstalledFeedbackTimestamp(0); } - device.setGcmId(null); - device.setApnId(null); - device.setVoipApnId(null); - device.setFetchesMessages(false); - } else { - device.setUninstalledFeedbackTimestamp(0); } } - } + }); + directoryUpdateAccounts.add(updatedAccount); }); - directoryUpdateAccounts.add(account); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PushFeedbackProcessorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PushFeedbackProcessorTest.java index 35e0eddba..bd4a8b65b 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PushFeedbackProcessorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/PushFeedbackProcessorTest.java @@ -15,6 +15,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; +import static org.whispersystems.textsecuregcm.tests.util.AccountsHelper.eqUuid; import java.util.Collections; import java.util.List; @@ -83,6 +84,11 @@ class PushFeedbackProcessorTest { when(stillActiveAccount.getDevices()).thenReturn(Set.of(stillActiveDevice)); when(undiscoverableAccount.getDevices()).thenReturn(Set.of(undiscoverableDevice)); + when(mixedAccount.getUuid()).thenReturn(UUID.randomUUID()); + when(freshAccount.getUuid()).thenReturn(UUID.randomUUID()); + when(cleanAccount.getUuid()).thenReturn(UUID.randomUUID()); + when(stillActiveAccount.getUuid()).thenReturn(UUID.randomUUID()); + when(uninstalledAccount.isEnabled()).thenReturn(true); when(uninstalledAccount.isDiscoverableByPhoneNumber()).thenReturn(true); when(uninstalledAccount.getUuid()).thenReturn(UUID.randomUUID()); @@ -92,6 +98,8 @@ class PushFeedbackProcessorTest { when(undiscoverableAccount.isDiscoverableByPhoneNumber()).thenReturn(false); when(undiscoverableAccount.getUuid()).thenReturn(UUID.randomUUID()); when(undiscoverableAccount.getNumber()).thenReturn("+18005559876"); + + AccountsHelper.setupMockGet(accountsManager, Set.of(uninstalledAccount, mixedAccount, freshAccount, cleanAccount, stillActiveAccount, undiscoverableAccount)); } @@ -113,7 +121,7 @@ class PushFeedbackProcessorTest { verify(uninstalledDevice).setGcmId(isNull()); verify(uninstalledDevice).setFetchesMessages(eq(false)); - verify(accountsManager).update(eq(uninstalledAccount), any()); + verify(accountsManager).update(eqUuid(uninstalledAccount), any()); verify(uninstalledDeviceTwo).setApnId(isNull()); verify(uninstalledDeviceTwo).setGcmId(isNull()); @@ -123,26 +131,26 @@ class PushFeedbackProcessorTest { verify(installedDevice, never()).setGcmId(any()); verify(installedDevice, never()).setFetchesMessages(anyBoolean()); - verify(accountsManager).update(eq(mixedAccount), any()); + verify(accountsManager).update(eqUuid(mixedAccount), any()); verify(recentUninstalledDevice, never()).setApnId(any()); verify(recentUninstalledDevice, never()).setGcmId(any()); verify(recentUninstalledDevice, never()).setFetchesMessages(anyBoolean()); - verify(accountsManager, never()).update(eq(freshAccount), any()); + verify(accountsManager, never()).update(eqUuid(freshAccount), any()); verify(installedDeviceTwo, never()).setApnId(any()); verify(installedDeviceTwo, never()).setGcmId(any()); verify(installedDeviceTwo, never()).setFetchesMessages(anyBoolean()); - verify(accountsManager, never()).update(eq(cleanAccount), any()); + verify(accountsManager, never()).update(eqUuid(cleanAccount), any()); 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), any()); + verify(accountsManager).update(eqUuid(stillActiveAccount), any()); final ArgumentCaptor> refreshedAccountArgumentCaptor = ArgumentCaptor.forClass(List.class); verify(directoryQueue).refreshRegisteredUsers(refreshedAccountArgumentCaptor.capture()); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java index 8f5287244..3ac9f34c0 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java @@ -14,6 +14,8 @@ import static org.mockito.Mockito.when; import com.fasterxml.jackson.databind.ObjectMapper; import java.io.IOException; +import java.util.Set; +import java.util.UUID; import java.util.function.Consumer; import org.mockito.MockingDetails; import org.mockito.stubbing.Stubbing; @@ -40,6 +42,24 @@ public class AccountsHelper { }); } + public static void setupMockGet(final AccountsManager mockAccountsManager, final Set mockAccounts) { + when(mockAccountsManager.get(any(UUID.class))).thenAnswer(answer -> { + + final UUID uuid = answer.getArgument(0, UUID.class); + + return mockAccounts.stream() + .filter(account -> uuid.equals(account.getUuid())) + .findFirst() + .map(account -> { + try { + return copyAndMarkStale(account); + } catch (final Exception e) { + throw new RuntimeException(e); + } + }); + }); + } + private static Account copyAndMarkStale(Account account) throws IOException { MockingDetails mockingDetails = mockingDetails(account);