-
-
Notifications
You must be signed in to change notification settings - Fork 339
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
Redesign HTTP transaction model #259
Comments
If I read you correctly, you're suggesting we treat the HTTP entities as immutable. If so, I'm totally in for this. Currently we do have a
Can you elaborate further? Or provide an example (e.g. adding SSL/TLS information as done in #252). How that would look like in your suggestion. Moreover, let's put Testing in the roadmap as we have much to improve on that side 👍 |
I thought that we already agreed about it in Slack. On 21st of January I wrote such message for plans after
|
Yes, this would be one thing to do but not the main one. I think that model should be separated into at least 3 tables. One step further would be to divide it to stuff like Another option that I was thinking of is to not create multiple tables but one with just a blob as a field that would be proto encoded data.
Yes. Basically repositories would not return database entities but some concrete domain type that can be displayed in the UI. So there would be type (class or interface) like
Do you have any code coverage target in mind? |
I think we should start with some realistic and easy to achieve numbers like 60% and gradually bump it by 5-10% depending on availability of free time and amount of changes we might have. |
Would like to return to this topic. |
To be honest, to make it simple, I would just divide and conquer and for now just split into three separate entities - |
Agree on having this as an intermediate step. It will make things easier for us to handle and we can make this change incremental. |
@vbuberen Can I assign myself to it? If you don't work on it, I think I could start working on it this or next month. I'd provide here my general plan on approaching it because there are some things I want to get feedback on. |
Yes, feel free. |
My plan for approaching it is as follows. Introduce new persistence models. These models would be pure data classes, and I want to remove all logic like URL formatting, things specific to HAR, etc. They would use internal data class HttpCall(
val id: Long,
val request: HttpRequest?,
val response: HttpResponse?,
)
internal sealed interface HttpBody {
data class Decoded(val value: String) : HttpBody
data class Encoded(val value: ByteString) : HttpBody
}
internal data class HttpRequest(
val id: Long,
val url: HttpUrl,
val method: String,
val redactedHeaders: Headers,
val unredactedHeadersByteSize: Long,
val body: HttpBody?,
)
internal data class HttpResponse(
val id: Long,
val protocol: String,
val body: HttpBody?,
) After this is covered, I'd like to see what is needed to deliver the results to the UI. New persistence models should be enough for our purposes, and I don't anticipate a need for any separate presentation model (at least not for this task, maybe for #204). Firstly I'd map old models to new ones and integrate them with the UI. If that works, I'd swap old data providers with new ones, remove mappings, and hopefully remove legacy code. I would most likely drop the Doing this gradually would allow us to tackle the main issue, but also split |
Sounds good to me. |
Looks good to me. The only thing that comes to my mind is:
I suppose we're going to write wrappers for those as well, right? chucker/library/src/main/kotlin/com/chuckerteam/chucker/internal/data/entity/HttpTransaction.kt Line 15 in 1f7d907
and related as this is making hard to adapt the library to other HTTP libraries. |
Random thoughts not directly connected with models redesign. What about adding some pagination to our DB, so we would be pretty efficient in transactions list? |
I was actually thinking about reusing |
Agree that there is no immediate value at this stage, but as we're redesigning the data model, having more decoupling will help tackle future scenarios. As the mobile space is quickly evolving (KMP and co.), I would do the extra effort to try to have a vanilla data model. |
Hi All, 👋 |
@vbuberen Can I assign this to me? I'd like to work on this a bit 👍 |
Yes, feel free to grab it. |
Providing a small update here as I've been working on this for long time now. This is going to be a massive rework of the internals. I'm not sure I will be able to split into smallers PRs at first. I hope to be able to send a draft PR with the whole rework before the end of the year + if there is an agreement from @vbuberen I'll work on splitting it in some form afterwards. |
Right now model for an HTTP transaction is a big structure that kind of serves as bag for everything. It makes it hard to work on new features that require new fields, different encoding format etc. Some links - #204, #202, #69.
💡 Describe the solution you'd like
I believe that there should be a separate table for transactions and different types of requests and responses. This way it would be easier to add new types of compression and so on.
This would require to not rely in UI layer on DB entities. They should be rather mapped to some common model (most likely a sealed class or an interface) that would be renderable on the view layer.
It would also allow to tidy a bit nullability of some fields. While request or response still might be null, some of their fields shouldn't be (like i.e. URL address).
📊 Describe alternatives you've considered
An interesting alternative would be to split HTTP transaction just into three tables (transaction, request, response) and store data as a blob which would be defined in a proto file. There should be no issue with backward compatibility as this is one of main advantages of protobuf. Additional benefit is that new tables would not appear for every new feature and only the proto schema would change.
🙋 Do you want to develop this feature yourself?
I could work on it but it is rather a big change proposal that requires some discussion, design and approval beforehand. Not sure what the best way would be. This ticket, draft PR, Slack channel or maybe something else?
The text was updated successfully, but these errors were encountered: