Add HTTPNetworkTransportDelegate and HTTPNetworkTransportInterception

[brennantaylor]

Add HTTPNetworkTransportDelegate and HTTPNetworkTransportInterception

So that mutating headers and other inspection can be done by the delegate. This
means being able to change headers without recreating clients or logging
request/response timing at a high level by instrumenting the calls to the
interception. Functionality this should give callers adequate control over the
http pipeline.

By using an Interception to encapsulate state it would be easy to implement a
timer for a request/response pair. By flattening it into the delegate itself
that information would be lost without using a unique id to tie them together.

Alternatively one could implement http chaining like seen in okhttp which would
give a cleaner contract in terms of showing that only one call to
Chain.proceed can perform an interception but is ultimately more complicated.

[apollo-cla]

@brennantaylor: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[MarkPerkins]

WOW!. this is awesome

[bmunge]

This is great! I'd love to be able to update the client auth headers without recreation.

[martijnwalraven]

@brennantaylor I would really like us to get a solution for this over the finish line. Thanks for opening this PR. One thing missing from this solution is support for asynchronous callbacks. I think that is a hard requirement to implement use cases like retries on errors and token refresh (see the Alamofire RequestRetrier for an example of this).

There is an existing PR that deals with this. Maybe we can continue the discussion there to keep everyone involved.

Definitely open to exploring an interception mechanism as an alternative to a delegate as well. We have something similar in JavaScript (Apollo Link), and that has proven to be a very fruitful extension mechanism. There is an existing PR that tried to explore this.

[RyanCodes]

@brennantaylor I would love for this to make it into master soon! This is exactly what i need :)

[wtrocki]

I'm using this directly on the application level to verify if this works for various use cases.
Question: what is the reason for this protocol - it looks like the only responsibility is to create interception for each request.

[wtrocki]

When using willSend I can access request fields but I cannot add headers (request is immutable object)

[wtrocki]

I have done some investigations with this by trying to write Alamofire version of the NetworkTransport.
From my point of view users should be able to use their own network layer if they need - which is possible for long time on Android by supplying okhttp implementation with all interceptors etc.

What is currently blocking users from IOS is that essential parts that are required to write your own NetworkTransport are not available. Some classes used in apollo network transport are internal - means that when writing your own version of it it will not be possible to reuse them.

IMHO easy fix will be to make change private func requestBody[1] to protected (fields and classes used inside are internal and cannot be used outside library). When this is done everyone will be free to write their own network transport and community can catch up with Alamofire NetworkTransport etc.

This PR provides some level of flexibility, but allowing users to write their own NetworkTransport will be ultimate fix that will support all use cases.

[1]

apollo-ios/Sources/Apollo/HTTPNetworkTransport.swift

Lines 140 to 149 in 2bd859d

	private func requestBody<Operation: GraphQLOperation>(for operation: Operation) -> GraphQLMap {
	if sendOperationIdentifiers {
	guard let operationIdentifier = operation.operationIdentifier else {
	preconditionFailure("To send operation identifiers, Apollo types must be generated with operationIdentifiers")
	}
	return ["id": operationIdentifier, "variables": operation.variables]
	}
	return ["query": operation.queryDocument, "variables": operation.variables]
	}
	}

[wtrocki]

I might miss the usage for this, but when testing this I needed to drop weak keyword to make everything work.
This keyword may be not needed here (although I might miss some context that author had when doing this)

[davidwellsmith]

Delegates should be weak to avoid reference cycles. As recommended in the SwiftLint rules.

[naknut]

Hi! Whats the status on this request? I would really need this feature for my use case. Whats blocking this from getting approved?

[wtrocki]

@naknut There are couple PR's currently open with similar approaches.
This will work if you bring this files into your project.

[tsorencraig]

Any update on this? @wtrocki @brennantaylor @naknut Lots of open issues and PRs for similar functionality, so would be great to get something merged in

[wtrocki]

@theadactyl There seem to be no interest to get the solution for this merged inside the apollo-ios. There are more than couple PR's solving the same issue.
Community build fully functional Transport: https://github.com/maxdesiatov/ApolloAlamofire

This just does the job and adding it to documentation will be quite handy as many people asking for it.

[designatednerd]

Hey @brennantaylor - Thank you for doing this, and sorry for the long radio silence on our end!

Unfortunately since so much has changed since this PR was opened, I'm going to close this in favor of the work happening in #602 to implement this feature. Please feel free to chime in there if you have thoughts or questions, and open issues on anything you feel we may be leaving out.