Swift with NSDocument and Revert To... browse all versions.


Bill Pitcher
 

I've been trying track the correct path from document to Control and back again.

Document automatically loads WindowController.document I use didSet to loop down the ViewControllers and set there ViewController.representedObject and use didSet  to UpdateMyControls
After user changes the data in controls I update the ViewController.representedObject and all is good. Save makes versions.

After returning from Revert To... restoring an old document, my document flicks back to displaying the original data when went into the version browser, BUT the backing document has been resorted correctly. Doing an UpdateMyControls in windowDidExitVersionBrowser does a bad visual, from new to old then new again. Any help or pointers with this really appreciated!

Thanks
Bill


Quincey Morris
 

On Jul 12, 2018, at 03:37 , Bill Pitcher <bill@...> wrote:

After returning from Revert To... restoring an old document, my document flicks back to displaying the original data when went into the version browser, BUT the backing document has been resorted correctly. Doing an UpdateMyControls in windowDidExitVersionBrowser does a bad visual, from new to old then new again.

Propagate the data reversion to your window controller and view controllers can be difficult, because any setup you did in windowDidLoad/viewDidLoad is not automatically redone. It can be especially confusing when the document (and the data model it provides/contains) is reverted, but individual references within you controller hierarchy refer to (and keep alive) various bits of the data model objects that were supposed to be discarded.

It’s also possible to be propagating the model data correctly to the view controllers, but still have the wrong information showing in the views because the propagation wasn’t fully KVO compliant.

There probably isn’t a simple way to solve this, other than the brute force approach of tracking the model data through the controllers to make sure it’s all being updated when necessary.


Bill Pitcher
 

Firstly I’ve uploaded a very cut down project which shows the problem.
https://www.ilike.co.nz/downloads/testDocument.zip

"Propagate the data reversion to your window controller and view controllers can be difficult, because any setup you did in windowDidLoad/viewDidLoad is not automatically redone.”
I found that the didLoad methods are called before the .document property has content. In windowDidLoad .document in nil.

"It can be especially confusing when the document (and the data model it provides/contains) is reverted, but individual references within you controller hierarchy refer to (and keep alive) various bits of the data model objects that were supposed to be discarded.”
Yes!

"It’s also possible to be propagating the model data correctly to the view controllers, but still have the wrong information showing in the views because the propagation wasn’t fully KVO compliant.”

My swift Objects are not KVO at all, I’m not using KVO. But I’m also seeing the same behaviour in my swift NSPersistentDocument and CoreData classes, and the Apple Demo App “PackagedDocumentforOSX”

"There probably isn’t a simple way to solve this, other than the brute force approach of tracking the model data through the controllers to make sure it’s all being updated when necessary.”
This is the problem, I can’t find a point in the where I can brute force the updates. The document didSet is called before the Window is displayed on the restored data. I would have thought that the didSet on WindowController.documet was the same a putting KVO on the property in viewDidLoad.

I find it crazy that using Swift the process of getting NSDocument data onto the screen and back again with Revert To… seems to me to be broken.


Bill Pitcher
bill@ilike.co.nz

On 13/07/2018, at 12:36 PM, Quincey Morris <quinceymorris@rivergatesoftware.com> wrote:

On Jul 12, 2018, at 03:37 , Bill Pitcher <bill@ilike.co.nz> wrote:

After returning from Revert To... restoring an old document, my document flicks back to displaying the original data when went into the version browser, BUT the backing document has been resorted correctly. Doing an UpdateMyControls in windowDidExitVersionBrowser does a bad visual, from new to old then new again.
Propagate the data reversion to your window controller and view controllers can be difficult, because any setup you did in windowDidLoad/viewDidLoad is not automatically redone. It can be especially confusing when the document (and the data model it provides/contains) is reverted, but individual references within you controller hierarchy refer to (and keep alive) various bits of the data model objects that were supposed to be discarded.

It’s also possible to be propagating the model data correctly to the view controllers, but still have the wrong information showing in the views because the propagation wasn’t fully KVO compliant.

There probably isn’t a simple way to solve this, other than the brute force approach of tracking the model data through the controllers to make sure it’s all being updated when necessary.


Quincey Morris
 

On Jul 12, 2018, at 18:22 , Bill Pitcher <bill@...> wrote:

Firstly I’ve uploaded a very cut down project which shows the problem.
https://www.ilike.co.nz/downloads/testDocument.zip

Well, you’re Doing It Wrong™, but it’s not entirely your fault. This is one of NSDocument’s big messes that we’ve been stuck with for the last 10 years or so.

What’s actually going wrong is that there are multiple document objects involved. Starting with your “latest edit” document (+ its controller hierarchy) A, when you call up the version browser, it displays A on the left and creates a new document B to display on the right. Assuming you choose to revert to B, what you see is B animate back to the original display position of A, while the rest of the version browser UI fades away.

That’s why you see the reversion happen correctly, seemingly. What happens next, though, is that document B is *closed*, since it was only a temporary adjunct needed for the version browser UI. When its window closes, A’s window is revealed underneath, and you see the content un-revert to the latest data that you had in A when this all started.

Why discard B and keep A, when B has the correct data? Because the reversion mechanism doesn’t want to leave you with a different instance of your NSDocument subclass (B) from the one you started with (A), since you might have code that knows and depends on its actual object pointer. It would break existing code if the pointer for the active document suddenly changed.

Instead, what actually happens is that (by default) your document’s reading method — read(from:ofType:) — is invoked anew *in A* but with the data from the reverted version store, and it causes A’s data model to be replaced with a new one that has the reverted data.

That would all be fine, except that your design doesn’t have any machinery to propagate this last data model change to your controller hierarchy. You have code to do it when the window controller’s “document” property changes, but it isn’t changing for document A here, so nothing happens.

There are a couple of things you should do differently:

— I *strongly* urge you not to override NSWindowController’s “document” property. NSWindowController is old-school, and even if the property is changed KVO-compliantly, there’s no guarantee it will actually go through the setter. (OTOH, I don’t think this has any impact on the current problem.)

— I *strongly* urge you not to make your view controller’s “representedObject” refer to the document, but rather to the root object of your data model.

— You need a way of getting notifications to the controller objects when the data model changes, rather than when the document changes. There are multiple approaches, but the one I usually use is to have the window controller observe its own “document.model” keypath (it would be “document.contents” in your sample project) to maintain its own “model” derived property KVO compliantly. Then I have the view controllers observe the window’s “model” property, and update their UI from that change notification. Normally, I don’t bother with the “representedObject” property (not least because it’s untyped and requires casting to be useful), but repeat the “model” property in the view controller if it’s useful to store it there too.

Note that there are other NSDocument override points that can intercept a revert operation earlier, if that’s useful. You might do that if recreating the entire document via read(from:ofType:) is expensive or has side effects, and you have a cheaper or safer way of doing the reversion, or if you want to be able to tell the difference between an “open” read(from:ofType:) and a “revert” one.


Bill Pitcher
 

Thank you,

You are spot on with explaining the problem and I understand your answer and concerns. Sending up a NotificationCenter rocket in read(from:ofType:) confirms your explanation.

Unless I’m mistaken KVO requires going back to NSObject for my model, and not plain Swift classes.

WindowController.document is Any? so doesn’t seem observable but I didn’t spend long on checking that.

What I think you are saying is build a KVO compatible model and then link in observers through to all the Controllers. I think from memory we start the Controller chain KVO when the Document create WindowController, I’m sure I’ve got a old Obj-C App that uses this method. I was really hoping with Swift we could do away with all this KVO linking. Get away from the big mess.

Thank you for looking at this for me and helping me accept that it just can’t be done.

Have a great weekend
Bill Pitcher
bill@ilike.co.nz

On 13/07/2018, at 5:08 PM, Quincey Morris <quinceymorris@rivergatesoftware.com> wrote:

On Jul 12, 2018, at 18:22 , Bill Pitcher <bill@ilike.co.nz> wrote:

Firstly I’ve uploaded a very cut down project which shows the problem.
https://www.ilike.co.nz/downloads/testDocument.zip
Well, you’re Doing It Wrong™, but it’s not entirely your fault. This is one of NSDocument’s big messes that we’ve been stuck with for the last 10 years or so.

What’s actually going wrong is that there are multiple document objects involved. Starting with your “latest edit” document (+ its controller hierarchy) A, when you call up the version browser, it displays A on the left and creates a new document B to display on the right. Assuming you choose to revert to B, what you see is B animate back to the original display position of A, while the rest of the version browser UI fades away.

That’s why you see the reversion happen correctly, seemingly. What happens next, though, is that document B is *closed*, since it was only a temporary adjunct needed for the version browser UI. When its window closes, A’s window is revealed underneath, and you see the content un-revert to the latest data that you had in A when this all started.

Why discard B and keep A, when B has the correct data? Because the reversion mechanism doesn’t want to leave you with a different instance of your NSDocument subclass (B) from the one you started with (A), since you might have code that knows and depends on its actual object pointer. It would break existing code if the pointer for the active document suddenly changed.

Instead, what actually happens is that (by default) your document’s reading method — read(from:ofType:) — is invoked anew *in A* but with the data from the reverted version store, and it causes A’s data model to be replaced with a new one that has the reverted data.

That would all be fine, except that your design doesn’t have any machinery to propagate this last data model change to your controller hierarchy. You have code to do it when the window controller’s “document” property changes, but it isn’t changing for document A here, so nothing happens.

There are a couple of things you should do differently:

— I *strongly* urge you not to override NSWindowController’s “document” property. NSWindowController is old-school, and even if the property is changed KVO-compliantly, there’s no guarantee it will actually go through the setter. (OTOH, I don’t think this has any impact on the current problem.)

— I *strongly* urge you not to make your view controller’s “representedObject” refer to the document, but rather to the root object of your data model.

— You need a way of getting notifications to the controller objects when the data model changes, rather than when the document changes. There are multiple approaches, but the one I usually use is to have the window controller observe its own “document.model” keypath (it would be “document.contents” in your sample project) to maintain its own “model” derived property KVO compliantly. Then I have the view controllers observe the window’s “model” property, and update their UI from that change notification. Normally, I don’t bother with the “representedObject” property (not least because it’s untyped and requires casting to be useful), but repeat the “model” property in the view controller if it’s useful to store it there too.

Note that there are other NSDocument override points that can intercept a revert operation earlier, if that’s useful. You might do that if recreating the entire document via read(from:ofType:) is expensive or has side effects, and you have a cheaper or safer way of doing the reversion, or if you want to be able to tell the difference between an “open” read(from:ofType:) and a “revert” one.


Quincey Morris
 

On Jul 13, 2018, at 03:49 , Bill Pitcher <bill@...> wrote:

Unless I’m mistaken KVO requires going back to NSObject for my model, and not plain Swift classes.

Well, things get more complicated when you use non-Obj-C types, but it’s not necessarily true that you can’t use Swift types. In the case of the model, it may be sufficient that your “model" property of NSDocument be @objc (but not a Swift-native type), to KVO-observe it.

WindowController.document is Any? so doesn’t seem observable but I didn’t spend long on checking that. 

It’s still an @obj property of an Obj-C object, so it’s observable. The type is actually ‘id’, which translates into Swift ‘Any’ or ‘Any?’ these days, not into ‘AnyObject’ as it used to.

What I think you are saying is build a KVO compatible model and then link in observers through to all the Controllers.

I wasn’t really saying that, just saying you should be able to use KVO to keep track of the model’s root object. Whether you use KVO or Obj-C compatible objects or properties further down the line is up to you.

Note, however, that there is still a *lot* in built-in pressure to stay Obj-C-compatible in this part of your app design, because a lot of familiar design patterns rely on KVO and KVC.


Bill Pitcher
 

Thank you for your help with this!

I’ve given it another try and hope some knowledgeable person could check that I’m still not Doing it Wrong ™

Currently WindowController.document still seems to be AnyObject? which caused me some consternation

I’ve switch the model to a NSObject subclass, and now KVOing, it fixes the initial problem I was having with Revert…

https://www.ilike.co.nz/downloads/kvoDocumentTest.zip

any feedback gratefully received
Bill Pitcher
bill@ilike.co.nz

On 14/07/2018, at 2:25 AM, Quincey Morris <quinceymorris@rivergatesoftware.com> wrote:

On Jul 13, 2018, at 03:49 , Bill Pitcher <bill@ilike.co.nz> wrote:

Unless I’m mistaken KVO requires going back to NSObject for my model, and not plain Swift classes.
Well, things get more complicated when you use non-Obj-C types, but it’s not necessarily true that you can’t use Swift types. In the case of the model, it may be sufficient that your “model" property of NSDocument be @objc (but not a Swift-native type), to KVO-observe it.

WindowController.document is Any? so doesn’t seem observable but I didn’t spend long on checking that.
It’s still an @obj property of an Obj-C object, so it’s observable. The type is actually ‘id’, which translates into Swift ‘Any’ or ‘Any?’ these days, not into ‘AnyObject’ as it used to.

What I think you are saying is build a KVO compatible model and then link in observers through to all the Controllers.
I wasn’t really saying that, just saying you should be able to use KVO to keep track of the model’s root object. Whether you use KVO or Obj-C compatible objects or properties further down the line is up to you.

Note, however, that there is still a *lot* in built-in pressure to stay Obj-C-compatible in this part of your app design, because a lot of familiar design patterns rely on KVO and KVC.


Quincey Morris
 

On Jul 23, 2018, at 16:56 , Bill Pitcher <bill@...> wrote:

check that I’m still not Doing it Wrong ™

It’s more or less correct, although I think you’re still making it a bit harder than it needs to be. Here are some inline comments on your window controller:

class WindowController: NSWindowController {


@objc dynamic var kvoDocument: Document? // NSWindowController.document is AnyObject? so keep a KVO reference so we can observe the model

You don’t actually need this. The real “document” property is type ‘id’ in Obj-C because it’s following a very old Obj-C convention, and that happens to come across to Swift as “AnyObject?”. It is, however, *always* a subclass of NSDocument (unless of course you futzed with the document machinery, but in this case you know you didn’t).

KVC/KVO doesn’t care what the compile-time type of the property is, so you don’t need this auxiliary property, but can refer to the real one with a keypath. See below.

override func windowDidLoad() {
super.windowDidLoad()
// Do any additional setup after loading the view.
self.addObserver(self, forKeyPath: #keyPath(document) , options: [.old, .new], context: nil)
self.addObserver(self, forKeyPath: #keyPath(kvoDocument.model) , options: [.old, .new], context: nil)
}

— Swift has an observation overlay that’s much easier to use than the Obj-C ‘addObserver’ method. See below.

— Since you’re observing a keypath with 2 components, you don’t need to observe the first component separately. If the “document” property changes, there will always be a change notification for “document.model” too.

override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) {
if keyPath == #keyPath(document) {
if let heldDocument = document as? Document {
self.kvoDocument = heldDocument
}
} else if keyPath == #keyPath(kvoDocument.model) {
if let heldViewController = self.contentViewController as? ViewController {
if let heldDocument = kvoDocument {
heldViewController.documentModel = heldDocument.model
heldViewController.documentUpdateChangeCountHandler = heldDocument.documentUpdateChangeCountHandler
}
}
}
}


deinit {
self.removeObserver(self, forKeyPath: #keyPath(document))
self.removeObserver(self, forKeyPath: #keyPath(kvoDocument.model))
}

With the Swift overlay, the awkward deinit isn’t necessary, either.

}

Revised version:

class WindowController: NSWindowController {
    private var documentObservation: NSKeyValueObservation!
    override func windowDidLoad() {
        super.windowDidLoad()
        documentObservation = self.observe(\WindowController.document.model) {
            windowController,_ in
            if let heldViewController = windowController.contentViewController as? ViewController,
                let document = windowController.document as? Document {
                heldViewController.documentModel = document.model
                heldViewController.documentUpdateChangeCountHandler = document.documentUpdateChangeCountHandler
            }
        }
    }

It’s not just shorter, but it’s clearer too, thanks to Swift’s improved ‘observe’ method. Note that the closure uses its window controller parameter rather than ‘self’. (If you capture self in this closure, you have to do it weakly, since a reference to the closure is stored and that would create a reference cycle with ’self’.)

To be pedantic, the “document” property of the window controller isn’t documented as being KVO compliant, so it’s not strictly safe to observe it. However, it always has been KVO compliant in practice, so it’s 101% unlikely that will ever change.

Note, also, that I removed the test whether the document is nil when updating the view controller. If that ever happens (and you may engineer it that way when things get more complicated, or it may happen temporarily when the window is being closed), you want to make sure it’s nil in the view controller, too. Otherwise, you may run into a reference cycle between your view controller and your document object, and that can lead to a hard-to-debug crash.

Related, in the view controller, I wouldn’t set the “documentModel” property initially to an empty document instance. When your document subclass gets more complicated, creating it (or destroying one) may have side effects, and you don’t want to cause problems via this empty instance.

A better choice is to declare it as an IUO:

var documentModel: DocumentModel!

and to be aware that it might actually be nil at certain times.



Bill Pitcher
 

K, the problem I’ve been working around with the kvoDocumet bs

documentObservation = self.observe(\WindowController.document.model) {

error "Type of expression is ambiguous without more context” under the \

confused
Bill Pitcher
bill@ilike.co.nz

On 24/07/2018, at 6:02 PM, Quincey Morris <quinceymorris@rivergatesoftware.com> wrote:

On Jul 23, 2018, at 16:56 , Bill Pitcher <bill@ilike.co.nz> wrote:

check that I’m still not Doing it Wrong ™
It’s more or less correct, although I think you’re still making it a bit harder than it needs to be. Here are some inline comments on your window controller:

class WindowController: NSWindowController {

@objc dynamic var kvoDocument: Document? // NSWindowController.document is AnyObject? so keep a KVO reference so we can observe the model
You don’t actually need this. The real “document” property is type ‘id’ in Obj-C because it’s following a very old Obj-C convention, and that happens to come across to Swift as “AnyObject?”. It is, however, *always* a subclass of NSDocument (unless of course you futzed with the document machinery, but in this case you know you didn’t).

KVC/KVO doesn’t care what the compile-time type of the property is, so you don’t need this auxiliary property, but can refer to the real one with a keypath. See below.

override func windowDidLoad() {
super.windowDidLoad()
// Do any additional setup after loading the view.
self.addObserver(self, forKeyPath: #keyPath(document) , options: [.old, .new], context: nil)
self.addObserver(self, forKeyPath: #keyPath(kvoDocument.model) , options: [.old, .new], context: nil)
}
— Swift has an observation overlay that’s much easier to use than the Obj-C ‘addObserver’ method. See below.

— Since you’re observing a keypath with 2 components, you don’t need to observe the first component separately. If the “document” property changes, there will always be a change notification for “document.model” too.

override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) {
if keyPath == #keyPath(document) {
if let heldDocument = document as? Document {
self.kvoDocument = heldDocument
}
} else if keyPath == #keyPath(kvoDocument.model) {
if let heldViewController = self.contentViewController as? ViewController {
if let heldDocument = kvoDocument {
heldViewController.documentModel = heldDocument.model
heldViewController.documentUpdateChangeCountHandler = heldDocument.documentUpdateChangeCountHandler
}
}
}
}

deinit {
self.removeObserver(self, forKeyPath: #keyPath(document))
self.removeObserver(self, forKeyPath: #keyPath(kvoDocument.model))
}
With the Swift overlay, the awkward deinit isn’t necessary, either.

}
Revised version:

class WindowController: NSWindowController {
private var documentObservation: NSKeyValueObservation!
override func windowDidLoad() {
super.windowDidLoad()
documentObservation = self.observe(\WindowController.document.model) {
windowController,_ in
if let heldViewController = windowController.contentViewController as? ViewController,
let document = windowController.document as? Document {
heldViewController.documentModel = document.model
heldViewController.documentUpdateChangeCountHandler = document.documentUpdateChangeCountHandler
}
}
}
It’s not just shorter, but it’s clearer too, thanks to Swift’s improved ‘observe’ method. Note that the closure uses its window controller parameter rather than ‘self’. (If you capture self in this closure, you have to do it weakly, since a reference to the closure is stored and that would create a reference cycle with ’self’.)

To be pedantic, the “document” property of the window controller isn’t documented as being KVO compliant, so it’s not strictly safe to observe it. However, it always has been KVO compliant in practice, so it’s 101% unlikely that will ever change.

Note, also, that I removed the test whether the document is nil when updating the view controller. If that ever happens (and you may engineer it that way when things get more complicated, or it may happen temporarily when the window is being closed), you want to make sure it’s nil in the view controller, too. Otherwise, you may run into a reference cycle between your view controller and your document object, and that can lead to a hard-to-debug crash.

Related, in the view controller, I wouldn’t set the “documentModel” property initially to an empty document instance. When your document subclass gets more complicated, creating it (or destroying one) may have side effects, and you don’t want to cause problems via this empty instance.

A better choice is to declare it as an IUO:

var documentModel: DocumentModel!
and to be aware that it might actually be nil at certain times.



Quincey Morris
 

On Jul 23, 2018, at 23:02 , Quincey Morris <quinceymorris@...> wrote:

Revised version:

Well, dang, I got that wrong. (I “fixed” a typo in the email, forgetting to check that it actually worked.) You can’t use a KeyPath expression like “\WindowController.document.model” in Swift, because “document” has the wrong compile-time type.

An alternative is to use ‘addObserver’ after all and specify a String keypath. Or, you can go ahead an make an extra document property of the correct type, like you did originally. Here’s a slightly cleaner way that can still use the ‘observe’ method:

class WindowController: NSWindowController {
    private var documentObservation: NSKeyValueObservation!
    @objc dynamic private var windowDocument: Document? {
        return document as? Document
    }
    @objc static var keyPathsForValuesAffectingWindowDocument: Set<String> {
        return ["document"]
    }
    override func windowDidLoad() {
        super.windowDidLoad()
        documentObservation = self.observe(\WindowController.windowDocument?.model) {
            windowController,_ in
            if let heldViewController = windowController.contentViewController as? ViewController,
                let document = windowController.windowDocument {
                heldViewController.documentModel = document.model
                heldViewController.documentUpdateChangeCountHandler = document.documentUpdateChangeCountHandler
            }
        }
    }
}

This derived “windowDocument” property is correctly KVO compliant through its use of the “keyPathsForValuesAffecting<Key>” convenience method. It’s a bit of a mouthful, but it’s easier than doing a manual observation.


Bill Pitcher
 

Quincey Morris
You truly are a Hero! and made an old hacker very happy

thank you
Bill Pitcher
bill@ilike.co.nz



On 24/07/2018, at 6:24 PM, Quincey Morris <quinceymorris@rivergatesoftware.com> wrote:

On Jul 23, 2018, at 23:02 , Quincey Morris <quinceymorris@rivergatesoftware.com> wrote:

Revised version:
Well, dang, I got that wrong. (I “fixed” a typo in the email, forgetting to check that it actually worked.) You can’t use a KeyPath expression like “\WindowController.document.model” in Swift, because “document” has the wrong compile-time type.

An alternative is to use ‘addObserver’ after all and specify a String keypath. Or, you can go ahead an make an extra document property of the correct type, like you did originally. Here’s a slightly cleaner way that can still use the ‘observe’ method:

class WindowController: NSWindowController {
private var documentObservation: NSKeyValueObservation!
@objc dynamic private var windowDocument: Document? {
return document as? Document
}
@objc static var keyPathsForValuesAffectingWindowDocument: Set<String> {
return ["document"]
}
override func windowDidLoad() {
super.windowDidLoad()
documentObservation = self.observe(\WindowController.windowDocument?.model) {
windowController,_ in
if let heldViewController = windowController.contentViewController as? ViewController,
let document = windowController.windowDocument {
heldViewController.documentModel = document.model
heldViewController.documentUpdateChangeCountHandler = document.documentUpdateChangeCountHandler
}
}
}
}
This derived “windowDocument” property is correctly KVO compliant through its use of the “keyPathsForValuesAffecting<Key>” convenience method. It’s a bit of a mouthful, but it’s easier than doing a manual observation.