Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion places-compose-demo/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
<activity
android:name=".MainActivity"
android:exported="true"
android:label="@string/app_name"
android:theme="@style/Theme.AndroidPlacesCompose">
<intent-filter>
<action android:name="android.intent.action.MAIN" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ import androidx.activity.enableEdgeToEdge
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.material3.Scaffold
import androidx.compose.material3.Text
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import com.google.android.libraries.places.compose.demo.ui.theme.AndroidPlacesComposeDemoTheme
import android.content.Intent
import androidx.compose.foundation.layout.systemBarsPadding
import androidx.compose.foundation.rememberScrollState
import androidx.compose.foundation.verticalScroll
import androidx.compose.material3.Button
Expand Down Expand Up @@ -60,7 +60,7 @@ class MainActivity : ComponentActivity() {
setContent {
AndroidPlacesComposeDemoTheme {
Surface(
modifier = Modifier.fillMaxSize().systemBarsPadding(),
modifier = Modifier.fillMaxSize(),
color = MaterialTheme.colorScheme.background
) {
ActivityButtons()
Expand All @@ -70,11 +70,17 @@ class MainActivity : ComponentActivity() {
}
}

data class ActivityItem(
val activityClass: Class<*>,
val titleRes: Int,
val descRes: Int,
)

val activities = listOf(
AutocompleteActivity::class.java to R.string.autocomplete_button,
LandmarkSelectionActivity::class.java to R.string.landmark_selection_button,
AddressCompletionActivity::class.java to R.string.address_validation_button,
PlacesAutocompleteMinimalActivity::class.java to R.string.minimal_autocomplete_button
ActivityItem(AutocompleteActivity::class.java, R.string.autocomplete_button, R.string.autocomplete_button_desc),
ActivityItem(LandmarkSelectionActivity::class.java, R.string.landmark_selection_button, R.string.landmark_selection_button_desc),
ActivityItem(AddressCompletionActivity::class.java, R.string.address_validation_button, R.string.address_validation_button_desc),
ActivityItem(PlacesAutocompleteMinimalActivity::class.java, R.string.minimal_autocomplete_button, R.string.minimal_autocomplete_button_desc),
)

@OptIn(ExperimentalMaterial3Api::class)
Expand All @@ -98,15 +104,33 @@ fun ActivityButtons() {
Column(
Modifier
.padding(paddingValues)
.padding(top = 16.dp)
.fillMaxSize()
.padding(horizontal = 24.dp, vertical = 20.dp)
.verticalScroll(rememberScrollState()),
horizontalAlignment = Alignment.CenterHorizontally,
verticalArrangement = Arrangement.spacedBy(10.dp)
verticalArrangement = Arrangement.spacedBy(14.dp)
) {
for ((activityClass, buttonTextResId) in activities) {
Button(onClick = { context.startActivity(Intent(context, activityClass)) }) {
Text(stringResource(buttonTextResId))
for (item in activities) {
Button(
onClick = { context.startActivity(Intent(context, item.activityClass)) },
modifier = Modifier.fillMaxWidth()
) {
Column(
modifier = Modifier
.fillMaxWidth()
.padding(vertical = 4.dp),
horizontalAlignment = Alignment.CenterHorizontally
) {
Text(
text = stringResource(item.titleRes),
style = MaterialTheme.typography.titleMedium
)
Text(
text = stringResource(item.descRes),
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.onPrimary.copy(alpha = 0.85f)
)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package com.google.android.libraries.places.compose.demo.data.repositories

import android.content.Context
import android.net.Uri
import android.util.Log
import com.android.volley.Request
import com.android.volley.toolbox.StringRequest
import com.android.volley.toolbox.Volley
Expand Down Expand Up @@ -47,6 +48,8 @@ class GeocoderRepository(
private val context: Context,
private val apiKeyProvider: ApiKeyProvider
) {
private val requestQueue by lazy { Volley.newRequestQueue(context.applicationContext) }

private val gson: Gson = GsonBuilder()
.setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES)
.create()
Expand All @@ -67,26 +70,44 @@ class GeocoderRepository(
suspend fun reverseGeocode(
latLng: LatLng,
includeAddressDescriptors: Boolean = true
): ReverseGeocodingResponse {
): ReverseGeocodingResponse? {
val url = buildRequestUrl(latLng, includeAddressDescriptors)

return suspendCancellableCoroutine { cont ->
val queue = Volley.newRequestQueue(context)
val stringRequest =
StringRequest(
Request.Method.GET,
url,
{ response -> cont.resume(parseFullGeocodeResponse(response)) },
{ error -> cont.resumeWithException(RuntimeException(error.localizedMessage)) },
)
queue.add(stringRequest)
return try {
suspendCancellableCoroutine { cont ->

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Coroutine Cancellation & RequestQueue Leak in GeocoderRepository.reverseGeocode()

Issues:
Thread/Memory Leak: Volley.newRequestQueue(context) creates a new thread pool (CacheDispatcher + 4 NetworkDispatcher threads) on every single geocoding request and never stops them. RequestQueue should be initialized once as an instance property on GeocoderRepository (e.g. private val requestQueue by lazy { Volley.newRequestQueue(context.applicationContext) }).

Missing Cancellation Hook: Add cont.invokeOnCancellation { stringRequest.cancel() } inside suspendCancellableCoroutine so cancelled coroutines cancel the underlying HTTP request.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated! Both issues resolved:

  1. Initialized RequestQueue once on GeocoderRepository via private val requestQueue by lazy { Volley.newRequestQueue(context.applicationContext) }, avoiding creating thread pools on each geocode request.
  2. Added cont.invokeOnCancellation { stringRequest.cancel() } inside suspendCancellableCoroutine so cancelled coroutines properly cancel the underlying Volley request.

val stringRequest =
StringRequest(
Request.Method.GET,
url,
{ response ->
try {
cont.resume(parseFullGeocodeResponse(response))
} catch (e: Exception) {
Log.e("GeocoderRepository", "Error parsing geocoder response", e)
cont.resume(null)
}
},
{ error ->
Log.e("GeocoderRepository", "Volley reverse geocode error: ${error.networkResponse?.statusCode} - ${error.localizedMessage}", error)
cont.resume(null)
},
)
cont.invokeOnCancellation {
stringRequest.cancel()
}
requestQueue.add(stringRequest)
}
} catch (e: Exception) {
Log.e("GeocoderRepository", "Reverse geocode exception", e)
null
}
}

private fun parseFullGeocodeResponse(response: String): ReverseGeocodingResponse {
val fullResult = gson.fromJson(response, ReverseGeocodingResponse::class.java)
// TODO: handle the status -- consider a monad pattern (Result?)

if (fullResult.status != "OK" && fullResult.status != "ZERO_RESULTS") {
Log.w("GeocoderRepository", "Geocode API warning: status=${fullResult.status}, error_message=${fullResult.errorMessage}")
}
return fullResult
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,12 @@ constructor(
* If we are already sending the mock location, advance to the next mock location, otherwise
* switch to presenting the mock location.
*/
fun nextMockLocation() {
if (_useMockLocation.value) {
mockLocationRepository.nextMockLocation()
fun nextMockLocation(): String {
return if (_useMockLocation.value) {
mockLocationRepository.nextMockLocation().first
} else {
_useMockLocation.value = true
mockLocationRepository.getCurrentMockLocation().first
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ class MockLocationRepository {
userClickedLocation.value = LabeledLocation(location, "User selected")
}

fun getCurrentMockLocation(): Pair<String, LatLng> {
return locations[_mockLocationNumber.value]
}

fun nextMockLocation(): Pair<String, LatLng> {
_mockLocationNumber.value = (_mockLocationNumber.value + 1) % locations.size
return locations[_mockLocationNumber.value]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,19 @@ import androidx.compose.foundation.verticalScroll
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalSoftwareKeyboardController
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Row
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.outlined.Info
import androidx.compose.material3.Card
import androidx.compose.material3.CardDefaults
import androidx.compose.material3.Icon
import com.google.android.gms.maps.model.LatLng
import com.google.android.libraries.places.compose.autocomplete.components.PlacesAutocompleteTextField
import com.google.android.libraries.places.compose.autocomplete.models.AutocompletePlace
Expand Down Expand Up @@ -150,6 +158,42 @@ private fun AddressEntryForm(
.padding(top = 16.dp)
.verticalScroll(rememberScrollState()),
) {
if (addressEntry.nearbyObjects.isEmpty()) {
Card(
modifier = Modifier
.fillMaxWidth()
.padding(bottom = 16.dp),
colors = CardDefaults.cardColors(
containerColor = MaterialTheme.colorScheme.surfaceVariant,
contentColor = MaterialTheme.colorScheme.onSurfaceVariant
)
) {
Column(
modifier = Modifier.padding(14.dp),
verticalArrangement = Arrangement.spacedBy(6.dp)
) {
Row(
verticalAlignment = Alignment.CenterVertically,
horizontalArrangement = Arrangement.spacedBy(8.dp)
) {
Icon(
imageVector = Icons.Outlined.Info,
contentDescription = null,
tint = MaterialTheme.colorScheme.primary
)
Text(
text = stringResource(R.string.address_descriptors_info_title),
style = MaterialTheme.typography.titleMedium
)
}
Text(
text = stringResource(R.string.address_descriptors_unavailable_message),
style = MaterialTheme.typography.bodyMedium
)
}
}
}

AddressDisplay(
address = addressEntry.displayAddress,
modifier = Modifier.fillMaxWidth(),
Expand Down
Loading