diff --git a/docs/docs/administration/settings/plugins/configuring-atlassian-confluence.mdx b/docs/docs/administration/settings/plugins/configuring-atlassian-confluence.mdx index 314c9bd18..aa0c84a54 100644 --- a/docs/docs/administration/settings/plugins/configuring-atlassian-confluence.mdx +++ b/docs/docs/administration/settings/plugins/configuring-atlassian-confluence.mdx @@ -12,7 +12,11 @@ Dispatch ships with Atlassian Confluence storage and document plugins. This page ### `API URL` \[Required\] -> URL of the confluence cloud instance. +> URL of the Confluence instance. For Cloud this is the site root, for example `https://your-site.atlassian.net/`, without the `/wiki` path. + +### `Hosting Type` \[Default: cloud\] + +> Either `cloud` or `server`. Dispatch reaches Confluence Cloud over the REST v2 API and Confluence Server/Data Center over REST v1; these are different APIs, so this must match the deployment. ### `Username` \[Required\] @@ -26,9 +30,9 @@ Dispatch ships with Atlassian Confluence storage and document plugins. This page > This is the page id of the template that contains the post incident review document details. The plugin uses this template to create a new page and then replace the supported variables with the incident details. -### `Default Space ID` \[Required\] +### `Default Space Key` \[Required\] -> This is the default space [key](https://confluence.atlassian.com/doc/space-keys-829076188.html) where all the pages will be created in confluence. +> This is the default space [key](https://confluence.atlassian.com/doc/space-keys-829076188.html) where all the pages will be created in confluence. The space key, not its numeric id: on Cloud, Dispatch resolves the key to the id the v2 API requires. ### `Parent ID of the pages` \[Required\] @@ -46,7 +50,11 @@ Dispatch ships with Atlassian Confluence storage and document plugins. This page ### `API URL` \[Required\] -> URL of the confluence cloud instance. +> URL of the Confluence instance. For Cloud this is the site root, for example `https://your-site.atlassian.net/`, without the `/wiki` path. + +### `Hosting Type` \[Default: cloud\] + +> Either `cloud` or `server`. Dispatch reaches Confluence Cloud over the REST v2 API and Confluence Server/Data Center over REST v1; these are different APIs, so this must match the deployment. ### `Username` \[Required\] diff --git a/pyproject.toml b/pyproject.toml index dc73c63dc..b8c84cbac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,7 +35,10 @@ dependencies = [ "aiohttp", "alembic", "anthropic==0.122.0", - "atlassian-python-api", + # 5.0.0 split Confluence into separate Cloud (REST v2) and Server (REST v1) + # classes with different method signatures. The major ceiling keeps the next + # such split from reaching production through a routine bump. + "atlassian-python-api>=5.0.3,<6", "attrs>=26.1.0", "bcrypt", "blockkit==2.1.3", diff --git a/requirements-lock.txt b/requirements-lock.txt index 3bd792cdc..6c4559ab6 100644 --- a/requirements-lock.txt +++ b/requirements-lock.txt @@ -164,9 +164,9 @@ anyio==4.14.2 \ # httpx2 # openai # starlette -atlassian-python-api==4.0.7 \ - --hash=sha256:46a70cb29eaab87c0a1697fccd3e25df1aa477e6aa4fb9ba936a9d46b425933c \ - --hash=sha256:8d9cc6068b1d2a48eb434e22e57f6bbd918a47fac9e46b95b7a3cefb00fceacb +atlassian-python-api==5.0.3 \ + --hash=sha256:7d0443a01568cdbf9af45efd30980472979923f6b605f2640e0e80dd02c1dc8a \ + --hash=sha256:c5265c39a8bceb7c3b254d768b4a006a63c7fd80153d293a686fa4e31d52e3c2 # via dispatch (pyproject.toml) attrs==26.1.0 \ --hash=sha256:c647aa4a12dfbad9333ca4e71fe62ddc36f4e63b2d260a37a8b83d2f043ac309 \ diff --git a/src/dispatch/plugins/dispatch_atlassian_confluence/client.py b/src/dispatch/plugins/dispatch_atlassian_confluence/client.py new file mode 100644 index 000000000..ac116f134 --- /dev/null +++ b/src/dispatch/plugins/dispatch_atlassian_confluence/client.py @@ -0,0 +1,133 @@ +"""Confluence client construction, for atlassian-python-api 5.x. + +5.x split Confluence into a Cloud client speaking REST v2 and a Server client +speaking REST v1, with different method signatures. The 4.x +``Confluence(cloud=...)`` factory routes any truthy ``cloud`` value to a class +carrying none of the methods used here, so ``hosting_type`` selects a class +rather than being passed through as a flag. Both call shapes live here so the +plugins never branch on the deployment type. +""" + +from atlassian import ConfluenceServer, ConfluenceV2 + +from dispatch.plugins.dispatch_atlassian_confluence.config import ( + ConfluenceConfigurationBase, + HostingType, +) + + +class ConfluenceApi: + """The Confluence operations Dispatch performs, over one 5.x client.""" + + client_class = None + + def __init__(self, configuration: ConfluenceConfigurationBase): + self.configuration = configuration + self.client = self.client_class( + url=str(configuration.api_url), + username=configuration.username, + password=configuration.password.get_secret_value(), + ) + + @property + def site_url(self) -> str: + """The configured site, without a trailing slash. + + AnyHttpUrl only appends one when the path is empty, so a Server + instance under a context path would otherwise be concatenated straight + onto the next path segment. + """ + return str(self.configuration.api_url).rstrip("/") + + +class CloudApi(ConfluenceApi): + """Confluence Cloud, over the REST v2 API. + + Atlassian removed the v1 Cloud content endpoints on 2025-04-30, so this is + the only API a Cloud deployment can be migrated onto. + """ + + client_class = ConfluenceV2 + + def create_page(self, *, space: str, title: str, body: str, parent_id: str) -> dict: + # v2 addresses spaces by numeric id. `space` is the configured space + # key, which the v2 page endpoint rejects, so resolve it first. + space_id = self.client.get_space_by_key(space)["id"] + return self.client.create_page( + space_id=str(space_id), + title=title, + body=body, + parent_id=parent_id, + body_format="storage", + ) + + def get_page(self, page_id: str) -> dict: + return self.client.get_page_by_id(page_id, body_format="storage") + + def update_page(self, *, page_id: str, title: str, body: str) -> dict: + # v2's PageUpdateRequest requires id, status, title, body and version. + # The client sends `status` only when asked to, so omitting it here + # produces a payload the API rejects. + return self.client.update_page( + page_id=page_id, title=title, body=body, body_format="storage", status="current" + ) + + @property + def site_url(self) -> str: + # The client appends /wiki itself, so an operator who already included + # it would otherwise get it twice in every weblink. + return super().site_url.removesuffix("/wiki") + + def page_weblink(self, *, space: str, page_id: str, title: str) -> str: + return f"{self.site_url}/wiki/spaces/{space}/pages/{page_id}/{title}" + + +class ServerApi(ConfluenceApi): + """Confluence Server/Data Center, over the REST v1 API. + + The v1 signatures are unchanged from 4.0.7, including ``editor`` and + ``full_width``, which have no v2 equivalent: v2 pages are always created in + the new editor. + """ + + client_class = ConfluenceServer + + def create_page(self, *, space: str, title: str, body: str, parent_id: str) -> dict: + return self.client.create_page( + space=space, + title=title, + body=body, + parent_id=parent_id, + type="page", + representation="storage", + editor="v2", + full_width=False, + ) + + def get_page(self, page_id: str) -> dict: + return self.client.get_page_by_id(page_id, expand="body.storage", status=None, version=None) + + def update_page(self, *, page_id: str, title: str, body: str) -> dict: + return self.client.update_page( + page_id=page_id, + title=title, + body=body, + representation="storage", + type="page", + parent_id=None, + minor_edit=False, + full_width=False, + ) + + def page_weblink(self, *, space: str, page_id: str, title: str) -> str: + # Server has no /wiki context path, and addressing by page id avoids + # having to encode the space key and title into the path. + return f"{self.site_url}/pages/viewpage.action?pageId={page_id}" + + +APIS = {HostingType.cloud: CloudApi, HostingType.server: ServerApi} + + +def confluence_api(configuration: ConfluenceConfigurationBase) -> ConfluenceApi: + """Builds an authenticated API for the configured deployment.""" + return APIS[configuration.hosting_type](configuration) diff --git a/src/dispatch/plugins/dispatch_atlassian_confluence/docs/plugin.py b/src/dispatch/plugins/dispatch_atlassian_confluence/docs/plugin.py index ae92413df..9ed6cf420 100644 --- a/src/dispatch/plugins/dispatch_atlassian_confluence/docs/plugin.py +++ b/src/dispatch/plugins/dispatch_atlassian_confluence/docs/plugin.py @@ -1,28 +1,21 @@ from dispatch.plugins.dispatch_atlassian_confluence import docs as confluence_doc_plugin from dispatch.plugins.bases import DocumentPlugin from dispatch.plugins.dispatch_atlassian_confluence.config import ConfluenceConfigurationBase -from atlassian import Confluence +from dispatch.plugins.dispatch_atlassian_confluence.client import ConfluenceApi, confluence_api -def replace_content(client: Confluence, document_id: str, replacements: list[str]) -> {}: +def replace_content(api: ConfluenceApi, document_id: str, replacements: dict[str, str]) -> dict: # read content based on document_id - current_content = client.get_page_by_id( - document_id, expand="body.storage", status=None, version=None - ) + current_content = api.get_page(document_id) current_content_body = current_content["body"]["storage"]["value"] for k, v in replacements.items(): if v: current_content_body = current_content_body.replace(k, v) - updated_content = client.update_page( + updated_content = api.update_page( page_id=document_id, title=current_content["title"], body=current_content_body, - representation="storage", - type="page", - parent_id=None, - minor_edit=False, - full_width=False, ) return updated_content @@ -42,10 +35,4 @@ def __init__(self): def update(self, document_id: str, **kwargs): """Replaces text in document.""" kwargs = {"{{" + k + "}}": v for k, v in kwargs.items()} - confluence_client = Confluence( - url=str(self.configuration.api_url), - username=self.configuration.username, - password=self.configuration.password.get_secret_value(), - cloud=self.configuration.hosting_type, - ) - return replace_content(confluence_client, document_id, kwargs) + return replace_content(confluence_api(self.configuration), document_id, kwargs) diff --git a/src/dispatch/plugins/dispatch_atlassian_confluence/plugin.py b/src/dispatch/plugins/dispatch_atlassian_confluence/plugin.py index 9d841a91d..23f9c20bc 100644 --- a/src/dispatch/plugins/dispatch_atlassian_confluence/plugin.py +++ b/src/dispatch/plugins/dispatch_atlassian_confluence/plugin.py @@ -1,10 +1,10 @@ from dispatch.plugins import dispatch_atlassian_confluence as confluence_plugin from dispatch.plugins.bases import StoragePlugin +from dispatch.plugins.dispatch_atlassian_confluence.client import confluence_api from dispatch.plugins.dispatch_atlassian_confluence.config import ConfluenceConfigurationBase from pydantic import Field -from atlassian import Confluence import requests from requests.auth import HTTPBasicAuth import logging @@ -12,6 +12,20 @@ logger = logging.getLogger(__name__) +def describe(exception: Exception) -> str: + """Renders an exception that may carry nothing but a response. + + The Cloud client's raise_for_status does not understand v2's error + envelope, so HTTPError arrives with an empty message and the status is the + only thing left to report. The body is deliberately not included: it is not + audited for what a Confluence error may quote back. + """ + response = getattr(exception, "response", None) + if str(exception) or response is None: + return str(exception) + return f"HTTP {response.status_code} {response.reason}" + + # TODO : Use the common config from the root directory. class ConfluenceConfiguration(ConfluenceConfigurationBase): """Confluence configuration description.""" @@ -20,7 +34,8 @@ class ConfluenceConfiguration(ConfluenceConfigurationBase): title="Incident template ID", description="This is the page id of the template." ) root_id: str = Field( - title="Default Space ID", description="Defines the default Confluence Space to use." + title="Default Space Key", + description="Key of the default Confluence Space to use, not its numeric id or display name.", ) parent_id: str = Field( title="Parent ID for the pages", @@ -51,74 +66,60 @@ def __init__(self): self.configuration_schema = ConfluenceConfiguration def create_file( - self, drive_id: str, name: str, participants: list[str] = None, file_type: str = "folder" + self, parent_id: str, name: str, participants: list[str] = None, file_type: str = "folder" ): """Creates a new Home page for the incident documents..""" try: if file_type not in ["document", "folder"]: return None - confluence_client = Confluence( - url=str(self.configuration.api_url), - username=self.configuration.username, - password=self.configuration.password.get_secret_value(), - cloud=self.configuration.hosting_type, - ) + # The storage interface is Drive-shaped: callers pass the project's + # storage root, which for Confluence is a space key. The ancestor + # page comes from the plugin configuration instead. + space = parent_id + api = confluence_api(self.configuration) child_display_body = """

Incident Documents:

modified true""" - page_details = confluence_client.create_page( - drive_id, - name, + page_details = api.create_page( + space=space, + title=name, body=child_display_body, parent_id=self.configuration.parent_id, - type="page", - representation="storage", - editor="v2", - full_width=False, ) return { - "weblink": f"{self.configuration.api_url}wiki/spaces/{drive_id}/pages/{page_details['id']}/{name}", + "weblink": api.page_weblink(space=space, page_id=page_details["id"], title=name), "id": page_details["id"], "name": name, "description": "", } except Exception as e: - logger.error(f"Exception happened while creating page: {e}") + logger.error(f"Exception happened while creating page: {describe(e)}") def copy_file(self, folder_id: str, file_id: str, name: str): # TODO : This is the function that is responsible for making the incident documents. try: - confluence_client = Confluence( - url=str(self.configuration.api_url), - username=self.configuration.username, - password=self.configuration.password.get_secret_value(), - cloud=self.configuration.hosting_type, - ) + api = confluence_api(self.configuration) logger.info(f"Copy_file function with args {folder_id}, {file_id}, {name}") - template_content = confluence_client.get_page_by_id( - self.configuration.template_id, expand="body.storage", status=None, version=None - ) - page_details = confluence_client.create_page( + template_content = api.get_page(self.configuration.template_id) + page_details = api.create_page( space=self.configuration.root_id, parent_id=folder_id, title=name, - type="page", - body=template_content["body"], - representation="storage", - editor="v2", - full_width=False, + body=template_content["body"]["storage"]["value"], ) if self.configuration.parent_id: """TODO: Find and fix why the page is not created under the parent_id, folder_id""" self.move_file_confluence(page_id_to_move=page_details["id"], parent_id=folder_id) return { - "weblink": f"{self.configuration.api_url}wiki/spaces/{folder_id}/pages/{page_details['id']}/{name}", + "weblink": api.page_weblink( + space=self.configuration.root_id, page_id=page_details["id"], title=name + ), "id": page_details["id"], "name": name, } except Exception as e: - logger.error(f"Exception happened while creating page: {e}") + logger.error(f"Exception happened while creating page: {describe(e)}") def move_file(self, new_folder_id: str, file_id: str, **kwargs): """Moves a file from one place to another. Not used in the plugin, @@ -135,4 +136,4 @@ def move_file_confluence(self, page_id_to_move: str, parent_id: str): response = requests.request("PUT", url, headers=headers, auth=auth) return response except Exception as e: - logger.error(f"Exception happened while moving page: {e}") + logger.error(f"Exception happened while moving page: {describe(e)}") diff --git a/src/dispatch/storage/flows.py b/src/dispatch/storage/flows.py index 9fae1a582..ea6051bb0 100644 --- a/src/dispatch/storage/flows.py +++ b/src/dispatch/storage/flows.py @@ -71,6 +71,14 @@ def create_storage(subject: Subject, storage_members: list[str], db_session: Ses plugin.instance.create_file(parent_id=external_storage["id"], name=folder_two_name) if subject.project.storage_use_folder_one_as_primary: + # A plugin that could not create the folder returns None, the same way + # the root above does. Subscripting it here would abort the whole + # incident flow, which has no except around this call. + if not folder_one: + log.error( + f"Storage not created. Plugin {plugin.plugin.slug} could not create {folder_one_name}." # noqa: E501 + ) + return external_storage = folder_one # we create the internal storage diff --git a/tests/plugins/dispatch_atlassian_confluence/conftest.py b/tests/plugins/dispatch_atlassian_confluence/conftest.py new file mode 100644 index 000000000..6077d7e26 --- /dev/null +++ b/tests/plugins/dispatch_atlassian_confluence/conftest.py @@ -0,0 +1,82 @@ +"""Fixtures for the Confluence plugin tests (issue #214). + +The fake itself lives in ``fake_confluence.py`` so it can be imported by name +from more than one test module without pytest and the importer giving the file +two module identities. + +``confluence_transport`` replaces only the session the real 5.x client would +have opened. The client class, its constructor arguments, its URL building and +its authentication all still run, so a test failure here means the plugin would +really have failed against Confluence. +""" + +import pytest +from atlassian import ConfluenceServer, ConfluenceV2 + +from dispatch.plugins.dispatch_atlassian_confluence.client import CloudApi, ServerApi +from tests.plugins.dispatch_atlassian_confluence.fake_confluence import ( + FakeConfluence, + build_configuration, + build_document_configuration, + session_for, +) + + +class _DirectRequests: + """Stands in for the `requests` module inside the storage plugin.""" + + def __init__(self, session): + self.session = session + + def request(self, method, url, **kwargs): + return self.session.request(method, url, **kwargs) + + +@pytest.fixture +def confluence(monkeypatch): + """Serve Confluence from memory for the duration of a test.""" + api = FakeConfluence() + + def transport(client_class): + def factory(**kwargs): + return client_class(session=session_for(api), **kwargs) + + return staticmethod(factory) + + monkeypatch.setattr(CloudApi, "client_class", transport(ConfluenceV2)) + monkeypatch.setattr(ServerApi, "client_class", transport(ConfluenceServer)) + + # move_file_confluence bypasses the client and calls `requests` directly, + # so it needs the same transport or the test opens a real socket. + monkeypatch.setattr( + "dispatch.plugins.dispatch_atlassian_confluence.plugin.requests", + _DirectRequests(session_for(api)), + raising=True, + ) + return api + + +@pytest.fixture(params=["cloud", "server"]) +def hosting_type(request): + """Runs a test once per deployment type.""" + return request.param + + +def storage_plugin(hosting_type, **overrides): + """A real ConfluencePagePlugin holding a real configuration.""" + from dispatch.plugins.dispatch_atlassian_confluence.plugin import ConfluencePagePlugin + + instance = ConfluencePagePlugin() + instance.configuration = build_configuration(hosting_type, **overrides) + return instance + + +def document_plugin(hosting_type, **overrides): + """A real ConfluencePageDocPlugin holding a real configuration.""" + from dispatch.plugins.dispatch_atlassian_confluence.docs.plugin import ( + ConfluencePageDocPlugin, + ) + + instance = ConfluencePageDocPlugin() + instance.configuration = build_document_configuration(hosting_type, **overrides) + return instance diff --git a/tests/plugins/dispatch_atlassian_confluence/fake_confluence.py b/tests/plugins/dispatch_atlassian_confluence/fake_confluence.py new file mode 100644 index 000000000..3cbe85f36 --- /dev/null +++ b/tests/plugins/dispatch_atlassian_confluence/fake_confluence.py @@ -0,0 +1,255 @@ +"""An in-memory Confluence, for driving the real plugins against (issue #214). + +The fake is installed as a ``requests`` transport adapter, below +atlassian-python-api, so the client's own URL building, ``/wiki`` context-path +handling, basic auth, JSON encoding, response parsing and error raising all run +for real against it. Replacing the client object with a mock instead would +assert only that the plugin called something -- and issue #214 is precisely a +case where the call was made, the constructor succeeded, and no method existed. + +Both REST versions are served, because 5.x reaches Confluence Cloud over v2 and +Confluence Server over v1, and the point of the migration is that each +deployment type reaches the right one. +""" + +import json +from urllib.parse import parse_qsl, urlparse + +import requests +from requests.adapters import HTTPAdapter + +# Obviously fake, and never a real-looking credential. +USERNAME = "dispatch-tests@example.com" +PASSWORD = "not-a-real-api-token" + +# Deliberately not "https://example.atlassian.net": ConfluenceV2 appends the +# /wiki context path only for Cloud hostnames, and a test written against a +# non-Cloud host would not notice if that stopped happening. +CLOUD_URL = "https://dispatch-tests.atlassian.net/" +SERVER_URL = "https://confluence.internal.example.com/" + +# The configured space key, and the numeric id Cloud's v2 API demands instead. +SPACE_KEY = "INC" +SPACE_ID = 655361 + +TEMPLATE_ID = "111111" +PARENT_ID = "222222" +ROOT_PAGE_ID = "333333" + +TEMPLATE_BODY = "

Commander: {{commander}} Status: {{status}}

" + +# Straight out of Atlassian's published v2 OpenAPI spec +# (developer.atlassian.com/cloud/confluence/openapi-v2.v3.json): the client +# sends only what it is told to, so a wrapper that omits one of these builds a +# request the API rejects and no amount of method-level mocking would notice. +V2_CREATE_REQUIRED = ("spaceId",) +V2_UPDATE_REQUIRED = ("id", "status", "title", "body", "version") + + +def build_configuration(hosting_type: str, **overrides): + """A ConfluenceConfiguration as the storage plugin really receives it.""" + from dispatch.plugins.dispatch_atlassian_confluence.plugin import ConfluenceConfiguration + + values = { + "api_url": CLOUD_URL if hosting_type == "cloud" else SERVER_URL, + "hosting_type": hosting_type, + "username": USERNAME, + "password": PASSWORD, + "template_id": TEMPLATE_ID, + "root_id": SPACE_KEY, + "parent_id": PARENT_ID, + } + values.update(overrides) + return ConfluenceConfiguration(**values) + + +def build_document_configuration(hosting_type: str, **overrides): + """A ConfluenceConfigurationBase, as the document plugin really receives it.""" + from dispatch.plugins.dispatch_atlassian_confluence.config import ( + ConfluenceConfigurationBase, + ) + + values = { + "api_url": CLOUD_URL if hosting_type == "cloud" else SERVER_URL, + "hosting_type": hosting_type, + "username": USERNAME, + "password": PASSWORD, + } + values.update(overrides) + return ConfluenceConfigurationBase(**values) + + +class Request: + """One HTTP request as Confluence would have received it.""" + + def __init__(self, prepared): + self.method = prepared.method + self.url = prepared.url + parsed = urlparse(prepared.url) + self.path = parsed.path + self.query = parsed.query + self.params = dict(parse_qsl(parsed.query)) + self.headers = prepared.headers + self.body = json.loads(prepared.body) if prepared.body else None + + def __repr__(self): + return f"" + + +class FakeConfluence: + """Records what reached the wire and decides what comes back. + + ``requests`` holds one entry per HTTP request the client actually issued, so + a test asserts on the request Confluence would have received rather than on + the arguments of an intercepted method call. + """ + + def __init__(self): + self.requests = [] + self.failure = None + + # -- what comes back --------------------------------------------------- + + def fail_with(self, status: int, body: dict | None = None): + """Every subsequent request fails, in Confluence's error envelope.""" + self.failure = (status, body if body is not None else {"message": "boom"}) + + # -- the routes -------------------------------------------------------- + + def handle(self, request: Request): + if self.failure: + status, body = self.failure + return status, body + + path, method = request.path, request.method + + # v2 (Cloud) + if path.endswith("/wiki/api/v2/spaces") and method == "GET": + # Honours the keys filter, so a page id sent where a space key + # belongs comes back empty, as it would from real Confluence. + keys = request.params.get("keys", "").split(",") + found = [{"id": SPACE_ID, "key": SPACE_KEY, "name": "Incidents"}] + return 200, {"results": found if SPACE_KEY in keys else []} + if path.endswith("/wiki/api/v2/pages") and method == "POST": + # PageCreateRequest requires spaceId, and the space must exist. + missing = self._missing(request.body, V2_CREATE_REQUIRED) + if missing: + return 400, {"errors": [{"title": f"missing required field(s): {missing}"}]} + if request.body["spaceId"] != str(SPACE_ID): + return 404, {"errors": [{"title": "space not found"}]} + return 200, self._v2_page( + page_id="900001", + title=request.body["title"], + body=request.body["body"]["storage"]["value"], + ) + if "/wiki/api/v2/pages/" in path and method == "GET": + return 200, self._v2_page( + page_id=path.rsplit("/", 1)[1], title="Incident Template", body=TEMPLATE_BODY + ) + if "/wiki/api/v2/pages/" in path and method == "PUT": + missing = self._missing(request.body, V2_UPDATE_REQUIRED) + if missing: + return 400, {"errors": [{"title": f"missing required field(s): {missing}"}]} + return 200, self._v2_page( + page_id=path.rsplit("/", 1)[1], + title=request.body["title"], + body=request.body["body"]["storage"]["value"], + version=request.body["version"]["number"], + ) + + # move_file_confluence issues this one with `requests` directly rather + # than through the client, and was left on v1 by the 5.x migration. + if "/move/append/" in path and method == "PUT": + return 200, {} + + # v1 (Server) + if path.endswith("/history") and method == "GET": + # update_page reads the current version from here before its PUT. + return 200, {"lastUpdated": {"number": 1}} + if path.rstrip("/").endswith("/rest/api/content") and method == "POST": + if request.body["space"]["key"] != SPACE_KEY: + return 400, {"message": f"unknown space key {request.body['space']['key']}"} + return 200, self._v1_page( + page_id="900001", + title=request.body["title"], + body=request.body["body"]["storage"]["value"], + ) + if "/rest/api/content/" in path and method == "GET": + return 200, self._v1_page( + page_id=path.rsplit("/", 1)[1], title="Incident Template", body=TEMPLATE_BODY + ) + if "/rest/api/content/" in path and method == "PUT": + return 200, self._v1_page( + page_id=path.rsplit("/", 1)[1], + title=request.body["title"], + body=request.body["body"]["storage"]["value"], + version=request.body["version"]["number"], + ) + + return 404, {"message": f"no route for {method} {path}"} + + @staticmethod + def _missing(body, required): + return sorted(field for field in required if not body.get(field)) + + @staticmethod + def _v2_page(*, page_id, title, body, version=1): + return { + "id": page_id, + "status": "current", + "title": title, + "spaceId": str(SPACE_ID), + "version": {"number": version}, + "body": {"storage": {"value": body, "representation": "storage"}}, + } + + @staticmethod + def _v1_page(*, page_id, title, body, version=1): + return { + "id": page_id, + "type": "page", + "status": "current", + "title": title, + "space": {"key": SPACE_KEY}, + "version": {"number": version}, + "body": {"storage": {"value": body, "representation": "storage"}}, + } + + # -- assertions helpers ------------------------------------------------ + + def last(self, method: str) -> Request: + """The most recent request issued with ``method``.""" + for request in reversed(self.requests): + if request.method == method: + return request + raise AssertionError(f"no {method} request was issued; got {self.requests}") + + +class _Adapter(HTTPAdapter): + """Serves ``FakeConfluence`` instead of opening a socket.""" + + def __init__(self, api: FakeConfluence): + self.api = api + super().__init__() + + def send(self, request, **kwargs): + recorded = Request(request) + self.api.requests.append(recorded) + status, payload = self.api.handle(recorded) + + response = requests.Response() + response.status_code = status + response.reason = "OK" if status < 400 else "Error" + response.url = request.url + response.request = request + response.headers["Content-Type"] = "application/json" + response._content = json.dumps(payload).encode() + return response + + +def session_for(api: FakeConfluence) -> requests.Session: + """A requests session whose every request is served from ``api``.""" + session = requests.Session() + session.mount("https://", _Adapter(api)) + session.mount("http://", _Adapter(api)) + return session diff --git a/tests/plugins/dispatch_atlassian_confluence/test_confluence_client.py b/tests/plugins/dispatch_atlassian_confluence/test_confluence_client.py new file mode 100644 index 000000000..3fa1aa2d9 --- /dev/null +++ b/tests/plugins/dispatch_atlassian_confluence/test_confluence_client.py @@ -0,0 +1,88 @@ +"""Which 5.x client each deployment type gets, and how it authenticates (#214).""" + +import base64 + +import pytest +from atlassian import Confluence, ConfluenceServer, ConfluenceV2 + +from dispatch.plugins.dispatch_atlassian_confluence.client import confluence_api +from tests.plugins.dispatch_atlassian_confluence.fake_confluence import ( + PASSWORD, + TEMPLATE_ID, + USERNAME, + build_configuration, +) + + +@pytest.mark.parametrize( + "hosting_type,expected", + [("cloud", ConfluenceV2), ("server", ConfluenceServer)], +) +def test_hosting_type_selects_the_client_class(hosting_type, expected): + api = confluence_api(build_configuration(hosting_type)) + assert type(api.client) is expected + + +def test_the_two_clients_are_not_interchangeable(): + """Neither class inherits the other, so picking the wrong one is fatal, not + merely suboptimal -- which is what makes the selection worth asserting.""" + assert not issubclass(ConfluenceV2, ConfluenceServer) + assert not issubclass(ConfluenceServer, ConfluenceV2) + + +def test_cloud_reads_pages_from_v2_under_the_wiki_context_path(confluence): + api = confluence_api(build_configuration("cloud")) + assert api.client.url == "https://dispatch-tests.atlassian.net/wiki" + + api.get_page(TEMPLATE_ID) + + request = confluence.last("GET") + assert request.url.startswith( + f"https://dispatch-tests.atlassian.net/wiki/api/v2/pages/{TEMPLATE_ID}" + ) + assert "body-format=storage" in request.query + + +def test_server_reads_pages_from_v1_with_no_context_path(confluence): + api = confluence_api(build_configuration("server")) + assert api.client.url == "https://confluence.internal.example.com" + + api.get_page(TEMPLATE_ID) + + request = confluence.last("GET") + assert request.url.startswith( + f"https://confluence.internal.example.com/rest/api/content/{TEMPLATE_ID}" + ) + assert "expand=body.storage" in request.query + + +def test_passing_hosting_type_as_the_cloud_flag_no_longer_works(): + """The 4.x call shape this migration replaced, asserted so a future bump + that quietly restores it is noticed. + + `Confluence(cloud=...)` in 5.x routes every truthy value -- including the + string "server" -- to a class carrying none of the methods Dispatch calls. + Both hosting types are non-empty strings, which is why #214 broke every + deployment rather than only Cloud ones. + """ + for hosting_type in ("cloud", "server"): + client = Confluence( + url="https://dispatch-tests.atlassian.net/", + username=USERNAME, + password=PASSWORD, + cloud=hosting_type, + ) + for method in ("create_page", "get_page_by_id", "update_page"): + assert not hasattr(client, method), f"{method} unexpectedly present for {hosting_type}" + + +def test_credentials_reach_the_wire_as_basic_auth(confluence, hosting_type): + api = confluence_api(build_configuration(hosting_type)) + api.get_page(TEMPLATE_ID) + + header = confluence.last("GET").headers["Authorization"] + scheme, encoded = header.split(" ", 1) + assert scheme == "Basic" + # get_secret_value(), not str(SecretStr): the latter is "**********", which + # authenticates against nothing and would still look like a token here. + assert base64.b64decode(encoded).decode() == f"{USERNAME}:{PASSWORD}" diff --git a/tests/plugins/dispatch_atlassian_confluence/test_confluence_document_plugin.py b/tests/plugins/dispatch_atlassian_confluence/test_confluence_document_plugin.py new file mode 100644 index 000000000..e5b337a5e --- /dev/null +++ b/tests/plugins/dispatch_atlassian_confluence/test_confluence_document_plugin.py @@ -0,0 +1,117 @@ +"""What ConfluencePageDocPlugin actually substitutes and sends (#214).""" + +import pytest +from atlassian.errors import ApiError +from requests import HTTPError + +from tests.plugins.dispatch_atlassian_confluence.conftest import document_plugin +from tests.plugins.dispatch_atlassian_confluence.fake_confluence import ROOT_PAGE_ID + + +def test_update_substitutes_placeholders_into_the_stored_body(confluence, hosting_type): + """The template body is `

Commander: {{commander}} Status: {{status}}

`; + the plugin wraps each kwarg name in braces before replacing.""" + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace", status="Stable") + + body = confluence.last("PUT").body["body"]["storage"]["value"] + assert body == "

Commander: Ada Lovelace Status: Stable

" + + +def test_update_leaves_placeholders_alone_when_a_value_is_empty(confluence, hosting_type): + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace", status="") + + body = confluence.last("PUT").body["body"]["storage"]["value"] + assert body == "

Commander: Ada Lovelace Status: {{status}}

" + + +def test_update_ignores_keys_the_template_does_not_mention(confluence, hosting_type): + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace", unrelated="ignored") + + body = confluence.last("PUT").body["body"]["storage"]["value"] + assert "ignored" not in body + + +def test_update_keeps_the_existing_page_title(confluence, hosting_type): + """Both APIs require a title on update; sending the wrong one renames the + incident document.""" + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace") + + assert confluence.last("PUT").body["title"] == "Incident Template" + + +def test_update_targets_the_document_on_the_platforms_own_endpoint(confluence, hosting_type): + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace") + + path = confluence.last("PUT").path + if hosting_type == "cloud": + assert path == f"/wiki/api/v2/pages/{ROOT_PAGE_ID}" + else: + assert path == f"/rest/api/content/{ROOT_PAGE_ID}" + + +def test_update_advances_the_page_version(confluence, hosting_type): + """Both APIs reject an update that does not move the version forward; v2 + makes the caller supply it, v1 reads it from the page history.""" + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace") + + assert confluence.last("PUT").body["version"]["number"] == 2 + + +def test_update_returns_the_updated_page(confluence, hosting_type): + plugin = document_plugin(hosting_type) + + result = plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace") + + assert result["id"] == ROOT_PAGE_ID + assert result["body"]["storage"]["value"] == "

Commander: Ada Lovelace Status: {{status}}

" + + +@pytest.mark.parametrize("status", [403, 404, 500]) +def test_update_propagates_confluence_failures(confluence, hosting_type, status): + """`DocumentPlugin.update` has no except of its own, and its callers treat a + raised error as the document flow failing. Swallowing it here would report + an unsubstituted document as successfully written. + + Both exception families are accepted because the two clients differ: the + Server client translates 403/404 into atlassian's own ApiError subclasses, + while the Cloud client lets requests' HTTPError through. + """ + plugin = document_plugin(hosting_type) + confluence.fail_with(status) + + with pytest.raises((HTTPError, ApiError)): + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace") + + assert all(request.method != "PUT" for request in confluence.requests) + + +def test_server_skips_the_write_when_nothing_changed(confluence): + """A documented platform difference: the v1 client compares the stored body + first and returns without a PUT when it matches, where v2 always writes.""" + plugin = document_plugin("server") + + plugin.update(ROOT_PAGE_ID, commander="") + + assert all(request.method != "PUT" for request in confluence.requests) + + +def test_cloud_writes_even_when_nothing_changed(confluence): + plugin = document_plugin("cloud") + + plugin.update(ROOT_PAGE_ID, commander="") + + assert confluence.last("PUT").body["body"]["storage"]["value"] == ( + "

Commander: {{commander}} Status: {{status}}

" + ) diff --git a/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py b/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py new file mode 100644 index 000000000..a45d22ae9 --- /dev/null +++ b/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py @@ -0,0 +1,246 @@ +"""What ConfluencePagePlugin actually sends and returns, per platform (#214).""" + +import pytest + +from tests.plugins.dispatch_atlassian_confluence.conftest import storage_plugin +from tests.plugins.dispatch_atlassian_confluence.fake_confluence import ( + PARENT_ID, + SPACE_ID, + SPACE_KEY, + TEMPLATE_BODY, + TEMPLATE_ID, +) + + +def test_create_file_creates_a_page_in_the_configured_space(confluence, hosting_type): + plugin = storage_plugin(hosting_type) + + plugin.create_file(parent_id=SPACE_KEY, name="Dispatch Incident") + + request = confluence.last("POST") + assert request.body["title"] == "Dispatch Incident" + assert "