Return place from feature if no gid present (#389)

Author: sarahsnow1

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<Result> 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<Result> 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<Result> 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;
-  }
 }

…ernal/PeliasFeatureToPlaceConverter.java …api/internal/PeliasFeatureConverter.javamapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasFeatureToPlaceConverter.java renamed to mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasFeatureConverter.java mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasFeatureToPlaceConverter.java renamed to mapzen-places-api/src/main/java/com/mapzen/places/api/internal/PeliasFeatureConverter.java

@@ -10,18 +10,18 @@
 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;
 
   /**
    * 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;
+  }
 }

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 {
 

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();

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<Result> 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);
+    }
   }
 
   /**

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);
 

…l/PeliasFeatureToPlaceConverterTest.java …internal/PeliasFeatureConverterTest.javamapzen-places-api/src/test/java/com/mapzen/places/api/internal/PeliasFeatureToPlaceConverterTest.java renamed to mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PeliasFeatureConverterTest.java mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PeliasFeatureToPlaceConverterTest.java renamed to 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 {

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,17 +20,19 @@
 
 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;
 
 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<Result> response = getTestResponseNoGid();
+    presenter.onResponse(response);
+    verify(detailFetcher, times(0)).fetchDetails("123abc", detailFetchListener);
+  }
+
+  @Test
+  public void onResponse_noGid_shouldReturnPlaceFromResponse() throws Exception {
+    Response<Result> 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<Result> 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);
+  }
+
 }