Conversation
d91e12b to
db742d9
Compare
db742d9 to
42dc311
Compare
95f1d6b to
9303940
Compare
| 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.
There was a problem hiding this comment.
Addressed in commit fa98be2e.
- Self-contained snippet execution: Updated
UtilsSnippets.javaso that dependent methods (clusterAnimation(),infoWindow(),clearClusterItems(),removeSingleClusterItem(),demonstrateClusterListeners(),removeGeoJsonLayerFile(),removeCustomHeatmap()) verify and run prerequisite setup (setUpClusterer(),addGeoJsonLayerFile(),addCustomHeatmap()) if uninitialized, ensuring every snippet executes predictably regardless of navigation order. - Live feedback on map & Street View:
- In
MapInitSnippets.java, updatedsetMapColorScheme()to invokemap.setMapColorScheme(...)on the active map, and applied satellite mode / UI settings tomapingoogleMapOptions(). - In
StreetViewSnippets.java, launchedStreetViewActivityfromsetLocation(),zoomPanorama(), andanimatePanorama()for immediate live feedback. - Forwarded
setMapColorScheme()andgetMapColorScheme()throughTrackedMap.java.
- In
| } | ||
|
|
||
| @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
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
|
|
||
| // Initialize the manager with the context and the map. | ||
| // (Activity extends context, so we can pass 'this' in the constructor.) | ||
| clusterManager = new ClusterManager<MyItem>(context, map.getDelegate()); |
There was a problem hiding this comment.
This file still contains 11 map.getDelegate() calls inside 7 active [START ...] / [END ...] tags (L131, L277, L287, L390, L403, L566–569, L573–575). Root Cause: Adding forwarder methods to TrackedMap fixed the other snippet classes, but ClusterManager, GeoJsonLayer, KmlLayer, and *Manager constructors in android-maps-utils require a real GoogleMap parameter (and GoogleMap is final, so TrackedMap cannot subclass it). Impact: developers.google.com will render map.getDelegate() verbatim in all 7 Utility Library docs snippets, which fails to compile against GoogleMap. Fix: Unwrap GoogleMap map = this.map.getDelegate(); outside the [START ...] tags (or hold private final GoogleMap map; alongside TrackedMap in UtilsSnippets) so the tagged blocks pass map directly.
| // [END maps_android_utils_kml_add_input_stream] | ||
|
|
||
| // [START maps_android_utils_kml_add_layer] | ||
| layer.addLayerToMap(); |
There was a problem hiding this comment.
maps_android_utils_kml_remove_layer is still missing from UtilsSnippets.java (and the 4 MapsObject tags are missing from Kotlin). Root Cause: The earlier region-tag diff grepped tag names repo-wide rather than per language (.java vs .kt). Because app-utils-ktx/KML.kt still existed on this branch, the script missed that deleting app-utils/KML.java dropped maps_android_utils_kml_remove_layer from Java (which was later restored only in Kotlin's UtilsSnippets.kt:L400 on #2427); conversely, restoring MapsObject.java here hid the fact that MapsObject.kt's 4 getting-started tags were deleted here and never restored in #2427. Impact: The Java tab on the KML docs page (maps_android_utils_kml_remove_layer) and the Kotlin tab on the Getting Started docs page (maps_android_on_map_ready_callback, etc.) will break on developers.google.com. Fix: Add removeKmlLayer() with // [START maps_android_utils_kml_remove_layer] to UtilsSnippets.java (matching UtilsSnippets.kt in #2427), and port MapsObject.kt into :snippets:kotlin-app.
| if (context instanceof android.app.Activity) { | ||
| MapView mapView = null; | ||
| if (context instanceof com.example.snippets.java.MapActivity) { | ||
| mapView = ((com.example.snippets.java.MapActivity) context).mapView; |
There was a problem hiding this comment.
Internal MapActivity / R.id.map_view_holder test-harness lookup code is inside [START maps_android_events_disable_clicks_mapview] without [START_EXCLUDE]. Root Cause: To satisfy EventsSnippetsTest, 15 lines of sample-app-specific view lookup (context instanceof com.example.snippets.java.MapActivity, com.example.snippets.common.R.id.map_view_holder) were placed directly inside the [START]/[END] block (and similarly com.example.snippets.common.R.id.map_container in MapInitSnippets.java:L70–78). Impact: developers.google.com mirrors tagged blocks verbatim, so external developers reading the docs will see internal com.example.snippets.* harness classes instead of the 2-line MapView snippet. Fix: Move the MapActivity lookup before [START ...] (or wrap it in // [START_EXCLUDE silent] ... // [END_EXCLUDE]) so only mapView.setClickable(false); is published.
| Marker marker = map.addMarker(new MarkerOptions() | ||
| .position(sydney) | ||
| .title("Marker in Sydney")); | ||
| if (marker != null) { |
There was a problem hiding this comment.
Unused getter variable dumps (MarkerSnippets:L59–69, MapInitSnippets:L89–90, OverlaySnippets:L64–73, L164–169, ShapesSnippets:L75–83, L148–158, L175–183) are inside published [START ...] tags. Root Cause: These dead local-variable reads (isClickable, isVisible, zIndex, setIndoorEnabled, etc.) were added inside core documentation tags (maps_android_markers_add_a_marker, maps_android_map_type, etc.) to satisfy catalog_api.py bytecode line-range checks. Impact: Because developers.google.com renders [START ...] / [END ...] blocks verbatim, introductory docs pages (like "Add a marker" and "Change map type") will display 10 lines of unused local getter reads. Fix: Remove the unused getter blocks or wrap them in // [START_EXCLUDE silent] ... // [END_EXCLUDE] (which hides them from DevSite while still keeping them within [START]/[END] for catalog_api.py).
| // Snippets | ||
| include(":snippets:common") | ||
| project(":snippets:common").projectDir = file("snippets/common") | ||
| include(":snippets:app") |
There was a problem hiding this comment.
Need to remove ":snippets:app" and ":snippets:app-utils" from scripts/verify_all.sh file also
| title = "2. Set Panorama Location", | ||
| description = "What it does: Sets the panorama view geographic coordinates, search radius, and outdoor source filter.\nHow to see the effect: The Street View camera jumps directly to target coordinates." | ||
| ) | ||
| public void setLocation() { |
There was a problem hiding this comment.
setLocation(), zoomPanorama(), and animatePanorama() still discard their local LatLng/StreetViewPanoramaCamera objects and all open the exact same default StreetViewActivity. Root Cause: Commit fa98be2 appended context.startActivity(new Intent(context, StreetViewActivity.class)) to each method without passing any Intent action/extra, and StreetViewActivity's setLocationOfThePanorama(), zoom(), pan(), tilt(), and animate() methods (StreetViewActivity.java:L65–137) are private and never invoked from onStreetViewPanoramaReady(). Fix: Pass an Intent extra specifying the requested Street View action and invoke the corresponding helper method in StreetViewActivity.onStreetViewPanoramaReady().
| } | ||
|
|
||
| if (!items.isEmpty()) { | ||
| items.sort(Comparator.comparing(SnippetItemInfo::getTitle)); |
There was a problem hiding this comment.
Sorts numbered snippets lexicographically (1, 10, 11, ..., 2, 3) in the UI and Next/Previous navigation. Root Cause: String comparison orders "10. ..." before "2. ..." across MarkerSnippets (1–14), MapInitSnippets (1–11), ShapesSnippets (1–11), and UtilsSnippets (1–11), unlike catalog_api.py:L220 which parses the leading integer prefix. Fix: Sort by the parsed leading number (with secondary lexicographical tie-breaker on the full title) so the in-app list and Next/Previous buttons follow numerical order.
| title = "2. Map Fragment Transaction", | ||
| description = "What it does: Dynamically adds a SupportMapFragment into the Activity view hierarchy programmatically.\nHow to see the effect: A new map fragment view is instantiated and rendered into the container layout frame." | ||
| ) | ||
| public void mapFragment() { |
There was a problem hiding this comment.
Running 2. Map Fragment Transaction permanently obscures MapActivity's MapView for all subsequent snippets. Root Cause: mapFragment() commits a SupportMapFragment into R.id.map_container (the root FrameLayout), but MapActivity.runSnippet() only recreates the child MapView inside R.id.map_view_holder and never removes fragments from SupportFragmentManager. Navigating to any other snippet via Next / Previous leaves the SupportMapFragment stacked on top of map_view_holder, hiding all subsequent snippet output. Fix: Remove any attached SupportMapFragment instances in MapActivity.runSnippet() when resetting the view hierarchy.
Summary
Stacked Base
Stacked on #2425 (
feat/snippets-common).Reviewers
@kikoso