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

Error Handling for urlConnection #2902

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
494f766
added try catch
anandwana001 Dec 9, 2024
e00b968
Merge branch 'master' into anandwana001/2683/prevent-socket-timeout-e…
anandwana001 Dec 10, 2024
d41ab05
not throwing
anandwana001 Dec 12, 2024
ff4c85f
Merge branch 'master' into anandwana001/2683/prevent-socket-timeout-e…
anandwana001 Dec 16, 2024
96871f2
added failure handling
anandwana001 Dec 16, 2024
60d4787
nit fix
anandwana001 Dec 16, 2024
b70eb4a
Merge branch 'master' into anandwana001/2683/prevent-socket-timeout-e…
anandwana001 Dec 16, 2024
a16a172
nit fixes
anandwana001 Dec 17, 2024
e8555fe
Merge remote-tracking branch 'origin/anandwana001/2683/prevent-socket…
anandwana001 Dec 17, 2024
6b8fa5e
Merge branch 'master' into anandwana001/2683/prevent-socket-timeout-e…
anandwana001 Dec 17, 2024
a0fe855
Merge branch 'master' into anandwana001/2683/prevent-socket-timeout-e…
anandwana001 Dec 19, 2024
9530810
Merge branch 'master' into anandwana001/2683/prevent-socket-timeout-e…
shobhitagarwal1612 Jan 1, 2025
48890ae
Merge branch 'master' into anandwana001/2683/prevent-socket-timeout-e…
anandwana001 Jan 8, 2025
7bcb1a0
partial test added
anandwana001 Jan 8, 2025
4d1c6ad
Merge branch 'master' into anandwana001/2683/prevent-socket-timeout-e…
anandwana001 Jan 13, 2025
011d10f
Merge branch 'master' into anandwana001/2683/prevent-socket-timeout-e…
anandwana001 Feb 3, 2025
2d7e8eb
import fix
anandwana001 Feb 3, 2025
f88c972
Merge branch 'master' into anandwana001/2683/prevent-socket-timeout-e…
anandwana001 Feb 5, 2025
ea08f37
merge latest
anandwana001 Feb 5, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ constructor(
* Downloads tiles in the specified bounds and stores them in the local filesystem. Emits the
* number of bytes processed and total expected bytes as the download progresses.
*/
suspend fun downloadTiles(bounds: Bounds): Flow<Pair<Int, Int>> = flow {
fun downloadTiles(bounds: Bounds): Flow<Pair<Int, Int>> = flow {
val requests = mogClient.buildTilesRequests(bounds)
val totalBytes = requests.sumOf { it.totalBytes }
var bytesDownloaded = 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import java.io.InputStream
import java.net.HttpURLConnection
import java.net.URL

const val READ_TIMEOUT_MS = 5 * 1000
private const val READ_TIMEOUT_MS = 5 * 1000

/**
* @constructor Creates a [UrlInputStream] by opening a connection to an actual URL, requesting the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.Toast
import androidx.compose.runtime.livedata.observeAsState
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import androidx.navigation.fragment.findNavController
import com.google.android.ground.R
import com.google.android.ground.databinding.OfflineAreaSelectorFragBinding
Expand Down Expand Up @@ -68,8 +71,17 @@ class OfflineAreaSelectorFragment : AbstractMapContainerFragment() {

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
viewModel.isDownloadProgressVisible.observe(viewLifecycleOwner) {
showDownloadProgressDialog(it)
viewLifecycleOwner.lifecycleScope.launch {
repeatOnLifecycle(Lifecycle.State.RESUMED) {
Comment on lines +74 to +75
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? Can it be removed given that coroutines aren't getting suspended.

viewModel.isDownloadProgressVisible.observe(viewLifecycleOwner) {
showDownloadProgressDialog(it)
}
viewModel.isFailure.observe(viewLifecycleOwner) {
if (it) {
Toast.makeText(context, R.string.offline_area_download_error, Toast.LENGTH_LONG).show()
}
}
}
}

lifecycleScope.launch {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.flow.catch
import kotlinx.coroutines.launch

private const val MIN_DOWNLOAD_ZOOM_LEVEL = 9
Expand Down Expand Up @@ -78,6 +79,7 @@ internal constructor(
val downloadProgress = MutableLiveData(0f)
val bottomText = MutableLiveData<String?>(null)
val downloadButtonEnabled = MutableLiveData(false)
val isFailure = MutableLiveData(false)

private val _navigate = MutableSharedFlow<UiState>(replay = 0)
val navigate = _navigate.asSharedFlow()
Expand All @@ -100,20 +102,29 @@ internal constructor(

isDownloadProgressVisible.value = true
downloadProgress.value = 0f
downloadJob =
viewModelScope.launch(ioDispatcher) {
offlineAreaRepository.downloadTiles(viewport!!).collect { (bytesDownloaded, totalBytes) ->
val progressValue =
if (totalBytes > 0) {
(bytesDownloaded.toFloat() / totalBytes.toFloat()).coerceIn(0f, 1f)
} else {
0f
}
downloadProgress.postValue(progressValue)
downloadJob = viewModelScope.launch(ioDispatcher) {
offlineAreaRepository
.downloadTiles(viewport!!)
.catch {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we log the exception to timber?

isFailure.postValue(true)
isDownloadProgressVisible.postValue(false)
}
isDownloadProgressVisible.postValue(false)
_navigate.emit(UiState.OfflineAreaBackToHomeScreen)
.collect { (bytesDownloaded, totalBytes) ->
updateDownloadProgress(bytesDownloaded, totalBytes)
}
isDownloadProgressVisible.postValue(false)
_navigate.emit(UiState.OfflineAreaBackToHomeScreen)
}
}

private fun updateDownloadProgress(bytesDownloaded: Int, totalBytes: Int) {
val progressValue =
if (totalBytes > 0) {
(bytesDownloaded.toFloat() / totalBytes.toFloat()).coerceIn(0f, 1f)
} else {
0f
}
downloadProgress.postValue(progressValue)
}

fun onCancelClick() {
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values-es/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
<string name="settings">Configuración</string>
<string name="offline_area_viewer_title">Área descargada</string>
<string name="offline_area_viewer_remove_button">Eliminar del dispositivo</string>
<string name="offline_area_download_error">¡Error de descarga. Por favor inténtalo de nuevo</string>
<!-- Label of button used enter the offline area selector. -->
<string name="offline_area_selector_select">Seleccionar área</string>
<string name="no_basemaps_downloaded">No se ha descargado ningún mapa para su uso sin conexión</string>
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values-fr/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
<string name="settings">Paramètres</string>
<string name="offline_area_viewer_title">Zone téléchargée</string>
<string name="offline_area_viewer_remove_button">Supprimer de l&#8217;appareil</string>
<string name="offline_area_download_error">Échec du téléchargement. Veuillez réessayer</string>
<!-- Label of button used enter the offline area selector. -->
<string name="offline_area_selector_select">Sélectionnez une zone</string>
<string name="no_basemaps_downloaded">Aucune image n&#8217;a été téléchargée pour l&#8217;utilisation hors ligne. Appuyez sur « Sélectionner et télécharger » pour commencer.</string>
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values-pt/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
<string name="settings">Configurações</string>
<string name="offline_area_viewer_title">Área descarregada</string>
<string name="offline_area_viewer_remove_button">Remover do dispositivo</string>
<string name="offline_area_download_error">Falha no download! Por favor, tente novamente</string>
<!-- Label of button used enter the offline area selector. -->
<string name="offline_area_selector_select">Selecionar área</string>
<string name="no_basemaps_downloaded">Nenhuma imagem de mapa descarregada para uso offline</string>
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
<string name="settings">Settings</string>
<string name="offline_area_viewer_title">Downloaded area</string>
<string name="offline_area_viewer_remove_button">Remove from device</string>
<string name="offline_area_download_error">Download failed. Please try again</string>
<!-- Label of button used enter the offline area selector. -->
<string name="offline_area_selector_select">Select area</string>
<string name="no_basemaps_downloaded">No map imagery downloaded for offline use</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import androidx.compose.ui.test.junit4.createAndroidComposeRule
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.lifecycle.Observer
import android.widget.Toast
import androidx.lifecycle.lifecycleScope
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.matcher.ViewMatchers.hasDescendant
Expand All @@ -33,20 +35,25 @@ import com.google.android.ground.BaseHiltTest
import com.google.android.ground.R
import com.google.android.ground.launchFragmentInHiltContainer
import com.google.android.ground.repository.OfflineAreaRepository
import com.google.common.truth.Truth.assertThat
import dagger.hilt.android.testing.HiltAndroidTest
import javax.inject.Inject
import junit.framework.Assert.assertFalse
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.test.advanceUntilIdle
import org.junit.Assert.assertNull
import junit.framework.TestCase.assertEquals
import kotlinx.coroutines.launch
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.mock
import org.mockito.kotlin.any
import org.mockito.kotlin.whenever
import org.mockito.Mockito.verify
import org.robolectric.RobolectricTestRunner
import org.robolectric.shadows.ShadowToast

@HiltAndroidTest
@RunWith(RobolectricTestRunner::class)
Expand Down Expand Up @@ -125,4 +132,31 @@ class OfflineAreaSelectorFragmentTest : BaseHiltTest() {

viewModel.downloadProgress.removeObserver(observer)
}

@Test
fun `test failure case displays toast`() = runWithTestDispatcher {
val isFailureObserver = mock(Observer::class.java) as Observer<Boolean>
fragment.viewLifecycleOwner.lifecycleScope.launch {
viewModel.isFailure.observeForever(isFailureObserver)
viewModel.isFailure.postValue(true)
Comment on lines +140 to +141
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to call the download flow and mock the downloadTiles to throw an error?

}

verify(isFailureObserver).onChanged(true)

ShadowToast.reset()

advanceUntilIdle()

val toast = ShadowToast.getLatestToast()
assertThat(ShadowToast.shownToastCount()).isEqualTo(1)
assertEquals(toast.duration, Toast.LENGTH_LONG)
assertEquals(
ShadowToast.getTextOfLatestToast(),
fragment.getString(R.string.offline_area_download_error),
)

fragment.viewLifecycleOwner.lifecycleScope.launch {
viewModel.isFailure.removeObserver(isFailureObserver)
}
}
}
Loading