From 24c82e38b840cef1b3f080268195a060f6af376d Mon Sep 17 00:00:00 2001 From: Nasser Anssari Date: Wed, 29 Jan 2025 13:33:20 +0300 Subject: [PATCH] add fallback body to paginator to handle empty responses --- .../sdk/core/model/paging/Paginator.kt | 9 ++++-- .../sdk/core/model/paging/ResponseState.kt | 9 ++++-- .../sdk/core/model/paging/PaginatorTest.kt | 28 ++++++++++--------- .../sdk/generators/openapi/MustacheHelpers.kt | 11 +++++++- 4 files changed, 38 insertions(+), 19 deletions(-) diff --git a/core/src/main/kotlin/com/expediagroup/sdk/core/model/paging/Paginator.kt b/core/src/main/kotlin/com/expediagroup/sdk/core/model/paging/Paginator.kt index 6e9644f12..cb690976e 100644 --- a/core/src/main/kotlin/com/expediagroup/sdk/core/model/paging/Paginator.kt +++ b/core/src/main/kotlin/com/expediagroup/sdk/core/model/paging/Paginator.kt @@ -24,6 +24,7 @@ import io.ktor.client.statement.HttpResponse sealed class BasePaginator( private val client: Client, firstResponse: Response, + private val fallbackBody: T, private val getBody: suspend (HttpResponse) -> T ) : Iterator { private var state: ResponseState = DefaultResponseState(firstResponse) @@ -41,7 +42,7 @@ sealed class BasePaginator( protected fun nextResponse(): Response { val response = state.getNextResponse() - state = ResponseStateFactory.getState(extractLink(response.headers), client, getBody) + state = ResponseStateFactory.getState(extractLink(response.headers), client, fallbackBody, getBody) return response } } @@ -56,8 +57,9 @@ sealed class BasePaginator( class Paginator( client: Client, firstResponse: Response, + fallbackBody: T, getBody: suspend (HttpResponse) -> T -) : BasePaginator(client, firstResponse, getBody) { +) : BasePaginator(client, firstResponse, fallbackBody, getBody) { /** * Returns the body of the next response. * @@ -76,8 +78,9 @@ class Paginator( class ResponsePaginator( client: Client, firstResponse: Response, + fallbackBody: T, getBody: suspend (HttpResponse) -> T -) : BasePaginator, T>(client, firstResponse, getBody) { +) : BasePaginator, T>(client, firstResponse, fallbackBody, getBody) { /** * Returns the next response. * diff --git a/core/src/main/kotlin/com/expediagroup/sdk/core/model/paging/ResponseState.kt b/core/src/main/kotlin/com/expediagroup/sdk/core/model/paging/ResponseState.kt index e678f68c6..21cb72eae 100644 --- a/core/src/main/kotlin/com/expediagroup/sdk/core/model/paging/ResponseState.kt +++ b/core/src/main/kotlin/com/expediagroup/sdk/core/model/paging/ResponseState.kt @@ -18,6 +18,7 @@ package com.expediagroup.sdk.core.model.paging import com.expediagroup.sdk.core.client.Client import com.expediagroup.sdk.core.model.Response import io.ktor.client.statement.HttpResponse +import io.ktor.client.statement.bodyAsBytes import kotlinx.coroutines.runBlocking internal interface ResponseState { @@ -51,6 +52,7 @@ internal class LastResponseState : ResponseState { internal class FetchLinkState( private val link: String, private val client: Client, + private val fallbackBody: T, private val getBody: suspend (HttpResponse) -> T ) : ResponseState { override fun getNextResponse(): Response { @@ -66,7 +68,9 @@ internal class FetchLinkState( } private suspend fun parseBody(response: HttpResponse): T { - return getBody(response) + // response.bodyAsBytes() applies all plugins + // if content-length header is set, response.contentLength could be used instead + return if (response.bodyAsBytes().isEmpty()) fallbackBody else getBody(response) } } @@ -75,9 +79,10 @@ internal class ResponseStateFactory { fun getState( link: String?, client: Client, + fallbackBody: T, getBody: suspend (HttpResponse) -> T ): ResponseState { - return link?.let { FetchLinkState(it, client, getBody) } ?: LastResponseState() + return link?.let { FetchLinkState(it, client, fallbackBody, getBody) } ?: LastResponseState() } } } diff --git a/core/src/test/kotlin/com/expediagroup/sdk/core/model/paging/PaginatorTest.kt b/core/src/test/kotlin/com/expediagroup/sdk/core/model/paging/PaginatorTest.kt index 4153cdd54..6cbf391c5 100644 --- a/core/src/test/kotlin/com/expediagroup/sdk/core/model/paging/PaginatorTest.kt +++ b/core/src/test/kotlin/com/expediagroup/sdk/core/model/paging/PaginatorTest.kt @@ -44,7 +44,7 @@ class PaginatorTest { fun `test paginator with one response`() { val firstResponse = Response(200, "first", emptyMap()) - val paginator = Paginator(client, firstResponse, getBody) + val paginator = Paginator(client, firstResponse, EMPTY_STRING, getBody) assertTrue(paginator.hasNext()) assertEquals("first", paginator.next()) assertFalse(paginator.hasNext()) @@ -54,7 +54,7 @@ class PaginatorTest { fun `test paginator with multiple responses`() { val firstResponse = Response(200, "first", mapOf("link" to listOf("; rel=\"next\""))) - val paginator = Paginator(client, firstResponse, getBody) + val paginator = Paginator(client, firstResponse, EMPTY_STRING, getBody) assertTrue(paginator.hasNext()) assertEquals("first", paginator.next()) assertTrue(paginator.hasNext()) @@ -66,7 +66,7 @@ class PaginatorTest { fun `test paginator with multiple responses and total results`() { val firstResponse = Response(200, "first", mapOf("link" to listOf("; rel=\"next\""), "pagination-total-results" to listOf("2"))) - val paginator = Paginator(client, firstResponse, getBody) + val paginator = Paginator(client, firstResponse, EMPTY_STRING, getBody) assertTrue(paginator.hasNext()) assertEquals("first", paginator.next()) assertTrue(paginator.hasNext()) @@ -79,7 +79,7 @@ class PaginatorTest { fun `test paginator as list`() { val firstResponse = Response(200, "first", mapOf("link" to listOf("; rel=\"next\""), "pagination-total-results" to listOf("2"))) - val paginator = Paginator(client, firstResponse, getBody) + val paginator = Paginator(client, firstResponse, EMPTY_STRING, getBody) val list = paginator.asSequence().toList() assertEquals(2, list.size) assertEquals("first", list[0]) @@ -93,7 +93,7 @@ class PaginatorTest { fun `test response paginator with one response`() { val firstResponse = Response(200, "first", emptyMap()) - val paginator = ResponsePaginator(client, firstResponse, getBody) + val paginator = ResponsePaginator(client, firstResponse, EMPTY_STRING, getBody) assertTrue(paginator.hasNext()) assertEquals("first", paginator.next().data) assertFalse(paginator.hasNext()) @@ -103,7 +103,7 @@ class PaginatorTest { fun `test response paginator with multiple responses`() { val firstResponse = Response(200, "first", mapOf("link" to listOf("; rel=\"next\""))) - val paginator = ResponsePaginator(client, firstResponse, getBody) + val paginator = ResponsePaginator(client, firstResponse, EMPTY_STRING, getBody) assertTrue(paginator.hasNext()) assertEquals("first", paginator.next().data) assertTrue(paginator.hasNext()) @@ -115,7 +115,7 @@ class PaginatorTest { fun `test response paginator with multiple responses and total results`() { val firstResponse = Response(200, "first", mapOf("link" to listOf("; rel=\"next\""), "pagination-total-results" to listOf("2"))) - val paginator = ResponsePaginator(client, firstResponse, getBody) + val paginator = ResponsePaginator(client, firstResponse, EMPTY_STRING, getBody) assertTrue(paginator.hasNext()) assertEquals("first", paginator.next().data) assertTrue(paginator.hasNext()) @@ -124,12 +124,13 @@ class PaginatorTest { assertEquals(2, paginator.paginationTotalResults) } - @Test - fun `should return empty value when next response body is empty`() { + @ParameterizedTest + @ValueSource(strings = [EMPTY_STRING, "second", "some_value"]) + fun `should return fallback value when next response body is empty`(fallbackBody: String) { val client = createRapidClient(createEmptyResponseEngine()) val firstResponse = Response(200, "first", mapOf("link" to listOf("; rel=\"next\""), "pagination-total-results" to listOf("2"))) - val paginator = ResponsePaginator(client, firstResponse, getBody) + val paginator = ResponsePaginator(client, firstResponse, EMPTY_STRING, getBody) assertTrue(paginator.hasNext()) assertEquals("first", paginator.next().data) assertTrue(paginator.hasNext()) @@ -137,12 +138,13 @@ class PaginatorTest { assertFalse(paginator.hasNext()) } - @Test - fun `should return empty value when next response body is empty and gzip encoded`() { + @ParameterizedTest + @ValueSource(strings = [EMPTY_STRING, "second", "some_value"]) + fun `should return fallback value when next response body is empty and gzip encoded`(fallbackBody: String) { val client = createRapidClient(createGzipEncodedEmptyResponseEngine()) val firstResponse = Response(200, "first", mapOf("link" to listOf("; rel=\"next\""), "pagination-total-results" to listOf("2"))) - val paginator = ResponsePaginator(client, firstResponse, getBody) + val paginator = ResponsePaginator(client, firstResponse, EMPTY_STRING, getBody) assertTrue(paginator.hasNext()) assertEquals("first", paginator.next().data) assertTrue(paginator.hasNext()) diff --git a/generator/openapi/src/main/kotlin/com/expediagroup/sdk/generators/openapi/MustacheHelpers.kt b/generator/openapi/src/main/kotlin/com/expediagroup/sdk/generators/openapi/MustacheHelpers.kt index 3c9fbab1d..1280a76b5 100644 --- a/generator/openapi/src/main/kotlin/com/expediagroup/sdk/generators/openapi/MustacheHelpers.kt +++ b/generator/openapi/src/main/kotlin/com/expediagroup/sdk/generators/openapi/MustacheHelpers.kt @@ -31,7 +31,16 @@ val mustacheHelpers = val paginationHeaders = listOf("Pagination-Total-Results", "Link") val availableHeaders = operation.responses.find { it.code == "200" }?.headers?.filter { it.baseName in paginationHeaders } if (availableHeaders?.size == paginationHeaders.size) { - fragment.execute(writer) + val fallbackBody = + when { + operation.returnType.startsWith("kotlin.collections.List") -> "emptyList()" + operation.returnType.startsWith("kotlin.collections.Map") -> "emptyMap()" + operation.returnType.startsWith("kotlin.collections.Set") -> "emptySet()" + else -> "" + } + + val context = mapOf("fallbackBody" to fallbackBody) + fragment.execute(context, writer) } } },