Merge pull request #3 from orchetect/dev-progress-labels

Operation queue cancellation progress reset improvements

Author: orchetect

.swiftpm/xcode/xcshareddata/xcschemes/OTOperations-CI.xcscheme

@@ -52,11 +52,6 @@
                BlueprintName = "OTOperationsTests"
                ReferencedContainer = "container:">
             </BuildableReference>
-            <SkippedTests>
-               <Test
-                  Identifier = "AtomicOperationQueue_Tests/testOp_concurrentAutomatic_Pause_Run()">
-               </Test>
-            </SkippedTests>
          </TestableReference>
       </Testables>
    </TestAction>

Sources/OTOperations/Operation/Foundational/BasicOperation.swift

@@ -92,8 +92,13 @@ open class BasicOperation: Operation, ProgressReporting {
 
         progressWeight = weight
         super.init()
+        
         if label != nil { labelProgress.label = label }
 
+        progress.cancellationHandler = { [progress] in
+            // automatically set progress to finished state if cancelled
+            progress.completedUnitCount = progress.totalUnitCount
+        }
     }
 
     // MARK: - Method Overrides
@@ -128,7 +133,7 @@ open class BasicOperation: Operation, ProgressReporting {
     /// If returning early from the operation due to `isCancelled` being true, call this with the `dueToCancellation` flag set to `true` to update this operation's progress as cancelled.
     public final func completeOperation(dueToCancellation: Bool = false) {
 
-        if isCancelled || dueToCancellation {
+        if dueToCancellation {
             progress.cancel()
         }
 
@@ -152,6 +157,14 @@ open class BasicOperation: Operation, ProgressReporting {
 
     }
 
+    deinit {
+        
+        // progress object MUST ALWAYS set completed == total, even if cancelled
+        // or it will not be released from its parent progress
+        progress.completedUnitCount = progress.totalUnitCount
+        
+    }
+    
 }
 
 #endif

Sources/OTOperations/OperationQueue/AtomicOperationQueue.swift

@@ -186,14 +186,13 @@ open class AtomicOperationQueue<T>: BasicOperationQueue {
         _ block: @escaping (_ atomicValue: VariableAccess) -> Void
     ) -> ClosureOperation {
 
-        let op = ClosureOperation(label: label,
-                                  weight: weight)
+        ClosureOperation(label: label,
+                         weight: weight)
         { [weak self] in
             guard let self = self else { return }
             let varAccess = VariableAccess(operationQueue: self)
             block(varAccess)
         }
-        return op
 
     }
 
@@ -207,14 +206,13 @@ open class AtomicOperationQueue<T>: BasicOperationQueue {
                             _ atomicValue: VariableAccess) -> Void
     ) -> InteractiveClosureOperation {
 
-        let op = InteractiveClosureOperation(label: label,
-                                             weight: weight)
+        InteractiveClosureOperation(label: label,
+                                    weight: weight)
         { [weak self] operation in
             guard let self = self else { return }
             let varAccess = VariableAccess(operationQueue: self)
             block(operation, varAccess)
         }
-        return op
 
     }
 

Sources/OTOperations/OperationQueue/BasicOperationQueue.swift

@@ -119,11 +119,20 @@ open class BasicOperationQueue: OperationQueue {
         // failsafe reset of progress to known state if queue is empty
         var resetTotalUnitCountNudge = false
         if resetProgressWhenFinished, operationCount == 0 {
-            #if DEBUG
-            if !progress.children.isEmpty {
-                assertionFailure("operationCount is 0 but progress children is not empty - possible retain cycle")
+            if let children = progress.value(forKeyPath: "children") as? NSMutableSet,
+               children.count > 0
+            {
+                // this is jank, but manually remove all children
+                children
+                    .allObjects
+                    .compactMap { $0 as? LabelProgress }
+                    .forEach {
+                        $0.setValue(nil, forKeyPath: "parent")
+                        children.remove($0)
+                    }
+                
+                //assertionFailure("operationCount is 0 but progress children is not empty - possible retain cycle")
             }
-            #endif
 
             progress.completedUnitCount = 0
             progress.totalUnitCount = 1
@@ -184,11 +193,20 @@ open class BasicOperationQueue: OperationQueue {
         // failsafe reset of progress to known state if queue is empty
         var resetTotalUnitCountNudge = false
         if resetProgressWhenFinished, operationCount == 0 {
-            #if DEBUG
-            if !progress.children.isEmpty {
-                assertionFailure("operationCount is 0 but progress children is not empty - possible retain cycle")
+            if let children = progress.value(forKeyPath: "children") as? NSMutableSet,
+               children.count > 0
+            {
+                // this is jank, but manually remove all children
+                children
+                    .allObjects
+                    .compactMap { $0 as? LabelProgress }
+                    .forEach {
+                        $0.setValue(nil, forKeyPath: "parent")
+                        children.remove($0)
+                    }
+                
+                //assertionFailure("operationCount is 0 but progress children is not empty - possible retain cycle")
             }
-            #endif
 
             progress.completedUnitCount = 0
             progress.totalUnitCount = 1

Tests/OTOperationsTests/Operation/Closure/AsyncClosureOperation Tests.swift

@@ -113,10 +113,9 @@ final class AsyncClosureOperation_Tests: XCTestCase {
         XCTAssertTrue(op.isCancelled)
         XCTAssertTrue(op.isExecuting)
         // progress
-        XCTAssertFalse(op.progress.isFinished)
+        XCTAssertTrue(op.progress.isFinished)
         XCTAssertTrue(op.progress.isCancelled)
-        XCTAssertEqual(op.progress.fractionCompleted, 0.0)
-        XCTAssertLessThan(op.progress.fractionCompleted, 1.0)
+        XCTAssertEqual(op.progress.fractionCompleted, 1.0)
         XCTAssertFalse(op.progress.isIndeterminate)
 
     }
@@ -210,9 +209,9 @@ final class AsyncClosureOperation_Tests: XCTestCase {
         XCTAssertTrue(op.isCancelled)
         XCTAssertTrue(op.isExecuting) // still executing
         // progress - operation
-        XCTAssertFalse(op.progress.isFinished)
+        XCTAssertTrue(op.progress.isFinished)
         XCTAssertTrue(op.progress.isCancelled)
-        XCTAssertEqual(op.progress.fractionCompleted, 0.0)
+        XCTAssertEqual(op.progress.fractionCompleted, 1.0)
         XCTAssertFalse(op.progress.isIndeterminate)
         // progress - queue
         XCTAssertTrue(opQ.progress.isFinished) // even if the async op is still running, this will be true now