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

SpeziAccount & SpeziFirebase #107

Draft
wants to merge 73 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
be77221
Add SpeziAccount parts
pauljohanneskraft Sep 20, 2024
291503e
Add account
pauljohanneskraft Sep 25, 2024
dec212c
Improve SpeziFirebase
pauljohanneskraft Sep 26, 2024
a9db0b5
Add new AccountDetails keys
pauljohanneskraft Sep 26, 2024
58de069
Add SpeziViews code
pauljohanneskraft Sep 27, 2024
1ea20b0
Add more interfaces
pauljohanneskraft Sep 27, 2024
5ebc29a
Improve
pauljohanneskraft Sep 27, 2024
d740dcf
Update
pauljohanneskraft Sep 27, 2024
10bd195
imrpove
pauljohanneskraft Sep 30, 2024
bfd7e97
Update
pauljohanneskraft Oct 1, 2024
341a9dd
detekt
pauljohanneskraft Oct 2, 2024
7339be9
Merge branch 'main' into feature/spezi-account
pauljohanneskraft Oct 21, 2024
579f4ce
Update some internal structure
pauljohanneskraft Oct 22, 2024
8ae6519
Merge branch 'main' into feature/spezi-account
pauljohanneskraft Oct 27, 2024
ce2c314
Continue work
pauljohanneskraft Oct 27, 2024
38c0ce6
Add SpeziValidation
pauljohanneskraft Oct 28, 2024
f25889f
Fix import
pauljohanneskraft Oct 28, 2024
6d7b4f2
Merge branch 'main' into feature/spezi-account
Basler182 Nov 3, 2024
18bdfbc
Continue impl
pauljohanneskraft Nov 8, 2024
7b6c87c
Improve
pauljohanneskraft Nov 8, 2024
aaf29df
SpeziViews
pauljohanneskraft Nov 9, 2024
11efb05
intermediate
pauljohanneskraft Nov 10, 2024
5fa1c8d
Update SuspendButton implementation
pauljohanneskraft Nov 10, 2024
37159b8
Update tests for PersonalInfo
pauljohanneskraft Nov 11, 2024
f3f516b
Finish up SpeziViews
pauljohanneskraft Nov 11, 2024
306b46b
Rename test
pauljohanneskraft Nov 11, 2024
dc8133b
Fix import issues
pauljohanneskraft Nov 11, 2024
4258420
Remove import issues
pauljohanneskraft Nov 11, 2024
e3baa1b
Add import
pauljohanneskraft Nov 11, 2024
0a94bc5
Move files into the correct repositories
pauljohanneskraft Nov 11, 2024
40601b4
Rename package
pauljohanneskraft Nov 11, 2024
728e534
Remove nonimported usages of PersonNameComponents
pauljohanneskraft Nov 11, 2024
7928eb2
Update previews
pauljohanneskraft Nov 11, 2024
d974d16
fix imports
pauljohanneskraft Nov 11, 2024
5143693
detekt
pauljohanneskraft Nov 11, 2024
2c8e57b
Fix tests
pauljohanneskraft Nov 11, 2024
10a2543
fix
pauljohanneskraft Nov 12, 2024
87f6f18
Merge branch 'feature/spezi-views' into feature/spezi-account
pauljohanneskraft Nov 12, 2024
11aa4f3
Update
pauljohanneskraft Nov 12, 2024
47b8841
Remove old state
pauljohanneskraft Nov 12, 2024
22c3e6d
Fill with more functionality
pauljohanneskraft Nov 12, 2024
bc50be6
Continuing implementation
pauljohanneskraft Nov 13, 2024
3db9aed
Most recent changes
pauljohanneskraft Nov 18, 2024
5c2d056
Update
pauljohanneskraft Nov 18, 2024
6f5b66c
review adaptions
pauljohanneskraft Nov 18, 2024
e02bad3
tests
pauljohanneskraft Nov 18, 2024
0a0fc2f
adapt tests
pauljohanneskraft Nov 18, 2024
35f5076
update
pauljohanneskraft Nov 19, 2024
307d84c
Adapt contact
pauljohanneskraft Nov 19, 2024
fcda9c7
Add modifier parameters
pauljohanneskraft Nov 19, 2024
088e66f
Fix tests
pauljohanneskraft Nov 19, 2024
2207da9
detekt
pauljohanneskraft Nov 19, 2024
9962c09
Merge branch 'feature/spezi-views' into feature/spezi-account
pauljohanneskraft Nov 19, 2024
1783d43
Readd PersonNameComponents
pauljohanneskraft Nov 19, 2024
c84e748
update
pauljohanneskraft Nov 19, 2024
d184b64
adapt tests
pauljohanneskraft Nov 19, 2024
58dc269
small update
pauljohanneskraft Nov 19, 2024
858c75f
Make sure to only trigger validation after the first input
pauljohanneskraft Nov 19, 2024
f3af7e3
detekt
pauljohanneskraft Nov 19, 2024
ace70d4
Add onSubmit
pauljohanneskraft Nov 19, 2024
ca54fef
Merge branch 'feature/spezi-views' into feature/spezi-account
pauljohanneskraft Nov 19, 2024
ace5c51
update
pauljohanneskraft Nov 22, 2024
df7d2b1
Merge branch 'main' into feature/spezi-account
pauljohanneskraft Nov 24, 2024
e4da71a
detekt
pauljohanneskraft Nov 24, 2024
6566ef5
update
pauljohanneskraft Nov 25, 2024
004be47
update
pauljohanneskraft Nov 26, 2024
04ab47a
Merge branch 'main' into feature/spezi-account
pauljohanneskraft Nov 26, 2024
286f416
Serialization
pauljohanneskraft Nov 27, 2024
c6a072d
Add AccountDetailsTest
pauljohanneskraft Nov 27, 2024
132f692
update
pauljohanneskraft Nov 27, 2024
5f3651f
update
pauljohanneskraft Nov 28, 2024
b535893
update
pauljohanneskraft Nov 29, 2024
de0fb1a
improve
pauljohanneskraft Nov 30, 2024
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package edu.stanford.spezi.module.account

import androidx.annotation.MainThread
import androidx.compose.material3.Text
import androidx.compose.material3.TextField
import androidx.compose.runtime.Composable
import androidx.compose.runtime.MutableState
import kotlin.reflect.KClass
import kotlin.reflect.KProperty1

// SpeziAccount

/*

interface AccountKeyInput<Value : Any> {
val id: String?
val name: String
val category: AccountKeyCategory
val displayView: @Composable (Value) -> Unit
val entryView: @Composable (MutableState<Value>) -> Unit
}

annotation class AccountValue<Input: AccountKeyInput<*>>

data class NameAccountKeyInput(val input: Unit): AccountKeyInput<String> {
override val id: String? get() = null
override val name: String get() = "Name"
override val category: AccountKeyCategory get() = AccountKeyCategory.name
override val displayView: @Composable (String) -> Unit get() = { text ->
Text(text)
}
override val entryView: @Composable (MutableState<String>) -> Unit get() = { state ->
TextField(value = state.value, onValueChange = { state.value = it })
}
}

@AccountValue<NameAccountKeyInput>
val AccountDetails.name: String? get() = null
*/
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package edu.stanford.spezi.module.account.account

import edu.stanford.spezi.module.account.account.service.AccountService
import edu.stanford.spezi.module.account.account.value.collections.AccountDetails

class Account(
service: AccountService,
configuration: AccountValueConfiguration = AccountValueConfiguration.default,
details: AccountDetails? = null
) {
var signedIn: Boolean = details != null
private set

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package edu.stanford.spezi.module.account.account

import edu.stanford.spezi.core.logging.speziLogger
import edu.stanford.spezi.module.account.account.service.AccountService
import edu.stanford.spezi.module.account.spezi.Module
import edu.stanford.spezi.module.account.spezi.Standard

class AccountConfiguration<Service: AccountService>: Module {
private val logger by speziLogger()

val account: Account = TODO()
private val externalStorage: ExternalAccountStorage = TODO()
private val accountService: Service = TODO()
private val storageProvider: List<Module> = TODO()
private val standard: Standard = TODO()

override fun configure() {
TODO()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package edu.stanford.spezi.module.account.account

import edu.stanford.spezi.core.logging.speziLogger
import edu.stanford.spezi.module.account.account.value.collections.AccountDetails
import edu.stanford.spezi.module.account.account.value.collections.AccountKey
import edu.stanford.spezi.module.account.account.value.collections.AccountModifications
import edu.stanford.spezi.module.account.spezi.Module
import kotlin.reflect.KClass

class AccountDetailsCache: Module {
private val logger by speziLogger()

private val localCache = mutableMapOf<String, AccountDetails>()

fun loadEntry(accountId: String, keys: Set<KClass<AccountKey<*>>>): AccountDetails? {
localCache[accountId]?.let {
return it
}

return null // TODO: lead from persistency as well
}

fun clearEntry(accountId: String) {
localCache.remove(accountId)
// TODO: Delete persistence as well
}

internal fun purgeMemoryCache(accountId: String) {
localCache.remove(accountId)
}

fun communicateModifications(accountId: String, modifications: AccountModifications) {
val details = AccountDetails()
localCache[accountId]?.let {
// TODO("AccountDetails.add(contentsOf:) missing!")
}
// TODO("AccountDetails.add(contentsOf:merge:) missing!")
// TODO("AccountDetails.removeAll() missing!")

communicateRemoteChanges(accountId, details)
}

fun communicateRemoteChanges(accountId: String, details: AccountDetails) {
localCache[accountId] = details

// TODO: Persistent store missing
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package edu.stanford.spezi.module.account.account

import android.text.style.TabStopSpan.Standard
import edu.stanford.spezi.module.account.account.value.collections.AccountDetails
import edu.stanford.spezi.module.account.account.value.keys.accountId
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.FlowCollector
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.onCompletion
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import java.util.UUID

class AccountNotifications {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great to align on the following pattern: Every public component will be defined as an interface, e.g. interface AccountNotifications, and the actual implementation as an internal class AccountNotificationsImpl : AccountNotifications. This approach will give the consumers of the library to use test doubles of their choice for testing, e.g. mocking or fakes.


Also, I would rethink the public API of this component, it should indeed not offer the respond to event method publicly as it allows mutation from outside, while we only want to mutate internally in Account class

sealed interface Event {
data class DeletingAccount(val accountId: String): Event
data class AssociatedAccount(val details: AccountDetails): Event
data class DetailsChanged(val previous: AccountDetails, val new: AccountDetails): Event
data class DisassociatingAccount(val details: AccountDetails): Event
}

private val standard: Standard = TODO()
Copy link
Contributor

Choose a reason for hiding this comment

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

we will define our own type right? as this is coming from android.text.style.TabStopSpan.Standard

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is just a placeholder for now

private val storage: ExternalAccountStorage = TODO()
private val collectors = mutableMapOf<UUID, FlowCollector<Event>>()
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: we can use a concurrent hash map and can avoid the need for the mutex here

private val collectors = ConcurrentHashMap<UUID, FlowCollector<Event>>()

private val mutex = Mutex()

val events: Flow<Event> get() = newSubscription()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to keep a reference of the collectors in the map above? Currently we are manually setting entries there in newSubscription() and from the API I don't see a need to assign collectors from outside. This is handled by default from kotlin flow and coroutines api when consumers will start collecting the public events property . You can create a single private mutable flow here and expose via this public events. Also there is no need to handle onCompletion as you are doing below. Consumers of these events will launch their own coroutine scope to listen for changes, e.g. a viewModelScope. When the view model will get destroyed, the scope will get cancelled automatically -> and also will stop listening to these changes. By design flow apis should honor consumers request to listen to the changes, and we should not manage this internally via the map. Responsibility of this component should be to notify the events, and not manage it's listeners.

TLDR: I know this ways done to match the internal implementation in iOS, but the equivalent in android can be achieved as follows:

private val _events = MutableSharedFlow<Event>()
val events: Flow<Event> = _events.asSharedFlow()

// emitting e.g. in respond to event
_events.emit(event) // this will trigger a new update (emission) in public events flow

This approach is also safer in regard to wrong usages from clients, because they might access the events property multiple times from their components, which would result into creating new instances every time - while with the proposal we are returning the same instance (single source of truth) on every access

The only difference is that the shared flow is hot - technical meaning is that it will emit always even though it might not have any active subscribers / collectors. However, flow api is pretty efficient and we can really ignore the fact the we might emit even though no one is listening


suspend fun respondToEvent(event: Event) {
(standard as? AccountNotifyConstraint)?.respondToEvent(event)

when (event) {
is Event.DeletingAccount -> {
storage.willDeleteAccount(event.accountId)
}
is Event.DisassociatingAccount -> {
storage.userWillDisassociate(event.details.accountId)
}
else -> {}
}

mutex.withLock {
for (collector in collectors.values) {
collector.emit(event)
}
}
}

private fun newSubscription(): Flow<Event> {
val key = UUID.randomUUID()
return flow {
mutex.withLock {
collectors[key] = this
}
}.onCompletion {
mutex.withLock {
collectors.remove(key)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package edu.stanford.spezi.module.account.account

interface AccountNotifyConstraint {
fun respondToEvent(event: AccountNotifications.Event)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package edu.stanford.spezi.module.account.account

import edu.stanford.spezi.module.account.account.value.collections.AccountDetails
import edu.stanford.spezi.module.account.account.value.collections.AccountKey
import edu.stanford.spezi.module.account.account.value.collections.AccountModifications
import edu.stanford.spezi.module.account.spezi.Module
import kotlin.reflect.KClass

interface AccountStorageProvider: Module {
suspend fun load(accountId: String, keys: Set<KClass<AccountKey<*>>>): AccountDetails?
suspend fun store(accountId: String, details: AccountDetails) {
val modifications = AccountModifications(modifiedDetails = details)
store(accountId, modifications)
}
suspend fun store(accountId: String, modifications: AccountModifications)
suspend fun disassociate(accountId: String)
suspend fun delete(accountId: String)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package edu.stanford.spezi.module.account.account

import edu.stanford.spezi.module.account.account.value.collections.AccountDetails
import edu.stanford.spezi.module.account.account.value.collections.AccountKey
import edu.stanford.spezi.module.account.account.value.collections.AccountModifications
import edu.stanford.spezi.module.account.account.value.keys.isIncomplete
import edu.stanford.spezi.module.account.spezi.Module
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.FlowCollector
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.onCompletion
import java.util.UUID
import kotlin.reflect.KClass

class ExternalAccountStorage: Module {

data class ExternallyStoredDetails(
val accountId: String,
val details: AccountDetails,
)

private var subscriptions = mutableMapOf<UUID, FlowCollector<ExternallyStoredDetails>>()
private var storageProvider: AccountStorageProvider? = null

val updatedDetails: Flow<ExternallyStoredDetails> get() {
val id = UUID.randomUUID()
return flow {
subscriptions[id] = this
}.onCompletion {
subscriptions.remove(id)
}
}

suspend fun notifyAboutUpdatedDetails(accountId: String, details: AccountDetails) {
val newDetails = details.copy()
newDetails.isIncomplete = false
val update = ExternallyStoredDetails(accountId, newDetails)
for (subscription in subscriptions) {
subscription.value.emit(update)
}
}

suspend fun requestExternalStorage(accountId: String, details: AccountDetails) {
// TODO: Check for emptiness and making sure storageProvider exists
storageProvider?.store(accountId, details)
}

suspend fun retrieveExternalStorage(accountId: String, keys: List<KClass<AccountKey<*>>>): AccountDetails {
if (keys.isEmpty()) return AccountDetails()

storageProvider?.let { storageProvider ->
storageProvider.load(accountId, keys)?.let { details ->
return details
}
val details = AccountDetails()
details.isIncomplete = true
return details
} ?: throw Error("")
}

suspend fun updateExternalStorage(accountId: String, modifications: AccountModifications) {
val storageProvider = storageProvider ?: throw Error("")
storageProvider.store(accountId, modifications)
}

suspend fun willDeleteAccount(accountId: String) {
storageProvider?.delete(accountId)
}

suspend fun userWillDisassociate(accountId: String) {
storageProvider?.disassociate(accountId)
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package edu.stanford.spezi.module.account.account.compositionLocal

import androidx.compose.runtime.compositionLocalOf

enum class FollowUpBehavior {
DISABLED, MINIMAL, REDUNDANT;

companion object {
val automatic: FollowUpBehavior get() = MINIMAL
}
}

val localFollowUpBehaviorAfterSetup = compositionLocalOf { FollowUpBehavior.automatic }
Loading
Loading