Possible API changes

[mikeabdullah]

Here is a list of API changes floating round my head that could be made before we declare 2.0 final:

Delegate lifecycle

Presently CK2FileManager references its delegate weakly. The number one issue to my mind is that changing is a messy business, and something clients almost certainly shouldn't be doing. We could turn the system round so that delegate is a strong reference and specified at initialisation. The client is then responsible for shutting the manager down in order to release the delegate.

Also gives a chance for the client to specify a queue for delivering delegate messages on.

Configuration

Clients would probably like to control things like timeouts for operations. Again like the delegate, awkward questions arise if such settings can be changed mid-lifecycle. It would be neat to provide this info upfront, but doing so can easily result in a ridiculously large init… method.

One option is to have CK2Configuration class which encapsulates all these details.

Another idea that springs to mind is the concept of a prototypeRequest. The client would provide a URL request configured to its liking that all operations would be derived from. There does seem a little impedance mismatch here. e.g. upload data and many HTTP headers would have to be ignored.

Task/Operation API

Presently ConnectionKit exposes the concept of an operation purely as an id. We could make this a public class with its own API for cancelling etc. CK2Task is nicely terse. CK2Operation is pretty good too, but I think subtly carries the expectation it be a subclass of NSOperation. Of course, perhaps it should be just such a subclass!

Include Task/Operation in delegate methods

Delegate methods could pass in a reference to the task/operation which they relate to, allowing the delegate to make an educated decision if multiple operations are in flight. The trick there is a subtle race condition exists whereby a delegate message could arrive before the client has had a chance to store a reference to the task/operation, leaving it unsure how to handle it.

A simple solution to that is to make the client explicitly start each operation itself. Something like:

CK2Operation *op = [fileManager createFileAtURL:…
[self addUploadOperation:op];
[op start];

If the client doesn't care about referencing the op, it can do:

[[fileManager createFileAtURL:…] start];

Would that be more confusing for clients? Perhaps the APIs ought to be renamed to reflect that they're creating a task/operation object, rather than acting immediately.

Another solution would be to ask that clients manage some form of synchronisation around calling the file manager to avoid such races. I have a nasty suspicion most wouldn't, and would only se the race condition happen sporadically once out in the wild. Hmmmm.

Oh, one other advantage of having clients explicitly start operations/tasks is that they could easily hand off to some form of queuing mechanism to manage that.

Move a little more in favour of delegate methods over blocks

For some of the more complex operations, the current block-based system might seem a little overwhelming. It might be better to shift back some control work to being done in delegate methods. This would require something as described in the previous section.

In particular uploads are an interesting problem since to handle arbitrary schemes requires handling/notification of three things:

• Upload progress
• Providence of a fresh stream
• Completion

Having three blocks passed to a method seems pretty damn unwieldy to me (I still find two to be awkward!)

Dual authentication delegate methods

CK2FileManager could make the distinction in its delegate methods between authenticating an individual operation, and a connection. This might be too confusing for clients though, I wonder. Technically I guess FTP and SFTP work purely by authenticating a connection.

Asynchronous delegation methods

It might be nice to standardise on a model where the delegate is free to take as long as it wants responding to a message, signalling it is ready to continue by calling a block provided by the manager.

NSURLAuthenticationChallenge already encapsulates a means of doing this in a pre-blocks world. Might be nice to generalise the principle.

Session

Originally, the main class in ConnectionKit was CK2Session which tended to confuse people in practice into thinking it represented a single connection. With upcoming Apple APIs, that may be less confusing. If we renamed it CK2FileSession that would produce delegate methods which carry less risk of conflicting with Apple's own ones in future. This is a pretty big change to make though!

[samdeane]

Regarding configuration, could this be provided via an optional delegate method? It could be passed a CK2Configuration object with default values, and asked to return a new one.

If you combine that with specifying the delegate at init time, you've a fairly predictable mechanism.

The delegate can answer any configuration questions when it's called (either once at startup, or whenever the file manager needs them - not sure which would be best).

Users who just want to use the defaults needn't bother to implement the method.

[mikeabdullah]

Something like -fileManager:operation:willUseConfiguration:, I guess?

I think this only becomes worth it if different tasks/operations within a session/manager require different configurations. I'm tempted to say people should just have to create a second file manager in which case.

Also, I expect there would be some kind of globally shared config or session/manager that clients could easily use for the default behaviour.

[MrNoodle]

I may be a little unclear on some of the issues here so please bear with me.

Delegate lifecycle:

In the cases where I use it, my object creates the file manager and sets itself as the delegate. It maintains and releases the file manager when it's done. Having a strong delegate here would create a retain loop and I'm not sure what issue it would be solving besides. Also, I do change delegates in one case. Not sure what issues are involved with this but I'm not seeing any odd effects. Note that things are done serially on my end which may avoid whatever problems you are seeing or anticipating.

Configuration

With timeouts, I've been doing that on the client side entirely. If the file manager doesn't call the completion block in time, I cancel the operation and deal with the timeout. Besides timeout, how many of these configuration parameters are you expecting? Is there a reason why they just can't be properties on the file manager that you set?

Task/Operation API

What do you want them to do beside be cancelable? I'd rather not create a new class if that's the only thing. There's a precedent for opaque handles that can be used to deregister/cancel, such as using blocks with NSNotificationCenter.

Include Task/Operation in delegate methods

Normally, you would use a semaphore for this type of thing. If you look at CK2OpenPanelController, it gets around it by only allowing changes to the operations data structures via the main thread. Personally, I'd rather not get too deep into abstracting out the operation stuff as there are threading mechanisms already in place. If you really want to do it, I'd suggest having separate methods which return the unstarted operation , like -operationToCreateFileAtURL:... NSTimer does something similar where you have a method to start it and one to just return one which you can start manually.

Move a little more in favour of delegate methods over blocks

I'm of the opposite mind on this one. I'd hate to have the delegate have to multiplex the different operations. It would end up being like -observerValueForKey:... which always ends up being a bunch of if/elses. The benefit of blocks is having the call back code in the same context from where it was called where it makes more sense. If you look at the completion blocks that CK2OpenPanelController defines, they are specific to the situation of the caller and it makes sense to have the code in the same area. Defining multiple blocks inline can be awkward but you can always define them elsewhere and store them in a var and I still find it preferable to what amounts to a huge switch statement.

In any case, my 2 cents (or maybe a bit more than that).

[MrNoodle]

Thinking about it more, if you really want to expand the operation and callback functions, here's another idea on how to implement it:

• Create a subclass of NSOperation.
• Have manager methods that return the operation without queuing it.
• The operation can either:
  • Take a delegate
  • Have blocks set on it.

Note that I'd still want the current methods around for those cases where I want to fire off an operation and just be notified when it's done via a completion block. But for more expanded interactions, I'd prefer this as you aren't forced to have one delegate to handle all operations. You can set the same delegate on all the operations if you want but I think it makes more sense to allow for context specific delegates depending on the operation. You can also have the operations be configurable with timeouts and such. And lastly, you can have the same configurations be settable on the manager and inherited by the operations when they are created (but overridable).

[mikeabdullah]

Big thanks for all the feedback, here's some followup:

Delegate lifecycle

Yes, this would create a retain cycle, and deliberately so. The delegate (or some other object) would be responsible for breaking the cycle by closing/invalidating the file manager when it's finished.

I shall have a look through your open panel code to better understand changing the delegate (or does this happen in other, private code of yours?).

Configuration

Interesting to know that you're managing timeouts yourself. I think everything in CK currently operates on a timeout of 60s, but not sure how accurate the libcurl-backend is for that!

Right now I don't have any great idea of what other configuration might be required (FTP encryption settings perhaps?). Could be good to have some caching controls in there one day, too.

Again I'm slightly wary of making this a setting on the file manager, as it raises awkward questions about what happens to existing operations when you make a change.

Task/Operation API

For uploads and downloads, it could be nice to support pausing and resuming them. It also gives scope to have various methods available on these objects to report things like progress on-demand, and perhaps attach your own app-specific data.

Include Task/Operation in delegate methods

I agree a semaphore would solve this. Seems to ask quite a lot of clients though, to get the subtle intricacies right. I like your suggestion of having different flavours of methods, though. Perhaps something like:

- (void)uploadData:(NSData *)data toURL:(NSURL *)url completionHandler…
- (CK2Task *)uploadTaskWithData:(NSData *)data URL:(NSURL *)url completionHandler…

That way, clients using the simpler, immediate API don't have a reference to the operation for race conditions to occur from.

Move a little more in favour of delegate methods over blocks

I absolutely don't want to ditch the use of blocks for indicating completion of an operation/task. That style of API is soooo much more pleasant than without blocks. It's the other callbacks, the one's mid-operation, that I'm considering switching to be delegate methods.

Providing a delegate to the individual operation/task is an interesting approach too; I'll have a think about that one.

[MrNoodle]

I'm still unclear on the need to retain the delegate in the first place. (and the code where I use it is in my own stuff, not in CK2OpenPanel so I don't have a public example). By doing this, you are forcing the delegate to have a separate step to indicate when things are done. I wouldn't want to impose this without a very good reason.

As for configuration, I'd think that any configuration settings would apply when the operation is created or run and wouldn't affect anything already in progress. Also, per my suggestion of doing an NSOperation subclass, this stuff should be set on the operation itself anyways to allow different timeouts for different operations. I'd only have this stuff in the file manager as a place for each operation to get default values. BTW, in my code, I do a timeout of my own and for certain operations (like upload), I base it on the size of the file. Note that this is a timeout of CK completing the operation as a whole. I'm assuming the timeout you are referring to would be an internal timeout for receiving a response.

Also, if you go the NSOperation subclass route, you already have a completion block. You can add support for other blocks like progress and such. It's also already has support for executing in different ways and canceling so really, it seems like a natural fit.

[mikeabdullah]

I think because file management is a complex, asynchronous task, I just like to minimise moving parts where I can. I'm not deadset on changing the delegate arrangement though. I really ought to get an understanding for why you have code that switches around the delegate first at the very least!

I agree that file manager-level settings could well be treated as defaults, with per-op customisation being possible beyond that.

My intent would be to have the same timeout logic as NSURLConnection and friends, which is based pretty much on socket activity, rather than overall transfer times. Anything beyond that I consider to be the client's problem to solve.

One issue with NSOperation's completion block API is that it has no concept of failures etc. That would likely have to be exposed as a property on the operation, which does unfortunately make it very easy to create retain cycles. There's also the argument that the concept of NSOperation as a unit of computation doesn't quite match up to the network-based tasks which CK performs.

[MrNoodle]

I have individual action objects which do the upload. At the same time, I felt it might be nice to share the file manager so I could reuse the connections. So, each time one of the upload actions has to do its thing, it grabs the shared file manager and sets itself as the delegate.

Right, I figured your timeout was different than mine. And I agree that what I'm doing should stay on my end.

As for NSOperation, ah yes, the error thing would be a bit more awkward in that you'd have to remember to query the operation object for it. Would the error have a reference to the operation? I'm not sure where the cycle would come in here. Also, NSOperation is meant to encapsulate a "task" according to the docs. I believe GCD is smart about not having I/O bound operations clog up the queues (I believe it will just expand the pool of concurrent threads in that case).

[mikeabdullah]

Ah, interesting. It's not worth sharing file manager instances like that, as connections are deliberately globally shared. I'll note that in the Readme.

[mikeabdullah]

I should probably admit at some point that most of the above list is heavily influenced by NSURLSession's design.