From cc0b9fe42a2f7d251d97abddb269e16c3f71b9b7 Mon Sep 17 00:00:00 2001 From: "Mark T. Holder" Date: Thu, 20 Jul 2023 13:12:42 -0500 Subject: [PATCH] Bug fix - collections were not saving. Minor refactoring of the logic for getting the auth_token from the request in the process of fixing this bug. --- phylesystem_api/phylesystem_api/api_utils.py | 45 ++++++++++++++----- .../phylesystem_api/views/amendment.py | 36 ++++----------- .../phylesystem_api/views/collection.py | 34 +++----------- .../phylesystem_api/views/default.py | 4 +- .../phylesystem_api/views/study.py | 6 +-- 5 files changed, 54 insertions(+), 71 deletions(-) diff --git a/phylesystem_api/phylesystem_api/api_utils.py b/phylesystem_api/phylesystem_api/api_utils.py index a6e04368..a6772bf0 100644 --- a/phylesystem_api/phylesystem_api/api_utils.py +++ b/phylesystem_api/phylesystem_api/api_utils.py @@ -331,6 +331,12 @@ def read_logging_config(request): logging_filepath = None return level, logging_format_name, logging_filepath +def _raise_missing_auth_token(): + raise HTTPBadRequest(json.dumps({ + "error": 1, + "description":"You must provide an auth_token to authenticate to the OpenTree API" + })) + def authenticate(request): """Verify that we received a valid Github authentication token @@ -345,13 +351,10 @@ def authenticate(request): """ # this is the GitHub API auth-token for a logged-in curator - auth_token = find_in_request(request, 'auth_token', '') + auth_token = find_in_request(request, 'auth_token', '') if not auth_token: - raise HTTPBadRequest(json.dumps({ - "error": 1, - "description":"You must provide an auth_token to authenticate to the OpenTree API" - })) + _raise_missing_auth_token() gh = Github(auth_token) gh_user = gh.get_user() auth_info = {} @@ -367,6 +370,7 @@ def authenticate(request): # generate API calls regardless of author_name/author_email being specifed auth_info['name'] = find_in_request(request, 'author_name', gh_user.name) auth_info['email'] = find_in_request(request, 'author_email', gh_user.email) + auth_info['auth_token'] = auth_token return auth_info ''' ## using logging module directly @@ -613,7 +617,7 @@ def call_http_json(url, return resp.status_code, resp.json() -def deferred_push_to_gh_call(request, resource_id, doc_type='nexson', **kwargs): +def deferred_push_to_gh_call(request, resource_id, doc_type='nexson', auth_token=None): ##TODO Thius needs to create a bare URL for collections, and pass in the resource id etc as data #_LOG.debug("deferred_push_to_gh_call") if READ_ONLY_MODE: @@ -626,10 +630,9 @@ def deferred_push_to_gh_call(request, resource_id, doc_type='nexson', **kwargs): else: data = {} url = compose_push_to_github_url(request, resource_id, doc_type) - auth_token = copy.copy(kwargs.get('auth_token')) - if auth_token is not None: - data['auth_token'] = auth_token - assert data.get('auth_token') + if not auth_token: + _raise_missing_auth_token() + data['auth_token'] = auth_token #call_http_json(url=url, verb='PUT', data=data) threading.Thread(target=call_http_json, args=(url, 'PUT', data,)).start() @@ -712,3 +715,25 @@ def remove_tags(markup): markup = markup.decode('utf-8') return markup +def extract_json_from_http_call(request, data_field_name='data', request_params=None): + """Returns the json blob (as a deserialized object) from `request_params` or the request.body. + + request_params can be the Pyramids request.params multidict or just a dict. + """ + json_obj = None + try: + # check for kwarg data_field_name, or load the full request body + if data_field_name in request_params: + json_obj = request_params.get(data_field_name, {}) + else: + json_obj = request.json_body + + if not isinstance(json_obj, dict): + json_obj = json.loads(json_obj) + if data_field_name in json_obj: + json_obj = json_obj[data_field_name] + except: + # _LOG = api_utils.get_logger(request, 'ot_api.default.v1') + # _LOG.exception('Exception getting JSON content in extract_json_from_http_call') + raise HTTPBadRequest(body=json.dumps({"error": 1, "description": 'no collection JSON found in request'})) + return json_obj \ No newline at end of file diff --git a/phylesystem_api/phylesystem_api/views/amendment.py b/phylesystem_api/phylesystem_api/views/amendment.py index e0c55820..78d329d1 100644 --- a/phylesystem_api/phylesystem_api/views/amendment.py +++ b/phylesystem_api/phylesystem_api/views/amendment.py @@ -13,7 +13,7 @@ from peyotl.api import OTI from peyotl.phylesystem.git_workflows import GitWorkflowError import phylesystem_api.api_utils as api_utils -from phylesystem_api.api_utils import find_in_request +from phylesystem_api.api_utils import find_in_request, extract_json_from_http_call import json import logging @@ -25,30 +25,10 @@ _LOG = logging.getLogger('phylesystem_api') -def __extract_json_from_http_call(request, data_field_name='data', **kwargs): - """Returns the json blob (as a deserialized object) from `kwargs` or the request.body""" - json_obj = None - try: - # check for kwarg data_field_name, or load the full request body - if data_field_name in kwargs: - json_obj = kwargs.get(data_field_name, {}) - else: - json_obj = request.json_body - - if not isinstance(json_obj, dict): - json_obj = json.loads(json_obj) - if data_field_name in json_obj: - json_obj = json_obj[data_field_name] - except: - # _LOG = api_utils.get_logger(request, 'ot_api.default.v1') - # _LOG.exception('Exception getting JSON content in __extract_json_from_http_call') - raise HTTPBadRequest(body=json.dumps({"error": 1, "description": 'no collection JSON found in request'})) - return json_obj - -def __extract_and_validate_amendment(request, **kwargs): +def __extract_and_validate_amendment(request, kwargs): from pprint import pprint try: - amendment_obj = __extract_json_from_http_call(request, data_field_name='json', **kwargs) + amendment_obj = extract_json_from_http_call(request, data_field_name='json', request_params=kwargs) except HTTPException as err: # payload not found return None, None, None @@ -98,7 +78,7 @@ def create_amendment(request, **kwargs): phylesystem = api_utils.get_phylesystem(request) # set READONLY flag before testing! api_utils.raise_if_read_only() # fetch and parse the JSON payload, if any - amendment_obj, amendment_errors, amendment_adapter = __extract_and_validate_amendment(request, **kwargs) + amendment_obj, amendment_errors, amendment_adapter = __extract_and_validate_amendment(request, kwargs) if (amendment_obj is None): raise HTTPBadRequest(body=json.dumps({"error": 1, "description": "amendment JSON expected for HTTP method {}".format(request.method) })) @@ -122,7 +102,7 @@ def create_amendment(request, **kwargs): # _LOG.debug('add_new_amendment failed with error code') raise HTTPBadRequest(body=json.dumps(commit_return)) _LOG.debug("create ammendemt deferred_push_to_gh_call") - api_utils.deferred_push_to_gh_call(request, new_amendment_id, doc_type='amendment', **request.params) + api_utils.deferred_push_to_gh_call(request, new_amendment_id, doc_type='amendment', auth_token=auth_info['auth_token']) return commit_return @@ -206,7 +186,7 @@ def update_amendment(request): commit_msg = None # fetch and parse the JSON payload, if any - amendment_obj, amendment_errors, amendment_adapter = __extract_and_validate_amendment(request, **kwargs) + amendment_obj, amendment_errors, amendment_adapter = __extract_and_validate_amendment(request, kwargs) if (amendment_obj is None): raise HTTPBadRequest(body=json.dumps({"error": 1, "description": "amendment JSON expected for HTTP method {}".format(request.method) })) @@ -234,7 +214,7 @@ def update_amendment(request): # check for 'merge needed'? mn = commit_return.get('merge_needed') if (mn is not None) and (not mn): - api_utils.deferred_push_to_gh_call(request, amendment_id, doc_type='amendment', **request.json_body) + api_utils.deferred_push_to_gh_call(request, amendment_id, doc_type='amendment', auth_token=auth_info['auth_token']) return commit_return @@ -275,7 +255,7 @@ def delete_amendment(request): parent_sha, commit_msg=commit_msg) if x.get('error') == 0: - api_utils.deferred_push_to_gh_call(request, None, doc_type='amendment', **request.json_body) + api_utils.deferred_push_to_gh_call(request, None, doc_type='amendment', auth_token=auth_info['auth_token']) return x except GitWorkflowError as err: raise HTTPBadRequest(body=err.msg) diff --git a/phylesystem_api/phylesystem_api/views/collection.py b/phylesystem_api/phylesystem_api/views/collection.py index 664660b3..9c12bd48 100644 --- a/phylesystem_api/phylesystem_api/views/collection.py +++ b/phylesystem_api/phylesystem_api/views/collection.py @@ -14,7 +14,7 @@ from peyotl.api import OTI from peyotl.phylesystem.git_workflows import GitWorkflowError import phylesystem_api.api_utils as api_utils -from phylesystem_api.api_utils import find_in_request +from phylesystem_api.api_utils import find_in_request, extract_json_from_http_call import json from peyotl.collections_store import OWNER_ID_PATTERN, \ COLLECTION_ID_PATTERN @@ -23,31 +23,10 @@ _LOG = logging.getLogger('phylesystem_api') - -def __extract_json_from_http_call(request, data_field_name='data', **kwargs): - """Returns the json blob (as a deserialized object) from `kwargs` or the request.body""" - json_obj = None - try: - # check for kwarg data_field_name, or load the full request body - if data_field_name in kwargs: - json_obj = kwargs.get(data_field_name, {}) - else: - json_obj = request.json_body - - if not isinstance(json_obj, dict): - json_obj = json.loads(json_obj) - if data_field_name in json_obj: - json_obj = json_obj[data_field_name] - except: - # _LOG = api_utils.get_logger(request, 'ot_api.default.v1') - # _LOG.exception('Exception getting JSON content in __extract_json_from_http_call') - raise HTTPBadRequest(body=json.dumps({"error": 1, "description": 'no collection JSON found in request'})) - return json_obj - -def __extract_and_validate_collection(request, **kwargs): +def __extract_and_validate_collection(request, request_params): from pprint import pprint try: - collection_obj = __extract_json_from_http_call(request, data_field_name='json', **kwargs) + collection_obj = extract_json_from_http_call(request, data_field_name='json', request_params=request_params) except HTTPException as err: # payload not found return None, None, None @@ -137,7 +116,7 @@ def create_collection(request): if commit_return['error'] != 0: # _LOG.debug('add_new_collection failed with error code') raise HTTPBadRequest(json.dumps(commit_return)) - api_utils.deferred_push_to_gh_call(request, new_collection_id, doc_type='collection', **request.params) + api_utils.deferred_push_to_gh_call(request, new_collection_id, doc_type='collection', auth_token=auth_info['auth_token']) return commit_return @view_config(route_name='fetch_collection', renderer='json') @@ -259,7 +238,7 @@ def update_collection(request): # check for 'merge needed'? mn = commit_return.get('merge_needed') if (mn is not None) and (not mn): - api_utils.deferred_push_to_gh_call(request, collection_id, doc_type='collection', **request.json_body) + api_utils.deferred_push_to_gh_call(request, collection_id, doc_type='collection', auth_token=auth_info['auth_token']) # Add updated commit history to the blob commit_return['versionHistory'] = docstore.get_version_history_for_doc_id(collection_id) return commit_return @@ -287,7 +266,6 @@ def delete_collection(request): api_utils.raise_if_read_only() # remove this collection from the docstore - auth_info = api_utils.authenticate(request) owner_id = auth_info.get('login', None) docstore = api_utils.get_tree_collection_store(request) parent_sha = find_in_request(request, 'starting_commit_SHA', None) @@ -299,7 +277,7 @@ def delete_collection(request): parent_sha, commit_msg=commit_msg) if x.get('error') == 0: - api_utils.deferred_push_to_gh_call(request, None, doc_type='collection', **request.params) + api_utils.deferred_push_to_gh_call(request, None, doc_type='collection', auth_token=auth_info['auth_token']) return x except GitWorkflowError as err: raise HTTPBadRequest(err.msg) diff --git a/phylesystem_api/phylesystem_api/views/default.py b/phylesystem_api/phylesystem_api/views/default.py index e1fa7faa..adbaafd5 100644 --- a/phylesystem_api/phylesystem_api/views/default.py +++ b/phylesystem_api/phylesystem_api/views/default.py @@ -307,7 +307,7 @@ def include_tree_in_synth(request): # check for 'merge needed'? mn = commit_return.get('merge_needed') if (mn is not None) and (not mn): - api_utils.deferred_push_to_gh_call(request, default_collection_id, doc_type='collection', **kwargs) + api_utils.deferred_push_to_gh_call(request, default_collection_id, doc_type='collection', auth_token=auth_info['auth_token']) # fetch and return the updated list of synth-input trees return trees_in_synth(kwargs) @@ -359,7 +359,7 @@ def exclude_tree_from_synth(request): # check for 'merge needed'? mn = commit_return.get('merge_needed') if (mn is not None) and (not mn): - api_utils.deferred_push_to_gh_call(request, coll_id, doc_type='collection', **kwargs) + api_utils.deferred_push_to_gh_call(request, coll_id, doc_type='collection', auth_token=auth_info['auth_token']) # fetch and return the updated list of synth-input trees return trees_in_synth(kwargs) diff --git a/phylesystem_api/phylesystem_api/views/study.py b/phylesystem_api/phylesystem_api/views/study.py index 4ea871f1..72a7eda2 100644 --- a/phylesystem_api/phylesystem_api/views/study.py +++ b/phylesystem_api/phylesystem_api/views/study.py @@ -498,7 +498,7 @@ def create_study(request): if commit_return['error'] != 0: # _LOG.debug('ingest_new_study failed with error code') raise HTTPBadRequest(json.dumps(commit_return)) - api_utils.deferred_push_to_gh_call(request, new_resource_id, doc_type='nexson', **request.params) + api_utils.deferred_push_to_gh_call(request, new_resource_id, doc_type='nexson', auth_token=auth_info['auth_token']) return commit_return @@ -557,7 +557,7 @@ def update_study(request): #TIMING = api_utils.log_time_diff(_LOG, 'blob creation', TIMING) mn = blob.get('merge_needed') if (mn is not None) and (not mn): - api_utils.deferred_push_to_gh_call(request, study_id, doc_type='nexson', **request.params) + api_utils.deferred_push_to_gh_call(request, study_id, doc_type='nexson', auth_token=auth_info['auth_token']) # Add updated commit history to the blob blob['versionHistory'] = phylesystem.get_version_history_for_study_id(study_id) return blob @@ -601,7 +601,7 @@ def delete_study(request): _LOG.warn(x) if x.get('error') == 0: _LOG.warn('calling deferred push...') - api_utils.deferred_push_to_gh_call(request, None, doc_type='nexson', **request.params) + api_utils.deferred_push_to_gh_call(request, None, doc_type='nexson', auth_token=auth_info['auth_token']) _LOG.warn('back from deferred push') return x except GitWorkflowError as err: