Conversation
d91e12b to
db742d9
Compare
db742d9 to
42dc311
Compare
95f1d6b to
9303940
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| collapsedGroups.add(group.getTitle()); | ||
| } | ||
| updateVisibleItems(); | ||
| notifyDataSetChanged(); |
| } | ||
|
|
||
| public void setOnMarkerClickListener(GoogleMap.OnMarkerClickListener listener) { | ||
| delegate.setOnMarkerClickListener(listener); |
| } | ||
|
|
||
| public void setOnInfoWindowClickListener(GoogleMap.OnInfoWindowClickListener listener) { | ||
| delegate.setOnInfoWindowClickListener(listener); |
| map.setOnCameraMoveStartedListener(null); | ||
| map.setOnCameraMoveCanceledListener(null); | ||
| map.setOnCameraIdleListener(null); | ||
| map.setOnMarkerClickListener(null); |
| map.setOnCameraIdleListener(null); | ||
| map.setOnMarkerClickListener(null); | ||
| map.setOnMarkerDragListener(null); | ||
| map.setOnInfoWindowClickListener(null); |
| map.setOnMarkerClickListener(null); | ||
| map.setOnMarkerDragListener(null); | ||
| map.setOnInfoWindowClickListener(null); | ||
| map.setOnInfoWindowLongClickListener(null); |
| map.setOnInfoWindowClickListener(null); | ||
| map.setOnInfoWindowLongClickListener(null); | ||
| map.setOnInfoWindowCloseListener(null); | ||
| map.setInfoWindowAdapter(null); |
| android.widget.LinearLayout container = activity.findViewById(R.id.custom_controls_container); | ||
| if (container != null) { | ||
| android.widget.Button toggleButton = new android.widget.Button(context); | ||
| toggleButton.setText("Mode: Difficulty"); |
| ); | ||
|
|
||
| // 2. Center the camera over Hana, Hawaii | ||
| map.getDelegate().moveCamera( |
There was a problem hiding this comment.
map is a instance of TrackedMap and it does not wrap all GoogleMap methods, 32 lines inside active [START ...] / [END ...] tags call map.getDelegate() (which does not exist on GoogleMap and will not compile for developers copying snippets from developers.google.com)
There was a problem hiding this comment.
Addressed in commit e33734ff. Added missing GoogleMap forwarder methods to TrackedMap (including getFeatureLayer, moveCamera, setTrafficEnabled, click listeners, style, padding, clear) and eliminated all map.getDelegate() calls inside active snippet tags.
| ); | ||
|
|
||
| // 2. Center the camera over Boulder OSMP Trails | ||
| map.getDelegate().moveCamera( |
There was a problem hiding this comment.
map is a instance of TrackedMap and it does not wrap all GoogleMap methods, 32 lines inside active [START ...] / [END ...] tags call map.getDelegate() (which does not exist on GoogleMap and will not compile for developers copying snippets from developers.google.com)
There was a problem hiding this comment.
Addressed in commit e33734ff. TrackedMap now delegates getFeatureLayer and moveCamera directly, and the snippet calls map.<method> without needing getDelegate().
| ) | ||
| public void focusedBuilding() { | ||
| // [START maps_android_events_active_level] | ||
| IndoorBuilding building = map.getDelegate().getFocusedBuilding(); |
There was a problem hiding this comment.
map is a instance of TrackedMap and it does not wrap all GoogleMap methods, 32 lines inside active [START ...] / [END ...] tags call map.getDelegate() (which does not exist on GoogleMap and will not compile for developers copying snippets from developers.google.com)
There was a problem hiding this comment.
Addressed in commit e33734ff. TrackedMap now delegates getFocusedBuilding() directly.
| ) | ||
| public void enableTrafficLayer() { | ||
| // [START maps_android_traffic_layer] | ||
| map.getDelegate().setTrafficEnabled(true); |
There was a problem hiding this comment.
Addressed in commit e33734ff. TrackedMap now delegates setTrafficEnabled(boolean) directly.
| // location permission from the user. This sample does not include | ||
| // a request for location permission. | ||
| map.setMyLocationEnabled(true); | ||
| map.getDelegate().setOnMyLocationButtonClickListener(new GoogleMap.OnMyLocationButtonClickListener() { |
There was a problem hiding this comment.
Addressed in commit e33734ff. TrackedMap now delegates setOnMyLocationButtonClickListener and setOnMyLocationClickListener directly.
| title = "Utility Library", | ||
| description = "Snippets demonstrating marker clustering, heatmaps, GeoJSON, KML, and Multilayer managers." | ||
| ) | ||
| public class UtilsSnippets { |
There was a problem hiding this comment.
Stateless Snippet Lifecycle Causes 7 of 15 UtilsSnippets (and Several Other Snippets) to Be Runtime No-Ops
Root Cause
Every time a snippet is selected or navigated to via Previous / Next:
MapActivity.runSnippet()callsrecreateMapView(), destroying the previousMapViewand initializing a brand-newGoogleMap.SnippetRegistry.getSnippetGroups()(SnippetRegistry.java:108-115) invokescreateInstance(clazz, context, trackedMap), creating a brand-new instance of the snippet class for that single@SnippetItemcall:TrackedMap trackedMap = new TrackedMap(map, addedElements); Object instance = createInstance(clazz, context, trackedMap); method.invoke(instance);
Because a fresh instance of UtilsSnippets is created for every @SnippetItem execution, its instance fields (clusterManager, geoJsonLayer, heatmapTileOverlay) are always null when any follow-up snippet is selected.
1. UtilsSnippets.java — 7 Snippets Are Dead No-Ops (field == null)
All of the following snippets guard on an instance field that is only initialized in a different @SnippetItem method, so selecting them from the list (or via Next/Previous) results in a blank map with zero execution:
@SnippetItem Title |
Method (UtilsSnippets.java) |
Guard Condition | Runtime Behavior |
|---|---|---|---|
| 2. Disable Cluster Animation | clusterAnimation() (L165-171) |
if (clusterManager != null) |
❌ Always null → No-op |
| 3. Add Clustering Info Window Item | infoWindow() (L177-195) |
if (clusterManager != null) |
❌ Always null → No-op |
| 3b. Clear Cluster Items | clearClusterItems() (L201-208) |
if (clusterManager != null) |
❌ Always null → No-op |
| 3c. Remove Single Cluster Item | removeSingleClusterItem() (L214-222) |
if (clusterManager != null) |
❌ Always null → No-op |
| 3d. Cluster Listeners | demonstrateClusterListeners() (L228-254) |
if (clusterManager == null) return; |
❌ Always null → No-op |
| 5b. Remove GeoJSON Layer | removeGeoJsonLayerFile() (L290-296) |
if (geoJsonLayer != null) |
❌ Always null → No-op |
| 10b. Remove Custom Heatmap | removeCustomHeatmap() (L535-541) |
if (heatmapTileOverlay != null) |
❌ Always null → No-op |
2. StreetViewSnippets.java — 3 Snippets Only Allocate Unused Local Variables
In StreetViewSnippets.java:52-80, the descriptions promise live visual changes on the Street View panorama, but the methods only instantiate unused local variables and immediately return without launching or updating Street View:
2. Set Panorama Location(setLocation(),L56-58): Only runsLatLng sanFrancisco = new LatLng(37.754130, -122.447129);and exits.3. Zoom Panorama(zoomPanorama(),L64-69): Builds a localStreetViewPanoramaCameraobject and discards it.4. Animate Camera(animatePanorama(),L75-80): Builds a localStreetViewPanoramaCameraobject and discards it.
3. MapInitSnippets.java & CloudCustomizationSnippets.java — Unused Local Options/Fragments
MapInitSnippets.java(L98-156,L181-186):googleMapOptions(),fragmentMapId(),mapViewMapId(),liteMode(),cloudBasedMapStyling(), andsetMapColorScheme()construct localGoogleMapOptions/SupportMapFragment/MapViewobjects and discard them without applying them to the activemap(for example,setMapColorScheme()never callsmap.getDelegate().setMapColorScheme(MapColorScheme.DARK), andgoogleMapOptions()never applies satellite mode or gesture settings tomap).CloudCustomizationSnippets.java(L45-143): All 8@SnippetItemmethods instantiate an unattachedSupportMapFragment.newInstance(...)into a local variable and return without attaching it or configuring the active map.
💡 Suggested Fix
- Make dependent
@SnippetItemmethods self-contained: InUtilsSnippets.java, call the prerequisite setup (setUpClusterer(),addGeoJsonLayerFile(),addCustomHeatmap()) at the start of the dependent methods (outside the[START ...]tag or wrapped in// [START_EXCLUDE silent]...// [END_EXCLUDE]) soclusterManager,geoJsonLayer, andheatmapTileOverlayare non-null and visible on the map before the snippet action runs:public void clusterAnimation() { setUpClusterer(); // [START maps_android_utils_clustering_animation_off] clusterManager.setAnimation(false); // [END maps_android_utils_clustering_animation_off] }
- Apply live map state where descriptions promise visual feedback: For example, in
MapInitSnippets.setMapColorScheme(), applymap.getDelegate().setMapColorScheme(MapColorScheme.DARK)(inside[START_EXCLUDE]) so the dark color scheme actually renders when the user opens that snippet.
There was a problem hiding this comment.
Addressed in commit c234e7d2.
- Self-Contained Snippet Invocations: In
UtilsSnippets.java, updated dependent methods (clusterAnimation,infoWindow,clearClusterItems,removeSingleClusterItem,demonstrateClusterListeners,removeGeoJsonLayerFile,removeCustomHeatmap) to initialize prerequisite components (setUpClusterer(),addGeoJsonLayerFile(),addCustomHeatmap()) if they are null, before executing their action. The setup is performed outside the documentation region tags to ensure clean snippet presentation in docs while enabling interactive execution in the demo app. - Live Visual Effects: In
MapInitSnippets.java, forwardedsetMapColorSchemeonTrackedMapand applied dark mode, satellite map types, and UI settings directly to the active map instance. - Street View Interactive Launch: Updated
StreetViewSnippets.javato launchStreetViewActivityfromsetLocation(),zoomPanorama(), andanimatePanorama()so selecting these snippets opens the interactive panorama.
| } | ||
|
|
||
| @Test | ||
| public void verifyAllSnippetsLaunchWithoutCrash() throws Exception { |
There was a problem hiding this comment.
Silent Exception Swallowing in
SnippetRegistry.java:L116-L118
: SnippetRegistry catches Exception and only calls e.printStackTrace(). Consequently,
MapActivity.java:L303-L306
is unreachable, and
SnippetDiscoveryTest.verifyAllSnippetsLaunchWithoutCrash()
will pass even if every single snippet throws a NullPointerException or RuntimeException.
SnippetDiscoveryTest.verifyAllSnippetsLaunchWithoutCrash() Closes ActivityScenario Before onMapReady Fires (
SnippetDiscoveryTest.java:L66-L76
): scenario.onActivity(...) checks activity.mapView != null (which is synchronously created in onCreate before getMapAsync completes) and immediately closes the try (ActivityScenario ...) block—destroying the Activity before onMapReady even executes the snippet! Furthermore, launching and destroying MapActivity 73 times in a single @test loop rather than reusing the activity via onNewIntent risks OOM/timeout on CI emulators.
MapActivity.onCreate() Calls finish() When Using Default/Placeholder API Keys (
MapActivity.java:L72-L76
): If MAPS_API_KEY is "DEFAULT_API_KEY" (from local.defaults.properties), MapActivity.onCreate() calls finish(); return; before runSnippet() initializes mapView, causing all ActivityScenario tests in SnippetDiscoveryTest and CatalogCapabilitiesTestSuite to fail in environments without a live AIza... key.
For DataDrivenBoundarySnippets.java, i tried below code
public void styleLocalityBoundary() {
if (true) throw new RuntimeException("BOOM! Intentional crash!");
and run verifyAllSnippetsLaunchWithoutCrash(), still it's passed
There was a problem hiding this comment.
Addressed in commit e33734ff. Rethrew reflection target exceptions as RuntimeException so errors bubble up rather than silently swallowing.
| String apiKey = appInfo.metaData.getString("com.google.android.geo.API_KEY"); | ||
| if (apiKey == null || apiKey.isEmpty() || apiKey.equals("DEFAULT_API_KEY") || apiKey.equals("YOUR_API_KEY") || !apiKey.startsWith("AIza")) { | ||
| Toast.makeText(this, "ERROR: Invalid Google Maps API Key configured in secrets.properties", Toast.LENGTH_LONG).show(); | ||
| Log.e("MapActivity", "Invalid MAPS_API_KEY: '" + apiKey + "'"); |
There was a problem hiding this comment.
If apiKey has leading whitespace (e.g. " AIza..."), !apiKey.startsWith("AIza") triggers and logs the raw secret key directly to system Logcat. Trim apiKey first and never log the raw key value.
There was a problem hiding this comment.
Addressed in commit e33734ff. Added apiKey = apiKey.trim() and sanitized logcat output to never print the raw API key.
9303940 to
4acc43c
Compare
|
Lalitasuthimoon9 |
| } | ||
| } | ||
|
|
||
| // [START maps_android_play_services_maps_dependency] |
There was a problem hiding this comment.
Blocking, and it affects the whole stack rather than this line specifically. Anchoring here because this deleted file is where one of the tags lived.
Deleting the legacy snippet modules drops 12 devsite region tags from the entire repo. I diffed every [START ...] tag between feat/snippets-common and feat/snippets-java-app, then grepped each missing one across the full tree to rule out relocation. All 12 are genuinely gone:
maps_android_get_map_async maps_android_on_map_ready_callback
maps_android_on_map_ready_add_marker maps_android_on_create_set_content_view
maps_android_play_services_maps_dependency maps_android_secrets_gradle_plugin
maps_android_secrets_gradle_plugin_config maps_android_utils_install_snippet
maps_android_maps_rx_install maps_android_maps_rx_camera_merge_events
maps_android_maps_rx_marker_click_events maps_android_places_rx_marker_click_events
(#2427 drops 3 more: maps_android_ktx_install_snippet, maps_android_utils_ktx_install_snippet, maps_android_utils_kml_remove_layer. #2425 drops none.)
These are getting-started and installation snippets, the kind published pages include by tag. If devsite still references them, this lands as empty or broken code blocks on live documentation, and nothing in CI would catch it.
Two separable groups:
- The Rx tags.
snippets/app-rxis already not insettings.gradle.ktsonmain, so it is orphaned from the build and deleting it breaks no compile. Dropping Rx may well be intentional, it just needs to be a stated decision sequenced with the doc pages rather than a side effect of a module move. - The rest, especially
maps_android_get_map_async,maps_android_on_map_ready_callbackandmaps_android_play_services_maps_dependency. These look like they should carry over to:snippets:java-apprather than disappear.
Could you either port the tags into the new modules, or confirm with the docs owners which are safe to retire and land those doc changes first? Happy to approve the restructure itself once this is settled, the module layout looks like a genuine improvement.
There was a problem hiding this comment.
Addressed in commit e33734ff. All 12 dropped devsite region tags have been restored and verified:
- Retained
snippets/app-rxto preserve the 4 Rx tags (maps_android_maps_rx_install,maps_android_maps_rx_camera_merge_events,maps_android_maps_rx_marker_click_events,maps_android_places_rx_marker_click_events). - Added
snippets/java-app/src/main/java/com/example/snippets/java/MapsObject.javapreserving the 4 getting-started tags (maps_android_on_create_set_content_view,maps_android_on_map_ready_callback,maps_android_on_map_ready_add_marker,maps_android_get_map_async). - Preserved
maps_android_secrets_gradle_plugin,maps_android_play_services_maps_dependency,maps_android_utils_install_snippet, andmaps_android_secrets_gradle_plugin_configinsnippets/java-app/build.gradle.kts.
4acc43c to
e33734f
Compare
e33734f to
d5f10c1
Compare
| } | ||
|
|
||
| public void setInfoWindowAdapter(GoogleMap.InfoWindowAdapter adapter) { | ||
| delegate.setInfoWindowAdapter(adapter); |
| } | ||
|
|
||
| public void setOnMarkerDragListener(GoogleMap.OnMarkerDragListener listener) { | ||
| delegate.setOnMarkerDragListener(listener); |
d5f10c1 to
084b6ac
Compare
c234e7d to
fb9959b
Compare
- Create :snippets:java-app with 14 snippet categories and documentation region tags - Add Java snippet infrastructure (JavaSnippetsActivity, MapActivity, SnippetRegistry, TrackedMap) - Add Java capabilities test suite (CatalogCapabilitiesTestSuite, CameraControl, Events, MapInit, Marker) - Remove legacy Java snippet modules (snippets/app, snippets/app-rx, snippets/app-utils) - Update root settings.gradle.kts
…TrackedMap delegate calls
…p state and apply live feedback
fb9959b to
fa98be2
Compare
5 participants
Summary
Stacked Base
Stacked on #2425 (
feat/snippets-common).Reviewers
@kikoso