Merge pull request #7194 from hotosm/hotfix/user-agent-osm-request

Author: ramyaragupathy

backend/config.py

@@ -280,6 +280,10 @@ def assemble_db_connection(
         "OHSOME_STATS_API_URL", "https://stats.now.ohsome.org/api"
     )
     OHSOME_STATS_TOPICS: str = os.getenv("OHSOME_STATS_TOPICS", None)
+    OSM_USER_AGENT: str = os.getenv(
+        "OSM_USER_AGENT",
+        "HOT-TaskingManager-API/5.0 (https://tasking-manager-production-api.hotosm.org)",
+    )
 
 
 class TestEnvironmentConfig(Settings):
@@ -297,6 +301,10 @@ class TestEnvironmentConfig(Settings):
     )
 
     LOG_LEVEL: str = "DEBUG"
+    OSM_USER_AGENT: str = os.getenv(
+        "OSM_USER_AGENT",
+        "HOT-TaskingManager-API/5.0 (https://tasking-manager-production-api.hotosm.org)",
+    )
 
 
 @lru_cache

backend/services/users/user_service.py

@@ -7,7 +7,6 @@
 from sqlalchemy import and_, desc, distinct, func, insert, select
 from httpx import AsyncClient
 
-from backend.config import Settings
 from backend.exceptions import NotFound
 from backend.models.dtos.interests_dto import InterestDTO, InterestsListDTO
 from backend.models.dtos.project_dto import ProjectFavoritesDTO, ProjectSearchResultsDTO
@@ -50,9 +49,7 @@
 from backend.services.users.osm_service import OSMService
 from backend.services.mapping_levels import MappingLevelService
 from fastapi import HTTPException
-
-
-settings = Settings()
+from backend.config import settings
 
 
 class UserServiceError(Exception):
@@ -181,12 +178,16 @@ async def get_and_save_stats(user_id: int, db: Database) -> dict:
         osm_user_details_url = f"{settings.OSM_SERVER_URL}/api/0.6/user/{user_id}.json"
 
         oh_some_headers = {"Authorization": f"Basic {settings.OHSOME_STATS_TOKEN}"}
+        osm_headers = {"User-Agent": settings.OSM_USER_AGENT}
 
         async with AsyncClient(timeout=10.0) as client:
             oh_some_response = await client.get(oh_some_url, headers=oh_some_headers)
-            changeset_response = await client.get(osm_user_details_url)
+            changeset_response = await client.get(
+                osm_user_details_url, headers=osm_headers
+            )
 
         if oh_some_response.status_code != 200:
+
             error_msg = (
                 "External-Error in Ohsome API: url=%s status_code=%s response=%s"
                 % (

example.env

@@ -38,6 +38,7 @@ OSM_SERVER_URL=${OSM_SERVER_URL:-https://www.openstreetmap.org}
 OSM_SERVER_API_URL=${OSM_SERVER_API_URL:-https://api.openstreetmap.org}
 OSM_NOMINATIM_SERVER_URL=${OSM_NOMINATIM_SERVER_URL:-https://nominatim.openstreetmap.org}
 OSM_REGISTER_URL=${OSM_REGISTER_URL:-https://www.openstreetmap.org/user/new}
+OSM_USER_AGENT=${OSM_USER_AGENT:-HOT-TaskingManager}
 
 # Information about the Editor URLs. Those are the default values on the frontend.
 # You only need to modify it in case you want to direct users to map on a different OSM instance.

tests/api/integration/services/users/test_user_service.py

@@ -12,6 +12,8 @@
     return_canned_user,
     create_mapping_levels,
 )
+from httpx import AsyncClient
+from backend.config import test_settings as settings
 
 
 @pytest.mark.anyio
@@ -143,3 +145,14 @@ async def test_register_user_creates_new_user(self):
         user = await UserService.get_user_by_id(canned.id, db=self.db)
         assert user.username == canned.username
         assert user.mapping_level == 1
+
+    async def test_osm_user_endpoint_not_rate_limited(self):
+        url = "https://www.openstreetmap.org/api/0.6/user/490556.json"
+
+        async with AsyncClient(timeout=10.0) as client:
+            response = await client.get(
+                url,
+                headers={"User-Agent": settings.OSM_USER_AGENT},
+            )
+        assert response.status_code == 200
+        assert response.status_code != 429

tests/api/unit/services/users/test_user_service.py

@@ -137,24 +137,45 @@ async def test_get_and_save_stats(self, mock_get):
         stats = await UserStats.get_for_user(self.test_user.id, self.db)
         assert stats.stats == '{"changeset": 251.0}'
 
-    @patch.object(AsyncClient, "get")
-    async def test_get_and_save_stats_error(self, mock_get):
-        # Arrange
-        mock_response = AsyncMock()
-        mock_response.status_code = 500
-        mock_response.json = MagicMock(
+    async def test_get_and_save_stats_handles_ohsome_500(self):
+        # Arrange: prepare an OHsome error response (500) and a harmless OSM response (200)
+        ohsome_resp = AsyncMock()
+        ohsome_resp.status_code = 500
+        ohsome_resp.text = "Internal Server Error"
+        ohsome_resp.json = MagicMock(
             return_value={
                 "status": 500,
                 "error": "Internal Server Error",
                 "path": "/api/stats/user",
-            },
+            }
         )
-        mock_get.return_value = mock_response
 
-        # Assert
-        with pytest.raises(UserServiceError):
+        changeset_resp = AsyncMock()
+        changeset_resp.status_code = 200
+        changeset_resp.json = MagicMock(
+            return_value={"user": {"changesets": {"count": 0}}}
+        )
+
+        # Patch AsyncClient.get and UserStats.update
+        with (
+            patch.object(AsyncClient, "get", new_callable=AsyncMock) as mock_get,
+            patch.object(UserStats, "update", new_callable=AsyncMock) as mock_update,
+        ):
+            # The function does two gets in sequence: ohsome then changeset
+            mock_get.side_effect = [ohsome_resp, changeset_resp]
+
             # Act
-            await UserService.get_and_save_stats(self.test_user.id, self.db)
+            result = await UserService.get_and_save_stats(self.test_user.id, self.db)
+
+            # Assert
+            # function should return empty dict on OHsome 500
+            assert result == {}
+
+            # Ensure we tried the external calls (at least awaited once)
+            mock_get.assert_awaited()
+
+            # And we must NOT call UserStats.update when an upstream failed
+            mock_update.assert_not_awaited()
 
     @patch.object(AsyncClient, "get")
     async def test_check_and_update_mapper_level_happy_path(self, mock_get):