Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle properly when getUser returns null - prefer using getUserOrDefault #7372

Merged
merged 8 commits into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7372.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Handle properly when getUser returns null - prefer using getUserOrDefault
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,40 @@

package org.matrix.android.sdk.internal.session.profile

import com.zhuinden.monarchy.Monarchy
import org.matrix.android.sdk.api.session.user.model.User
import org.matrix.android.sdk.api.util.JsonDict
import org.matrix.android.sdk.internal.di.SessionDatabase
import org.matrix.android.sdk.internal.network.GlobalErrorReceiver
import org.matrix.android.sdk.internal.network.executeRequest
import org.matrix.android.sdk.internal.session.user.UserEntityFactory
import org.matrix.android.sdk.internal.task.Task
import org.matrix.android.sdk.internal.util.awaitTransaction
import javax.inject.Inject

internal abstract class GetProfileInfoTask : Task<GetProfileInfoTask.Params, JsonDict> {
data class Params(
val userId: String
val userId: String,
val storeInDatabase: Boolean = true,
)
}

internal class DefaultGetProfileInfoTask @Inject constructor(
private val profileAPI: ProfileAPI,
private val globalErrorReceiver: GlobalErrorReceiver
private val globalErrorReceiver: GlobalErrorReceiver,
@SessionDatabase private val monarchy: Monarchy,
) : GetProfileInfoTask() {

override suspend fun execute(params: Params): JsonDict {
return executeRequest(globalErrorReceiver) {
profileAPI.getProfile(params.userId)
}.also { user ->
if (params.storeInDatabase) {
// Insert into DB
monarchy.awaitTransaction {
it.insertOrUpdate(UserEntityFactory.create(User.fromJson(params.userId, user)))
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,16 @@ internal class UpdateUserWorker(context: Context, params: WorkerParameters, sess
?.saveLocally()
}

private suspend fun fetchUsers(userIdsToFetch: Collection<String>) = userIdsToFetch.mapNotNull {
tryOrNull {
val profileJson = getProfileInfoTask.execute(GetProfileInfoTask.Params(it))
User.fromJson(it, profileJson)
private suspend fun fetchUsers(userIdsToFetch: Collection<String>): List<User> {
return userIdsToFetch.mapNotNull { userId ->
tryOrNull {
val profileJson = getProfileInfoTask.execute(GetProfileInfoTask.Params(
userId = userId,
// Bulk insert later, so tell the task not to store the User.
storeInDatabase = false,
))
User.fromJson(userId, profileJson)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ internal class UserDataSource @Inject constructor(
}
}

fun getUserOrDefault(userId: String): User = getUser(userId) ?: User(userId)

fun getUserLive(userId: String): LiveData<Optional<User>> {
val liveData = monarchy.findAllMappedWithChanges(
{ UserEntity.where(it, userId) },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import org.matrix.android.sdk.api.session.content.ContentUrlResolver
import org.matrix.android.sdk.api.session.events.model.Event
import org.matrix.android.sdk.api.session.events.model.toModel
import org.matrix.android.sdk.api.session.room.sender.SenderInfo
import org.matrix.android.sdk.api.session.user.model.User
import org.matrix.android.sdk.api.session.widgets.model.Widget
import org.matrix.android.sdk.api.session.widgets.model.WidgetContent
import org.matrix.android.sdk.api.session.widgets.model.WidgetType
Expand Down Expand Up @@ -74,7 +73,7 @@ internal class WidgetFactory @Inject constructor(
// Ref: https://github.com/matrix-org/matrix-widget-api/blob/master/src/templating/url-template.ts#L29-L33
fun computeURL(widget: Widget, isLightTheme: Boolean): String? {
var computedUrl = widget.widgetContent.url ?: return null
val myUser = userDataSource.getUser(userId) ?: User(userId)
val myUser = userDataSource.getUserOrDefault(userId)

val keyValue = widget.widgetContent.data.mapKeys { "\$${it.key}" }.toMutableMap()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import im.vector.app.core.di.ActiveSessionHolder
import im.vector.app.core.glide.GlideApp
import im.vector.app.core.utils.DimensionConverter
import im.vector.app.features.home.AvatarRenderer
import org.matrix.android.sdk.api.session.getUser
import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.util.toMatrixItem
import timber.log.Timber
import javax.inject.Inject
Expand Down Expand Up @@ -67,9 +67,9 @@ class LocationPinProvider @Inject constructor(

activeSessionHolder
.getActiveSession()
.getUser(userId)
?.toMatrixItem()
?.let { userItem ->
.getUserOrDefault(userId)
.toMatrixItem()
.let { userItem ->
val size = dimensionConverter.dpToPx(44)
val bgTintColor = matrixItemColorProvider.getColor(userItem)
avatarRenderer.render(glideRequests, userItem, object : CustomTarget<Drawable>(size, size) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import im.vector.app.core.glide.GlideApp
import im.vector.app.features.home.AvatarRenderer
import io.noties.markwon.core.spans.LinkSpan
import org.matrix.android.sdk.api.session.getRoomSummary
import org.matrix.android.sdk.api.session.getUser
import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.session.permalinks.PermalinkData
import org.matrix.android.sdk.api.session.permalinks.PermalinkParser
import org.matrix.android.sdk.api.session.room.model.RoomSummary
Expand Down Expand Up @@ -101,7 +101,7 @@ class PillsPostProcessor @AssistedInject constructor(

private fun PermalinkData.UserLink.toMatrixItem(roomId: String?): MatrixItem? =
if (roomId == null) {
sessionHolder.getSafeActiveSession()?.getUser(userId)?.toMatrixItem()
sessionHolder.getSafeActiveSession()?.getUserOrDefault(userId)?.toMatrixItem()
} else {
sessionHolder.getSafeActiveSession()?.roomService()?.getRoomMember(userId, roomId)?.toMatrixItem()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import org.matrix.android.sdk.api.extensions.orFalse
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.events.model.EventType
import org.matrix.android.sdk.api.session.getRoom
import org.matrix.android.sdk.api.session.getUser
import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.session.room.powerlevels.PowerLevelsHelper
import org.matrix.android.sdk.api.util.toMatrixItem
import timber.log.Timber
Expand Down Expand Up @@ -101,7 +101,7 @@ class LocationSharingViewModel @AssistedInject constructor(
}

private fun setUserItem() {
setState { copy(userItem = session.getUser(session.myUserId)?.toMatrixItem()) }
setState { copy(userItem = session.getUserOrDefault(session.myUserId).toMatrixItem()) }
}

private fun updatePin(isUserPin: Boolean? = true) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import im.vector.app.core.di.ActiveSessionHolder
import im.vector.app.features.home.room.detail.timeline.helper.LocationPinProvider
import im.vector.app.features.location.toLocationData
import kotlinx.coroutines.suspendCancellableCoroutine
import org.matrix.android.sdk.api.session.getUser
import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.session.room.model.livelocation.LiveLocationShareAggregatedSummary
import org.matrix.android.sdk.api.util.toMatrixItem
import javax.inject.Inject
Expand All @@ -45,19 +45,17 @@ class UserLiveLocationViewStateMapper @Inject constructor(
else -> {
locationPinProvider.create(userId) { pinDrawable ->
val session = activeSessionHolder.getActiveSession()
session.getUser(userId)?.toMatrixItem()?.let { matrixItem ->
val locationTimestampMillis = liveLocationShareAggregatedSummary.lastLocationDataContent?.getBestTimestampMillis()
val viewState = UserLiveLocationViewState(
matrixItem = matrixItem,
pinDrawable = pinDrawable,
locationData = locationData,
endOfLiveTimestampMillis = liveLocationShareAggregatedSummary.endOfLiveTimestampMillis,
locationTimestampMillis = locationTimestampMillis,
showStopSharingButton = userId == session.myUserId
)
continuation.resume(viewState) {
// do nothing on cancellation
}
val locationTimestampMillis = liveLocationShareAggregatedSummary.lastLocationDataContent?.getBestTimestampMillis()
val viewState = UserLiveLocationViewState(
matrixItem = session.getUserOrDefault(userId).toMatrixItem(),
pinDrawable = pinDrawable,
locationData = locationData,
endOfLiveTimestampMillis = liveLocationShareAggregatedSummary.endOfLiveTimestampMillis,
locationTimestampMillis = locationTimestampMillis,
showStopSharingButton = userId == session.myUserId
)
continuation.resume(viewState) {
// do nothing on cancellation
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import org.matrix.android.sdk.api.session.events.model.supportsNotification
import org.matrix.android.sdk.api.session.events.model.toModel
import org.matrix.android.sdk.api.session.getRoom
import org.matrix.android.sdk.api.session.getRoomSummary
import org.matrix.android.sdk.api.session.getUser
import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.session.room.getTimelineEvent
import org.matrix.android.sdk.api.session.room.model.Membership
import org.matrix.android.sdk.api.session.room.model.RoomMemberContent
Expand Down Expand Up @@ -112,7 +112,7 @@ class NotifiableEventResolver @Inject constructor(
val notificationAction = actions.toNotificationAction()

return if (notificationAction.shouldNotify) {
val user = session.getUser(event.senderId!!) ?: return null
val user = session.getUserOrDefault(event.senderId!!)

val timelineEvent = TimelineEvent(
root = event,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import im.vector.app.features.displayname.getBestName
import im.vector.app.features.settings.VectorPreferences
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.content.ContentUrlResolver
import org.matrix.android.sdk.api.session.getUser
import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.util.toMatrixItem
import timber.log.Timber
import javax.inject.Inject
Expand Down Expand Up @@ -186,11 +186,11 @@ class NotificationDrawerManager @Inject constructor(
}

private fun renderEvents(session: Session, eventsToRender: List<ProcessedEvent<NotifiableEvent>>) {
val user = session.getUser(session.myUserId)
val user = session.getUserOrDefault(session.myUserId)
// myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash
val myUserDisplayName = user?.toMatrixItem()?.getBestName() ?: session.myUserId
val myUserDisplayName = user.toMatrixItem().getBestName()
val myUserAvatarUrl = session.contentUrlResolver().resolveThumbnail(
contentUrl = user?.avatarUrl,
contentUrl = user.avatarUrl,
width = avatarSize,
height = avatarSize,
method = ContentUrlResolver.ThumbnailMethod.SCALE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@ import im.vector.app.core.di.MavericksAssistedViewModelFactory
import im.vector.app.core.di.SingletonEntryPoint
import im.vector.app.core.di.hiltMavericksViewModelFactory
import im.vector.app.core.platform.VectorViewModel
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.crypto.crosssigning.MXCrossSigningInfo
import org.matrix.android.sdk.api.session.crypto.model.CryptoDeviceInfo
import org.matrix.android.sdk.api.session.crypto.verification.VerificationMethod
import org.matrix.android.sdk.api.session.getUser
import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.util.MatrixItem
import org.matrix.android.sdk.api.util.toMatrixItem
import org.matrix.android.sdk.flow.flow
Expand All @@ -42,7 +44,6 @@ data class DeviceListViewState(
val userId: String,
val allowDeviceAction: Boolean,
val userItem: MatrixItem? = null,
val isMine: Boolean = false,
val memberCrossSigningKey: MXCrossSigningInfo? = null,
val cryptoDevices: Async<List<CryptoDeviceInfo>> = Loading(),
val selectedDevice: CryptoDeviceInfo? = null
Expand All @@ -61,23 +62,19 @@ class DeviceListBottomSheetViewModel @AssistedInject constructor(

companion object : MavericksViewModelFactory<DeviceListBottomSheetViewModel, DeviceListViewState> by hiltMavericksViewModelFactory() {

override fun initialState(viewModelContext: ViewModelContext): DeviceListViewState? {
override fun initialState(viewModelContext: ViewModelContext): DeviceListViewState {
val args = viewModelContext.args<DeviceListBottomSheet.Args>()
val userId = args.userId
val session = EntryPoints.get(viewModelContext.app(), SingletonEntryPoint::class.java).activeSessionHolder().getActiveSession()
return session.getUser(userId)?.toMatrixItem()?.let {
DeviceListViewState(
userId = userId,
allowDeviceAction = args.allowDeviceAction,
userItem = it,
isMine = userId == session.myUserId
)
} ?: return super.initialState(viewModelContext)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defaulting to super.initialState(viewModelContext) make the app crash because there is no default constructor for DeviceListViewState

return DeviceListViewState(
userId = userId,
allowDeviceAction = args.allowDeviceAction,
userItem = session.getUserOrDefault(userId).toMatrixItem(),
)
}
}

init {

session.flow().liveUserCryptoDevices(initialState.userId)
.execute {
copy(cryptoDevices = it).also {
Expand All @@ -89,6 +86,16 @@ class DeviceListBottomSheetViewModel @AssistedInject constructor(
.execute {
copy(memberCrossSigningKey = it.invoke()?.getOrNull())
}

updateMatrixItem()
}

private fun updateMatrixItem() {
viewModelScope.launch {
tryOrNull { session.userService().resolveUser(initialState.userId) }
?.toMatrixItem()
?.let { setState { copy(userItem = it) } }
}
}

override fun handle(action: DeviceListAction) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,16 @@ import im.vector.app.core.di.SingletonEntryPoint
import im.vector.app.core.di.hiltMavericksViewModelFactory
import im.vector.app.core.extensions.hasUnsavedKeys
import im.vector.app.core.platform.VectorViewModel
import im.vector.app.features.displayname.getBestName
import im.vector.app.features.login.LoginMode
import im.vector.app.features.login.toSsoState
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.auth.AuthenticationService
import org.matrix.android.sdk.api.auth.LoginType
import org.matrix.android.sdk.api.auth.data.LoginFlowTypes
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.getUser
import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.util.toMatrixItem
import timber.log.Timber

class SoftLogoutViewModel @AssistedInject constructor(
Expand Down Expand Up @@ -68,7 +70,7 @@ class SoftLogoutViewModel @AssistedInject constructor(
homeServerUrl = session.sessionParams.homeServerUrl,
userId = userId,
deviceId = session.sessionParams.deviceId.orEmpty(),
userDisplayName = session.getUser(userId)?.displayName ?: userId,
userDisplayName = session.getUserOrDefault(userId).toMatrixItem().getBestName(),
hasUnsavedKeys = session.hasUnsavedKeys(),
loginType = session.sessionParams.loginType,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.getRoomSummary
import org.matrix.android.sdk.api.session.getUser
import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.session.room.model.Membership
import org.matrix.android.sdk.api.session.room.model.RoomSummary
import org.matrix.android.sdk.api.session.room.peeking.PeekResult
Expand All @@ -49,15 +49,15 @@ class SpaceInviteBottomSheetViewModel @AssistedInject constructor(
session.getRoomSummary(initialState.spaceId)?.let { roomSummary ->
val knownMembers = roomSummary.otherMemberIds.filter {
session.roomService().getExistingDirectRoomWithUser(it) != null
}.mapNotNull { session.getUser(it) }
}.map { session.getUserOrDefault(it) }
// put one with avatar first, and take 5
val peopleYouKnow = (knownMembers.filter { it.avatarUrl != null } + knownMembers.filter { it.avatarUrl == null })
.take(5)

setState {
copy(
summary = Success(roomSummary),
inviterUser = roomSummary.inviterId?.let { session.getUser(it) }?.let { Success(it) } ?: Uninitialized,
inviterUser = roomSummary.inviterId?.let { session.getUserOrDefault(it) }?.let { Success(it) } ?: Uninitialized,
peopleYouKnow = Success(peopleYouKnow)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.getUser
import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.session.permalinks.PermalinkData
import org.matrix.android.sdk.api.session.permalinks.PermalinkParser
import org.matrix.android.sdk.api.session.user.model.User
Expand All @@ -46,10 +46,10 @@ class UserCodeSharedViewModel @AssistedInject constructor(
companion object : MavericksViewModelFactory<UserCodeSharedViewModel, UserCodeState> by hiltMavericksViewModelFactory()

init {
val user = session.getUser(initialState.userId)
val user = session.getUserOrDefault(initialState.userId)
setState {
copy(
matrixItem = user?.toMatrixItem(),
matrixItem = user.toMatrixItem(),
shareLink = session.permalinkService().createPermalink(initialState.userId)
)
}
Expand Down