Added support for additionalHeaders in HTTPNetworkTransport

[vandadnp]

No description provided.

[apollo-cla]

@vandadnp: 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/

[RolandasRazma]

don't think this is proper solution. In case of OAuth this would not be enough...

[rajkhatri]

@RolandasRazma I agree it may not be the proper solution for OAuth, but do you think it is important for us to be able to change headers dynamically within the app. Right now (and for the past year +) me and my company have been using Apollo with a similar solution, see PR #257 . There have been several takes on a similar problem, and it would be nice to - at the least - have support for dynamic headers. I like solution #257 better than this, but would be okay with any solution that allows us to dynamically change headers.

Please let me know your concerns, and if there are any other projects in the work that will solve this problem along with oauth.

[RolandasRazma]

@rajkhatri I personally don't mind, just it would be nice to be able dynamically update request. That would cover OAuth and any "additional headers".

"static" headers already (out of the box) can be added:

var configuration = URLSessionConfiguration.default
configuration.httpAdditionalHeaders = [:] // <- add anything you want
        
let networkTransport = HTTPNetworkTransport(url: ..., configuration: configuration, sendOperationIdentifiers: ...)
        
let apolloClient = ApolloClient(networkTransport: networkTransport, store: ...)

[michaelnisi]

@rajkhatri You are adding state additionalHeaders with undetermined scope, sometimes overriding URLSessionConfiguration, which is the destined configuration for that transport object. This is problematic. Who is responsible for resetting additionalHeaders? Subsequent operations might unintentionally use these headers. Delegates are good for passing transient parameters, as you’ve alluded to (#257).

[rajkhatri]

@RolandasRazma yep - we are able to add static headers, but we have our own in house version of OAuth/SAML and are changing the authorization header dynamically during the lifespan of the app in the fore/background.

@michaelnisi we are just changing the headers. I understand that there might be exposure to other things. In terms of whose responsible for resetting additionalHeaders. I mean aren't those additionalHeaders set by the user? and it then becomes available to the user (at their discretion) to use how they want. Just curious if there is an "interim" solution, that can be made official until a full blown solution has been made present.

[RolandasRazma]

@rajkhatri we also have "In house" OAuth support, but just adding extra headers is not enough to support it properly.
It would be great if we could reconfigure (auth, sign, extra headers) request on the fly out of the box on HTTPNetworkTransport instead of having to implement custom transport. Adding "extra headers" like this PR does would push that back (IMO).

[designatednerd]

Hey @vandadnp - thank you for this! We're going to be moving forward with the changes in #602, which will allow you to set additional headers. I'm going to close this in favor of that, but please feel free to comment there or file issues on any changes you feel I might have missed!