Reduce number of db calls to getGroup.

This commit is contained in:
Clark 2023-05-11 13:20:15 -04:00 committed by Greyson Parrelli
parent 6da36fe098
commit e9f1f781e1
14 changed files with 105 additions and 52 deletions

View file

@ -287,6 +287,15 @@ class GroupTable(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT
return getGroup(SqlUtil.Query("$TABLE_NAME.$EXPECTED_V2_ID = ?", buildArgs(gv2Id)))
}
/**
* @return A gv1 group whose expected v2 ID matches the one provided or a gv2 group whose ID matches the one provided.
*
* If a gv1 group is present, it will be returned first.
*/
fun getGroupV1OrV2ByExpectedV2(gv2Id: GroupId.V2): Optional<GroupRecord> {
return getGroup(SqlUtil.Query("$TABLE_NAME.$EXPECTED_V2_ID = ? OR $TABLE_NAME.$GROUP_ID = ? ORDER BY $TABLE_NAME.$EXPECTED_V2_ID DESC", buildArgs(gv2Id, gv2Id)))
}
fun getGroupByDistributionId(distributionId: DistributionId): Optional<GroupRecord> {
return getGroup(SqlUtil.Query("$TABLE_NAME.$DISTRIBUTION_ID = ?", buildArgs(distributionId)))
}
@ -347,7 +356,10 @@ class GroupTable(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT
}
fun isUnknownGroup(groupId: GroupId): Boolean {
val group = getGroup(groupId)
return isUnknownGroup(getGroup(groupId))
}
fun isUnknownGroup(group: Optional<GroupRecord>): Boolean {
if (!group.isPresent) {
return true
}

View file

@ -1780,7 +1780,8 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
recipientSettings.registered,
recipientSettings,
null,
false
false,
group.isActive
)
Recipient(recipientId, details, false)
} ?: Recipient.live(recipientId).get()

View file

@ -18,6 +18,7 @@ import org.thoughtcrime.securesms.database.SignalDatabase;
import org.thoughtcrime.securesms.database.model.GroupRecord;
import org.thoughtcrime.securesms.groups.v2.GroupInviteLinkUrl;
import org.thoughtcrime.securesms.groups.v2.GroupLinkPassword;
import org.thoughtcrime.securesms.groups.v2.processing.GroupsV2StateProcessor;
import org.thoughtcrime.securesms.profiles.AvatarHelper;
import org.thoughtcrime.securesms.recipients.Recipient;
import org.thoughtcrime.securesms.recipients.RecipientId;
@ -178,15 +179,15 @@ public final class GroupManager {
* processing deny messages.
*/
@WorkerThread
public static void updateGroupFromServer(@NonNull Context context,
@NonNull GroupMasterKey groupMasterKey,
int revision,
long timestamp,
@Nullable byte[] signedGroupChange)
public static GroupsV2StateProcessor.GroupUpdateResult updateGroupFromServer(@NonNull Context context,
@NonNull GroupMasterKey groupMasterKey,
int revision,
long timestamp,
@Nullable byte[] signedGroupChange)
throws GroupChangeBusyException, IOException, GroupNotAMemberException
{
try (GroupManagerV2.GroupUpdater updater = new GroupManagerV2(context).updater(groupMasterKey)) {
updater.updateLocalToServerRevision(revision, timestamp, signedGroupChange);
return updater.updateLocalToServerRevision(revision, timestamp, signedGroupChange);
}
}

View file

@ -797,11 +797,11 @@ final class GroupManagerV2 {
}
@WorkerThread
void updateLocalToServerRevision(int revision, long timestamp, @Nullable byte[] signedGroupChange)
GroupsV2StateProcessor.GroupUpdateResult updateLocalToServerRevision(int revision, long timestamp, @Nullable byte[] signedGroupChange)
throws IOException, GroupNotAMemberException
{
new GroupsV2StateProcessor(context).forGroup(serviceIds, groupMasterKey)
.updateLocalGroupToRevision(revision, timestamp, getDecryptedGroupChange(signedGroupChange));
return new GroupsV2StateProcessor(context).forGroup(serviceIds, groupMasterKey)
.updateLocalGroupToRevision(revision, timestamp, getDecryptedGroupChange(signedGroupChange));
}
@WorkerThread

View file

@ -133,7 +133,7 @@ public class GroupsV2StateProcessor {
this.latestServer = latestServer;
}
public GroupState getGroupState() {
public @NonNull GroupState getGroupState() {
return groupState;
}
@ -229,13 +229,14 @@ public class GroupsV2StateProcessor {
@Nullable DecryptedGroupChange signedGroupChange)
throws IOException, GroupNotAMemberException
{
if (localIsAtLeast(revision)) {
Optional<GroupRecord> localRecord = groupDatabase.getGroup(groupId);
if (localIsAtLeast(localRecord, revision)) {
return new GroupUpdateResult(GroupState.GROUP_CONSISTENT_OR_AHEAD, null);
}
GlobalGroupState inputGroupState = null;
Optional<GroupRecord> localRecord = groupDatabase.getGroup(groupId);
DecryptedGroup localState = localRecord.map(g -> g.requireV2GroupProperties().getDecryptedGroup()).orElse(null);
if (signedGroupChange != null &&
@ -540,11 +541,11 @@ public class GroupsV2StateProcessor {
/**
* @return true iff group exists locally and is at least the specified revision.
*/
private boolean localIsAtLeast(int revision) {
if (groupDatabase.isUnknownGroup(groupId) || revision == LATEST) {
private boolean localIsAtLeast(Optional<GroupRecord> localRecord, int revision) {
if (revision == LATEST || localRecord.isEmpty() || groupDatabase.isUnknownGroup(localRecord)) {
return false;
}
int dbRevision = groupDatabase.getGroup(groupId).get().requireV2GroupProperties().getGroupRevision();
int dbRevision = localRecord.get().requireV2GroupProperties().getGroupRevision();
return revision <= dbRevision;
}

View file

@ -122,8 +122,10 @@ object DataMessageProcessor {
val message: DataMessage = content.dataMessage
val groupId: GroupId.V2? = if (message.hasGroupContext) GroupId.v2(message.groupV2.groupMasterKey) else null
var groupProcessResult: MessageContentProcessorV2.Gv2PreProcessResult? = null
if (groupId != null) {
if (MessageContentProcessorV2.handleGv2PreProcessing(context, envelope.timestamp, content, metadata, groupId, message.groupV2, senderRecipient)) {
groupProcessResult = MessageContentProcessorV2.handleGv2PreProcessing(context, envelope.timestamp, content, metadata, groupId, message.groupV2, senderRecipient)
if (groupProcessResult == MessageContentProcessorV2.Gv2PreProcessResult.IGNORE) {
return
}
}
@ -146,8 +148,14 @@ object DataMessageProcessor {
message.hasGroupCallUpdate() -> handleGroupCallUpdateMessage(envelope, message, senderRecipient.id, groupId)
}
if (groupId != null && SignalDatabase.groups.isUnknownGroup(groupId)) {
handleUnknownGroupMessage(envelope.timestamp, message.groupV2)
if (groupId != null) {
val unknownGroup = when (groupProcessResult) {
MessageContentProcessorV2.Gv2PreProcessResult.GROUP_UP_TO_DATE -> threadRecipient.isUnknownGroup
else -> SignalDatabase.groups.isUnknownGroup(groupId)
}
if (unknownGroup) {
handleUnknownGroupMessage(envelope.timestamp, message.groupV2)
}
}
if (message.hasProfileKey()) {

View file

@ -78,7 +78,7 @@ object EditMessageProcessor {
return
}
if (groupId != null && MessageContentProcessorV2.handleGv2PreProcessing(context, envelope.timestamp, content, metadata, groupId, message.groupV2, senderRecipient)) {
if (groupId != null && MessageContentProcessorV2.handleGv2PreProcessing(context, envelope.timestamp, content, metadata, groupId, message.groupV2, senderRecipient) == MessageContentProcessorV2.Gv2PreProcessResult.IGNORE) {
warn(envelope.timestamp, "[handleEditMessage] Group processor indicated we should ignore this.")
return
}

View file

@ -17,6 +17,7 @@ import org.thoughtcrime.securesms.groups.GroupId
import org.thoughtcrime.securesms.groups.GroupManager
import org.thoughtcrime.securesms.groups.GroupNotAMemberException
import org.thoughtcrime.securesms.groups.GroupsV1MigrationUtil
import org.thoughtcrime.securesms.groups.v2.processing.GroupsV2StateProcessor
import org.thoughtcrime.securesms.jobs.NullMessageSendJob
import org.thoughtcrime.securesms.jobs.ResendMessageJob
import org.thoughtcrime.securesms.jobs.SenderKeyDistributionSendJob
@ -50,6 +51,12 @@ import java.io.IOException
open class MessageContentProcessorV2(private val context: Context) {
enum class Gv2PreProcessResult {
IGNORE,
GROUP_UPDATE,
GROUP_UP_TO_DATE
}
companion object {
const val TAG = "MessageProcessorV2"
@ -133,8 +140,7 @@ open class MessageContentProcessorV2(private val context: Context) {
return if (threadRecipient.isGroup && threadRecipient.isBlocked) {
true
} else if (threadRecipient.isGroup) {
val groupId = if (message.hasGroupV2()) GroupId.v2(message.groupV2.groupMasterKey) else null
if (groupId != null && SignalDatabase.groups.isUnknownGroup(groupId)) {
if (threadRecipient.isUnknownGroup) {
return senderRecipient.isBlocked
}
@ -143,7 +149,7 @@ open class MessageContentProcessorV2(private val context: Context) {
val isExpireMessage = message.isExpirationUpdate
val isGv2Update = message.hasSignedGroupChange
val isContentMessage = !isGv2Update && !isExpireMessage && (isTextMessage || isMediaMessage)
val isGroupActive = groupId != null && SignalDatabase.groups.isActive(groupId)
val isGroupActive = threadRecipient.isActiveGroup
isContentMessage && !isGroupActive || senderRecipient.isBlocked && !isGv2Update
} else {
@ -214,36 +220,46 @@ open class MessageContentProcessorV2(private val context: Context) {
groupId: GroupId.V2,
groupV2: SignalServiceProtos.GroupContextV2,
senderRecipient: Recipient
): Boolean {
val possibleGv1 = SignalDatabase.groups.getGroupV1ByExpectedV2(groupId)
if (possibleGv1.isPresent) {
GroupsV1MigrationUtil.performLocalMigration(context, possibleGv1.get().id.requireV1())
): Gv2PreProcessResult {
val possibleV1OrV2Group = SignalDatabase.groups.getGroupV1OrV2ByExpectedV2(groupId)
val needsV1Migration = possibleV1OrV2Group.isPresent && possibleV1OrV2Group.get().isV1Group
if (needsV1Migration) {
GroupsV1MigrationUtil.performLocalMigration(context, possibleV1OrV2Group.get().id.requireV1())
}
if (!updateGv2GroupFromServerOrP2PChange(context, timestamp, groupV2)) {
val groupUpdateResult = updateGv2GroupFromServerOrP2PChange(context, timestamp, groupV2)
if (groupUpdateResult == null) {
log(timestamp, "Ignoring GV2 message for group we are not currently in $groupId")
return true
return Gv2PreProcessResult.IGNORE
}
val groupRecord = if (groupUpdateResult.groupState == GroupsV2StateProcessor.GroupState.GROUP_UPDATED || needsV1Migration) {
SignalDatabase.groups.getGroup(groupId)
} else {
possibleV1OrV2Group
}
val groupRecord = SignalDatabase.groups.getGroup(groupId)
if (groupRecord.isPresent && !groupRecord.get().members.contains(senderRecipient.id)) {
log(timestamp, "Ignoring GV2 message from member not in group $groupId. Sender: ${formatSender(senderRecipient.id, metadata.sourceServiceId, metadata.sourceDeviceId)}")
return true
return Gv2PreProcessResult.IGNORE
}
if (groupRecord.isPresent && groupRecord.get().isAnnouncementGroup && !groupRecord.get().admins.contains(senderRecipient)) {
if (content.hasDataMessage()) {
if (content.dataMessage.hasDisallowedAnnouncementOnlyContent) {
Log.w(TAG, "Ignoring message from ${senderRecipient.id} because it has disallowed content, and they're not an admin in an announcement-only group.")
return true
return Gv2PreProcessResult.IGNORE
}
} else if (content.hasTypingMessage()) {
Log.w(TAG, "Ignoring typing indicator from ${senderRecipient.id} because they're not an admin in an announcement-only group.")
return true
return Gv2PreProcessResult.IGNORE
}
}
return false
return when (groupUpdateResult.groupState) {
GroupsV2StateProcessor.GroupState.GROUP_UPDATED -> Gv2PreProcessResult.GROUP_UPDATE
GroupsV2StateProcessor.GroupState.GROUP_CONSISTENT_OR_AHEAD -> Gv2PreProcessResult.GROUP_UP_TO_DATE
}
}
@Throws(IOException::class, GroupChangeBusyException::class)
@ -251,15 +267,14 @@ open class MessageContentProcessorV2(private val context: Context) {
context: Context,
timestamp: Long,
groupV2: SignalServiceProtos.GroupContextV2
): Boolean {
): GroupsV2StateProcessor.GroupUpdateResult? {
return try {
val signedGroupChange: ByteArray? = if (groupV2.hasSignedGroupChange) groupV2.signedGroupChange else null
val updatedTimestamp = if (signedGroupChange != null) timestamp else timestamp - 1
GroupManager.updateGroupFromServer(context, groupV2.groupMasterKey, groupV2.revision, updatedTimestamp, signedGroupChange)
true
} catch (e: GroupNotAMemberException) {
warn(timestamp, "Ignoring message for a group we're not in")
false
null
}
}

View file

@ -173,7 +173,7 @@ object SyncMessageProcessor {
val groupId: GroupId.V2? = if (dataMessage.hasGroupContext) GroupId.v2(dataMessage.groupV2.groupMasterKey) else null
if (groupId != null) {
if (MessageContentProcessorV2.handleGv2PreProcessing(context, envelope.timestamp, content, metadata, groupId, dataMessage.groupV2, senderRecipient)) {
if (MessageContentProcessorV2.handleGv2PreProcessing(context, envelope.timestamp, content, metadata, groupId, dataMessage.groupV2, senderRecipient) == MessageContentProcessorV2.Gv2PreProcessResult.IGNORE) {
return
}
}
@ -591,7 +591,7 @@ object SyncMessageProcessor {
val dataMessage: DataMessage = sent.message
val groupId: GroupId.V2? = dataMessage.groupV2.groupId
if (!MessageContentProcessorV2.updateGv2GroupFromServerOrP2PChange(context, envelope.timestamp, dataMessage.groupV2)) {
if (MessageContentProcessorV2.updateGv2GroupFromServerOrP2PChange(context, envelope.timestamp, dataMessage.groupV2) == null) {
log(envelope.timestamp, "Ignoring GV2 message for group we are not currently in $groupId")
}
}

View file

@ -216,10 +216,10 @@ public final class LiveRecipient {
avatarId = Optional.of(groupRecord.get().getAvatarId());
}
return new RecipientDetails(title, null, avatarId, false, false, record.getRegistered(), record, members, false);
return new RecipientDetails(title, null, avatarId, false, false, record.getRegistered(), record, members, false, groupRecord.get().isActive());
}
return new RecipientDetails(null, null, Optional.empty(), false, false, record.getRegistered(), record, null, false);
return new RecipientDetails(null, null, Optional.empty(), false, false, record.getRegistered(), record, null, false, false);
}
@WorkerThread

View file

@ -95,6 +95,7 @@ public class Recipient {
private final DistributionListId distributionListId;
private final List<RecipientId> participantIds;
private final Optional<Long> groupAvatarId;
private final boolean isActiveGroup;
private final boolean isSelf;
private final boolean blocked;
private final long muteUntil;
@ -433,6 +434,7 @@ public class Recipient {
this.badges = Collections.emptyList();
this.isReleaseNotesRecipient = false;
this.needsPniSignature = false;
this.isActiveGroup = false;
}
public Recipient(@NonNull RecipientId id, @NonNull RecipientDetails details, boolean resolved) {
@ -488,6 +490,7 @@ public class Recipient {
this.badges = details.badges;
this.isReleaseNotesRecipient = details.isReleaseChannel;
this.needsPniSignature = details.needsPniSignature;
this.isActiveGroup = details.isActiveGroup;
}
public @NonNull RecipientId getId() {
@ -881,8 +884,14 @@ public class Recipient {
}
public boolean isActiveGroup() {
RecipientId selfId = Recipient.self().getId();
return Stream.of(getParticipantIds()).anyMatch(p -> p.equals(selfId));
return isActiveGroup;
}
public boolean isUnknownGroup() {
boolean noMetadata = (groupAvatarId.isEmpty() || groupAvatarId.get() == - 1) && (groupName == null || groupName.isEmpty());
boolean noMembers = participantIds.isEmpty() || (participantIds.size() == 1 && participantIds.contains(Recipient.self().id));
return noMetadata && noMembers;
}
public boolean isInactiveGroup() {

View file

@ -64,6 +64,7 @@ public class RecipientDetails {
final ProfileAvatarFileDetails profileAvatarFileDetails;
final boolean profileSharing;
final boolean isHidden;
final boolean isActiveGroup;
final long lastProfileFetch;
final boolean systemContact;
final boolean isSelf;
@ -94,7 +95,8 @@ public class RecipientDetails {
@NonNull RegisteredState registeredState,
@NonNull RecipientRecord record,
@Nullable List<RecipientId> participantIds,
boolean isReleaseChannel)
boolean isReleaseChannel,
boolean isActiveGroup)
{
this.groupAvatarId = groupAvatarId;
this.systemContactPhoto = Util.uri(record.getSystemContactPhotoUri());
@ -115,6 +117,7 @@ public class RecipientDetails {
this.blocked = record.isBlocked();
this.expireMessages = record.getExpireMessages();
this.participantIds = participantIds == null ? new LinkedList<>() : participantIds;
this.isActiveGroup = isActiveGroup;
this.profileName = record.getProfileName();
this.defaultSubscriptionId = record.getDefaultSubscriptionId();
this.registered = registeredState;
@ -200,7 +203,8 @@ public class RecipientDetails {
this.hasGroupsInCommon = false;
this.badges = Collections.emptyList();
this.isReleaseChannel = false;
this.needsPniSignature = false;
this.needsPniSignature = false;
this.isActiveGroup = false;
}
public static @NonNull RecipientDetails forIndividual(@NonNull Context context, @NonNull RecipientRecord settings) {
@ -219,11 +223,11 @@ public class RecipientDetails {
}
}
return new RecipientDetails(null, settings.getSystemDisplayName(), Optional.empty(), systemContact, isSelf, registeredState, settings, null, isReleaseChannel);
return new RecipientDetails(null, settings.getSystemDisplayName(), Optional.empty(), systemContact, isSelf, registeredState, settings, null, isReleaseChannel, false);
}
public static @NonNull RecipientDetails forDistributionList(String title, @Nullable List<RecipientId> members, @NonNull RecipientRecord record) {
return new RecipientDetails(title, null, Optional.empty(), false, false, record.getRegistered(), record, members, false);
return new RecipientDetails(title, null, Optional.empty(), false, false, record.getRegistered(), record, members, false, false);
}
public static @NonNull RecipientDetails forUnknown() {

View file

@ -84,7 +84,8 @@ object RecipientDatabaseTestUtils {
extras: Recipient.Extras? = null,
hasGroupsInCommon: Boolean = false,
badges: List<Badge> = emptyList(),
isReleaseChannel: Boolean = false
isReleaseChannel: Boolean = false,
isActive: Boolean = true
): Recipient = Recipient(
recipientId,
RecipientDetails(
@ -155,7 +156,8 @@ object RecipientDatabaseTestUtils {
isHidden = false
),
participants,
isReleaseChannel
isReleaseChannel,
isActive
),
resolved
)

View file

@ -104,7 +104,7 @@ class GroupsV2StateProcessorTest {
val data = givenData(init)
every { groupTable.getGroup(any<GroupId.V2>()) } returns data.groupRecord
every { groupTable.isUnknownGroup(any()) } returns !data.groupRecord.isPresent
every { groupTable.isUnknownGroup(any<GroupId>()) } returns !data.groupRecord.isPresent
data.serverState?.let { serverState ->
val testPartial = object : PartialDecryptedGroup(null, serverState, null, null) {
@ -305,7 +305,7 @@ class GroupsV2StateProcessorTest {
apiCallParameters(2, true)
}
every { groupTable.isUnknownGroup(any()) } returns true
every { groupTable.isUnknownGroup(any<GroupId>()) } returns true
val result = processor.updateLocalGroupToRevision(2, 0, DecryptedGroupChange.getDefaultInstance())