diff --git a/backend/config.py b/backend/config.py index 4084e20879..40819a15fa 100644 --- a/backend/config.py +++ b/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 diff --git a/backend/services/users/user_service.py b/backend/services/users/user_service.py index 5ef2e48fbe..9eccc9e4e3 100644 --- a/backend/services/users/user_service.py +++ b/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" % ( diff --git a/example.env b/example.env index 93dceb5dfa..be7a1d84e5 100644 --- a/example.env +++ b/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. diff --git a/tests/api/integration/services/users/test_user_service.py b/tests/api/integration/services/users/test_user_service.py index 197c30771f..1a64b41ee0 100644 --- a/tests/api/integration/services/users/test_user_service.py +++ b/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 diff --git a/tests/api/unit/services/users/test_user_service.py b/tests/api/unit/services/users/test_user_service.py index 42cf03864f..265ce7f065 100644 --- a/tests/api/unit/services/users/test_user_service.py +++ b/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):