From 092fac3f1ee8fce5170812ca0facd9a1df83e270 Mon Sep 17 00:00:00 2001 From: HLCaptain Date: Sun, 26 Feb 2023 12:33:01 +0100 Subject: [PATCH 1/2] Fixed issue with Syncing User Preferences. --- .../jay/data/disk/dao/PreferencesDao.kt | 4 +- .../PreferencesNetworkDataSource.kt | 21 ++++- .../datasource/UserNetworkDataSource.kt | 91 ++++++++++++++----- .../domain/interactor/SettingsInteractor.kt | 17 ++-- .../jay/domain/model/DomainPreferences.kt | 2 +- 5 files changed, 99 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/illyan/jay/data/disk/dao/PreferencesDao.kt b/app/src/main/java/illyan/jay/data/disk/dao/PreferencesDao.kt index 9f723a37..ffb46b89 100644 --- a/app/src/main/java/illyan/jay/data/disk/dao/PreferencesDao.kt +++ b/app/src/main/java/illyan/jay/data/disk/dao/PreferencesDao.kt @@ -47,6 +47,6 @@ interface PreferencesDao { @Query("UPDATE preferences SET freeDriveAutoStart = :freeDriveAutoStart, lastUpdate = :lastUpdate WHERE userUUID IS :userUUID") fun setFreeDriveAutoStart(userUUID: String, freeDriveAutoStart: Boolean, lastUpdate: Long = Instant.now().toEpochMilli()) - @Query("UPDATE preferences SET shouldSync = :shouldSync, lastUpdate = :lastUpdate WHERE userUUID IS :userUUID") - fun setShouldSync(userUUID: String, shouldSync: Boolean, lastUpdate: Long = Instant.now().toEpochMilli()) + @Query("UPDATE preferences SET shouldSync = :shouldSync WHERE userUUID IS :userUUID") + fun setShouldSync(userUUID: String, shouldSync: Boolean) } \ No newline at end of file diff --git a/app/src/main/java/illyan/jay/data/network/datasource/PreferencesNetworkDataSource.kt b/app/src/main/java/illyan/jay/data/network/datasource/PreferencesNetworkDataSource.kt index 81d8e51f..0c0cd7f6 100644 --- a/app/src/main/java/illyan/jay/data/network/datasource/PreferencesNetworkDataSource.kt +++ b/app/src/main/java/illyan/jay/data/network/datasource/PreferencesNetworkDataSource.kt @@ -56,10 +56,23 @@ class PreferencesNetworkDataSource @Inject constructor( } else if (loading) { null } else { - // TODO: maybe insert settings - // possible states: - // - user not signed in: tell user they cannot sync settings? - // - user does not have settings yet + null + } + }.stateIn(coroutineScopeIO, SharingStarted.Eagerly, null) + } + + val cloudPreferences: StateFlow by lazy { + combine( + userNetworkDataSource.cloudUser, + userNetworkDataSource.isLoadingFromCloud + ) { user, loading -> + if (user?.preferences != null) { + val preferences = user.preferences.toDomainModel(userUUID = user.uuid) + Timber.d("Firebase got cloud preferences for user ${user.uuid.take(4)}") + preferences + } else if (loading) { + null + } else { null } }.stateIn(coroutineScopeIO, SharingStarted.Eagerly, null) diff --git a/app/src/main/java/illyan/jay/data/network/datasource/UserNetworkDataSource.kt b/app/src/main/java/illyan/jay/data/network/datasource/UserNetworkDataSource.kt index 6fc6c141..5d83dc6d 100644 --- a/app/src/main/java/illyan/jay/data/network/datasource/UserNetworkDataSource.kt +++ b/app/src/main/java/illyan/jay/data/network/datasource/UserNetworkDataSource.kt @@ -25,6 +25,7 @@ import com.google.firebase.firestore.DocumentSnapshot import com.google.firebase.firestore.EventListener import com.google.firebase.firestore.FirebaseFirestore import com.google.firebase.firestore.ListenerRegistration +import com.google.firebase.firestore.MetadataChanges import com.google.firebase.firestore.ktx.toObject import illyan.jay.data.network.model.FirestoreUser import illyan.jay.domain.interactor.AuthInteractor @@ -34,7 +35,9 @@ import kotlinx.coroutines.flow.asStateFlow import timber.log.Timber import java.util.concurrent.Executors import javax.inject.Inject +import javax.inject.Singleton +@Singleton class UserNetworkDataSource @Inject constructor( private val firestore: FirebaseFirestore, private val authInteractor: AuthInteractor, @@ -43,9 +46,21 @@ class UserNetworkDataSource @Inject constructor( private val _userListenerRegistration = MutableStateFlow(null) private val _userReference = MutableStateFlow(null) private val _user = MutableStateFlow(null) - val user: StateFlow get() { - if (_userListenerRegistration.value == null) loadUser() - return _user.asStateFlow() + private val _cloudUser = MutableStateFlow(null) + + val user: StateFlow by lazy { + if (_userListenerRegistration.value == null && !isLoading.value) { + Timber.d("User StateFlow requested, but listener registration is null, reloading it") + refreshUser() + } + _user.asStateFlow() + } + val cloudUser: StateFlow by lazy { + if (_userListenerRegistration.value == null && !isLoadingFromCloud.value) { + Timber.d("User StateFlow requested, but listener registration is null, reloading it") + refreshUser() + } + _cloudUser.asStateFlow() } private val _isLoading = MutableStateFlow(false) @@ -59,41 +74,53 @@ class UserNetworkDataSource @Inject constructor( init { authInteractor.addAuthStateListener { if (authInteractor.isUserSignedIn) { - Timber.d("Reloading snapshot listener for user ${_user.value?.uuid}") - loadUser() + if (authInteractor.userUUID != null && + authInteractor.userUUID != _user.value?.uuid + ) { + Timber.d("Reloading snapshot listener for user ${_user.value?.uuid?.take(4)}") + refreshUser() + } else { + Timber.d("User not changed from ${_user.value?.uuid?.take(4)}, not reloading snapshot listener on auth state change") + } } else { - Timber.d("Removing snapshot listener for user ${_user.value?.uuid}") - _userReference.value = null - _userListenerRegistration.value?.remove() - _user.value = null - _isLoading.value = false - _isLoadingFromCloud.value = false + Timber.d("Removing snapshot listener for user ${_user.value?.uuid?.take(4)}") + resetUserListenerData() } } appLifecycle.addObserver(this) } + private fun resetUserListenerData() { + _userListenerRegistration.value?.remove() + if (_userListenerRegistration.value != null) _userListenerRegistration.value = null + if (_userReference.value != null) _userReference.value = null + if (_isLoading.value) _isLoading.value = false + if (_isLoadingFromCloud.value) _isLoadingFromCloud.value = false + } + override fun onStart(owner: LifecycleOwner) { super.onStart(owner) - loadUser() + Timber.d("Reload user on App Lifecycle Start") + resetUserListenerData() + refreshUser() } override fun onStop(owner: LifecycleOwner) { super.onStop(owner) - _userListenerRegistration.value?.remove() - _userListenerRegistration.value = null + Timber.d("Remove user listener on App Lifecycle Stop") + resetUserListenerData() } - private fun loadUser( + private fun refreshUser( userUUID: String = authInteractor.userUUID.toString(), onError: (Exception) -> Unit = { Timber.e(it, "Error while getting user data: ${it.message}") }, onSuccess: (FirestoreUser) -> Unit = {}, ) { - if (!authInteractor.isUserSignedIn) return - if (_user.value == null) { - _isLoading.value = true - _isLoadingFromCloud.value = true + if (!authInteractor.isUserSignedIn || _isLoadingFromCloud.value) { + Timber.d("Not refreshing user, due to another being loaded in or user is not signed in") + return } + resetUserListenerData() Timber.d("Connecting snapshot listener to Firebase to get ${userUUID.take(4)} user's data") val snapshotListener = EventListener { snapshot, error -> Timber.v("New snapshot regarding user ${userUUID.take(4)}") @@ -104,7 +131,6 @@ class UserNetworkDataSource @Inject constructor( if (user == null) { onError(NoSuchElementException("User document does not exist")) } else { - Timber.d("Firebase loaded ${userUUID.take(4)} user's data") onSuccess(user) } if (snapshot != null) { @@ -114,24 +140,45 @@ class UserNetworkDataSource @Inject constructor( // If snapshot is null, then _userReference is invalid if not null. Assign null to it. _userReference.value = null } + + // Cache if (user != null) { + if (_user.value != null) { + Timber.v("Refreshing Cached ${userUUID.take(4)} user's data") + } else { + Timber.d("Firestore loaded ${userUUID.take(4)} user's data from Cache") + } _user.value = user } else if (_user.value != null) { + _user.value = null } if (_isLoading.value) { _isLoading.value = false } + + // Cloud + if (snapshot?.metadata?.isFromCache == false) { + if (user != null) { + if (_cloudUser.value != null) { + Timber.v("Firestore loaded fresh ${userUUID.take(4)} user's data from Cloud") + } else { + Timber.d("Firestore loaded ${userUUID.take(4)} user's data from Cloud") + } + _cloudUser.value = user + } else if (_cloudUser.value != null) { + _cloudUser.value = null + } + } if (_isLoadingFromCloud.value && snapshot?.metadata?.isFromCache == false) { _isLoadingFromCloud.value = false } } } - _userListenerRegistration.value?.remove() _userListenerRegistration.value = firestore .collection(FirestoreUser.CollectionName) .document(userUUID) - .addSnapshotListener(executor, snapshotListener) + .addSnapshotListener(executor, MetadataChanges.INCLUDE, snapshotListener) } fun deleteUserData( diff --git a/app/src/main/java/illyan/jay/domain/interactor/SettingsInteractor.kt b/app/src/main/java/illyan/jay/domain/interactor/SettingsInteractor.kt index c6d9bd42..d4fb1ab3 100644 --- a/app/src/main/java/illyan/jay/domain/interactor/SettingsInteractor.kt +++ b/app/src/main/java/illyan/jay/domain/interactor/SettingsInteractor.kt @@ -106,15 +106,20 @@ class SettingsInteractor @Inject constructor( val arePreferencesSynced by lazy { combine( isLoading, + preferencesNetworkDataSource.isLoadingFromCloud, localUserPreferences, syncedUserPreferences - ) { loading, local, synced -> - if (!loading) { + ) { loading, cloudLoading, local, synced -> + if (!loading && !cloudLoading) { local == synced } else { false } - }.stateIn(coroutineScopeIO, SharingStarted.Eagerly, false) + }.stateIn( + coroutineScopeIO, + SharingStarted.Eagerly, + localUserPreferences.value == syncedUserPreferences.value + ) } val shouldSyncPreferences by lazy { @@ -144,13 +149,12 @@ class SettingsInteractor @Inject constructor( return _isLocalLoading.asStateFlow() } - val syncedUserPreferences = preferencesNetworkDataSource.preferences + val syncedUserPreferences = preferencesNetworkDataSource.cloudPreferences val isLoading = combine( preferencesNetworkDataSource.isLoading, - preferencesNetworkDataSource.isLoadingFromCloud, isLocalLoading - ) { loadingFromCache, loadingFromCloud, loadingFromDisk -> + ) { loadingFromCache, loadingFromDisk -> loadingFromCache || loadingFromDisk }.stateIn(coroutineScopeIO, SharingStarted.Eagerly, false) @@ -216,7 +220,6 @@ class SettingsInteractor @Inject constructor( Timber.v("If cloud is loaded and local is not, returning cloud preferences") syncedPreferences } else { - // TODO: resolve preferences here, local and synced preferences are loaded if (localPreferences == null && syncedPreferences == null) { // User don't have local nor synced preferences? Create and upload local preferences. Timber.v("User don't have local nor synced preferences, create and upload one.") diff --git a/app/src/main/java/illyan/jay/domain/model/DomainPreferences.kt b/app/src/main/java/illyan/jay/domain/model/DomainPreferences.kt index 0534008a..1cd16b6b 100644 --- a/app/src/main/java/illyan/jay/domain/model/DomainPreferences.kt +++ b/app/src/main/java/illyan/jay/domain/model/DomainPreferences.kt @@ -30,7 +30,7 @@ data class DomainPreferences( val freeDriveAutoStart: Boolean = false, @Serializable(with = ZonedDateTimeSerializer::class) val lastUpdate: ZonedDateTime = ZonedDateTime.now(), - val shouldSync: Boolean = false + val shouldSync: Boolean = false // FIXME: change this to true, because user preferences would break bcuz it would never download cloud preferences. Maybe make sync not update lastupdate? ) { override fun equals(other: Any?): Boolean { return if (other != null && other is DomainPreferences) { From 3f87f1c729b37aff3e81029a7316e9716359074f Mon Sep 17 00:00:00 2001 From: HLCaptain Date: Sun, 26 Feb 2023 13:33:58 +0100 Subject: [PATCH 2/2] Fixed issue with property names, obfuscation caused on release builds. --- .../jay/data/disk/model/RoomPreferences.kt | 7 +++-- .../PreferencesNetworkDataSource.kt | 2 +- .../jay/data/network/model/FirestorePath.kt | 31 ++++++++++--------- .../data/network/model/FirestoreSession.kt | 19 ++++++------ .../jay/data/network/model/FirestoreUser.kt | 7 +++-- .../network/model/FirestoreUserPreferences.kt | 10 ++++-- .../jay/domain/model/DomainPreferences.kt | 2 +- 7 files changed, 43 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/illyan/jay/data/disk/model/RoomPreferences.kt b/app/src/main/java/illyan/jay/data/disk/model/RoomPreferences.kt index 2c85d6fc..ec845839 100644 --- a/app/src/main/java/illyan/jay/data/disk/model/RoomPreferences.kt +++ b/app/src/main/java/illyan/jay/data/disk/model/RoomPreferences.kt @@ -20,6 +20,7 @@ package illyan.jay.data.disk.model import androidx.room.Entity import androidx.room.PrimaryKey +import illyan.jay.domain.model.DomainPreferences import java.time.ZonedDateTime import java.util.UUID @@ -29,8 +30,8 @@ import java.util.UUID data class RoomPreferences( @PrimaryKey val userUUID: String = UUID.randomUUID().toString(), - val freeDriveAutoStart: Boolean = false, - val analyticsEnabled: Boolean = false, + val freeDriveAutoStart: Boolean = DomainPreferences.default.freeDriveAutoStart, + val analyticsEnabled: Boolean = DomainPreferences.default.analyticsEnabled, val lastUpdate: Long = ZonedDateTime.now().toInstant().toEpochMilli(), - val shouldSync: Boolean = false + val shouldSync: Boolean = DomainPreferences.default.shouldSync ) diff --git a/app/src/main/java/illyan/jay/data/network/datasource/PreferencesNetworkDataSource.kt b/app/src/main/java/illyan/jay/data/network/datasource/PreferencesNetworkDataSource.kt index 0c0cd7f6..43b09cb8 100644 --- a/app/src/main/java/illyan/jay/data/network/datasource/PreferencesNetworkDataSource.kt +++ b/app/src/main/java/illyan/jay/data/network/datasource/PreferencesNetworkDataSource.kt @@ -86,7 +86,7 @@ class PreferencesNetworkDataSource @Inject constructor( val userRef = firestore .collection(FirestoreUser.CollectionName) .document(authInteractor.userUUID!!) - val fieldMapToSet = mapOf(FirestoreUser.FieldSettings to preferences.toFirestoreModel()) + val fieldMapToSet = mapOf(FirestoreUser.FieldPreferences to preferences.toFirestoreModel()) batch.set( userRef, fieldMapToSet, diff --git a/app/src/main/java/illyan/jay/data/network/model/FirestorePath.kt b/app/src/main/java/illyan/jay/data/network/model/FirestorePath.kt index e051bfe8..3606d301 100644 --- a/app/src/main/java/illyan/jay/data/network/model/FirestorePath.kt +++ b/app/src/main/java/illyan/jay/data/network/model/FirestorePath.kt @@ -22,6 +22,7 @@ import android.os.Parcelable import com.google.firebase.Timestamp import com.google.firebase.firestore.DocumentId import com.google.firebase.firestore.GeoPoint +import com.google.firebase.firestore.PropertyName import kotlinx.parcelize.Parcelize import kotlinx.parcelize.TypeParceler @@ -31,21 +32,21 @@ import kotlinx.parcelize.TypeParceler data class FirestorePath( @DocumentId val uuid: String = "", - val sessionUUID: String = "", // reference of the session this path is part of - val ownerUUID: String = "", - val accuracyChangeTimestamps: List = emptyList(), - val accuracyChanges: List = emptyList(), - val altitudes: List = emptyList(), - val bearingAccuracyChangeTimestamps: List = emptyList(), - val bearingAccuracyChanges: List = emptyList(), - val bearings: List = emptyList(), - val coords: List = emptyList(), - val speeds: List = emptyList(), - val speedAccuracyChangeTimestamps: List = emptyList(), - val speedAccuracyChanges: List = emptyList(), - val timestamps: List = emptyList(), - val verticalAccuracyChangeTimestamps: List = emptyList(), - val verticalAccuracyChanges: List = emptyList() + @PropertyName(FieldSessionUUID) val sessionUUID: String = "", // reference of the session this path is part of + @PropertyName(FieldOwnerUUID) val ownerUUID: String = "", + @PropertyName(FieldAccuracyChangeTimestamps) val accuracyChangeTimestamps: List = emptyList(), + @PropertyName(FieldAccuracyChanges) val accuracyChanges: List = emptyList(), + @PropertyName(FieldAltitudes) val altitudes: List = emptyList(), + @PropertyName(FieldBearingAccuracyChangeTimestamps) val bearingAccuracyChangeTimestamps: List = emptyList(), + @PropertyName(FieldBearingAccuracyChanges) val bearingAccuracyChanges: List = emptyList(), + @PropertyName(FieldBearings) val bearings: List = emptyList(), + @PropertyName(FieldCoords) val coords: List = emptyList(), + @PropertyName(FieldSpeeds) val speeds: List = emptyList(), + @PropertyName(FieldSpeedAccuracyChangeTimestamps) val speedAccuracyChangeTimestamps: List = emptyList(), + @PropertyName(FieldSpeedAccuracyChanges) val speedAccuracyChanges: List = emptyList(), + @PropertyName(FieldTimestamps) val timestamps: List = emptyList(), + @PropertyName(FieldVerticalAccuracyChangeTimestamps) val verticalAccuracyChangeTimestamps: List = emptyList(), + @PropertyName(FieldVerticalAccuracyChanges) val verticalAccuracyChanges: List = emptyList() ) : Parcelable { companion object { const val CollectionName = "paths" diff --git a/app/src/main/java/illyan/jay/data/network/model/FirestoreSession.kt b/app/src/main/java/illyan/jay/data/network/model/FirestoreSession.kt index 6b1cbbb7..287b8ac2 100644 --- a/app/src/main/java/illyan/jay/data/network/model/FirestoreSession.kt +++ b/app/src/main/java/illyan/jay/data/network/model/FirestoreSession.kt @@ -20,19 +20,20 @@ package illyan.jay.data.network.model import com.google.firebase.Timestamp import com.google.firebase.firestore.GeoPoint +import com.google.firebase.firestore.PropertyName import illyan.jay.util.toTimestamp import java.time.Instant data class FirestoreSession( - val uuid: String = "", - val startDateTime: Timestamp = Instant.EPOCH.toTimestamp(), - val endDateTime: Timestamp? = null, - val startLocation: GeoPoint? = null, - val endLocation: GeoPoint? = null, - val startLocationName: String? = null, - val endLocationName: String? = null, - val distance: Float? = null, - val clientUUID: String? = null + @PropertyName(FieldUUID) val uuid: String = "", + @PropertyName(FieldStartDateTime) val startDateTime: Timestamp = Instant.EPOCH.toTimestamp(), + @PropertyName(FieldEndDateTime) val endDateTime: Timestamp? = null, + @PropertyName(FieldStartLocation) val startLocation: GeoPoint? = null, + @PropertyName(FieldEndLocation) val endLocation: GeoPoint? = null, + @PropertyName(FieldStartLocationName) val startLocationName: String? = null, + @PropertyName(FieldEndLocationName) val endLocationName: String? = null, + @PropertyName(FieldDistance) val distance: Float? = null, + @PropertyName(FieldClientUUID) val clientUUID: String? = null ) { companion object { const val FieldUUID = "uuid" diff --git a/app/src/main/java/illyan/jay/data/network/model/FirestoreUser.kt b/app/src/main/java/illyan/jay/data/network/model/FirestoreUser.kt index 77f6afc7..fb954398 100644 --- a/app/src/main/java/illyan/jay/data/network/model/FirestoreUser.kt +++ b/app/src/main/java/illyan/jay/data/network/model/FirestoreUser.kt @@ -19,16 +19,17 @@ package illyan.jay.data.network.model import com.google.firebase.firestore.DocumentId +import com.google.firebase.firestore.PropertyName data class FirestoreUser( @DocumentId val uuid: String = "", - val sessions: List = emptyList(), - val preferences: FirestoreUserPreferences? = null + @PropertyName(FieldSessions) val sessions: List = emptyList(), + @PropertyName(FieldPreferences) val preferences: FirestoreUserPreferences? = null ) { companion object { const val CollectionName = "users" const val FieldSessions = "sessions" - const val FieldSettings = "preferences" + const val FieldPreferences = "preferences" } } \ No newline at end of file diff --git a/app/src/main/java/illyan/jay/data/network/model/FirestoreUserPreferences.kt b/app/src/main/java/illyan/jay/data/network/model/FirestoreUserPreferences.kt index e5301715..8ad14784 100644 --- a/app/src/main/java/illyan/jay/data/network/model/FirestoreUserPreferences.kt +++ b/app/src/main/java/illyan/jay/data/network/model/FirestoreUserPreferences.kt @@ -19,13 +19,17 @@ package illyan.jay.data.network.model import com.google.firebase.Timestamp +import com.google.firebase.firestore.PropertyName +import illyan.jay.domain.model.DomainPreferences data class FirestoreUserPreferences( - val analyticsEnabled: Boolean = false, - val freeDriveAutoStart: Boolean = false, - val lastUpdate: Timestamp = Timestamp.now() + @PropertyName(FieldAnalyticsEnabled) val analyticsEnabled: Boolean = DomainPreferences.default.analyticsEnabled, + @PropertyName(FieldFreeDriveAutoStart) val freeDriveAutoStart: Boolean = DomainPreferences.default.freeDriveAutoStart, + @PropertyName(FieldLastUpdate) val lastUpdate: Timestamp = Timestamp.now() ) { companion object { const val FieldAnalyticsEnabled = "analyticsEnabled" + const val FieldFreeDriveAutoStart = "freeDriveAutoStart" + const val FieldLastUpdate = "lastUpdate" } } diff --git a/app/src/main/java/illyan/jay/domain/model/DomainPreferences.kt b/app/src/main/java/illyan/jay/domain/model/DomainPreferences.kt index 1cd16b6b..0534008a 100644 --- a/app/src/main/java/illyan/jay/domain/model/DomainPreferences.kt +++ b/app/src/main/java/illyan/jay/domain/model/DomainPreferences.kt @@ -30,7 +30,7 @@ data class DomainPreferences( val freeDriveAutoStart: Boolean = false, @Serializable(with = ZonedDateTimeSerializer::class) val lastUpdate: ZonedDateTime = ZonedDateTime.now(), - val shouldSync: Boolean = false // FIXME: change this to true, because user preferences would break bcuz it would never download cloud preferences. Maybe make sync not update lastupdate? + val shouldSync: Boolean = false ) { override fun equals(other: Any?): Boolean { return if (other != null && other is DomainPreferences) {