Skip to content

Conversation

@zerox80
Copy link
Contributor

@zerox80 zerox80 commented Nov 26, 2025

Optimize TUS upload: remove redundant HEAD request

Description

This PR optimizes the TUS upload flow by removing a redundant HEAD request that was previously performed immediately after creating a new upload.

Context

The TUS protocol (v1.0.0) specifies that the POST request used to create an upload resource may return the Upload-Offset header (usually 0 for new uploads). Previously, our client ignored this header and always performed a subsequent HEAD request to fetch the offset before starting the upload.

Changes

  • Updated CreateTusUploadRemoteOperation: Now parses the Upload-Offset header from the creation response and returns it along with the upload URL.
  • Updated TusUploadHelper: Uses the offset returned from the creation step directly. The separate HEAD request is now only performed if we are resuming an existing upload (where we don't have a fresh creation response) or if the creation response didn't provide an offset.
  • Updated Tests: Adjusted TusIntegrationTest to verify the new return type and behavior.

Benefits

  • Reduced Latency: Saves one network round-trip (RTT) for every new file upload.
  • Server Load: Reduces the number of requests the server needs to handle.

Code Explanation

CreateTusUploadRemoteOperation.kt

// Changed the return type from String (just the URL) to CreationResult (URL + Offset)
class CreateTusUploadRemoteOperation(
    ...
) : RemoteOperation<CreateTusUploadRemoteOperation.CreationResult>() {

    // Data class to hold both the Location (URL) and the Upload-Offset
    data class CreationResult(
        val uploadUrl: String,
        val uploadOffset: Long
    )

    // ...

    override fun run(client: OpenCloudClient): RemoteOperationResult<CreationResult> = try {
        // ... (request setup) ...

        val status = postMethod.execute(client)
        if (status == HttpStatus.SC_CREATED) {
            // ... (location parsing) ...

            // [NEW] Parse the Upload-Offset header from the response. Default to 0 if missing/invalid.
            val offsetHeader = postMethod.getResponseHeader(HttpConstants.UPLOAD_OFFSET)
            val offset = offsetHeader?.toLongOrNull() ?: 0L

            if (resolved != null) {
                // Return both the resolved URL and the offset
                RemoteOperationResult<CreationResult>(ResultCode.OK).apply {
                    data = CreationResult(resolved, offset)
                }
            } else {
                // ... (error handling) ...
            }
        } else {
            // ... (error handling) ...
        }
    } catch (e: Exception) {
        // ...
    }
}

TusUploadHelper.kt

class TusUploadHelper(...) {
    // ...
    
    // Variable to store the offset obtained during creation
    var createdOffset: Long? = null

    // ...

    // Inside the creation block:
    // We now receive 'creationResult' instead of just 'createdLocation'
    val creationResult = executeRemoteOperation {
        CreateTusUploadRemoteOperation(
            ...
        ).execute(client)
    }

    // Extract the URL and the Offset from the result
    tusUrl = creationResult?.uploadUrl
    createdOffset = creationResult?.uploadOffset

    // ...

    // When determining the start offset:
    // If we have a 'createdOffset' (from the POST response), use it directly.
    // Otherwise (e.g., resuming a previous attempt where we didn't just create it),
    // perform the HEAD request (GetTusUploadOffsetRemoteOperation).
    var offset = if (createdOffset != null) {
        createdOffset
    } else {
        try {
            executeRemoteOperation {
                GetTusUploadOffsetRemoteOperation(resolvedTusUrl).execute(client)
            }
        } catch (e: java.io.IOException) {
            // ...
        }
    }.coerceAtLeast(0L)
    
    // ...
}

TusIntegrationTest.kt

// Updated the test to handle the new CreationResult type
val creationResult = createResult.data
assertNotNull(creationResult)

// Verify we got the URL
val absoluteLocation = creationResult!!.uploadUrl
assertTrue(absoluteLocation.endsWith(locationPath))

// Verify we got the offset (should be 0 for a new upload)
val offset = creationResult.uploadOffset
assertEquals(0L, offset)

@zerox80 zerox80 closed this Nov 26, 2025
@zerox80 zerox80 deleted the tus-optimization branch November 26, 2025 18:47
@guruz
Copy link
Contributor

guruz commented Nov 28, 2025

Looks fine 👍 (yes i know it's part of the other PR now)

maybe later could add a separate test where there is an actual chunk uploaded on create so it's not assertEquals(0L, offset) but had the chunk uploaded

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants