Fixed a few bugs & added better tests

[makdad]

• Removed timer (retain cycle)
• Fixed a memory leak
• Fixed a big problem where multiple records weren't being added in contextDidSave: (whoops, too used to Ruby syntax)
• Improved tests by making sure they are more atomic (requests from previous tests aren't still running et al).

[ecordell]

Sorry this is going to take me a while to get through.

I realized that some stuff changed that I didn't notice before. Delete requests need to be handled differently (should only store route), and a delete request should only remove other queue items if there's a post request for that object.

[makdad]

Ahhh, I thought it was that if a delete request came through, we didn't care about any other items, because we're going to delete the sucker anyway. THEN, in the case that the create (POST) request was also in the queue, we just delete everything because the server never needs to know.

So, you think that if the "record" happens like this:

CREATE (sync) UPDATE UPDATE UPDATE DELETE (sync)

That the 2nd sync should send the 3 updates as well as the delete? I thought we could (using the batching strategy) remove those updates, since we're just going to delete it anyway. I'm fine either way -- as you know my use case is to send everything, all the time -- so it's really up to you how you think this should be implemented.

[ecordell]

@makdad Sorry, here's what should happen:

If there is a local create request followed by a delete request, we should delete all of the related queue items - it doesn't exist on the server yet - including the delete request itself.

If, on the other hand, it does exist on the server, we can delete all of the other requests but we definitely need to keep the delete request.

There's something wrong here, and I'm trying to track down what it is. Transparent syncing doesn't work for me, a lot of requests get lost. I'm going to merge this locally and try and fix it, and then push the result (it should close this automatically when I do.)

[ecordell]

@makdad

Okay so I've merged in your changes, but I had to change a lot of stuff to get things working. Sorry if things are unclear now, I'll try to clean it up soon. I added comments where I could to help explain, and my commit messages should help some.

First I changed it back so that it uses the objectRoute for deleted objects - if an object is deleted, and then the context is saved, there will be a RKManagedObjectSyncQueue record for an object that doesn't exist in the objectStore, so it can't be retrieved to create the request.

Doing that means that we can't reliably remove sync objects from the queue by looking them up via objectID, as you had added to the objectsDidFinishLoading method. So I had to rethink the queue removal process.

The solution I came up with was to nix the delegate calls, and use the RestKit block calls (e.g. postObject withBlock). I created a global "queue" array, which holds just the objects that need to be synced according to whatever method was called (not every RKManagedObjectSyncQueue object). I also added the failedQueueItems and completedQueueItems arrays. Then in the block calls, the queue item is passed in, and then sent to one of those two arrays depending on whether it completed successfully or it failed. When the total in both arrays equals the number of objects in the queue array, we're finished pushing, so we can clear out all of the arrays and remove the queue items that were successful from the core data queue.

Since the blocks are all called asynchronously (they're called when the request finishes), it meant that a push followed by a pull (as in the sync methods) calls pull before everything is pushed. So I had to create a flag for pulling after pushing, and it's set to yes when a sync method is called and no if you call push explicitly.

I think that outlines the most prominent changes. One thing just led to another so this turned out to be a bigger job than expected.

Note that all of this still respects your syncDirection stuff: that affects adding to the queue in the first place, so all of that works well.

[makdad]

Cool - all this looks good. I'll put my comments back on the RestKit main pull request.