From 1ed7c290d81f0d73a69f57e9f4616175c7040001 Mon Sep 17 00:00:00 2001 From: John Torcivia Date: Thu, 11 Nov 2021 23:01:01 -0500 Subject: [PATCH] additional error handing and documentation for various requests --- .../method_specific/POST_api_accounts_new.py | 30 ++++---- .../method_specific/POST_api_groups_delete.py | 28 ++++++-- .../method_specific/POST_api_groups_modify.py | 3 +- .../POST_api_objects_drafts_create.py | 40 +++++------ bco_api/api/scripts/utilities/DbUtils.py | 26 ++++--- bco_api/api/views.py | 71 +++++++++++++++---- 6 files changed, 127 insertions(+), 71 deletions(-) diff --git a/bco_api/api/scripts/method_specific/POST_api_accounts_new.py b/bco_api/api/scripts/method_specific/POST_api_accounts_new.py index 88585793..badf731e 100755 --- a/bco_api/api/scripts/method_specific/POST_api_accounts_new.py +++ b/bco_api/api/scripts/method_specific/POST_api_accounts_new.py @@ -43,7 +43,7 @@ def POST_api_accounts_new(request): if db.check_user_exists( p_app_label='api', p_model_name='new_users', p_email=bulk_request['email']) is None: if User.objects.filter(email=bulk_request['email']).exists(): # Account has already been activated. - return Response(status=status.HTTP_403_FORBIDDEN) + return Response(status=status.HTTP_409_CONFLICT, data={"message": "Account has already been activated."}) # The email has not already been asked for and # it has not been activated. @@ -57,7 +57,6 @@ def POST_api_accounts_new(request): temp_identifier = uuid.uuid4().hex if 'token' in bulk_request and 'hostname' in bulk_request: - p_data = { 'email': bulk_request['email'], 'temp_identifier': temp_identifier, @@ -66,19 +65,22 @@ def POST_api_accounts_new(request): } else: - p_data = { 'email': bulk_request['email'], 'temp_identifier': temp_identifier } - db.write_object( + objects_written = db.write_object( p_app_label='api', p_model_name='new_users', p_fields=['email', 'temp_identifier', 'hostname', 'token'], p_data=p_data ) + if objects_written < 1: + # There is a problem with the write. + return Response(status=status.HTTP_500_INTERNAL_SERVER_ERROR, data="Not able to save the new account.") + # Send an e-mail to let the requestor know that they # need to follow the activation link within 10 minutes. @@ -108,16 +110,12 @@ def POST_api_accounts_new(request): ) except Exception as e: - pass + # TODO: Should handle when the send_mail function fails? + return Response(status=status.HTTP_500_INTERNAL_SERVER_ERROR, data={"message": "Not able to send authentication email."}) - return ( - Response( - status=status.HTTP_201_CREATED - ) - ) + return Response(status=status.HTTP_201_CREATED) elif settings.PRODUCTION == 'False': - # Go straight to account activation. straight_activated = GET_activate_account( username=bulk_request['email'], @@ -133,7 +131,8 @@ def POST_api_accounts_new(request): return Response( data={ - 'message': 'New account succesfully created on development server ' + settings.PUBLIC_HOSTNAME + '. Parse the \'token\' key for your new token.', + 'message': 'New account successfully created on development server ' + settings.PUBLIC_HOSTNAME + '. Parse the \'token\' key for your ' + 'new token.', 'token': user_token, 'username': straight_activated.data['data']['username'] }, @@ -143,8 +142,5 @@ def POST_api_accounts_new(request): else: # Account has already been asked for. - return ( - Response( - status=status.HTTP_403_FORBIDDEN - ) - ) + return Response(status=status.HTTP_409_CONFLICT, data={"message": "Account has already been requested."}) + diff --git a/bco_api/api/scripts/method_specific/POST_api_groups_delete.py b/bco_api/api/scripts/method_specific/POST_api_groups_delete.py index 8dfdd073..3d7fd073 100755 --- a/bco_api/api/scripts/method_specific/POST_api_groups_delete.py +++ b/bco_api/api/scripts/method_specific/POST_api_groups_delete.py @@ -40,6 +40,7 @@ def POST_api_groups_delete(request): # Construct an array to return information about processing # the request. returning = [] + any_failed = False # Since bulk_request is an array, go over each # item in the array. @@ -60,18 +61,35 @@ def POST_api_groups_delete(request): # Delete all members of the group. User.objects.filter(groups__name=grouped.name).delete() # Delete the group itself. - grouped.delete() + deleted_count, deleted_info = grouped.delete() + if deleted_count < 1: + # Too few deleted, error with this delete + returning.append(db.messages(parameters={ + 'group': grouped.name })['404_missing_bulk_parameters']) + any_failed = True + continue + elif deleted_count > 1: + # We don't expect there to be duplicates, so while this was successful it should throw a warning + returning.append(db.messages(parameters={ + 'group': grouped.name })['418_too_many_deleted']) + any_failed = True + continue + # Everything looks OK returning.append(db.messages(parameters={'group': grouped.name})['200_OK_group_delete']) else: # Requestor is not the admin. - returning.append(db.messages(parameters={})['403_invalid_token']) + returning.append(db.messages(parameters={})['403_insufficient_permissions']) + any_failed = True else: # Update the request status. returning.append(db.messages(parameters={})['400_bad_request']) + any_failed = True # As this view is for a bulk operation, status 200 # means that the request was successfully processed, # but NOT necessarily each item in the request. - return ( - Response(status=status.HTTP_200_OK, data=returning) - ) + if any_failed: + return Response(status=status.HTTP_300_MULTIPLE_CHOICES, data=returning) + + return Response(status=status.HTTP_200_OK, data=returning) + diff --git a/bco_api/api/scripts/method_specific/POST_api_groups_modify.py b/bco_api/api/scripts/method_specific/POST_api_groups_modify.py index 8bb6279b..e5b1e37c 100755 --- a/bco_api/api/scripts/method_specific/POST_api_groups_modify.py +++ b/bco_api/api/scripts/method_specific/POST_api_groups_modify.py @@ -164,8 +164,7 @@ def POST_api_groups_modify(request): returning.append(db.messages(parameters={'group': grouped.name})['200_OK_group_modify']) else: # Requestor is not the admin. - # TODO: This is invalid permissions not exactly invalid token; might want to change - returning.append(db.messages(parameters={})['403_invalid_token']) + returning.append(db.messages(parameters={})['403_insufficient_permissions']) else: # Update the request status. returning.append(db.messages(parameters={})['400_bad_request']) diff --git a/bco_api/api/scripts/method_specific/POST_api_objects_drafts_create.py b/bco_api/api/scripts/method_specific/POST_api_objects_drafts_create.py index 88fb8889..8d7ba335 100755 --- a/bco_api/api/scripts/method_specific/POST_api_objects_drafts_create.py +++ b/bco_api/api/scripts/method_specific/POST_api_objects_drafts_create.py @@ -58,6 +58,7 @@ def POST_api_objects_drafts_create(incoming): # Construct an array to return the objects. returning = [] + any_failed = False # Since bulk_request is an array, go over each # item in the array. @@ -131,13 +132,18 @@ def POST_api_objects_drafts_create(incoming): creation_object['last_update'] = timezone.now() # Write to the database. - db.write_object( + objects_written = db.write_object( p_app_label = 'api', p_model_name = 'bco', p_fields = ['contents', 'last_update', 'object_id', 'owner_group', 'owner_user', 'prefix', 'schema', 'state'], p_data = creation_object ) + if objects_written < 1: + # Issue with writing out to DB + returning.append(db.messages(parameters={ })['400_bad_request']) + any_failed = True + # Object creator automatically has full permissions # on the object. This is checked by checking whether # or not the requestor matches the owner_user primary @@ -150,38 +156,24 @@ def POST_api_objects_drafts_create(incoming): # receiver in models.py # Update the request status. - returning.append( - db.messages( - parameters = creation_object - )['201_create'] - ) + returning.append(db.messages(parameters = creation_object)['201_create']) else: - # Update the request status. - returning.append( - db.messages( - parameters = {} - )['400_bad_request'] - ) + returning.append(db.messages(parameters = {})['400_bad_request']) + any_failed = True else: - # Update the request status. - returning.append( - db.messages( - parameters = { - 'prefix': creation_object['prefix'] - } - )['401_prefix_unauthorized'] - ) + returning.append(db.messages(parameters = {'prefix': creation_object['prefix']})['401_prefix_unauthorized']) + any_failed = True # As this view is for a bulk operation, status 200 # means that the request was successfully processed, # but NOT necessarily each item in the request. # For example, a table may not have been found for the first # requested draft. - return Response( - status = status.HTTP_200_OK, - data = returning - ) \ No newline at end of file + if any_failed: + return Response(status=status.HTTP_300_MULTIPLE_CHOICES, data=returning) + + return Response(status = status.HTTP_200_OK, data = returning) \ No newline at end of file diff --git a/bco_api/api/scripts/utilities/DbUtils.py b/bco_api/api/scripts/utilities/DbUtils.py index 91f6fe51..53d25b2a 100755 --- a/bco_api/api/scripts/utilities/DbUtils.py +++ b/bco_api/api/scripts/utilities/DbUtils.py @@ -636,6 +636,11 @@ def messages( 'status_code': '403', 'message': 'The token provided was not able to be used on this object.' }, + '404_missing_bulk_parameters': { + 'request_status': 'FAILURE', + 'status_code' : '404', + 'message' : 'One or more missing optional parameters are required for this call to have an effect.' + }, '404_missing_prefix': { 'request_status': 'FAILURE', 'status_code': '404', @@ -661,6 +666,11 @@ def messages( 'status_code': '409', 'message': 'The provided prefix \'' + parameters['prefix'] + '\' has already been created on this server.' }, + '418_too_many_deleted': { + 'request_status': 'FAILURE', + 'status_code' : '418', + 'message' : 'Only one object was expected to be deleted, but multiple were removed.' + }, } @@ -811,7 +821,7 @@ def publish( p_data = publishable ) - # Successfuly saved the object. + # Successfully saved the object. return { 'published_id': published['object_id'] } @@ -841,21 +851,18 @@ def write_object( incoming_fields = p_fields ) - serialized = serializer( - data = p_data - ) + serialized = serializer(data = p_data) # Save (update) it. if p_update is False: - # Write a new object. if serialized.is_valid(): serialized.save() + return 1 else: print(serialized.errors) - + return -1 else: - # Update an existing object. # apps.get_model( # app_label = p_app_label, @@ -866,7 +873,7 @@ def write_object( # contents = p_data['contents'] # ) - apps.get_model( + objects_modified = apps.get_model( app_label = p_app_label, model_name = p_model_name ).objects.filter( @@ -874,5 +881,8 @@ def write_object( ).update( contents = p_data['contents'] ) + + return objects_modified + def convert_id_form(oi_root): return oi_root.split("_")[0] + '{:06d}'.format(int(oi_root.split("_")[1])) diff --git a/bco_api/api/views.py b/bco_api/api/views.py index b82393f1..06a10f1f 100755 --- a/bco_api/api/views.py +++ b/bco_api/api/views.py @@ -179,7 +179,8 @@ class ApiGroupsCreate(APIView): -------------------- - This API call creates a BCO group in ths system. + This API call creates a BCO group in ths system. The name of the group is required but all other parameters + are optional. """ POST_api_groups_create_schema = openapi.Schema( type=openapi.TYPE_OBJECT, @@ -229,7 +230,9 @@ class ApiGroupsDelete(APIView): -------------------- - Deletes a group from the BCO API database. + Deletes one or more groups from the BCO API database. Even if not all requests are successful, the API + can return success. If a 300 response is returned then the caller should loop through the response + to understand which deletes failed and why. """ POST_api_groups_delete_schema = openapi.Schema(type=openapi.TYPE_OBJECT, required=['names'], @@ -251,8 +254,11 @@ class ApiGroupsDelete(APIView): @swagger_auto_schema(request_body=request_body, responses={ 200: "Group deletion is successful.", + 300: "Mixture of successes and failures in a bulk delete.", 400: "Bad request.", - 403: "Invalid token." + 403: "Invalid token.", + 404: "Missing optional bulk parameters, this request has no effect.", + 418: "More than the expected one group was deleted." }, tags=["Group Management"]) def post(self, request): return check_post_and_process(request, POST_api_groups_delete) @@ -264,7 +270,22 @@ class ApiGroupsModify(APIView): -------------------- - Modifies an already existing BCO group. + Modifies an already existing BCO group. An array of objects are taken where each of these objects + represents the instructions to modify a specific group. Within each of these objects, along with the + group name, the set of modifications to that group exists in a dictionary as defined below. + + Example request body which encodes renaming a group named `myGroup1` to `myGroup2`: + ``` + request_body = ['POST_api_groups_modify' : { + 'name': 'myGroup1', + 'actions': { + 'rename': 'myGroup2' + } + } + ] + ``` + + More than one action can be included for a specific group name. """ POST_api_groups_modify_schema = openapi.Schema( @@ -301,8 +322,8 @@ class ApiGroupsModify(APIView): }, description="Actions to take upon the group.") } - ) + request_body = openapi.Schema( type=openapi.TYPE_OBJECT, title="Group Modification Schema", @@ -317,7 +338,7 @@ class ApiGroupsModify(APIView): @swagger_auto_schema(request_body=request_body, responses={ 200: "Group modification is successful.", 400: "Bad request.", - 403: "Invalid token." + 403: "Insufficient privileges." }, tags=["Group Management"]) def post(self, request): return check_post_and_process(request, POST_api_groups_modify) @@ -354,7 +375,9 @@ class ApiAccountsNew(APIView): @swagger_auto_schema(request_body=request_body, responses={ 200: "Account creation is successful.", 400: "Bad request.", - 403: "Invalid token." + 403: "Invalid token.", + 409: "Account has already been authenticated or requested.", + 500: "Unable to save the new account or send authentication email." }, tags=["Account Management"]) def post(self, request) -> Response: print("Request: {}".format(request)) @@ -370,18 +393,36 @@ class ApiObjectsDraftsCreate(APIView): Creates a new BCO draft object. """ - # TODO: Need to get the schema that is being sent here from FE + POST_api_objects_draft_create_schema = openapi.Schema( + type=openapi.TYPE_OBJECT, + required=['prefix', 'owner_group', 'object_id', 'schema', 'contents'], + properties={ + 'prefix' : openapi.Schema(type=openapi.TYPE_STRING, + description='BCO Prefix to use'), + 'owner_group': openapi.Schema(type=openapi.TYPE_STRING, + description='Group which owns the BCO draft.'), + 'object_id': openapi.Schema(type=openapi.TYPE_STRING, + description='BCO Object ID.'), + 'schema': openapi.Schema(type=openapi.TYPE_STRING, + description='Which schema the BCO satisfies.'), + 'contents': openapi.Schema(type=openapi.TYPE_OBJECT, additional_properties=True, description="Contents of the BCO.") + } + ) + request_body = openapi.Schema( - type=openapi.TYPE_OBJECT, - title="Create BCO Draft Schema", - description="BCO draft creation description.", - properties={ - 'x': openapi.Schema(type=openapi.TYPE_STRING, description='Description of X'), - 'y': openapi.Schema(type=openapi.TYPE_STRING, description='Description of Y'), - }) + type=openapi.TYPE_OBJECT, + title="Create BCO Draft Schema", + description="Parameters that are supported when trying to create a draft BCO.", + required=['POST_api_objects_draft_create'], + properties={ + 'POST_api_objects_draft_create': openapi.Schema(type=openapi.TYPE_ARRAY, + items=POST_api_objects_draft_create_schema, + description='BCO Drafts to create.'), + }) @swagger_auto_schema(request_body=request_body, responses={ 200: "Creation of BCO draft is successful.", + 300: "Some requests failed and some succeeded.", 400: "Bad request.", 403: "Invalid token." }, tags=["BCO Management"])