From 7a97cd7d95b3bee826b6d60cef4fdea1f4880fc8 Mon Sep 17 00:00:00 2001 From: Max Metz Date: Tue, 10 Sep 2024 13:50:13 +0200 Subject: [PATCH 1/6] format of exceptions now always follows baseline format. 'errors'-key is always a list of dictionaries. --- src/server/BreCal/validators/validation_error.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/server/BreCal/validators/validation_error.py b/src/server/BreCal/validators/validation_error.py index 36f5cc8..e108222 100644 --- a/src/server/BreCal/validators/validation_error.py +++ b/src/server/BreCal/validators/validation_error.py @@ -35,7 +35,8 @@ def create_validation_error_response(ex:ValidationError, status_code:int=400, cr errors = {"undefined_schema":errors} errors = {k:v if isinstance(v,list) else [v] for k,v in errors.items()} - # hence, errors always has the following type: dict[str, list[str]] + # hence, errors always has the following type: list[dict[str, list[str]]] + errors = [errors] # example: From c90b002806e9d9de16501b96db8b36640073f988 Mon Sep 17 00:00:00 2001 From: Max Metz Date: Tue, 10 Sep 2024 14:17:03 +0200 Subject: [PATCH 2/6] Times POST no longer raises a ValidationError when the provided time is in the past. --- .../BreCal/validators/input_validation_times.py | 9 +++++---- src/server/BreCal/validators/time_logic.py | 14 +++++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/server/BreCal/validators/input_validation_times.py b/src/server/BreCal/validators/input_validation_times.py index e0726fc..aee8679 100644 --- a/src/server/BreCal/validators/input_validation_times.py +++ b/src/server/BreCal/validators/input_validation_times.py @@ -85,10 +85,13 @@ class InputValidationTimes(): # 2.) datasets may only be created, if the respective participant type did not already create one. InputValidationTimes.check_if_entry_already_exists_for_participant_type(user_data, loadedModel, content) - # 3.) Reference checking + # 3.) only users who are *not* of type BSMD may post times datasets. + InputValidationTimes.check_user_is_not_bsmd_type(user_data) + + # 4.) Reference checking InputValidationTimes.check_dataset_references(content) - # 4.) Value checking + # 5.) Value checking InputValidationTimes.check_dataset_values(user_data, loadedModel, content) return @@ -140,7 +143,6 @@ class InputValidationTimes(): @staticmethod def check_user_is_not_bsmd_type(user_data:dict): """a new dataset may only be created by a user who is *not* belonging to participant group BSMD""" - # this method is deprecated for /times POST requests. As the function may be used elsewhere, it will, for now, not be removed is_bsmd = check_if_user_is_bsmd_type(user_data) if is_bsmd: raise ValidationError({"participant_type":f"current user belongs to BSMD. Cannot post 'times' datasets. Found user data: {user_data}"}) @@ -171,7 +173,6 @@ class InputValidationTimes(): if not time_end_after_time_start: raise ValidationError({"etd":f"The provided time interval for the estimated departure time is invalid. The interval end takes place before the interval start. Found interval data: {loadedModel['etd_berth']} to {loadedModel['etd_interval_end']}"}) - if (loadedModel["eta_interval_end"] is not None) and (loadedModel["eta_berth"] is not None): time_end_after_time_start = loadedModel["eta_interval_end"] >= loadedModel["eta_berth"] if not time_end_after_time_start: diff --git a/src/server/BreCal/validators/time_logic.py b/src/server/BreCal/validators/time_logic.py index 27e80b8..4f8535b 100644 --- a/src/server/BreCal/validators/time_logic.py +++ b/src/server/BreCal/validators/time_logic.py @@ -19,9 +19,9 @@ def validate_time_is_in_future(value:datetime.datetime): def validate_time_is_in_not_too_distant_future(raise_validation_error:bool, value:datetime.datetime, seconds:int=60, minutes:int=60, hours:int=24, days:int=30, months:int=12)->bool: """ - combines two boolean operations. Returns True when both conditions are met. - a) value is in the future - b) value is not too distant (e.g., at max. 1 year in the future) + A time entry is considerd valid, when it meets the following condition(s): + a) value is not too distant (e.g., at max. 1 year in the future) + Previous variants of this function also included validating that a time must be in the future. This is deprecated. When the value is 'None', the validation will be skipped. A ValidationError is never issued, but the method returns 'False'. @@ -31,17 +31,17 @@ def validate_time_is_in_not_too_distant_future(raise_validation_error:bool, valu if value is None: return False - is_in_future = validate_time_is_in_future(value) + # is_in_future = validate_time_is_in_future(value) is_too_distant = validate_time_exceeds_threshold(value, seconds, minutes, hours, days, months) if raise_validation_error: - if not is_in_future: - raise ValidationError({"any_date":f"The provided value must be in the future. Current Time: {datetime.datetime.now()}, Value: {value}"}) + #if not is_in_future: + #raise ValidationError({"any_date":f"The provided value must be in the future. Current Time: {datetime.datetime.now()}, Value: {value}"}) if is_too_distant: raise ValidationError({"any_date":f"The provided value is in the too distant future and exceeds a threshold for 'reasonable' entries. Found: {value}"}) - return is_in_future & (not is_too_distant) + return (not is_too_distant) # & is_in_future class TimeLogic(): def __init__(self): From 590df30fef028914c90fe841004d15d2ba97d2d0 Mon Sep 17 00:00:00 2001 From: Max Metz Date: Tue, 10 Sep 2024 14:48:20 +0200 Subject: [PATCH 3/6] A ship's IMO-validation was used in POST and PUT requests. This caused an issue for POST requests. --- .../validators/input_validation_shipcall.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/server/BreCal/validators/input_validation_shipcall.py b/src/server/BreCal/validators/input_validation_shipcall.py index aae7e11..5e498ff 100644 --- a/src/server/BreCal/validators/input_validation_shipcall.py +++ b/src/server/BreCal/validators/input_validation_shipcall.py @@ -90,7 +90,7 @@ class InputValidationShipcall(): InputValidationShipcall.check_referenced_ids(loadedModel) # check for reasonable values in the shipcall fields and checks for forbidden keys. - InputValidationShipcall.check_shipcall_values(loadedModel, content, forbidden_keys=["evaluation", "evaluation_message"]) + InputValidationShipcall.check_shipcall_values(loadedModel, content, forbidden_keys=["evaluation", "evaluation_message"], is_put_data=True) # a canceled shipcall cannot be selected # Note: 'canceled' is allowed in PUT-requests, if it is not already set (which is checked by InputValidationShipcall.check_shipcall_is_cancel) @@ -98,12 +98,15 @@ class InputValidationShipcall(): return @staticmethod - def check_shipcall_values(loadedModel:dict, content:dict, forbidden_keys:list=["evaluation", "evaluation_message"]): + def check_shipcall_values(loadedModel:dict, content:dict, forbidden_keys:list=["evaluation", "evaluation_message"], is_put_data:bool=False): """ individually checks each value provided in the loadedModel/content. This function validates, whether the values are reasonable. Also, some data may not be set in a POST-request. + + options: + is_put_data: bool. Some validation rules do not apply to POST data, but apply to PUT data. This flag separates the two. """ # Note: BreCal.schemas.model.ShipcallSchema has an internal validation, which the marshmallow library provides. This is used # to verify values individually, when the schema is loaded with data. @@ -120,11 +123,12 @@ class InputValidationShipcall(): if check_if_int_is_valid_flag(flags_value, enum_object=ParticipantFlag): raise ValidationError({"flags":f"incorrect value provided for 'flags'. Must be a valid combination of the flags."}) - # time values must use future-dates - InputValidationShipcall.check_times_are_in_future(loadedModel, content) - - # the type of a shipcall may not be changed. It can only be set with the initial POST-request. - InputValidationShipcall.check_shipcall_type_is_unchanged(loadedModel) + if is_put_data: + # the type of a shipcall may not be changed. It can only be set with the initial POST-request. + InputValidationShipcall.check_shipcall_type_is_unchanged(loadedModel) + else: + # time values must use future-dates + InputValidationShipcall.check_times_are_in_future(loadedModel, content) # some arguments must not be provided InputValidationShipcall.check_forbidden_arguments(content, forbidden_keys=forbidden_keys) From 5b68ef95cb46dbf9bf88eedeab98e26244ead4af Mon Sep 17 00:00:00 2001 From: Max Metz Date: Tue, 10 Sep 2024 17:37:08 +0200 Subject: [PATCH 4/6] adapting exception handling and error responses for 400 responses. Using a simplified format, which only uses the keys 'error_field' and 'error_description' --- src/server/BreCal/impl/shipcalls.py | 9 +- .../BreCal/validators/validation_error.py | 90 ++++++++++--------- 2 files changed, 49 insertions(+), 50 deletions(-) diff --git a/src/server/BreCal/impl/shipcalls.py b/src/server/BreCal/impl/shipcalls.py index 83385cf..65bb000 100644 --- a/src/server/BreCal/impl/shipcalls.py +++ b/src/server/BreCal/impl/shipcalls.py @@ -10,6 +10,7 @@ from ..services.auth_guard import check_jwt from BreCal.database.update_database import evaluate_shipcall_state from BreCal.database.sql_queries import create_sql_query_shipcall_get, create_sql_query_shipcall_post, create_sql_query_shipcall_put, create_sql_query_history_post, create_sql_query_history_put, SQLQuery from marshmallow import Schema, fields, ValidationError +from BreCal.validators.validation_error import create_validation_error_response def GetShipcalls(options): """ @@ -152,9 +153,7 @@ def PostShipcalls(schemaModel): return json.dumps({"id" : new_id}), 201, {'Content-Type': 'application/json; charset=utf-8'} except ValidationError as ex: - logging.error(ex) - print(ex) - return json.dumps({"message":f"bad format. \nError Messages: {ex.messages}. \nValid Data: {ex.valid_data}"}), 400 + return create_validation_error_response(ex, status_code=400, create_log=True) except Exception as ex: logging.error(traceback.format_exc()) @@ -265,9 +264,7 @@ def PutShipcalls(schemaModel): return json.dumps({"id" : schemaModel["id"]}), 200 except ValidationError as ex: - logging.error(ex) - print(ex) - return json.dumps({"message":f"bad format. \nError Messages: {ex.messages}. \nValid Data: {ex.valid_data}"}), 400 + return create_validation_error_response(ex, status_code=400, create_log=True) except Exception as ex: logging.error(traceback.format_exc()) diff --git a/src/server/BreCal/validators/validation_error.py b/src/server/BreCal/validators/validation_error.py index e108222..4ee0c7e 100644 --- a/src/server/BreCal/validators/validation_error.py +++ b/src/server/BreCal/validators/validation_error.py @@ -1,58 +1,59 @@ import logging import typing import json +import sys from marshmallow import ValidationError -import werkzeug from werkzeug.exceptions import Forbidden -def create_default_error_format(error_description): +def create_default_json_response_format(error_field:str, error_description:str): + """ + The default format of a JSON response for exceptions is: + { + "message":str, + "errors":typing.Optional[ + list[dict[str,list[str]]] + ] + } + """ json_response = { - "message":f"{error_description}", - "errors":[], - "valid_data":{} + "error_field":error_field, + "error_description":error_description } return json_response +def unbundle_(errors, unbundled=[]): + """ + unbundles a dictionary entry into separate items and appends them to the list {unbundled}. + Example: + errors = {"key1":{"keya":"keyb","keyc":{"keyc12":12}}} + Returns: + [{'error_field':'keya', 'error_description':['keyb']}, {'error_field':'keyc12', 'error_description':[12]}] + As can be seen, only the subkeys and their respective value are received. Each value is *always* a list. + """ + {k:unbundle_(v,unbundled=unbundled) if isinstance(v,dict) else unbundled.append({"error_field":k, "error_description":v[0] if isinstance(v,list) else str(v)}) for k,v in errors.items()} + return + +def unbundle_validation_error_message(message): + """ + inputs: + message: ValidationError.messages object. A str, list or dictionary + """ + unbundled = [] + unbundle_(message, unbundled=unbundled) + if len(unbundled)>0: + error_field = "ValidationError in the following field(s): " + " & ".join([unb["error_field"] for unb in unbundled]) + error_description = "Error Description(s): " + " & ".join([unb["error_description"] for unb in unbundled]) + else: + error_field = "ValidationError" + error_description = "unknown validation error" + return (error_field, error_description) def create_validation_error_response(ex:ValidationError, status_code:int=400, create_log:bool=True)->typing.Tuple[str,int]: - # generate an overview the errors - #example: - # {'lock_time': ['The provided value must be in the future. Current Time: 2024-09-02 08:23:32.600791, Value: 2024-09-01 08:20:41.853000']} - # when the model schema returns an error, 'messages' is by default a dictionary. - # e.g., loadedModel = model.TimesSchema().load(data=content, many=False, partial=True) - # returns: {'eta_berth': ['The provided value must be in the future} - - # when raising a custom ValidationError, it can return a string, list or dict. - # we would like to ensure, that the content of the .messages is a dictionary. This can be accomplished by calling - # raise ValidationError({"example_key_which_fails":"the respective error message"}) - errors = ex.messages - - # raise ValidationError("example error") - # creates a .messages object, which is an array. e.g., ex.messages = ["example error"] - # the following conversion snipped ensures a dictionary output - if isinstance(errors, (str,list)): - errors = {"undefined_schema":errors} - errors = {k:v if isinstance(v,list) else [v] for k,v in errors.items()} - - # hence, errors always has the following type: list[dict[str, list[str]]] - errors = [errors] - - - # example: - # "Valid Data": { - # "id": 2894, - # "eta_berth": "datetime.datetime(2024, 9, 2, 11, 11, 43)", - # "eta_berth_fixed": false - # } - valid_data = ex.valid_data - - message = "ValidationError" - json_response = create_default_error_format(error_description=message) - json_response.update({ - "errors":errors, - "valid_data":valid_data - }) + # unbundles ValidationError into a dictionary of {'error_field':str, 'error_description':str}-format + message = ex.messages + (error_field, error_description) = unbundle_validation_error_message(message) + json_response = create_default_json_response_format(error_field=error_field, error_description=error_description) # json.dumps with default=str automatically converts non-serializable values to strings. Hence, datetime objects (which are not) # natively serializable are properly serialized. @@ -67,7 +68,7 @@ def create_werkzeug_error_response(ex:Forbidden, status_code:int=403, create_log # json.dumps with default=str automatically converts non-serializable values to strings. Hence, datetime objects (which are not) # natively serializable are properly serialized. message = ex.description - json_response = create_default_error_format(error_description=message) + json_response = create_default_json_response_format(error_field=str(repr(ex)), error_description=message) serialized_response = json.dumps(json_response, default=str) if create_log: @@ -77,7 +78,8 @@ def create_werkzeug_error_response(ex:Forbidden, status_code:int=403, create_log def create_dynamic_exception_response(ex, status_code:int=400, message:typing.Optional[str]=None, create_log:bool=True): message = repr(ex) if message is None else message - json_response = create_default_error_format(error_description=message) + json_response = create_default_json_response_format(error_field="Exception", error_description=message) + json_response["message"] = "call failed" serialized_response = json.dumps(json_response, default=str) From 6505ad758f50cbe98f4b5debc8999c02bb09fb9c Mon Sep 17 00:00:00 2001 From: Max Metz Date: Tue, 10 Sep 2024 17:45:32 +0200 Subject: [PATCH 5/6] bsmd authorization for times PUT --- src/server/BreCal/validators/input_validation_times.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/server/BreCal/validators/input_validation_times.py b/src/server/BreCal/validators/input_validation_times.py index aee8679..28e450f 100644 --- a/src/server/BreCal/validators/input_validation_times.py +++ b/src/server/BreCal/validators/input_validation_times.py @@ -85,13 +85,10 @@ class InputValidationTimes(): # 2.) datasets may only be created, if the respective participant type did not already create one. InputValidationTimes.check_if_entry_already_exists_for_participant_type(user_data, loadedModel, content) - # 3.) only users who are *not* of type BSMD may post times datasets. - InputValidationTimes.check_user_is_not_bsmd_type(user_data) - - # 4.) Reference checking + # 3.) Reference checking InputValidationTimes.check_dataset_references(content) - # 5.) Value checking + # 4.) Value checking InputValidationTimes.check_dataset_values(user_data, loadedModel, content) return From 7c5bc626d060cfd91c75efafdd9f9e3e815615ba Mon Sep 17 00:00:00 2001 From: Max Metz Date: Tue, 10 Sep 2024 17:47:44 +0200 Subject: [PATCH 6/6] improving documentation --- src/server/BreCal/validators/validation_error.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/server/BreCal/validators/validation_error.py b/src/server/BreCal/validators/validation_error.py index 4ee0c7e..823a924 100644 --- a/src/server/BreCal/validators/validation_error.py +++ b/src/server/BreCal/validators/validation_error.py @@ -10,10 +10,8 @@ def create_default_json_response_format(error_field:str, error_description:str): """ The default format of a JSON response for exceptions is: { - "message":str, - "errors":typing.Optional[ - list[dict[str,list[str]]] - ] + "error_field":str, + "error_description":str } """ json_response = {