re-implement file sync feature with scoped permissions via SimpleStorage library

[lrq3000]

Description

Re-implements PR #25 without requiring MANAGE_EXTERNAL_STORAGE permission, which is asks for permission to write ALL files on the Android device and which is rejected by the Google Play Store rules for all apps except those that really require it (such as file managers).

With this PR here, there is no need for special permissions anymore, the app asks for permission to write in a single folder. In fact we could rewrite to ask to write only in a single file, but having permission over a whole folder allows to write more files if there is a need in the future.

To test, there is a debug APK here: https://github.com/lrq3000/super-productivity-android/releases/tag/super-productivity-android_scoped-storage

Issues Resolved

Fixes #31, fixes #13.

Can also be used as a workaround for #9.

The library that is managing permissions, SimpleStorage, can also be used to implement #8.

Check List

• Functionality appears to work for me.
• Can others try and report if it works for them too?

[lrq3000]

I just tested syncing between an Android device under Android 10 and a desktop client on Windows 10 via SyncThing (SyncThing-Fork on Android, Synctrayzor on Windows). It worked perfectly well :-)

I did not know Super Productivity had in-built conflict detection and resolution, that's awesome! Thank you a lot @johannesjo for making and maintaining this software, and to all other contributors who added features and bugfixes :-)

[lrq3000]

ping @anggrayudi, author of SimpleStorage, he has been super helpful to implement scoped storage permissions handling correctly!

[johannesjo]

Thank you so much @lrq3000 ! This is a very important feature!

[johannesjo]

Sorry! I was a bit over eager with the merge :D Maybe we can proceed with a new PR for the remaining issues?

I found two issues:
1.It seems like the app only recognizes the given permission only the second time
2. If I try to sync I get "Error: Error: Java exception was raised during method invocation". Unfortunately I am unable to localize what is going on further. Logcat doesn't show any errors.

Let me know if I can help you debugging this somehow.

[anggrayudi]

Sorry for late review, but at least my comments will be useful for your future implementation.

[anggrayudi]

AppCompatActivity is subclass of ComponentActivity, thus no need to call storageHelper.onRequestPermissionsResult()

[anggrayudi]

No need to add API check here, because the library is smart enough to detect whether it uses scoped storage or not. Just write code like this:

val file = DocumentFileCompat.fromFullPath(activity, filePath, requiresWriteAccess = false)
file?.lastModified().toString()

[anggrayudi]

DocumentFileCompat.fromFullPath() works for non scoped-storage, so no need to add check for API level.

[anggrayudi]

DocumentFileCompat.fromFullPath() works for non scoped-storage, so no need to check the API level.

[lrq3000]

@anggrayudi thank you so much for your inputs once again! :D I know simplestorage can handle older versions but i wished to maintain the old behavior for older users, but maybe it's better if everything is done with simplestorage now? I'd suggest we drop older file management code after a while once enough users have tested the new version, just to be on the safe side :-)

@johannesjo

1. I guess you are referring to the situation when simplestorage detects that getting access to the folder is not sufficient and access to the root is also necessary, and hence it asks as 2nd time but first it asks for root access. I don't think there is another way around. If you try to create and get access to a folder on the sdcard, it should work the first time around, or also in the Download folder since it's a shared folder by default, but if you try to create a folder on the primary storage, then it will have to ask a 2nd time to get the root first and then the access to your folder. So it depends on user's choice unfortunately, this can't be detected apriori, it's Android's fault with their scoped storage system. @anggrayudi please tell me if I'm wrong on this :-)
2. This is an issue. You should have logcat messages before the exception, even if the exception is not caught in the logcat, the other logcat messages (tag:SuperProductivity) should help me debug where the issue is. But i also noticed a bug with reading the file (but not uploading, uploads work fine), i will have a look. May i ask how you synchronized? Did you already have a file to synchronize (eg, a file created on desktop), or did you try to create a new sync file on Android (this case is the one i tested)?

[anggrayudi]

On API 29, yes the user should select the root path first. But for API 30 and higher, no need to select the root path first. The user can select any folder directly and SimpleStorage will persist the permission in background. You can test these behaviors on emulators. Additionally, you can force the user to select/grant specific folders with expectedBasePath and expectedStorageType for API 30+.

[lrq3000]

@anggrayudi Ah ok thank you very much for the precision!

@johannesjo In fact about issue 1 you meant that the UI says "Needs permission" after first selection, right? I can reproduce the issue, I'll fix it. And likely issue 2 is the same I have about reading the file, so it should be fixed too in my next PR.

[lrq3000]

@johannesjo I'm sorry but I'll need your help to fix issue 1: it's a concurrency issue, what is happening is that the web app calls isGrantedFilePermission() before the permissions are saved! This is because in the web app, the file permission is coded like this in several places:

(IS_ANDROID_WEB_VIEW &&
            (androidInterface as any).grantFilePermission &&
            androidInterface.isGrantedFilePermission)

Hence, isGrantedFilePermission() is called right away after grantFilePermission() returns. This worked before because grantFilePermission() used to directly request for permission in a blocking manner in older Android versions, but now grantFilePermission() calls an asynchronous folder/file picker which itself calls back storageHelper.onFolderSelected() (and other internal functions to storageHelper) which saves the permission.

The solution is to rewrite the condition in the web app to wait for grantFilePermissionCallback() to be called. This function was already implemented, I guess to avoid this very issue, but was not used before because it wasn't necessary, but now it is. grantFilePermissionCallback() deletes the requestMap[rId], ie, deletes the requestId from the requestMap. So I guess we can check the absence of requestId inside the requestMap to check whether grantFilePermission() callbacks are done, and then proceed to call isGrantedFilePermission().

I unfortunately don't know how to code in typescript/nodejs, I hope my description above is sufficiently clear :-/ I have already implemented the necessary changes in the android app in my master branch (I will make another PR soon when I'll have tackled issue 2), so when the typescript web app will be updated, it should work.

Also, the proposed change above should be retrocompatible with older versions of Android anyway, and even current apps, because the grantFilePermissionCallback() was already implemented in onActivityResult(). I just also implemented it for the new file permission handling via SimpleStorage.