Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions Sources/TUSKit/Extensions/HTTPURLResponse+Headers.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//
// HTTPURLResponse+Headers.swift
//
//
// Created by Sai Raghu Varma Kallepalli on 11/07/23.
//

import Foundation

extension HTTPURLResponse {
func extractHeaders() -> [String: String] {
var headers: [String: String] = [:]
allHeaderFields.forEach { key, value in
headers[String(describing: key)] = String(describing: value)
}
return headers
}
}
14 changes: 8 additions & 6 deletions Sources/TUSKit/TUSAPI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ struct Status {
let offset: Int
}

typealias UploadTaskResult = (Result<(Int, responseHeaders: [String : String]?), TUSAPIError>)

/// The Uploader's responsibility is to perform work related to uploading.
/// This includes: Making requests, handling requests, handling errors.
final class TUSAPI {
Expand Down Expand Up @@ -259,7 +261,7 @@ final class TUSAPI {
/// - location: The location of where to upload to.
/// - completion: Completionhandler for when the upload is finished.
@discardableResult
func upload(data: Data, range: Range<Int>?, location: URL, metaData: UploadMetadata, customHeaders: [String: String], completion: @escaping (Result<Int, TUSAPIError>) -> Void) -> URLSessionUploadTask {
func upload(data: Data, range: Range<Int>?, location: URL, metaData: UploadMetadata, customHeaders: [String: String], completion: @escaping (UploadTaskResult) -> Void) -> URLSessionUploadTask {
let offset: Int
let length: Int
if let range = range {
Expand Down Expand Up @@ -300,7 +302,7 @@ final class TUSAPI {
let offset = Int(offsetStr) else {
throw TUSAPIError.couldNotRetrieveOffset
}
return offset
return (offset, response.extractHeaders())
}
}
}
Expand All @@ -310,7 +312,7 @@ final class TUSAPI {
return task
}

func upload(fromFile file: URL, offset: Int = 0, location: URL, metaData: UploadMetadata, customHeaders: [String: String], completion: @escaping (Result<Int, TUSAPIError>) -> Void) -> URLSessionUploadTask {
func upload(fromFile file: URL, offset: Int = 0, location: URL, metaData: UploadMetadata, customHeaders: [String: String], completion: @escaping (UploadTaskResult) -> Void) -> URLSessionUploadTask {
let length: Int
if let fileAttributes = try? FileManager.default.attributesOfItem(atPath: file.path) {
if let bytes = fileAttributes[.size] as? Int64 {
Expand Down Expand Up @@ -346,15 +348,15 @@ final class TUSAPI {
let offset = Int(offsetStr) else {
throw TUSAPIError.couldNotRetrieveOffset
}
return offset
return (offset, response.extractHeaders())
}
}
}
task.resume()
return task
}

func registerCallback(_ completion: @escaping (Result<Int, TUSAPIError>) -> Void, forMetadata metadata: UploadMetadata) {
func registerCallback(_ completion: @escaping (UploadTaskResult) -> Void, forMetadata metadata: UploadMetadata) {
queue.sync {
self.callbacks[metadata.id.uuidString] = { result in
processResult(completion: completion) {
Expand All @@ -363,7 +365,7 @@ final class TUSAPI {
let offset = Int(offsetStr) else {
throw TUSAPIError.couldNotRetrieveOffset
}
return offset
return (offset, response.extractHeaders())
}
}
}
Expand Down
21 changes: 17 additions & 4 deletions Sources/TUSKit/TUSClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ public protocol TUSClientDelegate: AnyObject {
/// TUSClient is starting an upload
func didStartUpload(id: UUID, context: [String: String]?, client: TUSClient)
/// `TUSClient` just finished an upload, returns the URL of the uploaded file.
@available(*, deprecated, message: "Use the new didFinishUpload(id: UUID, url: URL, context: [String: String]?, client: TUSClient, responseHeaders: [String: String]?) method instead.")
func didFinishUpload(id: UUID, url: URL, context: [String: String]?, client: TUSClient)
/// `TUSClient` just finished an upload, returns the URL of the uploaded file along with `responseHeaders` from server.
func didFinishUpload(id: UUID, url: URL, context: [String: String]?, client: TUSClient, responseHeaders: [String: String]?)
/// An upload failed. Returns an error. Could either be a TUSClientError or a networking related error.
func uploadFailed(id: UUID, error: Error, context: [String: String]?, client: TUSClient)

Expand Down Expand Up @@ -50,6 +53,12 @@ public extension TUSClientDelegate {
func progressFor(id: UUID, context: [String: String]?, progress: Float, client: TUSClient) {
// Optional
}

/// `TUSClient` just finished an upload, returns the URL of the uploaded file along with `responseHeaders` from server.
func didFinishUpload(id: UUID, url: URL, context: [String: String]?, client: TUSClient, responseHeaders: [String: String]?) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry, I actually think this would still be breaking since if I conform to the delegate protocol I must implement all methods required by the protocol. This slipped my mind earlier.

Maybe we can provide a default empty implementation in a protocol extension for this method so that existing users of the SDK wouldn't have to do any work after updating?

I do wonder if we could mark a protocol method as deprecated. If we can, we could deprecate the old one and tell clients to implement the new method

// Supports calling the existing implementation without forcing conformance to the new delegate function.
didFinishUpload(id: id, url: url, context: context, client: client)
}

func fileError(id: UUID?, error: TUSClientError, client: TUSClient) {
fileError(error: error, client: client)
Expand Down Expand Up @@ -717,13 +726,17 @@ extension TUSClient: SchedulerDelegate {
assertionFailure("Somehow uploaded task did not have a url")
return
}


didFinishUpload(id: uploadTask.metaData.id, url: url, context: uploadTask.metaData.context, client: self, responseHeaders: uploadTask.metaData.responseHeaders)
}

private func didFinishUpload(id: UUID, url: URL, context: [String: String]?, client: TUSClient, responseHeaders: [String: String]?) {
queue.sync {
self.uploads[uploadTask.metaData.id] = nil
self.uploads[id] = nil
}
headerGenerator.clearHeaders(for: uploadTask.metaData.id)
headerGenerator.clearHeaders(for: id)
reportingQueue.async {
self.delegate?.didFinishUpload(id: uploadTask.metaData.id, url: url, context: uploadTask.metaData.context, client: self)
self.delegate?.didFinishUpload(id: id, url: url, context: context, client: self, responseHeaders: responseHeaders)
}
}

Expand Down
14 changes: 7 additions & 7 deletions Sources/TUSKit/Tasks/UploadDataTask.swift
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,12 @@ final class UploadDataTask: NSObject, IdentifiableTask {
}
}

func taskCompleted(result: Result<Int, TUSAPIError>, completed: @escaping TaskCompletion) {
func taskCompleted(result: UploadTaskResult, completed: @escaping TaskCompletion) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In the test app, the response headers never come through even when there are response headers passed to this method as part of the upload task result.

The headers are read from the upload metadata object but they are never assigned. The taskCompleted method seems like the most appropriate place to write the upload headers to the metadata object so they can be extracted later.

One thing to keep in mind here, is that taskCompleted can be called more than once when we're performing uploads in chunks. I think the last received set of headers is the one that folks would be most interested in so it might be okay to just overwrite on every call so that when we get around to informing the delegate about the finished upload, only the most recent headers are available?

I'd love for @LefterisHaritou to weigh in on this design decision because it's probably heavily use case dependent.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For us the usecase is that our api will provide us with the header once task is completed and not during chunk upload. So as you've correctly said, we are interested only on the last set of received headers

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That said, it doesn't seem to return the response headers when the upload finishes

do {
let receivedOffset = try result.get()
let receivedOffset = try result.get().0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This needs to change to:

let (receivedOffset, responseHeaders) = try result.get()
metaData.responseHeaders = responseHeaders

let currentOffset = metaData.uploadedRange?.upperBound ?? 0
metaData.uploadedRange = 0..<receivedOffset

let hasFinishedUploading = receivedOffset == metaData.size
if hasFinishedUploading {
try files.encodeAndStore(metaData: metaData)
Expand All @@ -144,23 +144,23 @@ final class UploadDataTask: NSObject, IdentifiableTask {
// assertionFailure("Server returned a new uploaded offset \(offset), but it's lower than what's already uploaded \(metaData.uploadedRange!), according to the metaData. Either the metaData is wrong, or the server is returning a wrong value offset.")
throw TUSClientError.receivedUnexpectedOffset
}

try files.encodeAndStore(metaData: metaData)

// If the task has been canceled
// we don't continue to create subsequent UploadDataTasks
if self.isCanceled {
throw TUSClientError.taskCancelled
}

let nextRange: Range<Int>?
if let range = range {
let chunkSize = range.count
nextRange = receivedOffset..<min((receivedOffset + chunkSize), metaData.size)
} else {
nextRange = nil
}

let task = try UploadDataTask(api: api, metaData: metaData, files: files, range: nextRange, headerGenerator: headerGenerator)
task.progressDelegate = progressDelegate
completed(.success([task]))
Expand Down
9 changes: 7 additions & 2 deletions Sources/TUSKit/UploadMetada.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ final class UploadMetadata: Codable {
case customHeaders
case size
case errorCount
case responseHeaders
case appliedCustomHeaders

}

var isFinished: Bool {
Expand All @@ -50,6 +50,8 @@ final class UploadMetadata: Codable {
}
}

var responseHeaders: [String: String]?

private var _remoteDestination: URL?
var remoteDestination: URL? {
get {
Expand Down Expand Up @@ -112,10 +114,11 @@ final class UploadMetadata: Codable {
}
}

init(id: UUID, filePath: URL, uploadURL: URL, size: Int, customHeaders: [String: String]? = nil, mimeType: String? = nil, context: [String: String]? = nil) {
init(id: UUID, filePath: URL, uploadURL: URL, responseHeaders: [String: String]? = nil, size: Int, customHeaders: [String: String]? = nil, mimeType: String? = nil, context: [String: String]? = nil) {
self.id = id
self._filePath = filePath
self.uploadURL = uploadURL
self.responseHeaders = responseHeaders
self.size = size
self._customHeaders = customHeaders
self.mimeType = mimeType
Expand All @@ -138,6 +141,7 @@ final class UploadMetadata: Codable {
_customHeaders = try values.decode([String: String]?.self, forKey: .customHeaders)
size = try values.decode(Int.self, forKey: .size)
_errorCount = try values.decode(Int.self, forKey: .errorCount)
responseHeaders = try values.decode([String: String].self, forKey: .responseHeaders)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should be
responseHeaders = try values.decodeIfPresent([String: String].self, forKey: .responseHeaders)

_appliedCustomHeaders = try values.decode([String: String]?.self, forKey: .appliedCustomHeaders)
}

Expand All @@ -154,6 +158,7 @@ final class UploadMetadata: Codable {
try container.encode(_customHeaders, forKey: .customHeaders)
try container.encode(size, forKey: .size)
try container.encode(_errorCount, forKey: .errorCount)
try container.encode(responseHeaders, forKey: .responseHeaders)
try container.encode(_appliedCustomHeaders, forKey: .appliedCustomHeaders)
}

Expand Down
8 changes: 8 additions & 0 deletions TUSKitExample/TUSKitExample/Helpers/TUSWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@ extension TUSWrapper: TUSClientDelegate {
}

func didFinishUpload(id: UUID, url: URL, context: [String : String]?, client: TUSClient) {
/// This function is called if the following
/// `didFinishUpload(id:UUID, url:URL, context:[String:String]?, client:TUSClient, responseHeaders:[String:String]?)`
/// delegate method is not implemented.
}

func didFinishUpload(id: UUID, url: URL, context: [String : String]?, client: TUSClient, responseHeaders: [String : String]?) {
Task { @MainActor in
withAnimation {
uploads[id] = .uploaded(url: url)
Expand All @@ -123,7 +129,9 @@ extension TUSWrapper: TUSClientDelegate {
}
}

func fileError(error: TUSClientError, client: TUSClient) {}
func fileError(id: UUID?, error: TUSClientError, client: TUSClient) { }

func totalProgress(bytesUploaded: Int, totalBytes: Int, client: TUSClient) {
print("total progress: \(bytesUploaded) / \(totalBytes) => \(Int(Double(bytesUploaded) / Double(totalBytes) * 100))%")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ extension UploadsListView {
Spacer()
Text(uploadCategory.noRecoredMessage)
if uploadCategory == .all {
(Text("Upload files for ") + (Text("Upload files ") + Text(Image(systemName: Icon.uploadFileFilled.rawValue))).foregroundColor(.blue) + Text(" tab"))
(Text("Upload files from ") + (Text("Upload files ") + Text(Image(systemName: Icon.uploadFileFilled.rawValue))).foregroundColor(.blue) + Text(" tab"))
}
Spacer()
}
Expand Down