Skip to content

feat!: migrate android-maps-ktx into android-maps-utils (v6.0.0) - #1716

Open
dkhawk wants to merge 11 commits into
mainfrom
feat/migrate-ktx-to-utils
Open

dkhawk wants to merge 11 commits into
mainfrom
feat/migrate-ktx-to-utils

Conversation

@dkhawk

@dkhawk dkhawk commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Migrates all Kotlin extensions from android-maps-ktx (maps-ktx and maps-utils-ktx) directly into android-maps-utils, establishing v6.0.0 as the consolidated, single-source release for both Java utilities and Kotlin Coroutine/Flow extensions.

Key Changes

  1. Consolidated Kotlin Extensions:
    • Reactive coroutine suspensions (awaitMap(), awaitMapsSdkInitialized(), awaitAnimateCamera()).
    • Reactive Flow event streams (mapClickEvents(), cameraMoveEvents(), markerClickEvents()).
    • DSL option builders (addMarker { ... }, addPolyline { ... }, addPolygon { ... }).
  2. Canonical & Compatibility Packages:
    • All extensions and builders live under canonical com.google.maps.android.* packages.
    • Preserves com.google.maps.android.ktx.* with @Deprecated(level = DeprecationLevel.WARNING) forwarding wrappers and typealiases for seamless backward compatibility.
  3. Release Please Configuration:
    • Configured release-please-config.json and .release-please-manifest.json for final v6.0.0 stable release.
    • Updated documentation (README.md, MIGRATION.md, llm-integration-prompt.md, .gemini/skills/android-maps-utils/SKILL.md).
  4. Rebase & Test Validation:
    • Rebased onto latest main (e8ef093c, Kover migration).
    • 100% unit tests pass across all modules (:library, :clustering, :data, :heatmaps, :ui, :demo).

Base automatically changed from feat/rewrite-android-maps-utils to main July 15, 2026 16:38
dkhawk added a commit that referenced this pull request Aug 5, 2026
…1716)

- Consolidate Kotlin Extensions (KTX into Utils): Move all reactive Coroutine/Flow extensions (awaitMap, mapClickEvents, cameraMoveEvents) and option builder DSLs (addMarker, addPolyline, addPolygon) from android-maps-ktx directly into android-maps-utils.

- Canonical Non-KTX Packages: Place all reactive Coroutine/Flow extensions and DSL builders in canonical com.google.maps.android.* packages.

- Deprecated Compatibility Layer: Preserve the legacy com.google.maps.android.ktx.* package structure with @deprecated(level = DeprecationLevel.WARNING, replaceWith = ReplaceWith(...)) forwarding wrappers and typealiases so existing imports compile seamlessly with deprecation warnings.

- Multi-Module Integration: Integrate KTX extensions across :library, :clustering, :heatmaps, and :data modules.

- Demo & Test Consolidation: Include KtxExtensionsDemoActivity in :demo and integrate all 18 KTX unit test suites with both canonical and shim test coverage.
@dkhawk
dkhawk force-pushed the feat/migrate-ktx-to-utils branch from d5b5d7a to f4a5753 Compare August 5, 2026 17:27
dkhawk added a commit that referenced this pull request Aug 5, 2026
…1716)

- Consolidate Kotlin Extensions (KTX into Utils): Move all reactive Coroutine/Flow extensions (awaitMap, mapClickEvents, cameraMoveEvents) and option builder DSLs (addMarker, addPolyline, addPolygon) from android-maps-ktx directly into android-maps-utils.

- Canonical Non-KTX Packages: Place all reactive Coroutine/Flow extensions and DSL builders in canonical com.google.maps.android.* packages.

- Deprecated Compatibility Layer: Preserve the legacy com.google.maps.android.ktx.* package structure with @deprecated(level = DeprecationLevel.WARNING, replaceWith = ReplaceWith(...)) forwarding wrappers and typealiases so existing imports compile seamlessly with deprecation warnings.

- Multi-Module Integration: Integrate KTX extensions across :library, :clustering, :heatmaps, and :data modules.

- Demo & Test Consolidation: Include KtxExtensionsDemoActivity in :demo and integrate all 18 KTX unit test suites with both canonical and shim test coverage.
@dkhawk
dkhawk force-pushed the feat/migrate-ktx-to-utils branch from f4a5753 to 407fded Compare August 5, 2026 17:48
Comment thread demo/src/main/res/values/strings.xml Fixed
@googlemaps-bot

googlemaps-bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Code Coverage

Overall Project 52.56% -1.78% 🍏
Files changed 77.99% 🍏

Module Coverage
Kover Gradle Plugin XML report for :library 88.08% -7.11% 🍏
Kover Gradle Plugin XML report for :heatmaps 85.02% -3.61%
Kover Gradle Plugin XML report for :data 48.19% -1.03%
Kover Gradle Plugin XML report for :clustering 32.93% 🍏
Files
Module File Coverage
Kover Gradle Plugin XML report for :library SupportStreetViewPanoramaFragment.kt 100% 🍏
MapView.kt 100% 🍏
MapFragment.kt 100% 🍏
SupportMapFragment.kt 100% 🍏
StreetViewPanoramaFragment.kt 100% 🍏
Polyline.kt 100% 🍏
LatLng.kt 100% 🍏
SupportStreetViewPanoramaFragment.kt 100% 🍏
MapView.kt 100% 🍏
SupportMapFragment.kt 100% 🍏
Polyline.kt 100% 🍏
LatLng.kt 100% 🍏
StreetViewPanoramaView.kt 100% 🍏
MapFragment.kt 100% 🍏
MapsInitializer.kt 100% 🍏
StreetViewPanoramaFragment.kt 100% 🍏
AttributionIdInitializer.kt 100% 🍏
PolylineOptions.kt 100% 🍏
MarkerOptions.kt 100% 🍏
PolygonOptions.kt 100% 🍏
CircleOptions.kt 100% 🍏
StreetViewPanoramaOrientation.kt 100% 🍏
CameraPosition.kt 100% 🍏
GroundOverlayOptions.kt 100% 🍏
TileOverlayOptions.kt 100% 🍏
StreetViewPanoramaCamera.kt 100% 🍏
PolylineOptions.kt 100% 🍏
MarkerOptions.kt 100% 🍏
PolygonOptions.kt 100% 🍏
CircleOptions.kt 100% 🍏
StreetViewPanoramaOrientation.kt 100% 🍏
CameraPosition.kt 100% 🍏
GroundOverlayOptions.kt 100% 🍏
TileOverlayOptions.kt 100% 🍏
StreetViewPanoramaCamera.kt 100% 🍏
MarkerManagerFlows.kt 91.2% -8.8% 🍏
LocationManager.kt 86.71% -13.29% 🍏
FusedLocationProvider.kt 84.62% -15.38% 🍏
GoogleMap.kt 83.75% -16.25% 🍏
Polygon.kt 80.95% -19.05% 🍏
PolylineManagerFlows.kt 78% -22% 🍏
GroundOverlayManagerFlows.kt 78% -22% 🍏
PolygonManagerFlows.kt 78% -22% 🍏
CircleManagerFlows.kt 78% -22% 🍏
Polygon.kt 70.73% -29.27% 🍏
MapsInitializer.kt 66.67% -33.33% 🍏
MarkerManager.kt 60% -40% 🍏
GoogleMap.kt 50.63% -49.37%
LocationManager.kt 50% -50%
FusedLocationProvider.kt 38.46% -61.54%
PolygonManager.kt 33.33% -66.67%
CircleManager.kt 33.33% -66.67%
GroundOverlayManager.kt 33.33% -66.67%
PolylineManager.kt 33.33% -66.67%
StreetViewPanoramaView.kt 27.27% -72.73%
Kover Gradle Plugin XML report for :heatmaps Heatmap.kt 15.15% -84.85%
Heatmap.kt 13.73% -86.27%
Kover Gradle Plugin XML report for :data GeoJson.kt 0%
GeoJson.kt 0%
Kml.kt 0%
Kml.kt 0%
Kover Gradle Plugin XML report for :clustering ClusterManagerFlows.kt 100% 🍏
ClusterManager.kt 100% 🍏
Point.kt 100% 🍏
PointExtensions.kt 100% 🍏

dkhawk added a commit that referenced this pull request Aug 5, 2026
…1716)

- Consolidate Kotlin Extensions (KTX into Utils): Move all reactive Coroutine/Flow extensions (awaitMap, mapClickEvents, cameraMoveEvents) and option builder DSLs (addMarker, addPolyline, addPolygon) from android-maps-ktx directly into android-maps-utils.

- Canonical Non-KTX Packages: Place all reactive Coroutine/Flow extensions and DSL builders in canonical com.google.maps.android.* packages.

- Deprecated Compatibility Layer: Preserve the legacy com.google.maps.android.ktx.* package structure with @deprecated(level = DeprecationLevel.WARNING, replaceWith = ReplaceWith(...)) forwarding wrappers and typealiases so existing imports compile seamlessly with deprecation warnings.

- Multi-Module Integration: Integrate KTX extensions across :library, :clustering, :heatmaps, and :data modules.

- Demo & Test Consolidation: Include KtxExtensionsDemoActivity in :demo and integrate all 18 KTX unit test suites with both canonical and shim test coverage.
@dkhawk
dkhawk force-pushed the feat/migrate-ktx-to-utils branch from 407fded to 96b0801 Compare August 5, 2026 19:48
dkhawk added a commit that referenced this pull request Aug 5, 2026
…1716)

- Consolidate Kotlin Extensions (KTX into Utils): Move all reactive Coroutine/Flow extensions (awaitMap, mapClickEvents, cameraMoveEvents) and option builder DSLs (addMarker, addPolyline, addPolygon) from android-maps-ktx directly into android-maps-utils.

- Canonical Non-KTX Packages: Place all reactive Coroutine/Flow extensions and DSL builders in canonical com.google.maps.android.* packages.

- Deprecated Compatibility Layer: Preserve the legacy com.google.maps.android.ktx.* package structure with @deprecated(level = DeprecationLevel.WARNING, replaceWith = ReplaceWith(...)) forwarding wrappers and typealiases so existing imports compile seamlessly with deprecation warnings.

- Multi-Module Integration: Integrate KTX extensions across :library, :clustering, :heatmaps, and :data modules.

- Demo & Test Consolidation: Include KtxExtensionsDemoActivity in :demo and integrate all 18 KTX unit test suites with both canonical and shim test coverage.
@dkhawk
dkhawk force-pushed the feat/migrate-ktx-to-utils branch from 96b0801 to ff8f979 Compare August 5, 2026 19:56
dkhawk added a commit that referenced this pull request Aug 7, 2026
…1716)

- Consolidate Kotlin Extensions (KTX into Utils): Move all reactive Coroutine/Flow extensions (awaitMap, mapClickEvents, cameraMoveEvents) and option builder DSLs (addMarker, addPolyline, addPolygon) from android-maps-ktx directly into android-maps-utils.

- Canonical Non-KTX Packages: Place all reactive Coroutine/Flow extensions and DSL builders in canonical com.google.maps.android.* packages.

- Deprecated Compatibility Layer: Preserve the legacy com.google.maps.android.ktx.* package structure with @deprecated(level = DeprecationLevel.WARNING, replaceWith = ReplaceWith(...)) forwarding wrappers and typealiases so existing imports compile seamlessly with deprecation warnings.

- Multi-Module Integration: Integrate KTX extensions across :library, :clustering, :heatmaps, and :data modules.

- Demo & Test Consolidation: Include KtxExtensionsDemoActivity in :demo and integrate all 18 KTX unit test suites with both canonical and shim test coverage.
@dkhawk
dkhawk force-pushed the feat/migrate-ktx-to-utils branch 3 times, most recently from d00d357 to f1a4793 Compare August 10, 2026 17:42
dkhawk added a commit that referenced this pull request Aug 20, 2026
…1716)

- Consolidate Kotlin Extensions (KTX into Utils): Move all reactive Coroutine/Flow extensions (awaitMap, mapClickEvents, cameraMoveEvents) and option builder DSLs (addMarker, addPolyline, addPolygon) from android-maps-ktx directly into android-maps-utils.

- Canonical Non-KTX Packages: Place all reactive Coroutine/Flow extensions and DSL builders in canonical com.google.maps.android.* packages.

- Deprecated Compatibility Layer: Preserve the legacy com.google.maps.android.ktx.* package structure with @deprecated(level = DeprecationLevel.WARNING, replaceWith = ReplaceWith(...)) forwarding wrappers and typealiases so existing imports compile seamlessly with deprecation warnings.

- Multi-Module Integration: Integrate KTX extensions across :library, :clustering, :heatmaps, and :data modules.

- Demo & Test Consolidation: Include KtxExtensionsDemoActivity in :demo and integrate all 18 KTX unit test suites with both canonical and shim test coverage.
@dkhawk
dkhawk force-pushed the feat/migrate-ktx-to-utils branch 2 times, most recently from 5cdce08 to 3ce966a Compare August 21, 2026 22:15
dkhawk added a commit that referenced this pull request Aug 31, 2026
…1716)

- Consolidate Kotlin Extensions (KTX into Utils): Move all reactive Coroutine/Flow extensions (awaitMap, mapClickEvents, cameraMoveEvents) and option builder DSLs (addMarker, addPolyline, addPolygon) from android-maps-ktx directly into android-maps-utils.

- Canonical Non-KTX Packages: Place all reactive Coroutine/Flow extensions and DSL builders in canonical com.google.maps.android.* packages.

- Deprecated Compatibility Layer: Preserve the legacy com.google.maps.android.ktx.* package structure with @deprecated(level = DeprecationLevel.WARNING, replaceWith = ReplaceWith(...)) forwarding wrappers and typealiases so existing imports compile seamlessly with deprecation warnings.

- Multi-Module Integration: Integrate KTX extensions across :library, :clustering, :heatmaps, and :data modules.

- Demo & Test Consolidation: Include KtxExtensionsDemoActivity in :demo and integrate all 18 KTX unit test suites with both canonical and shim test coverage.
@dkhawk
dkhawk force-pushed the feat/migrate-ktx-to-utils branch from 3ce966a to 5f6c90f Compare August 31, 2026 23:45
@dkhawk dkhawk changed the title feat: migrate android-maps-ktx into android-maps-utils (v6.0.0-rc01) feat: migrate android-maps-ktx into android-maps-utils (v6.0.0-rc03) Aug 31, 2026
@dkhawk
dkhawk requested a review from LoyalAbbas August 31, 2026 23:59
dkhawk added a commit that referenced this pull request Sep 4, 2026
…1716)

- Consolidate Kotlin Extensions (KTX into Utils): Move all reactive Coroutine/Flow extensions (awaitMap, mapClickEvents, cameraMoveEvents) and option builder DSLs (addMarker, addPolyline, addPolygon) from android-maps-ktx directly into android-maps-utils.

- Canonical Non-KTX Packages: Place all reactive Coroutine/Flow extensions and DSL builders in canonical com.google.maps.android.* packages.

- Deprecated Compatibility Layer: Preserve the legacy com.google.maps.android.ktx.* package structure with @deprecated(level = DeprecationLevel.WARNING, replaceWith = ReplaceWith(...)) forwarding wrappers and typealiases so existing imports compile seamlessly with deprecation warnings.

- Multi-Module Integration: Integrate KTX extensions across :library, :clustering, :heatmaps, and :data modules.

- Demo & Test Consolidation: Include KtxExtensionsDemoActivity in :demo and integrate all 18 KTX unit test suites with both canonical and shim test coverage.
@dkhawk
dkhawk force-pushed the feat/migrate-ktx-to-utils branch from 5f6c90f to a2164ec Compare September 4, 2026 17:10
…1716)

- Consolidate Kotlin Extensions (KTX into Utils): Move all reactive Coroutine/Flow extensions (awaitMap, mapClickEvents, cameraMoveEvents) and option builder DSLs (addMarker, addPolyline, addPolygon) from android-maps-ktx directly into android-maps-utils.

- Canonical Non-KTX Packages: Place all reactive Coroutine/Flow extensions and DSL builders in canonical com.google.maps.android.* packages.

- Deprecated Compatibility Layer: Preserve the legacy com.google.maps.android.ktx.* package structure with @deprecated(level = DeprecationLevel.WARNING, replaceWith = ReplaceWith(...)) forwarding wrappers and typealiases so existing imports compile seamlessly with deprecation warnings.

- Multi-Module Integration: Integrate KTX extensions across :library, :clustering, :heatmaps, and :data modules.

- Demo & Test Consolidation: Include KtxExtensionsDemoActivity in :demo and integrate all 18 KTX unit test suites with both canonical and shim test coverage.
…aps-ktx

- Add canonical Context.awaitMapsSdkInitialized(preferredRenderer) suspending extension in com.google.maps.android.
- Add deprecated backward-compatibility shim in com.google.maps.android.ktx.
- Add canonical and shim unit test suites for MapsInitializer coroutine extensions.
- Showcase awaitMapsSdkInitialized in KtxExtensionsDemoActivity and register demo in MainActivity.
- Update README.md documentation with awaitMapsSdkInitialized usage example.
@dkhawk
dkhawk requested a review from a team as a code owner September 14, 2026 20:46
@dkhawk
dkhawk requested a review from kikoso September 14, 2026 20:58
Comment thread .vscode/settings.json Outdated
Comment thread clustering/src/main/java/com/google/maps/android/ktx/utils/geometry/Point.kt Outdated
Comment thread library/src/main/java/com/google/maps/android/MapView.kt Outdated
Comment thread library/src/main/java/com/google/maps/android/location/LocationManager.kt Outdated
Comment thread library/src/main/java/com/google/maps/android/location/LocationManager.kt Outdated
Comment thread library/src/main/java/com/google/maps/android/location/FusedLocationProvider.kt Outdated
Comment thread library/src/main/java/com/google/maps/android/LatLng.kt Outdated
Comment thread library/src/test/java/com/google/maps/android/ktx/GoogleMapTest.kt Outdated
Comment thread library/src/main/java/com/google/maps/android/collections/MarkerManager.kt Outdated
Comment thread CHANGELOG.md
### Bug Fixes

* prevent StackOverflowError when parsing deeply nested KML containers and multi-geometries ([#1710](https://github.com/googlemaps/android-maps-utils/issues/1710)) ([1463cc5](https://github.com/googlemaps/android-maps-utils/commit/1463cc572da85b8a2317690fa94b0e2432995a96))
>>>>>>> origin/main

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any purpose for adding this ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge error, probably!

Comment thread build.gradle.kts
}
}
systemProperty("user.home", testHome.absolutePath)
environment("ANDROID_HOME", System.getenv("ANDROID_HOME") ?: "/usr/local/google/home/dkhawk/Android/Sdk")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this "/usr/local/google/home/dkhawk/Android/Sdk" path ?

public fun GoogleMap.cameraEvents(): Flow<CameraEvent> = this.canonicalCameraEvents()

@Deprecated("Moved to com.google.maps.android.awaitAnimateCamera", ReplaceWith("awaitAnimateCamera(cameraUpdate, durationMs)", "com.google.maps.android.awaitAnimateCamera"))
public suspend inline fun GoogleMap.awaitAnimateCamera(cameraUpdate: CameraUpdate, durationMs: Int = 3000): Unit = this.canonicalAwaitAnimateCamera(cameraUpdate, durationMs)

@LoyalAbbas LoyalAbbas Sep 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When calling awaitAnimateCamera(), we now need to pass 0 if we don't want to set a duration. Is there a reason we didn't make this parameter optional?

*/
public suspend inline fun GoogleMap.awaitAnimateCamera(
cameraUpdate: CameraUpdate,
durationMs: Int = 3000

@LoyalAbbas LoyalAbbas Sep 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When calling awaitAnimateCamera(), we now need to pass 0 if we don't want to set a duration. Is there a reason we didn't make this parameter optional ?

var callbackInvoked = false
val status = MapsInitializer.initialize(this, preferredRenderer) { renderer ->
callbackInvoked = true
continuation.resume(renderer)

@LoyalAbbas LoyalAbbas Sep 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If status != ConnectionResult.SUCCESS, the continuation resumes with an exception immediately. If Play Services later invokes the OnMapsSdkInitializedCallback asynchronously (or if the coroutine was cancelled while waiting), calling continuation.resume(renderer) without checking if (continuation.isActive), it might be throw IllegalStateException

What did you think on this ??

Fix: Guard resumption with if (continuation.isActive) continuation.resume(...), and add continuation.invokeOnCancellation { setOnMapLoadedCallback(null) } to awaitMapLoad()

LocationManager.PASSIVE_PROVIDER
}

requestLocationUpdates(provider, minTimeMs, minDistanceM, listener)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike FusedLocationProvider.kt(which defaults to looper: Looper = Looper.getMainLooper()), LocationManager.coarseLocationEvents and fineLocationEvents call the 4-argument requestLocationUpdates(provider, minTimeMs, minDistanceM, listener) without a Looper. If collected on Dispatchers.IO or Dispatchers.Default, LocationManager calls Looper.myLooper() and crashes immediately with RuntimeException: Can't create handler inside thread that has not called Looper.prepare().

Fix: Add looper: Looper = Looper.getMainLooper() parameter to coarseLocationEvents and fineLocationEvents and pass it to requestLocationUpdates(provider, minTimeMs, minDistanceM, listener, looper).

}
}

requestLocationUpdates(LocationManager.GPS_PROVIDER, minTimeMs, minDistanceM, listener)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike FusedLocationProvider.kt(which defaults to looper: Looper = Looper.getMainLooper()), LocationManager.coarseLocationEvents and fineLocationEvents call the 4-argument requestLocationUpdates(provider, minTimeMs, minDistanceM, listener) without a Looper. If collected on Dispatchers.IO or Dispatchers.Default, LocationManager calls Looper.myLooper() and crashes immediately with RuntimeException: Can't create handler inside thread that has not called Looper.prepare().

Fix: Add looper: Looper = Looper.getMainLooper() parameter to coarseLocationEvents and fineLocationEvents and pass it to requestLocationUpdates(provider, minTimeMs, minDistanceM, listener, looper).

* **How we know it is correct:** Asserts the emitted coordinates equal the exact `target` LatLng.
*/
@Test
public fun testMapClickEvents(): Unit = runTest {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently all test cases are false positive.

If we pass assertThat(event).isEqualTo(LatLng(9999.0, 9999.0)) or even if onMapClick(target) is never called at all, the test still pass, because the assertion line is dead code that gets cancelled before execution.

Solution :

Use async { ... } and deferred.await()
Using async instead of launch is cleaner, shorter, and eliminates manual job.cancel() calls altogether:

@test
public fun testMapClickEvents(): Unit = runTest {
val target = LatLng(10.0, 20.0)
val deferred = async {
googleMap.mapClickEvents().first()
}
advanceUntilIdle()
verify(googleMap).setOnMapClickListener(mapClickListener.capture())
mapClickListener.value.onMapClick(target)
assertThat(deferred.await()).isEqualTo(target)
}

Note : We need to do similar kind of handling for all the test cases

}

@Test
public fun testClusterInfoWindowClickEvents(): Unit = runTest {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently all test cases are false positive.

If we pass any cluster value still test will pass, because the assertion line is dead code that gets cancelled before execution.

Solution :

Use async { ... } and deferred.await()
Using async instead of launch is cleaner, shorter, and eliminates manual job.cancel() calls altogether:

@test
public fun testClusterInfoWindowClickEvents(): Unit = runTest {
val deferred = async {
clusterManager.clusterInfoWindowClickEvents().first()
}
advanceUntilIdle()
verify(clusterManager).setOnClusterInfoWindowClickListener(clusterInfoWindowClickListener.capture())
clusterInfoWindowClickListener.value.onClusterInfoWindowClick(cluster)
assertThat(deferred.await()).isEqualTo(cluster)
}

Note : We need to do similar kind of handling for all the test cases

}

@Test
public fun testMarkerCollectionClickEvents(): Unit = runTest {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test case is false positive.

If we pass any Marker value here, still this test will pass, because the assertion line is dead code that gets cancelled before execution.

Solution :

Use async { ... } and deferred.await()
Using async instead of launch is cleaner, shorter, and eliminates manual job.cancel() calls altogether:

@Test
public fun testMarkerCollectionClickEvents(): Unit = runTest {
    val deferred = async {
        markerCollection.clickEvents().first()
    }
    advanceUntilIdle()
    // Trigger the event via our tracked active listener slot!
    assertThat(activeMarkerClickListener).isNotNull()
    activeMarkerClickListener?.onMarkerClick(marker)

    assertThat(deferred.await()).isEqualTo(marker)
}

Note : We need to do similar kind of handling in other test cases also

Comment thread gradle/libs.versions.toml
espresso-core = { group = "androidx.test.espresso", name = "espresso-core", version.ref = "espresso-core" }
junit = { module = "junit:junit", version.ref = "junit" }
mockito-core = { module = "org.mockito:mockito-core", version.ref = "mockito-core" }
mockito-kotlin = { module = "com.nhaarman.mockitokotlin2:mockito-kotlin", version.ref = "mockito-kotlin" }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any specific reason, why we use this instead of "org.mockito.kotlin:mockito-kotlin" lib ?

Comment thread library/build.gradle.kts
dependsOn(generateArtifactIdFile)
}

tasks.named("dokkaGeneratePublicationHtml") {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate call

kikoso

This comment was marked as duplicate.

@kikoso kikoso left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second pass, this time anchored to the specific files. Most of it is packaging and release config rather than the Kotlin itself, plus one follow up on the inline thread.

* suppressing default SDK behavior (such as zooming). Under backpressure if the buffer is full,
* `trySend` returns `false`, allowing default SDK click handling to proceed.
*/
public fun <T : ClusterItem> ClusterManager<T>.clusterClickEvents(): Flow<Cluster<T>> =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These six functions are public and return Flow<...>, but clustering/build.gradle.kts still has implementation(libs.kotlinx.coroutines.android). The api(coroutines) you added to :library doesn't help either, since clustering depends on it with implementation.

So the published POM for android-maps-utils-clustering puts coroutines at runtime scope, and an app calling clusterClickEvents() gets "cannot access class kotlinx.coroutines.flow.Flow". Our unit tests won't catch it because they compile inside the module.

Can you move it to api(libs.kotlinx.coroutines.core) and check the other modules while you're there?

Comment thread library/build.gradle.kts
api(libs.play.services.maps)
implementation(libs.kotlinx.coroutines.android)
api(libs.play.services.location)
api(libs.kotlinx.coroutines.android)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only coroutines-core is actually in our API surface. coroutines-android is just the Dispatchers.Main artifact, so implementation is enough for it.

Separately on line 66: api(play-services-location) means everyone pulling maps-utils now gets play-services-location, including people who only wanted :data or :heatmaps. AGENTS.md also asks for an issue before adding deps to library modules. See my note on the demo file about where this package should live.

One more while this file is open: explicitApi() isn't enabled anywhere and we have no binary compatibility validator, yet we're adding roughly 200 public symbols in a major. All the new code already writes public explicitly so flipping explicitApi() on is nearly free, and an .api dump in version control would back up the compatibility promise the shim layer is making. Happy for that to be a follow up if you'd rather not grow this PR.

android:value="androidx.startup" />
<meta-data
android:name="com.google.maps.android.ktx.utils.attribution.AttributionIdInitializer"
android:value="androidx.startup" />

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This registers the attribution initializer a second time, so both it and the canonical one call addInternalUsageAttributionId with the same value on every app start.

The shim class is also internal, so nobody outside the module could ever have referenced it and the @Deprecated on it does nothing. I'd delete both the shim class and this manifest entry.

@@ -1,3 +1,3 @@
{
".": "5.2.0"
".": "6.0.0-rc04"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AGENTS.md says not to hand edit this or CHANGELOG.md. The title is already feat!: so release-please lands on 6.0.0 by itself. Can we revert both and let it do its job?

Also worth checking: the description says final 6.0.0, but release-please-config.json is untouched and still has "prerelease": true with "prerelease-type": "rc". As it stands the next run gives us 6.0.0-rc05, not 6.0.0.

Comment thread gradle.properties
# If true, publishToMavenCentral will also close and release the staging repository
mavenCentralAutomaticRelease=false

VERSION_NAME=6.0.0-rc04

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gradle.properties isn't in the extra-files list in release-please-config.json, so this goes stale on the next release. Do we need it at all? allprojects { version = ... } in the root build file is already marker managed.

(Minor: trailing blank lines at the end of the file too.)

* @param cameraUpdate the [CameraUpdate] to apply on the map
* @param durationMs the duration in milliseconds of the animation. Defaults to 3 seconds.
*/
public suspend inline fun GoogleMap.awaitAnimateCamera(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on the inline thread: most of them are gone, but three are still inline with no lambda parameter. This one, MapFragment.awaitMap() and SupportMapFragment.awaitMap(). Those files don't carry @file:Suppress("NOTHING_TO_INLINE") either, so they should be warning right now. Can you catch those too?

// 1. Canonical awaitMapsSdkInitialized() coroutine suspension
context.awaitMapsSdkInitialized(MapsInitializer.Renderer.LATEST)

mapView.onCreate(Bundle())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MapView lifecycle is driven by hand here (onCreate / onStart / onResume) and onPause, onStop, onDestroy and onLowMemory are never called, so the MapView and its native resources leak. This wants a DisposableEffect tied to the lifecycle.

Related on line 100: lifecycleScope.launch { repeatOnLifecycle { ... } } is nested inside the LaunchedEffect, so that inner coroutine outlives the composable leaving composition. Moving the repeatOnLifecycle into the LaunchedEffect body fixes it.

This is the sample the README points people at, so I'd rather it show the right pattern. (Also trailing whitespace on line 81.)

}

override fun onProviderDisabled(provider: String) {
close(CancellationException("Location provider $provider was disabled"))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This turns "user switched GPS off" into cancellation, which propagates into the collector's scope instead of being something they can catch and handle. Was that deliberate? A dedicated exception, or just completing the flow, would be easier to work with. Same on line 113.

Broader question on this whole package: should location live in :library at all, given it's what drags play-services-location onto every consumer? A separate :ktx module, or at least a :location one, might be the better home. This is the piece I'd most like settled before merge since it's baked into the 6.0.0 surface.

Comment thread gradle/libs.versions.toml
appcompat = "1.8.0"
core-ktx = "1.19.0"
kotlin = "2.4.10"
kotlin = "2.4.20"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kotlin 2.4.10 to 2.4.20, Gradle wrapper 9.6.1 to 9.7.1, Robolectric 4.16.1 to 4.17, Compose BOM, Navigation SDK, lint, plus a codeql-action SHA bump are all riding along in an 8.5k line migration.

Can those move to a separate build(deps): PR? They'll fight with Renovate, and they make this one harder to revert if we ever need to.

Comment thread build.gradle.kts
// {x-release-please-start-version}
version = "5.2.0"
version = "6.0.0-rc04"
// {x-release-please-end}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: stray blank line. Same kind of thing in MIGRATION.md line 22 and inside the code fence in .gemini/skills/android-maps-utils/SKILL.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants