Skip to content

Commit

Permalink
replace userManager.currentUser usage by currentUserProvider usage
Browse files Browse the repository at this point in the history
userManager.currentUser was called too often. I was not able to prove that a bug is related to it but i think it may fix some hidden bugs.

CurrentUserProviderImpl is now used throughout the code to access the current user.
userManager.currentUser is only used from CurrentUserProviderImpl whenever the _currentUser was null (should only happen on app startup)

To avoid multiple initialization of CurrentUserProviderImpl it was changed to be a @singleton

The handling should soon be replaced with coroutine flows. However for the v21.0.0 release it's still done with RxJava to avoid bugs.

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
  • Loading branch information
mahibi committed Feb 6, 2025
1 parent b747765 commit 99a190d
Show file tree
Hide file tree
Showing 33 changed files with 140 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public void login() throws InterruptedException {

activityScenario.onActivity(activity -> {
assertEquals(loginName,
Objects.requireNonNull(activity.userManager.getCurrentUser().blockingGet()).getUserId());
Objects.requireNonNull(activity.currentUserProvider.currentUser.blockingGet()).getUserId());
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ class AccountVerificationActivity : BaseActivity() {
cookieManager.cookieStore.removeAll()

if (userManager.users.blockingGet().size == 1 ||
userManager.currentUser.blockingGet().id != internalAccountId
currentUserProvider.currentUser.blockingGet().id != internalAccountId
) {
val userToSetAsActive = userManager.getUserWithId(internalAccountId).blockingGet()
Log.d(TAG, "userToSetAsActive: " + userToSetAsActive.username)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ class WebViewLoginActivity : BaseActivity() {
if (!reauthorizeAccount) {
alias = appPreferences.temporaryClientCertAlias
}
val user = userManager.currentUser.blockingGet()
val user = currentUserProvider.currentUser.blockingGet()
if (TextUtils.isEmpty(alias) && user != null) {
alias = user.clientCertificate
}
Expand Down Expand Up @@ -373,7 +373,7 @@ class WebViewLoginActivity : BaseActivity() {
}

private fun updateUserAndRestartApp(loginData: LoginData) {
val currentUser = userManager.currentUser.blockingGet()
val currentUser = currentUserProvider.currentUser.blockingGet()
if (currentUser != null) {
currentUser.clientCertificate = appPreferences.temporaryClientCertAlias
currentUser.token = loginData.token
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ class MainActivity : BaseActivity(), ActionBarProvider {
val user = userId.substringBeforeLast("@")
val baseUrl = userId.substringAfterLast("@")

if (userManager.currentUser.blockingGet()?.baseUrl!!.endsWith(baseUrl) == true) {
if (currentUserProvider.currentUser.blockingGet()?.baseUrl!!.endsWith(baseUrl) == true) {
startConversation(user)
} else {
Snackbar.make(
Expand All @@ -181,7 +181,7 @@ class MainActivity : BaseActivity(), ActionBarProvider {
private fun startConversation(userId: String) {
val roomType = "1"

val currentUser = userManager.currentUser.blockingGet()
val currentUser = currentUserProvider.currentUser.blockingGet()

val apiVersion = ApiUtils.getConversationApiVersion(currentUser, intArrayOf(ApiUtils.API_V4, 1))
val credentials = ApiUtils.getCredentials(currentUser?.username, currentUser?.token)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ import com.nextcloud.talk.utils.CapabilitiesUtil
import com.nextcloud.talk.utils.CharPolicy
import com.nextcloud.talk.utils.ImageEmojiEditText
import com.nextcloud.talk.utils.SpreedFeatures
import com.nextcloud.talk.utils.database.user.CurrentUserProviderNew
import com.nextcloud.talk.utils.text.Spans
import com.otaliastudios.autocomplete.Autocomplete
import com.stfalcon.chatkit.commons.models.IMessage
Expand Down Expand Up @@ -116,6 +117,9 @@ class MessageInputFragment : Fragment() {
@Inject
lateinit var userManager: UserManager

@Inject
lateinit var currentUserProvider: CurrentUserProviderNew

@Inject
lateinit var networkMonitor: NetworkMonitor

Expand Down Expand Up @@ -217,7 +221,7 @@ class MessageInputFragment : Fragment() {
if (show) {
binding.fragmentCallStarted.callAuthorChip.text = message.actorDisplayName
binding.fragmentCallStarted.callAuthorChipSecondary.text = message.actorDisplayName
val user = userManager.currentUser.blockingGet()
val user = currentUserProvider.currentUser.blockingGet()
val url: String = if (message.actorType == "guests" || message.actorType == "guest") {
ApiUtils.getUrlForGuestAvatar(user!!.baseUrl!!, message.actorDisplayName, true)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ class ContactsActivity :
}
}

currentUser = userManager.currentUser.blockingGet()
currentUser = currentUserProvider.currentUser.blockingGet()
if (currentUser != null) {
credentials = ApiUtils.getCredentials(currentUser!!.username, currentUser!!.token)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@ import com.nextcloud.talk.data.user.model.User
import com.nextcloud.talk.models.RetrofitBucket
import com.nextcloud.talk.models.json.autocomplete.AutocompleteOverall
import com.nextcloud.talk.models.json.conversations.RoomOverall
import com.nextcloud.talk.users.UserManager
import com.nextcloud.talk.utils.ApiUtils
import com.nextcloud.talk.utils.ContactUtils
import com.nextcloud.talk.utils.database.user.CurrentUserProviderNew
import javax.inject.Inject

class ContactsRepositoryImpl(
class ContactsRepositoryImpl @Inject constructor(
private val ncApiCoroutines: NcApiCoroutines,
private val userManager: UserManager
currentUserProvider: CurrentUserProviderNew
) : ContactsRepository {
private val _currentUser = userManager.currentUser.blockingGet()

private val _currentUser = currentUserProvider.currentUser.blockingGet()
val currentUser: User = _currentUser
val credentials = ApiUtils.getCredentials(_currentUser.username, _currentUser.token)
val apiVersion = ApiUtils.getConversationApiVersion(_currentUser, intArrayOf(ApiUtils.API_V4, 1))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,22 @@ import com.nextcloud.talk.models.domain.ConversationModel
import com.nextcloud.talk.models.json.conversations.RoomOverall
import com.nextcloud.talk.models.json.generic.GenericOverall
import com.nextcloud.talk.models.json.participants.AddParticipantOverall
import com.nextcloud.talk.users.UserManager
import com.nextcloud.talk.utils.ApiUtils
import com.nextcloud.talk.utils.ApiUtils.getRetrofitBucketForAddParticipant
import com.nextcloud.talk.utils.ApiUtils.getRetrofitBucketForAddParticipantWithSource
import com.nextcloud.talk.utils.Mimetype
import com.nextcloud.talk.utils.database.user.CurrentUserProviderNew
import okhttp3.MediaType.Companion.toMediaTypeOrNull
import okhttp3.MultipartBody
import okhttp3.RequestBody.Companion.asRequestBody
import java.io.File
import javax.inject.Inject

class ConversationCreationRepositoryImpl(
class ConversationCreationRepositoryImpl @Inject constructor(
private val ncApiCoroutines: NcApiCoroutines,
private val userManager: UserManager
currentUserProvider: CurrentUserProviderNew
) : ConversationCreationRepository {
private val _currentUser = userManager.currentUser.blockingGet()
private val _currentUser = currentUserProvider.currentUser.blockingGet()
val currentUser: User = _currentUser
val credentials = ApiUtils.getCredentials(_currentUser.username, _currentUser.token)
val apiVersion = ApiUtils.getConversationApiVersion(_currentUser, intArrayOf(ApiUtils.API_V4, ApiUtils.API_V1))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ import com.nextcloud.talk.models.json.autocomplete.AutocompleteUser
import com.nextcloud.talk.models.json.conversations.Conversation
import com.nextcloud.talk.models.json.generic.GenericMeta
import com.nextcloud.talk.repositories.conversations.ConversationsRepositoryImpl.Companion.STATUS_CODE_OK
import com.nextcloud.talk.users.UserManager
import com.nextcloud.talk.utils.database.user.CurrentUserProviderNew
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.launch
import javax.inject.Inject

class ConversationCreationViewModel @Inject constructor(
private val repository: ConversationCreationRepository,
private val userManager: UserManager
private val currentUserProvider: CurrentUserProviderNew
) : ViewModel() {
private val _selectedParticipants = MutableStateFlow<List<AutocompleteUser>>(emptyList())
val selectedParticipants: StateFlow<List<AutocompleteUser>> = _selectedParticipants
Expand All @@ -35,7 +35,7 @@ class ConversationCreationViewModel @Inject constructor(
private val _selectedImageUri = MutableStateFlow<Uri?>(null)
val selectedImageUri: StateFlow<Uri?> = _selectedImageUri

private val _currentUser = userManager.currentUser.blockingGet()
private val _currentUser = currentUserProvider.currentUser.blockingGet()
val currentUser: User = _currentUser

private val _isPasswordEnabled = mutableStateOf(false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import com.nextcloud.talk.conversationlist.data.OfflineConversationsRepository
import com.nextcloud.talk.invitation.data.InvitationsModel
import com.nextcloud.talk.invitation.data.InvitationsRepository
import com.nextcloud.talk.users.UserManager
import com.nextcloud.talk.utils.database.user.CurrentUserProviderNew
import io.reactivex.Observer
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.Disposable
Expand All @@ -31,6 +32,9 @@ class ConversationsListViewModel @Inject constructor(
@Inject
lateinit var invitationsRepository: InvitationsRepository

@Inject
lateinit var currentUserProvider: CurrentUserProviderNew

sealed interface ViewState

object GetRoomsStartState : ViewState
Expand Down Expand Up @@ -90,7 +94,7 @@ class ConversationsListViewModel @Inject constructor(
}

override fun onNext(invitationsModel: InvitationsModel) {
val currentUser = userManager.currentUser.blockingGet()
val currentUser = currentUserProvider.currentUser.blockingGet()

if (invitationsModel.user.userId?.equals(currentUser.userId) == true &&
invitationsModel.user.baseUrl?.equals(currentUser.baseUrl) == true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ import com.nextcloud.talk.shareditems.repositories.SharedItemsRepository
import com.nextcloud.talk.shareditems.repositories.SharedItemsRepositoryImpl
import com.nextcloud.talk.translate.repositories.TranslateRepository
import com.nextcloud.talk.translate.repositories.TranslateRepositoryImpl
import com.nextcloud.talk.users.UserManager
import com.nextcloud.talk.utils.DateUtils
import com.nextcloud.talk.utils.database.user.CurrentUserProviderNew
import dagger.Module
Expand Down Expand Up @@ -183,12 +182,14 @@ class RepositoryModule {
)

@Provides
fun provideContactsRepository(ncApiCoroutines: NcApiCoroutines, userManager: UserManager): ContactsRepository =
ContactsRepositoryImpl(ncApiCoroutines, userManager)
fun provideContactsRepository(
ncApiCoroutines: NcApiCoroutines,
currentUserProviderNew: CurrentUserProviderNew
): ContactsRepository = ContactsRepositoryImpl(ncApiCoroutines, currentUserProviderNew)

@Provides
fun provideConversationCreationRepository(
ncApiCoroutines: NcApiCoroutines,
userManager: UserManager
): ConversationCreationRepository = ConversationCreationRepositoryImpl(ncApiCoroutines, userManager)
currentUserProviderNew: CurrentUserProviderNew
): ConversationCreationRepository = ConversationCreationRepositoryImpl(ncApiCoroutines, currentUserProviderNew)
}
22 changes: 10 additions & 12 deletions app/src/main/java/com/nextcloud/talk/diagnose/DiagnoseActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -306,51 +306,49 @@ class DiagnoseActivity : BaseActivity() {
}

private fun setupAccountValues() {
val currentUser = currentUserProvider.currentUser.blockingGet()

addHeadline(context.resources.getString(R.string.nc_diagnose_account_category_title))

addKey(context.resources.getString(R.string.nc_diagnose_account_server))
addValue(userManager.currentUser.blockingGet().baseUrl!!)
addValue(currentUser.baseUrl!!)

addKey(context.resources.getString(R.string.nc_diagnose_account_user_name))
addValue(userManager.currentUser.blockingGet().displayName!!)
addValue(currentUser.displayName!!)

addKey(context.resources.getString(R.string.nc_diagnose_account_user_status_enabled))
addValue(
translateBoolean(
(userManager.currentUser.blockingGet().capabilities?.userStatusCapability?.enabled)
(currentUser.capabilities?.userStatusCapability?.enabled)
)
)

addKey(context.resources.getString(R.string.nc_diagnose_account_server_notification_app))
addValue(
translateBoolean(
userManager.currentUser.blockingGet().capabilities?.notificationsCapability?.features?.isNotEmpty()
)
translateBoolean(currentUser.capabilities?.notificationsCapability?.features?.isNotEmpty())
)

if (isGooglePlayServicesAvailable) {
setupPushRegistrationDiagnose()
}

addKey(context.resources.getString(R.string.nc_diagnose_server_version))
addValue(userManager.currentUser.blockingGet().serverVersion?.versionString!!)
addValue(currentUser.serverVersion?.versionString!!)

addKey(context.resources.getString(R.string.nc_diagnose_server_talk_version))
addValue(userManager.currentUser.blockingGet().capabilities?.spreedCapability?.version!!)
addValue(currentUser.capabilities?.spreedCapability?.version!!)

addKey(context.resources.getString(R.string.nc_diagnose_signaling_mode_title))

if (userManager.currentUser.blockingGet().externalSignalingServer?.externalSignalingServer?.isNotEmpty()
== true
) {
if (currentUser.externalSignalingServer?.externalSignalingServer?.isNotEmpty() == true) {
addValue(context.resources.getString(R.string.nc_diagnose_signaling_mode_extern))
} else {
addValue(context.resources.getString(R.string.nc_diagnose_signaling_mode_intern))
}
}

private fun setupPushRegistrationDiagnose() {
val accountId = UserIdUtils.getIdForUser(userManager.currentUser.blockingGet())
val accountId = UserIdUtils.getIdForUser(currentUserProvider.currentUser.blockingGet())

val latestPushRegistrationAtServer = arbitraryStorageManager.getStorageSetting(
accountId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import com.nextcloud.talk.users.UserManager
import com.nextcloud.talk.utils.ApiUtils
import com.nextcloud.talk.utils.ContactUtils
import com.nextcloud.talk.utils.DateConstants
import com.nextcloud.talk.utils.database.user.CurrentUserProviderNew
import com.nextcloud.talk.utils.preferences.AppPreferences
import io.reactivex.Observer
import io.reactivex.android.schedulers.AndroidSchedulers
Expand All @@ -57,6 +58,9 @@ class ContactAddressBookWorker(val context: Context, workerParameters: WorkerPar
@Inject
lateinit var userManager: UserManager

@Inject
lateinit var currentUserProvider: CurrentUserProviderNew

@Inject
lateinit var appPreferences: AppPreferences

Expand All @@ -66,7 +70,7 @@ class ContactAddressBookWorker(val context: Context, workerParameters: WorkerPar
override fun doWork(): Result {
sharedApplication!!.componentApplication.inject(this)

val currentUser = userManager.currentUser.blockingGet()
val currentUser = currentUserProvider.currentUser.blockingGet()

accountName = context.getString(R.string.nc_app_product_name)
accountType = BuildConfig.APPLICATION_ID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import com.nextcloud.talk.application.NextcloudTalkApplication
import com.nextcloud.talk.data.user.model.User
import com.nextcloud.talk.users.UserManager
import com.nextcloud.talk.utils.ApiUtils
import com.nextcloud.talk.utils.database.user.CurrentUserProviderNew
import com.nextcloud.talk.utils.preferences.AppPreferences
import okhttp3.ResponseBody
import java.io.BufferedInputStream
Expand All @@ -39,6 +40,9 @@ class DownloadFileToCacheWorker(val context: Context, workerParameters: WorkerPa
@Inject
lateinit var userManager: UserManager

@Inject
lateinit var currentUserProvider: CurrentUserProviderNew

@Inject
lateinit var appPreferences: AppPreferences

Expand All @@ -50,7 +54,7 @@ class DownloadFileToCacheWorker(val context: Context, workerParameters: WorkerPa
}

try {
val currentUser = userManager.currentUser.blockingGet()
val currentUser = currentUserProvider.currentUser.blockingGet()
val baseUrl = inputData.getString(KEY_BASE_URL)
val userId = inputData.getString(KEY_USER_ID)
val attachmentFolder = inputData.getString(KEY_ATTACHMENT_FOLDER)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import com.nextcloud.talk.utils.ApiUtils.getConversationApiVersion
import com.nextcloud.talk.utils.ApiUtils.getCredentials
import com.nextcloud.talk.utils.ApiUtils.getUrlForParticipantsSelf
import com.nextcloud.talk.utils.bundle.BundleKeys
import com.nextcloud.talk.utils.database.user.CurrentUserProviderNew
import io.reactivex.Observer
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.Disposable
Expand All @@ -43,12 +44,15 @@ class LeaveConversationWorker(context: Context, workerParams: WorkerParameters)
@Inject
lateinit var userManager: UserManager

@Inject
lateinit var currentUserProvider: CurrentUserProviderNew

private val result = SettableFuture.create<Result>()

override fun startWork(): ListenableFuture<Result> {
NextcloudTalkApplication.sharedApplication!!.componentApplication.inject(this)
val conversationToken = inputData.getString(BundleKeys.KEY_ROOM_TOKEN)
val currentUser = userManager.currentUser.blockingGet()
val currentUser = currentUserProvider.currentUser.blockingGet()

if (currentUser != null && conversationToken != null) {
val credentials = getCredentials(currentUser.username, currentUser.token)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import com.nextcloud.talk.utils.NotificationUtils
import com.nextcloud.talk.utils.RemoteFileUtils
import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_INTERNAL_USER_ID
import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN
import com.nextcloud.talk.utils.database.user.CurrentUserProviderNew
import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil
import com.nextcloud.talk.utils.preferences.AppPreferences
import okhttp3.MediaType.Companion.toMediaTypeOrNull
Expand All @@ -58,6 +59,9 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa
@Inject
lateinit var userManager: UserManager

@Inject
lateinit var currentUserProvider: CurrentUserProviderNew

@Inject
lateinit var appPreferences: AppPreferences

Expand Down Expand Up @@ -95,7 +99,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa
}

return try {
currentUser = userManager.currentUser.blockingGet()
currentUser = currentUserProvider.currentUser.blockingGet()
val sourceFile = inputData.getString(DEVICE_SOURCE_FILE)
roomToken = inputData.getString(ROOM_TOKEN)!!
conversationName = inputData.getString(CONVERSATION_NAME)!!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ class LocationPickerActivity :
"{\"type\":\"geo-location\",\"id\":\"geo:$selectedLat,$selectedLon\",\"latitude\":\"$selectedLat\"," +
"\"longitude\":\"$selectedLon\",\"name\":\"$locationNameToShare\"}"

val currentUser = userManager.currentUser.blockingGet()
val currentUser = currentUserProvider.currentUser.blockingGet()

ncApi.sendLocation(
ApiUtils.getCredentials(currentUser.username, currentUser.token),
Expand Down
Loading

0 comments on commit 99a190d

Please sign in to comment.