Amplitude-iOS icon indicating copy to clipboard operation
Amplitude-iOS copied to clipboard

Allow injection of custom Network Client

Open fpg1503 opened this issue 7 years ago • 3 comments

Hello! In the business I work we are required to only make connections using mTLS, and, for that reason, we cannot use the SDK as is.

We have an internal proxy server that uses mTLS and talks to the "outside world", but, in order to use that, we need to inject our custom network client that enforces mTLS and uses our custom request encoding.

What do you think about the current implementation? I made it in a way that it won't break the existing API. The Android version of this PR is Android #187.

fpg1503 avatar Oct 27 '18 15:10 fpg1503

@fpg1503 Thank you very much for your effort! The support to use a custom http implementation for the Android variant of the SDK would also be very much appreciated to be able to omit additional libraries for Xamarin.Android bindings.

thisisthekap avatar Nov 05 '18 17:11 thisisthekap

@fpg1503 thank you so much for submitting this PR! Apologies for taking so long to review it. Some of my initial thoughts:

  1. Rather than passing the client in during initWithInstanceName and getInstance methods, it might be cleaner to leave those alone and have a setNetworkClient method to set it on the instance? That would also fix a bunch of the getInstance unit tests that are broken in your branch
  2. Would it be cleaner to have NSURLSession as an input to the the network client's uploadEvents method instead of keeping it as an instance variable on every client instance?

@fpg1503 thoughts?

djih avatar Dec 11 '18 02:12 djih

@djih thanks for your review so far! I've implemented your suggestions, do you think there are any other points to address?

fpg1503 avatar Dec 17 '18 21:12 fpg1503