Recreating objects

[skurfer]

If a QSObject isn't in the catalog, it doesn't exist after a restart. This has been a problem with triggers for a long time, and will soon be a problem for Synonyms.

I think in the majority of cases, the various object sources could probably recreate the object knowing only the identifier. So, what I propose is that in +[QSObject objectWithIdentifier:], we continue to return an existing object if possible, but if it doesn't exist, do something like this:

SEL recreate = @selector(recreateObjectWithIdentifier:);
id handler = [self handlerForSelector:selector];
if (handler) {
    return [handler recreateObjectWithIdentifier:anIdentifier];
}

And then of course we'd have to implement recreateObjectWithIdentifier: in various object classes, and make sure objectWithIdentifier: gets called at some point to trigger the behavior.

Places to test:

• [ ] Triggers made with internal objects (e.g. Pasteboard ⇥ Paste) should still work once the catalog entry's been disabled (see the 'Internal Objects' catalog entry). See #1373 for more info

[pjrobertson]

There's already a method similar to recreateObjectWithIdentifier: in QSObject. It's called makeObjectWithIdentifier

This tries to return an existing object, otherwise it creates the object. Perhaps that method just needs to be better so that it actually figures out what the object is (using the identifier) and sets the appropriate types/name etc.

[skurfer]

There's already a method similar to recreateObjectWithIdentifier: in QSObject.

I knew that would end up being the case, but I couldn't find anything.

It's called makeObjectWithIdentifier This tries to return an existing object, otherwise it creates the object.

Actually, I knew about that one and use it all the time, but I only use it in cases where I know in advance such an object doesn't exist. I think the check for existing objects is just to prevent dupes.

Perhaps that method just needs to be better so that it actually figures out what the object is (using the identifier) and sets the appropriate types/name etc.

So yeah, we could make the changes there instead. Although, in my mind makeObjectWithIdentifier: is for explicitly creating a new object, while objectWithIdentifier: is for retrieving one that already exists (or should exist). So I'd prefer to modify the latter for this.

Probably irrelevant though. It occurred to me this morning that using [self blah] to ask for information like type and handler inside a class method is going to be pretty disappointing. :-) We probably need a new method. Something like objectFromDictionary: that works like objectFromFile:. But the most important thing (for finding the right handler) is the type, and that isn't stored along with the identifier in the Triggers.plist, so we might be out of luck there unless we're willing to store more information with each trigger. Hmmm.

[skurfer]

We probably need a new method. Something like objectFromDictionary: that works like objectFromFile:.

Such a thing already exists in QSObject (PropertyList).

[pjrobertson]

Just as a sidenote from here, it seems like the displays module has trouble recreating objects from triggers.

[skurfer]

it seems like the displays module has trouble recreating objects from triggers.

I saw that. I was going to reply, but I won't if you are.

[pjrobertson]

I saw that. I was going to reply, but I won't if you are

Looks like I did, but off the list. I've forwarded my reply to the list now

On 10 January 2013 13:39, Rob McBroom notifications@github.com wrote:

it seems like the displays module has trouble recreating objects from triggers.

I saw that. I was going to reply, but I won't if you are.

— Reply to this email directly or view it on GitHubhttps://github.com/quicksilver/Quicksilver/issues/1277#issuecomment-12096898.

[skurfer]

OK, let's start simple. Am I right about these two things?

1. Given an identifier and a handler (object source), we should be able to recreate any object.
2. The primary type can reliably lead us to the right handler.

[skurfer]

2 is wrong. Proxy objects have the same type, but the handlers are all over the place. So my plan is to store the name of the handler class along-side the identifier in places where this matters. (Triggers, synonyms, etc.)

It would be nice to keep it all together in a dictionary, but that would require existing triggers to be restructured.

Correction

The handler for proxies is always the same, too. It's the provider that differs. So we can store type after all.

[tiennou]

I'm posting notes because that's something I tried to fix (see c0dcb2c56d5f5ce7ee930b791f11c44fd6f4e1c4). My approach was to piggy-back on -dictionaryRepresentation and use that to recreate objects (that's what the trigger system uses if an object has no identifier). I added some class key to the dictionary returned (to help the runtime in reconstructing objects with their correct class, like QSBasicObject not being a QSProxyObject), but that's about all. I also considered using the handler to do that (with a new objectFromDictionary: method) but never completed because it didn't feel right (though now, almost 5 years ago, I'm not sure anymore ;-)). IIRC the rational was that any object, given its class and the contents of its data dictionary, ought to be able to reconstruct itself, or use its handler if that's not possible (think QSProcessType). It's an example of why the QS typing system is interesting, because those are both file-system objects and processes objects at the same time.

[skurfer]

Thanks for the notes. I was thinking we could have objectWithIdentifier: recreate them for us, but I suppose there would need to be a new method that took both identifier and type, so that will be messier than I originally thought.

That might still be the way to go. I'll think about it tonight. I'm not too worried about missing other information, like multiple types. After all, the catalog source that adds applications to the catalog doesn't know some of its objects might become "running processes" too, but it still works.

Storing the entire dictionary representation seems heavy handed, but it might be a good option at this point. Unfortunately, like I said, whatever we do has to be stored in addition to the identifier. We can't replace it without breaking all existing triggers, commands, etc.

And yes, I was thinking each object source would need to implement a new method that takes an identifier and returns a valid object. It feels OK to me. :-)

[pjrobertson]

I think I'm a bit lost in all this but I think I just about follow. This all sounds plausible to me, I'd just like to throw some spanners in the works:

Related to fa6a99054be587af8b98e27a5d796910350bd6bf How can a primaryType be converted into a class/used to find the provider class. Is this all stored in QSReg already? (Actually now I think about it, it might be)

and also, how will this new fancy identifier method deal with:

• Multiple objects with different primary types/handles
• QSCommand objects (I think these are actually fine)

[tiennou]

IIRC the current serialization method (a.k.a -dictionaryRepresentation) works by saving the data and meta dictionaries from QSObject so in theory objects that are subclasses of it ought to work (again, this is what triggers should use if the object they're saving has no identifier, a.k.a direct/action/indirectArchive). meta contains the identifier, so I think everything that's needed is available ;-).

I added the class thing because I don't think QSReg has that info — but that would work too, except it would have to be provided in the plugin Info.plist files (or maintained at runtime through +load or some other mean), and I don't think that's the best way to go.

Maybe add an Object Source for those 'temporary' objects ? Feels wrong ;-). Additionally, clearing the caches would drop them completely (which is something to think about because those very objects aren't in the catalog).

I'd say investigate whether -dictRep works for this purpose and try to use that as a basis. IIRC its main problem is non-plist-objects exceptions ;-).

[skurfer]

How can a primaryType be converted into a class/used to find the provider class. Is this all stored in QSReg already? (Actually now I think about it, it might be)

It definitely works. This is how it figures out where to look for iconForObject:, detailsOfObject:, objectHasChildren:, loadChildrenForObject:, etc. In fact, I think that's all primaryType is for. In other contexts, all types are on equal footing. I'm proposing that we add something like recreateObjectOfType:withIdentifier: to that list.

The more I think about it (and the more I listen to past woes), the more I think we should be using type/identifier and not trying to archive a full-blown dictionary. I could go on and on about it, but I think my time is better spent writing code to see if I'm right. :-)

RE: objects with no identifier @tiennou… I think that's a separate issue. I don't think that's very common, and I think the fix in those cases is to find out where the objects come from and assign an identifier. I'm more worried about the many, many places where we do have an identifier for something that isn't in the catalog.

[tiennou]

How can a primaryType be converted into a class/used to find the provider class. Is this all stored in QSReg already? (Actually now I think about it, it might be)

Yeah, what I have written yesterday is unclear : there's a Primary Type => Object Handler relation published in QSReg. But there's nothing that actually tells the runtime which class it should instantiate at deserialization, that's what the class key I added to -dictRep is for.

@skurfer: I'll let you code then ;-). I actually banged my head pretty hard against that very problem not too long ago — or was it ? ;-) — if someone disabled a plugin :

• trigger class missing at runtime, exception => blank Triggers prefpane
• an object failing serialization because it wasn't plist-compatible, exception => no Triggers.plist for you

Keep in mind plugins will get disabled, and then that object's provider won't be there to help you anymore ;-).

[pjrobertson]

This is a big job, but could it be a task for the sprint?

[skurfer]

Maybe. I’ve already done it and still have the branch, but I’m sure it needs more work at this point.

If you could look over #1363 and see if you have any comments/concerns about my chosen implementation, that would help.

[tiennou]

Wall of text incoming, apologies in advance. I'll comment here speaking about #1363 because let's say that this is the meta issue (metapullrequest incoming 😉).

Rather than try to preserve objects in dictionary form, we just ask the class that created them in the first place to create them again.

I can think of a bunch of cases where the identifier* is not sufficient to recreate something. For example, menu items from the Accessibility plugin (which I think people use to rebind Triggers).

* unless you craft it carefully, which I'm not fond of. That would need identifiers like bundle-id->menu->menu->... My personal point of view is that identifiers should be mandatory NSUUIDs and be done with it. They don't convey any meaning, they're just identifiers used to check if we already have that object "in cache" (e.g. the Catalog + the set of runtime objects). With maybe a few wrinkles because filesystem objects must be kept fast.

All in all, I'd really much prefer the mechanism to use the object's -dictionaryRepresentation in some way, because that's the only general way I can think of that would work for any case.

Now, looking at how the code is orchestrated, I'm left with wondering if this isn't a mostly-solved problem : the Catalog maintains a list of "indexes" (one for each object source). Maybe we could fix the trigger problem by always serializing everything, instead of the current "if it has an identifier, then save that, else save this, or else save whatever" ? This means that when triggers are saved, the whole representation is there for the next time and can be reconstructed. Then the same goes for synonyms, where maybe just saying that -entryCanBeIndexed: is sufficient :wink: ?

[pjrobertson]

Maybe we could fix the trigger problem by always serializing everything, instead of the current "if it has an identifier, then save that, else save this, or else save whatever" ?

You mean, like... using NSCoding? Here's a PR I made earlier (like 2.5 years earlier!) https://github.com/quicksilver/Quicksilver/pull/1761

I guess my NSCoding implementation is effectively the same as Etienne's dictionaryRepresentation (basically I'm just encoding the QSObject's identifier and data dictionary into an NSCoding object, but it's muuuch cleaner

[tiennou]

Or using -dictionaryRepresentation, which makes it easier to hand-modify (provided you don't store unwanted things in data, but NSCoding has the same issue anyway).

Please notice that QSTrigger's -dictionaryRepresentation is messy "by design", because it actively tries to not duplicate objects already in the Catalog. The one in QSObject is much cleaner than that, and is already used by the indexing machinery (and Triggers, for that matter).

I don't really have a preference between NSCoding and NSPropertyListSerialization anyway. I find the latter easier to use because it's text-based (you don't have to use plutil/PlistBuddy), that's all.

[skurfer]

My personal point of view is that identifiers should be mandatory NSUUIDs and be done with it. They don't convey any meaning, they're just identifiers used to check if we already have that object "in cache" (e.g. the Catalog + the set of runtime objects).

That makes dealing with identifiers much simpler, and more importantly (assuming some central "authority" like QSLibrarian was assigning them), you eliminate the risk of collisions.

However, as things are now, the identifier is meaningful to the object source and even if it won't always work, it's often the only way an object source can get enough information to recreate something.

Now, I already know the response to that because I've heard it from both of you repeatedly over the years, including in these very comments…

All in all, I'd really much prefer the mechanism to use the object's -dictionaryRepresentation in some way, because that's the only general way I can think of that would work for any case.

That would not work for every case. Off the top of my head, any subclass of SBObject (iTunes EQ presets, AirPlay devices) can't be stuffed in a property list.

I think you've said in the past that those things shouldn't be stored in the object's type dictionary. So, say we throw it out. Now those objects don't do anything. What difference does it make that we can safely store and retrieve them from disk?

I prefer letting plug-in developers throw whatever they want in there, because it makes life easier for them, and really, how would we enforce type limitations in setObject:forType: anyway?

Having said that, what you're shooting for (either dictionaries or NSCoding) is so much cleaner and it would be great if it worked. So, give me a way to use centrally assigned UUIDs, property-list-safe types that can be stored to disk, etc. but when it comes time to run an action, still have access to what the action needs to work. (Some form of SBObject for example.)

At the very least, the information in the dictionary needs to convey that something important is missing.

[skurfer]

Here’s an idea (for getting to a point where QSObjects can be saved to disk in their entirety). We already have a dictionary that can tell us information about a type: QSTypeDefinitions. We could add a “can be indexed” boolean to that dictionary (defaulting to YES).

This would take the place of entryCanBeIndexed: on catalog entries. So we’d say which types are or aren’t safe to index, instead of which catalog entries. That makes more sense anyway, in my opinion.

There’s still the issue of what to do with the non-indexable types. Just have the NSCoding methods store a placeholder for that type, and then ask the handler to fill it in when it gets “woken up” from disk, I think.

Restoring the value for those types could still be challenge, but that’s probably unavoidable. The advantage here is that we can pass the handler the full object instead of just type and identifier.

[stale[bot]]

This issue hasn't been updated in over 2 years. It has been marked as 'stale' and will be closed in 30 days. Please check whether this is still an issue with the latest version of Quicksilver. If so, update or comment on this issue to keep it open.

[pjrobertson]

I've just (properly) implemented NSCoding into QSObject (and QSCommand and QSTrigger) so you should now be able to write any object/command/trigger to disk with an NSKeyedArchiver. Should be really easy, as I showed previously here: #1761

I plan on moving triggers etc. over to just writing to the disk using NSKeyedArchver, then all these problems should just go away.

For the question of:

Having said that, what you're shooting for (either dictionaries or NSCoding) is so much cleaner and it would be great if it worked. So, give me a way to use centrally assigned UUIDs, property-list-safe types that can be stored to disk, etc. but when it comes time to run an action, still have access to what the action needs to work. (Some form of SBObject for example.)

If you wanted to save the SBObject to disk, then that would be the responsibility of the QSObjectHandler of your plugin. We could extend QSObjectHandler to have a method called -[QSObjectHandler archiveObject:XXX forType:YYY] that would do whatever it wants with archived data. Otherwise, we just throw it out as you say.