From 5b36d29553dede4781ec28f14b01263dc70ce637 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20Lo=CC=81pez=20Man=CC=83as?= Date: Thu, 1 Oct 2026 17:42:23 +0200 Subject: [PATCH 1/2] test(data): cover KML and GeoJSON styles and KML layer parsing Adds behaviour tests for the parts of the data module that need no map: - KmlStyle: KML aabbggrr colors, fill, outline, width, marker hue, heading, hotspot, color modes, balloon text and computeRandomColor. - GeoJsonPolygonStyle, GeoJsonLineStringStyle, GeoJsonPointStyle: every setter notifies observers once, and the to*Options conversions carry every property. - KmlLayer parsing: document and nested folder containers, placemark properties and extended data, shared and inline styles, polygons with holes, ground overlays and placemarks across nested folders. data line coverage goes from 57.1% to 68.2%. Five tests are ignored because they document bugs found while writing them: - KmlStyle random icon color mode randomizes the hue value instead of the color, so a green marker turns blue. - GeoJsonLineStringStyle.toPolylineOptions drops the pattern and caps. - Styles referenced through a StyleMap are never applied. - A shared Style without an id crashes KmlLayer. - A ground overlay without a LatLonBox (gx:LatLonQuad) crashes KmlLayer. --- .../android/data/geojson/GeoJsonStylesTest.kt | 232 +++++++++++ .../android/data/kml/KmlLayerParsingTest.kt | 374 ++++++++++++++++++ .../maps/android/data/kml/KmlStyleTest.kt | 269 +++++++++++++ 3 files changed, 875 insertions(+) create mode 100644 data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt create mode 100644 data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt create mode 100644 data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt diff --git a/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt b/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt new file mode 100644 index 000000000..0a8b22433 --- /dev/null +++ b/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt @@ -0,0 +1,232 @@ +/* + * 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.model.BitmapDescriptor +import com.google.android.gms.maps.model.Dash +import com.google.android.gms.maps.model.Gap +import com.google.android.gms.maps.model.JointType +import com.google.android.gms.maps.model.RoundCap +import com.google.android.gms.maps.model.SquareCap +import com.google.common.truth.Truth.assertThat +import io.mockk.mockk +import org.junit.Ignore +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.util.Observable + +/** + * The GeoJSON styles notify their observers on every change (GeoJsonLayer redraws features from + * those notifications) and convert to the Maps SDK options used to draw each geometry. + */ +@Suppress("DEPRECATION") +@RunWith(RobolectricTestRunner::class) +class GeoJsonStylesTest { + private fun Observable.countNotifications(): () -> Int { + var count = 0 + addObserver { _, _ -> count++ } + return { count } + } + + @Test + fun `polygon style setters notify observers once each`() { + val style = GeoJsonPolygonStyle() + val notifications = style.countNotifications() + + style.fillColor = 0x7F00FF00 + style.setGeodesic(true) + style.setStrokeColor(0xFFFF0000.toInt()) + style.setStrokeJointType(JointType.ROUND) + style.setStrokePattern(listOf(Dash(10f), Gap(5f))) + style.setStrokeWidth(3f) + style.setZIndex(2f) + style.setVisible(false) + style.setClickable(false) + + assertThat(notifications()).isEqualTo(9) + } + + @Test + fun `polygon options carry every polygon style property`() { + val style = GeoJsonPolygonStyle() + style.fillColor = 0x7F00FF00 + style.setGeodesic(true) + style.setStrokeColor(0xFFFF0000.toInt()) + style.setStrokeJointType(JointType.BEVEL) + style.setStrokePattern(listOf(Dash(10f), Gap(5f))) + style.setStrokeWidth(3f) + style.setZIndex(2f) + style.setVisible(false) + style.setClickable(false) + + val options = style.toPolygonOptions() + + assertThat(options.fillColor).isEqualTo(0x7F00FF00) + assertThat(options.isGeodesic).isTrue() + assertThat(options.strokeColor).isEqualTo(0xFFFF0000.toInt()) + assertThat(options.strokeJointType).isEqualTo(JointType.BEVEL) + assertThat(options.strokePattern).containsExactly(Dash(10f), Gap(5f)).inOrder() + assertThat(options.strokeWidth).isEqualTo(3f) + assertThat(options.zIndex).isEqualTo(2f) + assertThat(options.isVisible).isFalse() + assertThat(options.isClickable).isFalse() + assertThat(style.toString()).contains("fill color=${0x7F00FF00}") + } + + @Test + fun `polygon style applies to polygons and collections`() { + assertThat(GeoJsonPolygonStyle().getGeometryType()) + .asList() + .containsExactly("Polygon", "MultiPolygon", "GeometryCollection") + } + + @Test + fun `polygon style defaults are clickable and visible`() { + val style = GeoJsonPolygonStyle() + + assertThat(style.isClickable()).isTrue() + assertThat(style.isVisible()).isTrue() + assertThat(style.isGeodesic()).isFalse() + } + + @Test + fun `line string style setters notify observers once each`() { + val style = GeoJsonLineStringStyle() + val notifications = style.countNotifications() + + style.color = 0xFF0000FF.toInt() + style.setClickable(false) + style.setGeodesic(true) + style.setWidth(6f) + style.setZIndex(1f) + style.setVisible(false) + style.setPattern(listOf(Dash(4f))) + style.setStartCap(RoundCap()) + style.setEndCap(SquareCap()) + + assertThat(notifications()).isEqualTo(9) + } + + @Test + fun `polyline options carry the line string style properties`() { + val style = GeoJsonLineStringStyle() + style.color = 0xFF0000FF.toInt() + style.setClickable(false) + style.setGeodesic(true) + style.setWidth(6f) + style.setZIndex(1f) + style.setVisible(false) + + val options = style.toPolylineOptions() + + assertThat(options.color).isEqualTo(0xFF0000FF.toInt()) + assertThat(options.isClickable).isFalse() + assertThat(options.isGeodesic).isTrue() + assertThat(options.width).isEqualTo(6f) + assertThat(options.zIndex).isEqualTo(1f) + assertThat(options.isVisible).isFalse() + assertThat(style.toString()).contains("width=6.0") + } + + /** + * Inside `PolylineOptions().apply { }`, toPolylineOptions calls getPattern(), getStartCap() + * and getEndCap() unqualified, which resolve to the new PolylineOptions' own getters rather + * than the style's. The pattern and caps of a GeoJSON line style are therefore never drawn. + */ + @Ignore("Known bug: toPolylineOptions drops the pattern and the start and end caps") + @Test + fun `polyline options carry the pattern and caps`() { + val style = GeoJsonLineStringStyle() + style.setPattern(listOf(Dash(4f), Gap(2f))) + style.setStartCap(RoundCap()) + style.setEndCap(SquareCap()) + + val options = style.toPolylineOptions() + + assertThat(options.startCap).isInstanceOf(RoundCap::class.java) + assertThat(options.endCap).isInstanceOf(SquareCap::class.java) + assertThat(options.pattern).containsExactly(Dash(4f), Gap(2f)).inOrder() + } + + @Test + fun `line string style applies to lines and collections`() { + assertThat(GeoJsonLineStringStyle().getGeometryType()) + .asList() + .containsExactly("LineString", "MultiLineString", "GeometryCollection") + } + + @Test + fun `point style setters notify observers once each`() { + val style = GeoJsonPointStyle() + val notifications = style.countNotifications() + + style.setAlpha(0.5f) + style.setAnchor(0.2f, 0.8f) + style.setDraggable(true) + style.setFlat(true) + style.setIcon(mockk()) + style.setInfoWindowAnchor(0.5f, 0.1f) + style.setRotation(30f) + style.setSnippet("snippet") + style.setTitle("title") + style.setVisible(false) + style.setZIndex(3f) + + assertThat(notifications()).isEqualTo(11) + } + + @Test + fun `marker options carry every point style property`() { + val icon = mockk() + val style = GeoJsonPointStyle() + style.setAlpha(0.5f) + style.setAnchor(0.2f, 0.8f) + style.setDraggable(true) + style.setFlat(true) + style.setIcon(icon) + style.setInfoWindowAnchor(0.5f, 0.1f) + style.setRotation(30f) + style.setSnippet("snippet") + style.setTitle("title") + style.setVisible(false) + style.setZIndex(3f) + + val options = style.toMarkerOptions() + + assertThat(options.alpha).isEqualTo(0.5f) + assertThat(options.anchorU).isEqualTo(0.2f) + assertThat(options.anchorV).isEqualTo(0.8f) + assertThat(options.isDraggable).isTrue() + assertThat(options.isFlat).isTrue() + assertThat(options.icon).isSameInstanceAs(icon) + assertThat(options.infoWindowAnchorU).isEqualTo(0.5f) + assertThat(options.infoWindowAnchorV).isEqualTo(0.1f) + assertThat(options.rotation).isEqualTo(30f) + assertThat(options.snippet).isEqualTo("snippet") + assertThat(options.title).isEqualTo("title") + assertThat(options.isVisible).isFalse() + assertThat(options.zIndex).isEqualTo(3f) + assertThat(style.toString()).contains("title=title") + } + + @Test + fun `point style applies to points and collections`() { + assertThat(GeoJsonPointStyle().getGeometryType()) + .asList() + .containsExactly("Point", "MultiPoint", "GeometryCollection") + } +} diff --git a/data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt b/data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt new file mode 100644 index 000000000..8f69b029d --- /dev/null +++ b/data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt @@ -0,0 +1,374 @@ +/* + * 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.kml + +import android.content.Context +import android.graphics.Color +import androidx.test.core.app.ApplicationProvider +import com.google.android.gms.maps.GoogleMap +import com.google.android.gms.maps.model.LatLng +import com.google.common.truth.Truth.assertThat +import io.mockk.mockk +import org.junit.Ignore +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * How KmlLayer turns a KML document into its legacy model: containers for the document and + * folders, placemarks with their properties and resolved styles, polygons and ground overlays. + */ +@Suppress("DEPRECATION") +@RunWith(RobolectricTestRunner::class) +class KmlLayerParsingTest { + private fun layerOf(kml: String): KmlLayer = + KmlLayer( + mockk(relaxed = true), + kml.trimIndent().byteInputStream(), + ApplicationProvider.getApplicationContext(), + ) + + private fun kml(body: String): String = + """ + + + $body + + """ + + @Test + fun `document becomes a root container with nested folders`() { + val layer = layerOf( + kml( + """ + + Trip + Summer + + Day 1 + Morning + Hotel2.0,41.0 + + Day 2 + + """ + ) + ) + + assertThat(layer.hasContainers()).isTrue() + assertThat(layer.hasPlacemarks()).isFalse() + val root = layer.getContainers().single() + assertThat(root.getContainerId()).isEqualTo("Trip") + assertThat(root.getProperty("name")).isEqualTo("Trip") + assertThat(root.getProperty("description")).isEqualTo("Summer") + assertThat(root.getProperties()).containsExactly("name", "description") + + val folders = root.getContainers().toList() + assertThat(folders.map { it.getProperty("name") }).containsExactly("Day 1", "Day 2").inOrder() + val day1 = folders.first() + assertThat(day1.hasContainers()).isTrue() + assertThat(day1.getContainers().single().getProperty("name")).isEqualTo("Morning") + assertThat(day1.hasPlacemarks()).isTrue() + assertThat(day1.getPlacemarks().single().getProperty("name")).isEqualTo("Hotel") + assertThat(folders[1].hasContainers()).isFalse() + assertThat(folders[1].hasPlacemarks()).isFalse() + } + + @Test + fun `document without a name is called Document`() { + val layer = layerOf(kml("")) + + val root = layer.getContainers().single() + assertThat(root.getContainerId()).isEqualTo("Document") + assertThat(root.hasProperties()).isFalse() + } + + @Test + fun `top level placemark without a document is a layer placemark`() { + val layer = layerOf( + kml("Solo1.0,2.0") + ) + + assertThat(layer.hasContainers()).isFalse() + val placemark = layer.getPlacemarks().single() + assertThat(placemark.getProperty("name")).isEqualTo("Solo") + assertThat(placemark.getGeometry()!!.getGeometryObject()).isEqualTo(LatLng(2.0, 1.0)) + } + + @Test + fun `placemark properties include extended data`() { + val layer = layerOf( + kml( + """ + + + Peak + Highest point + + 8848 + Himalaya + + 86.925,27.988,8848 + + + """ + ) + ) + + val placemark = layer.getContainers().single().getPlacemarks().single() + assertThat(placemark.getProperty("name")).isEqualTo("Peak") + assertThat(placemark.getProperty("description")).isEqualTo("Highest point") + assertThat(placemark.getProperty("elevation")).isEqualTo("8848") + assertThat(placemark.getProperty("range")).isEqualTo("Himalaya") + } + + @Test + fun `shared style is resolved from the placemark style url`() { + val layer = layerOf( + kml( + """ + + + + #red + + 0,0 1,0 1,1 0,0 + + + + """ + ) + ) + + val root = layer.getContainers().single() + assertThat(root.getStyle("red")).isNotNull() + val placemark = root.getPlacemarks().single() + assertThat(placemark.getStyleId()).isEqualTo("red") + assertThat(placemark.getPolygonOptions()!!.fillColor).isEqualTo(Color.RED) + } + + @Test + fun `style map keeps the normal style url`() { + val layer = layerOf( + kml( + """ + + + normal#red + highlight#blue + + + """ + ) + ) + + assertThat(layer.getContainers().single().getStyleIdFromMap("highlightable")) + .isEqualTo("#red") + } + + /** + * Style maps store the normal styleUrl with its leading '#' ("#red"), but shared styles are + * keyed by bare id ("red"). Resolving a placemark through a StyleMap therefore looks up + * "#red", finds nothing and leaves the placemark unstyled. Google Earth exports put a + * StyleMap on almost every placemark. + */ + @Ignore("Known bug: styles referenced through a StyleMap are never applied") + @Test + fun `style map resolves to the normal shared style`() { + val layer = layerOf( + kml( + """ + + + + + normal#red + highlight#blue + + + #highlightable + + 0,0 1,0 1,1 0,0 + + + + """ + ) + ) + + val placemark = layer.getContainers().single().getPlacemarks().single() + assertThat(placemark.getPolygonOptions()).isNotNull() + assertThat(placemark.getPolygonOptions()!!.fillColor).isEqualTo(Color.RED) + } + + @Test + fun `inline style wins over the shared style`() { + val layer = layerOf( + kml( + """ + + + + #red + + + 0,0 1,0 1,1 0,0 + + + + """ + ) + ) + + val placemark = layer.getContainers().single().getPlacemarks().single() + assertThat(placemark.getPolygonOptions()!!.fillColor).isEqualTo(Color.GREEN) + } + + @Test + fun `polygon keeps its outer and inner boundaries`() { + val layer = layerOf( + kml( + """ + + + + 0,0 10,0 10,10 0,10 0,0 + + + 2,2 3,2 3,3 2,2 + + + 5,5 6,5 6,6 5,5 + + + + """ + ) + ) + + val polygon = layer.getPlacemarks().single().getGeometry() as KmlPolygon + assertThat(polygon.getGeometryType()).isEqualTo("Polygon") + assertThat(polygon.getOuterBoundaryCoordinates()).hasSize(5) + assertThat(polygon.getOuterBoundaryCoordinates()[1]).isEqualTo(LatLng(0.0, 10.0)) + assertThat(polygon.getInnerBoundaryCoordinates()).hasSize(2) + assertThat(polygon.getInnerBoundaryCoordinates()[1][0]).isEqualTo(LatLng(5.0, 5.0)) + assertThat(polygon.getGeometryObject()).hasSize(3) + } + + @Test + fun `polygon without holes has an empty inner boundary list`() { + val polygon = KmlPolygon(listOf(LatLng(0.0, 0.0), LatLng(1.0, 0.0), LatLng(0.0, 0.0)), null) + + assertThat(polygon.getInnerBoundaryCoordinates()).isEmpty() + assertThat(polygon.getGeometryObject()).hasSize(1) + assertThat(polygon.toString()).contains("outer coordinates=") + } + + @Test + fun `ground overlay keeps its image, bounds and draw order`() { + val layer = layerOf( + kml( + """ + + + Survey + 3 + https://example.com/overlay.png + + 10-1020-20 + + + + """ + ) + ) + + val overlay = layer.getContainers().single().getGroundOverlays().single() + assertThat(overlay.getImageUrl()).isEqualTo("https://example.com/overlay.png") + assertThat(overlay.getLatLngBox().southwest).isEqualTo(LatLng(-10.0, -20.0)) + assertThat(overlay.getLatLngBox().northeast).isEqualTo(LatLng(10.0, 20.0)) + assertThat(overlay.getProperty("name")).isEqualTo("Survey") + assertThat(overlay.getGroundOverlayOptions().zIndex).isEqualTo(3f) + } + + @Test + fun `all placemarks are collected across nested folders`() { + val layer = layerOf( + kml( + """ + + a0,0 + + b0,0 + + c0,0 + + + + """ + ) + ) + + assertThat(layer.getAllPlacemarks().map { it.getProperty("name") }).containsExactly("a", "b", "c") + } + + /** + * Shared styles are indexed with `it.id!!`. A shared Style without an id is unusual but + * valid KML, and it makes the KmlLayer constructor throw a NullPointerException. + */ + @Ignore("Known bug: a shared Style without an id crashes KmlLayer") + @Test + fun `shared style without an id is ignored`() { + val layer = layerOf( + kml( + """ + + + 0,0 + + """ + ) + ) + + assertThat(layer.getContainers().single().hasPlacemarks()).isTrue() + } + + /** + * Ground overlays read `latLonBox!!`. Overlays positioned with gx:LatLonQuad, which Google + * Earth writes for rotated overlays, have no LatLonBox and make the constructor throw. + */ + @Ignore("Known bug: a ground overlay without a LatLonBox crashes KmlLayer") + @Test + fun `ground overlay without a LatLonBox does not crash the layer`() { + val layer = layerOf( + kml( + """ + + + https://example.com/overlay.png + + 0,0 1,0 1,1 0,1 + + + 0,0 + + """ + ) + ) + + assertThat(layer.getContainers().single().hasPlacemarks()).isTrue() + } +} diff --git a/data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt b/data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt new file mode 100644 index 000000000..9c1d2f78c --- /dev/null +++ b/data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt @@ -0,0 +1,269 @@ +/* + * 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.kml + +import android.graphics.Color +import com.google.android.gms.maps.model.BitmapDescriptorFactory +import com.google.common.truth.Truth.assertThat +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.slot +import io.mockk.unmockkStatic +import io.mockk.verify +import org.junit.After +import org.junit.Before +import org.junit.Ignore +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@Suppress("DEPRECATION") +@RunWith(RobolectricTestRunner::class) +class KmlStyleTest { + private val hues = mutableListOf() + + @Before + fun setUp() { + mockkStatic(BitmapDescriptorFactory::class) + val hue = slot() + every { BitmapDescriptorFactory.defaultMarker(capture(hue)) } answers { + hues += hue.captured + mockk() + } + } + + @After + fun tearDown() { + unmockkStatic(BitmapDescriptorFactory::class) + } + + @Test + fun defaults() { + val style = KmlStyle() + + assertThat(style.hasFill()).isTrue() + assertThat(style.hasOutline()).isTrue() + assertThat(style.getIconScale()).isEqualTo(1.0) + assertThat(style.getIconUrl()).isNull() + assertThat(style.getStyleId()).isNull() + assertThat(style.hasBalloonStyle()).isFalse() + assertThat(style.isIconRandomColorMode()).isFalse() + assertThat(style.isLineRandomColorMode()).isFalse() + assertThat(style.isPolyRandomColorMode()).isFalse() + assertThat(style.isStyleSet("fillColor")).isFalse() + assertThat(style.getPolylineOptions().isClickable).isTrue() + assertThat(style.getPolygonOptions().isClickable).isTrue() + } + + @Test + fun `fill color converts KML aabbggrr to ARGB`() { + val style = KmlStyle() + + // KML colors are alpha, blue, green, red: this is half-transparent green. + style.setFillColor("7f00ff00") + + assertThat(style.getPolygonOptions().fillColor).isEqualTo(0x7F00FF00) + assertThat(style.isStyleSet("fillColor")).isTrue() + } + + @Test + fun `fill color reads red from the last two digits`() { + val style = KmlStyle() + + style.setFillColor("ff0000ff") + + assertThat(style.getPolygonOptions().fillColor).isEqualTo(Color.RED) + } + + @Test + fun `six digit colors are bbggrr and opaque`() { + val style = KmlStyle() + + style.setFillColor("0000ff") + + assertThat(style.getPolygonOptions().fillColor).isEqualTo(Color.RED) + } + + @Test + fun `surrounding whitespace in colors is ignored`() { + val style = KmlStyle() + + style.setFillColor(" ff00ff00\n") + + assertThat(style.getPolygonOptions().fillColor).isEqualTo(Color.GREEN) + } + + @Test + fun `outline color and width apply to lines and polygon strokes`() { + val style = KmlStyle() + + style.setOutlineColor("ffff0000") + style.setWidth(4f) + + assertThat(style.getPolylineOptions().color).isEqualTo(Color.BLUE) + assertThat(style.getPolylineOptions().width).isEqualTo(4f) + assertThat(style.getPolygonOptions().strokeColor).isEqualTo(Color.BLUE) + assertThat(style.getPolygonOptions().strokeWidth).isEqualTo(4f) + assertThat(style.isStyleSet("outlineColor")).isTrue() + assertThat(style.isStyleSet("width")).isTrue() + } + + @Test + fun `polygon without fill keeps the default transparent fill`() { + val style = KmlStyle() + style.setFillColor("ff00ff00") + + style.setFill(false) + + assertThat(style.hasFill()).isFalse() + assertThat(style.getPolygonOptions().fillColor).isEqualTo(0) + } + + @Test + fun `polygon without outline has no stroke`() { + val style = KmlStyle() + style.setOutlineColor("ffff0000") + style.setWidth(4f) + + style.setOutline(false) + + assertThat(style.hasOutline()).isFalse() + assertThat(style.getPolygonOptions().strokeWidth).isEqualTo(0f) + assertThat(style.isStyleSet("outline")).isTrue() + } + + @Test + fun `marker color sets the default marker hue`() { + val style = KmlStyle() + + style.setMarkerColor("ffff0000") + + verify { BitmapDescriptorFactory.defaultMarker(240f) } + assertThat(style.mMarkerColor).isEqualTo(240f) + assertThat(style.isStyleSet("markerColor")).isTrue() + } + + @Test + fun `heading and fraction hotspot end up in the marker options`() { + val style = KmlStyle() + + style.setHeading(90f) + style.setHotSpot(0.25f, 0.75f, "fraction", "fraction") + + val options = style.getMarkerOptions() + assertThat(options.rotation).isEqualTo(90f) + assertThat(options.anchorU).isEqualTo(0.25f) + assertThat(options.anchorV).isEqualTo(0.75f) + assertThat(style.isStyleSet("heading")).isTrue() + assertThat(style.isStyleSet("hotSpot")).isTrue() + } + + @Test + fun `hotspot units other than fraction fall back to the bottom center`() { + val style = KmlStyle() + + style.setHotSpot(10f, 20f, "pixels", "insetPixels") + + val options = style.getMarkerOptions() + assertThat(options.anchorU).isEqualTo(0.5f) + assertThat(options.anchorV).isEqualTo(1.0f) + } + + @Test + fun `marker options are a copy`() { + val style = KmlStyle() + style.setHeading(45f) + + val first = style.getMarkerOptions() + first.rotation(180f) + + assertThat(style.getMarkerOptions().rotation).isEqualTo(45f) + } + + @Test + fun `color modes are random only for the value random`() { + val style = KmlStyle() + + style.setIconColorMode("random") + style.setLineColorMode("normal") + style.setPolyColorMode("random") + + assertThat(style.isIconRandomColorMode()).isTrue() + assertThat(style.isLineRandomColorMode()).isFalse() + assertThat(style.isPolyRandomColorMode()).isTrue() + assertThat(style.isStyleSet("iconColorMode")).isTrue() + assertThat(style.isStyleSet("lineColorMode")).isTrue() + assertThat(style.isStyleSet("polyColorMode")).isTrue() + } + + @Test + fun `balloon text, icon and id setters`() { + val style = KmlStyle() + + style.setInfoWindowText("Hello") + style.setIconUrl("https://example.com/icon.png") + style.setIconScale(2.5) + style.setStyleId("#highlight") + + assertThat(style.hasBalloonStyle()).isTrue() + assertThat(style.getBalloonOptions()).containsExactly("text", "Hello") + assertThat(style.getIconUrl()).isEqualTo("https://example.com/icon.png") + assertThat(style.getIconScale()).isEqualTo(2.5) + assertThat(style.getStyleId()).isEqualTo("#highlight") + assertThat(style.isStyleSet("iconUrl")).isTrue() + assertThat(style.isStyleSet("iconScale")).isTrue() + assertThat(style.toString()).contains("style id=#highlight") + } + + @Test + fun `random color keeps zero components at zero and never brightens`() { + val color = Color.rgb(0, 200, 0) + + repeat(50) { + val random = KmlStyle.computeRandomColor(color) + assertThat(Color.red(random)).isEqualTo(0) + assertThat(Color.blue(random)).isEqualTo(0) + assertThat(Color.green(random)).isLessThan(200) + } + } + + @Test + fun `random color of black is black`() { + assertThat(KmlStyle.computeRandomColor(Color.BLACK)).isEqualTo(Color.BLACK) + } + + /** + * In random icon color mode, getMarkerOptions passes the marker's hue (0..360) to + * computeRandomColor, which expects an ARGB color. A green marker (hue 120) is treated as + * the color 0x00000078, so the random marker is always blue (hue 240) or black (hue 0) + * instead of a random shade of the style's green. + */ + @Ignore("Known bug: random icon color mode randomizes the hue value instead of the color") + @Test + fun `random icon color mode keeps the marker hue`() { + val style = KmlStyle() + style.setMarkerColor("ff00ff00") + style.setIconColorMode("random") + hues.clear() + + repeat(20) { style.getMarkerOptions() } + + // A random linear scale of pure green is still green, or black once scaled to zero. + assertThat(hues).isNotEmpty() + hues.forEach { assertThat(it).isAnyOf(120f, 0f) } + } +} From 4ae5ad9c102c90476fc8797322c37e98dc307868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20L=C3=B3pez-Ma=C3=B1as?= Date: Thu, 1 Oct 2026 20:02:49 +0200 Subject: [PATCH 2/2] fix(data): apply StyleMap styles and fix four KML and GeoJSON style bugs (#1809) - KmlLayer resolved StyleMap entries with their leading '#' while shared styles are keyed by bare id, so placemarks styled through a StyleMap were unstyled. - KmlLayer no longer throws on a shared Style or StyleMap without an id; they are skipped since nothing can reference them. - KmlLayer no longer throws on a GroundOverlay without a LatLonBox (for example gx:LatLonQuad); the overlay is skipped with a warning. - GeoJsonLineStringStyle.toPolylineOptions now keeps the pattern and the start and end caps. - KmlStyle random icon color mode now randomizes the marker color instead of its hue, so markers keep their color family. The regression tests from the data coverage tests are no longer ignored. --- .../data/geojson/GeoJsonLineStringStyle.kt | 8 ++-- .../google/maps/android/data/kml/KmlLayer.kt | 42 ++++++++++++++----- .../google/maps/android/data/kml/KmlStyle.kt | 6 ++- .../android/data/geojson/GeoJsonStylesTest.kt | 7 +--- .../android/data/kml/KmlLayerParsingTest.kt | 17 +++----- .../maps/android/data/kml/KmlStyleTest.kt | 8 +--- 6 files changed, 51 insertions(+), 37 deletions(-) diff --git a/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLineStringStyle.kt b/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLineStringStyle.kt index 37cb056e7..f62f7b955 100644 --- a/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLineStringStyle.kt +++ b/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLineStringStyle.kt @@ -85,9 +85,11 @@ public class GeoJsonLineStringStyle : visible(mPolylineOptions.isVisible) width(mPolylineOptions.width) zIndex(mPolylineOptions.zIndex) - pattern(getPattern()) - startCap(getStartCap()) - endCap(getEndCap()) + // Read from mPolylineOptions: inside apply, getPattern() and the cap getters would + // resolve to the new PolylineOptions instead of this style. + pattern(mPolylineOptions.pattern) + startCap(mPolylineOptions.startCap) + endCap(mPolylineOptions.endCap) } override fun toString(): String = diff --git a/data/src/main/java/com/google/maps/android/data/kml/KmlLayer.kt b/data/src/main/java/com/google/maps/android/data/kml/KmlLayer.kt index 3eac2d5c3..2ef0bc1b5 100644 --- a/data/src/main/java/com/google/maps/android/data/kml/KmlLayer.kt +++ b/data/src/main/java/com/google/maps/android/data/kml/KmlLayer.kt @@ -17,6 +17,7 @@ package com.google.maps.android.data.kml import android.content.Context import android.graphics.Color +import android.util.Log import com.google.android.gms.maps.GoogleMap import com.google.android.gms.maps.model.LatLng import com.google.android.gms.maps.model.LatLngBounds @@ -37,6 +38,8 @@ import java.io.IOException import java.io.InputStream import java.nio.charset.StandardCharsets +private const val LOG_TAG = "KmlLayer" + @Deprecated("Use the new platform-agnostic data layer and renderer instead.") public class KmlLayer : Layer { private val mPlacemarks = mutableListOf() @@ -131,8 +134,16 @@ public class KmlLayer : Layer { // Parse Document kmlObj.document?.let { doc -> - val styles = doc.styles.associate { it.id!! to toLegacyStyle(it) } - val styleMaps = doc.styleMaps.associate { it.id!! to (it.pairs.firstOrNull { p -> p.key == "normal" }?.styleUrl ?: "") } + // Shared styles and style maps can only be referenced by id, so ones without an id + // are skipped rather than failing the whole layer. + val styles = doc.styles.mapNotNull { style -> style.id?.let { it to toLegacyStyle(style) } }.toMap() + val styleMaps = + doc.styleMaps + .mapNotNull { styleMap -> + styleMap.id?.let { id -> + id to (styleMap.pairs.firstOrNull { p -> p.key == "normal" }?.styleUrl ?: "") + } + }.toMap() val placemarksMap = HashMap() doc.placemarks.forEach { p -> @@ -146,7 +157,7 @@ public class KmlLayer : Layer { val groundOverlaysMap = HashMap() doc.groundOverlays.forEach { g -> - groundOverlaysMap[toLegacyGroundOverlay(g)] = null + toLegacyGroundOverlay(g)?.let { groundOverlaysMap[it] = null } } val properties = HashMap() @@ -171,7 +182,7 @@ public class KmlLayer : Layer { if (kmlObj.document == null) { kmlObj.placemark?.let { mPlacemarks.add(toLegacyPlacemark(it, emptyMap(), emptyMap())) } kmlObj.folder?.let { mContainers.add(toLegacyContainer(it, emptyMap(), emptyMap())) } - kmlObj.groundOverlay?.let { mGroundOverlays.add(toLegacyGroundOverlay(it)) } + kmlObj.groundOverlay?.let { overlay -> toLegacyGroundOverlay(overlay)?.let { mGroundOverlays.add(it) } } } } @@ -213,7 +224,9 @@ public class KmlLayer : Layer { } val styleUrl = placemark.styleUrl?.substringAfter("#") ?: "" - val resolvedStyleUrl = styleMaps[styleUrl] ?: styleUrl + // Style map entries keep the '#' of their normal styleUrl, while shared styles are keyed + // by bare id. + val resolvedStyleUrl = styleMaps[styleUrl]?.substringAfter("#") ?: styleUrl val inlineStyle = placemark.style?.let { toLegacyStyle(it) } ?: styles[resolvedStyleUrl] return KmlPlacemark(geometry, resolvedStyleUrl, inlineStyle, properties) @@ -240,7 +253,7 @@ public class KmlLayer : Layer { val groundOverlaysMap = HashMap() folder.groundOverlays.forEach { g -> - groundOverlaysMap[toLegacyGroundOverlay(g)] = null + toLegacyGroundOverlay(g)?.let { groundOverlaysMap[it] = null } } return KmlContainer( @@ -254,14 +267,23 @@ public class KmlLayer : Layer { ) } - private fun toLegacyGroundOverlay(groundOverlay: com.google.maps.android.data.parser.kml.GroundOverlay): KmlGroundOverlay { + /** + * Returns null for overlays without a LatLonBox, such as those positioned with gx:LatLonQuad, + * which a ground overlay on the map cannot represent. + */ + private fun toLegacyGroundOverlay(groundOverlay: com.google.maps.android.data.parser.kml.GroundOverlay): KmlGroundOverlay? { + val latLonBox = groundOverlay.latLonBox + if (latLonBox == null) { + Log.w(LOG_TAG, "Skipping GroundOverlay ${groundOverlay.name ?: ""} without a LatLonBox") + return null + } val properties = mutableMapOf() groundOverlay.name?.let { properties["name"] = it } val bounds = LatLngBounds( - LatLng(groundOverlay.latLonBox!!.south, groundOverlay.latLonBox.west), - LatLng(groundOverlay.latLonBox.north, groundOverlay.latLonBox.east), + LatLng(latLonBox.south, latLonBox.west), + LatLng(latLonBox.north, latLonBox.east), ) return KmlGroundOverlay( @@ -270,7 +292,7 @@ public class KmlLayer : Layer { drawOrder = groundOverlay.drawOrder?.toFloat() ?: 0f, visibility = if (groundOverlay.visibility) 1 else 0, properties = properties, - rotation = groundOverlay.latLonBox.rotation?.toFloat() ?: 0f, + rotation = latLonBox.rotation?.toFloat() ?: 0f, ) } diff --git a/data/src/main/java/com/google/maps/android/data/kml/KmlStyle.kt b/data/src/main/java/com/google/maps/android/data/kml/KmlStyle.kt index 696b95b09..73892d982 100644 --- a/data/src/main/java/com/google/maps/android/data/kml/KmlStyle.kt +++ b/data/src/main/java/com/google/maps/android/data/kml/KmlStyle.kt @@ -40,6 +40,9 @@ public class KmlStyle : Style() { private var mPolyRandomColorMode = false internal var mMarkerColor = 0f + // The ARGB color behind mMarkerColor, which is only a hue. KML's default color is white. + private var mMarkerArgb = Color.WHITE + public fun setInfoWindowText(text: String) { mBalloonOptions["text"] = text } @@ -89,6 +92,7 @@ public class KmlStyle : Style() { public fun setMarkerColor(color: String) { val integerColor = Color.parseColor("#" + convertColor(color)) + mMarkerArgb = integerColor mMarkerColor = getHueValue(integerColor) mMarkerOptions.icon(BitmapDescriptorFactory.defaultMarker(mMarkerColor)) mStylesSet.add("markerColor") @@ -149,7 +153,7 @@ public class KmlStyle : Style() { newMarkerOption.rotation(mMarkerOptions.rotation) newMarkerOption.anchor(mMarkerOptions.anchorU, mMarkerOptions.anchorV) if (mIconRandomColorMode) { - val hue = getHueValue(computeRandomColor(mMarkerColor.toInt())) + val hue = getHueValue(computeRandomColor(mMarkerArgb)) mMarkerOptions.icon(BitmapDescriptorFactory.defaultMarker(hue)) } newMarkerOption.icon(mMarkerOptions.icon) diff --git a/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt b/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt index 0a8b22433..805a408a1 100644 --- a/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt +++ b/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt @@ -23,7 +23,6 @@ import com.google.android.gms.maps.model.RoundCap import com.google.android.gms.maps.model.SquareCap import com.google.common.truth.Truth.assertThat import io.mockk.mockk -import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -143,11 +142,9 @@ class GeoJsonStylesTest { } /** - * Inside `PolylineOptions().apply { }`, toPolylineOptions calls getPattern(), getStartCap() - * and getEndCap() unqualified, which resolve to the new PolylineOptions' own getters rather - * than the style's. The pattern and caps of a GeoJSON line style are therefore never drawn. + * Regression test: toPolylineOptions used to call getPattern() and the cap getters inside + * `PolylineOptions().apply { }`, which resolved to the new options instead of the style. */ - @Ignore("Known bug: toPolylineOptions drops the pattern and the start and end caps") @Test fun `polyline options carry the pattern and caps`() { val style = GeoJsonLineStringStyle() diff --git a/data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt b/data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt index 8f69b029d..f04fb37f2 100644 --- a/data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt +++ b/data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt @@ -22,7 +22,6 @@ import com.google.android.gms.maps.GoogleMap import com.google.android.gms.maps.model.LatLng import com.google.common.truth.Truth.assertThat import io.mockk.mockk -import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -180,12 +179,9 @@ class KmlLayerParsingTest { } /** - * Style maps store the normal styleUrl with its leading '#' ("#red"), but shared styles are - * keyed by bare id ("red"). Resolving a placemark through a StyleMap therefore looks up - * "#red", finds nothing and leaves the placemark unstyled. Google Earth exports put a - * StyleMap on almost every placemark. + * Regression test: style maps keep the '#' of their normal styleUrl ("#red") while shared + * styles are keyed by bare id, so placemarks styled through a StyleMap used to be unstyled. */ - @Ignore("Known bug: styles referenced through a StyleMap are never applied") @Test fun `style map resolves to the normal shared style`() { val layer = layerOf( @@ -326,10 +322,8 @@ class KmlLayerParsingTest { } /** - * Shared styles are indexed with `it.id!!`. A shared Style without an id is unusual but - * valid KML, and it makes the KmlLayer constructor throw a NullPointerException. + * Regression test: a shared Style without an id used to throw a NullPointerException. */ - @Ignore("Known bug: a shared Style without an id crashes KmlLayer") @Test fun `shared style without an id is ignored`() { val layer = layerOf( @@ -347,10 +341,9 @@ class KmlLayerParsingTest { } /** - * Ground overlays read `latLonBox!!`. Overlays positioned with gx:LatLonQuad, which Google - * Earth writes for rotated overlays, have no LatLonBox and make the constructor throw. + * Regression test: overlays without a LatLonBox (gx:LatLonQuad, written by Google Earth for + * rotated overlays) used to throw a NullPointerException. They are now skipped. */ - @Ignore("Known bug: a ground overlay without a LatLonBox crashes KmlLayer") @Test fun `ground overlay without a LatLonBox does not crash the layer`() { val layer = layerOf( diff --git a/data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt b/data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt index 9c1d2f78c..37350b864 100644 --- a/data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt +++ b/data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt @@ -26,7 +26,6 @@ import io.mockk.unmockkStatic import io.mockk.verify import org.junit.After import org.junit.Before -import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -247,12 +246,9 @@ class KmlStyleTest { } /** - * In random icon color mode, getMarkerOptions passes the marker's hue (0..360) to - * computeRandomColor, which expects an ARGB color. A green marker (hue 120) is treated as - * the color 0x00000078, so the random marker is always blue (hue 240) or black (hue 0) - * instead of a random shade of the style's green. + * Random icon color mode applies a random scale to the marker's ARGB color, so a green + * marker stays a shade of green (or black, once scaled to zero). */ - @Ignore("Known bug: random icon color mode randomizes the hue value instead of the color") @Test fun `random icon color mode keeps the marker hue`() { val style = KmlStyle()