diff --git a/services-geocoding/src/main/java/com/mapbox/api/geocoding/v5/models/CarmenFeature.java b/services-geocoding/src/main/java/com/mapbox/api/geocoding/v5/models/CarmenFeature.java index 532c69c07..84b501457 100644 --- a/services-geocoding/src/main/java/com/mapbox/api/geocoding/v5/models/CarmenFeature.java +++ b/services-geocoding/src/main/java/com/mapbox/api/geocoding/v5/models/CarmenFeature.java @@ -11,6 +11,7 @@ import com.mapbox.api.geocoding.v5.GeocodingCriteria.GeocodingTypeCriteria; import com.mapbox.geojson.BoundingBox; import com.mapbox.geojson.Feature; +import com.mapbox.geojson.GeoJson; import com.mapbox.geojson.Geometry; import com.mapbox.geojson.Point; import com.mapbox.geojson.gson.BoundingBoxDeserializer; @@ -24,7 +25,7 @@ /** * The Features key in the geocoding API response contains the majority of information you'll want - * to use. It extends the {@link Feature} object in GeoJSON and adds several additional attribute + * to use. It extends the {@link GeoJson} object in GeoJSON and adds several additional attribute * which further describe the geocoding result. *

* A Geocoding id is a String in the form {@code {type}.{id}} where {@code {type}} is the lowest @@ -41,7 +42,7 @@ * @since 1.0.0 */ @AutoValue -public abstract class CarmenFeature implements Serializable { +public abstract class CarmenFeature implements GeoJson, Serializable { private static final String TYPE = "Feature"; @@ -60,7 +61,12 @@ public static CarmenFeature fromJson(@NonNull String json) { .registerTypeAdapter(BoundingBox.class, new BoundingBoxDeserializer()) .registerTypeAdapterFactory(GeocodingAdapterFactory.create()) .create(); - return gson.fromJson(json, CarmenFeature.class); + CarmenFeature feature = gson.fromJson(json, CarmenFeature.class); + // Even thought properties are Nullable, + // Feature object will be created with properties set to an empty object, + return feature.properties() == null + ? feature.toBuilder().properties(new JsonObject()).build() + : feature; } /** @@ -72,12 +78,14 @@ public static CarmenFeature fromJson(@NonNull String json) { @NonNull public static Builder builder() { return new AutoValue_CarmenFeature.Builder() - .type(TYPE); + .type(TYPE) + .properties(new JsonObject()); } // // Feature specific attributes // + // Note that CarmenFeature cannot extend Feature due to AutoValue limitations /** * This describes the TYPE of GeoJson geometry this object is, thus this will always return @@ -89,6 +97,7 @@ public static Builder builder() { */ @NonNull @SerializedName("type") + @Override public abstract String type(); /** @@ -102,6 +111,7 @@ public static Builder builder() { * @since 3.0.0 */ @Nullable + @Override public abstract BoundingBox bbox(); /** @@ -132,7 +142,7 @@ public static Builder builder() { * @return a {@link JsonObject} which holds this features current properties * @since 1.0.0 */ - @NonNull + @Nullable public abstract JsonObject properties(); // @@ -282,6 +292,7 @@ public static TypeAdapter typeAdapter(Gson gson) { * @return a JSON string which represents this CarmenFeature * @since 3.0.0 */ + @Override @SuppressWarnings("unused") public String toJson() { Gson gson = new GsonBuilder() @@ -289,7 +300,14 @@ public String toJson() { .registerTypeAdapter(BoundingBox.class, new BoundingBoxSerializer()) .registerTypeAdapterFactory(GeocodingAdapterFactory.create()) .create(); - return gson.toJson(this, CarmenFeature.class); + + // Empty properties -> should not appear in json string + CarmenFeature feature = this; + if (properties() != null && properties().size() == 0) { + feature = toBuilder().properties(null).build(); + } + + return gson.toJson(feature, CarmenFeature.class); } /** @@ -355,7 +373,7 @@ public abstract static class Builder { * @return this builder for chaining options together * @since 3.0.0 */ - public abstract Builder properties(@NonNull JsonObject properties); + public abstract Builder properties(@Nullable JsonObject properties); /** * A string representing the feature in the requested language. diff --git a/services-geocoding/src/test/java/com/mapbox/api/geocoding/v5/models/CarmenFeatureTest.java b/services-geocoding/src/test/java/com/mapbox/api/geocoding/v5/models/CarmenFeatureTest.java index d1dce31e3..7628b3b6a 100644 --- a/services-geocoding/src/test/java/com/mapbox/api/geocoding/v5/models/CarmenFeatureTest.java +++ b/services-geocoding/src/test/java/com/mapbox/api/geocoding/v5/models/CarmenFeatureTest.java @@ -7,17 +7,24 @@ import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.IsEqual.equalTo; import static org.hamcrest.junit.MatcherAssert.assertThat; +import static org.junit.Assert.assertFalse; import com.google.gson.JsonObject; import com.mapbox.api.geocoding.v5.GeocodingTestUtils; import com.mapbox.api.geocoding.v5.MapboxGeocoding; import com.mapbox.core.TestUtils; import com.mapbox.geojson.CoordinateContainer; +import com.mapbox.geojson.Feature; +import com.mapbox.geojson.LineString; import com.mapbox.geojson.Point; + +import org.junit.Assert; import org.junit.Test; import retrofit2.Response; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.Locale; public class CarmenFeatureTest extends GeocodingTestUtils { @@ -149,4 +156,27 @@ public void ForwardGeocode_withValidChineseResponse() throws Exception { assertThat(feature.bbox().east(), equalTo(107.025123596)); assertThat(feature.bbox().north(), equalTo(39.6012458800001)); } + + @Test + public void testNullProperties() { + CarmenFeature feature = CarmenFeature.builder() + .geometry(Point.fromLngLat(-77, 38)) + .build(); + String jsonString = feature.toJson(); + assertFalse(jsonString.contains("\"properties\":{}")); + + // Feature (empty Properties) -> Json (null Properties) -> Equavalent Feature + CarmenFeature featureFromJson = CarmenFeature.fromJson(jsonString); + assertEquals(featureFromJson, feature); + } + + @Test + public void testNullPropertiesJson() { + String jsonString = "{\"type\":\"Feature\",\"geometry\":{\"type\":\"Point\",\"coordinates\":[-77.0, 38.0]}}"; + CarmenFeature feature = CarmenFeature.fromJson(jsonString); + + // Json( null Properties) -> Feature (empty Properties) -> Json(null Properties) + String fromFeatureJsonString = feature.toJson(); + assertEquals(fromFeatureJsonString, jsonString); + } } diff --git a/services-geocoding/src/test/resources/forward_feature_valid.json b/services-geocoding/src/test/resources/forward_feature_valid.json index 94b37456e..87516c3f9 100644 --- a/services-geocoding/src/test/resources/forward_feature_valid.json +++ b/services-geocoding/src/test/resources/forward_feature_valid.json @@ -1 +1 @@ -{"id":"address.3982178573139850","type":"Feature","place_type":["address"],"relevance":1,"properties":{},"text":"Pennsylvania Ave NW","place_name":"1600 Pennsylvania Ave NW, Washington, District of Columbia 20006, United States","center":[-77.036543,38.897702],"geometry":{"type":"Point","coordinates":[-77.036543,38.897702]},"address":"1600","context":[{"id":"neighborhood.291451","text":"Downtown"},{"id":"postcode.8031694603652840","text":"20006"},{"id":"place.11387590027246050","wikidata":"Q61","text":"Washington"},{"id":"region.3403","short_code":"US-DC","wikidata":"Q61","text":"District of Columbia"},{"id":"country.3145","short_code":"us","wikidata":"Q30","text":"United States"}]} +{"id":"address.3982178573139850","type":"Feature","place_type":["address"],"relevance":1,"text":"Pennsylvania Ave NW","place_name":"1600 Pennsylvania Ave NW, Washington, District of Columbia 20006, United States","center":[-77.036543,38.897702],"geometry":{"type":"Point","coordinates":[-77.036543,38.897702]},"address":"1600","context":[{"id":"neighborhood.291451","text":"Downtown"},{"id":"postcode.8031694603652840","text":"20006"},{"id":"place.11387590027246050","wikidata":"Q61","text":"Washington"},{"id":"region.3403","short_code":"US-DC","wikidata":"Q61","text":"District of Columbia"},{"id":"country.3145","short_code":"us","wikidata":"Q30","text":"United States"}]} diff --git a/services-geojson/src/main/java/com/mapbox/geojson/Feature.java b/services-geojson/src/main/java/com/mapbox/geojson/Feature.java index 401ca0f93..d4853fbfa 100644 --- a/services-geojson/src/main/java/com/mapbox/geojson/Feature.java +++ b/services-geojson/src/main/java/com/mapbox/geojson/Feature.java @@ -66,7 +66,16 @@ public static Feature fromJson(@NonNull String json) { gson.registerTypeAdapter(Point.class, new PointDeserializer()); gson.registerTypeAdapter(BoundingBox.class, new BoundingBoxDeserializer()); gson.registerTypeAdapter(Geometry.class, new GeometryDeserializer()); - return gson.create().fromJson(json, Feature.class); + Feature feature = gson.create().fromJson(json, Feature.class); + + // Even thought properties are Nullable, + // Feature object will be created with properties set to an empty object, + // so that addProperties() would work + if (feature.properties() != null) { + return feature; + } + return new AutoValue_Feature(TYPE, feature.bbox(), + feature.id(), feature.geometry(), new JsonObject()); } /** @@ -105,8 +114,9 @@ public static Feature fromGeometry(@Nullable Geometry geometry, @Nullable Boundi * method * @since 1.0.0 */ - public static Feature fromGeometry(@Nullable Geometry geometry, @NonNull JsonObject properties) { - return new AutoValue_Feature(TYPE, null, null, geometry, properties); + public static Feature fromGeometry(@Nullable Geometry geometry, @Nullable JsonObject properties) { + return new AutoValue_Feature(TYPE, null, null, geometry, + properties == null ? new JsonObject() : properties); } /** @@ -120,9 +130,10 @@ public static Feature fromGeometry(@Nullable Geometry geometry, @NonNull JsonObj * method * @since 1.0.0 */ - public static Feature fromGeometry(@Nullable Geometry geometry, @NonNull JsonObject properties, + public static Feature fromGeometry(@Nullable Geometry geometry, @Nullable JsonObject properties, @Nullable BoundingBox bbox) { - return new AutoValue_Feature(TYPE, bbox, null, geometry, properties); + return new AutoValue_Feature(TYPE, bbox, null, geometry, + properties == null ? new JsonObject() : properties); } /** @@ -135,9 +146,10 @@ public static Feature fromGeometry(@Nullable Geometry geometry, @NonNull JsonObj * @return {@link Feature} * @since 1.0.0 */ - public static Feature fromGeometry(@Nullable Geometry geometry, @NonNull JsonObject properties, + public static Feature fromGeometry(@Nullable Geometry geometry, @Nullable JsonObject properties, @Nullable String id) { - return new AutoValue_Feature(TYPE, null, id, geometry, properties); + return new AutoValue_Feature(TYPE, null, id, geometry, + properties == null ? new JsonObject() : properties); } /** @@ -225,7 +237,14 @@ public String toJson() { GsonBuilder gson = new GsonBuilder(); gson.registerTypeAdapter(Point.class, new PointSerializer()); gson.registerTypeAdapter(BoundingBox.class, new BoundingBoxSerializer()); - return gson.create().toJson(this); + + // Empty properties -> should not appear in json string + Feature feature = this; + if (properties().size() == 0) { + feature = new AutoValue_Feature(TYPE, bbox(), id(), geometry(), null); + } + + return gson.create().toJson(feature); } /** diff --git a/services-geojson/src/test/java/com/mapbox/geojson/FeatureTest.java b/services-geojson/src/test/java/com/mapbox/geojson/FeatureTest.java index e1c9ea82f..42d4a69e1 100644 --- a/services-geojson/src/test/java/com/mapbox/geojson/FeatureTest.java +++ b/services-geojson/src/test/java/com/mapbox/geojson/FeatureTest.java @@ -1,6 +1,7 @@ package com.mapbox.geojson; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -47,7 +48,7 @@ public void bbox_doesNotSerializeWhenNotPresent() throws Exception { Feature feature = Feature.fromGeometry(lineString); compareJson(feature.toJson(), "{\"type\":\"Feature\",\"geometry\":{\"type\":" - + "\"LineString\",\"coordinates\":[[1,2],[2,3]]},\"properties\":{}}"); + + "\"LineString\",\"coordinates\":[[1,2],[2,3]]}}"); } @Test @@ -76,7 +77,7 @@ public void bbox_doesSerializeWhenPresent() throws Exception { BoundingBox bbox = BoundingBox.fromLngLats(1.0, 2.0, 3.0, 4.0); Feature feature = Feature.fromGeometry(lineString, bbox); compareJson("{\"type\":\"Feature\",\"bbox\":[1.0,2.0,3.0,4.0],\"geometry\":" - + "{\"type\":\"LineString\",\"coordinates\":[[1,2],[2,3]]},\"properties\":{}}", + + "{\"type\":\"LineString\",\"coordinates\":[[1,2],[2,3]]}}", feature.toJson()); } @@ -103,7 +104,12 @@ public void testNullProperties() { coordinates.add(Point.fromLngLat(4.5, 6.7)); LineString line = LineString.fromLngLats(coordinates); Feature feature = Feature.fromGeometry(line); - assertTrue(feature.toJson().contains("\"properties\":{}")); + String jsonString = feature.toJson(); + assertFalse(jsonString.contains("\"properties\":{}")); + + // Feature (empty Properties) -> Json (null Properties) -> Equavalent Feature + Feature featureFromJson = Feature.fromJson(jsonString); + assertEquals(featureFromJson, feature); } @Test @@ -115,6 +121,21 @@ public void testNonNullProperties() { JsonObject properties = new JsonObject(); properties.addProperty("key", "value"); Feature feature = Feature.fromGeometry(line, properties); - assertTrue(feature.toJson().contains("\"properties\":{\"key\":\"value\"}")); + String jsonString = feature.toJson(); + assertTrue(jsonString.contains("\"properties\":{\"key\":\"value\"}")); + + // Feature (non-empty Properties) -> Json (non-empty Properties) -> Equavalent Feature + assertEquals(Feature.fromJson(jsonString), feature); + } + + @Test + public void testNullPropertiesJson() { + String jsonString = "{\"type\":\"Feature\",\"bbox\":[1.0,2.0,3.0,4.0],\"geometry\":" + + "{\"type\":\"LineString\",\"coordinates\":[[1.0,2.0],[2.0,3.0]]}}"; + Feature feature = Feature.fromJson(jsonString); + + // Json( null Properties) -> Feature (empty Properties) -> Json(null Properties) + String fromFeatureJsonString = feature.toJson(); + assertEquals(fromFeatureJsonString, jsonString); } }