Entity inheritance
Closed this issue · 16 comments
MMRecordRepresentation is built on NSEntityDescription but does not keep in account entity inheritance: i.e. it would seem reasonable that a sub-entity check if its parent entity defines a MMRecordEntityPrimaryAttributeKey.
Good point, again!
I want to make sure I fully understand the issue here. Can you describe the use-case in a bit more detail? I agree with you in principle, that it does make sense to have a check like that, but I want to make sure I clearly understand the problem it solves.
Also, I'd be curious to know if you're already using -shouldUseSubEntityRecordClassToRepresentData and whether or not that is helpful or pertains to this at all.
Thanks,
- Conrad
In the scenario I am currently working on, every entity has some attributes in common (an id, a title and a text, possibly some images connected), then every sub entity has its proper ones. I can't use -shouldUseSubEntityRecordClassToRepresentData because the representation does not hold metadata information that would help me to say which entity should I refer to (well I could check presence/absence of properties, but..). I have a set of different URNs instead, each returning different entities.
To tell the truth this scenario can be handled with MMRecord in a set of ways (i.e. giving inheritance up and creating a distinct entity for each URN), but actually the (one of the?) great thing about MMRecord is its integration in CoreData UI, and this particular pattern is someway natural to be adopted working with CoreData. On the other side it is difficult to debug.
I'm definitely interested in supporting this. I've added this to my todo list to investigate for the next release. If you have an idea already of how you think it should support it, please do submit a pull request.
For what its worth, the way we supported something similar in a recent project was by using the "dynamic" representation that simply stores the original dictionary in a transformable attribute. Then each subclass overrode the getters for various properties and returned the appropriate values for them, possibly with some extras. It wasn't as clean as straight up Core Data Model Editor configuration, but it was effective.
Thanks,
- Conrad
So does the PR we accepted fully address this issue, or are there more concerns about how sub entities should be handled?
Well, this PR goes upside-down, from parent entities to children until it finds a subclass that can handle that specific representation. It assumes that representations contain metadata that help to match with the class they represent, so it does not cover the scenario which I described here (I know the object returned by a specific URN but representation does not contain metadata about which entity is represented).
The inheritance handling I describe goes the opposite way (downside-up): mapping between URN and entities is being made in the implementation, then the primary key is being inherited by parent entities traversing the model graph. Maybe I could try to submit a PR as soon as I find time to, but I wouldn't close this for now..
Yep, I won't close it. I'd love to see a PR at some point.
"Mapping between URN and entities" sounds kind of like what has to happen when you decide to start a request in the first place. One of the general assumptions we're making is that when you make a "Tweet" request to a given URN that it returns tweets. Are you saying that the issue is that assumption may not be valid?
I'm over at CocoaConf today, but I'll try to think more about this later this weekend.
Thanks,
- Conrad
Here it is: #57
Sorry I didn't test it thoroughly but I think the idea is clearer in code..
Enjoy CocoaConf!
Oh, ok, this makes much more sense now 👍
:)
I know that PR is sort of a pseudo draft. I'll make a few comments on it, but before we finalize it I guess the last question I have is whether or not this should flow from sub-entities up through super entities, or from super entities down through sub-entities.
I'm trying to understand what the benefit of this is. Is the use-case that you define a new entity in the model editor, and that you simply don't have to assign its primary key on the userInfo dictionary and instead you can get it straight from the super entity or sub entity? Is that a really significant issue?
I feel like this should also be possible by subclassing the representation, but it doesn't look like it actually is. I'm actually more interested in fixing that than even in fixing this particular issue :)
I'm trying to understand what the benefit of this is. Is the use-case that you define a new entity in the model editor, and that you simply don't have to assign its primary key on the userInfo dictionary and instead you can get it straight from the super entity or sub entity?
Yes! :)
Is that a really significant issue?
Well, probably not, but it depends on what you mean with significant: as I said, this scenario can be already handled in a whole set of different ways using current MMRecord implementation. The point here is that CoreData introduces the concept of entity inheritance, and since MMRecord sits on CoreData I would expect as a developer that every property defined for an entity in MMRecord domain would be inherited by children entities. It is simply weird that it does not, and before you discover it you have to pass some time debugging and trying to figure out if something is wrong with your code or what. So maybe I would say that it is significant from the point of view of the API user experience, and that it would help the developer approaching MMRecord for the first time not to get lost.
I agree, its worth supporting it :)
I accepted your PR and followed it up with one of my own. Please let me know what you think.
I decided to move the inheritance piece into a private method that will be called no matter what in the initializer, meaning that the class will always support inheriting keys from a super entity. Then, I defined a new method that is public and supports subclassing. The idea is that method is the one that gets called repeatedly, and gives people a chance to define different behavior for determining what the public key is, like if they don't want to use the userInfo dictionary, for example.
Also added some docs :)
Let me know what you think.
That's it! 🍺
On a second thought: what does it happen if I subclass MMRecordRepresentation, override primaryKeyPropertyNameForEntityDescription: and return a key name that does not correspond to any property defined in the NSEntityDescription passed as a parameter?
Since it is meant to be subclassed, maybe I would check in representationPrimaryKeyForEntityDescription: if the key is acceptable and raise an exception if it is not.
Thats a good point. An error needs to be handled if the returned string does not map to a property on that entity description. Good call.
Circling back on this. I'm adding a todo to revisit the error handling here. I'm sort of starting to re-think how errors are handled across the whole library, so I'm going to hold off making a decision here until that gets sorted out.