makiolaj/agendula
1
0
Fork 0
Code Issues Pull Requests Actions Releases 7 Activity

feat(store)!: delete the vendored dmfs provider

Browse Source 
Phase 5 of docs/OWN-STORE.md. The :provider module goes — 84 Java files,
14,555 lines, its <provider>, its two custom permissions, its 13
translated strings and its three dmfs runtime dependencies. Room has been
the default since the previous commit and every v0.3.x install has been
imported, so nothing reads it any more.

StorageMode.LOCAL is gone with it; OWN and EXTERNAL are what remain.
ProviderResolver narrows to what it was always really for — discovering
external providers — and answers null in OWN mode, where there is no
authority to resolve. Callers that need to tell that apart from "External
with nothing installed" ask mode(). ProviderStatus is unconditionally
READY in OWN mode: the permission gate only ever applied to External, and
that is now visibly true rather than a special case inside it.

A stored LOCAL is read as OWN rather than as an unparseable value. Left
to fall through to autoMode, someone who had explicitly chosen local
storage while also having OpenTasks granted would have been sent to
OpenTasks instead.

ProviderChangeReceiver's manifest filter drops our own authority — safe
now, because nothing of ours broadcasts ACTION_PROVIDER_CHANGED. In OWN
mode Room's InvalidationTracker covers foreground changes and nothing
outside the app can change our data. When SYNC.md phase 3 lands, the sync
worker must call ReminderScheduler.sync() itself; that is the replacement
for the broadcast and it belongs in the sync work.

lib-recur stays as a direct dependency and is still Apache-2.0 dmfs, so
the attribution is still owed — now as a normal third-party dependency.
provider/PROVENANCE.md is replaced by a postscript in STORAGE-DECISION.md
recording that the fork existed, why, and the one detail that still binds
us: tasks.org is DB 22 and has no is_recurring, so TaskMapper must keep
deriving recurrence from rrule/rdate.

BREAKING: the de.jeanlucmakiola.agendula.tasks authority and both custom
permissions are gone. Anyone who pointed DAVx5 or another app at that
authority loses it; External mode is the answer. Needs calling out in the
release notes.

Verified: the APK declares no ContentProvider, no custom permission and
no agendula.tasks authority, and carries no dmfs provider classes.
This commit is contained in:
Jean-Luc Makiola
2026-08-13 16:33:19 +02:00
parent 76f9ae6780
commit 1ed192f150
137 changed files with 129 additions and 17088 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
app/build.gradle.kts
View File

@@ -145,11 +145,6 @@ ksp {
}
dependencies {
// Agendula's own task store — the dmfs provider vendored under our authority.
// Contributes a <provider> to the merged manifest; no app code imports from it
// except ProviderResolver, which reads the authority out of its resources.
implementation(project(":provider"))
implementation(libs.androidx.core.ktx)
implementation(libs.androidx.appcompat)
implementation(libs.androidx.lifecycle.runtime.ktx)
@@ -170,10 +165,9 @@ dependencies {
implementation(libs.androidx.navigation.compose)
ksp(libs.hilt.compiler)
// RFC 5545 recurrence expansion, in-process. Pulled in directly rather than
// through :provider: the own store expands occurrences itself. Pinned at
// 0.12.2 — 0.16.0 removed RecurrenceSet. rfc5545-datetime comes with it and
// is part of its API surface, so it isn't declared separately.
// RFC 5545 recurrence expansion, in-process. Pinned at 0.12.2 — 0.16.0
// removed RecurrenceSet. rfc5545-datetime comes with it and is part of its
// API surface, so it isn't declared separately.
implementation(libs.dmfs.lib.recur)
implementation(libs.androidx.room.runtime)

16
app/src/main/AndroidManifest.xml
View File

@@ -8,9 +8,9 @@
runtime by the permission flow, and only once the user has actually
selected External mode. Both are dangerous-level.
Agendula's own bundled provider needs NO entry here: it runs under our uid
and a same-uid caller bypasses a provider's permission checks outright.
Its permissions are declared by the :provider module, for other apps. -->
StorageMode.OWN needs nothing here: it is a Room database in our own data
directory. Agendula publishes no ContentProvider and declares no
permissions of its own. -->
<uses-permission android:name="org.dmfs.permission.READ_TASKS" />
<uses-permission android:name="org.dmfs.permission.WRITE_TASKS" />
<uses-permission android:name="org.tasks.permission.READ_TASKS" />
@@ -80,16 +80,16 @@
</intent-filter>
</receiver>
<!-- Re-sync reminders when the provider changes — our own writes, and
external sync (DAVx5) in External mode. Every authority we might be
pointed at needs listing, because an intent-filter host must be a
literal: our bundled provider first, then the two external ones. -->
<!-- Re-sync reminders when an external provider changes — DAVx5 pulling
tasks while Agendula is backgrounded. External mode only: in OWN mode
nothing outside the app can change our data, and Room's
InvalidationTracker covers our own writes. An intent-filter host must
be a literal, so both external authorities are listed. -->
<receiver
android:name=".data.reminders.ProviderChangeReceiver"
android:exported="true">
<intent-filter>
<action android:name="android.intent.action.PROVIDER_CHANGED" />
<data android:scheme="content" android:host="de.jeanlucmakiola.agendula.tasks" />
<data android:scheme="content" android:host="org.tasks.opentasks" />
<data android:scheme="content" android:host="org.dmfs.tasks" />
</intent-filter>

16
app/src/main/java/de/jeanlucmakiola/agendula/data/prefs/SettingsPrefs.kt
View File

@@ -95,13 +95,15 @@ class SettingsPrefs @Inject constructor(
* upgrading Posture A user pointed at the provider that holds their data.
*/
val storageMode: Flow<StorageMode?> = dataStore.data.map { p ->
p[STORAGE_MODE]
?.let { runCatching { StorageMode.valueOf(it) }.getOrNull() }
// A stored LOCAL means "the bundled dmfs provider", and after the
// one-shot import that store no longer holds the user's tasks — its
// file has been renamed away. Read it as OWN so someone who chose
// local storage explicitly lands on the store their data is now in.
?.let { if (it == StorageMode.LOCAL) StorageMode.OWN else it }
when (val stored = p[STORAGE_MODE]) {
null -> null
// 0.3.x's value for the bundled dmfs provider. That store is gone and
// its data was imported into OWN, so read it as OWN rather than
// letting it fall through to autoMode — someone who chose local
// storage explicitly would otherwise be sent to an external provider.
"LOCAL" -> StorageMode.OWN
else -> runCatching { StorageMode.valueOf(stored) }.getOrNull()
}
}
suspend fun setStorageMode(mode: StorageMode) = dataStore.edit { it[STORAGE_MODE] = mode.name }

7
app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ModeRoutingTasksDataSource.kt
View File

@@ -16,8 +16,7 @@ import kotlin.time.Instant
* [ProviderResolver] without rebuilding the object graph. Both delegates are
* singletons, so this chooses between two existing objects.
*
* The [StorageMode.LOCAL] branch disappears with the `:provider` module; after
* that this is just Room versus a third-party ContentProvider.
* Room versus a third-party ContentProvider — nothing else.
*/
class ModeRoutingTasksDataSource(
private val resolver: ProviderResolver,
@@ -26,9 +25,9 @@ class ModeRoutingTasksDataSource(
) : TasksDataSource {
private fun active(): TasksDataSource =
when (resolver.storageMode ?: resolver.autoMode()) {
when (resolver.mode()) {
StorageMode.OWN -> room.get()
StorageMode.LOCAL, StorageMode.EXTERNAL -> external.get()
StorageMode.EXTERNAL -> external.get()
}
override fun taskLists(): List<TaskList> = active().taskLists()

15
app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ProviderEnvironment.kt
View File

@@ -8,7 +8,7 @@ import javax.inject.Inject
import javax.inject.Singleton
/**
* The three platform facts [ProviderResolver] needs, behind an interface.
* The two platform facts [ProviderResolver] needs, behind an interface.
*
* Same seam the data source uses, for the same reason: which store a returning
* user lands on is decided by [ProviderResolver.autoMode], getting it wrong shows
@@ -18,12 +18,6 @@ import javax.inject.Singleton
*/
interface ProviderEnvironment {
/** Our own bundled provider's authority, from the `:provider` module's resources. */
val ownAuthority: String
/** Our own package name. */
val ownPackage: String
/** The package declaring [authority], or `null` when nothing on the device does. */
fun packageDeclaring(authority: String): String?
@@ -36,13 +30,6 @@ class AndroidProviderEnvironment @Inject constructor(
@ApplicationContext private val context: Context,
) : ProviderEnvironment {
override val ownAuthority: String
// Read from the module that declares it, never written as a literal: the
// authority lives in exactly one place, its own string resource.
get() = context.getString(de.jeanlucmakiola.agendula.provider.R.string.agendula_tasks_authority)
override val ownPackage: String get() = context.packageName
override fun packageDeclaring(authority: String): String? =
context.packageManager.resolveContentProvider(authority, 0)?.packageName

77
app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ProviderResolver.kt
View File

@@ -4,39 +4,26 @@ import javax.inject.Inject
import javax.inject.Singleton
/**
* A tasks provider Agendula can talk to. The same dmfs `TaskProvider` backs every
* candidate — ours included, since `:provider` *is* that provider vendored — so
* the [TasksContract] columns apply regardless of which is active.
* An external tasks provider Agendula can talk to. Every candidate runs the same
* dmfs `TaskProvider`, so the [TasksContract] columns apply to either.
*/
data class TaskProvider(
val authority: String,
val readPermission: String,
val writePermission: String,
val packageName: String? = null,
/**
* True for Agendula's own bundled provider. It runs in our process under our
* uid, and a same-uid caller bypasses a provider's permission checks outright,
* so [ProviderResolver.hasPermission] must never gate on a grant for it.
*/
val isOwn: Boolean = false,
)
/**
* The A/B seam: the only class in the app that knows an authority exists.
* Discovers the *external* tasks providers (OpenTasks, tasks.org) that
* [StorageMode.EXTERNAL] can be pointed at.
*
* - **Posture A** — an *external* provider (OpenTasks, tasks.org). Still fully
* supported; it stopped being the default and became a user choice.
* - **Posture B** — our own bundled provider, under our **own** authority. It
* coexists with everything and replaces nothing.
* This used to be the A/B seam between an external provider and one Agendula
* bundled itself. That second half is gone: [StorageMode.OWN] is a Room database
* with no authority, no ContentResolver and nothing to permit, so there is
* nothing here for it to resolve. See `docs/OWN-STORE.md`.
*
* Both terms were redefined by `docs/STORAGE-AND-SYNC.md`. Posture B used to mean
* "bundle OpenTasks and squat `org.dmfs.tasks`"; that is a dead end and is not
* coming back, because two apps cannot declare the same authority
* (`INSTALL_FAILED_CONFLICTING_PROVIDER`) or the same permission name
* (`INSTALL_FAILED_DUPLICATE_PERMISSION`) — anyone with OpenTasks installed would
* simply have been unable to install Agendula.
*
* Which one is active comes from [storageMode]; how that gets decided when the
* Which store is active comes from [storageMode]; how that gets decided when the
* user has not chosen is [autoMode].
*/
@Singleton
@@ -54,23 +41,17 @@ class ProviderResolver @Inject constructor(
@Volatile
var storageMode: StorageMode? = null
/** Agendula's own provider. Always present — it ships inside the APK. */
val own: TaskProvider by lazy {
TaskProvider(
authority = environment.ownAuthority,
readPermission = OWN_READ_PERMISSION,
writePermission = OWN_WRITE_PERMISSION,
packageName = environment.ownPackage,
isOwn = true,
)
}
/** The active store, resolving the undecided case through [autoMode]. */
fun mode(): StorageMode = storageMode ?: autoMode()
/** The active provider, or `null` when [StorageMode.EXTERNAL] is chosen and none is installed. */
fun resolve(): TaskProvider? = when (storageMode ?: autoMode()) {
StorageMode.LOCAL -> own
// Room has no authority, no ContentResolver and nothing to permit. The
// provider entry stands in so ProviderStatus stays READY; nothing queries it.
StorageMode.OWN -> own
/**
* The provider to query, or `null` — either because [StorageMode.OWN] is
* active and there is no provider involved at all, or because
* [StorageMode.EXTERNAL] is and none is installed. Callers that need to tell
* those apart ask [mode].
*/
fun resolve(): TaskProvider? = when (mode()) {
StorageMode.OWN -> null
StorageMode.EXTERNAL -> resolveExternal()
}
@@ -78,9 +59,9 @@ class ProviderResolver @Inject constructor(
* What to use when the user has not chosen — and the one piece of real
* judgement in this class, because getting it wrong loses people their data.
*
* Ranking our own provider first unconditionally would be wrong: someone who
* Ranking our own store first unconditionally would be wrong: someone who
* has been using Agendula over OpenTasks since 0.3.x would update, land on an
* empty bundled store, and reasonably conclude their tasks were deleted.
* empty database, and reasonably conclude their tasks were deleted.
*
* So the tell is **whether we already hold an external provider's runtime
* permission**. That is a dangerous permission — it can only be there because
@@ -106,23 +87,9 @@ class ProviderResolver @Inject constructor(
}
fun hasPermission(provider: TaskProvider): Boolean =
// Same uid, same process: there is nothing to grant, and asking would put a
// permission dialog in front of a purely local app for no reason. This is
// the bypass docs/STORAGE-AND-SYNC.md calls for — without it
// ProviderStatus.NEEDS_PERMISSION fires in Local mode and the onboarding
// gate asks for a permission that can never be granted.
provider.isOwn ||
(environment.isGranted(provider.readPermission) && environment.isGranted(provider.writePermission))
environment.isGranted(provider.readPermission) && environment.isGranted(provider.writePermission)
companion object {
/**
* Declared by the `:provider` module's manifest. Listed here so the app can
* name them; nothing ever requests them, since [hasPermission] short-circuits
* for our own provider.
*/
const val OWN_READ_PERMISSION = "de.jeanlucmakiola.agendula.permission.READ_TASKS"
const val OWN_WRITE_PERMISSION = "de.jeanlucmakiola.agendula.permission.WRITE_TASKS"
/**
* Verified on-device: tasks.org exposes `org.tasks.opentasks` backed by
* `org.dmfs.provider.tasks.TaskProvider`, guarded by `org.tasks.permission.*`

42
app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StorageMode.kt
View File

@@ -1,46 +1,26 @@
package de.jeanlucmakiola.agendula.data.tasks
/**
* Which task store backs the app — the user's choice, per `docs/STORAGE-AND-SYNC.md`.
* Which task store backs the app — the user's choice, per `docs/OWN-STORE.md`.
*
* Only two values, though the document describes three modes. **Synced is not a
* third store**: it is [LOCAL] with an account attached, so it is derived state
* (does an account of ours exist?) rather than something the user picks. Adding
* a `SYNCED` constant here would imply otherwise.
*
* ⚠️ This used to add "so switching sync on is never a migration". That is wrong.
* `TaskLists.ACCOUNT_TYPE` is write-once in the provider — `processors/lists/
* Validating.java:68-76` throws `IllegalArgumentException` on any attempt to
* change it — so attaching an account to an existing local list means recreating
* every list and every task under the new account. See `docs/SYNC.md`.
*
* Nothing above the data layer reads this; it selects an authority for
* [ProviderResolver] and stops there.
* Only two values, though `docs/STORAGE-AND-SYNC.md` describes three modes.
* **Synced is not a third store**: it is [OWN] with an account attached to a
* list, which is derived state rather than something the user picks. Attaching
* one is a plain `UPDATE task_lists SET account_id = ?` — not the full data
* migration it was under the dmfs provider, whose `ACCOUNT_TYPE` was write-once.
*/
enum class StorageMode {
/**
* Agendula's own bundled provider (the `:provider` module). Always available —
* it ships in the APK — and needs no permission grant at all, because
* same-uid access to your own provider skips the permission check entirely.
*
* On its way out: [OWN] replaces it once the Room store is the default, and
* this constant leaves with the `:provider` module. See `docs/OWN-STORE.md`.
*/
LOCAL,
/**
* Agendula's own Room database. Named as a third value rather than renaming
* [LOCAL] because both stores exist at once while the migration runs — a
* rename now would make `OWN` mean the dmfs provider for several phases and
* Room afterwards.
* Agendula's own Room database. The default, and always available: there is
* no authority, no ContentResolver and no permission to grant.
*/
OWN,
/**
* A tasks provider app already on the device (OpenTasks, tasks.org), synced by
* whatever that provider's engine is — DAVx5 and friends. This is the original
* Posture A, still fully supported, but now a choice rather than the only way.
* Requires that provider's runtime read/write permissions.
* whatever that provider's engine is — DAVx5 and friends. Still fully
* supported, now a choice rather than the only way. Requires that provider's
* runtime read/write permissions.
*/
EXTERNAL,
}

4
app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt
View File

@@ -127,6 +127,10 @@ class TasksRepositoryImpl @Inject constructor(
withContext(io) { dataSource.createLocalList(name, color) }
override fun providerStatus(): ProviderStatus {
// Our own store is always ready: it ships with the app, needs no provider
// and no grant. The permission gate only ever applied to External mode —
// now that is visibly true rather than a special case inside it.
if (providerResolver.mode() == StorageMode.OWN) return ProviderStatus.READY
val provider = providerResolver.resolve() ?: return ProviderStatus.NO_PROVIDER
return if (providerResolver.hasPermission(provider)) ProviderStatus.READY
else ProviderStatus.NEEDS_PERMISSION

7
app/src/main/java/de/jeanlucmakiola/agendula/ui/permission/PermissionViewModel.kt
View File

@@ -41,13 +41,8 @@ class PermissionViewModel @Inject constructor(
val provider = providerResolver.resolve()
_state.value = PermissionUiState(
status = repository.providerStatus(),
// Never our own provider's permissions. They are declared for *other*
// apps to hold; requesting them here would show a dialog for a
// permission that a same-uid caller does not need and the system will
// not meaningfully grant. In practice this branch is unreachable in
// Local mode, since the status is already READY — belt and braces.
// Null in OWN mode, where there is no provider and nothing to grant.
permissionsToRequest = provider
?.takeUnless { it.isOwn }
?.let { listOf(it.readPermission, it.writePermission) }
.orEmpty(),
)

46
app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ProviderResolverTest.kt
View File

@@ -21,8 +21,6 @@ class ProviderResolverTest {
val installed: Map<String, String> = emptyMap(),
val granted: Set<String> = emptySet(),
) : ProviderEnvironment {
override val ownAuthority = "de.jeanlucmakiola.agendula.tasks"
override val ownPackage = "de.jeanlucmakiola.agendula"
override fun packageDeclaring(authority: String): String? = installed[authority]
override fun isGranted(permission: String): Boolean = permission in granted
}
@@ -39,26 +37,16 @@ class ProviderResolverTest {
private val openTasksGranted = setOf(openTasks.readPermission, openTasks.writePermission)
@Nested
inner class OwnProvider {
inner class OwnStore {
@Test
fun `needs no permission grant`() {
// The bypass the whole Local mode rests on: our provider runs under our
// own uid, so there is nothing to grant and nothing to ask for.
val resolver = resolver()
assertThat(resolver.hasPermission(resolver.own)).isTrue()
}
@Test
fun `is always resolvable because it ships in the APK`() {
assertThat(resolver(mode = StorageMode.LOCAL).resolve()).isNotNull()
}
@Test
fun `does not use a dmfs authority`() {
// Squatting org.dmfs.tasks would make Agendula and OpenTasks mutually
// uninstallable. Guards against a careless resync of the vendored module.
assertThat(resolver().own.authority).isEqualTo("de.jeanlucmakiola.agendula.tasks")
fun `resolves to no provider at all`() {
// Room has no authority and no ContentResolver, so there is nothing
// here to resolve — which is the point. Callers that need to tell this
// apart from "External, none installed" ask mode().
val resolver = resolver(mode = StorageMode.OWN)
assertThat(resolver.resolve()).isNull()
assertThat(resolver.mode()).isEqualTo(StorageMode.OWN)
}
}
@@ -104,10 +92,12 @@ class ProviderResolverTest {
fun `overrides the automatic answer in both directions`() {
val wouldBeExternal = FakeEnvironment(openTasksInstalled, openTasksGranted)
val forcedLocal = ProviderResolver(wouldBeExternal).apply { storageMode = StorageMode.LOCAL }
assertThat(forcedLocal.resolve()?.isOwn).isTrue()
val forcedOwn = ProviderResolver(wouldBeExternal).apply { storageMode = StorageMode.OWN }
assertThat(forcedOwn.mode()).isEqualTo(StorageMode.OWN)
assertThat(forcedOwn.resolve()).isNull()
val forcedExternal = ProviderResolver(FakeEnvironment()).apply { storageMode = StorageMode.EXTERNAL }
assertThat(forcedExternal.mode()).isEqualTo(StorageMode.EXTERNAL)
assertThat(forcedExternal.resolve()).isNull()
}
@@ -152,10 +142,14 @@ class ProviderResolverTest {
}
@Test
fun `never include our own provider`() {
// EXTERNAL must mean "somebody else's store". If ours ever leaked into
// this list, choosing External would silently keep using it.
assertThat(ProviderResolver.EXTERNAL_CANDIDATES.none { it.isOwn }).isTrue()
fun `never name an authority of ours`() {
// EXTERNAL must mean "somebody else's store", and Agendula publishes no
// provider at all any more.
assertThat(
ProviderResolver.EXTERNAL_CANDIDATES.none {
it.authority.startsWith("de.jeanlucmakiola")
},
).isTrue()
}
}
}