Closed jcgruenhage closed 4 years ago
Hi @jcgruenhage and thanks for reporting this issue/idea! :) What would be the benefit of implementing this (just curious since I also haven't digged into the Storage Access Framework, so I have no idea what this could then be used for).
At the moment we are very busy rewriting the instant upload mechanism to make it work again for Android 7 devices (and some OnePlus devices running the latest OS).
No worries about Nextcloud App internal knowledge. If you want to work on this task (even when we are talking mid/end December) @tobiasKaminsky and I are here to help :) The original implementation of the document provider has been done by @przybylski.
The benefit of the Subtree selection is that it allows apps to get access to a folder instead of a file only, so that the app has access to all files and subfolder of a folder, and not only to that single file.
Imagine a music player where you have to explicitly open every song through the file picker for the first time, before it is added to the library. Or an audiobook player, where you have an audio file for each chapter, that would take an eternity for "A Song of Ice and Fire", for example.
Is there any app which is used and also makes use of such DOCUMENT_TREE feature?
See edit below. https://play.google.com/store/apps/details?id=com.kodarkooperativet.blackplayerfree
This for example is a music player, which supports selecting a different "SD-Card", which essentially is implemented by the subtree selection. That way I could play music from nextcloud directly, with a few seconds delay as the song is loading at the beginning. Alternatively I would need to download everything in advance instead. And I remember one or two times, where an app asked for storage permissions through this, I don't remember what they were though..
Edit: I was wrong, blackplayer only uses this to geht write access to the sdcard..
I have to admit that even Google Drive does not support this though, and no other cloud Provider that I have tried, which I can't understand, since the API has been around since Lollipop.
Since still has the "need info" label, what information do you need?
I think I fixed it (way less work than I thought, few minutes), but I can't test it right now because somehow webdav won't work, just so no one else has to take a look into this.
Hi @jcgruenhage, my mistake. No open questions regarding the feature (as in to be clarified requirements). Just unclear how this can be implemented (needs research) but if I understand you correctly then you have solved it (locally).
Yes, I (think I) have solved it, just need to get my server fixed before I can test it, should anyone be interested sooner, I could create a fork and upload the untested code, but I'd rather test it first :D
I haven't finished testing it yet (the application I use to test it is based on the JobScheduler, the background process exporting the files can take while to start), but Nextcloud now shows up as DocumentsProvider when selecting a folder.
Edit: The main problem is fixed, but there is an underlying problem, the DocumentsProvider does not support creation of folders and files at the moment.
W/DocumentsContract: Failed to create document
java.lang.UnsupportedOperationException: Create not supported
at android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:167)
at android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:135)
at android.content.ContentProviderProxy.call(ContentProviderNative.java:646)
at android.content.ContentProviderClient.call(ContentProviderClient.java:456)
at android.provider.DocumentsContract.createDocument(DocumentsContract.java:1082)
at android.provider.DocumentsContract.createDocument(DocumentsContract.java:1065)
at android.support.v4.provider.DocumentsContractApi21.createFile(DocumentsContractApi21.java:33)
at android.support.v4.provider.TreeDocumentFile.createFile(TreeDocumentFile.java:34)
Looking through the implementation of the DocumentsProvider, it seems as though it is read only in general. @przybylski can you confirm this? (Edit: Nevermind, found that in the PR, it is read only)
in case anyone is interested in the progress till now: https://gist.github.com/jcgruenhage/7de5065b5a9774e9fadb3670a953f669
@jcgruenhage would you like to have write access to the repo or fork+PR? Anything related to JobScheduler @tobiasKaminsky is our expert :)
@AndyScherzinger i'd prefer fork+PR, this way I can break less :D
Anything related to uploading/updating files should be done through the FileUploader, right? My idea would be to copy the files in the private app directory and then tell the FileUploader to do it's thing.
What do you think should be done with failed uploads? Just retry until it works? And what about uploads failed due to things like a fully used quota or something like that? The SAF does not provide a way to inform apps that saving a file did not work the way they intended it to. One way could be to move the file to a publicly readable directory and tell the user about it. Does this way sound right to you (@all)?
@jcgruenhage fork+PR sounds great, github lately added the ability to give others write access to a PR, so you could even add @tobiasKaminsky and me for any help/collaboration
@tobiasKaminsky can you answer @jcgruenhage's questions, since I am not to familiar with the upload mechanisms and how the different scenarios should be handled. As for the retry/error issue in general, if the files are located where the app can access them, then having the upload entry with the failure reason sound fine, since the user can then restart the upload via the Nextcloud app's upload view.
I just read a bit more about this, if I understand this correctly, then these steps are still needed:
openDocument(..)
, this requires:
createDocument(..)
, this requires:
FLAG_SUPPORTS_CREATE
to the rootsFLAG_DIR_SUPPORTS_CREATE
to each directoryThey are all implemented now, they just don't work as intended yet.
This has nothing to do with this issue, but are there any plans of updating gradle, build tools, compileSdk and libraries? Maybe switching to JACK?
This has nothing to do with this issue, but are there any plans of updating gradle, build tools, compileSdk and libraries?
Those will change if it is necessary.
Maybe switching to JACK?
Not planned currently.
Okay, thanks for the info ^^
Those will change if it is necessary.
Which means yes, but there are some dependencies regarding Continuous Integration which block compile and lib updates at the moment.
Ok, since I wrote DocumentProvider in nextcloud and owncloud I feel like I can answer this question.
If you want to implement openDocument() you need to return a ParcelFileDescriptor
, which isn't that hard to do, you just download (or update) file an give it to the user.
But the problem occurs when user ends file editing. To the best of my knowledge, app is not notified when file is modified or descriptor is close (technically there is a close
method, but it its called from the different context). So client will not know when to update the content by uploading to the server. We could listen to the files system changes but thats an overkill for OS, battery and privacy, and requires user permission (at least first time).
Moreover if app gets killed by OS or user we won't know when, and if, the file is updated.
Also descriptor can be duplicated or detached from descriptor, then we totally loose control on the parcel.
When I was implementing provider I thought of some potential solutions but none of those were perfect. But I can revisit and check what I can come up with this time :)
And about ACTION_OPEN_DOCUMENT_TREE
i don't remember exactly why I haven't implemented it. It seems easy.
Quote from the blog post linked above:
Note: if you’re syncing the file elsewhere and need to know when the file was closed, consider using the open() call that takes an OnCloseListener to know exactly when the other app is done writing to the file — be sure to check the IOException to know if the remote side actually succeeded or ran into an error.
This listener is what we need, and you have to specify a handler from which the listener is called, so we can specify the context (using the handler) too.
The point where I am stuck at the moment is how to create new files, and sync old ones that were changed. I think I need to call the UploadRequester, but these methods require two additional parameters, an Account and a createdBy int, which I don't know where to get from.
those two parameters are easy to put. Account can be set while constructing and set createdBy
to UploadFileOperation.CREATED_BY_USER
. But you have to be extra careful when you will extend ParcelFileDescriptor
to allow duplicating descriptor and pass all data to it, including handler, account, and createdBy
(which is a bad name btw ;) )
@przybylski My problem is not how to put them into the new upload, my problem is where to get them from. I found that the storage manager has a reference to the account I can use, but where do I get the createdBy
int from? And what does it do? I can not find any documentation about that.
This is the method I am trying to use, from com.owncloud.android.files.services.FileUploader.UploadRequester
.
By the way, is there a reason, that the methods of this static inner class, that has not attributes at all, and whose methods have no dependency whatsoever, are not static?
/**
* Call to upload a new single file
*/
public void uploadNewFile(Context context, Account account, String localPath, String remotePath, int
behaviour, String mimeType, boolean createRemoteFile, int createdBy)
Is using this to create new files on the server even the right way? I can not find any documentation how to do this, and searching in the source code and using what I find, could be the right thing, or I could end up using some class that is not intended to be used directly or something..
About the ParcelFileDescriptor, I do not plan to extend it, I will just use the other open()
method. Until now, the used method was this: ParcelFileDescriptor open (File file, int mode)
, which I plan to replace by this: ParcelFileDescriptor open (File file, int mode, Handler handler, ParcelFileDescriptor.OnCloseListener listener)
.
What is the createdBy
int in com.owncloud.android.files.services.FileUploader.UploadRequester.uploadNewFile()
I have set that to 0 for now.
I have committed what I have done until now to my repo: https://github.com/jcgruenhage/nextcloud-android/commit/41bd045fcce23397b525052cfa00639201b2b1ab.
I tried uploading something, and it got stuck in a loop while apparently trying to download a folder:
01-22 16:15:34.892 1644-1746/com.nextcloud.client D/OwnCloudClient #7: REQUEST GET /remote.php/webdav/Media/Music/
01-22 16:15:35.388 1644-1746/com.nextcloud.client E/DownloadRemoteFileOperation: Could not read modification time from response downloading /Media/Music/
01-22 16:15:35.397 1644-1746/com.nextcloud.client E/DownloadRemoteFileOperation: Could not read eTag from response downloading /Media/Music/
01-22 16:15:35.404 1644-1746/com.nextcloud.client I/DownloadRemoteFileOperation: Download of /Media/Music/ to /storage/emulated/0/nextcloud/tmp/mcgruenhage@nextcloud.jcg.re/Media/Music/: Operation finished with HTTP status code 200 (success)
01-22 16:15:35.412 1644-1746/com.nextcloud.client I/DownloadFileOperation: Download of /Media/Music/ to /storage/emulated/0/nextcloud/mcgruenhage@nextcloud.jcg.re/Media/Music/: Operation finished with HTTP status code 200 (success)
01-22 16:15:35.495 1644-1746/com.nextcloud.client D/FileDataStorageManager: Number of files updated with CONFLICT: 1
01-22 16:15:35.498 1644-1746/com.nextcloud.client D/FileDataStorageManager: checking parents to remove conflict; STARTING with /Media/
01-22 16:15:35.518 1644-1746/com.nextcloud.client D/FileDataStorageManager: NO MORE conflicts in /Media/
01-22 16:15:35.551 1644-1746/com.nextcloud.client D/FileDataStorageManager: checking parents to remove conflict; NEXT /
01-22 16:15:35.567 1644-1746/com.nextcloud.client D/FileDataStorageManager: NO MORE conflicts in /
01-22 16:15:35.597 1644-1746/com.nextcloud.client D/FileDataStorageManager: checking parents to remove conflict; NEXT
01-22 16:15:35.605 1644-1746/com.nextcloud.client D/FileDownloader: Stopping after command with id 1
01-22 16:15:35.606 1644-1644/com.nextcloud.client D/FileObserverService: Received broadcast intent Intent { act=com.owncloud.android.files.services.FileDownloaderDOWNLOAD_FINISH flg=0x10 (has extras) }
01-22 16:15:35.608 1644-1644/com.nextcloud.client D/FileObserverService: No observer for path /storage/emulated/0/nextcloud/mcgruenhage@nextcloud.jcg.re/Media/Music
01-22 16:15:35.845 1644-1659/com.nextcloud.client I/Nextcloud DocProvider: waiting...
01-22 16:15:36.849 1644-1659/com.nextcloud.client I/Nextcloud DocProvider: waiting...
01-22 16:15:37.854 1644-1659/com.nextcloud.client I/Nextcloud DocProvider: waiting...
01-22 16:15:38.872 1644-1659/com.nextcloud.client I/Nextcloud DocProvider: waiting...
Checking the path mentioned in the log, there is a text file, containing this:
This is the WebDAV interface. It can only be accessed by WebDAV clients such as the ownCloud desktop sync client.
Without downloading the folder, getting it's local storage path returns null, which then prevents me from creating a subdirectory.
~Also, I am not sure how to create directories remotely, creating a directory locally and then uploading it does not make very much sense, since it is not really a file.~ I have found a way to create folders remotely, I still need to make them available locally somehow I think.
Any hints where I should look?
Sorry for spamming this thread, I promise this is the last post for today :D Creating remote folders works now, but it throws a null pointer exception. Shouldn't this work?
if (mimeType.equals("vnd.android.document/directory")) {
String folderPath = parent.getRemotePath()
+ OCFile.PATH_SEPARATOR
+ displayName;
folderPath = folderPath.replace("//", "/");
new CreateFolderOperation(folderPath, true)
.execute(mCurrentStorageManager, getContext());
return "" + mCurrentStorageManager.getFileByPath(folderPath).getFileId();
}
01-22 20:52:50.939 3888-5178/com.nextcloud.client I/DocProvider: try to create a file of the mime type: vnd.android.document/directory
01-22 20:52:50.971 3888-5178/com.nextcloud.client D/SimpleFactoryManager: getClientFor(OwnCloudAccount ... :
01-22 20:52:50.971 3888-5178/com.nextcloud.client D/OwnCloudClient #19: Creating OwnCloudClient
01-22 20:52:50.972 3888-5178/com.nextcloud.client V/SimpleFactoryManager: new client {mcgruenhage@nextcloud.jcg.re, 113946029}
01-22 20:52:50.978 3888-5178/com.nextcloud.client D/FileUtils: path ....... /Media/Music/Oasis
01-22 20:52:50.979 3888-5178/com.nextcloud.client D/OwnCloudClient #19: REQUEST MKCOL /remote.php/webdav/Media/Music/Oasis
01-22 20:52:50.982 3888-5178/com.nextcloud.client D/AdvancedSslSocketFactory: Creating SSL Socket with remote nextcloud.jcg.re:443, local null:0, params: org.apache.commons.httpclient.params.HttpConnectionParams@a23a27e
01-22 20:52:50.982 3888-5178/com.nextcloud.client D/AdvancedSslSocketFactory: ... with connection timeout 5000 and socket timeout 30000
01-22 20:52:51.000 3888-5178/com.nextcloud.client I/ServerNameIndicator: SSLSocket implementation: com.android.org.conscrypt.OpenSSLSocketImpl
01-22 20:52:51.001 3888-5178/com.nextcloud.client I/ServerNameIndicator: SNI done, hostname: nextcloud.jcg.re
01-22 20:52:51.289 3888-5178/com.nextcloud.client D/CreateRemoteFolderOperation: Create directory /Media/Music/Oasis: Operation finished with HTTP status code 201 (success)
01-22 20:52:51.424 3888-5178/com.nextcloud.client D/CreateFolderOperation: Create directory /Media/Music/Oasis in Database
01-22 20:52:51.592 3888-4986/com.nextcloud.client I/DocProvider: try to create a file of the mime type: vnd.android.document/directory
01-22 20:52:51.619 3888-4986/com.nextcloud.client D/SimpleFactoryManager: getClientFor(OwnCloudAccount ... :
01-22 20:52:51.619 3888-4986/com.nextcloud.client D/OwnCloudClient #20: Creating OwnCloudClient
01-22 20:52:51.619 3888-4986/com.nextcloud.client V/SimpleFactoryManager: new client {mcgruenhage@nextcloud.jcg.re, 156933364}
01-22 20:52:51.623 3888-4986/com.nextcloud.client D/FileUtils: path ....... /Media/Music/Oasis/(What's The Story) Morning Glory
01-22 20:52:51.623 3888-4986/com.nextcloud.client D/OwnCloudClient #20: REQUEST MKCOL /remote.php/webdav/Media/Music/Oasis/(What's%20The%20Story)%20Morning%20Glory
01-22 20:52:51.846 3888-4986/com.nextcloud.client D/CreateRemoteFolderOperation: Create directory /Media/Music/Oasis/(What's The Story) Morning Glory: Operation finished with HTTP status code 201 (success)
01-22 20:52:51.937 3888-4986/com.nextcloud.client D/CreateFolderOperation: Create directory /Media/ in Database
01-22 20:52:52.008 3888-4986/com.nextcloud.client D/CreateFolderOperation: Create directory /Media/Music/ in Database
01-22 20:52:52.094 3888-4986/com.nextcloud.client D/CreateFolderOperation: Create directory /Media/Music/Oasis/ in Database
01-22 20:52:52.170 3888-4986/com.nextcloud.client D/CreateFolderOperation: Create directory /Media/Music/Oasis/(What's The Story) Morning Glory/ in Database
01-22 20:52:52.185 3888-4986/com.nextcloud.client E/DatabaseUtils: Writing exception to parcel
java.lang.NullPointerException: Attempt to invoke virtual method 'long com.owncloud.android.datamodel.OCFile.getFileId()' on a null object reference
at org.nextcloud.providers.DocumentsStorageProvider.createDocument(DocumentsStorageProvider.java:181)
at android.provider.DocumentsProvider.callUnchecked(DocumentsProvider.java:757)
at android.provider.DocumentsProvider.call(DocumentsProvider.java:716)
at android.content.ContentProvider$Transport.call(ContentProvider.java:400)
at android.content.ContentProviderNative.onTransact(ContentProviderNative.java:283)
at android.os.Binder.execTransact(Binder.java:565)
For the full source code, look here: https://github.com/jcgruenhage/nextcloud-android/commit/aadcbc290b8b629e86312d0988e81860724282ac
@przybylski My problem is not how to put them into the new upload, my problem is where to get them from. I found that the storage manager has a reference to the account I can use, but where do I get the createdBy int from? And what does it do? I can not find any documentation about that.
createdBy
is just an indicator for internal use to where from the intent was send, since user is modifying the file it is clearly from the user so please use UploadFileOperation.CREATED_BY_USER
constant.
Without downloading the folder, getting it's local storage path returns null, which then prevents me from creating a subdirectory.
you should not download the entire directory, instead create a virtual structure which mimics the internally stored database and give it to the user. Download files if needed.
Creating remote folders works now, but it throws a null pointer exception. Shouldn't this work?
it depends, is the path valid? if it is not then storage manager will return null, thats why you are getting NPE. Make sure that storage manager is given a correct path.
createdBy is just an indicator for internal use to where from the intent was send, since user is modifying the file it is clearly from the user so please use UploadFileOperation.CREATED_BY_USER constant.
Ahh I thought createdBy is about which user created the file, in case there are multiple accounts, thanks :D Now that makes sense.
you should not download the entire directory, instead create a virtual structure which mimics the internally stored database and give it to the user. Download files if needed.
I meant to "download" the folder itself, not all elements contained in there, or to more specifically, have that the OCFile representing the folder, does not return null when getStoragePath()
is called, but the path to a usable folder on the local storage.
it depends, is the path valid? if it is not then storage manager will return null, thats why you are getting NPE. Make sure that storage manager is given a correct path.
I would say, that the path that I just created, and that appears online is valid, yes.
I first create the path synchronously using this:
new CreateFolderOperation(folderPath, true).execute(mCurrentStorageManager, getContext());
and then I try to access it using this, which throws the NPE:
mCurrentStorageManager.getFileByPath(folderPath).getFileId();
What am I doing wrong there, shouldn't it find that folder and it's ID?
Let's move this discussion to IRC, i am using nickname aqu or przybylski on #nexcloud-mobile channel.
I am on freenode, channel #nextcloud-mobile
@AndyScherzinger If I may, I would like to express support for this feature... As @jcgruenhage mentioned there are many places where subtree selection would be useful. An example is an app giving the user to select a backup/export location and the app can continuously backup/export there. This is especially useful when several backups need to be kept rather than just the most recent one. The user can then always view the files in desktop version of file sync service.
The GnuCash Android app (which I maintain) could really use the ACTION_OPEN_DOCUMENT_TREE intent. But unfortunately, I haven't found a sync app which supports it - not even Google Drive :(
I am/was working on that, but I have had some problems and no time to work on it, but if there are other people than me, I might start working on it again now ^^
There are a few other PRs on other project that I need to finish first though, mainly because they are nearly finished :D
Is this going somewhere? It's required for proper Orgzly integration https://github.com/orgzly/orgzly-android/issues/25#issuecomment-383872854, unless Orgzly implements sync with Nextcloud itself via WebDAV or similar… which would be kind of sad.
I know it is not a satisfying answer, but currently we lack on time and therefore this is postponed. I hope I can find some time in Decemeber to test this.
How far along is the support for this? With the latest version from the Play Store, I can select a folder in Nextcloud with the ACTION_OPEN_DOCUMENT_TREE
intent, but calling DocumentFile.createFile
on the tree URI returns null.
It works for me with version 3.10. If you can select Nextcloud in ACTION_OPEN_DOCUMENT_TREE
basic support is there. Maybe you are creating the DocumentFile
wrong?
@grote Which app are you testing Nextcloud's implementation of ACTION_OPEN_DOCUMENT_TREE
with? If I can get it to work with that as well, then there's definitely something wrong with my code.
@alexbakker I am currently facing the same problem, did you guys manage to get this to work somehow?
@flocke No, the issue is still present unfortunately. When using ACTION_OPEN_DOCUMENT_TREE to select a location on the device's storage, everything works fine, but when selecting a location on Nextcloud it fails. I haven't taken the time yet to debug this on the Nextcloud end.
I debugged a little bit more and I get a java.io.FileNotFoundException: Failed to upload document with path ...
error in my logcat when trying to create a file in the document tree: https://paste.bka.li/view/raw/95ee2856
The java.io.FileNotFoundException
seems to be a little misleading. I attached a debugger and this is the actual exception that occurs:
android.os.NetworkOnMainThreadException
at android.os.StrictMode$AndroidBlockGuardPolicy.onNetwork(StrictMode.java:1565)
at java.net.Inet6AddressImpl.lookupHostByName(Inet6AddressImpl.java:115)
at java.net.Inet6AddressImpl.lookupAllHostAddr(Inet6AddressImpl.java:103)
at java.net.InetAddress.getByName(InetAddress.java:1106)
at com.owncloud.android.lib.common.network.AdvancedSslSocketFactory.getInetAddressForHost(AdvancedSslSocketFactory.java:192)
at com.owncloud.android.lib.common.network.AdvancedSslSocketFactory.createSocket(AdvancedSslSocketFactory.java:182)
at org.apache.commons.httpclient.HttpConnection.open(HttpConnection.java:707)
at org.apache.commons.httpclient.MultiThreadedHttpConnectionManager$HttpConnectionAdapter.open(MultiThreadedHttpConnectionManager.java:1361)
at org.apache.commons.httpclient.HttpMethodDirector.executeWithRetry(HttpMethodDirector.java:387)
at org.apache.commons.httpclient.HttpMethodDirector.executeMethod(HttpMethodDirector.java:171)
at org.apache.commons.httpclient.HttpClient.executeMethod(HttpClient.java:397)
at org.apache.commons.httpclient.HttpClient.executeMethod(HttpClient.java:323)
at com.owncloud.android.lib.common.OwnCloudClient.executeMethod(OwnCloudClient.java:219)
at com.owncloud.android.lib.resources.files.UploadFileRemoteOperation.uploadFile(UploadFileRemoteOperation.java:180)
at com.owncloud.android.lib.resources.files.UploadFileRemoteOperation.run(UploadFileRemoteOperation.java:135)
at com.owncloud.android.lib.common.operations.RemoteOperation.execute(RemoteOperation.java:184)
at com.owncloud.android.providers.DocumentsStorageProvider.createFile(DocumentsStorageProvider.java:539)
at com.owncloud.android.providers.DocumentsStorageProvider.createDocument(DocumentsStorageProvider.java:460)
at android.provider.DocumentsProvider.callUnchecked(DocumentsProvider.java:1121)
at android.provider.DocumentsProvider.call(DocumentsProvider.java:1067)
at android.content.ContentProvider.call(ContentProvider.java:2152)
at android.content.ContentProvider$Transport.call(ContentProvider.java:477)
at android.content.ContentProviderNative.onTransact(ContentProviderNative.java:277)
at android.os.Binder.execTransactInternal(Binder.java:1021)
at android.os.Binder.execTransact(Binder.java:994)
Should I create a new issue for my findings above, or is this feature known to not be ready for prime time yet?
I think this can be closed as it is implemented and supported. I am currently fixing issues around this though.
Still having the same issue with version 3.13.1. Is this being worked on?
What exactly is the "same" issue? Please open a new issue and fill out template.
Actual behaviour
In an App requesting access to a directory via the Storage Access Framework and Intent.ACTION_OPEN_DOCUMENT_TREE, the Nextcloud DocumentsProvider is not shown, since it does not support subtree selection.
Desired behaviour
The DocumentsProvider should be shown, and you should be able to select Nextcloud directories.
Android Dev Reference:
If you're implementing a DocumentsProvider and want to support subtree selection, implement isChildDocument() and include FLAG_SUPPORTS_IS_CHILD in your COLUMN_FLAGS.
I could maybe implement that, but it would need a lot of time (which I don't have until around christmas probably), since I know nearly nothing about the Storage Access Framework and how it works, and completely nothing about the internals of the Nextcloud App, so someone else would probably be able to do this faster (and better).