swiftlang / swift-corelibs-foundation

The Foundation Project, providing core utilities, internationalization, and OS independence
swift.org
Apache License 2.0
5.29k stars 1.13k forks source link

[SR-6117] Using NSKeyValueObservingCustomization to customise dependent keys is broken if first observer is using NObject.addObserver #3801

Open bobergj opened 7 years ago

bobergj commented 7 years ago
Previous ID SR-6117
Radar None
Original Reporter @bobergj
Type Bug

Attachment: Download

Environment Xcode Version 10.3 (10G8), Xcode Version 11.0 beta 4 (11M374r) Swift 5, Swift 5.1
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 1 | |Component/s | Foundation | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 54396a5f746d1959b402b2457c7ba2b1

Issue Description:

NSKeyValueObservingCustomization methods are never called if the very first observer is added using NObject.addObserver (or from Objective-C) rather than using the newer NSObject.observe API.

How to reproduce:

Given a class A with dependent keys, and an observer ObjcObserver:

struct MyStructWithPos {
    var pos: CGPoint
}

class A: NSObject, NSKeyValueObservingCustomization {

    var myStruct: MyStructWithPos = MyStructWithPos(pos: .zero)

    static func keyPathsAffectingValue(for key: AnyKeyPath) -> Set<AnyKeyPath> {
        if key == \A.pos {
            return [\A.posX, \A.posY]
        } else {
            return []
        }
    }

    static func automaticallyNotifiesObservers(for key: AnyKeyPath) -> Bool {
        return true
    }

    @objc dynamic var pos: CGPoint {
        set {
            myStruct.pos = newValue
        }

        get {
            return myStruct.pos
        }
    }

    @objc dynamic var posX: CGFloat {
        set {
            myStruct.pos.x = newValue
        }

        get {
            return myStruct.pos.x
        }
    }

    @objc dynamic var posY: CGFloat {
        set {
            myStruct.pos.y = newValue
        }

        get {
            return myStruct.pos.y
        }
    }
}

class ObjcObserver: NSObject {

    private static var observerContext = 0

    init(a: A) {
        super.init()
        a.addObserver(self, forKeyPath: "pos", options: [], context: &ObjcObserver.observerContext)
    }

    override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) {
        if context == &ObjcObserver.observerContext {
            print("observed change to pos in NSObject observeValue method")
        } else {
            super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context)
        }
    }

}

If NSObject.observe is called first:

let a = A()

let swiftObserver = a.observe(\A.pos) { (_, _) in
    print("observed change to pos in observe block")
}

let objcObserver = ObjcObserver(a: a)

a.pos = CGPoint(x: 100, y: 100)
a.posX = 200

The output is:

observed change to pos in NSObject observeValue method
observed change to pos in observe block
observed change to pos in NSObject observeValue method
observed change to pos in observe block

If NSObject.addObserver is called first:

let a = A()

let objcObserver = ObjcObserver(a: a)

let swiftObserver = a.observe(\A.pos) { (_, _) in
    print("observed change to pos in observe block")
}

a.pos = CGPoint(x: 100, y: 100)
a.posX = 200

Then we have instead have following (incorrect) output:

observed change to pos in observe block
observed change to pos in NSObject observeValue method

Workaround:

Implement the old string based keyPathsForValuesAffectingValue method in A instead:

class A {
 ... 
     override class func keyPathsForValuesAffectingValue(forKey key: String) -> Set<String> {
        if key == #keyPath(pos) {
            return [#keyPath(posX), #keyPath(posY)]
        } else {
            return []
        }
    }
 ...
}

Related issue:

NSKeyValueObservingCustomization doesn't play well with a class hierarchy. If you try to override the methods in a subclass you'll get an error "error: cannot override static method".

belkadan commented 7 years ago

cc @jckarter, @Catfish-Man