Skip to content

feat: support folder selection from other apps via Storage Access Framework - #202

Open
alvaroemtnez wants to merge 2 commits into
opencloud-eu:mainfrom
alvaroemtnez:improvement/saf_folder_selection
Open

feat: support folder selection from other apps via Storage Access Framework#202
alvaroemtnez wants to merge 2 commits into
opencloud-eu:mainfrom
alvaroemtnez:improvement/saf_folder_selection

Conversation

@alvaroemtnez

Copy link
Copy Markdown

Summary

OpenCloud has a correct implementation to select its locations from the system files app via the Storage Access Framework. However, as stated in #99 and #45, selecting a folder from an app was not supported, as the ACTION_OPEN_DOCUMENT_TREE intent of the Android Storage Access Framework (SAF) needed to be implemented.

This PR adds support for that intent, and also, as some apps filter the SAF locations and allow only local storage (which OpenCloud can be, if a folder is selected as "available offline", so filtering it makes no sense but hardcoding it as local storage doesn't either), adds a toggle in the Advanced section of settings to pretend to be so, by adding the FLAG_LOCAL_ONLY flag, as other apps like RSAF do, and the corresponding translations for the settings item.

As granting an app access to a location in OpenCloud can mean that the app in question does multiple concurrent operations, light changes have been made, replacing fileToUpload singleton with ConcurrentHashMap and UUIDs to support multi-threaded file creation.

Code changes

  • Added the isChildDocument function override to opencloudApp/src/main/java/eu/opencloud/android/presentation/documentsprovider/DocumentsStorageProvider.kt to implement the ACTION_OPEN_DOCUMENT_TREE intent.
  • Also in opencloudApp/src/main/java/eu/opencloud/android/presentation/documentsprovider/DocumentsStorageProvider.kt, minor changes to use ConcurrentHashMap and random UUIDs for a more robust upload queue.
  • Added support in the addRoot function in opencloudApp/src/main/java/eu/opencloud/android/presentation/documentsprovider/cursors/RootCursor.kt to pass a boolean to enable pretending to be local storage.
  • Added a settings option in the Advanced menu to pretend to be local storage (modified opencloudApp/src/main/res/xml/settings_advanced.xml and opencloudApp/src/main/java/eu/opencloud/android/presentation/settings/advanced/SettingsAdvancedFragment.kt)
  • Added translations for the advanced menu settings item for all supported languages. A subtitle warns the user of the possible unpredictable behaviour pretending to be local storage could originate in slow network connections.

Limitations

  • The URI displayed to the user (if the app in question displays an URI) can be counter-intuitive, as it will be similar to content://eu.opencloud.documents/tree/[ID], where [ID] is an integer defining the directory or file in the internal app tree. As far as I know, displaying the full URI that the user sees in their cloud web UI would require deep and breaking changes (which would probably not be worth, at the moment).
  • Concurrent operations are supported with the UUID based queue (which didn't require deep changes), however, if an app requires thousands of concurrent file operations (i.e., Signal backups, that concurrently read, write, delete, move, etc. files, which is very intensive), that can lead to unexpected behaviour (upload errors, or the entire files directory that signal uses replaced by an empty version). Signal was the only use case I tested where this happened, and only in the very recent new backup system, which is designed to be local only. These use cases seem out of scope for this PR, and would require a regular folder sync like in the desktop app, or a straight WebDAV mount (which RSAF and others already do), which wouldn't be appropriate to implement only to support one edge case. SeedVault could also maybe create the same problems, but again, extremely operation intensive cases seem out of scope when there are solutions specifically tailored for this, and those backups were meant to be done locally, not in a network mount.

AI use

LLMs (Gemini) were used for assistance in code generation and for the translations. All generated code was reviewed and tested and can be easily audited.

…k, include a toggle to pretend to be local storage and solve COLUMN_FLAGS being declared twice in RootCursor.kt
@guruz

guruz commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

@zerox80 ^

@zerox80

zerox80 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

ill take a look

val parentFile = getFileByIdOrException(parentIdInt)

// Check if the child belongs to the same account and its path sits inside the parent's path
childFile.owner == parentFile.owner && childFile.remotePath.startsWith(parentFile.remotePath)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep tree grants within the selected space

isChildDocument() is part of SAF's permission boundary: Android calls it before allowing access to a document through a tree URI.

OpenCloud paths are relative to a space, so two spaces belonging to the same account can both contain paths such as / or /Documents/file.txt. The current check only compares owner and remotePath. Consequently, after the user grants access to Space A, a client can construct a URI containing a document ID from Space B and this method will incorrectly accept it as a descendant.

At minimum, the space must be part of the comparison:

Suggested change
childFile.owner == parentFile.owner && childFile.remotePath.startsWith(parentFile.remotePath)
childFile.owner == parentFile.owner &&
childFile.spaceId == parentFile.spaceId &&
childFile.remotePath.startsWith(parentFile.remotePath)

newFile.parentFile?.mkdirs()
fileToUpload = OCFile(

val pendingId = "pending_" + UUID.randomUUID().toString()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the temporary upload path unique as well

The UUID currently makes only the map key unique. The backing file is still created as tempDir/displayName.

Therefore, two concurrent creations with the same basename in different remote folders, for example folderA/config.json and folderB/config.json, point to the same local file. Their writes can overwrite each other, and the two queued MOVE uploads then race for the same source path. This can result in incorrect contents or a missing second upload; it only takes two concurrent files, not thousands of operations.

A UUID-specific staging directory keeps the original filename while making the complete path unique:

Suggested change
val pendingId = "pending_" + UUID.randomUUID().toString()
val pendingId = "pending_${UUID.randomUUID()}"
val tempDir = File(FileStorageUtils.getTemporalPath(parentDocument.owner, parentDocument.spaceId))
val newFile = File(File(tempDir, pendingId), displayName)
newFile.parentFile?.mkdirs()

Timber.d("A file with id $documentId has been closed! Time to synchronize it with server.")
// If only needs to upload that file
if (uploadOnly) {
pendingUploads.remove(documentId)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the document ID valid after the first close

createDocument() returns pending_<uuid> as the document's ID, but this entry is removed as soon as the first descriptor closes. There is no mapping from that UUID to the final database file ID.

Any later queryDocument() or openDocument() using the URI returned by createDocument() therefore fails. openDocument() can additionally reach the uninitialized fileToUpload fallback. Losing the map after process recreation causes the same problem.

There is a related metadata issue: queryDocument(pendingId) calls addFile() with an OCFile whose id is null, so FileCursor reports "null" as COLUMN_DOCUMENT_ID instead of the returned UUID.

Android requires a document ID returned by createDocument() to remain stable because it can be used by persistent URI grants:
https://developer.android.com/reference/android/provider/DocumentsProvider#createDocument(java.lang.String,%20java.lang.String,%20java.lang.String)

This should use a persisted UUID-to-file mapping rather than a process-local map. The cursor API could then explicitly expose the provider ID:

fun addFile(
    file: OCFile,
    documentId: String = requireNotNull(file.id).toString(),
) {
    // ...
    newRow()
        .add(Document.COLUMN_DOCUMENT_ID, documentId)
        // ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants