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 e589ed926..a73c19a24 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 @@ -33,6 +33,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 { @@ -41,8 +42,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 @@ -123,6 +127,7 @@ public class GeoJsonLayer : Layer { } } + mFeatures.forEach { it.addObserver(mFeatureObserver) } // Calculate bounding box calculateBoundingBox() } @@ -231,8 +236,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 } @@ -240,8 +244,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 } @@ -288,6 +291,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 } @@ -354,14 +369,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) @@ -371,6 +390,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 af8e91edd..aa300a768 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 @@ -109,6 +109,7 @@ class MapViewRenderer( } override fun addFeature(feature: Feature) { + removeFeature(feature) val mapObjects = mutableListOf() addGeometry(feature.geometry, feature, mapObjects) if (mapObjects.isNotEmpty()) { @@ -116,6 +117,7 @@ class MapViewRenderer( } } + /** * Renders a single [geometry] (recursing into [MultiGeometry] members) with [feature]'s style and * properties, accumulating every created map object into [mapObjects] so nested geometries stay 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 d7a538f8f..f19216ab6 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,6 +20,8 @@ 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.android.gms.maps.model.Polyline import com.google.android.gms.maps.model.PolylineOptions @@ -32,11 +34,13 @@ 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 com.google.maps.android.data.renderer.model.PolygonStyle + 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 /** @@ -153,7 +157,7 @@ class MapViewRendererTest { } @Test -fun testRemoveFeature_multiGeometry_removesAllRenderedObjects() { + fun testRemoveFeature_multiGeometry_removesAllRenderedObjects() { // Given val mockMap = mockk(relaxed = true) val mockIconProvider = mockk(relaxed = true) @@ -305,4 +309,41 @@ fun testRemoveFeature_multiGeometry_removesAllRenderedObjects() { // Then verify(exactly = 1) { mockPolygon.remove() } } + + @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() } + } }