Skip to content

Commit

Permalink
Remove TwoStepsToOneStepSearchEngineAdapter
Browse files Browse the repository at this point in the history
Making multiple subsequent calls to /retrieve turned out
to be undesired for billing and performance reasons, so this
code change only calls /retrieve for the final selection.
  • Loading branch information
Eugene committed Feb 17, 2024
1 parent 131a003 commit f3606e9
Show file tree
Hide file tree
Showing 32 changed files with 644 additions and 426 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Breaking changes
- [CORE] Access token needs to be assigned via `MapboxOptions.accessToken` now
- [Address Autofill, Place Autocomplete] Search is a two-steps action now, i.e. it returns `Suggestion`s (without the geo coordinate) at the first step and `Result` after suggestion selection. The `coordinate` field is no longer available in `Suggestion`.
- [Address Autofill] `suggestions()` is renamed to `reverseGeocoding()`

### Mapbox dependencies
- Search Native SDK `2.0.0-alpha.4`
Expand Down
6 changes: 3 additions & 3 deletions MapboxSearch/autofill/api/api-metalava.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ package com.mapbox.search.autofill {
public interface AddressAutofill {
method public default static com.mapbox.search.autofill.AddressAutofill create(com.mapbox.common.location.LocationProvider? locationProvider = <anonymous class>());
method public default static final com.mapbox.search.autofill.AddressAutofill create();
method public suspend Object? reverseGeocoding(com.mapbox.geojson.Point point, com.mapbox.search.autofill.AddressAutofillOptions options, kotlin.coroutines.Continuation<? super com.mapbox.bindgen.Expected<java.lang.Exception,java.util.List<com.mapbox.search.autofill.AddressAutofillResult>>> p);
method public suspend Object? select(com.mapbox.search.autofill.AddressAutofillSuggestion suggestion, kotlin.coroutines.Continuation<? super com.mapbox.bindgen.Expected<java.lang.Exception,com.mapbox.search.autofill.AddressAutofillResult>> p);
method public suspend Object? suggestions(com.mapbox.geojson.Point point, com.mapbox.search.autofill.AddressAutofillOptions options, kotlin.coroutines.Continuation<? super com.mapbox.bindgen.Expected<java.lang.Exception,java.util.List<com.mapbox.search.autofill.AddressAutofillSuggestion>>> p);
method public suspend Object? suggestions(com.mapbox.search.autofill.Query query, com.mapbox.search.autofill.AddressAutofillOptions options, kotlin.coroutines.Continuation<? super com.mapbox.bindgen.Expected<java.lang.Exception,java.util.List<com.mapbox.search.autofill.AddressAutofillSuggestion>>> p);
field public static final com.mapbox.search.autofill.AddressAutofill.Companion Companion;
}
Expand All @@ -26,16 +26,16 @@ package com.mapbox.search.autofill {

@kotlinx.parcelize.Parcelize public final class AddressAutofillResult implements android.os.Parcelable {
method public com.mapbox.search.autofill.AddressComponents getAddress();
method public com.mapbox.geojson.Point getCoordinate();
method public com.mapbox.search.autofill.AddressAutofillSuggestion getSuggestion();
property public final com.mapbox.search.autofill.AddressComponents address;
property public final com.mapbox.geojson.Point coordinate;
property public final com.mapbox.search.autofill.AddressAutofillSuggestion suggestion;
}

@kotlinx.parcelize.Parcelize public final class AddressAutofillSuggestion implements android.os.Parcelable {
method public com.mapbox.geojson.Point getCoordinate();
method public String getFormattedAddress();
method public String getName();
property public final com.mapbox.geojson.Point coordinate;
property public final String formattedAddress;
property public final String name;
}
Expand Down
4 changes: 2 additions & 2 deletions MapboxSearch/autofill/api/autofill.api
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ public abstract interface class com/mapbox/search/autofill/AddressAutofill {
public static final field Companion Lcom/mapbox/search/autofill/AddressAutofill$Companion;
public static fun create ()Lcom/mapbox/search/autofill/AddressAutofill;
public static fun create (Lcom/mapbox/common/location/LocationProvider;)Lcom/mapbox/search/autofill/AddressAutofill;
public abstract fun reverseGeocoding (Lcom/mapbox/geojson/Point;Lcom/mapbox/search/autofill/AddressAutofillOptions;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
public abstract fun select (Lcom/mapbox/search/autofill/AddressAutofillSuggestion;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
public abstract fun suggestions (Lcom/mapbox/geojson/Point;Lcom/mapbox/search/autofill/AddressAutofillOptions;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
public abstract fun suggestions (Lcom/mapbox/search/autofill/Query;Lcom/mapbox/search/autofill/AddressAutofillOptions;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
}

Expand Down Expand Up @@ -41,6 +41,7 @@ public final class com/mapbox/search/autofill/AddressAutofillResult : android/os
public fun describeContents ()I
public fun equals (Ljava/lang/Object;)Z
public final fun getAddress ()Lcom/mapbox/search/autofill/AddressComponents;
public final fun getCoordinate ()Lcom/mapbox/geojson/Point;
public final fun getSuggestion ()Lcom/mapbox/search/autofill/AddressAutofillSuggestion;
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
Expand All @@ -59,7 +60,6 @@ public final class com/mapbox/search/autofill/AddressAutofillSuggestion : androi
public static final field CREATOR Landroid/os/Parcelable$Creator;
public fun describeContents ()I
public fun equals (Ljava/lang/Object;)Z
public final fun getCoordinate ()Lcom/mapbox/geojson/Point;
public final fun getFormattedAddress ()Ljava/lang/String;
public final fun getName ()Ljava/lang/String;
public fun hashCode ()I
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ import com.mapbox.search.base.core.CoreApiType
import com.mapbox.search.base.core.CoreEngineOptions
import com.mapbox.search.base.core.CoreSearchEngine
import com.mapbox.search.base.core.getUserActivityReporter
import com.mapbox.search.base.engine.TwoStepsToOneStepSearchEngineAdapter
import com.mapbox.search.base.location.LocationEngineAdapter
import com.mapbox.search.base.location.WrapperLocationProvider
import com.mapbox.search.base.location.defaultLocationProvider
import com.mapbox.search.common.IsoCountryCode
import com.mapbox.search.common.IsoLanguageCode
import com.mapbox.search.common.SearchRequestException
import kotlinx.coroutines.async
import kotlinx.coroutines.awaitAll
import kotlinx.coroutines.runBlocking
import okhttp3.mockwebserver.Dispatcher
import okhttp3.mockwebserver.MockResponse
Expand Down Expand Up @@ -53,7 +54,7 @@ internal class AddressAutofillIntegrationTest {
)

addressAutofill = AddressAutofillImpl(
searchEngine = engine,
autofillEngine = engine,
activityReporter = getUserActivityReporter()
)
}
Expand Down Expand Up @@ -96,7 +97,7 @@ internal class AddressAutofillIntegrationTest {
val point = Point.fromLngLat(-77.03398187174899, 38.8999032596197)

runBlocking {
addressAutofill.suggestions(point, options)
addressAutofill.reverseGeocoding(point, options)
}

val request = mockServer.takeRequest()
Expand Down Expand Up @@ -148,7 +149,6 @@ internal class AddressAutofillIntegrationTest {

val firstSuggestion = results.first()
assertEquals("740 15th St NW", firstSuggestion.name)
assertEquals(Point.fromLngLat(-77.03375, 38.89936), firstSuggestion.coordinate)

val selectionResponse = runBlocking {
addressAutofill.select(firstSuggestion)
Expand All @@ -167,6 +167,13 @@ internal class AddressAutofillIntegrationTest {
assertEquals("United States", resultAddress.country)
assertEquals("us", resultAddress.countryIso1)
assertEquals("US-DC", resultAddress.countryIso2)

val autofillResult = runBlocking {
addressAutofill.select(firstSuggestion)
}
assertTrue(autofillResult.isValue)
val autofillResultValue: AddressAutofillResult = requireNotNull(autofillResult.value)
assertEquals(Point.fromLngLat(-77.03375, 38.89936), autofillResultValue.coordinate)
}

@Test
Expand All @@ -187,7 +194,16 @@ internal class AddressAutofillIntegrationTest {
assertTrue(response.isValue)

val results = requireNotNull(response.value)
assertEquals(2, results.size)
assertEquals(3, results.size)

val autofillResult = runBlocking {
results.map { suggestion ->
async {
addressAutofill.select(suggestion)
}
}.awaitAll()
}.filter { result -> result.isValue }
assertEquals(2, autofillResult.size)
}

@Test
Expand All @@ -208,10 +224,15 @@ internal class AddressAutofillIntegrationTest {
val response = runBlocking {
addressAutofill.suggestions(TEST_QUERY, AddressAutofillOptions())
}
assertTrue(response.isValue)
assertEquals(3, response.value!!.size)

assertTrue(response.isError)
val error = requireNotNull(response.error)
assertEquals(SearchRequestException("", 501), error)
response.value!!.map { suggestion ->
val selectionResult = runBlocking {
addressAutofill.select(suggestion)
}
assertTrue(selectionResult.isError)
}
}

@Test
Expand Down Expand Up @@ -282,7 +303,7 @@ internal class AddressAutofillIntegrationTest {
app: Application,
url: String,
locationProvider: LocationProvider?
): TwoStepsToOneStepSearchEngineAdapter {
): AutofillSearchEngine {
val coreEngine = CoreSearchEngine(
CoreEngineOptions(
baseUrl = url,
Expand All @@ -295,8 +316,7 @@ internal class AddressAutofillIntegrationTest {
),
)

return TwoStepsToOneStepSearchEngineAdapter(
apiType = CoreApiType.AUTOFILL,
return AutofillSearchEngine(
coreEngine = coreEngine,
requestContextProvider = SearchRequestContextProvider(app),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ public interface AddressAutofill {
* @param options Request options.
* @return Result of the search request, one of error or value.
*/
public suspend fun suggestions(
public suspend fun reverseGeocoding(
point: Point,
options: AddressAutofillOptions
): Expected<Exception, List<AddressAutofillSuggestion>>
): Expected<Exception, List<AddressAutofillResult>>

/**
* Performs forward geocoding request.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ import com.mapbox.search.base.core.CoreSearchEngine
import com.mapbox.search.base.core.createCoreReverseGeoOptions
import com.mapbox.search.base.core.createCoreSearchOptions
import com.mapbox.search.base.core.getUserActivityReporter
import com.mapbox.search.base.engine.TwoStepsToOneStepSearchEngineAdapter
import com.mapbox.search.base.location.LocationEngineAdapter
import com.mapbox.search.base.location.WrapperLocationProvider
import com.mapbox.search.base.record.IndexableRecordResolver
import com.mapbox.search.base.record.SearchHistoryService
import com.mapbox.search.base.result.BaseSearchResult
import com.mapbox.search.base.result.BaseSearchSuggestion
import com.mapbox.search.base.result.SearchResultFactory
import com.mapbox.search.base.utils.extension.flatMap
import com.mapbox.search.internal.bindgen.UserActivityReporterInterface
import java.util.concurrent.ExecutorService
import java.util.concurrent.Executors
Expand All @@ -28,14 +29,15 @@ import java.util.concurrent.Executors
* Temporary implementation of the [AddressAutofill] based on the two-step search.
*/
internal class AddressAutofillImpl(
private val searchEngine: TwoStepsToOneStepSearchEngineAdapter,
private val activityReporter: UserActivityReporterInterface
private val autofillEngine: AutofillSearchEngine,
private val activityReporter: UserActivityReporterInterface,
private val resultFactory: AddressAutofillResultFactory = AddressAutofillResultFactory()
) : AddressAutofill {

override suspend fun suggestions(
override suspend fun reverseGeocoding(
point: Point,
options: AddressAutofillOptions
): Expected<Exception, List<AddressAutofillSuggestion>> {
): Expected<Exception, List<AddressAutofillResult>> {
activityReporter.reportActivity("address-autofill-reverse-geocoding")

val coreOptions = createCoreReverseGeoOptions(
Expand All @@ -44,8 +46,17 @@ internal class AddressAutofillImpl(
language = options.language?.let { listOf(it.code) },
)

return searchEngine.reverseGeocoding(coreOptions).mapValue { (results, _) ->
results.toAddressAutofillSuggestions()
return autofillEngine.search(coreOptions).mapValue { (results, _) ->
results.mapNotNull {
val expected = resultFactory.createAddressAutofillResultOrNull(it)
if (expected.isValue) expected.value else null
}
}.let { result ->
if (result.isValue && result.value.isNullOrEmpty()) {
ExpectedFactory.createError(Exception("No results for point $point"))
} else {
result
}
}
}

Expand All @@ -62,8 +73,9 @@ internal class AddressAutofillImpl(
ignoreUR = true,
addonAPI = mapOf("types" to "address", "streets" to "true")
)
return searchEngine.searchResolveImmediately(query.query, coreOptions).mapValue {
it.toAddressAutofillSuggestions()

return autofillEngine.search(query.query, coreOptions).mapValue { (suggestions, _) ->
resultFactory.createAddressAutofillSuggestions(suggestions)
}
}

Expand All @@ -72,7 +84,28 @@ internal class AddressAutofillImpl(
): Expected<Exception, AddressAutofillResult> {
activityReporter.reportActivity("address-autofill-suggestion-select")

return ExpectedFactory.createValue(AddressAutofillResult(suggestion, suggestion.address))
return if (suggestion.underlying == null) {
ExpectedFactory.createError(Exception("AddressAutofillSuggestion doesn't contain underlying suggestion"))
} else {
val baseResult = selectRaw(suggestion.underlying).value
if (baseResult == null) {
ExpectedFactory.createError(Exception("No results for suggestion $suggestion"))
} else {
resultFactory.createAddressAutofillResultOrNull(baseResult)
}
}
}

private suspend fun selectRaw(suggestion: BaseSearchSuggestion): Expected<Exception, BaseSearchResult> {
return autofillEngine.select(suggestion).flatMap {
when (it) {
is AutofillSearchEngine.SearchSelectionResponse.Result -> ExpectedFactory.createValue(it.result)
else -> {
// Shouldn't happen because we don't allow suggestions of type Category and Query
ExpectedFactory.createError(Exception("Unsupported suggestion type: $suggestion"))
}
}
}
}

internal companion object {
Expand All @@ -98,8 +131,7 @@ internal class AddressAutofillImpl(
),
)

val engine = TwoStepsToOneStepSearchEngineAdapter(
apiType = CoreApiType.AUTOFILL,
val engine = AutofillSearchEngine(
coreEngine = coreEngine,
requestContextProvider = SearchRequestContextProvider(app),
historyService = SearchHistoryService.STUB,
Expand All @@ -108,23 +140,9 @@ internal class AddressAutofillImpl(
)

return AddressAutofillImpl(
searchEngine = engine,
autofillEngine = engine,
activityReporter = getUserActivityReporter()
)
}

private fun List<BaseSearchResult>.toAddressAutofillSuggestions() = mapNotNull { it.toAddressAutofillSuggestion() }

private fun BaseSearchResult.toAddressAutofillSuggestion(): AddressAutofillSuggestion? {
// Filtering incomplete results
val autofillAddress = AddressComponents.fromCoreSdkAddress(address, metadata) ?: return null

return AddressAutofillSuggestion(
name = name,
formattedAddress = fullAddress ?: autofillAddress.formattedAddress(),
address = autofillAddress,
coordinate = coordinate,
)
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.mapbox.search.autofill

import android.os.Parcelable
import com.mapbox.geojson.Point
import kotlinx.parcelize.Parcelize

/**
Expand All @@ -14,6 +15,11 @@ public class AddressAutofillResult internal constructor(
*/
public val suggestion: AddressAutofillSuggestion,

/**
* Place geographic point.
*/
public val coordinate: Point,

/**
* Detailed address components like street, house number, etc.
*/
Expand All @@ -30,6 +36,7 @@ public class AddressAutofillResult internal constructor(
other as AddressAutofillResult

if (suggestion != other.suggestion) return false
if (coordinate != other.coordinate) return false
if (address != other.address) return false

return true
Expand All @@ -40,6 +47,7 @@ public class AddressAutofillResult internal constructor(
*/
override fun hashCode(): Int {
var result = suggestion.hashCode()
result = 31 * result + coordinate.hashCode()
result = 31 * result + address.hashCode()
return result
}
Expand All @@ -48,6 +56,6 @@ public class AddressAutofillResult internal constructor(
* @suppress
*/
override fun toString(): String {
return "AddressAutofillResult(suggestion=$suggestion, address=$address)"
return "AddressAutofillResult(suggestion=$suggestion, coordinate=$coordinate, address=$address)"
}
}
Loading

0 comments on commit f3606e9

Please sign in to comment.