From 472829d2b8deca9550d57f7edfc0c0b13c3c7018 Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 3 Mar 2024 20:25:02 -0500 Subject: [PATCH 01/19] publisher: minimize rest variable name Reduce the length of the member variable for publisher's reference to the REST client entity. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/cmd/report.py | 2 +- sphinxcontrib/confluencebuilder/publisher.py | 64 +++++++++---------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/sphinxcontrib/confluencebuilder/cmd/report.py b/sphinxcontrib/confluencebuilder/cmd/report.py index 7d3ffb71..f76461dc 100644 --- a/sphinxcontrib/confluencebuilder/cmd/report.py +++ b/sphinxcontrib/confluencebuilder/cmd/report.py @@ -152,7 +152,7 @@ def report_main(args_parser): publisher.connect() info += ' connected: yes\n' - session = publisher.rest_client.session + session = publisher.rest.session except Exception: # noqa: BLE001 sys.stdout.flush() logger.error(traceback.format_exc()) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index c99437e9..f2f30fb9 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -64,11 +64,11 @@ def init(self, config, cloud=None): rlog.setLevel(logging.DEBUG) def connect(self): - self.rest_client = Rest(self.config) + self.rest = Rest(self.config) server_url = self.config.confluence_server_url try: - rsp = self.rest_client.get(f'space/{self.space_key}') + rsp = self.rest.get(f'space/{self.space_key}') except ConfluenceBadApiError as ex: if ex.status_code == 404: pw_set = bool(self.config.confluence_server_pass) @@ -113,7 +113,7 @@ def connect(self): self.space_type = rsp['type'] def disconnect(self): - self.rest_client.close() + self.rest.close() def archive_page(self, page_id): if self.dryrun: @@ -129,7 +129,7 @@ def archive_page(self, page_id): 'pages': [{'id': page_id}], } - rsp = self.rest_client.post('content/archive', data) + rsp = self.rest.post('content/archive', data) longtask_id = rsp['id'] # wait for the archiving of the page to complete @@ -138,7 +138,7 @@ def archive_page(self, page_id): while attempt <= MAX_WAIT_FOR_PAGE_ARCHIVE: time.sleep(0.5) - rsp = self.rest_client.get(f'longtask/{longtask_id}') + rsp = self.rest.get(f'longtask/{longtask_id}') if rsp['finished']: break @@ -173,7 +173,7 @@ def archive_pages(self, page_ids): # Note, multi-page archive can result in Confluence reporting the # following message: # Cannot use bulk archive feature for non premium edition - self.rest_client.post('content/archive', data) + self.rest.post('content/archive', data) except ConfluencePermissionError as ex: msg = ( @@ -222,7 +222,7 @@ def get_base_page_id(self): return base_page_id - rsp = self.rest_client.get('content', { + rsp = self.rest.get('content', { 'type': 'page', 'spaceKey': self.space_key, 'title': self.parent_ref, @@ -327,7 +327,7 @@ def _get_descendants(self, page_id, mode): # needed to fetch a complete descendants set (for larger sets). search_fields['limit'] = 1000 - rsp = self.rest_client.get(api_endpoint, search_fields) + rsp = self.rest.get(api_endpoint, search_fields) idx = 0 while rsp['size'] > 0: for result in rsp['results']: @@ -340,7 +340,7 @@ def _get_descendants(self, page_id, mode): idx += int(rsp['limit']) sub_search_fields = dict(search_fields) sub_search_fields['start'] = idx - rsp = self.rest_client.get(api_endpoint, sub_search_fields) + rsp = self.rest.get(api_endpoint, sub_search_fields) return descendants @@ -397,7 +397,7 @@ def get_attachment(self, page_id, name): attachment_id = None url = f'content/{page_id}/child/attachment' - rsp = self.rest_client.get(url, { + rsp = self.rest.get(url, { 'filename': name, }) @@ -431,7 +431,7 @@ def get_attachments(self, page_id): # needed to fetch a complete attachment set (for larger sets). search_fields['limit'] = 1000 - rsp = self.rest_client.get(url, search_fields) + rsp = self.rest.get(url, search_fields) idx = 0 while rsp['size'] > 0: for result in rsp['results']: @@ -444,7 +444,7 @@ def get_attachments(self, page_id): idx += int(rsp['limit']) sub_search_fields = dict(search_fields) sub_search_fields['start'] = idx - rsp = self.rest_client.get(url, sub_search_fields) + rsp = self.rest.get(url, sub_search_fields) return attachment_info @@ -468,7 +468,7 @@ def get_page(self, page_name, expand='version', status='current'): page = None page_id = None - rsp = self.rest_client.get('content', { + rsp = self.rest.get('content', { 'type': 'page', 'spaceKey': self.space_key, 'title': page_name, @@ -500,7 +500,7 @@ def get_page_by_id(self, page_id, expand='version'): the page id and page object """ - page = self.rest_client.get(f'content/{page_id}', { + page = self.rest.get(f'content/{page_id}', { 'status': 'current', 'expand': expand, }) @@ -536,7 +536,7 @@ def get_page_case_insensitive(self, page_name): '" and type=page and title~"' + page_name + '"'} search_fields['limit'] = 1000 - rsp = self.rest_client.get('content/search', search_fields) + rsp = self.rest.get('content/search', search_fields) idx = 0 while rsp['size'] > 0: for result in rsp['results']: @@ -551,7 +551,7 @@ def get_page_case_insensitive(self, page_name): idx += int(rsp['limit']) sub_search_fields = dict(search_fields) sub_search_fields['start'] = idx - rsp = self.rest_client.get('content/search', sub_search_fields) + rsp = self.rest.get('content/search', sub_search_fields) return page_id, page @@ -575,7 +575,7 @@ def get_page_properties(self, page_id, expand='version'): try: property_path = f'content/{page_id}/property/{PROP_KEY}' - props = self.rest_client.get(property_path, { + props = self.rest.get(property_path, { 'status': 'current', 'expand': expand, }) @@ -663,7 +663,7 @@ def store_attachment(self, page_id, name, data, mimetype, hash_, force=False): url = f'content/{page_id}/child/attachment' try: - rsp = self.rest_client.post(url, None, files=data) + rsp = self.rest.post(url, None, files=data) uploaded_attachment_id = rsp['results'][0]['id'] except ConfluenceBadApiError as ex: # file type restricted? generate a warning @@ -708,11 +708,11 @@ def store_attachment(self, page_id, name, data, mimetype, hash_, force=False): if attachment: url = 'content/{}/child/attachment/{}/data'.format( page_id, attachment['id']) - rsp = self.rest_client.post(url, None, files=data) + rsp = self.rest.post(url, None, files=data) uploaded_attachment_id = rsp['id'] if not self.watch: - self.rest_client.delete('user/watch/content', + self.rest.delete('user/watch/content', uploaded_attachment_id) except ConfluencePermissionError as ex: msg = ( @@ -862,7 +862,7 @@ def store_page(self, page_name, data, parent_id=None): new_page['ancestors'] = [{'id': parent_id}] try: - rsp = self.rest_client.post('content', new_page) + rsp = self.rest.post('content', new_page) if 'id' not in rsp: api_err = ( @@ -883,7 +883,7 @@ def store_page(self, page_name, data, parent_id=None): labels = new_page['metadata']['labels'] if not self.cloud and labels: url = f'content/{uploaded_page_id}/label' - self.rest_client.post(url, labels) + self.rest.post(url, labels) except ConfluenceBadApiError as ex: if str(ex).find('CDATA block has embedded') != -1: @@ -935,7 +935,7 @@ def store_page(self, page_name, data, parent_id=None): raise ConfluencePermissionError(msg) from ex if not self.watch: - self.rest_client.delete('user/watch/content', uploaded_page_id) + self.rest.delete('user/watch/content', uploaded_page_id) return uploaded_page_id @@ -988,7 +988,7 @@ def store_page_by_id(self, page_name, page_id, data): raise ConfluencePermissionError(msg) from ex if not self.watch: - self.rest_client.delete('user/watch/content', page_id) + self.rest.delete('user/watch/content', page_id) return page_id @@ -1005,7 +1005,7 @@ def store_page_properties(self, page_id, data): """ property_path = f'{page_id}/property/{PROP_KEY}' - self.rest_client.put('content', property_path, data) + self.rest.put('content', property_path, data) def remove_attachment(self, id_): """ @@ -1026,7 +1026,7 @@ def remove_attachment(self, id_): return try: - self.rest_client.delete('content', id_) + self.rest.delete('content', id_) except ConfluencePermissionError as ex: msg = ( 'Publish user does not have permission to delete ' @@ -1045,7 +1045,7 @@ def remove_page(self, page_id): try: try: - self.rest_client.delete('content', page_id) + self.rest.delete('content', page_id) except ConfluenceBadApiError as ex: if str(ex).find('Transaction rolled back') == -1: raise @@ -1054,7 +1054,7 @@ def remove_page(self, page_id): logger.warn('delete failed; retrying...') time.sleep(3) - self.rest_client.delete('content', page_id) + self.rest.delete('content', page_id) except ConfluenceBadApiError as ex: # Check if Confluence reports that this content does not exist. If @@ -1099,9 +1099,9 @@ def update_space_home(self, page_id): self._onlynew('space home updates restricted') return - page = self.rest_client.get('content/' + page_id, None) + page = self.rest.get('content/' + page_id, None) try: - self.rest_client.put('space', self.space_key, { + self.rest.put('space', self.space_key, { 'key': self.space_key, 'name': self.space_display_name, 'homepage': page, @@ -1206,7 +1206,7 @@ def _update_page(self, page, page_name, data, parent_id=None): page_id_explicit = page['id'] + '?status=current' try: - self.rest_client.put('content', page_id_explicit, update_page) + self.rest.put('content', page_id_explicit, update_page) except ConfluenceBadApiError as ex: if str(ex).find('CDATA block has embedded') != -1: raise ConfluenceUnexpectedCdataError from ex @@ -1241,7 +1241,7 @@ def _update_page(self, page, page_name, data, parent_id=None): time.sleep(3) try: - self.rest_client.put('content', page_id_explicit, update_page) + self.rest.put('content', page_id_explicit, update_page) except ConfluenceBadApiError as ex: if 'unreconciled' in str(ex): raise ConfluenceUnreconciledPageError( From b3721425d60588c2373482fa40c18069560555c2 Mon Sep 17 00:00:00 2001 From: James Knight Date: Mon, 4 Mar 2024 00:19:49 -0500 Subject: [PATCH 02/19] move api path prefixes into publisher This commit refactors the logic used when building API URLs to query. The original "bind path"/path-prefix logic would be added inside a Rest client whenever a request was prepared. However, to use Confluence's v2 API, this prefix varies (instead of `rest/api`, it is `api/v2`). This extension needs to be able to perform a mixed set of API calls, since both a series of v1/v2 APIs are available, select v1 deprecated APIs will no longer be available in the upcoming months (potential) and certain v1 active APIs are still required for some actions. To support various API paths, responsibility is now being placed inside the publisher when forming requests. The REST client class will assume the full path is provided via the URL configuration and the provided path value. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 70 +++++++++++-------- sphinxcontrib/confluencebuilder/rest.py | 9 +-- .../confluencebuilder/std/confluence.py | 4 +- sphinxcontrib/confluencebuilder/util.py | 8 +-- 4 files changed, 48 insertions(+), 43 deletions(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index f2f30fb9..0f1724a9 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -21,6 +21,7 @@ from sphinxcontrib.confluencebuilder.exceptions import ConfluenceUnreconciledPageError from sphinxcontrib.confluencebuilder.logger import ConfluenceLogger as logger from sphinxcontrib.confluencebuilder.rest import Rest +from sphinxcontrib.confluencebuilder.std.confluence import API_REST_V1 from sphinxcontrib.confluencebuilder.util import ConfluenceUtil import json import logging @@ -52,6 +53,12 @@ def init(self, config, cloud=None): self.space_key = config.confluence_space_key self.watch = config.confluence_watch + # track api prefix values to apply + if config.confluence_publish_disable_api_prefix: + self.APIV1 = '' + else: + self.APIV1 = f'{API_REST_V1}/' + # append labels by default if self.append_labels is None: self.append_labels = True @@ -68,7 +75,7 @@ def connect(self): server_url = self.config.confluence_server_url try: - rsp = self.rest.get(f'space/{self.space_key}') + rsp = self.rest.get(f'{self.APIV1}space/{self.space_key}') except ConfluenceBadApiError as ex: if ex.status_code == 404: pw_set = bool(self.config.confluence_server_pass) @@ -129,7 +136,7 @@ def archive_page(self, page_id): 'pages': [{'id': page_id}], } - rsp = self.rest.post('content/archive', data) + rsp = self.rest.post(f'{self.APIV1}content/archive', data) longtask_id = rsp['id'] # wait for the archiving of the page to complete @@ -138,7 +145,7 @@ def archive_page(self, page_id): while attempt <= MAX_WAIT_FOR_PAGE_ARCHIVE: time.sleep(0.5) - rsp = self.rest.get(f'longtask/{longtask_id}') + rsp = self.rest.get(f'{self.APIV1}longtask/{longtask_id}') if rsp['finished']: break @@ -173,7 +180,7 @@ def archive_pages(self, page_ids): # Note, multi-page archive can result in Confluence reporting the # following message: # Cannot use bulk archive feature for non premium edition - self.rest.post('content/archive', data) + self.rest.post(f'{self.APIV1}content/archive', data) except ConfluencePermissionError as ex: msg = ( @@ -222,7 +229,7 @@ def get_base_page_id(self): return base_page_id - rsp = self.rest.get('content', { + rsp = self.rest.get(f'{self.APIV1}content', { 'type': 'page', 'spaceKey': self.space_key, 'title': self.parent_ref, @@ -309,13 +316,13 @@ def _get_descendants(self, page_id, mode): the descendants """ - api_endpoint = 'content/search' + api_endpoint = f'{self.APIV1}content/search' descendants = set() search_fields = {} if page_id: if 'direct' in mode: - api_endpoint = f'content/{page_id}/descendant/page' + api_endpoint = f'{self.APIV1}content/{page_id}/descendant/page' else: search_fields['cql'] = f'ancestor={page_id}' else: @@ -396,7 +403,7 @@ def get_attachment(self, page_id, name): attachment = None attachment_id = None - url = f'content/{page_id}/child/attachment' + url = f'{self.APIV1}content/{page_id}/child/attachment' rsp = self.rest.get(url, { 'filename': name, }) @@ -423,7 +430,7 @@ def get_attachments(self, page_id): """ attachment_info = {} - url = f'content/{page_id}/child/attachment' + url = f'{self.APIV1}content/{page_id}/child/attachment' search_fields = {} # Configure a larger limit value than the default (no provided @@ -468,7 +475,7 @@ def get_page(self, page_name, expand='version', status='current'): page = None page_id = None - rsp = self.rest.get('content', { + rsp = self.rest.get(f'{self.APIV1}content', { 'type': 'page', 'spaceKey': self.space_key, 'title': page_name, @@ -500,7 +507,7 @@ def get_page_by_id(self, page_id, expand='version'): the page id and page object """ - page = self.rest.get(f'content/{page_id}', { + page = self.rest.get(f'{self.APIV1}content/{page_id}', { 'status': 'current', 'expand': expand, }) @@ -536,7 +543,7 @@ def get_page_case_insensitive(self, page_name): '" and type=page and title~"' + page_name + '"'} search_fields['limit'] = 1000 - rsp = self.rest.get('content/search', search_fields) + rsp = self.rest.get(f'{self.APIV1}content/search', search_fields) idx = 0 while rsp['size'] > 0: for result in rsp['results']: @@ -551,7 +558,8 @@ def get_page_case_insensitive(self, page_name): idx += int(rsp['limit']) sub_search_fields = dict(search_fields) sub_search_fields['start'] = idx - rsp = self.rest.get('content/search', sub_search_fields) + rsp = self.rest.get(f'{self.APIV1}content/search', + sub_search_fields) return page_id, page @@ -574,8 +582,8 @@ def get_page_properties(self, page_id, expand='version'): props = None try: - property_path = f'content/{page_id}/property/{PROP_KEY}' - props = self.rest.get(property_path, { + prop_path = f'{self.APIV1}content/{page_id}/property/{PROP_KEY}' + props = self.rest.get(prop_path, { 'status': 'current', 'expand': expand, }) @@ -660,7 +668,7 @@ def store_attachment(self, page_id, name, data, mimetype, hash_, force=False): data['minorEdit'] = 'true' if not attachment: - url = f'content/{page_id}/child/attachment' + url = f'{self.APIV1}content/{page_id}/child/attachment' try: rsp = self.rest.post(url, None, files=data) @@ -706,13 +714,13 @@ def store_attachment(self, page_id, name, data, mimetype, hash_, force=False): _, attachment = self.get_attachment(page_id, name) if attachment: - url = 'content/{}/child/attachment/{}/data'.format( - page_id, attachment['id']) + url = '{}content/{}/child/attachment/{}/data'.format( + self.APIV1, page_id, attachment['id']) rsp = self.rest.post(url, None, files=data) uploaded_attachment_id = rsp['id'] if not self.watch: - self.rest.delete('user/watch/content', + self.rest.delete(f'{self.APIV1}user/watch/content', uploaded_attachment_id) except ConfluencePermissionError as ex: msg = ( @@ -862,7 +870,7 @@ def store_page(self, page_name, data, parent_id=None): new_page['ancestors'] = [{'id': parent_id}] try: - rsp = self.rest.post('content', new_page) + rsp = self.rest.post(f'{self.APIV1}content', new_page) if 'id' not in rsp: api_err = ( @@ -882,7 +890,7 @@ def store_page(self, page_name, data, parent_id=None): # initial labels need to be applied in their own request labels = new_page['metadata']['labels'] if not self.cloud and labels: - url = f'content/{uploaded_page_id}/label' + url = f'{self.APIV1}content/{uploaded_page_id}/label' self.rest.post(url, labels) except ConfluenceBadApiError as ex: @@ -935,7 +943,8 @@ def store_page(self, page_name, data, parent_id=None): raise ConfluencePermissionError(msg) from ex if not self.watch: - self.rest.delete('user/watch/content', uploaded_page_id) + self.rest.delete(f'{self.APIV1}user/watch/content', + uploaded_page_id) return uploaded_page_id @@ -988,7 +997,7 @@ def store_page_by_id(self, page_name, page_id, data): raise ConfluencePermissionError(msg) from ex if not self.watch: - self.rest.delete('user/watch/content', page_id) + self.rest.delete(f'{self.APIV1}user/watch/content', page_id) return page_id @@ -1005,7 +1014,7 @@ def store_page_properties(self, page_id, data): """ property_path = f'{page_id}/property/{PROP_KEY}' - self.rest.put('content', property_path, data) + self.rest.put(f'{self.APIV1}content', property_path, data) def remove_attachment(self, id_): """ @@ -1026,7 +1035,7 @@ def remove_attachment(self, id_): return try: - self.rest.delete('content', id_) + self.rest.delete(f'{self.APIV1}content', id_) except ConfluencePermissionError as ex: msg = ( 'Publish user does not have permission to delete ' @@ -1045,7 +1054,7 @@ def remove_page(self, page_id): try: try: - self.rest.delete('content', page_id) + self.rest.delete(f'{self.APIV1}content', page_id) except ConfluenceBadApiError as ex: if str(ex).find('Transaction rolled back') == -1: raise @@ -1054,7 +1063,7 @@ def remove_page(self, page_id): logger.warn('delete failed; retrying...') time.sleep(3) - self.rest.delete('content', page_id) + self.rest.delete(f'{self.APIV1}content', page_id) except ConfluenceBadApiError as ex: # Check if Confluence reports that this content does not exist. If @@ -1099,7 +1108,7 @@ def update_space_home(self, page_id): self._onlynew('space home updates restricted') return - page = self.rest.get('content/' + page_id, None) + page = self.rest.get(f'{self.APIV1}content/{page_id}', None) try: self.rest.put('space', self.space_key, { 'key': self.space_key, @@ -1206,7 +1215,7 @@ def _update_page(self, page, page_name, data, parent_id=None): page_id_explicit = page['id'] + '?status=current' try: - self.rest.put('content', page_id_explicit, update_page) + self.rest.put(f'{self.APIV1}content', page_id_explicit, update_page) except ConfluenceBadApiError as ex: if str(ex).find('CDATA block has embedded') != -1: raise ConfluenceUnexpectedCdataError from ex @@ -1241,7 +1250,8 @@ def _update_page(self, page, page_name, data, parent_id=None): time.sleep(3) try: - self.rest.put('content', page_id_explicit, update_page) + self.rest.put(f'{self.APIV1}content', + page_id_explicit, update_page) except ConfluenceBadApiError as ex: if 'unreconciled' in str(ex): raise ConfluenceUnreconciledPageError( diff --git a/sphinxcontrib/confluencebuilder/rest.py b/sphinxcontrib/confluencebuilder/rest.py index db3e6bc5..0e2584f1 100644 --- a/sphinxcontrib/confluencebuilder/rest.py +++ b/sphinxcontrib/confluencebuilder/rest.py @@ -16,7 +16,6 @@ from sphinxcontrib.confluencebuilder.exceptions import ConfluenceSslError from sphinxcontrib.confluencebuilder.exceptions import ConfluenceTimeoutError from sphinxcontrib.confluencebuilder.logger import ConfluenceLogger as logger -from sphinxcontrib.confluencebuilder.std.confluence import API_REST_BIND_PATH from sphinxcontrib.confluencebuilder.std.confluence import NOCHECK from sphinxcontrib.confluencebuilder.std.confluence import RSP_HEADER_RETRY_AFTER from requests.adapters import HTTPAdapter @@ -164,7 +163,6 @@ class Rest: CONFLUENCE_DEFAULT_ENCODING = 'utf-8' def __init__(self, config): - self.bind_path = API_REST_BIND_PATH self.config = config self.last_retry = 1 self.next_delay = None @@ -176,9 +174,6 @@ def __init__(self, config): self.session = self._setup_session(config) - if config.confluence_publish_disable_api_prefix: - self.bind_path = '' - def __del__(self): if self.session: self.session.close() @@ -320,7 +315,7 @@ def _format_error(self, rsp, path): err = "" err += f"REQ: {rsp.request.method}\n" err += "RSP: " + str(rsp.status_code) + "\n" - err += "URL: " + self.url + self.bind_path + "\n" + err += "URL: " + self.url + "\n" err += "API: " + path + "\n" try: err += f'DATA: {json.dumps(rsp.json(), indent=2)}' @@ -331,7 +326,7 @@ def _format_error(self, rsp, path): def _process_request(self, method, path, *args, **kwargs): dump = PublishDebug.headers in self.config.confluence_publish_debug - rest_url = f'{self.url}{self.bind_path}/{path}' + rest_url = f'{self.url}{path}' base_req = requests.Request(method, rest_url, *args, **kwargs) req = self.session.prepare_request(base_req) diff --git a/sphinxcontrib/confluencebuilder/std/confluence.py b/sphinxcontrib/confluencebuilder/std/confluence.py index af2b0f40..0c4630a9 100644 --- a/sphinxcontrib/confluencebuilder/std/confluence.py +++ b/sphinxcontrib/confluencebuilder/std/confluence.py @@ -4,8 +4,8 @@ from sphinxcontrib.confluencebuilder.std.sphinx import DEFAULT_HIGHLIGHT_STYLE import os -# confluence trailing bind path for rest api -API_REST_BIND_PATH = 'rest/api' +# prefix for all api path requests to a confluence instance (v1 api) +API_REST_V1 = 'rest/api' # default width for a table (v2 editor) # diff --git a/sphinxcontrib/confluencebuilder/util.py b/sphinxcontrib/confluencebuilder/util.py index 66f71673..dd5358d7 100644 --- a/sphinxcontrib/confluencebuilder/util.py +++ b/sphinxcontrib/confluencebuilder/util.py @@ -3,7 +3,7 @@ from contextlib import contextmanager from pathlib import Path -from sphinxcontrib.confluencebuilder.std.confluence import API_REST_BIND_PATH +from sphinxcontrib.confluencebuilder.std.confluence import API_REST_V1 from sphinxcontrib.confluencebuilder.std.confluence import FONT_SIZE from sphinxcontrib.confluencebuilder.std.confluence import FONT_X_HEIGHT from hashlib import sha256 @@ -77,9 +77,9 @@ def normalize_base_url(url): # removing any trailing forward slash user provided if url.endswith('/'): url = url[:-1] - # check for rest bind path; strip and return if found - if url.endswith(API_REST_BIND_PATH): - url = url[:-len(API_REST_BIND_PATH)] + # check for rest api prefix; strip and return if found + if url.endswith(API_REST_V1): + url = url[:-len(API_REST_V1)] # restore trailing forward flash elif not url.endswith('/'): url += '/' From 74649d2ad3b6c97b3e70024f5fa6882312d5ebe2 Mon Sep 17 00:00:00 2001 From: James Knight Date: Mon, 4 Mar 2024 23:38:44 -0500 Subject: [PATCH 03/19] support initial api-modes Provides initial support to interact with a Confluence instance using REST v2 API. This updates the initial space search request to use a v2 API `spaces` for Confluence Cloud instances. Otherwise, it uses v1 for all other cases (unless explicitly overridden). This is a first step towards updating this extension to avoid the use of any deprecated API call. Since we now aim to support two types of API calls, the existing `confluence_publish_disable_api_prefix` configuration does not completely work as expected. Users may need a way to configure custom prefixes based on whether a call uses v2 API or v1 API (since an API mode may require using a mix of versions). To support this, introducing a new option called `confluence_publish_override_api_prefix`, which allows a user to tailor specific prefix values to use per API request type being performed. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/__init__.py | 8 ++- .../confluencebuilder/config/checks.py | 19 ++++++ .../confluencebuilder/config/defaults.py | 10 ++++ .../confluencebuilder/config/exceptions.py | 11 ++++ .../confluencebuilder/config/notifications.py | 2 + sphinxcontrib/confluencebuilder/publisher.py | 58 ++++++++++++++++--- .../confluencebuilder/std/confluence.py | 9 +++ tests/unit-tests/test_config_checks.py | 16 +++++ ...d_path.py => test_publisher_api_prefix.py} | 52 ++++++++++++++--- 9 files changed, 168 insertions(+), 17 deletions(-) rename tests/unit-tests/{test_publisher_api_bind_path.py => test_publisher_api_prefix.py} (52%) diff --git a/sphinxcontrib/confluencebuilder/__init__.py b/sphinxcontrib/confluencebuilder/__init__.py index 2715e5b1..7d53a9f7 100644 --- a/sphinxcontrib/confluencebuilder/__init__.py +++ b/sphinxcontrib/confluencebuilder/__init__.py @@ -115,6 +115,8 @@ def setup(app): cm.add_conf_bool('singleconfluence_toctree', 'singleconfluence') # (configuration - publishing) + # API mode to use for REST calls. + cm.add_conf('confluence_api_mode') # Request for publish password to come from interactive session. cm.add_conf_bool('confluence_ask_password') # Request for publish username to come from interactive session. @@ -195,12 +197,12 @@ def setup(app): cm.add_conf('confluence_publish_denylist') # Whether to check for changes on remote before publishing. cm.add_conf_bool('confluence_publish_force') - # Disable adding `rest/api` to REST requests. - cm.add_conf_bool('confluence_publish_disable_api_prefix') # Header(s) to use for Confluence REST interaction. cm.add_conf('confluence_publish_headers') # Whether to publish a generated intersphinx database to the root document cm.add_conf_bool('confluence_publish_intersphinx') + # Override the path prefixes for various REST API requests. + cm.add_conf('confluence_publish_override_api_prefix') # Manipulate a requests instance. cm.add_conf('confluence_request_session_override') # Authentication passthrough for Confluence REST interaction. @@ -263,6 +265,8 @@ def setup(app): cm.add_conf_bool('confluence_adv_permit_raw_html') # replaced by confluence_root_homepage cm.add_conf('confluence_master_homepage') + # replaced by confluence_publish_override_api_prefix + cm.add_conf_bool('confluence_publish_disable_api_prefix') # replaced by confluence_publish_allowlist cm.add_conf('confluence_publish_subset') # replaced by confluence_purge_from_root diff --git a/sphinxcontrib/confluencebuilder/config/checks.py b/sphinxcontrib/confluencebuilder/config/checks.py index 920dc80d..2afd30db 100644 --- a/sphinxcontrib/confluencebuilder/config/checks.py +++ b/sphinxcontrib/confluencebuilder/config/checks.py @@ -2,6 +2,7 @@ # Copyright Sphinx Confluence Builder Contributors (AUTHORS) from pathlib import Path +from sphinxcontrib.confluencebuilder.config.exceptions import ConfluenceApiModeConfigError from sphinxcontrib.confluencebuilder.config.exceptions import ConfluenceCleanupSearchModeConfigError from sphinxcontrib.confluencebuilder.config.exceptions import ConfluenceClientCertBadTupleConfigError from sphinxcontrib.confluencebuilder.config.exceptions import ConfluenceClientCertMissingCertConfigError @@ -37,6 +38,7 @@ from sphinxcontrib.confluencebuilder.config.notifications import warnings from sphinxcontrib.confluencebuilder.config.validation import ConfigurationValidation from sphinxcontrib.confluencebuilder.debug import PublishDebug +from sphinxcontrib.confluencebuilder.std.confluence import API_MODES from sphinxcontrib.confluencebuilder.std.confluence import EDITORS from sphinxcontrib.confluencebuilder.util import handle_cli_file_subset from requests.auth import AuthBase @@ -112,6 +114,17 @@ def validate_configuration(builder): # ################################################################## + # confluence_api_mode + validator.conf('confluence_api_mode') \ + .string() + + if config.confluence_api_mode: + if config.confluence_api_mode not in API_MODES: + modes = '\n - '.join(API_MODES) + raise ConfluenceApiModeConfigError(modes) + + # ################################################################## + # confluence_append_labels validator.conf('confluence_append_labels') \ .bool() @@ -539,6 +552,12 @@ def conf_translate(value): # ################################################################## + # confluence_publish_orphan_container + validator.conf('confluence_publish_override_api_prefix') \ + .dict_str_str() + + # ################################################################## + # confluence_publish_postfix validator.conf('confluence_publish_postfix') \ .string() diff --git a/sphinxcontrib/confluencebuilder/config/defaults.py b/sphinxcontrib/confluencebuilder/config/defaults.py index 1b99ca73..611a5a82 100644 --- a/sphinxcontrib/confluencebuilder/config/defaults.py +++ b/sphinxcontrib/confluencebuilder/config/defaults.py @@ -108,6 +108,16 @@ def apply_defaults(builder): if conf.confluence_publish_orphan is None: conf.confluence_publish_orphan = True + if conf.confluence_publish_override_api_prefix is None: + # confluence_publish_disable_api_prefix is deprecated, but we will + # use its presence to configure v1 api for old config support + if conf.confluence_publish_disable_api_prefix: + conf.confluence_publish_override_api_prefix = { + 'v1': '', + } + else: + conf.confluence_publish_override_api_prefix = {} + if conf.confluence_remove_title is None: conf.confluence_remove_title = True diff --git a/sphinxcontrib/confluencebuilder/config/exceptions.py b/sphinxcontrib/confluencebuilder/config/exceptions.py index 45440167..7fe0c13a 100644 --- a/sphinxcontrib/confluencebuilder/config/exceptions.py +++ b/sphinxcontrib/confluencebuilder/config/exceptions.py @@ -9,6 +9,17 @@ class ConfluenceConfigError(ConfluenceError, ConfigError): pass +class ConfluenceApiModeConfigError(ConfluenceConfigError): + def __init__(self, modes): + super().__init__(f'''\ +invalid api version provided in confluence_api_mode + +The following API modes are supported: + + - {modes} +''') + + class ConfluenceCleanupSearchModeConfigError(ConfluenceConfigError): def __init__(self, msg): super().__init__(f'''\ diff --git a/sphinxcontrib/confluencebuilder/config/notifications.py b/sphinxcontrib/confluencebuilder/config/notifications.py index d5128234..8d7765f1 100644 --- a/sphinxcontrib/confluencebuilder/config/notifications.py +++ b/sphinxcontrib/confluencebuilder/config/notifications.py @@ -22,6 +22,8 @@ 'option does nothing', 'confluence_parent_page_id_check': '"confluence_parent_page" now accepts a page id', + 'confluence_publish_disable_api_prefix': + 'use "confluence_publish_override_api_prefix" instead', 'confluence_publish_subset': 'use "confluence_publish_allowlist" instead', 'confluence_purge_from_master': diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index 0f1724a9..0098c6e3 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -22,6 +22,7 @@ from sphinxcontrib.confluencebuilder.logger import ConfluenceLogger as logger from sphinxcontrib.confluencebuilder.rest import Rest from sphinxcontrib.confluencebuilder.std.confluence import API_REST_V1 +from sphinxcontrib.confluencebuilder.std.confluence import API_REST_V2 from sphinxcontrib.confluencebuilder.util import ConfluenceUtil import json import logging @@ -54,10 +55,20 @@ def init(self, config, cloud=None): self.watch = config.confluence_watch # track api prefix values to apply - if config.confluence_publish_disable_api_prefix: - self.APIV1 = '' + prefix_overrides = config.confluence_publish_override_api_prefix or {} + self.APIV1 = prefix_overrides.get('v1', f'{API_REST_V1}/') + self.APIV2 = prefix_overrides.get('v2', f'{API_REST_V2}/') + + # determine api mode to use + # - if an explicit api mode is configured, use it + # - if this is a cloud instance, use v2 + # - for all other cases, use v1 + if config.confluence_api_mode: + self.api_mode = config.confluence_api_mode + elif self.cloud: + self.api_mode = 'v2' else: - self.APIV1 = f'{API_REST_V1}/' + self.api_mode = 'v1' # append labels by default if self.append_labels is None: @@ -74,13 +85,46 @@ def connect(self): self.rest = Rest(self.config) server_url = self.config.confluence_server_url + # Example space fetch points: + # https://sphinxcontrib-confluencebuilder.atlassian.net/wiki/rest/api/space/STABLE + # https://sphinxcontrib-confluencebuilder.atlassian.net/wiki/api/v2/spaces?keys=STABLE + + pw_set = bool(self.config.confluence_server_pass) + token_set = bool(self.config.confluence_publish_token) + try: - rsp = self.rest.get(f'{self.APIV1}space/{self.space_key}') + if self.api_mode == 'v2': + spaces_url = f'{self.APIV2}spaces' + rsp_spaces = self.rest.get(spaces_url, { + 'keys': self.space_key, + 'limit': 1, + }) + + # if no size entry is provided, this a non-Confluence instance + if 'results' not in rsp_spaces: + raise ConfluenceBadServerUrlError(server_url, + 'server provided an unexpected response; no results') + + # handle if the provided space key was not found + if len(rsp_spaces['results']) == 1: + rsp = rsp_spaces['results'][0] + else: + raise ConfluenceUnknownInstanceError( + server_url, + self.space_key, + self.config.confluence_server_user, + pw_set, + token_set, + ) + else: + rsp = self.rest.get(f'{self.APIV1}space/{self.space_key}') except ConfluenceBadApiError as ex: if ex.status_code == 404: - pw_set = bool(self.config.confluence_server_pass) - token_set = bool(self.config.confluence_publish_token) - + # if this is a 404 (not found), give a more custom message + # since on an initial connect, this may be either that the + # instance url is wrong, the space could not be found since + # the key is wrong or that the user does not have permission + # to see that the space exists raise ConfluenceUnknownInstanceError( server_url, self.space_key, diff --git a/sphinxcontrib/confluencebuilder/std/confluence.py b/sphinxcontrib/confluencebuilder/std/confluence.py index 0c4630a9..7a918936 100644 --- a/sphinxcontrib/confluencebuilder/std/confluence.py +++ b/sphinxcontrib/confluencebuilder/std/confluence.py @@ -7,6 +7,15 @@ # prefix for all api path requests to a confluence instance (v1 api) API_REST_V1 = 'rest/api' +# prefix for all api path requests to a confluence instance (v2 api) +API_REST_V2 = 'api/v2' + +# list of supported api modes to operate with +API_MODES = { + 'v1', + 'v2', +} + # default width for a table (v2 editor) # # It has been observed that when attempting to fix specific column widths on diff --git a/tests/unit-tests/test_config_checks.py b/tests/unit-tests/test_config_checks.py index 6f4ba231..18340b30 100644 --- a/tests/unit-tests/test_config_checks.py +++ b/tests/unit-tests/test_config_checks.py @@ -831,6 +831,22 @@ def test_config_check_publish_orphan_container(self): with self.assertRaises(ConfluenceConfigError): self._try_config() + def test_config_check_publish_override_api_prefix(self): + self.config['confluence_publish_override_api_prefix'] = {} + self._try_config() + + self.config['confluence_publish_override_api_prefix'] = { + 'v1': 'override1/', + 'v2': 'override2/', + } + self._try_config() + + self.config['confluence_publish_override_api_prefix'] = { + 'v2': None, + } + with self.assertRaises(ConfluenceConfigError): + self._try_config() + def test_config_check_publish_postfix(self): self.config['confluence_publish_postfix'] = '' self._try_config() diff --git a/tests/unit-tests/test_publisher_api_bind_path.py b/tests/unit-tests/test_publisher_api_prefix.py similarity index 52% rename from tests/unit-tests/test_publisher_api_bind_path.py rename to tests/unit-tests/test_publisher_api_prefix.py index 9a2f9b77..c5f3f26f 100644 --- a/tests/unit-tests/test_publisher_api_bind_path.py +++ b/tests/unit-tests/test_publisher_api_prefix.py @@ -8,7 +8,7 @@ import unittest -class TestConfluencePublisherApiBindPath(unittest.TestCase): +class TestConfluencePublisherApiPrefix(unittest.TestCase): @classmethod def setUpClass(cls): cls.config = prepare_conf_publisher() @@ -21,8 +21,14 @@ def setUpClass(cls): 'type': 'global', } - def test_publisher_api_bind_path_default(self): - """validate publisher includes api bind path""" + cls.std_spaces_connect_rsp = { + 'results': [ + cls.std_space_connect_rsp, + ], + } + + def test_publisher_api_prefix_default(self): + """validate publisher includes an api prefix""" # # Verify that a publisher will perform requests which target the # `/rest/api` endpoint. @@ -40,14 +46,17 @@ def test_publisher_api_bind_path_default(self): req_path, _ = connect_req self.assertTrue('/rest/api/' in req_path) - def test_publisher_api_bind_path_disabled(self): - """validate publisher can disable api bind path""" + def test_publisher_api_prefix_override_v1(self): + """validate publisher can disable a v1 api prefix""" # # Verify that a publisher can perform requests disables the use of - # the `/rest/api` endpoint. + # the `rest/api` prefix. config = self.config.clone() - config.confluence_publish_disable_api_prefix = True + config.confluence_api_mode = 'v1' + config.confluence_publish_override_api_prefix = { + 'v1': 'my-custom-v1/', + } with mock_confluence_instance(config) as daemon, \ autocleanup_publisher(ConfluencePublisher) as publisher: @@ -60,4 +69,31 @@ def test_publisher_api_bind_path_disabled(self): connect_req = daemon.pop_get_request() self.assertIsNotNone(connect_req) req_path, _ = connect_req - self.assertTrue('/rest/api/' not in req_path) + self.assertFalse('/rest/api/' in req_path) + self.assertTrue('/my-custom-v1/' in req_path) + + def test_publisher_api_prefix_override_v2(self): + """validate publisher can disable a v2 api prefix""" + # + # Verify that a publisher can perform requests disables the use of + # the `api/v2` prefix. + + config = self.config.clone() + config.confluence_api_mode = 'v2' + config.confluence_publish_override_api_prefix = { + 'v2': 'my-custom-v2/', + } + + with mock_confluence_instance(config) as daemon, \ + autocleanup_publisher(ConfluencePublisher) as publisher: + daemon.register_get_rsp(200, self.std_spaces_connect_rsp) + + publisher.init(config) + publisher.connect() + + # connect request + connect_req = daemon.pop_get_request() + self.assertIsNotNone(connect_req) + req_path, _ = connect_req + self.assertFalse('/api/v2/' in req_path) + self.assertTrue('/my-custom-v2/' in req_path) From d49deb06a03b56ed6a53d565f4ebc00c0231172b Mon Sep 17 00:00:00 2001 From: James Knight Date: Mon, 4 Mar 2024 23:40:44 -0500 Subject: [PATCH 04/19] Documentation: document new api and prefix options Adds documentation for the recently added `confluence_api_mode` and `confluence_publish_disable_api_prefix` options. Signed-off-by: James Knight --- doc/configuration.rst | 71 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 60 insertions(+), 11 deletions(-) diff --git a/doc/configuration.rst b/doc/configuration.rst index d68d828f..00445cd7 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -489,6 +489,26 @@ Generic configuration Publishing configuration ------------------------ +.. |confluence_api_mode| replace:: ``confluence_api_mode`` +.. _confluence_api_mode: + +.. confval:: confluence_api_mode + + Configures the API mode to use for REST requests. Certain Confluence + instances support a newer version of REST APIs (e.g. Confluence Cloud). + This extension will attempt to use an appropriate API mode for a + configuration set. However, a user can override the operating API mode + based on preference or when handling situations where this extension + cannot automatically determine the best API mode to use. Values + accepted are either ``v1`` or ``v2``. + + .. code-block:: python + + confluence_api_mode = 'v2' + + By default, if a Confluence Cloud configuration is detected, this + extension will use ``v2``. For all other cases, the default is ``v1``. + .. |confluence_ask_password| replace:: ``confluence_ask_password`` .. _confluence_ask_password: @@ -1478,17 +1498,6 @@ Advanced publishing configuration See also |confluence_publish_allowlist|_. -.. confval:: confluence_publish_disable_api_prefix - - A boolean value which explicitly disables the use of the ``rest/api`` in - the Confluence publish URL. This can be useful for environments where the - API endpoint for a Confluence instance is proxied through a non-standard - location. By default, API prefixes are enabled with a value of ``False``. - - .. code-block:: python - - confluence_publish_disable_api_prefix = True - .. |confluence_publish_dryrun| replace:: ``confluence_publish_dryrun`` .. _confluence_publish_dryrun: @@ -1622,6 +1631,40 @@ Advanced publishing configuration See also |confluence_publish_orphan|_. +.. |confluence_publish_override_api_prefix| replace:: ``confluence_publish_override_api_prefix`` +.. _confluence_publish_override_api_prefix: + +.. confval:: confluence_publish_override_api_prefix + + .. versionadded:: 2.5 + + Allows a user to override the path-prefix value used for API requests. + API paths are commonly prefixed, such as ``rest/api/`` for API v1 and + ``api/v2/`` for API v2. However, if a user is interacting with a Confluence + instance which system administrators have configured non-standard + locations for API endpoints, requests made by this extension will fail. + + To support custom API endpoint paths, this option can be used to indicate + what prefix to use, if any. By default, this extension operates with an + API prefix configuration matching the following: + + .. code-block:: python + + confluence_publish_override_api_prefix = { + 'v1': 'rest/api/', + 'v2': 'api/v2/', + } + + Users may define a dictionary using |confluence_api_mode|_ values for + keys, followed by a prefix override for their environment. For example, + to disable prefixes for any API v1 request, the following may be used: + + .. code-block:: python + + confluence_publish_override_api_prefix = { + 'v1': '', + } + .. confval:: confluence_parent_override_transform .. versionadded:: 2.2 @@ -2071,6 +2114,12 @@ Deprecated options See also |confluence_parent_page|_. +.. confval:: confluence_publish_disable_api_prefix + + .. versionchanged:: 2.5 + + This option has been replaced by |confluence_publish_override_api_prefix|_. + .. confval:: confluence_publish_subset .. versionchanged:: 1.3 From 950b3adfcb1f07b97154e7667017c5be5fe37c0c Mon Sep 17 00:00:00 2001 From: James Knight Date: Mon, 4 Mar 2024 23:44:37 -0500 Subject: [PATCH 05/19] publisher: add predefined rest client attribute Adds a `rest` client attribute by default. This allows a user of this class to invoke `disconnect` without having to have worried about first invoking `connect`. This change is primarily for debugging purposes, to avoid the chances of throwing an attribute exception when a test's connect stage is unable to be run (but a cleanup operation is invoked anyways). Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index 0098c6e3..e361e43d 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -44,6 +44,8 @@ def __init__(self): def init(self, config, cloud=None): self.cloud = cloud self.config = config + self.rest = None + self.append_labels = config.confluence_append_labels self.dryrun = config.confluence_publish_dryrun self.notify = not config.confluence_disable_notifications @@ -164,7 +166,8 @@ def connect(self): self.space_type = rsp['type'] def disconnect(self): - self.rest.close() + if self.rest: + self.rest.close() def archive_page(self, page_id): if self.dryrun: From 9c488eeca08211771d33d126aad9169d56f767d7 Mon Sep 17 00:00:00 2001 From: James Knight Date: Thu, 7 Mar 2024 22:30:09 -0500 Subject: [PATCH 06/19] publisher: support ancestor fetch using api v2 Updating the logic to find ancestors to use the REST API v2 when configured for a 'v2' API mode. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index e361e43d..494f0edb 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -253,11 +253,17 @@ def get_ancestors(self, page_id): assert page_id ancestors = set() - _, page = self.get_page_by_id(page_id, 'ancestors') + if self.api_mode == 'v2': + rsp = self.rest.get(f'{self.APIV2}pages/{page_id}/ancestors') - if 'ancestors' in page: - for ancestor in page['ancestors']: - ancestors.add(ancestor['id']) + for result in rsp['results']: + ancestors.add(result['id']) + else: + _, page = self.get_page_by_id(page_id, 'ancestors') + + if 'ancestors' in page: + for ancestor in page['ancestors']: + ancestors.add(ancestor['id']) return ancestors From b7a9ca40105a7ce59a7da842de8b94020c436192 Mon Sep 17 00:00:00 2001 From: James Knight Date: Thu, 7 Mar 2024 22:33:14 -0500 Subject: [PATCH 07/19] publisher: adjust select single page fetch calls to use api v2 Updating a select number of calls that perform a single page fetch (using an ID) to use the REST API v2 when configured for a 'v2' API mode. This commit only updates a few cases. There will need to be some plumbing fixes to address other page fetches where originally we could also request ancestor/labels/etc., but now looks to require separate calls. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index 494f0edb..df468285 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -560,10 +560,13 @@ def get_page_by_id(self, page_id, expand='version'): the page id and page object """ - page = self.rest.get(f'{self.APIV1}content/{page_id}', { - 'status': 'current', - 'expand': expand, - }) + if self.api_mode == 'v2': + page = self.rest.get(f'{self.APIV2}pages/{page_id}') + else: + page = self.rest.get(f'{self.APIV1}content/{page_id}', { + 'status': 'current', + 'expand': expand, + }) if page: assert int(page_id) == int(page['id']) @@ -1161,7 +1164,10 @@ def update_space_home(self, page_id): self._onlynew('space home updates restricted') return - page = self.rest.get(f'{self.APIV1}content/{page_id}', None) + if self.api_mode == 'v2': + page = self.rest.get(f'{self.APIV2}pages/{page_id}') + else: + page = self.rest.get(f'{self.APIV1}content/{page_id}', None) try: self.rest.put('space', self.space_key, { 'key': self.space_key, From 80d309225f10060dd580f5960dedea6c6a2c7676 Mon Sep 17 00:00:00 2001 From: James Knight Date: Thu, 7 Mar 2024 22:41:25 -0500 Subject: [PATCH 08/19] publisher: correct homepage update call to use api prefix Updates adding prefixes into the publisher [1] failed to update the put call used to update a space's homepage; correcting. [1]: d920ee75a2deda0291f09595f49981a2075988a3 Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index df468285..d98f5dab 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -1169,7 +1169,7 @@ def update_space_home(self, page_id): else: page = self.rest.get(f'{self.APIV1}content/{page_id}', None) try: - self.rest.put('space', self.space_key, { + self.rest.put(f'{self.APIV1}space', self.space_key, { 'key': self.space_key, 'name': self.space_display_name, 'homepage': page, From 25867bcdea8ed03710f24183f5956088db539308 Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 17 Mar 2024 19:23:54 -0400 Subject: [PATCH 09/19] publisher: drop use of size/limit Confluence's original REST API provided results-based responses that include a `size` and `limit` fields. For Confluence v2 API calls, such fields do not exist and can be inferred by using the requested limit and the size of the results array. Updating all API calls to drop the use of `size` and `limit` fields to provide flexibility no matter what version of API is used. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 37 ++++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index d98f5dab..6b1e1a4b 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -29,6 +29,10 @@ import time +# number of elements to fetch for bulk requests +# (Confluence v2 APIs indicate a max of 250; a good enough number as any) +BULK_LIMIT = 250 + # key used for managing this extension's properties on a Confluence instance PROP_KEY = 'sphinx' @@ -288,7 +292,7 @@ def get_base_page_id(self): 'title': self.parent_ref, 'status': 'current', }) - if rsp['size'] == 0: + if not rsp['results']: msg = 'Configured parent page name does not exist.' raise ConfluenceConfigError(msg) page = rsp['results'][0] @@ -385,19 +389,20 @@ def _get_descendants(self, page_id, mode): # Configure a larger limit value than the default (no provided # limit defaults to 25). This should reduce the number of queries # needed to fetch a complete descendants set (for larger sets). - search_fields['limit'] = 1000 + search_fields['limit'] = BULK_LIMIT rsp = self.rest.get(api_endpoint, search_fields) idx = 0 - while rsp['size'] > 0: + while rsp['results']: for result in rsp['results']: descendants.add(result['id']) self._name_cache[result['id']] = result['title'] - if rsp['size'] != rsp['limit']: + count = len(rsp['results']) + if count != BULK_LIMIT: break - idx += int(rsp['limit']) + idx += count sub_search_fields = dict(search_fields) sub_search_fields['start'] = idx rsp = self.rest.get(api_endpoint, sub_search_fields) @@ -461,7 +466,7 @@ def get_attachment(self, page_id, name): 'filename': name, }) - if rsp['size'] != 0: + if rsp['results']: attachment = rsp['results'][0] attachment_id = attachment['id'] self._name_cache[attachment_id] = name @@ -489,19 +494,20 @@ def get_attachments(self, page_id): # Configure a larger limit value than the default (no provided # limit defaults to 25). This should reduce the number of queries # needed to fetch a complete attachment set (for larger sets). - search_fields['limit'] = 1000 + search_fields['limit'] = BULK_LIMIT rsp = self.rest.get(url, search_fields) idx = 0 - while rsp['size'] > 0: + while rsp['results']: for result in rsp['results']: attachment_info[result['id']] = result['title'] self._name_cache[result['id']] = result['title'] - if rsp['size'] != rsp['limit']: + count = len(rsp['results']) + if count != BULK_LIMIT: break - idx += int(rsp['limit']) + idx += count sub_search_fields = dict(search_fields) sub_search_fields['start'] = idx rsp = self.rest.get(url, sub_search_fields) @@ -536,7 +542,7 @@ def get_page(self, page_name, expand='version', status='current'): 'expand': expand, }) - if rsp['size'] != 0: + if rsp['results']: page = rsp['results'][0] page_id = page['id'] self._name_cache[page_id] = page_name @@ -597,21 +603,22 @@ def get_page_case_insensitive(self, page_name): page_name = page_name.lower() search_fields = {'cql': 'space="' + self.space_key + '" and type=page and title~"' + page_name + '"'} - search_fields['limit'] = 1000 + search_fields['limit'] = BULK_LIMIT rsp = self.rest.get(f'{self.APIV1}content/search', search_fields) idx = 0 - while rsp['size'] > 0: + while rsp['results']: for result in rsp['results']: result_title = result['title'] if page_name == result_title.lower(): page_id, page = self.get_page(result_title) break - if page_id or rsp['size'] != rsp['limit']: + count = len(rsp['results']) + if page_id or count != BULK_LIMIT: break - idx += int(rsp['limit']) + idx += count sub_search_fields = dict(search_fields) sub_search_fields['start'] = idx rsp = self.rest.get(f'{self.APIV1}content/search', From 4fb22e228356cb4b841a46d7d880e5c69f0063df Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 17 Mar 2024 19:28:27 -0400 Subject: [PATCH 10/19] publisher: support v2 api with attachment fetching Updating the calls which fetch attachment data to support both API v1 and v2. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 22 ++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index 6b1e1a4b..291e62b8 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -461,7 +461,11 @@ def get_attachment(self, page_id, name): attachment = None attachment_id = None - url = f'{self.APIV1}content/{page_id}/child/attachment' + if self.api_mode == 'v2': + url = f'{self.APIV2}pages/{page_id}/attachments' + else: + url = f'{self.APIV1}content/{page_id}/child/attachment' + rsp = self.rest.get(url, { 'filename': name, }) @@ -488,7 +492,11 @@ def get_attachments(self, page_id): """ attachment_info = {} - url = f'{self.APIV1}content/{page_id}/child/attachment' + if self.api_mode == 'v2': + url = f'{self.APIV2}pages/{page_id}/attachments' + else: + url = f'{self.APIV1}content/{page_id}/child/attachment' + search_fields = {} # Configure a larger limit value than the default (no provided @@ -687,10 +695,12 @@ def store_attachment(self, page_id, name, data, mimetype, hash_, force=False): # check if attachment (of same hash) is already published to this page comment = None - if attachment and 'metadata' in attachment: - metadata = attachment['metadata'] - if 'comment' in metadata: - comment = metadata['comment'] + if attachment: + if self.api_mode == 'v2': + comment = attachment.get('comment') + elif 'metadata' in attachment: + metadata = attachment['metadata'] + comment = metadata.get('comment') if not force and comment: parts = comment.split(HASH_KEY + ':', 1) From 52c36054481891ab2e39cd22bef4c6331d07c050 Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 17 Mar 2024 19:32:46 -0400 Subject: [PATCH 11/19] publisher: support retry on attachment delete transaction failure Support the ability to retry the deletion an attachment if a transaction failure is detected (in the same manner done if a page delete failed). Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index 291e62b8..0585b0d7 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -1107,8 +1107,21 @@ def remove_attachment(self, id_): self._onlynew('attachment removal restricted', id_) return + delete_path = f'{self.APIV1}content' + try: - self.rest.delete(f'{self.APIV1}content', id_) + try: + self.rest.delete(delete_path, id_) + except ConfluenceBadApiError as ex: + if str(ex).find('Transaction rolled back') == -1: + raise + + with skip_warningiserror(): + logger.warn('delete failed; retrying...') + time.sleep(3) + + self.rest.delete(delete_path, id_) + except ConfluencePermissionError as ex: msg = ( 'Publish user does not have permission to delete ' From 80ad3a6c627a8821378e7e3ff2884112330e75b4 Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 17 Mar 2024 19:34:07 -0400 Subject: [PATCH 12/19] publisher: support v2 api with deleting pages/attachments Updating the calls which delete pages and attachments to support both API v1 and v2. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index 0585b0d7..ab7309d1 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -1107,7 +1107,10 @@ def remove_attachment(self, id_): self._onlynew('attachment removal restricted', id_) return - delete_path = f'{self.APIV1}content' + if self.api_mode == 'v2': + delete_path = f'{self.APIV2}attachments' + else: + delete_path = f'{self.APIV1}content' try: try: @@ -1138,9 +1141,14 @@ def remove_page(self, page_id): self._onlynew('page removal restricted', page_id) return + if self.api_mode == 'v2': + delete_path = f'{self.APIV2}pages' + else: + delete_path = f'{self.APIV1}content' + try: try: - self.rest.delete(f'{self.APIV1}content', page_id) + self.rest.delete(delete_path, page_id) except ConfluenceBadApiError as ex: if str(ex).find('Transaction rolled back') == -1: raise @@ -1149,7 +1157,7 @@ def remove_page(self, page_id): logger.warn('delete failed; retrying...') time.sleep(3) - self.rest.delete(f'{self.APIV1}content', page_id) + self.rest.delete(delete_path, page_id) except ConfluenceBadApiError as ex: # Check if Confluence reports that this content does not exist. If From a1172e3ebab1898a0961ce4e7d5de2510c6bd867 Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 17 Mar 2024 19:36:28 -0400 Subject: [PATCH 13/19] cleanup spelling Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index ab7309d1..cbc9040c 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -1181,7 +1181,7 @@ def restrict_ancestors(self, ancestors): Registers the provided set of ancestors from being used when page updates will move the location of a page. This is a pre-check update - requests so that a page cannot be flagged as a descendant of itsel + requests so that a page cannot be flagged as a descendant of itself (where Confluence self-hosted instances may not report an ideal error message). From fd49ba01af0b93adbe191937c81b263dc0addfb4 Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 17 Mar 2024 19:46:48 -0400 Subject: [PATCH 14/19] publisher: introduce a post-page action call Perform a little refactoring where we manage un-watch updates from creating/updating a space into a specific call, and adjust the page update events to call this post-action method. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 23 +++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index cbc9040c..3bce576c 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -1015,9 +1015,8 @@ def store_page(self, page_name, data, parent_id=None): ) raise ConfluencePermissionError(msg) from ex - if not self.watch: - self.rest.delete(f'{self.APIV1}user/watch/content', - uploaded_page_id) + # perform any required post-page update actions + self._post_page_actions(uploaded_page_id) return uploaded_page_id @@ -1069,8 +1068,8 @@ def store_page_by_id(self, page_name, page_id, data): ) raise ConfluencePermissionError(msg) from ex - if not self.watch: - self.rest.delete(f'{self.APIV1}user/watch/content', page_id) + # perform any required post-page update actions + self._post_page_actions(page_id) return page_id @@ -1264,6 +1263,20 @@ def _build_page(self, page_name, data): return page + def _post_page_actions(self, page_id): + """ + post page actions + + Perform additional actions needed after creating or updating a page. + + Args: + page_id: the identifier of the new/updated page + """ + + # ensure remove any watch flags on the update if watching is disabled + if not self.watch: + self.rest.delete(f'{self.APIV1}user/watch/content', page_id) + def _update_page(self, page, page_name, data, parent_id=None): """ build a page update and publish it to the confluence instance From 6ff67f229192c39767f963c334330b330db83cd7 Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 17 Mar 2024 19:57:44 -0400 Subject: [PATCH 15/19] publisher: update page property management to support v2 api Reworking this extension's property key usage when storing extension-specific data on Confluence pages. This update provides support for both v1 and v2 REST APIs. These changes also perform two minor changes: - The property key name has been changed from `sphinx` to `sphinxcontrib.confluencebuilder`, to ensure a unique ID for this extension. - Hash caching on pages was only applied to generic page store calls, but not when a store call was made to a specific base ID. Updating the implementation to process hash tracking the same in both calls. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 118 +++++++++++++------ 1 file changed, 79 insertions(+), 39 deletions(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index 3bce576c..fa9a90ce 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -34,7 +34,7 @@ BULK_LIMIT = 250 # key used for managing this extension's properties on a Confluence instance -PROP_KEY = 'sphinx' +CB_PROP_KEY = 'sphinxcontrib.confluencebuilder' class ConfluencePublisher: @@ -634,33 +634,42 @@ def get_page_case_insensitive(self, page_name): return page_id, page - def get_page_properties(self, page_id, expand='version'): + def get_page_property(self, page_id, key, default=None): """ - get properties from the provided page id + get a property from the provided page id - Performs an API call to acquire known properties about a specific page. + Performs an API call to acquire a property held on a specific page. This call can returns the page properties dictionary if found; otherwise ``None`` will be returned. Args: page_id: the page identifier - expand (optional): data to expand on + key: the property key + default (optional): default value if no property exists Returns: - the properties + the property value """ - props = None + props = { + 'value': default, + } - try: - prop_path = f'{self.APIV1}content/{page_id}/property/{PROP_KEY}' - props = self.rest.get(prop_path, { - 'status': 'current', - 'expand': expand, - }) - except ConfluenceBadApiError as ex: - if ex.status_code != 404: - raise + if page_id: + try: + if self.api_mode == 'v2': + prop_path = f'{self.APIV2}pages/{page_id}/properties' + rsp = self.rest.get(prop_path, { + 'key': key, + }) + if rsp['results']: + props = rsp['results'][0] + else: + prop_path = f'{self.APIV1}content/{page_id}/property/{key}' + props = self.rest.get(prop_path) + except ConfluenceBadApiError as ex: + if ex.status_code != 404: + raise return props @@ -871,7 +880,8 @@ def store_page(self, page_name, data, parent_id=None): return page['id'] # fetch known properties (associated with this extension) from the page - props = self.get_page_properties(page['id']) if page else None + page_id = page['id'] if page else None + cb_props = self.get_page_property(page_id, CB_PROP_KEY, {}) # calculate the hash for a page; we will first use this to check if # there is a update to apply, and if we do need to update, we will @@ -927,8 +937,8 @@ def store_page(self, page_name, data, parent_id=None): # if we are not force uploading, check if the new page hash matches # the remote hash; if so, do not publish - if props and not force_publish: - remote_hash = props.get('value', {}).get('hash') + if cb_props and not force_publish: + remote_hash = cb_props.get('value', {}).get('hash') if new_page_hash == remote_hash: logger.verbose(f'no changes in page: {page_name}') return page['id'] @@ -994,20 +1004,6 @@ def store_page(self, page_name, data, parent_id=None): self._update_page(page, page_name, data, parent_id=parent_id) uploaded_page_id = page['id'] - if not props: - props = { - 'value': {}, - 'version': { - 'number': 1, - }, - } - else: - last_props_version = int(props['version']['number']) # pylint: disable=unsubscriptable-object - props['version']['number'] = last_props_version + 1 # pylint: disable=unsubscriptable-object - - props['value']['hash'] = new_page_hash - self.store_page_properties(uploaded_page_id, props) - except ConfluencePermissionError as ex: msg = ( 'Publish user does not have permission to add page ' @@ -1015,8 +1011,11 @@ def store_page(self, page_name, data, parent_id=None): ) raise ConfluencePermissionError(msg) from ex + # update page hash + cb_props['value']['hash'] = new_page_hash + # perform any required post-page update actions - self._post_page_actions(uploaded_page_id) + self._post_page_actions(uploaded_page_id, cb_props) return uploaded_page_id @@ -1059,6 +1058,22 @@ def store_page_by_id(self, page_name, page_id, data): raise raise ConfluenceMissingPageIdError(self.space_key, page_id) from ex + # fetch known properties (associated with this extension) from the page + cb_props = self.get_page_property(page_id, CB_PROP_KEY, {}) + + # calculate the hash for a page; we will first use this to check if + # there is a update to apply, and if we do need to update, we will + # add this value into the page's properties + new_page_hash = ConfluenceUtil.hash(data['content']) + + # if we are not force uploading, check if the new page hash matches + # the remote hash; if so, do not publish + if cb_props and not self.config.confluence_publish_force: + remote_hash = cb_props.get('value', {}).get('hash') + if new_page_hash == remote_hash: + logger.verbose(f'no changes in page: {page_name}') + return page_id + try: self._update_page(page, page_name, data) except ConfluencePermissionError as ex: @@ -1068,12 +1083,15 @@ def store_page_by_id(self, page_name, page_id, data): ) raise ConfluencePermissionError(msg) from ex + # update page hash + cb_props['value']['hash'] = new_page_hash + # perform any required post-page update actions - self._post_page_actions(page_id) + self._post_page_actions(page_id, cb_props) return page_id - def store_page_properties(self, page_id, data): + def store_page_property(self, page_id, key, data): """ request to store properties on a page to a confluence instance @@ -1082,11 +1100,28 @@ def store_page_properties(self, page_id, data): Args: page_id: the id of the page to update + key: the property key data: the properties data to apply """ - property_path = f'{page_id}/property/{PROP_KEY}' - self.rest.put(f'{self.APIV1}content', property_path, data) + # set or bump the known version on this property + prop_version = data.setdefault('version', {}) + last_props_version = int(prop_version.get('number', 0)) + prop_version['number'] = last_props_version + 1 + + if self.api_mode == 'v2': + # v2 api expects the key in the body + data['key'] = key + property_path = f'{self.APIV2}pages/{page_id}/properties' + + if prop_version['number'] == 1: + self.rest.post(property_path, data) + else: + prop_id = data.pop('id') + self.rest.put(property_path, prop_id, data) + else: + property_path = f'{self.APIV1}content/{page_id}/property' + self.rest.put(property_path, key, data) def remove_attachment(self, id_): """ @@ -1263,7 +1298,7 @@ def _build_page(self, page_name, data): return page - def _post_page_actions(self, page_id): + def _post_page_actions(self, page_id, cb_props): """ post page actions @@ -1271,8 +1306,13 @@ def _post_page_actions(self, page_id): Args: page_id: the identifier of the new/updated page + cb_props: confluence builder page properties """ + # push an updated to confluence builder property which includes an + # updated hash value + self.store_page_property(page_id, CB_PROP_KEY, cb_props) + # ensure remove any watch flags on the update if watching is disabled if not self.watch: self.rest.delete(f'{self.APIV1}user/watch/content', page_id) From 4c4cf07f276fc97692ba419dc8d2cf59c18230ab Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 17 Mar 2024 20:01:36 -0400 Subject: [PATCH 16/19] publisher: rework get-base-page-id to use common get-page call When a request to `get_base_page_id` results in using a page title to find the base page, it did so using a custom REST call in the method. However, a call to `get_page` would achieve the same results. Refactoring the call to use `get_page` to simplify the implementation. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index fa9a90ce..2be1dc29 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -277,6 +277,7 @@ def get_base_page_id(self): if not self.parent_ref: return base_page_id + # fetching a base page by a numerical identifier if isinstance(self.parent_ref, int): base_page_id, page = self.get_page_by_id(self.parent_ref) @@ -286,20 +287,17 @@ def get_base_page_id(self): return base_page_id - rsp = self.rest.get(f'{self.APIV1}content', { - 'type': 'page', - 'spaceKey': self.space_key, - 'title': self.parent_ref, - 'status': 'current', - }) - if not rsp['results']: + # fetching a base page by a page-name identifier + base_page_id, page = self.get_page(self.parent_ref) + + if not page: msg = 'Configured parent page name does not exist.' raise ConfluenceConfigError(msg) - page = rsp['results'][0] - if self.parent_id and page['id'] != str(self.parent_id): + + if self.parent_id and base_page_id != str(self.parent_id): msg = 'Configured parent page ID and name do not match.' raise ConfluenceConfigError(msg) - base_page_id = page['id'] + self._name_cache[base_page_id] = self.parent_ref if not base_page_id and self.parent_id: From 9b4eee11e8913d961f7be2f04916f444b2afe041 Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 17 Mar 2024 20:05:33 -0400 Subject: [PATCH 17/19] publisher: complete initial v2 api support Adds the final implementation to drop the use of deprecated API v1 calls (on Confluence Cloud) to use v2 API calls instead. This does perform some major tweaks to the page fetch and update calls, where certain actions such as ancestor and label processing needing to be performed in their own requests. Signed-off-by: James Knight --- sphinxcontrib/confluencebuilder/publisher.py | 278 ++++++++++++++++--- 1 file changed, 245 insertions(+), 33 deletions(-) diff --git a/sphinxcontrib/confluencebuilder/publisher.py b/sphinxcontrib/confluencebuilder/publisher.py index 2be1dc29..7ea06ca6 100644 --- a/sphinxcontrib/confluencebuilder/publisher.py +++ b/sphinxcontrib/confluencebuilder/publisher.py @@ -41,6 +41,7 @@ class ConfluencePublisher: def __init__(self): self.cloud = None self.space_display_name = None + self.space_id = None self.space_type = None self._ancestors_cache = set() self._name_cache = {} @@ -167,6 +168,7 @@ def connect(self): # track required space information self.space_display_name = rsp['name'] + self.space_id = rsp['id'] self.space_type = rsp['type'] def disconnect(self): @@ -540,19 +542,68 @@ def get_page(self, page_name, expand='version', status='current'): page = None page_id = None - rsp = self.rest.get(f'{self.APIV1}content', { - 'type': 'page', - 'spaceKey': self.space_key, - 'title': page_name, - 'status': status, - 'expand': expand, - }) + if self.api_mode == 'v2': + rsp = self.rest.get(f'{self.APIV2}pages', { + 'body-format': 'storage', + 'space-id': self.space_id, + 'status': status, + 'title': page_name, + }) + else: + rsp = self.rest.get(f'{self.APIV1}content', { + 'type': 'page', + 'spaceKey': self.space_key, + 'title': page_name, + 'status': status, + 'expand': expand, + }) if rsp['results']: page = rsp['results'][0] page_id = page['id'] self._name_cache[page_id] = page_name + # if `expand` is set and this is a v2 API request, perform additional + # queries for various options requested; we will emulate the response + # observed in a v1 request (by populating a page with additional data; + # which we later need to strip if updating a page) + if page_id and self.api_mode == 'v2': + assert 'ancestors' not in page + assert 'metadata' not in page + + opts = expand.split(',') + metadata = page.setdefault('metadata', {}) + meta_props = metadata.setdefault('properties', {}) + + if 'ancestors' in opts: + rsp = self.rest.get(f'{self.APIV2}pages/{page_id}/ancestors', { + 'limit': BULK_LIMIT, + }) + page['ancestors'] = rsp['results'] + + if 'metadata.labels' in opts: + rsp = self.rest.get(f'{self.APIV2}pages/{page_id}/labels', { + 'limit': BULK_LIMIT, + }) + + metadata['labels'] = rsp + + props_to_fetch = [] + + # if certain properties are request, ensure we generate a + # request to fetch these values; we will populate the "legacy" + # metadata field for processed, but also keep track of these + # properties for possible updates + if 'metadata.properties.content_appearance_published' in opts: + props_to_fetch.append('content-appearance-published') + + if 'metadata.properties.editor' in opts: + props_to_fetch.append('editor') + + for prop_key in props_to_fetch: + prop_entry = self.get_page_property(page_id, prop_key) + meta_props[prop_key] = prop_entry + return page_id, page def get_page_by_id(self, page_id, expand='version'): @@ -945,35 +996,44 @@ def store_page(self, page_name, data, parent_id=None): # new page if not page: new_page = self._build_page(page_name, data) + self._populate_labels(new_page, data['labels']) - if parent_id: - new_page['ancestors'] = [{'id': parent_id}] + new_labels = None + new_prop_requests = [] + if self.api_mode == 'v2': + # use newer space id refrence for v2 + new_page.pop('space', None) + new_page['spaceId'] = self.space_id - try: - rsp = self.rest.post(f'{self.APIV1}content', new_page) + # strip out metadata updates that need to be processed + # in a different request + new_metadata = new_page.pop('metadata', None) + new_labels = new_metadata.get('labels') + new_meta_props = new_metadata.setdefault('properties', {}) - if 'id' not in rsp: - api_err = ( - 'Confluence reports a successful page ' - 'creation; however, provided no identifier.\n\n' - ) - try: - api_err += 'DATA: {}'.format(json.dumps( - rsp, indent=2)) - except TypeError: - api_err += 'DATA: ' - raise ConfluenceBadApiError(-1, api_err) # noqa: TRY301 + for prop_key, entry in new_meta_props.items(): + new_prop = { + 'key': prop_key, + 'value': entry['value'], + } - uploaded_page_id = rsp['id'] + new_prop_requests.append(new_prop) - # if we have labels and this is a non-cloud instance, - # initial labels need to be applied in their own request - labels = new_page['metadata']['labels'] - if not self.cloud and labels: - url = f'{self.APIV1}content/{uploaded_page_id}/label' - self.rest.post(url, labels) + # configure parent page for this new page + if parent_id: + if self.api_mode == 'v2': + new_page['parentId'] = parent_id + else: + new_page['ancestors'] = [{'id': parent_id}] + try: + if self.api_mode == 'v2': + build_path = f'{self.APIV2}pages' + else: + build_path = f'{self.APIV1}content' + + rsp = self.rest.post(build_path, new_page) except ConfluenceBadApiError as ex: if str(ex).find('CDATA block has embedded') != -1: raise ConfluenceUnexpectedCdataError from ex @@ -996,6 +1056,37 @@ def store_page(self, page_name, data, parent_id=None): if self.onlynew: self._onlynew('skipping existing page', page['id']) return page['id'] + else: + if 'id' not in rsp: + api_err = ( + 'Confluence reports a successful page ' + 'creation; however, provided no identifier.\n\n' + ) + try: + json_data = json.dumps(rsp, indent=2) + api_err += f'DATA: {json_data}' + except TypeError: + api_err += 'DATA: (not-or-invalid-json)' + raise ConfluenceBadApiError(-1, api_err) + + uploaded_page_id = rsp['id'] + + # we have properties we would like to apply, but we cannot + # just create new ones if Confluence already created ones + # implicitly in the new page update -- we will need to + # query the page for any of these properties are form + # either new or update requests + self._update_page_properties(rsp['id'], new_prop_requests) + + # if we have labels and this is a non-cloud instance, + # initial labels need to be applied in their own request + if not self.cloud and self.api_mode == 'v1': + new_labels = new_page['metadata']['labels'] + + # add new labels if we have any to force add + if new_labels: + url = f'{self.APIV1}content/{uploaded_page_id}/label' + self.rest.post(url, new_labels) # update existing page if page: @@ -1089,6 +1180,61 @@ def store_page_by_id(self, page_name, page_id, data): return page_id + def _update_page_properties(self, page_id, properties): + """ + update properties on a specific page + + Perform a request to update properties on a page. This call will + fetch an existing property value to determine if an update is + required. + + Args: + page_id: the id of the page to update + properties: the properties to update + """ + + # we have properties we would like to apply, but we cannot + # just create new ones if Confluence already created ones + # implicitly in the new page update -- we will need to + # query the page for any of these properties are form + # either new or update requests + for prop in properties: + # we permit two attempts to update a property as it has been + # observed when we create a new page, Confluence may build a + # desired property that an initial `get_page_property` will not + # return (timing issue); so if we get a conflict error (409), + # we will retry a fetch/update again + MAX_ATTEMPTS_TO_UPDATE_PROPERTY = 2 + attempt = 1 + while attempt <= MAX_ATTEMPTS_TO_UPDATE_PROPERTY: + prop_key = prop['key'] + prop_entry = self.get_page_property(page_id, prop_key) + + # ignore if the property already matches the desired + # value + if prop_entry['value'] == prop['value']: + break + + prop_entry['value'] = prop['value'] + + try: + self.store_page_property( + page_id, + prop_key, + prop_entry, + ) + except ConfluenceBadApiError as ex: + if ex.status_code != 409: + raise + + # retry on conflict + with skip_warningiserror(): + logger.warn('property update conflict; retrying...') + + attempt += 1 + else: + break + def store_page_property(self, page_id, key, data): """ request to store properties on a page to a confluence instance @@ -1361,9 +1507,66 @@ def _update_page(self, page, page_name, data, parent_id=None): elif parent_id is not None: update_page['ancestors'] = [{'id': '1'}] - page_id_explicit = page['id'] + '?status=current' + # if this is an api v2 mode, prepare any extra requests needed for + # populating ancestors or metadata information + pending_new_labels = [] + pending_prop_requests = [] + if self.api_mode == 'v2': + orig_metadata = page.get('metadata', None) + update_metadata = update_page.pop('metadata', None) + + # configure parent page for this page update + # + # For v2, we need to strip out `ancestors` and place it with an + # expected `parentId` value. + ancestors_request = update_page.pop('ancestors', None) + if ancestors_request: + update_page['parentId'] = ancestors_request[0]['id'] + + # extract labels to set + pending_new_labels = update_metadata.get('labels') + + # build a list of property creation/update requests needed + # + # This call will look at the `update_page` page for newly set + # properties needed to be set. When cycling through updated + # properties, we will also compare against the original page's + # properties (`page`) to see if properties are being created or + # updated, where we can track the identifier for updated entries + # as well as pre-populating a version bump. This will also check + # if each property value to be updated has changed. If there is + # no change to a given property, it will be ignored. + orig_meta_props = orig_metadata.setdefault('properties', {}) + update_meta_props = update_metadata.setdefault('properties', {}) + + for prop_key, entry in update_meta_props.items(): + updated_prop = { + 'key': prop_key, + 'value': entry['value'], + 'version': { + 'number': 1, + }, + } + + orig_entry = orig_meta_props.get(prop_key, None) + if orig_entry: + if orig_entry['value'] == entry['value']: + continue + + updated_prop['id'] = orig_entry['id'] + last_props_version = int(orig_entry['version']['number']) + updated_prop['version']['number'] = last_props_version + 1 + + pending_prop_requests.append(updated_prop) + + if self.api_mode == 'v2': + update_path = f'{self.APIV2}pages' + else: + update_path = f'{self.APIV1}content' + + update_page['status'] = 'current' try: - self.rest.put(f'{self.APIV1}content', page_id_explicit, update_page) + self.rest.put(update_path, page['id'], update_page) except ConfluenceBadApiError as ex: if str(ex).find('CDATA block has embedded') != -1: raise ConfluenceUnexpectedCdataError from ex @@ -1398,8 +1601,7 @@ def _update_page(self, page, page_name, data, parent_id=None): time.sleep(3) try: - self.rest.put(f'{self.APIV1}content', - page_id_explicit, update_page) + self.rest.put(update_path, page['id'], update_page) except ConfluenceBadApiError as ex: if 'unreconciled' in str(ex): raise ConfluenceUnreconciledPageError( @@ -1407,6 +1609,16 @@ def _update_page(self, page, page_name, data, parent_id=None): raise + # post-update requests (api v2 mode) + update_page_id = update_page['id'] + + self._update_page_properties(update_page_id, pending_prop_requests) + + # add any new labels + if pending_new_labels: + url = f'{self.APIV1}content/{update_page_id}/label' + self.rest.post(url, pending_new_labels) + def _dryrun(self, msg, id_=None, misc=''): """ log a dry run mode message From cdb32cb0355020eec7c49d23fc804287b2af5266 Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 17 Mar 2024 20:07:16 -0400 Subject: [PATCH 18/19] tests: tweak publish page tests for new property calls Changes have been made to the base page processing to ensure a hash value is cached in its properties. Updating the publish check calls to included expected requests/responses that would now be generated from this change. Signed-off-by: James Knight --- tests/unit-tests/test_publisher_page.py | 65 +++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/unit-tests/test_publisher_page.py b/tests/unit-tests/test_publisher_page.py index fc5af45f..199aa9bc 100644 --- a/tests/unit-tests/test_publisher_page.py +++ b/tests/unit-tests/test_publisher_page.py @@ -2,6 +2,7 @@ # Copyright Sphinx Confluence Builder Contributors (AUTHORS) from collections import defaultdict +from sphinxcontrib.confluencebuilder.publisher import CB_PROP_KEY from sphinxcontrib.confluencebuilder.publisher import ConfluencePublisher from tests.lib import autocleanup_publisher from tests.lib import mock_confluence_instance @@ -56,9 +57,31 @@ def test_publisher_page_store_page_id_allow_watch(self): } daemon.register_get_rsp(200, page_fetch_rsp) + # prepare response for properties fetch + scb_fetch_props_rsp = { + 'id': '1234', + 'key': CB_PROP_KEY, + 'value': {}, + 'version': { + 'number': '1', + }, + } + daemon.register_get_rsp(200, scb_fetch_props_rsp) + # prepare response for update event daemon.register_put_rsp(200, dict(page_fetch_rsp)) + # prepare response for updated properties + scb_updated_props_rsp = { + 'id': '1234', + 'key': CB_PROP_KEY, + 'value': {}, + 'version': { + 'number': '2', + }, + } + daemon.register_put_rsp(200, scb_updated_props_rsp) + # perform page update request data = defaultdict(str) data['content'] = 'dummy page data' @@ -80,6 +103,16 @@ def test_publisher_page_store_page_id_allow_watch(self): update_req = daemon.pop_put_request() self.assertIsNotNone(update_req) + # check that the property request on the page was done + props_fetch_req = daemon.pop_get_request() + self.assertIsNotNone(props_fetch_req) + + # check that the page property (e.g. hash update) was made + update_req = daemon.pop_put_request() + self.assertIsNotNone(update_req) + req_path, _ = update_req + self.assertTrue(req_path.endswith(CB_PROP_KEY)) + # verify that no other request was made daemon.check_unhandled_requests() @@ -114,9 +147,31 @@ def test_publisher_page_store_page_id_default(self): } daemon.register_get_rsp(200, page_fetch_rsp) + # prepare response for properties fetch + scb_fetch_props_rsp = { + 'id': '1234', + 'key': CB_PROP_KEY, + 'value': {}, + 'version': { + 'number': '1', + }, + } + daemon.register_get_rsp(200, scb_fetch_props_rsp) + # prepare response for update event daemon.register_put_rsp(200, dict(page_fetch_rsp)) + # prepare response for updated properties + scb_updated_props_rsp = { + 'id': '1234', + 'key': CB_PROP_KEY, + 'value': {}, + 'version': { + 'number': '2', + }, + } + daemon.register_put_rsp(200, scb_updated_props_rsp) + # prepare response for unwatch event daemon.register_delete_rsp(200) @@ -141,6 +196,16 @@ def test_publisher_page_store_page_id_default(self): update_req = daemon.pop_put_request() self.assertIsNotNone(update_req) + # check that the property request on the page was done + props_fetch_req = daemon.pop_get_request() + self.assertIsNotNone(props_fetch_req) + + # check that the page property (e.g. hash update) was made + update_req = daemon.pop_put_request() + self.assertIsNotNone(update_req) + req_path, _ = update_req + self.assertTrue(req_path.endswith(CB_PROP_KEY)) + # check that the page is unwatched unwatch_req = daemon.pop_delete_request() self.assertIsNotNone(unwatch_req) From 6860c3789bff302fbf0402ad28f19f574d0ee900 Mon Sep 17 00:00:00 2001 From: James Knight Date: Sun, 17 Mar 2024 20:08:40 -0400 Subject: [PATCH 19/19] tests: enforce deprecated publish checks in validation When performing validation testing, force enable deprecated API checks to inform developers if a used API call is deprecated sooner than later. Signed-off-by: James Knight --- tests/test_validation.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_validation.py b/tests/test_validation.py index 8655be60..0da99044 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -40,6 +40,7 @@ def setUpClass(cls): cls.config['confluence_parent_page'] = None cls.config['confluence_prev_next_buttons_location'] = 'both' cls.config['confluence_publish'] = True + cls.config['confluence_publish_debug'] = 'deprecated' cls.config['confluence_server_pass'] = os.getenv(AUTH_ENV_KEY) cls.config['confluence_server_url'] = DEFAULT_TEST_URL cls.config['confluence_server_user'] = DEFAULT_TEST_USER