Skip to content

Commit

Permalink
chore(fix): update network client retry strategies and content wrappe…
Browse files Browse the repository at this point in the history
…r defaults
  • Loading branch information
wax911 committed Dec 30, 2024
1 parent 490bb97 commit 03bbaa7
Show file tree
Hide file tree
Showing 21 changed files with 101 additions and 70 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -242,3 +242,5 @@ fabric.properties
jacoco.exec

.idea/

.kotlin/
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import androidx.compose.foundation.layout.size
import androidx.compose.material3.CircularProgressIndicator
import androidx.compose.material3.FilledTonalButton
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.Surface
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
Expand Down Expand Up @@ -74,25 +75,28 @@ private fun LoadingContent(
config: IStateLayoutConfig,
modifier: Modifier = Modifier,
) {
Column(
modifier = modifier,
verticalArrangement = Arrangement.Center,
horizontalAlignment = Alignment.CenterHorizontally
) {
ContentImage(drawableResource = config.loadingDrawable)
Spacer(modifier = Modifier.height(8.dp))
Row(
horizontalArrangement = Arrangement.spacedBy(16.dp),
verticalAlignment = Alignment.CenterVertically,
Surface {
Column(
modifier = modifier,
verticalArrangement = Arrangement.Center,
horizontalAlignment = Alignment.CenterHorizontally
) {
CircularProgressIndicator(
modifier = Modifier.size(24.dp).align(alignment = Alignment.CenterVertically),
color = MaterialTheme.colorScheme.surfaceVariant,
strokeWidth = 4.dp,
trackColor = MaterialTheme.colorScheme.secondary,
)
config.loadingMessage?.also {
ContentText(text = stringResource(it))
ContentImage(drawableResource = config.loadingDrawable)
Spacer(modifier = Modifier.height(8.dp))
Row(
horizontalArrangement = Arrangement.spacedBy(8.dp),
verticalAlignment = Alignment.CenterVertically,
) {
CircularProgressIndicator(
modifier = Modifier.size(16.dp)
.align(alignment = Alignment.CenterVertically),
color = MaterialTheme.colorScheme.surfaceVariant,
strokeWidth = 2.dp,
trackColor = MaterialTheme.colorScheme.secondary,
)
config.loadingMessage?.also {
ContentText(text = stringResource(it))
}
}
}
}
Expand All @@ -106,22 +110,24 @@ private fun ErrorContent(
onClick: suspend () -> Unit,
) {
val scope = rememberCoroutineScope()
Column(
modifier = modifier,
verticalArrangement = Arrangement.Center,
horizontalAlignment = Alignment.CenterHorizontally
) {
ContentImage(drawableResource = config.errorDrawable)
Spacer(modifier = Modifier.height(16.dp))
state.details.message?.also {
ContentText(text = it)
}
config.retryAction?.also {
Surface {
Column(
modifier = modifier,
verticalArrangement = Arrangement.Center,
horizontalAlignment = Alignment.CenterHorizontally
) {
ContentImage(drawableResource = config.errorDrawable)
Spacer(modifier = Modifier.height(16.dp))
FilledTonalButton(
onClick = { scope.launch { onClick() } },
) {
Text(text = stringResource(it))
state.details.message?.also {
ContentText(text = it)
}
config.retryAction?.also {
Spacer(modifier = Modifier.height(16.dp))
FilledTonalButton(
onClick = { scope.launch { onClick() } },
) {
Text(text = stringResource(it))
}
}
}
}
Expand Down Expand Up @@ -174,7 +180,8 @@ fun <P: IParam> ContentWrapper(
}


@AniTrendPreview.Mobile
@AniTrendPreview.Light
@AniTrendPreview.Dark
@Composable
private fun ContentWrapperPreview(
@PreviewParameter(provider = ContentWrapperPreviewParameter::class) loadState: LoadState
Expand All @@ -187,7 +194,7 @@ private fun ContentWrapperPreview(
defaultMessage = co.anitrend.core.android.R.string.app_controller_message_missing_param,
retryAction = co.anitrend.core.android.R.string.action_share
)
PreviewTheme(wrapInSurface = true) {
PreviewTheme {
when (loadState) {
is LoadState.Error -> ErrorContent(
config = config,
Expand Down
15 changes: 12 additions & 3 deletions app-core/src/main/kotlin/co/anitrend/core/koin/Modules.kt
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ import co.anitrend.core.coil.client.CoilRequestClient
import co.anitrend.core.coil.fetch.RequestImageFetcher
import co.anitrend.core.coil.mapper.RequestImageMapper
import co.anitrend.data.android.koin.dataModules
import co.anitrend.data.android.network.cache.CacheHelper
import co.anitrend.data.android.network.model.NetworkMessage
import co.anitrend.data.core.extensions.defaultBuilder
import coil.ImageLoader
import coil.decode.GifDecoder
import coil.decode.ImageDecoderDecoder
Expand All @@ -45,6 +47,7 @@ import coil.disk.DiskCache
import coil.memory.MemoryCache
import io.wax911.emojify.EmojiManager
import io.wax911.emojify.serializer.kotlinx.KotlinxDeserializer
import okhttp3.CookieJar
import okhttp3.OkHttpClient
import okhttp3.logging.HttpLoggingInterceptor
import org.koin.android.ext.koin.androidContext
Expand Down Expand Up @@ -113,9 +116,15 @@ private val configurationModule = module {
val memoryLimit = if (isLowRamDevice) 0.15 else 0.35
val storageController = get<IStorageController>()

val client = get<OkHttpClient.Builder> {
parametersOf(HttpLoggingInterceptor.Level.BASIC)
}.build()
val client = defaultBuilder()
.cookieJar(get<CookieJar>())
.cache(
CacheHelper.createCache(
androidContext(),
"coil-okhttp"
)
)
.build()

val imageCache = storageController.getImageCache(
context = androidContext()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import co.anitrend.arch.request.callback.RequestCallback
import co.anitrend.data.android.network.model.NetworkMessage
import retrofit2.HttpException
import java.net.SocketTimeoutException
import java.net.UnknownHostException

/**
* Contract for controller strategy
Expand Down Expand Up @@ -49,6 +50,13 @@ abstract class ControllerStrategy<D> {
cause
)
}
is UnknownHostException -> {
RequestError(
networkMessage.connectivityErrorTittle,
networkMessage.connectivityErrorMessage,
cause
)
}
else -> {
if (cause == null)
RequestError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class OfflineStrategy<D> private constructor(
): D? {
runCatching {
block()
}.onSuccess {result ->
}.onSuccess { result ->
callback.recordSuccess()
return result
}.onFailure { exception ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ abstract class DeferrableNetworkClient<T> : AbstractNetworkClient<Async<Response
*/
override fun defaultShouldRetry(exception: Throwable) = when (exception) {
is HttpException -> exception.code() == 429
is SocketTimeoutException,
is IOException -> true
is SocketTimeoutException -> true
else -> false
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ abstract class OkHttpCallNetworkClient : AbstractNetworkClient<Call, Response>()
*/
override fun defaultShouldRetry(exception: Throwable) = when (exception) {
is OkHttpException -> exception.code == 429
is SocketTimeoutException,
is IOException -> true
is SocketTimeoutException -> true
else -> false
}

Expand Down Expand Up @@ -118,4 +117,4 @@ abstract class OkHttpCallNetworkClient : AbstractNetworkClient<Call, Response>()
maxAttempts,
shouldRetry
).bodyOrThrow()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ internal abstract class RetrofitCallNetworkClient<T> : AbstractNetworkClient<Cal
*/
override fun defaultShouldRetry(exception: Throwable) = when (exception) {
is HttpException -> exception.code() == 429
is SocketTimeoutException,
is IOException -> true
is SocketTimeoutException -> true
else -> false
}

Expand Down Expand Up @@ -112,4 +111,4 @@ internal abstract class RetrofitCallNetworkClient<T> : AbstractNetworkClient<Cal
maxAttempts,
shouldRetry
).bodyOrThrow()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,31 @@ import org.junit.jupiter.api.Assertions.assertEquals
import kotlin.test.Test

class CommonExtensionsKtTest {

//@Test
@Test
fun `test hash for word action`() {
val given = "Action"
val actual = given.toHashId()
val expected = 1955883606L
assertEquals(expected, actual)
}

//@Test
@Test
fun `test hash for word adventure`() {
val given = "Adventure"
val actual = given.toHashId()
val expected = 1309873904L
assertEquals(expected, actual)
}

//@Test
@Test
fun `test hash for word comedy`() {
val given = "Comedy"
val actual = given.toHashId()
val expected = 2024011449L
assertEquals(expected, actual)
}

//@Test
@Test
fun `test hash for word drama`() {
val given = "Drama"
val actual = given.toHashId()
Expand Down
3 changes: 1 addition & 2 deletions app-data-edge/.config/configuration.properties
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
#edgeHost="https://api.anitrend.docker.localhost/graphql/"
edgeHost="https://api.anitrend.co/graphql"
edgeHost="https://api.anitrend.co"
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ internal class EdgeApiFactory(
}

companion object {
const val BASE_ENDPOINT_PATH = "/graphql/"
const val BASE_ENDPOINT_PATH = "/graphql"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package co.anitrend.data.edge.genre.datasource

import androidx.room.Dao
import androidx.room.Query
import co.anitrend.data.android.source.AbstractLocalSource
import co.anitrend.data.android.source.local.AbstractLocalSource
import co.anitrend.data.edge.genre.entity.EdgeGenreEntity

@Dao
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import co.anitrend.data.feed.news.datasource.remote.NewsRemoteSource
import co.anitrend.data.feed.news.source.contract.NewsSource
import co.anitrend.domain.news.entity.News
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.async
import kotlinx.coroutines.flow.Flow

internal class NewsSourceImpl(
Expand Down
3 changes: 1 addition & 2 deletions app-data/graphql.config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ extensions:
user-agent: JS GraphQL
introspect: true
AniTrend:
url: https://graphql.anitrend.co/graphql/
url: https://api.anitrend.co/graphql
headers:
User-Agent: "JS GraphQL"
Accept: "*/*"
HOST: "graphql.anitrend.co"
Accept-Encoding: "gzip, deflate, br"
Accept-Language: "en-GB,en-US;q=0.9,en;q=0.8"
Content-Type: "application/json"
introspect: true
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ internal abstract class AuthLocalSource : AbstractLocalSource<AuthEntity>() {
delete from auth
where user_id = :userId
""")
abstract fun clearByUserId(userId: Long)
abstract suspend fun clearByUserId(userId: Long)

@Query("""
select * from auth
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ fun MediaCarouselItem(
items(
count = carouselItems.size,
key = { carouselItems[it].hashCode() },
contentType = { carouselItems[it].carouselType },
) { index ->
val carouselItem = carouselItems[index]
CarouselHeader(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ fun MediaCompactItemList(
items(
count = mediaItems.size,
key = { mediaItems[it].hashCode() },
contentType = { mediaItems[it].category.type }
contentType = { mediaItems[it].category }
) { index ->
MediaCompactItem(
media = mediaItems[index],
Expand Down Expand Up @@ -185,7 +185,13 @@ private fun MediaCompactItemPreview() {
mediaPreferenceData = MediaPreferenceData(
ScoreFormat.POINT_10_DECIMAL,
),
mediaItemClick = {}
mediaItemClick = {},
modifier = Modifier
.padding(8.dp)
.size(
height = 265.dp,
width = 150.dp,
),
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ package co.anitrend.media.carousel.component.content
import android.os.Bundle
import android.view.LayoutInflater
import android.view.ViewGroup
import androidx.lifecycle.liveData
import co.anitrend.arch.domain.entities.LoadState
import co.anitrend.arch.extension.ext.argument
import co.anitrend.common.media.ui.controller.extensions.openMediaListSheetFor
import co.anitrend.core.android.compose.design.ContentWrapper
Expand All @@ -36,6 +38,7 @@ import co.anitrend.navigation.MediaDiscoverRouter
import co.anitrend.navigation.MediaListEditorRouter
import co.anitrend.navigation.MediaRouter
import co.anitrend.navigation.extensions.asNavPayload
import co.anitrend.navigation.extensions.nameOf
import co.anitrend.navigation.extensions.startActivity
import org.koin.androidx.viewmodel.ext.android.viewModel
import timber.log.Timber
Expand All @@ -46,7 +49,9 @@ class CarouselContent(
) : AniTrendComposition() {

private val viewModel by viewModel<CarouselViewModel>()
private val param by argument<MediaCarouselRouter.MediaCarouselRouterParam>()
private val param by argument<MediaCarouselRouter.MediaCarouselRouterParam>(
key = nameOf<MediaCarouselRouter.MediaCarouselRouterParam>()
)

private fun paramOrDefault(): MediaCarouselRouter.MediaCarouselRouterParam {
return param ?: MediaCarouselRouter.MediaCarouselRouterParam(
Expand Down Expand Up @@ -89,9 +94,9 @@ class CarouselContent(
) = composable(context = inflater.context) {
AniTrendTheme3 {
ContentWrapper(
stateFlow = viewModelState().combinedLoadState,
stateFlow = liveData { emit(LoadState.Idle()) },
param = paramOrDefault(),
onLoad = viewModelState()::invoke,
onLoad = viewModel::invoke,
onClick = viewModelState()::retry,
) {
CarouselScreen(
Expand Down
Loading

0 comments on commit 03bbaa7

Please sign in to comment.