From 22c57710b7461b3875e811c2f43738586fe7a7b3 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 6 Nov 2026 18:21:26 +0100 Subject: [PATCH] Refuse read-only provider tables before the store check (#137) The provider contract tests failed whenever Agendula ran on an external provider: the store check fired first. Value checks still need the own store, so those two tests now skip without it. --- .../provider/AgendulaTaskProviderTest.kt | 5 +++++ .../data/tasks/provider/ProviderWriter.kt | 22 +++++++++++++------ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/AgendulaTaskProviderTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/AgendulaTaskProviderTest.kt index fd0f678..5e82779 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/AgendulaTaskProviderTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/AgendulaTaskProviderTest.kt @@ -124,6 +124,7 @@ class AgendulaTaskProviderTest { @Test fun aBadWriteReachesTheClientAsIllegalArgument() { + assumeTrue("Agendula is running on an external provider", ownStore()) // No list has this id, so nothing in the app's real store is touched. val values = ContentValues().apply { put("list_id", Long.MAX_VALUE) @@ -135,6 +136,7 @@ class AgendulaTaskProviderTest { @Test fun aFailingBatchWritesNothing() { + assumeTrue("Agendula is running on an external provider", ownStore()) val missingList = Long.MAX_VALUE val operations = arrayListOf( ContentProviderOperation.newInsert(uri("tasks")).withValue("list_id", missingList).withValue("title", "a").build(), @@ -145,6 +147,9 @@ class AgendulaTaskProviderTest { .use { assertThat(it.count).isEqualTo(0) } } + /** Whether the app's own store is active: only then does the provider take writes at all. */ + private fun ownStore() = runCatching { resolver.delete(uri("tasks"), "_id = -1", null) }.isSuccess + /** stdout and stderr of [command]; the denial lands on stderr. */ private fun shell(command: String): String { val (stdout, stdin, stderr) = InstrumentationRegistry.getInstrumentation().uiAutomation diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt index b262286..8afa7ca 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt @@ -62,16 +62,15 @@ class ProviderWriter internal constructor( private var batchWrote = false /** Inserts one row and returns its published id. */ - fun insert(table: ProviderTable, values: ProviderValues): Long = write { + fun insert(table: ProviderTable, values: ProviderValues): Long = writable(table, INSERTABLE_TABLES) { when (table) { ProviderTable.TASKS -> insertTask(values) - ProviderTable.PROPERTIES -> insertProperty(values) - else -> throw UnsupportedOperationException("${table.path} is read-only") + else -> insertProperty(values) } } /** Updates every row [selection] matches and returns how many changed. */ - fun update(table: ProviderTable, values: ProviderValues, selection: String?, args: Array?): Int = write { + fun update(table: ProviderTable, values: ProviderValues, selection: String?, args: Array?): Int = writable(table, WRITABLE_TABLES) { when (table) { ProviderTable.TASKS -> { val writable = ProviderTaskWriter.writable(table, values, WRITABLE_TASK_COLUMNS) @@ -83,17 +82,17 @@ class ProviderWriter internal constructor( instances(selection, args).count { updateInstance(it, writable) } } ProviderTable.PROPERTIES -> ids(table, selection, args).count { updateProperty(it, values) } - else -> throw UnsupportedOperationException("${table.path} is read-only") + else -> error("unreachable") } } /** Deletes every row [selection] matches and returns how many went. */ - fun delete(table: ProviderTable, selection: String?, args: Array?): Int = write { + fun delete(table: ProviderTable, selection: String?, args: Array?): Int = writable(table, WRITABLE_TABLES) { when (table) { ProviderTable.TASKS -> ids(table, selection, args).count { deleteTask(it) } ProviderTable.INSTANCES -> instances(selection, args).count { deleteInstance(it) } ProviderTable.PROPERTIES -> ids(table, selection, args).count { deleteProperty(it) } - else -> throw UnsupportedOperationException("${table.path} is read-only") + else -> error("unreachable") } } @@ -113,6 +112,12 @@ class ProviderWriter internal constructor( } } + /** A read-only table is refused as such, whichever store is active. */ + private fun writable(table: ProviderTable, allowed: Set, block: () -> T): T { + if (table !in allowed) throw UnsupportedOperationException("${table.path} is read-only") + return write(block) + } + private fun write(block: () -> T): T = source.locked { check(resolver.mode() == StorageMode.OWN) { "Agendula is working on top of an external tasks provider; write to that one instead" @@ -435,3 +440,6 @@ class ProviderWriter internal constructor( private fun zone(): String = ZoneId.systemDefault().id } + +private val INSERTABLE_TABLES = setOf(ProviderTable.TASKS, ProviderTable.PROPERTIES) +private val WRITABLE_TABLES = INSERTABLE_TABLES + ProviderTable.INSTANCES