Skip to content
This repository was archived by the owner on Mar 15, 2026. It is now read-only.

Treat last_modified and mime_type columns from DocumentProvider as nullable#100

Open
saket wants to merge 1 commit intogoogle:mainfrom
saket:saket/jan4/nullable-columns
Open

Treat last_modified and mime_type columns from DocumentProvider as nullable#100
saket wants to merge 1 commit intogoogle:mainfrom
saket:saket/jan4/nullable-columns

Conversation

@saket
Copy link
Copy Markdown
Contributor

@saket saket commented Jan 4, 2023 •
edited
Loading

OpenableColumns makes it clear that a DocumentProvider is only required to implement two columns: OpenableColumns.DISPLAY_NAME and OpenableColumns.SIZE. All other columns are optional and can cause modernstorage to crash because it expects last_modified and mime_type to be always present.

Example 1: Files by Google

FATAL EXCEPTION: main
    Process: me.saket.teleport.debug, PID: 9944
    java.lang.IllegalArgumentException: Invalid column last_modified
        at android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:172)
        at android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:142)
        at android.content.ContentProviderProxy.query(ContentProviderNative.java:481)
        at android.content.ContentResolver.query(ContentResolver.java:1219)
        at android.content.ContentResolver.query(ContentResolver.java:1151)
        at android.content.ContentResolver.query(ContentResolver.java:1107)
        at com.google.modernstorage.storage.AndroidFileSystem.fetchMetadataFromDocumentProvider(AndroidFileSystem.kt:302)
        at com.google.modernstorage.storage.AndroidFileSystem.metadataOrNull(AndroidFileSystem.kt:185)

Example 2: Cx file explorer

FATAL EXCEPTION: main
    Process: me.saket.teleport.debug, PID: 9439
    java.lang.IllegalStateException: Couldn't read row 0, col 3 from CursorWindow.  Make sure the Cursor is initialized correctly before accessing data from it.
        at android.database.CursorWindow.nativeGetLong(Native Method)
        at android.database.CursorWindow.getLong(CursorWindow.java:539)
        at android.database.AbstractWindowedCursor.getLong(AbstractWindowedCursor.java:78)
        at android.database.CursorWrapper.getLong(CursorWrapper.java:131)
        at com.google.modernstorage.storage.AndroidFileSystem.fetchMetadataFromDocumentProvider(AndroidFileSystem.kt:327)
        at com.google.modernstorage.storage.AndroidFileSystem.metadataOrNull(AndroidFileSystem.kt:185)

This PR treats those two columns as nullable. Thoughts?

saket
saket commented Jan 4, 2023
View reviewed changes
Comment thread storage/src/main/java/com/google/modernstorage/storage/AndroidFileSystem.kt
// These two columns are optional and may not be implemented by the providing app.
val lastModifiedTime = cursor.getLongOrNull(cursor.getColumnIndex(DocumentsContract.Document.COLUMN_LAST_MODIFIED))
val mimeType = cursor.getStringOrNull(cursor.getColumnIndex(DocumentsContract.Document.COLUMN_MIME_TYPE))
?: contentResolver.getType(uri)
Copy link
Copy Markdown
Contributor Author

@saket saket Jan 4, 2023

Choose a reason for hiding this comment

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

I noticed ContentResolver#getType(Uri) is a nice fallback and works in most cases.

saket
saket commented Jan 4, 2023
View reviewed changes
Comment thread storage/src/main/java/com/google/modernstorage/storage/AndroidFileSystem.kt
// DocumentsContract.Document.COLUMN_SIZE
val size = cursor.getLong(3)
val displayName = cursor.getString(cursor.getColumnIndexOrThrow(OpenableColumns.DISPLAY_NAME))
val size = cursor.getLong(cursor.getColumnIndexOrThrow(OpenableColumns.SIZE))
Copy link
Copy Markdown
Contributor Author

@saket saket Jan 4, 2023

Choose a reason for hiding this comment

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

I replaced the two DocumentsContract.Document.* column names with OpenableColumns.* because the class-doc of OpenableColumns makes it clear that document providers are only required to implement these two columns.

I can undo this if you want because they do look slightly inconsistent with DocumentsContract.Document.COLUMN_LAST_MODIFIED and DocumentsContract.Document.COLUMN_MIME_TYPE.

saket
saket commented Jan 4, 2023
View reviewed changes
Comment thread storage/src/main/java/com/google/modernstorage/storage/AndroidFileSystem.kt
put(Uri::class, uri)
put(MetadataExtras.DisplayName::class, MetadataExtras.DisplayName(displayName))
if (mimeType != null) {
put(MetadataExtras.MimeType::class, MetadataExtras.MimeType(mimeType))
Copy link
Copy Markdown
Contributor Author

@saket saket Jan 4, 2023

Choose a reason for hiding this comment

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

This will be a breaking change for apps that always expect a mime type to be present, but maybe that's okay considering you've only done alpha releases so far?

@saket saket mentioned this pull request Sep 29, 2023
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@saket