[Android] isExists will return false on correct bookmarked Folder URIs

[licryle]

Rebonjour!

Thanks again for your hard work :)

Working on migrating from Kotlin to Kotlin KMP, I've had to port over my previous persisted "non-bookmarked" Folder URIs to "bookmarking" in PlatformFile.

So I know the permission is correct, the app has access to it, however the following kept failing "Bookmark target is no longer accessible":
PlatformFile.fromBookmarkData(PlatformFile(context.contentResolver.persistedUriPermissions[0].uri).bookmarkData())

After investigating, I found the issue to be located with getDocumentFile():

private fun getDocumentFile(uri: Uri): DocumentFile? {
    return DocumentFile.fromSingleUri(FileKit.context, uri)
        ?: DocumentFile.fromTreeUri(FileKit.context, uri)
}

For folders, the DocumentFile.fromSingleUri() will succeed, however its isDirectory() or exists() are both false, even for a director!
This, in turns, makes PlatformFile.isExists() return false and makes PlatformFile.fromBookmarkData() throw.

As a result, I believe getDocumentFile should check if it's a directory or File first:

private fun getDocumentFile(uri: Uri): DocumentFile? {
    val tree = DocumentFile.fromTreeUri(FileKit.context, uri)

    return if (tree?.isDirectory == true) {
        tree
    } else {
        DocumentFile.fromSingleUri(FileKit.context, uri)
    }
}

I couldn't file a test battery, and gemini tells me what you did was "more correct" (and clearly more efficient), so don't trust my change by any means, but there's also something fishy that fromSingleUri() will succeed for a directory then reject at exists() or isDirectory()

Encore merci!

[licryle]

Hmm there's something weirder at play...

It can store and restore fine, but then calling .name on the PlatformFile will return "Unsupported Uri content://com.android.externalstorage.documents/tree/primary:HSKWidget"

because of FileKit.context.contentResolver.query(uri), so definitely don't trust the above solution, at least not alone.

[licryle]

Ok, I got it.
Fundamentally PlatformFile should mostly be storing files Uris, which is why the file picker does...

            val platformDirectory = treeUri.let {
                // Transform the treeUri to a documentUri
                val documentUri = DocumentsContract.buildDocumentUriUsingTree(
                    treeUri,
                    DocumentsContract.getTreeDocumentId(treeUri)
                )
                PlatformFile(documentUri)
            }

Applying that logic to my legacy Uri solved all issues.
But I'm also seeing that code in bookmarkData() itself so I shouldn't have had to do it.
I'll try to have a look tomorrow into why.

[licryle]

Coming back with a Fresh mind.

TLDR: the lib doesn't seem to properly handle TreeURI as input. In bookmarkData it doesn't coerce it to a DocumentURI like FilePicker does, and getDocumentFile silently make TreeURI pass for DocumentURI for a subsequent failure.

How to repro:

• Give permissions to an Android folder outside of PlatformFile
• Execute

PlatformFile.fromBookmarkData(
                     PlatformFile(context.contentResolver.persistedUriPermissions[0].uri)
                     .bookmarkData()
)

• Notice the "Bookmark target is no longer accessible" while it's clearly accessible.

More in details:

• The FilePicker always returns a DocumentUri, even for folders/TreeUri using buildDocumentUriUsingTree
• The fromBookmarkData will call exists(), which calls getDocumentFile, which assumes anything is a fromSingleUri -- or rather, fromSingleUri will not fail on a TreeUri, so silently return the wrong result to exists() later on
• bookmarkData does not coerce TreeURI to a SingleURI like FilePicker does, but stores the original URI, which could (or not) be from FilePicker.

I believe there's an issue in the discrepancy in the approach: some places coerce, others do not. Of course this is a sneaky bug as it wouldn't be noticed if only using the lib to retrieve and store, but as soon as you feed a TreeURI, things go heyway.

Now, I think there are 2 paths: one where everything stored is a DocumentUri, even for folders, to match what currently happens with FilePicker. That would essentially mean modifying bookmarkData() to persist the documentURI.

public suspend fun PlatformFile.bookmarkData(): BookmarkData = withContext(Dispatchers.IO) {
    when (androidFile) {
        is AndroidFile.FileWrapper -> {
            val data = androidFile.file.path
            BookmarkData(data.encodeToByteArray())
        }

        is AndroidFile.UriWrapper -> {
            val uri = androidFile.uri

            val flags =
                Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_GRANT_WRITE_URI_PERMISSION

            FileKit.context.contentResolver.takePersistableUriPermission(uri, flags)

            // Check if this is a tree URI (directory) or document URI (file)
            val uriToStore = if (isDirectory()) {
                // For directories, we need to get the tree URI
                val documentId = DocumentsContract.getTreeDocumentId(uri)
                DocumentsContract.buildDocumentUriUsingTree(uri, documentId)
            } else {
                // For files, use the URI directly
                uri
            }
            val data = uriToStore.toString()
            BookmarkData(data.encodeToByteArray())
        }
    }
}

However, being not an expert I deferred to Gemini who tells me coercing a Tree as a Document is less robust than storing the TreeUri, like bookmarkData does at the moment.

In which case, the fixes required would be to:

• fix getDocumentFile to call TreeURI first,
• file FilePicker to return the TreeURI as well
• fix other places as needed if there are any

What do you think? :[]
Happy to help