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 10 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
* 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 {

Check warning on line 88 in ground/src/main/java/com/google/android/ground/repository/OfflineAreaRepository.kt

View check run for this annotation

Codecov / codecov/patch

ground/src/main/java/com/google/android/ground/repository/OfflineAreaRepository.kt#L88

Added line #L88 was not covered by tests
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,12 +20,16 @@
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.compose.ui.platform.ComposeView
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
import com.google.android.ground.ui.common.AbstractMapContainerFragment
import com.google.android.ground.ui.common.BaseMapViewModel
Expand Down Expand Up @@ -62,8 +66,17 @@

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
viewModel.isDownloadProgressVisible.observe(viewLifecycleOwner) {
showDownloadProgressDialog(it)
viewLifecycleOwner.lifecycleScope.launch {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add tests for this logic?

repeatOnLifecycle(Lifecycle.State.RESUMED) {
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()

Check warning on line 76 in ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorFragment.kt

View check run for this annotation

Codecov / codecov/patch

ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorFragment.kt#L76

Added line #L76 was not covered by tests
}
}
}
}

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.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 @@ -77,6 +78,7 @@
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 @@ -98,20 +100,30 @@
isDownloadProgressVisible.value = true
downloadProgress.value = 0f
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)
}
offlineAreaRepository
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you also add tests for this logic as well?

.downloadTiles(viewport!!)

Check warning on line 104 in ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt

View check run for this annotation

Codecov / codecov/patch

ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt#L103-L104

Added lines #L103 - L104 were not covered by tests
.catch {
isFailure.postValue(true)
isDownloadProgressVisible.postValue(false)
}
.collect { (bytesDownloaded, totalBytes) ->
updateDownloadProgress(bytesDownloaded, totalBytes)
}

Check warning on line 111 in ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt

View check run for this annotation

Codecov / codecov/patch

ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt#L106-L111

Added lines #L106 - L111 were not covered by tests
isDownloadProgressVisible.postValue(false)
_navigate.emit(UiState.OfflineAreaBackToHomeScreen)
}
}

private fun updateDownloadProgress(bytesDownloaded: Int, totalBytes: Int) {
val progressValue =

Check warning on line 118 in ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt

View check run for this annotation

Codecov / codecov/patch

ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt#L118

Added line #L118 was not covered by tests
if (totalBytes > 0) {
(bytesDownloaded.toFloat() / totalBytes.toFloat()).coerceIn(0f, 1f)

Check warning on line 120 in ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt

View check run for this annotation

Codecov / codecov/patch

ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt#L120

Added line #L120 was not covered by tests
} else {
0f

Check warning on line 122 in ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt

View check run for this annotation

Codecov / codecov/patch

ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt#L122

Added line #L122 was not covered by tests
}
downloadProgress.postValue(progressValue)
}

Check warning on line 125 in ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt

View check run for this annotation

Codecov / codecov/patch

ground/src/main/java/com/google/android/ground/ui/offlineareas/selector/OfflineAreaSelectorViewModel.kt#L124-L125

Added lines #L124 - L125 were not covered by tests

fun onCancelClick() {
viewModelScope.launch { _navigate.emit(UiState.Up) }
}
Expand Down
1 change: 1 addition & 0 deletions ground/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 ground/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 ground/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 ground/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
Loading