From 1a205b24190d7c979241d4b9d507c84b0ae2904f Mon Sep 17 00:00:00 2001 From: AptS-1547 Date: Thu, 23 Jul 2026 13:50:15 +0800 Subject: [PATCH 1/2] feat(upload): require explicit session_kind and remove legacy chunk assembly **Breaking Changes:** - `upload_sessions.session_kind` is now `NOT NULL` - Removed 0.4.x payload-per-chunk `chunk_N`, `assembled` assembly/relay, kind inference, and assembly limiter - Migration will fail if null or invalid `session_kind` values exist; deployments must clean up expired sessions before upgrading to 0.5.0 **Changes:** - Chunk PUT, Progress, Complete, Cancel/Cleanup now only accept persisted explicit `UploadSessionKind` - Still validate multipart, temp key, and provider session metadata invariants - OffsetStaging, StreamStaging, provider/remote relay multipart, presigned, and provider resumable paths remain unchanged - Upload service continues generating Init plan from connector-owned transport without introducing `DriverType` routing matrix - Removed `LegacyChunkFiles` kind, `assemble_legacy_local_chunks_to_temp_file`, `stream_legacy_local_chunks_into_writer`, compatibility classifier for null rows, and chunk assembly limiter - Removed `UploadRuntime` from `PrimaryAppState` (no longer needed without assembly serialization) - Simplified `resolve_upload_session_kind` to direct field read without fallback inference - Added migration `m20260723_000001_require_upload_session_kind` with pre-flight validation that rejects upgrades containing legacy rows - Updated tests to create sessions with explicit kinds and verify migration rollback/reapply behavior --- CHANGELOG.md | 9 + developer-docs/zh-CN/api/files.md | 4 +- .../design/upload-finalization-contracts.md | 25 +- frontend-panel/src/services/api.generated.ts | 8 +- migration/src/lib.rs | 2 + ...0723_000001_require_upload_session_kind.rs | 342 ++++++++++++++++ src/api/routes/files/access.rs | 1 - src/api/routes/frontend.rs | 1 - src/api/routes/health.rs | 1 - src/api/routes/share_public.rs | 1 - src/entities/upload_session.rs | 5 +- src/runtime/mod.rs | 18 +- src/runtime/startup/primary.rs | 1 - src/runtime/tasks.rs | 1 - src/services/files/file/deletion/tests.rs | 1 - src/services/files/file/download/tests.rs | 1 - src/services/files/file/resource_handle.rs | 1 - src/services/files/lock/tests.rs | 1 - src/services/files/upload/chunk.rs | 383 +----------------- src/services/files/upload/complete/chunked.rs | 285 +------------ src/services/files/upload/complete/mod.rs | 10 +- .../files/upload/complete/object_multipart.rs | 4 +- src/services/files/upload/complete/plan.rs | 4 +- src/services/files/upload/complete/tests.rs | 2 +- src/services/files/upload/init/context.rs | 2 +- src/services/files/upload/init/mod.rs | 3 +- src/services/files/upload/kind.rs | 266 ++---------- src/services/files/upload/lifecycle.rs | 39 +- src/services/files/upload/mod.rs | 2 - src/services/files/upload/progress.rs | 34 +- src/services/files/upload/runtime.rs | 178 -------- src/services/files/upload/shared.rs | 2 +- src/services/files/upload/staging.rs | 21 +- src/services/media/metadata/tests.rs | 1 - src/services/ops/audit/tests.rs | 2 - src/services/ops/config/system.rs | 1 - .../storage_policy/credential/oauth/tests.rs | 1 - .../storage_policy/policy/policies.rs | 1 - src/services/task/dispatch/tests.rs | 1 - src/services/workspace/scope/mod.rs | 1 - src/services/workspace/storage/tests.rs | 1 - src/storage/connectors/mod.rs | 2 +- src/storage/connectors/tests.rs | 45 -- src/storage/connectors/upload.rs | 32 -- src/types/upload_session.rs | 6 - src/webdav/auth.rs | 1 - src/webdav/file/tests.rs | 1 - src/webdav/handler_tests/mod.rs | 1 - tests/common/mod.rs | 2 - tests/test_auth.rs | 2 - tests/test_database_backends.rs | 2 +- tests/test_maintenance.rs | 7 +- tests/test_migrations.rs | 142 ++++++- tests/test_policies.rs | 8 +- tests/test_remote_storage.rs | 2 +- tests/test_services.rs | 4 +- tests/test_upload.rs | 342 ++-------------- 57 files changed, 663 insertions(+), 1603 deletions(-) create mode 100644 migration/src/m20260723_000001_require_upload_session_kind.rs delete mode 100644 src/services/files/upload/runtime.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index d0e31f036..6e70a6296 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Breaking + +- **上传 session 0.5.0 边界** — `upload_sessions.session_kind` 收紧为 `NOT NULL`，并删除 0.4.x payload-per-chunk `chunk_N`、`assembled` 拼装/relay、kind 推断和 assembly limiter。升级迁移遇到 null 或非法 kind 会停止且保留原行；部署方需要先清理已过期的旧上传 session。 + +### Changed + +- Chunk PUT、Progress、Complete、Cancel/Cleanup 只接受持久化的显式 `UploadSessionKind`，仍会校验 multipart、temp key 和 provider session metadata 的组合不变量。 +- OffsetStaging、StreamStaging、provider/remote relay multipart、presigned 和 provider resumable 主路径保持不变；upload service 继续从 connector-owned transport 生成 Init plan，不引入 `DriverType` 分流矩阵。 + ## [v0.4.0] - 2026-07-23 ### Release Highlights diff --git a/developer-docs/zh-CN/api/files.md b/developer-docs/zh-CN/api/files.md index a764acb9f..defd779f7 100644 --- a/developer-docs/zh-CN/api/files.md +++ b/developer-docs/zh-CN/api/files.md @@ -104,11 +104,11 @@ 当前新建的 server-managed chunked session 使用 offset staging：Init 在 session 临时目录预创建 `.offset-staging-v1`，每个 Chunk PUT 按 `chunk_number * chunk_size` 写入对应 range；`upload_session_parts` 中的本地 receipt 表示该 range 已 durable publish。新 session 不创建 `chunk_N` marker，Complete 也不会再次拼写整文件。 -每个需要 session 的 Init 都会在 `upload_sessions.session_kind` 持久化 connector-owned 执行计划（例如 `offset_staging`、`stream_staging`、`provider_relay_multipart`、`provider_presigned_multipart`）。上传服务不把 `DriverType` 硬编码到 Chunk/Complete 路径；兼容旧 row 时才根据 policy transport 和独立 staging 路径恢复 kind。kind 与 multipart 字段不一致会返回 `upload.session_corrupted`，不会静默切换到本地 chunk 文件路径。 +每个需要 session 的 Init 都会在 `upload_sessions.session_kind` 持久化 connector-owned 执行计划（例如 `offset_staging`、`stream_staging`、`provider_relay_multipart`、`provider_presigned_multipart`）。从 0.5.0 起该字段为非空强约束；上传服务不把 `DriverType` 硬编码到 Chunk/Complete 路径，也不为旧 row 推断 kind。kind 与 multipart 字段不一致会返回 `upload.session_corrupted`，不会静默切换到本地 chunk 文件路径。 本地 Chunk PUT 会先对 staging range 执行 `sync_data`，再在只含 SQL 的短 writer transaction 中 insert-only 登记 chunk receipt 并更新 `received_count`。客户端重试只校验已有 receipt，不会重写已提交 range，也不会重复计数。 -升级前已经存在的 payload-per-chunk session 仍走 legacy compatibility path：这类 session 的 `chunk_N` 是实际 payload，Complete 可能创建 `assembled`。新旧格式通过 `.offset-staging-v1` 专用路径区分，不通过 `assembled` 是否存在判断；legacy `assembled` 即使在失败后残留，也仍按 legacy session 重试。 +0.5.0 不继续 payload-per-chunk session，也不创建或复用 `assembled`。升级时若数据库仍有 null/非法 `session_kind`，迁移会直接失败并保留原行，部署方需要先清理过期 session。 完成阶段的服务端行为分两类： diff --git a/developer-docs/zh-CN/design/upload-finalization-contracts.md b/developer-docs/zh-CN/design/upload-finalization-contracts.md index 069608f53..fa5e7e822 100644 --- a/developer-docs/zh-CN/design/upload-finalization-contracts.md +++ b/developer-docs/zh-CN/design/upload-finalization-contracts.md @@ -5,7 +5,7 @@ 上传路径分成两组： - 普通 HTTP multipart 上传：入口在 `storage::multipart`，直接在一次请求内落到正式文件。 -- upload session 型上传：入口在 `upload::{init, chunk, complete}`，先持久化 `upload_session`，complete 阶段再把临时对象、分片或 assembled 文件收口成正式文件。 +- upload session 型上传：入口在 `upload::{init, chunk, complete}`，先持久化 `upload_session`，complete 阶段再把临时对象或 staging 文件收口成正式文件。 最终落文件时必须保持三个不变量： @@ -60,7 +60,7 @@ etag = aster-drive-offset-staging-receipt-v1 size = expected_chunk_size ``` -`.offset-staging-v1` 是旧 session 的兼容格式线索，但新 session 的权威格式字段是 `upload_sessions.session_kind`。Complete、Chunk PUT、Progress 和 lifecycle 先使用显式 kind；只有 `session_kind IS NULL` 的迁移前 row 才通过统一 compatibility classifier 读取 policy transport、multipart 字段和 `.offset-staging-v1`。legacy compatibility path 创建的通用 `assembled` 文件不参与判断。这个边界很重要：legacy 首次拼装后如果 storage/DB 阶段出现可重试失败，`assembled` 可能保留到下一次 Complete；若拿它判断格式，就会把 payload-sized `chunk_N` 错当成 offset receipt。 +`upload_sessions.session_kind` 是所有可操作 session 的权威数据面字段。它必须是合法的非空值；Complete、Chunk PUT、Progress 和 lifecycle 都直接校验显式 kind，不再根据临时文件、policy transport 或 `assembled` 推断路径。 ### 显式 session kind @@ -74,9 +74,8 @@ Init 根据 connector-owned `PolicyUploadTransport` 持久化执行计划，不 | `provider_presigned_single` / `remote_presigned_single` | provider temp object | presigned single complete | | `provider_presigned_multipart` / `remote_presigned_multipart` | provider multipart parts | presigned multipart complete | | `provider_direct_resumable` | provider upload session（浏览器直传 range） | provider resumable complete | -| `legacy_chunk_files` | 迁移前 `chunk_N` payload | legacy assemble/relay compatibility | -`session_kind` 在 0.5.0 前保持 nullable，以便读取升级前的 session；新 Init 永远写入非空值。显式 kind 与 multipart 字段组合不一致时，接口返回 `upload.session_corrupted`，不会降级到另一条数据面。 +从 0.5.0 起 `session_kind` 为 `NOT NULL`。升级迁移遇到 null 或非法 kind 会直接失败并保留原行；部署方需要先清理这些过期 session。显式 kind 与 multipart 字段组合不一致时，接口返回 `upload.session_corrupted`，不会降级到另一条数据面。 ### Chunk PUT 的 durable receipt 顺序 @@ -109,18 +108,7 @@ Complete 必须同时校验： Local completion 直接消费这份 staging file：开启 `content_dedup` 时会先流式计算 SHA-256，再按 content-addressed key promote；关闭 dedup 时把同一 staging file 写入预分配的独立 Blob。两种情况都不会再完整写一份 assembled 文件。需要 generic stream upload 的 connector 从 staging file 串流到目标 driver。S3-compatible、Azure Blob、Tencent COS 等已经协商到 provider relay multipart 的 session 不走这条本地 staging 路径。 -### Legacy compatibility path - -升级前创建的 session 仍可能采用 payload-per-chunk 目录： - -```text -// -├── chunk_0 # payload -├── chunk_1 # payload -└── assembled # Complete 拼装产物，失败/崩溃后可能保留 -``` - -这条路径会在 Complete 时取得 `chunk_assembly_to_local_temp_file` limiter，然后拼装或串流 legacy chunk。它只保护 legacy assembly，不限制新 `.offset-staging-v1` session。兼容路径计划在 `0.5.0` 移除；移除前必须保留“已有 assembled 仍按 legacy 重试”的回归测试。 +0.5.0 不读取或迁移 0.4.x 的 payload-per-chunk session，也不创建或复用 `assembled`。升级迁移会在发现 null/非法 `session_kind` 时停止，旧 session 的清理责任属于部署方。 ## Provider direct resumable 契约 @@ -156,7 +144,6 @@ Progress 不解本地 receipt，而是解密 `provider_session_ciphertext` 后 | local direct | 不创建 upload session；local policy 且有 `declared_size` 时直接写入 local staging path | 写入 local staging file 时累计的 `size`，必须等于 `declared_size`；dedup 时同流计算 hash | 使用已解析 local policy；和 server path 一样通过 `store_from_temp_with_hints` 做 precheck / 事务内 atomic charge | `upload_local_direct` -> `store_from_temp_with_hints` | 写入、大小不匹配、空文件或 store 结束后删除 staging file；重复请求不会通过 session 幂等，只按普通创建语义处理 | | streaming direct | 不创建 upload session；relay request body 到 driver 的 prepared non-dedup blob | driver `metadata(storage_path).size`，必须等于 `declared_size`，并再次检查 policy max file size | relay 前先用 `declared_size` 做 quota precheck；metadata 复验后再用 `actual_size` precheck；DB 事务内再次 `check_quota` 并 `update_storage_used` | `upload_streaming_direct` -> `store_preuploaded_nondedup` | storage upload、relay、metadata、size validation、quota validation 或 DB finalize 失败时 cleanup prepared blob；成功后按正式 blob 管理 | | local chunked / offset staging | session status `uploading`；Init 预创建 `.offset-staging-v1`，Chunk PUT 按 offset 写 range 并登记 DB receipt | 每块必须等于 `expected_chunk_size_for_upload`；Complete 校验全部 receipt 和 staging file 的 `session.total_size`；dedup 时从 staging 流式计算 SHA-256 | chunk receipt 与 `received_count` 在只含 SQL 的短 writer transaction 内幂等登记；最终 quota 仍由 `finalize_upload_session_blob_with_actor_username` 原子落账 | `complete_chunked_upload_with_actor_username` -> `finalize_chunked_upload_session` -> `load_offset_staging_file` -> `stage_chunked_temp_file` -> `persist_chunked_upload` | staging range 先 `sync_data`；receipt 是唯一 completion index，唯一键避免重试重复计数；Complete 成功后删除 upload temp dir | -| legacy local chunk files | 升级前 session 的 `chunk_N` 是 payload；Complete 拼写 `assembled`，generic stream connector 则依次读取 chunk | 拼装路径按实际读取字节累计 size；legacy stream 使用 session total size 作为声明值并由下游 storage contract 校验 | 最终 quota 与当前 chunked path 相同；legacy assembly limiter 只限制本地拼装写，不影响 offset staging | `assemble_legacy_local_chunks_to_temp_file` 或 `stream_legacy_local_chunks_into_writer` -> 当前 chunked persist/finalize | `assembled` 可能在 retryable storage/DB 失败后保留，但不能触发 offset-staging 判断；进入兼容路径会 warn，计划在 `0.5.0` 移除 | | presigned single | session status `presigned`；客户端 PUT 到 `object_temp_key` | complete 前读取 temp object metadata；copy 到 final key 后再次读取 final object metadata；两者都必须等于 `session.total_size` | complete 阶段没有独立 quota precheck；`finalize_upload_session_file` 在 DB 事务内创建 blob/file、atomic charge、标 completed | `complete_presigned_upload` -> `copy_presigned_object_to_final_key` -> `finalize_verified_opaque_upload_session` -> `finalize_upload_session_file` | temp object 缺失或大小不匹配会失败，大小不匹配会尝试删除 temp object；DB finalize 失败后删除 copied final object；成功后 best-effort 删除 temp object；completed retry 通过 `find_file_by_session` 返回已有文件 | | presigned object multipart | session status `presigned`；客户端直传 object multipart parts，complete 时客户端回传 parts | provider `list_uploaded_part_details` 的 part size 求和，必须等于 `session.total_size`；multipart complete 后再读 object metadata | multipart complete 前先用 part size total `check_quota`；`finalize_upload_session_file` 在 DB 事务内 atomic charge、标 completed | `complete_presigned_multipart` -> `complete_object_multipart_upload_session` -> `finalize_verified_opaque_upload_session` -> `finalize_upload_session_file` | completed parts 和 provider uploaded parts 必须连续且数量匹配；preflight size/parts/quota 失败会 abort multipart；complete 出现 retryable storage error 且 object 已存在时继续 finalize；multipart object 一旦 complete，`VerifiedUploadedBlob.cleanup = RetainCompletedMultipartObject`，因此 `finalize_upload_session_file`/DB finalize 失败后不删除已完成对象，留给后续重试或 orphan cleanup；completed retry 返回已有文件 | | relay object multipart | session status `uploading`；每个 chunk 由服务端 relay 到 object multipart，并把 part metadata 写入 `upload_session_parts` | chunk 阶段按 `expected_chunk_size_for_upload` 验每个 payload；complete 阶段读取服务端 parts 清单，再用 provider part details 求和，必须等于 `session.total_size` | chunk 阶段不 charge；complete multipart 前用 verified part total precheck；`finalize_upload_session_file` 在 DB 事务内 atomic charge、标 completed | `complete_relay_multipart` -> `complete_object_multipart_upload_session` -> `finalize_verified_opaque_upload_session` -> `finalize_upload_session_file` | part claim 防止同一 part 并发重复上传；upload 或 DB 写 part metadata 失败会 release claim；complete preflight 失败会 abort multipart；multipart object 一旦 complete，`VerifiedUploadedBlob.cleanup = RetainCompletedMultipartObject`，因此 `finalize_upload_session_file`/DB finalize 失败后不删除已完成对象，留给后续重试或 orphan cleanup；completed retry 返回已有文件 | @@ -172,7 +159,7 @@ Progress 不解本地 receipt，而是解密 `provider_session_ciphertext` 后 - `provider_presigned_multipart` / `remote_presigned_multipart` -> `CompletePresignedMultipart`，客户端必须提交 parts。 - `provider_relay_multipart` / `remote_relay_multipart` -> `CompleteRelayMultipart`，parts 来自服务端已保存的 `upload_session_parts`。 - `provider_direct_resumable` -> `CompleteProviderResumable`，parts 不存在于服务端；以 provider 侧对象 metadata 为完成依据。 -- `offset_staging` / `stream_staging` / `legacy_chunk_files` -> `CompleteChunked`，要求 `received_count == total_chunks`；只有 legacy null row 才需要 compatibility classifier。 +- `offset_staging` / `stream_staging` -> `CompleteChunked`，要求 `received_count == total_chunks`。 `run_upload_completion_stage` 会先把 expected status 切到 `assembling`。非 retryable 失败会把 session 标为 `failed`；retryable storage error 会尝试恢复到原状态，允许客户端重试。 @@ -184,5 +171,5 @@ Progress 不解本地 receipt，而是解密 `provider_session_ciphertext` 后 - `store_from_temp` 路径继续使用 `VerifiedTempStoreBlob` 或同等明确的 verified finalization input；新 temp-store 入口必须显式声明 staged dedup/preuploaded cleanup 责任。 - `store_preuploaded_nondedup` 路径继续使用 `VerifiedPreuploadedNondedupStoreBlob` 或同等明确的 verified finalization input；新 preuploaded store 入口必须显式校验 prepared blob 的 size/policy/storage path 一致性。 - 每个被迁移路径都要补 quota、size mismatch 和 DB finalize failure cleanup 测试；`completed retry 不重复计费` 只适用于 session complete flow。 -- offset-staging 改动必须覆盖：不同 chunk 确实并行、同一 chunk 确实排他、partial range 覆盖、receipt 缺失、receipt 损坏、staging 截断和 legacy assembled 残留重试。并发测试需要 barrier/failpoint 证明任务进入了关键区，不能只用 `join!` 假设发生过竞争。 +- offset-staging 改动必须覆盖：不同 chunk 确实并行、同一 chunk 确实排他、partial range 覆盖、receipt 缺失、receipt 损坏和 staging 截断。并发测试需要 barrier/failpoint 证明任务进入了关键区，不能只用 `join!` 假设发生过竞争。 - 保持 public API request/response、session status 语义和现有成功上传行为不变。 diff --git a/frontend-panel/src/services/api.generated.ts b/frontend-panel/src/services/api.generated.ts index bc8d63649..35a88eda9 100644 --- a/frontend-panel/src/services/api.generated.ts +++ b/frontend-panel/src/services/api.generated.ts @@ -8608,7 +8608,8 @@ export interface components { policy_id: number; /** Format: int32 */ received_count: number; - session_kind?: null | components["schemas"]["UploadSessionKind"]; + /** @description Explicit data-plane kind selected and persisted during upload initialization. */ + session_kind: components["schemas"]["UploadSessionKind"]; status: components["schemas"]["UploadSessionStatus"]; /** Format: int64 */ team_id?: number | null; @@ -8622,12 +8623,9 @@ export interface components { }; /** * @description Persisted data plane for an upload session. - * - * `None` is reserved for sessions created before this column existed. New sessions must always - * persist a kind so lifecycle code does not infer the data plane from nullable provider fields. * @enum {string} */ - UploadSessionKind: "offset_staging" | "stream_staging" | "provider_relay_multipart" | "provider_presigned_single" | "provider_presigned_multipart" | "remote_relay_multipart" | "remote_presigned_single" | "remote_presigned_multipart" | "provider_direct_resumable" | "legacy_chunk_files"; + UploadSessionKind: "offset_staging" | "stream_staging" | "provider_relay_multipart" | "provider_presigned_single" | "provider_presigned_multipart" | "remote_relay_multipart" | "remote_presigned_single" | "remote_presigned_multipart" | "provider_direct_resumable"; /** * @description 上传 session 状态 * @enum {string} diff --git a/migration/src/lib.rs b/migration/src/lib.rs index 4a2f79825..f72f20b6e 100644 --- a/migration/src/lib.rs +++ b/migration/src/lib.rs @@ -57,6 +57,7 @@ mod m20260713_000003_scheduled_tasks; mod m20260716_000001_bind_external_auth_login_flows; mod m20260717_000001_add_upload_session_kind; mod m20260719_000001_add_upload_provider_session; +mod m20260723_000001_require_upload_session_kind; mod search_acceleration; mod time; @@ -179,6 +180,7 @@ impl MigratorTrait for CurrentMigrator { Box::new(m20260716_000001_bind_external_auth_login_flows::Migration), Box::new(m20260717_000001_add_upload_session_kind::Migration), Box::new(m20260719_000001_add_upload_provider_session::Migration), + Box::new(m20260723_000001_require_upload_session_kind::Migration), ] } } diff --git a/migration/src/m20260723_000001_require_upload_session_kind.rs b/migration/src/m20260723_000001_require_upload_session_kind.rs new file mode 100644 index 000000000..b41799d53 --- /dev/null +++ b/migration/src/m20260723_000001_require_upload_session_kind.rs @@ -0,0 +1,342 @@ +//! Require every upload session to carry an explicit data-plane kind. + +use sea_orm_migration::prelude::*; +use sea_orm_migration::sea_orm::{ConnectionTrait, DbBackend, Statement}; + +#[derive(DeriveMigrationName)] +pub struct Migration; + +const SQLITE_REBUILT_TABLE: &str = "upload_sessions__session_kind_rebuild"; +const VALID_SESSION_KINDS: [&str; 9] = [ + "offset_staging", + "stream_staging", + "provider_relay_multipart", + "provider_presigned_single", + "provider_presigned_multipart", + "remote_relay_multipart", + "remote_presigned_single", + "remote_presigned_multipart", + "provider_direct_resumable", +]; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + reject_legacy_sessions(manager).await?; + set_session_kind_nullability(manager, false).await + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + set_session_kind_nullability(manager, true).await + } +} + +async fn reject_legacy_sessions(manager: &SchemaManager<'_>) -> Result<(), DbErr> { + let valid_values = VALID_SESSION_KINDS + .iter() + .map(|value| format!("'{value}'")) + .collect::>() + .join(", "); + let sql = format!( + "SELECT COUNT(*) FROM upload_sessions \ + WHERE session_kind IS NULL OR session_kind NOT IN ({valid_values})" + ); + let row = manager + .get_connection() + .query_one_raw(Statement::from_string(manager.get_database_backend(), sql)) + .await? + .ok_or_else(|| DbErr::Migration("upload session validation returned no row".to_string()))?; + let invalid_count = row.try_get_by_index::(0).map_err(|error| { + DbErr::Migration(format!( + "failed to decode invalid upload session count: {error}" + )) + })?; + if invalid_count > 0 { + return Err(DbErr::Migration(format!( + "cannot require upload_sessions.session_kind: {invalid_count} legacy or invalid upload session(s) remain; remove them before upgrading to 0.5.0" + ))); + } + Ok(()) +} + +async fn set_session_kind_nullability( + manager: &SchemaManager<'_>, + nullable: bool, +) -> Result<(), DbErr> { + match manager.get_database_backend() { + DbBackend::Sqlite => rebuild_sqlite_upload_sessions(manager, nullable).await, + DbBackend::Postgres | DbBackend::MySql => { + let mut column = ColumnDef::new(UploadSessions::SessionKind); + column.string_len(32); + if nullable { + column.null(); + } else { + column.not_null(); + } + manager + .alter_table( + Table::alter() + .table(UploadSessions::Table) + .modify_column(column) + .to_owned(), + ) + .await + } + backend => Err(DbErr::Migration(format!( + "unsupported database backend for upload session kind migration: {backend:?}" + ))), + } +} + +async fn rebuild_sqlite_upload_sessions( + manager: &SchemaManager<'_>, + nullable: bool, +) -> Result<(), DbErr> { + let connection = manager.get_connection(); + connection + .execute_unprepared("PRAGMA foreign_keys = OFF") + .await?; + + let rebuild_result = async { + manager + .drop_table( + Table::drop() + .table(Alias::new(SQLITE_REBUILT_TABLE)) + .if_exists() + .to_owned(), + ) + .await?; + manager + .create_table(upload_sessions_table( + Alias::new(SQLITE_REBUILT_TABLE), + nullable, + )) + .await?; + + let columns = upload_session_columns(); + let mut select = Query::select(); + select.columns(columns).from(UploadSessions::Table); + let mut insert = Query::insert(); + insert + .into_table(Alias::new(SQLITE_REBUILT_TABLE)) + .columns(columns) + .select_from(select) + .map_err(|error| { + DbErr::Migration(format!("failed to build upload session data copy: {error}")) + })?; + manager.execute(insert).await?; + + manager + .drop_table(Table::drop().table(UploadSessions::Table).to_owned()) + .await?; + manager + .rename_table( + Table::rename() + .table(Alias::new(SQLITE_REBUILT_TABLE), UploadSessions::Table) + .to_owned(), + ) + .await?; + create_upload_session_indexes(manager).await + } + .await; + + let restore_result = connection + .execute_unprepared("PRAGMA foreign_keys = ON") + .await; + rebuild_result?; + restore_result?; + + let violations = connection + .query_all_raw(Statement::from_string( + DbBackend::Sqlite, + "PRAGMA foreign_key_check", + )) + .await?; + if !violations.is_empty() { + return Err(DbErr::Migration(format!( + "upload session table rebuild introduced {} foreign key violation(s)", + violations.len() + ))); + } + Ok(()) +} + +fn upload_sessions_table(table: T, nullable: bool) -> TableCreateStatement +where + T: IntoIden, +{ + let mut session_kind = ColumnDef::new(UploadSessions::SessionKind); + session_kind.string_len(32); + if nullable { + session_kind.null(); + } else { + session_kind.not_null(); + } + + Table::create() + .table(table) + .col( + ColumnDef::new(UploadSessions::Id) + .string_len(36) + .not_null() + .primary_key(), + ) + .col( + ColumnDef::new(UploadSessions::UserId) + .big_integer() + .not_null(), + ) + .col(ColumnDef::new(UploadSessions::TeamId).big_integer().null()) + .col( + ColumnDef::new(UploadSessions::FrontendClientId) + .string_len(36) + .null(), + ) + .col( + ColumnDef::new(UploadSessions::Filename) + .string_len(255) + .not_null(), + ) + .col( + ColumnDef::new(UploadSessions::TotalSize) + .big_integer() + .not_null(), + ) + .col( + ColumnDef::new(UploadSessions::ChunkSize) + .big_integer() + .not_null(), + ) + .col( + ColumnDef::new(UploadSessions::TotalChunks) + .integer() + .not_null(), + ) + .col( + ColumnDef::new(UploadSessions::ReceivedCount) + .integer() + .not_null() + .default(0), + ) + .col( + ColumnDef::new(UploadSessions::FolderId) + .big_integer() + .null(), + ) + .col( + ColumnDef::new(UploadSessions::PolicyId) + .big_integer() + .not_null(), + ) + .col( + ColumnDef::new(UploadSessions::Status) + .string_len(16) + .not_null() + .default("uploading"), + ) + .col(session_kind) + .col(ColumnDef::new(UploadSessions::ObjectTempKey).text().null()) + .col( + ColumnDef::new(UploadSessions::ObjectMultipartId) + .text() + .null(), + ) + .col( + ColumnDef::new(UploadSessions::ProviderSessionCiphertext) + .text() + .null(), + ) + .col(ColumnDef::new(UploadSessions::FileId).big_integer().null()) + .col(ColumnDef::new(UploadSessions::CreatedAt).text().not_null()) + .col(ColumnDef::new(UploadSessions::ExpiresAt).text().not_null()) + .col(ColumnDef::new(UploadSessions::UpdatedAt).text().not_null()) + .foreign_key( + ForeignKey::create() + .from(Alias::new(SQLITE_REBUILT_TABLE), UploadSessions::UserId) + .to(Users::Table, Users::Id) + .on_delete(ForeignKeyAction::Cascade), + ) + .to_owned() +} + +const fn upload_session_columns() -> [UploadSessions; 20] { + [ + UploadSessions::Id, + UploadSessions::UserId, + UploadSessions::TeamId, + UploadSessions::FrontendClientId, + UploadSessions::Filename, + UploadSessions::TotalSize, + UploadSessions::ChunkSize, + UploadSessions::TotalChunks, + UploadSessions::ReceivedCount, + UploadSessions::FolderId, + UploadSessions::PolicyId, + UploadSessions::Status, + UploadSessions::SessionKind, + UploadSessions::ObjectTempKey, + UploadSessions::ObjectMultipartId, + UploadSessions::ProviderSessionCiphertext, + UploadSessions::FileId, + UploadSessions::CreatedAt, + UploadSessions::ExpiresAt, + UploadSessions::UpdatedAt, + ] +} + +async fn create_upload_session_indexes(manager: &SchemaManager<'_>) -> Result<(), DbErr> { + for index in [ + Index::create() + .name("idx_upload_sessions_team_id") + .table(UploadSessions::Table) + .col(UploadSessions::TeamId) + .to_owned(), + Index::create() + .name("idx_upload_sessions_status_expires_at") + .table(UploadSessions::Table) + .col(UploadSessions::Status) + .col(UploadSessions::ExpiresAt) + .to_owned(), + Index::create() + .name("idx_upload_sessions_frontend_client") + .table(UploadSessions::Table) + .col(UploadSessions::UserId) + .col(UploadSessions::TeamId) + .col(UploadSessions::FrontendClientId) + .to_owned(), + ] { + manager.create_index(index).await?; + } + Ok(()) +} + +#[derive(DeriveIden, Clone, Copy)] +enum UploadSessions { + Table, + Id, + UserId, + TeamId, + FrontendClientId, + Filename, + TotalSize, + ChunkSize, + TotalChunks, + ReceivedCount, + FolderId, + PolicyId, + Status, + SessionKind, + ObjectTempKey, + ObjectMultipartId, + ProviderSessionCiphertext, + FileId, + CreatedAt, + ExpiresAt, + UpdatedAt, +} + +#[derive(DeriveIden)] +enum Users { + Table, + Id, +} diff --git a/src/api/routes/files/access.rs b/src/api/routes/files/access.rs index 58c3798da..ee78e9820 100644 --- a/src/api/routes/files/access.rs +++ b/src/api/routes/files/access.rs @@ -1262,7 +1262,6 @@ mod tests { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; (state, user, file) diff --git a/src/api/routes/frontend.rs b/src/api/routes/frontend.rs index c62a44107..e00613033 100644 --- a/src/api/routes/frontend.rs +++ b/src/api/routes/frontend.rs @@ -320,7 +320,6 @@ mod tests { share_download_rollback, background_task_dispatch_wakeup: PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), } } diff --git a/src/api/routes/health.rs b/src/api/routes/health.rs index cb1aeea83..73ff32f2a 100644 --- a/src/api/routes/health.rs +++ b/src/api/routes/health.rs @@ -289,7 +289,6 @@ mod tests { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), } } diff --git a/src/api/routes/share_public.rs b/src/api/routes/share_public.rs index c9308e1ef..796a7de7f 100644 --- a/src/api/routes/share_public.rs +++ b/src/api/routes/share_public.rs @@ -1393,7 +1393,6 @@ mod tests { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; (state, user, file, folder) diff --git a/src/entities/upload_session.rs b/src/entities/upload_session.rs index 879767046..3c14bb6cc 100644 --- a/src/entities/upload_session.rs +++ b/src/entities/upload_session.rs @@ -27,9 +27,8 @@ pub struct Model { pub folder_id: Option, pub policy_id: i64, pub status: UploadSessionStatus, - /// Explicit data-plane kind. Null is only for pre-migration sessions resolved by compatibility - /// classification; every new session persists this value at init. - pub session_kind: Option, + /// Explicit data-plane kind selected and persisted during upload initialization. + pub session_kind: UploadSessionKind, /// Driver-agnostic temporary object key used by object/presigned multipart upload flows. pub object_temp_key: Option, /// Driver-agnostic multipart upload id; empty for direct/stream upload transports. diff --git a/src/runtime/mod.rs b/src/runtime/mod.rs index 86202295e..f37658b5a 100644 --- a/src/runtime/mod.rs +++ b/src/runtime/mod.rs @@ -9,8 +9,7 @@ pub mod tasks; use crate::config::{Config, RuntimeConfig}; use crate::metrics::SharedMetricsRecorder; use crate::services::{ - events::storage_change::StorageChangeEvent, files::upload::UploadRuntime, - share::ShareDownloadRollbackQueue, + events::storage_change::StorageChangeEvent, share::ShareDownloadRollbackQueue, }; use crate::storage::{DriverRegistry, PolicySnapshot, remote_protocol::RemoteProtocolRuntime}; use aster_forge_db::DbHandles; @@ -38,8 +37,6 @@ pub struct PrimaryAppState { pub background_task_dispatch_wakeup: Arc, /// Remote storage protocol runtime, including reverse tunnel state. pub remote_protocol: Arc, - /// Shared execution resources for upload workflows. - pub upload_runtime: Arc, } #[derive(Clone)] @@ -97,10 +94,6 @@ impl PrimaryAppState { Arc::new(RemoteProtocolRuntime::new()) } - pub fn new_upload_runtime() -> Arc { - Arc::new(UploadRuntime::new()) - } - pub fn sqlite_read_write_split(&self) -> bool { self.db_handles.sqlite_read_write_split() } @@ -361,7 +354,6 @@ mod tests { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; state .driver_registry @@ -417,12 +409,4 @@ mod tests { .await .expect("background dispatcher wakeup should notify waiters"); } - - #[tokio::test] - async fn primary_state_clones_share_upload_runtime() { - let state = setup_state().await; - let cloned = state.clone(); - - assert!(Arc::ptr_eq(&state.upload_runtime, &cloned.upload_runtime)); - } } diff --git a/src/runtime/startup/primary.rs b/src/runtime/startup/primary.rs index 5a5b564b6..ac15ba855 100644 --- a/src/runtime/startup/primary.rs +++ b/src/runtime/startup/primary.rs @@ -56,7 +56,6 @@ pub async fn prepare_primary() -> Result { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol, - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }, share_download_rollback_worker, }) diff --git a/src/runtime/tasks.rs b/src/runtime/tasks.rs index 9ac6b5194..21ccd39e4 100644 --- a/src/runtime/tasks.rs +++ b/src/runtime/tasks.rs @@ -729,7 +729,6 @@ pub(crate) mod test_support { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }) } } diff --git a/src/services/files/file/deletion/tests.rs b/src/services/files/file/deletion/tests.rs index def8392f2..9b9792776 100644 --- a/src/services/files/file/deletion/tests.rs +++ b/src/services/files/file/deletion/tests.rs @@ -230,7 +230,6 @@ async fn build_deletion_test_state() -> ( background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; (state, user, policy, driver) diff --git a/src/services/files/file/download/tests.rs b/src/services/files/file/download/tests.rs index 305d4949b..90d34451a 100644 --- a/src/services/files/file/download/tests.rs +++ b/src/services/files/file/download/tests.rs @@ -396,7 +396,6 @@ where background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; let blob = file_repo::create_blob( diff --git a/src/services/files/file/resource_handle.rs b/src/services/files/file/resource_handle.rs index b2b411603..d8110cc75 100644 --- a/src/services/files/file/resource_handle.rs +++ b/src/services/files/file/resource_handle.rs @@ -639,7 +639,6 @@ mod tests { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; let blob = file_repo::create_blob( diff --git a/src/services/files/lock/tests.rs b/src/services/files/lock/tests.rs index a781d1338..c2201741d 100644 --- a/src/services/files/lock/tests.rs +++ b/src/services/files/lock/tests.rs @@ -167,7 +167,6 @@ async fn build_lock_test_state() -> (PrimaryAppState, user::Model, file::Model) background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; (state, user, file) diff --git a/src/services/files/upload/chunk.rs b/src/services/files/upload/chunk.rs index 6eb3ea1e3..06f640a06 100644 --- a/src/services/files/upload/chunk.rs +++ b/src/services/files/upload/chunk.rs @@ -7,7 +7,6 @@ //! offset-staging 的提交边界是：先 `sync_data` 内容，再在只包含 SQL 的短 DB 事务中登记 //! receipt/增加 `received_count`。receipt 是唯一 completion index；重试只需校验旧 receipt， //! 不会再次写文件或重复计数。 -//! 没有 `.offset-staging-v1` 的旧 session 继续使用独立 payload-sized chunk 文件。 use aster_forge_db::transaction; use bytes::Bytes; @@ -34,7 +33,7 @@ use crate::services::files::upload::shared::{ }; use crate::services::files::upload::staging; use crate::types::UploadSessionStatus; -use aster_forge_utils::numbers::{i64_to_u64, usize_to_i64}; +use aster_forge_utils::numbers::usize_to_i64; use aster_forge_utils::paths; const RELAY_STREAM_PIPE_BUFFER_SIZE: usize = 64 * 1024; @@ -46,13 +45,6 @@ static STAGING_WRITE_TEST_HOOKS: LazyLock< >, > = LazyLock::new(|| StdMutex::new(std::collections::HashMap::new())); -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum ExistingLocalChunk { - Missing, - Complete, - RemovedCorrupt, -} - struct LocalChunkWriteLock { file: std::fs::File, } @@ -196,101 +188,6 @@ async fn has_staged_chunk_receipt( Ok(true) } -async fn remove_local_chunk_file(path: &str, upload_id: &str, chunk_number: i32, reason: &str) { - match tokio::fs::remove_file(path).await { - Ok(()) => {} - Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} - Err(error) => { - tracing::warn!( - upload_id, - chunk_number, - path, - "failed to remove local chunk file after {reason}: {error}" - ); - } - } -} - -async fn inspect_existing_local_chunk( - chunk_path: &str, - expected_size: i64, - upload_id: &str, - chunk_number: i32, -) -> Result { - let metadata = match tokio::fs::metadata(chunk_path).await { - Ok(metadata) => metadata, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => { - return Ok(ExistingLocalChunk::Missing); - } - Err(error) => { - return Err(chunk_upload_error_with_code( - ApiErrorCode::UploadChunkPersistFailed, - format!("stat existing chunk file: {error}"), - )); - } - }; - - let expected_size = i64_to_u64(expected_size, "expected chunk size")?; - if metadata.is_file() && metadata.len() == expected_size { - return Ok(ExistingLocalChunk::Complete); - } - - tracing::warn!( - upload_id, - chunk_number, - chunk_path, - actual_size = metadata.len(), - expected_size, - is_file = metadata.is_file(), - "removing corrupt local upload chunk" - ); - remove_local_chunk_file(chunk_path, upload_id, chunk_number, "corrupt local chunk").await; - Ok(ExistingLocalChunk::RemovedCorrupt) -} - -async fn write_local_chunk_temp( - temp_path: &str, - data: &[u8], - upload_id: &str, - chunk_number: i32, -) -> Result<()> { - use tokio::fs::OpenOptions; - use tokio::io::AsyncWriteExt; - - let write_result = async { - let mut file = OpenOptions::new() - .write(true) - .create_new(true) - .open(temp_path) - .await - .map_err(|error| { - chunk_upload_error_with_code( - ApiErrorCode::UploadChunkPersistFailed, - format!("create temp chunk file: {error}"), - ) - })?; - - file.write_all(data) - .await - .map_aster_err_ctx("write chunk", |message| { - chunk_upload_error_with_code(ApiErrorCode::UploadChunkPersistFailed, message) - })?; - file.flush() - .await - .map_aster_err_ctx("flush chunk", |message| { - chunk_upload_error_with_code(ApiErrorCode::UploadChunkPersistFailed, message) - })?; - Ok::<(), AsterError>(()) - } - .await; - - if write_result.is_err() { - remove_local_chunk_file(temp_path, upload_id, chunk_number, "temp chunk write error").await; - } - - write_result -} - async fn write_chunk_to_staging_file( state: &PrimaryAppState, session: &upload_session::Model, @@ -398,59 +295,6 @@ async fn drain_chunk_payload_exact_size( ensure_chunk_body_exact_size(size, expected_size, chunk_number) } -async fn write_local_chunk_temp_stream( - temp_path: &str, - payload: &mut actix_web::web::Payload, - expected_size: i64, - upload_id: &str, - chunk_number: i32, -) -> Result<()> { - use tokio::fs::OpenOptions; - use tokio::io::BufWriter; - - let write_result = async { - let file = OpenOptions::new() - .write(true) - .create_new(true) - .open(temp_path) - .await - .map_err(|error| { - chunk_upload_error_with_code( - ApiErrorCode::UploadChunkPersistFailed, - format!("create temp chunk file: {error}"), - ) - })?; - let mut file = BufWriter::new(file); - let mut size = 0i64; - - while let Some(chunk) = payload.next().await { - let chunk = chunk.map_aster_err_with(chunk_body_read_failed)?; - size = add_chunk_body_len(size, chunk.len())?; - ensure_chunk_body_not_too_large(size, expected_size, chunk_number)?; - file.write_all(&chunk) - .await - .map_aster_err_ctx("write chunk", |message| { - chunk_upload_error_with_code(ApiErrorCode::UploadChunkPersistFailed, message) - })?; - } - - ensure_chunk_body_exact_size(size, expected_size, chunk_number)?; - file.flush() - .await - .map_aster_err_ctx("flush chunk", |message| { - chunk_upload_error_with_code(ApiErrorCode::UploadChunkPersistFailed, message) - })?; - Ok::<(), AsterError>(()) - } - .await; - - if write_result.is_err() { - remove_local_chunk_file(temp_path, upload_id, chunk_number, "temp chunk write error").await; - } - - write_result -} - async fn write_chunk_payload_to_staging_file( state: &PrimaryAppState, session: &upload_session::Model, @@ -599,65 +443,6 @@ fn prioritize_multipart_part_results( } } -async fn publish_local_chunk_temp( - temp_path: &str, - chunk_path: &str, - expected_size: i64, - upload_id: &str, - chunk_number: i32, -) -> Result { - for _ in 0..2 { - match tokio::fs::hard_link(temp_path, chunk_path).await { - Ok(()) => { - remove_local_chunk_file(temp_path, upload_id, chunk_number, "chunk publish").await; - return Ok(true); - } - Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => { - match inspect_existing_local_chunk( - chunk_path, - expected_size, - upload_id, - chunk_number, - ) - .await? - { - ExistingLocalChunk::Complete => { - remove_local_chunk_file( - temp_path, - upload_id, - chunk_number, - "duplicate chunk publish", - ) - .await; - return Ok(false); - } - ExistingLocalChunk::Missing | ExistingLocalChunk::RemovedCorrupt => continue, - } - } - Err(error) => { - remove_local_chunk_file(temp_path, upload_id, chunk_number, "chunk publish error") - .await; - return Err(chunk_upload_error_with_code( - ApiErrorCode::UploadChunkPersistFailed, - format!("publish chunk file: {error}"), - )); - } - } - } - - remove_local_chunk_file( - temp_path, - upload_id, - chunk_number, - "chunk publish retry exhausted", - ) - .await; - Err(chunk_upload_error_with_code( - ApiErrorCode::UploadChunkPersistFailed, - "publish chunk file: existing chunk stayed unavailable", - )) -} - async fn upload_chunk_impl( state: &PrimaryAppState, session: upload_session::Model, @@ -691,7 +476,7 @@ async fn upload_chunk_impl( } let expected_size = expected_chunk_size_for_upload(&session, chunk_number)?; - let session_kind = resolve_upload_session_kind(state, &session).await?; + let session_kind = resolve_upload_session_kind(&session)?; if matches!( session_kind, crate::types::UploadSessionKind::ProviderPresignedSingle @@ -815,11 +600,6 @@ async fn upload_chunk_impl( }); } - let chunk_path = paths::upload_chunk_path( - &state.config().server.upload_temp_dir, - upload_id, - chunk_number, - ); let chunk_dir = paths::upload_temp_dir(&state.config().server.upload_temp_dir, upload_id); if matches!( @@ -867,73 +647,13 @@ async fn upload_chunk_impl( }); } - if session_kind == crate::types::UploadSessionKind::LegacyChunkFiles - && inspect_existing_local_chunk(&chunk_path, expected_size, upload_id, chunk_number).await? - == ExistingLocalChunk::Complete - { - let updated = upload_session_repo::find_by_id(db, upload_id).await?; - tracing::debug!( - upload_id, - chunk_number, - received_count = updated.received_count, - total_chunks = updated.total_chunks, - "skipping already uploaded chunk" - ); - return Ok(ChunkUploadResponse { - received_count: updated.received_count, - total_chunks: updated.total_chunks, - }); - } - - let temp_chunk_path = paths::temp_file_path( - &chunk_dir, - &format!( - ".chunk_{chunk_number}.{}.partial", - aster_forge_utils::id::new_uuid() + Err(crate::errors::upload_assembly_error_with_code( + ApiErrorCode::UploadSessionCorrupted, + format!( + "session kind {} does not accept server chunk PUT", + session_kind.as_str() ), - ); - - write_local_chunk_temp(&temp_chunk_path, data.as_ref(), upload_id, chunk_number).await?; - - if !publish_local_chunk_temp( - &temp_chunk_path, - &chunk_path, - expected_size, - upload_id, - chunk_number, - ) - .await? - { - let updated = upload_session_repo::find_by_id(db, upload_id).await?; - tracing::debug!( - upload_id, - chunk_number, - received_count = updated.received_count, - total_chunks = updated.total_chunks, - "skipping already uploaded chunk" - ); - return Ok(ChunkUploadResponse { - received_count: updated.received_count, - total_chunks: updated.total_chunks, - }); - } - - // 本地 chunk 模式的幂等语义靠最终 chunk 路径的无覆盖发布保证： - // 同一块重复上传不会覆盖旧文件，而是直接回读 session 进度返回给客户端。 - increment_session_received_count(db, upload_id).await?; - - let updated = upload_session_repo::find_by_id(db, upload_id).await?; - tracing::debug!( - upload_id, - chunk_number, - received_count = updated.received_count, - total_chunks = updated.total_chunks, - "stored upload chunk" - ); - Ok(ChunkUploadResponse { - received_count: updated.received_count, - total_chunks: updated.total_chunks, - }) + )) } async fn upload_chunk_payload_impl( @@ -968,7 +688,7 @@ async fn upload_chunk_payload_impl( } let expected_size = expected_chunk_size_for_upload(&session, chunk_number)?; - let session_kind = resolve_upload_session_kind(state, &session).await?; + let session_kind = resolve_upload_session_kind(&session)?; if matches!( session_kind, crate::types::UploadSessionKind::ProviderPresignedSingle @@ -1089,11 +809,6 @@ async fn upload_chunk_payload_impl( }); } - let chunk_path = paths::upload_chunk_path( - &state.config().server.upload_temp_dir, - upload_id, - chunk_number, - ); let chunk_dir = paths::upload_temp_dir(&state.config().server.upload_temp_dir, upload_id); if matches!( @@ -1149,79 +864,13 @@ async fn upload_chunk_payload_impl( }); } - if session_kind == crate::types::UploadSessionKind::LegacyChunkFiles - && inspect_existing_local_chunk(&chunk_path, expected_size, upload_id, chunk_number).await? - == ExistingLocalChunk::Complete - { - drain_chunk_payload_exact_size(&mut payload, expected_size, chunk_number).await?; - let updated = upload_session_repo::find_by_id(db, upload_id).await?; - tracing::debug!( - upload_id, - chunk_number, - received_count = updated.received_count, - total_chunks = updated.total_chunks, - "skipping already uploaded chunk" - ); - return Ok(ChunkUploadResponse { - received_count: updated.received_count, - total_chunks: updated.total_chunks, - }); - } - - let temp_chunk_path = paths::temp_file_path( - &chunk_dir, - &format!( - ".chunk_{chunk_number}.{}.partial", - aster_forge_utils::id::new_uuid() + Err(crate::errors::upload_assembly_error_with_code( + ApiErrorCode::UploadSessionCorrupted, + format!( + "session kind {} does not accept server chunk PUT", + session_kind.as_str() ), - ); - - write_local_chunk_temp_stream( - &temp_chunk_path, - &mut payload, - expected_size, - upload_id, - chunk_number, - ) - .await?; - - if !publish_local_chunk_temp( - &temp_chunk_path, - &chunk_path, - expected_size, - upload_id, - chunk_number, - ) - .await? - { - let updated = upload_session_repo::find_by_id(db, upload_id).await?; - tracing::debug!( - upload_id, - chunk_number, - received_count = updated.received_count, - total_chunks = updated.total_chunks, - "skipping already uploaded chunk" - ); - return Ok(ChunkUploadResponse { - received_count: updated.received_count, - total_chunks: updated.total_chunks, - }); - } - - increment_session_received_count(db, upload_id).await?; - - let updated = upload_session_repo::find_by_id(db, upload_id).await?; - tracing::debug!( - upload_id, - chunk_number, - received_count = updated.received_count, - total_chunks = updated.total_chunks, - "stored upload chunk" - ); - Ok(ChunkUploadResponse { - received_count: updated.received_count, - total_chunks: updated.total_chunks, - }) + )) } /// 上传单个分片 @@ -1479,7 +1128,7 @@ mod tests { folder_id: None, policy_id: 1, status: UploadSessionStatus::Uploading, - session_kind: None, + session_kind: crate::types::UploadSessionKind::ProviderRelayMultipart, object_temp_key: object_temp_key.map(str::to_string), object_multipart_id: object_multipart_id.map(str::to_string), provider_session_ciphertext: None, diff --git a/src/services/files/upload/complete/chunked.rs b/src/services/files/upload/complete/chunked.rs index c0b0f534d..9d0c08a7a 100644 --- a/src/services/files/upload/complete/chunked.rs +++ b/src/services/files/upload/complete/chunked.rs @@ -1,8 +1,6 @@ //! Server-managed chunked-upload completion. //! -//! `.offset-staging-v1` is the explicit format discriminator for current sessions. The generic -//! `assembled` path belongs only to the deprecated payload-per-chunk compatibility path and may -//! survive a retryable storage/DB failure; its presence must never select offset-staging validation. +//! `.offset-staging-v1` is the explicit format discriminator for current sessions. use chrono::Utc; use sea_orm::DbBackend; @@ -19,12 +17,9 @@ use crate::services::files::upload::shared::{ use crate::services::files::upload::staging; use crate::services::workspace::storage; use crate::storage::StorageDriver; -use crate::storage::connectors::resolve_policy_upload_transport; use crate::types::{UploadSessionKind, UploadSessionStatus}; -use aster_forge_utils::numbers::{i32_to_usize, i64_to_u64, usize_to_i64}; -use aster_forge_utils::paths; +use aster_forge_utils::numbers::{i32_to_usize, i64_to_u64}; use tokio::io::AsyncReadExt; -use tokio::io::AsyncWriteExt; use super::contract::{ VerifiedUploadSource, VerifiedUploadedBlob, cleanup_verified_upload_after_db_failure, @@ -36,15 +31,6 @@ struct ChunkedTempFile { file_hash: Option, } -fn warn_legacy_chunk_file_completion(session: &upload_session::Model, completion_mode: &str) { - tracing::warn!( - upload_id = %session.id, - completion_mode, - removal_version = "0.5.0", - "using deprecated legacy per-chunk file completion path" - ); -} - pub(super) async fn complete_chunked_upload_with_actor_username( state: &PrimaryAppState, session: upload_session::Model, @@ -86,42 +72,26 @@ async fn finalize_chunked_upload_session( session_kind: UploadSessionKind, actor_username: Option<&str>, ) -> Result { - // Only the pre-migration legacy kind may consult the connector fallback. New sessions already - // carry an explicit execution plan, so capability probing cannot redirect their completion. - let legacy_stream_relay = session_kind == UploadSessionKind::LegacyChunkFiles - && resolve_policy_upload_transport(policy)?.chunked_completion() - == crate::storage::connectors::StorageConnectorChunkedCompletion::RelayLocalChunksToStreamUpload; - if matches!(session_kind, UploadSessionKind::StreamStaging) || legacy_stream_relay { - return finalize_stream_relay_chunked_upload_session( + if session_kind == UploadSessionKind::StreamStaging { + return finalize_offset_staging_stream_relay( state, session, policy, driver, - session_kind, actor_username, ) .await; } + if session_kind != UploadSessionKind::OffsetStaging { + return Err(upload_assembly_error_with_code( + ApiErrorCode::UploadSessionCorrupted, + "chunked completion requires an offset or stream staging session", + )); + } let prepare_started_at = Instant::now(); let should_dedup = storage::local_content_dedup_enabled(policy); - let (chunked_temp, used_offset_staging, legacy_assembly_wait_elapsed_ms) = if let Some(staged) = - load_offset_staging_file(state, session, session_kind, should_dedup).await? - { - (staged, true, 0) - } else { - warn_legacy_chunk_file_completion(session, "assemble_to_local_temp_file"); - let assembly_wait_started_at = Instant::now(); - let assembly_permit = state - .upload_runtime - .acquire_chunk_assembly_to_local_temp_file() - .await?; - let assembly_wait_elapsed_ms = assembly_wait_started_at.elapsed().as_millis(); - let assembled = - assemble_legacy_local_chunks_to_temp_file(state, session, should_dedup).await?; - drop(assembly_permit); - (assembled, false, assembly_wait_elapsed_ms) - }; + let chunked_temp = load_offset_staging_file(state, session, should_dedup).await?; let prepare_elapsed_ms = prepare_started_at.elapsed().as_millis(); let stage_started_at = Instant::now(); @@ -137,8 +107,6 @@ async fn finalize_chunked_upload_session( upload_id = %session.id, file_id = file.id, size = staged_size, - used_offset_staging, - legacy_assembly_wait_elapsed_ms, prepare_elapsed_ms, stage_elapsed_ms, persist_elapsed_ms = persist_started_at.elapsed().as_millis(), @@ -200,29 +168,19 @@ async fn validate_offset_staging_file( async fn load_offset_staging_file( state: &PrimaryAppState, session: &upload_session::Model, - session_kind: UploadSessionKind, should_dedup: bool, -) -> Result> { - if !matches!( - session_kind, - UploadSessionKind::OffsetStaging | UploadSessionKind::StreamStaging - ) { - return Ok(None); - } - - // A staging kind is an explicit execution contract. Missing or malformed staging must fail - // the session rather than silently switching to the legacy `chunk_N` assembly path. +) -> Result { let path = validate_offset_staging_file(state, session).await?; let file_hash = if should_dedup { Some(hash_staging_file(&path).await?) } else { None }; - Ok(Some(ChunkedTempFile { + Ok(ChunkedTempFile { path, size: session.total_size, file_hash, - })) + }) } async fn hash_staging_file(path: &str) -> Result { @@ -252,90 +210,6 @@ async fn hash_staging_file(path: &str) -> Result { Ok(aster_forge_crypto::sha256_digest_to_hex(&hasher.finalize())) } -async fn finalize_stream_relay_chunked_upload_session( - state: &PrimaryAppState, - session: &upload_session::Model, - policy: &storage_policy::Model, - driver: &dyn StorageDriver, - session_kind: UploadSessionKind, - actor_username: Option<&str>, -) -> Result { - if session_kind == UploadSessionKind::StreamStaging { - return finalize_offset_staging_stream_relay( - state, - session, - policy, - driver, - actor_username, - ) - .await; - } - - warn_legacy_chunk_file_completion(session, "stream_local_chunk_files"); - - const CHUNK_RELAY_BUFFER_SIZE: usize = 64 * 1024; - - let prepared = storage::prepare_non_dedup_blob_upload( - policy, - session.total_size, - Some(&session.filename), - )?; - let (writer, reader) = tokio::io::duplex(CHUNK_RELAY_BUFFER_SIZE); - let relay_task = tokio::spawn(stream_legacy_local_chunks_into_writer( - state.config().server.upload_temp_dir.clone(), - session.id.clone(), - session.total_chunks, - writer, - )); - - let upload_started_at = Instant::now(); - let upload_result = storage::upload_reader_to_prepared_blob( - driver, - &prepared, - Box::new(reader), - session.total_size, - ) - .await; - let upload_elapsed_ms = upload_started_at.elapsed().as_millis(); - - let relay_result = relay_task.await.map_err(|error| { - upload_assembly_error_with_code( - ApiErrorCode::UploadChunkRelayFailed, - format!("stream chunk relay task failed: {error}"), - ) - })?; - - if let Err(error) = upload_result { - storage::cleanup_preuploaded_blob_upload( - driver, - &prepared, - "chunked upload storage write error", - ) - .await; - return Err(error); - } - if let Err(error) = relay_result { - storage::cleanup_preuploaded_blob_upload(driver, &prepared, "chunked upload relay error") - .await; - return Err(error); - } - - let persist_started_at = Instant::now(); - let verified = VerifiedUploadedBlob::preuploaded_non_dedup(prepared)?; - persist_verified_chunked_upload(state, session, driver, &verified, actor_username) - .await - .inspect(|file| { - tracing::debug!( - upload_id = %session.id, - file_id = file.id, - size = session.total_size, - upload_elapsed_ms, - persist_elapsed_ms = persist_started_at.elapsed().as_millis(), - "stream relay chunked upload finalized" - ); - }) -} - async fn finalize_offset_staging_stream_relay( state: &PrimaryAppState, session: &upload_session::Model, @@ -389,137 +263,6 @@ async fn finalize_offset_staging_stream_relay( }) } -/// Compatibility path for sessions created before offset staging was introduced. -/// Scheduled for removal in 0.5.0 after all 24-hour upload sessions have expired. -async fn stream_legacy_local_chunks_into_writer( - upload_temp_dir: String, - upload_id: String, - total_chunks: i32, - mut writer: tokio::io::DuplexStream, -) -> Result<()> { - const STREAM_BUFFER_SIZE: usize = 64 * 1024; - - let mut buffer = vec![0u8; STREAM_BUFFER_SIZE]; - for chunk_number in 0..total_chunks { - let chunk_path = paths::upload_chunk_path(&upload_temp_dir, &upload_id, chunk_number); - let mut chunk_file = tokio::fs::File::open(&chunk_path).await.map_aster_err_ctx( - &format!("open chunk {chunk_number}"), - |message| { - upload_assembly_error_with_code(ApiErrorCode::UploadAssemblyIoFailed, message) - }, - )?; - - loop { - let read = chunk_file.read(&mut buffer).await.map_aster_err_ctx( - &format!("read chunk {chunk_number}"), - |message| { - upload_assembly_error_with_code(ApiErrorCode::UploadAssemblyIoFailed, message) - }, - )?; - if read == 0 { - break; - } - writer.write_all(&buffer[..read]).await.map_aster_err_ctx( - "relay upload chunk", - |message| { - upload_assembly_error_with_code(ApiErrorCode::UploadChunkRelayFailed, message) - }, - )?; - } - } - - writer - .shutdown() - .await - .map_aster_err_ctx("shutdown stream chunk relay", |message| { - upload_assembly_error_with_code(ApiErrorCode::UploadChunkRelayFailed, message) - })?; - Ok(()) -} - -/// Compatibility path for sessions created before offset staging was introduced. -/// Scheduled for removal in 0.5.0 after all 24-hour upload sessions have expired. -async fn assemble_legacy_local_chunks_to_temp_file( - state: &PrimaryAppState, - session: &upload_session::Model, - should_dedup: bool, -) -> Result { - use sha2::{Digest, Sha256}; - use tokio::io::{AsyncReadExt, AsyncWriteExt}; - - const ASSEMBLY_BUFFER_SIZE: usize = 64 * 1024; - - let upload_id = session.id.as_str(); - let assembled_path = - paths::upload_assembled_path(&state.config().server.upload_temp_dir, upload_id); - let mut out_file = tokio::fs::File::create(&assembled_path) - .await - .map_aster_err_ctx("create assembled file", |message| { - upload_assembly_error_with_code(ApiErrorCode::UploadAssemblyIoFailed, message) - })?; - let mut hasher = should_dedup.then(Sha256::new); - let mut size: i64 = 0; - let mut buffer = vec![0u8; ASSEMBLY_BUFFER_SIZE]; - - // 本地 chunk 模式：先按顺序把所有 chunk 拼成 assembled 文件。 - // 如果 local 策略启用了 dedup，会在拼装过程中顺便流式计算 hash， - // 避免第二遍再把 assembled 文件完整读一遍。 - for i in 0..session.total_chunks { - let chunk_path = - paths::upload_chunk_path(&state.config().server.upload_temp_dir, upload_id, i); - let mut chunk_file = tokio::fs::File::open(&chunk_path).await.map_aster_err_ctx( - &format!("open chunk {i}"), - |message| { - upload_assembly_error_with_code(ApiErrorCode::UploadAssemblyIoFailed, message) - }, - )?; - - loop { - let n = chunk_file.read(&mut buffer).await.map_aster_err_ctx( - &format!("read chunk {i}"), - |message| { - upload_assembly_error_with_code(ApiErrorCode::UploadAssemblyIoFailed, message) - }, - )?; - if n == 0 { - break; - } - - let data = &buffer[..n]; - if let Some(hasher) = hasher.as_mut() { - hasher.update(data); - } - let chunk_len = usize_to_i64(n, "assembled chunk length")?; - size = size.checked_add(chunk_len).ok_or_else(|| { - upload_assembly_error_with_code( - ApiErrorCode::UploadAssemblySizeOverflow, - "assembled upload size exceeds i64 range", - ) - })?; - out_file - .write_all(data) - .await - .map_aster_err_ctx("write assembled", |message| { - upload_assembly_error_with_code(ApiErrorCode::UploadAssemblyIoFailed, message) - })?; - } - } - out_file - .flush() - .await - .map_aster_err_ctx("flush assembled", |message| { - upload_assembly_error_with_code(ApiErrorCode::UploadAssemblyIoFailed, message) - })?; - drop(out_file); - - Ok(ChunkedTempFile { - path: assembled_path, - size, - file_hash: hasher - .map(|hasher| aster_forge_crypto::sha256_digest_to_hex(&hasher.finalize())), - }) -} - async fn stage_chunked_temp_file( driver: &dyn StorageDriver, policy: &storage_policy::Model, diff --git a/src/services/files/upload/complete/mod.rs b/src/services/files/upload/complete/mod.rs index 5b5e5375e..15bc1e46f 100644 --- a/src/services/files/upload/complete/mod.rs +++ b/src/services/files/upload/complete/mod.rs @@ -1,7 +1,7 @@ //! 上传完成阶段。 //! //! 这里把各种“临时上传状态”收口成正式文件： -//! - offset staging file 校验，或兼容旧 session 的本地 chunk 文件组装 +//! - offset staging file 校验 //! - presigned 单文件确认 //! - presigned object multipart 完成 //! - relay object multipart 完成 @@ -68,19 +68,13 @@ async fn complete_upload_impl_with_hints( let upload_id = session.id.clone(); let completed_retry = session.status == UploadSessionStatus::Completed; let complete_started_at = Instant::now(); - // Check terminal states before classifying legacy provider fields. A pre-migration row can - // retain an old multipart id even after its policy snapshot has changed. let is_terminal = matches!( session.status, UploadSessionStatus::Completed | UploadSessionStatus::Assembling | UploadSessionStatus::Failed ); - let session_kind = if is_terminal { - crate::types::UploadSessionKind::LegacyChunkFiles - } else { - resolve_upload_session_kind(state, &session).await? - }; + let session_kind = resolve_upload_session_kind(&session)?; let plan = determine_completion_plan(&session, session_kind, parts)?; let plan_label = completion_plan_label(&plan); let mode = if is_terminal { diff --git a/src/services/files/upload/complete/object_multipart.rs b/src/services/files/upload/complete/object_multipart.rs index 656bebb6f..b8ae93066 100644 --- a/src/services/files/upload/complete/object_multipart.rs +++ b/src/services/files/upload/complete/object_multipart.rs @@ -213,7 +213,7 @@ pub(super) async fn finalize_verified_opaque_upload_session( verified: &VerifiedUploadedBlob, actor_username: Option<&str>, ) -> Result { - // 直传模式不会经过本地 assembled 文件，complete 阶段只负责把已经存在的对象 + // 直传模式不会经过本地临时文件，complete 阶段只负责把已经存在的对象 // 记成 blob + file，并原子更新配额和 session 状态。 let file_hash = verified.file_hash().ok_or_else(|| { upload_assembly_error_with_code( @@ -810,7 +810,7 @@ mod tests { folder_id: None, policy_id: 1, status: crate::types::UploadSessionStatus::Assembling, - session_kind: Some(crate::types::UploadSessionKind::ProviderRelayMultipart), + session_kind: crate::types::UploadSessionKind::ProviderRelayMultipart, object_temp_key: Some("temp".to_string()), object_multipart_id: Some("multipart".to_string()), provider_session_ciphertext: None, diff --git a/src/services/files/upload/complete/plan.rs b/src/services/files/upload/complete/plan.rs index c2d2f54e5..619138fed 100644 --- a/src/services/files/upload/complete/plan.rs +++ b/src/services/files/upload/complete/plan.rs @@ -61,9 +61,7 @@ pub(super) fn determine_completion_plan( Ok(CompletionPlan::CompleteRelayMultipart) } UploadSessionKind::ProviderDirectResumable => Ok(CompletionPlan::CompleteProviderResumable), - UploadSessionKind::OffsetStaging - | UploadSessionKind::StreamStaging - | UploadSessionKind::LegacyChunkFiles => { + UploadSessionKind::OffsetStaging | UploadSessionKind::StreamStaging => { if session.received_count != session.total_chunks { return Err(upload_assembly_error_with_code( ApiErrorCode::UploadIncompleteChunks, diff --git a/src/services/files/upload/complete/tests.rs b/src/services/files/upload/complete/tests.rs index 01f5b44b8..8bb8a062c 100644 --- a/src/services/files/upload/complete/tests.rs +++ b/src/services/files/upload/complete/tests.rs @@ -19,7 +19,7 @@ fn mock_session(status: UploadSessionStatus) -> upload_session::Model { folder_id: None, policy_id: 1, status, - session_kind: None, + session_kind: UploadSessionKind::OffsetStaging, object_temp_key: None, object_multipart_id: None, provider_session_ciphertext: None, diff --git a/src/services/files/upload/init/context.rs b/src/services/files/upload/init/context.rs index 680517e50..78d91abf7 100644 --- a/src/services/files/upload/init/context.rs +++ b/src/services/files/upload/init/context.rs @@ -373,7 +373,7 @@ fn upload_session_active_model( folder_id: Set(folder_id), policy_id: Set(policy_id), status: Set(status), - session_kind: Set(Some(session_kind)), + session_kind: Set(session_kind), object_temp_key: Set(object_temp_key.map(str::to_string)), object_multipart_id: Set(object_multipart_id.map(str::to_string)), provider_session_ciphertext: Set(provider_session_ciphertext.map(str::to_string)), diff --git a/src/services/files/upload/init/mod.rs b/src/services/files/upload/init/mod.rs index fb9ed8be0..ca597a5dd 100644 --- a/src/services/files/upload/init/mod.rs +++ b/src/services/files/upload/init/mod.rs @@ -152,8 +152,7 @@ async fn init_chunked_upload_session( ) -> Result { // 本地 / 其他非 direct 场景：服务端维护 upload session，并预创建格式专用的 // `.offset-staging-v1` 文件。每个 Chunk PUT 按 offset 写入并登记 DB receipt，Complete - // 只校验 receipt 和 staging 内容后推进存储和元数据；legacy `assembled` 路径不参与新 - // session 的格式判断。 + // 只校验 receipt 和 staging 内容后推进存储和元数据。 let transport = resolve_policy_upload_transport(&ctx.policy)?; let chunk_size = ctx.policy.chunk_size; let total_chunks = numbers::calc_total_chunks(ctx.total_size, chunk_size, "chunked upload")?; diff --git a/src/services/files/upload/kind.rs b/src/services/files/upload/kind.rs index f2fc10f82..c84ddc1d1 100644 --- a/src/services/files/upload/kind.rs +++ b/src/services/files/upload/kind.rs @@ -1,109 +1,17 @@ -//! Upload session data-plane classification. -//! -//! New sessions persist their kind at init. Only pre-migration rows reach the compatibility -//! branch below; keeping that inference in one place prevents completion, progress and cleanup -//! from disagreeing about the same session. +//! Upload session data-plane validation. use crate::api::api_error_code::ApiErrorCode; use crate::entities::upload_session; use crate::errors::{Result, upload_assembly_error_with_code}; -use crate::runtime::SharedRuntimeState; -use crate::services::files::upload::staging; -use crate::services::workspace::storage::{PolicyUploadTransport, resolve_policy_upload_transport}; -use crate::types::{ - ObjectStorageUploadStrategy, ProviderResumableUploadStrategy, RemoteUploadStrategy, UploadMode, - UploadSessionKind, UploadSessionStatus, -}; +use crate::types::{UploadMode, UploadSessionKind}; -pub(crate) async fn resolve_upload_session_kind( - state: &impl SharedRuntimeState, +pub(crate) fn resolve_upload_session_kind( session: &upload_session::Model, ) -> Result { - if let Some(kind) = session.session_kind { - return validate_persisted_kind(session, kind); - } - - // Rows created before session_kind existed remain readable until 0.5.0. Their provider - // fields are only a compatibility hint; local staging is identified by its dedicated path, - // never by the legacy `assembled` output. - let transport = resolve_policy_upload_transport_for_session(state, session)?; - let kind = if session.status == UploadSessionStatus::Presigned { - compatibility_presigned_kind(transport, session.object_multipart_id.is_some()) - } else if session.object_multipart_id.is_some() { - compatibility_relay_kind(transport)? - } else if staging::exists(state, &session.id).await? { - compatibility_staging_kind(transport)? - } else { - UploadSessionKind::LegacyChunkFiles - }; - - validate_persisted_kind(session, kind) -} - -fn compatibility_presigned_kind( - transport: PolicyUploadTransport, - has_multipart_id: bool, -) -> UploadSessionKind { - match (transport, has_multipart_id) { - (PolicyUploadTransport::ObjectStorage(ObjectStorageUploadStrategy::Presigned), true) => { - UploadSessionKind::ProviderPresignedMultipart - } - (PolicyUploadTransport::Remote(RemoteUploadStrategy::Presigned), true) => { - UploadSessionKind::RemotePresignedMultipart - } - (PolicyUploadTransport::ObjectStorage(ObjectStorageUploadStrategy::Presigned), false) => { - UploadSessionKind::ProviderPresignedSingle - } - (PolicyUploadTransport::Remote(RemoteUploadStrategy::Presigned), false) => { - UploadSessionKind::RemotePresignedSingle - } - // Old rows may outlive a policy snapshot change. Presigned status plus a multipart - // id remains the compatibility marker, so keep provider as the conservative default. - (_, true) => UploadSessionKind::ProviderPresignedMultipart, - (_, false) => UploadSessionKind::ProviderPresignedSingle, - } -} - -fn compatibility_relay_kind(transport: PolicyUploadTransport) -> Result { - match transport { - PolicyUploadTransport::ObjectStorage(ObjectStorageUploadStrategy::RelayStream) => { - Ok(UploadSessionKind::ProviderRelayMultipart) - } - PolicyUploadTransport::Remote(RemoteUploadStrategy::RelayStream) => { - Ok(UploadSessionKind::RemoteRelayMultipart) - } - _ => Err(corrupted( - "relay multipart session has incompatible upload transport", - )), - } -} - -fn compatibility_staging_kind(transport: PolicyUploadTransport) -> Result { - match transport { - PolicyUploadTransport::Local => Ok(UploadSessionKind::OffsetStaging), - PolicyUploadTransport::ProviderResumable(ProviderResumableUploadStrategy::ServerRelay) - | PolicyUploadTransport::Sftp => Ok(UploadSessionKind::StreamStaging), - PolicyUploadTransport::ObjectStorage(ObjectStorageUploadStrategy::RelayStream) - | PolicyUploadTransport::Remote(RemoteUploadStrategy::RelayStream) => { - Ok(UploadSessionKind::StreamStaging) - } - _ => Err(corrupted( - "local staging session has incompatible upload transport", - )), - } + validate_persisted_kind(session, session.session_kind) } -fn resolve_policy_upload_transport_for_session( - state: &impl SharedRuntimeState, - session: &upload_session::Model, -) -> Result { - let policy = state - .policy_snapshot() - .get_policy_or_err(session.policy_id)?; - resolve_policy_upload_transport(&policy) -} - -fn validate_persisted_kind( +pub(crate) fn validate_persisted_kind( session: &upload_session::Model, kind: UploadSessionKind, ) -> Result { @@ -165,18 +73,12 @@ pub(crate) fn mode_for_kind(kind: UploadSessionKind) -> UploadMode { #[cfg(test)] mod tests { - use super::{ - compatibility_presigned_kind, compatibility_relay_kind, compatibility_staging_kind, - mode_for_kind, validate_persisted_kind, - }; + use super::{mode_for_kind, validate_persisted_kind}; use crate::entities::upload_session; - use crate::services::workspace::storage::PolicyUploadTransport; - use crate::types::{ - ObjectStorageUploadStrategy, ProviderResumableUploadStrategy, RemoteUploadStrategy, - }; use crate::types::{UploadMode, UploadSessionKind, UploadSessionStatus}; fn session( + kind: UploadSessionKind, object_temp_key: Option<&str>, object_multipart_id: Option<&str>, ) -> upload_session::Model { @@ -194,7 +96,7 @@ mod tests { folder_id: None, policy_id: 1, status: UploadSessionStatus::Uploading, - session_kind: None, + session_kind: kind, object_temp_key: object_temp_key.map(str::to_string), object_multipart_id: object_multipart_id.map(str::to_string), provider_session_ciphertext: None, @@ -230,25 +132,33 @@ mod tests { } #[test] - fn persisted_kind_validation_rejects_each_missing_provider_field() { + fn persisted_kind_validation_rejects_incompatible_fields() { assert!( validate_persisted_kind( - &session(Some("files/temp"), None), - UploadSessionKind::ProviderRelayMultipart, + &session( + UploadSessionKind::ProviderRelayMultipart, + Some("files/temp"), + None + ), + UploadSessionKind::ProviderRelayMultipart ) .is_err() ); assert!( validate_persisted_kind( - &session(None, Some("multipart")), - UploadSessionKind::ProviderRelayMultipart, + &session( + UploadSessionKind::ProviderRelayMultipart, + None, + Some("multipart") + ), + UploadSessionKind::ProviderRelayMultipart ) .is_err() ); assert!( validate_persisted_kind( - &session(None, None), - UploadSessionKind::ProviderPresignedSingle, + &session(UploadSessionKind::ProviderPresignedSingle, None, None), + UploadSessionKind::ProviderPresignedSingle ) .is_err() ); @@ -256,135 +166,25 @@ mod tests { #[test] fn provider_direct_kind_requires_temp_key_and_encrypted_session_metadata() { - let mut valid = session(Some("files/temp"), None); + let mut valid = session( + UploadSessionKind::ProviderDirectResumable, + Some("files/temp"), + None, + ); valid.provider_session_ciphertext = Some("encrypted-upload-url".to_string()); assert!( validate_persisted_kind(&valid, UploadSessionKind::ProviderDirectResumable).is_ok() ); - - let missing_ciphertext = session(Some("files/temp"), None); - assert!( - validate_persisted_kind( - &missing_ciphertext, - UploadSessionKind::ProviderDirectResumable, - ) - .is_err() - ); - - let missing_temp_key = session(None, None); assert!( validate_persisted_kind( - &missing_temp_key, + &session( + UploadSessionKind::ProviderDirectResumable, + Some("files/temp"), + None + ), UploadSessionKind::ProviderDirectResumable ) .is_err() ); - - let mut relay_with_provider_metadata = session(Some("files/temp"), None); - relay_with_provider_metadata.provider_session_ciphertext = - Some("encrypted-upload-url".to_string()); - assert!( - validate_persisted_kind( - &relay_with_provider_metadata, - UploadSessionKind::StreamStaging, - ) - .is_err() - ); - } - - #[test] - fn compatibility_presigned_kind_distinguishes_provider_and_remote_variants() { - assert_eq!( - compatibility_presigned_kind( - PolicyUploadTransport::ObjectStorage(ObjectStorageUploadStrategy::Presigned), - false, - ), - UploadSessionKind::ProviderPresignedSingle - ); - assert_eq!( - compatibility_presigned_kind( - PolicyUploadTransport::ObjectStorage(ObjectStorageUploadStrategy::Presigned), - true, - ), - UploadSessionKind::ProviderPresignedMultipart - ); - assert_eq!( - compatibility_presigned_kind( - PolicyUploadTransport::Remote(RemoteUploadStrategy::Presigned), - false, - ), - UploadSessionKind::RemotePresignedSingle - ); - assert_eq!( - compatibility_presigned_kind( - PolicyUploadTransport::Remote(RemoteUploadStrategy::Presigned), - true, - ), - UploadSessionKind::RemotePresignedMultipart - ); - assert_eq!( - compatibility_presigned_kind(PolicyUploadTransport::Local, true), - UploadSessionKind::ProviderPresignedMultipart - ); - assert_eq!( - compatibility_presigned_kind(PolicyUploadTransport::Local, false), - UploadSessionKind::ProviderPresignedSingle - ); - } - - #[test] - fn compatibility_relay_kind_rejects_non_relay_transport() { - assert_eq!( - compatibility_relay_kind(PolicyUploadTransport::ObjectStorage( - ObjectStorageUploadStrategy::RelayStream, - )) - .unwrap(), - UploadSessionKind::ProviderRelayMultipart - ); - assert_eq!( - compatibility_relay_kind(PolicyUploadTransport::Remote( - RemoteUploadStrategy::RelayStream, - )) - .unwrap(), - UploadSessionKind::RemoteRelayMultipart - ); - assert!(compatibility_relay_kind(PolicyUploadTransport::Local).is_err()); - assert!( - compatibility_relay_kind(PolicyUploadTransport::ProviderResumable( - ProviderResumableUploadStrategy::ServerRelay, - )) - .is_err() - ); - } - - #[test] - fn compatibility_staging_kind_covers_local_stream_and_relay_transports() { - assert_eq!( - compatibility_staging_kind(PolicyUploadTransport::Local).unwrap(), - UploadSessionKind::OffsetStaging - ); - for transport in [ - PolicyUploadTransport::ProviderResumable(ProviderResumableUploadStrategy::ServerRelay), - PolicyUploadTransport::Sftp, - PolicyUploadTransport::ObjectStorage(ObjectStorageUploadStrategy::RelayStream), - PolicyUploadTransport::Remote(RemoteUploadStrategy::RelayStream), - ] { - assert_eq!( - compatibility_staging_kind(transport).unwrap(), - UploadSessionKind::StreamStaging - ); - } - assert!( - compatibility_staging_kind(PolicyUploadTransport::ObjectStorage( - ObjectStorageUploadStrategy::Presigned, - )) - .is_err() - ); - assert!( - compatibility_staging_kind(PolicyUploadTransport::Remote( - RemoteUploadStrategy::Presigned, - )) - .is_err() - ); } } diff --git a/src/services/files/upload/lifecycle.rs b/src/services/files/upload/lifecycle.rs index 6faa0b608..988fcfd15 100644 --- a/src/services/files/upload/lifecycle.rs +++ b/src/services/files/upload/lifecycle.rs @@ -111,7 +111,7 @@ async fn cleanup_remote_upload_state( session: &upload_session::Model, allow_corrupted_object_fields: bool, ) -> UploadRemoteCleanupOutcome { - let kind = match resolve_upload_session_kind(state, session).await { + let kind = match resolve_upload_session_kind(session) { Ok(kind) => Some(kind), Err(error) if allow_corrupted_object_fields => { // A corrupted classifier must not block administrative deletion. Recorded object @@ -130,23 +130,19 @@ async fn cleanup_remote_upload_state( return UploadRemoteCleanupOutcome::DeferredIntervention; } }; - if let Some(kind) = kind { - let has_legacy_remote_temp = - kind == UploadSessionKind::LegacyChunkFiles && session.object_temp_key.is_some(); - if !has_legacy_remote_temp - && !matches!( - kind, - UploadSessionKind::ProviderRelayMultipart - | UploadSessionKind::ProviderPresignedSingle - | UploadSessionKind::ProviderPresignedMultipart - | UploadSessionKind::RemoteRelayMultipart - | UploadSessionKind::RemotePresignedSingle - | UploadSessionKind::RemotePresignedMultipart - | UploadSessionKind::ProviderDirectResumable - ) - { - return UploadRemoteCleanupOutcome::Complete; - } + if let Some(kind) = kind + && !matches!( + kind, + UploadSessionKind::ProviderRelayMultipart + | UploadSessionKind::ProviderPresignedSingle + | UploadSessionKind::ProviderPresignedMultipart + | UploadSessionKind::RemoteRelayMultipart + | UploadSessionKind::RemotePresignedSingle + | UploadSessionKind::RemotePresignedMultipart + | UploadSessionKind::ProviderDirectResumable + ) + { + return UploadRemoteCleanupOutcome::Complete; } let Some(temp_key) = session.object_temp_key.as_deref() else { @@ -321,8 +317,7 @@ async fn upload_session_mode_label_for_cancel( match upload_session_mode_label(state, session).await { Ok(mode) => mode, Err(error) => { - // Cancellation must remain able to quarantine a corrupted legacy session. Metrics - // classification failure is recorded explicitly instead of blocking cleanup. + // Metrics classification failure is recorded explicitly instead of blocking cleanup. tracing::warn!( upload_id = %session.id, "failed to classify upload session for cancel metrics: {error}" @@ -333,10 +328,10 @@ async fn upload_session_mode_label_for_cancel( } async fn upload_session_mode_label( - state: &PrimaryAppState, + _state: &PrimaryAppState, session: &upload_session::Model, ) -> Result<&'static str> { - Ok(resolve_upload_session_kind(state, session).await?.as_str()) + Ok(resolve_upload_session_kind(session)?.as_str()) } fn record_upload_cancel_metric(state: &impl SharedRuntimeState, mode: &'static str, success: bool) { diff --git a/src/services/files/upload/mod.rs b/src/services/files/upload/mod.rs index 25c26956c..410c36ef0 100644 --- a/src/services/files/upload/mod.rs +++ b/src/services/files/upload/mod.rs @@ -12,7 +12,6 @@ mod lifecycle; mod progress; mod provider_session; mod responses; -mod runtime; mod scope; mod shared; mod staging; @@ -51,7 +50,6 @@ pub use responses::{ ChunkUploadResponse, InitUploadResponse, ProviderResumableUploadResponse, RecoverableUploadSessionResponse, UploadProgressResponse, }; -pub use runtime::UploadRuntime; #[derive(Clone, Copy)] pub(crate) struct UploadInScopeParams<'a> { diff --git a/src/services/files/upload/progress.rs b/src/services/files/upload/progress.rs index 38ee52083..b7073f621 100644 --- a/src/services/files/upload/progress.rs +++ b/src/services/files/upload/progress.rs @@ -25,7 +25,6 @@ use crate::services::files::upload::staging; use crate::services::workspace::storage; use crate::storage::StorageErrorKind; use crate::types::{UploadSessionKind, UploadSessionStatus}; -use aster_forge_utils::paths; use futures::{StreamExt, stream}; const RECOVERABLE_UPLOAD_SESSIONS_LIMIT: u64 = 100; @@ -45,7 +44,7 @@ async fn get_progress_impl( "loading upload progress" ); - let kind = resolve_upload_session_kind(state, &session).await?; + let kind = resolve_upload_session_kind(&session)?; let (chunks_on_disk, provider_resumable) = match kind { UploadSessionKind::ProviderPresignedMultipart | UploadSessionKind::RemotePresignedMultipart => { @@ -81,11 +80,8 @@ async fn get_progress_impl( UploadSessionKind::OffsetStaging | UploadSessionKind::StreamStaging => { (list_offset_staging_chunks(state, &session).await?, None) } - UploadSessionKind::LegacyChunkFiles => { - (scan_received_chunks(state, &session.id).await, None) - } UploadSessionKind::ProviderPresignedSingle | UploadSessionKind::RemotePresignedSingle => { - (scan_received_chunks(state, &session.id).await, None) + (Vec::new(), None) } UploadSessionKind::ProviderDirectResumable => { let secret = decrypt_provider_session(state, &session)?; @@ -219,7 +215,7 @@ async fn recoverable_session_response( state: &PrimaryAppState, session: upload_session::Model, ) -> Result { - let mode = mode_for_kind(resolve_upload_session_kind(state, &session).await?); + let mode = mode_for_kind(resolve_upload_session_kind(&session)?); let progress = get_progress_impl(state, session.clone()).await?; let completed_parts = upload_session_part_repo::list_by_upload(state.reader_db(), &session.id) .await? @@ -476,31 +472,11 @@ pub async fn presign_parts_for_team( presign_parts_impl(state, session, part_numbers).await } -/// 扫描临时目录中实际存在的 chunk 文件，返回排序后的 chunk 编号列表 -async fn scan_received_chunks(state: &PrimaryAppState, upload_id: &str) -> Vec { - let dir = paths::upload_temp_dir(&state.config().server.upload_temp_dir, upload_id); - let mut received = Vec::new(); - let Ok(mut entries) = tokio::fs::read_dir(&dir).await else { - return received; - }; - while let Ok(Some(entry)) = entries.next_entry().await { - let name = entry.file_name(); - let name = name.to_string_lossy(); - if let Some(num_str) = name.strip_prefix("chunk_") - && let Ok(n) = num_str.parse::() - { - received.push(n); - } - } - received.sort(); - received -} - #[cfg(test)] mod tests { use super::presigned_multipart_fields; use crate::entities::upload_session; - use crate::types::UploadSessionStatus; + use crate::types::{UploadSessionKind, UploadSessionStatus}; fn session( object_temp_key: Option<&str>, @@ -520,7 +496,7 @@ mod tests { folder_id: None, policy_id: 1, status: UploadSessionStatus::Presigned, - session_kind: None, + session_kind: UploadSessionKind::OffsetStaging, object_temp_key: object_temp_key.map(str::to_string), object_multipart_id: object_multipart_id.map(str::to_string), provider_session_ciphertext: None, diff --git a/src/services/files/upload/runtime.rs b/src/services/files/upload/runtime.rs deleted file mode 100644 index 015fb7a9c..000000000 --- a/src/services/files/upload/runtime.rs +++ /dev/null @@ -1,178 +0,0 @@ -//! Upload execution resources owned by the primary application runtime. - -use std::sync::Arc; - -use tokio::sync::{OwnedSemaphorePermit, Semaphore}; - -use crate::errors::{AsterError, Result}; - -const CHUNK_ASSEMBLY_TO_LOCAL_TEMP_FILE_CONCURRENCY: usize = 1; - -#[derive(Debug)] -pub struct UploadRuntime { - chunk_assembly_to_local_temp_file: Arc, -} - -impl UploadRuntime { - pub fn new() -> Self { - Self { - chunk_assembly_to_local_temp_file: Arc::new(Semaphore::new( - CHUNK_ASSEMBLY_TO_LOCAL_TEMP_FILE_CONCURRENCY, - )), - } - } - - pub(crate) async fn acquire_chunk_assembly_to_local_temp_file( - &self, - ) -> Result { - self.chunk_assembly_to_local_temp_file - .clone() - .acquire_owned() - .await - .map_err(|error| { - AsterError::internal_error(format!( - "chunk assembly to local temp file limiter closed: {error}" - )) - }) - } -} - -impl Default for UploadRuntime { - fn default() -> Self { - Self::new() - } -} - -#[cfg(test)] -mod tests { - use std::sync::Arc; - use std::sync::atomic::{AtomicUsize, Ordering}; - use std::time::Duration; - - use super::UploadRuntime; - - fn update_peak(peak: &AtomicUsize, current: usize) { - let mut observed = peak.load(Ordering::SeqCst); - while current > observed { - match peak.compare_exchange(observed, current, Ordering::SeqCst, Ordering::SeqCst) { - Ok(_) => break, - Err(actual) => observed = actual, - } - } - } - - #[tokio::test] - async fn chunk_assembly_to_local_temp_file_is_serialized() { - let runtime = UploadRuntime::new(); - let first = runtime - .acquire_chunk_assembly_to_local_temp_file() - .await - .expect("first assembly permit should be available"); - - assert!( - tokio::time::timeout( - Duration::from_millis(20), - runtime.acquire_chunk_assembly_to_local_temp_file(), - ) - .await - .is_err(), - "second assembly should wait while the first permit is held" - ); - - drop(first); - - let second = tokio::time::timeout( - Duration::from_millis(100), - runtime.acquire_chunk_assembly_to_local_temp_file(), - ) - .await - .expect("assembly permit should be released by RAII") - .expect("assembly limiter should remain open"); - drop(second); - } - - #[tokio::test] - async fn concurrent_chunk_assemblies_to_local_temp_file_never_exceed_one() { - let runtime = Arc::new(UploadRuntime::new()); - let active = Arc::new(AtomicUsize::new(0)); - let peak = Arc::new(AtomicUsize::new(0)); - let mut tasks = Vec::new(); - - for _ in 0..8 { - let runtime = runtime.clone(); - let active = active.clone(); - let peak = peak.clone(); - tasks.push(tokio::spawn(async move { - let _permit = runtime - .acquire_chunk_assembly_to_local_temp_file() - .await - .expect("assembly permit should be available eventually"); - let current = active.fetch_add(1, Ordering::SeqCst) + 1; - update_peak(&peak, current); - tokio::time::sleep(Duration::from_millis(5)).await; - active.fetch_sub(1, Ordering::SeqCst); - })); - } - - for task in tasks { - task.await.expect("assembly task should not panic"); - } - - assert_eq!(active.load(Ordering::SeqCst), 0); - assert_eq!(peak.load(Ordering::SeqCst), 1); - } - - #[tokio::test] - async fn cancelled_waiter_does_not_consume_the_next_permit() { - let runtime = Arc::new(UploadRuntime::new()); - let first = runtime - .acquire_chunk_assembly_to_local_temp_file() - .await - .expect("first assembly permit should be available"); - - let waiting_runtime = runtime.clone(); - let waiter = tokio::spawn(async move { - waiting_runtime - .acquire_chunk_assembly_to_local_temp_file() - .await - .expect("cancelled waiter should only fail by cancellation") - }); - tokio::task::yield_now().await; - waiter.abort(); - let cancelled = waiter.await; - assert!(cancelled.is_err_and(|error| error.is_cancelled())); - - drop(first); - - let next = tokio::time::timeout( - Duration::from_millis(100), - runtime.acquire_chunk_assembly_to_local_temp_file(), - ) - .await - .expect("cancelled waiter should not block the next assembly") - .expect("assembly limiter should remain open"); - drop(next); - } - - #[tokio::test] - async fn permit_is_released_when_guarded_work_fails() { - let runtime = UploadRuntime::new(); - let guarded_result = async { - let _permit = runtime.acquire_chunk_assembly_to_local_temp_file().await?; - Err::<(), crate::errors::AsterError>(crate::errors::AsterError::internal_error( - "synthetic assembly failure", - )) - } - .await; - assert!(guarded_result.is_err()); - - let next = tokio::time::timeout( - Duration::from_millis(100), - runtime.acquire_chunk_assembly_to_local_temp_file(), - ) - .await - .expect("failed guarded work should release its permit") - .expect("assembly limiter should remain open"); - drop(next); - } -} diff --git a/src/services/files/upload/shared.rs b/src/services/files/upload/shared.rs index 73b002f09..b34087ef4 100644 --- a/src/services/files/upload/shared.rs +++ b/src/services/files/upload/shared.rs @@ -491,7 +491,7 @@ mod tests { folder_id: None, policy_id: 1, status: UploadSessionStatus::Uploading, - session_kind: None, + session_kind: crate::types::UploadSessionKind::OffsetStaging, object_temp_key: None, object_multipart_id: None, provider_session_ciphertext: None, diff --git a/src/services/files/upload/staging.rs b/src/services/files/upload/staging.rs index 12ecb37e2..e7dbd8d8d 100644 --- a/src/services/files/upload/staging.rs +++ b/src/services/files/upload/staging.rs @@ -1,14 +1,12 @@ //! Local staging-file contract for server-managed chunked uploads. //! -//! New sessions use a format-specific `.offset-staging-v1` file instead of the legacy `assembled` -//! path. Init preallocates it to `total_size`; each Chunk PUT writes its range at +//! Server-managed sessions use a format-specific `.offset-staging-v1` file. Init preallocates it +//! to `total_size`; each Chunk PUT writes its range at //! `chunk_number * chunk_size`. The database receipt table is the durable completion index, while //! the staging file may still contain unwritten sparse ranges until every receipt exists. //! -//! The distinct path is also the session-format discriminator used by Chunk PUT and Complete. -//! Legacy completion may create `assembled` before a retryable storage/DB failure, so treating that -//! generic output path as an offset-staging signal would misread payload-sized legacy chunks as -//! completion records on retry. +//! The persisted `session_kind` is the session-format discriminator used by Chunk PUT and Complete; +//! temporary directory contents are never used to infer it. use std::io::SeekFrom; #[cfg(unix)] @@ -90,17 +88,6 @@ async fn sync_parent_directory(_path: &str) -> Result<()> { Ok(()) } -pub(crate) async fn exists(state: &impl SharedRuntimeState, upload_id: &str) -> Result { - match tokio::fs::metadata(file_path(state, upload_id)).await { - Ok(metadata) => Ok(metadata.is_file()), - Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), - Err(error) => Err(chunk_upload_error_with_code( - ApiErrorCode::UploadChunkPersistFailed, - format!("stat chunk staging file: {error}"), - )), - } -} - pub(crate) fn chunk_receipt_etag() -> &'static str { CHUNK_RECEIPT_ETAG } diff --git a/src/services/media/metadata/tests.rs b/src/services/media/metadata/tests.rs index 8eb594cf5..b89587d07 100644 --- a/src/services/media/metadata/tests.rs +++ b/src/services/media/metadata/tests.rs @@ -581,6 +581,5 @@ async fn test_state_with_driver_and_options( background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), } } diff --git a/src/services/ops/audit/tests.rs b/src/services/ops/audit/tests.rs index f63a221de..329939de6 100644 --- a/src/services/ops/audit/tests.rs +++ b/src/services/ops/audit/tests.rs @@ -392,7 +392,6 @@ async fn log_writes_synchronously_without_global_manager() { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; super::log( @@ -508,7 +507,6 @@ async fn log_with_details_skips_details_when_action_scope_excludes_action() { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; let calls = AtomicUsize::new(0); diff --git a/src/services/ops/config/system.rs b/src/services/ops/config/system.rs index 47747a178..c417a4dcb 100644 --- a/src/services/ops/config/system.rs +++ b/src/services/ops/config/system.rs @@ -484,7 +484,6 @@ mod tests { background_task_dispatch_wakeup: PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }, subscription, ) diff --git a/src/services/storage_policy/credential/oauth/tests.rs b/src/services/storage_policy/credential/oauth/tests.rs index 2c5ba8c4e..e99d091ea 100644 --- a/src/services/storage_policy/credential/oauth/tests.rs +++ b/src/services/storage_policy/credential/oauth/tests.rs @@ -251,7 +251,6 @@ async fn build_oauth_test_state( background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), } } diff --git a/src/services/storage_policy/policy/policies.rs b/src/services/storage_policy/policy/policies.rs index 338b0f0e5..41f23a1a3 100644 --- a/src/services/storage_policy/policy/policies.rs +++ b/src/services/storage_policy/policy/policies.rs @@ -835,7 +835,6 @@ mod tests { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), } } diff --git a/src/services/task/dispatch/tests.rs b/src/services/task/dispatch/tests.rs index 16ad87b56..fef550288 100644 --- a/src/services/task/dispatch/tests.rs +++ b/src/services/task/dispatch/tests.rs @@ -81,7 +81,6 @@ async fn build_dispatch_test_state() -> crate::runtime::PrimaryAppState { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), } } diff --git a/src/services/workspace/scope/mod.rs b/src/services/workspace/scope/mod.rs index 0bc912544..8c49e462b 100644 --- a/src/services/workspace/scope/mod.rs +++ b/src/services/workspace/scope/mod.rs @@ -569,7 +569,6 @@ mod tests { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), } } diff --git a/src/services/workspace/storage/tests.rs b/src/services/workspace/storage/tests.rs index df5ab5000..a90f2d6ec 100644 --- a/src/services/workspace/storage/tests.rs +++ b/src/services/workspace/storage/tests.rs @@ -853,7 +853,6 @@ async fn build_test_state() -> (PrimaryAppState, PathBuf, storage_policy::Model, background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; (state, temp_root, policy, user) diff --git a/src/storage/connectors/mod.rs b/src/storage/connectors/mod.rs index 978cfaf38..77fad71fe 100644 --- a/src/storage/connectors/mod.rs +++ b/src/storage/connectors/mod.rs @@ -62,7 +62,7 @@ use remote::RemoteConnector; use s3::S3Connector; use sftp::SftpConnector; use tencent_cos::TencentCosConnector; -pub use upload::{StorageConnectorChunkedCompletion, StorageConnectorUploadTransport}; +pub use upload::StorageConnectorUploadTransport; #[async_trait(?Send)] trait StorageConnector: StorageConnectorDescriptorProvider + Send + Sync + Sized { diff --git a/src/storage/connectors/tests.rs b/src/storage/connectors/tests.rs index 6b919e576..e4e27f09b 100644 --- a/src/storage/connectors/tests.rs +++ b/src/storage/connectors/tests.rs @@ -1175,10 +1175,6 @@ fn local_policy_resolves_direct_and_chunked_modes() { assert!(!transport.supports_streaming_direct_upload(&policy, 100)); assert!(!transport.uses_relay_multipart_tracking()); assert_eq!(transport.opaque_blob_hash_prefix(), None); - assert_eq!( - transport.chunked_completion(), - StorageConnectorChunkedCompletion::AssembleLocalChunks - ); assert!( !presigned_download_enabled(&policy).expect("presigned download support should resolve") ); @@ -1283,10 +1279,6 @@ fn s3_relay_stream_uses_effective_chunk_size_and_relay_tracking() { assert!(!transport.supports_streaming_direct_upload(&policy, 5_242_881)); assert!(transport.uses_relay_multipart_tracking()); assert_eq!(transport.opaque_blob_hash_prefix(), Some("s3")); - assert_eq!( - transport.chunked_completion(), - StorageConnectorChunkedCompletion::AssembleLocalChunks - ); } #[test] @@ -1314,10 +1306,6 @@ fn s3_presigned_uses_presigned_modes() { assert!(!transport.supports_streaming_direct_upload(&policy, 1024)); assert!(!transport.uses_relay_multipart_tracking()); assert_eq!(transport.opaque_blob_hash_prefix(), Some("s3")); - assert_eq!( - transport.chunked_completion(), - StorageConnectorChunkedCompletion::AssembleLocalChunks - ); } #[test] @@ -1346,10 +1334,6 @@ fn azure_blob_relay_stream_uses_object_storage_transport_modes() { assert!(transport.supports_streaming_direct_upload(&policy, 1024)); assert!(!transport.supports_streaming_direct_upload(&policy, 5_242_881)); assert!(transport.uses_relay_multipart_tracking()); - assert_eq!( - transport.chunked_completion(), - StorageConnectorChunkedCompletion::AssembleLocalChunks - ); } #[test] @@ -1434,10 +1418,6 @@ fn remote_relay_stream_uses_direct_and_chunked_modes() { assert!(transport.supports_streaming_direct_upload(&policy, 100)); assert!(transport.uses_relay_multipart_tracking()); assert_eq!(transport.opaque_blob_hash_prefix(), Some("remote")); - assert_eq!( - transport.chunked_completion(), - StorageConnectorChunkedCompletion::RelayLocalChunksToStreamUpload - ); } #[test] @@ -1465,10 +1445,6 @@ fn remote_presigned_keeps_presigned_init_but_allows_server_streaming_fast_path() assert!(transport.supports_streaming_direct_upload(&policy, 100)); assert!(!transport.uses_relay_multipart_tracking()); assert_eq!(transport.opaque_blob_hash_prefix(), Some("remote")); - assert_eq!( - transport.chunked_completion(), - StorageConnectorChunkedCompletion::RelayLocalChunksToStreamUpload - ); } #[test] @@ -1495,10 +1471,6 @@ fn onedrive_uses_server_relay_without_presigned_or_multipart_tracking() { assert!(!transport.supports_streaming_direct_upload(&policy, 0)); assert!(!transport.uses_relay_multipart_tracking()); assert_eq!(transport.opaque_blob_hash_prefix(), Some("provider")); - assert_eq!( - transport.chunked_completion(), - StorageConnectorChunkedCompletion::RelayLocalChunksToStreamUpload - ); } #[test] @@ -1548,10 +1520,6 @@ fn sftp_uses_server_relay_without_presigned_or_multipart_tracking() { assert!(!transport.supports_streaming_direct_upload(&policy, 1025)); assert!(!transport.uses_relay_multipart_tracking()); assert_eq!(transport.opaque_blob_hash_prefix(), Some("sftp")); - assert_eq!( - transport.chunked_completion(), - StorageConnectorChunkedCompletion::RelayLocalChunksToStreamUpload - ); } #[test] @@ -1580,7 +1548,6 @@ fn upload_workflow_descriptors_match_default_connector_transports() { frontend_direct_provider_resumable: false, small_mode: UploadMode::Direct, large_mode: UploadMode::Chunked, - chunked_completion: StorageConnectorChunkedCompletion::AssembleLocalChunks, }, ); for driver_type in [ @@ -1601,7 +1568,6 @@ fn upload_workflow_descriptors_match_default_connector_transports() { frontend_direct_provider_resumable: false, small_mode: UploadMode::Direct, large_mode: UploadMode::Chunked, - chunked_completion: StorageConnectorChunkedCompletion::AssembleLocalChunks, }, ); } @@ -1616,7 +1582,6 @@ fn upload_workflow_descriptors_match_default_connector_transports() { frontend_direct_provider_resumable: false, small_mode: UploadMode::Direct, large_mode: UploadMode::Chunked, - chunked_completion: StorageConnectorChunkedCompletion::RelayLocalChunksToStreamUpload, }, ); assert_upload_workflow_alignment( @@ -1630,7 +1595,6 @@ fn upload_workflow_descriptors_match_default_connector_transports() { frontend_direct_provider_resumable: false, small_mode: UploadMode::Direct, large_mode: UploadMode::Chunked, - chunked_completion: StorageConnectorChunkedCompletion::RelayLocalChunksToStreamUpload, }, ); assert_upload_workflow_alignment( @@ -1646,7 +1610,6 @@ fn upload_workflow_descriptors_match_default_connector_transports() { frontend_direct_provider_resumable: true, small_mode: UploadMode::Direct, large_mode: UploadMode::Chunked, - chunked_completion: StorageConnectorChunkedCompletion::RelayLocalChunksToStreamUpload, }, ); } @@ -1671,7 +1634,6 @@ fn upload_workflow_descriptors_match_presigned_connector_transports() { frontend_direct_provider_resumable: false, small_mode: UploadMode::Presigned, large_mode: UploadMode::PresignedMultipart, - chunked_completion: StorageConnectorChunkedCompletion::AssembleLocalChunks, }, ); } @@ -1686,7 +1648,6 @@ fn upload_workflow_descriptors_match_presigned_connector_transports() { frontend_direct_provider_resumable: false, small_mode: UploadMode::Presigned, large_mode: UploadMode::PresignedMultipart, - chunked_completion: StorageConnectorChunkedCompletion::RelayLocalChunksToStreamUpload, }, ); } @@ -1700,7 +1661,6 @@ struct ExpectedUploadWorkflow { frontend_direct_provider_resumable: bool, small_mode: UploadMode, large_mode: UploadMode, - chunked_completion: StorageConnectorChunkedCompletion, } fn assert_upload_workflow_alignment( @@ -1800,11 +1760,6 @@ fn assert_upload_workflow_alignment( expected.large_mode, "{driver_type:?} large upload mode is inconsistent with workflow descriptor" ); - assert_eq!( - transport.chunked_completion(), - expected.chunked_completion, - "{driver_type:?} chunked completion strategy is inconsistent with upload transport" - ); assert_eq!( matches!( transport, diff --git a/src/storage/connectors/upload.rs b/src/storage/connectors/upload.rs index f1ff9921e..2e2ab3efc 100644 --- a/src/storage/connectors/upload.rs +++ b/src/storage/connectors/upload.rs @@ -24,14 +24,6 @@ pub enum StorageConnectorUploadTransport { Sftp, } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum StorageConnectorChunkedCompletion { - /// Assemble local chunk files into one temp file before storing it. - AssembleLocalChunks, - /// Pipe local chunk files directly into `StreamUploadDriver::put_reader`. - RelayLocalChunksToStreamUpload, -} - impl StorageConnectorUploadTransport { /// 返回当前传输模型下实际使用的 chunk size。 /// @@ -121,30 +113,6 @@ impl StorageConnectorUploadTransport { } } - /// 决定 chunked upload complete 阶段如何处理已落地的本地 chunk。 - /// - /// object storage 仍组装成本地文件再进入对象存储路径；remote/provider stream upload - /// 可以把 chunk 串成 reader 直接喂给 `StreamUploadDriver`，避免再造第二份完整临时文件。 - /// - /// 这是迁移前 `legacy_chunk_files` 的兼容 fallback，不是新 session 的主路由事实来源。 - /// 新 session 已由 `UploadSessionKind` 选定执行计划，不能再次通过 capability 推断改道。 - pub fn chunked_completion(self) -> StorageConnectorChunkedCompletion { - match self { - // Remote and provider stream uploads should not assemble a second - // full temp file. They can relay stored chunks into the connector's - // stream-upload implementation, which lets the concrete driver own - // any provider-native resumable/session behavior internally. - Self::Remote(_) - | Self::ProviderResumable(ProviderResumableUploadStrategy::ServerRelay) - | Self::Sftp => StorageConnectorChunkedCompletion::RelayLocalChunksToStreamUpload, - Self::Local - | Self::ObjectStorage(_) - | Self::ProviderResumable(ProviderResumableUploadStrategy::FrontendDirect) => { - StorageConnectorChunkedCompletion::AssembleLocalChunks - } - } - } - fn fits_single_request(self, policy: &storage_policy::Model, total_size: i64) -> bool { let chunk_size = self.effective_chunk_size(policy); chunk_size == 0 || total_size <= chunk_size diff --git a/src/types/upload_session.rs b/src/types/upload_session.rs index 622756f39..65ab6cc05 100644 --- a/src/types/upload_session.rs +++ b/src/types/upload_session.rs @@ -4,9 +4,6 @@ use serde::{Deserialize, Serialize}; use utoipa::ToSchema; /// Persisted data plane for an upload session. -/// -/// `None` is reserved for sessions created before this column existed. New sessions must always -/// persist a kind so lifecycle code does not infer the data plane from nullable provider fields. #[derive(Debug, Clone, Copy, PartialEq, Eq, EnumIter, DeriveActiveEnum, Serialize, Deserialize)] #[cfg_attr(all(debug_assertions, feature = "openapi"), derive(ToSchema))] #[sea_orm(rs_type = "String", db_type = "String(StringLen::N(32))")] @@ -30,8 +27,6 @@ pub enum UploadSessionKind { RemotePresignedMultipart, #[sea_orm(string_value = "provider_direct_resumable")] ProviderDirectResumable, - #[sea_orm(string_value = "legacy_chunk_files")] - LegacyChunkFiles, } impl UploadSessionKind { @@ -46,7 +41,6 @@ impl UploadSessionKind { Self::RemotePresignedSingle => "remote_presigned_single", Self::RemotePresignedMultipart => "remote_presigned_multipart", Self::ProviderDirectResumable => "provider_direct_resumable", - Self::LegacyChunkFiles => "legacy_chunk_files", } } } diff --git a/src/webdav/auth.rs b/src/webdav/auth.rs index 7afdb4ffd..eca9dc243 100644 --- a/src/webdav/auth.rs +++ b/src/webdav/auth.rs @@ -297,7 +297,6 @@ mod tests { background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), } } diff --git a/src/webdav/file/tests.rs b/src/webdav/file/tests.rs index c5aba72c0..01bbbe56d 100644 --- a/src/webdav/file/tests.rs +++ b/src/webdav/file/tests.rs @@ -276,7 +276,6 @@ async fn build_s3_direct_test_state() -> (PrimaryAppState, user::Model, MockDire background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; (state, user, mock_driver) diff --git a/src/webdav/handler_tests/mod.rs b/src/webdav/handler_tests/mod.rs index 177571a83..e68a93879 100644 --- a/src/webdav/handler_tests/mod.rs +++ b/src/webdav/handler_tests/mod.rs @@ -150,7 +150,6 @@ async fn build_webdav_test_state( background_task_dispatch_wakeup: crate::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol: crate::runtime::PrimaryAppState::new_remote_protocol(), - upload_runtime: crate::runtime::PrimaryAppState::new_upload_runtime(), }; (state, user, policy, temp_root) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 28a0413f1..ac6306d7e 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -899,7 +899,6 @@ pub async fn setup_with_memory_cache() -> PrimaryAppState { share_download_rollback: base.share_download_rollback, background_task_dispatch_wakeup: base.background_task_dispatch_wakeup, remote_protocol: base.remote_protocol, - upload_runtime: base.upload_runtime, } } @@ -1301,7 +1300,6 @@ pub async fn setup_with_database_url(database_url: &str) -> PrimaryAppState { background_task_dispatch_wakeup: aster_drive::runtime::PrimaryAppState::new_background_task_dispatch_wakeup(), remote_protocol, - upload_runtime: aster_drive::runtime::PrimaryAppState::new_upload_runtime(), } } diff --git a/tests/test_auth.rs b/tests/test_auth.rs index 2f7601781..a14342559 100644 --- a/tests/test_auth.rs +++ b/tests/test_auth.rs @@ -4007,7 +4007,6 @@ async fn test_user_status_cached_in_auth_middleware() { share_download_rollback: base.share_download_rollback, background_task_dispatch_wakeup: base.background_task_dispatch_wakeup, remote_protocol: base.remote_protocol, - upload_runtime: base.upload_runtime, }; let app = create_test_app!(state); let (token, _) = register_and_login!(app); @@ -4056,7 +4055,6 @@ async fn test_disable_user_invalidates_status_cache() { share_download_rollback: base.share_download_rollback, background_task_dispatch_wakeup: base.background_task_dispatch_wakeup, remote_protocol: base.remote_protocol, - upload_runtime: base.upload_runtime, }; let app = create_test_app!(state); let (admin_token, _) = register_and_login!(app); diff --git a/tests/test_database_backends.rs b/tests/test_database_backends.rs index ba179278a..cf13711a8 100644 --- a/tests/test_database_backends.rs +++ b/tests/test_database_backends.rs @@ -209,7 +209,7 @@ async fn assert_upload_session_kind_column(db: &DatabaseConnection, backend: DbB let is_nullable: String = row .try_get_by_index(0) .expect("session_kind metadata should include is_nullable"); - assert_eq!(is_nullable, "YES"); + assert_eq!(is_nullable, "NO"); } async fn assert_background_task_display_name_column_len( diff --git a/tests/test_maintenance.rs b/tests/test_maintenance.rs index ef8d18fc6..426c4035e 100644 --- a/tests/test_maintenance.rs +++ b/tests/test_maintenance.rs @@ -119,7 +119,12 @@ async fn create_upload_session( folder_id: Set(None), policy_id: Set(policy.id), status: Set(spec.status), - session_kind: Set(None), + session_kind: Set(match (spec.object_temp_key, spec.object_multipart_id) { + (Some(_), Some(_)) => aster_drive::types::UploadSessionKind::ProviderRelayMultipart, + (Some(_), None) => aster_drive::types::UploadSessionKind::ProviderPresignedSingle, + (None, None) => aster_drive::types::UploadSessionKind::OffsetStaging, + (None, Some(_)) => panic!("multipart upload fixture requires a temp key"), + }), object_temp_key: Set(spec.object_temp_key.map(str::to_string)), object_multipart_id: Set(spec.object_multipart_id.map(str::to_string)), provider_session_ciphertext: Set(None), diff --git a/tests/test_migrations.rs b/tests/test_migrations.rs index 216ed37e9..99190deb8 100644 --- a/tests/test_migrations.rs +++ b/tests/test_migrations.rs @@ -29,6 +29,7 @@ const BIND_EXTERNAL_AUTH_LOGIN_FLOWS_MIGRATION: &str = "m20260716_000001_bind_external_auth_login_flows"; const ADD_UPLOAD_SESSION_KIND_MIGRATION: &str = "m20260717_000001_add_upload_session_kind"; const ADD_UPLOAD_PROVIDER_SESSION_MIGRATION: &str = "m20260719_000001_add_upload_provider_session"; +const REQUIRE_UPLOAD_SESSION_KIND_MIGRATION: &str = "m20260723_000001_require_upload_session_kind"; async fn setup_current_schema() -> sea_orm::DatabaseConnection { let db = Database::connect("sqlite::memory:") @@ -68,7 +69,7 @@ async fn external_auth_login_flow_browser_binding_migration_is_registered_and_re } #[tokio::test] -async fn upload_session_kind_migration_is_nullable_and_reversible() { +async fn upload_session_kind_column_migrations_are_reversible() { assert!( CurrentMigrator::migrations() .iter() @@ -79,10 +80,7 @@ async fn upload_session_kind_migration_is_nullable_and_reversible() { let db = setup_current_schema().await; let current_columns = sqlite_table_columns(&db, "upload_sessions").await; assert!(has_column(¤t_columns, "session_kind")); - assert!( - !sqlite_column_is_not_null(&db, "upload_sessions", "session_kind").await, - "pre-0.5.0 rows must remain readable with a null session kind" - ); + assert!(sqlite_column_is_not_null(&db, "upload_sessions", "session_kind").await); let rollback_steps = steps_to_roll_back_migration(ADD_UPLOAD_SESSION_KIND_MIGRATION); CurrentMigrator::down(&db, Some(rollback_steps)) @@ -96,6 +94,140 @@ async fn upload_session_kind_migration_is_nullable_and_reversible() { .expect("upload session kind migration should reapply"); let reapplied_columns = sqlite_table_columns(&db, "upload_sessions").await; assert!(has_column(&reapplied_columns, "session_kind")); + assert!(sqlite_column_is_not_null(&db, "upload_sessions", "session_kind").await); +} + +#[tokio::test] +async fn required_upload_session_kind_migration_rolls_back_and_reapplies() { + assert!( + CurrentMigrator::migrations() + .iter() + .any(|migration| migration.name() == REQUIRE_UPLOAD_SESSION_KIND_MIGRATION), + "required upload session kind migration should be registered" + ); + + let db = setup_current_schema().await; + assert!(sqlite_column_is_not_null(&db, "upload_sessions", "session_kind").await); + + CurrentMigrator::down(&db, Some(1)) + .await + .expect("required upload session kind migration should roll back"); + assert!( + !sqlite_column_is_not_null(&db, "upload_sessions", "session_kind").await, + "0.4.x rollback should restore nullable session kind" + ); + + CurrentMigrator::up(&db, Some(1)) + .await + .expect("required upload session kind migration should reapply"); + assert!(sqlite_column_is_not_null(&db, "upload_sessions", "session_kind").await); +} + +#[tokio::test] +async fn required_upload_session_kind_migration_rejects_null_rows_without_deleting_them() { + let db = Database::connect("sqlite::memory:") + .await + .expect("sqlite memory database should connect"); + let pre_boundary_steps = u32::try_from(CurrentMigrator::migrations().len() - 1) + .expect("migration count should fit u32"); + CurrentMigrator::up(&db, Some(pre_boundary_steps)) + .await + .expect("0.4.x-compatible migrations should apply"); + + db.execute_unprepared("PRAGMA foreign_keys = OFF") + .await + .expect("foreign keys should disable for isolated legacy fixture"); + db.execute_unprepared( + "INSERT INTO upload_sessions (\ + id, user_id, team_id, frontend_client_id, filename, total_size, chunk_size, \ + total_chunks, received_count, folder_id, policy_id, status, session_kind, \ + object_temp_key, object_multipart_id, provider_session_ciphertext, file_id, \ + created_at, expires_at, updated_at\ + ) VALUES (\ + 'legacy-null-kind', 1, NULL, NULL, 'legacy.bin', 1, 1, 1, 0, NULL, 1, \ + 'uploading', NULL, NULL, NULL, NULL, NULL, \ + '2026-07-23T00:00:00Z', '2026-07-24T00:00:00Z', '2026-07-23T00:00:00Z'\ + )", + ) + .await + .expect("legacy null-kind upload session should insert"); + db.execute_unprepared("PRAGMA foreign_keys = ON") + .await + .expect("foreign keys should re-enable"); + + let error = CurrentMigrator::up(&db, Some(1)) + .await + .expect_err("null-kind upload session must block the 0.5.0 migration"); + assert!( + error + .to_string() + .contains("legacy or invalid upload session") + ); + + let row = db + .query_one_raw(Statement::from_string( + DbBackend::Sqlite, + "SELECT COUNT(*) FROM upload_sessions WHERE id = 'legacy-null-kind'", + )) + .await + .expect("legacy row count should query") + .expect("legacy row count should return one row"); + assert_eq!(row.try_get_by_index::(0).unwrap(), 1); +} + +#[tokio::test] +async fn required_upload_session_kind_migration_rejects_invalid_values_without_deleting_them() { + let db = Database::connect("sqlite::memory:") + .await + .expect("sqlite memory database should connect"); + let pre_boundary_steps = u32::try_from(CurrentMigrator::migrations().len() - 1) + .expect("migration count should fit u32"); + CurrentMigrator::up(&db, Some(pre_boundary_steps)) + .await + .expect("0.4.x-compatible migrations should apply"); + + db.execute_unprepared("PRAGMA foreign_keys = OFF") + .await + .expect("foreign keys should disable for isolated legacy fixture"); + db.execute_unprepared( + "INSERT INTO upload_sessions (\ + id, user_id, team_id, frontend_client_id, filename, total_size, chunk_size, \ + total_chunks, received_count, folder_id, policy_id, status, session_kind, \ + object_temp_key, object_multipart_id, provider_session_ciphertext, file_id, \ + created_at, expires_at, updated_at\ + ) VALUES (\ + 'invalid-kind', 1, NULL, NULL, 'invalid.bin', 1, 1, 1, 0, NULL, 1, \ + 'uploading', 'legacy_chunk_files', NULL, NULL, NULL, NULL, \ + '2026-07-23T00:00:00Z', '2026-07-24T00:00:00Z', '2026-07-23T00:00:00Z'\ + )", + ) + .await + .expect("legacy invalid-kind upload session should insert"); + db.execute_unprepared("PRAGMA foreign_keys = ON") + .await + .expect("foreign keys should re-enable"); + + let error = CurrentMigrator::up(&db, Some(1)) + .await + .expect_err("invalid session kind must block the 0.5.0 migration"); + assert!( + error + .to_string() + .contains("legacy or invalid upload session") + ); + + let row = db + .query_one_raw(Statement::from_string( + DbBackend::Sqlite, + "SELECT session_kind FROM upload_sessions WHERE id = 'invalid-kind'", + )) + .await + .expect("invalid row query should succeed") + .expect("invalid row should remain present"); + assert_eq!( + row.try_get_by_index::(0).unwrap(), + "legacy_chunk_files" + ); } #[tokio::test] diff --git a/tests/test_policies.rs b/tests/test_policies.rs index 742b422c2..9d165f08d 100644 --- a/tests/test_policies.rs +++ b/tests/test_policies.rs @@ -474,7 +474,11 @@ async fn create_policy_upload_session( status: Set(spec .status .unwrap_or(aster_drive::types::UploadSessionStatus::Uploading)), - session_kind: Set(None), + session_kind: Set(if spec.object_temp_key.is_some() { + aster_drive::types::UploadSessionKind::ProviderPresignedSingle + } else { + aster_drive::types::UploadSessionKind::OffsetStaging + }), object_temp_key: Set(spec.object_temp_key.map(str::to_string)), object_multipart_id: Set(None), provider_session_ciphertext: Set(None), @@ -1445,7 +1449,7 @@ async fn test_policy_force_delete_removes_corrupted_session_without_temp_object( .await .unwrap() .into_active_model(); - session.session_kind = Set(Some(UploadSessionKind::ProviderPresignedSingle)); + session.session_kind = Set(UploadSessionKind::ProviderPresignedSingle); session.update(&db).await.unwrap(); let req = test::TestRequest::delete() diff --git a/tests/test_remote_storage.rs b/tests/test_remote_storage.rs index 96747c80b..cbdd3e3b1 100644 --- a/tests/test_remote_storage.rs +++ b/tests/test_remote_storage.rs @@ -1087,7 +1087,7 @@ async fn test_remote_ingress_profile_update_without_driver_change_keeps_remote_a ApiErrorCode::ManagedIngressDriverUnsupported ); assert!( - error.message().contains("remote storage request failed"), + error.message().contains("update remote storage target"), "unexpected error message: {}", error.message() ); diff --git a/tests/test_services.rs b/tests/test_services.rs index 1f04f11bd..9768bacf9 100644 --- a/tests/test_services.rs +++ b/tests/test_services.rs @@ -2683,7 +2683,7 @@ async fn test_team_archive_cleanup_deletes_expired_team_data() { folder_id: Set(Some(folder.id)), policy_id: Set(default_policy_id), status: Set(aster_drive::types::UploadSessionStatus::Uploading), - session_kind: Set(None), + session_kind: Set(aster_drive::types::UploadSessionKind::ProviderPresignedSingle), object_temp_key: Set(Some(format!("files/{upload_id}"))), object_multipart_id: Set(None), provider_session_ciphertext: Set(None), @@ -2823,7 +2823,7 @@ async fn test_team_archive_cleanup_keeps_team_when_upload_temp_delete_fails() { folder_id: Set(None), policy_id: Set(policy.id), status: Set(aster_drive::types::UploadSessionStatus::Uploading), - session_kind: Set(None), + session_kind: Set(aster_drive::types::UploadSessionKind::ProviderPresignedSingle), object_temp_key: Set(Some(temp_key.clone())), object_multipart_id: Set(None), provider_session_ciphertext: Set(None), diff --git a/tests/test_upload.rs b/tests/test_upload.rs index d5eb28a80..21610d95f 100644 --- a/tests/test_upload.rs +++ b/tests/test_upload.rs @@ -174,7 +174,7 @@ struct UploadSessionSpec<'a> { expires_at: chrono::DateTime, total_chunks: i32, received_count: i32, - session_kind: Option, + session_kind: aster_drive::types::UploadSessionKind, policy_id: Option, object_temp_key: Option<&'a str>, object_multipart_id: Option<&'a str>, @@ -193,7 +193,7 @@ impl<'a> UploadSessionSpec<'a> { expires_at, total_chunks: 0, received_count: 0, - session_kind: None, + session_kind: aster_drive::types::UploadSessionKind::OffsetStaging, policy_id: None, object_temp_key: None, object_multipart_id: None, @@ -213,7 +213,7 @@ impl<'a> UploadSessionSpec<'a> { } fn session_kind(mut self, kind: aster_drive::types::UploadSessionKind) -> Self { - self.session_kind = Some(kind); + self.session_kind = kind; self } @@ -306,7 +306,7 @@ async fn test_upload_session_try_create_reports_id_conflict() { folder_id: Set(None), policy_id: Set(policy.id), status: Set(aster_drive::types::UploadSessionStatus::Uploading), - session_kind: Set(None), + session_kind: Set(aster_drive::types::UploadSessionKind::OffsetStaging), object_temp_key: Set(None), object_multipart_id: Set(None), provider_session_ciphertext: Set(None), @@ -369,7 +369,7 @@ async fn test_upload_session_try_create_preserves_non_id_unique_conflict() { folder_id: Set(None), policy_id: Set(policy.id), status: Set(aster_drive::types::UploadSessionStatus::Uploading), - session_kind: Set(None), + session_kind: Set(aster_drive::types::UploadSessionKind::OffsetStaging), object_temp_key: Set(None), object_multipart_id: Set(None), provider_session_ciphertext: Set(None), @@ -1950,6 +1950,15 @@ async fn test_chunked_upload_offset_staging_preserves_content() { ); } + let temp_dir = + aster_forge_utils::paths::upload_temp_dir(&state.config.server.upload_temp_dir, &upload_id); + tokio::fs::write(format!("{temp_dir}/chunk_0"), b"legacy payload") + .await + .unwrap(); + tokio::fs::write(format!("{temp_dir}/assembled"), b"legacy assembled payload") + .await + .unwrap(); + let file = upload::complete_upload(&state, &upload_id, user.id, None) .await .unwrap(); @@ -3000,7 +3009,7 @@ async fn test_chunked_init_persists_explicit_offset_staging_kind() { upload_session_repo::find_by_id(state.writer_db(), response.upload_id.as_deref().unwrap()) .await .unwrap(); - assert_eq!(session.session_kind, Some(UploadSessionKind::OffsetStaging)); + assert_eq!(session.session_kind, UploadSessionKind::OffsetStaging); } #[tokio::test] @@ -3040,15 +3049,15 @@ async fn test_explicit_session_kind_rejects_incompatible_fields() { } #[tokio::test] -async fn test_legacy_relay_kind_rejects_missing_object_temp_key() { +async fn test_relay_kind_rejects_missing_object_temp_key() { use aster_drive::services::files::upload; use aster_drive::types::UploadSessionStatus; let state = common::setup().await; let user = common::create_test_account( &state, - "legacykindbad", - "legacy-kind-corrupt@test.com", + "relaykindbad", + "relay-kind-corrupt@test.com", "password123", ) .await @@ -3065,12 +3074,13 @@ async fn test_legacy_relay_kind_rejects_missing_object_temp_key() { ) .chunks(1, 0) .policy(policy.id) + .session_kind(aster_drive::types::UploadSessionKind::ProviderRelayMultipart) .object_upload(None, Some("multipart")), ) .await; let error = match upload::get_progress(&state, &upload_id, user.id).await { - Ok(_) => panic!("legacy relay session without temp key must be rejected"), + Ok(_) => panic!("relay session without temp key must be rejected"), Err(error) => error, }; assert_eq!(error.code(), "E057"); @@ -3210,7 +3220,7 @@ async fn test_explicit_staging_kind_does_not_fall_back_when_file_is_missing() { let error = upload::complete_upload(&state, &upload_id, user.id, None) .await - .expect_err("missing explicit staging file must not enter legacy assembly"); + .expect_err("missing explicit staging file must fail completion"); assert!(error.message().contains("stat chunk staging file")); } @@ -3313,6 +3323,12 @@ async fn test_concurrent_chunk_upload_idempotent() { )) .await .unwrap(); + let staging_path = upload::test_support::offset_staging_file_path( + &state.config.server.upload_temp_dir, + &upload_id, + ); + let staging_file = tokio::fs::File::create(&staging_path).await.unwrap(); + staging_file.set_len(10).await.unwrap(); let chunk_data = b"12345".to_vec(); let state1 = Arc::clone(&state); @@ -3351,53 +3367,6 @@ async fn test_concurrent_chunk_upload_idempotent() { ); } -#[tokio::test] -async fn test_upload_chunk_replaces_stale_partial_local_chunk() { - use aster_drive::services::files::upload; - - let state = common::setup().await; - let user = - common::create_test_account(&state, "stalechunk", "stale-chunk@test.com", "password123") - .await - .unwrap(); - let upload_id = new_test_upload_id(); - create_upload_session( - &state, - user.id, - UploadSessionSpec::new( - &upload_id, - aster_drive::types::UploadSessionStatus::Uploading, - chrono::Utc::now() + chrono::Duration::hours(1), - ) - .chunks(2, 0), - ) - .await; - - let chunk_dir = - aster_forge_utils::paths::upload_temp_dir(&state.config.server.upload_temp_dir, &upload_id); - tokio::fs::create_dir_all(&chunk_dir).await.unwrap(); - let chunk_path = aster_forge_utils::paths::upload_chunk_path( - &state.config.server.upload_temp_dir, - &upload_id, - 0, - ); - tokio::fs::write(&chunk_path, b"bad").await.unwrap(); - - let response = upload::upload_chunk(&state, &upload_id, 0, user.id, b"12345") - .await - .unwrap(); - assert_eq!(response.received_count, 1); - - let stored = tokio::fs::read(&chunk_path).await.unwrap(); - assert_eq!(stored, b"12345"); - - let progress = upload::get_progress(&state, &upload_id, user.id) - .await - .unwrap(); - assert_eq!(progress.received_count, 1); - assert_eq!(progress.chunks_on_disk, vec![0]); -} - #[tokio::test] async fn test_upload_session_part_upsert_updates_existing_row_without_duplicates() { use aster_drive::db::repository::{upload_session_part_repo, upload_session_repo}; @@ -3731,155 +3700,6 @@ async fn test_complete_upload_rejects_truncated_offset_staging_file() { ); } -#[actix_web::test] -async fn test_legacy_assembly_failure_releases_limiter() { - use aster_drive::services::files::upload; - - let state = common::setup().await; - let user = common::create_test_account( - &state, - "legacyfailure", - "legacy-failure@test.com", - "password123", - ) - .await - .unwrap(); - - let broken_upload_id = new_test_upload_id(); - create_upload_session( - &state, - user.id, - UploadSessionSpec::new( - &broken_upload_id, - aster_drive::types::UploadSessionStatus::Uploading, - chrono::Utc::now() + chrono::Duration::hours(1), - ) - .chunks(2, 2), - ) - .await; - let broken_chunk0 = aster_forge_utils::paths::upload_chunk_path( - &state.config.server.upload_temp_dir, - &broken_upload_id, - 0, - ); - tokio::fs::create_dir_all(std::path::Path::new(&broken_chunk0).parent().unwrap()) - .await - .unwrap(); - tokio::fs::write(&broken_chunk0, b"12345").await.unwrap(); - upload::complete_upload(&state, &broken_upload_id, user.id, None) - .await - .unwrap_err(); - - let next_upload_id = new_test_upload_id(); - create_upload_session( - &state, - user.id, - UploadSessionSpec::new( - &next_upload_id, - aster_drive::types::UploadSessionStatus::Uploading, - chrono::Utc::now() + chrono::Duration::hours(1), - ) - .chunks(2, 2), - ) - .await; - let next_chunk0 = aster_forge_utils::paths::upload_chunk_path( - &state.config.server.upload_temp_dir, - &next_upload_id, - 0, - ); - let next_chunk1 = aster_forge_utils::paths::upload_chunk_path( - &state.config.server.upload_temp_dir, - &next_upload_id, - 1, - ); - tokio::fs::create_dir_all(std::path::Path::new(&next_chunk0).parent().unwrap()) - .await - .unwrap(); - tokio::fs::write(&next_chunk0, b"12345").await.unwrap(); - tokio::fs::write(&next_chunk1, b"67890").await.unwrap(); - - tokio::time::timeout( - std::time::Duration::from_secs(2), - upload::complete_upload(&state, &next_upload_id, user.id, None), - ) - .await - .expect("legacy assembly failure must release the compatibility limiter") - .unwrap(); -} - -#[actix_web::test] -async fn test_complete_legacy_chunk_files_after_upgrade() { - use aster_drive::db::repository::file_repo; - use aster_drive::services::files::upload; - - let state = common::setup().await; - let user = common::create_test_account( - &state, - "legacychunk", - "legacy-chunk@test.com", - "password123", - ) - .await - .unwrap(); - let upload_id = new_test_upload_id(); - create_upload_session( - &state, - user.id, - UploadSessionSpec::new( - &upload_id, - aster_drive::types::UploadSessionStatus::Uploading, - chrono::Utc::now() + chrono::Duration::hours(1), - ) - .chunks(2, 2), - ) - .await; - - let chunk0 = aster_forge_utils::paths::upload_chunk_path( - &state.config.server.upload_temp_dir, - &upload_id, - 0, - ); - let chunk1 = aster_forge_utils::paths::upload_chunk_path( - &state.config.server.upload_temp_dir, - &upload_id, - 1, - ); - tokio::fs::create_dir_all(std::path::Path::new(&chunk0).parent().unwrap()) - .await - .unwrap(); - tokio::fs::write(&chunk0, b"12345").await.unwrap(); - tokio::fs::write(&chunk1, b"67890").await.unwrap(); - - // A retryable failure or process crash may leave the first legacy assembly behind. That - // artifact must not reclassify the session as offset staging on the next completion attempt. - let assembled_path = aster_forge_utils::paths::upload_assembled_path( - &state.config.server.upload_temp_dir, - &upload_id, - ); - tokio::fs::write(&assembled_path, b"stale legacy assembly") - .await - .unwrap(); - - let file = upload::complete_upload(&state, &upload_id, user.id, None) - .await - .unwrap(); - let blob = file_repo::find_blob_by_id(state.writer_db(), file.blob_id) - .await - .unwrap(); - let policy = - aster_drive::db::repository::policy_repo::find_by_id(state.writer_db(), blob.policy_id) - .await - .unwrap(); - let stored = state - .driver_registry - .get_driver(&policy) - .unwrap() - .get(&blob.storage_path) - .await - .unwrap(); - assert_eq!(stored, b"1234567890"); -} - #[actix_web::test] async fn test_complete_upload_keeps_presigned_multipart_session_retryable_after_storage_error() { use aster_drive::db::repository::upload_session_repo; @@ -3908,6 +3728,7 @@ async fn test_complete_upload_keeps_presigned_multipart_session_retryable_after_ ) .chunks(2, 0) .policy(remote_policy.id) + .session_kind(aster_drive::types::UploadSessionKind::RemotePresignedMultipart) .object_upload( Some("upload/data/files/presigned-retry-temp"), Some("presigned-retry-multipart"), @@ -3942,77 +3763,6 @@ async fn test_complete_upload_keeps_presigned_multipart_session_retryable_after_ assert_eq!(session_after_retry.file_id, None); } -#[actix_web::test] -async fn test_complete_legacy_remote_chunk_files_remain_retryable_after_storage_error() { - use aster_drive::db::repository::upload_session_repo; - use aster_drive::services::files::upload; - use aster_drive::types::UploadSessionStatus; - - let state = common::setup().await; - let user = common::create_test_account( - &state, - "rchunkretry", - "remote-chunk-retry@test.com", - "password123", - ) - .await - .unwrap(); - let remote_policy = create_dead_remote_policy(&state).await; - let upload_id = new_test_upload_id(); - - create_upload_session( - &state, - user.id, - UploadSessionSpec::new( - &upload_id, - UploadSessionStatus::Uploading, - chrono::Utc::now() + chrono::Duration::hours(1), - ) - .chunks(2, 2) - .policy(remote_policy.id), - ) - .await; - - let chunk0 = aster_forge_utils::paths::upload_chunk_path( - &state.config.server.upload_temp_dir, - &upload_id, - 0, - ); - let chunk1 = aster_forge_utils::paths::upload_chunk_path( - &state.config.server.upload_temp_dir, - &upload_id, - 1, - ); - let chunk_dir = std::path::Path::new(&chunk0) - .parent() - .expect("chunk path should have parent"); - tokio::fs::create_dir_all(chunk_dir).await.unwrap(); - tokio::fs::write(&chunk0, b"12345").await.unwrap(); - tokio::fs::write(&chunk1, b"67890").await.unwrap(); - - let err = upload::complete_upload(&state, &upload_id, user.id, None) - .await - .unwrap_err(); - assert_eq!(err.code(), "E031"); - - let session = upload_session_repo::find_by_id(state.writer_db(), &upload_id) - .await - .unwrap(); - assert_eq!(session.status, UploadSessionStatus::Uploading); - assert_eq!(session.file_id, None); - - let retry_err = upload::complete_upload(&state, &upload_id, user.id, None) - .await - .unwrap_err(); - assert_eq!(retry_err.code(), "E031"); - - let session_after_retry = upload_session_repo::find_by_id(state.writer_db(), &upload_id) - .await - .unwrap(); - assert_eq!(session_after_retry.status, UploadSessionStatus::Uploading); - assert_eq!(session_after_retry.file_id, None); -} - #[actix_web::test] async fn test_complete_offset_staging_stream_relay_remains_retryable_after_storage_error() { use aster_drive::db::repository::upload_session_repo; @@ -4167,6 +3917,7 @@ async fn test_file_upload_complete_presigned_multipart_requires_parts() { chrono::Utc::now() + chrono::Duration::hours(1), ) .chunks(2, 0) + .session_kind(aster_drive::types::UploadSessionKind::ProviderPresignedMultipart) .object_upload(Some("files/temp-key"), Some("multipart-id")), ) .await; @@ -4178,7 +3929,8 @@ async fn test_file_upload_complete_presigned_multipart_requires_parts() { } #[actix_web::test] -async fn test_file_upload_get_progress_scans_and_sorts_local_chunks() { +async fn test_file_upload_get_progress_lists_and_sorts_staging_receipts() { + use aster_drive::db::repository::upload_session_part_repo; use aster_drive::services::files::upload; let state = common::setup().await; @@ -4195,33 +3947,29 @@ async fn test_file_upload_get_progress_scans_and_sorts_local_chunks() { aster_drive::types::UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), ) - .chunks(3, 2), + .chunks(2, 2), ) .await; - let temp_dir = - aster_forge_utils::paths::upload_temp_dir(&state.config.server.upload_temp_dir, &upload_id); - tokio::fs::create_dir_all(&temp_dir).await.unwrap(); - tokio::fs::write(format!("{temp_dir}/chunk_2"), b"two") - .await - .unwrap(); - tokio::fs::write(format!("{temp_dir}/chunk_0"), b"zero") - .await - .unwrap(); - tokio::fs::write(format!("{temp_dir}/chunk_bad"), b"ignored") - .await - .unwrap(); - tokio::fs::write(format!("{temp_dir}/notes.txt"), b"ignored") + for part_number in [2, 1] { + upload_session_part_repo::upsert_part( + state.writer_db(), + &upload_id, + part_number, + upload::test_support::offset_staging_receipt_etag(), + 5, + ) .await .unwrap(); + } let progress = upload::get_progress(&state, &upload_id, user.id) .await .unwrap(); assert_eq!(progress.upload_id, upload_id); assert_eq!(progress.received_count, 2); - assert_eq!(progress.total_chunks, 3); - assert_eq!(progress.chunks_on_disk, vec![0, 2]); + assert_eq!(progress.total_chunks, 2); + assert_eq!(progress.chunks_on_disk, vec![0, 1]); } #[actix_web::test] @@ -4274,9 +4022,7 @@ async fn test_file_upload_get_progress_uses_db_parts_for_terminal_relay_multipar folder_id: Set(None), policy_id: Set(relay_policy.id), status: Set(status), - session_kind: Set(Some( - aster_drive::types::UploadSessionKind::ProviderRelayMultipart, - )), + session_kind: Set(aster_drive::types::UploadSessionKind::ProviderRelayMultipart), object_temp_key: Set(Some(format!("files/{upload_id}"))), object_multipart_id: Set(Some(format!("multipart-{status_name}"))), provider_session_ciphertext: Set(None), From e5bef1b49f7a60dcc2a1bbfe523b84003b1ba306 Mon Sep 17 00:00:00 2001 From: AptS-1547 Date: Thu, 23 Jul 2026 15:02:20 +0800 Subject: [PATCH 2/2] refactor(upload): improve test synchronization and remove redundant parameters - Add rendezvous barrier before staging write lock acquisition for more reliable concurrency testing - Replace timeout-based overlap detection with entry barrier and counter - Remove redundant `kind` parameter from `validate_persisted_kind` function - Fix foreign key reference in migration to use correct table identifier - Update test helper to require explicit `session_kind` in constructor - Improve concurrent chunk upload test with proper synchronization verification - Add entry count assertion to duplicate chunk test - Update remote storage test assertion to check for transient error kind --- ...0723_000001_require_upload_session_kind.rs | 5 +- src/services/files/upload/chunk.rs | 50 ++++++---- src/services/files/upload/kind.rs | 56 +++++------ tests/test_remote_storage.rs | 9 +- tests/test_upload.rs | 94 ++++++++++++++----- 5 files changed, 138 insertions(+), 76 deletions(-) diff --git a/migration/src/m20260723_000001_require_upload_session_kind.rs b/migration/src/m20260723_000001_require_upload_session_kind.rs index b41799d53..9f3c23423 100644 --- a/migration/src/m20260723_000001_require_upload_session_kind.rs +++ b/migration/src/m20260723_000001_require_upload_session_kind.rs @@ -165,6 +165,7 @@ fn upload_sessions_table(table: T, nullable: bool) -> TableCreateStatement where T: IntoIden, { + let table = table.into_iden(); let mut session_kind = ColumnDef::new(UploadSessions::SessionKind); session_kind.string_len(32); if nullable { @@ -174,7 +175,7 @@ where } Table::create() - .table(table) + .table(table.clone()) .col( ColumnDef::new(UploadSessions::Id) .string_len(36) @@ -252,7 +253,7 @@ where .col(ColumnDef::new(UploadSessions::UpdatedAt).text().not_null()) .foreign_key( ForeignKey::create() - .from(Alias::new(SQLITE_REBUILT_TABLE), UploadSessions::UserId) + .from(table, UploadSessions::UserId) .to(Users::Table, Users::Id) .on_delete(ForeignKeyAction::Cascade), ) diff --git a/src/services/files/upload/chunk.rs b/src/services/files/upload/chunk.rs index 06f640a06..5d6344df7 100644 --- a/src/services/files/upload/chunk.rs +++ b/src/services/files/upload/chunk.rs @@ -607,6 +607,8 @@ async fn upload_chunk_impl( crate::types::UploadSessionKind::OffsetStaging | crate::types::UploadSessionKind::StreamStaging ) { + #[cfg(debug_assertions)] + test_support::rendezvous_before_staging_write_lock(upload_id).await; let _chunk_write_lock = acquire_local_chunk_write_lock(&chunk_dir, upload_id, chunk_number).await?; #[cfg(debug_assertions)] @@ -816,6 +818,8 @@ async fn upload_chunk_payload_impl( crate::types::UploadSessionKind::OffsetStaging | crate::types::UploadSessionKind::StreamStaging ) { + #[cfg(debug_assertions)] + test_support::rendezvous_before_staging_write_lock(upload_id).await; let _chunk_write_lock = acquire_local_chunk_write_lock(&chunk_dir, upload_id, chunk_number).await?; #[cfg(debug_assertions)] @@ -955,9 +959,8 @@ pub mod test_support { Arc, atomic::{AtomicBool, AtomicUsize, Ordering}, }; - use std::time::Duration; - use tokio::sync::{Barrier, Notify}; + use tokio::sync::Barrier; use super::STAGING_WRITE_TEST_HOOKS; @@ -971,10 +974,10 @@ pub mod test_support { } struct ExclusiveObservation { + entry_barrier: Barrier, + entry_count: AtomicUsize, active: AtomicUsize, overlap_observed: AtomicBool, - first_entry_wait_pending: AtomicBool, - overlap: Notify, } pub struct StagingWriteTestHook { @@ -991,6 +994,15 @@ pub mod test_support { } } } + + pub fn entry_count(&self) -> usize { + match &self.state.mode { + StagingWriteTestHookMode::Rendezvous(_) => 0, + StagingWriteTestHookMode::ObserveExclusive(observation) => { + observation.entry_count.load(Ordering::SeqCst) + } + } + } } impl Drop for StagingWriteTestHook { @@ -1021,10 +1033,10 @@ pub mod test_support { install_hook( upload_id, StagingWriteTestHookMode::ObserveExclusive(Arc::new(ExclusiveObservation { + entry_barrier: Barrier::new(2), + entry_count: AtomicUsize::new(0), active: AtomicUsize::new(0), overlap_observed: AtomicBool::new(false), - first_entry_wait_pending: AtomicBool::new(true), - overlap: Notify::new(), })), ) } @@ -1074,16 +1086,6 @@ pub mod test_support { let previous = observation.active.fetch_add(1, Ordering::SeqCst); if previous > 0 { observation.overlap_observed.store(true, Ordering::SeqCst); - observation.overlap.notify_waiters(); - } else if observation - .first_entry_wait_pending - .swap(false, Ordering::SeqCst) - { - let _ = tokio::time::timeout( - Duration::from_millis(250), - observation.overlap.notified(), - ) - .await; } StagingWriteTestGuard { exclusive: Some(Arc::clone(observation)), @@ -1092,6 +1094,22 @@ pub mod test_support { } } + pub(super) async fn rendezvous_before_staging_write_lock(upload_id: &str) { + let hook = STAGING_WRITE_TEST_HOOKS + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .get(upload_id) + .cloned(); + let Some(hook) = hook else { + return; + }; + let StagingWriteTestHookMode::ObserveExclusive(observation) = &hook.mode else { + return; + }; + observation.entry_count.fetch_add(1, Ordering::SeqCst); + observation.entry_barrier.wait().await; + } + pub fn offset_staging_file_path(upload_temp_dir: &str, upload_id: &str) -> String { crate::services::files::upload::staging::file_path_in_upload_temp_dir( upload_temp_dir, diff --git a/src/services/files/upload/kind.rs b/src/services/files/upload/kind.rs index c84ddc1d1..21ce1cc17 100644 --- a/src/services/files/upload/kind.rs +++ b/src/services/files/upload/kind.rs @@ -8,13 +8,13 @@ use crate::types::{UploadMode, UploadSessionKind}; pub(crate) fn resolve_upload_session_kind( session: &upload_session::Model, ) -> Result { - validate_persisted_kind(session, session.session_kind) + validate_persisted_kind(session) } pub(crate) fn validate_persisted_kind( session: &upload_session::Model, - kind: UploadSessionKind, ) -> Result { + let kind = session.session_kind; let has_multipart_id = session.object_multipart_id.is_some(); let expects_multipart_id = matches!( kind, @@ -134,32 +134,27 @@ mod tests { #[test] fn persisted_kind_validation_rejects_incompatible_fields() { assert!( - validate_persisted_kind( - &session( - UploadSessionKind::ProviderRelayMultipart, - Some("files/temp"), - None - ), - UploadSessionKind::ProviderRelayMultipart - ) + validate_persisted_kind(&session( + UploadSessionKind::ProviderRelayMultipart, + Some("files/temp"), + None + )) .is_err() ); assert!( - validate_persisted_kind( - &session( - UploadSessionKind::ProviderRelayMultipart, - None, - Some("multipart") - ), - UploadSessionKind::ProviderRelayMultipart - ) + validate_persisted_kind(&session( + UploadSessionKind::ProviderRelayMultipart, + None, + Some("multipart") + )) .is_err() ); assert!( - validate_persisted_kind( - &session(UploadSessionKind::ProviderPresignedSingle, None, None), - UploadSessionKind::ProviderPresignedSingle - ) + validate_persisted_kind(&session( + UploadSessionKind::ProviderPresignedSingle, + None, + None + )) .is_err() ); } @@ -172,18 +167,13 @@ mod tests { None, ); valid.provider_session_ciphertext = Some("encrypted-upload-url".to_string()); + assert!(validate_persisted_kind(&valid).is_ok()); assert!( - validate_persisted_kind(&valid, UploadSessionKind::ProviderDirectResumable).is_ok() - ); - assert!( - validate_persisted_kind( - &session( - UploadSessionKind::ProviderDirectResumable, - Some("files/temp"), - None - ), - UploadSessionKind::ProviderDirectResumable - ) + validate_persisted_kind(&session( + UploadSessionKind::ProviderDirectResumable, + Some("files/temp"), + None + )) .is_err() ); } diff --git a/tests/test_remote_storage.rs b/tests/test_remote_storage.rs index cbdd3e3b1..035da2168 100644 --- a/tests/test_remote_storage.rs +++ b/tests/test_remote_storage.rs @@ -1086,10 +1086,11 @@ async fn test_remote_ingress_profile_update_without_driver_change_keeps_remote_a error.api_error_code(), ApiErrorCode::ManagedIngressDriverUnsupported ); - assert!( - error.message().contains("update remote storage target"), - "unexpected error message: {}", - error.message() + assert_eq!( + error.storage_error_kind(), + Some(aster_drive::storage::StorageErrorKind::Transient), + "rename-only update should reach the remote transport: {}", + error.message(), ); } diff --git a/tests/test_upload.rs b/tests/test_upload.rs index 21610d95f..ef63148c1 100644 --- a/tests/test_upload.rs +++ b/tests/test_upload.rs @@ -8,6 +8,7 @@ use aster_drive::api::api_error_code::ApiErrorCode; use aster_drive::db::repository::policy_repo; use aster_drive::runtime::SharedRuntimeState; use aster_drive::services::auth::local; +use aster_drive::types::UploadSessionKind; use serde_json::Value; use testcontainers::{GenericImage, ImageExt, runners::AsyncRunner}; use tokio::task::JoinSet; @@ -186,6 +187,7 @@ impl<'a> UploadSessionSpec<'a> { upload_id: &'a str, status: aster_drive::types::UploadSessionStatus, expires_at: chrono::DateTime, + session_kind: UploadSessionKind, ) -> Self { Self { upload_id, @@ -193,7 +195,7 @@ impl<'a> UploadSessionSpec<'a> { expires_at, total_chunks: 0, received_count: 0, - session_kind: aster_drive::types::UploadSessionKind::OffsetStaging, + session_kind, policy_id: None, object_temp_key: None, object_multipart_id: None, @@ -212,11 +214,6 @@ impl<'a> UploadSessionSpec<'a> { self } - fn session_kind(mut self, kind: aster_drive::types::UploadSessionKind) -> Self { - self.session_kind = kind; - self - } - fn object_upload( mut self, object_temp_key: Option<&'a str>, @@ -2071,6 +2068,11 @@ async fn test_concurrent_duplicate_staged_chunk_keeps_one_complete_payload() { .expect("duplicate chunk uploads must finish while the exclusion observer is installed"); first_result.unwrap(); second_result.unwrap(); + assert_eq!( + exclusion.entry_count(), + 2, + "both duplicate requests must reach the staging write lock entry" + ); assert!( !exclusion.overlap_observed(), "duplicate chunk uploads entered the staging write critical section together" @@ -3034,9 +3036,9 @@ async fn test_explicit_session_kind_rejects_incompatible_fields() { &upload_id, UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::ProviderRelayMultipart, ) - .chunks(1, 0) - .session_kind(UploadSessionKind::ProviderRelayMultipart), + .chunks(1, 0), ) .await; @@ -3071,10 +3073,10 @@ async fn test_relay_kind_rejects_missing_object_temp_key() { &upload_id, UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::ProviderRelayMultipart, ) .chunks(1, 0) .policy(policy.id) - .session_kind(aster_drive::types::UploadSessionKind::ProviderRelayMultipart) .object_upload(None, Some("multipart")), ) .await; @@ -3112,9 +3114,9 @@ async fn test_relay_chunk_endpoint_rejects_missing_object_temp_key() { &upload_id, UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::ProviderRelayMultipart, ) .chunks(1, 0) - .session_kind(UploadSessionKind::ProviderRelayMultipart) .object_upload(None, Some("multipart")), ) .await; @@ -3156,9 +3158,9 @@ async fn test_relay_progress_rejects_zero_and_out_of_range_part_numbers() { &upload_id, UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::ProviderRelayMultipart, ) .chunks(2, 1) - .session_kind(UploadSessionKind::ProviderRelayMultipart) .object_upload(Some("files/temp"), Some("multipart")), ) .await; @@ -3203,9 +3205,9 @@ async fn test_explicit_staging_kind_does_not_fall_back_when_file_is_missing() { &upload_id, UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::OffsetStaging, ) - .chunks(1, 1) - .session_kind(UploadSessionKind::OffsetStaging), + .chunks(1, 1), ) .await; upload_session_part_repo::upsert_part( @@ -3247,9 +3249,9 @@ async fn test_presigned_session_kind_rejects_server_chunk_put() { &upload_id, UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::ProviderPresignedMultipart, ) .chunks(1, 0) - .session_kind(UploadSessionKind::ProviderPresignedMultipart) .object_upload(Some("files/temp"), Some("multipart")), ) .await; @@ -3269,9 +3271,9 @@ async fn test_presigned_session_kind_rejects_server_chunk_put() { &payload_upload_id, UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::ProviderPresignedMultipart, ) .chunks(1, 0) - .session_kind(UploadSessionKind::ProviderPresignedMultipart) .object_upload(Some("files/temp"), Some("multipart")), ) .await; @@ -3313,6 +3315,7 @@ async fn test_concurrent_chunk_upload_idempotent() { &upload_id, aster_drive::types::UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::OffsetStaging, ) .chunks(2, 0), ) @@ -3337,11 +3340,16 @@ async fn test_concurrent_chunk_upload_idempotent() { let upload_id2 = upload_id.clone(); let chunk1 = chunk_data.clone(); let chunk2 = chunk_data.clone(); + let concurrency = upload::test_support::install_same_chunk_exclusion_observer(&upload_id); - let (first, second) = tokio::join!( - upload::upload_chunk(&state1, &upload_id1, 0, user.id, &chunk1), - upload::upload_chunk(&state2, &upload_id2, 0, user.id, &chunk2), - ); + let (first, second) = tokio::time::timeout(std::time::Duration::from_secs(2), async { + tokio::join!( + upload::upload_chunk(&state1, &upload_id1, 0, user.id, &chunk1), + upload::upload_chunk(&state2, &upload_id2, 0, user.id, &chunk2), + ) + }) + .await + .expect("duplicate requests must rendezvous at the staging write lock entry"); let first = first.unwrap(); let second = second.unwrap(); @@ -3349,6 +3357,15 @@ async fn test_concurrent_chunk_upload_idempotent() { .await .unwrap(); + assert_eq!( + concurrency.entry_count(), + 2, + "both duplicate requests must reach the staging write lock entry" + ); + assert!( + !concurrency.overlap_observed(), + "same-chunk writes must remain mutually exclusive" + ); assert!( [first.received_count, second.received_count].contains(&1), "at least one concurrent upload should observe received_count=1" @@ -3358,6 +3375,27 @@ async fn test_concurrent_chunk_upload_idempotent() { "duplicate concurrent chunk upload should not increment count twice" ); + let receipts = aster_drive::db::repository::upload_session_part_repo::list_all_by_upload( + state.writer_db(), + &upload_id, + ) + .await + .unwrap(); + assert_eq!( + receipts.len(), + 1, + "duplicate chunk must persist one receipt" + ); + assert_eq!(receipts[0].part_number, 1); + assert_eq!( + receipts[0].etag, + upload::test_support::offset_staging_receipt_etag() + ); + assert_eq!(receipts[0].size, chunk_data.len() as i64); + let staged = tokio::fs::read(&staging_path).await.unwrap(); + assert_eq!(&staged[..chunk_data.len()], chunk_data.as_slice()); + + drop(concurrency); let third = upload::upload_chunk(&state, &upload_id, 0, user.id, &chunk_data) .await .unwrap(); @@ -3384,6 +3422,7 @@ async fn test_upload_session_part_upsert_updates_existing_row_without_duplicates &upload_id, aster_drive::types::UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::OffsetStaging, ) .chunks(2, 0), ) @@ -3725,10 +3764,10 @@ async fn test_complete_upload_keeps_presigned_multipart_session_retryable_after_ &upload_id, UploadSessionStatus::Presigned, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::RemotePresignedMultipart, ) .chunks(2, 0) .policy(remote_policy.id) - .session_kind(aster_drive::types::UploadSessionKind::RemotePresignedMultipart) .object_upload( Some("upload/data/files/presigned-retry-temp"), Some("presigned-retry-multipart"), @@ -3787,6 +3826,7 @@ async fn test_complete_offset_staging_stream_relay_remains_retryable_after_stora &upload_id, UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::OffsetStaging, ) .chunks(2, 0) .policy(remote_policy.id), @@ -3854,6 +3894,7 @@ async fn test_file_upload_complete_rejects_assembling_session() { &upload_id, aster_drive::types::UploadSessionStatus::Assembling, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::OffsetStaging, ) .chunks(2, 2), ) @@ -3882,6 +3923,7 @@ async fn test_file_upload_complete_completed_without_file_id_returns_refresh_hin &upload_id, aster_drive::types::UploadSessionStatus::Completed, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::OffsetStaging, ) .chunks(0, 0), ) @@ -3915,9 +3957,9 @@ async fn test_file_upload_complete_presigned_multipart_requires_parts() { &upload_id, aster_drive::types::UploadSessionStatus::Presigned, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::ProviderPresignedMultipart, ) .chunks(2, 0) - .session_kind(aster_drive::types::UploadSessionKind::ProviderPresignedMultipart) .object_upload(Some("files/temp-key"), Some("multipart-id")), ) .await; @@ -3946,6 +3988,7 @@ async fn test_file_upload_get_progress_lists_and_sorts_staging_receipts() { &upload_id, aster_drive::types::UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::OffsetStaging, ) .chunks(2, 2), ) @@ -4085,6 +4128,7 @@ async fn test_sqlite_reader_routes_do_not_wait_for_busy_writer_pool() { &upload_id, aster_drive::types::UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::OffsetStaging, ) .chunks(3, 2), ) @@ -4142,6 +4186,7 @@ async fn test_file_upload_presign_parts_rejects_non_multipart_session() { &upload_id, aster_drive::types::UploadSessionStatus::Presigned, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::ProviderPresignedSingle, ) .chunks(1, 0) .object_upload(Some("files/temp-key"), None), @@ -4175,6 +4220,7 @@ async fn test_file_upload_presign_parts_validates_part_number_batch() { &upload_id, aster_drive::types::UploadSessionStatus::Presigned, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::ProviderPresignedMultipart, ) .chunks(3, 0) .object_upload(Some("files/temp-key"), Some("multipart-id")), @@ -4220,6 +4266,7 @@ async fn test_file_upload_cleanup_expired_removes_local_sessions_only() { &expired_id, aster_drive::types::UploadSessionStatus::Uploading, chrono::Utc::now() - chrono::Duration::minutes(5), + UploadSessionKind::OffsetStaging, ) .chunks(2, 1), ) @@ -4233,6 +4280,7 @@ async fn test_file_upload_cleanup_expired_removes_local_sessions_only() { &completed_id, aster_drive::types::UploadSessionStatus::Completed, chrono::Utc::now() - chrono::Duration::minutes(5), + UploadSessionKind::OffsetStaging, ) .chunks(0, 0) .file_id(123), @@ -4247,6 +4295,7 @@ async fn test_file_upload_cleanup_expired_removes_local_sessions_only() { &assembling_id, aster_drive::types::UploadSessionStatus::Assembling, chrono::Utc::now() - chrono::Duration::minutes(5), + UploadSessionKind::OffsetStaging, ) .chunks(2, 2), ) @@ -4318,6 +4367,7 @@ async fn test_file_upload_cleanup_expired_keeps_remote_sessions_when_storage_is_ &upload_id, UploadSessionStatus::Uploading, chrono::Utc::now() - chrono::Duration::minutes(5), + UploadSessionKind::RemoteRelayMultipart, ) .chunks(2, 1) .policy(remote_policy.id) @@ -4511,6 +4561,7 @@ async fn test_cancel_upload_keeps_remote_session_when_object_cleanup_is_unavaila &upload_id, UploadSessionStatus::Uploading, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::RemotePresignedSingle, ) .chunks(2, 0) .policy(remote_policy.id) @@ -4572,6 +4623,7 @@ async fn test_upload_chunk_returns_session_expired_for_failed_multipart_session( &upload_id, UploadSessionStatus::Failed, chrono::Utc::now() + chrono::Duration::hours(1), + UploadSessionKind::ProviderRelayMultipart, ) .chunks(2, 0) .object_upload(Some("files/temp-key"), Some("multipart-id")),