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.

Sign up for GitHub

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

db refactor #973

Merged
merged 17 commits into from
Jul 12, 2023
Merged

db refactor #973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9299528
db refactor
yate Jul 5, 2023
52b5cf9
Merge branch 'main' into db-refactor
yate Jul 5, 2023
71fda4f
formatting
yate Jul 5, 2023
2268b6f
fix race condition where it would sometimes get the site info for the…
yate Jul 5, 2023
4127be2
Merge branch 'main' into db-refactor
yate Jul 5, 2023
8db1842
add back the login icon enable condition I accidentally removed
yate Jul 5, 2023
5c0b689
Merge branch 'main' into db-refactor
yate Jul 8, 2023
d0c7f3c
merge main/fix conflicts
yate Jul 8, 2023
2ad2dbc
Merge branch 'main' into db-refactor
yate Jul 9, 2023
bf13d05
import reorder
yate Jul 9, 2023
076491d
Merge branch 'main' into db-refactor
yate Jul 12, 2023
7bac450
remove viewModel injection for view model parameters
yate Jul 12, 2023
010eefd
- move view models to model package
yate Jul 12, 2023
5f3ab5a
formatting
yate Jul 12, 2023
d5d7c6b
formatting
yate Jul 12, 2023
5970f59
Merge branch 'main' into db-refactor
MV-GH Jul 12, 2023
df94188
Merge branch 'main' into db-refactor
MV-GH Jul 12, 2023
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
50 changes: 26 additions & 24 deletions app/src/main/java/com/jerboa/MainActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import androidx.compose.runtime.livedata.observeAsState
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.ui.platform.LocalContext
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.viewmodel.CreationExtras
import androidx.lifecycle.viewmodel.compose.viewModel
import androidx.navigation.compose.NavHost
import androidx.navigation.compose.composable
Expand All @@ -36,15 +38,13 @@ import com.jerboa.api.API
import com.jerboa.api.ApiState
import com.jerboa.api.MINIMUM_API_VERSION
import com.jerboa.db.APP_SETTINGS_DEFAULT
import com.jerboa.db.AccountRepository
import com.jerboa.db.AccountViewModel
import com.jerboa.db.AccountViewModelFactory
import com.jerboa.db.AppDB
import com.jerboa.db.AppSettingsRepository
import com.jerboa.db.AppSettingsViewModel
import com.jerboa.db.AppSettingsViewModelFactory
import com.jerboa.db.AppDBContainer
import com.jerboa.model.AccountSettingsViewModel
import com.jerboa.model.AccountSettingsViewModelFactory
import com.jerboa.model.AccountViewModel
import com.jerboa.model.AccountViewModelFactory
import com.jerboa.model.AppSettingsViewModel
import com.jerboa.model.AppSettingsViewModelFactory
import com.jerboa.model.CommunityViewModel
import com.jerboa.model.ReplyItem
import com.jerboa.model.SiteViewModel
Expand All @@ -58,7 +58,7 @@ import com.jerboa.ui.components.common.Route
import com.jerboa.ui.components.common.ShowChangelog
import com.jerboa.ui.components.common.ShowOutdatedServerDialog
import com.jerboa.ui.components.common.SwipeToNavigateBack
import com.jerboa.ui.components.common.getCurrentAccountSync
import com.jerboa.ui.components.common.getCurrentAccount
import com.jerboa.ui.components.common.takeDepsFromRoot
import com.jerboa.ui.components.common.toView
import com.jerboa.ui.components.community.CommunityActivity
Expand Down Expand Up @@ -88,30 +88,28 @@ import com.jerboa.util.BackConfirmationMode
import com.jerboa.util.ShowConfirmationDialog

class JerboaApplication : Application() {
private val database by lazy { AppDB.getDatabase(this) }
val accountRepository by lazy { AccountRepository(database.accountDao()) }
val appSettingsRepository by lazy { AppSettingsRepository(database.appSettingsDao()) }
lateinit var container: AppDBContainer

override fun onCreate() {
super.onCreate()
container = AppDBContainer(this)
}
}

fun CreationExtras.jerboaApplication(): JerboaApplication =
(this[ViewModelProvider.AndroidViewModelFactory.APPLICATION_KEY] as JerboaApplication)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is referenced in view model factories here https://github.com/dessalines/jerboa/pull/973/files#diff-41754fa505a06441b96271ba020ae2e014472578466e14a44fd7ce01a6c578f4R37, based on the compose room workshop again https://github.com/google-developer-training/basic-android-kotlin-compose-training-inventory-app/blob/room/app/src/main/java/com/example/inventory/ui/AppViewModelProvider.kt#L65

class MainActivity : AppCompatActivity() {
private val siteViewModel by viewModels<SiteViewModel>()
private val accountSettingsViewModel by viewModels<AccountSettingsViewModel> {
AccountSettingsViewModelFactory((application as JerboaApplication).accountRepository)
}
private val accountViewModel: AccountViewModel by viewModels {
AccountViewModelFactory((application as JerboaApplication).accountRepository)
}
private val appSettingsViewModel: AppSettingsViewModel by viewModels {
AppSettingsViewModelFactory((application as JerboaApplication).appSettingsRepository)
}

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

val accountSync = getCurrentAccountSync(accountViewModel)

setContent {
val ctx = LocalContext.current
val accountViewModel: AccountViewModel = viewModel(factory = AccountViewModelFactory.Factory)
val appSettingsViewModel: AppSettingsViewModel = viewModel(factory = AppSettingsViewModelFactory.Factory)
val accountSettingsViewModel: AccountSettingsViewModel = viewModel(factory = AccountSettingsViewModelFactory.Factory)

API.errorHandler = {
Log.e("jerboa", it.toString())
Expand All @@ -125,8 +123,12 @@ class MainActivity : AppCompatActivity() {
null
}

LaunchedEffect(Unit) {
fetchInitialData(accountSync, siteViewModel)
val account = getCurrentAccount(accountViewModel)

LaunchedEffect(account) {
if (account == null || account.id != -1) {
Copy link
Contributor Author

@yate yate Jul 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There was an occasional race condition type issue where getCurrentAccount returns an initial value of null because it hasn't gotten the data from the database yet and there was no initial state set on the observeAsState method here https://github.com/dessalines/jerboa/pull/973/files#diff-697367ddff23d571ded643b597f312aeba4b29669b058e9edb03edf9bb030d92R14. It ended up making two calls to fetchInitialData. Once with the null initial value, then again with the actual data from the database. This caused two site data calls to go out, depending on which one returned first, the user data in the siteViewModel could set incorrectly set to the wrong value.

As a solution to this, I set an initial state value in observeAsState, so I know if the account id is -1, we haven't actually gotten any data yet from Room, so there is no need to call fetchInitialData. If there is no account in Room, it will return null and work like before.

Let me know if there's a better way to handle something like this

Copy link
Member

Choose a reason for hiding this comment

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

Seems fine to me.

fetchInitialData(account, siteViewModel)
}
}

val appSettings by appSettingsViewModel.appSettings.observeAsState(APP_SETTINGS_DEFAULT)
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/com/jerboa/Utils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ import com.jerboa.api.API
import com.jerboa.api.ApiState
import com.jerboa.api.DEFAULT_INSTANCE
import com.jerboa.datatypes.types.*
import com.jerboa.db.Account
import com.jerboa.db.entity.Account
import com.jerboa.model.HomeViewModel
import com.jerboa.model.SiteViewModel
import com.jerboa.ui.components.common.Route
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/com/jerboa/api/Http.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package com.jerboa.api
import android.content.Context
import android.util.Log
import com.jerboa.datatypes.types.*
import com.jerboa.db.Account
import com.jerboa.db.entity.Account
import com.jerboa.toastException
import com.jerboa.util.CustomHttpLoggingInterceptor
import okhttp3.MultipartBody
Expand Down
Loading