diff --git a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasCallbackHandler.java b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasCallbackHandler.java index 0540a363..f8666d82 100644 --- a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasCallbackHandler.java +++ b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasCallbackHandler.java @@ -13,13 +13,13 @@ */ class PeliasCallbackHandler { - private PeliasFeatureToPlaceConverter converter; + private PeliasFeatureConverter converter; /** * Constructor. */ PeliasCallbackHandler() { - converter = new PeliasFeatureToPlaceConverter(); + converter = new PeliasFeatureConverter(); } /** @@ -41,7 +41,7 @@ void handleSuccess(String title, Response response, for (Feature feature : response.body().getFeatures()) { if (feature.properties.name.equals(title)) { Place place = converter.getFetchedPlace(feature); - String details = getDetails(feature, title); + String details = converter.getDetails(feature, title); listener.onFetchSuccess(place, details); } } @@ -63,9 +63,8 @@ void handleSuccess(Response response, OnPlaceDetailsFetchedListener list } Feature feature = response.body().getFeatures().get(0); - String title = feature.properties.name; Place place = converter.getFetchedPlace(feature); - String details = getDetails(feature, title); + String details = converter.getDetails(feature); listener.onFetchSuccess(place, details); } @@ -88,10 +87,4 @@ private boolean isValidResponse(Response response) { void handleFailure(OnPlaceDetailsFetchedListener listener) { listener.onFetchFailure(); } - - private String getDetails(Feature feature, String title) { - String label = feature.properties.label; - label = label.replace(title + ",", "").trim(); - return title + "\n" + label; - } } diff --git a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasFeatureToPlaceConverter.java b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasFeatureConverter.java similarity index 72% rename from mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasFeatureToPlaceConverter.java rename to mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasFeatureConverter.java index 445fb6fb..095fa5ce 100644 --- a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasFeatureToPlaceConverter.java +++ b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasFeatureConverter.java @@ -10,10 +10,10 @@ import java.util.Locale; /** - * Handles converting Pelias {@link Feature} objects into {@link Place} objects for - * {@link PeliasCallbackHandler}. + * Handles converting Pelias {@link Feature} objects into {@link Place} objects and detail strings + * for {@link PeliasCallbackHandler}. */ -class PeliasFeatureToPlaceConverter { +class PeliasFeatureConverter { private PeliasLayerToPlaceTypeConverter layerConverter; private PointToBoundsConverter pointConverter; @@ -21,7 +21,7 @@ class PeliasFeatureToPlaceConverter { /** * Constructor. */ - PeliasFeatureToPlaceConverter() { + PeliasFeatureConverter() { layerConverter = new PeliasLayerToPlaceTypeConverter(); pointConverter = new PointToBoundsConverter(); } @@ -59,4 +59,26 @@ Place getFetchedPlace(Feature feature) { .setWebsiteUri(null) .build(); } + + /** + * Constructs a detail string from a {@link Feature}. + * @param feature + * @return + */ + String getDetails(Feature feature) { + String title = feature.properties.name; + return getDetails(feature, title); + } + + /** + * Constructs a detail string from a {@link Feature} and title. + * @param feature + * @param title + * @return + */ + String getDetails(Feature feature, String title) { + String label = feature.properties.label; + label = label.replace(title + ",", "").trim(); + return title + "\n" + label; + } } diff --git a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasLayerToPlaceTypeConverter.java b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasLayerToPlaceTypeConverter.java index 814d016e..720d09bf 100644 --- a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasLayerToPlaceTypeConverter.java +++ b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasLayerToPlaceTypeConverter.java @@ -31,7 +31,7 @@ /** * Converts a "layer" returned from Pelias into a list of {@link com.mapzen.places.api.Place} types - * for {@link PeliasFeatureToPlaceConverter}. + * for {@link PeliasFeatureConverter}. */ class PeliasLayerToPlaceTypeConverter { diff --git a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlaceAutocompleteActivity.java b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlaceAutocompleteActivity.java index 238ddb8d..1bb8da29 100644 --- a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlaceAutocompleteActivity.java +++ b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlaceAutocompleteActivity.java @@ -87,9 +87,11 @@ public class PlaceAutocompleteActivity extends AppCompatActivity PeliasCallbackHandler callbackHandler = new PeliasCallbackHandler(); PlaceDetailFetcher detailFetcher = new PeliasPlaceDetailFetcher(mapzenSearch.getPelias(), callbackHandler); + PeliasFeatureConverter featureConverter = new PeliasFeatureConverter(); OnPlaceDetailsFetchedListener detailFetchListener = new AutocompleteDetailFetchListener(this); FilterMapper filterMapper = new PeliasFilterMapper(); - presenter = new PlaceAutocompletePresenter(detailFetcher, detailFetchListener, filterMapper); + presenter = new PlaceAutocompletePresenter(detailFetcher, featureConverter, detailFetchListener, + filterMapper); presenter.setBounds((LatLngBounds) safeGetExtra(EXTRA_BOUNDS)); presenter.setFilter((AutocompleteFilter) safeGetExtra(EXTRA_FILTER)); LostApiClient client = new LostApiClient.Builder(this).build(); diff --git a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlaceAutocompletePresenter.java b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlaceAutocompletePresenter.java index 2c79fb43..0f8a726d 100644 --- a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlaceAutocompletePresenter.java +++ b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlaceAutocompletePresenter.java @@ -3,10 +3,12 @@ import com.mapzen.android.lost.api.LocationServices; import com.mapzen.android.lost.api.LostApiClient; import com.mapzen.pelias.BoundingBox; +import com.mapzen.pelias.gson.Feature; import com.mapzen.pelias.gson.Properties; import com.mapzen.pelias.gson.Result; import com.mapzen.places.api.AutocompleteFilter; import com.mapzen.places.api.LatLngBounds; +import com.mapzen.places.api.Place; import android.location.Location; import android.util.Log; @@ -22,6 +24,7 @@ class PlaceAutocompletePresenter { private static final double LON_DEFAULT = 0.0; private final PlaceDetailFetcher detailFetcher; + private final PeliasFeatureConverter featureConverter; private final OnPlaceDetailsFetchedListener detailFetchListener; private final FilterMapper filterMapper; private final PointToBoundsConverter pointConverter; @@ -33,8 +36,10 @@ class PlaceAutocompletePresenter { * Creates a new instance. */ PlaceAutocompletePresenter(PlaceDetailFetcher detailFetcher, + PeliasFeatureConverter featureConverter, OnPlaceDetailsFetchedListener detailFetchListener, FilterMapper filterMapper) { this.detailFetcher = detailFetcher; + this.featureConverter = featureConverter; this.detailFetchListener = detailFetchListener; this.filterMapper = filterMapper; this.pointConverter = new PointToBoundsConverter(); @@ -64,9 +69,16 @@ void setLostClient(LostApiClient client) { * @param response parsed result returned by the service. */ void onResponse(Response response) { - Properties properties = response.body().getFeatures().get(0).properties; + Feature feature = response.body().getFeatures().get(0); + Properties properties = feature.properties; String gid = properties.gid; - detailFetcher.fetchDetails(gid, detailFetchListener); + if (gid.isEmpty()) { + Place place = featureConverter.getFetchedPlace(feature); + String details = featureConverter.getDetails(feature); + detailFetchListener.onFetchSuccess(place, details); + } else { + detailFetcher.fetchDetails(gid, detailFetchListener); + } } /** diff --git a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlacePickerViewController.java b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlacePickerViewController.java index 7c1e8cb7..1f5f5c03 100644 --- a/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlacePickerViewController.java +++ b/mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PlacePickerViewController.java @@ -9,15 +9,15 @@ interface PlacePickerViewController { /** * Show an alert dialog to represent the place selected by the user. - * @param id - * @param title + * @param id the selected {@link Place}'s id. + * @param title the dialog title. */ void showDialog(String id, String title); /** * Update the visible dialog with more details about the selected place. - * @param id - * @param message + * @param id the selected {@link Place}'s id. + * @param message the dialog message. */ void updateDialog(String id, String message); diff --git a/mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PeliasFeatureToPlaceConverterTest.java b/mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PeliasFeatureConverterTest.java similarity index 96% rename from mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PeliasFeatureToPlaceConverterTest.java rename to mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PeliasFeatureConverterTest.java index a7e96ca4..ee2e2f0e 100644 --- a/mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PeliasFeatureToPlaceConverterTest.java +++ b/mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PeliasFeatureConverterTest.java @@ -12,9 +12,9 @@ import static com.mapzen.places.api.Place.TYPE_POINT_OF_INTEREST; import static org.assertj.core.api.Assertions.assertThat; -public class PeliasFeatureToPlaceConverterTest { +public class PeliasFeatureConverterTest { - PeliasFeatureToPlaceConverter converter = new PeliasFeatureToPlaceConverter(); + PeliasFeatureConverter converter = new PeliasFeatureConverter(); @Test public void getFetchedPlace_shouldHaveCorrectAddress() throws Exception { diff --git a/mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PlaceAutocompletePresenterTest.java b/mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PlaceAutocompletePresenterTest.java index 033b6ca7..d0013753 100644 --- a/mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PlaceAutocompletePresenterTest.java +++ b/mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PlaceAutocompletePresenterTest.java @@ -11,6 +11,7 @@ import com.mapzen.pelias.gson.Result; import com.mapzen.places.api.LatLng; import com.mapzen.places.api.LatLngBounds; +import com.mapzen.places.api.Place; import org.junit.Test; @@ -19,6 +20,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import retrofit2.Response; @@ -26,10 +28,11 @@ public class PlaceAutocompletePresenterTest { private PlaceDetailFetcher detailFetcher = mock(PlaceDetailFetcher.class); + private PeliasFeatureConverter featureConverter = mock(PeliasFeatureConverter.class); private OnPlaceDetailsFetchedListener detailFetchListener = mock( OnPlaceDetailsFetchedListener.class); private PlaceAutocompletePresenter presenter = new PlaceAutocompletePresenter(detailFetcher, - detailFetchListener, null); + featureConverter, detailFetchListener, null); @Test public void shouldNotBeNull() throws Exception { @@ -43,6 +46,25 @@ public void onResponse_shouldFetchPlaceDetails() throws Exception { verify(detailFetcher).fetchDetails("123abc", detailFetchListener); } + @Test + public void onResponse_noGid_shouldNotFetchDetails() throws Exception { + Response response = getTestResponseNoGid(); + presenter.onResponse(response); + verify(detailFetcher, times(0)).fetchDetails("123abc", detailFetchListener); + } + + @Test + public void onResponse_noGid_shouldReturnPlaceFromResponse() throws Exception { + Response response = getTestResponseNoGid(); + Place place = mock(Place.class); + String details = "Test detail"; + Feature feature = response.body().getFeatures().get(0); + when(featureConverter.getFetchedPlace(feature)).thenReturn(place); + when(featureConverter.getDetails(feature)).thenReturn(details); + presenter.onResponse(response); + verify(detailFetchListener).onFetchSuccess(place, details); + } + @Test public void getBoundingBox_shouldReturnCorrectBox() throws Exception { LatLng sw = new LatLng(0.0, 0.0); @@ -120,4 +142,15 @@ public void getLon_noBoundingBox_noLostClient_shouldReturnDefaultLat() throws Ex return Response.success(result); } + @NonNull private Response getTestResponseNoGid() { + Result result = new Result(); + Feature feature = new Feature(); + Properties properties = new Properties(); + properties.gid = ""; + properties.name = "Test Name No Gid"; + feature.properties = properties; + result.getFeatures().add(feature); + return Response.success(result); + } + }