search

OliverLetterer / SLRESTfulCoreData

Objc naming conventions, autogenerated accessors at runtime, URL substitutions and intelligent attribute mapping
MIT License
178 stars 13 forks source link

Parse for fetched properties #26

Closed itsthejb closed 10 years ago

itsthejb commented 10 years ago

Hi Oliver,

Feature suggestion here: in our model we can receive child entities in a number of different ways - the relationships are quite complex and there are a range of endpoints for fetching data in various circumstances. It's not quite your straightforward artists/album/track sort of hierarchy :wink:

As things have been progressing I've been increasingly refactoring direct relationships into fetched properties since this serves our needs better. It eventually got to the point where I started wanting to break up even direct relationships so that the model became even more dynamic.

TL;DR I've added a rough initial implementation of checking for and parsing child entities that are actually accessed through fetched properties in practice. For example, even though you might receive an artist/album/track hierarchy I want the artist->album and album->track relationships to be one-way relationships via fetched properties.

This isn't too hard to achieve, just consider the fetched properties when parsing relationships, create/fetch the objects as normal and there's also no need to assign the collections (in either direction).

I'm sure you could implement this cleaner and there are probably edge cases I haven't considered since I did this quite quickly in order to keep moving.

In any case, if you think this would be useful as an upstream feature then let's chat about it. :smile:

PS. Keep meaning to respond to a couple of other PRs/issues but have been too busy. Soon, hopefully!

Thanks

OliverLetterer commented 10 years ago 
PS. Keep meaning to respond to a couple of other PRs/issues but have been too busy. Soon, hopefully!

Ahmen to that :D This whole project is actually due for a larger refactoring and a more modern and modular architecture but I can't find any time for that :(

Although I have never used NSFetchedPropertyDescription so far, I think supporting it makes definitely sense. I'm not sure yet if this is a feature which should be supported in the SLRESTfulCoreData Core:

The issue I see here is that SLRESTfulCoreData cannot make any promises that the objects returned from a NSFetchedPropertyDescription relation are in any way related to the objects fetched from the API. Therefore the deleteEveryOtherObject flag might get quiet dangerous. Maybe an extension (even first hand extension coming from this repository) makes more sense? What do You think?

My approach would be a new CocoaPod SLRESTfulCoreData+NSFetchedPropertyDescription which contains just a single .m file with a category on NSManagedObject. The category would swizzle -[NSManagedObject fetchObjectsForRelationship:fromURL:deleteEveryOtherObject:completionHandler:] and check if the relationship is a NSFetchedPropertyDescription and implement Your approach from this pull request.

itsthejb commented 10 years ago

Hey Oliver,

Seems a very small feature for a sub-spec. Perhaps it would make more sense to specify this in the mapping description somehow?

As for the deleteEveryOtherObject flag, this has already been a little awkward for me and I raised a while ago! You mentioned it was a relic of your own original proprietary use cases. Perhaps this is something that isn't really useful for the wider public?

Very interested to hear about a major refactoring. Once our v1 has launched I may have free time to contribute.

OliverLetterer commented 10 years ago

I introduced support for fetched properties with 51c10fa7a35856728bd903654dd97f01d100bbd1 and decided to take a little different approach:

Does that make sense for Your use case?

itsthejb commented 10 years ago

Hey Oliver.

I'll check this out in the week and let you know.

itsthejb commented 10 years ago

Getting regressions with this at the moment which I don't have time to look into right now - but this is probably my own issues. Will report back when I have less pressure coming from elsewhere :wink:

itsthejb commented 10 years ago

Hi Oliver,

Just had a chance to look into this properly. It doesn't currently seem to support our use case since this code path only seems to be hit from the CRUD features. We're not using those since they don't serve our needs. What we want if you just be able to create objects for one-way to-many relationships that are children of an object in JSON, but we don't want the 'hard' link of a normal to-many relationship. Hence why we use fetched properties instead.

Doesn't seem like to would need too much to feature this in the normal parsing code path as I did in my original implementation? In any case, very keen to use your upstream implementation rather than mine if at all possible.

Thanks!

OliverLetterer commented 10 years ago

Can't You use +[NSManagedObject fetchObjectsFromURL:deleteEveryOtherObject:NO withCompletionHandler:] in Your own code when You can't use the CRUD features?