Skip to content

Commit

Permalink
Fixing a crash when loading Feeds
Browse files Browse the repository at this point in the history
Apparently there is a chance that the API just straight up returns null as a valid response.
In this scenario our JsonDeserializer fails and crashes the app. And in the case that this value is in the first page of the feed,
we will constantly crash on startup.

In order to safeguard against this, I just created a wrapper type and a custom deserializer that just checks if a gauranteed property
exists in the json response, and if it does, use the normal item deserializer, otherwise map it to an empty object that is mostly a marker
type, we can just ignore it.
  • Loading branch information
Rahkeen committed Jul 25, 2024
1 parent c6173bd commit fe0ab0f
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package com.emergetools.hackernews.data

import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonContentPolymorphicSerializer
import kotlinx.serialization.json.JsonElement
import kotlinx.serialization.json.jsonObject
import okhttp3.MediaType.Companion.toMediaType
import okhttp3.OkHttpClient
import retrofit2.Retrofit
Expand All @@ -11,19 +14,32 @@ import retrofit2.http.Path

const val BASE_FIREBASE_URL = "https://hacker-news.firebaseio.com/v0/"

@Serializable
data class Item(
val id: Long,
val type: String,
val time: Long,
val by: String? = null,
val title: String? = null,
val score: Int? = null,
val url: String? = null,
val descendants: Int? = null,
val kids: List<Long>? = null,
val text: String? = null
)
@Serializable(with = BaseResponseSerializer::class)
sealed interface BaseResponse {
@Serializable
data class Item(
val id: Long,
val type: String,
val time: Long,
val by: String? = null,
val title: String? = null,
val score: Int? = null,
val url: String? = null,
val descendants: Int? = null,
val kids: List<Long>? = null,
val text: String? = null
): BaseResponse

@Serializable
data object NullResponse: BaseResponse
}

object BaseResponseSerializer : JsonContentPolymorphicSerializer<BaseResponse>(BaseResponse::class) {
override fun selectDeserializer(element: JsonElement) = when {
"id" in element.jsonObject -> BaseResponse.Item.serializer()
else -> BaseResponse.NullResponse.serializer()
}
}

interface HackerNewsBaseApi {
@GET("topstories.json")
Expand All @@ -33,7 +49,7 @@ interface HackerNewsBaseApi {
suspend fun getNewStoryIds(): List<Long>

@GET("item/{id}.json")
suspend fun getItem(@Path("id") itemId: Long): Item
suspend fun getItem(@Path("id") itemId: Long): BaseResponse
}

class HackerNewsBaseDataSource(json: Json, client: OkHttpClient) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.emergetools.hackernews.data

import com.emergetools.hackernews.data.BaseResponse.Item
import com.emergetools.hackernews.data.BaseResponse.NullResponse
import com.emergetools.hackernews.features.stories.FeedType
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
Expand Down Expand Up @@ -27,14 +29,16 @@ class ItemRepository(
return withContext(Dispatchers.IO) {
val result = mutableListOf<Item>()
page.forEach { itemId ->
val item = baseClient.api.getItem(itemId)
result.add(item)
val response = baseClient.api.getItem(itemId)
if (response is Item) {
result.add(response)
}
}
result.toList()
}
}
}

fun MutableList<Page>.next() = removeFirst()
fun MutableList<Page>.next() = removeAt(0)


Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import kotlinx.serialization.json.JsonNull.content
import java.time.OffsetDateTime

sealed interface CommentsState {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,10 @@ import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.heightIn
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.wrapContentHeight
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.rounded.Person
import androidx.compose.material.icons.rounded.ThumbUp
import androidx.compose.material3.Icon
import androidx.compose.material3.MaterialTheme
Expand All @@ -37,7 +34,6 @@ import androidx.compose.ui.draw.clip
import androidx.compose.ui.draw.drawBehind
import androidx.compose.ui.graphics.Brush
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.RadialGradient
import androidx.compose.ui.graphics.graphicsLayer
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.text.font.FontWeight
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ import android.util.Log
import androidx.lifecycle.ViewModel
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.viewModelScope
import com.emergetools.hackernews.data.BaseResponse
import com.emergetools.hackernews.data.BaseResponse.Item
import com.emergetools.hackernews.data.BookmarkDao
import com.emergetools.hackernews.data.Item
import com.emergetools.hackernews.data.ItemRepository
import com.emergetools.hackernews.data.Page
import com.emergetools.hackernews.data.next
Expand Down

0 comments on commit fe0ab0f

Please sign in to comment.