Skip to content

[Operation] add willChangeValue() and didChangeValue() #836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 23, 2017
Merged
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
19 changes: 18 additions & 1 deletion Foundation/NSOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ open class Operation : NSObject {
#endif
}

/// - Note: Operations that are asynchronous from the execution of the operation queue itself are not supported since there is no KVO to trigger the finish.
open func start() {
Copy link
Contributor

Choose a reason for hiding this comment

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

So the problem now is that if someone overrides main but not start and detaches from the current operation's queue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we document and live with that for now to unblock this particular use case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any updates on this? The limitation @phausler identifies seems less serious than the current one.

Copy link
Contributor

Choose a reason for hiding this comment

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

it seems like the lesser of two evils. so seems like a decent work-around until we have a better answer

Copy link
Contributor

Choose a reason for hiding this comment

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

could you perhaps add a simple unit test to verify the expected behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@phausler @parkera I've added a test for this. Please could you review and run CI?

main()
finish()
Expand Down Expand Up @@ -159,6 +158,24 @@ open class Operation : NSObject {
}
}

/// The following two methods are added to provide support for Operations which
/// are asynchronous from the execution of the operation queue itself. On Darwin,
/// this is supported via KVO notifications. In the absence of KVO on non-Darwin
/// platforms, these two methods (which are defined in NSObject on Darwin) are
/// temporarily added here. They should be removed once a permanent solution is
/// found.
extension Operation {
public func willChangeValue(forKey key: String) {
// do nothing
}

public func didChangeValue(forKey key: String) {
if key == "isFinished" && isFinished {
finish()
}
}
}

extension Operation {
public enum QueuePriority : Int {
case veryLow
Expand Down
76 changes: 75 additions & 1 deletion TestFoundation/TestNSOperationQueue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ import XCTest
import SwiftFoundation
import SwiftXCTest
#endif
import Dispatch

class TestNSOperationQueue : XCTestCase {
static var allTests: [(String, (TestNSOperationQueue) -> () throws -> Void)] {
return [
("test_OperationPriorities", test_OperationPriorities),
("test_OperationCount", test_OperationCount)
("test_OperationCount", test_OperationCount),
("test_AsyncOperation", test_AsyncOperation)
]
}

Expand Down Expand Up @@ -65,4 +67,76 @@ class TestNSOperationQueue : XCTestCase {
XCTAssertEqual(msgOperations[2], "Operation2 executed")
XCTAssertEqual(msgOperations[3], "Operation4 executed")
}

func test_AsyncOperation() {
let operation = AsyncOperation()
XCTAssertFalse(operation.isExecuting)
XCTAssertFalse(operation.isFinished)

operation.start()

while !operation.isFinished {
// do nothing
}

XCTAssertFalse(operation.isExecuting)
XCTAssertTrue(operation.isFinished)
}
}

class AsyncOperation: Operation {

private let queue = DispatchQueue(label: "async.operation.queue")
private let lock = NSLock()

private var _executing = false
private var _finished = false

override internal(set) var isExecuting: Bool {
get {
return _executing
}
set {
if _executing != newValue {
willChangeValue(forKey: "isExecuting")
_executing = newValue
didChangeValue(forKey: "isExecuting")
}
}
}

override internal(set) var isFinished: Bool {
get {
return _finished
}
set {
if _finished != newValue {
willChangeValue(forKey: "isFinished")
_finished = newValue
didChangeValue(forKey: "isFinished")
}
}
}

override var isAsynchronous: Bool {
return true
}

override func start() {
if isCancelled {
isFinished = true
return
}

isExecuting = true

queue.async {
sleep(1)
self.lock.lock()
self.isExecuting = false
self.isFinished = true
self.lock.unlock()
}
}

}