Skip to content

Commit 82b5df6

Browse files
zsmb13zsmb13
committed
Add logging to MultiplatformSettingsStorage
Breaks the dependency cycle between the storage class and the logger so that the storage implementation can also contribute to logs.
1 parent 4757060 commit 82b5df6

File tree

3 files changed

+92
-22
lines changed

3 files changed

+92
-22
lines changed

‎shared/src/commonMain/kotlin/org/jetbrains/kotlinconf/AppInit.kt‎

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import com.mmk.kmpnotifier.notification.configuration.NotificationPlatformConfig
77
import kotlinx.coroutines.CoroutineScope
88
import kotlinx.coroutines.Dispatchers
99
import kotlinx.coroutines.SupervisorJob
10+
import kotlinx.coroutines.flow.first
11+
import kotlinx.coroutines.launch
1012
import org.jetbrains.kotlinconf.navigation.navigateToSession
1113
import org.jetbrains.kotlinconf.screens.AboutConferenceViewModel
1214
import org.jetbrains.kotlinconf.screens.LicensesViewModel
@@ -19,6 +21,7 @@ import org.jetbrains.kotlinconf.screens.SpeakersViewModel
1921
import org.jetbrains.kotlinconf.screens.StartNotificationsViewModel
2022
import org.jetbrains.kotlinconf.storage.ApplicationStorage
2123
import org.jetbrains.kotlinconf.storage.MultiplatformSettingsStorage
24+
import org.jetbrains.kotlinconf.utils.BufferedDelegatingLogger
2225
import org.jetbrains.kotlinconf.utils.DebugLogger
2326
import org.jetbrains.kotlinconf.utils.Logger
2427
import org.jetbrains.kotlinconf.utils.NoopProdLogger
@@ -36,18 +39,31 @@ fun initApp(
3639
platformModule: Module,
3740
flags: Flags = Flags(),
3841
) {
39-
val koin = initKoin(platformLogger, platformModule, flags)
40-
initNotifier(configuration = koin.get(), logger = koin.get())
42+
val appScope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
43+
val koin = initKoin(appScope, platformModule, flags)
44+
initLogging(
45+
appScope = appScope,
46+
platformLogger = platformLogger,
47+
bufferedDelegatingLogger = koin.get(),
48+
applicationStorage = koin.get(),
49+
)
50+
initNotifier(
51+
configuration = koin.get(),
52+
logger = koin.get(),
53+
)
4154
}
4255

4356
private fun initKoin(
44-
platformLogger: Logger,
57+
appScope: CoroutineScope,
4558
platformModule: Module,
4659
platformFlags: Flags,
4760
): Koin {
4861
return startKoin {
4962
val appModule = module {
50-
single<ApplicationStorage> { MultiplatformSettingsStorage(get()) }
63+
single { BufferedDelegatingLogger() }
64+
single<Logger> { get<BufferedDelegatingLogger>() }
65+
66+
single<ApplicationStorage> { MultiplatformSettingsStorage(get(), get()) }
5167
single {
5268
val flags = get<ApplicationStorage>().getFlagsBlocking()
5369
val endpoint = when {
@@ -63,15 +79,8 @@ private fun initKoin(
6379
else -> ServerBasedTimeProvider(get())
6480
}
6581
}
66-
single<Logger> {
67-
val flags = get<ApplicationStorage>().getFlagsBlocking()
68-
when {
69-
flags != null && flags.debugLogging -> DebugLogger(platformLogger)
70-
else -> NoopProdLogger()
71-
}
72-
}
7382
single { FlagsManager(platformFlags, get(), get()) }
74-
single { CoroutineScope(SupervisorJob() + Dispatchers.Default) }
83+
single { appScope }
7584
singleOf(::ConferenceService)
7685
}
7786

@@ -93,6 +102,23 @@ private fun initKoin(
93102
}.koin
94103
}
95104

105+
private fun initLogging(
106+
appScope: CoroutineScope,
107+
platformLogger: Logger,
108+
bufferedDelegatingLogger: BufferedDelegatingLogger,
109+
applicationStorage: ApplicationStorage,
110+
) {
111+
appScope.launch {
112+
val flags = applicationStorage.getFlags().first()
113+
bufferedDelegatingLogger.attach(
114+
when {
115+
flags != null && flags.debugLogging -> DebugLogger(platformLogger)
116+
else -> NoopProdLogger()
117+
}
118+
)
119+
}
120+
}
121+
96122
private fun initNotifier(
97123
configuration: NotificationPlatformConfiguration,
98124
logger: Logger,

‎shared/src/commonMain/kotlin/org/jetbrains/kotlinconf/storage/MultiplatformSettingsStorage.kt‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,18 @@ import org.jetbrains.kotlinconf.SessionId
1616
import org.jetbrains.kotlinconf.Theme
1717
import org.jetbrains.kotlinconf.VoteInfo
1818
import org.jetbrains.kotlinconf.utils.Logger
19-
import org.jetbrains.kotlinconf.utils.TaggedLogger
2019
import org.jetbrains.kotlinconf.utils.tagged
2120

2221
@OptIn(ExperimentalSettingsApi::class)
2322
class MultiplatformSettingsStorage(
2423
private val settings: ObservableSettings,
25-
logger: Logger? = null, // TODO inject a logger here https://github.com/JetBrains/kotlinconf-app/issues/544
24+
logger: Logger,
2625
) : ApplicationStorage {
2726
private val json = Json {
2827
ignoreUnknownKeys = true
2928
}
3029

31-
private var taggedLogger: TaggedLogger? = logger?.tagged("MultiplatformSettingsStorage")
30+
private var taggedLogger = logger.tagged("MultiplatformSettingsStorage")
3231

3332
private inline fun <reified T> String?.decodeOrNull(): T? {
3433
if (this == null) return null
@@ -92,39 +91,50 @@ class MultiplatformSettingsStorage(
9291
override fun ensureCurrentVersion() {
9392
var version = settings.getInt(Keys.STORAGE_VERSION, 0)
9493

95-
taggedLogger?.log { "Storage version is $version" }
94+
taggedLogger.log { "Storage version is $version" }
9695

9796
if (version == 0) {
98-
// Fully destructive migration on unknown previous version
97+
taggedLogger.log { "Unknown previous storage version, performing destructive migration" }
9998
destructiveUpgrade()
10099
return
101100
}
102101

102+
if (version > CURRENT_STORAGE_VERSION) {
103+
taggedLogger.log { "Storage version not recognized, performing destructive migration" }
104+
destructiveUpgrade()
105+
return
106+
}
107+
108+
if (version == CURRENT_STORAGE_VERSION) {
109+
taggedLogger.log { "Storage version matches expected version, no need to migrate" }
110+
return
111+
}
112+
103113
while (version < CURRENT_STORAGE_VERSION) {
104-
taggedLogger?.log { "Finding migrations from $version to $CURRENT_STORAGE_VERSION..." }
114+
taggedLogger.log { "Finding migrations from $version to $CURRENT_STORAGE_VERSION..." }
105115

106116
// Find a migration from the current version that takes us as far forward as possible
107117
val nextMigration = migrations.filter { it.from == version }.maxByOrNull { it.to }
108118
if (nextMigration == null) {
109-
taggedLogger?.log { "No matching migrations found" }
119+
taggedLogger.log { "No matching migrations found" }
110120

111121
// Failed to find a migration path to latest, fall back to destructive
112122
destructiveUpgrade()
113123
return
114124
}
115125

116-
taggedLogger?.log { "Running migration from ${nextMigration.from} to ${nextMigration.to}" }
126+
taggedLogger.log { "Running migration from ${nextMigration.from} to ${nextMigration.to}" }
117127

118128
nextMigration.migrate()
119129
version = nextMigration.to
120130
settings.set(Keys.STORAGE_VERSION, version)
121131

122-
taggedLogger?.log { "Successfully migrated to $version" }
132+
taggedLogger.log { "Successfully migrated to $version" }
123133
}
124134
}
125135

126136
private fun destructiveUpgrade() {
127-
taggedLogger?.log { "Performing destructive upgrade to $CURRENT_STORAGE_VERSION" }
137+
taggedLogger.log { "Performing destructive upgrade to $CURRENT_STORAGE_VERSION" }
128138
settings.clear()
129139
settings.set(Keys.STORAGE_VERSION, CURRENT_STORAGE_VERSION)
130140
}

‎shared/src/commonMain/kotlin/org/jetbrains/kotlinconf/utils/Logger.kt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,37 @@ class NoopProdLogger : Logger {
2020
// No logging in prod
2121
}
2222
}
23+
24+
/**
25+
* A logger that's constructed during startup and later attached to a real logger.
26+
* Until attached, it buffers log entries in memory. When [attach] is called,
27+
* it forwards the buffered entries to the attached logger in order, and from
28+
* that point on it delegates all calls directly.
29+
*/
30+
class BufferedDelegatingLogger : Logger {
31+
private var delegate: Logger? = null
32+
33+
private data class Entry(val tag: String, val message: String)
34+
35+
private val buffer = mutableListOf<Entry>()
36+
37+
override fun log(tag: String, lazyMessage: () -> String) {
38+
val current = delegate
39+
if (current != null) {
40+
current.log(tag, lazyMessage)
41+
return
42+
}
43+
44+
buffer += Entry(tag, lazyMessage())
45+
}
46+
47+
fun attach(realLogger: Logger) {
48+
require(delegate == null) { "Logger delegate was already set, this should only happen once" }
49+
50+
buffer.forEach { entry ->
51+
realLogger.log(entry.tag) { entry.message }
52+
}
53+
buffer.clear()
54+
delegate = realLogger
55+
}
56+
}

0 commit comments

Comments
 (0)