diff --git a/AGENTS.md b/AGENTS.md index 8e812c2f..09eac400 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -168,6 +168,11 @@ To maintain a clear separation between fast, offline unit tests and containerize ### 20. Explicit Top-Level Class Imports - Always use top-level `import` statements at the top of Java files instead of writing fully qualified package class names inline in method signatures or method bodies (e.g. add `import me.desair.tus.server.util.Utils;` at the top of the file and call `Utils.interruptStream(...)` instead of writing `me.desair.tus.server.util.Utils.interruptStream(...)`). +### 21. Single Responsibility Principle & Constructor Simplicity +- Every class MUST have a single, well-defined main purpose (Single Responsibility Principle). +- Do NOT mix data serialization models (DTOs / JSON metadata objects) with active process or lifecycle management components (such as lock handles with scheduled thread executors or storage clients). +- Keep constructors focused and minimal (typically 1 or 2 constructors per class). Classes requiring data models MUST accept the dedicated data object (e.g., `LeaseData`) in their constructor rather than defining multiple telescoping or metadata-only constructor overloads. + ## IETF Resumable Uploads for HTTP (RUFH) Spec Maintenance & Update Playbook ### 1. Spec Diff Review diff --git a/CHANGELOG.md b/CHANGELOG.md index 5628bf6a..1fbba1bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this project will be documented in this file. ### New -- **NFS- & SMB-Safe Lease Locking (`LeaseFileLockingService`)**: Added distributed, container-safe filesystem locking using atomic directory creation (`mkdir`) and TTL-based JSON lease files with background heartbeat renewal. Operates reliably across multi-server replicas on NFS (v3/v4), AWS EFS, Azure Files, Windows SMB/CIFS, and local disks without requiring Redis, ZooKeeper, or OS-level `FileLock` daemons. Comprehensive guide and legacy opt-out instructions available in `docs/DISK_BASED_LOCKING.md`. +- **NFS- & SMB-Safe Lease Locking (`LeaseFileLockingService` & `LeaseFileMutex`)**: Added distributed, container-safe filesystem locking using atomic sibling mutex directories (`.mutex/`), in-place expired lock takeover, and TTL-based JSON lease files with background heartbeat renewal and ownership fencing. Operates reliably across multi-server replicas on NFS (v3/v4), AWS EFS, Azure Files, Windows SMB/CIFS, and local disks without requiring Redis, ZooKeeper, or OS-level `FileLock` daemons. Comprehensive guide and legacy opt-out instructions available in `docs/DISK_BASED_LOCKING.md`. - **S3-Compatible Storage & Distributed Locking**: Added native S3 storage support via `S3StorageService` (MinIO SDK), distributed locking via `S3LockingService` (S3 conditional writes with TTL leases and interrupt signals for multi-replica container deployments), S3-native concatenation via `S3ConcatenationService`, and complete documentation in `docs/S3_STORAGE.md`. - **Azure Blob Storage & Distributed Leases**: Added native Azure Blob Storage support via `AzureBlobStorageService` (Block Blob staging with streaming appends, sub-threshold buffering, truncation, and deduplication), distributed locking via `AzureBlobLockingService` (Azure Blob Leases with auto-renewal, JVM interruption, cross-replica `.stop` signals, and clean shutdown), zero-copy server-side concatenation via `AzureBlobConcatenationService` (`stageBlockFromUrl`), and comprehensive documentation in `docs/AZURE_BLOB_STORAGE.md`. - **IETF Resumable Uploads for HTTP (RUFH) Protocol**: Implemented full support for the official IETF Resumable Uploads for HTTP specification (`draft-ietf-httpbis-resumable-upload-12`). diff --git a/README.md b/README.md index 83762866..3d931890 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ You can add the latest stable version of this library to your application using me.desair.tus tus-java-server - 2.0.0-SNAPSHOT + 2.0.0 ``` @@ -117,7 +117,7 @@ Besides the [core protocol](https://tus.io/protocols/resumable-upload.html#core- The first step is to create a `TusFileUploadService` object using its constructor. You can make this object available as a (Spring bean) singleton or create a new instance for each request. After creating the object, you can configure it using the following methods: * `withUploadUri(String)`: Set the relative path (e.g. `/files/upload`) or absolute base URL (e.g. `https://upload.example.com/files/upload`) under which the main tus upload endpoint will be made available. When configured with an absolute URL, the `Location` header returned upon upload creation (201 Created, 200 OK, or 104 Interim Response) will contain the full URL. Optionally, this URI may contain regex parameters in order to support endpoints that contain URL parameters, for example `/users/[0-9]+/files/upload` or `https://upload.example.com/users/[0-9]+/files/upload`. -* `withSupportedProtocolVersions(ProtocolVersion)`: Configure supported protocol versions (`ProtocolVersion.AUTO` for automatic header-based detection, `ProtocolVersion.TUS_1_0_0` for Tus 1.0.0 only, or `ProtocolVersion.IETF` for IETF Resumable Uploads only). +* `withSupportedProtocolVersions(ProtocolVersion)`: Configure supported protocol versions (`ProtocolVersion.AUTO` for automatic header-based detection, `ProtocolVersion.TUS_1_0_0` for Tus 1.0.0 only, or `ProtocolVersion.RUFH` for IETF Resumable Uploads only). * `withMaxUploadSize(Long)`: Specify the maximum number of bytes that can be uploaded per upload. If you don't call this method, the maximum number of bytes is `Long.MAX_VALUE`. * `withStoragePath(String)`: If you're using the default file system-based storage service, you can use this method to specify the path where to store the uploaded bytes and upload information. * `withMaxLockRetries(int)`: Specify the maximum number of retries the service will attempt to acquire an upload lock before failing with an `UploadAlreadyLockedException` during lock contention resolution (e.g. for `HEAD` or `DELETE` requests). Default is `40` retries (with a 200ms sleep between retries, resulting in an 8.0-second retry budget). @@ -181,7 +181,7 @@ This server implementation has been tested with: For detailed instructions on running our native conformity test suite and interpreting results, see the **[Conformity Testing Guide (docs/CONFORMITY_TESTING.md)](docs/CONFORMITY_TESTING.md)**. -This repository also contains comprehensive automated integration test suites (`ITTusFileUploadService`, `IetfProtocolCreationTest`, `IetfProtocolAppendTest`, `IetfProtocolHeadTest`, `IetfProtocolCancellationTest`) validating both protocol specifications. +This repository also contains comprehensive automated integration test suites (`ITTusFileUploadService`, `RufhProtocolCreationTest`, `RufhProtocolAppendTest`, `RufhProtocolHeadTest`, `RufhProtocolCancellationTest`) validating both protocol specifications. ## Versioning This artifact follows `MAJOR.MINOR.PATCH` semantic versioning. Version `2.0.0` introduces major dual-protocol support for both Tus 1.0.0 and the IETF Resumable Uploads for HTTP specification (`draft-ietf-httpbis-resumable-upload`). Version `1.0.0-3.3` was the last Tus protocol-only version. diff --git a/docs/DISK_BASED_LOCKING.md b/docs/DISK_BASED_LOCKING.md index e5998998..13f275c6 100644 --- a/docs/DISK_BASED_LOCKING.md +++ b/docs/DISK_BASED_LOCKING.md @@ -16,19 +16,20 @@ The legacy `DiskLockingService` relies on OS kernel-level file locks (`java.nio. - **Ungraceful Crashes**: Pod crashes (`kill -9`, OOM killer, node eviction) leave locks stuck in NFS server state for minutes or indefinitely. - **Cross-Pod Coordination**: Kernel file locks are tracked in OS memory and do not coordinate cleanly across multi-replica container clusters. -### The Solution: Application-Level Lease Directories -`LeaseFileLockingService` replaces OS kernel locks with atomic directory creation (`mkdir`) and JSON lease files with background heartbeat renewal. This provides **zero external dependencies** (no Redis, ZooKeeper, or etcd cluster required) and works seamlessly on both local disks and distributed network shares. +### The Solution: Application-Level Lease Directories & Sibling Mutexes +`LeaseFileLockingService` replaces OS kernel locks with atomic sibling mutex directory creation (`mkdir`) and JSON lease files with background heartbeat renewal. This provides **zero external dependencies** (no Redis, ZooKeeper, or etcd cluster required) and works seamlessly on both local disks and distributed network shares. --- ## 2. Lock Directory Layout & Mechanics -Locks are structured as a dedicated directory containing a JSON lease metadata file: +Locks are structured as a dedicated directory containing a JSON lease metadata file, synchronized via a transient sibling mutex directory: ``` /locks/ -├── .lock/ # Dedicated lock directory (Atomic existence primitive) +├── .lock/ # Dedicated lock directory (contains lease.json) │ └── lease.json # JSON lease metadata (holderId, expiresAt, acquiredAt) +├── .mutex/ # Transient sibling atomic mutex directory (age <= 5s) └── .stop # Empty signal file for lock contention interruption ``` @@ -45,40 +46,37 @@ Locks are structured as a dedicated directory containing a JSON lease metadata f ``` ### Architectural Rationale: -1. **Universal Atomic Directory Staging & Renames**: Rather than creating an empty directory directly at `.lock` and subsequently writing metadata into it (which creates a window where concurrent nodes observe an empty, uninitialized directory), new locks are staged in a temporary sibling directory (`.lock.stage.`) with `lease.json` pre-written, then moved atomically into place (`Files.move` with `StandardCopyOption.ATOMIC_MOVE`). Directory moves map directly to atomic server-side RPCs on both POSIX NFS (`rename(2)`) and Windows SMB (`SetFileInformationByHandle`), ensuring `.lock` is born on disk 100% complete and valid. -2. **Clean Encapsulation**: Placing `lease.json` inside `.lock/` prevents metadata clutter and guarantees that lease updates and watchdog renewals are scoped directly to the lock entity. -3. **Atomic Eviction & Move Isolation**: Stale lock cleanup isolates the target directory by atomically renaming it (`StandardCopyOption.ATOMIC_MOVE` to `.evicting.`) before inspecting and deleting its contents. This isolates expired state and allows post-move rollback verification, preventing race collisions between multiple recovering nodes. +1. **Sibling Mutex Isolation (`.mutex/`)**: All state-modifying operations (acquisition, in-place takeover, release, and cleanup) acquire `.mutex/` via atomic `Files.createDirectory`. Because the mutex is a sibling of `.lock/`, the lock directory contains only `lease.json`, avoiding nested directory deletion races (`DirectoryNotEmptyException`) and Windows handle locking conflicts. +2. **In-Place Takeover (Zero Directory Moves)**: Rather than moving or displacing the lock directory during eviction (which creates a TOCTOU hole where the directory temporarily vanishes from disk), expired locks are updated in place directly inside `.lock/` under mutex protection. +3. **Fencing & Ownership Verification**: Both `lock.close()` and heartbeat renewals verify that `holderId` in `lease.json` matches the current holder before modifying or deleting files, ensuring a paused node never corrupts a successor's active lease. +4. **5-Second Crash Recovery**: Stale mutex directories left behind by crashed nodes are detected via `now - mtime >= 5000ms`, cleanly recovered, and retried. --- ## 3. Distributed Concurrency & Contention Resolution -### 1. Lock Acquisition Flow (Atomic Directory Staging) -To ensure that an observing process never encounters an empty or partially written lock directory, lock creation uses **Atomic Directory Staging**: +### 1. Lock Acquisition Flow (`tryAcquireLock`) +To acquire or take over a lock: 1. Extract `UploadId` from the request URI. -2. Verify that `/locks/.lock` does not already exist. If it exists: - - If `lease.json` is unexpired: Lock is actively held on another replica $\rightarrow$ throw `UploadAlreadyLockedException`. - - If `lease.json` is expired: Holder crashed $\rightarrow$ proceed to **Safe Atomic Eviction** and retry acquisition. -3. Create a unique temporary staging directory: `/locks/.lock.stage.`. -4. Write the complete `lease.json` file inside the staging directory. -5. Execute `Files.move(stageDir, lockDirPath, StandardCopyOption.ATOMIC_MOVE)`. - - **Success**: The lock directory appears on disk atomically with a valid, fully populated `lease.json` already inside it. Start the background heartbeat daemon (renews every $\text{leaseDuration} / 3$) and return `LeaseFileUploadLock`. - - **Collision (Already Exists)**: `Files.move` fails because another node acquired the lock in the interim. Clean up `stageDir` and throw `UploadAlreadyLockedException`. - -### 2. TOCTOU Mitigation in Expired Lock Eviction (Post-Move Verification & Rollback) -When multiple cluster nodes concurrently discover an expired lock left behind by a crashed pod, a **Time-of-Check to Time-of-Use (TOCTOU)** race condition can arise: -1. **Time of Check (TOC)**: Node A and Node B both inspect `.lock` and observe that its lease has expired. -2. **Node A Wins**: Node A renames the expired directory to `.evicting.`, deletes it, and stages/moves a brand-new active lock. -3. **Time of Use (TOU) Hazard**: Node B (having verified expiration in Step 1) executes eviction on Node A's **active** directory. Without post-move verification, Node B destroys Node A's directory and acquires a second lock handle, causing dual ownership. - -**The Solution: Post-Move Verification & Rollback**: -- When Node B isolates the directory via `Files.move(lockDirPath, evictPath, ATOMIC_MOVE)`, it immediately re-inspects `evictPath` post-move. -- If `evictPath` contains an active lease (created by Node A right before Node B's move), Node B recognizes that it lost the race. -- Node B immediately rolls back the move via `Files.move(evictPath, lockDirPath, ATOMIC_MOVE)` and aborts eviction. -- Exactly one node wins the eviction and acquisition, preserving single-owner lock exclusivity. +2. Acquire sibling mutex `/locks/.mutex/` via `Files.createDirectory`. + - **Collision (Already Exists)**: If `mtime` is $< 5$s old, another live node is modifying the lock $\rightarrow$ throw `UploadAlreadyLockedException`. If $\ge 5$s old, clean up stale mutex and retry. +3. Under mutex protection: + - If `.lock/lease.json` exists and is **unexpired**: lock is actively held on another replica $\rightarrow$ throw `UploadAlreadyLockedException`. + - If `.lock/` does not exist: create directory. + - If `lease.json` is missing or expired: write updated `lease.json` via a temporary file and atomically rename it into place (`Files.move` with `ATOMIC_MOVE, REPLACE_EXISTING`). This prevents concurrent read-only queries (like `isLocked()`) from ever observing an empty (0-byte) or partially written JSON file during disk flushes. + - Start background heartbeat daemon (renews every $\text{leaseDuration} / 3$) and return `LeaseFileUploadLock`. +4. Release `.mutex/`. + +### 2. Lock Release Flow (`lock.close()`) +When an active upload completes or is aborted: +1. Stop background heartbeat daemon. +2. Acquire sibling mutex `.mutex/`. +3. Read `lease.json` and verify `holderId == this.holderId`. +4. If ownership matches: delete `lease.json` and delete `.lock/`. +5. Release `.mutex/`. ### 3. Heartbeat Lease Auto-Renewal -Active streaming uploads periodically renew their lease by updating `expiresAt` in `lease.json` every $\text{leaseDuration} / 3$ (default: every 10 seconds for a 30s lease). When the request completes, `lock.close()` stops the daemon and removes the lock directory. +Active streaming uploads periodically renew their lease by updating `expiresAt` in `lease.json` every $\text{leaseDuration} / 3$ (default: every 10 seconds for a 30s lease). The renewal verifies `holderId` ownership under the sibling mutex to ensure it aborts if the lease was taken over after a long pause. ### 4. Lock Contention & `.stop` Signal Files When a client sends a `HEAD` or `DELETE` request to resume or cancel an upload while a stalled `PATCH` stream holds the lock: diff --git a/docs/LOCKING.md b/docs/LOCKING.md index 70385988..0b946612 100644 --- a/docs/LOCKING.md +++ b/docs/LOCKING.md @@ -26,7 +26,7 @@ All locking mechanisms in `tus-java-server` implement the `UploadLockingService` ### `UploadLockingService` Interface ```java -public interface UploadLockingService extends Closeable { +public interface UploadLockingService { // Acquires an exclusive lock on an upload resource UploadLock lockUploadByUri(String requestUri) throws TusException, IOException; @@ -37,17 +37,17 @@ public interface UploadLockingService extends Closeable { // Cleans up stale or expired locks void cleanupStaleLocks() throws IOException; + // Injects the UploadIdFactory instance used to parse upload IDs from request URIs + void setIdFactory(UploadIdFactory idFactory); + // Registers the active request input stream so it can be interrupted cleanly default void registerInputStream(String requestUri, InputStream inputStream) {} // Requests that any active lock for the URI be released default void requestLockRelease(String requestUri) {} - // Injects the UploadIdFactory instance used to parse upload IDs from request URIs - default void setIdFactory(UploadIdFactory idFactory) {} - - // Injects the upload expiration period in milliseconds - default void setUploadExpirationPeriod(Long expirationPeriod) {} + // Closes resources, interrupts in-flight streams, and shuts down background daemon threads + default void close() throws IOException {} } ``` @@ -84,7 +84,7 @@ public interface UploadLock extends Closeable { | Storage Backend / Environment | Locking Service Class | Key Characteristics & Architecture | Documentation File | |---|---|---|---| -| **Disk & Network Filesystems (Default)** | `LeaseFileLockingService` | Atomic directory staging & renames (`mkdir`/`rename`), TTL-based JSON lease files with heartbeat renewal, TOCTOU-safe eviction with rollback, and `.stop` signal files. Fully safe on NFSv3/v4, AWS EFS, SMB/CIFS, Kubernetes containers, and local disks. | [`docs/DISK_BASED_LOCKING.md`](file:///Users/tom/projects/tus-java-server/docs/DISK_BASED_LOCKING.md) | +| **Disk & Network Filesystems (Default)** | `LeaseFileLockingService` | Atomic sibling mutex directory (`.mutex/`), in-place expired lock takeover, TTL-based JSON lease files with heartbeat renewal, ownership fencing, and `.stop` signal files. Fully safe on NFSv3/v4, AWS EFS, SMB/CIFS, Kubernetes containers, and local disks. | [`docs/DISK_BASED_LOCKING.md`](file:///Users/tom/projects/tus-java-server/docs/DISK_BASED_LOCKING.md) | | **Local File System (Legacy Opt-Out)** | `DiskLockingService` | OS kernel-level exclusive POSIX `FileLock` (`fcntl`) with JVM shutdown hooks and `.stop` signal files. Best for single-node deployments on local disk. | [`docs/DISK_BASED_LOCKING.md`](file:///Users/tom/projects/tus-java-server/docs/DISK_BASED_LOCKING.md) | | **Amazon S3 / S3-Compatible** | `S3LockingService` | S3 object-backed TTL lease objects (`.lock`), conditional writes (`If-None-Match: *`), jittered read-after-write verification, heartbeat renewal, and cross-pod `.stop` signal object polling watchdog. | [`docs/S3_STORAGE.md`](file:///Users/tom/projects/tus-java-server/docs/S3_STORAGE.md) | | **Azure Blob Storage** | `AzureBlobLockingService` | Native Azure Blob Storage exclusive 30-second leases (`BlobLeaseClient`), background daemon renewal, and `.stop` signal blob polling watchdog. | [`docs/AZURE_BLOB_STORAGE.md`](file:///Users/tom/projects/tus-java-server/docs/AZURE_BLOB_STORAGE.md) | diff --git a/docs/MIGRATION.md b/docs/MIGRATION.md index b3da9c84..f70a0fe0 100644 --- a/docs/MIGRATION.md +++ b/docs/MIGRATION.md @@ -24,7 +24,7 @@ import me.desair.tus.server.TusFileUploadService; TusFileUploadService tusService = new TusFileUploadService() .withUploadUri("/files") - // Enabled by default: AUTO mode detects TUS_1_0_0 vs IETF per request + // Enabled by default: AUTO mode detects TUS_1_0_0 vs RUFH per request .withSupportedProtocolVersions(ProtocolVersion.AUTO); ``` diff --git a/docs/RELEASE.md b/docs/RELEASE.md index 231eefcd..6f053c5d 100644 --- a/docs/RELEASE.md +++ b/docs/RELEASE.md @@ -133,16 +133,13 @@ After the release is successfully deployed to staging, clean up the temporary re mvn release:clean ``` -### 6. Verify and Release on Sonatype Nexus +### 6. Verify and Release on Sonatype Central Portal -Finally, log into Sonatype Nexus to verify and officially publish the staged artifacts to Maven Central: +Finally, log into Sonatype Central Portal to verify and officially publish the staged deployment to Maven Central: 1. Go to [https://central.sonatype.com/publishing/deployments](https://central.sonatype.com/publishing/deployments) -3. Find the deployment based on the ID that was printed during the release process. It should have status `Validated`. -4. Open the `Component Files` section. Verify the POM versions and the presence of `.jar`, `-javadoc.jar`, `-sources.jar`, and their `.asc` signatures. -5. If everything looks correct, click the **Publish** button in the top menu to publish the release to world. -6. The status of the deployment will go to `Publishing` and it will take a few minutes before it completes. - - -6. Once successfully closed, click the **Release** button to publish the artifacts to Maven Central. *(Note: It may take a few hours for the artifacts to sync and appear on search.maven.org).* -7. If anything went wrong during your verification, click **Drop** instead, fix the issue in the codebase, and restart the release process. +2. Find the deployment based on the ID printed during the release process. It should have status `Validated`. +3. Open the `Component Files` section. Verify the POM versions and the presence of `.jar`, `-javadoc.jar`, `-sources.jar`, and their `.asc` signatures. +4. If everything looks correct, click the **Publish** button in the top menu to publish the release to Maven Central. +5. The status of the deployment will transition to `Publishing` and complete within a few minutes. *(Note: It may take a few hours for the artifacts to sync across global CDN edges and appear on search.maven.org).* +6. If anything went wrong during your verification, click **Drop** instead, fix the issue in the codebase, and restart the release process. diff --git a/docs/S3_STORAGE.md b/docs/S3_STORAGE.md index 6e0023f1..6db4261a 100644 --- a/docs/S3_STORAGE.md +++ b/docs/S3_STORAGE.md @@ -311,13 +311,14 @@ When the test suite executes: | Test Class | Purpose | Execution Mode | |------------|---------|----------------| | `UploadInfoJsonSerializerTest` | Unit test for Jackson JSON serialization (`me.desair.tus.server.util`) | Mocked / JVM | -| `S3UploadLockJsonSerializerTest` | Unit test for S3 lock object JSON serialization (`me.desair.tus.server.util`) | Mocked / JVM | +| `LeaseDataJsonSerializerTest` | Unit test for lease data JSON serialization (`me.desair.tus.server.util`) | Mocked / JVM | | `S3StorageServiceTest` | Fast unit test for S3 storage logic | Mocked `MinioClient` | | `S3LockingServiceTest` | Fast unit test for S3 distributed locking | Mocked `MinioClient` | | `S3ConcatenationServiceTest` | Fast unit test for S3 concatenation logic | Mocked `MinioClient` | -| `ITS3StorageServiceTest` | Integration test for S3 storage | Live MinIO Testcontainer | -| `ITS3LockingServiceTest` | Integration test for S3 distributed locking & contention | Live MinIO Testcontainer | -| `ITS3TusFileUploadServiceTest` | Full end-to-end HTTP protocol lifecycle test | Live MinIO Testcontainer | +| `ITS3StorageService` | Integration test for S3 storage | Live MinIO Testcontainer | +| `ITS3LockingService` | Integration test for S3 distributed locking & contention | Live MinIO Testcontainer | +| `ITS3RufhProtocol` | IETF RUFH protocol integration suite for S3 backend | Live MinIO Testcontainer | +| `ITS3TusFileUploadService` | Full end-to-end HTTP protocol lifecycle test | Live MinIO Testcontainer | ### Troubleshooting diff --git a/docs/TESTING.md b/docs/TESTING.md index 7f2b72ab..c664614c 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -16,7 +16,7 @@ Open the `spring-boot-rest/pom.xml` file in the sibling `tus-java-server-spring- me.desair.tus tus-java-server - 1.0.0-3.2-SNAPSHOT + 2.0.0 ``` @@ -104,7 +104,7 @@ curl -X HEAD -H "Tus-Resumable: 1.0.0" -I ${UPLOAD_URL} - **First Terminal (PATCH)**: - The stalled/throttled `PATCH` upload should immediately abort with an I/O error or exit because the server terminated its input stream in response to the lock release request. - **Second Terminal (HEAD)**: - - The `HEAD` request should succeed with status `204 No Content` after a short delay (the server retries up to 25 times at 200ms intervals while releasing the lock). + - The `HEAD` request should succeed with status `204 No Content` after a short delay (the server retries up to 40 times at 200ms intervals, an 8.0-second retry budget, while releasing the lock). - The response will contain the `Upload-Offset` header reflecting the number of bytes successfully written to disk before the stream was interrupted (e.g., `Upload-Offset: 153600`). - **Subsequent Uploads**: - You can immediately resume the upload using a new `PATCH` request starting from the offset returned in the `HEAD` response. diff --git a/src/main/java/me/desair/tus/server/HttpProblemDetails.java b/src/main/java/me/desair/tus/server/HttpProblemDetails.java index 8d1c525c..ad7187bf 100644 --- a/src/main/java/me/desair/tus/server/HttpProblemDetails.java +++ b/src/main/java/me/desair/tus/server/HttpProblemDetails.java @@ -164,13 +164,7 @@ public void writeTo(HttpServletResponse response) throws IOException { * @throws IOException When writing to response stream fails */ public void writeTo(TusServletResponse response) throws IOException { - Objects.requireNonNull(response, "Response cannot be null"); - byte[] jsonBytes = toJson().getBytes(java.nio.charset.StandardCharsets.UTF_8); - response.setStatus(status); - response.setHeader(HttpHeader.CONTENT_TYPE, HttpHeader.CONTENT_TYPE_PROBLEM_JSON); - response.setContentLength(jsonBytes.length); - response.getWriter().write(new String(jsonBytes, java.nio.charset.StandardCharsets.UTF_8)); - response.getWriter().flush(); + writeTo((HttpServletResponse) response); } public int getStatus() { diff --git a/src/main/java/me/desair/tus/server/TusFileUploadService.java b/src/main/java/me/desair/tus/server/TusFileUploadService.java index e6698f00..c6299713 100644 --- a/src/main/java/me/desair/tus/server/TusFileUploadService.java +++ b/src/main/java/me/desair/tus/server/TusFileUploadService.java @@ -31,7 +31,6 @@ import me.desair.tus.server.upload.UploadStorageService; import me.desair.tus.server.upload.UuidUploadIdFactory; import me.desair.tus.server.upload.cache.ThreadLocalCachedStorageAndLockingService; -import me.desair.tus.server.upload.disk.DiskLockingService; import me.desair.tus.server.upload.disk.DiskStorageService; import me.desair.tus.server.upload.disk.LeaseFileLockingService; import me.desair.tus.server.util.TusServletRequest; @@ -43,7 +42,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** Helper class that implements the server side tus v1.0.0 upload protocol */ +/** + * Helper class that implements the server side tus v1.0.0 upload protocol and the official IETF + * Resumable Uploads for HTTP (RUFH) specification. + */ public class TusFileUploadService implements Closeable { public static final String TUS_API_VERSION = "1.0.0"; @@ -66,7 +68,7 @@ public class TusFileUploadService implements Closeable { public TusFileUploadService() { String storagePath = FileUtils.getTempDirectoryPath() + File.separator + "tus"; this.uploadStorageService = new DiskStorageService(idFactory, storagePath); - this.uploadLockingService = new DiskLockingService(idFactory, storagePath); + this.uploadLockingService = new LeaseFileLockingService(idFactory, storagePath); initFeatures(); } diff --git a/src/main/java/me/desair/tus/server/upload/AbstractLeaseLock.java b/src/main/java/me/desair/tus/server/upload/AbstractLeaseLock.java index 93a50f48..e9a7d072 100644 --- a/src/main/java/me/desair/tus/server/upload/AbstractLeaseLock.java +++ b/src/main/java/me/desair/tus/server/upload/AbstractLeaseLock.java @@ -1,10 +1,8 @@ package me.desair.tus.server.upload; -import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonInclude; import java.io.InputStream; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import me.desair.tus.server.util.Utils; @@ -14,98 +12,57 @@ /** * Abstract base class for distributed lease-based implementations of {@link UploadLock}. * - *

Provides common state and lifecycle management for TTL-based locks including: + *

Provides common lifecycle management for TTL-based locks including: * *

    - *
  • Lease holder identification, expiration timestamps, and target upload URIs. *
  • Background heartbeat lease auto-renewal daemon scheduling. *
  • Active request input stream registration tracking and clean shutdown. *
*/ -@JsonIgnoreProperties(ignoreUnknown = true) -@JsonInclude(JsonInclude.Include.NON_NULL) public abstract class AbstractLeaseLock implements UploadLock { private static final Logger log = LoggerFactory.getLogger(AbstractLeaseLock.class); - private String holderId; - private String requestUri; - private long leaseDurationMs; - private long expiresAt; - private long acquiredAt; - - @JsonIgnore private ScheduledExecutorService heartbeatExecutor; - @JsonIgnore private Map activeInputStreams; - - /** Default constructor for Jackson JSON deserialization. */ - protected AbstractLeaseLock() {} - - /** - * Base constructor for metadata serialization. - * - * @param holderId Unique identifier of the lock holder - * @param requestUri Target upload URI - * @param leaseDurationMs Lease duration in milliseconds - * @param expiresAt Absolute expiration epoch timestamp in milliseconds - */ - protected AbstractLeaseLock( - String holderId, String requestUri, long leaseDurationMs, long expiresAt) { - this.holderId = holderId; - this.requestUri = requestUri; - this.leaseDurationMs = leaseDurationMs; - this.expiresAt = expiresAt; - this.acquiredAt = System.currentTimeMillis(); - } + private final LeaseData leaseData; + private ScheduledExecutorService heartbeatExecutor; + private final Map activeInputStreams; /** * Constructs an active lock and schedules the background heartbeat renewal daemon. * - * @param holderId Unique identifier of the lock holder - * @param leaseDurationMs Lease duration in milliseconds - * @param requestUri Target upload URI + * @param leaseData The lease metadata * @param activeInputStreams Map of active request input streams in the JVM * @param watchdogThreadName Name prefix for the watchdog heartbeat thread */ protected AbstractLeaseLock( - String holderId, - long leaseDurationMs, - String requestUri, - Map activeInputStreams, - String watchdogThreadName) { - this(holderId, leaseDurationMs, requestUri, activeInputStreams, null, watchdogThreadName); + LeaseData leaseData, Map activeInputStreams, String watchdogThreadName) { + this(leaseData, activeInputStreams, null, watchdogThreadName); } /** * Full constructor allowing injection of a custom executor (e.g. for testing). * - * @param holderId Unique identifier of the lock holder - * @param leaseDurationMs Lease duration in milliseconds - * @param requestUri Target upload URI + * @param leaseData The lease metadata * @param activeInputStreams Map of active request input streams in the JVM * @param heartbeatExecutor ScheduledExecutorService for lease renewal, or null to auto-schedule * @param watchdogThreadName Name prefix for the watchdog heartbeat thread */ protected AbstractLeaseLock( - String holderId, - long leaseDurationMs, - String requestUri, + LeaseData leaseData, Map activeInputStreams, ScheduledExecutorService heartbeatExecutor, String watchdogThreadName) { - this.holderId = holderId; - this.leaseDurationMs = leaseDurationMs; - this.requestUri = requestUri; + this.leaseData = Objects.requireNonNull(leaseData, "leaseData must not be null"); this.activeInputStreams = activeInputStreams; - this.acquiredAt = System.currentTimeMillis(); - this.expiresAt = this.acquiredAt + leaseDurationMs; + long leaseDurationMs = leaseData.getLeaseDurationMs(); if (heartbeatExecutor != null) { this.heartbeatExecutor = heartbeatExecutor; } else if (leaseDurationMs > 0 && watchdogThreadName != null) { long renewalPeriodMs = Math.max(1000L, leaseDurationMs / 3); this.heartbeatExecutor = Utils.scheduleWatchdog( - watchdogThreadName + "-" + holderId, + watchdogThreadName + "-" + leaseData.getHolderId(), this::renewLease, renewalPeriodMs, renewalPeriodMs, @@ -113,49 +70,37 @@ protected AbstractLeaseLock( } } - public String getHolderId() { - return holderId; + public LeaseData getLeaseData() { + return leaseData; } - public void setHolderId(String holderId) { - this.holderId = holderId; + public String getHolderId() { + return leaseData.getHolderId(); } public String getRequestUri() { - return requestUri; - } - - public void setRequestUri(String requestUri) { - this.requestUri = requestUri; + return leaseData.getRequestUri(); } public long getLeaseDurationMs() { - return leaseDurationMs; - } - - public void setLeaseDurationMs(long leaseDurationMs) { - this.leaseDurationMs = leaseDurationMs; + return leaseData.getLeaseDurationMs(); } public long getExpiresAt() { - return expiresAt; + return leaseData.getExpiresAt(); } public void setExpiresAt(long expiresAt) { - this.expiresAt = expiresAt; + leaseData.setExpiresAt(expiresAt); } public long getAcquiredAt() { - return acquiredAt; - } - - public void setAcquiredAt(long acquiredAt) { - this.acquiredAt = acquiredAt; + return leaseData.getAcquiredAt(); } @Override public String getUploadUri() { - return requestUri; + return leaseData.getRequestUri(); } @Override @@ -169,8 +114,8 @@ public void close() { Utils.shutdownExecutor(heartbeatExecutor); // 2. Remove active stream registration from JVM heap - if (activeInputStreams != null && requestUri != null) { - activeInputStreams.remove(requestUri); + if (activeInputStreams != null && getRequestUri() != null) { + activeInputStreams.remove(getRequestUri()); } // 3. Release backend storage resources @@ -181,7 +126,7 @@ public void close() { * Renew the lock lease by advancing the expiration timestamp and persisting the updated metadata. */ public void renewLease() { - this.expiresAt = System.currentTimeMillis() + leaseDurationMs; + leaseData.setExpiresAt(System.currentTimeMillis() + leaseData.getLeaseDurationMs()); doRenewLease(); } diff --git a/src/main/java/me/desair/tus/server/upload/AbstractLeaseLockingService.java b/src/main/java/me/desair/tus/server/upload/AbstractLeaseLockingService.java index 02daa98d..e82f64a8 100644 --- a/src/main/java/me/desair/tus/server/upload/AbstractLeaseLockingService.java +++ b/src/main/java/me/desair/tus/server/upload/AbstractLeaseLockingService.java @@ -153,7 +153,12 @@ protected void doCleanupOnClose() throws IOException {} */ protected UploadLock acquireOrEvictExpiredLock( UploadId uploadId, String holderId, String requestUri) throws TusException, IOException { - UploadLock lock = tryAcquireLock(uploadId, holderId, requestUri); + long now = System.currentTimeMillis(); + long expiresAt = now + leaseDurationMs; + LeaseData leaseData = + new LeaseData(holderId, requestUri, leaseDurationMs, expiresAt, now, null, null); + + UploadLock lock = tryAcquireLock(uploadId, leaseData); if (lock != null) { return lock; } @@ -163,7 +168,10 @@ protected UploadLock acquireOrEvictExpiredLock( // Lock is expired or abandoned: evict and retry acquisition boolean evicted = evictExpiredLock(uploadId); if (evicted) { - lock = tryAcquireLock(uploadId, holderId, requestUri); + now = System.currentTimeMillis(); + leaseData.setAcquiredAt(now); + leaseData.setExpiresAt(now + leaseDurationMs); + lock = tryAcquireLock(uploadId, leaseData); if (lock != null) { return lock; } @@ -176,13 +184,12 @@ protected UploadLock acquireOrEvictExpiredLock( * Subclass implementation of atomic primary lock acquisition. * * @param uploadId The upload identifier - * @param holderId The unique identifier of the lock contender - * @param requestUri The target upload request URI + * @param leaseData The lease metadata describing the lock to acquire * @return Acquired {@link UploadLock}, or null if already held * @throws IOException If an I/O error occurs */ - protected abstract UploadLock tryAcquireLock( - UploadId uploadId, String holderId, String requestUri) throws IOException; + protected abstract UploadLock tryAcquireLock(UploadId uploadId, LeaseData leaseData) + throws IOException; /** * Subclass implementation determining if an upload lock is currently expired or abandoned. diff --git a/src/main/java/me/desair/tus/server/upload/LeaseData.java b/src/main/java/me/desair/tus/server/upload/LeaseData.java new file mode 100644 index 00000000..66f0be3d --- /dev/null +++ b/src/main/java/me/desair/tus/server/upload/LeaseData.java @@ -0,0 +1,154 @@ +package me.desair.tus.server.upload; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import java.io.Serializable; +import java.util.Objects; + +/** + * Serializable data transfer object representing the on-disk or cloud JSON metadata of an upload + * lock lease. + * + *

Contains the lease holder identifier, target request URI, duration, acquisition timestamp, + * expiration timestamp, lock resource path/key, and contention stop signal path/key. + */ +@JsonIgnoreProperties(ignoreUnknown = true) +@JsonInclude(JsonInclude.Include.NON_NULL) +public class LeaseData implements Serializable { + + private static final long serialVersionUID = 1L; + + private String holderId; + private String requestUri; + private long leaseDurationMs; + private long acquiredAt; + private long expiresAt; + private String lockPath; + private String stopPath; + + /** Default constructor for Jackson JSON deserialization. */ + public LeaseData() {} + + /** + * Convenience constructor initializing timestamps automatically. + * + * @param holderId Unique identifier of the lock holder + * @param requestUri Target upload URI + * @param leaseDurationMs Lease duration in milliseconds + * @param expiresAt Timestamp when lease expires + */ + public LeaseData(String holderId, String requestUri, long leaseDurationMs, long expiresAt) { + this(holderId, requestUri, leaseDurationMs, expiresAt, System.currentTimeMillis(), null, null); + } + + /** + * Full constructor. + * + * @param holderId Unique identifier of the lock holder + * @param requestUri Target upload URI + * @param leaseDurationMs Lease duration in milliseconds + * @param expiresAt Timestamp when lease expires + * @param acquiredAt Timestamp when lease was acquired + * @param lockPath Storage path or S3 key of the lock + * @param stopPath Contention stop signal path or S3 key + */ + public LeaseData( + String holderId, + String requestUri, + long leaseDurationMs, + long expiresAt, + long acquiredAt, + String lockPath, + String stopPath) { + this.holderId = holderId; + this.requestUri = requestUri; + this.leaseDurationMs = leaseDurationMs; + this.expiresAt = expiresAt; + this.acquiredAt = acquiredAt; + this.lockPath = lockPath; + this.stopPath = stopPath; + } + + public String getHolderId() { + return holderId; + } + + public void setHolderId(String holderId) { + this.holderId = holderId; + } + + public String getRequestUri() { + return requestUri; + } + + public void setRequestUri(String requestUri) { + this.requestUri = requestUri; + } + + public long getLeaseDurationMs() { + return leaseDurationMs; + } + + public void setLeaseDurationMs(long leaseDurationMs) { + this.leaseDurationMs = leaseDurationMs; + } + + public long getAcquiredAt() { + return acquiredAt; + } + + public void setAcquiredAt(long acquiredAt) { + this.acquiredAt = acquiredAt; + } + + public long getExpiresAt() { + return expiresAt; + } + + public void setExpiresAt(long expiresAt) { + this.expiresAt = expiresAt; + } + + public String getLockPath() { + return lockPath; + } + + public void setLockPath(String lockPath) { + this.lockPath = lockPath; + } + + public String getStopPath() { + return stopPath; + } + + public void setStopPath(String stopPath) { + this.stopPath = stopPath; + } + + public boolean isExpired(long now) { + return expiresAt < now; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof LeaseData that)) { + return false; + } + return leaseDurationMs == that.leaseDurationMs + && acquiredAt == that.acquiredAt + && expiresAt == that.expiresAt + && Objects.equals(holderId, that.holderId) + && Objects.equals(requestUri, that.requestUri) + && Objects.equals(lockPath, that.lockPath) + && Objects.equals(stopPath, that.stopPath); + } + + @Override + public int hashCode() { + return Objects.hash( + holderId, requestUri, leaseDurationMs, acquiredAt, expiresAt, lockPath, stopPath); + } +} diff --git a/src/main/java/me/desair/tus/server/upload/azure/AzureBlobStorageService.java b/src/main/java/me/desair/tus/server/upload/azure/AzureBlobStorageService.java index 3d3f4aff..c53751c1 100644 --- a/src/main/java/me/desair/tus/server/upload/azure/AzureBlobStorageService.java +++ b/src/main/java/me/desair/tus/server/upload/azure/AzureBlobStorageService.java @@ -678,6 +678,11 @@ public boolean isUploadDeduplicationEnabled() { return deduplicationEnabled; } + @Override + public boolean isJsonSerializationEnabled() { + return true; + } + @Override public void setUploadConcatenationService(UploadConcatenationService concatenationService) { this.concatenationService = concatenationService; diff --git a/src/main/java/me/desair/tus/server/upload/disk/DiskLockingService.java b/src/main/java/me/desair/tus/server/upload/disk/DiskLockingService.java index 9c89fae3..dfb7c696 100644 --- a/src/main/java/me/desair/tus/server/upload/disk/DiskLockingService.java +++ b/src/main/java/me/desair/tus/server/upload/disk/DiskLockingService.java @@ -285,11 +285,7 @@ private void checkStopFileAndInterrupt(String idStr, InterruptibleInputStream st log.info("Watchdog detected stop file for upload ID {}. Interrupting stream.", idStr); Utils.interruptStream(stream); activeLocks.remove(idStr); - try { - Files.deleteIfExists(stopFilePath); - } catch (IOException e) { - // ignore - } + Utils.deletePathQuietly(stopFilePath); } } @@ -334,11 +330,7 @@ public void release() { } finally { activeLocks.remove(uploadIdStr); if (stopFilePath != null) { - try { - Files.deleteIfExists(stopFilePath); - } catch (IOException e) { - log.warn("Unable to delete stop file " + stopFilePath, e); - } + Utils.deletePathQuietly(stopFilePath); } } } @@ -350,11 +342,7 @@ public void close() throws IOException { } finally { activeLocks.remove(uploadIdStr); if (stopFilePath != null) { - try { - Files.deleteIfExists(stopFilePath); - } catch (IOException e) { - log.warn("Unable to delete stop file " + stopFilePath, e); - } + Utils.deletePathQuietly(stopFilePath); } } } diff --git a/src/main/java/me/desair/tus/server/upload/disk/DiskStorageService.java b/src/main/java/me/desair/tus/server/upload/disk/DiskStorageService.java index fcad464a..709f8eca 100644 --- a/src/main/java/me/desair/tus/server/upload/disk/DiskStorageService.java +++ b/src/main/java/me/desair/tus/server/upload/disk/DiskStorageService.java @@ -30,6 +30,7 @@ import me.desair.tus.server.upload.UploadType; import me.desair.tus.server.upload.concatenation.UploadConcatenationService; import me.desair.tus.server.upload.concatenation.VirtualConcatenationService; +import me.desair.tus.server.util.UploadInfoJsonSerializer; import me.desair.tus.server.util.Utils; import org.apache.commons.codec.binary.Base64; import org.apache.commons.io.FileUtils; @@ -198,19 +199,27 @@ private void saveUploadInfo(UploadInfo info, Path path) throws IOException { // exclusive single-writer access across cluster replicas. Fine-grained OS FileLock on .info // files is redundant and fails on NFS/SMB network mounts and unprivileged containers. if (isJsonSerializationEnabled()) { - Utils.writeJson(info, path, false); + UploadInfoJsonSerializer.serializeToPath(info, path); } else { - Utils.writeSerializable(info, path, false); + Utils.writeSerializable(info, path); } } private UploadInfo loadUploadInfo(Path path) throws IOException { - // Coarse request locks guarantee safe reads without needing fine-grained OS FileLock. if (isJsonSerializationEnabled()) { - UploadInfo info = Utils.readJson(path, UploadInfo.class, false); - return info != null ? info : Utils.readSerializable(path, UploadInfo.class, false); + UploadInfo info = null; + try { + info = UploadInfoJsonSerializer.deserialize(path); + } catch (Exception e) { + log.debug( + "Unable to deserialize upload info as JSON from {}, falling back to Java deserialization: {}", + path, + e.getMessage()); + info = null; + } + return info != null ? info : Utils.readSerializable(path, UploadInfo.class); } else { - return Utils.readSerializable(path, UploadInfo.class, false); + return Utils.readSerializable(path, UploadInfo.class); } } @@ -253,7 +262,7 @@ public void update(UploadInfo uploadInfo) throws IOException, UploadNotFoundExce // Delete the child's own data file if it exists try { Path childDataPath = getPathInUploadDir(uploadInfo.getId(), DATA_FILE); - Files.deleteIfExists(childDataPath); + Utils.deletePathQuietly(childDataPath); } catch (UploadNotFoundException e) { // It doesn't exist yet, which is fine } diff --git a/src/main/java/me/desair/tus/server/upload/disk/FileBasedLock.java b/src/main/java/me/desair/tus/server/upload/disk/FileBasedLock.java index 208af237..cf20b054 100644 --- a/src/main/java/me/desair/tus/server/upload/disk/FileBasedLock.java +++ b/src/main/java/me/desair/tus/server/upload/disk/FileBasedLock.java @@ -7,7 +7,6 @@ import java.nio.channels.FileChannel; import java.nio.channels.FileLock; import java.nio.channels.OverlappingFileLockException; -import java.nio.file.Files; import java.nio.file.Path; import java.util.Objects; import me.desair.tus.server.exception.UploadAlreadyLockedException; @@ -86,7 +85,7 @@ public void release() { try { // Closing the channel will also release the lock fileChannel.close(); - Files.deleteIfExists(lockPath); + Utils.deletePathQuietly(lockPath); } catch (IOException e) { log.warn("Unable to release file lock for URI " + getUploadUri(), e); } diff --git a/src/main/java/me/desair/tus/server/upload/disk/LeaseFileLockingService.java b/src/main/java/me/desair/tus/server/upload/disk/LeaseFileLockingService.java index c1af45ef..917fa534 100644 --- a/src/main/java/me/desair/tus/server/upload/disk/LeaseFileLockingService.java +++ b/src/main/java/me/desair/tus/server/upload/disk/LeaseFileLockingService.java @@ -3,19 +3,18 @@ import java.io.IOException; import java.io.InputStream; import java.nio.file.DirectoryStream; -import java.nio.file.FileSystemException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.nio.file.StandardCopyOption; import java.nio.file.attribute.FileTime; -import java.util.UUID; import me.desair.tus.server.upload.AbstractLeaseLockingService; +import me.desair.tus.server.upload.LeaseData; import me.desair.tus.server.upload.UploadId; import me.desair.tus.server.upload.UploadIdFactory; import me.desair.tus.server.upload.UploadLock; import me.desair.tus.server.upload.UploadLockingService; import me.desair.tus.server.upload.UuidUploadIdFactory; +import me.desair.tus.server.util.LeaseDataJsonSerializer; import me.desair.tus.server.util.Utils; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.Validate; @@ -24,41 +23,33 @@ /** * Distributed, NFS- and SMB-safe implementation of {@link UploadLockingService} using atomic - * directory staging, atomic renames, and TTL-based JSON lease files. + * sibling mutex directories ({@link LeaseFileMutex}) and TTL-based JSON lease files. * *

Key Architectural Features & Distributed Concurrency Guide: * *

    - *
  • Atomic Directory Staging & Renames: Lock acquisition stages new locks in a temporary - * sibling directory ({@code /locks/.lock.stage.}) with {@code - * lease.json} pre-populated, then atomically moves it into place using {@link - * StandardCopyOption#ATOMIC_MOVE}. Directory moves map to atomic server-side RPCs on POSIX - * NFS ({@code rename(2)}) and Windows SMB ({@code SetFileInformationByHandle}), ensuring - * {@code .lock} is born on disk 100% valid and eliminating empty directory race - * windows. - *
  • Deterministic TTL Leases & Crash Recovery: Each lock directory contains a small JSON - * lease file ({@code lease.json}) with an absolute expiration timestamp. If a node crashes - * ungracefully ({@code kill -9}, OOM), the lease auto-expires within the TTL (default 30s), - * allowing peer cluster nodes to evict and re-acquire the lock without admin intervention. - *
  • Heartbeat Lease Auto-Renewal: Active locks run a background daemon thread that - * periodically updates {@code expiresAt} in {@code lease.json} every {@code leaseDuration / - * 3} (e.g. every 10s for a 30s lease), preventing lock expiration during long streaming - * uploads. - *
  • TOCTOU Mitigation with Post-Move Verification & Rollback: When multiple nodes race - * to evict an expired lock simultaneously, eviction isolates the directory via atomic move to - * a unique {@code .evicting.} directory and re-inspects the lease post-move. If an - * active lease is detected (created by a concurrent winning peer right before the move), the - * move is automatically rolled back and eviction is aborted, ensuring single-winner - * exclusivity. - *
  • 5-Second Directory Grace Period: Fallback protection for un-staged or corrupted - * directories: if a contender encounters a directory where {@code lease.json} is missing or - * corrupted and the directory is newer than 5 seconds, it is treated as actively acquiring; - * if older than 5 seconds, it is treated as abandoned and evicted. + *
  • Atomic Sibling Mutex Directory ({@code .mutex/}): All state-modifying + * operations (lock acquisition, expired lease takeover, release, and cleanup) acquire a + * {@link LeaseFileMutex} before interacting with lock content. Atomic {@code mkdir} is + * guaranteed across POSIX filesystems, Windows SMB, and NFS (v3/v4 / AWS EFS). + *
  • In-Place Expired Lock Takeover (Zero Directory Moves): When an expired lease is + * encountered, the winning contender acquires the {@link LeaseFileMutex}, updates {@code + * lease.json} directly inside {@code .lock/}, and releases the mutex. The canonical + * lock directory never moves or disappears from disk, eliminating Time-of-Check to + * Time-of-Use (TOCTOU) race windows. + *
  • 5-Second Crash Recovery: If an ungracefully crashed node leaves {@code + * .mutex/} behind, subsequent contenders detect {@code now - mtime >= 5000ms}, + * remove the abandoned mutex directory, and safely retry acquisition. + *
  • Deterministic TTL Leases & Heartbeat Renewal: Lock directories contain {@code + * lease.json} with an absolute expiration timestamp. Active locks run a background daemon + * thread renewing {@code expiresAt} every {@code leaseDuration / 3} (default 10s for 30s + * lease). + *
  • Ownership Verification & Fencing: When releasing or renewing a lock, the service + * verifies that {@code holderId} in {@code lease.json} still matches the current holder, + * preventing an expired/unpaused node from corrupting a successor's active lease. *
  • Cross-Replica Lock Contention & .stop Signals: When a concurrent request arrives for - * a locked upload (e.g. HEAD or DELETE while a PATCH is streaming), the service writes a - * {@code /locks/.stop} signal file. A background watchdog thread polls - * every 1.5 seconds and interrupts the active input stream immediately, allowing the resuming - * request to proceed without false lock conflicts. + * a locked upload (e.g. HEAD or DELETE while PATCH is streaming), the service writes {@code + * .stop}. A background watchdog polls every 1.5 seconds and interrupts the stream. *
*/ public class LeaseFileLockingService extends AbstractLeaseLockingService { @@ -146,11 +137,20 @@ public void cleanupStaleLocks() throws IOException { if (isLockDirectoryExpired(path, now)) { atomicEvictExpiredLock(path); } + } else if (fileName.endsWith(".mutex") && Files.isDirectory(path)) { + try { + FileTime mtime = Files.getLastModifiedTime(path); + if (now - mtime.toMillis() > 10_000L) { + FileUtils.deleteDirectory(path.toFile()); + } + } catch (IOException ignored) { + // Ignore transient cleanup error + } } else if (fileName.endsWith(".stop") && Files.isRegularFile(path)) { try { FileTime mtime = Files.getLastModifiedTime(path); if (now - mtime.toMillis() > 10_000L) { - Files.deleteIfExists(path); + Utils.deletePathQuietly(path); } } catch (IOException ignored) { // Ignore transient cleanup error @@ -160,57 +160,101 @@ public void cleanupStaleLocks() throws IOException { } } + /** + * Attempts primary lock acquisition or in-place takeover of an expired lease under the protection + * of a {@link LeaseFileMutex}. + * + *

Detailed Concurrency Workflow: + * + *

    + *
  1. Acquire Sibling Mutex Directory ({@code .mutex/}): Acquires {@link + * LeaseFileMutex}. Because directory creation maps to atomic {@code mkdir} at the + * OS/filesystem layer, exactly one thread or process across the cluster succeeds. If the + * mutex already exists and is < 5s old, contention is in progress and we return {@code + * null}. If >= 5s old, the previous holder crashed, so {@link LeaseFileMutex} removes + * the stale mutex and retries. + *
  2. Inspect Existing Lease Under Mutex: With exclusive access guaranteed, we read + * {@code .lock/lease.json}. If it exists and {@code !isExpired(now)}, another + * node holds an active unexpired lock; we abort and return {@code null}. + *
  3. In-Place Acquisition / Takeover: If the lock directory does not exist, we create + * it. If {@code lease.json} does not exist or is expired, we update {@code leaseData} and + * write the new {@code lease.json} atomically via a temporary file rename. The lock + * directory never disappears from disk, eliminating TOCTOU race windows. + *
  4. Release Sibling Mutex: When exiting the try-with-resources block, {@code + * .mutex/} is automatically deleted, allowing future contenders to inspect or + * acquire. + *
+ * + * @param uploadId The upload identifier + * @param leaseData The lease metadata describing the lock to acquire + * @return Acquired {@link UploadLock} handle, or null if locked by another active process + * @throws IOException If an I/O error occurs + */ @Override - protected UploadLock tryAcquireLock(UploadId uploadId, String holderId, String requestUri) - throws IOException { + protected UploadLock tryAcquireLock(UploadId uploadId, LeaseData leaseData) throws IOException { + if (uploadId == null || leaseData == null) { + return null; + } + Path lockDirPath = getLockDirPath(uploadId); Path stopFilePath = getStopFilePath(uploadId); - if (Files.exists(lockDirPath)) { + if (lockDirPath == null) { return null; } - Path stageDir = - lockDirPath.resolveSibling(lockDirPath.getFileName() + ".stage." + UUID.randomUUID()); - try { - Utils.ensureDirectoryExists(lockDirPath.getParent()); - Files.createDirectory(stageDir); + // Step 1: Acquire sibling mutex (.mutex) directly in try-with-resources + try (LeaseFileMutex mutex = new LeaseFileMutex(lockDirPath)) { + if (!mutex.isAcquired()) { + return null; + } - LeaseFileUploadLock lock = - new LeaseFileUploadLock( - lockDirPath, stopFilePath, holderId, leaseDurationMs, requestUri, activeInputStreams); + long now = System.currentTimeMillis(); + Path leaseFile = lockDirPath.resolve("lease.json"); + + // Step 2: Under exclusive mutex, inspect existing lease + if (Files.exists(lockDirPath)) { + if (Files.exists(leaseFile)) { + try { + LeaseData existingLease = LeaseDataJsonSerializer.deserialize(leaseFile); + if (existingLease != null && !existingLease.isExpired(now)) { + // Active unexpired lease held by another live node + return null; + } + } catch (Exception e) { + // Corrupted lease: check directory grace period + FileTime mtime = Files.getLastModifiedTime(lockDirPath); + if (now - mtime.toMillis() < EMPTY_DIR_GRACE_PERIOD_MS) { + return null; + } + } + } else { + // Empty directory: check directory grace period + FileTime mtime = Files.getLastModifiedTime(lockDirPath); + if (now - mtime.toMillis() < EMPTY_DIR_GRACE_PERIOD_MS) { + return null; + } + } + } else { + Utils.ensureDirectoryExists(lockDirPath); + } - // Write lease metadata JSON file inside the staged directory - Path leaseFile = stageDir.resolve("lease.json"); - Utils.writeJson(lock, leaseFile, false); + // Step 3: Fresh acquisition or in-place takeover of expired/abandoned lease + leaseData.setLockPath(lockDirPath.toString()); + leaseData.setStopPath(stopFilePath != null ? stopFilePath.toString() : null); - // Atomically move the staged directory to the target lock directory. - // This guarantees that the lock directory appears atomically with a fully valid lease.json - // inside it, completely eliminating any empty-directory or partial-write windows. - Files.move(stageDir, lockDirPath, StandardCopyOption.ATOMIC_MOVE); + // Write lease metadata atomically via serializeToPath (which renames via temporary file) + LeaseDataJsonSerializer.serializeToPath(leaseData, leaseFile); // Clear any lingering stop signal file from prior contention - try { - Files.deleteIfExists(stopFilePath); - } catch (IOException ignored) { - // Safe to ignore + if (stopFilePath != null) { + Utils.deletePathQuietly(stopFilePath); } - return lock; - } catch (FileSystemException e) { - // Lock directory already exists or cannot be moved atomically over existing dir - return null; + return new LeaseFileUploadLock(leaseData, lockDirPath, stopFilePath, activeInputStreams); } catch (Exception e) { - log.debug("Error initializing lease for lock directory {}", lockDirPath, e); + log.debug("Error acquiring lease for upload {}", uploadId, e); return null; - } finally { - if (Files.exists(stageDir)) { - try { - FileUtils.deleteDirectory(stageDir.toFile()); - } catch (IOException ignored) { - // Safe to ignore - } - } } } @@ -257,11 +301,7 @@ protected void checkStopSignalForEntry(String uri, InputStream inputStream) { log.info("Watchdog detected stop file for upload ID {}. Interrupting stream.", uploadId); Utils.interruptStream(inputStream); activeInputStreams.remove(uri); - try { - Files.deleteIfExists(stopFilePath); - } catch (IOException ignored) { - // Safe to ignore - } + Utils.deletePathQuietly(stopFilePath); } } @@ -284,9 +324,9 @@ boolean isLockDirectoryExpired(Path lockDirPath, long now) { Path leaseFile = lockDirPath.resolve("lease.json"); if (Files.exists(leaseFile)) { try { - LeaseFileUploadLock lease = Utils.readJson(leaseFile, LeaseFileUploadLock.class, false); + LeaseData lease = LeaseDataJsonSerializer.deserialize(leaseFile); if (lease != null) { - return lease.getExpiresAt() < now; + return lease.isExpired(now); } } catch (Exception e) { log.debug("Failed to read lease file {}, checking grace period", leaseFile, e); @@ -306,58 +346,29 @@ boolean isLockDirectoryExpired(Path lockDirPath, long now) { } /** - * Atomically evicts an expired or abandoned lock directory by renaming it to a unique temporary - * directory before deletion, preventing contention races where multiple nodes try to evict the - * same directory simultaneously. + * Safely evicts an expired lock directory under sibling mutex protection. * * @param lockDirPath Path to the lock directory to evict - * @return {@code true} if the expired directory was successfully evicted; {@code false} if - * another contender already evicted it or if the moved directory was an active lock + * @return {@code true} if the expired directory was successfully evicted; {@code false} if the + * lock is actively held or mutex could not be acquired */ boolean atomicEvictExpiredLock(Path lockDirPath) { - if (lockDirPath == null) { - return false; - } - long now = System.currentTimeMillis(); - // 1. Fast pre-check: avoid moving if we can already observe it is active - if (!isLockDirectoryExpired(lockDirPath, now)) { - return false; - } - - Path evictPath = - lockDirPath.resolveSibling(lockDirPath.getFileName() + ".evicting." + UUID.randomUUID()); - try { - // Atomic directory rename guarantees exactly one contender wins the right to isolate and - // evict - Files.move(lockDirPath, evictPath, StandardCopyOption.ATOMIC_MOVE); - } catch (Exception e) { - // Another contender already moved or removed the directory; safe to proceed + if (lockDirPath == null || !Files.exists(lockDirPath)) { return false; } - - // 2. Post-move verification (TOCTOU mitigation): ensure the isolated directory was genuinely - // expired and not a fresh active lock created by another winning node right before our move - now = System.currentTimeMillis(); - if (!isLockDirectoryExpired(evictPath, now)) { - // We moved a fresh active lock: restore it immediately to preserve the active lock holder - try { - Files.move(evictPath, lockDirPath, StandardCopyOption.ATOMIC_MOVE); - } catch (Exception ignored) { + try (LeaseFileMutex mutex = new LeaseFileMutex(lockDirPath)) { + if (!mutex.isAcquired()) { + return false; } - return false; - } - - // Delete files inside evicted directory, then delete directory itself - try { - try (DirectoryStream stream = Files.newDirectoryStream(evictPath)) { - for (Path file : stream) { - Files.deleteIfExists(file); - } + long now = System.currentTimeMillis(); + if (!isLockDirectoryExpired(lockDirPath, now)) { + return false; } - Files.deleteIfExists(evictPath); - } catch (IOException ignored) { + FileUtils.deleteDirectory(lockDirPath.toFile()); + return true; + } catch (IOException e) { + return false; } - return true; } Path getLockDirPath(UploadId id) { diff --git a/src/main/java/me/desair/tus/server/upload/disk/LeaseFileMutex.java b/src/main/java/me/desair/tus/server/upload/disk/LeaseFileMutex.java new file mode 100644 index 00000000..1d7909e4 --- /dev/null +++ b/src/main/java/me/desair/tus/server/upload/disk/LeaseFileMutex.java @@ -0,0 +1,157 @@ +package me.desair.tus.server.upload.disk; + +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.FileTime; +import me.desair.tus.server.upload.UploadId; +import me.desair.tus.server.util.Utils; +import org.apache.commons.io.FileUtils; + +/** + * An atomic filesystem mutex directory ({@code .mutex/}) providing mutual exclusion, + * crash recovery, and {@link AutoCloseable} resource management across concurrent processes and + * cluster replicas. + * + *

Atomic directory creation ({@link Files#createDirectory(Path)}) maps to atomic {@code mkdir} + * across POSIX filesystems, Windows, and shared network storage (NFS v3/v4, AWS EFS, SMB/CIFS). + * + *

Constructors attempt acquisition immediately. Callers verify {@link #isAcquired()} inside a + * {@code try-with-resources} block. {@link #close()} only releases the directory if acquisition + * succeeded. + */ +public class LeaseFileMutex implements AutoCloseable { + + public static final long MUTEX_GRACE_PERIOD_MS = 5_000L; // 5 seconds timeout for stale mutexes + + private final Path mutexDir; + private boolean acquired; + + /** + * Constructs a LeaseFileMutex for a target lock directory path and attempts acquisition + * immediately. + * + * @param lockDirPath Path to the {@code .lock} directory + */ + public LeaseFileMutex(Path lockDirPath) { + this.mutexDir = resolveMutexDir(lockDirPath); + this.acquired = tryAcquire(); + } + + /** + * Constructs a LeaseFileMutex for a given base storage path and upload ID and attempts + * acquisition immediately. + * + * @param storagePath Path to the base locks storage directory + * @param uploadId The upload identifier + */ + public LeaseFileMutex(Path storagePath, UploadId uploadId) { + this.mutexDir = + (storagePath != null && uploadId != null) + ? storagePath.resolve(uploadId.toString() + ".mutex") + : null; + this.acquired = tryAcquire(); + } + + /** + * Constructs a LeaseFileMutex with an explicit mutex directory path and attempts acquisition + * immediately. + * + * @param mutexDir Path to the mutex directory + * @param isExplicitPath Flag indicating explicit path usage + */ + public LeaseFileMutex(Path mutexDir, boolean isExplicitPath) { + this.mutexDir = mutexDir; + this.acquired = tryAcquire(); + } + + /** + * Indicates whether the mutex directory was successfully acquired. + * + * @return {@code true} if acquired; {@code false} otherwise + */ + public boolean isAcquired() { + return acquired; + } + + /** + * Returns the underlying filesystem path to the mutex directory. + * + * @return Path to the mutex directory + */ + public Path getPath() { + return mutexDir; + } + + /** + * Attempts to acquire the mutex directory via atomic {@link Files#createDirectory(Path)}. + * + *

If a collision occurs ({@link FileAlreadyExistsException}) and the existing directory's + * modification time is older than {@link #MUTEX_GRACE_PERIOD_MS} (5s), it is treated as abandoned + * by a crashed node, cleaned up, and retried once. + * + * @return {@code true} if the mutex directory was successfully created; {@code false} if held by + * a live contender or I/O error occurred + */ + private boolean tryAcquire() { + if (mutexDir == null) { + return false; + } + try { + Utils.ensureDirectoryExists(mutexDir.getParent()); + Files.createDirectory(mutexDir); + return true; + } catch (FileAlreadyExistsException e) { + try { + FileTime mtime = Files.getLastModifiedTime(mutexDir); + long ageMs = System.currentTimeMillis() - mtime.toMillis(); + if (ageMs >= MUTEX_GRACE_PERIOD_MS) { + // Mutex holder crashed; clean up stale mutex directory and retry once + FileUtils.deleteDirectory(mutexDir.toFile()); + Files.createDirectory(mutexDir); + return true; + } + } catch (Exception ignored) { + // Another thread or process resolved it + } + return false; + } catch (Exception e) { + return false; + } + } + + /** Releases the mutex by deleting the directory from disk if it was acquired by this instance. */ + public void release() { + if (acquired && mutexDir != null) { + try { + Utils.deletePathQuietly(mutexDir); + } finally { + acquired = false; + } + } + } + + /** + * Releases the mutex when exiting a try-with-resources block if it was acquired by this instance. + */ + @Override + public void close() { + release(); + } + + /** + * Resolves the sibling mutex directory path for a given lock directory path. + * + * @param lockDirPath Path to the lock directory + * @return Path to the sibling mutex directory, or null if lockDirPath is null + */ + public static Path resolveMutexDir(Path lockDirPath) { + if (lockDirPath == null) { + return null; + } + String fileName = lockDirPath.getFileName().toString(); + String idStr = + fileName.endsWith(".lock") ? fileName.substring(0, fileName.length() - 5) : fileName; + return lockDirPath.resolveSibling(idStr + ".mutex"); + } +} diff --git a/src/main/java/me/desair/tus/server/upload/disk/LeaseFileUploadLock.java b/src/main/java/me/desair/tus/server/upload/disk/LeaseFileUploadLock.java index eb557f1f..04cc3fd0 100644 --- a/src/main/java/me/desair/tus/server/upload/disk/LeaseFileUploadLock.java +++ b/src/main/java/me/desair/tus/server/upload/disk/LeaseFileUploadLock.java @@ -1,130 +1,150 @@ package me.desair.tus.server.upload.disk; -import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonInclude; -import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardCopyOption; import java.util.Map; -import java.util.UUID; +import java.util.concurrent.ScheduledExecutorService; import me.desair.tus.server.upload.AbstractLeaseLock; -import me.desair.tus.server.upload.UploadLock; +import me.desair.tus.server.upload.LeaseData; +import me.desair.tus.server.util.LeaseDataJsonSerializer; import me.desair.tus.server.util.Utils; +import org.apache.commons.lang3.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * A distributed lease-based implementation of {@link UploadLock} that holds an exclusive lock lease - * on an upload resource via an atomic directory and a JSON lease file. Serves both as the JSON - * payload representation stored in {@code lease.json} on disk/NFS and the active lock handle with - * heartbeat lease auto-renewal. + * Distributed upload lock implementation backed by atomic directory leases on the local or shared + * filesystem. * - *

Why Lease Renewal is Required (NFS & Shared Drives vs Local FileLock): - * - *

    - *
  • Local OS FileLock: Relies on OS kernel file descriptor tracking (POSIX {@code fcntl} - * / Windows byte-range lock). When a process crashes, the OS cleans up file descriptors. - * However, network filesystems (NFSv3/v4, AWS EFS, SMB/CIFS) frequently drop or stall lock - * state, fail in unprivileged container network namespaces without {@code statd}, or hang - * when mounted with {@code nolock}. - *
  • Lease File Locking: Replaces OS locks with an atomic directory and a short - * Time-To-Live (TTL) lease JSON file. If a node crashes unexpectedly (e.g. OOM killer, {@code - * kill -9}, network partition), the lease auto-expires within the TTL (default 30s), allowing - * peer cluster replicas to evict and re-acquire it cleanly. - *
  • Heartbeat Renewal: Because TUS uploads can stream for minutes or hours, a background - * daemon thread periodically updates {@code expiresAt} in {@code lease.json} every {@code - * leaseDuration / 3} milliseconds. - *
+ *

Wraps an active lease and maintains a background daemon executor that periodically renews the + * lease metadata on disk to prevent lock expiry during long-running streaming uploads. */ -@JsonIgnoreProperties(ignoreUnknown = true) -@JsonInclude(JsonInclude.Include.NON_NULL) public class LeaseFileUploadLock extends AbstractLeaseLock { private static final Logger log = LoggerFactory.getLogger(LeaseFileUploadLock.class); - private String storagePath; - - @JsonIgnore private Path lockDirPath; - @JsonIgnore private Path stopFilePath; - - /** Default constructor for Jackson JSON deserialization. */ - public LeaseFileUploadLock() { - super(); - } + private final Path lockDirPath; + private final Path stopFilePath; + private final LeaseData leaseData; /** - * Constructs an active LeaseFileUploadLock and starts the background heartbeat lease renewal - * daemon. + * Constructs an active {@link LeaseFileUploadLock} and starts the background heartbeat lease + * renewal daemon. * + * @param leaseData The lease metadata * @param lockDirPath Dedicated lock directory path * @param stopFilePath Lock contention stop signal file path - * @param holderId Unique identifier of the lock holder - * @param leaseDurationMs Lease duration in milliseconds - * @param requestUri Target upload URI * @param activeInputStreams Map of active request input streams in the JVM */ public LeaseFileUploadLock( + LeaseData leaseData, Path lockDirPath, Path stopFilePath, - String holderId, - long leaseDurationMs, - String requestUri, Map activeInputStreams) { - super(holderId, leaseDurationMs, requestUri, activeInputStreams, "lease-file-lock-heartbeat"); + this(leaseData, lockDirPath, stopFilePath, activeInputStreams, null); + } + + /** + * Testing constructor allowing injection of a custom heartbeat executor. + * + * @param leaseData The lease metadata + * @param lockDirPath Dedicated lock directory path + * @param stopFilePath Lock contention stop signal file path + * @param activeInputStreams Map of active request input streams in the JVM + * @param heartbeatExecutor Custom executor service for heartbeats + */ + LeaseFileUploadLock( + LeaseData leaseData, + Path lockDirPath, + Path stopFilePath, + Map activeInputStreams, + ScheduledExecutorService heartbeatExecutor) { + super(leaseData, activeInputStreams, heartbeatExecutor, "lease-file-lock-heartbeat"); + this.leaseData = leaseData; this.lockDirPath = lockDirPath; this.stopFilePath = stopFilePath; - this.storagePath = lockDirPath != null ? lockDirPath.toString() : null; } - public String getStoragePath() { - return storagePath; + public LeaseData getLeaseData() { + return leaseData; } - public void setStoragePath(String storagePath) { - this.storagePath = storagePath; + public String getStoragePath() { + return lockDirPath != null ? lockDirPath.toString() : null; } + /** + * Renews the active lease by advancing {@code expiresAt} and persisting the updated metadata to + * disk under the protection of the {@link LeaseFileMutex}. Includes ownership verification to + * ensure a paused or ungracefully expired holder does not overwrite a successor's active lease. + */ @Override protected void doRenewLease() { if (lockDirPath == null || !Files.exists(lockDirPath)) { return; } - try { - Path leaseTmpFile = lockDirPath.resolve("lease.json.tmp." + UUID.randomUUID()); + try (LeaseFileMutex mutex = new LeaseFileMutex(lockDirPath)) { + if (!mutex.isAcquired()) { + return; + } + if (!doesLockOwnershipMatch()) { + log.info("Lease for {} was taken over by another holder. Aborting renewal.", lockDirPath); + return; + } + Path leaseFile = lockDirPath.resolve("lease.json"); - Utils.writeJson(this, leaseTmpFile, false); - Files.move( - leaseTmpFile, - leaseFile, - StandardCopyOption.ATOMIC_MOVE, - StandardCopyOption.REPLACE_EXISTING); + leaseData.setExpiresAt(getExpiresAt()); + LeaseDataJsonSerializer.serializeToPath(leaseData, leaseFile); } catch (Exception e) { log.warn("Failed to renew lease for lock directory {}", lockDirPath, e); } } + /** + * Releases the lock resource upon request completion. Synchronized via the {@link LeaseFileMutex} + * and verifies {@code holderId} ownership so that an expired or paused holder does not delete a + * successor's lock directory. + */ @Override protected void releaseLockResource() { - // 1. Delete lease file and lock directory - if (lockDirPath != null) { - try { - Files.deleteIfExists(lockDirPath.resolve("lease.json")); - Files.deleteIfExists(lockDirPath); - } catch (IOException e) { - log.warn("Failed to remove lock directory {}", lockDirPath, e); + if (lockDirPath != null && Files.exists(lockDirPath)) { + try (LeaseFileMutex mutex = new LeaseFileMutex(lockDirPath)) { + if (mutex.isAcquired() && doesLockOwnershipMatch()) { + Utils.deletePathQuietly(lockDirPath.resolve("lease.json")); + Utils.deletePathQuietly(lockDirPath); + } } } // 2. Delete .stop signal file if present if (stopFilePath != null) { + Utils.deletePathQuietly(stopFilePath); + } + } + + /** + * Verifies that the on-disk {@code lease.json} file is still owned by this lock holder. + * + * @return {@code true} if the lease file does not exist, is unparseable, or matches this holder's + * ID; {@code false} if owned by a different holder + */ + boolean doesLockOwnershipMatch() { + if (lockDirPath == null) { + return false; + } + Path leaseFile = lockDirPath.resolve("lease.json"); + if (Files.exists(leaseFile)) { try { - Files.deleteIfExists(stopFilePath); - } catch (IOException ignored) { - // Safe to ignore stop file deletion failure + LeaseData existingLease = LeaseDataJsonSerializer.deserialize(leaseFile); + if (existingLease != null + && !Strings.CS.equals(existingLease.getHolderId(), getHolderId())) { + return false; + } + } catch (Exception ignored) { + // If unparseable or corrupt, consider owned/proceed } } + return true; } } diff --git a/src/main/java/me/desair/tus/server/upload/s3/S3LockingService.java b/src/main/java/me/desair/tus/server/upload/s3/S3LockingService.java index 89ab762b..61ef467a 100644 --- a/src/main/java/me/desair/tus/server/upload/s3/S3LockingService.java +++ b/src/main/java/me/desair/tus/server/upload/s3/S3LockingService.java @@ -15,12 +15,13 @@ import java.util.Collections; import java.util.Objects; import me.desair.tus.server.upload.AbstractLeaseLockingService; +import me.desair.tus.server.upload.LeaseData; import me.desair.tus.server.upload.UploadId; import me.desair.tus.server.upload.UploadIdFactory; import me.desair.tus.server.upload.UploadLock; import me.desair.tus.server.upload.UploadLockingService; import me.desair.tus.server.upload.UuidUploadIdFactory; -import me.desair.tus.server.util.S3UploadLockJsonSerializer; +import me.desair.tus.server.util.LeaseDataJsonSerializer; import me.desair.tus.server.util.Utils; import org.apache.commons.lang3.Strings; import org.slf4j.Logger; @@ -145,20 +146,22 @@ public void cleanupStaleLocks() throws IOException { } @Override - protected UploadLock tryAcquireLock(UploadId uploadId, String holderId, String requestUri) { + protected UploadLock tryAcquireLock(UploadId uploadId, LeaseData leaseData) { + if (leaseData == null) { + return null; + } String lockKey = buildLockKey(uploadId); String stopKey = buildStopKey(uploadId); if (!isLockExpired(lockKey)) { - return false ? null : null; + return null; } - long expiresAt = System.currentTimeMillis() + leaseDurationMs; try { - S3UploadLock lock = - new S3UploadLock( - holderId, requestUri, bucket, lockKey, stopKey, leaseDurationMs, expiresAt); - byte[] lockContentBytes = S3UploadLockJsonSerializer.serializeToBytes(lock); + leaseData.setLockPath(lockKey); + leaseData.setStopPath(stopKey); + + byte[] lockContentBytes = LeaseDataJsonSerializer.serializeToBytes(leaseData); // Layer 1: Conditional PutObject with "If-None-Match: *" // AWS S3 and compliant servers reject this with 412 Precondition Failed if the object already @@ -176,19 +179,11 @@ protected UploadLock tryAcquireLock(UploadId uploadId, String holderId, String r // For emulators or S3 backends where If-None-Match is not strictly enforced, // pause for a small randomized jitter (20-60ms) and verify our holderId is still the owner applyJitter(); - if (!verifyLockOwnership(lockKey, holderId)) { + if (!verifyLockOwnership(lockKey, leaseData.getHolderId())) { return null; } - return new S3UploadLock( - minioClient, - bucket, - lockKey, - stopKey, - holderId, - leaseDurationMs, - requestUri, - activeInputStreams); + return new S3UploadLock(leaseData, minioClient, bucket, lockKey, stopKey, activeInputStreams); } catch (ErrorResponseException e) { S3ErrorType errorType = S3Utils.parseErrorResponse(e); if (errorType == S3ErrorType.PRECONDITION_FAILED || errorType == S3ErrorType.CONFLICT) { @@ -208,8 +203,7 @@ protected boolean isLockExpired(UploadId uploadId) { if (uploadId == null) { return true; } - String lockKey = buildLockKey(uploadId); - return isLockExpired(lockKey); + return isLockExpired(buildLockKey(uploadId)); } @Override @@ -218,22 +212,19 @@ protected boolean evictExpiredLock(UploadId uploadId) { return false; } String lockKey = buildLockKey(uploadId); - deleteExpiredLockQuietly(lockKey); - return true; + return isLockExpired(lockKey); } @Override protected void writeStopSignal(UploadId uploadId) { String stopKey = buildStopKey(uploadId); try { - byte[] empty = new byte[0]; - // Write empty .stop signal object to S3 minioClient.putObject( PutObjectArgs.builder().bucket(bucket).object(stopKey).stream( - new ByteArrayInputStream(empty), 0L, -1L) + new ByteArrayInputStream(new byte[0]), 0L, -1L) .build()); } catch (Exception e) { - log.debug("Failed to write lock stop signal to S3 key {}", stopKey, e); + log.warn("Failed to write lock stop signal object {} in bucket {}", stopKey, bucket, e); } } @@ -263,8 +254,11 @@ protected void checkStopSignalForEntry(String uri, InputStream inputStream) { boolean verifyLockOwnership(String lockKey, String expectedHolderId) { try (InputStream stream = minioClient.getObject(GetObjectArgs.builder().bucket(bucket).object(lockKey).build())) { - S3UploadLock remoteLock = S3UploadLockJsonSerializer.deserialize(stream); - return remoteLock != null && Strings.CS.equals(remoteLock.getHolderId(), expectedHolderId); + LeaseData remoteLock = LeaseDataJsonSerializer.deserialize(stream); + return remoteLock != null + && Strings.CS.equals(remoteLock.getHolderId(), expectedHolderId) + && (remoteLock.getLockPath() == null + || Strings.CS.equals(remoteLock.getLockPath(), lockKey)); } catch (Exception e) { log.debug("Failed to verify lock ownership for key {}", lockKey, e); return false; @@ -281,7 +275,7 @@ boolean isLockExpired(String lockKey) { try (InputStream stream = minioClient.getObject(GetObjectArgs.builder().bucket(bucket).object(lockKey).build())) { - S3UploadLock lock = S3UploadLockJsonSerializer.deserialize(stream); + LeaseData lock = LeaseDataJsonSerializer.deserialize(stream); if (lock == null) { return true; } diff --git a/src/main/java/me/desair/tus/server/upload/s3/S3StorageService.java b/src/main/java/me/desair/tus/server/upload/s3/S3StorageService.java index 7eadced5..7a921866 100644 --- a/src/main/java/me/desair/tus/server/upload/s3/S3StorageService.java +++ b/src/main/java/me/desair/tus/server/upload/s3/S3StorageService.java @@ -590,6 +590,11 @@ public boolean isUploadDeduplicationEnabled() { return deduplicationEnabled; } + @Override + public boolean isJsonSerializationEnabled() { + return true; + } + @Override public void setUploadConcatenationService(UploadConcatenationService concatenationService) { this.concatenationService = concatenationService; diff --git a/src/main/java/me/desair/tus/server/upload/s3/S3UploadLock.java b/src/main/java/me/desair/tus/server/upload/s3/S3UploadLock.java index 5ae5abf9..bf6fe497 100644 --- a/src/main/java/me/desair/tus/server/upload/s3/S3UploadLock.java +++ b/src/main/java/me/desair/tus/server/upload/s3/S3UploadLock.java @@ -1,8 +1,5 @@ package me.desair.tus.server.upload.s3; -import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonInclude; import io.minio.GetObjectArgs; import io.minio.MinioClient; import io.minio.PutObjectArgs; @@ -13,124 +10,59 @@ import java.util.Map; import java.util.concurrent.ScheduledExecutorService; import me.desair.tus.server.upload.AbstractLeaseLock; +import me.desair.tus.server.upload.LeaseData; import me.desair.tus.server.upload.UploadLock; -import me.desair.tus.server.util.S3UploadLockJsonSerializer; +import me.desair.tus.server.util.LeaseDataJsonSerializer; import org.apache.commons.lang3.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * A MinIO S3-backed implementation of {@link UploadLock} that holds an exclusive lock lease on an - * upload resource using S3 objects. Serves both as the JSON payload representation stored in S3 and - * the active lock handle with heartbeat lease auto-renewal. + * upload resource using S3 objects. * - *

Why Lease Renewal is Required (S3 vs File-Based Locking): - * - *

    - *
  • File-Based Locks (OS Kernel Managed): {@code FileBasedLock} uses OS file channel - * locks ({@link java.nio.channels.FileLock}). When a JVM process crashes or is killed, the OS - * kernel automatically closes open file descriptors and releases the lock. Thus, no lock - * expiration or renewal is needed. - *
  • S3 Locks (Stateless HTTP Storage): S3 has no persistent process connections or OS - * file descriptors. Locks are stored as S3 objects ({@code .lock}). To prevent crashed pods - * from leaving permanently orphaned lock objects, S3 locks use a short Time-To-Live (TTL) - * lease. - *
  • Heartbeat Renewal Requirement: Because TUS uploads can stream for minutes or hours, - * a background daemon thread periodically renews the lease ({@link #renewLease()}). Removing - * lease renewal would either cause locks to expire mid-upload on long streams (leading to - * race conditions and data corruption) or cause pod crashes to permanently deadlock upload - * IDs. - *
- * - *

Lock Lease Mechanics for Developers: - * - *

    - *
  • Heartbeat Lease Renewal: When initialized, a background daemon thread executes - * {@link #renewLease()} at a periodic interval (one-third of {@code leaseDurationMs}, e.g. - * every 10s for a 30s lease). - *
  • Clean Lock Release: When the HTTP request finishes, {@link #close()} shuts down the - * heartbeat thread and deletes both the {@code .lock} lease object (if still owned) and any - * lingering {@code .stop} signal objects from S3. - *
+ *

Wraps an active lease and maintains a background daemon executor that periodically renews the + * lease metadata in S3 to prevent lock expiry during long-running streaming uploads. */ -@JsonIgnoreProperties(ignoreUnknown = true) -@JsonInclude(JsonInclude.Include.NON_NULL) public class S3UploadLock extends AbstractLeaseLock { private static final Logger log = LoggerFactory.getLogger(S3UploadLock.class); - private String bucket; - private String lockKey; - private String stopKey; - - @JsonIgnore private MinioClient minioClient; - - /** Default constructor for Jackson JSON deserialization. */ - public S3UploadLock() { - super(); - } - - /** Constructor for serializing lock lease metadata to S3. */ - public S3UploadLock( - String holderId, - String requestUri, - String bucket, - String lockKey, - String stopKey, - long leaseDurationMs, - long expiresAt) { - super(holderId, requestUri, leaseDurationMs, expiresAt); - this.bucket = bucket; - this.lockKey = lockKey; - this.stopKey = stopKey; - } + private final String bucket; + private final String lockKey; + private final String stopKey; + private final MinioClient minioClient; /** * Constructs a new S3UploadLock instance using MinIO Java SDK and starts the lease renewal * daemon. * + * @param leaseData The lease metadata * @param minioClient The MinIO client * @param bucket The S3 bucket * @param lockKey The S3 object key for the lock lease * @param stopKey The S3 object key for the interrupt stop signal - * @param holderId Unique ID identifying the lock holder - * @param leaseDurationMs Lease duration in milliseconds - * @param requestUri The request URI linked to this lock * @param inputStreamMap Map of active request input streams */ public S3UploadLock( + LeaseData leaseData, MinioClient minioClient, String bucket, String lockKey, String stopKey, - String holderId, - long leaseDurationMs, - String requestUri, Map inputStreamMap) { - super(holderId, leaseDurationMs, requestUri, inputStreamMap, "s3-lock-heartbeat"); - this.minioClient = minioClient; - this.bucket = bucket; - this.lockKey = lockKey; - this.stopKey = stopKey; + this(leaseData, minioClient, bucket, lockKey, stopKey, inputStreamMap, null); } S3UploadLock( + LeaseData leaseData, MinioClient minioClient, String bucket, String lockKey, String stopKey, - String holderId, - long leaseDurationMs, - String requestUri, Map inputStreamMap, ScheduledExecutorService heartbeatExecutor) { - super( - holderId, - leaseDurationMs, - requestUri, - inputStreamMap, - heartbeatExecutor, - "s3-lock-heartbeat"); + super(leaseData, inputStreamMap, heartbeatExecutor, "s3-lock-heartbeat"); this.minioClient = minioClient; this.bucket = bucket; this.lockKey = lockKey; @@ -141,30 +73,28 @@ public String getBucket() { return bucket; } - public void setBucket(String bucket) { - this.bucket = bucket; - } - public String getLockKey() { return lockKey; } - public void setLockKey(String lockKey) { - this.lockKey = lockKey; - } - public String getStopKey() { return stopKey; } - public void setStopKey(String stopKey) { - this.stopKey = stopKey; - } - @Override protected void doRenewLease() { + if (minioClient == null || lockKey == null) { + return; + } try { - byte[] lockContentBytes = S3UploadLockJsonSerializer.serializeToBytes(this); + if (!doesLockOwnershipMatch(lockKey)) { + log.info( + "Skipping renewal of S3 lock key {}: lock was taken over by another node", lockKey); + return; + } + + getLeaseData().setExpiresAt(getExpiresAt()); + byte[] lockContentBytes = LeaseDataJsonSerializer.serializeToBytes(getLeaseData()); minioClient.putObject( PutObjectArgs.builder().bucket(bucket).object(lockKey).stream( @@ -187,19 +117,9 @@ void deleteS3LockObjectIfOwner(String key) { return; } try { - // Re-verify that the remote lock object is still owned by this lock handle before deleting it - try (InputStream stream = - minioClient.getObject(GetObjectArgs.builder().bucket(bucket).object(key).build())) { - S3UploadLock remoteLock = S3UploadLockJsonSerializer.deserialize(stream); - if (remoteLock != null && !Strings.CS.equals(remoteLock.getHolderId(), getHolderId())) { - log.info( - "Skipping deletion of S3 lock key {}: lock is currently held by another node {}", - key, - remoteLock.getHolderId()); - return; - } - } catch (ErrorResponseException e) { - // Object is already gone or missing + if (!doesLockOwnershipMatch(key)) { + log.info( + "Skipping deletion of S3 lock key {}: lock is currently held by another node", key); return; } minioClient.removeObject(RemoveObjectArgs.builder().bucket(bucket).object(key).build()); @@ -208,6 +128,31 @@ void deleteS3LockObjectIfOwner(String key) { } } + /** + * Verifies that the S3 lock object is still owned by this lock holder. + * + * @param key The S3 lock object key + * @return {@code true} if the lock object is missing or owned by this holder; {@code false} if + * owned by a different holder or arguments are invalid + */ + boolean doesLockOwnershipMatch(String key) { + if (key == null || minioClient == null || bucket == null) { + return false; + } + try (InputStream stream = + minioClient.getObject(GetObjectArgs.builder().bucket(bucket).object(key).build())) { + LeaseData remoteLock = LeaseDataJsonSerializer.deserialize(stream); + if (remoteLock != null && !Strings.CS.equals(remoteLock.getHolderId(), getHolderId())) { + return false; + } + } catch (ErrorResponseException e) { + // Object is already gone or missing, safe to proceed + } catch (Exception e) { + log.debug("Error checking lock ownership for S3 key {}", key, e); + } + return true; + } + private void deleteS3ObjectQuietly(String key) { if (key == null || minioClient == null || bucket == null) { return; diff --git a/src/main/java/me/desair/tus/server/util/LeaseDataJsonSerializer.java b/src/main/java/me/desair/tus/server/util/LeaseDataJsonSerializer.java new file mode 100644 index 00000000..388d995a --- /dev/null +++ b/src/main/java/me/desair/tus/server/util/LeaseDataJsonSerializer.java @@ -0,0 +1,163 @@ +package me.desair.tus.server.util; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import me.desair.tus.server.upload.LeaseData; + +/** + * Utility class responsible for serializing and deserializing {@link LeaseData} objects to and from + * JSON format across disk-based and S3-based distributed locking services. + * + *

Configured with {@code FAIL_ON_UNKNOWN_PROPERTIES = false} to ensure forward and backward + * compatibility when adding or modifying lease properties. + */ +public class LeaseDataJsonSerializer { + + private static final ObjectMapper OBJECT_MAPPER = + new ObjectMapper() + .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) + .setSerializationInclusion(JsonInclude.Include.NON_NULL); + + private LeaseDataJsonSerializer() { + // Utility class + } + + /** + * Serialize the given {@link LeaseData} object to a JSON string. + * + * @param data The lease data to serialize + * @return A JSON string representation of the data, or null if input is null + * @throws IOException If serialization fails + */ + public static String serialize(LeaseData data) throws IOException { + if (data == null) { + return null; + } + return OBJECT_MAPPER.writeValueAsString(data); + } + + /** + * Serialize the given {@link LeaseData} object directly to a UTF-8 byte array. + * + * @param data The lease data to serialize + * @return A UTF-8 byte array containing the JSON data, or null if input is null + * @throws IOException If serialization fails + */ + public static byte[] serializeToBytes(LeaseData data) throws IOException { + if (data == null) { + return null; + } + return OBJECT_MAPPER.writeValueAsBytes(data); + } + + /** + * Serialize the given {@link LeaseData} object directly to an {@link OutputStream}. + * + * @param data The lease data to serialize + * @param outputStream The target output stream + * @throws IOException If serialization fails + */ + public static void serializeToStream(LeaseData data, OutputStream outputStream) + throws IOException { + if (data != null && outputStream != null) { + OBJECT_MAPPER.writeValue(outputStream, data); + } + } + + /** + * Serialize the given {@link LeaseData} object directly to a {@link File}. + * + * @param data The lease data to serialize + * @param file The target destination file + * @throws IOException If serialization fails + */ + public static void serializeToFile(LeaseData data, File file) throws IOException { + if (data != null && file != null) { + OBJECT_MAPPER.writeValue(file, data); + } + } + + /** + * Serialize the given {@link LeaseData} object atomically to a destination {@link Path} via a + * temporary file rename. + * + *

Writing to a temporary file first and renaming via {@link StandardCopyOption#ATOMIC_MOVE} + * prevents concurrent read-only queries (like {@code isLocked()}) from observing an empty + * (0-byte) or partially written JSON file during disk flushes. + * + * @param data The lease data to serialize + * @param path The target destination path + * @throws IOException If serialization or file move fails + */ + public static void serializeToPath(LeaseData data, Path path) throws IOException { + if (data != null && path != null) { + try (Utils.TempPath tempPath = new Utils.TempPath(path)) { + serializeToFile(data, tempPath.getPath().toFile()); + Utils.atomicMove(tempPath.getPath(), path); + } + } + } + + /** + * Deserialize a {@link LeaseData} object from a JSON string. + * + * @param json The JSON string representation of the lease data + * @return The deserialized LeaseData instance, or null if input is blank + * @throws IOException If deserialization fails + */ + public static LeaseData deserialize(String json) throws IOException { + if (json == null || json.trim().isEmpty()) { + return null; + } + return OBJECT_MAPPER.readValue(json, LeaseData.class); + } + + /** + * Deserialize a {@link LeaseData} object from an {@link InputStream}. + * + * @param inputStream The input stream containing the JSON data + * @return The deserialized LeaseData instance, or null if input stream is null + * @throws IOException If deserialization fails + */ + public static LeaseData deserialize(InputStream inputStream) throws IOException { + if (inputStream == null) { + return null; + } + return OBJECT_MAPPER.readValue(inputStream, LeaseData.class); + } + + /** + * Deserialize a {@link LeaseData} object from a {@link File}. + * + * @param file The file containing the JSON data + * @return The deserialized LeaseData instance, or null if file is null or doesn't exist + * @throws IOException If deserialization fails + */ + public static LeaseData deserialize(File file) throws IOException { + if (file == null || !file.exists() || file.length() == 0) { + return null; + } + return OBJECT_MAPPER.readValue(file, LeaseData.class); + } + + /** + * Deserialize a {@link LeaseData} object from a {@link Path}. + * + * @param path The path containing the JSON data + * @return The deserialized LeaseData instance, or null if path is null or doesn't exist + * @throws IOException If deserialization fails + */ + public static LeaseData deserialize(Path path) throws IOException { + if (path == null) { + return null; + } + return deserialize(path.toFile()); + } +} diff --git a/src/main/java/me/desair/tus/server/util/S3UploadLockJsonSerializer.java b/src/main/java/me/desair/tus/server/util/S3UploadLockJsonSerializer.java deleted file mode 100644 index 15e2fa4b..00000000 --- a/src/main/java/me/desair/tus/server/util/S3UploadLockJsonSerializer.java +++ /dev/null @@ -1,124 +0,0 @@ -package me.desair.tus.server.util; - -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.databind.DeserializationFeature; -import com.fasterxml.jackson.databind.ObjectMapper; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import me.desair.tus.server.upload.s3.S3UploadLock; - -/** - * Utility class responsible for serializing and deserializing {@link S3UploadLock} objects to and - * from JSON format using Jackson. - * - *

Configured with {@code FAIL_ON_UNKNOWN_PROPERTIES = false} to ensure forward and backward - * compatibility when adding or modifying lock properties across versions. - */ -public class S3UploadLockJsonSerializer { - - private static final ObjectMapper OBJECT_MAPPER = - new ObjectMapper() - .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) - .setSerializationInclusion(JsonInclude.Include.NON_NULL); - - private S3UploadLockJsonSerializer() { - // Utility class - } - - /** - * Serialize the given object to a JSON string. - * - * @param object The object to serialize - * @return A JSON string representation of the object, or null if object is null - * @throws IOException If serialization fails - */ - public static String serialize(Object object) throws IOException { - if (object == null) { - return null; - } - return OBJECT_MAPPER.writeValueAsString(object); - } - - /** - * Serialize the given object directly to a byte array. - * - * @param object The object to serialize - * @return A UTF-8 byte array containing the JSON data, or null if object is null - * @throws IOException If serialization fails - */ - public static byte[] serializeToBytes(Object object) throws IOException { - if (object == null) { - return null; - } - return OBJECT_MAPPER.writeValueAsBytes(object); - } - - /** - * Serialize the given object directly to an {@link OutputStream}. - * - * @param object The object to serialize - * @param outputStream The target output stream - * @throws IOException If serialization fails - */ - public static void serializeToStream(Object object, OutputStream outputStream) - throws IOException { - if (object != null && outputStream != null) { - OBJECT_MAPPER.writeValue(outputStream, object); - } - } - - /** - * Deserialize an {@link S3UploadLock} object from a JSON string. - * - * @param json The JSON string representation of the lock data - * @return The deserialized S3UploadLock instance, or null if input is blank - * @throws IOException If deserialization fails - */ - public static S3UploadLock deserialize(String json) throws IOException { - return deserialize(json, S3UploadLock.class); - } - - /** - * Deserialize an object of the specified class from a JSON string. - * - * @param The target object type - * @param json The JSON string representation - * @param clazz The target class type - * @return The deserialized instance, or null if input is blank - * @throws IOException If deserialization fails - */ - public static T deserialize(String json, Class clazz) throws IOException { - if (json == null || json.trim().isEmpty() || clazz == null) { - return null; - } - return OBJECT_MAPPER.readValue(json, clazz); - } - - /** - * Deserialize an {@link S3UploadLock} object from an {@link InputStream}. - * - * @param inputStream The input stream containing the JSON data - * @return The deserialized S3UploadLock instance, or null if input stream is null - * @throws IOException If deserialization fails - */ - public static S3UploadLock deserialize(InputStream inputStream) throws IOException { - return deserialize(inputStream, S3UploadLock.class); - } - - /** - * Deserialize an object of the specified class from an {@link InputStream}. - * - * @param The target object type - * @param inputStream The input stream containing the JSON data - * @param clazz The target class type - * @return The deserialized instance, or null if input stream is null - * @throws IOException If deserialization fails - */ - public static T deserialize(InputStream inputStream, Class clazz) throws IOException { - if (inputStream == null || clazz == null) { - return null; - } - return OBJECT_MAPPER.readValue(inputStream, clazz); - } -} diff --git a/src/main/java/me/desair/tus/server/util/UploadInfoJsonSerializer.java b/src/main/java/me/desair/tus/server/util/UploadInfoJsonSerializer.java index 65db5469..f3a02bdc 100644 --- a/src/main/java/me/desair/tus/server/util/UploadInfoJsonSerializer.java +++ b/src/main/java/me/desair/tus/server/util/UploadInfoJsonSerializer.java @@ -10,15 +10,20 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializerProvider; import com.fasterxml.jackson.databind.module.SimpleModule; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.file.Path; import me.desair.tus.server.upload.UploadId; import me.desair.tus.server.upload.UploadInfo; /** * Utility class responsible for serializing and deserializing {@link UploadInfo} and other domain - * objects to and from JSON format using Jackson. + * objects to and from JSON format using Jackson across disk-based, S3, and Azure storage services. + * + *

Configured with {@code FAIL_ON_UNKNOWN_PROPERTIES = false} to ensure forward and backward + * compatibility when adding or modifying domain properties. */ public class UploadInfoJsonSerializer { @@ -66,7 +71,7 @@ private UploadInfoJsonSerializer() { * Serialize the given object to a JSON string. * * @param object The object to serialize - * @return A JSON string representation of the object + * @return A JSON string representation of the object, or null if input is null * @throws IOException If serialization fails */ public static String serialize(Object object) throws IOException { @@ -76,6 +81,20 @@ public static String serialize(Object object) throws IOException { return OBJECT_MAPPER.writeValueAsString(object); } + /** + * Serialize the given object directly to a UTF-8 byte array. + * + * @param object The object to serialize + * @return A UTF-8 byte array containing the JSON data, or null if input is null + * @throws IOException If serialization fails + */ + public static byte[] serializeToBytes(Object object) throws IOException { + if (object == null) { + return null; + } + return OBJECT_MAPPER.writeValueAsBytes(object); + } + /** * Serialize the given object directly to an {@link OutputStream}. * @@ -90,6 +109,36 @@ public static void serializeToStream(Object object, OutputStream outputStream) } } + /** + * Serialize the given object directly to a {@link File}. + * + * @param object The object to serialize + * @param file The target destination file + * @throws IOException If serialization fails + */ + public static void serializeToFile(Object object, File file) throws IOException { + if (object != null && file != null) { + OBJECT_MAPPER.writeValue(file, object); + } + } + + /** + * Serialize the given object atomically to a destination {@link Path} via a temporary file + * rename. + * + * @param object The object to serialize + * @param path The target destination path + * @throws IOException If serialization or file move fails + */ + public static void serializeToPath(Object object, Path path) throws IOException { + if (object != null && path != null) { + try (Utils.TempPath tempPath = new Utils.TempPath(path)) { + serializeToFile(object, tempPath.getPath().toFile()); + Utils.atomicMove(tempPath.getPath(), path); + } + } + } + /** * Deserialize an {@link UploadInfo} object from a JSON string. * @@ -117,6 +166,33 @@ public static T deserialize(String json, Class clazz) throws IOException return OBJECT_MAPPER.readValue(json, clazz); } + /** + * Deserialize an {@link UploadInfo} object from a byte array. + * + * @param bytes The byte array containing the JSON data + * @return The deserialized UploadInfo instance, or null if input is null or empty + * @throws IOException If deserialization fails + */ + public static UploadInfo deserialize(byte[] bytes) throws IOException { + return deserialize(bytes, UploadInfo.class); + } + + /** + * Deserialize an object of the specified class from a byte array. + * + * @param The target object type + * @param bytes The byte array containing the JSON data + * @param clazz The target class type + * @return The deserialized instance, or null if input is null or empty + * @throws IOException If deserialization fails + */ + public static T deserialize(byte[] bytes, Class clazz) throws IOException { + if (bytes == null || bytes.length == 0 || clazz == null) { + return null; + } + return OBJECT_MAPPER.readValue(bytes, clazz); + } + /** * Deserialize an {@link UploadInfo} object from an {@link InputStream}. * @@ -143,4 +219,58 @@ public static T deserialize(InputStream inputStream, Class clazz) throws } return OBJECT_MAPPER.readValue(inputStream, clazz); } + + /** + * Deserialize an {@link UploadInfo} object from a {@link File}. + * + * @param file The file containing the JSON data + * @return The deserialized UploadInfo instance, or null if file is null or doesn't exist + * @throws IOException If deserialization fails + */ + public static UploadInfo deserialize(File file) throws IOException { + return deserialize(file, UploadInfo.class); + } + + /** + * Deserialize an object of the specified class from a {@link File}. + * + * @param The target object type + * @param file The file containing the JSON data + * @param clazz The target class type + * @return The deserialized instance, or null if file is null or doesn't exist + * @throws IOException If deserialization fails + */ + public static T deserialize(File file, Class clazz) throws IOException { + if (file == null || !file.exists() || file.length() == 0 || clazz == null) { + return null; + } + return OBJECT_MAPPER.readValue(file, clazz); + } + + /** + * Deserialize an {@link UploadInfo} object from a {@link Path}. + * + * @param path The path containing the JSON data + * @return The deserialized UploadInfo instance, or null if path is null or doesn't exist + * @throws IOException If deserialization fails + */ + public static UploadInfo deserialize(Path path) throws IOException { + return deserialize(path, UploadInfo.class); + } + + /** + * Deserialize an object of the specified class from a {@link Path}. + * + * @param The target object type + * @param path The path containing the JSON data + * @param clazz The target class type + * @return The deserialized instance, or null if path is null or doesn't exist + * @throws IOException If deserialization fails + */ + public static T deserialize(Path path, Class clazz) throws IOException { + if (path == null) { + return null; + } + return deserialize(path.toFile(), clazz); + } } diff --git a/src/main/java/me/desair/tus/server/util/Utils.java b/src/main/java/me/desair/tus/server/util/Utils.java index cf2373fc..19a4bc3b 100644 --- a/src/main/java/me/desair/tus/server/util/Utils.java +++ b/src/main/java/me/desair/tus/server/util/Utils.java @@ -1,7 +1,6 @@ package me.desair.tus.server.util; import static java.nio.file.StandardOpenOption.CREATE; -import static java.nio.file.StandardOpenOption.READ; import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING; import static java.nio.file.StandardOpenOption.WRITE; @@ -13,16 +12,18 @@ import java.io.ObjectOutputStream; import java.io.OutputStream; import java.io.Serializable; -import java.nio.channels.Channels; import java.nio.channels.FileChannel; import java.nio.channels.FileLock; import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; import java.util.EnumSet; import java.util.LinkedList; import java.util.List; import java.util.Objects; +import java.util.UUID; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -90,175 +91,140 @@ public static List parseConcatenationIDsFromHeader(String uploadConcatVa } /** - * Reads a serializable object from disk, optionally acquiring a shared file lock during the read - * operation. + * Reads a serializable object from disk. * * @param Target object type * @param path The file path to read from * @param clazz The target object class - * @param lockFile If true, acquires a shared file lock before reading * @return Deserialized object instance, or null if reading fails or file does not exist - * @throws IOException If file access or locking fails + * @throws IOException If file access fails */ - public static T readSerializable(Path path, Class clazz, boolean lockFile) - throws IOException { + public static T readSerializable(Path path, Class clazz) throws IOException { T info = null; if (path != null && Files.exists(path)) { - try (FileChannel channel = FileChannel.open(path, READ)) { - // Lock will be released when the channel is closed - if (!lockFile || lockFileShared(channel) != null) { - - try (ValidatingObjectInputStream ois = - new ValidatingObjectInputStream(Channels.newInputStream(channel))) { - ois.accept("java.lang.*", "java.util.*", "me.desair.tus.server.*"); - info = clazz.cast(ois.readObject()); - } catch (ClassNotFoundException - | java.io.EOFException - | java.io.StreamCorruptedException e) { - // File may be corrupted due to unexpected server shutdown - log.warn("Unable to read serializable file {}: {}", path, e.getMessage()); - info = null; - } - } else { - throw new IOException("Unable to lock file " + path); - } + try (InputStream is = Files.newInputStream(path); + ValidatingObjectInputStream ois = new ValidatingObjectInputStream(is)) { + ois.accept("java.lang.*", "java.util.*", "me.desair.tus.server.*"); + info = clazz.cast(ois.readObject()); + } catch (ClassNotFoundException | java.io.EOFException | java.io.StreamCorruptedException e) { + // File may be corrupted due to unexpected server shutdown + log.warn("Unable to read serializable file {}: {}", path, e.getMessage()); + info = null; } } return info; } /** - * Reads a serializable object from disk, acquiring a shared file lock during the read operation. + * Deletes a file or directory if it exists, quietly ignoring any exceptions that occur. * - * @param Target object type - * @param path The file path to read from - * @param clazz The target object class - * @return Deserialized object instance, or null if reading fails or file does not exist - * @throws IOException If file access or locking fails + * @param path The path to delete + * @return {@code true} if the file was deleted; {@code false} if it did not exist or deletion + * failed */ - public static T readSerializable(Path path, Class clazz) throws IOException { - return readSerializable(path, clazz, true); + public static boolean deletePathQuietly(Path path) { + if (path == null) { + return false; + } + try { + return Files.deleteIfExists(path); + } catch (Exception e) { + log.debug("Failed to delete path quietly: {}", path, e); + return false; + } } /** - * Writes a serializable object to a file on disk, optionally acquiring an exclusive file lock - * during the write operation. + * Resolves a unique temporary sibling path for a target destination file path. * - * @param object The serializable object to write - * @param path The file path to write to - * @param lockFile If true, acquires an exclusive file lock before writing - * @throws IOException If file access or locking fails + * @param targetPath The destination file path + * @return A temporary sibling path with a unique UUID suffix, or null if targetPath is null */ - public static void writeSerializable(Serializable object, Path path, boolean lockFile) - throws IOException { - if (path != null) { - try (FileChannel channel = FileChannel.open(path, WRITE, CREATE, TRUNCATE_EXISTING)) { - // Lock will be released when the channel is closed - if (!lockFile || lockFileExclusively(channel) != null) { - - try (OutputStream buffer = new BufferedOutputStream(Channels.newOutputStream(channel)); - ObjectOutput output = new ObjectOutputStream(buffer)) { - - output.writeObject(object); - } - } else { - throw new IOException("Unable to lock file " + path); - } - } + public static Path createTempSiblingPath(Path targetPath) { + if (targetPath == null) { + return null; } + Path parent = targetPath.getParent(); + String tempFileName = targetPath.getFileName().toString() + ".tmp." + UUID.randomUUID(); + return parent != null ? parent.resolve(tempFileName) : Paths.get(tempFileName); } /** - * Writes a serializable object to a file on disk, acquiring an exclusive file lock during the - * write operation. + * Creates an {@link AutoCloseable} {@link TempPath} that generates a unique temporary sibling + * path and automatically deletes it upon closing if it still exists. * - * @param object The serializable object to write - * @param path The file path to write to - * @throws IOException If file access or locking fails + * @param targetPath The destination file path + * @return An AutoCloseable TempPath instance */ - public static void writeSerializable(Serializable object, Path path) throws IOException { - writeSerializable(object, path, true); + public static TempPath createTempSibling(Path targetPath) { + return new TempPath(targetPath); } /** - * Reads an object from a JSON file on disk, optionally acquiring a shared file lock during the - * read operation. + * Atomically moves a source file to a destination path, replacing any existing destination file. * - * @param Target object type - * @param path The file path to read from - * @param clazz The target object class - * @param lockFile If true, acquires a shared file lock before reading - * @return Deserialized object instance, or null if reading fails or file does not exist - * @throws IOException If file access or locking fails + * @param source The source file path to move + * @param destination The target destination file path + * @throws IOException If moving the file fails */ - public static T readJson(Path path, Class clazz, boolean lockFile) throws IOException { - T info = null; - if (path != null && Files.exists(path)) { - try (FileChannel channel = FileChannel.open(path, READ)) { - // Lock will be released when the channel is closed - if (!lockFile || lockFileShared(channel) != null) { - try (InputStream is = Channels.newInputStream(channel)) { - info = UploadInfoJsonSerializer.deserialize(is, clazz); - } catch (Exception e) { - log.warn("Unable to read JSON file {}: {}", path, e.getMessage()); - info = null; - } - } else { - throw new IOException("Unable to lock file " + path); - } - } + public static void atomicMove(Path source, Path destination) throws IOException { + if (source != null && destination != null) { + Files.move( + source, destination, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); } - return info; - } - - /** - * Reads an object from a JSON file on disk, acquiring a shared file lock during the read - * operation. - * - * @param Target object type - * @param path The file path to read from - * @param clazz The target object class - * @return Deserialized object instance, or null if reading fails or file does not exist - * @throws IOException If file access or locking fails - */ - public static T readJson(Path path, Class clazz) throws IOException { - return readJson(path, clazz, true); } /** - * Writes an object to a file in JSON format, optionally acquiring an exclusive file lock during - * the write operation. + * Writes a serializable object to a file on disk atomically via a temporary file rename. * - * @param object The object to serialize to JSON + * @param object The serializable object to write * @param path The file path to write to - * @param lockFile If true, acquires an exclusive file lock before writing - * @throws IOException If file access or locking fails + * @throws IOException If file access or move fails */ - public static void writeJson(Object object, Path path, boolean lockFile) throws IOException { - if (path != null) { - try (FileChannel channel = FileChannel.open(path, WRITE, CREATE, TRUNCATE_EXISTING)) { - // Lock will be released when the channel is closed - if (!lockFile || lockFileExclusively(channel) != null) { - try (OutputStream buffer = new BufferedOutputStream(Channels.newOutputStream(channel))) { - UploadInfoJsonSerializer.serializeToStream(object, buffer); - } - } else { - throw new IOException("Unable to lock file " + path); + public static void writeSerializable(Serializable object, Path path) throws IOException { + if (path != null && object != null) { + try (TempPath tempPath = new TempPath(path)) { + try (OutputStream os = + Files.newOutputStream(tempPath.getPath(), WRITE, CREATE, TRUNCATE_EXISTING); + OutputStream buffer = new BufferedOutputStream(os); + ObjectOutput output = new ObjectOutputStream(buffer)) { + + output.writeObject(object); } + atomicMove(tempPath.getPath(), path); } } } /** - * Writes an object to a file in JSON format, acquiring an exclusive file lock during the write - * operation. - * - * @param object The object to serialize to JSON - * @param path The file path to write to - * @throws IOException If file access or locking fails + * AutoCloseable temporary sibling path that automatically cleans up (deletes) the temporary file + * upon closing if it still exists. */ - public static void writeJson(Object object, Path path) throws IOException { - writeJson(object, path, true); + public static class TempPath implements AutoCloseable { + private final Path path; + + /** + * Constructs a TempPath resolving a unique sibling path for the given target path. + * + * @param targetPath The destination file path + */ + public TempPath(Path targetPath) { + this.path = createTempSiblingPath(targetPath); + } + + /** + * Returns the underlying temporary sibling {@link Path}. + * + * @return The temporary path, or null if targetPath was null + */ + public Path getPath() { + return path; + } + + @Override + public void close() { + deletePathQuietly(path); + } } public static FileLock lockFileExclusively(FileChannel channel) throws IOException { @@ -653,10 +619,10 @@ public static void cleanupTempFiles(Path dir, String globPattern, long maxAgeMil for (Path file : stream) { try { if (Files.isRegularFile(file) && Files.getLastModifiedTime(file).toMillis() < cutoff) { - Files.deleteIfExists(file); + deletePathQuietly(file); } } catch (Exception e) { - log.debug("Error deleting stale temporary file {}: {}", file, e.getMessage()); + log.debug("Error checking temporary file age for {}: {}", file, e.getMessage()); } } } catch (Exception e) { diff --git a/src/test/java/me/desair/tus/server/TusFileUploadServiceTest.java b/src/test/java/me/desair/tus/server/TusFileUploadServiceTest.java index dc3b8c0e..667eb7e0 100644 --- a/src/test/java/me/desair/tus/server/TusFileUploadServiceTest.java +++ b/src/test/java/me/desair/tus/server/TusFileUploadServiceTest.java @@ -47,20 +47,16 @@ public void testAcquireUploadLockFallback() throws Exception { UploadLockingService mockLockingService = mock(UploadLockingService.class); UploadLock mockLock = mock(UploadLock.class); - // We throw exception 25 times and then succeed. - // To avoid waiting 5 seconds (25 * 200ms) in the test, we mock Thread.sleep by interrupting - // inside the mock, - // but wait, mockLockingService doesn't run sleep. Sleep runs in the service itself. - // Instead of doing 25 times which takes 5 seconds, let's just do it. 5 seconds is perfectly - // fine for a fallback test. - var stubbing = when(mockLockingService.lockUploadByUri(anyString())); - for (int i = 0; i < 25; i++) { - stubbing = stubbing.thenThrow(new UploadAlreadyLockedException("Locked")); - } - stubbing.thenReturn(mockLock); + // Verify retry mechanism by failing 2 times before succeeding + when(mockLockingService.lockUploadByUri(anyString())) + .thenThrow(new UploadAlreadyLockedException("Locked")) + .thenThrow(new UploadAlreadyLockedException("Locked")) + .thenReturn(mockLock); TusFileUploadService service = - new TusFileUploadService().withUploadLockingService(mockLockingService); + new TusFileUploadService() + .withUploadLockingService(mockLockingService) + .withMaxLockRetries(5); UploadLock lock = service.acquireUploadLock(HttpMethod.HEAD, "/files/test"); assertNotNull(lock); diff --git a/src/test/java/me/desair/tus/server/upload/AbstractLeaseLockTest.java b/src/test/java/me/desair/tus/server/upload/AbstractLeaseLockTest.java index 2783c8b6..38693e08 100644 --- a/src/test/java/me/desair/tus/server/upload/AbstractLeaseLockTest.java +++ b/src/test/java/me/desair/tus/server/upload/AbstractLeaseLockTest.java @@ -24,37 +24,19 @@ private static class TestLeaseLock extends AbstractLeaseLock { private final AtomicInteger renewCount = new AtomicInteger(0); private final AtomicBoolean released = new AtomicBoolean(false); - public TestLeaseLock() { - super(); - } - - public TestLeaseLock(String holderId, String requestUri, long leaseDurationMs, long expiresAt) { - super(holderId, requestUri, leaseDurationMs, expiresAt); - } - public TestLeaseLock( - String holderId, - long leaseDurationMs, - String requestUri, + LeaseData leaseData, Map activeInputStreams, String watchdogThreadName) { - super(holderId, leaseDurationMs, requestUri, activeInputStreams, watchdogThreadName); + super(leaseData, activeInputStreams, watchdogThreadName); } public TestLeaseLock( - String holderId, - long leaseDurationMs, - String requestUri, + LeaseData leaseData, Map activeInputStreams, ScheduledExecutorService heartbeatExecutor, String watchdogThreadName) { - super( - holderId, - leaseDurationMs, - requestUri, - activeInputStreams, - heartbeatExecutor, - watchdogThreadName); + super(leaseData, activeInputStreams, heartbeatExecutor, watchdogThreadName); } @Override @@ -69,13 +51,10 @@ protected void releaseLockResource() { } @Test - public void testDefaultConstructorAndGettersSetters() { - TestLeaseLock lock = new TestLeaseLock(); - lock.setHolderId("holder-1"); - lock.setRequestUri("/files/upload-1"); - lock.setLeaseDurationMs(30000L); - lock.setExpiresAt(100000L); - lock.setAcquiredAt(50000L); + public void testGettersAndSetters() { + LeaseData leaseData = + new LeaseData("holder-1", "/files/upload-1", 30000L, 100000L, 50000L, "/path", "/stop"); + TestLeaseLock lock = new TestLeaseLock(leaseData, null, null); assertThat(lock.getHolderId(), is("holder-1")); assertThat(lock.getRequestUri(), is("/files/upload-1")); @@ -83,17 +62,10 @@ public void testDefaultConstructorAndGettersSetters() { assertThat(lock.getLeaseDurationMs(), is(30000L)); assertThat(lock.getExpiresAt(), is(100000L)); assertThat(lock.getAcquiredAt(), is(50000L)); - } + assertThat(lock.getLeaseData(), is(leaseData)); - @Test - public void testSerializationConstructor() { - TestLeaseLock lock = new TestLeaseLock("holder-2", "/files/upload-2", 15000L, 99999L); - - assertThat(lock.getHolderId(), is("holder-2")); - assertThat(lock.getRequestUri(), is("/files/upload-2")); - assertThat(lock.getLeaseDurationMs(), is(15000L)); - assertThat(lock.getExpiresAt(), is(99999L)); - assertThat(lock.getAcquiredAt(), greaterThan(0L)); + lock.setExpiresAt(200000L); + assertThat(lock.getExpiresAt(), is(200000L)); } @Test @@ -101,8 +73,9 @@ public void testActiveLockConstructorWithAutoWatchdog() throws Exception { Map activeStreams = new ConcurrentHashMap<>(); activeStreams.put("/files/upload-3", new ByteArrayInputStream("test".getBytes())); - TestLeaseLock lock = - new TestLeaseLock("holder-3", 10000L, "/files/upload-3", activeStreams, "test-watchdog"); + LeaseData leaseData = + new LeaseData("holder-3", "/files/upload-3", 10000L, System.currentTimeMillis() + 10000L); + TestLeaseLock lock = new TestLeaseLock(leaseData, activeStreams, "test-watchdog"); assertThat(lock.getHolderId(), is("holder-3")); assertThat(lock.getLeaseDurationMs(), is(10000L)); @@ -126,9 +99,9 @@ public void testActiveLockConstructorWithCustomExecutor() { ScheduledExecutorService mockExecutor = mock(ScheduledExecutorService.class); Map activeStreams = new ConcurrentHashMap<>(); - TestLeaseLock lock = - new TestLeaseLock( - "holder-4", 10000L, "/files/upload-4", activeStreams, mockExecutor, "test-watchdog"); + LeaseData leaseData = + new LeaseData("holder-4", "/files/upload-4", 10000L, System.currentTimeMillis() + 10000L); + TestLeaseLock lock = new TestLeaseLock(leaseData, activeStreams, mockExecutor, "test-watchdog"); assertNotNull(lock); lock.close(); @@ -139,28 +112,28 @@ public void testActiveLockConstructorWithCustomExecutor() { @Test public void testActiveLockConstructorWithZeroLeaseDurationDoesNotSchedule() { - TestLeaseLock lock = - new TestLeaseLock("holder-5", 0L, "/files/upload-5", null, "test-watchdog"); + LeaseData leaseData = new LeaseData("holder-5", "/files/upload-5", 0L, 0L); + TestLeaseLock lock = new TestLeaseLock(leaseData, null, "test-watchdog"); lock.close(); assertTrue(lock.released.get()); - TestLeaseLock lockNullWatchdog = - new TestLeaseLock("holder-5b", 10000L, "/files/upload-5b", null, null); + LeaseData leaseData2 = new LeaseData("holder-5b", "/files/upload-5b", 10000L, 10000L); + TestLeaseLock lockNullWatchdog = new TestLeaseLock(leaseData2, null, null); lockNullWatchdog.close(); assertTrue(lockNullWatchdog.released.get()); } @Test public void testCloseWithNullStreamsOrNullUri() { - TestLeaseLock lockNullStreams = - new TestLeaseLock("holder-6", 10000L, "/files/upload-6", null, "test-watchdog"); + LeaseData leaseData = new LeaseData("holder-6", "/files/upload-6", 10000L, 10000L); + TestLeaseLock lockNullStreams = new TestLeaseLock(leaseData, null, "test-watchdog"); lockNullStreams.close(); assertTrue(lockNullStreams.released.get()); Map activeStreams = new ConcurrentHashMap<>(); - TestLeaseLock lockNullUri = - new TestLeaseLock("holder-7", 10000L, null, activeStreams, "test-watchdog"); + LeaseData leaseDataNullUri = new LeaseData("holder-7", null, 10000L, 10000L); + TestLeaseLock lockNullUri = new TestLeaseLock(leaseDataNullUri, activeStreams, "test-watchdog"); lockNullUri.close(); assertTrue(lockNullUri.released.get()); } diff --git a/src/test/java/me/desair/tus/server/upload/AbstractLeaseLockingServiceTest.java b/src/test/java/me/desair/tus/server/upload/AbstractLeaseLockingServiceTest.java index eba4dfd9..d547ef9c 100644 --- a/src/test/java/me/desair/tus/server/upload/AbstractLeaseLockingServiceTest.java +++ b/src/test/java/me/desair/tus/server/upload/AbstractLeaseLockingServiceTest.java @@ -38,7 +38,7 @@ public TestLeaseLockingService() { } @Override - protected UploadLock tryAcquireLock(UploadId uploadId, String holderId, String requestUri) { + protected UploadLock tryAcquireLock(UploadId uploadId, LeaseData leaseData) { if (acquireShouldSucceed) { return mock(UploadLock.class); } @@ -120,8 +120,7 @@ public void testLockUploadByUriEvictAndRetrySuccess() throws Exception { private int acquireAttempts = 0; @Override - protected UploadLock tryAcquireLock( - UploadId uploadId, String holderId, String requestUri) { + protected UploadLock tryAcquireLock(UploadId uploadId, LeaseData leaseData) { acquireAttempts++; if (acquireAttempts == 1) { return null; diff --git a/src/test/java/me/desair/tus/server/upload/disk/LeaseFileLockingServiceTest.java b/src/test/java/me/desair/tus/server/upload/disk/LeaseFileLockingServiceTest.java index 54e3c80a..7d8190ba 100644 --- a/src/test/java/me/desair/tus/server/upload/disk/LeaseFileLockingServiceTest.java +++ b/src/test/java/me/desair/tus/server/upload/disk/LeaseFileLockingServiceTest.java @@ -24,11 +24,13 @@ import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicInteger; import me.desair.tus.server.exception.UploadAlreadyLockedException; +import me.desair.tus.server.upload.LeaseData; import me.desair.tus.server.upload.UploadId; import me.desair.tus.server.upload.UploadIdFactory; import me.desair.tus.server.upload.UploadLock; import me.desair.tus.server.upload.UuidUploadIdFactory; import me.desair.tus.server.util.InterruptibleInputStream; +import me.desair.tus.server.util.LeaseDataJsonSerializer; import me.desair.tus.server.util.Utils; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; @@ -116,7 +118,7 @@ public void testLockAcquireCreatesDirectoryAndLeaseFile() throws Exception { Path leaseFile = lockDir.resolve("lease.json"); assertTrue(Files.exists(leaseFile)); - LeaseFileUploadLock lease = Utils.readJson(leaseFile, LeaseFileUploadLock.class, false); + LeaseData lease = LeaseDataJsonSerializer.deserialize(leaseFile); assertNotNull(lease); assertNotNull(lease.getHolderId()); assertTrue(lease.getExpiresAt() > System.currentTimeMillis()); @@ -151,17 +153,16 @@ public void testExpiredLeaseEvictedAndReacquired() throws Exception { // Write an already-expired lease.json long pastTime = System.currentTimeMillis() - 10_000L; - LeaseFileUploadLock expiredLease = + LeaseData expiredLease = createExpiredLease("expired-holder", uri, lockDir.toString(), pastTime); - Utils.writeJson(expiredLease, lockDir.resolve("lease.json"), false); + LeaseDataJsonSerializer.serializeToPath(expiredLease, lockDir.resolve("lease.json")); Files.setLastModifiedTime(lockDir, FileTime.fromMillis(pastTime)); // Contender acquires lock: expired directory should be evicted and new lock acquired UploadLock lock = lockingService.lockUploadByUri(uri); assertNotNull(lock); - LeaseFileUploadLock activeLease = - Utils.readJson(lockDir.resolve("lease.json"), LeaseFileUploadLock.class, false); + LeaseData activeLease = LeaseDataJsonSerializer.deserialize(lockDir.resolve("lease.json")); assertNotNull(activeLease); assertEquals(activeLease.getHolderId(), ((LeaseFileUploadLock) lock).getHolderId()); assertTrue(activeLease.getExpiresAt() > System.currentTimeMillis()); @@ -194,11 +195,9 @@ public void testEmptyLockDirectoryAfterGracePeriodEvicted() throws Exception { long tenSecondsAgo = System.currentTimeMillis() - 10_000L; Files.setLastModifiedTime(lockDir, FileTime.fromMillis(tenSecondsAgo)); - // Must be treated as abandoned and evicted + // Empty lock directory older than 5s grace period: treat as abandoned crash and evict UploadLock lock = lockingService.lockUploadByUri(uri); assertNotNull(lock); - - assertTrue(Files.exists(lockDir.resolve("lease.json"))); lock.close(); } @@ -212,7 +211,7 @@ public void testCorruptedLeaseFileWithinGracePeriodThrowsLocked() throws Excepti Files.write(lockDir.resolve("lease.json"), "invalid-json-content-{{{".getBytes()); Files.setLastModifiedTime(lockDir, FileTime.fromMillis(System.currentTimeMillis())); - // Within grace period: treat as locked + // Within 5s grace period: treat corrupted lease.json as active write in progress lockingService.lockUploadByUri(uri); } @@ -231,8 +230,7 @@ public void testCorruptedLeaseFileAfterGracePeriodEvicted() throws Exception { UploadLock lock = lockingService.lockUploadByUri(uri); assertNotNull(lock); - LeaseFileUploadLock lease = - Utils.readJson(lockDir.resolve("lease.json"), LeaseFileUploadLock.class, false); + LeaseData lease = LeaseDataJsonSerializer.deserialize(lockDir.resolve("lease.json")); assertNotNull(lease); lock.close(); @@ -267,10 +265,10 @@ public void testCleanupStaleLocksRemovesExpiredOnly() throws Exception { Path expiredLockDir = storagePath.resolve("locks").resolve(expiredIdStr + ".lock"); Files.createDirectories(expiredLockDir); long pastTime = System.currentTimeMillis() - 10_000L; - LeaseFileUploadLock expiredLease = + LeaseData expiredLease = createExpiredLease( "expired", UPLOAD_URL + "/" + expiredIdStr, expiredLockDir.toString(), pastTime); - Utils.writeJson(expiredLease, expiredLockDir.resolve("lease.json"), false); + LeaseDataJsonSerializer.serializeToPath(expiredLease, expiredLockDir.resolve("lease.json")); // 3. Create stale .stop file Path staleStopFile = storagePath.resolve("locks").resolve(staleStopIdStr + ".stop"); @@ -409,29 +407,12 @@ public void testConcurrentEvictionContention() throws Exception { Path lockDir = storagePath.resolve("locks").resolve(uploadIdStr + ".lock"); Files.createDirectories(lockDir); long pastTime = System.currentTimeMillis() - 10_000L; - LeaseFileUploadLock expiredLease = - createExpiredLease("expired", uri, lockDir.toString(), pastTime); - Utils.writeJson(expiredLease, lockDir.resolve("lease.json"), false); + LeaseData expiredLease = createExpiredLease("expired", uri, lockDir.toString(), pastTime); + LeaseDataJsonSerializer.serializeToPath(expiredLease, lockDir.resolve("lease.json")); Files.setLastModifiedTime(lockDir, FileTime.fromMillis(pastTime)); // Simulate 10 concurrent threads simultaneously discovering the expired lock - // and racing to evict it and acquire a fresh lock. - // - // TOCTOU (Time-of-Check to Time-of-Use) explanation: - // Without post-move verification in atomicEvictExpiredLock: - // 1. Thread A & Thread B both check that the lock directory is expired (Time of Check: true). - // 2. Thread A renames the directory, deletes it, and creates a brand-new active lock. - // 3. Thread B (having already checked expiration earlier) renames Thread A's NEW lock directory - // and deletes it (Time of Use), then acquires a second lock handle. - // 4. Result: Both Thread A and Thread B believe they hold exclusive ownership (successCount = - // 2). - // - // With post-move verification in atomicEvictExpiredLock: - // - When Thread B renames the directory, it inspects the isolated directory (evictPath) - // post-move. - // - Thread B discovers that evictPath contains Thread A's active lease, restores it back to - // lockDirPath, and aborts eviction. - // - Result: Exactly 1 thread wins the eviction and acquisition (successCount = 1). + // and racing to evict it and acquire a fresh lock under sibling mutex protection. int threadCount = 10; ExecutorService executor = Executors.newFixedThreadPool(threadCount); List> tasks = new ArrayList<>(); @@ -487,6 +468,65 @@ public void testConcurrentEvictionContention() throws Exception { } } + @Test + public void testConcurrentEvictionContentionStress() throws Exception { + // Run 5 iterations of 50 concurrent threads racing on expired locks to ensure 0% flakiness + for (int iter = 0; iter < 5; iter++) { + String uploadIdStr = UUID.randomUUID().toString(); + String uri = UPLOAD_URL + "/" + uploadIdStr; + + Path lockDir = storagePath.resolve("locks").resolve(uploadIdStr + ".lock"); + Files.createDirectories(lockDir); + long pastTime = System.currentTimeMillis() - 10_000L; + LeaseData expiredLease = + createExpiredLease("expired-" + iter, uri, lockDir.toString(), pastTime); + LeaseDataJsonSerializer.serializeToPath(expiredLease, lockDir.resolve("lease.json")); + Files.setLastModifiedTime(lockDir, FileTime.fromMillis(pastTime)); + + int threadCount = 50; + ExecutorService executor = Executors.newFixedThreadPool(threadCount); + List> tasks = new ArrayList<>(); + AtomicInteger successCount = new AtomicInteger(0); + AtomicInteger lockedExceptionCount = new AtomicInteger(0); + List acquiredLocks = new java.util.concurrent.CopyOnWriteArrayList<>(); + java.util.concurrent.CountDownLatch startLatch = new java.util.concurrent.CountDownLatch(1); + + for (int i = 0; i < threadCount; i++) { + tasks.add( + () -> { + try { + startLatch.await(); + UploadLock lock = lockingService.lockUploadByUri(uri); + if (lock != null) { + successCount.incrementAndGet(); + acquiredLocks.add(lock); + return true; + } + } catch (UploadAlreadyLockedException e) { + lockedExceptionCount.incrementAndGet(); + } + return false; + }); + } + + startLatch.countDown(); + List> futures = executor.invokeAll(tasks); + executor.shutdown(); + + assertEquals("Iteration " + iter + " failed: successCount", 1, successCount.get()); + assertEquals( + "Iteration " + iter + " failed: lockedExceptionCount", 49, lockedExceptionCount.get()); + assertEquals(1, acquiredLocks.size()); + + for (UploadLock lock : acquiredLocks) { + lock.close(); + } + if (Files.exists(lockDir)) { + FileUtils.deleteDirectory(lockDir.toFile()); + } + } + } + @Test public void testConstructorsAndSetIdFactory() { LeaseFileLockingService s1 = new LeaseFileLockingService(storagePath.toString()); @@ -619,12 +659,19 @@ public void testCleanupStaleLocksWithVariousFileTypes() throws Exception { Files.setLastModifiedTime( staleStopFile, FileTime.fromMillis(System.currentTimeMillis() - 20_000L)); + // 5. Stale .mutex directory older than 10 seconds (should be deleted) + Path staleMutexDir = locksDir.resolve("stale-" + UUID.randomUUID() + ".mutex"); + Files.createDirectories(staleMutexDir); + Files.setLastModifiedTime( + staleMutexDir, FileTime.fromMillis(System.currentTimeMillis() - 20_000L)); + lockingService.cleanupStaleLocks(); assertTrue(Files.exists(regularLockFile)); assertTrue(Files.exists(stopDir)); assertTrue(Files.exists(unrelatedFile)); assertFalse(Files.exists(staleStopFile)); + assertFalse(Files.exists(staleMutexDir)); Files.deleteIfExists(regularLockFile); Files.deleteIfExists(stopDir); @@ -735,7 +782,6 @@ public void testAtomicEvictExpiredLockOnActiveLockReturnsFalse() throws Exceptio Path lockDir = lockingService.getLockDirPath(new UploadId(uploadIdStr)); - // Verify that atomicEvictExpiredLock checks both pre-move and post-move expiration: // Attempting atomic eviction on an active, unexpired lock directory must return false // and must never destroy the active lock directory or its lease metadata. boolean evicted = lockingService.atomicEvictExpiredLock(lockDir); @@ -746,7 +792,7 @@ public void testAtomicEvictExpiredLockOnActiveLockReturnsFalse() throws Exceptio } @Test - public void testAtomicEvictExpiredLockPostMoveRollbackOnActiveLease() throws Exception { + public void testAtomicEvictExpiredLockUnderMutexAbortsWhenLeaseIsActive() throws Exception { String uploadIdStr = UUID.randomUUID().toString(); String uri = UPLOAD_URL + "/" + uploadIdStr; Path lockDir = storagePath.resolve("locks").resolve(uploadIdStr + ".lock"); @@ -754,37 +800,16 @@ public void testAtomicEvictExpiredLockPostMoveRollbackOnActiveLease() throws Exc // Write an active unexpired lease into lockDir long futureTime = System.currentTimeMillis() + 30_000L; - LeaseFileUploadLock activeLease = + LeaseData activeLease = createExpiredLease("active-holder", uri, lockDir.toString(), futureTime); - Utils.writeJson(activeLease, lockDir.resolve("lease.json"), false); - - // Create a service subclass to simulate a TOCTOU race: - // 1. Time-of-Check (pre-check in atomicEvictExpiredLock): simulates observing an expired lock - // before another node's write - // 2. Time-of-Use (post-move check in atomicEvictExpiredLock): accurately inspects evictPath and - // finds the active lease - AtomicInteger checkCount = new AtomicInteger(0); - LeaseFileLockingService serviceWithRaceSimulation = - new LeaseFileLockingService(idFactory, storagePath.toString()) { - @Override - boolean isLockDirectoryExpired(Path dir, long now) { - if (checkCount.incrementAndGet() == 1) { - // Simulate stale pre-check returning true (expired) - return true; - } - // Real post-move check on evictPath - return super.isLockDirectoryExpired(dir, now); - } - }; + LeaseDataJsonSerializer.serializeToPath(activeLease, lockDir.resolve("lease.json")); - // atomicEvictExpiredLock MUST detect the active lease post-move, roll back the move, - // restore lockDir to its original location, and return false - boolean evicted = serviceWithRaceSimulation.atomicEvictExpiredLock(lockDir); + // atomicEvictExpiredLock must inspect the lease under mutex, find it active, and return false + boolean evicted = lockingService.atomicEvictExpiredLock(lockDir); assertFalse(evicted); assertTrue(Files.exists(lockDir)); assertTrue(Files.exists(lockDir.resolve("lease.json"))); - serviceWithRaceSimulation.close(); FileUtils.deleteDirectory(lockDir.toFile()); } @@ -799,6 +824,7 @@ public void testWriteStopSignalWhenStopFileCannotBeWritten() throws Exception { // writeStopSignal catches IOException and logs warning lockingService.writeStopSignal(id); + assertTrue(Files.isDirectory(stopPath)); FileUtils.deleteDirectory(stopPath.toFile()); } @@ -827,12 +853,41 @@ public void testConstructorsAndNullChecks() throws Exception { assertNull(lockingService.getStopFilePath(null)); } - private LeaseFileUploadLock createExpiredLease( + @Test(expected = StoragePathNotAvailableException.class) + public void testInitStoragePathThrowsWhenPathIsFile() throws Exception { + Path filePath = storagePath.resolve("a-regular-file"); + Files.write(filePath, new byte[0]); + // Passing a path inside a regular file will fail mkdirs + new LeaseFileLockingService(filePath.resolve("sub-dir").toString()); + } + + @Test + public void testTryAcquireLockWhenParentCannotBeCreated() throws Exception { + Path regularFile = storagePath.resolve("blocking-file"); + Files.write(regularFile, new byte[0]); + + LeaseFileLockingService service = + new LeaseFileLockingService(idFactory, storagePath.toString()) { + @Override + Path getLockDirPath(UploadId id) { + return regularFile.resolve("child.lock"); + } + }; + + LeaseData leaseData = + new LeaseData( + "holder-1", "/files/upload/test-id", 30000L, System.currentTimeMillis() + 30000L); + UploadLock lock = service.tryAcquireLock(new UploadId("test-id"), leaseData); + assertNull(lock); + service.close(); + } + + private LeaseData createExpiredLease( String holderId, String uri, String storagePath, long expiresAt) { - LeaseFileUploadLock lease = new LeaseFileUploadLock(); + LeaseData lease = new LeaseData(); lease.setHolderId(holderId); lease.setRequestUri(uri); - lease.setStoragePath(storagePath); + lease.setLockPath(storagePath); lease.setLeaseDurationMs(30_000L); lease.setExpiresAt(expiresAt); lease.setAcquiredAt(expiresAt - 30_000L); diff --git a/src/test/java/me/desair/tus/server/upload/disk/LeaseFileMutexTest.java b/src/test/java/me/desair/tus/server/upload/disk/LeaseFileMutexTest.java new file mode 100644 index 00000000..5e9b632b --- /dev/null +++ b/src/test/java/me/desair/tus/server/upload/disk/LeaseFileMutexTest.java @@ -0,0 +1,131 @@ +package me.desair.tus.server.upload.disk; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.attribute.FileTime; +import java.util.UUID; +import me.desair.tus.server.upload.UploadId; +import org.apache.commons.io.FileUtils; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * Unit tests for {@link LeaseFileMutex} verifying atomic acquisition, crash recovery, and cleanup. + */ +public class LeaseFileMutexTest { + + private static Path storagePath; + + @BeforeClass + public static void setUpFolder() throws IOException { + storagePath = Paths.get("target", "tus", "lease-mutex-test").toAbsolutePath(); + Files.createDirectories(storagePath); + } + + @AfterClass + public static void tearDownFolder() throws IOException { + FileUtils.deleteDirectory(storagePath.toFile()); + } + + @Test + public void testAcquireAndReleaseWithTryWithResources() throws Exception { + Path lockDir = storagePath.resolve("test-" + UUID.randomUUID() + ".lock"); + + try (LeaseFileMutex mutex = new LeaseFileMutex(lockDir)) { + assertNotNull(mutex.getPath()); + assertTrue(mutex.getPath().getFileName().toString().endsWith(".mutex")); + assertTrue(mutex.isAcquired()); + assertTrue(Files.exists(mutex.getPath())); + } + + Path mutexDir = LeaseFileMutex.resolveMutexDir(lockDir); + assertNotNull(mutexDir); + assertFalse(Files.exists(mutexDir)); + } + + @Test + public void testConstructorsAndPathResolution() { + assertNull(new LeaseFileMutex((Path) null).getPath()); + assertNull(new LeaseFileMutex(null, null).getPath()); + assertNull(new LeaseFileMutex(storagePath, null).getPath()); + assertNull(new LeaseFileMutex(null, new UploadId("123")).getPath()); + assertNull(LeaseFileMutex.resolveMutexDir(null)); + + UploadId id = new UploadId("upload-abc"); + LeaseFileMutex mutexFromId = new LeaseFileMutex(storagePath, id); + assertEquals(storagePath.resolve("upload-abc.mutex"), mutexFromId.getPath()); + assertTrue(mutexFromId.isAcquired()); + mutexFromId.release(); + + Path lockPath = storagePath.resolve("upload-xyz.lock"); + LeaseFileMutex mutexFromLock = new LeaseFileMutex(lockPath); + assertEquals(storagePath.resolve("upload-xyz.mutex"), mutexFromLock.getPath()); + assertTrue(mutexFromLock.isAcquired()); + mutexFromLock.release(); + + Path explicitPath = storagePath.resolve("explicit.mutex"); + LeaseFileMutex explicitMutex = new LeaseFileMutex(explicitPath, true); + assertEquals(explicitPath, explicitMutex.getPath()); + assertTrue(explicitMutex.isAcquired()); + explicitMutex.release(); + } + + @Test + public void testDuplicateAcquireFailsAndUnacquiredCloseDoesNotDeleteExistingMutex() + throws Exception { + Path lockDir = storagePath.resolve("duplicate-" + UUID.randomUUID() + ".lock"); + try (LeaseFileMutex mutex1 = new LeaseFileMutex(lockDir)) { + assertTrue(mutex1.isAcquired()); + assertTrue(Files.exists(mutex1.getPath())); + + // Second concurrent contender fails to acquire + try (LeaseFileMutex mutex2 = new LeaseFileMutex(lockDir)) { + assertFalse(mutex2.isAcquired()); + } + + // Closing unacquired mutex2 must NOT delete mutex1's active directory! + assertTrue(Files.exists(mutex1.getPath())); + } + + // After mutex1 is closed, directory is deleted and a new contender can acquire + try (LeaseFileMutex mutex3 = new LeaseFileMutex(lockDir)) { + assertTrue(mutex3.isAcquired()); + assertTrue(Files.exists(mutex3.getPath())); + } + } + + @Test + public void testStaleMutexRecoveryAfterGracePeriod() throws Exception { + Path lockDir = storagePath.resolve("stale-" + UUID.randomUUID() + ".lock"); + Path mutexDir = LeaseFileMutex.resolveMutexDir(lockDir); + assertNotNull(mutexDir); + + Files.createDirectories(mutexDir); + // Set mtime to 10 seconds ago (> 5s grace period) + Files.setLastModifiedTime(mutexDir, FileTime.fromMillis(System.currentTimeMillis() - 10_000L)); + + // Stale mutex from crashed node must be recovered and acquired in constructor + try (LeaseFileMutex mutex = new LeaseFileMutex(lockDir)) { + assertTrue(mutex.isAcquired()); + assertTrue(Files.exists(mutexDir)); + } + + assertFalse(Files.exists(mutexDir)); + } + + @Test + public void testNullAndErrorHandling() { + LeaseFileMutex nullMutex = new LeaseFileMutex((Path) null); + assertFalse(nullMutex.isAcquired()); + nullMutex.release(); + } +} diff --git a/src/test/java/me/desair/tus/server/upload/disk/LeaseFileUploadLockTest.java b/src/test/java/me/desair/tus/server/upload/disk/LeaseFileUploadLockTest.java index 79eca199..fe0439b8 100644 --- a/src/test/java/me/desair/tus/server/upload/disk/LeaseFileUploadLockTest.java +++ b/src/test/java/me/desair/tus/server/upload/disk/LeaseFileUploadLockTest.java @@ -4,7 +4,9 @@ import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.io.ByteArrayInputStream; @@ -16,7 +18,8 @@ import java.util.Map; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; -import me.desair.tus.server.util.Utils; +import me.desair.tus.server.upload.LeaseData; +import me.desair.tus.server.util.LeaseDataJsonSerializer; import org.apache.commons.io.FileUtils; import org.junit.After; import org.junit.AfterClass; @@ -58,6 +61,21 @@ public void tearDown() throws IOException { FileUtils.deleteDirectory(testLockDir.toFile()); } Files.deleteIfExists(testStopFile); + Path mutexDir = LeaseFileMutex.resolveMutexDir(testLockDir); + if (mutexDir != null && Files.exists(mutexDir)) { + FileUtils.deleteDirectory(mutexDir.toFile()); + } + } + + private LeaseData createLeaseData(String holderId, String requestUri, long durationMs) { + return new LeaseData( + holderId, + requestUri, + durationMs, + System.currentTimeMillis() + durationMs, + System.currentTimeMillis(), + testLockDir != null ? testLockDir.toString() : null, + testStopFile != null ? testStopFile.toString() : null); } @Test @@ -67,9 +85,9 @@ public void testHeartbeatRenewalUpdatesExpiresAt() throws Exception { long leaseDurationMs = 30_000L; Map activeStreams = new ConcurrentHashMap<>(); + LeaseData leaseData = createLeaseData(holderId, requestUri, leaseDurationMs); LeaseFileUploadLock lock = - new LeaseFileUploadLock( - testLockDir, testStopFile, holderId, leaseDurationMs, requestUri, activeStreams); + new LeaseFileUploadLock(leaseData, testLockDir, testStopFile, activeStreams); long initialExpiresAt = lock.getExpiresAt(); Thread.sleep(50L); @@ -83,7 +101,7 @@ public void testHeartbeatRenewalUpdatesExpiresAt() throws Exception { Path leaseFile = testLockDir.resolve("lease.json"); assertTrue(Files.exists(leaseFile)); - LeaseFileUploadLock deserialized = Utils.readJson(leaseFile, LeaseFileUploadLock.class, false); + LeaseData deserialized = LeaseDataJsonSerializer.deserialize(leaseFile); assertThat(deserialized, is(notNullValue())); assertThat(deserialized.getExpiresAt(), is(lock.getExpiresAt())); assertThat(deserialized.getHolderId(), is(holderId)); @@ -103,9 +121,9 @@ public void testCloseTerminatesHeartbeatAndDeletesLockDir() throws Exception { Files.write(testStopFile, new byte[0]); assertTrue(Files.exists(testStopFile)); + LeaseData leaseData = createLeaseData(holderId, requestUri, leaseDurationMs); LeaseFileUploadLock lock = - new LeaseFileUploadLock( - testLockDir, testStopFile, holderId, leaseDurationMs, requestUri, activeStreams); + new LeaseFileUploadLock(leaseData, testLockDir, testStopFile, activeStreams); // Write initial lease file lock.renewLease(); @@ -132,9 +150,9 @@ public void testReleaseDelegatesToClose() throws Exception { long leaseDurationMs = 30_000L; Map activeStreams = new ConcurrentHashMap<>(); + LeaseData leaseData = createLeaseData(holderId, requestUri, leaseDurationMs); LeaseFileUploadLock lock = - new LeaseFileUploadLock( - testLockDir, testStopFile, holderId, leaseDurationMs, requestUri, activeStreams); + new LeaseFileUploadLock(leaseData, testLockDir, testStopFile, activeStreams); assertThat(lock.getUploadUri(), is(requestUri)); @@ -144,29 +162,32 @@ public void testReleaseDelegatesToClose() throws Exception { } @Test - public void testGettersAndSettersAndDefaultConstructor() { - LeaseFileUploadLock lock = new LeaseFileUploadLock(); - - lock.setHolderId("test-holder"); - lock.setRequestUri("/files/upload/uri"); - lock.setStoragePath("/var/storage"); - lock.setLeaseDurationMs(15000L); - lock.setExpiresAt(200000L); - lock.setAcquiredAt(100000L); - - assertThat(lock.getHolderId(), is("test-holder")); - assertThat(lock.getRequestUri(), is("/files/upload/uri")); - assertThat(lock.getStoragePath(), is("/var/storage")); - assertThat(lock.getLeaseDurationMs(), is(15000L)); - assertThat(lock.getExpiresAt(), is(200000L)); - assertThat(lock.getAcquiredAt(), is(100000L)); - assertThat(lock.getUploadUri(), is("/files/upload/uri")); + public void testLeaseDataGettersAndSetters() { + LeaseData data = new LeaseData(); + + data.setHolderId("test-holder"); + data.setRequestUri("/files/upload/uri"); + data.setLockPath("/var/storage/locks/1.lock"); + data.setStopPath("/var/storage/locks/1.stop"); + data.setLeaseDurationMs(15000L); + data.setExpiresAt(200000L); + data.setAcquiredAt(100000L); + + assertThat(data.getHolderId(), is("test-holder")); + assertThat(data.getRequestUri(), is("/files/upload/uri")); + assertThat(data.getLockPath(), is("/var/storage/locks/1.lock")); + assertThat(data.getStopPath(), is("/var/storage/locks/1.stop")); + assertThat(data.getLeaseDurationMs(), is(15000L)); + assertThat(data.getExpiresAt(), is(200000L)); + assertThat(data.getAcquiredAt(), is(100000L)); + assertTrue(data.isExpired(300000L)); + assertFalse(data.isExpired(100000L)); } @Test public void testActiveLockConstructorInitializesFields() { - LeaseFileUploadLock lock = - new LeaseFileUploadLock(testLockDir, testStopFile, "holder", 10000L, "/uri", null); + LeaseData leaseData = createLeaseData("holder", "/uri", 10000L); + LeaseFileUploadLock lock = new LeaseFileUploadLock(leaseData, testLockDir, testStopFile, null); assertThat(lock.getHolderId(), is("holder")); assertThat(lock.getRequestUri(), is("/uri")); @@ -174,32 +195,37 @@ public void testActiveLockConstructorInitializesFields() { assertThat(lock.getLeaseDurationMs(), is(10000L)); assertThat(lock.getExpiresAt(), greaterThan(0L)); assertThat(lock.getAcquiredAt(), greaterThan(0L)); + assertThat(lock.getLeaseData(), is(leaseData)); lock.close(); } @Test public void testConstructorWithZeroLeaseDurationDoesNotScheduleWatchdog() { - LeaseFileUploadLock lock = - new LeaseFileUploadLock(testLockDir, testStopFile, "holder", 0L, "/uri", null); + LeaseData leaseData = createLeaseData("holder", "/uri", 0L); + LeaseFileUploadLock lock = new LeaseFileUploadLock(leaseData, testLockDir, testStopFile, null); + assertEquals("holder", lock.getHolderId()); lock.close(); } @Test public void testRenewLeaseWithNullLockDirShouldBeNoOp() { - LeaseFileUploadLock lock = new LeaseFileUploadLock(null, null, "holder", 10000L, "/uri", null); + LeaseData leaseData = createLeaseData("holder", "/uri", 10000L); + LeaseFileUploadLock lock = new LeaseFileUploadLock(leaseData, null, null, null); // Should safely do nothing without throwing exception lock.renewLease(); + assertEquals("holder", lock.getHolderId()); lock.close(); } @Test - public void testRenewLeaseWhenLockDirIsDeletedOrInvalid() throws Exception { + public void testRenewLeaseWhenLockDirIsDeletedOrInvalid() { Path nonExistentDir = storagePath.resolve("non-existent-lock-dir-" + UUID.randomUUID()); - LeaseFileUploadLock lock = - new LeaseFileUploadLock(nonExistentDir, null, "holder", 10000L, "/uri", null); + LeaseData leaseData = createLeaseData("holder", "/uri", 10000L); + LeaseFileUploadLock lock = new LeaseFileUploadLock(leaseData, nonExistentDir, null, null); // When directory doesn't exist, renewLease logs a warning and does not throw lock.renewLease(); + assertEquals("holder", lock.getHolderId()); lock.close(); } @@ -209,9 +235,11 @@ public void testCloseWhenLockDirIsAlreadyDeleted() throws Exception { Files.createDirectories(dir); FileUtils.deleteDirectory(dir.toFile()); - LeaseFileUploadLock lock = new LeaseFileUploadLock(dir, null, "holder", 10000L, "/uri", null); + LeaseData leaseData = createLeaseData("holder", "/uri", 10000L); + LeaseFileUploadLock lock = new LeaseFileUploadLock(leaseData, dir, null, null); // Should execute cleanly without error lock.close(); + assertFalse(Files.exists(dir)); } @Test @@ -224,8 +252,8 @@ public void testCloseWithActiveStreamsAndStopFile() throws Exception { Map streams = new ConcurrentHashMap<>(); streams.put("/active/uri", new ByteArrayInputStream("test".getBytes())); - LeaseFileUploadLock lock = - new LeaseFileUploadLock(dir, stopFile, "holder", 10000L, "/active/uri", streams); + LeaseData leaseData = createLeaseData("holder", "/active/uri", 10000L); + LeaseFileUploadLock lock = new LeaseFileUploadLock(leaseData, dir, stopFile, streams); assertTrue(Files.exists(stopFile)); lock.close(); @@ -240,15 +268,18 @@ public void testCloseWithNullRequestUriOrNullStreams() throws Exception { Files.createDirectories(dir); Map streams = new ConcurrentHashMap<>(); + LeaseData leaseDataNullUri = createLeaseData("holder", null, 10000L); LeaseFileUploadLock lockWithNullUri = - new LeaseFileUploadLock(dir, null, "holder", 10000L, null, streams); + new LeaseFileUploadLock(leaseDataNullUri, dir, null, streams); lockWithNullUri.close(); + assertNotNull(lockWithNullUri.getHolderId()); Path dir2 = storagePath.resolve("null-streams-" + UUID.randomUUID()); Files.createDirectories(dir2); - LeaseFileUploadLock lockWithNullStreams = - new LeaseFileUploadLock(dir2, null, "holder", 10000L, "/uri", null); + LeaseData leaseData = createLeaseData("holder", "/uri", 10000L); + LeaseFileUploadLock lockWithNullStreams = new LeaseFileUploadLock(leaseData, dir2, null, null); lockWithNullStreams.close(); + assertNotNull(lockWithNullStreams.getHolderId()); } @Test @@ -259,9 +290,11 @@ public void testCloseWhenLockDirCannotBeDeleted() throws Exception { // DirectoryNotEmptyException Files.write(dir.resolve("extra-file.txt"), "data".getBytes()); - LeaseFileUploadLock lock = new LeaseFileUploadLock(dir, null, "holder", 10000L, "/uri", null); + LeaseData leaseData = createLeaseData("holder", "/uri", 10000L); + LeaseFileUploadLock lock = new LeaseFileUploadLock(leaseData, dir, null, null); // Should catch DirectoryNotEmptyException, log warning, and complete cleanly lock.close(); + assertTrue(Files.exists(dir)); FileUtils.deleteDirectory(dir.toFile()); } @@ -271,11 +304,96 @@ public void testRenewLeaseWhenLockDirPathIsRegularFile() throws Exception { Path fileAsDir = storagePath.resolve("file-as-dir-" + UUID.randomUUID()); Files.write(fileAsDir, "not a directory".getBytes()); - LeaseFileUploadLock lock = - new LeaseFileUploadLock(fileAsDir, null, "holder", 10000L, "/uri", null); + LeaseData leaseData = createLeaseData("holder", "/uri", 10000L); + LeaseFileUploadLock lock = new LeaseFileUploadLock(leaseData, fileAsDir, null, null); // Should catch exception attempting to create file inside a regular file and log warning lock.renewLease(); + assertTrue(Files.exists(fileAsDir)); Files.deleteIfExists(fileAsDir); } + + @Test + public void testCloseDoesNotDeleteSuccessorLockWhenHolderIdMismatch() throws Exception { + Path dir = storagePath.resolve("successor-test-" + UUID.randomUUID() + ".lock"); + Files.createDirectories(dir); + + // Simulate that a successor node took over the lock with a new holderId + LeaseData successorLease = + new LeaseData( + "successor-holder", + "/files/upload/test", + 30_000L, + System.currentTimeMillis() + 30_000L, + System.currentTimeMillis(), + dir.toString(), + null); + LeaseDataJsonSerializer.serializeToPath(successorLease, dir.resolve("lease.json")); + + // Original lock holder (who unpaused or awoke late) calls close() + LeaseData originalLease = + new LeaseData( + "original-holder", + "/files/upload/test", + 30_000L, + System.currentTimeMillis() - 1000L, + System.currentTimeMillis() - 31_000L, + dir.toString(), + null); + LeaseFileUploadLock originalLock = new LeaseFileUploadLock(originalLease, dir, null, null); + + originalLock.close(); + + // The successor's lock directory and lease file must NOT be deleted + assertTrue(Files.exists(dir)); + assertTrue(Files.exists(dir.resolve("lease.json"))); + + LeaseData preservedLease = LeaseDataJsonSerializer.deserialize(dir.resolve("lease.json")); + assertNotNull(preservedLease); + assertEquals("successor-holder", preservedLease.getHolderId()); + + FileUtils.deleteDirectory(dir.toFile()); + } + + @Test + public void testRenewLeaseAbortsWhenHolderIdMismatch() throws Exception { + Path dir = storagePath.resolve("successor-renew-test-" + UUID.randomUUID() + ".lock"); + Files.createDirectories(dir); + + // Simulate successor node took over with a new holderId + long successorExpiry = System.currentTimeMillis() + 60_000L; + LeaseData successorLease = + new LeaseData( + "successor-holder", + "/files/upload/test", + 30_000L, + successorExpiry, + System.currentTimeMillis(), + dir.toString(), + null); + LeaseDataJsonSerializer.serializeToPath(successorLease, dir.resolve("lease.json")); + + // Stale original lock holder calls renewLease() + LeaseData originalLease = + new LeaseData( + "original-holder", + "/files/upload/test", + 30_000L, + System.currentTimeMillis() + 10_000L, + System.currentTimeMillis(), + dir.toString(), + null); + LeaseFileUploadLock originalLock = new LeaseFileUploadLock(originalLease, dir, null, null); + + originalLock.renewLease(); + + // Successor lease must remain untouched with successor's holderId + LeaseData currentLease = LeaseDataJsonSerializer.deserialize(dir.resolve("lease.json")); + assertNotNull(currentLease); + assertEquals("successor-holder", currentLease.getHolderId()); + assertEquals(successorExpiry, currentLease.getExpiresAt()); + + originalLock.close(); + FileUtils.deleteDirectory(dir.toFile()); + } } diff --git a/src/test/java/me/desair/tus/server/upload/s3/S3LockingServiceTest.java b/src/test/java/me/desair/tus/server/upload/s3/S3LockingServiceTest.java index 50f14d10..96a22098 100644 --- a/src/test/java/me/desair/tus/server/upload/s3/S3LockingServiceTest.java +++ b/src/test/java/me/desair/tus/server/upload/s3/S3LockingServiceTest.java @@ -25,6 +25,7 @@ import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import me.desair.tus.server.exception.UploadAlreadyLockedException; +import me.desair.tus.server.upload.LeaseData; import me.desair.tus.server.upload.UploadId; import me.desair.tus.server.upload.UploadLock; import me.desair.tus.server.util.InterruptibleInputStream; @@ -422,24 +423,25 @@ public void testCleanupStaleLocksWithExpiredAndNonExpiredLocks() throws Exceptio Mockito.when(mockClient.listObjects(Mockito.any(ListObjectsArgs.class))) .thenReturn(java.util.Arrays.asList(res1, res2)); - S3UploadLock expiredLock = - new S3UploadLock( + LeaseData expiredLock = + new LeaseData( "holder1", "/files/upload/1", - "test-bucket", - "locks/expired.lock", - "locks/expired.stop", 30000L, - 1000L); - S3UploadLock validLock = - new S3UploadLock( + 1000L, + 1000L, + "locks/expired.lock", + "locks/expired.stop"); + + LeaseData validLock = + new LeaseData( "holder2", "/files/upload/2", - "test-bucket", - "locks/valid.lock", - "locks/valid.stop", 30000L, - System.currentTimeMillis() + 1000000L); + System.currentTimeMillis() + 1000000L, + System.currentTimeMillis(), + "locks/valid.lock", + "locks/valid.stop"); GetObjectResponse expiredStream = new GetObjectResponse( @@ -448,7 +450,7 @@ public void testCleanupStaleLocksWithExpiredAndNonExpiredLocks() throws Exceptio "us-east-1", "locks/expired.lock", new ByteArrayInputStream( - me.desair.tus.server.util.S3UploadLockJsonSerializer.serialize(expiredLock) + me.desair.tus.server.util.LeaseDataJsonSerializer.serialize(expiredLock) .getBytes(StandardCharsets.UTF_8))); GetObjectResponse validStream = new GetObjectResponse( @@ -457,7 +459,7 @@ public void testCleanupStaleLocksWithExpiredAndNonExpiredLocks() throws Exceptio "us-east-1", "locks/valid.lock", new ByteArrayInputStream( - me.desair.tus.server.util.S3UploadLockJsonSerializer.serialize(validLock) + me.desair.tus.server.util.LeaseDataJsonSerializer.serialize(validLock) .getBytes(StandardCharsets.UTF_8))); Mockito.when(mockClient.getObject(Mockito.any(GetObjectArgs.class))) @@ -536,21 +538,21 @@ public void testConditionalWriteConflictThrowsUploadAlreadyLockedException() thr } @Test(expected = UploadAlreadyLockedException.class) - public void testReadAfterWriteVerificationFailsWhenHolderIdMismatch() throws Exception { + public void testLockUploadByUriWithContentionOnPostPutVerificationThrows() throws Exception { // Simulate a TOCTOU race where PutObject succeeds, but another contender overwrote the lock // before our read-after-write verification Mockito.when(minioClient.putObject(Mockito.any(PutObjectArgs.class))).thenReturn(null); - S3UploadLock rivalLock = - new S3UploadLock( + LeaseData rivalLock = + new LeaseData( "rival-holder", "/files/upload/24249a5b-01a4-4bf8-b67a-364273bb5a2e", - "test-bucket", - "locks/24249a5b-01a4-4bf8-b67a-364273bb5a2e.lock", - "locks/24249a5b-01a4-4bf8-b67a-364273bb5a2e.stop", 30000L, - System.currentTimeMillis() + 30000L); - String rivalJson = me.desair.tus.server.util.S3UploadLockJsonSerializer.serialize(rivalLock); + System.currentTimeMillis() + 30000L, + System.currentTimeMillis(), + "locks/24249a5b-01a4-4bf8-b67a-364273bb5a2e.lock", + "locks/24249a5b-01a4-4bf8-b67a-364273bb5a2e.stop"); + String rivalJson = me.desair.tus.server.util.LeaseDataJsonSerializer.serialize(rivalLock); // Initial check sees expired/missing, but post-put verification sees rival holder java.util.concurrent.atomic.AtomicInteger getCallCount = @@ -562,18 +564,20 @@ public void testReadAfterWriteVerificationFailsWhenHolderIdMismatch() throws Exc // First call: check if expired (missing -> not locked) ErrorResponse errorResponse = Mockito.mock(ErrorResponse.class); Mockito.when(errorResponse.code()).thenReturn("NoSuchKey"); - throw new ErrorResponseException(errorResponse, null, null); + throw new io.minio.errors.ErrorResponseException(errorResponse, null, null); } - // Second call: read-after-write verification returns rival's lock + // Second call: read-after-write verification sees rivalLock return new GetObjectResponse( null, "test-bucket", "us-east-1", - "locks/24249a5b.lock", + "locks/24249a5b-01a4-4bf8-b67a-364273bb5a2e.lock", new ByteArrayInputStream(rivalJson.getBytes(StandardCharsets.UTF_8))); }); - lockingService.lockUploadByUri("/files/upload/24249a5b-01a4-4bf8-b67a-364273bb5a2e"); + UploadLock lock = + lockingService.lockUploadByUri("/files/upload/24249a5b-01a4-4bf8-b67a-364273bb5a2e"); + assertNull(lock); } @Test @@ -639,17 +643,17 @@ public void testConcurrentEvictionContentionS3() throws Exception { // Seed an expired lock in simulated S3 storage String lockKey = "locks/" + uploadIdStr + ".lock"; - S3UploadLock expiredLock = - new S3UploadLock( + LeaseData expiredLock = + new LeaseData( "expired-holder", uri, - "test-bucket", - lockKey, - "locks/" + uploadIdStr + ".stop", 30000L, - System.currentTimeMillis() - 10_000L); + System.currentTimeMillis() - 10_000L, + System.currentTimeMillis() - 40_000L, + lockKey, + "locks/" + uploadIdStr + ".stop"); byte[] expiredBytes = - me.desair.tus.server.util.S3UploadLockJsonSerializer.serializeToBytes(expiredLock); + me.desair.tus.server.util.LeaseDataJsonSerializer.serializeToBytes(expiredLock); s3StorageMap.put(lockKey, expiredBytes); int threadCount = 10; diff --git a/src/test/java/me/desair/tus/server/upload/s3/S3UploadLockTest.java b/src/test/java/me/desair/tus/server/upload/s3/S3UploadLockTest.java index f00655a8..d80f4e85 100644 --- a/src/test/java/me/desair/tus/server/upload/s3/S3UploadLockTest.java +++ b/src/test/java/me/desair/tus/server/upload/s3/S3UploadLockTest.java @@ -11,6 +11,8 @@ import java.io.InputStream; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import me.desair.tus.server.upload.LeaseData; +import me.desair.tus.server.util.LeaseDataJsonSerializer; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; @@ -26,24 +28,30 @@ public void setUp() { inputStreamMap = new ConcurrentHashMap<>(); } + private LeaseData createLeaseData(String holderId, String requestUri) { + return new LeaseData(holderId, requestUri, 60000L, System.currentTimeMillis() + 60000L); + } + @Test public void testLockGettersReleaseAndRenewLease() throws Exception { InputStream mockStream = mock(InputStream.class); inputStreamMap.put("/files/upload-1", mockStream); + LeaseData leaseData = createLeaseData("holder-123", "/files/upload-1"); S3UploadLock lock = new S3UploadLock( + leaseData, minioClient, "test-bucket", "tus-locks/upload-1.lock", "tus-locks/upload-1.stop", - "holder-123", - 60000L, - "/files/upload-1", inputStreamMap); assertEquals("holder-123", lock.getHolderId()); assertEquals("/files/upload-1", lock.getUploadUri()); + assertEquals("test-bucket", lock.getBucket()); + assertEquals("tus-locks/upload-1.lock", lock.getLockKey()); + assertEquals("tus-locks/upload-1.stop", lock.getStopKey()); // Explicitly call renewLease() to verify lease renewal lock.renewLease(); @@ -58,18 +66,18 @@ public void testRenewLeaseExceptionHandling() throws Exception { .when(minioClient) .putObject(any(PutObjectArgs.class)); + LeaseData leaseData = createLeaseData("holder-123", "/files/upload-1"); S3UploadLock lock = new S3UploadLock( + leaseData, minioClient, "test-bucket", "tus-locks/upload-1.lock", "tus-locks/upload-1.stop", - "holder-123", - 60000L, - "/files/upload-1", inputStreamMap); lock.renewLease(); + assertEquals("holder-123", lock.getHolderId()); } @Test @@ -78,46 +86,38 @@ public void testLockDeleteQuietlyWithNullKeysAndExceptionHandling() throws Excep .when(minioClient) .removeObject(any(RemoveObjectArgs.class)); + LeaseData leaseData = createLeaseData("holder-123", "/files/upload-1"); S3UploadLock lockWithNullKeys = - new S3UploadLock( - minioClient, - "test-bucket", - null, - null, - "holder-123", - 60000L, - "/files/upload-1", - inputStreamMap); + new S3UploadLock(leaseData, minioClient, "test-bucket", null, null, inputStreamMap); lockWithNullKeys.close(); S3UploadLock lockWithKeys = new S3UploadLock( + leaseData, minioClient, "test-bucket", "tus-locks/upload-1.lock", "tus-locks/upload-1.stop", - "holder-123", - 60000L, - "/files/upload-1", inputStreamMap); lockWithKeys.close(); + assertEquals("holder-123", lockWithKeys.getHolderId()); } @Test public void testDeleteS3LockObjectIfOwnerSkipsWhenHolderMismatch() throws Exception { // Simulate remote lock owned by another holder - S3UploadLock otherLock = - new S3UploadLock( + LeaseData otherLock = + new LeaseData( "other-holder", "/files/upload-1", - "test-bucket", - "tus-locks/upload-1.lock", - "tus-locks/upload-1.stop", 60000L, - System.currentTimeMillis() + 60000L); - String json = me.desair.tus.server.util.S3UploadLockJsonSerializer.serialize(otherLock); + System.currentTimeMillis() + 60000L, + System.currentTimeMillis(), + "tus-locks/upload-1.lock", + "tus-locks/upload-1.stop"); + String json = LeaseDataJsonSerializer.serialize(otherLock); io.minio.GetObjectResponse response = new io.minio.GetObjectResponse( @@ -130,15 +130,14 @@ public void testDeleteS3LockObjectIfOwnerSkipsWhenHolderMismatch() throws Except Mockito.when(minioClient.getObject(any(io.minio.GetObjectArgs.class))).thenReturn(response); + LeaseData myLeaseData = createLeaseData("my-holder", "/files/upload-1"); S3UploadLock lock = new S3UploadLock( + myLeaseData, minioClient, "test-bucket", "tus-locks/upload-1.lock", "tus-locks/upload-1.stop", - "my-holder", - 60000L, - "/files/upload-1", inputStreamMap); lock.deleteS3LockObjectIfOwner("tus-locks/upload-1.lock"); @@ -153,16 +152,16 @@ public void testDeleteS3LockObjectIfOwnerSkipsWhenHolderMismatch() throws Except @Test public void testDeleteS3LockObjectIfOwnerDeletesWhenHolderMatches() throws Exception { // Simulate remote lock owned by this holder - S3UploadLock myLock = - new S3UploadLock( + LeaseData myLock = + new LeaseData( "my-holder", "/files/upload-1", - "test-bucket", - "tus-locks/upload-1.lock", - "tus-locks/upload-1.stop", 60000L, - System.currentTimeMillis() + 60000L); - String json = me.desair.tus.server.util.S3UploadLockJsonSerializer.serialize(myLock); + System.currentTimeMillis() + 60000L, + System.currentTimeMillis(), + "tus-locks/upload-1.lock", + "tus-locks/upload-1.stop"); + String json = LeaseDataJsonSerializer.serialize(myLock); io.minio.GetObjectResponse response = new io.minio.GetObjectResponse( @@ -175,15 +174,14 @@ public void testDeleteS3LockObjectIfOwnerDeletesWhenHolderMatches() throws Excep Mockito.when(minioClient.getObject(any(io.minio.GetObjectArgs.class))).thenReturn(response); + LeaseData myLeaseData = createLeaseData("my-holder", "/files/upload-1"); S3UploadLock lock = new S3UploadLock( + myLeaseData, minioClient, "test-bucket", "tus-locks/upload-1.lock", "tus-locks/upload-1.stop", - "my-holder", - 60000L, - "/files/upload-1", inputStreamMap); lock.deleteS3LockObjectIfOwner("tus-locks/upload-1.lock"); @@ -205,18 +203,18 @@ public void testDeleteS3LockObjectIfOwnerHandlesNoSuchKey() throws Exception { Mockito.when(minioClient.getObject(any(io.minio.GetObjectArgs.class))).thenThrow(ex); + LeaseData myLeaseData = createLeaseData("my-holder", "/files/upload-1"); S3UploadLock lock = new S3UploadLock( + myLeaseData, minioClient, "test-bucket", "tus-locks/upload-1.lock", "tus-locks/upload-1.stop", - "my-holder", - 60000L, - "/files/upload-1", inputStreamMap); lock.deleteS3LockObjectIfOwner("tus-locks/upload-1.lock"); + assertEquals("my-holder", lock.getHolderId()); } @Test @@ -225,32 +223,31 @@ public void testCloseHeartbeatExecutorShutdownException() throws Exception { mock(java.util.concurrent.ScheduledExecutorService.class); Mockito.doThrow(new RuntimeException("Shutdown error")).when(mockExecutor).shutdownNow(); + LeaseData myLeaseData = createLeaseData("holder-123", "/files/upload-1"); S3UploadLock lock = new S3UploadLock( + myLeaseData, minioClient, "test-bucket", "tus-locks/upload-1.lock", "tus-locks/upload-1.stop", - "holder-123", - 60000L, - "/files/upload-1", inputStreamMap, mockExecutor); lock.close(); + assertEquals("holder-123", lock.getHolderId()); } @Test public void testDeleteS3LockObjectIfOwnerNullChecksAndExceptionHandling() throws Exception { + LeaseData myLeaseData = createLeaseData("holder-123", "/files/upload-1"); S3UploadLock lock = new S3UploadLock( + myLeaseData, minioClient, "test-bucket", "tus-locks/upload-1.lock", "tus-locks/upload-1.stop", - "holder-123", - 60000L, - "/files/upload-1", inputStreamMap); // Null key check @@ -261,5 +258,88 @@ public void testDeleteS3LockObjectIfOwnerNullChecksAndExceptionHandling() throws .when(minioClient) .removeObject(any(RemoveObjectArgs.class)); lock.deleteS3LockObjectIfOwner("tus-locks/upload-1.lock"); + assertEquals("holder-123", lock.getHolderId()); + } + + @Test + public void testRenewLeaseSkipsWhenHolderMismatch() throws Exception { + LeaseData otherLock = + new LeaseData( + "other-holder", + "/files/upload-1", + 60000L, + System.currentTimeMillis() + 60000L, + System.currentTimeMillis(), + "tus-locks/upload-1.lock", + "tus-locks/upload-1.stop"); + String json = LeaseDataJsonSerializer.serialize(otherLock); + + io.minio.GetObjectResponse response = + new io.minio.GetObjectResponse( + null, + "test-bucket", + "us-east-1", + "tus-locks/upload-1.lock", + new java.io.ByteArrayInputStream( + json.getBytes(java.nio.charset.StandardCharsets.UTF_8))); + + Mockito.when(minioClient.getObject(any(io.minio.GetObjectArgs.class))).thenReturn(response); + + LeaseData myLeaseData = createLeaseData("my-holder", "/files/upload-1"); + S3UploadLock lock = + new S3UploadLock( + myLeaseData, + minioClient, + "test-bucket", + "tus-locks/upload-1.lock", + "tus-locks/upload-1.stop", + inputStreamMap); + + lock.renewLease(); + + // Must NOT call putObject because lock is now held by other-holder + Mockito.verify(minioClient, Mockito.never()).putObject(any(PutObjectArgs.class)); + lock.close(); + } + + @Test + public void testRenewLeaseSucceedsWhenHolderMatches() throws Exception { + LeaseData myLock = + new LeaseData( + "my-holder", + "/files/upload-1", + 60000L, + System.currentTimeMillis() + 60000L, + System.currentTimeMillis(), + "tus-locks/upload-1.lock", + "tus-locks/upload-1.stop"); + String json = LeaseDataJsonSerializer.serialize(myLock); + + io.minio.GetObjectResponse response = + new io.minio.GetObjectResponse( + null, + "test-bucket", + "us-east-1", + "tus-locks/upload-1.lock", + new java.io.ByteArrayInputStream( + json.getBytes(java.nio.charset.StandardCharsets.UTF_8))); + + Mockito.when(minioClient.getObject(any(io.minio.GetObjectArgs.class))).thenReturn(response); + + LeaseData myLeaseData = createLeaseData("my-holder", "/files/upload-1"); + S3UploadLock lock = + new S3UploadLock( + myLeaseData, + minioClient, + "test-bucket", + "tus-locks/upload-1.lock", + "tus-locks/upload-1.stop", + inputStreamMap); + + lock.renewLease(); + + // Must call putObject because holder matches + Mockito.verify(minioClient).putObject(any(PutObjectArgs.class)); + lock.close(); } } diff --git a/src/test/java/me/desair/tus/server/util/LeaseDataJsonSerializerTest.java b/src/test/java/me/desair/tus/server/util/LeaseDataJsonSerializerTest.java new file mode 100644 index 00000000..7bb12b43 --- /dev/null +++ b/src/test/java/me/desair/tus/server/util/LeaseDataJsonSerializerTest.java @@ -0,0 +1,164 @@ +package me.desair.tus.server.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import me.desair.tus.server.upload.LeaseData; +import org.apache.commons.io.FileUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** Unit tests for {@link LeaseDataJsonSerializer}. */ +public class LeaseDataJsonSerializerTest { + + private Path tempDir; + + @Before + public void setUp() throws Exception { + tempDir = Files.createTempDirectory("lease-serializer-test-"); + } + + @After + public void tearDown() throws Exception { + if (tempDir != null && Files.exists(tempDir)) { + FileUtils.deleteDirectory(tempDir.toFile()); + } + } + + @Test + public void testSerializeAndDeserializeString() throws Exception { + LeaseData data = + new LeaseData( + "holder-123", + "/files/upload/test-1", + 30000L, + 1700000030000L, + 1700000000000L, + "/path/to/lock", + "/path/to/stop"); + + String json = LeaseDataJsonSerializer.serialize(data); + assertNotNull(json); + + LeaseData deserialized = LeaseDataJsonSerializer.deserialize(json); + assertNotNull(deserialized); + assertEquals("holder-123", deserialized.getHolderId()); + assertEquals("/files/upload/test-1", deserialized.getRequestUri()); + assertEquals(30000L, deserialized.getLeaseDurationMs()); + assertEquals(1700000030000L, deserialized.getExpiresAt()); + assertEquals(1700000000000L, deserialized.getAcquiredAt()); + assertEquals("/path/to/lock", deserialized.getLockPath()); + assertEquals("/path/to/stop", deserialized.getStopPath()); + } + + @Test + public void testSerializeAndDeserializeBytesAndStreams() throws Exception { + LeaseData data = new LeaseData("holder-456", "/files/upload/test-2", 15000L, 200000L); + + byte[] bytes = LeaseDataJsonSerializer.serializeToBytes(data); + assertNotNull(bytes); + + LeaseData deserialized = LeaseDataJsonSerializer.deserialize(new ByteArrayInputStream(bytes)); + assertNotNull(deserialized); + assertEquals("holder-456", deserialized.getHolderId()); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + LeaseDataJsonSerializer.serializeToStream(data, baos); + LeaseData deserializedFromStream = + LeaseDataJsonSerializer.deserialize(new ByteArrayInputStream(baos.toByteArray())); + assertNotNull(deserializedFromStream); + assertEquals("holder-456", deserializedFromStream.getHolderId()); + } + + @Test + public void testSerializeAndDeserializeFilesAndPaths() throws Exception { + LeaseData data = new LeaseData("holder-file", "/files/upload/test-file", 10000L, 300000L); + Path filePath = tempDir.resolve("lease.json"); + File file = filePath.toFile(); + + LeaseDataJsonSerializer.serializeToPath(data, filePath); + LeaseData deserializedPath = LeaseDataJsonSerializer.deserialize(filePath); + assertNotNull(deserializedPath); + assertEquals("holder-file", deserializedPath.getHolderId()); + + Path relPath = Paths.get("target", "temp-rel-" + java.util.UUID.randomUUID() + ".json"); + try { + LeaseDataJsonSerializer.serializeToPath(data, relPath); + LeaseData deserializedRel = LeaseDataJsonSerializer.deserialize(relPath); + assertNotNull(deserializedRel); + assertEquals("holder-file", deserializedRel.getHolderId()); + } finally { + Files.deleteIfExists(relPath); + } + + LeaseDataJsonSerializer.serializeToFile(data, file); + LeaseData deserializedFile = LeaseDataJsonSerializer.deserialize(file); + assertNotNull(deserializedFile); + assertEquals("holder-file", deserializedFile.getHolderId()); + } + + @Test + public void testForwardCompatibilityUnknownProperties() throws Exception { + String jsonWithExtra = + "{\"holderId\":\"holder-extra\",\"futureProperty\":\"new-feature-value\"}"; + LeaseData deserialized = LeaseDataJsonSerializer.deserialize(jsonWithExtra); + assertNotNull(deserialized); + assertEquals("holder-extra", deserialized.getHolderId()); + } + + @Test + public void testNullAndEmptyInputs() throws Exception { + assertNull(LeaseDataJsonSerializer.serialize(null)); + assertNull(LeaseDataJsonSerializer.serializeToBytes(null)); + assertNull(LeaseDataJsonSerializer.deserialize((String) null)); + assertNull(LeaseDataJsonSerializer.deserialize((InputStream) null)); + assertNull(LeaseDataJsonSerializer.deserialize((File) null)); + assertNull(LeaseDataJsonSerializer.deserialize((Path) null)); + assertNull(LeaseDataJsonSerializer.deserialize("")); + assertNull(LeaseDataJsonSerializer.deserialize(" ")); + } + + @Test(expected = IOException.class) + public void testInvalidJsonThrowsIOException() throws Exception { + LeaseDataJsonSerializer.deserialize("invalid-json-{"); + } + + @Test + public void testEqualsAndHashCode() { + LeaseData d1 = new LeaseData("h1", "/u1", 1000L, 2000L, 500L, "/lock1", "/stop1"); + LeaseData d2 = new LeaseData("h1", "/u1", 1000L, 2000L, 500L, "/lock1", "/stop1"); + LeaseData d3 = new LeaseData("h2", "/u1", 1000L, 2000L, 500L, "/lock1", "/stop1"); + LeaseData d4 = new LeaseData("h1", "/u2", 1000L, 2000L, 500L, "/lock1", "/stop1"); + LeaseData d5 = new LeaseData("h1", "/u1", 2000L, 2000L, 500L, "/lock1", "/stop1"); + LeaseData d6 = new LeaseData("h1", "/u1", 1000L, 3000L, 500L, "/lock1", "/stop1"); + LeaseData d7 = new LeaseData("h1", "/u1", 1000L, 2000L, 600L, "/lock1", "/stop1"); + LeaseData d8 = new LeaseData("h1", "/u1", 1000L, 2000L, 500L, "/lock2", "/stop1"); + LeaseData d9 = new LeaseData("h1", "/u1", 1000L, 2000L, 500L, "/lock1", "/stop2"); + + assertTrue(d1.equals(d1)); + assertTrue(d1.equals(d2)); + assertEquals(d1.hashCode(), d2.hashCode()); + + assertFalse(d1.equals(null)); + assertFalse(d1.equals("not-a-lease-data")); + assertFalse(d1.equals(d3)); + assertFalse(d1.equals(d4)); + assertFalse(d1.equals(d5)); + assertFalse(d1.equals(d6)); + assertFalse(d1.equals(d7)); + assertFalse(d1.equals(d8)); + assertFalse(d1.equals(d9)); + } +} diff --git a/src/test/java/me/desair/tus/server/util/S3UploadLockJsonSerializerTest.java b/src/test/java/me/desair/tus/server/util/S3UploadLockJsonSerializerTest.java deleted file mode 100644 index 1b0fcb36..00000000 --- a/src/test/java/me/desair/tus/server/util/S3UploadLockJsonSerializerTest.java +++ /dev/null @@ -1,97 +0,0 @@ -package me.desair.tus.server.util; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.InputStream; -import me.desair.tus.server.upload.s3.S3UploadLock; -import org.junit.Test; - -public class S3UploadLockJsonSerializerTest { - - @Test - public void testSerializeAndDeserializeUploadLock() throws Exception { - long now = System.currentTimeMillis(); - long expiry = now + 30000L; - S3UploadLock lock = - new S3UploadLock( - "holder-uuid-1234", - "/files/test-upload-id", - "test-bucket", - "locks/test.lock", - "locks/test.stop", - 30000L, - expiry); - - // Test String serialization and deserialization - String json = S3UploadLockJsonSerializer.serialize(lock); - assertNotNull(json); - assertTrue(json.contains("\"bucket\":\"test-bucket\"")); - assertTrue(json.contains("\"requestUri\":\"/files/test-upload-id\"")); - assertTrue(json.contains("\"leaseDurationMs\":30000")); - - S3UploadLock deserialized = S3UploadLockJsonSerializer.deserialize(json); - assertNotNull(deserialized); - assertEquals("holder-uuid-1234", deserialized.getHolderId()); - assertEquals("/files/test-upload-id", deserialized.getRequestUri()); - assertEquals("/files/test-upload-id", deserialized.getUploadUri()); - assertEquals("test-bucket", deserialized.getBucket()); - assertEquals("locks/test.lock", deserialized.getLockKey()); - assertEquals("locks/test.stop", deserialized.getStopKey()); - assertEquals(30000L, deserialized.getLeaseDurationMs()); - assertEquals(expiry, deserialized.getExpiresAt()); - assertTrue(deserialized.getAcquiredAt() > 0); - - // Test byte array serialization - byte[] bytes = S3UploadLockJsonSerializer.serializeToBytes(lock); - assertNotNull(bytes); - - // Test InputStream deserialization - S3UploadLock fromStream = - S3UploadLockJsonSerializer.deserialize(new ByteArrayInputStream(bytes)); - assertNotNull(fromStream); - assertEquals("holder-uuid-1234", fromStream.getHolderId()); - assertEquals("test-bucket", fromStream.getBucket()); - - // Test OutputStream serialization - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - S3UploadLockJsonSerializer.serializeToStream(lock, baos); - S3UploadLock fromStream2 = - S3UploadLockJsonSerializer.deserialize(new ByteArrayInputStream(baos.toByteArray())); - assertNotNull(fromStream2); - assertEquals("holder-uuid-1234", fromStream2.getHolderId()); - assertEquals("locks/test.lock", fromStream2.getLockKey()); - } - - @Test - public void testForwardCompatibilityUnknownProperties() throws Exception { - // Future-proofing: Extra unknown properties must be ignored without throwing an exception - String futureJson = - "{\"holderId\":\"future-holder\",\"requestUri\":\"/files/1\",\"bucket\":\"b\",\"lockKey\":\"l\",\"stopKey\":\"s\",\"leaseDurationMs\":30000,\"expiresAt\":1700000000000,\"newProperty\":\"someValue\",\"anotherField\":123}"; - S3UploadLock futureLock = S3UploadLockJsonSerializer.deserialize(futureJson); - assertNotNull(futureLock); - assertEquals("future-holder", futureLock.getHolderId()); - assertEquals("/files/1", futureLock.getRequestUri()); - assertEquals(1700000000000L, futureLock.getExpiresAt()); - } - - @Test - public void testNullAndEmptyHandling() throws Exception { - assertNull(S3UploadLockJsonSerializer.serialize(null)); - assertNull(S3UploadLockJsonSerializer.serializeToBytes(null)); - assertNull(S3UploadLockJsonSerializer.deserialize((String) null)); - assertNull(S3UploadLockJsonSerializer.deserialize((InputStream) null)); - assertNull(S3UploadLockJsonSerializer.deserialize("")); - assertNull(S3UploadLockJsonSerializer.deserialize(" ")); - - try { - S3UploadLockJsonSerializer.deserialize("invalid-json"); - } catch (Exception expected) { - // expected - } - } -} diff --git a/src/test/java/me/desair/tus/server/util/UploadInfoJsonSerializerTest.java b/src/test/java/me/desair/tus/server/util/UploadInfoJsonSerializerTest.java index 9a8a5d33..e616441b 100644 --- a/src/test/java/me/desair/tus/server/util/UploadInfoJsonSerializerTest.java +++ b/src/test/java/me/desair/tus/server/util/UploadInfoJsonSerializerTest.java @@ -6,16 +6,36 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; import java.io.InputStream; -import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import me.desair.tus.server.upload.UploadId; import me.desair.tus.server.upload.UploadInfo; +import org.apache.commons.io.FileUtils; +import org.junit.After; +import org.junit.Before; import org.junit.Test; public class UploadInfoJsonSerializerTest { + private Path tempDir; + + @Before + public void setUp() throws Exception { + tempDir = Files.createTempDirectory("upload-info-serializer-test-"); + } + + @After + public void tearDown() throws Exception { + if (tempDir != null && Files.exists(tempDir)) { + FileUtils.deleteDirectory(tempDir.toFile()); + } + } + @Test - public void testSerializeAndDeserializeUploadInfo() throws Exception { + public void testSerializeAndDeserializeUploadInfoString() throws Exception { UploadInfo info = new UploadInfo(); info.setId(new UploadId("24249a5b-01a4-4bf8-b67a-364273bb5a2e")); info.setLength(1024L); @@ -33,38 +53,168 @@ public void testSerializeAndDeserializeUploadInfo() throws Exception { assertEquals(Long.valueOf(512L), deserialized.getOffset()); assertEquals("owner-1", deserialized.getOwnerKey()); assertEquals("custom-storage-id", deserialized.getStorageUploadId()); + } - // Test InputStream overload - UploadInfo fromStream = - UploadInfoJsonSerializer.deserialize( - new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8))); - assertNotNull(fromStream); - assertEquals("24249a5b-01a4-4bf8-b67a-364273bb5a2e", fromStream.getId().toString()); + @Test + public void testSerializeAndDeserializeBytesAndStreams() throws Exception { + UploadInfo info = new UploadInfo(); + info.setId(new UploadId("test-id-123")); + info.setLength(2048L); + + byte[] bytes = UploadInfoJsonSerializer.serializeToBytes(info); + assertNotNull(bytes); + + UploadInfo deserializedBytes = UploadInfoJsonSerializer.deserialize(bytes); + assertNotNull(deserializedBytes); + assertEquals("test-id-123", deserializedBytes.getId().toString()); + + UploadInfo deserializedBytesGeneric = + UploadInfoJsonSerializer.deserialize(bytes, UploadInfo.class); + assertNotNull(deserializedBytesGeneric); + assertEquals("test-id-123", deserializedBytesGeneric.getId().toString()); - // Test OutputStream overload ByteArrayOutputStream baos = new ByteArrayOutputStream(); UploadInfoJsonSerializer.serializeToStream(info, baos); - UploadInfo fromStream2 = + UploadInfo fromStream = UploadInfoJsonSerializer.deserialize(new ByteArrayInputStream(baos.toByteArray())); - assertNotNull(fromStream2); - assertEquals("24249a5b-01a4-4bf8-b67a-364273bb5a2e", fromStream2.getId().toString()); + assertNotNull(fromStream); + assertEquals("test-id-123", fromStream.getId().toString()); + + UploadInfo fromStreamGeneric = + UploadInfoJsonSerializer.deserialize( + new ByteArrayInputStream(baos.toByteArray()), UploadInfo.class); + assertNotNull(fromStreamGeneric); + assertEquals("test-id-123", fromStreamGeneric.getId().toString()); + } + + @Test + public void testSerializeAndDeserializeFilesAndPaths() throws Exception { + UploadInfo info = new UploadInfo(); + info.setId(new UploadId("file-path-id")); + info.setLength(4096L); + + Path filePath = tempDir.resolve("upload.info"); + File file = filePath.toFile(); + + UploadInfoJsonSerializer.serializeToPath(info, filePath); + UploadInfo deserializedPath = UploadInfoJsonSerializer.deserialize(filePath); + assertNotNull(deserializedPath); + assertEquals("file-path-id", deserializedPath.getId().toString()); + + Path relPath = + java.nio.file.Paths.get("target", "temp-info-rel-" + java.util.UUID.randomUUID() + ".json"); + try { + UploadInfoJsonSerializer.serializeToPath(info, relPath); + UploadInfo deserializedRel = UploadInfoJsonSerializer.deserialize(relPath); + assertNotNull(deserializedRel); + assertEquals("file-path-id", deserializedRel.getId().toString()); + } finally { + Files.deleteIfExists(relPath); + } + + UploadInfo deserializedPathGeneric = + UploadInfoJsonSerializer.deserialize(filePath, UploadInfo.class); + assertNotNull(deserializedPathGeneric); + assertEquals("file-path-id", deserializedPathGeneric.getId().toString()); + + UploadInfoJsonSerializer.serializeToFile(info, file); + UploadInfo deserializedFile = UploadInfoJsonSerializer.deserialize(file); + assertNotNull(deserializedFile); + assertEquals("file-path-id", deserializedFile.getId().toString()); + + UploadInfo deserializedFileGeneric = + UploadInfoJsonSerializer.deserialize(file, UploadInfo.class); + assertNotNull(deserializedFileGeneric); + assertEquals("file-path-id", deserializedFileGeneric.getId().toString()); + } + + @Test + public void testGenericClassDeserialization() throws Exception { + TestModel model = new TestModel("custom-name", 42); + String json = UploadInfoJsonSerializer.serialize(model); + assertNotNull(json); + + TestModel deserialized = UploadInfoJsonSerializer.deserialize(json, TestModel.class); + assertNotNull(deserialized); + assertEquals("custom-name", deserialized.getName()); + assertEquals(42, deserialized.getValue()); + } + + @Test + public void testForwardCompatibilityUnknownProperties() throws Exception { + String jsonWithExtra = "{\"id\":\"known-id\",\"futureField\":\"new-feature-value\"}"; + UploadInfo deserialized = UploadInfoJsonSerializer.deserialize(jsonWithExtra); + assertNotNull(deserialized); + assertEquals("known-id", deserialized.getId().toString()); } @Test public void testNullAndEmptyHandling() throws Exception { assertNull(UploadInfoJsonSerializer.serialize(null)); + assertNull(UploadInfoJsonSerializer.serializeToBytes(null)); + UploadInfoJsonSerializer.serializeToStream(null, null); + UploadInfoJsonSerializer.serializeToFile(null, null); + UploadInfoJsonSerializer.serializeToPath(null, null); + assertNull(UploadInfoJsonSerializer.deserialize((String) null)); - assertNull(UploadInfoJsonSerializer.deserialize((InputStream) null)); + assertNull(UploadInfoJsonSerializer.deserialize((String) null, UploadInfo.class)); assertNull(UploadInfoJsonSerializer.deserialize("")); + assertNull(UploadInfoJsonSerializer.deserialize(" ")); + assertNull(UploadInfoJsonSerializer.deserialize(" ", UploadInfo.class)); + assertNull(UploadInfoJsonSerializer.deserialize((byte[]) null)); + assertNull(UploadInfoJsonSerializer.deserialize(new byte[0])); + assertNull(UploadInfoJsonSerializer.deserialize((byte[]) null, UploadInfo.class)); + assertNull(UploadInfoJsonSerializer.deserialize((InputStream) null)); + assertNull(UploadInfoJsonSerializer.deserialize((InputStream) null, UploadInfo.class)); + assertNull(UploadInfoJsonSerializer.deserialize((File) null)); + assertNull(UploadInfoJsonSerializer.deserialize((File) null, UploadInfo.class)); + assertNull(UploadInfoJsonSerializer.deserialize((Path) null)); + assertNull(UploadInfoJsonSerializer.deserialize((Path) null, UploadInfo.class)); + + Path missingPath = tempDir.resolve("missing.info"); + assertNull(UploadInfoJsonSerializer.deserialize(missingPath)); + assertNull(UploadInfoJsonSerializer.deserialize(missingPath.toFile())); + + Path emptyPath = tempDir.resolve("empty.info"); + Files.createFile(emptyPath); + assertNull(UploadInfoJsonSerializer.deserialize(emptyPath)); + assertNull(UploadInfoJsonSerializer.deserialize(emptyPath.toFile())); UploadInfo emptyIdInfo = UploadInfoJsonSerializer.deserialize("{\"id\":\"\"}"); assertNotNull(emptyIdInfo); assertNull(emptyIdInfo.getId()); + } - try { - UploadInfoJsonSerializer.deserialize("invalid-json"); - } catch (Exception expected) { - // expected + @Test(expected = IOException.class) + public void testInvalidJsonThrowsIOException() throws Exception { + UploadInfoJsonSerializer.deserialize("invalid-json-{"); + } + + public static class TestModel { + private String name; + private int value; + + public TestModel() {} + + public TestModel(String name, int value) { + this.name = name; + this.value = value; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public int getValue() { + return value; + } + + public void setValue(int value) { + this.value = value; } } } diff --git a/src/test/java/me/desair/tus/server/util/UtilsTest.java b/src/test/java/me/desair/tus/server/util/UtilsTest.java index e8f96e1b..ec0aaf18 100644 --- a/src/test/java/me/desair/tus/server/util/UtilsTest.java +++ b/src/test/java/me/desair/tus/server/util/UtilsTest.java @@ -106,91 +106,13 @@ public void readSerializableWithNullPath() throws Exception { // Should return null when path is null TestSerializable result = Utils.readSerializable(null, TestSerializable.class); assertThat(result, is(nullValue())); - - TestSerializable resultNonLocking = Utils.readSerializable(null, TestSerializable.class, false); - assertThat(resultNonLocking, is(nullValue())); - } - - @Test - public void writeAndReadSerializableNonLocking() throws Exception { - Path testFile = storagePath.resolve("nonlocking-serializable-" + UUID.randomUUID()); - TestSerializable original = new TestSerializable("nonlocking-value"); - - Utils.writeSerializable(original, testFile, false); - TestSerializable result = Utils.readSerializable(testFile, TestSerializable.class, false); - - assertThat(result, is(notNullValue())); - assertThat(result.getValue(), is("nonlocking-value")); - - Files.deleteIfExists(testFile); } @Test public void writeSerializableWithNullPathShouldBeNoOp() throws Exception { // Should safely do nothing without throwing exception Utils.writeSerializable(new TestSerializable("test"), null); - Utils.writeSerializable(new TestSerializable("test"), null, false); - } - - @Test - public void writeAndReadJsonWithValidObject() throws Exception { - Path testFile = storagePath.resolve("valid-json-" + UUID.randomUUID()); - TestJsonObject original = new TestJsonObject("hello", 42); - - Utils.writeJson(original, testFile); - TestJsonObject result = Utils.readJson(testFile, TestJsonObject.class); - - assertThat(result, is(notNullValue())); - assertThat(result.getName(), is("hello")); - assertThat(result.getCount(), is(42)); - - Files.deleteIfExists(testFile); - } - - @Test - public void writeAndReadJsonNonLocking() throws Exception { - Path testFile = storagePath.resolve("nonlocking-json-" + UUID.randomUUID()); - TestJsonObject original = new TestJsonObject("nonlocking-json", 100); - - Utils.writeJson(original, testFile, false); - TestJsonObject result = Utils.readJson(testFile, TestJsonObject.class, false); - - assertThat(result, is(notNullValue())); - assertThat(result.getName(), is("nonlocking-json")); - assertThat(result.getCount(), is(100)); - - Files.deleteIfExists(testFile); - } - - @Test - public void readJsonWithNullOrNonExistentPath() throws Exception { - assertThat(Utils.readJson(null, TestJsonObject.class), is(nullValue())); - assertThat(Utils.readJson(null, TestJsonObject.class, false), is(nullValue())); - - Path nonExistent = storagePath.resolve("missing-json-" + UUID.randomUUID()); - assertThat(Utils.readJson(nonExistent, TestJsonObject.class), is(nullValue())); - assertThat(Utils.readJson(nonExistent, TestJsonObject.class, false), is(nullValue())); - } - - @Test - public void writeJsonWithNullPathShouldBeNoOp() throws Exception { - // Should safely do nothing without throwing exception - Utils.writeJson(new TestJsonObject("test", 1), null); - Utils.writeJson(new TestJsonObject("test", 1), null, false); - } - - @Test - public void readJsonWithCorruptedFileShouldReturnNull() throws Exception { - Path corruptedFile = storagePath.resolve("corrupted-json-" + UUID.randomUUID()); - Files.write(corruptedFile, "invalid-json-content-{{{{".getBytes()); - - TestJsonObject resultLocking = Utils.readJson(corruptedFile, TestJsonObject.class, true); - assertThat(resultLocking, is(nullValue())); - - TestJsonObject resultNonLocking = Utils.readJson(corruptedFile, TestJsonObject.class, false); - assertThat(resultNonLocking, is(nullValue())); - - Files.deleteIfExists(corruptedFile); + assertThat(Utils.readSerializable(null, TestSerializable.class), is(nullValue())); } @Test @@ -249,6 +171,17 @@ public void testWriteAndReadSerializable() throws Exception { } finally { Files.deleteIfExists(tempFile); } + + Path relPath = Paths.get("target", "temp-serializable-rel-" + UUID.randomUUID() + ".bin"); + try { + String expected = "Tus Test Relative Object"; + Utils.writeSerializable(expected, relPath); + + String actual = Utils.readSerializable(relPath, String.class); + assertThat(actual, is(expected)); + } finally { + Files.deleteIfExists(relPath); + } } @Test @@ -260,6 +193,75 @@ public void testReadSerializableNullPath() throws Exception { public void testWriteSerializableNullPath() throws Exception { // Should do nothing without exception Utils.writeSerializable("test", null); + Utils.writeSerializable(null, Paths.get("target", "ignored.bin")); + } + + @Test + public void testCreateTempSiblingPath() { + assertThat(Utils.createTempSiblingPath(null), is(nullValue())); + + Path absPath = Paths.get("target", "test-dir", "data.json").toAbsolutePath(); + Path tempAbs = Utils.createTempSiblingPath(absPath); + assertThat(tempAbs, is(notNullValue())); + assertThat(tempAbs.getParent(), is(absPath.getParent())); + assertThat(tempAbs.getFileName().toString().startsWith("data.json.tmp."), is(true)); + + Path relPathNoParent = Paths.get("data.json"); + Path tempRelNoParent = Utils.createTempSiblingPath(relPathNoParent); + assertThat(tempRelNoParent, is(notNullValue())); + assertThat(tempRelNoParent.getFileName().toString().startsWith("data.json.tmp."), is(true)); + } + + @Test + public void testDeletePathQuietly() throws Exception { + assertThat(Utils.deletePathQuietly(null), is(false)); + assertThat(Utils.deletePathQuietly(Paths.get("non-existent-" + UUID.randomUUID())), is(false)); + + Path tempFile = Files.createTempFile("tus-delete-quietly", ".tmp"); + assertThat(Utils.deletePathQuietly(tempFile), is(true)); + assertThat(Files.exists(tempFile), is(false)); + } + + @Test + public void testTempPathAutoCloseable() throws Exception { + Path targetFile = Paths.get("target", "temp-target-" + UUID.randomUUID() + ".json"); + Path tempFilePath; + + try (Utils.TempPath tempPath = Utils.createTempSibling(targetFile)) { + tempFilePath = tempPath.getPath(); + assertThat(tempFilePath, is(notNullValue())); + Files.write(tempFilePath, "temp data".getBytes()); + assertThat(Files.exists(tempFilePath), is(true)); + } + + // AutoCloseable should have automatically deleted the temp file + assertThat(Files.exists(tempFilePath), is(false)); + + // Null path handling + try (Utils.TempPath nullTempPath = new Utils.TempPath(null)) { + assertThat(nullTempPath.getPath(), is(nullValue())); + } + } + + @Test + public void testAtomicMove() throws Exception { + // Null inputs should not throw exception + Utils.atomicMove(null, Paths.get("dest")); + Utils.atomicMove(Paths.get("src"), null); + + Path src = Files.createTempFile("tus-atomic-src", ".tmp"); + Path dst = Files.createTempFile("tus-atomic-dst", ".tmp"); + try { + Files.write(src, "atomic test content".getBytes()); + Utils.atomicMove(src, dst); + + assertThat(Files.exists(src), is(false)); + assertThat(Files.exists(dst), is(true)); + assertThat(new String(Files.readAllBytes(dst)), is("atomic test content")); + } finally { + Files.deleteIfExists(src); + Files.deleteIfExists(dst); + } } @Test @@ -918,33 +920,4 @@ public String getValue() { return value; } } - - /** Simple JSON data class for testing. */ - public static class TestJsonObject { - private String name; - private int count; - - public TestJsonObject() {} - - public TestJsonObject(String name, int count) { - this.name = name; - this.count = count; - } - - public String getName() { - return name; - } - - public void setName(String name) { - this.name = name; - } - - public int getCount() { - return count; - } - - public void setCount(int count) { - this.count = count; - } - } }