Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Requirements for File Manager compatibility #531

Open
x0b opened this issue Apr 20, 2020 · 6 comments
Open

Requirements for File Manager compatibility #531

x0b opened this issue Apr 20, 2020 · 6 comments

Comments

@x0b
Copy link

x0b commented Apr 20, 2020

Thank you for adding RCX to the wiki page.

I was wondering what the requirements are for full compatibility, since it is currently just "Partially. Works from the application itself". Does KeePassDX act as SAF client or provider? If it is the second one, can it deal with pipe or socket file descriptors?

EDIT: It does seem to require a file backed descriptor, is that correct?

@J-Jamet J-Jamet changed the title Requirements for Compatibility Requirements for File Manager compatibility Apr 22, 2020
@J-Jamet
Copy link
Member

J-Jamet commented Apr 22, 2020

KeePassDX uses SAF. To be fully compatible, you must manage the intent ACTION_OPEN_DOCUMENT and ACTION_CREATE_DOCUMENT to retrieve an URI with persist link.

EDIT: It does seem to require a file backed descriptor, is that correct?

No, the code you highlight is only there to retrieve data from the stream by the content resolver.

@x0b
Copy link
Author

x0b commented Apr 22, 2020

Thanks. I wasn't sure with the highlighted section, but since you are reading only 10 bytes anyway before reset it should work within any reasonable default buffer size. It actually works now, there was just a different bug.

A final question: I'm seeing three meta queries for the file whenever I open KeePassDX. Since they are so close together they will at most cause a single network call (any reasonable provider will cache this), but as you can see from the timestamps, it still incurs a ~7-9ms call time for each query. Is there a reason for this design?

// Meta Query 1
20:52:41.956 Provider: queryDocument(): rclone/remotes/Dropbox:/keepass.kdbx
20:52:41.958 Provider: retrieving node: Dropbox:/keepass.kdbx
20:52:41.959 Provider: getFileItem: using cached for Dropbox:/keepass.kdbx
20:52:41.965 Provider: queryDocument(rclone/remotes/Dropbox:/keepass.kdbx): keepass.kdbx, application/x-keepass, keepass.kdbx, 1587579790760

// Meta Query 2
20:52:42.072 Provider: queryDocument(): rclone/remotes/Dropbox:/keepass.kdbx
20:52:42.073 Provider: retrieving node: Dropbox:/keepass.kdbx
20:52:42.075 Provider: getFileItem: using cached for Dropbox:/keepass.kdbx
20:52:42.079 Provider: queryDocument(rclone/remotes/Dropbox:/keepass.kdbx): keepass.kdbx, application/x-keepass, keepass.kdbx, 1587579790760

// Meta Query 3
20:52:42.094 Provider: queryDocument(): rclone/remotes/Dropbox:/keepass.kdbx
20:52:42.096 Provider: retrieving node: Dropbox:/keepass.kdbx
20:52:42.098 Provider: getFileItem: using cached for Dropbox:/keepass.kdbx
20:52:42.102 Provider: queryDocument(rclone/remotes/Dropbox:/keepass.kdbx): keepass.kdbx, application/x-keepass, keepass.kdbx, 1587579790760

// User enters password, Open query
20:52:46.126 Provider: openDocument: rclone/remotes/Dropbox:/keepass.kdbx, mode=r
20:52:46.129 Provider: retrieving node: Dropbox:/keepass.kdbx
20:52:46.131 Provider: getFileItem: using cached for Dropbox:/keepass.kdbx
20:52:46.141 Provider: Running Pipe Transfer...
20:52:49.335 Provider: Stopping Pipe Transfer, cancelled=false

I think it is caused by the DocumentFile abstraction being called here - .exists (1), getModificationString() (2) and getSizeString() (3).

It would probably be more efficient to issue a single call and reuse the result. Since this is executed in a view method, the calls in the log might have dropped about 8-9 frames.

@J-Jamet
Copy link
Member

J-Jamet commented Apr 22, 2020

Thanks. I wasn't sure with the highlighted section, but since you are reading only 10 bytes anyway before reset it should work within any reasonable default buffer size. It actually works now, there was just a different bug.

I may have misunderstood the question, I just do not know the internal workings of file managers, I just use the mark and the reset of the URI stream to be able to read the header of the file and choose the type of database to build.

A final question: I'm seeing three meta queries for the file whenever I open KeePassDX. Since they are so close together they will at most cause a single network call (any reasonable provider will cache this), but as you can see from the timestamps, it still incurs a ~7-9ms call time for each query. Is there a reason for this design?

I'm using the DocumentFile methods to be able to retrieve the metadata to display. What I can do is a background thread to get this metadata in the recyclerview.

It would probably be more efficient to issue a single call and reuse the result. Since this is executed in a view method, the calls in the log might have dropped about 8-9 frames.

If you can do that, it would be great indeed.

I was wondering what the requirements are for full compatibility

To answer your first question, I have not tested RCX in depth, but I noticed that requesting the URI with the "Open existing database" button from KeePassDX did not display your application to open a document. Maybe because of your DocumentsProvider.

@x0b
Copy link
Author

x0b commented Apr 23, 2020

If you can do that, it would be great indeed.

It is nothing I can do on my end - the SAF client, i.e. KeePassDX, would need to be changed. In my experience, DocumentFile is a terrible abstraction, since everyone - even you with your FileDatabaseInfo class - has to roll their own wrapper anyway. That being said, I didn't actually notice the frame drops visually, so this might not be a problem worth spending time on.

If you want though, you could introduce a method like queryMetadata() into your FileDatabaseInfo

ContentResolver contentResolver = context.getContentResolver();
try (Cursor cursor = contentResolver.query(docUri, new String[]{
       // Or whatever columns/properties are needed
        DocumentsContract.Document.COLUMN_DOCUMENT_ID,
        DocumentsContract.Document.COLUMN_DISPLAY_NAME,
        DocumentsContract.Document.COLUMN_MIME_TYPE,
        DocumentsContract.Document.COLUMN_LAST_MODIFIED,
        DocumentsContract.Document.COLUMN_SIZE
}, null, null, null)) {
   if (null == cursor) {
      // file does not exist
   }
   long modified = cursor.getLong(3);
   long size = cursor.getLong(4);
}

Obviously, a lot more low-level, and if no one (other than me) has said something about this I would not change it.

To answer your first question, I have not tested RCX in depth, but I noticed that requesting the URI with the "Open existing database" button from KeePassDX did not display your application to open a document. Maybe because of your DocumentsProvider.

Yes, exactly, there are some things in the currently released version that prevent it from working properly, but thanks to your information this will work in the future.

@J-Jamet
Copy link
Member

J-Jamet commented Apr 23, 2020

Thank you for the metadata code, I will use it because, as you say, it is more optimized, even if there is no issue for the moment. I didn't have in mind that I could do it this way. I will keep the current code in the catch if there is an error.

@J-Jamet
Copy link
Member

J-Jamet commented Nov 4, 2023

Just checked and the DocumentFile with fromSingleUri call SingleDocumentFile class that call DocumentsContractAPI19, it's exactly the same code as #531 (comment) so it's not more optimized.

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

No branches or pull requests

2 participants