matomo-org / matomo-sdk-ios

Matomo iOS, tvOS and macOS SDK: a Matomo tracker written in Swift
MIT License
390 stars 164 forks source link

Custom header injection as a part of MatomoTracker initialization #404

Open wvezey opened 2 years ago

wvezey commented 2 years ago

Our app requires all requests to be proxied, including the tracker requests; the proxy endpoint requires custom headers. Because tracking requests are built in URLSessionDispatcher, we created a custom dispatcher class, sent as a part of MatomoTracker initialization. The public send method in URLSessionDispatcher requires Event, which in turn requires Visitor and Session, and the cascade continues. The various dependent class methods and properties are not marked public, resulting in the importing of many Matomo Swift package classes into the app code base. This makes the tracking functionality upgrade-fragile. Thus, the need to inject custom headers to the proxied tracking request resulted in the need to create a custom Dispatcher implementation and the import of number of native Matomo Swift package classes.

Proposal: update MatomoTracker.init() to accept an array headers that are ultimately added as a part of URLSessionDispatcher.buildRequest(). This assumes that the same header values would be required for all tracking requests, which is likely, since the baseURL is a constant. Alternatively, ease the burden of creating a custom Dispatcher implementation by marking dependent class methods and properties as public.

I know, too, that I might be missing something obvious, and unnecessarily created a lot of work for myself. Wouldn't be the first time.

brototyp commented 2 years ago

Hey @wvezey, thanks for your ticket.

I understand your issue. The idea about the URLSessionDispatcher is that it should cover 95% of all cases and that we offer the possibility to implement custom dispatchers. I would say this case falls rather in the 5% of cases.

I think we should ease the way of writing custom Dispatchers. There are a few more thoughts I wanted to throw in the round:

  1. What do you think about exposing the logic of the EventAPISerializer so it can be used by other Dispatchers? I guess by far the most of all custom Dispatchers will use exactly the same format for the body of the requests.
  2. We could also think about making the URLSessionDispatcher subclass-able. In your case, for example, all you would need is a custom buildRequest function for you to set your headers.

Curious about your thoughts.

wvezey commented 2 years ago

Cornelius, thank you for your reply. Good thinking.

Making URLSessionDispatcher sub-classable makes more sense to me; it feel like a more elegant solution. It would allow for overridden methods and properties, without having to import native Matomo classes into the app source code.

Please let me know if you need anything from me to move forward with this. I could prepare a PR, if that helps.

wvezey commented 1 year ago

@brototyp I will create a PR around this. This task will be added to my personal backlog, so it might take a couple of months. The issue remains for us, so it's well worth doing.