From 98b88450158170b17d36c6688536e53a90b6d692 Mon Sep 17 00:00:00 2001 From: Max Metz Date: Wed, 15 May 2024 00:31:14 +0200 Subject: [PATCH] extending the capabilities of InputValidationShipcall and performing unit tests to check proper implementation. --- src/server/BreCal/api/shipcalls.py | 11 +-- src/server/BreCal/database/sql_queries.py | 2 + src/server/BreCal/impl/shipcalls.py | 8 ++- .../BreCal/validators/input_validation.py | 3 +- .../validators/input_validation_shipcall.py | 68 ++++++++++--------- .../validators/input_validation_utils.py | 41 +++++++++-- 6 files changed, 88 insertions(+), 45 deletions(-) diff --git a/src/server/BreCal/api/shipcalls.py b/src/server/BreCal/api/shipcalls.py index 6dfef58..f22fed0 100644 --- a/src/server/BreCal/api/shipcalls.py +++ b/src/server/BreCal/api/shipcalls.py @@ -9,6 +9,7 @@ from BreCal.validators.input_validation_shipcall import InputValidationShipcall import logging import json +import traceback bp = Blueprint('shipcalls', __name__) @@ -47,16 +48,17 @@ def PostShipcalls(): user_data = check_jwt() # validate the posted shipcall data - validate_posted_shipcall_data(user_data, loadedModel, content) - # InputValidationShipcall.evaluate_post_data(user_data, loadedModel, content) + # validate_posted_shipcall_data(user_data, loadedModel, content) + InputValidationShipcall.evaluate_post_data(user_data, loadedModel, content) except ValidationError as ex: logging.error(ex) print(ex) - return json.dumps(f"bad format. \nError Messages: {ex.messages}. \nValid Data: {ex.valid_data}"), 400 + return json.dumps({"message":f"bad format. \nError Messages: {ex.messages}. \nValid Data: {ex.valid_data}"}), 400 except Exception as ex: logging.error(ex) + logging.error(traceback.format_exc()) print(ex) return json.dumps("bad format"), 400 @@ -69,7 +71,6 @@ def PutShipcalls(): try: content = request.get_json(force=True) - logging.info(content) loadedModel = model.ShipcallSchema().load(data=content, many=False, partial=True) # read the user data from the JWT token (set when login is performed) @@ -85,7 +86,7 @@ def PutShipcalls(): except ValidationError as ex: logging.error(ex) print(ex) - return json.dumps(f"bad format. \nError Messages: {ex.messages}. \nValid Data: {ex.valid_data}"), 400 + return json.dumps({"message":f"bad format. \nError Messages: {ex.messages}. \nValid Data: {ex.valid_data}"}), 400 except Exception as ex: logging.error(ex) diff --git a/src/server/BreCal/database/sql_queries.py b/src/server/BreCal/database/sql_queries.py index 0940254..55ddf90 100644 --- a/src/server/BreCal/database/sql_queries.py +++ b/src/server/BreCal/database/sql_queries.py @@ -1,3 +1,4 @@ +import logging def create_sql_query_shipcall_get(options:dict)->str: @@ -9,6 +10,7 @@ def create_sql_query_shipcall_get(options:dict)->str: options : dict. A dictionary, which must contains the 'past_days' key (int). Determines the range by which shipcalls are filtered. """ + logging.info(options) query = ("SELECT s.id as id, ship_id, type, eta, voyage, etd, arrival_berth_id, departure_berth_id, tug_required, pilot_required, " + "flags, s.pier_side, bunkering, replenishing_terminal, replenishing_lock, draft, tidal_window_from, " + "tidal_window_to, rain_sensitive_cargo, recommended_tugs, anchored, moored_lock, canceled, evaluation, " + diff --git a/src/server/BreCal/impl/shipcalls.py b/src/server/BreCal/impl/shipcalls.py index 0dfcf74..54f85cd 100644 --- a/src/server/BreCal/impl/shipcalls.py +++ b/src/server/BreCal/impl/shipcalls.py @@ -8,7 +8,7 @@ from .. import local_db 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 +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 def GetShipcalls(options): """ @@ -142,6 +142,7 @@ def PostShipcalls(schemaModel): # save history data # TODO: set ETA properly user_data = check_jwt() + # query = create_sql_query_history_post() query = "INSERT INTO history (participant_id, shipcall_id, user_id, timestamp, eta, type, operation) VALUES (?pid?, ?scid?, ?uid?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 1, 1)" commands.execute(query, {"scid" : new_id, "pid" : user_data["participant_id"], "uid" : user_data["id"]}) @@ -150,7 +151,7 @@ def PostShipcalls(schemaModel): except ValidationError as ex: logging.error(ex) print(ex) - return json.dumps(f"bad format. \nError Messages: {ex.messages}. \nValid Data: {ex.valid_data}"), 400 + return json.dumps({"message":f"bad format. \nError Messages: {ex.messages}. \nValid Data: {ex.valid_data}"}), 400 except Exception as ex: logging.error(traceback.format_exc()) @@ -261,6 +262,7 @@ def PutShipcalls(schemaModel): # save history data # TODO: set ETA properly user_data = check_jwt() + # query = create_sql_query_history_put() query = "INSERT INTO history (participant_id, shipcall_id, user_id, timestamp, eta, type, operation) VALUES (?pid?, ?scid?, ?uid?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 1, 2)" commands.execute(query, {"scid" : schemaModel["id"], "pid" : user_data["participant_id"], "uid" : user_data["id"]}) @@ -269,7 +271,7 @@ def PutShipcalls(schemaModel): except ValidationError as ex: logging.error(ex) print(ex) - return json.dumps(f"bad format. \nError Messages: {ex.messages}. \nValid Data: {ex.valid_data}"), 400 + return json.dumps({"message":f"bad format. \nError Messages: {ex.messages}. \nValid Data: {ex.valid_data}"}), 400 except Exception as ex: logging.error(traceback.format_exc()) diff --git a/src/server/BreCal/validators/input_validation.py b/src/server/BreCal/validators/input_validation.py index 6cddf47..3e5a801 100644 --- a/src/server/BreCal/validators/input_validation.py +++ b/src/server/BreCal/validators/input_validation.py @@ -22,6 +22,7 @@ from BreCal.validators.input_validation_utils import check_if_user_is_bsmd_type, def validate_posted_shipcall_data(user_data:dict, loadedModel:dict, content:dict): """this function applies more complex validation functions to data, which is sent to a post-request of shipcalls""" + # DEPRECATED: this function has been refactored into InputValidationShipcall (see methods for POST and PUT evaluation) # #TODO_refactor: this function is pretty complex. One may instead build an object, which calls the methods separately. ##### Section 1: check user_data ##### @@ -44,7 +45,7 @@ def validate_posted_shipcall_data(user_data:dict, loadedModel:dict, content:dict if not valid_departure_berth_id: raise ValidationError(f"provided an invalid departure berth id, which is not found in the database: {loadedModel.get('departure_berth_id', None)}") - valid_participant_ids = check_if_participant_ids_are_valid(participant_ids=loadedModel.get("participants",[])) + valid_participant_ids = check_if_participant_ids_are_valid(participants=loadedModel.get("participants",[])) if not valid_participant_ids: raise ValidationError(f"one of the provided participant ids is invalid. Could not find one of these in the database: {loadedModel.get('participants', None)}") diff --git a/src/server/BreCal/validators/input_validation_shipcall.py b/src/server/BreCal/validators/input_validation_shipcall.py index d1ded52..4fdae69 100644 --- a/src/server/BreCal/validators/input_validation_shipcall.py +++ b/src/server/BreCal/validators/input_validation_shipcall.py @@ -10,7 +10,7 @@ from BreCal.impl.ships import GetShips from BreCal.impl.berths import GetBerths from BreCal.database.enums import ParticipantType, ParticipantFlag -from BreCal.validators.input_validation_utils import check_if_user_is_bsmd_type, check_if_ship_id_is_valid, check_if_berth_id_is_valid, check_if_participant_ids_are_valid, check_if_string_has_special_characters, get_shipcall_id_dictionary, get_participant_type_from_user_data, check_if_int_is_valid_flag +from BreCal.validators.input_validation_utils import check_if_user_is_bsmd_type, check_if_ship_id_is_valid, check_if_berth_id_is_valid, check_if_participant_ids_are_valid, check_if_participant_ids_and_types_are_valid, check_if_string_has_special_characters, get_shipcall_id_dictionary, get_participant_type_from_user_data, check_if_int_is_valid_flag class InputValidationShipcall(): @@ -35,8 +35,8 @@ class InputValidationShipcall(): checks: 1. permission: only participants that belong to the BSMD group are allowed to POST shipcalls 2. reference checks: all refered objects within the Shipcall must exist - 3. reasonable values: validates the values within the Shipcall - 4. existance of required fields + 3. existance of required fields + 4. reasonable values: validates the values within the Shipcall """ # check for permission (only BSMD-type participants) InputValidationShipcall.check_user_is_bsmd_type(user_data) @@ -44,14 +44,14 @@ class InputValidationShipcall(): # check references (referred IDs must exist) InputValidationShipcall.check_referenced_ids(loadedModel) - # check for reasonable values in the shipcall fields - InputValidationShipcall.check_shipcall_values(loadedModel, content, forbidden_keys=["canceled", "evaluation", "evaluation_message"]) - # POST-request only: check the existance of required fields based on the ShipcallType InputValidationShipcall.check_required_fields_exist_based_on_type(loadedModel, content) # POST-request only: check the existance of a participant list, when the user is of type agency - InputValidationShipcall.check_participant_list_not_empty_when_user_is_agency(user_data, loadedModel) + InputValidationShipcall.check_participant_list_not_empty_when_user_is_agency(loadedModel) + + # check for reasonable values in the shipcall fields + InputValidationShipcall.check_shipcall_values(loadedModel, content, forbidden_keys=["canceled", "evaluation", "evaluation_message"]) return @staticmethod @@ -62,9 +62,9 @@ class InputValidationShipcall(): checks: 1. whether the user belongs to participant group type BSMD 2. users of the agency may edit the shipcall, when the shipcall-participant-map entry lists them - 3. all value-rules of the POST evaluation - 4. a canceled shipcall may not be changed - 5. existance of required fields + 3. existance of required fields + 4. all value-rules of the POST evaluation + 5. a canceled shipcall may not be changed """ # check for permission (only BSMD-type participants) InputValidationShipcall.check_user_is_bsmd_type(user_data) @@ -72,14 +72,14 @@ class InputValidationShipcall(): # check, whether an agency is listed in the shipcall-participant-map # InputValidationShipcall.check_agency_in_shipcall_participant_map() # args? + # the ID field is required, all missing fields will be ignored in the update + InputValidationShipcall.check_required_fields_of_put_request(content) + # check for reasonable values in the shipcall fields and checks for forbidden keys. Note: 'canceled' is allowed in PUT-requests. InputValidationShipcall.check_shipcall_values(loadedModel, content, forbidden_keys=["evaluation", "evaluation_message"]) # a canceled shipcall cannot be selected InputValidationShipcall.check_shipcall_is_canceled(loadedModel, content) - - # the ID field is required, all missing fields will be ignored in the update - InputValidationShipcall.check_required_fields_of_put_request(content) return @staticmethod @@ -136,7 +136,7 @@ class InputValidationShipcall(): ship_id = loadedModel.get("ship_id", None) arrival_berth_id = loadedModel.get("arrival_berth_id", None) departure_berth_id = loadedModel.get("departure_berth_id", None) - participant_ids = loadedModel.get("participants",[]) + participants = loadedModel.get("participants",[]) valid_ship_id = check_if_ship_id_is_valid(ship_id=ship_id) if not valid_ship_id: @@ -150,11 +150,14 @@ class InputValidationShipcall(): if not valid_departure_berth_id: raise ValidationError(f"provided an invalid departure berth id, which is not found in the database: {departure_berth_id}") - valid_participant_ids = check_if_participant_ids_are_valid(participant_ids=participant_ids) + valid_participant_ids = check_if_participant_ids_are_valid(participants=participants) if not valid_participant_ids: - raise ValidationError(f"one of the provided participant ids is invalid. Could not find one of these in the database: {participant_ids}") - return - + raise ValidationError(f"one of the provided participant ids are invalid. Could not find one of these in the database: {participants}") + + valid_participant_types = check_if_participant_ids_and_types_are_valid(participants=participants) + if not valid_participant_types: + raise ValidationError(f"every participant id and type should be listed only once. Found multiple entries for one of the participants.") + @staticmethod def check_forbidden_arguments(content:dict, forbidden_keys=["canceled", "evaluation", "evaluation_message"]): """ @@ -175,11 +178,15 @@ class InputValidationShipcall(): depending on the ShipcallType, some fields are *required* in a POST-request """ type_ = loadedModel.get("type", int(ShipcallType.undefined)) + ship_id = content.get("ship_id", None) eta = content.get("eta", None) etd = content.get("etd", None) arrival_berth_id = content.get("arrival_berth_id", None) departure_berth_id = content.get("departure_berth_id", None) + if ship_id is None: + raise ValidationError(f"providing 'ship_id' is mandatory. Missing key!") + if int(type_)==int(ShipcallType.undefined): raise ValidationError(f"providing 'type' is mandatory. Missing key!") @@ -244,16 +251,16 @@ class InputValidationShipcall(): if int(type_)==int(ShipcallType.undefined): raise ValidationError(f"providing 'type' is mandatory. Missing key!") elif int(type_)==int(ShipcallType.arrival): - if not eta >= time_now: + if not eta > time_now: raise ValidationError(f"'eta' must be in the future. Incorrect datetime provided. Current Time: {time_now}. ETA: {eta}.") elif int(type_)==int(ShipcallType.departure): - if not etd >= time_now: + if not etd > time_now: raise ValidationError(f"'etd' must be in the future. Incorrect datetime provided. Current Time: {time_now}. ETD: {etd}.") elif int(type_)==int(ShipcallType.shifting): - if (not eta >= time_now) or (not etd >= time_now): + if (not eta > time_now) or (not etd > time_now): raise ValidationError(f"'eta' and 'etd' must be in the future. Incorrect datetime provided. Current Time: {time_now}. ETA: {eta}. ETD: {etd}") - if (not eta >= etd): - raise ValidationError(f"'etd' must be larger than 'eta'. The ship cannot depart, before it has arrived. Found: ETA {eta}, ETA: {etd}") + if (not etd > eta): + raise ValidationError(f"'etd' must be larger than 'eta'. The ship cannot depart, before it has arrived. Found: ETA {eta}, ETD: {etd}") return @staticmethod @@ -272,18 +279,15 @@ class InputValidationShipcall(): return @staticmethod - def check_participant_list_not_empty_when_user_is_agency(user_data, loadedModel): + def check_participant_list_not_empty_when_user_is_agency(loadedModel): """ - participant types use an IntFlag to assign multiple roles to a user. When a user is assigned to the - AGENCY role, the user must provide a non-empty list of participants in POST-requests. + For each POST request, one of the participants in the list must be assigned as a ParticipantType.AGENCY """ - participant_type = get_participant_type_from_user_data(user_data) + participants = loadedModel.get("participants", []) + is_agency_participant = [ParticipantType.AGENCY in ParticipantType(participant.get("type")) for participant in participants] - if int(ParticipantType.AGENCY) in int(participant_type): - participants = loadedModel.get("participants",[]) - - if len(participants)==0: - raise ValidationError(f"A user of type 'ParticipantType.AGENCY' is required to provide a list of valid participants.") + if not any(is_agency_participant): + raise ValidationError(f"One of the assigned participants *must* be of type 'ParticipantType.AGENCY'. Found list of participants: {participants}") return @staticmethod diff --git a/src/server/BreCal/validators/input_validation_utils.py b/src/server/BreCal/validators/input_validation_utils.py index dd38a4a..4476041 100644 --- a/src/server/BreCal/validators/input_validation_utils.py +++ b/src/server/BreCal/validators/input_validation_utils.py @@ -1,5 +1,7 @@ +import logging import json from string import ascii_letters, digits +from collections import Counter from BreCal.impl.participant import GetParticipant from BreCal.impl.ships import GetShips @@ -104,9 +106,16 @@ def check_if_berth_id_is_valid(berth_id): berth_id_is_valid = berth_id in list(berths.keys()) return berth_id_is_valid -def check_if_participant_id_is_valid(participant_id): - """check, whether the provided ID is valid. If it is 'None', it will be considered valid. This is, because a shipcall POST-request, does not have to include all IDs at once""" +def check_if_participant_id_is_valid(participant:dict): + """ + check, whether the provided ID is valid. If it is 'None', it will be considered valid. This is, because a shipcall POST-request, does not have to include all IDs at once + + Following the common BreCal.schemas.model.ParticipantAssignmentSchema, a participant dictionary contains the keys: + 'participant_id' : int + 'type' : ParticipantType + """ # #TODO1: Daniel Schick: 'types may only appear once and must not include type "BSMD"' + participant_id = participant.get("participant_id", None) if participant_id is None: return True @@ -118,13 +127,37 @@ def check_if_participant_id_is_valid(participant_id): participant_id_is_valid = participant_id in list(participants.keys()) return participant_id_is_valid -def check_if_participant_ids_are_valid(participant_ids): +def check_if_participant_ids_are_valid(participants:list[dict]): + """ + + args: + participants (list of participant-elements) + Following the common BreCal.schemas.model.ParticipantAssignmentSchema, a participant dictionary contains the keys: + 'participant_id' : int + 'type' : ParticipantType + """ # check each participant id individually - valid_participant_ids = [check_if_participant_id_is_valid(participant_id) for participant_id in participant_ids] + valid_participant_ids = [check_if_participant_id_is_valid(participant) for participant in participants] # boolean check, whether all participant ids are valid return all(valid_participant_ids) +def check_if_participant_ids_and_types_are_valid(participants:list[dict[str,int]]): + # creates a Counter object, which counts the number of unique elements + # key of counter: type, value of counter: number of listings in 'participants' + # e.g., {1: 4, 2: 1, 8: 1} (type 1 occurs 4 times in this example) + counter_type = Counter([participant.get("type") for participant in participants]) + counter_id = Counter([participant.get("type") for participant in participants]) + + # obtains the maximum count from the counter's values + max_count_type = max(list(counter_type.values())) if len(list(counter_type.values()))>0 else 0 + max_count_ids = max(list(counter_id.values())) if len(list(counter_id.values()))>0 else 0 + + # when 0 or 1 count for the participant ids or types, return true. Return false, when there is more than one entry. + return max_count_type <= 1 and max_count_ids <= 1 + + + def check_if_string_has_special_characters(text:str): """