Only delete profile avatars during explicit delete actions

This preserves the avatar during re-registration, when PIN recovery might occur.
This commit is contained in:
Chris Eager
2025-07-14 16:09:36 -05:00
committed by ravi-signal
parent 58b9fa100d
commit ca9f29f984
10 changed files with 43 additions and 24 deletions

View File

@@ -6,6 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -145,7 +146,7 @@ public class AccountCreationDeletionIntegrationTest {
when(messagesManager.clear(any())).thenReturn(CompletableFuture.completedFuture(null));
final ProfilesManager profilesManager = mock(ProfilesManager.class);
when(profilesManager.deleteAll(any())).thenReturn(CompletableFuture.completedFuture(null));
when(profilesManager.deleteAll(any(), anyBoolean())).thenReturn(CompletableFuture.completedFuture(null));
final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager =
mock(RegistrationRecoveryPasswordsManager.class);

View File

@@ -9,6 +9,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -138,7 +139,7 @@ class AccountsManagerChangeNumberIntegrationTest {
when(messagesManager.clear(any())).thenReturn(CompletableFuture.completedFuture(null));
final ProfilesManager profilesManager = mock(ProfilesManager.class);
when(profilesManager.deleteAll(any())).thenReturn(CompletableFuture.completedFuture(null));
when(profilesManager.deleteAll(any(), anyBoolean())).thenReturn(CompletableFuture.completedFuture(null));
final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager =
mock(RegistrationRecoveryPasswordsManager.class);

View File

@@ -14,6 +14,7 @@ import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyByte;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.anyLong;
@@ -231,7 +232,7 @@ class AccountsManagerTest {
when(keysManager.deleteSingleUsePreKeys(any())).thenReturn(CompletableFuture.completedFuture(null));
when(messagesManager.clear(any())).thenReturn(CompletableFuture.completedFuture(null));
when(profilesManager.deleteAll(any())).thenReturn(CompletableFuture.completedFuture(null));
when(profilesManager.deleteAll(any(), anyBoolean())).thenReturn(CompletableFuture.completedFuture(null));
CLOCK = TestClock.now();
@@ -867,7 +868,7 @@ class AccountsManagerTest {
verify(keysManager, times(2)).deleteSingleUsePreKeys(existingUuid);
verify(keysManager, times(2)).deleteSingleUsePreKeys(phoneNumberIdentifiersByE164.get(e164));
verify(messagesManager, times(2)).clear(existingUuid);
verify(profilesManager, times(2)).deleteAll(existingUuid);
verify(profilesManager, times(2)).deleteAll(existingUuid, false);
verify(disconnectionRequestManager).requestDisconnection(existingUuid);
}

View File

@@ -10,6 +10,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
@@ -145,7 +146,7 @@ class AccountsManagerUsernameIntegrationTest {
final MessagesManager messageManager = mock(MessagesManager.class);
final ProfilesManager profileManager = mock(ProfilesManager.class);
when(messageManager.clear(any())).thenReturn(CompletableFuture.completedFuture(null));
when(profileManager.deleteAll(any())).thenReturn(CompletableFuture.completedFuture(null));
when(profileManager.deleteAll(any(), anyBoolean())).thenReturn(CompletableFuture.completedFuture(null));
final DisconnectionRequestManager disconnectionRequestManager = mock(DisconnectionRequestManager.class);
when(disconnectionRequestManager.requestDisconnection(any())).thenReturn(CompletableFuture.completedFuture(null));

View File

@@ -6,6 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyByte;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.mock;
@@ -145,7 +146,7 @@ public class AddRemoveDeviceIntegrationTest {
when(messagesManager.clear(any(), anyByte())).thenReturn(CompletableFuture.completedFuture(null));
final ProfilesManager profilesManager = mock(ProfilesManager.class);
when(profilesManager.deleteAll(any())).thenReturn(CompletableFuture.completedFuture(null));
when(profilesManager.deleteAll(any(), anyBoolean())).thenReturn(CompletableFuture.completedFuture(null));
final RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager =
mock(RegistrationRecoveryPasswordsManager.class);

View File

@@ -15,6 +15,7 @@ import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
@@ -28,6 +29,8 @@ import java.util.concurrent.CompletableFuture;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.signal.libsignal.protocol.ServiceId;
import org.signal.libsignal.zkgroup.InvalidInputException;
import org.signal.libsignal.zkgroup.profiles.ProfileKey;
@@ -241,8 +244,9 @@ public class ProfilesManagerTest {
verifyNoMoreInteractions(profiles);
}
@Test
public void testDeleteAll() {
@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testDeleteAll(final boolean includeAvatar) {
final UUID uuid = UUID.randomUUID();
final String avatarOne = "avatar1";
@@ -253,17 +257,21 @@ public class ProfilesManagerTest {
.thenReturn(CompletableFuture.completedFuture(null))
.thenReturn(CompletableFuture.failedFuture(new RuntimeException("some error")));
profilesManager.deleteAll(uuid).join();
profilesManager.deleteAll(uuid, includeAvatar).join();
verify(profiles).deleteAll(uuid);
verify(asyncCommands).del(ProfilesManager.getCacheKey(uuid));
verify(s3Client).deleteObject(DeleteObjectRequest.builder()
.bucket(BUCKET)
.key(avatarOne)
.build());
verify(s3Client).deleteObject(DeleteObjectRequest.builder()
.bucket(BUCKET)
.key(avatarTwo)
.build());
if (includeAvatar) {
verify(s3Client).deleteObject(DeleteObjectRequest.builder()
.bucket(BUCKET)
.key(avatarOne)
.build());
verify(s3Client).deleteObject(DeleteObjectRequest.builder()
.bucket(BUCKET)
.key(avatarTwo)
.build());
} else {
verifyNoInteractions(s3Client);
}
}
}