From 221649cee06091e888ba310f485397d3c15fe480 Mon Sep 17 00:00:00 2001 From: sooperset Date: Thu, 27 Nov 2025 01:14:02 +0900 Subject: [PATCH] fix(jira): migrate Cloud search to v3 API endpoint The Jira Cloud v2 search API (/rest/api/*/search) has been deprecated and is now returning errors. This commit migrates Cloud deployments to the new v3 search endpoint (POST /rest/api/3/search/jql). Changes: - Cloud: Use POST /rest/api/3/search/jql with nextPageToken pagination - Server/DC: Continue using the v2 /rest/api/2/search endpoint unchanged - Updated tests to mock the new v3 API calls for Cloud scenarios The v3 API has some differences: - Uses JSON body instead of query parameters - Uses nextPageToken for pagination instead of startAt - Does not return total count (returns -1) - Max 100 results per request (was 50 for Server/DC) Fixes #720 Github-Issue:#720 --- src/mcp_atlassian/jira/search.py | 83 +++++----- tests/unit/jira/test_search.py | 261 +++++++++++++------------------ 2 files changed, 155 insertions(+), 189 deletions(-) diff --git a/src/mcp_atlassian/jira/search.py b/src/mcp_atlassian/jira/search.py index 09a22ada8..ee4b87b17 100644 --- a/src/mcp_atlassian/jira/search.py +++ b/src/mcp_atlassian/jira/search.py @@ -1,6 +1,7 @@ """Module for Jira search operations.""" import logging +from typing import Any import requests from requests.exceptions import HTTPError @@ -86,55 +87,61 @@ def search_issues( fields_param = fields if self.config.is_cloud: - actual_total = -1 - try: - # Call 1: Get metadata (including total) using standard search API - metadata_params = {"jql": jql, "maxResults": 0} - metadata_response = self.jira.get( - self.jira.resource_url("search"), params=metadata_params - ) + # Cloud: Use v3 API endpoint POST /rest/api/3/search/jql + # The old v2 /rest/api/*/search endpoint is deprecated + # See: https://developer.atlassian.com/changelog/#CHANGE-2046 + + # Build request body for v3 API + fields_list = fields_param.split(",") if fields_param else ["id", "key"] + request_body: dict[str, Any] = { + "jql": jql, + "maxResults": min(limit, 100), # v3 API max is 100 per request + "fields": fields_list, + } + # Note: v3 API uses 'expand' as a comma-separated string, not an array + if expand: + request_body["expand"] = expand - if ( - isinstance(metadata_response, dict) - and "total" in metadata_response - ): - try: - actual_total = int(metadata_response["total"]) - except (ValueError, TypeError): - logger.warning( - f"Could not parse 'total' from metadata response for JQL: {jql}. Received: {metadata_response.get('total')}" - ) - else: - logger.warning( - f"Could not retrieve total count from metadata response for JQL: {jql}. Response type: {type(metadata_response)}" - ) - except Exception as meta_err: - logger.error( - f"Error fetching metadata for JQL '{jql}': {str(meta_err)}" - ) + # Fetch issues using v3 API with nextPageToken pagination + all_issues: list[dict[str, Any]] = [] + next_page_token: str | None = None - # Call 2: Get the actual issues using the enhanced method - issues_response_list = self.jira.enhanced_jql_get_list_of_tickets( - jql, fields=fields_param, limit=limit, expand=expand - ) + while len(all_issues) < limit: + if next_page_token: + request_body["nextPageToken"] = next_page_token - if not isinstance(issues_response_list, list): - msg = f"Unexpected return value type from `jira.enhanced_jql_get_list_of_tickets`: {type(issues_response_list)}" - logger.error(msg) - raise TypeError(msg) + response = self.jira.post( + "rest/api/3/search/jql", json=request_body + ) - response_dict_for_model = { - "issues": issues_response_list, - "total": actual_total, + if not isinstance(response, dict): + msg = f"Unexpected response type from v3 search API: {type(response)}" + logger.error(msg) + raise TypeError(msg) + + issues = response.get("issues", []) + all_issues.extend(issues) + + # Check for more pages + next_page_token = response.get("nextPageToken") + if not next_page_token: + break + + # Build response dict for model + # Note: v3 API doesn't provide total count, so we use -1 + response_dict: dict[str, Any] = { + "issues": all_issues[:limit], + "total": -1, + "startAt": 0, + "maxResults": limit, } search_result = JiraSearchResult.from_api_response( - response_dict_for_model, + response_dict, base_url=self.config.url, requested_fields=fields_param, ) - # Return the full search result object return search_result else: limit = min(limit, 50) diff --git a/tests/unit/jira/test_search.py b/tests/unit/jira/test_search.py index 8c5795ab7..4886e0d59 100644 --- a/tests/unit/jira/test_search.py +++ b/tests/unit/jira/test_search.py @@ -52,69 +52,68 @@ def mock_issues_response(self) -> dict: "maxResults": 50, } - @pytest.mark.parametrize( - "is_cloud, expected_method_name", - [ - (True, "enhanced_jql_get_list_of_tickets"), # Cloud scenario - (False, "jql"), # Server/DC scenario - ], - ) - def test_search_issues_calls_correct_method( + def test_search_issues_calls_v3_api_for_cloud( self, search_mixin: SearchMixin, mock_issues_response, - is_cloud, - expected_method_name, ): - """Test that the correct Jira API method is called based on Cloud/Server setting.""" - # Setup: Mock config.is_cloud - search_mixin.config.is_cloud = is_cloud - search_mixin.config.projects_filter = None # No filter for this test - search_mixin.config.url = ( - "https://test.example.com" # Model creation needs this - ) + """Test that Cloud uses POST /rest/api/3/search/jql (v3 API).""" + # Setup: Mock config.is_cloud = True + search_mixin.config.is_cloud = True + search_mixin.config.projects_filter = None + search_mixin.config.url = "https://test.example.com" - # Setup: Mock response for both API methods - search_mixin.jira.enhanced_jql_get_list_of_tickets = MagicMock( - return_value=mock_issues_response["issues"] - ) + # Setup: Mock v3 API response + search_mixin.jira.post = MagicMock(return_value=mock_issues_response) search_mixin.jira.jql = MagicMock(return_value=mock_issues_response) - # Determine other method name for assertion - other_method_name = ( - "jql" - if expected_method_name == "enhanced_jql_get_list_of_tickets" - else "enhanced_jql_get_list_of_tickets" - ) - # Act jql_query = "project = TEST" result = search_mixin.search_issues(jql_query, limit=10, start=0) # Assert: Basic result verification assert isinstance(result, JiraSearchResult) - assert len(result.issues) > 0 # Based on mocked response + assert len(result.issues) > 0 - # Assert: Correct method call verification - expected_method_mock = getattr(search_mixin.jira, expected_method_name) + # Assert: v3 API (POST) was called for Cloud + search_mixin.jira.post.assert_called_once() + call_args = search_mixin.jira.post.call_args + assert call_args[0][0] == "rest/api/3/search/jql" + assert call_args[1]["json"]["jql"] == jql_query - # Define expected kwargs based on whether it's Cloud or Server - expected_kwargs = { - "limit": 10, - "expand": None, - } + # Assert: v2 API (jql) was NOT called + search_mixin.jira.jql.assert_not_called() - # Add start param only for Server/DC - if not is_cloud: - expected_kwargs["start"] = 0 + def test_search_issues_calls_jql_for_server( + self, + search_mixin: SearchMixin, + mock_issues_response, + ): + """Test that Server/DC uses jql method (v2 API).""" + # Setup: Mock config.is_cloud = False + search_mixin.config.is_cloud = False + search_mixin.config.projects_filter = None + search_mixin.config.url = "https://test.example.com" + + # Setup: Mock response + search_mixin.jira.post = MagicMock(return_value=mock_issues_response) + search_mixin.jira.jql = MagicMock(return_value=mock_issues_response) + + # Act + jql_query = "project = TEST" + result = search_mixin.search_issues(jql_query, limit=10, start=0) + + # Assert: Basic result verification + assert isinstance(result, JiraSearchResult) + assert len(result.issues) > 0 - expected_method_mock.assert_called_once_with( - jql_query, fields=ANY, **expected_kwargs + # Assert: jql method was called for Server/DC + search_mixin.jira.jql.assert_called_once_with( + jql_query, fields=ANY, start=0, limit=10, expand=None ) - # Assert: Other method was not called - other_method_mock = getattr(search_mixin.jira, other_method_name) - other_method_mock.assert_not_called() + # Assert: v3 API (POST) was NOT called + search_mixin.jira.post.assert_not_called() def test_search_issues_basic(self, search_mixin: SearchMixin): """Test basic search functionality.""" @@ -581,51 +580,39 @@ def test_search_issues_with_projects_filter_jql_construction( search_mixin.config.url = "https://test.example.com" # Setup mock response for both API methods - search_mixin.jira.enhanced_jql_get_list_of_tickets = MagicMock( - return_value=mock_issues_response["issues"] - ) + search_mixin.jira.post = MagicMock(return_value=mock_issues_response) search_mixin.jira.jql = MagicMock(return_value=mock_issues_response) - api_method_mock = getattr( - search_mixin.jira, "enhanced_jql_get_list_of_tickets" if is_cloud else "jql" - ) + + # Helper to get the JQL from the appropriate mock + def get_jql_from_call(): + if is_cloud: + return search_mixin.jira.post.call_args[1]["json"]["jql"] + else: + return search_mixin.jira.jql.call_args[0][0] # Act: Single project filter search_mixin.search_issues("text ~ 'test'", projects_filter="TEST") - # Define expected kwargs based on is_cloud - expected_kwargs = { - "fields": ANY, - "limit": ANY, - "expand": ANY, - } - # Add start parameter only for Server/DC - if not is_cloud: - expected_kwargs["start"] = ANY - # Assert: JQL verification - api_method_mock.assert_called_with( - "(text ~ 'test') AND project = \"TEST\"", # Check constructed JQL - **expected_kwargs, - ) + assert get_jql_from_call() == "(text ~ 'test') AND project = \"TEST\"" - # Reset mock for next call - api_method_mock.reset_mock() + # Reset mocks for next call + search_mixin.jira.post.reset_mock() + search_mixin.jira.jql.reset_mock() # Act: Multiple projects filter search_mixin.search_issues("text ~ 'test'", projects_filter="TEST, DEV") # Assert: JQL verification - api_method_mock.assert_called_with( - '(text ~ \'test\') AND project IN ("TEST", "DEV")', # Check constructed JQL - **expected_kwargs, - ) + assert get_jql_from_call() == '(text ~ \'test\') AND project IN ("TEST", "DEV")' - # Reset mock for next call - api_method_mock.reset_mock() + # Reset mocks for next call + search_mixin.jira.post.reset_mock() + search_mixin.jira.jql.reset_mock() # Act: Call with both JQL and filter search_mixin.search_issues("project = OTHER", projects_filter="TEST") # Assert: JQL verification (existing JQL has priority) - api_method_mock.assert_called_with("project = OTHER", **expected_kwargs) + assert get_jql_from_call() == "project = OTHER" @pytest.mark.parametrize("is_cloud", [True, False]) def test_search_issues_with_config_projects_filter_jql_construction( @@ -638,40 +625,31 @@ def test_search_issues_with_config_projects_filter_jql_construction( search_mixin.config.url = "https://test.example.com" # Setup mock response for both API methods - search_mixin.jira.enhanced_jql_get_list_of_tickets = MagicMock( - return_value=mock_issues_response["issues"] - ) + search_mixin.jira.post = MagicMock(return_value=mock_issues_response) search_mixin.jira.jql = MagicMock(return_value=mock_issues_response) - api_method_mock = getattr( - search_mixin.jira, "enhanced_jql_get_list_of_tickets" if is_cloud else "jql" - ) - # Define expected kwargs based on is_cloud - expected_kwargs = { - "fields": ANY, - "limit": ANY, - "expand": ANY, - } - # Add start parameter only for Server/DC - if not is_cloud: - expected_kwargs["start"] = ANY + # Helper to get the JQL from the appropriate mock + def get_jql_from_call(): + if is_cloud: + return search_mixin.jira.post.call_args[1]["json"]["jql"] + else: + return search_mixin.jira.jql.call_args[0][0] # Act: Use config filter search_mixin.search_issues("text ~ 'test'") # Assert: JQL verification - api_method_mock.assert_called_with( - '(text ~ \'test\') AND project IN ("CONF1", "CONF2")', **expected_kwargs + assert ( + get_jql_from_call() == '(text ~ \'test\') AND project IN ("CONF1", "CONF2")' ) - # Reset mock for next call - api_method_mock.reset_mock() + # Reset mocks for next call + search_mixin.jira.post.reset_mock() + search_mixin.jira.jql.reset_mock() # Act: Override config filter with parameter search_mixin.search_issues("text ~ 'test'", projects_filter="OVERRIDE") # Assert: JQL verification - api_method_mock.assert_called_with( - "(text ~ 'test') AND project = \"OVERRIDE\"", **expected_kwargs - ) + assert get_jql_from_call() == "(text ~ 'test') AND project = \"OVERRIDE\"" @pytest.mark.parametrize("is_cloud", [True, False]) def test_search_issues_with_empty_jql_and_projects_filter( @@ -684,43 +662,35 @@ def test_search_issues_with_empty_jql_and_projects_filter( search_mixin.config.url = "https://test.example.com" # Setup mock response for both API methods - search_mixin.jira.enhanced_jql_get_list_of_tickets = MagicMock( - return_value=mock_issues_response["issues"] - ) + search_mixin.jira.post = MagicMock(return_value=mock_issues_response) search_mixin.jira.jql = MagicMock(return_value=mock_issues_response) - api_method_mock = getattr( - search_mixin.jira, "enhanced_jql_get_list_of_tickets" if is_cloud else "jql" - ) - # Define expected kwargs based on is_cloud - expected_kwargs = { - "fields": ANY, - "limit": ANY, - "expand": ANY, - } - # Add start parameter only for Server/DC - if not is_cloud: - expected_kwargs["start"] = ANY + # Helper to get the JQL from the appropriate mock + def get_jql_from_call(): + if is_cloud: + return search_mixin.jira.post.call_args[1]["json"]["jql"] + else: + return search_mixin.jira.jql.call_args[0][0] # Test 1: Empty string JQL with single project search_mixin.search_issues("", projects_filter="PROJ1") - api_method_mock.assert_called_with('project = "PROJ1"', **expected_kwargs) + assert get_jql_from_call() == 'project = "PROJ1"' - # Reset mock - api_method_mock.reset_mock() + # Reset mocks + search_mixin.jira.post.reset_mock() + search_mixin.jira.jql.reset_mock() # Test 2: Empty string JQL with multiple projects search_mixin.search_issues("", projects_filter="PROJ1,PROJ2") - api_method_mock.assert_called_with( - 'project IN ("PROJ1", "PROJ2")', **expected_kwargs - ) + assert get_jql_from_call() == 'project IN ("PROJ1", "PROJ2")' - # Reset mock - api_method_mock.reset_mock() + # Reset mocks + search_mixin.jira.post.reset_mock() + search_mixin.jira.jql.reset_mock() # Test 3: None JQL with projects filter result = search_mixin.search_issues(None, projects_filter="PROJ1") - api_method_mock.assert_called_with('project = "PROJ1"', **expected_kwargs) + assert get_jql_from_call() == 'project = "PROJ1"' assert isinstance(result, JiraSearchResult) @pytest.mark.parametrize("is_cloud", [True, False]) @@ -734,57 +704,46 @@ def test_search_issues_with_order_by_and_projects_filter( search_mixin.config.url = "https://test.example.com" # Setup mock response for both API methods - search_mixin.jira.enhanced_jql_get_list_of_tickets = MagicMock( - return_value=mock_issues_response["issues"] - ) + search_mixin.jira.post = MagicMock(return_value=mock_issues_response) search_mixin.jira.jql = MagicMock(return_value=mock_issues_response) - api_method_mock = getattr( - search_mixin.jira, "enhanced_jql_get_list_of_tickets" if is_cloud else "jql" - ) - # Define expected kwargs based on is_cloud - expected_kwargs = { - "fields": ANY, - "limit": ANY, - "expand": ANY, - } - # Add start parameter only for Server/DC - if not is_cloud: - expected_kwargs["start"] = ANY + # Helper to get the JQL from the appropriate mock + def get_jql_from_call(): + if is_cloud: + return search_mixin.jira.post.call_args[1]["json"]["jql"] + else: + return search_mixin.jira.jql.call_args[0][0] # Test 1: ORDER BY with single project search_mixin.search_issues("ORDER BY created DESC", projects_filter="PROJ1") - api_method_mock.assert_called_with( - 'project = "PROJ1" ORDER BY created DESC', **expected_kwargs - ) + assert get_jql_from_call() == 'project = "PROJ1" ORDER BY created DESC' - # Reset mock - api_method_mock.reset_mock() + # Reset mocks + search_mixin.jira.post.reset_mock() + search_mixin.jira.jql.reset_mock() # Test 2: ORDER BY with multiple projects search_mixin.search_issues( "ORDER BY created DESC", projects_filter="PROJ1,PROJ2" ) - api_method_mock.assert_called_with( - 'project IN ("PROJ1", "PROJ2") ORDER BY created DESC', **expected_kwargs + assert ( + get_jql_from_call() == 'project IN ("PROJ1", "PROJ2") ORDER BY created DESC' ) - # Reset mock - api_method_mock.reset_mock() + # Reset mocks + search_mixin.jira.post.reset_mock() + search_mixin.jira.jql.reset_mock() # Test 3: Case insensitive ORDER BY search_mixin.search_issues("order by updated ASC", projects_filter="PROJ1") - api_method_mock.assert_called_with( - 'project = "PROJ1" order by updated ASC', **expected_kwargs - ) + assert get_jql_from_call() == 'project = "PROJ1" order by updated ASC' - # Reset mock - api_method_mock.reset_mock() + # Reset mocks + search_mixin.jira.post.reset_mock() + search_mixin.jira.jql.reset_mock() # Test 4: ORDER BY with extra spaces search_mixin.search_issues( " ORDER BY priority DESC ", projects_filter="PROJ1" ) - api_method_mock.assert_called_with( - 'project = "PROJ1" ORDER BY priority DESC ', **expected_kwargs - ) + assert get_jql_from_call() == 'project = "PROJ1" ORDER BY priority DESC '