Skip to content

Commit 0960af1

Browse files
settings: keep un-flushed edits across reload
reload() clobbered in-memory mutations with the on-disk copy when the debounced save had not flushed yet, silently reverting font, text size, colors, etc. It now flushes pending writes instead of re-reading while a save is pending, and only re-reads external changes when nothing is pending. Completes commit 201e2b5, which only guarded the sdCardRoot setter. Also routes the previously silent saveToDisk failures to error.log.
1 parent 57def88 commit 0960af1

2 files changed

Lines changed: 77 additions & 5 deletions

File tree

app/src/main/java/dev/cannoli/scorza/settings/SettingsRepository.kt

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ class SettingsRepository @Inject constructor(@ApplicationContext private val con
3131
private val saveHandler = Handler(saveThread.looper)
3232
private val saveRunnable = Runnable { saveToDisk() }
3333

34+
// True while an in-memory mutation has not yet been persisted. reload() must not
35+
// clobber the json with the on-disk copy while this is set, or un-flushed edits
36+
// are lost (the bug behind font/text-size/color changes silently reverting).
37+
@Volatile
38+
private var pendingSave = false
39+
3440
private inline fun <T> jsonRead(block: JSONObject.() -> T): T = synchronized(jsonLock) { json.block() }
3541
private inline fun jsonWrite(block: JSONObject.() -> Unit) { synchronized(jsonLock) { json.block() }; scheduleSave() }
3642

@@ -68,6 +74,7 @@ class SettingsRepository @Inject constructor(@ApplicationContext private val con
6874
}
6975

7076
private fun scheduleSave() {
77+
pendingSave = true
7178
saveHandler.removeCallbacks(saveRunnable)
7279
saveHandler.postDelayed(saveRunnable, 100)
7380
}
@@ -83,13 +90,22 @@ class SettingsRepository @Inject constructor(@ApplicationContext private val con
8390

8491
private fun saveToDisk() {
8592
settingsFile?.let { file ->
86-
if (!setupCompleted) return
87-
if (!hasStoragePermission()) return
93+
if (!setupCompleted) {
94+
dev.cannoli.scorza.util.ErrorLog.write("settings save skipped: setup not completed (${file.absolutePath})")
95+
return
96+
}
97+
if (!hasStoragePermission()) {
98+
dev.cannoli.scorza.util.ErrorLog.write("settings save skipped: no storage permission (${file.absolutePath})")
99+
return
100+
}
88101
try {
89102
file.parentFile?.mkdirs()
90103
synchronized(jsonLock) { file.writeText(json.toString(2)) }
91-
} catch (_: IOException) {
92-
} catch (_: SecurityException) {
104+
pendingSave = false
105+
} catch (e: IOException) {
106+
dev.cannoli.scorza.util.ErrorLog.error("settings save failed writing ${file.absolutePath}", e)
107+
} catch (e: SecurityException) {
108+
dev.cannoli.scorza.util.ErrorLog.error("settings save failed writing ${file.absolutePath}", e)
93109
}
94110
}
95111
}
@@ -100,7 +116,9 @@ class SettingsRepository @Inject constructor(@ApplicationContext private val con
100116
}
101117

102118
fun reload() {
103-
loadFromDisk()
119+
// Un-flushed local edits are newer than disk; persist them rather than letting
120+
// loadFromDisk overwrite them. Only re-read external changes when nothing is pending.
121+
if (pendingSave) flush() else loadFromDisk()
104122
}
105123

106124
fun shutdown() {
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package dev.cannoli.scorza.settings
2+
3+
import android.content.Context
4+
import androidx.test.core.app.ApplicationProvider
5+
import org.junit.Assert.assertEquals
6+
import org.junit.Rule
7+
import org.junit.Test
8+
import org.junit.rules.TemporaryFolder
9+
import org.junit.runner.RunWith
10+
import org.robolectric.RobolectricTestRunner
11+
import org.robolectric.annotation.Config
12+
import java.io.File
13+
14+
@RunWith(RobolectricTestRunner::class)
15+
@Config(sdk = [34])
16+
class SettingsRepositoryTest {
17+
18+
@get:Rule val tmp = TemporaryFolder()
19+
20+
private fun newRepo() = SettingsRepository(ApplicationProvider.getApplicationContext<Context>())
21+
22+
private fun writeSettingsJson(root: File, json: String) {
23+
File(root, "Config").apply { mkdirs() }
24+
File(root, "Config/settings.json").writeText(json)
25+
}
26+
27+
@Test fun `reload keeps un-flushed in-memory edits instead of reverting to disk`() {
28+
writeSettingsJson(tmp.root, """{"font":"old"}""")
29+
val settings = newRepo()
30+
settings.sdCardRoot = tmp.root.absolutePath
31+
assertEquals("old", settings.font)
32+
33+
// Edit with the debounced save still pending; disk continues to hold "old".
34+
// reload() must not let loadFromDisk clobber the newer in-memory value.
35+
settings.font = "new"
36+
settings.reload()
37+
38+
assertEquals("new", settings.font)
39+
}
40+
41+
@Test fun `reload picks up external disk changes when nothing is pending`() {
42+
writeSettingsJson(tmp.root, """{"font":"old"}""")
43+
val settings = newRepo()
44+
settings.sdCardRoot = tmp.root.absolutePath
45+
assertEquals("old", settings.font)
46+
47+
// No pending local edit: a clean reload should still surface a file an
48+
// external writer (e.g. the Kitchen web UI) changed underneath us.
49+
writeSettingsJson(tmp.root, """{"font":"external"}""")
50+
settings.reload()
51+
52+
assertEquals("external", settings.font)
53+
}
54+
}

0 commit comments

Comments
 (0)