Skip to content

Commit

Permalink
Simplify storing storage-service-specific recipient values.
Browse files Browse the repository at this point in the history 
This gives us the ability to separate things we need for the Recipient
class from things we only need for storage syncing.

Not only does this simplify the storage service model building code
(i.e. we no longer need to pass around a set of archived recipients),
but it also eliminates a join on the Identity table for building regular
recipients, which should help perf.
  • Loading branch information
@greyson-signal
greyson-signal committed Oct 7, 2020
1 parent 38fa58c commit e003976
Show file tree
Hide file tree
Showing 11 changed files with 180 additions and 142 deletions.
201 changes: 117 additions & 84 deletions app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public class ThreadDatabase extends Database {
.toList();

private static final List<String> COMBINED_THREAD_RECIPIENT_GROUP_PROJECTION = Stream.concat(Stream.concat(Stream.of(TYPED_THREAD_PROJECTION),
Stream.of(RecipientDatabase.TYPED_RECIPIENT_PROJECTION)),
Stream.of(RecipientDatabase.TYPED_RECIPIENT_PROJECTION_NO_ID)),
Stream.of(GroupDatabase.TYPED_GROUP_PROJECTION))
.toList();

Expand Down
4 changes: 2 additions & 2 deletions app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ private void handleCallOfferMessage(@NonNull SignalServiceContent content,
Intent intent = new Intent(context, WebRtcCallService.class);
Recipient recipient = Recipient.externalHighTrustPush(context, content.getSender());
RemotePeer remotePeer = new RemotePeer(recipient.getId());
byte[] remoteIdentityKey = recipient.getIdentityKey();
byte[] remoteIdentityKey = DatabaseFactory.getIdentityDatabase(context).getIdentity(recipient.getId()).transform(record -> record.getIdentityKey().serialize()).orNull();

intent.setAction(WebRtcCallService.ACTION_RECEIVE_OFFER)
.putExtra(WebRtcCallService.EXTRA_CALL_ID, message.getId())
Expand All @@ -538,7 +538,7 @@ private void handleCallAnswerMessage(@NonNull SignalServiceContent content,
Intent intent = new Intent(context, WebRtcCallService.class);
Recipient recipient = Recipient.externalHighTrustPush(context, content.getSender());
RemotePeer remotePeer = new RemotePeer(recipient.getId());
byte[] remoteIdentityKey = recipient.getIdentityKey();
byte[] remoteIdentityKey = DatabaseFactory.getIdentityDatabase(context).getIdentity(recipient.getId()).transform(record -> record.getIdentityKey().serialize()).orNull();

intent.setAction(WebRtcCallService.ACTION_RECEIVE_ANSWER)
.putExtra(WebRtcCallService.EXTRA_CALL_ID, message.getId())
Expand Down
3 changes: 1 addition & 2 deletions app/src/main/java/org/thoughtcrime/securesms/jobs/StorageForcePushJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,10 @@ protected void onRun() throws IOException, RetryLaterException {

long newVersion = currentVersion + 1;
Map<RecipientId, StorageId> newContactStorageIds = generateContactStorageIds(oldContactStorageIds);
Set<RecipientId> archivedRecipients = DatabaseFactory.getThreadDatabase(context).getArchivedRecipients();
List<SignalStorageRecord> inserts = Stream.of(oldContactStorageIds.keySet())
.map(recipientDatabase::getRecipientSettingsForSync)
.withoutNulls()
.map(s -> StorageSyncModels.localToRemoteRecord(s, Objects.requireNonNull(newContactStorageIds.get(s.getId())).getRaw(), archivedRecipients))
.map(s -> StorageSyncModels.localToRemoteRecord(s, Objects.requireNonNull(newContactStorageIds.get(s.getId())).getRaw()))
.toList();

SignalStorageRecord accountRecord = StorageSyncHelper.buildAccountRecord(context, Recipient.self().fresh());
Expand Down
13 changes: 5 additions & 8 deletions app/src/main/java/org/thoughtcrime/securesms/jobs/StorageSyncJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ private boolean performSync() throws IOException, RetryLaterException, InvalidKe
if (!keyDifference.isEmpty()) {
Log.i(TAG, "[Remote Newer] There's a difference in keys. Local-only: " + keyDifference.getLocalOnlyKeys().size() + ", Remote-only: " + keyDifference.getRemoteOnlyKeys().size());

Set<RecipientId> archivedRecipients = DatabaseFactory.getThreadDatabase(context).getArchivedRecipients();
List<SignalStorageRecord> localOnly = buildLocalStorageRecords(context, keyDifference.getLocalOnlyKeys(), archivedRecipients);
List<SignalStorageRecord> localOnly = buildLocalStorageRecords(context, keyDifference.getLocalOnlyKeys());
List<SignalStorageRecord> remoteOnly = accountManager.readStorageRecords(storageServiceKey, keyDifference.getRemoteOnlyKeys());
MergeResult mergeResult = StorageSyncHelper.resolveConflict(remoteOnly, localOnly);
WriteOperationResult writeOperationResult = StorageSyncHelper.createWriteOperation(remoteManifest.get().getVersion(), allLocalStorageKeys, mergeResult);
Expand Down Expand Up @@ -212,15 +211,13 @@ private boolean performSync() throws IOException, RetryLaterException, InvalidKe
List<RecipientSettings> pendingDeletions = recipientDatabase.getPendingRecipientSyncDeletions();
Optional<SignalAccountRecord> pendingAccountInsert = StorageSyncHelper.getPendingAccountSyncInsert(context, self);
Optional<SignalAccountRecord> pendingAccountUpdate = StorageSyncHelper.getPendingAccountSyncUpdate(context, self);
Set<RecipientId> archivedRecipients = DatabaseFactory.getThreadDatabase(context).getArchivedRecipients();
Optional<LocalWriteResult> localWriteResult = StorageSyncHelper.buildStorageUpdatesForLocal(localManifestVersion,
allLocalStorageKeys,
pendingUpdates,
pendingInsertions,
pendingDeletions,
pendingAccountUpdate,
pendingAccountInsert,
archivedRecipients);
pendingAccountInsert);

if (localWriteResult.isPresent()) {
Log.i(TAG, String.format(Locale.ENGLISH, "[Local Changes] Local changes present. %d updates, %d inserts, %d deletes, account update: %b, account insert: %b.", pendingUpdates.size(), pendingInsertions.size(), pendingDeletions.size(), pendingAccountUpdate.isPresent(), pendingAccountInsert.isPresent()));
Expand Down Expand Up @@ -273,7 +270,7 @@ private boolean performSync() throws IOException, RetryLaterException, InvalidKe
DatabaseFactory.getStorageKeyDatabase(context).getAllKeys());
}

private static @NonNull List<SignalStorageRecord> buildLocalStorageRecords(@NonNull Context context, @NonNull List<StorageId> ids, @NonNull Set<RecipientId> archivedRecipients) {
private static @NonNull List<SignalStorageRecord> buildLocalStorageRecords(@NonNull Context context, @NonNull List<StorageId> ids) {
Recipient self = Recipient.self().fresh();
RecipientDatabase recipientDatabase = DatabaseFactory.getRecipientDatabase(context);
StorageKeyDatabase storageKeyDatabase = DatabaseFactory.getStorageKeyDatabase(context);
Expand All @@ -287,10 +284,10 @@ private boolean performSync() throws IOException, RetryLaterException, InvalidKe
case ManifestRecord.Identifier.Type.GROUPV2_VALUE:
RecipientSettings settings = recipientDatabase.getByStorageId(id.getRaw());
if (settings != null) {
if (settings.getGroupType() == RecipientDatabase.GroupType.SIGNAL_V2 && settings.getGroupMasterKey() == null) {
if (settings.getGroupType() == RecipientDatabase.GroupType.SIGNAL_V2 && settings.getSyncExtras().getGroupMasterKey() == null) {
Log.w(TAG, "Missing master key on gv2 recipient");
} else {
records.add(StorageSyncModels.localToRemoteRecord(settings, archivedRecipients));
records.add(StorageSyncModels.localToRemoteRecord(settings));
}
} else {
Log.w(TAG, "Missing local recipient model! Type: " + id.getType());
Expand Down
14 changes: 0 additions & 14 deletions app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,6 @@ public class Recipient {
private final Capability groupsV2Capability;
private final InsightsBannerTier insightsBannerTier;
private final byte[] storageId;
private final byte[] identityKey;
private final VerifiedStatus identityStatus;
private final MentionSetting mentionSetting;


Expand Down Expand Up @@ -317,8 +315,6 @@ public class Recipient {
this.uuidCapability = Capability.UNKNOWN;
this.groupsV2Capability = Capability.UNKNOWN;
this.storageId = null;
this.identityKey = null;
this.identityStatus = VerifiedStatus.DEFAULT;
this.mentionSetting = MentionSetting.ALWAYS_NOTIFY;
}

Expand Down Expand Up @@ -361,8 +357,6 @@ public Recipient(@NonNull RecipientId id, @NonNull RecipientDetails details, boo
this.uuidCapability = details.uuidCapability;
this.groupsV2Capability = details.groupsV2Capability;
this.storageId = details.storageId;
this.identityKey = details.identityKey;
this.identityStatus = details.identityStatus;
this.mentionSetting = details.mentionSetting;
}

Expand Down Expand Up @@ -782,14 +776,6 @@ public boolean hasProfileKeyCredential() {
return storageId;
}

public @NonNull VerifiedStatus getIdentityVerifiedStatus() {
return identityStatus;
}

public @Nullable byte[] getIdentityKey() {
return identityKey;
}

public @NonNull UnidentifiedAccessMode getUnidentifiedAccessMode() {
return unidentifiedAccessMode;
}
Expand Down
8 changes: 1 addition & 7 deletions app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ public class RecipientDetails {
final Recipient.Capability uuidCapability;
final Recipient.Capability groupsV2Capability;
final InsightsBannerTier insightsBannerTier;
final byte[] storageId;
final byte[] identityKey;
final VerifiedStatus identityStatus;
final byte[] storageId;
final MentionSetting mentionSetting;

public RecipientDetails(@Nullable String name,
Expand Down Expand Up @@ -113,8 +111,6 @@ public RecipientDetails(@Nullable String name,
this.groupsV2Capability = settings.getGroupsV2Capability();
this.insightsBannerTier = settings.getInsightsBannerTier();
this.storageId = settings.getStorageId();
this.identityKey = settings.getIdentityKey();
this.identityStatus = settings.getIdentityStatus();
this.mentionSetting = settings.getMentionSetting();

if (name == null) this.name = settings.getSystemDisplayName();
Expand Down Expand Up @@ -162,8 +158,6 @@ public RecipientDetails(@Nullable String name,
this.uuidCapability = Recipient.Capability.UNKNOWN;
this.groupsV2Capability = Recipient.Capability.UNKNOWN;
this.storageId = null;
this.identityKey = null;
this.identityStatus = VerifiedStatus.DEFAULT;
this.mentionSetting = MentionSetting.ALWAYS_NOTIFY;
}

Expand Down
11 changes: 5 additions & 6 deletions app/src/main/java/org/thoughtcrime/securesms/storage/StorageSyncHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ public final class StorageSyncHelper {
@NonNull List<RecipientSettings> inserts,
@NonNull List<RecipientSettings> deletes,
@NonNull Optional<SignalAccountRecord> accountUpdate,
@NonNull Optional<SignalAccountRecord> accountInsert,
@NonNull Set<RecipientId> archivedRecipients)
@NonNull Optional<SignalAccountRecord> accountInsert)
{
int accountCount = Stream.of(currentLocalKeys)
.filter(id -> id.getType() == ManifestRecord.Identifier.Type.ACCOUNT_VALUE)
Expand Down Expand Up @@ -119,12 +118,12 @@ public final class StorageSyncHelper {
Map<RecipientId, byte[]> storageKeyUpdates = new HashMap<>();

for (RecipientSettings insert : inserts) {
if (insert.getGroupType() == RecipientDatabase.GroupType.SIGNAL_V2 && insert.getGroupMasterKey() == null) {
if (insert.getGroupType() == RecipientDatabase.GroupType.SIGNAL_V2 && insert.getSyncExtras().getGroupMasterKey() == null) {
Log.w(TAG, "Missing master key on gv2 recipient");
continue;
}

storageInserts.add(StorageSyncModels.localToRemoteRecord(insert, archivedRecipients));
storageInserts.add(StorageSyncModels.localToRemoteRecord(insert));

switch (insert.getGroupType()) {
case NONE:
Expand Down Expand Up @@ -173,7 +172,7 @@ public final class StorageSyncHelper {
throw new AssertionError("Unsupported type!");
}

storageInserts.add(StorageSyncModels.localToRemoteRecord(update, newId.getRaw(), archivedRecipients));
storageInserts.add(StorageSyncModels.localToRemoteRecord(update, newId.getRaw()));
storageDeletes.add(ByteBuffer.wrap(oldId.getRaw()));
completeIds.remove(oldId);
completeIds.add(newId);
Expand Down Expand Up @@ -413,7 +412,7 @@ public static SignalStorageRecord buildAccountRecord(@NonNull Context context, @
RecipientSettings settings = DatabaseFactory.getRecipientDatabase(context).getRecipientSettingsForSync(self.getId());

SignalAccountRecord account = new SignalAccountRecord.Builder(self.getStorageServiceId())
.setUnknownFields(settings != null ? settings.getStorageProto() : null)
.setUnknownFields(settings != null ? settings.getSyncExtras().getStorageProto() : null)
.setProfileKey(self.getProfileKey())
.setGivenName(self.getProfileName().getGivenName())
.setFamilyName(self.getProfileName().getFamilyName())
Expand Down

0 comments on commit e003976

Please sign in to comment.