Skip to content

Improved AFURLConnectionOperation and AFHTTPRequestOperation designed around NSBlockOperation's execution blocks #127

Closed
wants to merge 58 commits into from

3 participants

@zbowling

In order to solve a few subtle bugs and issues, we refactored at SeatMe the AFURLConnectionOperation class to make it similar to the NSBlockOperation design.

We need operations not to mark they are finished until the entire operation is entirely done and when data is ready to be read from it. We have operations that expect that their dependent operations had fully finished and left things in a certain state before they should start (in our case that those operations had successfully written something to our database that are needed by the next operation).

To solve this we refactored AFURLConnectionOperation to have a model similar to NSBlockOperation which takes an array of executionBlocks. Those execution blocks are executed on a shared dispatch queue after the connection downloads all the data and before the operation is marked as fully finished.

This actually kills two birds with one stone because the subclassed operations that do processing on their own (like AFJSONRequestOperation, AFXMLRequesetOperation, AFXMLDocumentRequestOperation, AFPropertyListRequestOperation, AFImageRequestOperation) can now just register another execution block to process the output. This removes the need for our previous method we had in our branch at doing it this with method that the subclasses had to override provided by AFHTTPRequestOperation before. It ends up being a lot cleaner in the end.

This also allows us to add execution blocks that do work for us against the response inside the operation without subclassing it (for example, saving the output to a database). Since the block isn't marked as finished at this point, dependent operations can expect work they needed to be completed will done before they start. (Before we understood this was an issue, in our code we were using our own NSConditional locks around the completion callback because performAllAndWait wasn't working as intended).

This also solves a few other smaller bugs around using NSOperationQueue when using dependencies since it allows a NSOperation to have isCancalled == YES while also having isExecuting == YES and before isFinished is set to YES to allow the operation to close out why still executing which is a valid state. It also allows (per the NSOperationQueue docs) for an operation to be canceled earlier by shortcutting if isCancelled == YES when start is called to send directly to isFinished even if isReady == NO.

At the AFHTTPRequestOperation level, this branch also doesn't break anyone that still wants to set completionBlock on NSOperation or change it's behavior in any real way. It introduces a new finishedBlock property though which works similarly to the completionBlock but instead always returns on the calling queue that created the block by default (but also is configurable with the callbackQueue property if the user wants to set it). This gives the user a place he can handle operation events asynchronously without stopping the running NSOperationQueue that may be running.

This removes the setCompletionBlockWithSuccess:Failure method since it broke the user's own use of using completionBlock and didn't work consistently across subclasses (returning different types each time). However the finishedBlock property should be a usable replacement for this instead.

This branch also improves performance slightly in a few small places, fixes a brew crashing issues ([[... retain] autorelease] of returns), and contains the support for iOS Multitasking that we sent a pull request for earlier, and some other minor changes.

Thanks!

zbowling and others added some commits Sep 22, 2011
@zbowling zbowling async fixes. changes for json d3af887
@zbowling zbowling I fixed something e1d89f8
@zbowling zbowling small change' dd84bad
@zbowling zbowling better foundation json support ef5bf73
@zbowling zbowling Merge branch 'merge_foundation_json' of github.com:seatme/AFNetworking
Conflicts:
	AFNetworking/AFHTTPClient.m
	AFNetworking/AFHTTPRequestOperation.m
	AFNetworking/AFJSONRequestOperation.m
	AFNetworking/AFXMLRequestOperation.m
	AFNetworking/NSString+AFNetworking.h
	Example/Classes/AFGowallaAPIClient.h
	Mac Example/AppDelegate.h
	Mac Example/AppDelegate.m
	Mac Example/Classes/AFGowallaAPIClient.h
	iOS Example/Classes/AFGowallaAPIClient.h
521fd35
@zbowling zbowling brought to much over in the merge d55245e
@zbowling zbowling better JSON support 2c0ef65
@zbowling zbowling better idea e42d89d
@zbowling zbowling class custer method 23ac76c
@zbowling zbowling should be self d6ffe69
@zbowling zbowling much much better ec9a8b0
@zbowling zbowling moved this to the same file 54f90d4
@zbowling zbowling moved this to the same file b3cf84e
@zbowling zbowling merged in upstream changes 6c1cd78
@zbowling zbowling Merge branch 'experimental-0.8' of https://github.com/gowalla/AFNetwo…
…rking

Conflicts:
	AFNetworking/AFHTTPClient.m
	AFNetworking/AFHTTPRequestOperation.h
	AFNetworking/AFHTTPRequestOperation.m
	AFNetworking/AFJSONRequestOperation.h
	AFNetworking/AFJSONRequestOperation.m
	AFNetworking/AFPropertyListRequestOperation.h
	AFNetworking/AFPropertyListRequestOperation.m
	AFNetworking/AFXMLRequestOperation.h
	AFNetworking/AFXMLRequestOperation.m
e5e6f97
@zbowling zbowling fixed a lot of various changes after the merge. much better bb96088
@zbowling zbowling forgot to mark this as concurrent. a3c3322
@zbowling zbowling self is never delegate. don't unset the delegate if someone else want…
…ed to be the delegate.
1511a45
@zbowling zbowling no such thing as an error here. 1cfe657
@zbowling zbowling comments. 70fd2d8
@zbowling zbowling dead property 9834ad4
@zbowling zbowling comments 932d710
@zbowling zbowling Foundation json should be last. 2a939e5
@zbowling zbowling conventions and leaks 9a6d541
@zbowling zbowling Merge branch 'experimental-0.8' of https://github.com/gowalla/AFNetwo…
…rking

Conflicts:
	AFNetworking/AFJSONUtilities.h
bcbf99c
@zbowling zbowling Merge branch 'experimental-0.8' of https://github.com/gowalla/AFNetwo…
…rking

Conflicts:
	AFNetworking/AFHTTPClient.m
a0a83a6
@zbowling zbowling fix path of this. a9e645d
@zbowling zbowling Pull this change in from other branch daaba29
@zbowling zbowling Merge github.com:seatme/AFNetworking b4335c0
@mattt mattt Updating README to point to new AFNetworking URL (https://github.com/… 920b01d
@zbowling zbowling Fix memory leak ccfcd43
@zbowling zbowling Merge branch 'master' of github.com:zbowling/AFNetworking e86aa7a
@zbowling zbowling Merge branch 'master' of git://github.com/gowalla/AFNetworking
Conflicts:
	AFNetworking/AFHTTPClient.m
	AFNetworking/AFImageRequestOperation.h
	AFNetworking/AFImageRequestOperation.m
	AFNetworking/AFJSONRequestOperation.h
	AFNetworking/AFXMLRequestOperation.m
1d12038
@zbowling zbowling much better solution 9088768
@zbowling zbowling change upstream and merge 11057cf
@zbowling zbowling fix a few lingering issues after the merge b3fb0bb
@zbowling zbowling new README 695ea9c
@zbowling zbowling New README for specifying the changes in our fork. 8dd9494
@zbowling zbowling Update README.md e746bf4
@zbowling zbowling Remove silly, overly paranoid code in setCompletionBlock and let the …
…parent have a say in the isReady process so it can check dependent NSOperations.
23ca8b4
@zbowling zbowling add support for iOS multitasking. 16b5839
@zbowling zbowling Merge branch 'master' of github.com:seatme/AFNetworking f1cb7aa
@zbowling zbowling Update README.md 87feccd
@zbowling zbowling move multitasking support up to AFHTTPURLRequest d56f873
@zbowling zbowling Merge branch 'master' of github.com:seatme/AFNetworking 552e0cf
@zbowling zbowling add cache policy support e2e14cc
@zbowling zbowling update readme c40ff8f
@zbowling zbowling unnecessary cast. d40e197
@zbowling zbowling Operation shouldn't finish until after the operations finish processi…
…ng their responses. Dependent operations may need the response after one has completed. This also simplifies things a little.
11159ee
@zbowling zbowling A much improved design, taking cues from NSBlockOperation. 76c33a9
@zbowling zbowling move readme back for upstream push 60753bf
@zbowling zbowling autorelease return. fix regression. d2cc7af
@zbowling zbowling Merge branch 'master' into execution_blocks 27fe304
@zbowling zbowling Update README.md 15264db
@zbowling zbowling Merge branch 'master' of github.com:seatme/AFNetworking 2449e59
@zbowling zbowling should move to finished if canceled and just started f312dde
@zbowling zbowling Merge branch 'master' into execution_blocks
Conflicts:
	README.md
fb80b47
@zbowling zbowling prevent crash (need to check for retain cycles) c5b38c7
@duanefields

I think this address my issue #206

@mattt mattt closed this Feb 21, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.