Skip to content

Commit

Permalink
move api path prefixes into publisher
Browse files Browse the repository at this point in the history
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 <james.d.knight@live.com>
  • Loading branch information
jdknight committed Mar 4, 2024
1 parent 8bc75b1 commit d920ee7
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 43 deletions.
70 changes: 40 additions & 30 deletions sphinxcontrib/confluencebuilder/publisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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 = (
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
})
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
})
Expand Down Expand Up @@ -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']:
Expand All @@ -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

Expand All @@ -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,
})
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 = (
Expand Down Expand Up @@ -863,7 +871,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 = (
Expand All @@ -883,7 +891,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:
Expand Down Expand Up @@ -936,7 +944,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

Expand Down Expand Up @@ -989,7 +998,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

Expand All @@ -1006,7 +1015,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_):
"""
Expand All @@ -1027,7 +1036,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 '
Expand All @@ -1046,7 +1055,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
Expand All @@ -1055,7 +1064,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
Expand Down Expand Up @@ -1100,7 +1109,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,
Expand Down Expand Up @@ -1207,7 +1216,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
Expand Down Expand Up @@ -1242,7 +1251,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(
Expand Down
9 changes: 2 additions & 7 deletions sphinxcontrib/confluencebuilder/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -174,9 +172,6 @@ def __init__(self, config):
self.verbosity = config.sphinx_verbosity
self._reported_large_delay = False

if config.confluence_publish_disable_api_prefix:
self.bind_path = ''

def __del__(self):
self.session.close()

Expand Down Expand Up @@ -316,7 +311,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)}'
Expand All @@ -327,7 +322,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)

Expand Down
4 changes: 2 additions & 2 deletions sphinxcontrib/confluencebuilder/std/confluence.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
#
Expand Down
8 changes: 4 additions & 4 deletions sphinxcontrib/confluencebuilder/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 += '/'
Expand Down

0 comments on commit d920ee7

Please sign in to comment.