Fix Cesium entity ID collision bug (issue #136)#161
Conversation
- Replace len(self._layers) based ID generation with proper entity counter - Add _generate_entity_id() method that ensures unique IDs across all entity types - Fix applies to add_point, add_billboard, add_polyline, and add_polygon methods - Prevents 'An entity with id point_0 already exists in this collection' error - Entity counter increments globally ensuring no ID reuse even after entity removal Resolves #136
|
🚀 Deployed on https://697e27e171e11d48068cbcbe--opengeos.netlify.app |
There was a problem hiding this comment.
Pull request overview
This pull request fixes a critical bug in Cesium entity ID generation that caused "entity already exists" errors when adding multiple landmarks or entities to a Cesium map (issue #136).
Changes:
- Introduced a monotonically-incrementing
_entity_counterto ensure unique entity IDs - Added
_generate_entity_id()helper method for consistent ID generation across all entity types - Updated all entity creation methods (
add_point,add_billboard,add_polyline,add_polygon) to use the new ID generator
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _generate_entity_id(self, prefix: str) -> str: | ||
| """Generate a unique entity ID with the given prefix. | ||
|
|
||
| Args: | ||
| prefix: The prefix for the entity ID (e.g., 'point', 'billboard') | ||
|
|
||
| Returns: | ||
| A unique entity ID string | ||
| """ | ||
| entity_id = f"{prefix}_{self._entity_counter}" | ||
| self._entity_counter += 1 | ||
| return entity_id |
There was a problem hiding this comment.
This fix for entity ID uniqueness should include a corresponding test case. Both LeafletMap and OpenLayersMap have test_layer_id_generation tests (see tests/test_leaflet.py:271 and tests/test_openlayers.py:313) that verify ID uniqueness by adding multiple entities and confirming all IDs are unique and properly prefixed. Consider adding a similar test_entity_id_generation test to tests/test_anymap.py in the TestCesiumMap class that creates multiple entities of different types and verifies all generated IDs are unique and don't collide.
Problem
Fixes issue #136 where adding multiple landmarks/entities to a Cesium map results in the error:
An entity with id point_0 already exists in this collection.Root Cause
The existing code generates entity IDs using
len(self._layers)which causes ID collisions because:len(self._layers)can decrease and reuse previous IDsSolution
self._entity_counterthat only increments, never decreases_generate_entity_id(prefix)method that creates guaranteed unique IDsadd_point,add_billboard,add_polyline, andadd_polygonmethodsChanges
anymap/cesium.py:_entity_counter = 0in constructor_generate_entity_id(prefix)methodf"{type}_{len(self._layers)}"withself._generate_entity_id(type)in all entity creation methodsTesting
Verified the fix works correctly:
['point_0', 'point_1', 'billboard_2', 'polyline_3', 'polygon_4']Impact
Closes #136