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

db refactor #973

Merged
merged 17 commits into from
Jul 12, 2023
Merged
72 changes: 23 additions & 49 deletions app/src/main/java/com/jerboa/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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 @@ -32,11 +34,9 @@ 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.AppDBContainer
import com.jerboa.db.AppSettingsViewModel
import com.jerboa.db.AppSettingsViewModelFactory
import com.jerboa.ui.components.comment.edit.CommentEditActivity
Expand All @@ -50,7 +50,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.community.CommunityActivity
import com.jerboa.ui.components.community.CommunityViewModel
Expand All @@ -71,35 +71,28 @@ import com.jerboa.ui.components.report.post.CreatePostReportActivity
import com.jerboa.ui.components.settings.SettingsActivity
import com.jerboa.ui.components.settings.about.AboutActivity
import com.jerboa.ui.components.settings.account.AccountSettingsActivity
import com.jerboa.ui.components.settings.account.AccountSettingsViewModel
import com.jerboa.ui.components.settings.account.AccountSettingsViewModelFactory
import com.jerboa.ui.components.settings.lookandfeel.LookAndFeelActivity
import com.jerboa.ui.theme.JerboaTheme

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.

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)
}

@OptIn(ExperimentalAnimationApi::class)
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

val accountSync = getCurrentAccountSync(accountViewModel)

setContent {
val ctx = LocalContext.current

Expand All @@ -115,10 +108,17 @@ class MainActivity : AppCompatActivity() {
null
}

LaunchedEffect(Unit) {
fetchInitialData(accountSync, siteViewModel)
val accountViewModel: AccountViewModel = viewModel(factory = AccountViewModelFactory.Factory)

val account = getCurrentAccount(accountViewModel)

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

@yate yate Jul 5, 2023

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 appSettingsViewModel: AppSettingsViewModel = viewModel(factory = AppSettingsViewModelFactory.Factory)
val appSettings by appSettingsViewModel.appSettings.observeAsState(APP_SETTINGS_DEFAULT)

JerboaTheme(
Expand All @@ -133,7 +133,7 @@ class MainActivity : AppCompatActivity() {
appSettings.usePrivateTabs,
)

ShowChangelog(appSettingsViewModel = appSettingsViewModel)
ShowChangelog()
Copy link
Member

Choose a reason for hiding this comment

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

Stuff like this scares me, it seems like it has no actual dependencies now, when in reality the appSettings does need to be injected.


when (val siteRes = siteViewModel.siteRes) {
is ApiState.Success -> {
Expand Down Expand Up @@ -165,17 +165,14 @@ class MainActivity : AppCompatActivity() {
) {
LoginActivity(
navController = navController,
accountViewModel = accountViewModel,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw in the workshop, the viewModel parameters weren't passed in directly, they used the factory in the composable instead, like https://github.com/dessalines/jerboa/pull/973/files#diff-6ca2c69ae054a88358d16c98f47efd02bb16eae48a09295b4db9bd5e99ba8e76R25.

siteViewModel = siteViewModel,
)
}

composable(route = Route.HOME) {
BottomNavActivity(
navController = navController,
accountViewModel = accountViewModel,
siteViewModel = siteViewModel,
appSettingsViewModel = appSettingsViewModel,
appSettings = appSettings,
)
}
Expand All @@ -201,8 +198,6 @@ class MainActivity : AppCompatActivity() {
communityArg = Either.Left(args.id),
navController = navController,
communityViewModel = communityViewModel,
accountViewModel = accountViewModel,
appSettingsViewModel = appSettingsViewModel,
showVotingArrowsInListView = appSettings.showVotingArrowsInListView,
siteViewModel = siteViewModel,
useCustomTabs = appSettings.useCustomTabs,
Expand Down Expand Up @@ -233,8 +228,6 @@ class MainActivity : AppCompatActivity() {
communityArg = Either.Right(qualifiedName),
navController = navController,
communityViewModel = communityViewModel,
accountViewModel = accountViewModel,
appSettingsViewModel = appSettingsViewModel,
showVotingArrowsInListView = appSettings.showVotingArrowsInListView,
siteViewModel = siteViewModel,
useCustomTabs = appSettings.useCustomTabs,
Expand Down Expand Up @@ -299,8 +292,6 @@ class MainActivity : AppCompatActivity() {
personArg = Either.Left(args.id),
savedMode = args.saved,
navController = navController,
accountViewModel = accountViewModel,
appSettingsViewModel = appSettingsViewModel,
showVotingArrowsInListView = appSettings.showVotingArrowsInListView,
siteViewModel = siteViewModel,
useCustomTabs = appSettings.useCustomTabs,
Expand Down Expand Up @@ -330,8 +321,6 @@ class MainActivity : AppCompatActivity() {
personArg = Either.Right(qualifiedName),
savedMode = false,
navController = navController,
accountViewModel = accountViewModel,
appSettingsViewModel = appSettingsViewModel,
showVotingArrowsInListView = appSettings.showVotingArrowsInListView,
siteViewModel = siteViewModel,
useCustomTabs = appSettings.useCustomTabs,
Expand All @@ -352,7 +341,6 @@ class MainActivity : AppCompatActivity() {
val args = Route.CommunityListArgs(it)
CommunityListActivity(
navController = navController,
accountViewModel = accountViewModel,
siteViewModel = siteViewModel,
selectMode = args.select,
blurNSFW = appSettings.blurNSFW,
Expand Down Expand Up @@ -389,7 +377,6 @@ class MainActivity : AppCompatActivity() {

CreatePostActivity(
navController = navController,
accountViewModel = accountViewModel,
initialUrl = url,
initialBody = body,
initialImage = image,
Expand All @@ -405,7 +392,6 @@ class MainActivity : AppCompatActivity() {
) {
InboxActivity(
navController = navController,
accountViewModel = accountViewModel,
siteViewModel = siteViewModel,
blurNSFW = appSettings.blurNSFW,
)
Expand All @@ -426,7 +412,6 @@ class MainActivity : AppCompatActivity() {
SwipeToNavigateBack(navController = navController) {
PostActivity(
id = Either.Left(args.id),
accountViewModel = accountViewModel,
navController = navController,
showCollapsedCommentContent = appSettings.showCollapsedCommentContent,
showActionBarByDefault = appSettings.showCommentActionBarByDefault,
Expand Down Expand Up @@ -455,7 +440,6 @@ class MainActivity : AppCompatActivity() {
val args = Route.CommentArgs(it)
PostActivity(
id = Either.Right(args.id),
accountViewModel = accountViewModel,
navController = navController,
useCustomTabs = appSettings.useCustomTabs,
usePrivateTabs = appSettings.usePrivateTabs,
Expand All @@ -482,7 +466,6 @@ class MainActivity : AppCompatActivity() {

CommentReplyActivity(
replyItem = replyItem,
accountViewModel = accountViewModel,
navController = navController,
siteViewModel = siteViewModel,
isModerator = args.isModerator,
Expand All @@ -500,7 +483,6 @@ class MainActivity : AppCompatActivity() {
val commentView by navController.takeDepsFromRoot<CommentEditDeps>()
CommentEditActivity(
commentView = commentView,
accountViewModel = accountViewModel,
navController = navController,
)
}
Expand All @@ -509,7 +491,6 @@ class MainActivity : AppCompatActivity() {
val postView by navController.takeDepsFromRoot<PostEditDeps>()
PostEditActivity(
postView = postView,
accountViewModel = accountViewModel,
navController = navController,
)
}
Expand All @@ -518,7 +499,6 @@ class MainActivity : AppCompatActivity() {
val privateMessage by navController.takeDepsFromRoot<PrivateMessageDeps>()
PrivateMessageReplyActivity(
privateMessageView = privateMessage,
accountViewModel = accountViewModel,
navController = navController,
siteViewModel = siteViewModel,
)
Expand All @@ -535,7 +515,6 @@ class MainActivity : AppCompatActivity() {
val args = Route.CommentReportArgs(it)
CreateCommentReportActivity(
commentId = args.id,
accountViewModel = accountViewModel,
navController = navController,
)
}
Expand All @@ -551,22 +530,19 @@ class MainActivity : AppCompatActivity() {
val args = Route.PostReportArgs(it)
CreatePostReportActivity(
postId = args.id,
accountViewModel = accountViewModel,
navController = navController,
)
}

composable(route = Route.SETTINGS) {
SettingsActivity(
navController = navController,
accountViewModel = accountViewModel,
)
}

composable(route = Route.LOOK_AND_FEEL) {
LookAndFeelActivity(
navController = navController,
appSettingsViewModel = appSettingsViewModel,
)
}

Expand All @@ -578,9 +554,7 @@ class MainActivity : AppCompatActivity() {
) {
AccountSettingsActivity(
navController = navController,
accountViewModel = accountViewModel,
siteViewModel = siteViewModel,
accountSettingsViewModel = accountSettingsViewModel,
)
}

Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/com/jerboa/Utils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,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.ui.components.common.Route
import com.jerboa.ui.components.home.HomeViewModel
import com.jerboa.ui.components.home.SiteViewModel
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/com/jerboa/api/Http.kt
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
40 changes: 40 additions & 0 deletions app/src/main/java/com/jerboa/db/AccountViewModel.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.jerboa.db

import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import androidx.lifecycle.viewmodel.initializer
import androidx.lifecycle.viewmodel.viewModelFactory
import com.jerboa.db.entity.Account
import com.jerboa.db.repository.AccountRepository
import com.jerboa.jerboaApplication
import kotlinx.coroutines.launch

class AccountViewModel(private val repository: AccountRepository) : ViewModel() {

val currentAccount = repository.currentAccount
val allAccounts = repository.allAccounts

fun insert(account: Account) = viewModelScope.launch {
repository.insert(account)
}

fun removeCurrent() = viewModelScope.launch {
repository.removeCurrent()
}

fun setCurrent(accountId: Int) = viewModelScope.launch {
repository.setCurrent(accountId)
}

fun delete(account: Account) = viewModelScope.launch {
repository.delete(account)
}
}

object AccountViewModelFactory {
val Factory = viewModelFactory {
initializer {
AccountViewModel(jerboaApplication().container.accountRepository)
}
}
}
Loading