Skip to content

Commit

Permalink
Merge pull request element-hq#7879 from vector-im/feature/bma/still_i…
Browse files Browse the repository at this point in the history
…nvestigating

Reduce number of crypto database transactions when handling the sync response
  • Loading branch information
bmarty authored Jan 6, 2023
2 parents 41bcdd7 + dbf3b76 commit b7076a1
Show file tree
Hide file tree
Showing 18 changed files with 380 additions and 183 deletions.
1 change: 1 addition & 0 deletions changelog.d/7879.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reduce number of crypto database transactions when handling the sync response
3 changes: 3 additions & 0 deletions library/ui-strings/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3506,4 +3506,7 @@
<string name="message_reply_to_sender_sent_video">sent a video.</string>
<string name="message_reply_to_sender_sent_sticker">sent a sticker.</string>
<string name="message_reply_to_sender_created_poll">created a poll.</string>

<string name="settings_access_token">Access Token</string>
<string name="settings_access_token_summary">Your access token gives full access to your account. Do not share it with anyone.</string>
</resources>
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright 2023 The Matrix.org Foundation C.I.C.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.matrix.android.sdk.api.session.crypto.crosssigning

/**
* Container for the three cross signing keys: master, self signing and user signing.
*/
data class UserIdentity(
val masterKey: CryptoCrossSigningKey?,
val selfSigningKey: CryptoCrossSigningKey?,
val userSigningKey: CryptoCrossSigningKey?,
)
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ import org.matrix.android.sdk.internal.crypto.model.SessionInfo
import org.matrix.android.sdk.internal.crypto.model.toRest
import org.matrix.android.sdk.internal.crypto.repository.WarnOnUnknownDeviceRepository
import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore
import org.matrix.android.sdk.internal.crypto.store.db.CryptoStoreAggregator
import org.matrix.android.sdk.internal.crypto.tasks.DeleteDeviceTask
import org.matrix.android.sdk.internal.crypto.tasks.GetDeviceInfoTask
import org.matrix.android.sdk.internal.crypto.tasks.GetDevicesTask
Expand Down Expand Up @@ -192,21 +193,21 @@ internal class DefaultCryptoService @Inject constructor(
private val isStarting = AtomicBoolean(false)
private val isStarted = AtomicBoolean(false)

fun onStateEvent(roomId: String, event: Event) {
fun onStateEvent(roomId: String, event: Event, cryptoStoreAggregator: CryptoStoreAggregator?) {
when (event.type) {
EventType.STATE_ROOM_ENCRYPTION -> onRoomEncryptionEvent(roomId, event)
EventType.STATE_ROOM_MEMBER -> onRoomMembershipEvent(roomId, event)
EventType.STATE_ROOM_HISTORY_VISIBILITY -> onRoomHistoryVisibilityEvent(roomId, event)
EventType.STATE_ROOM_HISTORY_VISIBILITY -> onRoomHistoryVisibilityEvent(roomId, event, cryptoStoreAggregator)
}
}

fun onLiveEvent(roomId: String, event: Event, isInitialSync: Boolean) {
fun onLiveEvent(roomId: String, event: Event, isInitialSync: Boolean, cryptoStoreAggregator: CryptoStoreAggregator?) {
// handle state events
if (event.isStateEvent()) {
when (event.type) {
EventType.STATE_ROOM_ENCRYPTION -> onRoomEncryptionEvent(roomId, event)
EventType.STATE_ROOM_MEMBER -> onRoomMembershipEvent(roomId, event)
EventType.STATE_ROOM_HISTORY_VISIBILITY -> onRoomHistoryVisibilityEvent(roomId, event)
EventType.STATE_ROOM_HISTORY_VISIBILITY -> onRoomHistoryVisibilityEvent(roomId, event, cryptoStoreAggregator)
}
}

Expand Down Expand Up @@ -430,8 +431,10 @@ internal class DefaultCryptoService @Inject constructor(
* A sync response has been received.
*
* @param syncResponse the syncResponse
* @param cryptoStoreAggregator data aggregated during the sync response treatment to store
*/
fun onSyncCompleted(syncResponse: SyncResponse) {
fun onSyncCompleted(syncResponse: SyncResponse, cryptoStoreAggregator: CryptoStoreAggregator) {
cryptoStore.storeData(cryptoStoreAggregator)
cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
runCatching {
if (syncResponse.deviceLists != null) {
Expand Down Expand Up @@ -998,15 +1001,26 @@ internal class DefaultCryptoService @Inject constructor(
}
}

private fun onRoomHistoryVisibilityEvent(roomId: String, event: Event) {
private fun onRoomHistoryVisibilityEvent(roomId: String, event: Event, cryptoStoreAggregator: CryptoStoreAggregator?) {
if (!event.isStateEvent()) return
val eventContent = event.content.toModel<RoomHistoryVisibilityContent>()
val historyVisibility = eventContent?.historyVisibility
if (historyVisibility == null) {
cryptoStore.setShouldShareHistory(roomId, false)
if (cryptoStoreAggregator != null) {
cryptoStoreAggregator.setShouldShareHistoryData[roomId] = false
} else {
// Store immediately
cryptoStore.setShouldShareHistory(roomId, false)
}
} else {
cryptoStore.setShouldEncryptForInvitedMembers(roomId, historyVisibility != RoomHistoryVisibility.JOINED)
cryptoStore.setShouldShareHistory(roomId, historyVisibility.shouldShareHistory())
if (cryptoStoreAggregator != null) {
cryptoStoreAggregator.setShouldEncryptForInvitedMembersData[roomId] = historyVisibility != RoomHistoryVisibility.JOINED
cryptoStoreAggregator.setShouldShareHistoryData[roomId] = historyVisibility.shouldShareHistory()
} else {
// Store immediately
cryptoStore.setShouldEncryptForInvitedMembers(roomId, historyVisibility != RoomHistoryVisibility.JOINED)
cryptoStore.setShouldShareHistory(roomId, historyVisibility.shouldShareHistory())
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ import org.matrix.android.sdk.api.auth.data.Credentials
import org.matrix.android.sdk.api.extensions.measureMetric
import org.matrix.android.sdk.api.metrics.DownloadDeviceKeysMetricsPlugin
import org.matrix.android.sdk.api.session.crypto.crosssigning.DeviceTrustLevel
import org.matrix.android.sdk.api.session.crypto.crosssigning.UserIdentity
import org.matrix.android.sdk.api.session.crypto.model.CryptoDeviceInfo
import org.matrix.android.sdk.api.session.crypto.model.MXUsersDevicesMap
import org.matrix.android.sdk.internal.crypto.model.CryptoInfoMapper
import org.matrix.android.sdk.internal.crypto.model.rest.KeysQueryResponse
import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore
import org.matrix.android.sdk.internal.crypto.store.UserDataToStore
import org.matrix.android.sdk.internal.crypto.tasks.DownloadKeysForUsersTask
import org.matrix.android.sdk.internal.session.SessionScope
import org.matrix.android.sdk.internal.session.sync.SyncTokenStore
Expand Down Expand Up @@ -371,6 +373,8 @@ internal class DeviceListManager @Inject constructor(
Timber.v("## CRYPTO | doKeyDownloadForUsers() : Got keys for " + filteredUsers.size + " users")
}

val userDataToStore = UserDataToStore()

for (userId in filteredUsers) {
// al devices =
val models = response.deviceKeys?.get(userId)?.mapValues { entry -> CryptoInfoMapper.map(entry.value) }
Expand Down Expand Up @@ -404,7 +408,7 @@ internal class DeviceListManager @Inject constructor(
}
// Update the store
// Note that devices which aren't in the response will be removed from the stores
cryptoStore.storeUserDevices(userId, workingCopy)
userDataToStore.userDevices[userId] = workingCopy
}

val masterKey = response.masterKeys?.get(userId)?.toCryptoModel().also {
Expand All @@ -416,14 +420,15 @@ internal class DeviceListManager @Inject constructor(
val userSigningKey = response.userSigningKeys?.get(userId)?.toCryptoModel()?.also {
Timber.v("## CRYPTO | CrossSigning : Got keys for $userId : USK ${it.unpaddedBase64PublicKey}")
}
cryptoStore.storeUserCrossSigningKeys(
userId,
masterKey,
selfSigningKey,
userSigningKey
userDataToStore.userIdentities[userId] = UserIdentity(
masterKey = masterKey,
selfSigningKey = selfSigningKey,
userSigningKey = userSigningKey
)
}

cryptoStore.storeData(userDataToStore)

// Update devices trust for these users
// dispatchDeviceChange(downloadUsers)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import org.matrix.android.sdk.api.session.crypto.GlobalCryptoConfig
import org.matrix.android.sdk.api.session.crypto.NewSessionListener
import org.matrix.android.sdk.api.session.crypto.OutgoingKeyRequest
import org.matrix.android.sdk.api.session.crypto.OutgoingRoomKeyRequestState
import org.matrix.android.sdk.api.session.crypto.crosssigning.CryptoCrossSigningKey
import org.matrix.android.sdk.api.session.crypto.crosssigning.MXCrossSigningInfo
import org.matrix.android.sdk.api.session.crypto.crosssigning.PrivateKeysInfo
import org.matrix.android.sdk.api.session.crypto.crosssigning.UserIdentity
import org.matrix.android.sdk.api.session.crypto.keysbackup.SavedKeyBackupKeyInfo
import org.matrix.android.sdk.api.session.crypto.model.AuditTrail
import org.matrix.android.sdk.api.session.crypto.model.CryptoDeviceInfo
Expand All @@ -39,6 +39,7 @@ import org.matrix.android.sdk.api.util.Optional
import org.matrix.android.sdk.internal.crypto.model.MXInboundMegolmSessionWrapper
import org.matrix.android.sdk.internal.crypto.model.OlmSessionWrapper
import org.matrix.android.sdk.internal.crypto.model.OutboundGroupSessionWrapper
import org.matrix.android.sdk.internal.crypto.store.db.CryptoStoreAggregator
import org.matrix.android.sdk.internal.crypto.store.db.model.KeysBackupDataEntity
import org.matrix.olm.OlmAccount
import org.matrix.olm.OlmOutboundGroupSession
Expand Down Expand Up @@ -230,11 +231,12 @@ internal interface IMXCryptoStore {
*/
fun storeUserDevices(userId: String, devices: Map<String, CryptoDeviceInfo>?)

fun storeUserCrossSigningKeys(
/**
* Store the cross signing keys for the user userId.
*/
fun storeUserIdentity(
userId: String,
masterKey: CryptoCrossSigningKey?,
selfSigningKey: CryptoCrossSigningKey?,
userSigningKey: CryptoCrossSigningKey?
userIdentity: UserIdentity
)

/**
Expand Down Expand Up @@ -290,6 +292,13 @@ internal interface IMXCryptoStore {

fun shouldEncryptForInvitedMembers(roomId: String): Boolean

/**
* Sets a boolean flag that will determine whether or not this device should encrypt Events for
* invited members.
*
* @param roomId the room id
* @param shouldEncryptForInvitedMembers The boolean flag
*/
fun setShouldEncryptForInvitedMembers(roomId: String, shouldEncryptForInvitedMembers: Boolean)

fun shouldShareHistory(roomId: String): Boolean
Expand Down Expand Up @@ -580,4 +589,14 @@ internal interface IMXCryptoStore {
fun areDeviceKeysUploaded(): Boolean
fun tidyUpDataBase()
fun getOutgoingRoomKeyRequests(inStates: Set<OutgoingRoomKeyRequestState>): List<OutgoingKeyRequest>

/**
* Store a bunch of data collected during a sync response treatment. @See [CryptoStoreAggregator].
*/
fun storeData(cryptoStoreAggregator: CryptoStoreAggregator)

/**
* Store a bunch of data related to the users. @See [UserDataToStore].
*/
fun storeData(userDataToStore: UserDataToStore)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright (c) 2023 The Matrix.org Foundation C.I.C.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.matrix.android.sdk.internal.crypto.store

import org.matrix.android.sdk.api.session.crypto.crosssigning.UserIdentity
import org.matrix.android.sdk.api.session.crypto.model.CryptoDeviceInfo

internal data class UserDataToStore(
/**
* Map of userId -> (Map of deviceId -> [CryptoDeviceInfo]).
*/
val userDevices: MutableMap<String, Map<String, CryptoDeviceInfo>> = mutableMapOf(),
/**
* Map of userId -> [UserIdentity].
*/
val userIdentities: MutableMap<String, UserIdentity> = mutableMapOf(),
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (c) 2023 The Matrix.org Foundation C.I.C.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.matrix.android.sdk.internal.crypto.store.db

data class CryptoStoreAggregator(
val setShouldShareHistoryData: MutableMap<String, Boolean> = mutableMapOf(),
val setShouldEncryptForInvitedMembersData: MutableMap<String, Boolean> = mutableMapOf(),
) {
fun isEmpty(): Boolean {
return setShouldShareHistoryData.isEmpty() &&
setShouldEncryptForInvitedMembersData.isEmpty()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ import android.util.Base64
import io.realm.Realm
import io.realm.RealmConfiguration
import io.realm.RealmObject
import timber.log.Timber
import java.io.ByteArrayOutputStream
import java.io.ObjectOutputStream
import java.util.zip.GZIPInputStream
import java.util.zip.GZIPOutputStream
import kotlin.system.measureTimeMillis

/**
* Get realm, invoke the action, close realm, and return the result of the action.
Expand Down Expand Up @@ -55,10 +57,12 @@ internal fun <T : RealmObject> doRealmQueryAndCopyList(realmConfiguration: Realm
/**
* Get realm instance, invoke the action in a transaction and close realm.
*/
internal fun doRealmTransaction(realmConfiguration: RealmConfiguration, action: (Realm) -> Unit) {
Realm.getInstance(realmConfiguration).use { realm ->
realm.executeTransaction { action.invoke(it) }
}
internal fun doRealmTransaction(tag: String, realmConfiguration: RealmConfiguration, action: (Realm) -> Unit) {
measureTimeMillis {
Realm.getInstance(realmConfiguration).use { realm ->
realm.executeTransaction { action.invoke(it) }
}
}.also { Timber.w("doRealmTransaction for $tag took $it millis") }
}

internal fun doRealmTransactionAsync(realmConfiguration: RealmConfiguration, action: (Realm) -> Unit) {
Expand Down
Loading

0 comments on commit b7076a1

Please sign in to comment.