Fix CardContentProvider to support parallel builds

[iamllama]

Purpose / Description

CardContentProvider is currently broken for parallel builds

Fixes

• Fixes [BUG]: CardContentProvider uses the wrong authority uri in parallel builds #17309 (partially, see below)

Approach

Makes the authority uri and permission rely on the application id, instead of being hardcoded for the release and debug builds.

Work still needs to be done to update the relevant tests, but I don't know how to get at the build config in there 🤔

How Has This Been Tested?

Tested on the debug build

Checklist

Please, go through these checks before submitting the PR.

• You have a descriptive commit message with a short title (first line, max 50 chars).
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the Google Accessibility Scanner

[welcome]

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

[iamllama]

The existing changes are enough to get the content provider working on parallel builds as long as users construct their own uris.

All the provided uris in FlashCardContract are statically constructed using the hardcoded authority. And honestly, I'm not sure how to go about removing that.

It doesn't seem ideal to publish parallel builds of the api lib along with ankidroid, because they'd only differ by authority and end-user apps would have to anticipate and import all variants that they might use. But then that's where the tests and apps get the content uris from, so there has to be some way to specify the suffix.

1. What if FlashCardContract took the application id (of the targetted anki build) as a ctor arg and used it to build the authority uri and permission? That would break existing code in the wild.

2. What if FlashCardContract had a static method like getAuthorityUri(applicationId: String): Uri?

3. What if we got rid of the "A/B/debug" suffix in the authority and permission, and moved it into the uri itself? I.e. .../debug/notes_v2, .../a/media. This would also be breaking. But it'd be somewhat in line with the idea of parallel builds as profiles 🤷

I'd be in favour of option 3, seeing as how it anticipates the arrival of profiles, were it not for the fact that android only allows one content provider per authority 🤔

[mikehardy]

I don't think there is a way around parallel builds or debug etc installing separate ContentProviders - they must do that because this is how we test entirely different code, not just for some future state where there are multiple profiles living under a single ContentProvider (option 3)

But, what about....discovery?

https://developer.android.com/reference/android/content/pm/PackageManager#queryIntentContentProviders(android.content.Intent,%20int)

Any API consumer could take advantage of multiple AnkiDroid installs (debug, parallel, whatever) if it knows they are possible and queries for all the ContentProviders that answer the AnkiDroid API, and allows the user to pick, then uses the correct URIs as a dymamic thing after pick

Could that work?

[mikehardy]

This PR seems sensible to me as a start, and perhaps content provider discover can solve the problem on client side of selecting the right URI

One doubt:

Is FlashCardsContract.AUTHORITY and/or FlashCardsContract.READ_WRITE_PERMISSION still in use then?

Perhaps those constants should be changed where they are defined instead of leaving them behind and building fresh constants from the possibly-different package name here ?

[iamllama]

But, what about....discovery?

TIL 😅! An app could get all authorities via

packageManager!!.queryIntentContentProviders(Intent("com.ichi2.anki.CARD_CONTENT_PROVIDER"), 0).mapNotNull{ it?.providerInfo?.authority }

for example, with com.ichi2.anki.CARD_CONTENT_PROVIDER replaced by something like FlashCardsContract.PROVIDER_INTENT

Is FlashCardsContract.AUTHORITY and/or FlashCardsContract.READ_WRITE_PERMISSION still in use then?

They're still used in AddContentApi and extensively in ContentProviderTest's tests indirectly. For example

Anki-Android/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt

Line 638 in e9fe007

val allModels = cr.query(FlashCardsContract.Model.CONTENT_URI, null, null, null, null)

where FlashCardsContract.Model.CONTENT_URI is constructed using FlashCardsContract.AUTHORITY

These tests currently don't fail because api's build system accounts for the debug build, but not .A, .B

Anki-Android/api/build.gradle.kts

Lines 34 to 42 in e9fe007

	buildTypes {
	debug {
	buildConfigField(
	"String",
	"READ_WRITE_PERMISSION",
	"\"com.ichi2.anki.debug.permission.READ_WRITE_DATABASE\""
	)
	buildConfigField("String", "AUTHORITY", "\"com.ichi2.anki.debug.flashcards\"")
	}

which sets BuildConfig and in turn:

Anki-Android/api/src/main/java/com/ichi2/anki/FlashCardsContract.kt

Lines 90 to 91 in e9fe007

	public const val AUTHORITY: String = BuildConfig.AUTHORITY
	public const val READ_WRITE_PERMISSION: String = BuildConfig.READ_WRITE_PERMISSION

Perhaps those constants should be changed where they are defined instead of leaving them behind and building fresh constants from the possibly-different package name here ?

api is packaged separately, which I imagine is the reason for hardcoding those build config fields, so we'd need a way to pass the custom suffixes in 🤔

[iamllama]

Also, following com.ichi2.anki.debug.permission.READ_WRITE_DATABASE, I've jumped the gun and made permission rely on the suffix as well, so parallel builds would require com.ichi2.anki.a.permission.READ_WRITE_DATABASE and so on.

Would it be better for all non-debug builds to use the suffix-less com.ichi2.anki.permission.READ_WRITE_DATABASE?

In any case, apps can request runtime perms for each parallel build it finds via discovery

[mikehardy]

If you can please please avoid merge commits as they make the commit history very difficult to read - we typically rebase in this repo, and we rebase almost 100% of the time for complicated functionality, with the commits broken into independent "ideas" implemented one commit at a time for easier review. Review is frequently the blocker for merging PRs so doing this - using small atomic commits that each do one thing, and using rebase commits, can frequently speed up getting features in

[iamllama]

Undid the merge, sorry about that!!

[mikehardy]

No worries at all, it's really nice to have someone taking a good look at the ContentProvider and I think there are going to be some really good advances from all of this. Most repos just merge away but we're just picky I guess 😆 . Cheers

[iamllama]

I've added convenience methods to let apps (that want to support parallel builds) construct content uris for the provider.

This should also allow apps that rely on the existing hardcoded content uris to continue to work as normal.

Now, instead of using FlashCardsContract.Note.CONTENT_URI, for example, an app could use FlashCardsContract.Note.getContentUri(authority), with authority being found from discovery and constructed via FlashCardsContract.getAuthorityUri

And in CardContentProvider, authority and permission are no longer hardcoded:

Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/provider/CardContentProvider.kt

Lines 140 to 141 in 26b876d

	private val AUTHORITY = FlashCardsContract.getAuthority(BuildConfig.APPLICATION_ID)
	private val PERMISSION = FlashCardsContract.getPermission(BuildConfig.APPLICATION_ID)

Would this work? 🤔