Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom header injection as a part of MatomoTracker initialization #404

Open
wvezey opened this issue Dec 21, 2021 · 3 comments
Open

Custom header injection as a part of MatomoTracker initialization #404

wvezey opened this issue Dec 21, 2021 · 3 comments

Comments

@wvezey
Copy link

wvezey commented Dec 21, 2021

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
Copy link
Member

brototyp commented Jan 8, 2022

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
Copy link
Author

wvezey commented Jan 13, 2022

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
Copy link
Author

wvezey commented Jan 31, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants