Feature/add extension to support tests

PSOperations/GroupOperation.swift

@@ -26,6 +26,7 @@ open class GroupOperation: Operation {
     fileprivate let startingOperation = Foundation.BlockOperation(block: {})
     fileprivate let finishingOperation = Foundation.BlockOperation(block: {})
 
+    fileprivate let errorsLock = NSLock()
     fileprivate var aggregatedErrors = [NSError]()
 
     public convenience init(operations: Foundation.Operation...) {
@@ -65,7 +66,9 @@ open class GroupOperation: Operation {
         of errors reported to observers and to the `finished(_:)` method.
     */
     public final func aggregateError(_ error: NSError) {
-        aggregatedErrors.append(error)
+        errorsLock.withCriticalScope {
+            aggregatedErrors.append(error)
+        }
     }
 
     open func operationDidFinish(_ operation: Foundation.Operation, withErrors errors: [NSError]) {
@@ -100,14 +103,31 @@ extension GroupOperation: OperationQueueDelegate {
     }
 
     final public func operationQueue(_ operationQueue: OperationQueue, operationDidFinish operation: Foundation.Operation, withErrors errors: [NSError]) {
-        aggregatedErrors.append(contentsOf: errors)
+        errorsLock.withCriticalScope {
+            aggregatedErrors.append(contentsOf: errors)
+        }
 
         if operation === finishingOperation {
             internalQueue.isSuspended = true
-            finish(aggregatedErrors)
+
+            let errors = errorsLock.withCriticalScope {
+                return aggregatedErrors
+            }
+
+            finish(errors)
         }
         else if operation !== startingOperation {
             operationDidFinish(operation, withErrors: errors)
         }
     }
 }
+
+/* Internal extensions for UnitTesting */
+
+extension GroupOperation {
+    func stubGroupOperation(withError error: NSError) {
+        internalQueue.cancelAllOperations()
+        cancelWithError(error)
+    }
+}
+

PSOperations/Operation.swift

@@ -16,6 +16,8 @@ import Foundation
 */
 open class Operation: Foundation.Operation {
 
+    private var observerContext = 4321
+
     /* The completionBlock property has unexpected behaviors such as executing twice and executing on unexpected threads. BlockObserver
      * executes in an expected manner.
      */
@@ -29,12 +31,7 @@ open class Operation: Foundation.Operation {
         }
     }
 
-
     // use the KVO mechanism to indicate that changes to "state" affect other properties as well
-    class func keyPathsForValuesAffectingIsReady() -> Set<NSObject> {
-        return ["state" as NSObject, "cancelledState" as NSObject]
-    }
-
     class func keyPathsForValuesAffectingIsExecuting() -> Set<NSObject> {
         return ["state" as NSObject]
     }
@@ -47,6 +44,15 @@ open class Operation: Foundation.Operation {
         return ["cancelledState" as NSObject]
     }
 
+    public override init() {
+        super.init()
+        self.addObserver(self, forKeyPath: "isReady", context: &observerContext)
+    }
+
+    deinit {
+        self.removeObserver(self, forKeyPath: "isReady", context: &observerContext)
+    }
+
     // MARK: State Management
 
     fileprivate enum State: Int, Comparable {
@@ -142,48 +148,49 @@ open class Operation: Foundation.Operation {
 
                 assert(_state.canTransitionToState(newState, operationIsCancelled: isCancelled), "Performing invalid state transition.")
                 _state = newState
+                updateReadiness()
             }
 
             didChangeValue(forKey: "state")
         }
     }
 
-    // Here is where we extend our definition of "readiness".
-    override open var isReady: Bool {
-
-        var _ready = false
-
+    open override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) {
+        if context == &observerContext {
+            updateReadiness()
+        } else {
+            super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context)
+        }
+    }
+
+    private func updateReadiness() {
         stateLock.withCriticalScope {
-            switch state {
-
-            case .initialized:
-                // If the operation has been cancelled, "isReady" should return true
-                _ready = isCancelled
-
-            case .pending:
-                // If the operation has been cancelled, "isReady" should return true
-                guard !isCancelled else {
+            let cancelled = isCancelled
+
+            if state == .pending {
+                if cancelled {
                     state = .ready
-                    _ready = true
-                    return
-                }
-
-                // If super isReady, conditions can be evaluated
-                if super.isReady {
+                } else if super.isReady {
                     evaluateConditions()
-                    _ready = state == .ready
                 }
-
-            case .ready:
-                _ready = super.isReady || isCancelled
-
-            default:
-                _ready = false
             }
-
+
+            // Generate KVO notitification only once when operation is really ready
+            let newReady = state >= .ready || (state == .initialized && cancelled)
+            if !_ready && newReady {
+                willChangeValue(forKey: "isReady")
+                _ready = true
+                didChangeValue(forKey: "isReady")
+            }
+        }
+    }
+
+    private var _ready: Bool = false
+
+    override open var isReady: Bool {
+        return stateLock.withCriticalScope {
+            return _ready && super.isReady
         }
-
-        return _ready
     }
 
     open var userInitiated: Bool {
@@ -214,6 +221,7 @@ open class Operation: Foundation.Operation {
         didSet {
             didChangeValue(forKey: "cancelledState")
             if _cancelled != oldValue && _cancelled == true {
+                updateReadiness()
 
                 for observer in observers {
                     observer.operationDidCancel(self)
@@ -380,10 +388,9 @@ open class Operation: Foundation.Operation {
         A private property to ensure we only notify the observers once that the 
         operation has finished.
     */
-    fileprivate var hasFinishedAlready = false
+    fileprivate var hasFinishedAlready: UInt8 = 0
     public final func finish(_ errors: [NSError] = []) {
-        if !hasFinishedAlready {
-            hasFinishedAlready = true
+        if !OSAtomicTestAndSet(0, &hasFinishedAlready) {
             state = .finishing
 
             _internalErrors += errors

PSOperations/OperationQueue.swift

@@ -46,7 +46,9 @@ open class OperationQueue: Foundation.OperationQueue {
                 finishHandler: { [weak self] finishedOperation, errors in
                     if let q = self {
 
-                        q.delegate?.operationQueue?(q, operationDidFinish: finishedOperation, withErrors: errors)
+                        if let delegate = q.delegate {
+                            delegate.operationQueue?(q, operationDidFinish: finishedOperation, withErrors: errors)
+                        }
                         //Remove deps to avoid cascading deallocation error
                         //http://stackoverflow.com/questions/19693079/nsoperationqueue-bug-with-dependencies
                         finishedOperation.dependencies.forEach { finishedOperation.removeDependency($0) }
@@ -96,7 +98,9 @@ open class OperationQueue: Foundation.OperationQueue {
             */
             operation.addCompletionBlock { [weak self, weak operation] in
                 guard let queue = self, let operation = operation else { return }
-                queue.delegate?.operationQueue?(queue, operationDidFinish: operation, withErrors: [])
+                if let delegate = queue.delegate {
+                    delegate.operationQueue?(queue, operationDidFinish: operation, withErrors: [])
+                }
                 //Remove deps to avoid cascading deallocation error
                 //http://stackoverflow.com/questions/19693079/nsoperationqueue-bug-with-dependencies
                 operation.dependencies.forEach { operation.removeDependency($0) }