diff --git a/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLayer.kt b/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLayer.kt index 07c2c4442..95460a2c9 100644 --- a/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLayer.kt +++ b/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLayer.kt @@ -32,6 +32,7 @@ import org.json.JSONException import org.json.JSONObject import java.io.IOException import java.io.InputStream +import java.util.Observer @Deprecated("Use the new platform-agnostic data layer and renderer instead.") public class GeoJsonLayer : Layer { @@ -40,8 +41,11 @@ public class GeoJsonLayer : Layer { private var mRenderer: MapViewRenderer? = null private var mIsLayerOnMap = false private val mFeatureMap = HashMap() - private val mModelToLegacyFeatures = HashMap() + private val mModelToLegacyFeatures = java.util.IdentityHashMap() private var mFeatureClickListener: OnFeatureClickListener? = null + private val mFeatureObserver = Observer { observable, _ -> + if (observable is GeoJsonFeature) onFeatureChanged(observable) + } public interface GeoJsonOnFeatureClickListener : OnFeatureClickListener @@ -122,6 +126,7 @@ public class GeoJsonLayer : Layer { } } + mFeatures.forEach { it.addObserver(mFeatureObserver) } // Calculate bounding box calculateBoundingBox() } @@ -230,8 +235,7 @@ public class GeoJsonLayer : Layer { override fun addLayerToMap() { val renderer = mRenderer ?: return mFeatures.forEach { feature -> - val modelFeature = toModelFeature(feature) - renderer.addFeature(modelFeature) + if (feature.getGeometry() != null) renderer.addFeature(toModelFeature(feature)) } mIsLayerOnMap = true } @@ -239,8 +243,7 @@ public class GeoJsonLayer : Layer { override fun removeLayerFromMap() { val renderer = mRenderer ?: return mFeatures.forEach { feature -> - val modelFeature = toModelFeature(feature) - renderer.removeFeature(modelFeature) + mFeatureMap[feature]?.let { renderer.removeFeature(it) } } mIsLayerOnMap = false } @@ -284,6 +287,18 @@ public class GeoJsonLayer : Layer { ) } + is com.google.maps.android.data.renderer.model.MultiGeometry -> { + if (geometry is GeoJsonMultiPolygon) { + val polygonStyle = feature.polygonStyle ?: mDefaultPolygonStyle + com.google.maps.android.data.renderer.model.PolygonStyle( + fillColor = polygonStyle.fillColor, + strokeColor = polygonStyle.getStrokeColor(), + strokeWidth = polygonStyle.getStrokeWidth(), + geodesic = polygonStyle.isGeodesic(), + ) + } else null + } + else -> { null } @@ -350,14 +365,18 @@ public class GeoJsonLayer : Layer { get() = mFeatures public fun addFeature(feature: GeoJsonFeature) { - mFeatures.add(feature) - if (mIsLayerOnMap) { + if (!mFeatures.contains(feature)) { + mFeatures.add(feature) + feature.addObserver(mFeatureObserver) + } + if (mIsLayerOnMap && feature.getGeometry() != null) { mRenderer?.addFeature(toModelFeature(feature)) } } public fun removeFeature(feature: GeoJsonFeature) { - mFeatures.remove(feature) + if (!mFeatures.remove(feature)) return + feature.deleteObserver(mFeatureObserver) val modelFeature = mFeatureMap.remove(feature) if (modelFeature != null) { mModelToLegacyFeatures.remove(modelFeature) @@ -367,6 +386,17 @@ public class GeoJsonLayer : Layer { } } + private fun onFeatureChanged(feature: GeoJsonFeature) { + mFeatureMap.remove(feature)?.let { oldModel -> + mModelToLegacyFeatures.remove(oldModel) + if (mIsLayerOnMap) mRenderer?.removeFeature(oldModel) + } + if (mFeatures.contains(feature) && feature.getGeometry() != null) { + val updatedModel = toModelFeature(feature) + if (mIsLayerOnMap) mRenderer?.addFeature(updatedModel) + } + } + override fun setOnFeatureClickListener(listener: OnFeatureClickListener) { mFeatureClickListener = listener mGoogleMap?.let { map -> diff --git a/data/src/main/java/com/google/maps/android/data/renderer/mapview/MapViewRenderer.kt b/data/src/main/java/com/google/maps/android/data/renderer/mapview/MapViewRenderer.kt index f599e7735..fbab8f7ad 100644 --- a/data/src/main/java/com/google/maps/android/data/renderer/mapview/MapViewRenderer.kt +++ b/data/src/main/java/com/google/maps/android/data/renderer/mapview/MapViewRenderer.kt @@ -29,6 +29,7 @@ import com.google.maps.android.data.renderer.IconProvider import com.google.maps.android.data.renderer.model.DataLayer import com.google.maps.android.data.renderer.model.DataScene import com.google.maps.android.data.renderer.model.Feature +import com.google.maps.android.data.renderer.model.Geometry import com.google.maps.android.data.renderer.model.GroundOverlay import com.google.maps.android.data.renderer.model.GroundOverlayStyle import com.google.maps.android.data.renderer.model.LineString @@ -108,10 +109,22 @@ class MapViewRenderer( } override fun addFeature(feature: Feature) { + removeFeature(feature) val mapObjects = mutableListOf() - when (feature.geometry) { + renderGeometry(feature, feature.geometry, mapObjects) + if (mapObjects.isNotEmpty()) { + renderedFeatures[feature] = mapObjects + } + } + + private fun renderGeometry( + feature: Feature, + geometry: Geometry, + mapObjects: MutableList, + ) { + when (geometry) { is PointGeometry -> { - val point = feature.geometry.point + val point = geometry.point val style = feature.style as? PointStyle if (useAdvancedMarkers) { val markerOptions = createAdvancedMarkerOptions(point, style, feature.properties) @@ -159,30 +172,26 @@ class MapViewRenderer( } is LineString -> { - val lineString = feature.geometry val style = feature.style as? LineStyle - val polylineOptions = createPolylineOptions(lineString, style) + val polylineOptions = createPolylineOptions(geometry, style) mapObjects.add(map.addPolyline(polylineOptions)) } is Polygon -> { - val polygon = feature.geometry val style = feature.style as? PolygonStyle - val polygonOptions = createPolygonOptions(polygon, style) + val polygonOptions = createPolygonOptions(geometry, style) mapObjects.add(map.addPolygon(polygonOptions)) } is MultiGeometry -> { - feature.geometry.geometries.forEach { geometry -> - // Recursively add each geometry in the MultiGeometry - addFeature(feature.copy(geometry = geometry)) + geometry.geometries.forEach { childGeometry -> + renderGeometry(feature, childGeometry, mapObjects) } } is GroundOverlay -> { - val groundOverlay = feature.geometry val style = feature.style as? GroundOverlayStyle - val options = createGroundOverlayOptions(groundOverlay, style) + val options = createGroundOverlayOptions(geometry, style) style?.iconUrl?.let { url -> val localBitmap = localImages[url] @@ -223,9 +232,6 @@ class MapViewRenderer( } } } - if (mapObjects.isNotEmpty()) { - renderedFeatures[feature] = mapObjects - } } override fun removeFeature(feature: Feature) { diff --git a/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonLayerObserverTest.kt b/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonLayerObserverTest.kt new file mode 100644 index 000000000..338b760ab --- /dev/null +++ b/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonLayerObserverTest.kt @@ -0,0 +1,245 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.maps.android.data.geojson + +import com.google.android.gms.maps.GoogleMap +import com.google.android.gms.maps.model.LatLng +import com.google.android.gms.maps.model.Polygon +import com.google.android.gms.maps.model.PolygonOptions +import com.google.maps.android.data.Feature +import io.mockk.Runs +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertSame +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class GeoJsonLayerObserverTest { + @Test + fun addedPolygonStyleChange_removesAndRedrawsFeature() { + val map = mockk(relaxed = true) + val firstPolygon = mockk(relaxed = true) + val secondPolygon = mockk(relaxed = true) + val options = mutableListOf() + every { map.addPolygon(capture(options)) } returnsMany listOf(firstPolygon, secondPolygon) + val layer = emptyLayer(map) + val (feature, style) = polygonFeature(INITIAL_COLOR) + + layer.addLayerToMap() + layer.addFeature(feature) + style.fillColor = UPDATED_COLOR + + verify(exactly = 1) { firstPolygon.remove() } + verify(exactly = 2) { map.addPolygon(any()) } + assertEquals(INITIAL_COLOR, options[0].fillColor) + assertEquals(UPDATED_COLOR, options[1].fillColor) + } + + @Test + fun parsedFeatureStyleChangeWhileOffMap_isUsedWhenLayerIsReadded() { + val map = mockk(relaxed = true) + val firstPolygon = mockk(relaxed = true) + val secondPolygon = mockk(relaxed = true) + val options = mutableListOf() + every { map.addPolygon(capture(options)) } returnsMany listOf(firstPolygon, secondPolygon) + val layer = + GeoJsonLayer( + map, + JSONObject( + """{"type":"Feature","geometry":{"type":"Polygon","coordinates":[[[0,0],[1,0],[1,1],[0,0]]]}}""", + ), + ) + val feature = layer.features.single() + + layer.addLayerToMap() + layer.removeLayerFromMap() + feature.polygonStyle!!.fillColor = UPDATED_COLOR + + layer.addLayerToMap() + + verify(exactly = 1) { firstPolygon.remove() } + verify(exactly = 2) { map.addPolygon(any()) } + assertEquals(UPDATED_COLOR, options.last().fillColor) + } + + @Test + fun removedFeature_stopsObservingStyleChanges() { + val map = mockk(relaxed = true) + val polygon = mockk(relaxed = true) + every { map.addPolygon(any()) } returns polygon + val layer = emptyLayer(map) + val (feature, style) = polygonFeature(INITIAL_COLOR) + layer.addLayerToMap() + layer.addFeature(feature) + + assertEquals(1, feature.countObservers()) + layer.removeFeature(feature) + assertEquals(0, feature.countObservers()) + style.fillColor = UPDATED_COLOR + + verify(exactly = 1) { polygon.remove() } + verify(exactly = 1) { map.addPolygon(any()) } + } + + @Test + fun featureWithoutGeometry_isIgnoredByLayerLifecycle() { + val map = mockk(relaxed = true) + val layer = emptyLayer(map) + val feature = GeoJsonFeature(null, null, null, null) + + layer.addLayerToMap() + layer.addFeature(feature) + layer.removeLayerFromMap() + layer.addLayerToMap() + + assertEquals(listOf(feature), layer.features.toList()) + verify(exactly = 0) { map.addPolygon(any()) } + } + + @Test + fun multiPolygonStyleChange_redrawsAndRemovesAllChildrenWithParentClickLookup() { + val map = mockk(relaxed = true) + val polygons = List(4) { mockk(relaxed = true) } + val options = mutableListOf() + val polygonClickListener = slot() + every { map.addPolygon(capture(options)) } returnsMany polygons + every { map.setOnPolygonClickListener(capture(polygonClickListener)) } just Runs + val layer = emptyLayer(map) + val (feature, style) = multiPolygonFeature(INITIAL_COLOR) + var clickedFeature: Feature? = null + + layer.addLayerToMap() + layer.setOnFeatureClickListener { clickedFeature = it } + layer.addFeature(feature) + style.fillColor = UPDATED_COLOR + + assertEquals(listOf(INITIAL_COLOR, INITIAL_COLOR, UPDATED_COLOR, UPDATED_COLOR), options.map { it.fillColor }) + verify(exactly = 1) { polygons[0].remove() } + verify(exactly = 1) { polygons[1].remove() } + + polygonClickListener.captured.onPolygonClick(polygons[2]) + assertSame(feature, clickedFeature) + + layer.removeFeature(feature) + + verify(exactly = 1) { polygons[2].remove() } + verify(exactly = 1) { polygons[3].remove() } + } + + @Test + fun addingSameFeatureTwice_replacesRenderingWithoutDuplicatingLifecycle() { + val map = mockk(relaxed = true) + val firstPolygon = mockk(relaxed = true) + val secondPolygon = mockk(relaxed = true) + every { map.addPolygon(any()) } returnsMany listOf(firstPolygon, secondPolygon) + val layer = emptyLayer(map) + val (feature, style) = polygonFeature(INITIAL_COLOR) + + layer.addLayerToMap() + layer.addFeature(feature) + layer.addFeature(feature) + + assertEquals(listOf(feature), layer.features.toList()) + assertEquals(1, feature.countObservers()) + verify(exactly = 1) { firstPolygon.remove() } + + layer.removeFeature(feature) + style.fillColor = UPDATED_COLOR + + assertEquals(emptyList(), layer.features.toList()) + assertEquals(0, feature.countObservers()) + verify(exactly = 1) { secondPolygon.remove() } + verify(exactly = 2) { map.addPolygon(any()) } + } + + @Test + fun equalModelFeatures_clickLookupUsesModelIdentity() { + val map = mockk(relaxed = true) + val firstPolygon = mockk(relaxed = true) + val secondPolygon = mockk(relaxed = true) + val polygonClickListener = slot() + every { map.addPolygon(any()) } returnsMany listOf(firstPolygon, secondPolygon) + every { map.setOnPolygonClickListener(capture(polygonClickListener)) } just Runs + val layer = emptyLayer(map) + val (firstFeature) = polygonFeature(INITIAL_COLOR) + val (secondFeature) = polygonFeature(INITIAL_COLOR) + var clickedFeature: Feature? = null + + layer.addLayerToMap() + layer.setOnFeatureClickListener { clickedFeature = it } + layer.addFeature(firstFeature) + layer.addFeature(secondFeature) + + polygonClickListener.captured.onPolygonClick(firstPolygon) + assertSame(firstFeature, clickedFeature) + polygonClickListener.captured.onPolygonClick(secondPolygon) + assertSame(secondFeature, clickedFeature) + } + + private fun emptyLayer(map: GoogleMap): GeoJsonLayer = + GeoJsonLayer( + map, + JSONObject("""{"type":"FeatureCollection","features":[]}"""), + ) + + private fun polygonFeature(fillColor: Int): Pair { + val geometry = + GeoJsonPolygon( + listOf( + listOf( + LatLng(0.0, 0.0), + LatLng(0.0, 1.0), + LatLng(1.0, 1.0), + LatLng(0.0, 0.0), + ), + ), + ) + val style = GeoJsonPolygonStyle().apply { this.fillColor = fillColor } + return GeoJsonFeature(geometry, null, null, null).also { it.polygonStyle = style } to style + } + + private fun multiPolygonFeature(fillColor: Int): Pair { + val firstPolygon = polygon(0.0) + val secondPolygon = polygon(2.0) + val style = GeoJsonPolygonStyle().apply { this.fillColor = fillColor } + return GeoJsonFeature(GeoJsonMultiPolygon(listOf(firstPolygon, secondPolygon)), null, null, null) + .also { it.polygonStyle = style } to style + } + + private fun polygon(offset: Double): GeoJsonPolygon = + GeoJsonPolygon( + listOf( + listOf( + LatLng(offset, offset), + LatLng(offset, offset + 1.0), + LatLng(offset + 1.0, offset + 1.0), + LatLng(offset, offset), + ), + ), + ) + + private companion object { + const val INITIAL_COLOR = -15654349 + const val UPDATED_COLOR = -12298906 + } +} diff --git a/data/src/test/java/com/google/maps/android/data/renderer/MapViewRendererTest.kt b/data/src/test/java/com/google/maps/android/data/renderer/MapViewRendererTest.kt index d1ba1c6b5..17c7ab943 100644 --- a/data/src/test/java/com/google/maps/android/data/renderer/MapViewRendererTest.kt +++ b/data/src/test/java/com/google/maps/android/data/renderer/MapViewRendererTest.kt @@ -20,15 +20,20 @@ import com.google.android.gms.maps.model.AdvancedMarkerOptions import com.google.android.gms.maps.model.LatLng import com.google.android.gms.maps.model.Marker import com.google.android.gms.maps.model.MarkerOptions +import com.google.android.gms.maps.model.Polygon as MapPolygon +import com.google.android.gms.maps.model.PolygonOptions import com.google.maps.android.data.renderer.mapview.MapViewRenderer import com.google.maps.android.data.renderer.model.Feature +import com.google.maps.android.data.renderer.model.MultiGeometry import com.google.maps.android.data.renderer.model.Point import com.google.maps.android.data.renderer.model.PointGeometry +import com.google.maps.android.data.renderer.model.Polygon import io.mockk.every import io.mockk.mockk import io.mockk.slot import io.mockk.verify import org.junit.Assert.assertEquals +import org.junit.Assert.assertSame import org.junit.Test /** @@ -143,4 +148,41 @@ class MapViewRendererTest { assertEquals("Critical Right Turn", capturedOptions.title) assertEquals("Be careful here!", capturedOptions.snippet) } + + @Test + fun nestedMultiGeometry_childrenBelongToParentFeature() { + val map = mockk(relaxed = true) + val firstPolygon = mockk(relaxed = true) + val secondPolygon = mockk(relaxed = true) + every { map.addPolygon(any()) } returnsMany listOf(firstPolygon, secondPolygon) + val renderer = MapViewRenderer(map, mockk(relaxed = true)) + val polygon = + Polygon( + listOf( + Point(0.0, 0.0), + Point(0.0, 1.0), + Point(1.0, 1.0), + Point(0.0, 0.0), + ), + ) + val feature = + Feature( + MultiGeometry( + listOf( + polygon, + MultiGeometry(listOf(polygon.copy())), + ), + ), + ) + + renderer.addFeature(feature) + + assertSame(feature, renderer.getFeatureForMapObject(firstPolygon)) + assertSame(feature, renderer.getFeatureForMapObject(secondPolygon)) + + renderer.removeFeature(feature) + + verify(exactly = 1) { firstPolygon.remove() } + verify(exactly = 1) { secondPolygon.remove() } + } }