-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
Release/0.7.0 #59
Release/0.7.0 #59
Conversation
Codecov Report
@@ Coverage Diff @@
## master #59 +/- ##
===========================================
+ Coverage 94.55% 94.76% +0.2%
- Complexity 67 86 +19
===========================================
Files 17 21 +4
Lines 202 210 +8
Branches 12 13 +1
===========================================
+ Hits 191 199 +8
Misses 2 2
Partials 9 9
Continue to review full report at Codecov.
|
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.
Good overall. Need to improve provided tests checking request body content type.
@@ -100,9 +100,14 @@ val response: Response = httpPost { | |||
"email" to "john.doe@gmail.com" // email=john.doe@gmail.com | |||
} | |||
} | |||
// or | |||
body { | |||
form("login=user&email=john.doe@gmail.com") |
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.
Have a feeling that it’s better to provide a form content value outside httpPost { } scope to provide a more valuable case.
How do u think @shtykh
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 don't see a lot of value in it. Can you make an example please?
import okhttp3.RequestBody | ||
import java.io.File | ||
|
||
@HttpDslMarker |
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.
This annotation fixes issue #57
|
||
fun body(init: BodyContext.() -> RequestBody) { | ||
body = BodyContext().init() | ||
fun body(contentType: String? = null, init: BodyContext.() -> RequestBody) { |
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.
Content type could be declared both in body and in header. Need to check who wins in okhttp. @shtykh
form("login=user&email=john.doe@gmail.com") | ||
} | ||
}.use { | ||
println(it.body()?.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.
Can we add real checks instead of prints in this class?
import java.io.File | ||
|
||
@HttpDslMarker | ||
class BodyContext(type: 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.
Need to test for a while to understand how to deal with a multiparty later
4a9640c
to
eb85626
Compare
private fun MediaType.create(contentProducer: () -> Any): RequestBody { | ||
return when (val content = contentProducer()) { | ||
is String -> RequestBody.create(this, content) | ||
is File -> RequestBody.create(this, content) |
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.
@shtykh this is unreachable code. The reason is:
fun file(content: File): RequestBody = RequestBody.create(mediaType, content)
fun file(content: File): RequestBody = RequestBody.create(mediaType, content)
fun bytes(content: ByteArray): RequestBody = RequestBody.create(mediaType, content)
All functions invoke RequestBody.create
method, but MediaType.create
is not used.
The only usege is for json { }
and form { }
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.
fixed
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.
Will modify BodyContext
class and doublecheck code coverage
@shtykh pls make a final review |
fixed a typo in exception msg
a4584a2
to
83f7d43
Compare
No description provided.