diff --git a/pecha_api/search/search_response_models.py b/pecha_api/search/search_response_models.py index 8c7792e8..777614e7 100644 --- a/pecha_api/search/search_response_models.py +++ b/pecha_api/search/search_response_models.py @@ -77,3 +77,8 @@ class MultilingualSearchResponse(BaseModel): skip: int limit: int total: int + + +class SegmentLinkResponse(BaseModel): + text_id: str + segment_id: str diff --git a/pecha_api/search/search_service.py b/pecha_api/search/search_service.py index 22701fa9..204a5e08 100644 --- a/pecha_api/search/search_service.py +++ b/pecha_api/search/search_service.py @@ -1,5 +1,8 @@ from elastic_transport import ObjectApiResponse -from pecha_api.plans.response_message import NO_SEGMENTATION_IDS_RETURNED, PECHA_SEGMENT_NOT_FOUND +from fastapi import HTTPException +from starlette import status + +from pecha_api.plans.response_message import NO_SEGMENTATION_IDS_RETURNED from .search_enums import SearchType from .search_client import search_client from pecha_api.config import get @@ -20,7 +23,8 @@ ExternalSearchResponse, MultilingualSegmentMatch, MultilingualSourceResult, - MultilingualSearchResponse + MultilingualSearchResponse, + SegmentLinkResponse, ) logger = logging.getLogger(__name__) @@ -446,17 +450,20 @@ async def build_multilingual_sources(segments: List[Segment], results_map: Dict[ return sources -async def get_url_link(pecha_segment_id: str) -> str: - +async def get_url_link(pecha_segment_id: str) -> SegmentLinkResponse: try: segment = await Segment.get_segment_by_pecha_segment_id(pecha_segment_id=pecha_segment_id) - + if not segment: - return PECHA_SEGMENT_NOT_FOUND - - url = f"/chapter?text_id={segment.text_id}&segment_id={str(segment.id)}" - return url - + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Pecha segment not found") + + return SegmentLinkResponse( + text_id=segment.text_id, + segment_id=str(segment.id), + ) + + except HTTPException: + raise except Exception as e: logger.error(f"Error generating URL for pecha_segment_id {pecha_segment_id}: {str(e)}", exc_info=True) - return "" \ No newline at end of file + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Failed to retrieve segment link") \ No newline at end of file diff --git a/pecha_api/search/search_views.py b/pecha_api/search/search_views.py index 1597a068..592e5646 100644 --- a/pecha_api/search/search_views.py +++ b/pecha_api/search/search_views.py @@ -12,7 +12,8 @@ from .search_response_models import ( SearchResponse, - MultilingualSearchResponse + MultilingualSearchResponse, + SegmentLinkResponse, ) search_router = APIRouter( @@ -55,5 +56,5 @@ async def multilingual_search( ) @search_router.get("/chat/{pecha_segment_id}", status_code=status.HTTP_200_OK) -async def get_url_link(pecha_segment_id: str) -> str: +async def get_url_link(pecha_segment_id: str) -> SegmentLinkResponse: return await get_url_link_service(pecha_segment_id) \ No newline at end of file diff --git a/tests/search/test_search_service.py b/tests/search/test_search_service.py index 2ad4c899..9774e3a5 100644 --- a/tests/search/test_search_service.py +++ b/tests/search/test_search_service.py @@ -760,144 +760,155 @@ async def test_build_multilingual_sources_sorting(): async def test_get_url_link_success(): """Test get_url_link service with valid pecha_segment_id""" from pecha_api.search.search_service import get_url_link - + mock_segment = Mock() mock_segment.id = uuid4() mock_segment.text_id = "text123" mock_segment.pecha_segment_id = "pecha_seg_123" - + with patch("pecha_api.search.search_service.Segment.get_segment_by_pecha_segment_id", new_callable=AsyncMock, return_value=mock_segment): result = await get_url_link("pecha_seg_123") - + assert result is not None - assert result == f"/chapter?text_id=text123&segment_id={str(mock_segment.id)}" - assert "text_id=" in result - assert "segment_id=" in result + assert result.text_id == "text123" + assert result.segment_id == str(mock_segment.id) @pytest.mark.asyncio async def test_get_url_link_segment_not_found(): """Test get_url_link service when segment is not found""" + from fastapi import HTTPException + from starlette import status + from pecha_api.search.search_service import get_url_link - from pecha_api.plans.response_message import PECHA_SEGMENT_NOT_FOUND - + with patch("pecha_api.search.search_service.Segment.get_segment_by_pecha_segment_id", new_callable=AsyncMock, return_value=None): - result = await get_url_link("nonexistent_segment_id") - - assert result == PECHA_SEGMENT_NOT_FOUND - assert result == "Pecha segment not found" + with pytest.raises(HTTPException) as exc_info: + await get_url_link("nonexistent_segment_id") + + assert exc_info.value.status_code == status.HTTP_404_NOT_FOUND + assert exc_info.value.detail == "Pecha segment not found" @pytest.mark.asyncio async def test_get_url_link_with_uuid_segment_id(): """Test get_url_link service with UUID-formatted segment ID""" from pecha_api.search.search_service import get_url_link - + segment_uuid = uuid4() text_uuid = uuid4() - + mock_segment = Mock() mock_segment.id = segment_uuid mock_segment.text_id = str(text_uuid) mock_segment.pecha_segment_id = str(uuid4()) - + with patch("pecha_api.search.search_service.Segment.get_segment_by_pecha_segment_id", new_callable=AsyncMock, return_value=mock_segment): result = await get_url_link(mock_segment.pecha_segment_id) - + assert result is not None - assert result == f"/chapter?text_id={str(text_uuid)}&segment_id={str(segment_uuid)}" - assert str(text_uuid) in result - assert str(segment_uuid) in result + assert result.text_id == str(text_uuid) + assert result.segment_id == str(segment_uuid) @pytest.mark.asyncio async def test_get_url_link_with_special_characters(): """Test get_url_link service with special characters in pecha_segment_id""" from pecha_api.search.search_service import get_url_link - + mock_segment = Mock() mock_segment.id = uuid4() mock_segment.text_id = "text-abc-123" mock_segment.pecha_segment_id = "pecha-seg_123-xyz" - + with patch("pecha_api.search.search_service.Segment.get_segment_by_pecha_segment_id", new_callable=AsyncMock, return_value=mock_segment): result = await get_url_link("pecha-seg_123-xyz") - + assert result is not None - assert result == f"/chapter?text_id=text-abc-123&segment_id={str(mock_segment.id)}" + assert result.text_id == "text-abc-123" + assert result.segment_id == str(mock_segment.id) @pytest.mark.asyncio async def test_get_url_link_database_exception(): """Test get_url_link service when database raises an exception""" + from fastapi import HTTPException + from starlette import status + from pecha_api.search.search_service import get_url_link - + with patch("pecha_api.search.search_service.Segment.get_segment_by_pecha_segment_id", new_callable=AsyncMock, side_effect=Exception("Database connection error")): - result = await get_url_link("error_segment_id") - - assert result == "" + with pytest.raises(HTTPException) as exc_info: + await get_url_link("error_segment_id") + + assert exc_info.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + assert exc_info.value.detail == "Failed to retrieve segment link" @pytest.mark.asyncio async def test_get_url_link_with_long_pecha_segment_id(): """Test get_url_link service with very long pecha_segment_id""" from pecha_api.search.search_service import get_url_link - + long_segment_id = "a" * 500 - + mock_segment = Mock() mock_segment.id = uuid4() mock_segment.text_id = "text123" mock_segment.pecha_segment_id = long_segment_id - + with patch("pecha_api.search.search_service.Segment.get_segment_by_pecha_segment_id", new_callable=AsyncMock, return_value=mock_segment): result = await get_url_link(long_segment_id) - + assert result is not None - assert result == f"/chapter?text_id=text123&segment_id={str(mock_segment.id)}" + assert result.text_id == "text123" + assert result.segment_id == str(mock_segment.id) @pytest.mark.asyncio async def test_get_url_link_with_empty_text_id(): """Test get_url_link service when segment has empty text_id""" from pecha_api.search.search_service import get_url_link - + mock_segment = Mock() mock_segment.id = uuid4() mock_segment.text_id = "" mock_segment.pecha_segment_id = "pecha_seg_123" - + with patch("pecha_api.search.search_service.Segment.get_segment_by_pecha_segment_id", new_callable=AsyncMock, return_value=mock_segment): result = await get_url_link("pecha_seg_123") - + assert result is not None - assert result == f"/chapter?text_id=&segment_id={str(mock_segment.id)}" + assert result.text_id == "" + assert result.segment_id == str(mock_segment.id) @pytest.mark.asyncio async def test_get_url_link_multiple_calls(): """Test get_url_link service with multiple sequential calls""" from pecha_api.search.search_service import get_url_link - + mock_segment1 = Mock() mock_segment1.id = uuid4() mock_segment1.text_id = "text1" mock_segment1.pecha_segment_id = "seg1" - + mock_segment2 = Mock() mock_segment2.id = uuid4() mock_segment2.text_id = "text2" mock_segment2.pecha_segment_id = "seg2" - + with patch("pecha_api.search.search_service.Segment.get_segment_by_pecha_segment_id", new_callable=AsyncMock) as mock_get: mock_get.return_value = mock_segment1 result1 = await get_url_link("seg1") - assert result1 == f"/chapter?text_id=text1&segment_id={str(mock_segment1.id)}" - + assert result1.text_id == "text1" + assert result1.segment_id == str(mock_segment1.id) + mock_get.return_value = mock_segment2 result2 = await get_url_link("seg2") - assert result2 == f"/chapter?text_id=text2&segment_id={str(mock_segment2.id)}" - + assert result2.text_id == "text2" + assert result2.segment_id == str(mock_segment2.id) + assert mock_get.call_count == 2 @@ -905,17 +916,18 @@ async def test_get_url_link_multiple_calls(): async def test_get_url_link_none_segment_id(): """Test get_url_link service when segment.id is None""" from pecha_api.search.search_service import get_url_link - + mock_segment = Mock() mock_segment.id = None mock_segment.text_id = "text123" mock_segment.pecha_segment_id = "pecha_seg_123" - + with patch("pecha_api.search.search_service.Segment.get_segment_by_pecha_segment_id", new_callable=AsyncMock, return_value=mock_segment): result = await get_url_link("pecha_seg_123") - + assert result is not None - assert result == f"/chapter?text_id=text123&segment_id=None" + assert result.text_id == "text123" + assert result.segment_id == "None" @pytest.mark.asyncio async def test_get_multilingual_search_external_limit_calculation(): diff --git a/tests/search/test_search_views.py b/tests/search/test_search_views.py index 8df2ac66..4cda6465 100644 --- a/tests/search/test_search_views.py +++ b/tests/search/test_search_views.py @@ -609,57 +609,75 @@ def test_multilingual_search_invalid_pagination(): def test_get_url_link_success(): """Test get_url_link endpoint with valid pecha_segment_id""" - mock_url = "/chapter?text_id=text123&segment_id=segment456" - - with patch("pecha_api.search.search_views.get_url_link_service", new_callable=AsyncMock, return_value=mock_url): + from pecha_api.search.search_response_models import SegmentLinkResponse + + mock_response = SegmentLinkResponse(text_id="text123", segment_id="segment456") + + with patch("pecha_api.search.search_views.get_url_link_service", new_callable=AsyncMock, return_value=mock_response): response = client.get("/search/chat/pecha_seg_123") - + assert response.status_code == status.HTTP_200_OK - assert response.json() == mock_url - assert "/chapter?text_id=" in response.json() - assert "segment_id=" in response.json() + data = response.json() + assert data["text_id"] == "text123" + assert data["segment_id"] == "segment456" def test_get_url_link_segment_not_found(): """Test get_url_link endpoint when segment is not found""" - with patch("pecha_api.search.search_views.get_url_link_service", new_callable=AsyncMock, return_value=""): + with patch( + "pecha_api.search.search_views.get_url_link_service", + new_callable=AsyncMock, + side_effect=HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Pecha segment not found"), + ): response = client.get("/search/chat/nonexistent_segment_id") - - assert response.status_code == status.HTTP_200_OK - assert response.json() == "" + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.json()["detail"] == "Pecha segment not found" def test_get_url_link_with_special_characters(): """Test get_url_link endpoint with special characters in pecha_segment_id""" - mock_url = "/chapter?text_id=text-abc-123&segment_id=seg_456_xyz" - - with patch("pecha_api.search.search_views.get_url_link_service", new_callable=AsyncMock, return_value=mock_url): + from pecha_api.search.search_response_models import SegmentLinkResponse + + mock_response = SegmentLinkResponse(text_id="text-abc-123", segment_id="seg_456_xyz") + + with patch("pecha_api.search.search_views.get_url_link_service", new_callable=AsyncMock, return_value=mock_response): response = client.get("/search/chat/pecha-seg_123-xyz") - + assert response.status_code == status.HTTP_200_OK - assert response.json() == mock_url + data = response.json() + assert data["text_id"] == "text-abc-123" + assert data["segment_id"] == "seg_456_xyz" def test_get_url_link_service_error(): """Test get_url_link endpoint when service raises an exception""" - with patch("pecha_api.search.search_views.get_url_link_service", new_callable=AsyncMock, return_value=""): + with patch( + "pecha_api.search.search_service.Segment.get_segment_by_pecha_segment_id", + new_callable=AsyncMock, + side_effect=Exception("Service error"), + ): response = client.get("/search/chat/error_segment_id") - - assert response.status_code == status.HTTP_200_OK - assert response.json() == "" + + assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR def test_get_url_link_with_uuid_format(): """Test get_url_link endpoint with UUID-like pecha_segment_id""" - mock_url = "/chapter?text_id=550e8400-e29b-41d4-a716-446655440000&segment_id=660e8400-e29b-41d4-a716-446655440001" - - with patch("pecha_api.search.search_views.get_url_link_service", new_callable=AsyncMock, return_value=mock_url): + from pecha_api.search.search_response_models import SegmentLinkResponse + + mock_response = SegmentLinkResponse( + text_id="550e8400-e29b-41d4-a716-446655440000", + segment_id="660e8400-e29b-41d4-a716-446655440001", + ) + + with patch("pecha_api.search.search_views.get_url_link_service", new_callable=AsyncMock, return_value=mock_response): response = client.get("/search/chat/550e8400-e29b-41d4-a716-446655440000") - + assert response.status_code == status.HTTP_200_OK - assert response.json() == mock_url - assert "text_id=" in response.json() - assert "segment_id=" in response.json() + data = response.json() + assert data["text_id"] == "550e8400-e29b-41d4-a716-446655440000" + assert data["segment_id"] == "660e8400-e29b-41d4-a716-446655440001" def test_get_url_link_empty_pecha_segment_id(): @@ -671,11 +689,15 @@ def test_get_url_link_empty_pecha_segment_id(): def test_get_url_link_with_long_pecha_segment_id(): """Test get_url_link endpoint with very long pecha_segment_id""" + from pecha_api.search.search_response_models import SegmentLinkResponse + long_segment_id = "a" * 500 - mock_url = f"/chapter?text_id=text123&segment_id=seg456" - - with patch("pecha_api.search.search_views.get_url_link_service", new_callable=AsyncMock, return_value=mock_url): + mock_response = SegmentLinkResponse(text_id="text123", segment_id="seg456") + + with patch("pecha_api.search.search_views.get_url_link_service", new_callable=AsyncMock, return_value=mock_response): response = client.get(f"/search/chat/{long_segment_id}") - + assert response.status_code == status.HTTP_200_OK - assert response.json() == mock_url \ No newline at end of file + data = response.json() + assert data["text_id"] == "text123" + assert data["segment_id"] == "seg456" \ No newline at end of file