Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deleting orphans #4

Closed
awoolf opened this issue Apr 15, 2013 · 12 comments
Closed

Deleting orphans #4

awoolf opened this issue Apr 15, 2013 · 12 comments
Milestone

Comments

@awoolf
Copy link

awoolf commented Apr 15, 2013

Is there a best practice for deleting orphans in core data? My first pass at this was to add code in the result block of the startRequestWithURN: call. The only problem I see with this is that the managed object context gets saved once for the new objects added, then saved again after I delete the orphans.

[CSTripSegment startRequestWithURN:@"TestJSON_TripSegments"
                              data:nil
                           context:moc
                            domain:self
                       resultBlock:^(NSArray *records) {

                           // Find and delete orphan trip segements

                           NSArray *priorTripSegments = [self tripSegmentsInContext:moc];
                           NSMutableArray *orphanTripSegments = [NSMutableArray array];

                           for (CSTripSegment *priorTripSegment in priorTripSegments) {
                               if ([records containsObject:priorTripSegment] == NO) {
                                   [orphanTripSegments addObject:priorTripSegment];
                               }
                           }

                           for (CSTripSegment *orphanTripSegment in orphanTripSegments) {
                               [moc deleteObject:orphanTripSegment];   // Yes, this could probably be done in the prior loop safely.
                           }

                           if (orphanTripSegments.count > 0) {
                               NSError *error;
                               [moc save:&error];
                               // !!!: Add error handling
                           }

                       }
                      failureBlock:^(NSError *error) {
                      }];

I love what you have built! Thank for sharing this.

@cnstoll
Copy link
Contributor

cnstoll commented Apr 16, 2013

Hola,

Thanks for bringing this up. I think that this is an important use case, and I want to make sure I fully understand the issue from all angles. We have indeed run into this before, and we've handled it differently each time. Because of that, I'm not sure if MMRecord can provide native support for this...but perhaps it can do something to make the process easier for folks.

So, here's a few questions.

  1. Are the trip segments supposed to be persisted across app launches? Or are they more temporal (aka, search results style)?

  2. Is there ever a case where you would not want orphans to be deleted?

  3. Is there ever a case where some, but not all, orphans would be deleted? (perhaps based on a modification date)

  4. Would the server ever be responsible for telling you which objects you may have stored to delete?

Another approach that you might consider is simply deleting everything first, and then making the request. This should be the same, or faster, performance-wise, unless you are already showing data related to those objects in the UI (which would make this solution invalid).

The direction I think I want to head with this is providing some kind of hook where you could do the deletion yourself while the parsing operation is in flight for the request. I don't think we should make MMRecord do it for you, but I would think that we can find a way to make it easier.

I'm really glad that you like the library! Thank you again for the feedback. To reiterate, I think we should continue the discussion around this to find the right way to solve the problem.

Thanks,

  • Conrad

@awoolf
Copy link
Author

awoolf commented Apr 17, 2013

I like the direction your heading. Deleting orphans is complex. I can't imagine an automatic deletion of orphans working in most real world cases. Your idea of providing a hook while the parsing is in flight (or even complete) but before the core data save, would be great.

For this particular app, we are using core data's changes to update the UI (NSFetchedResultsController and core data notifications). We will have tables or collection views showing records while we ping the server for updates. It would be great to have the addition and deleting of record to be saved in one batch in order to avoid flicker and also have animations all start at the same time.

  1. Yes, we would like trip segments persisted mainly for offline access, but also for performance. FYI, for this project we are still deciding between Core Data with an in-memory store + HTTP caching vs. Core Data with SQLite persistence.

  2. Not deleting orphans could be desirable in a few cases:

  • 2a) We might request a subset of trip segments (by date range in this case) and we would only want to delete orphans in the date range. (This is more determining which records are orphans as opposed to "not deleting" them though)
  • 2b) This JSON response has children and grandchildren entities included. We are currently using cascading deletes in core data to clean up the children, but there is a hole. There is a grandchild record, User, that will have relationships to other entities so we will not know if it is an orphan just from the JSON. It would be easy to add a little code to the above example to look at the User objects and see if we are deleting its last relationship.
  • 2c) We will create new records on the device (trip segments in this case) that don't exist on the server yet. Off the top of my head there are several ways to handle this:
    • Cancel all our requests that could cause deleting before entering the trip segment create/edit screens.
    • Work in a different managed object context.
  1. 2b above is an the only example of some, but not all orphans being deleted that I have run across.

  2. The server specifying which records to delete is an interesting concept, but haven't gone there.

Thanks,
-Alex

@blakewatters
Copy link

Its worth mentioning that we have generalized orphan deletion support within RestKit that works in a couple of ways:

  1. You can register a block that returns an NSFetchRequest when given the NSURL for the URL being loaded. The fetch request is then executed and the set of objects mapped from the request is compared with the set returned by the fetch request and any orphans are deleted. It also supports deeply nested child entities as more than one fetch request can be returned for a given URL and the aggregate set of entities mapped is diffed with the aggregate set of fetched entities. This is actually pretty flexible and covers most of the orphan cases I've encountered where its practical to have an endpoint that returns a known set that can be compared. It becomes problematic with pagination scenarios, but in those cases you probably are better off with transient data anyway.
  2. The second approach that we support is the use of a 'tombstone' deletion style. This lets you specify a NSFetchRequest that will match any objects that the server has flagged for deletion. This is great if you do soft deletes on the server or maintain an activity log that can be used to send down a tombstone record in place of the deleted entity.

All of this deletion occurs before the request's MOC is saved, so it integrates cleanly with an NSFRC.

Anyway, just thought I'd chime in on how we've tackled the problem in RK as some food for thought. This is indeed a pretty challenging problem. Best of luck.

@cnstoll
Copy link
Contributor

cnstoll commented Apr 18, 2013

Hey Guys,

First, thanks Blake for chiming in! I had not seen that before, but I think that is a good approach. Pagination was indeed a scenario that I thought might be problematic. If we do implement this type of functionality here, I may want to disable it entirely for paged requests to keep things simple...we'll see.

Thanks for answering those questions Alex. I was trying to get an understanding for a) whether data could just be transient, as Blake mentioned, and b) how customizable this would need to be.

I think what I have in mind involves adding a block to the MMRecordOptions that can be used to support deleting orphans. I think using MMRecordOptions is appropriate, because I don't think this is something that will be common, and it's something that can be wrapped up in a method if it's repeatable in a specific person's use case.

The question I have now is whether or not to make it a generalized solution, using NSFetchRequest, or if I just want to let the user define a block that takes as parameters the request MOC, the parsed records, and the response object. In the latter scenario, the user would then be responsible for deleting the orphans manually, but it would give them an easy way to do that.

Any thoughts on the two approaches? I'm going to add this one to my todo list, but feel free to submit a pull request if anyone wants to take a stab at it.

Thanks,

  • Conrad

@kwylez
Copy link

kwylez commented May 28, 2013

Conrad, Blake, Alex

First let me say that I really enjoy the discussions here.

IMHO everyone, in one way or another, is making the case for the best solution as: well it depends. I don't think there is a good generic way to automagically handle orphan records. The business rules differ too much from app to app…mainly because there is a large server side component that is involved. For example, does a 404 for a record indicate that it is deleted and if so does that translate to trigger for deletion locally on the app? Should that also included related entities..etc.?

Keeping the client and server in sync is never an easy task. I've been making changes to my backend so that each resource has an event handler that is fired off on when a record is deleted and that uri is saved, along with a created_at property. What that allows for me to query for only those resources that have been deleted since the last update. This allows to keep web clients and native clients sync'd.

For example if I have a resource of photos (/photos) and when the app opens start two separate operations,

  1. for the new / updated records
  2. deleted records (1/deleted_photos?created_at=datetimestamp1)

From the response of the /deleted_photos resource I know that id's, and any other pertinent information, of the objects that need to be deleted with as little overhead as possible. This might not be feasible for all applications, but illustrates that rules and solutions will differ among developers and applications as inferred from:

The question I have now is whether or not to make it a generalized solution, using NSFetchRequest, or if I just want to let the user define a block that takes as parameters the request MOC, the parsed records, and the response object. In the latter scenario, the user would then be responsible for deleting the orphans manually, but it would give them an easy way to do that.

Once again, IMHO, this is the optimal solution. It allows for incorporation / support of the features that MMRecord provides while giving the user the ability to craft the logic to handle specific use cases per their business rules.

-Cory

@cnstoll
Copy link
Contributor

cnstoll commented May 30, 2013

Hi Cory,

Thanks very much for contributing the discussion :) I think that solution to deleting records is a very good one. In one project where we had a fair amount of influence over the backend systems, we implemented this using something similar. If a project has that kind of flexibility then they could likely implement that system without dependence on MMRecord. Is that what you are thinking as well?

I want to make sure I understand your last paragraph correctly. Are you saying that the optimal solution is the block based one where in "the user would be responsible for deleting the orphans manually..."? Or do you mean the generic NSFetchRequest one?

As a follow up question, if we were to go with a block based approach that was part of a given request operation, how would you see that interacting with the scenario you described above with a deleted_photos endpoint?

Thanks,

  • Conrad

@kwylez
Copy link

kwylez commented May 31, 2013

Conrad,

Yes…I'm advocating that the user handle deletions manually. I liked your user defined block based approach

…the user define a block that takes as parameters the request MOC, the parsed records, and the response object."

I'm not sure if it is the best approach, but in my scenario I would add a dependent NSOperation that would "query" the deleted_photos, return an tombstone id's then iterate over those to delete from core data. Obviously, there could be issues with that, but in theory should work.

-Cory

@cnstoll
Copy link
Contributor

cnstoll commented Jun 30, 2013

I'm taking a crack at this finally. Check out my new branch whenever you guys get a chance...cnstoll_deleteOrphans.

:)

Thanks,

  • Conrad

@bilby91
Copy link

bilby91 commented Nov 3, 2013

Hi @cnstoll , i really really like your project, this is what i was looking for a while! I love the flexibility users have to customise lots of things. I wanted to know what happened to this branch, i couldn't find it. Are you planning to merge it?

Thanks!

@cnstoll
Copy link
Contributor

cnstoll commented Nov 3, 2013

Hey @bilby91, the branch is still there. Here's a link to it. The branch should also include an example project which implements the change to support conditionally deleting orphans using a block. The example can be found in the MMRecordTwitter example project.

I haven't merged the branch yet because I've been waiting to gather more feedback on this solution. Do you feel like checking it out and telling me what you think?

https://github.com/mutualmobile/MMRecord/tree/cnstoll_deleteOrphans

Personally I think this is an ok solution that should fit most people's general needs. An alternative option for dealing with orphans is actually to subclass the Marshaller. We did that on one project and it actually worked fairly well. We had one entity type which needed to check for, and possibly remove, existing instances of the same entity before populating the new one.

Thanks,

  • Conrad

@bilby91
Copy link

bilby91 commented Nov 3, 2013

I definitely will test it out. I'll try to do it next week.

Posting the feedback here.

Thanks a lot!

@cnstoll
Copy link
Contributor

cnstoll commented Dec 23, 2013

Going to go ahead and merge this in. Delete all the orphans!

@cnstoll cnstoll closed this as completed Dec 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants