From fe0ab0fc7e347cf6c34c07777e8c2067c045c2c2 Mon Sep 17 00:00:00 2001 From: Rikin Marfatia Date: Thu, 25 Jul 2024 10:00:59 -0700 Subject: [PATCH] Fixing a crash when loading Feeds 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. --- .../data/HackerNewsBaseDataSource.kt | 44 +++++++++++++------ .../hackernews/data/ItemRepository.kt | 10 +++-- .../features/comments/CommentsDomain.kt | 1 - .../features/settings/SettingsScreen.kt | 4 -- .../features/stories/StoriesDomain.kt | 3 +- 5 files changed, 39 insertions(+), 23 deletions(-) diff --git a/android/app/src/main/java/com/emergetools/hackernews/data/HackerNewsBaseDataSource.kt b/android/app/src/main/java/com/emergetools/hackernews/data/HackerNewsBaseDataSource.kt index 9966b9f8..d1fe7068 100644 --- a/android/app/src/main/java/com/emergetools/hackernews/data/HackerNewsBaseDataSource.kt +++ b/android/app/src/main/java/com/emergetools/hackernews/data/HackerNewsBaseDataSource.kt @@ -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 @@ -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? = 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? = null, + val text: String? = null + ): BaseResponse + + @Serializable + data object NullResponse: BaseResponse +} + +object BaseResponseSerializer : JsonContentPolymorphicSerializer(BaseResponse::class) { + override fun selectDeserializer(element: JsonElement) = when { + "id" in element.jsonObject -> BaseResponse.Item.serializer() + else -> BaseResponse.NullResponse.serializer() + } +} interface HackerNewsBaseApi { @GET("topstories.json") @@ -33,7 +49,7 @@ interface HackerNewsBaseApi { suspend fun getNewStoryIds(): List @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) { diff --git a/android/app/src/main/java/com/emergetools/hackernews/data/ItemRepository.kt b/android/app/src/main/java/com/emergetools/hackernews/data/ItemRepository.kt index 19d5fa88..a52eb320 100644 --- a/android/app/src/main/java/com/emergetools/hackernews/data/ItemRepository.kt +++ b/android/app/src/main/java/com/emergetools/hackernews/data/ItemRepository.kt @@ -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 @@ -27,14 +29,16 @@ class ItemRepository( return withContext(Dispatchers.IO) { val result = mutableListOf() 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.next() = removeFirst() +fun MutableList.next() = removeAt(0) diff --git a/android/app/src/main/java/com/emergetools/hackernews/features/comments/CommentsDomain.kt b/android/app/src/main/java/com/emergetools/hackernews/features/comments/CommentsDomain.kt index 4491b1f5..8b4700ef 100644 --- a/android/app/src/main/java/com/emergetools/hackernews/features/comments/CommentsDomain.kt +++ b/android/app/src/main/java/com/emergetools/hackernews/features/comments/CommentsDomain.kt @@ -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 { diff --git a/android/app/src/main/java/com/emergetools/hackernews/features/settings/SettingsScreen.kt b/android/app/src/main/java/com/emergetools/hackernews/features/settings/SettingsScreen.kt index f491c612..4128d9b0 100644 --- a/android/app/src/main/java/com/emergetools/hackernews/features/settings/SettingsScreen.kt +++ b/android/app/src/main/java/com/emergetools/hackernews/features/settings/SettingsScreen.kt @@ -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 @@ -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 diff --git a/android/app/src/main/java/com/emergetools/hackernews/features/stories/StoriesDomain.kt b/android/app/src/main/java/com/emergetools/hackernews/features/stories/StoriesDomain.kt index 66acaeb5..095e60e1 100644 --- a/android/app/src/main/java/com/emergetools/hackernews/features/stories/StoriesDomain.kt +++ b/android/app/src/main/java/com/emergetools/hackernews/features/stories/StoriesDomain.kt @@ -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