ChuckerTeam / chucker

🔎 An HTTP inspector for Android & OkHTTP (like Charles but on device)
Apache License 2.0
3.97k stars 350 forks source link

Expose data model #579

Open speekha opened 3 years ago

speekha commented 3 years ago

:warning: Is your feature request related to a problem? Please describe

We recently added a feature allowing users to export all sorts of logs in some builds of our app. Since all network logs are already store in chucker's database, it would have been very helpful to be able to query that database through Chucker's API, but it wasn't possible because most of Chucker's code is declared as internal and thus not accessible from another module.

:bulb: Describe the solution you'd like

Repository and model classes could be exposed as public.

:bar_chart: Describe alternatives you've considered

A current workaround is for devs to query the database themselves (basically duplicating Chucker's code), which implies running the risk of their code being broken when Chucker's design evolves without the safety of compile time verifications.

:raising_hand: Do you want to develop this feature yourself?

Could offer help by pushing a PR if needed

MiSikora commented 3 years ago

Generally speaking I would like to do this but not right now. The problem is that current data model falls a bit under the concept of "bag of not necessarily related properties" and is not extensible without breaking changes which would limit our releases.

However, as I mentioned it is definietly something that I personally have in mind but from my perspective 3 things should happen first.

This would make me more confident that the model is open to modifications without introducing breaking changes or that the breaking changes are really needed to provide new functionalities.

vbuberen commented 3 years ago

Thanks for suggestion. From my side I don't see this feature as something I would like to see in Chucker. I don't think that we are ready to commit that much to expose even data modules. It makes us much less flexible as any change would require tons of back and forth communicating the change, etc. Would love to avoid agreeing every single internal change with users.

MiSikora commented 3 years ago

Clarification from my side – I don't see it as something useful on its own. What I would like to achive in the future is support for other HTTP clients. This imposes modularization and mapping from client specific type to our types thus making them public. But this is not something that I see doing anytime soon.

cortinico commented 3 years ago

Generally speaking I would like to do this but not right now

Same for me.

To give some historical context: Chuck used to have a ContentProvider to expose the data to other apps. It was removed once we forked the project as it was unused and yet another feature to maintain.

Recently I was thinking about the "export" capabilities of Chucker, and ideally we could provide an extension point where you can bring your own Exporter. So we don't have to support multiple formats like Json, XML, HAR (#417) and so on but those can be plugged in by our users.

This obviously requires to expose the data model in some form. As @MiSikora mentioned, we never really designed this with this feature in mind so we probably need to polish it a bit, before we can actually expose it.

Make Chucker a multi-module project.

One note here, as a side effect of this shift, all of our internal models will have to be declared public. Therefore if we shift to a multi-module project, the exposure of our internals is basically unavoidable.

We could still keep the API contract around the .internal package -> but I believe people will start using the data model in some form if they just want.

speekha commented 3 years ago

To be honest with you, I would be happy with that last option: keeping it in the internal package, but removing the "internal" accessor. Officially, those classes would not be part of chucker's public API, and you wouldn't have to worry about breaking anything with each release. But it would still allow those who are willing to run the risk to use the classes and bear the cost of regular breaking changes if they had to. I know I would, because right now, I've had to write my own sqlite queries and cursor adapter to access your database. Which would be even more prone to breaking with even less warning or indication that something changed.

Basically, I'm not asking for complete support with a public API, just being allowed to use those classes at my own risks.

cortinico commented 3 years ago

To be honest with you, I would be happy with that last option: keeping it in the internal package, but removing the "internal" accessor. Officially, those classes would not be part of chucker's public API, and you wouldn't have to worry about breaking anything with each release. But it would still allow those who are willing to run the risk to use the classes and bear the cost of regular breaking changes if they had to.

I don't see this happening in the near future (at least till we don't move to a multi module setup).

But it would still allow those who are willing to run the risk to use the classes and bear the cost of regular breaking changes if they had to. I know I would, because right now, I've had to write my own sqlite queries and cursor adapter to access your database. Which would be even more prone to breaking with even less warning or indication that something changed.

Have you considered writing a Java class to access those internal declarations (they're effectively public in the bytecode). So you would still bear the costs of potential breaking changes but we won't have to expose this API, at least for now and we can follow the normal evolution of the library.

Basically, I'm not asking for complete support with a public API, just being allowed to use those classes at my own risks.

You can basically already do it with Java :)