Skip to content

Commit

Permalink
Fix Jackson extension functions and their tests (kittinunf#777)
Browse files Browse the repository at this point in the history 
* Consistent return type in extensions (kittinunf#775)

Make all Jackson shorthand functions for asynchronous requests return
an object of `CancellableRequest` type.

* Fix tests for async Jackson extensions (kittinunf#776)

Assertions in the tests were failing silently due to the fact that
the callback was executed asynchronously. Many tests eventually became
out of date, and because of the async assertions this remained unnoticed.

This change awaits on the result of each asynchronous call to ensure all
assertions in the asynchronous callbacks are executed. Should an error
occur, it will bubble up and hence fail the test now.

This also required quite some changes in the setup procedure and assertions
to bring tests back into a good shape.
  • Loading branch information
@detouched
detouched authored Oct 14, 2020
1 parent e8993d4 commit ab45908
Show file tree
Hide file tree
Showing 2 changed files with 148 additions and 78 deletions.
12 changes: 5 additions & 7 deletions fuel-jackson/src/main/kotlin/com/github/kittinunf/fuel/jackson/FuelJackson.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ import java.io.Reader
val defaultMapper = ObjectMapper().registerKotlinModule()
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)

inline fun <reified T : Any> Request.responseObject(noinline handler: (Request, Response, Result<T, FuelError>) -> Unit) {
response(jacksonDeserializerOf(), handler)
}
inline fun <reified T : Any> Request.responseObject(noinline handler: (Request, Response, Result<T, FuelError>) -> Unit) =
response(jacksonDeserializerOf(), handler)

inline fun <reified T : Any> Request.responseObject(mapper: ObjectMapper, noinline handler: (Request, Response, Result<T, FuelError>) -> Unit) {
response(jacksonDeserializerOf(mapper), handler)
}
inline fun <reified T : Any> Request.responseObject(mapper: ObjectMapper, noinline handler: (Request, Response, Result<T, FuelError>) -> Unit) =
response(jacksonDeserializerOf(mapper), handler)

inline fun <reified T : Any> Request.responseObject(mapper: ObjectMapper, handler: ResponseHandler<T>) = response(jacksonDeserializerOf(mapper), handler)

Expand All @@ -49,4 +47,4 @@ inline fun <reified T : Any> jacksonDeserializerOf(mapper: ObjectMapper = defaul
override fun deserialize(inputStream: InputStream): T? {
return mapper.readValue(inputStream)
}
}
}
214 changes: 143 additions & 71 deletions fuel-jackson/src/test/kotlin/com/github/kittinunf/fuel/FuelJacksonTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.databind.PropertyNamingStrategy
import com.fasterxml.jackson.module.kotlin.registerKotlinModule
import com.github.kittinunf.fuel.core.FuelError
import com.github.kittinunf.fuel.core.HttpException
import com.github.kittinunf.fuel.core.Request
import com.github.kittinunf.fuel.core.Response
import com.github.kittinunf.fuel.core.ResponseHandler
Expand All @@ -20,6 +21,7 @@ import org.hamcrest.CoreMatchers.notNullValue
import org.hamcrest.CoreMatchers.nullValue
import org.junit.Assert.assertNotEquals
import org.junit.Assert.assertThat
import org.junit.Assert.fail
import org.junit.Test
import org.mockserver.matchers.Times
import java.net.HttpURLConnection
Expand All @@ -38,25 +40,33 @@ class FuelJacksonTest : MockHttpTestCase() {

Fuel.get(mock.path("user-agent"))
.responseObject(jacksonDeserializerOf<HttpBinUserAgentModel>()) { _, _, result ->
assertThat(result.component1(), instanceOf(HttpBinUserAgentModel::class.java))
assertThat(result.component1()?.userAgent, not(""))
assertThat(result.component2(), instanceOf(FuelError::class.java))
assertThat(result, instanceOf(Result.Success::class.java))
with(result as Result.Success) {
assertThat(value, instanceOf(HttpBinUserAgentModel::class.java))
assertThat(value.userAgent, not(""))
}
}
.get()
}

@Test
fun jacksonTestResponseObjectWithCustomMapper() {
mock.chain(
request = mock.request().withPath("/user-agent"),
response = mock.reflect()
response = mock.response()
.withBody("""{ "user_agent": "test" }""")
.withStatusCode(HttpURLConnection.HTTP_OK)
)

Fuel.get(mock.path("user-agent"))
.responseObject(jacksonDeserializerOf<HttpBinUserAgentModel>(createCustomMapper())) { _, _, result ->
assertThat(result.component1(), instanceOf(HttpBinUserAgentModel::class.java))
assertThat(result.component1()?.userAgent, not(""))
assertThat(result.component2(), instanceOf(FuelError::class.java))
.responseObject(jacksonDeserializerOf<HttpBinUserAgentModel>(createCustomMapper())) { _, _, result ->
assertThat(result, instanceOf(Result.Success::class.java))
with(result as Result.Success) {
assertThat(value, instanceOf(HttpBinUserAgentModel::class.java))
assertThat(value.userAgent, not(""))
}
}
.get()
}

@Test
Expand All @@ -68,9 +78,12 @@ class FuelJacksonTest : MockHttpTestCase() {

Fuel.get(mock.path("user-agent"))
.responseObject(jacksonDeserializerOf<HttpBinUserAgentModel>()) { _, _, result ->
assertThat(result.component1(), notNullValue())
assertThat(result.component2(), instanceOf(Result.Failure::class.java))
assertThat(result, instanceOf(Result.Failure::class.java))
with(result as Result.Failure) {
assertThat(error, notNullValue())
}
}
.get()
}

@Test
Expand All @@ -81,10 +94,13 @@ class FuelJacksonTest : MockHttpTestCase() {
)

Fuel.get(mock.path("user-agent"))
.responseObject(jacksonDeserializerOf<HttpBinUserAgentModel>(createCustomMapper())) { _, _, result ->
assertThat(result.component1(), notNullValue())
assertThat(result.component2(), instanceOf(Result.Failure::class.java))
.responseObject(jacksonDeserializerOf<HttpBinUserAgentModel>(createCustomMapper())) { _, _, result ->
assertThat(result, instanceOf(Result.Failure::class.java))
with(result as Result.Failure) {
assertThat(error, notNullValue())
}
}
.get()
}

@Test
Expand All @@ -96,23 +112,33 @@ class FuelJacksonTest : MockHttpTestCase() {

Fuel.get(mock.path("user-agent"))
.responseObject<HttpBinUserAgentModel> { _, _, result ->
assertThat(result.component1(), notNullValue())
assertThat(result.component2(), notNullValue())
assertThat(result, instanceOf(Result.Success::class.java))
with(result as Result.Success) {
assertThat(value, instanceOf(HttpBinUserAgentModel::class.java))
assertThat(value.userAgent, not(""))
}
}
.get()
}

@Test
fun jacksonTestResponseDeserializerObjectWithCustomMapper() {
mock.chain(
request = mock.request().withPath("/user-agent"),
response = mock.reflect()
response = mock.response()
.withBody("""{ "user_agent": "test" }""")
.withStatusCode(HttpURLConnection.HTTP_OK)
)

Fuel.get(mock.path("user-agent"))
.responseObject<HttpBinUserAgentModel>(createCustomMapper()) { _, _, result ->
assertThat(result.component1(), notNullValue())
assertThat(result.component2(), notNullValue())
.responseObject<HttpBinUserAgentModel>(createCustomMapper()) { _, _, result ->
assertThat(result, instanceOf(Result.Success::class.java))
with(result as Result.Success) {
assertThat(value, instanceOf(HttpBinUserAgentModel::class.java))
assertThat(value.userAgent, not(""))
}
}
.get()
}

@Test
Expand All @@ -124,9 +150,12 @@ class FuelJacksonTest : MockHttpTestCase() {

Fuel.get(mock.path("user-agent"))
.responseObject<HttpBinUserAgentModel> { _, _, result ->
assertThat(result.component1(), notNullValue())
assertThat(result.component2(), instanceOf(Result.Failure::class.java))
assertThat(result, instanceOf(Result.Failure::class.java))
with(result as Result.Failure) {
assertThat(error, notNullValue())
}
}
.get()
}

@Test
Expand All @@ -143,9 +172,10 @@ class FuelJacksonTest : MockHttpTestCase() {
}

override fun failure(request: Request, response: Response, error: FuelError) {
assertThat(error, notNullValue())
fail("Request shouldn't have failed")
}
})
.get()
}

@Test
Expand All @@ -156,15 +186,16 @@ class FuelJacksonTest : MockHttpTestCase() {
)

Fuel.get(mock.path("user-agent"))
.responseObject(createCustomMapper(), object : ResponseHandler<HttpBinUserAgentModel> {
override fun success(request: Request, response: Response, value: HttpBinUserAgentModel) {
assertThat(value, notNullValue())
}

override fun failure(request: Request, response: Response, error: FuelError) {
assertThat(error, notNullValue())
}
})
.responseObject(createCustomMapper(), object : ResponseHandler<HttpBinUserAgentModel> {
override fun success(request: Request, response: Response, value: HttpBinUserAgentModel) {
assertThat(value, notNullValue())
}

override fun failure(request: Request, response: Response, error: FuelError) {
fail("Request shouldn't have failed")
}
})
.get()
}

@Test
Expand All @@ -177,13 +208,14 @@ class FuelJacksonTest : MockHttpTestCase() {
Fuel.get(mock.path("user-agent"))
.responseObject(createCustomMapper(), object : ResponseHandler<HttpBinUserAgentModel> {
override fun success(request: Request, response: Response, value: HttpBinUserAgentModel) {
assertThat(value, notNullValue())
fail("Request should have failed")
}

override fun failure(request: Request, response: Response, error: FuelError) {
assertThat(error, instanceOf(Result.Failure::class.java))
assertThat(error.exception, instanceOf(HttpException::class.java))
}
})
.get()
}

@Test
Expand Down Expand Up @@ -240,54 +272,94 @@ class FuelJacksonTest : MockHttpTestCase() {
fun testProcessingGenericList() {
mock.chain(
request = mock.request().withPath("/issues"),
response = mock.response().withBody("[ " +
"{ \"id\": 1, \"title\": \"issue 1\", \"number\": null }, " +
"{ \"id\": 2, \"title\": \"issue 2\", \"number\": 32 }, " +
" ]").withStatusCode(HttpURLConnection.HTTP_OK)
response = mock.response()
.withBody(
"""
[
{
"id": 1,
"title": "issue 1",
"number": null
},
{
"id": 2,
"title": "issue 2",
"number": 32
}
]
"""
)
.withStatusCode(HttpURLConnection.HTTP_OK)
)

Fuel.get(mock.path("issues")).responseObject<List<IssueInfo>> { _, _, result ->
val issues = result.get()
assertNotEquals(issues.size, 0)
assertThat(issues[0], isA(IssueInfo::class.java))
}
Fuel.get(mock.path("issues"))
.responseObject<List<IssueInfo>> { _, _, result ->
val issues = result.get()
assertNotEquals(issues.size, 0)
assertThat(issues[0], isA(IssueInfo::class.java))
}
.get()
}

@Test
fun manualDeserializationShouldWork() {
mock.chain(
request = mock.request().withPath("/issues"),
response = mock.response().withBody("[ " +
"{ \"id\": 1, \"title\": \"issue 1\", \"number\": null }, " +
"{ \"id\": 2, \"title\": \"issue 2\", \"number\": 32 }, " +
" ]").withStatusCode(HttpURLConnection.HTTP_OK),
times = Times.exactly(2)
response = mock.response()
.withBody(
"""
[
{
"id": 1,
"title": "issue 1",
"number": null
},
{
"id": 2,
"title": "issue 2",
"number": 32
}
]
"""
)
.withStatusCode(HttpURLConnection.HTTP_OK),
times = Times.exactly(5)
)

Fuel.get(mock.path("issues")).response { _: Request, response: Response, _ ->
val issueList = jacksonDeserializerOf<List<IssueInfo>>().deserialize(response)
assertThat(issueList[0], isA(IssueInfo::class.java))
}

Fuel.get(mock.path("issues")).response { _: Request, response: Response, _ ->
val issueList = jacksonDeserializerOf<List<IssueInfo>>().deserialize(response.body().toStream())!!
assertThat(issueList[0], isA(IssueInfo::class.java))
}

Fuel.get(mock.path("issues")).response { _: Request, response: Response, _ ->
val issueList = jacksonDeserializerOf<List<IssueInfo>>().deserialize(response.body().toStream().reader())!!
assertThat(issueList[0], isA(IssueInfo::class.java))
}

Fuel.get(mock.path("issues")).response { _: Request, _, result: Result<ByteArray, FuelError> ->
val issueList = jacksonDeserializerOf<List<IssueInfo>>().deserialize(result.get())!!
assertThat(issueList[0], isA(IssueInfo::class.java))
}

Fuel.get(mock.path("issues")).responseString { _: Request, _: Response, result: Result<String, FuelError> ->
val issueList = jacksonDeserializerOf<List<IssueInfo>>().deserialize(result.get())!!
assertThat(issueList[0], isA(IssueInfo::class.java))
}
Fuel.get(mock.path("issues"))
.response { _: Request, response: Response, _ ->
val issueList = jacksonDeserializerOf<List<IssueInfo>>().deserialize(response)
assertThat(issueList[0], isA(IssueInfo::class.java))
}
.get()

Fuel.get(mock.path("issues"))
.response { _: Request, response: Response, _ ->
val issueList = jacksonDeserializerOf<List<IssueInfo>>().deserialize(response.body().toStream())!!
assertThat(issueList[0], isA(IssueInfo::class.java))
}
.get()

Fuel.get(mock.path("issues"))
.response { _: Request, response: Response, _ ->
val issueList = jacksonDeserializerOf<List<IssueInfo>>().deserialize(response.body().toStream().reader())!!
assertThat(issueList[0], isA(IssueInfo::class.java))
}
.get()

Fuel.get(mock.path("issues"))
.response { _: Request, _, result: Result<ByteArray, FuelError> ->
val issueList = jacksonDeserializerOf<List<IssueInfo>>().deserialize(result.get())!!
assertThat(issueList[0], isA(IssueInfo::class.java))
}
.get()

Fuel.get(mock.path("issues"))
.responseString { _: Request, _: Response, result: Result<String, FuelError> ->
val issueList = jacksonDeserializerOf<List<IssueInfo>>().deserialize(result.get())!!
assertThat(issueList[0], isA(IssueInfo::class.java))
}
.get()
}

private fun createCustomMapper(): ObjectMapper {
Expand Down

0 comments on commit ab45908

Please sign in to comment.