-
-
Notifications
You must be signed in to change notification settings - Fork 350
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 body decoding #555
Custom body decoding #555
Changes from all commits
733178f
f0b44c6
32b2bcf
c83fdcc
64e62d4
f392539
8541522
b3bfa99
039d0b6
f171e71
ca6fb44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package com.chuckerteam.chucker.api | ||
|
||
import okhttp3.Request | ||
import okhttp3.Response | ||
import okio.ByteString | ||
import okio.IOException | ||
|
||
/** | ||
* No-op declaration | ||
*/ | ||
public interface BodyDecoder { | ||
@Throws(IOException::class) | ||
public fun decodeRequest(request: Request, body: ByteString): String? | ||
|
||
@Throws(IOException::class) | ||
public fun decodeResponse(response: Response, body: ByteString): String? | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package com.chuckerteam.chucker.api | ||
|
||
import okhttp3.Request | ||
import okhttp3.Response | ||
import okio.ByteString | ||
import okio.IOException | ||
|
||
/** | ||
* Decodes HTTP request and response bodies to humanβreadable texts. | ||
*/ | ||
public interface BodyDecoder { | ||
/** | ||
* Returns a text representation of [body] that will be displayed in Chucker UI transaction, | ||
* or `null` if [request] cannot be handled by this decoder. [Body][body] is no longer than | ||
* [max content length][ChuckerInterceptor.Builder.maxContentLength] and is guaranteed to be | ||
* gunzipped even if [request] has gzip header. | ||
*/ | ||
@Throws(IOException::class) | ||
public fun decodeRequest(request: Request, body: ByteString): String? | ||
Comment on lines
+18
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: This is more an API design question. What we're modelling here is an operation that could have 3 outcomes: success ( Have you considered different approaches (e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with this nitpick that it might be a good idea to model the result to leave less ambiguity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I considered it and I don't think it is the right choice. I don't see how it helps us in the processing pipeline. We are interested only in the first successfully decoded body.
Returning There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for your insights π I think we're fine with the current modelling for the reasons you mentioned + is the one that has the smaller API surface There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This thread started with |
||
|
||
/** | ||
* Returns a text representation of [body] that will be displayed in Chucker UI transaction, | ||
* or `null` if [response] cannot be handled by this decoder. [Body][body] is no longer than | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same typo with 2 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same, not a typo. :) |
||
* [max content length][ChuckerInterceptor.Builder.maxContentLength] and is guaranteed to be | ||
* gunzipped even if [response] has gzip header. | ||
*/ | ||
@Throws(IOException::class) | ||
public fun decodeResponse(response: Response, body: ByteString): String? | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a typo with 2
body
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe is actually valid KDoc where you can specify the display text for a link π€
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is deliberate. It allows to link to
body
property and start the sentence with a capital letter.