Skip to content
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

Wrap parse exceptions in Response object. #4

Merged
merged 5 commits into from
Feb 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Catch XML parse errors and wrap them in a new Response object instead.
  • Loading branch information
Bram-- committed Feb 11, 2024
commit dd77506056c5839e4448e9f567a21caf6c8a62f0
6 changes: 3 additions & 3 deletions src/main/kotlin/org/audux/bgg/BggClient.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import org.audux.bgg.module.BggKtorClient
import org.audux.bgg.module.BggXmlObjectMapper
import org.audux.bgg.module.appModule
import org.audux.bgg.request.Request
import org.audux.bgg.response.Response
import org.jetbrains.annotations.VisibleForTesting
import org.koin.core.component.KoinComponent
import org.koin.core.component.inject
Expand Down Expand Up @@ -76,10 +77,10 @@ class BggClient : KoinComponent, AutoCloseable {
}

/** Calls/Launches a request and returns it's response. */
internal suspend fun <T> call(request: suspend () -> T) = request()
internal suspend fun <T> call(request: suspend () -> Response<T>) = request()

/** Returns a wrapped request for later execution. */
internal fun <T> request(request: suspend () -> T) = Request(this, request)
internal fun <T> request(request: suspend () -> Response<T>) = Request(this, request)

/**
* Returns the current [io.ktor.client.engine.HttpClientEngine] used by this client. Used for
Expand Down Expand Up @@ -118,7 +119,6 @@ class BggClient : KoinComponent, AutoCloseable {
/** Isolated Koin Context for BGG Client. */
class BggClientKoinContext {
private val koinApp = koinApplication {
BggClient.setLoggerSeverity(BggClient.Companion.Severity.Warn)
logger(KermitKoinLogger(Logger.withTag("koin")))
modules(appModule)
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/kotlin/org/audux/bgg/module/AppModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ val appModule = module {

factory(named<BggKtorClient>()) {
HttpClient(get(HttpClientEngine::class, named<BggHttpEngine>())) {
install(ClientRateLimitPlugin) { requestLimit = 10 }
install(HttpRequestRetry) {
exponentialDelay()
retryIf(maxRetries = 5) { _, response ->
Expand All @@ -80,7 +81,6 @@ val appModule = module {
}
}
}
install(ClientRateLimitPlugin) { requestLimit = 10 }

expectSuccess = true
}
Expand Down
3 changes: 2 additions & 1 deletion src/main/kotlin/org/audux/bgg/request/Collection.kt
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import org.audux.bgg.request.Constants.PATH_COLLECTION
import org.audux.bgg.request.Constants.REQUEST_DATE_TIME_FORMAT
import org.audux.bgg.request.Constants.XML2_API_URL
import org.audux.bgg.response.Collection
import org.audux.bgg.response.Response

/**
* Request details about a user's collection.
Expand Down Expand Up @@ -190,5 +191,5 @@ fun BggClient.collection(
}
}
}
.let { mapper.readValue(it.bodyAsText(), Collection::class.java) }
.let { Response.from<Collection>(it.bodyAsText(), mapper) }
}
3 changes: 2 additions & 1 deletion src/main/kotlin/org/audux/bgg/request/FamilyItems.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import org.audux.bgg.BggClient
import org.audux.bgg.common.FamilyType
import org.audux.bgg.common.HotListType
import org.audux.bgg.response.Family
import org.audux.bgg.response.Response

/**
* Family thing endpoint that retrieve details about the given family ID and associated `Link`
Expand All @@ -42,5 +43,5 @@ fun BggClient.familyItems(ids: Array<Int>, types: Array<FamilyType> = arrayOf())
}
}
}
.let { mapper.readValue(it.bodyAsText(), Family::class.java) }
.let { Response.from<Family>(it.bodyAsText(), mapper) }
}
3 changes: 2 additions & 1 deletion src/main/kotlin/org/audux/bgg/request/ForumLists.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import org.audux.bgg.BggClient
import org.audux.bgg.common.ForumListType
import org.audux.bgg.response.Family
import org.audux.bgg.response.ForumList
import org.audux.bgg.response.Response
import org.audux.bgg.response.Thing

/**
Expand All @@ -40,5 +41,5 @@ fun BggClient.forumList(id: Int, type: ForumListType) = request {
}
}
}
.let { mapper.readValue(it.bodyAsText(), ForumList::class.java) }
.let { Response.from<ForumList>(it.bodyAsText(), mapper) }
}
3 changes: 2 additions & 1 deletion src/main/kotlin/org/audux/bgg/request/Forums.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import io.ktor.client.statement.bodyAsText
import io.ktor.http.appendPathSegments
import org.audux.bgg.BggClient
import org.audux.bgg.response.Forum
import org.audux.bgg.response.Response

/**
* Retrieves the list of threads for the given forum id.
Expand All @@ -38,5 +39,5 @@ fun BggClient.forum(id: Int, page: Int? = null) = request {
page?.let { parameters.append(Constants.PARAM_PAGE, page.toString()) }
}
}
.let { mapper.readValue(it.bodyAsText(), Forum::class.java) }
.let { Response.from<Forum>(it.bodyAsText(), mapper) }
}
3 changes: 2 additions & 1 deletion src/main/kotlin/org/audux/bgg/request/GeekList.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.audux.bgg.request.Constants.PARAM_COMMENTS
import org.audux.bgg.request.Constants.PATH_GEEK_LIST
import org.audux.bgg.request.Constants.XML1_API_URL
import org.audux.bgg.response.GeekList
import org.audux.bgg.response.Response

/**
* Geek list endpoint, retrieves a specific geek list by its ID.
Expand All @@ -39,5 +40,5 @@ fun BggClient.geekList(id: Number, comments: Inclusion? = null) = request {
comments?.let { parameters.append(PARAM_COMMENTS, it.toParam()) }
}
}
.let { mapper.readValue(it.bodyAsText(), GeekList::class.java) }
.let { Response.from<GeekList>(it.bodyAsText(), mapper) }
}
3 changes: 2 additions & 1 deletion src/main/kotlin/org/audux/bgg/request/Guilds.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import org.audux.bgg.request.Constants.PARAM_SORT
import org.audux.bgg.request.Constants.PATH_GUILDS
import org.audux.bgg.request.Constants.XML2_API_URL
import org.audux.bgg.response.Guild
import org.audux.bgg.response.Response

/**
* Retrieve information about the given guild (id) like name, description, members etc.
Expand All @@ -50,5 +51,5 @@ fun BggClient.guilds(
page?.let { parameters.append(PARAM_PAGE, it.toString()) }
}
}
.let { mapper.readValue(it.bodyAsText(), Guild::class.java) }
.let { Response.from<Guild>(it.bodyAsText(), mapper) }
}
3 changes: 2 additions & 1 deletion src/main/kotlin/org/audux/bgg/request/HotItems.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.audux.bgg.request.Constants.PARAM_TYPE
import org.audux.bgg.request.Constants.PATH_HOT
import org.audux.bgg.request.Constants.XML2_API_URL
import org.audux.bgg.response.HotList
import org.audux.bgg.response.Response

/**
* Hotness endpoint that retrieve the list of most 50 active items on the site filtered by type.
Expand All @@ -37,5 +38,5 @@ fun BggClient.hotItems(type: HotListType? = null) = request {
type?.let { parameters.append(PARAM_TYPE, it.param) }
}
}
.let { mapper.readValue(it.bodyAsText(), HotList::class.java) }
.let { Response.from<HotList>(it.bodyAsText(), mapper) }
}
4 changes: 2 additions & 2 deletions src/main/kotlin/org/audux/bgg/request/Plays.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import org.audux.bgg.request.Constants.PARAM_USERNAME
import org.audux.bgg.request.Constants.PATH_PLAYS
import org.audux.bgg.request.Constants.XML2_API_URL
import org.audux.bgg.response.Plays
import org.audux.bgg.response.Response

/**
* Request a list of plays (max 100 at the time) for the given user.
Expand All @@ -55,7 +56,6 @@ fun BggClient.plays(
page: Number? = null,
) = request {
val formatter = DateTimeFormatter.ofPattern(Constants.REQUEST_DATE_FORMAT)

client
.get(XML2_API_URL) {
url {
Expand All @@ -73,5 +73,5 @@ fun BggClient.plays(
page?.let { parameters.append(PARAM_PAGE, it.toString()) }
}
}
.let { mapper.readValue(it.bodyAsText(), Plays::class.java) }
.let { Response.from<Plays>(it.bodyAsText(), mapper) }
}
5 changes: 3 additions & 2 deletions src/main/kotlin/org/audux/bgg/request/Request.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@
package org.audux.bgg.request

import org.audux.bgg.BggClient
import org.audux.bgg.response.Response

/** Encapsulates a request to BGG so it can be scheduled or queued for later execution. */
class Request<T>(private val client: BggClient, private val request: suspend () -> T) {
class Request<T>(private val client: BggClient, private val request: suspend () -> Response<T>) {
/**
* Execute the encapsulated [T] request asynchronously and returns the `response` in the
* provided block if successful.
*/
fun callAsync(response: (T) -> Unit) {
fun callAsync(response: (Response<T>) -> Unit) {
client.callAsync(request, response)
}

Expand Down
3 changes: 2 additions & 1 deletion src/main/kotlin/org/audux/bgg/request/Search.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import org.audux.bgg.request.Constants.PARAM_QUERY
import org.audux.bgg.request.Constants.PARAM_TYPE
import org.audux.bgg.request.Constants.PATH_SEARCH
import org.audux.bgg.request.Constants.XML2_API_URL
import org.audux.bgg.response.Response
import org.audux.bgg.response.SearchResults

/**
Expand Down Expand Up @@ -52,5 +53,5 @@ fun BggClient.search(
}
}
}
.let { mapper.readValue(it.bodyAsText(), SearchResults::class.java) }
.let { Response.from<SearchResults>(it.bodyAsText(), mapper) }
}
3 changes: 2 additions & 1 deletion src/main/kotlin/org/audux/bgg/request/Things.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import org.audux.bgg.request.Constants.PARAM_VERSIONS
import org.audux.bgg.request.Constants.PARAM_VIDEOS
import org.audux.bgg.request.Constants.PATH_THING
import org.audux.bgg.request.Constants.XML2_API_URL
import org.audux.bgg.response.Response
import org.audux.bgg.response.Things

/**
Expand Down Expand Up @@ -99,5 +100,5 @@ fun BggClient.things(
}
}
}
.let { mapper.readValue(it.bodyAsText(), Things::class.java) }
.let { Response.from<Things>(it.bodyAsText(), mapper) }
}
3 changes: 2 additions & 1 deletion src/main/kotlin/org/audux/bgg/request/Thread.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import io.ktor.http.appendPathSegments
import java.time.LocalDateTime
import java.time.format.DateTimeFormatter
import org.audux.bgg.BggClient
import org.audux.bgg.response.Response
import org.audux.bgg.response.Thread

/**
Expand Down Expand Up @@ -53,5 +54,5 @@ fun BggClient.thread(
}
}
}
.let { mapper.readValue(it.bodyAsText(), Thread::class.java) }
.let { Response.from<Thread>(it.bodyAsText(), mapper) }
}
3 changes: 2 additions & 1 deletion src/main/kotlin/org/audux/bgg/request/User.kt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import org.audux.bgg.request.Constants.PARAM_PAGE
import org.audux.bgg.request.Constants.PARAM_TOP
import org.audux.bgg.request.Constants.PATH_USER
import org.audux.bgg.request.Constants.XML2_API_URL
import org.audux.bgg.response.Response
import org.audux.bgg.response.User

/**
Expand Down Expand Up @@ -69,5 +70,5 @@ fun BggClient.user(
page?.let { parameters.append(PARAM_PAGE, it.toString()) }
}
}
.let { mapper.readValue(it.bodyAsText(), User::class.java) }
.let { Response.from<User>(it.bodyAsText(), mapper) }
}
3 changes: 3 additions & 0 deletions src/main/kotlin/org/audux/bgg/response/Collection.kt
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ data class CollectionItem(
/** Name the user has given the thing or primary name of the thing by default. */
val name: String,

/** The primary name of the thing by default. */
val originalName: String? = null,

/** Optional year of publishing. */
val yearPublished: Number? = null,

Expand Down
1 change: 0 additions & 1 deletion src/main/kotlin/org/audux/bgg/response/Guild.kt
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ data class Guild(
val description: String?,
val location: Location?,
val members: GuildMembers?,
val error: String?,
)

data class Location(
Expand Down
41 changes: 41 additions & 0 deletions src/main/kotlin/org/audux/bgg/response/Response.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* Copyright 2023-2024 Bram Wijnands
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http:https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package org.audux.bgg.response

import co.touchlab.kermit.Logger
import com.fasterxml.jackson.core.JacksonException
import com.fasterxml.jackson.databind.ObjectMapper
import io.ktor.client.statement.bodyAsText

data class Response<T>(
val error: String? = null,
val data: T? = null,
) {
/** Whether the request was successful or not. */
fun isSuccess() = error.isNullOrBlank()

/** Whether the request was erroneous or not. */
fun isError() = !isSuccess()

companion object {
inline fun <reified T> from(bodyAsText: String, mapper: ObjectMapper): Response<T> {
return try {
Response(data = mapper.readValue(bodyAsText, T::class.java))
} catch (e: JacksonException) {
Logger.i("Error parsing response", e)
Response(error = bodyAsText)
}
}
}
}
48 changes: 46 additions & 2 deletions src/test/kotlin/org/audux/bgg/request/CollectionRequestTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,48 @@ import org.koin.test.KoinTest

/** Unit tests for [collection] extension function. */
class CollectionRequestTest : KoinTest {
@Test
fun `Makes a request with a user that does not exist`() {
runBlocking {
val client = TestUtils.setupEngineAndRequest("collection?username=userdoesnotexist")

val response =
client
.collection(userName = "userdoesnotexist", subType = ThingType.RPG_ITEM)
.call()

val engine = client.engine() as MockEngine
val request = engine.requestHistory[0]
assertThat(engine.requestHistory).hasSize(1)
assertThat(request.method).isEqualTo(HttpMethod.Get)
assertThat(request.headers)
.isEqualTo(
Headers.build {
appendAll("Accept", listOf("*/*"))
appendAll("Accept-Charset", listOf("UTF-8"))
}
)
assertThat(request.url)
.isEqualTo(
Url(
"https://boardgamegeek.com/xmlapi2/collection?username=userdoesnotexist&subtype=rpgitem"
)
)
assertThat(response.isError()).isTrue()
assertThat(response.isSuccess()).isFalse()
assertThat(response.error)
.isEqualTo(
"<?xml version=\"1.0\" encoding=\"utf-8\" standalone=\"yes\" ?>\n" +
"<errors>\n" +
" <error>\n" +
" <message>Invalid username specified</message>\n" +
" </error>\n" +
"</errors>"
)
assertThat(response.data).isNull()
}
}

@Test
fun `Makes a request with minimum parameters`() {
runBlocking {
Expand Down Expand Up @@ -59,7 +101,9 @@ class CollectionRequestTest : KoinTest {
"https://boardgamegeek.com/xmlapi2/collection?username=Noveaux&subtype=rpgitem"
)
)
assertThat(response.items).hasSize(1)
assertThat(response.isError()).isFalse()
assertThat(response.isSuccess()).isTrue()
assertThat(response.data!!.items).hasSize(1)
}
}

Expand Down Expand Up @@ -147,7 +191,7 @@ class CollectionRequestTest : KoinTest {
)
.build()
assertThat(request.url).isEqualTo(expectedUrl)
assertThat(response.items).hasSize(105)
assertThat(response.data!!.items).hasSize(105)
}
}
}
Loading
Loading