Skip to content

Commit aaeb824

Browse files
authored
fix: add v2 API support for confluence_get_page with OAuth (sooperset#579)
This fix addresses the deprecated v1 REST API endpoint issue when using OAuth authentication. The v1 endpoint has been removed for OAuth but continues to work with API tokens. - Extended ConfluenceV2Adapter with get_page method - Updated PagesMixin to use v2 API for OAuth authentication - Added comprehensive unit tests for OAuth flow - Maintained backward compatibility for API token users Reported-by: dannymyers Github-Issue: sooperset#576
1 parent c6fea35 commit aaeb824

File tree

4 files changed

+279
-3
lines changed

4 files changed

+279
-3
lines changed

src/mcp_atlassian/confluence/pages.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,25 @@ def get_page_content(
4848
Exception: If there is an error retrieving the page
4949
"""
5050
try:
51-
page = self.confluence.get_page_by_id(
52-
page_id=page_id, expand="body.storage,version,space,children.attachment"
53-
)
51+
# Use v2 API for OAuth authentication, v1 API for token/basic auth
52+
v2_adapter = self._v2_adapter
53+
if v2_adapter:
54+
logger.debug(
55+
f"Using v2 API for OAuth authentication to get page '{page_id}'"
56+
)
57+
page = v2_adapter.get_page(
58+
page_id=page_id,
59+
expand="body.storage,version,space,children.attachment",
60+
)
61+
else:
62+
logger.debug(
63+
f"Using v1 API for token/basic authentication to get page '{page_id}'"
64+
)
65+
page = self.confluence.get_page_by_id(
66+
page_id=page_id,
67+
expand="body.storage,version,space,children.attachment",
68+
)
69+
5470
space_key = page.get("space", {}).get("key", "")
5571
content = page["body"]["storage"]["value"]
5672
processed_html, processed_markdown = self.preprocessor.process_html_content(

src/mcp_atlassian/confluence/v2_adapter.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,74 @@ def _get_space_key_from_id(self, space_id: str) -> str:
272272
# Return the space_id as fallback
273273
return space_id
274274

275+
def get_page(
276+
self,
277+
page_id: str,
278+
expand: str | None = None,
279+
) -> dict[str, Any]:
280+
"""Get a page using the v2 API.
281+
282+
Args:
283+
page_id: The ID of the page to retrieve
284+
expand: Fields to expand in the response (not used in v2 API, for compatibility only)
285+
286+
Returns:
287+
The page data from the API response in v1-compatible format
288+
289+
Raises:
290+
ValueError: If page retrieval fails
291+
"""
292+
try:
293+
# Make the v2 API call to get the page
294+
url = f"{self.base_url}/api/v2/pages/{page_id}"
295+
296+
# Convert v1 expand parameters to v2 format
297+
params = {"body-format": "storage"}
298+
299+
response = self.session.get(url, params=params)
300+
response.raise_for_status()
301+
302+
v2_response = response.json()
303+
logger.debug(f"Successfully retrieved page '{page_id}' with v2 API")
304+
305+
# Get space key from space ID
306+
space_id = v2_response.get("spaceId")
307+
space_key = self._get_space_key_from_id(space_id) if space_id else "unknown"
308+
309+
# Convert v2 response to v1-compatible format
310+
v1_compatible = self._convert_v2_to_v1_format(v2_response, space_key)
311+
312+
# Add body.storage structure if body content exists
313+
if "body" in v2_response and v2_response["body"].get("storage"):
314+
storage_value = v2_response["body"]["storage"].get("value", "")
315+
v1_compatible["body"] = {
316+
"storage": {"value": storage_value, "representation": "storage"}
317+
}
318+
319+
# Add space information with more details
320+
if space_id:
321+
v1_compatible["space"] = {
322+
"key": space_key,
323+
"id": space_id,
324+
}
325+
326+
# Add version information
327+
if "version" in v2_response:
328+
v1_compatible["version"] = {
329+
"number": v2_response["version"].get("number", 1)
330+
}
331+
332+
return v1_compatible
333+
334+
except HTTPError as e:
335+
logger.error(f"HTTP error getting page '{page_id}': {e}")
336+
if e.response is not None:
337+
logger.error(f"Response content: {e.response.text}")
338+
raise ValueError(f"Failed to get page '{page_id}': {e}") from e
339+
except Exception as e:
340+
logger.error(f"Error getting page '{page_id}': {e}")
341+
raise ValueError(f"Failed to get page '{page_id}': {e}") from e
342+
275343
def delete_page(self, page_id: str) -> bool:
276344
"""Delete a page using the v2 API.
277345

tests/unit/confluence/test_pages.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,6 +1029,61 @@ def test_update_page_oauth_uses_v2_api(self, oauth_pages_mixin):
10291029
assert result.id == page_id
10301030
assert result.title == title
10311031

1032+
def test_get_page_content_oauth_uses_v2_api(self, oauth_pages_mixin):
1033+
"""Test that OAuth authentication uses v2 API for getting page content."""
1034+
# Arrange
1035+
page_id = "oauth_get_123"
1036+
1037+
# Mock the v2 adapter
1038+
with patch(
1039+
"mcp_atlassian.confluence.pages.ConfluenceV2Adapter"
1040+
) as mock_v2_adapter_class:
1041+
mock_v2_adapter = MagicMock()
1042+
mock_v2_adapter_class.return_value = mock_v2_adapter
1043+
1044+
# Mock v2 API response
1045+
mock_v2_adapter.get_page.return_value = {
1046+
"id": page_id,
1047+
"title": "OAuth Test Page",
1048+
"body": {"storage": {"value": "<p>OAuth page content</p>"}},
1049+
"space": {"key": "PROJ", "name": "Project"},
1050+
"version": {"number": 3},
1051+
}
1052+
1053+
# Mock the preprocessor
1054+
oauth_pages_mixin.preprocessor.process_html_content.return_value = (
1055+
"<p>Processed HTML</p>",
1056+
"Processed OAuth content",
1057+
)
1058+
1059+
# Act
1060+
result = oauth_pages_mixin.get_page_content(
1061+
page_id, convert_to_markdown=True
1062+
)
1063+
1064+
# Assert that v2 API was used instead of v1
1065+
mock_v2_adapter.get_page.assert_called_once_with(
1066+
page_id=page_id, expand="body.storage,version,space,children.attachment"
1067+
)
1068+
1069+
# Verify v1 API was NOT called
1070+
oauth_pages_mixin.confluence.get_page_by_id.assert_not_called()
1071+
1072+
# Verify the preprocessor was called
1073+
oauth_pages_mixin.preprocessor.process_html_content.assert_called_once_with(
1074+
"<p>OAuth page content</p>",
1075+
space_key="PROJ",
1076+
confluence_client=oauth_pages_mixin.confluence,
1077+
)
1078+
1079+
# Verify result is a ConfluencePage with correct data
1080+
assert isinstance(result, ConfluencePage)
1081+
assert result.id == page_id
1082+
assert result.title == "OAuth Test Page"
1083+
assert result.content == "Processed OAuth content"
1084+
assert result.space.key == "PROJ"
1085+
assert result.version.number == 3
1086+
10321087
def test_delete_page_oauth_uses_v2_api(self, oauth_pages_mixin):
10331088
"""Test that OAuth authentication uses v2 API for deleting pages."""
10341089
# Arrange
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
"""Unit tests for ConfluenceV2Adapter class."""
2+
3+
from unittest.mock import MagicMock, Mock
4+
5+
import pytest
6+
import requests
7+
from requests.exceptions import HTTPError
8+
9+
from mcp_atlassian.confluence.v2_adapter import ConfluenceV2Adapter
10+
11+
12+
class TestConfluenceV2Adapter:
13+
"""Test cases for ConfluenceV2Adapter."""
14+
15+
@pytest.fixture
16+
def mock_session(self):
17+
"""Create a mock session."""
18+
return MagicMock(spec=requests.Session)
19+
20+
@pytest.fixture
21+
def v2_adapter(self, mock_session):
22+
"""Create a ConfluenceV2Adapter instance."""
23+
return ConfluenceV2Adapter(
24+
session=mock_session, base_url="https://example.atlassian.net/wiki"
25+
)
26+
27+
def test_get_page_success(self, v2_adapter, mock_session):
28+
"""Test successful page retrieval."""
29+
# Mock the v2 API response
30+
mock_response = Mock()
31+
mock_response.status_code = 200
32+
mock_response.json.return_value = {
33+
"id": "123456",
34+
"status": "current",
35+
"title": "Test Page",
36+
"spaceId": "789",
37+
"version": {"number": 5},
38+
"body": {
39+
"storage": {"value": "<p>Test content</p>", "representation": "storage"}
40+
},
41+
"_links": {"webui": "/pages/viewpage.action?pageId=123456"},
42+
}
43+
mock_session.get.return_value = mock_response
44+
45+
# Mock space key lookup
46+
space_response = Mock()
47+
space_response.status_code = 200
48+
space_response.json.return_value = {"key": "TEST"}
49+
mock_session.get.side_effect = [mock_response, space_response]
50+
51+
# Call the method
52+
result = v2_adapter.get_page("123456")
53+
54+
# Verify the API call
55+
assert mock_session.get.call_count == 2
56+
mock_session.get.assert_any_call(
57+
"https://example.atlassian.net/wiki/api/v2/pages/123456",
58+
params={"body-format": "storage"},
59+
)
60+
61+
# Verify the response format
62+
assert result["id"] == "123456"
63+
assert result["type"] == "page"
64+
assert result["title"] == "Test Page"
65+
assert result["space"]["key"] == "TEST"
66+
assert result["space"]["id"] == "789"
67+
assert result["version"]["number"] == 5
68+
assert result["body"]["storage"]["value"] == "<p>Test content</p>"
69+
assert result["body"]["storage"]["representation"] == "storage"
70+
71+
def test_get_page_not_found(self, v2_adapter, mock_session):
72+
"""Test page retrieval when page doesn't exist."""
73+
# Mock a 404 response
74+
mock_response = Mock()
75+
mock_response.status_code = 404
76+
mock_response.text = "Page not found"
77+
mock_response.raise_for_status.side_effect = HTTPError(response=mock_response)
78+
mock_session.get.return_value = mock_response
79+
80+
# Call the method and expect an exception
81+
with pytest.raises(ValueError, match="Failed to get page '999999'"):
82+
v2_adapter.get_page("999999")
83+
84+
def test_get_page_with_minimal_response(self, v2_adapter, mock_session):
85+
"""Test page retrieval with minimal v2 response."""
86+
# Mock the v2 API response without optional fields
87+
mock_response = Mock()
88+
mock_response.status_code = 200
89+
mock_response.json.return_value = {
90+
"id": "123456",
91+
"status": "current",
92+
"title": "Minimal Page",
93+
}
94+
mock_session.get.return_value = mock_response
95+
96+
# Call the method
97+
result = v2_adapter.get_page("123456")
98+
99+
# Verify the response handles missing fields gracefully
100+
assert result["id"] == "123456"
101+
assert result["type"] == "page"
102+
assert result["title"] == "Minimal Page"
103+
assert result["space"]["key"] == "unknown" # Fallback when no spaceId
104+
assert result["version"]["number"] == 1 # Default version
105+
106+
def test_get_page_network_error(self, v2_adapter, mock_session):
107+
"""Test page retrieval with network error."""
108+
# Mock a network error
109+
mock_session.get.side_effect = requests.RequestException("Network error")
110+
111+
# Call the method and expect an exception
112+
with pytest.raises(ValueError, match="Failed to get page '123456'"):
113+
v2_adapter.get_page("123456")
114+
115+
def test_get_page_with_expand_parameter(self, v2_adapter, mock_session):
116+
"""Test that expand parameter is accepted but not used."""
117+
# Mock the v2 API response
118+
mock_response = Mock()
119+
mock_response.status_code = 200
120+
mock_response.json.return_value = {
121+
"id": "123456",
122+
"status": "current",
123+
"title": "Test Page",
124+
}
125+
mock_session.get.return_value = mock_response
126+
127+
# Call with expand parameter
128+
result = v2_adapter.get_page("123456", expand="body.storage,version")
129+
130+
# Verify the API call doesn't include expand in params
131+
mock_session.get.assert_called_once_with(
132+
"https://example.atlassian.net/wiki/api/v2/pages/123456",
133+
params={"body-format": "storage"},
134+
)
135+
136+
# Verify we still get a result
137+
assert result["id"] == "123456"

0 commit comments

Comments
 (0)