Merge pull request #45 from apivideo/bugfix/ios_request_queue

fix(ios): fix the way RequestTaskQueue operation is synchronous

.github/workflows/build.yml

@@ -0,0 +1,63 @@
+name: Build
+
+on: [push]
+
+jobs:
+  build-swift:
+    name: Build with swift
+    runs-on: macos-12
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+      - name: xcode version
+        uses: maxim-lobanov/setup-xcode@v1
+        with:
+          xcode-version: '13.4.1'
+      - name: Build Package with swift
+        run: swift build
+
+  build-xcodebuild:
+    name: Build with xcodebuild
+    runs-on: macos-12
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+      - name: xcode version
+        uses: maxim-lobanov/setup-xcode@v1
+        with:
+          xcode-version: '13.4.1'
+      - name: Set Default Scheme
+        run: |
+          scheme_list=$(xcodebuild -list -json | tr -d "\n")
+          default=$(echo $scheme_list | ruby -e "require 'json'; puts JSON.parse(STDIN.gets)['workspace']['schemes'][0]")
+          echo $default | cat >default
+          echo Using default scheme: $default
+      - name: Build Package with xcodebuild
+        env:
+          scheme: ${{ 'default' }}
+        run: |
+          if [ $scheme = default ]; then scheme=$(cat default); fi
+          xcodebuild -scheme $scheme  -destination 'platform=iOS Simulator,name=iPhone 13'
+      - name: Build Example
+        env:
+          scheme: ${{ 'default' }}
+        run: |
+          if [ $scheme = default ]; then scheme=$(cat default); fi
+          xcodebuild clean build -project Example/Example.xcodeproj -scheme $scheme -sdk iphoneos
+ 
+  cocoapods:
+    name: Verify cocopods podspec
+    needs: [ build-xcodebuild ]
+    runs-on: macos-12
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+      - name: xcode version
+        uses: maxim-lobanov/setup-xcode@v1
+        with:
+          xcode-version: '13.4.1'
+      - name: Verify cocoapods
+        run: pod lib lint --allow-warnings

ApiVideoUploader.podspec

@@ -5,8 +5,8 @@ Pod::Spec.new do |s|
   s.tvos.deployment_target = '10.0'
   # Add back when CocoaPods/CocoaPods#11558 is released
   #s.watchos.deployment_target = '3.0'
-  s.version = '1.1.0'
-  s.source = { :git => 'https://github.com/apivideo/api.video-ios-uploader', :tag => 'v1.1.0' }
+  s.version = '1.1.1'
+  s.source = { :git => 'https://github.com/apivideo/api.video-ios-uploader', :tag => 'v1.1.1' }
   s.authors = { 'Ecosystem Team' => 'ecosystem@api.video' }
   s.license = { :type => 'MIT' }
   s.homepage = 'https://docs.api.video'

CHANGELOG.md

@@ -1,6 +1,11 @@
 # Changelog
 All changes to this project will be documented in this file.
 
+## [1.1.1] - 2022-12-09
+- Fix on upload by chunk and progressive upload.
+- Add test on progressive upload.
+- Add a `build.yml` CI workflow.
+
 ## [1.1.0] - 2022-12-06
 - Refactor upload by chunk and progressive upload. It is now possible to cancel an upload.
 - Add timeout API

README.md

@@ -39,14 +39,14 @@ It allows you to upload videos in two ways:
 Specify it in your `Cartfile`:
 
 ```
-github "apivideo/api.video-ios-uploader" ~> 1.1.0
+github "apivideo/api.video-ios-uploader" ~> 1.1.1
 ```
 
 Run `carthage update`
 
 ### CocoaPods
 
-Add `pod 'ApiVideoUploader', '1.1.0'` in your `Podfile`
+Add `pod 'ApiVideoUploader', '1.1.1'` in your `Podfile`
 
 Run `pod install`
 

Sources/APIs.swift

@@ -8,7 +8,7 @@ import Foundation
 public class ApiVideoUploader {
     public static var apiKey: String? = nil
     public static var basePath = "https://ws.api.video"
-    internal static var customHeaders:[String: String] = ["AV-Origin-Client": "ios-uploader:1.1.0"]
+    internal static var customHeaders:[String: String] = ["AV-Origin-Client": "ios-uploader:1.1.1"]
     private static var chunkSize: Int = 50 * 1024 * 1024
     internal static var requestBuilderFactory: RequestBuilderFactory = AlamofireRequestBuilderFactory()
     internal static var credential = ApiVideoCredential()

Sources/APIs/VideosAPI.swift

@@ -40,7 +40,7 @@ open class VideosAPI {
     /**
      * Create a progressive upload session
      *
-     * - returns: a progressive upload session
+     - returns: a progressive upload session
      */
     public class func buildProgressiveUploadSession(videoId: String) -> ProgressiveUploadSession {
         ProgressiveUploadSession(videoId: videoId)
@@ -55,6 +55,7 @@ open class VideosAPI {
             self.videoId = videoId
             super.init(queueLabel: videoId)
         }
+
 
         public func uploadPart(file: URL, onProgressReady: ((Progress) -> Void)? = nil, apiResponseQueue: DispatchQueue = ApiVideoUploader.apiResponseQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) -> RequestTask {
             let chunkId = partId
@@ -82,13 +83,14 @@ open class VideosAPI {
                 numOfChunks = partId
             }
             let requestBuilder = uploadWithRequestBuilder(videoId: videoId, file: file, chunkId: partId, numOfChunks: numOfChunks, onProgressReady: onProgressReady)
-            execute(requestBuilder, apiResponseQueue: apiResponseQueue, completion: completion)
+            execute(requestBuilder, apiResponseQueue: apiResponseQueue) { data, error in
+                completion(data, error)
+            }
             return requestBuilder.requestTask
         }
     }
 
 
-
     /**
      Upload a video
      - POST /videos/{videoId}/source
@@ -233,7 +235,7 @@ The latter allows you to split a video source into X chunks and send those chunk
     /**
      * Create a progressive uploadWithUploadToken session
      *
-     * - returns: a progressive uploadWithUploadToken session
+     - returns: a progressive uploadWithUploadToken session
      */
     public class func buildProgressiveUploadWithUploadTokenSession(token: String) -> ProgressiveUploadWithUploadTokenSession {
         ProgressiveUploadWithUploadTokenSession(token: token)
@@ -249,6 +251,12 @@ The latter allows you to split a video source into X chunks and send those chunk
             self.token = token
             super.init(queueLabel: token)
         }
+
+        override func willExecuteRequestBuilder(requestBuilder: RequestBuilder<Video>) -> Void {
+            if let videoId = videoId {
+                uploadAddVideoIdParameterWithRequestBuilder(requestBuilder: requestBuilder, videoId: videoId)
+            }
+        }
 
         public func uploadPart(file: URL, onProgressReady: ((Progress) -> Void)? = nil, apiResponseQueue: DispatchQueue = ApiVideoUploader.apiResponseQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) -> RequestTask {
             let chunkId = partId
@@ -276,11 +284,28 @@ The latter allows you to split a video source into X chunks and send those chunk
                 numOfChunks = partId
             }
             let requestBuilder = uploadWithUploadTokenWithRequestBuilder(token: token, file: file, videoId: videoId, chunkId: partId, numOfChunks: numOfChunks, onProgressReady: onProgressReady)
-            execute(requestBuilder, apiResponseQueue: apiResponseQueue, completion: completion)
+            execute(requestBuilder, apiResponseQueue: apiResponseQueue) { data, error in
+                if let data = data {
+                    self.videoId = data.videoId
+                }
+                completion(data, error)
+            }
             return requestBuilder.requestTask
         }
     }
-
+    /**
+     * Add a videoId to the request builder if it does not exist already.
+     - parameter requestBuilder: the request builder
+     - parameter videoId: the videoId to add to the request
+     */
+    internal class func uploadAddVideoIdParameterWithRequestBuilder(requestBuilder: RequestBuilder<Video>, videoId: String) {
+        guard let parameters = requestBuilder.parameters else {
+            return
+        }
+        if (!parameters.keys.contains("videoId")) {
+            requestBuilder.parameters!["videoId"] = videoId
+        }
+    }
 
 
     /**

Sources/Upload/ProgressiveUploadSessionProtocol.swift

@@ -1,12 +1,14 @@
-//  ProgressiveUploadSessioning.swift
+//  ProgressiveUploadSessionProtocol.swift
 //
 
 import Foundation
 
 public protocol ProgressiveUploadSessionProtocol {
-    func uploadPart(file: URL, onProgressReady: ((Progress) -> Void)?, apiResponseQueue: DispatchQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) throws -> RequestTask
-    func uploadLastPart(file: URL, onProgressReady: ((Progress) -> Void)?, apiResponseQueue: DispatchQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) throws -> RequestTask
-    func uploadPart(file: URL, partId: Int, onProgressReady: ((Progress) -> Void)?, apiResponseQueue: DispatchQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) throws -> RequestTask
-    func uploadLastPart(file: URL, partId: Int, onProgressReady: ((Progress) -> Void)?, apiResponseQueue: DispatchQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) throws -> RequestTask
-    func uploadPart(file: URL, partId: Int, isLastPart: Bool, onProgressReady: ((Progress) -> Void)?, apiResponseQueue: DispatchQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) throws -> RequestTask
+    func uploadPart(file: URL, onProgressReady: ((Progress) -> Void)?, apiResponseQueue: DispatchQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) -> RequestTask
+    func uploadLastPart(file: URL, onProgressReady: ((Progress) -> Void)?, apiResponseQueue: DispatchQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) -> RequestTask
+    func uploadPart(file: URL, partId: Int, onProgressReady: ((Progress) -> Void)?, apiResponseQueue: DispatchQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) -> RequestTask
+    func uploadLastPart(file: URL, partId: Int, onProgressReady: ((Progress) -> Void)?, apiResponseQueue: DispatchQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) -> RequestTask
+    func uploadPart(file: URL, partId: Int, isLastPart: Bool, onProgressReady: ((Progress) -> Void)?, apiResponseQueue: DispatchQueue, completion: @escaping ((_ data: Video?, _ error: Error?) -> Void)) -> RequestTask
+
+    func cancel()
 }

Sources/Upload/RequestTaskQueue.swift

@@ -3,7 +3,6 @@ import Alamofire
 
 public class RequestTaskQueue<T>: RequestTask {
     private let operationQueue: OperationQueue
-    private let operationLock = NSLock()
     private var requestBuilders: [RequestBuilder<T>] = []
 
     private let _downloadProgress = Progress(totalUnitCount: 0)
@@ -53,7 +52,7 @@ public class RequestTaskQueue<T>: RequestTask {
                           apiResponseQueue: DispatchQueue = ApiVideoUploader.apiResponseQueue,
                           completion: @escaping (_ data: T?, _ error: Error?) -> Void) -> Void {
         requestBuilders.append(requestBuilder)
-        return operationQueue.addOperation(RequestOperation(lock: operationLock, requestBuilder: requestBuilder, apiResponseQueue: apiResponseQueue, willExecuteRequestBuilder: willExecuteRequestBuilder, completion: completion))
+        return operationQueue.addOperation(RequestOperation(requestBuilder: requestBuilder, apiResponseQueue: apiResponseQueue, willExecuteRequestBuilder: willExecuteRequestBuilder, completion: completion))
     }
 
 
@@ -66,14 +65,13 @@ public class RequestTaskQueue<T>: RequestTask {
 }
 
 final class RequestOperation<T>: Operation {
-    private let lock: NSLock
     private let requestBuilder: RequestBuilder<T>
     private let apiResponseQueue: DispatchQueue
     private let completion: (_ data: T?, _ error: Error?) -> Void
     private let willExecuteRequestBuilder: (_: RequestBuilder<T>) -> Void
-
-    init(lock: NSLock, requestBuilder: RequestBuilder<T>, apiResponseQueue: DispatchQueue, willExecuteRequestBuilder: @escaping (_: RequestBuilder<T>) -> Void, completion: @escaping (_ data: T?, _ error: Error?) -> Void) {
-        self.lock = lock
+    private let group = DispatchGroup()
+
+    init(requestBuilder: RequestBuilder<T>, apiResponseQueue: DispatchQueue, willExecuteRequestBuilder: @escaping (_: RequestBuilder<T>) -> Void, completion: @escaping (_ data: T?, _ error: Error?) -> Void) {
         self.requestBuilder = requestBuilder
         self.apiResponseQueue = apiResponseQueue
         self.willExecuteRequestBuilder = willExecuteRequestBuilder
@@ -82,11 +80,11 @@ final class RequestOperation<T>: Operation {
     }
 
     override func main() {
-        self.lock.lock()
         guard !isCancelled else {
-            self.lock.unlock()
             return
         }
+        group.enter()
+
         self.willExecuteRequestBuilder(requestBuilder)
         requestBuilder.execute(apiResponseQueue) { result in
             switch result {
@@ -95,8 +93,9 @@ final class RequestOperation<T>: Operation {
             case let .failure(error):
                 self.completion(nil, error)
             }
-            self.lock.unlock()
+            self.group.leave()
         }
-
+        // Make task synchronous
+        group.wait()
     }
 }

Sources/Upload/UploadChunkRequestTaskQueue.swift

@@ -69,7 +69,7 @@ public class UploadChunkRequestTaskQueue: RequestTaskQueue<Video> {
 
     override func willExecuteRequestBuilder(requestBuilder: RequestBuilder<Video>) -> Void {
         if let videoId = videoId {
-            uploadAddVideoIdParameterWithRequestBuilder(requestBuilder: requestBuilder, videoId: videoId)
+            VideosAPI.uploadAddVideoIdParameterWithRequestBuilder(requestBuilder: requestBuilder, videoId: videoId)
         }
         requestBuilder.onProgressReady = progressReadyHook
     }
@@ -101,18 +101,4 @@ public class UploadChunkRequestTaskQueue: RequestTaskQueue<Video> {
             }
         }
     }
-
-    /**
-    * Add a videoId to the request builder if it does not exist already.
-    - parameter requestBuilder: the request builder
-    - parameter videoId: the videoId to add to the request
-    */
-    private func uploadAddVideoIdParameterWithRequestBuilder(requestBuilder: RequestBuilder<Video>, videoId: String) {
-        guard let parameters = requestBuilder.parameters else {
-            return
-        }
-        if (!parameters.keys.contains("videoId")) {
-            requestBuilder.parameters!["videoId"] = videoId
-        }
-    }
 }

project.yml

@@ -7,7 +7,7 @@ targets:
     sources: [Sources]
     info:
       path: ./Info.plist
-      version: 1.1.0
+      version: 1.1.1
     settings:
       APPLICATION_EXTENSION_API_ONLY: true
     scheme: {}