From 987bb3413a4370496b0fdfa7a632ea087283d1b4 Mon Sep 17 00:00:00 2001 From: LuisFigueroaG Date: Thu, 22 Oct 2026 20:59:37 -0300 Subject: [PATCH] [Confluence] Server attach_content: look up the existing attachment by filename Since #1666, attach_content() lists the page's attachments without the filename filter that 4.x sent, and only searches the first page of results (50 by default). When the page has more attachments and the file isn't on that page, it tries to create it again and Confluence rejects the upload because the name is already taken. Send params={"filename": name} again so Confluence returns the matching attachment directly. --- atlassian/confluence/server/__init__.py | 5 ++-- tests/confluence/test_confluence_server.py | 29 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/atlassian/confluence/server/__init__.py b/atlassian/confluence/server/__init__.py index e749d9e44..56371ad49 100644 --- a/atlassian/confluence/server/__init__.py +++ b/atlassian/confluence/server/__init__.py @@ -1407,10 +1407,11 @@ def attach_content( try: # Confluence Server doesn't support PUT on the child/attachment endpoint. # We need to check if the attachment exists first, then create or update accordingly. - # GET existing attachment to check if it exists + # GET existing attachment to check if it exists. Filter by name: the + # unfiltered list is paginated (50 per page by default) and may miss it. existing_attachment = None try: - attachments = self.get(path=path, headers=headers) + attachments = self.get(path=path, headers=headers, params={"filename": name}) if "results" in attachments: for attachment in attachments["results"]: if attachment.get("title") == name: diff --git a/tests/confluence/test_confluence_server.py b/tests/confluence/test_confluence_server.py index 8eb2174fb..f195cd745 100644 --- a/tests/confluence/test_confluence_server.py +++ b/tests/confluence/test_confluence_server.py @@ -171,6 +171,7 @@ def test_attach_content_checks_existing_attachments_before_create(self, mock_pos mock_get.assert_called_once_with( path="rest/api/content/123/child/attachment", headers={"X-Atlassian-Token": "no-check", "Accept": "application/json"}, + params={"filename": "diagram.png"}, ) assert mock_post.call_args.kwargs["path"] == "rest/api/content/123/child/attachment" assert mock_post.call_args.kwargs["headers"] == { @@ -207,6 +208,34 @@ def test_attach_content_updates_existing_attachment_with_post(self, mock_post, m assert mock_post.call_args.kwargs["path"] == "rest/api/content/123/child/attachment/att-1/data" assert mock_post.call_args.kwargs["files"] == {"file": ("diagram.png", content, "image/png")} + def test_attach_content_finds_existing_attachment_beyond_first_page(self, confluence_server): + # Confluence returns 50 attachments per page by default, so the existing + # file has to be looked up by name rather than in the first page. + first_page = [{"id": f"att-{i}", "title": f"file-{i}.png"} for i in range(50)] + calls = [] + + def request(method, url, **kwargs): + response = Response() + response.status_code = 200 + response.reason = "OK" + if method == "GET" and "filename=diagram.png" in url: + body = {"results": [{"id": "att-51", "title": "diagram.png"}], "size": 1} + elif method == "GET": + body = {"results": first_page, "size": 50} + else: + body = {"id": "att-51"} + response._content = json.dumps(body).encode() + calls.append((method, url)) + return response + + with patch.object(confluence_server._session, "request", side_effect=request): + confluence_server.attach_content(io.BytesIO(b"new image"), "diagram.png", "image/png", page_id="123") + + assert calls[-1] == ( + "POST", + "https://test.confluence.com/rest/api/content/123/child/attachment/att-51/data", + ) + def test_attach_content_rejects_empty_attachment_name(self, confluence_server): with pytest.raises(ApiValueError, match="must contain a filename"): confluence_server.attach_content(io.BytesIO(b"content"), "/", page_id="123")