From 1284892234abebd4aa7184d9a82bc8c96c4246ba Mon Sep 17 00:00:00 2001 From: PeterPablo <4340267+PeterPablo@users.noreply.github.com> Date: Mon, 9 Nov 2026 21:58:11 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20one=20row=20per=20file=20=E2=80=94=20uni?= =?UTF-8?q?que=20(path,=20file=5Fowner)=20index=20on=20filelist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `filelist` had no unique index, and the query that feeds the file list (`WHERE parent = ?`) has neither an owner filter nor a DISTINCT. Every write path used an INSERT with some CONFLICT_* clause, which had nothing to conflict with, so a second row for the same path was accepted silently — and then both rows were handed out. A SAF consumer shows that as the same folder twice. Observed with two different SAF clients on the same tree (the file manager Twig and Material Files): duplicate listings in one, a truncated or empty listing in the other, and a trace of the duplicates through the provider cursor, which maps 1:1 onto the rows. A code-side lookup cannot make the invariant hold, and — unlike a code fix — it does not help anybody who already has the duplicates, short of clearing the app data. So make it an invariant of the database: * Migration 106 → 107 collapses the twins (oldest `_id` per path/owner wins, rows without a path are left alone) and then creates the unique index. * The same index is declared on `FileEntity`: Room builds a fresh database from the entity, so the migration alone would leave new installs unprotected. * `upsertSingleFile` now falls back to the stored row when its insert is refused, instead of returning a URI with `-1` in it. Verified on an Android 16 emulator: the migration test seeds three twins, a row of another account and two path-less rows, runs Room's `runMigrationsAndValidate(…, 107, …)` and asserts the outcome; the existing `FileContentProviderTests` (12 tests) and the JVM unit tests (634) stay green. Assisted-by: GitHub Copilot (Copilot SDK in VS Code) Fixes: #17051 Signed-off-by: PeterPablo <4340267+PeterPablo@users.noreply.github.com> --- .../107.json | 1400 +++++++++++++++++ .../migrations/Migration106to107Test.kt | 93 ++ .../client/database/NextcloudDatabase.kt | 3 + .../client/database/entity/FileEntity.kt | 16 +- .../database/migrations/Migration106to107.kt | 40 + .../com/owncloud/android/db/ProviderMeta.java | 2 +- .../providers/FileContentProvider.java | 40 +- 7 files changed, 1584 insertions(+), 10 deletions(-) create mode 100644 app/schemas/com.nextcloud.client.database.NextcloudDatabase/107.json create mode 100644 app/src/androidTest/java/com/nextcloud/client/database/migrations/Migration106to107Test.kt create mode 100644 app/src/main/java/com/nextcloud/client/database/migrations/Migration106to107.kt diff --git a/app/schemas/com.nextcloud.client.database.NextcloudDatabase/107.json b/app/schemas/com.nextcloud.client.database.NextcloudDatabase/107.json new file mode 100644 index 000000000000..fc7dfcc3e412 --- /dev/null +++ b/app/schemas/com.nextcloud.client.database.NextcloudDatabase/107.json @@ -0,0 +1,1400 @@ +{ + "formatVersion": 1, + "database": { + "version": 107, + "identityHash": "4d8f3a13a93f08725aeafa3fe7b7153e", + "entities": [ + { + "tableName": "arbitrary_data", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`_id` INTEGER PRIMARY KEY AUTOINCREMENT, `cloud_id` TEXT, `key` TEXT, `value` TEXT)", + "fields": [ + { + "fieldPath": "id", + "columnName": "_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "cloudId", + "columnName": "cloud_id", + "affinity": "TEXT" + }, + { + "fieldPath": "key", + "columnName": "key", + "affinity": "TEXT" + }, + { + "fieldPath": "value", + "columnName": "value", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "_id" + ] + } + }, + { + "tableName": "capabilities", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`_id` INTEGER PRIMARY KEY AUTOINCREMENT, `assistant` INTEGER, `account` TEXT, `version_mayor` INTEGER, `version_minor` INTEGER, `version_micro` INTEGER, `version_string` TEXT, `version_edition` TEXT, `extended_support` INTEGER, `core_pollinterval` INTEGER, `sharing_api_enabled` INTEGER, `sharing_public_enabled` INTEGER, `sharing_public_password_enforced` INTEGER, `sharing_public_expire_date_enabled` INTEGER, `sharing_public_expire_date_days` INTEGER, `sharing_public_expire_date_enforced` INTEGER, `sharing_public_send_mail` INTEGER, `sharing_public_upload` INTEGER, `sharing_user_send_mail` INTEGER, `sharing_resharing` INTEGER, `sharing_federation_outgoing` INTEGER, `sharing_federation_incoming` INTEGER, `files_bigfilechunking` INTEGER, `files_undelete` INTEGER, `files_versioning` INTEGER, `external_links` INTEGER, `server_name` TEXT, `server_color` TEXT, `server_text_color` TEXT, `server_element_color` TEXT, `server_slogan` TEXT, `server_logo` TEXT, `background_url` TEXT, `end_to_end_encryption` INTEGER, `end_to_end_encryption_keys_exist` INTEGER, `end_to_end_encryption_api_version` TEXT, `activity` INTEGER, `background_default` INTEGER, `background_plain` INTEGER, `richdocument` INTEGER, `richdocument_mimetype_list` TEXT, `richdocument_direct_editing` INTEGER, `richdocument_direct_templates` INTEGER, `richdocument_optional_mimetype_list` TEXT, `sharing_public_ask_for_optional_password` INTEGER, `richdocument_product_name` TEXT, `direct_editing_etag` TEXT, `user_status` INTEGER, `user_status_supports_emoji` INTEGER, `etag` TEXT, `files_locking_version` TEXT, `groupfolders` INTEGER, `drop_account` INTEGER, `security_guard` INTEGER, `governance` INTEGER, `forbidden_filename_characters` TEXT, `forbidden_filenames` TEXT, `forbidden_filename_extensions` TEXT, `forbidden_filename_basenames` TEXT, `files_download_limit` INTEGER, `files_download_limit_default` INTEGER, `recommendation` INTEGER, `notes_folder_path` TEXT, `default_permissions` INTEGER, `user_status_supports_busy` INTEGER, `windows_compatible_filenames` INTEGER, `has_valid_subscription` INTEGER, `client_integration_json` TEXT, `mod_rewrite_working` INTEGER, `chunked_upload_max_size` INTEGER, `sharing_json` TEXT)", + "fields": [ + { + "fieldPath": "id", + "columnName": "_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "assistant", + "columnName": "assistant", + "affinity": "INTEGER" + }, + { + "fieldPath": "accountName", + "columnName": "account", + "affinity": "TEXT" + }, + { + "fieldPath": "versionMajor", + "columnName": "version_mayor", + "affinity": "INTEGER" + }, + { + "fieldPath": "versionMinor", + "columnName": "version_minor", + "affinity": "INTEGER" + }, + { + "fieldPath": "versionMicro", + "columnName": "version_micro", + "affinity": "INTEGER" + }, + { + "fieldPath": "versionString", + "columnName": "version_string", + "affinity": "TEXT" + }, + { + "fieldPath": "versionEditor", + "columnName": "version_edition", + "affinity": "TEXT" + }, + { + "fieldPath": "extendedSupport", + "columnName": "extended_support", + "affinity": "INTEGER" + }, + { + "fieldPath": "corePollinterval", + "columnName": "core_pollinterval", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingApiEnabled", + "columnName": "sharing_api_enabled", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingPublicEnabled", + "columnName": "sharing_public_enabled", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingPublicPasswordEnforced", + "columnName": "sharing_public_password_enforced", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingPublicExpireDateEnabled", + "columnName": "sharing_public_expire_date_enabled", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingPublicExpireDateDays", + "columnName": "sharing_public_expire_date_days", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingPublicExpireDateEnforced", + "columnName": "sharing_public_expire_date_enforced", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingPublicSendMail", + "columnName": "sharing_public_send_mail", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingPublicUpload", + "columnName": "sharing_public_upload", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingUserSendMail", + "columnName": "sharing_user_send_mail", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingResharing", + "columnName": "sharing_resharing", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingFederationOutgoing", + "columnName": "sharing_federation_outgoing", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingFederationIncoming", + "columnName": "sharing_federation_incoming", + "affinity": "INTEGER" + }, + { + "fieldPath": "filesBigfilechunking", + "columnName": "files_bigfilechunking", + "affinity": "INTEGER" + }, + { + "fieldPath": "filesUndelete", + "columnName": "files_undelete", + "affinity": "INTEGER" + }, + { + "fieldPath": "filesVersioning", + "columnName": "files_versioning", + "affinity": "INTEGER" + }, + { + "fieldPath": "externalLinks", + "columnName": "external_links", + "affinity": "INTEGER" + }, + { + "fieldPath": "serverName", + "columnName": "server_name", + "affinity": "TEXT" + }, + { + "fieldPath": "serverColor", + "columnName": "server_color", + "affinity": "TEXT" + }, + { + "fieldPath": "serverTextColor", + "columnName": "server_text_color", + "affinity": "TEXT" + }, + { + "fieldPath": "serverElementColor", + "columnName": "server_element_color", + "affinity": "TEXT" + }, + { + "fieldPath": "serverSlogan", + "columnName": "server_slogan", + "affinity": "TEXT" + }, + { + "fieldPath": "serverLogo", + "columnName": "server_logo", + "affinity": "TEXT" + }, + { + "fieldPath": "serverBackgroundUrl", + "columnName": "background_url", + "affinity": "TEXT" + }, + { + "fieldPath": "endToEndEncryption", + "columnName": "end_to_end_encryption", + "affinity": "INTEGER" + }, + { + "fieldPath": "endToEndEncryptionKeysExist", + "columnName": "end_to_end_encryption_keys_exist", + "affinity": "INTEGER" + }, + { + "fieldPath": "endToEndEncryptionApiVersion", + "columnName": "end_to_end_encryption_api_version", + "affinity": "TEXT" + }, + { + "fieldPath": "activity", + "columnName": "activity", + "affinity": "INTEGER" + }, + { + "fieldPath": "serverBackgroundDefault", + "columnName": "background_default", + "affinity": "INTEGER" + }, + { + "fieldPath": "serverBackgroundPlain", + "columnName": "background_plain", + "affinity": "INTEGER" + }, + { + "fieldPath": "richdocument", + "columnName": "richdocument", + "affinity": "INTEGER" + }, + { + "fieldPath": "richdocumentMimetypeList", + "columnName": "richdocument_mimetype_list", + "affinity": "TEXT" + }, + { + "fieldPath": "richdocumentDirectEditing", + "columnName": "richdocument_direct_editing", + "affinity": "INTEGER" + }, + { + "fieldPath": "richdocumentTemplates", + "columnName": "richdocument_direct_templates", + "affinity": "INTEGER" + }, + { + "fieldPath": "richdocumentOptionalMimetypeList", + "columnName": "richdocument_optional_mimetype_list", + "affinity": "TEXT" + }, + { + "fieldPath": "sharingPublicAskForOptionalPassword", + "columnName": "sharing_public_ask_for_optional_password", + "affinity": "INTEGER" + }, + { + "fieldPath": "richdocumentProductName", + "columnName": "richdocument_product_name", + "affinity": "TEXT" + }, + { + "fieldPath": "directEditingEtag", + "columnName": "direct_editing_etag", + "affinity": "TEXT" + }, + { + "fieldPath": "userStatus", + "columnName": "user_status", + "affinity": "INTEGER" + }, + { + "fieldPath": "userStatusSupportsEmoji", + "columnName": "user_status_supports_emoji", + "affinity": "INTEGER" + }, + { + "fieldPath": "etag", + "columnName": "etag", + "affinity": "TEXT" + }, + { + "fieldPath": "filesLockingVersion", + "columnName": "files_locking_version", + "affinity": "TEXT" + }, + { + "fieldPath": "groupfolders", + "columnName": "groupfolders", + "affinity": "INTEGER" + }, + { + "fieldPath": "dropAccount", + "columnName": "drop_account", + "affinity": "INTEGER" + }, + { + "fieldPath": "securityGuard", + "columnName": "security_guard", + "affinity": "INTEGER" + }, + { + "fieldPath": "governance", + "columnName": "governance", + "affinity": "INTEGER" + }, + { + "fieldPath": "forbiddenFileNameCharacters", + "columnName": "forbidden_filename_characters", + "affinity": "TEXT" + }, + { + "fieldPath": "forbiddenFileNames", + "columnName": "forbidden_filenames", + "affinity": "TEXT" + }, + { + "fieldPath": "forbiddenFileNameExtensions", + "columnName": "forbidden_filename_extensions", + "affinity": "TEXT" + }, + { + "fieldPath": "forbiddenFilenameBaseNames", + "columnName": "forbidden_filename_basenames", + "affinity": "TEXT" + }, + { + "fieldPath": "filesDownloadLimit", + "columnName": "files_download_limit", + "affinity": "INTEGER" + }, + { + "fieldPath": "filesDownloadLimitDefault", + "columnName": "files_download_limit_default", + "affinity": "INTEGER" + }, + { + "fieldPath": "recommendation", + "columnName": "recommendation", + "affinity": "INTEGER" + }, + { + "fieldPath": "notesFolderPath", + "columnName": "notes_folder_path", + "affinity": "TEXT" + }, + { + "fieldPath": "defaultPermissions", + "columnName": "default_permissions", + "affinity": "INTEGER" + }, + { + "fieldPath": "userStatusSupportsBusy", + "columnName": "user_status_supports_busy", + "affinity": "INTEGER" + }, + { + "fieldPath": "isWCFEnabled", + "columnName": "windows_compatible_filenames", + "affinity": "INTEGER" + }, + { + "fieldPath": "hasValidSubscription", + "columnName": "has_valid_subscription", + "affinity": "INTEGER" + }, + { + "fieldPath": "clientIntegrationJson", + "columnName": "client_integration_json", + "affinity": "TEXT" + }, + { + "fieldPath": "modRewriteWorking", + "columnName": "mod_rewrite_working", + "affinity": "INTEGER" + }, + { + "fieldPath": "chunkedUploadMaxSize", + "columnName": "chunked_upload_max_size", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharingJson", + "columnName": "sharing_json", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "_id" + ] + } + }, + { + "tableName": "external_links", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`_id` INTEGER PRIMARY KEY AUTOINCREMENT, `icon_url` TEXT, `language` TEXT, `type` INTEGER, `name` TEXT, `url` TEXT, `redirect` INTEGER)", + "fields": [ + { + "fieldPath": "id", + "columnName": "_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "iconUrl", + "columnName": "icon_url", + "affinity": "TEXT" + }, + { + "fieldPath": "language", + "columnName": "language", + "affinity": "TEXT" + }, + { + "fieldPath": "type", + "columnName": "type", + "affinity": "INTEGER" + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT" + }, + { + "fieldPath": "url", + "columnName": "url", + "affinity": "TEXT" + }, + { + "fieldPath": "redirect", + "columnName": "redirect", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "_id" + ] + } + }, + { + "tableName": "filelist", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`_id` INTEGER PRIMARY KEY AUTOINCREMENT, `filename` TEXT, `encrypted_filename` TEXT, `path` TEXT, `path_decrypted` TEXT, `parent` INTEGER, `created` INTEGER, `modified` INTEGER, `content_type` TEXT, `content_length` INTEGER, `media_path` TEXT, `file_owner` TEXT, `last_sync_date` INTEGER, `last_sync_date_for_data` INTEGER, `modified_at_last_sync_for_data` INTEGER, `etag` TEXT, `etag_on_server` TEXT, `share_by_link` INTEGER, `permissions` TEXT, `remote_id` TEXT, `local_id` INTEGER NOT NULL DEFAULT -1, `update_thumbnail` INTEGER, `is_downloading` INTEGER, `favorite` INTEGER, `hidden` INTEGER, `is_encrypted` INTEGER, `etag_in_conflict` TEXT, `shared_via_users` INTEGER, `mount_type` INTEGER, `has_preview` INTEGER, `unread_comments_count` INTEGER, `owner_id` TEXT, `owner_display_name` TEXT, `note` TEXT, `sharees` TEXT, `rich_workspace` TEXT, `metadata_size` TEXT, `metadata_live_photo` TEXT, `locked` INTEGER, `lock_type` INTEGER, `lock_owner` TEXT, `lock_owner_display_name` TEXT, `lock_owner_editor` TEXT, `lock_timestamp` INTEGER, `lock_timeout` INTEGER, `lock_token` TEXT, `tags` TEXT, `metadata_gps` TEXT, `e2e_counter` INTEGER, `internal_two_way_sync_timestamp` INTEGER, `internal_two_way_sync_result` TEXT, `uploaded` INTEGER, `is_read_only` INTEGER)", + "fields": [ + { + "fieldPath": "id", + "columnName": "_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "name", + "columnName": "filename", + "affinity": "TEXT" + }, + { + "fieldPath": "encryptedName", + "columnName": "encrypted_filename", + "affinity": "TEXT" + }, + { + "fieldPath": "path", + "columnName": "path", + "affinity": "TEXT" + }, + { + "fieldPath": "pathDecrypted", + "columnName": "path_decrypted", + "affinity": "TEXT" + }, + { + "fieldPath": "parent", + "columnName": "parent", + "affinity": "INTEGER" + }, + { + "fieldPath": "creation", + "columnName": "created", + "affinity": "INTEGER" + }, + { + "fieldPath": "modified", + "columnName": "modified", + "affinity": "INTEGER" + }, + { + "fieldPath": "contentType", + "columnName": "content_type", + "affinity": "TEXT" + }, + { + "fieldPath": "contentLength", + "columnName": "content_length", + "affinity": "INTEGER" + }, + { + "fieldPath": "storagePath", + "columnName": "media_path", + "affinity": "TEXT" + }, + { + "fieldPath": "accountOwner", + "columnName": "file_owner", + "affinity": "TEXT" + }, + { + "fieldPath": "lastSyncDate", + "columnName": "last_sync_date", + "affinity": "INTEGER" + }, + { + "fieldPath": "lastSyncDateForData", + "columnName": "last_sync_date_for_data", + "affinity": "INTEGER" + }, + { + "fieldPath": "modifiedAtLastSyncForData", + "columnName": "modified_at_last_sync_for_data", + "affinity": "INTEGER" + }, + { + "fieldPath": "etag", + "columnName": "etag", + "affinity": "TEXT" + }, + { + "fieldPath": "etagOnServer", + "columnName": "etag_on_server", + "affinity": "TEXT" + }, + { + "fieldPath": "sharedViaLink", + "columnName": "share_by_link", + "affinity": "INTEGER" + }, + { + "fieldPath": "permissions", + "columnName": "permissions", + "affinity": "TEXT" + }, + { + "fieldPath": "remoteId", + "columnName": "remote_id", + "affinity": "TEXT" + }, + { + "fieldPath": "localId", + "columnName": "local_id", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "-1" + }, + { + "fieldPath": "updateThumbnail", + "columnName": "update_thumbnail", + "affinity": "INTEGER" + }, + { + "fieldPath": "isDownloading", + "columnName": "is_downloading", + "affinity": "INTEGER" + }, + { + "fieldPath": "favorite", + "columnName": "favorite", + "affinity": "INTEGER" + }, + { + "fieldPath": "hidden", + "columnName": "hidden", + "affinity": "INTEGER" + }, + { + "fieldPath": "isEncrypted", + "columnName": "is_encrypted", + "affinity": "INTEGER" + }, + { + "fieldPath": "etagInConflict", + "columnName": "etag_in_conflict", + "affinity": "TEXT" + }, + { + "fieldPath": "sharedWithSharee", + "columnName": "shared_via_users", + "affinity": "INTEGER" + }, + { + "fieldPath": "mountType", + "columnName": "mount_type", + "affinity": "INTEGER" + }, + { + "fieldPath": "hasPreview", + "columnName": "has_preview", + "affinity": "INTEGER" + }, + { + "fieldPath": "unreadCommentsCount", + "columnName": "unread_comments_count", + "affinity": "INTEGER" + }, + { + "fieldPath": "ownerId", + "columnName": "owner_id", + "affinity": "TEXT" + }, + { + "fieldPath": "ownerDisplayName", + "columnName": "owner_display_name", + "affinity": "TEXT" + }, + { + "fieldPath": "note", + "columnName": "note", + "affinity": "TEXT" + }, + { + "fieldPath": "sharees", + "columnName": "sharees", + "affinity": "TEXT" + }, + { + "fieldPath": "richWorkspace", + "columnName": "rich_workspace", + "affinity": "TEXT" + }, + { + "fieldPath": "metadataSize", + "columnName": "metadata_size", + "affinity": "TEXT" + }, + { + "fieldPath": "metadataLivePhoto", + "columnName": "metadata_live_photo", + "affinity": "TEXT" + }, + { + "fieldPath": "locked", + "columnName": "locked", + "affinity": "INTEGER" + }, + { + "fieldPath": "lockType", + "columnName": "lock_type", + "affinity": "INTEGER" + }, + { + "fieldPath": "lockOwner", + "columnName": "lock_owner", + "affinity": "TEXT" + }, + { + "fieldPath": "lockOwnerDisplayName", + "columnName": "lock_owner_display_name", + "affinity": "TEXT" + }, + { + "fieldPath": "lockOwnerEditor", + "columnName": "lock_owner_editor", + "affinity": "TEXT" + }, + { + "fieldPath": "lockTimestamp", + "columnName": "lock_timestamp", + "affinity": "INTEGER" + }, + { + "fieldPath": "lockTimeout", + "columnName": "lock_timeout", + "affinity": "INTEGER" + }, + { + "fieldPath": "lockToken", + "columnName": "lock_token", + "affinity": "TEXT" + }, + { + "fieldPath": "tags", + "columnName": "tags", + "affinity": "TEXT" + }, + { + "fieldPath": "metadataGPS", + "columnName": "metadata_gps", + "affinity": "TEXT" + }, + { + "fieldPath": "e2eCounter", + "columnName": "e2e_counter", + "affinity": "INTEGER" + }, + { + "fieldPath": "internalTwoWaySync", + "columnName": "internal_two_way_sync_timestamp", + "affinity": "INTEGER" + }, + { + "fieldPath": "internalTwoWaySyncResult", + "columnName": "internal_two_way_sync_result", + "affinity": "TEXT" + }, + { + "fieldPath": "uploaded", + "columnName": "uploaded", + "affinity": "INTEGER" + }, + { + "fieldPath": "isReadOnly", + "columnName": "is_read_only", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "_id" + ] + }, + "indices": [ + { + "name": "idx_filelist_path_owner", + "unique": true, + "columnNames": [ + "path", + "file_owner" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `idx_filelist_path_owner` ON `${TABLE_NAME}` (`path`, `file_owner`)" + } + ] + }, + { + "tableName": "filesystem", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`_id` INTEGER PRIMARY KEY AUTOINCREMENT, `local_path` TEXT, `remote_path` TEXT, `is_folder` INTEGER, `found_at` INTEGER, `upload_triggered` INTEGER, `syncedfolder_id` TEXT, `crc32` TEXT, `modified_at` INTEGER)", + "fields": [ + { + "fieldPath": "id", + "columnName": "_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "localPath", + "columnName": "local_path", + "affinity": "TEXT" + }, + { + "fieldPath": "remotePath", + "columnName": "remote_path", + "affinity": "TEXT" + }, + { + "fieldPath": "fileIsFolder", + "columnName": "is_folder", + "affinity": "INTEGER" + }, + { + "fieldPath": "fileFoundRecently", + "columnName": "found_at", + "affinity": "INTEGER" + }, + { + "fieldPath": "fileSentForUpload", + "columnName": "upload_triggered", + "affinity": "INTEGER" + }, + { + "fieldPath": "syncedFolderId", + "columnName": "syncedfolder_id", + "affinity": "TEXT" + }, + { + "fieldPath": "crc32", + "columnName": "crc32", + "affinity": "TEXT" + }, + { + "fieldPath": "fileModified", + "columnName": "modified_at", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "_id" + ] + }, + "indices": [ + { + "name": "index_filesystem_local_path_syncedfolder_id", + "unique": false, + "columnNames": [ + "local_path", + "syncedfolder_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_filesystem_local_path_syncedfolder_id` ON `${TABLE_NAME}` (`local_path`, `syncedfolder_id`)" + }, + { + "name": "index_filesystem_syncedfolder_id_upload_triggered_is_folder", + "unique": false, + "columnNames": [ + "syncedfolder_id", + "upload_triggered", + "is_folder" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_filesystem_syncedfolder_id_upload_triggered_is_folder` ON `${TABLE_NAME}` (`syncedfolder_id`, `upload_triggered`, `is_folder`)" + } + ] + }, + { + "tableName": "ocshares", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`_id` INTEGER PRIMARY KEY AUTOINCREMENT, `file_source` INTEGER, `item_source` INTEGER, `share_type` INTEGER, `shate_with` TEXT, `path` TEXT, `permissions` INTEGER, `shared_date` INTEGER, `expiration_date` INTEGER, `token` TEXT, `shared_with_display_name` TEXT, `is_directory` INTEGER, `user_id` TEXT, `id_remote_shared` INTEGER, `owner_share` TEXT, `is_password_protected` INTEGER, `note` TEXT, `hide_download` INTEGER, `share_link` TEXT, `share_label` TEXT, `download_limit_limit` INTEGER, `download_limit_count` INTEGER, `attributes` TEXT)", + "fields": [ + { + "fieldPath": "id", + "columnName": "_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "fileSource", + "columnName": "file_source", + "affinity": "INTEGER" + }, + { + "fieldPath": "itemSource", + "columnName": "item_source", + "affinity": "INTEGER" + }, + { + "fieldPath": "shareType", + "columnName": "share_type", + "affinity": "INTEGER" + }, + { + "fieldPath": "shareWith", + "columnName": "shate_with", + "affinity": "TEXT" + }, + { + "fieldPath": "path", + "columnName": "path", + "affinity": "TEXT" + }, + { + "fieldPath": "permissions", + "columnName": "permissions", + "affinity": "INTEGER" + }, + { + "fieldPath": "sharedDate", + "columnName": "shared_date", + "affinity": "INTEGER" + }, + { + "fieldPath": "expirationDate", + "columnName": "expiration_date", + "affinity": "INTEGER" + }, + { + "fieldPath": "token", + "columnName": "token", + "affinity": "TEXT" + }, + { + "fieldPath": "shareWithDisplayName", + "columnName": "shared_with_display_name", + "affinity": "TEXT" + }, + { + "fieldPath": "isDirectory", + "columnName": "is_directory", + "affinity": "INTEGER" + }, + { + "fieldPath": "userId", + "columnName": "user_id", + "affinity": "TEXT" + }, + { + "fieldPath": "idRemoteShared", + "columnName": "id_remote_shared", + "affinity": "INTEGER" + }, + { + "fieldPath": "accountOwner", + "columnName": "owner_share", + "affinity": "TEXT" + }, + { + "fieldPath": "isPasswordProtected", + "columnName": "is_password_protected", + "affinity": "INTEGER" + }, + { + "fieldPath": "note", + "columnName": "note", + "affinity": "TEXT" + }, + { + "fieldPath": "hideDownload", + "columnName": "hide_download", + "affinity": "INTEGER" + }, + { + "fieldPath": "shareLink", + "columnName": "share_link", + "affinity": "TEXT" + }, + { + "fieldPath": "shareLabel", + "columnName": "share_label", + "affinity": "TEXT" + }, + { + "fieldPath": "downloadLimitLimit", + "columnName": "download_limit_limit", + "affinity": "INTEGER" + }, + { + "fieldPath": "downloadLimitCount", + "columnName": "download_limit_count", + "affinity": "INTEGER" + }, + { + "fieldPath": "attributes", + "columnName": "attributes", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "_id" + ] + } + }, + { + "tableName": "synced_folders", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`_id` INTEGER PRIMARY KEY AUTOINCREMENT, `local_path` TEXT, `remote_path` TEXT, `wifi_only` INTEGER, `charging_only` INTEGER, `existing` INTEGER, `enabled` INTEGER, `enabled_timestamp_ms` INTEGER, `subfolder_by_date` INTEGER, `account` TEXT, `upload_option` INTEGER, `name_collision_policy` INTEGER, `type` INTEGER, `hidden` INTEGER, `sub_folder_rule` INTEGER, `exclude_hidden` INTEGER, `last_scan_timestamp_ms` INTEGER)", + "fields": [ + { + "fieldPath": "id", + "columnName": "_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "localPath", + "columnName": "local_path", + "affinity": "TEXT" + }, + { + "fieldPath": "remotePath", + "columnName": "remote_path", + "affinity": "TEXT" + }, + { + "fieldPath": "wifiOnly", + "columnName": "wifi_only", + "affinity": "INTEGER" + }, + { + "fieldPath": "chargingOnly", + "columnName": "charging_only", + "affinity": "INTEGER" + }, + { + "fieldPath": "existing", + "columnName": "existing", + "affinity": "INTEGER" + }, + { + "fieldPath": "enabled", + "columnName": "enabled", + "affinity": "INTEGER" + }, + { + "fieldPath": "enabledTimestampMs", + "columnName": "enabled_timestamp_ms", + "affinity": "INTEGER" + }, + { + "fieldPath": "subfolderByDate", + "columnName": "subfolder_by_date", + "affinity": "INTEGER" + }, + { + "fieldPath": "account", + "columnName": "account", + "affinity": "TEXT" + }, + { + "fieldPath": "uploadAction", + "columnName": "upload_option", + "affinity": "INTEGER" + }, + { + "fieldPath": "nameCollisionPolicy", + "columnName": "name_collision_policy", + "affinity": "INTEGER" + }, + { + "fieldPath": "type", + "columnName": "type", + "affinity": "INTEGER" + }, + { + "fieldPath": "hidden", + "columnName": "hidden", + "affinity": "INTEGER" + }, + { + "fieldPath": "subFolderRule", + "columnName": "sub_folder_rule", + "affinity": "INTEGER" + }, + { + "fieldPath": "excludeHidden", + "columnName": "exclude_hidden", + "affinity": "INTEGER" + }, + { + "fieldPath": "lastScanTimestampMs", + "columnName": "last_scan_timestamp_ms", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "_id" + ] + } + }, + { + "tableName": "list_of_uploads", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`_id` INTEGER PRIMARY KEY AUTOINCREMENT, `local_path` TEXT, `remote_path` TEXT, `account_name` TEXT, `file_size` INTEGER, `status` INTEGER, `local_behaviour` INTEGER, `upload_time` INTEGER, `name_collision_policy` INTEGER, `is_create_remote_folder` INTEGER, `upload_end_timestamp` INTEGER, `upload_end_timestamp_long` INTEGER, `last_result` INTEGER, `is_while_charging_only` INTEGER, `is_wifi_only` INTEGER, `created_by` INTEGER, `folder_unlock_token` TEXT)", + "fields": [ + { + "fieldPath": "id", + "columnName": "_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "localPath", + "columnName": "local_path", + "affinity": "TEXT" + }, + { + "fieldPath": "remotePath", + "columnName": "remote_path", + "affinity": "TEXT" + }, + { + "fieldPath": "accountName", + "columnName": "account_name", + "affinity": "TEXT" + }, + { + "fieldPath": "fileSize", + "columnName": "file_size", + "affinity": "INTEGER" + }, + { + "fieldPath": "status", + "columnName": "status", + "affinity": "INTEGER" + }, + { + "fieldPath": "localBehaviour", + "columnName": "local_behaviour", + "affinity": "INTEGER" + }, + { + "fieldPath": "uploadTime", + "columnName": "upload_time", + "affinity": "INTEGER" + }, + { + "fieldPath": "nameCollisionPolicy", + "columnName": "name_collision_policy", + "affinity": "INTEGER" + }, + { + "fieldPath": "isCreateRemoteFolder", + "columnName": "is_create_remote_folder", + "affinity": "INTEGER" + }, + { + "fieldPath": "uploadEndTimestamp", + "columnName": "upload_end_timestamp", + "affinity": "INTEGER" + }, + { + "fieldPath": "uploadEndTimestampLong", + "columnName": "upload_end_timestamp_long", + "affinity": "INTEGER" + }, + { + "fieldPath": "lastResult", + "columnName": "last_result", + "affinity": "INTEGER" + }, + { + "fieldPath": "isWhileChargingOnly", + "columnName": "is_while_charging_only", + "affinity": "INTEGER" + }, + { + "fieldPath": "isWifiOnly", + "columnName": "is_wifi_only", + "affinity": "INTEGER" + }, + { + "fieldPath": "createdBy", + "columnName": "created_by", + "affinity": "INTEGER" + }, + { + "fieldPath": "folderUnlockToken", + "columnName": "folder_unlock_token", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "_id" + ] + }, + "indices": [ + { + "name": "index_list_of_uploads_account_name_status", + "unique": false, + "columnNames": [ + "account_name", + "status" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_list_of_uploads_account_name_status` ON `${TABLE_NAME}` (`account_name`, `status`)" + }, + { + "name": "index_list_of_uploads_remote_path_account_name", + "unique": false, + "columnNames": [ + "remote_path", + "account_name" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_list_of_uploads_remote_path_account_name` ON `${TABLE_NAME}` (`remote_path`, `account_name`)" + }, + { + "name": "index_list_of_uploads_account_name_local_path", + "unique": false, + "columnNames": [ + "account_name", + "local_path" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_list_of_uploads_account_name_local_path` ON `${TABLE_NAME}` (`account_name`, `local_path`)" + } + ] + }, + { + "tableName": "virtual", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`_id` INTEGER PRIMARY KEY AUTOINCREMENT, `type` TEXT, `ocfile_id` INTEGER)", + "fields": [ + { + "fieldPath": "id", + "columnName": "_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "type", + "columnName": "type", + "affinity": "TEXT" + }, + { + "fieldPath": "ocFileId", + "columnName": "ocfile_id", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "_id" + ] + } + }, + { + "tableName": "offline_operations", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`_id` INTEGER PRIMARY KEY AUTOINCREMENT, `offline_operations_parent_oc_file_id` INTEGER, `offline_operations_path` TEXT, `offline_operations_type` TEXT, `offline_operations_file_name` TEXT, `offline_operations_created_at` INTEGER, `offline_operations_modified_at` INTEGER)", + "fields": [ + { + "fieldPath": "id", + "columnName": "_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "parentOCFileId", + "columnName": "offline_operations_parent_oc_file_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "path", + "columnName": "offline_operations_path", + "affinity": "TEXT" + }, + { + "fieldPath": "type", + "columnName": "offline_operations_type", + "affinity": "TEXT" + }, + { + "fieldPath": "filename", + "columnName": "offline_operations_file_name", + "affinity": "TEXT" + }, + { + "fieldPath": "createdAt", + "columnName": "offline_operations_created_at", + "affinity": "INTEGER" + }, + { + "fieldPath": "modifiedAt", + "columnName": "offline_operations_modified_at", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "_id" + ] + } + }, + { + "tableName": "recommended_files", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`_id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `name` TEXT NOT NULL, `directory` TEXT NOT NULL, `extension` TEXT NOT NULL, `mime_type` TEXT NOT NULL, `has_preview` INTEGER NOT NULL, `reason` TEXT NOT NULL, `timestamp` INTEGER NOT NULL, `account_name` TEXT)", + "fields": [ + { + "fieldPath": "id", + "columnName": "_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "directory", + "columnName": "directory", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "extension", + "columnName": "extension", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "mimeType", + "columnName": "mime_type", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "hasPreview", + "columnName": "has_preview", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "reason", + "columnName": "reason", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "timestamp", + "columnName": "timestamp", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "accountName", + "columnName": "account_name", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "_id" + ] + } + }, + { + "tableName": "assistant", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `accountName` TEXT, `type` TEXT, `status` TEXT, `userId` TEXT, `appId` TEXT, `input` TEXT, `output` TEXT, `completionExpectedAt` INTEGER, `progress` INTEGER, `lastUpdated` INTEGER, `scheduledAt` INTEGER, `endedAt` INTEGER)", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "accountName", + "columnName": "accountName", + "affinity": "TEXT" + }, + { + "fieldPath": "type", + "columnName": "type", + "affinity": "TEXT" + }, + { + "fieldPath": "status", + "columnName": "status", + "affinity": "TEXT" + }, + { + "fieldPath": "userId", + "columnName": "userId", + "affinity": "TEXT" + }, + { + "fieldPath": "appId", + "columnName": "appId", + "affinity": "TEXT" + }, + { + "fieldPath": "input", + "columnName": "input", + "affinity": "TEXT" + }, + { + "fieldPath": "output", + "columnName": "output", + "affinity": "TEXT" + }, + { + "fieldPath": "completionExpectedAt", + "columnName": "completionExpectedAt", + "affinity": "INTEGER" + }, + { + "fieldPath": "progress", + "columnName": "progress", + "affinity": "INTEGER" + }, + { + "fieldPath": "lastUpdated", + "columnName": "lastUpdated", + "affinity": "INTEGER" + }, + { + "fieldPath": "scheduledAt", + "columnName": "scheduledAt", + "affinity": "INTEGER" + }, + { + "fieldPath": "endedAt", + "columnName": "endedAt", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + } + } + ], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '4d8f3a13a93f08725aeafa3fe7b7153e')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/java/com/nextcloud/client/database/migrations/Migration106to107Test.kt b/app/src/androidTest/java/com/nextcloud/client/database/migrations/Migration106to107Test.kt new file mode 100644 index 000000000000..e3d4cc5c1b98 --- /dev/null +++ b/app/src/androidTest/java/com/nextcloud/client/database/migrations/Migration106to107Test.kt @@ -0,0 +1,93 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 PeterPablo <4340267+PeterPablo@users.noreply.github.com> + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +package com.nextcloud.client.database.migrations + +import androidx.room.testing.MigrationTestHelper +import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.nextcloud.client.database.NextcloudDatabase +import org.junit.Assert.assertEquals +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import java.io.IOException + +/** + * 106 → 107 collapses the rows that were stored twice for one file and then forbids new ones. + * + * The duplicates were reachable because `filelist` had no unique index while + * [com.owncloud.android.providers.FileContentProvider.upsertSingleFile] relied on an `INSERT … CONFLICT_IGNORE` to + * detect an existing row — with no constraint, nothing could conflict, so a second row was always written and the + * `UPDATE` behind that check was unreachable. A SAF consumer then showed the folder twice (nextcloud/android#17051). + */ +@RunWith(AndroidJUnit4::class) +class Migration106to107Test { + + @get:Rule + val helper: MigrationTestHelper = MigrationTestHelper( + InstrumentationRegistry.getInstrumentation(), + NextcloudDatabase::class.java.canonicalName, + FrameworkSQLiteOpenHelperFactory() + ) + + @Test + @Throws(IOException::class) + fun twinsCollapseToOldestRow_otherAccountsAndPathlessRowsSurvive() { + var db = helper.createDatabase(TEST_DB, 106) + + db.execSQL("INSERT INTO filelist (filename, path, file_owner, parent) VALUES ('tmp','/tmp/','a@cloud',1)") + db.execSQL("INSERT INTO filelist (filename, path, file_owner, parent) VALUES ('tmp','/tmp/','a@cloud',1)") + db.execSQL("INSERT INTO filelist (filename, path, file_owner, parent) VALUES ('tmp','/tmp/','a@cloud',1)") + // same path, different account: a different file, not a twin + db.execSQL("INSERT INTO filelist (filename, path, file_owner, parent) VALUES ('tmp','/tmp/','b@cloud',1)") + // no path: nothing to compare, must not be collapsed + db.execSQL("INSERT INTO filelist (filename, path, file_owner, parent) VALUES ('pending',NULL,'a@cloud',9)") + db.execSQL("INSERT INTO filelist (filename, path, file_owner, parent) VALUES ('pending',NULL,'a@cloud',9)") + db.close() + + // runMigrationsAndValidate also asserts the result matches 107.json — i.e. this migration and the entity agree + db = helper.runMigrationsAndValidate(TEST_DB, 107, true, MIGRATION_106_107) + + assertEquals( + "the three twins must collapse to one row, the other account stays", + 2, + count(db, "SELECT count(*) FROM filelist WHERE parent = 1") + ) + assertEquals( + "rows without a path have no identity to compare and must survive", + 2, + count(db, "SELECT count(*) FROM filelist WHERE path IS NULL") + ) + assertEquals( + "the oldest row of the twins is the one kept", + 1, + count(db, "SELECT _id FROM filelist WHERE path='/tmp/' AND file_owner='a@cloud'") + ) + + // the invariant now lives in the database: a further twin cannot be written at all + db.execSQL( + "INSERT OR IGNORE INTO filelist (filename, path, file_owner, parent) VALUES ('tmp','/tmp/','a@cloud',1)" + ) + assertEquals( + "a unique index on (path, file_owner) must reject the second row", + 2, + count(db, "SELECT count(*) FROM filelist WHERE parent = 1") + ) + + db.close() + } + + private fun count(db: androidx.sqlite.db.SupportSQLiteDatabase, sql: String): Int = db.query(sql).use { cursor -> + cursor.moveToFirst() + cursor.getInt(0) + } + + companion object { + private const val TEST_DB = "migration-106-107-test" + } +} diff --git a/app/src/main/java/com/nextcloud/client/database/NextcloudDatabase.kt b/app/src/main/java/com/nextcloud/client/database/NextcloudDatabase.kt index b6676caf0a1b..00ad5d710837 100644 --- a/app/src/main/java/com/nextcloud/client/database/NextcloudDatabase.kt +++ b/app/src/main/java/com/nextcloud/client/database/NextcloudDatabase.kt @@ -41,6 +41,7 @@ import com.nextcloud.client.database.entity.VirtualEntity import com.nextcloud.client.database.migrations.DatabaseMigrationUtil import com.nextcloud.client.database.migrations.MIGRATION_88_89 import com.nextcloud.client.database.migrations.MIGRATION_97_98 +import com.nextcloud.client.database.migrations.MIGRATION_106_107 import com.nextcloud.client.database.migrations.MIGRATION_99_100 import com.nextcloud.client.database.migrations.Migration67to68 import com.nextcloud.client.database.migrations.RoomMigration @@ -109,6 +110,7 @@ import com.owncloud.android.db.ProviderMeta ], exportSchema = true ) +@Suppress("Detekt.UnnecessaryAbstractClass") // needed by Room @TypeConverters(OfflineOperationTypeConverter::class) abstract class NextcloudDatabase : RoomDatabase() { @@ -145,6 +147,7 @@ abstract class NextcloudDatabase : RoomDatabase() { .addMigrations(MIGRATION_88_89) .addMigrations(MIGRATION_97_98) .addMigrations(MIGRATION_99_100) + .addMigrations(MIGRATION_106_107) .build() } return instance!! diff --git a/app/src/main/java/com/nextcloud/client/database/entity/FileEntity.kt b/app/src/main/java/com/nextcloud/client/database/entity/FileEntity.kt index 89ff2b616cb9..020cbfa05905 100644 --- a/app/src/main/java/com/nextcloud/client/database/entity/FileEntity.kt +++ b/app/src/main/java/com/nextcloud/client/database/entity/FileEntity.kt @@ -9,10 +9,24 @@ package com.nextcloud.client.database.entity import androidx.room.ColumnInfo import androidx.room.Entity +import androidx.room.Index import androidx.room.PrimaryKey import com.owncloud.android.db.ProviderMeta.ProviderTableMeta -@Entity(tableName = ProviderTableMeta.FILE_TABLE_NAME) +// One row per (path, account). Without it a writer that spelled the path differently — `/dir` vs `/dir/` — added a +// second row, and `SELECT … WHERE parent = ?` served both to the SAF provider (nextcloud/android#17051). The index is +// declared here as well as in MIGRATION_106_107 because Room creates a *fresh* database from this declaration: the +// migration only reaches installs that already exist. +@Entity( + tableName = ProviderTableMeta.FILE_TABLE_NAME, + indices = [ + Index( + value = [ProviderTableMeta.FILE_PATH, ProviderTableMeta.FILE_ACCOUNT_OWNER], + unique = true, + name = "idx_filelist_path_owner" + ) + ] +) data class FileEntity( @PrimaryKey(autoGenerate = true) @ColumnInfo(name = ProviderTableMeta._ID) diff --git a/app/src/main/java/com/nextcloud/client/database/migrations/Migration106to107.kt b/app/src/main/java/com/nextcloud/client/database/migrations/Migration106to107.kt new file mode 100644 index 000000000000..5e9ce3d7a2e6 --- /dev/null +++ b/app/src/main/java/com/nextcloud/client/database/migrations/Migration106to107.kt @@ -0,0 +1,40 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 PeterPablo <4340267+PeterPablo@users.noreply.github.com> + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.client.database.migrations + +import androidx.room.migration.Migration +import androidx.sqlite.db.SupportSQLiteDatabase + +/** + * Makes "one row per file" an invariant of the database instead of a hope. + * + * `filelist` had no unique index at all: every write path used `INSERT … CONFLICT_*`, which had + * nothing to conflict with, and the read that serves the file list, + * `SELECT … FROM filelist WHERE parent = ?`, has no owner filter and no `DISTINCT`. So a second row + * for the same path was silently accepted, and both rows were then handed to whoever asked — a SAF + * consumer shows that as the same folder twice (nextcloud/android#17051). `upsertSingleFile` + * narrows the window by looking the row up first, but a lookup cannot make the invariant hold while + * the index is missing. + * + * Two steps, in this order: collapse the twins that exist, then forbid new ones. + */ +@Suppress("MagicNumber") +val MIGRATION_106_107 = object : Migration(106, 107) { + override fun migrate(db: SupportSQLiteDatabase) { + // Keep the oldest row per (path, file_owner). Rows without a path are left alone: they have no + // identity to compare, and SQLite counts NULLs as distinct in a unique index anyway. + db.execSQL( + """ + DELETE FROM filelist + WHERE path IS NOT NULL + AND _id NOT IN (SELECT MIN(_id) FROM filelist WHERE path IS NOT NULL GROUP BY path, file_owner) + """.trimIndent() + ) + db.execSQL("CREATE UNIQUE INDEX IF NOT EXISTS idx_filelist_path_owner ON filelist (path, file_owner)") + } +} diff --git a/app/src/main/java/com/owncloud/android/db/ProviderMeta.java b/app/src/main/java/com/owncloud/android/db/ProviderMeta.java index 3886a51210d6..535063a0560c 100644 --- a/app/src/main/java/com/owncloud/android/db/ProviderMeta.java +++ b/app/src/main/java/com/owncloud/android/db/ProviderMeta.java @@ -24,7 +24,7 @@ */ public class ProviderMeta { public static final String DB_NAME = "filelist"; - public static final int DB_VERSION = 106; + public static final int DB_VERSION = 107; private ProviderMeta() { // No instance diff --git a/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java b/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java index fb87de46c992..7f5dcf9fb1cd 100644 --- a/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java +++ b/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java @@ -21,6 +21,7 @@ import android.content.UriMatcher; import android.database.Cursor; import android.database.SQLException; +import android.database.sqlite.SQLiteConstraintException; import android.database.sqlite.SQLiteDatabase; import android.net.Uri; import android.os.Binder; @@ -341,6 +342,36 @@ public Uri upsertSingleFile(SupportSQLiteDatabase db, Uri uri, ContentValues val String where = ProviderTableMeta.FILE_PATH + "=? AND " + ProviderTableMeta.FILE_ACCOUNT_OWNER + "=?"; String[] whereArgs = {filePath, accountOwner}; + Long existingRowId = queryFileId(db, where, whereArgs); + if (existingRowId != null) { + db.update(ProviderTableMeta.FILE_TABLE_NAME, SQLiteDatabase.CONFLICT_NONE, values, where, whereArgs); + return ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, existingRowId); + } + + // A concurrent writer can slip in between the lookup above and this insert. The unique index on + // (path, file_owner) then refuses our row — that is the point of it — so take the id of the row + // that is stored now instead of handing out a broken one. + long rowId; + try { + rowId = db.insert(ProviderTableMeta.FILE_TABLE_NAME, SQLiteDatabase.CONFLICT_NONE, values); + } catch (SQLiteConstraintException e) { + Long storedRowId = queryFileId(db, where, whereArgs); + if (storedRowId == null) { + throw e; + } + rowId = storedRowId; + } + if (rowId <= 0) { + // insert() reports a refused row as -1 on some API levels instead of throwing. + Long storedRowId = queryFileId(db, where, whereArgs); + if (storedRowId != null) { + rowId = storedRowId; + } + } + return ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, rowId); + } + + private Long queryFileId(SupportSQLiteDatabase db, String where, String... whereArgs) { try (Cursor cursor = db.query( new SimpleSQLiteQuery( "SELECT " + ProviderTableMeta._ID + @@ -348,15 +379,8 @@ public Uri upsertSingleFile(SupportSQLiteDatabase db, Uri uri, ContentValues val " WHERE " + where, whereArgs ))) { - if (cursor.moveToFirst()) { - long existingRowId = cursor.getLong(0); - db.update(ProviderTableMeta.FILE_TABLE_NAME, SQLiteDatabase.CONFLICT_NONE, values, where, whereArgs); - return ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, existingRowId); - } + return cursor.moveToFirst() ? cursor.getLong(0) : null; } - - long rowId = db.insert(ProviderTableMeta.FILE_TABLE_NAME, SQLiteDatabase.CONFLICT_NONE, values); - return ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, rowId); } private void updateFilesTableAccordingToShareInsertion(SupportSQLiteDatabase db, ContentValues newShare) {