search

matomo-org / matomo-sdk-android

SDK for Android to measure your apps with Matomo. Works on Android phones, tablets, Fire TV sticks, and more!
BSD 3-Clause "New" or "Revised" License
388 stars 162 forks source link

Cannot provide custom dispatcher #169

Closed dipendrapkrl closed 6 years ago

dipendrapkrl commented 7 years ago

First of all, thank you very much for this open source gem. So, I was trying to use my own dispatcher and then I realized that I cannot do it. Looking at the source of Piwik class; it is singleton class which means that I cannot subclass it(it has private constructor). But the method getDispatcherFactory() in the same class is protected probably meant to be exposed to the subclass. Doesn't these two statement contradict each other? I think if the Piwik class is not singleton, it would make the sdk open to customization. Any help would highly be appreciated.

d4rken commented 7 years ago

There was no intention to allow custom dispatchers. The DispatcherFactory was introduced to improve unit testing.

It wouldn't be out of this projects scope to allow custom dispatchers though. Probably not via subclassing Piwik but rather by providing a default dispatcher and a setDispatcherFactory. Need to also decide on an API for that and change Dispatcher to an interface.

What's your use case for this feature?

dipendrapkrl commented 7 years ago

Yes. the Setter for DispatcherFactory would be nice. I want the traffic to go through my own okhttp client which is the reason I am looking for custom DispatcherFactory.

d4rken commented 7 years ago

What advantage does using your own okhttp client have?

dipendrapkrl commented 7 years ago

Since I am using okhttp to route all the traffic that generates from my app, I would prefer this library also allow me to do so with some work but without a fork. The requirement for me is that the server that I am sending metrics expects auth tokens in the header of the request. I dont see how I can do so without my own dispatcher.

d4rken commented 7 years ago

That's a good use case. I'll look into making this possible.

dipendrapkrl commented 7 years ago

Thank you very much.

dabitdev commented 7 years ago

it is important to distinguish metrics requests with low priority from other important request that the application needs to work. Having custom dispatch makes sense to allow any application to extend in any way.

d4rken commented 7 years ago

Please see #172, does that suffice for your use case @dipendrapkrl ?

dipendrapkrl commented 7 years ago

Thanks @d4rken. This totally helps providing custom dispatcher for my use case. Also since Piwik SDK auto appends to the provided URL(appends piwik.php), it would be great if Packet.getTargetURL() would be public instead of protected. This is required because we want to know where to send the data by the custom dispatcher. Right now I am using reflection to set custom dispatcher and to get the target url. Thanks for the great work.

d4rken commented 6 years ago

I think this was implemented in #172