From 86d5a4950ab5167ebb4aa5520ae48541a9ebb59b Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Tue, 2 Feb 2021 11:19:35 +0100 Subject: [PATCH 01/16] Introduce patient hash (1) --- tests/test_utilities/create_dataclasses.py | 121 +++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 tests/test_utilities/create_dataclasses.py diff --git a/tests/test_utilities/create_dataclasses.py b/tests/test_utilities/create_dataclasses.py new file mode 100644 index 000000000..0ff2e904c --- /dev/null +++ b/tests/test_utilities/create_dataclasses.py @@ -0,0 +1,121 @@ +from txmatching.patients.hla_model import HLAAntibodies, HLAType, HLATyping +from txmatching.patients.patient import (Donor, DonorType, Recipient, + RecipientRequirements) +from txmatching.patients.patient_parameters import PatientParameters +from txmatching.utils.blood_groups import BloodGroup +from txmatching.utils.enums import Country, Sex + +RAW_CODES = [ + 'A1', + 'A32', + 'B7', + 'B51', + 'DR11', + 'DR15' +] + + +def get_test_donors(): + return [ + Donor( + db_id=1, + medical_id='1', + parameters=PatientParameters( + blood_group=BloodGroup.A, + country_code=Country.CZE, + hla_typing=HLATyping( + hla_types_list=[ + HLAType(raw_code=RAW_CODES[0]), + HLAType(raw_code=RAW_CODES[1]), + HLAType(raw_code='B44'), + HLAType(raw_code='DR10') + ] + ), + sex=Sex.M, + height=180, + weight=70, + year_of_birth=1985 + ), + related_recipient_db_id=None, + donor_type=DonorType.DONOR + ), + Donor( + db_id=2, + medical_id='2', + parameters=PatientParameters( + blood_group=BloodGroup.A, + country_code=Country.CZE, + hla_typing=HLATyping( + hla_types_list=[ + HLAType(raw_code=RAW_CODES[1]), + HLAType(raw_code=RAW_CODES[2]), + HLAType(raw_code='DR10') + ] + ), + sex=Sex.M, + height=180, + weight=70, + year_of_birth=1985 + ), + related_recipient_db_id=None, + donor_type=DonorType.DONOR + ) + ] + + +def get_test_recipients(): + return [ + Recipient( + db_id=3, + medical_id='3', + parameters=PatientParameters( + blood_group=BloodGroup.A, + country_code=Country.CZE, + hla_typing=HLATyping( + hla_types_list=[ + HLAType(raw_code=RAW_CODES[1]), + HLAType(raw_code=RAW_CODES[2]), + HLAType(raw_code='DR1') + ] + ), + sex=Sex.M, + height=180, + weight=70, + year_of_birth=1985 + ), + related_donor_db_id=1, + acceptable_blood_groups=[], + recipient_cutoff=None, + hla_antibodies=HLAAntibodies([]), + recipient_requirements=RecipientRequirements(), + waiting_since=None, + previous_transplants=None + ), + Recipient( + db_id=4, + medical_id='4', + parameters=PatientParameters( + blood_group=BloodGroup.A, + country_code=Country.CZE, + hla_typing=HLATyping( + hla_types_list=[ + HLAType(raw_code='A3'), + HLAType(raw_code='B38'), + HLAType(raw_code=RAW_CODES[4]), + HLAType(raw_code=RAW_CODES[5]), + ] + ), + sex=Sex.M, + height=180, + weight=70, + year_of_birth=1985 + ), + related_donor_db_id=1, + acceptable_blood_groups=[], + recipient_cutoff=None, + hla_antibodies=HLAAntibodies([]), + recipient_requirements=RecipientRequirements(), + waiting_since=None, + previous_transplants=None + ), + ] From ea624fbf8627ed96062c4b4f99b1756c5d155224 Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Tue, 2 Feb 2021 11:25:39 +0100 Subject: [PATCH 02/16] Introduce patient hash (2) --- .../database/services/test_patient_service.py | 71 ++++++++++- tests/utils/test_matching.py | 113 +----------------- .../database/services/patient_service.py | 8 +- txmatching/patients/hla_model.py | 12 ++ txmatching/patients/patient.py | 16 ++- txmatching/patients/patient_parameters.py | 2 +- txmatching/web/api/matching_api.py | 3 +- 7 files changed, 107 insertions(+), 118 deletions(-) diff --git a/tests/database/services/test_patient_service.py b/tests/database/services/test_patient_service.py index 602d7d301..fd3b3f00f 100644 --- a/tests/database/services/test_patient_service.py +++ b/tests/database/services/test_patient_service.py @@ -1,3 +1,5 @@ +from tests.test_utilities.create_dataclasses import (get_test_donors, + get_test_recipients) from tests.test_utilities.populate_db import create_or_overwrite_txm_event from tests.test_utilities.prepare_app import DbTests from txmatching.data_transfer_objects.patients.upload_dto.donor_upload_dto import \ @@ -9,17 +11,19 @@ from txmatching.data_transfer_objects.patients.upload_dto.recipient_upload_dto import \ RecipientUploadDTO from txmatching.database.db import db +from txmatching.database.services.patient_service import get_patients_hash from txmatching.database.services.patient_upload_service import \ replace_or_add_patients_from_one_country from txmatching.database.sql_alchemy_schema import ConfigModel -from txmatching.patients.patient import DonorType +from txmatching.patients.hla_model import HLAType +from txmatching.patients.patient import DonorType, TxmEvent from txmatching.utils.blood_groups import BloodGroup from txmatching.utils.enums import Country, Sex from txmatching.utils.logged_user import get_current_user_id TXM_EVENT_NAME = 'test' -DONORS = [ +DONOR_UPLOAD_DTOS = [ DonorUploadDTO( medical_id='D1', blood_group=BloodGroup.A, @@ -61,7 +65,7 @@ ), ] -RECIPIENTS = [ +RECIPIENT_UPLOAD_DTOS = [ RecipientUploadDTO( acceptable_blood_groups=[ BloodGroup.A, @@ -138,8 +142,8 @@ PATIENT_UPLOAD_DTO = PatientUploadDTOIn( country=Country.CZE, txm_event_name=TXM_EVENT_NAME, - donors=DONORS, - recipients=RECIPIENTS + donors=DONOR_UPLOAD_DTOS, + recipients=RECIPIENT_UPLOAD_DTOS ) @@ -164,5 +168,62 @@ def test_update_txm_event_patients(self): replace_or_add_patients_from_one_country(PATIENT_UPLOAD_DTO) # Validate that all configs of particular TXM event are deleted. + # TODOO configs = ConfigModel.query.filter(ConfigModel.txm_event_id == txm_event.db_id).all() self.assertEqual(0, len(configs)) + + def test_get_patients_hash(self): + txm_event_1 = TxmEvent( + 1, 'event_name_1', + all_donors=get_test_donors(), + all_recipients=get_test_recipients(), + active_donors_dict=None, active_recipients_dict=None + ) + hash_1 = get_patients_hash(txm_event_1) + + # changing event db id, event name does not change the hash + txm_event_2 = TxmEvent( + 2, 'event_name_2', + all_donors=get_test_donors(), + all_recipients=get_test_recipients(), + active_donors_dict=None, active_recipients_dict=None + ) + hash_2 = get_patients_hash(txm_event_2) + self.assertEqual(hash_1, hash_2) + + # Changing donors changes the hash + txm_event_3 = TxmEvent( + 1, 'event_name_1', + all_donors=[], + all_recipients=get_test_recipients(), + active_donors_dict=None, active_recipients_dict=None + ) + hash_3 = get_patients_hash(txm_event_3) + self.assertNotEqual(hash_1, hash_3) + + # Changing recipients changes the hash + txm_event_4 = TxmEvent( + 1, 'event_name_1', + all_donors=get_test_donors(), + all_recipients=[], + active_donors_dict=None, active_recipients_dict=None + ) + hash_4 = get_patients_hash(txm_event_4) + self.assertNotEqual(hash_1, hash_4) + + # changing hla type changes the hash + new_donors = get_test_donors() + self.assertEqual( + new_donors[0].parameters.hla_typing.hla_per_groups[0].hla_types[0], + HLAType('A1') + ) + new_donors[0].parameters.hla_typing.hla_per_groups[0].hla_types[0] = HLAType('A3') + + txm_event_5 = TxmEvent( + 1, 'event_name_1', + all_donors=new_donors, + all_recipients=get_test_recipients(), + active_donors_dict=None, active_recipients_dict=None + ) + hash_5 = get_patients_hash(txm_event_5) + self.assertNotEqual(hash_1, hash_5) diff --git a/tests/utils/test_matching.py b/tests/utils/test_matching.py index b22f2f210..a14ba6e70 100644 --- a/tests/utils/test_matching.py +++ b/tests/utils/test_matching.py @@ -1,10 +1,8 @@ import unittest -from txmatching.patients.hla_model import (HLAAntibodies, HLAAntibody, HLAType, - HLATyping) -from txmatching.patients.patient import (Donor, DonorType, Recipient, - RecipientRequirements) -from txmatching.patients.patient_parameters import PatientParameters +from tests.test_utilities.create_dataclasses import (get_test_donors, + get_test_recipients) +from txmatching.patients.hla_model import HLAAntibodies, HLAAntibody from txmatching.scorers.matching import ( calculate_compatibility_index_for_group, get_count_of_transplants, get_matching_hla_typing_code) @@ -12,8 +10,7 @@ from txmatching.solvers.matching.matching_with_score import MatchingWithScore from txmatching.solvers.matching.transplant_cycle import TransplantCycle from txmatching.solvers.matching.transplant_sequence import TransplantSequence -from txmatching.utils.blood_groups import BloodGroup -from txmatching.utils.enums import Country, HLAGroup, Sex +from txmatching.utils.enums import HLAGroup RAW_CODES = [ 'A1', @@ -24,107 +21,9 @@ 'DR15' ] -DONORS = [ - Donor( - db_id=1, - medical_id='1', - parameters=PatientParameters( - blood_group=BloodGroup.A, - country_code=Country.CZE, - hla_typing=HLATyping( - hla_types_list=[ - HLAType(raw_code=RAW_CODES[0]), - HLAType(raw_code=RAW_CODES[1]), - HLAType(raw_code='B44'), - HLAType(raw_code='DR10') - ] - ), - sex=Sex.M, - height=180, - weight=70, - year_of_birth=1985 - ), - related_recipient_db_id=None, - donor_type=DonorType.DONOR - ), - Donor( - db_id=2, - medical_id='2', - parameters=PatientParameters( - blood_group=BloodGroup.A, - country_code=Country.CZE, - hla_typing=HLATyping( - hla_types_list=[ - HLAType(raw_code=RAW_CODES[1]), - HLAType(raw_code=RAW_CODES[2]), - HLAType(raw_code='DR10') - ] - ), - sex=Sex.M, - height=180, - weight=70, - year_of_birth=1985 - ), - related_recipient_db_id=None, - donor_type=DonorType.DONOR - ) -] +DONORS = get_test_donors() -RECIPIENTS = [ - Recipient( - db_id=3, - medical_id='3', - parameters=PatientParameters( - blood_group=BloodGroup.A, - country_code=Country.CZE, - hla_typing=HLATyping( - hla_types_list=[ - HLAType(raw_code=RAW_CODES[1]), - HLAType(raw_code=RAW_CODES[2]), - HLAType(raw_code='DR1') - ] - ), - sex=Sex.M, - height=180, - weight=70, - year_of_birth=1985 - ), - related_donor_db_id=1, - acceptable_blood_groups=[], - recipient_cutoff=None, - hla_antibodies=HLAAntibodies([]), - recipient_requirements=RecipientRequirements(), - waiting_since=None, - previous_transplants=None - ), - Recipient( - db_id=4, - medical_id='4', - parameters=PatientParameters( - blood_group=BloodGroup.A, - country_code=Country.CZE, - hla_typing=HLATyping( - hla_types_list=[ - HLAType(raw_code='A3'), - HLAType(raw_code='B38'), - HLAType(raw_code=RAW_CODES[4]), - HLAType(raw_code=RAW_CODES[5]), - ] - ), - sex=Sex.M, - height=180, - weight=70, - year_of_birth=1985 - ), - related_donor_db_id=1, - acceptable_blood_groups=[], - recipient_cutoff=None, - hla_antibodies=HLAAntibodies([]), - recipient_requirements=RecipientRequirements(), - waiting_since=None, - previous_transplants=None - ), -] +RECIPIENTS = get_test_recipients() TEST_ANTIGENS = [ 'A7', diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index da929e315..ceef51857 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -24,7 +24,7 @@ from txmatching.patients.hla_model import (HLAAntibodies, HLAAntibody, HLAType, HLATyping) from txmatching.patients.patient import (Donor, Patient, Recipient, - RecipientRequirements) + RecipientRequirements, TxmEvent) from txmatching.patients.patient_parameters import PatientParameters from txmatching.utils.hla_system.hla_transformations import \ preprocess_hla_code_in @@ -144,6 +144,12 @@ def update_donor(donor_update_dto: DonorUpdateDTO, txm_event_db_id: int) -> Dono return get_donor_from_donor_model(DonorModel.query.get(donor_update_dto.db_id)) +def get_patients_hash(txm_event: TxmEvent): + donors = tuple(txm_event.active_donors_dict.values()) if txm_event.active_donors_dict is not None else None + recipients = tuple(txm_event.active_recipients_dict.values()) if txm_event.active_recipients_dict is not None else None + return hash((donors, recipients)) + + def _get_base_patient_from_patient_model(patient_model: Union[DonorModel, RecipientModel]) -> Patient: return Patient( db_id=patient_model.id, diff --git a/txmatching/patients/hla_model.py b/txmatching/patients/hla_model.py index 91faeed23..9619153d2 100644 --- a/txmatching/patients/hla_model.py +++ b/txmatching/patients/hla_model.py @@ -36,6 +36,9 @@ class HLAPerGroup: hla_group: HLAGroup hla_types: List[HLAType] + def __hash__(self): + return hash((self.hla_group, *self.hla_types)) + @dataclass class HLATyping: @@ -47,6 +50,9 @@ def __post_init__(self): hla_types = [hla_type for hla_type in self.hla_types_list if hla_type.code] self.hla_per_groups = _split_hla_types_to_groups(hla_types) + def __hash__(self): + return hash(tuple(self.hla_per_groups)) + @dataclass class HLAAntibody: @@ -75,6 +81,9 @@ class AntibodiesPerGroup: hla_group: HLAGroup hla_antibody_list: List[HLAAntibody] + def __hash__(self): + return hash((self.hla_group, *self.hla_antibody_list)) + @dataclass class HLAAntibodies: @@ -111,6 +120,9 @@ def _group_key(hla_antibody: HLAAntibody) -> Tuple[int, str]: self.hla_antibodies_per_groups = _split_antibodies_to_groups(hla_antibodies_over_cutoff_list) + def __hash__(self): + return hash(tuple(self.hla_antibodies_per_groups)) + def _split_hla_types_to_groups(hla_types: List[HLAType]) -> List[HLAPerGroup]: hla_types_in_groups = _split_hla_codes_to_groups(hla_types) diff --git a/txmatching/patients/patient.py b/txmatching/patients/patient.py index 78d594ff0..98af13a37 100644 --- a/txmatching/patients/patient.py +++ b/txmatching/patients/patient.py @@ -18,7 +18,7 @@ class DonorType(str, Enum): NON_DIRECTED = 'NON_DIRECTED' -@dataclass +@dataclass(unsafe_hash=True) class Patient: db_id: int medical_id: str @@ -31,14 +31,14 @@ def is_donor(self) -> bool: return isinstance(self, Donor) -@dataclass +@dataclass(unsafe_hash=True) class Donor(Patient): related_recipient_db_id: Optional[RecipientDbId] = None donor_type: DonorType = DonorType.DONOR active: bool = True -@dataclass +@dataclass(unsafe_hash=True) class RecipientRequirements: """ Attributes: @@ -66,6 +66,16 @@ def __post_init__(self): if self.recipient_cutoff is None: self.recipient_cutoff = calculate_cutoff(self.hla_antibodies.hla_antibodies_list) + def __hash__(self): + return hash(( + *self.acceptable_blood_groups, + self.recipient_cutoff, + self.hla_antibodies, + self.recipient_requirements, + self.waiting_since, + self.previous_transplants + )) + @dataclass class TxmEvent: diff --git a/txmatching/patients/patient_parameters.py b/txmatching/patients/patient_parameters.py index f6b08e5df..f8b7bf2be 100644 --- a/txmatching/patients/patient_parameters.py +++ b/txmatching/patients/patient_parameters.py @@ -9,7 +9,7 @@ Centimeters = int -@dataclass +@dataclass(unsafe_hash=True) class PatientParameters: blood_group: BloodGroup country_code: Country diff --git a/txmatching/web/api/matching_api.py b/txmatching/web/api/matching_api.py index ecb64f15d..d51f0e893 100644 --- a/txmatching/web/api/matching_api.py +++ b/txmatching/web/api/matching_api.py @@ -52,7 +52,7 @@ def post(self, txm_event_id: int) -> str: else: raise CachingNotReadyException('Caching is not ready') # TODO move this commented out section to another endpoint in and also remove the raising of error and - # return some object telling FE that the cached mathcing does not exist. + # return some object telling FE that the cached matching does not exist. # https://github.com/mild-blue/txmatching/issues/372 # pairing_result = solve_from_configuration(configuration, txm_event=txm_event) # solver_service.save_pairing_result(pairing_result, user_id) @@ -64,4 +64,5 @@ def post(self, txm_event_id: int) -> str: :configuration.max_matchings_to_show_to_viewer] calculated_matchings_dto.show_not_all_matchings_found = False + calculated_matchings_dto.calculated_matchings = calculated_matchings_dto.calculated_matchings[:50] return jsonify(calculated_matchings_dto) From 65e01cffa55f7a5de929abe797b0d76c2198e5ba Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Tue, 2 Feb 2021 14:53:31 +0100 Subject: [PATCH 03/16] Add hash to db and use it --- tests/database/services/test_configuration.py | 2 +- tests/database/services/test_file_upload.py | 2 +- .../database/services/test_patient_service.py | 2 +- .../db_migrations/0010.add-patients-hash.sql | 7 +++++ .../database/services/config_service.py | 28 +++++++++++++++---- .../database/services/patient_service.py | 10 +++---- .../database/services/solver_service.py | 4 ++- txmatching/database/sql_alchemy_schema.py | 1 + txmatching/web/api/matching_api.py | 2 +- 9 files changed, 43 insertions(+), 15 deletions(-) create mode 100644 txmatching/database/db_migrations/0010.add-patients-hash.sql diff --git a/tests/database/services/test_configuration.py b/tests/database/services/test_configuration.py index 36da9f124..de31aff3d 100644 --- a/tests/database/services/test_configuration.py +++ b/tests/database/services/test_configuration.py @@ -17,7 +17,7 @@ def test_configuration(self): time.sleep(1) configuration = Configuration( forbidden_country_combinations=[ForbiddenCountryCombination(Country.CZE, Country.AUT)]) - save_configuration_to_db(configuration, txm_event.db_id, 1) + save_configuration_to_db(configuration, txm_event, 1) self.assertEqual(Country.CZE, configuration.forbidden_country_combinations[0].donor_country) configuration = get_configuration_for_txm_event(txm_event) self.assertEqual(Country.CZE, configuration.forbidden_country_combinations[0].donor_country) diff --git a/tests/database/services/test_file_upload.py b/tests/database/services/test_file_upload.py index 5cfce5698..777701f74 100644 --- a/tests/database/services/test_file_upload.py +++ b/tests/database/services/test_file_upload.py @@ -31,7 +31,7 @@ def test_saving_patients_from_obfuscated_excel(self): # Insert config and validates that it is stored into DB user_id = get_current_user_id() - config = ConfigModel( + config = ConfigModel( # TODOO txm_event_id=txm_event.db_id, parameters={}, created_by=user_id diff --git a/tests/database/services/test_patient_service.py b/tests/database/services/test_patient_service.py index fd3b3f00f..a270b8841 100644 --- a/tests/database/services/test_patient_service.py +++ b/tests/database/services/test_patient_service.py @@ -154,7 +154,7 @@ def test_update_txm_event_patients(self): # Insert config and validates that it is stored into DB user_id = get_current_user_id() - config = ConfigModel( + config = ConfigModel( # TODOO txm_event_id=txm_event.db_id, parameters={}, created_by=user_id diff --git a/txmatching/database/db_migrations/0010.add-patients-hash.sql b/txmatching/database/db_migrations/0010.add-patients-hash.sql new file mode 100644 index 000000000..001d961f9 --- /dev/null +++ b/txmatching/database/db_migrations/0010.add-patients-hash.sql @@ -0,0 +1,7 @@ +-- +-- file: txmatching/database/db_migrations/0010.add-patients-hash.sql +-- depends: 0009.drop-unique-uploaded-data +-- + +ALTER TABLE config + ADD COLUMN patients_hash bigint not null default 0 diff --git a/txmatching/database/services/config_service.py b/txmatching/database/services/config_service.py index 290936a86..f27b34af4 100644 --- a/txmatching/database/services/config_service.py +++ b/txmatching/database/services/config_service.py @@ -4,6 +4,7 @@ from typing import Dict, Optional from dacite import Config, from_dict +from sqlalchemy import and_ from txmatching.configuration.configuration import Configuration from txmatching.data_transfer_objects.matchings.matchings_model import \ @@ -11,6 +12,7 @@ from txmatching.data_transfer_objects.matchings.pairing_result import \ DatabasePairingResult from txmatching.database.db import db +from txmatching.database.services.patient_service import get_patients_hash from txmatching.database.sql_alchemy_schema import (ConfigModel, PairingResultModel) from txmatching.patients.patient import TxmEvent @@ -34,6 +36,7 @@ def configuration_from_dict(config_model: Dict) -> Configuration: def get_configuration_for_txm_event(txm_event: TxmEvent) -> Configuration: + # TODOO current_config_model = get_latest_config_model_for_txm_event(txm_event.db_id) if current_config_model is None: return Configuration() @@ -41,8 +44,9 @@ def get_configuration_for_txm_event(txm_event: TxmEvent) -> Configuration: return configuration_from_dict(current_config_model.parameters) -def save_configuration_to_db(configuration: Configuration, txm_event_db_id: int, user_id: int) -> int: - config_model = _configuration_to_config_model(configuration, txm_event_db_id, user_id) +def save_configuration_to_db(configuration: Configuration, txm_event: TxmEvent, user_id: int) -> int: + patients_hash = get_patients_hash(txm_event) + config_model = _configuration_to_config_model(configuration, txm_event.db_id, patients_hash, user_id) db.session.add(config_model) db.session.commit() @@ -59,13 +63,27 @@ def remove_configs_from_txm_event(txm_event_db_id: int): ConfigModel.query.filter(ConfigModel.txm_event_id == txm_event_db_id).delete() -def _configuration_to_config_model(configuration: Configuration, txm_event_db_id: int, user_id: int) -> ConfigModel: - return ConfigModel(parameters=dataclasses.asdict(configuration), created_by=user_id, txm_event_id=txm_event_db_id) +def _configuration_to_config_model(configuration: Configuration, txm_event_db_id: int, patients_hash: int, user_id: int) -> ConfigModel: + return ConfigModel( + parameters=dataclasses.asdict(configuration), + txm_event_id=txm_event_db_id, + patients_hash=patients_hash, + created_by=user_id + ) def find_configuration_db_id_for_configuration(configuration: Configuration, txm_event: TxmEvent) -> Optional[int]: - for config_model in ConfigModel.query.filter(ConfigModel.txm_event_id == txm_event.db_id).all(): + patients_hash = get_patients_hash(txm_event) + # TODOO + logger.info(f'Got patients hash {patients_hash}') + + config_models = ConfigModel.query.filter(and_( + ConfigModel.txm_event_id == txm_event.db_id, + ConfigModel.patients_hash == patients_hash + )).all() + logger.debug(config_models) + for config_model in config_models: config_from_model = configuration_from_dict(config_model.parameters) if configuration == config_from_model: return config_model.id diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index ceef51857..3d1428436 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -15,8 +15,6 @@ from txmatching.data_transfer_objects.patients.update_dtos.recipient_update_dto import \ RecipientUpdateDTO from txmatching.database.db import db -from txmatching.database.services.config_service import \ - remove_configs_from_txm_event from txmatching.database.services.parsing_utils import get_hla_code from txmatching.database.sql_alchemy_schema import ( ConfigModel, DonorModel, RecipientAcceptableBloodModel, @@ -119,7 +117,7 @@ def update_recipient(recipient_update_dto: RecipientUpdateDTO, txm_event_db_id: recipient_update_dict['recipient_cutoff'] = recipient_update_dto.cutoff RecipientModel.query.filter(RecipientModel.id == recipient_update_dto.db_id).update(recipient_update_dict) - remove_configs_from_txm_event(txm_event_db_id) + # remove_configs_from_txm_event(txm_event_db_id) # TODOO db.session.commit() return get_recipient_from_recipient_model(RecipientModel.query.get(recipient_update_dto.db_id)) @@ -139,14 +137,16 @@ def update_donor(donor_update_dto: DonorUpdateDTO, txm_event_db_id: int) -> Dono if donor_update_dto.active is not None: donor_update_dict['active'] = donor_update_dto.active DonorModel.query.filter(DonorModel.id == donor_update_dto.db_id).update(donor_update_dict) - remove_configs_from_txm_event(txm_event_db_id) + # remove_configs_from_txm_event(txm_event_db_id) # TODOO db.session.commit() return get_donor_from_donor_model(DonorModel.query.get(donor_update_dto.db_id)) -def get_patients_hash(txm_event: TxmEvent): +def get_patients_hash(txm_event: TxmEvent) -> int: + # TODOO donors = tuple(txm_event.active_donors_dict.values()) if txm_event.active_donors_dict is not None else None recipients = tuple(txm_event.active_recipients_dict.values()) if txm_event.active_recipients_dict is not None else None + logger.info(f'Getting hash for {len(donors)} donors and {len(recipients)} recipients') return hash((donors, recipients)) diff --git a/txmatching/database/services/solver_service.py b/txmatching/database/services/solver_service.py index e3e0a8f08..6067bdc88 100644 --- a/txmatching/database/services/solver_service.py +++ b/txmatching/database/services/solver_service.py @@ -8,6 +8,7 @@ from txmatching.database.services.config_service import \ save_configuration_to_db from txmatching.database.services.scorer_service import score_matrix_to_dto +from txmatching.database.services.txm_event_service import get_txm_event from txmatching.database.sql_alchemy_schema import PairingResultModel from txmatching.solvers.pairing_result import PairingResult @@ -17,7 +18,8 @@ def save_pairing_result(pairing_result: PairingResult, user_id: int): _calculated_matchings_to_model(pairing_result) ) - config_id = save_configuration_to_db(pairing_result.configuration, pairing_result.txm_event_db_id, user_id) + txm_event = get_txm_event(pairing_result.txm_event_db_id) + config_id = save_configuration_to_db(pairing_result.configuration, txm_event, user_id) pairing_result_model = PairingResultModel( score_matrix=score_matrix_to_dto(pairing_result.score_matrix), diff --git a/txmatching/database/sql_alchemy_schema.py b/txmatching/database/sql_alchemy_schema.py index 41fff6868..ee3aa1f71 100644 --- a/txmatching/database/sql_alchemy_schema.py +++ b/txmatching/database/sql_alchemy_schema.py @@ -22,6 +22,7 @@ class ConfigModel(db.Model): id = db.Column(Integer, primary_key=True, autoincrement=True, nullable=False) txm_event_id = db.Column(db.Integer, ForeignKey('txm_event.id', ondelete='CASCADE'), unique=False, nullable=False) parameters = db.Column(db.JSON, unique=False, nullable=False) + patients_hash = db.Column(db.Integer, unique=False, nullable=False) created_by = db.Column(db.Integer, unique=False, nullable=False) # created at and updated at is not handled by triggers as then am not sure how tests would work, as triggers # seem to be specific as per db and I do not think its worth the effort as this simple approach works fine diff --git a/txmatching/web/api/matching_api.py b/txmatching/web/api/matching_api.py index d51f0e893..108aec2b4 100644 --- a/txmatching/web/api/matching_api.py +++ b/txmatching/web/api/matching_api.py @@ -64,5 +64,5 @@ def post(self, txm_event_id: int) -> str: :configuration.max_matchings_to_show_to_viewer] calculated_matchings_dto.show_not_all_matchings_found = False - calculated_matchings_dto.calculated_matchings = calculated_matchings_dto.calculated_matchings[:50] + calculated_matchings_dto.calculated_matchings = calculated_matchings_dto.calculated_matchings[:50] # TODOO return jsonify(calculated_matchings_dto) From 7a52e142fef29198c005fe7849145ca7eebb658f Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Tue, 2 Feb 2021 22:35:44 +0100 Subject: [PATCH 04/16] Create patient hash using hashlib --- .../database/services/test_patient_service.py | 36 ++++++- .../database/services/patient_service.py | 102 ++++++++++++++++-- .../services/patient_upload_service.py | 1 - txmatching/patients/hla_model.py | 12 --- txmatching/patients/patient.py | 16 +-- txmatching/patients/patient_parameters.py | 2 +- 6 files changed, 129 insertions(+), 40 deletions(-) diff --git a/tests/database/services/test_patient_service.py b/tests/database/services/test_patient_service.py index a270b8841..81baa7942 100644 --- a/tests/database/services/test_patient_service.py +++ b/tests/database/services/test_patient_service.py @@ -1,3 +1,5 @@ +import hashlib + from tests.test_utilities.create_dataclasses import (get_test_donors, get_test_recipients) from tests.test_utilities.populate_db import create_or_overwrite_txm_event @@ -11,7 +13,8 @@ from txmatching.data_transfer_objects.patients.upload_dto.recipient_upload_dto import \ RecipientUploadDTO from txmatching.database.db import db -from txmatching.database.services.patient_service import get_patients_hash +from txmatching.database.services.patient_service import (_update_hash, + get_patients_hash) from txmatching.database.services.patient_upload_service import \ replace_or_add_patients_from_one_country from txmatching.database.sql_alchemy_schema import ConfigModel @@ -157,6 +160,7 @@ def test_update_txm_event_patients(self): config = ConfigModel( # TODOO txm_event_id=txm_event.db_id, parameters={}, + patients_hash=get_patients_hash(txm_event), created_by=user_id ) @@ -167,10 +171,34 @@ def test_update_txm_event_patients(self): replace_or_add_patients_from_one_country(PATIENT_UPLOAD_DTO) - # Validate that all configs of particular TXM event are deleted. - # TODOO + # Validate that configs of particular TXM event are not deleted. configs = ConfigModel.query.filter(ConfigModel.txm_event_id == txm_event.db_id).all() - self.assertEqual(0, len(configs)) + self.assertEqual(1, len(configs)) + + # Validate that patients hash has changed + self.assertNotEqual(config.patients_hash) + + def test_hashing(self): + def _assert_hash(value, expected_hash_digest): + hash_ = hashlib.md5() + _update_hash(hash_, value) + #print(hash_.hexdigest()) # TODOO + #return + self.assertEqual(hash_.hexdigest(), expected_hash_digest) + + _assert_hash('foo', '56c527b4cc0b522b127062dec3201194') + _assert_hash('42', 'aa0a056d9e7d1b3b17530b46107b91a3') + _assert_hash(42, 'd1e2cf72d8bf073f0bc2d0e8794b31ae') + _assert_hash(42.0, 'ee8b51ea1d5859dc45035c4ee9fcaedc') + _assert_hash(True, '3b3e200b7cda75063ec203db706d2463') + _assert_hash([42], '7c4aa0d7ffabc559d31ae902ae2b93a6') + _assert_hash([1, 2], '50d69227171683e044fc85f530d31568') + _assert_hash({1, 2}, '26b3a59f9692f43f20db24e6ab242cb7') + _assert_hash((1, True), 'e70e4791e78da2db05688c6043a20d86') + _assert_hash({'a': 'b'}, 'e4625008dde72175d331df31f62572e9') + _assert_hash(None, '6af5817033462a81dfdff478e27e824d') + _assert_hash(get_test_donors(), 'e799dd070b8c420c7b9c026969dbf663') + _assert_hash(get_test_recipients(), '344d1610eab01d736c40b1938492515a') def test_get_patients_hash(self): txm_event_1 = TxmEvent( diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index 3d1428436..9b9d58fd8 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -1,9 +1,9 @@ import dataclasses +import hashlib import logging from typing import Optional, Union import dacite -from sqlalchemy import and_ from txmatching.auth.exceptions import InvalidArgumentException from txmatching.data_transfer_objects.patients.patient_parameters_dto import \ @@ -17,9 +17,10 @@ from txmatching.database.db import db from txmatching.database.services.parsing_utils import get_hla_code from txmatching.database.sql_alchemy_schema import ( - ConfigModel, DonorModel, RecipientAcceptableBloodModel, - RecipientHLAAntibodyModel, RecipientModel) -from txmatching.patients.hla_model import (HLAAntibodies, HLAAntibody, HLAType, + DonorModel, RecipientAcceptableBloodModel, RecipientHLAAntibodyModel, + RecipientModel) +from txmatching.patients.hla_model import (AntibodiesPerGroup, HLAAntibodies, + HLAAntibody, HLAPerGroup, HLAType, HLATyping) from txmatching.patients.patient import (Donor, Patient, Recipient, RecipientRequirements, TxmEvent) @@ -128,8 +129,6 @@ def update_donor(donor_update_dto: DonorUpdateDTO, txm_event_db_id: int) -> Dono old_donor_model = DonorModel.query.get(donor_update_dto.db_id) if txm_event_db_id != old_donor_model.txm_event_id: raise InvalidArgumentException('Trying to update patient the user has no access to') - ConfigModel.query.filter( - and_(ConfigModel.id > 0, ConfigModel.txm_event_id == txm_event_db_id)).delete() donor_update_dict = {} if donor_update_dto.hla_typing: @@ -143,11 +142,96 @@ def update_donor(donor_update_dto: DonorUpdateDTO, txm_event_db_id: int) -> Dono def get_patients_hash(txm_event: TxmEvent) -> int: - # TODOO donors = tuple(txm_event.active_donors_dict.values()) if txm_event.active_donors_dict is not None else None recipients = tuple(txm_event.active_recipients_dict.values()) if txm_event.active_recipients_dict is not None else None - logger.info(f'Getting hash for {len(donors)} donors and {len(recipients)} recipients') - return hash((donors, recipients)) + + hash_ = hashlib.md5() + _update_hash(hash_, donors) + _update_hash(hash_, recipients) + hash_int = int(hash_.hexdigest(), 16) % 2**64 # convert to int8 + return hash_int + + +def _update_hash(hash_, value): + # logger.info(f"Updating hash for type {type(value)} ({value})") + + type_str = f'__{type(value).__name__}__' + # logger.info(f"Hash update: {type_str}") + hash_.update(type_str.encode('ASCII')) + + # TODOO: remove comments + #f = open("hash_output.txt", "a") + #f.write(type_str) + #f.close() + + if isinstance(value, str): + #f = open("hash_output.txt", "a") + #f.write(value) + #f.close() + # logger.info(f"Hash update: {value}") + hash_.update(value.encode('ASCII')) + elif isinstance(value, int) or isinstance(value, bool) or isinstance(value, float): + _update_hash(hash_, str(value)) + elif isinstance(value, list) or isinstance(value, set) or isinstance(value, tuple): + for item in value: + _update_hash(hash_, item) + elif isinstance(value, dict): + for k, v in value.items(): + _update_hash(hash_, k) + _update_hash(hash_, v) + elif value is None: + _update_hash(hash_, '__none__') + elif isinstance(value, Patient): + _update_hash(hash_, value.medical_id) + _update_hash(hash_, value.parameters) + if isinstance(value, Donor): + _update_hash(hash_, value.related_recipient_db_id) + _update_hash(hash_, value.donor_type) + _update_hash(hash_, value.active) + if isinstance(value, Recipient): + _update_hash(hash_, value.related_donor_db_id) + _update_hash(hash_, sorted(value.acceptable_blood_groups)) + _update_hash(hash_, value.recipient_cutoff) + _update_hash(hash_, value.hla_antibodies) + _update_hash(hash_, value.recipient_requirements) + _update_hash(hash_, value.waiting_since) + _update_hash(hash_, value.previous_transplants) + elif isinstance(value, PatientParameters): + _update_hash(hash_, value.blood_group) + _update_hash(hash_, value.country_code) + _update_hash(hash_, value.hla_typing) + _update_hash(hash_, value.sex) + _update_hash(hash_, value.height) + _update_hash(hash_, value.weight) + _update_hash(hash_, value.year_of_birth) + elif isinstance(value, HLATyping): + _update_hash(hash_, value.hla_per_groups) + elif isinstance(value, HLAPerGroup): + _update_hash(hash_, value.hla_group) + _update_hash(hash_, sorted(value.hla_types, key=lambda hla_type: hla_type.raw_code)) + elif isinstance(value, HLAType): + _update_hash(hash_, value.raw_code) + elif isinstance(value, RecipientRequirements): + # Treat None as False + _update_hash(hash_, bool(value.require_better_match_in_compatibility_index_or_blood_group)) + _update_hash(hash_, bool(value.require_better_match_in_compatibility_index)) + _update_hash(hash_, bool(value.require_compatible_blood_group)) + elif isinstance(value, HLAAntibodies): + _update_hash(hash_, value.hla_antibodies_per_groups) + elif isinstance(value, AntibodiesPerGroup): + _update_hash(hash_, value.hla_group) + _update_hash(hash_, sorted(value.hla_antibody_list, + key=lambda hla_type: ( + hla_type.raw_code, + hla_type.mfi, + hla_type.cutoff + ))) + elif isinstance(value, HLAAntibody): + _update_hash(hash_, value.raw_code) + _update_hash(hash_, value.mfi) + _update_hash(hash_, value.cutoff) + else: + raise NotImplementedError(f'Hashing type {type(value)} ({value}) not implemented') def _get_base_patient_from_patient_model(patient_model: Union[DonorModel, RecipientModel]) -> Patient: diff --git a/txmatching/database/services/patient_upload_service.py b/txmatching/database/services/patient_upload_service.py index 63cc7ab6f..6e5caf69c 100644 --- a/txmatching/database/services/patient_upload_service.py +++ b/txmatching/database/services/patient_upload_service.py @@ -38,7 +38,6 @@ def replace_or_add_patients_from_one_country(patient_upload_dto: PatientUploadDT remove_donors_and_recipients_from_txm_event_for_country(txm_event_db_id, patient_upload_dto.country) - remove_configs_from_txm_event(txm_event_db_id) _add_patients_from_one_country( donors=patient_upload_dto.donors, recipients=patient_upload_dto.recipients, diff --git a/txmatching/patients/hla_model.py b/txmatching/patients/hla_model.py index 9619153d2..91faeed23 100644 --- a/txmatching/patients/hla_model.py +++ b/txmatching/patients/hla_model.py @@ -36,9 +36,6 @@ class HLAPerGroup: hla_group: HLAGroup hla_types: List[HLAType] - def __hash__(self): - return hash((self.hla_group, *self.hla_types)) - @dataclass class HLATyping: @@ -50,9 +47,6 @@ def __post_init__(self): hla_types = [hla_type for hla_type in self.hla_types_list if hla_type.code] self.hla_per_groups = _split_hla_types_to_groups(hla_types) - def __hash__(self): - return hash(tuple(self.hla_per_groups)) - @dataclass class HLAAntibody: @@ -81,9 +75,6 @@ class AntibodiesPerGroup: hla_group: HLAGroup hla_antibody_list: List[HLAAntibody] - def __hash__(self): - return hash((self.hla_group, *self.hla_antibody_list)) - @dataclass class HLAAntibodies: @@ -120,9 +111,6 @@ def _group_key(hla_antibody: HLAAntibody) -> Tuple[int, str]: self.hla_antibodies_per_groups = _split_antibodies_to_groups(hla_antibodies_over_cutoff_list) - def __hash__(self): - return hash(tuple(self.hla_antibodies_per_groups)) - def _split_hla_types_to_groups(hla_types: List[HLAType]) -> List[HLAPerGroup]: hla_types_in_groups = _split_hla_codes_to_groups(hla_types) diff --git a/txmatching/patients/patient.py b/txmatching/patients/patient.py index 98af13a37..78d594ff0 100644 --- a/txmatching/patients/patient.py +++ b/txmatching/patients/patient.py @@ -18,7 +18,7 @@ class DonorType(str, Enum): NON_DIRECTED = 'NON_DIRECTED' -@dataclass(unsafe_hash=True) +@dataclass class Patient: db_id: int medical_id: str @@ -31,14 +31,14 @@ def is_donor(self) -> bool: return isinstance(self, Donor) -@dataclass(unsafe_hash=True) +@dataclass class Donor(Patient): related_recipient_db_id: Optional[RecipientDbId] = None donor_type: DonorType = DonorType.DONOR active: bool = True -@dataclass(unsafe_hash=True) +@dataclass class RecipientRequirements: """ Attributes: @@ -66,16 +66,6 @@ def __post_init__(self): if self.recipient_cutoff is None: self.recipient_cutoff = calculate_cutoff(self.hla_antibodies.hla_antibodies_list) - def __hash__(self): - return hash(( - *self.acceptable_blood_groups, - self.recipient_cutoff, - self.hla_antibodies, - self.recipient_requirements, - self.waiting_since, - self.previous_transplants - )) - @dataclass class TxmEvent: diff --git a/txmatching/patients/patient_parameters.py b/txmatching/patients/patient_parameters.py index f8b7bf2be..f6b08e5df 100644 --- a/txmatching/patients/patient_parameters.py +++ b/txmatching/patients/patient_parameters.py @@ -9,7 +9,7 @@ Centimeters = int -@dataclass(unsafe_hash=True) +@dataclass class PatientParameters: blood_group: BloodGroup country_code: Country From e67a09fbf762cad1ed5cfe5b5562c677a3c3eba9 Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Tue, 2 Feb 2021 22:50:22 +0100 Subject: [PATCH 05/16] Solve some todos --- tests/database/services/test_file_upload.py | 6 ++++-- tests/database/services/test_patient_service.py | 6 ++++-- txmatching/database/services/config_service.py | 5 +---- txmatching/database/services/patient_service.py | 5 ++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/database/services/test_file_upload.py b/tests/database/services/test_file_upload.py index 777701f74..a97bac6e4 100644 --- a/tests/database/services/test_file_upload.py +++ b/tests/database/services/test_file_upload.py @@ -6,6 +6,7 @@ from txmatching.auth.exceptions import InvalidArgumentException from txmatching.configuration.configuration import Configuration from txmatching.database.db import db +from txmatching.database.services.patient_service import get_patients_hash from txmatching.database.services.patient_upload_service import \ replace_or_add_patients_from_excel from txmatching.database.services.txm_event_service import ( @@ -31,9 +32,10 @@ def test_saving_patients_from_obfuscated_excel(self): # Insert config and validates that it is stored into DB user_id = get_current_user_id() - config = ConfigModel( # TODOO + config = ConfigModel( txm_event_id=txm_event.db_id, parameters={}, + patients_hash=get_patients_hash(txm_event), created_by=user_id ) @@ -74,7 +76,7 @@ def test_saving_patients_from_obfuscated_excel(self): ) for donor in txm_event.active_donors_dict.values()] - self.assertEqual(0, len(configs)) + self.assertEqual(1, len(configs)) self.assertEqual(34, len(recipients)) self.assertEqual(38, len(donors)) self.assertEqual(3, len({donor.country for donor in donors})) diff --git a/tests/database/services/test_patient_service.py b/tests/database/services/test_patient_service.py index 81baa7942..54f6f5561 100644 --- a/tests/database/services/test_patient_service.py +++ b/tests/database/services/test_patient_service.py @@ -17,6 +17,7 @@ get_patients_hash) from txmatching.database.services.patient_upload_service import \ replace_or_add_patients_from_one_country +from txmatching.database.services.txm_event_service import get_txm_event from txmatching.database.sql_alchemy_schema import ConfigModel from txmatching.patients.hla_model import HLAType from txmatching.patients.patient import DonorType, TxmEvent @@ -157,7 +158,7 @@ def test_update_txm_event_patients(self): # Insert config and validates that it is stored into DB user_id = get_current_user_id() - config = ConfigModel( # TODOO + config = ConfigModel( txm_event_id=txm_event.db_id, parameters={}, patients_hash=get_patients_hash(txm_event), @@ -176,7 +177,8 @@ def test_update_txm_event_patients(self): self.assertEqual(1, len(configs)) # Validate that patients hash has changed - self.assertNotEqual(config.patients_hash) + txm_event_new = get_txm_event(txm_event.db_id) + self.assertNotEqual(config.patients_hash, get_patients_hash(txm_event_new)) def test_hashing(self): def _assert_hash(value, expected_hash_digest): diff --git a/txmatching/database/services/config_service.py b/txmatching/database/services/config_service.py index f27b34af4..50db29296 100644 --- a/txmatching/database/services/config_service.py +++ b/txmatching/database/services/config_service.py @@ -36,7 +36,6 @@ def configuration_from_dict(config_model: Dict) -> Configuration: def get_configuration_for_txm_event(txm_event: TxmEvent) -> Configuration: - # TODOO current_config_model = get_latest_config_model_for_txm_event(txm_event.db_id) if current_config_model is None: return Configuration() @@ -75,9 +74,6 @@ def _configuration_to_config_model(configuration: Configuration, txm_event_db_id def find_configuration_db_id_for_configuration(configuration: Configuration, txm_event: TxmEvent) -> Optional[int]: patients_hash = get_patients_hash(txm_event) - # TODOO - logger.info(f'Got patients hash {patients_hash}') - config_models = ConfigModel.query.filter(and_( ConfigModel.txm_event_id == txm_event.db_id, ConfigModel.patients_hash == patients_hash @@ -88,6 +84,7 @@ def find_configuration_db_id_for_configuration(configuration: Configuration, if configuration == config_from_model: return config_model.id + logger.info(f'Configuration for event {txm_event.db_id} and patients hash {patients_hash} not found') return None diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index 9b9d58fd8..ab9cab8ac 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -1,6 +1,7 @@ import dataclasses import hashlib import logging +from datetime import datetime from typing import Optional, Union import dacite @@ -118,7 +119,6 @@ def update_recipient(recipient_update_dto: RecipientUpdateDTO, txm_event_db_id: recipient_update_dict['recipient_cutoff'] = recipient_update_dto.cutoff RecipientModel.query.filter(RecipientModel.id == recipient_update_dto.db_id).update(recipient_update_dict) - # remove_configs_from_txm_event(txm_event_db_id) # TODOO db.session.commit() return get_recipient_from_recipient_model(RecipientModel.query.get(recipient_update_dto.db_id)) @@ -136,7 +136,6 @@ def update_donor(donor_update_dto: DonorUpdateDTO, txm_event_db_id: int) -> Dono if donor_update_dto.active is not None: donor_update_dict['active'] = donor_update_dto.active DonorModel.query.filter(DonorModel.id == donor_update_dto.db_id).update(donor_update_dict) - # remove_configs_from_txm_event(txm_event_db_id) # TODOO db.session.commit() return get_donor_from_donor_model(DonorModel.query.get(donor_update_dto.db_id)) @@ -170,7 +169,7 @@ def _update_hash(hash_, value): #f.close() # logger.info(f"Hash update: {value}") hash_.update(value.encode('ASCII')) - elif isinstance(value, int) or isinstance(value, bool) or isinstance(value, float): + elif isinstance(value, int) or isinstance(value, bool) or isinstance(value, float) or isinstance(value, datetime): _update_hash(hash_, str(value)) elif isinstance(value, list) or isinstance(value, set) or isinstance(value, tuple): for item in value: From 73b84e59dc370d160588a6c74acd2f8714e79122 Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Tue, 2 Feb 2021 23:10:48 +0100 Subject: [PATCH 06/16] Fix tests --- tests/database/services/test_patient_service.py | 2 +- tests/database/services/test_update_donor_recipient.py | 4 ++-- txmatching/database/services/patient_service.py | 2 +- txmatching/database/sql_alchemy_schema.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/database/services/test_patient_service.py b/tests/database/services/test_patient_service.py index 54f6f5561..cb101a371 100644 --- a/tests/database/services/test_patient_service.py +++ b/tests/database/services/test_patient_service.py @@ -200,7 +200,7 @@ def _assert_hash(value, expected_hash_digest): _assert_hash({'a': 'b'}, 'e4625008dde72175d331df31f62572e9') _assert_hash(None, '6af5817033462a81dfdff478e27e824d') _assert_hash(get_test_donors(), 'e799dd070b8c420c7b9c026969dbf663') - _assert_hash(get_test_recipients(), '344d1610eab01d736c40b1938492515a') + _assert_hash(get_test_recipients(), '84ab071fa11443c016f991ce32113442') def test_get_patients_hash(self): txm_event_1 = TxmEvent( diff --git a/tests/database/services/test_update_donor_recipient.py b/tests/database/services/test_update_donor_recipient.py index cf0e12755..44530c96a 100644 --- a/tests/database/services/test_update_donor_recipient.py +++ b/tests/database/services/test_update_donor_recipient.py @@ -42,7 +42,7 @@ def test_update_recipient(self): ), txm_event_db_id) configs = ConfigModel.query.filter(ConfigModel.txm_event_id == txm_event_db_id).all() - self.assertEqual(0, len(configs)) + self.assertEqual(1, len(configs)) self.assertSetEqual({'AB'}, {blood.blood_type for blood in RecipientModel.query.get(1).acceptable_blood}) self.assertSetEqual({'B42', 'DQ6', 'DQA1'}, {code.code for code in RecipientModel.query.get(1).hla_antibodies}) @@ -71,7 +71,7 @@ def test_update_donor(self): ), txm_event_db_id) configs = ConfigModel.query.filter(ConfigModel.txm_event_id == txm_event_db_id).all() - self.assertEqual(0, len(configs)) + self.assertEqual(1, len(configs)) self.assertSetEqual({'A11', 'DQ6', 'DQA1'}, {hla_type['code'] for hla_type in DonorModel.query.get(1).hla_typing['hla_types_list']}) diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index ab9cab8ac..5b517c5df 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -147,7 +147,7 @@ def get_patients_hash(txm_event: TxmEvent) -> int: hash_ = hashlib.md5() _update_hash(hash_, donors) _update_hash(hash_, recipients) - hash_int = int(hash_.hexdigest(), 16) % 2**64 # convert to int8 + hash_int = int(hash_.hexdigest(), 16) % 2**63 # convert to int8 return hash_int diff --git a/txmatching/database/sql_alchemy_schema.py b/txmatching/database/sql_alchemy_schema.py index ee3aa1f71..eab019153 100644 --- a/txmatching/database/sql_alchemy_schema.py +++ b/txmatching/database/sql_alchemy_schema.py @@ -22,7 +22,7 @@ class ConfigModel(db.Model): id = db.Column(Integer, primary_key=True, autoincrement=True, nullable=False) txm_event_id = db.Column(db.Integer, ForeignKey('txm_event.id', ondelete='CASCADE'), unique=False, nullable=False) parameters = db.Column(db.JSON, unique=False, nullable=False) - patients_hash = db.Column(db.Integer, unique=False, nullable=False) + patients_hash = db.Column(db.BigInteger, unique=False, nullable=False) created_by = db.Column(db.Integer, unique=False, nullable=False) # created at and updated at is not handled by triggers as then am not sure how tests would work, as triggers # seem to be specific as per db and I do not think its worth the effort as this simple approach works fine From 4e09b23085fd8cb8b7d85be62534ae9ef58855a0 Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Tue, 2 Feb 2021 23:18:18 +0100 Subject: [PATCH 07/16] Fix lint --- .../database/services/config_service.py | 7 ++++- .../database/services/patient_service.py | 30 +++++++------------ .../services/patient_upload_service.py | 2 -- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/txmatching/database/services/config_service.py b/txmatching/database/services/config_service.py index 50db29296..1a2b980c1 100644 --- a/txmatching/database/services/config_service.py +++ b/txmatching/database/services/config_service.py @@ -62,7 +62,12 @@ def remove_configs_from_txm_event(txm_event_db_id: int): ConfigModel.query.filter(ConfigModel.txm_event_id == txm_event_db_id).delete() -def _configuration_to_config_model(configuration: Configuration, txm_event_db_id: int, patients_hash: int, user_id: int) -> ConfigModel: +def _configuration_to_config_model( + configuration: Configuration, + txm_event_db_id: int, + patients_hash: int, + user_id: int +) -> ConfigModel: return ConfigModel( parameters=dataclasses.asdict(configuration), txm_event_id=txm_event_db_id, diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index 5b517c5df..ad28b0161 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -141,8 +141,10 @@ def update_donor(donor_update_dto: DonorUpdateDTO, txm_event_db_id: int) -> Dono def get_patients_hash(txm_event: TxmEvent) -> int: - donors = tuple(txm_event.active_donors_dict.values()) if txm_event.active_donors_dict is not None else None - recipients = tuple(txm_event.active_recipients_dict.values()) if txm_event.active_recipients_dict is not None else None + donors = tuple(txm_event.active_donors_dict.values()) \ + if txm_event.active_donors_dict is not None else None + recipients = tuple(txm_event.active_recipients_dict.values()) \ + if txm_event.active_recipients_dict is not None else None hash_ = hashlib.md5() _update_hash(hash_, donors) @@ -151,33 +153,23 @@ def get_patients_hash(txm_event: TxmEvent) -> int: return hash_int +# pylint: disable=too-many-branches, too-many-statements +# Many branches and statements are still readable here def _update_hash(hash_, value): - # logger.info(f"Updating hash for type {type(value)} ({value})") - type_str = f'__{type(value).__name__}__' - # logger.info(f"Hash update: {type_str}") hash_.update(type_str.encode('ASCII')) - # TODOO: remove comments - #f = open("hash_output.txt", "a") - #f.write(type_str) - #f.close() - if isinstance(value, str): - #f = open("hash_output.txt", "a") - #f.write(value) - #f.close() - # logger.info(f"Hash update: {value}") hash_.update(value.encode('ASCII')) - elif isinstance(value, int) or isinstance(value, bool) or isinstance(value, float) or isinstance(value, datetime): + elif isinstance(value, (int, bool, float, datetime)): _update_hash(hash_, str(value)) - elif isinstance(value, list) or isinstance(value, set) or isinstance(value, tuple): + elif isinstance(value, (list, set, tuple)): for item in value: _update_hash(hash_, item) elif isinstance(value, dict): - for k, v in value.items(): - _update_hash(hash_, k) - _update_hash(hash_, v) + for key, val in value.items(): + _update_hash(hash_, key) + _update_hash(hash_, val) elif value is None: _update_hash(hash_, '__none__') elif isinstance(value, Patient): diff --git a/txmatching/database/services/patient_upload_service.py b/txmatching/database/services/patient_upload_service.py index 6e5caf69c..bab3823bc 100644 --- a/txmatching/database/services/patient_upload_service.py +++ b/txmatching/database/services/patient_upload_service.py @@ -13,8 +13,6 @@ from txmatching.data_transfer_objects.patients.upload_dto.recipient_upload_dto import \ RecipientUploadDTO from txmatching.database.db import db -from txmatching.database.services.config_service import \ - remove_configs_from_txm_event from txmatching.database.services.parsing_utils import ( check_existing_ids_for_duplicates, get_hla_code, parse_date_to_datetime) from txmatching.database.services.txm_event_service import ( From c690f6c1e405df337bd12feddf82b115c93ae189 Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Wed, 3 Feb 2021 11:43:05 +0100 Subject: [PATCH 08/16] Last changes --- tests/database/services/test_patient_service.py | 2 -- txmatching/database/services/patient_service.py | 3 ++- txmatching/web/api/matching_api.py | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/database/services/test_patient_service.py b/tests/database/services/test_patient_service.py index cb101a371..84db518fb 100644 --- a/tests/database/services/test_patient_service.py +++ b/tests/database/services/test_patient_service.py @@ -184,8 +184,6 @@ def test_hashing(self): def _assert_hash(value, expected_hash_digest): hash_ = hashlib.md5() _update_hash(hash_, value) - #print(hash_.hexdigest()) # TODOO - #return self.assertEqual(hash_.hexdigest(), expected_hash_digest) _assert_hash('foo', '56c527b4cc0b522b127062dec3201194') diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index ad28b0161..498757883 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -149,7 +149,8 @@ def get_patients_hash(txm_event: TxmEvent) -> int: hash_ = hashlib.md5() _update_hash(hash_, donors) _update_hash(hash_, recipients) - hash_int = int(hash_.hexdigest(), 16) % 2**63 # convert to int8 + # Decrease hash size so that it would fit to postgres BIGINT (INT8) + hash_int = int(hash_.hexdigest(), 16) % 2**63 return hash_int diff --git a/txmatching/web/api/matching_api.py b/txmatching/web/api/matching_api.py index 108aec2b4..2973fb6a5 100644 --- a/txmatching/web/api/matching_api.py +++ b/txmatching/web/api/matching_api.py @@ -64,5 +64,5 @@ def post(self, txm_event_id: int) -> str: :configuration.max_matchings_to_show_to_viewer] calculated_matchings_dto.show_not_all_matchings_found = False - calculated_matchings_dto.calculated_matchings = calculated_matchings_dto.calculated_matchings[:50] # TODOO + calculated_matchings_dto.calculated_matchings = calculated_matchings_dto.calculated_matchings return jsonify(calculated_matchings_dto) From 36e0471bb9b462e490617d0dacd432c11706955c Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Wed, 3 Feb 2021 18:20:33 +0100 Subject: [PATCH 09/16] Apply CR comments --- .../database/services/patient_service.py | 14 ++++++++---- txmatching/patients/patient.py | 22 ++++++++++--------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index 498757883..3a1fab0dd 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -141,10 +141,8 @@ def update_donor(donor_update_dto: DonorUpdateDTO, txm_event_db_id: int) -> Dono def get_patients_hash(txm_event: TxmEvent) -> int: - donors = tuple(txm_event.active_donors_dict.values()) \ - if txm_event.active_donors_dict is not None else None - recipients = tuple(txm_event.active_recipients_dict.values()) \ - if txm_event.active_recipients_dict is not None else None + donors = tuple(txm_event.active_donors_dict.values()) + recipients = tuple(txm_event.active_recipients_dict.values()) hash_ = hashlib.md5() _update_hash(hash_, donors) @@ -157,6 +155,10 @@ def get_patients_hash(txm_event: TxmEvent) -> int: # pylint: disable=too-many-branches, too-many-statements # Many branches and statements are still readable here def _update_hash(hash_, value): + """ + Create hash for a given value that is persistent across app sessions. Contrary to this function, a hash created + using `hash()` function is not persistent (see https://docs.python.org/3/using/cmdline.html#envvar-PYTHONHASHSEED) + """ type_str = f'__{type(value).__name__}__' hash_.update(type_str.encode('ASCII')) @@ -165,9 +167,13 @@ def _update_hash(hash_, value): elif isinstance(value, (int, bool, float, datetime)): _update_hash(hash_, str(value)) elif isinstance(value, (list, set, tuple)): + # Note: Creating hash of set using this way could be possibly not secure + # (see: https://stackoverflow.com/questions/15479928) for item in value: _update_hash(hash_, item) elif isinstance(value, dict): + # Note: Creating hash of dict using this way could be possibly not secure + # (see: https://stackoverflow.com/questions/15479928) for key, val in value.items(): _update_hash(hash_, key) _update_hash(hash_, val) diff --git a/txmatching/patients/patient.py b/txmatching/patients/patient.py index 78d594ff0..0c2b5478d 100644 --- a/txmatching/patients/patient.py +++ b/txmatching/patients/patient.py @@ -67,21 +67,23 @@ def __post_init__(self): self.recipient_cutoff = calculate_cutoff(self.hla_antibodies.hla_antibodies_list) -@dataclass +@dataclass(init=False) class TxmEvent: db_id: int name: str all_donors: List[Donor] all_recipients: List[Recipient] - active_donors_dict: Optional[Dict[DonorDbId, Donor]] = None - active_recipients_dict: Optional[Dict[RecipientDbId, Recipient]] = None - - def __post_init__(self): - if not self.active_donors_dict: - self.active_donors_dict = {donor.db_id: donor for donor in self.all_donors if donor.active} - if not self.active_recipients_dict: - self.active_recipients_dict = {recipient.db_id: recipient for recipient in self.all_recipients if - recipient.related_donor_db_id in self.active_donors_dict} + active_donors_dict: Dict[DonorDbId, Donor] + active_recipients_dict: Dict[RecipientDbId, Recipient] + + def __init__(self, db_id: int, name: str, all_donors: List[Donor], all_recipients: List[Recipient]): + self.db_id = db_id + self.name = name + self.all_donors = all_donors + self.all_recipients = all_recipients + self.active_donors_dict = {donor.db_id: donor for donor in self.all_donors if donor.active} + self.active_recipients_dict = {recipient.db_id: recipient for recipient in self.all_recipients if + recipient.related_donor_db_id in self.active_donors_dict} def calculate_cutoff(hla_antibodies_list: List[HLAAntibody]) -> int: From d0441f3cae27d1e51c73f9e83805a8d213c9b805 Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Wed, 3 Feb 2021 22:34:22 +0100 Subject: [PATCH 10/16] Fix test --- tests/database/services/test_patient_service.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/tests/database/services/test_patient_service.py b/tests/database/services/test_patient_service.py index 84db518fb..477be89b2 100644 --- a/tests/database/services/test_patient_service.py +++ b/tests/database/services/test_patient_service.py @@ -204,8 +204,7 @@ def test_get_patients_hash(self): txm_event_1 = TxmEvent( 1, 'event_name_1', all_donors=get_test_donors(), - all_recipients=get_test_recipients(), - active_donors_dict=None, active_recipients_dict=None + all_recipients=get_test_recipients() ) hash_1 = get_patients_hash(txm_event_1) @@ -213,8 +212,7 @@ def test_get_patients_hash(self): txm_event_2 = TxmEvent( 2, 'event_name_2', all_donors=get_test_donors(), - all_recipients=get_test_recipients(), - active_donors_dict=None, active_recipients_dict=None + all_recipients=get_test_recipients() ) hash_2 = get_patients_hash(txm_event_2) self.assertEqual(hash_1, hash_2) @@ -223,8 +221,7 @@ def test_get_patients_hash(self): txm_event_3 = TxmEvent( 1, 'event_name_1', all_donors=[], - all_recipients=get_test_recipients(), - active_donors_dict=None, active_recipients_dict=None + all_recipients=get_test_recipients() ) hash_3 = get_patients_hash(txm_event_3) self.assertNotEqual(hash_1, hash_3) @@ -233,8 +230,7 @@ def test_get_patients_hash(self): txm_event_4 = TxmEvent( 1, 'event_name_1', all_donors=get_test_donors(), - all_recipients=[], - active_donors_dict=None, active_recipients_dict=None + all_recipients=[] ) hash_4 = get_patients_hash(txm_event_4) self.assertNotEqual(hash_1, hash_4) @@ -250,8 +246,7 @@ def test_get_patients_hash(self): txm_event_5 = TxmEvent( 1, 'event_name_1', all_donors=new_donors, - all_recipients=get_test_recipients(), - active_donors_dict=None, active_recipients_dict=None + all_recipients=get_test_recipients() ) hash_5 = get_patients_hash(txm_event_5) self.assertNotEqual(hash_1, hash_5) From 4fb845e0df096e89414d737e515a0ac4582a13c0 Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Sun, 7 Feb 2021 13:09:35 +0100 Subject: [PATCH 11/16] Move persistent hashing functionality to classes --- tests/database/services/test_file_upload.py | 5 +- .../database/services/test_patient_service.py | 41 ++----- tests/utils/test_persistent_hash.py | 41 +++++++ .../database/services/config_service.py | 7 +- .../database/services/patient_service.py | 100 ++---------------- txmatching/patients/hla_model.py | 52 +++++++-- txmatching/patients/patient.py | 40 ++++++- txmatching/patients/patient_parameters.py | 14 ++- txmatching/utils/persistent_hash.py | 52 +++++++++ 9 files changed, 214 insertions(+), 138 deletions(-) create mode 100644 tests/utils/test_persistent_hash.py create mode 100644 txmatching/utils/persistent_hash.py diff --git a/tests/database/services/test_file_upload.py b/tests/database/services/test_file_upload.py index a97bac6e4..62a7b72b9 100644 --- a/tests/database/services/test_file_upload.py +++ b/tests/database/services/test_file_upload.py @@ -6,7 +6,8 @@ from txmatching.auth.exceptions import InvalidArgumentException from txmatching.configuration.configuration import Configuration from txmatching.database.db import db -from txmatching.database.services.patient_service import get_patients_hash +from txmatching.database.services.patient_service import \ + get_patients_persistent_hash from txmatching.database.services.patient_upload_service import \ replace_or_add_patients_from_excel from txmatching.database.services.txm_event_service import ( @@ -35,7 +36,7 @@ def test_saving_patients_from_obfuscated_excel(self): config = ConfigModel( txm_event_id=txm_event.db_id, parameters={}, - patients_hash=get_patients_hash(txm_event), + patients_hash=get_patients_persistent_hash(txm_event), created_by=user_id ) diff --git a/tests/database/services/test_patient_service.py b/tests/database/services/test_patient_service.py index 477be89b2..b50d1f908 100644 --- a/tests/database/services/test_patient_service.py +++ b/tests/database/services/test_patient_service.py @@ -1,5 +1,3 @@ -import hashlib - from tests.test_utilities.create_dataclasses import (get_test_donors, get_test_recipients) from tests.test_utilities.populate_db import create_or_overwrite_txm_event @@ -13,8 +11,8 @@ from txmatching.data_transfer_objects.patients.upload_dto.recipient_upload_dto import \ RecipientUploadDTO from txmatching.database.db import db -from txmatching.database.services.patient_service import (_update_hash, - get_patients_hash) +from txmatching.database.services.patient_service import \ + get_patients_persistent_hash from txmatching.database.services.patient_upload_service import \ replace_or_add_patients_from_one_country from txmatching.database.services.txm_event_service import get_txm_event @@ -161,7 +159,7 @@ def test_update_txm_event_patients(self): config = ConfigModel( txm_event_id=txm_event.db_id, parameters={}, - patients_hash=get_patients_hash(txm_event), + patients_hash=get_patients_persistent_hash(txm_event), created_by=user_id ) @@ -178,27 +176,7 @@ def test_update_txm_event_patients(self): # Validate that patients hash has changed txm_event_new = get_txm_event(txm_event.db_id) - self.assertNotEqual(config.patients_hash, get_patients_hash(txm_event_new)) - - def test_hashing(self): - def _assert_hash(value, expected_hash_digest): - hash_ = hashlib.md5() - _update_hash(hash_, value) - self.assertEqual(hash_.hexdigest(), expected_hash_digest) - - _assert_hash('foo', '56c527b4cc0b522b127062dec3201194') - _assert_hash('42', 'aa0a056d9e7d1b3b17530b46107b91a3') - _assert_hash(42, 'd1e2cf72d8bf073f0bc2d0e8794b31ae') - _assert_hash(42.0, 'ee8b51ea1d5859dc45035c4ee9fcaedc') - _assert_hash(True, '3b3e200b7cda75063ec203db706d2463') - _assert_hash([42], '7c4aa0d7ffabc559d31ae902ae2b93a6') - _assert_hash([1, 2], '50d69227171683e044fc85f530d31568') - _assert_hash({1, 2}, '26b3a59f9692f43f20db24e6ab242cb7') - _assert_hash((1, True), 'e70e4791e78da2db05688c6043a20d86') - _assert_hash({'a': 'b'}, 'e4625008dde72175d331df31f62572e9') - _assert_hash(None, '6af5817033462a81dfdff478e27e824d') - _assert_hash(get_test_donors(), 'e799dd070b8c420c7b9c026969dbf663') - _assert_hash(get_test_recipients(), '84ab071fa11443c016f991ce32113442') + self.assertNotEqual(config.patients_hash, get_patients_persistent_hash(txm_event_new)) def test_get_patients_hash(self): txm_event_1 = TxmEvent( @@ -206,7 +184,8 @@ def test_get_patients_hash(self): all_donors=get_test_donors(), all_recipients=get_test_recipients() ) - hash_1 = get_patients_hash(txm_event_1) + hash_1 = get_patients_persistent_hash(txm_event_1) + self.assertEqual(hash_1, 1602329590066289936) # changing event db id, event name does not change the hash txm_event_2 = TxmEvent( @@ -214,7 +193,7 @@ def test_get_patients_hash(self): all_donors=get_test_donors(), all_recipients=get_test_recipients() ) - hash_2 = get_patients_hash(txm_event_2) + hash_2 = get_patients_persistent_hash(txm_event_2) self.assertEqual(hash_1, hash_2) # Changing donors changes the hash @@ -223,7 +202,7 @@ def test_get_patients_hash(self): all_donors=[], all_recipients=get_test_recipients() ) - hash_3 = get_patients_hash(txm_event_3) + hash_3 = get_patients_persistent_hash(txm_event_3) self.assertNotEqual(hash_1, hash_3) # Changing recipients changes the hash @@ -232,7 +211,7 @@ def test_get_patients_hash(self): all_donors=get_test_donors(), all_recipients=[] ) - hash_4 = get_patients_hash(txm_event_4) + hash_4 = get_patients_persistent_hash(txm_event_4) self.assertNotEqual(hash_1, hash_4) # changing hla type changes the hash @@ -248,5 +227,5 @@ def test_get_patients_hash(self): all_donors=new_donors, all_recipients=get_test_recipients() ) - hash_5 = get_patients_hash(txm_event_5) + hash_5 = get_patients_persistent_hash(txm_event_5) self.assertNotEqual(hash_1, hash_5) diff --git a/tests/utils/test_persistent_hash.py b/tests/utils/test_persistent_hash.py new file mode 100644 index 000000000..038716233 --- /dev/null +++ b/tests/utils/test_persistent_hash.py @@ -0,0 +1,41 @@ +import unittest + +from tests.test_utilities.create_dataclasses import (get_test_donors, + get_test_recipients) +from txmatching.patients.hla_model import HLAType +from txmatching.utils.persistent_hash import (initialize_persistent_hash, + update_persistent_hash) + + +class TestPersistentHash(unittest.TestCase): + + def _assert_hash(self, value, expected_hash_digest): + hash_ = initialize_persistent_hash() + update_persistent_hash(hash_, value) + self.assertEqual(hash_.hexdigest(), expected_hash_digest) + + def test_hashing(self): + self._assert_hash('foo', '56c527b4cc0b522b127062dec3201194') + self._assert_hash('42', 'aa0a056d9e7d1b3b17530b46107b91a3') + self._assert_hash(42, 'd1e2cf72d8bf073f0bc2d0e8794b31ae') + self._assert_hash(42.0, 'ee8b51ea1d5859dc45035c4ee9fcaedc') + self._assert_hash(True, '3b3e200b7cda75063ec203db706d2463') + self._assert_hash([42], '7c4aa0d7ffabc559d31ae902ae2b93a6') + self._assert_hash([1, 2], '50d69227171683e044fc85f530d31568') + with self.assertRaises(NotImplementedError): + self._assert_hash({1, 2}, '26b3a59f9692f43f20db24e6ab242cb7') + self._assert_hash((1, True), 'e70e4791e78da2db05688c6043a20d86') + with self.assertRaises(NotImplementedError): + self._assert_hash({'a': 'b'}, 'e4625008dde72175d331df31f62572e9') + self._assert_hash(None, '6af5817033462a81dfdff478e27e824d') + self._assert_hash(HLAType('A9'), 'b81f11cc22faf6f2dc6259676d9c87ed') + self._assert_hash(get_test_donors(), 'e5ccc5da231213e03d5c8a052f83ab1e') + self._assert_hash(get_test_recipients(), 'd8ad62ecbd76d13c265f163a74c58f79') + + def test_functions_update_persistent_hash(self): + value = HLAType('A9') + hash_1 = initialize_persistent_hash() + update_persistent_hash(hash_1, value) + hash_2 = initialize_persistent_hash() + value.update_persistent_hash(hash_2) + self.assertEqual(hash_1.hexdigest(), hash_2.hexdigest()) diff --git a/txmatching/database/services/config_service.py b/txmatching/database/services/config_service.py index 1a2b980c1..dd086b5e6 100644 --- a/txmatching/database/services/config_service.py +++ b/txmatching/database/services/config_service.py @@ -12,7 +12,8 @@ from txmatching.data_transfer_objects.matchings.pairing_result import \ DatabasePairingResult from txmatching.database.db import db -from txmatching.database.services.patient_service import get_patients_hash +from txmatching.database.services.patient_service import \ + get_patients_persistent_hash from txmatching.database.sql_alchemy_schema import (ConfigModel, PairingResultModel) from txmatching.patients.patient import TxmEvent @@ -44,7 +45,7 @@ def get_configuration_for_txm_event(txm_event: TxmEvent) -> Configuration: def save_configuration_to_db(configuration: Configuration, txm_event: TxmEvent, user_id: int) -> int: - patients_hash = get_patients_hash(txm_event) + patients_hash = get_patients_persistent_hash(txm_event) config_model = _configuration_to_config_model(configuration, txm_event.db_id, patients_hash, user_id) db.session.add(config_model) @@ -78,7 +79,7 @@ def _configuration_to_config_model( def find_configuration_db_id_for_configuration(configuration: Configuration, txm_event: TxmEvent) -> Optional[int]: - patients_hash = get_patients_hash(txm_event) + patients_hash = get_patients_persistent_hash(txm_event) config_models = ConfigModel.query.filter(and_( ConfigModel.txm_event_id == txm_event.db_id, ConfigModel.patients_hash == patients_hash diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index 3a1fab0dd..3f315baf1 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -1,7 +1,5 @@ import dataclasses -import hashlib import logging -from datetime import datetime from typing import Optional, Union import dacite @@ -20,8 +18,7 @@ from txmatching.database.sql_alchemy_schema import ( DonorModel, RecipientAcceptableBloodModel, RecipientHLAAntibodyModel, RecipientModel) -from txmatching.patients.hla_model import (AntibodiesPerGroup, HLAAntibodies, - HLAAntibody, HLAPerGroup, HLAType, +from txmatching.patients.hla_model import (HLAAntibodies, HLAAntibody, HLAType, HLATyping) from txmatching.patients.patient import (Donor, Patient, Recipient, RecipientRequirements, TxmEvent) @@ -31,6 +28,9 @@ from txmatching.utils.hla_system.hla_transformations_store import \ parse_hla_raw_code_and_store_parsing_error_in_db from txmatching.utils.logging_tools import PatientAdapter +from txmatching.utils.persistent_hash import (get_hash_digest, + initialize_persistent_hash, + update_persistent_hash) logger = logging.getLogger(__name__) @@ -140,96 +140,14 @@ def update_donor(donor_update_dto: DonorUpdateDTO, txm_event_db_id: int) -> Dono return get_donor_from_donor_model(DonorModel.query.get(donor_update_dto.db_id)) -def get_patients_hash(txm_event: TxmEvent) -> int: +def get_patients_persistent_hash(txm_event: TxmEvent) -> int: donors = tuple(txm_event.active_donors_dict.values()) recipients = tuple(txm_event.active_recipients_dict.values()) - hash_ = hashlib.md5() - _update_hash(hash_, donors) - _update_hash(hash_, recipients) - # Decrease hash size so that it would fit to postgres BIGINT (INT8) - hash_int = int(hash_.hexdigest(), 16) % 2**63 - return hash_int - - -# pylint: disable=too-many-branches, too-many-statements -# Many branches and statements are still readable here -def _update_hash(hash_, value): - """ - Create hash for a given value that is persistent across app sessions. Contrary to this function, a hash created - using `hash()` function is not persistent (see https://docs.python.org/3/using/cmdline.html#envvar-PYTHONHASHSEED) - """ - type_str = f'__{type(value).__name__}__' - hash_.update(type_str.encode('ASCII')) - - if isinstance(value, str): - hash_.update(value.encode('ASCII')) - elif isinstance(value, (int, bool, float, datetime)): - _update_hash(hash_, str(value)) - elif isinstance(value, (list, set, tuple)): - # Note: Creating hash of set using this way could be possibly not secure - # (see: https://stackoverflow.com/questions/15479928) - for item in value: - _update_hash(hash_, item) - elif isinstance(value, dict): - # Note: Creating hash of dict using this way could be possibly not secure - # (see: https://stackoverflow.com/questions/15479928) - for key, val in value.items(): - _update_hash(hash_, key) - _update_hash(hash_, val) - elif value is None: - _update_hash(hash_, '__none__') - elif isinstance(value, Patient): - _update_hash(hash_, value.medical_id) - _update_hash(hash_, value.parameters) - if isinstance(value, Donor): - _update_hash(hash_, value.related_recipient_db_id) - _update_hash(hash_, value.donor_type) - _update_hash(hash_, value.active) - if isinstance(value, Recipient): - _update_hash(hash_, value.related_donor_db_id) - _update_hash(hash_, sorted(value.acceptable_blood_groups)) - _update_hash(hash_, value.recipient_cutoff) - _update_hash(hash_, value.hla_antibodies) - _update_hash(hash_, value.recipient_requirements) - _update_hash(hash_, value.waiting_since) - _update_hash(hash_, value.previous_transplants) - elif isinstance(value, PatientParameters): - _update_hash(hash_, value.blood_group) - _update_hash(hash_, value.country_code) - _update_hash(hash_, value.hla_typing) - _update_hash(hash_, value.sex) - _update_hash(hash_, value.height) - _update_hash(hash_, value.weight) - _update_hash(hash_, value.year_of_birth) - elif isinstance(value, HLATyping): - _update_hash(hash_, value.hla_per_groups) - elif isinstance(value, HLAPerGroup): - _update_hash(hash_, value.hla_group) - _update_hash(hash_, sorted(value.hla_types, key=lambda hla_type: hla_type.raw_code)) - elif isinstance(value, HLAType): - _update_hash(hash_, value.raw_code) - elif isinstance(value, RecipientRequirements): - # Treat None as False - _update_hash(hash_, bool(value.require_better_match_in_compatibility_index_or_blood_group)) - _update_hash(hash_, bool(value.require_better_match_in_compatibility_index)) - _update_hash(hash_, bool(value.require_compatible_blood_group)) - elif isinstance(value, HLAAntibodies): - _update_hash(hash_, value.hla_antibodies_per_groups) - elif isinstance(value, AntibodiesPerGroup): - _update_hash(hash_, value.hla_group) - _update_hash(hash_, sorted(value.hla_antibody_list, - key=lambda hla_type: ( - hla_type.raw_code, - hla_type.mfi, - hla_type.cutoff - ))) - elif isinstance(value, HLAAntibody): - _update_hash(hash_, value.raw_code) - _update_hash(hash_, value.mfi) - _update_hash(hash_, value.cutoff) - else: - raise NotImplementedError(f'Hashing type {type(value)} ({value}) not implemented') + hash_ = initialize_persistent_hash() + update_persistent_hash(hash_, donors) + update_persistent_hash(hash_, recipients) + return get_hash_digest(hash_) def _get_base_patient_from_patient_model(patient_model: Union[DonorModel, RecipientModel]) -> Patient: diff --git a/txmatching/patients/hla_model.py b/txmatching/patients/hla_model.py index 91faeed23..e473078a5 100644 --- a/txmatching/patients/hla_model.py +++ b/txmatching/patients/hla_model.py @@ -9,10 +9,12 @@ from txmatching.utils.hla_system.hla_transformations import ( get_mfi_from_multiple_hla_codes, parse_hla_raw_code) from txmatching.utils.logging_tools import PatientAdapter +from txmatching.utils.persistent_hash import (PersistentlyHashable, + update_persistent_hash) @dataclass -class HLAType: +class HLAType(PersistentlyHashable): raw_code: str code: Optional[str] = None @@ -30,15 +32,24 @@ def __eq__(self, other): def __hash__(self): return hash(self.raw_code) + def update_persistent_hash(self, hash_): + update_persistent_hash(hash_, HLAType) + update_persistent_hash(hash_, self.raw_code) + @dataclass -class HLAPerGroup: +class HLAPerGroup(PersistentlyHashable): hla_group: HLAGroup hla_types: List[HLAType] + def update_persistent_hash(self, hash_): + update_persistent_hash(hash_, HLAPerGroup) + update_persistent_hash(hash_, self.hla_group) + update_persistent_hash(hash_, sorted(self.hla_types, key=lambda hla_type: hla_type.raw_code)) + @dataclass -class HLATyping: +class HLATyping(PersistentlyHashable): hla_types_list: List[HLAType] hla_per_groups: Optional[List[HLAPerGroup]] = None @@ -47,9 +58,13 @@ def __post_init__(self): hla_types = [hla_type for hla_type in self.hla_types_list if hla_type.code] self.hla_per_groups = _split_hla_types_to_groups(hla_types) + def update_persistent_hash(self, hash_): + update_persistent_hash(hash_, HLATyping) + update_persistent_hash(hash_, self.hla_per_groups) + @dataclass -class HLAAntibody: +class HLAAntibody(PersistentlyHashable): raw_code: str mfi: int cutoff: int @@ -69,15 +84,36 @@ def __eq__(self, other): def __hash__(self): return hash((self.raw_code, self.mfi, self.cutoff)) + def update_persistent_hash(self, hash_): + update_persistent_hash(hash_, HLAAntibody) + update_persistent_hash(hash_, self.raw_code) + update_persistent_hash(hash_, self.mfi) + update_persistent_hash(hash_, self.cutoff) + @dataclass -class AntibodiesPerGroup: +class AntibodiesPerGroup(PersistentlyHashable): hla_group: HLAGroup hla_antibody_list: List[HLAAntibody] + def update_persistent_hash(self, hash_): + update_persistent_hash(hash_, AntibodiesPerGroup) + update_persistent_hash(hash_, self.hla_group) + update_persistent_hash( + hash_, + sorted( + self.hla_antibody_list, + key=lambda hla_type: ( + hla_type.raw_code, + hla_type.mfi, + hla_type.cutoff + ) + ) + ) + @dataclass -class HLAAntibodies: +class HLAAntibodies(PersistentlyHashable): hla_antibodies_list: List[HLAAntibody] = field(default_factory=list) hla_antibodies_per_groups: List[AntibodiesPerGroup] = field(default_factory=list) @@ -111,6 +147,10 @@ def _group_key(hla_antibody: HLAAntibody) -> Tuple[int, str]: self.hla_antibodies_per_groups = _split_antibodies_to_groups(hla_antibodies_over_cutoff_list) + def update_persistent_hash(self, hash_): + update_persistent_hash(hash_, HLAAntibodies) + update_persistent_hash(hash_, self.hla_antibodies_per_groups) + def _split_hla_types_to_groups(hla_types: List[HLAType]) -> List[HLAPerGroup]: hla_types_in_groups = _split_hla_codes_to_groups(hla_types) diff --git a/txmatching/patients/patient.py b/txmatching/patients/patient.py index 0c2b5478d..aefb528fa 100644 --- a/txmatching/patients/patient.py +++ b/txmatching/patients/patient.py @@ -8,6 +8,8 @@ from txmatching.patients.patient_types import DonorDbId, RecipientDbId from txmatching.utils.blood_groups import BloodGroup from txmatching.utils.hla_system.hla_transformations import parse_hla_raw_code +from txmatching.utils.persistent_hash import (PersistentlyHashable, + update_persistent_hash) DEFAULT_CUTOFF = 2000 @@ -19,7 +21,7 @@ class DonorType(str, Enum): @dataclass -class Patient: +class Patient(PersistentlyHashable): db_id: int medical_id: str parameters: PatientParameters @@ -30,16 +32,28 @@ def is_recipient(self) -> bool: def is_donor(self) -> bool: return isinstance(self, Donor) + def update_persistent_hash(self, hash_): + update_persistent_hash(hash_, Patient) + update_persistent_hash(hash_, self.medical_id) + update_persistent_hash(hash_, self.parameters) + @dataclass -class Donor(Patient): +class Donor(Patient, PersistentlyHashable): related_recipient_db_id: Optional[RecipientDbId] = None donor_type: DonorType = DonorType.DONOR active: bool = True + def update_persistent_hash(self, hash_): + super().update_persistent_hash(hash_) + update_persistent_hash(hash_, Donor) + update_persistent_hash(hash_, self.related_recipient_db_id) + update_persistent_hash(hash_, self.donor_type) + update_persistent_hash(hash_, self.active) + @dataclass -class RecipientRequirements: +class RecipientRequirements(PersistentlyHashable): """ Attributes: require_new_donor_having_better_match_in_compatibility_index: New donor for recipient needs to have @@ -51,9 +65,16 @@ class RecipientRequirements: require_better_match_in_compatibility_index_or_blood_group: Optional[bool] = None require_compatible_blood_group: Optional[bool] = None + def update_persistent_hash(self, hash_): + update_persistent_hash(hash_, RecipientRequirements) + # Treat None as False + update_persistent_hash(hash_, bool(self.require_better_match_in_compatibility_index_or_blood_group)) + update_persistent_hash(hash_, bool(self.require_better_match_in_compatibility_index)) + update_persistent_hash(hash_, bool(self.require_compatible_blood_group)) + @dataclass -class Recipient(Patient): +class Recipient(Patient, PersistentlyHashable): related_donor_db_id: DonorDbId acceptable_blood_groups: List[BloodGroup] recipient_cutoff: Optional[int] = None @@ -66,6 +87,17 @@ def __post_init__(self): if self.recipient_cutoff is None: self.recipient_cutoff = calculate_cutoff(self.hla_antibodies.hla_antibodies_list) + def update_persistent_hash(self, hash_): + super().update_persistent_hash(hash_) + update_persistent_hash(hash_, Recipient) + update_persistent_hash(hash_, self.related_donor_db_id) + update_persistent_hash(hash_, sorted(self.acceptable_blood_groups)) + update_persistent_hash(hash_, self.recipient_cutoff) + update_persistent_hash(hash_, self.hla_antibodies) + update_persistent_hash(hash_, self.recipient_requirements) + update_persistent_hash(hash_, self.waiting_since) + update_persistent_hash(hash_, self.previous_transplants) + @dataclass(init=False) class TxmEvent: diff --git a/txmatching/patients/patient_parameters.py b/txmatching/patients/patient_parameters.py index f6b08e5df..dce24807e 100644 --- a/txmatching/patients/patient_parameters.py +++ b/txmatching/patients/patient_parameters.py @@ -4,13 +4,15 @@ from txmatching.patients.hla_model import HLATyping from txmatching.utils.blood_groups import BloodGroup from txmatching.utils.enums import Country, Sex +from txmatching.utils.persistent_hash import (PersistentlyHashable, + update_persistent_hash) Kilograms = float Centimeters = int @dataclass -class PatientParameters: +class PatientParameters(PersistentlyHashable): blood_group: BloodGroup country_code: Country hla_typing: HLATyping @@ -18,3 +20,13 @@ class PatientParameters: height: Optional[Centimeters] = None weight: Optional[Kilograms] = None year_of_birth: Optional[int] = None + + def update_persistent_hash(self, hash_): + update_persistent_hash(hash_, PatientParameters) + update_persistent_hash(hash_, self.blood_group) + update_persistent_hash(hash_, self.country_code) + update_persistent_hash(hash_, self.hla_typing) + update_persistent_hash(hash_, self.sex) + update_persistent_hash(hash_, self.height) + update_persistent_hash(hash_, self.weight) + update_persistent_hash(hash_, self.year_of_birth) diff --git a/txmatching/utils/persistent_hash.py b/txmatching/utils/persistent_hash.py new file mode 100644 index 000000000..34b3ce0f7 --- /dev/null +++ b/txmatching/utils/persistent_hash.py @@ -0,0 +1,52 @@ +import hashlib +from abc import ABC, abstractmethod +from datetime import datetime + + +class PersistentlyHashable(ABC): + @abstractmethod + def update_persistent_hash(self, hash_): + pass + + def persistent_hash(self): + hash_ = initialize_persistent_hash() + self.update_persistent_hash(hash_) + return get_hash_digest(hash_) + + +def initialize_persistent_hash(): + return hashlib.md5() + + +def get_hash_digest(hash_) -> int: + # Decrease hash size so that it would fit to postgres BIGINT (INT8) + return int(hash_.hexdigest(), 16) % 2**63 + + +def update_persistent_hash(hash_, value): + """ + Create hash for a given value that is persistent across app sessions. Contrary to this function, a hash created + using `hash()` function is not persistent (see https://docs.python.org/3/using/cmdline.html#envvar-PYTHONHASHSEED) + """ + + if isinstance(value, type): + hash_.update(f'__{value.__name__}__'.encode('ASCII')) + elif isinstance(value, PersistentlyHashable): + value.update_persistent_hash(hash_) + else: + update_persistent_hash(hash_, type(value)) + + # Note: Creating hash of set or dict using this way would be possibly not secure + # (see: https://stackoverflow.com/questions/15479928) + # One option for implementing that is to sort the values or keys by their persistent hash + if isinstance(value, str): + hash_.update(value.encode('ASCII')) + elif isinstance(value, (int, bool, float, datetime)): + update_persistent_hash(hash_, str(value)) + elif isinstance(value, (list, tuple)): + for item in value: + update_persistent_hash(hash_, item) + elif value is None: + update_persistent_hash(hash_, '__none__') + else: + raise NotImplementedError(f'Hashing type {type(value)} ({value}) not implemented') From bfa17d46ed3b93808e8876de9e6e20a1f01d3f5c Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Sun, 7 Feb 2021 13:17:59 +0100 Subject: [PATCH 12/16] Measure hashing time --- txmatching/database/services/patient_service.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index 3f315baf1..635ffe006 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -1,5 +1,6 @@ import dataclasses import logging +import time from typing import Optional, Union import dacite @@ -144,10 +145,15 @@ def get_patients_persistent_hash(txm_event: TxmEvent) -> int: donors = tuple(txm_event.active_donors_dict.values()) recipients = tuple(txm_event.active_recipients_dict.values()) + start = time.time() hash_ = initialize_persistent_hash() update_persistent_hash(hash_, donors) update_persistent_hash(hash_, recipients) - return get_hash_digest(hash_) + hash_digest = get_hash_digest(hash_) + end = time.time() + + logger.debug(f'Creating persistent hash of {len(donors) + len(recipients)} patients took {end - start} seconds') + return hash_digest def _get_base_patient_from_patient_model(patient_model: Union[DonorModel, RecipientModel]) -> Patient: From 4b00d31686ecfecb7f4c1d2da3132b574777f18b Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Sun, 7 Feb 2021 13:21:35 +0100 Subject: [PATCH 13/16] Fix merge --- txmatching/database/services/patient_upload_service.py | 1 - 1 file changed, 1 deletion(-) diff --git a/txmatching/database/services/patient_upload_service.py b/txmatching/database/services/patient_upload_service.py index effe6156e..147a59594 100644 --- a/txmatching/database/services/patient_upload_service.py +++ b/txmatching/database/services/patient_upload_service.py @@ -33,7 +33,6 @@ def add_donor_recipient_pair(donor_recipient_pair_dto: DonorRecipientPairDTO, txm_event_db_id: int): - remove_configs_from_txm_event(txm_event_db_id) if donor_recipient_pair_dto.recipient: donor_recipient_pair_dto.donor.related_recipient_medical_id = donor_recipient_pair_dto.recipient.medical_id From dc392eff6097bcac26f4a3c960183e176b539f62 Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Sun, 7 Feb 2021 23:20:42 +0100 Subject: [PATCH 14/16] Apply TK comments --- tests/database/services/test_patient_service.py | 2 +- txmatching/database/services/config_service.py | 4 ---- txmatching/database/services/patient_service.py | 3 --- txmatching/web/api/matching_api.py | 1 - 4 files changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/database/services/test_patient_service.py b/tests/database/services/test_patient_service.py index b4a1d2dc4..33f791d89 100644 --- a/tests/database/services/test_patient_service.py +++ b/tests/database/services/test_patient_service.py @@ -187,7 +187,7 @@ def test_get_patients_hash(self): hash_1 = get_patients_persistent_hash(txm_event_1) self.assertEqual(hash_1, 1602329590066289936) - # changing event db id, event name does not change the hash + # changing event db id or event name does not change the hash txm_event_2 = TxmEvent( 2, 'event_name_2', all_donors=get_test_donors(), diff --git a/txmatching/database/services/config_service.py b/txmatching/database/services/config_service.py index dd086b5e6..490fb58e4 100644 --- a/txmatching/database/services/config_service.py +++ b/txmatching/database/services/config_service.py @@ -59,10 +59,6 @@ def get_latest_config_model_for_txm_event(txm_event_db_id: int) -> Optional[Conf return config -def remove_configs_from_txm_event(txm_event_db_id: int): - ConfigModel.query.filter(ConfigModel.txm_event_id == txm_event_db_id).delete() - - def _configuration_to_config_model( configuration: Configuration, txm_event_db_id: int, diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index a997cd4a9..198a2b4b3 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -152,14 +152,11 @@ def get_patients_persistent_hash(txm_event: TxmEvent) -> int: donors = tuple(txm_event.active_donors_dict.values()) recipients = tuple(txm_event.active_recipients_dict.values()) - start = time.time() hash_ = initialize_persistent_hash() update_persistent_hash(hash_, donors) update_persistent_hash(hash_, recipients) hash_digest = get_hash_digest(hash_) - end = time.time() - logger.debug(f'Creating persistent hash of {len(donors) + len(recipients)} patients took {end - start} seconds') return hash_digest diff --git a/txmatching/web/api/matching_api.py b/txmatching/web/api/matching_api.py index 2973fb6a5..a105dcae4 100644 --- a/txmatching/web/api/matching_api.py +++ b/txmatching/web/api/matching_api.py @@ -64,5 +64,4 @@ def post(self, txm_event_id: int) -> str: :configuration.max_matchings_to_show_to_viewer] calculated_matchings_dto.show_not_all_matchings_found = False - calculated_matchings_dto.calculated_matchings = calculated_matchings_dto.calculated_matchings return jsonify(calculated_matchings_dto) From 9a2763471a8ac156614bbc161cc7da630c723cb6 Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Mon, 8 Feb 2021 11:30:30 +0100 Subject: [PATCH 15/16] Move testing objects --- tests/test_utilities/create_dataclasses.py | 38 ++++++++++++++----- tests/utils/test_matching.py | 34 +++-------------- .../database/services/patient_service.py | 1 - 3 files changed, 33 insertions(+), 40 deletions(-) diff --git a/tests/test_utilities/create_dataclasses.py b/tests/test_utilities/create_dataclasses.py index 0ff2e904c..8982f4311 100644 --- a/tests/test_utilities/create_dataclasses.py +++ b/tests/test_utilities/create_dataclasses.py @@ -1,11 +1,12 @@ -from txmatching.patients.hla_model import HLAAntibodies, HLAType, HLATyping +from txmatching.patients.hla_model import (HLAAntibodies, HLAAntibody, HLAType, + HLATyping) from txmatching.patients.patient import (Donor, DonorType, Recipient, RecipientRequirements) from txmatching.patients.patient_parameters import PatientParameters from txmatching.utils.blood_groups import BloodGroup from txmatching.utils.enums import Country, Sex -RAW_CODES = [ +_RAW_CODES = [ 'A1', 'A32', 'B7', @@ -15,6 +16,10 @@ ] +def get_test_raw_codes(): + return _RAW_CODES.copy() + + def get_test_donors(): return [ Donor( @@ -25,8 +30,8 @@ def get_test_donors(): country_code=Country.CZE, hla_typing=HLATyping( hla_types_list=[ - HLAType(raw_code=RAW_CODES[0]), - HLAType(raw_code=RAW_CODES[1]), + HLAType(raw_code=_RAW_CODES[0]), + HLAType(raw_code=_RAW_CODES[1]), HLAType(raw_code='B44'), HLAType(raw_code='DR10') ] @@ -47,8 +52,8 @@ def get_test_donors(): country_code=Country.CZE, hla_typing=HLATyping( hla_types_list=[ - HLAType(raw_code=RAW_CODES[1]), - HLAType(raw_code=RAW_CODES[2]), + HLAType(raw_code=_RAW_CODES[1]), + HLAType(raw_code=_RAW_CODES[2]), HLAType(raw_code='DR10') ] ), @@ -73,8 +78,8 @@ def get_test_recipients(): country_code=Country.CZE, hla_typing=HLATyping( hla_types_list=[ - HLAType(raw_code=RAW_CODES[1]), - HLAType(raw_code=RAW_CODES[2]), + HLAType(raw_code=_RAW_CODES[1]), + HLAType(raw_code=_RAW_CODES[2]), HLAType(raw_code='DR1') ] ), @@ -101,8 +106,8 @@ def get_test_recipients(): hla_types_list=[ HLAType(raw_code='A3'), HLAType(raw_code='B38'), - HLAType(raw_code=RAW_CODES[4]), - HLAType(raw_code=RAW_CODES[5]), + HLAType(raw_code=_RAW_CODES[4]), + HLAType(raw_code=_RAW_CODES[5]), ] ), sex=Sex.M, @@ -119,3 +124,16 @@ def get_test_recipients(): previous_transplants=None ), ] + + +def get_test_antibodies(): + return HLAAntibodies( + hla_antibodies_list=[ + HLAAntibody('A7', 1200, 1000, 'A7'), + HLAAntibody('B32', 1200, 1000, 'B32'), + HLAAntibody('DR40', 1200, 1000, 'DR40'), + HLAAntibody('B5', 1200, 1000, 'B5'), + HLAAntibody('DR9', 1200, 1000, 'DR9'), + HLAAntibody('A23', 1200, 1000, 'A23') + ] + ) diff --git a/tests/utils/test_matching.py b/tests/utils/test_matching.py index a14ba6e70..5a2252097 100644 --- a/tests/utils/test_matching.py +++ b/tests/utils/test_matching.py @@ -1,8 +1,9 @@ import unittest -from tests.test_utilities.create_dataclasses import (get_test_donors, +from tests.test_utilities.create_dataclasses import (get_test_antibodies, + get_test_donors, + get_test_raw_codes, get_test_recipients) -from txmatching.patients.hla_model import HLAAntibodies, HLAAntibody from txmatching.scorers.matching import ( calculate_compatibility_index_for_group, get_count_of_transplants, get_matching_hla_typing_code) @@ -12,38 +13,13 @@ from txmatching.solvers.matching.transplant_sequence import TransplantSequence from txmatching.utils.enums import HLAGroup -RAW_CODES = [ - 'A1', - 'A32', - 'B7', - 'B51', - 'DR11', - 'DR15' -] +RAW_CODES = get_test_raw_codes() DONORS = get_test_donors() RECIPIENTS = get_test_recipients() -TEST_ANTIGENS = [ - 'A7', - 'B32', - 'DR40', - 'B5', - 'DR9', - 'A23' -] - -TEST_ANTIBODIES = HLAAntibodies( - hla_antibodies_list=[ - HLAAntibody('A7', 1200, 1000, 'A7'), - HLAAntibody('B32', 1200, 1000, 'B32'), - HLAAntibody('DR40', 1200, 1000, 'DR40'), - HLAAntibody('B5', 1200, 1000, 'B5'), - HLAAntibody('DR9', 1200, 1000, 'DR9'), - HLAAntibody('A23', 1200, 1000, 'A23') - ] -) +TEST_ANTIBODIES = get_test_antibodies() class TestMatching(unittest.TestCase): diff --git a/txmatching/database/services/patient_service.py b/txmatching/database/services/patient_service.py index 198a2b4b3..cee81cee5 100644 --- a/txmatching/database/services/patient_service.py +++ b/txmatching/database/services/patient_service.py @@ -1,6 +1,5 @@ import dataclasses import logging -import time from typing import Optional, Union import dacite From 8406f36a19f243ea3cc7852492b331e9ac059909 Mon Sep 17 00:00:00 2001 From: Tomas Pavlin Date: Mon, 8 Feb 2021 15:39:24 +0100 Subject: [PATCH 16/16] Annote hash type --- txmatching/patients/hla_model.py | 14 +++++++------- txmatching/patients/patient.py | 10 +++++----- txmatching/patients/patient_parameters.py | 4 ++-- txmatching/utils/persistent_hash.py | 13 +++++++++---- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/txmatching/patients/hla_model.py b/txmatching/patients/hla_model.py index b0fbd5b64..d8a485c4d 100644 --- a/txmatching/patients/hla_model.py +++ b/txmatching/patients/hla_model.py @@ -9,7 +9,7 @@ from txmatching.utils.hla_system.hla_transformations import ( get_mfi_from_multiple_hla_codes, parse_hla_raw_code) from txmatching.utils.logging_tools import PatientAdapter -from txmatching.utils.persistent_hash import (PersistentlyHashable, +from txmatching.utils.persistent_hash import (HashType, PersistentlyHashable, update_persistent_hash) @@ -31,7 +31,7 @@ def __eq__(self, other): def __hash__(self): return hash(self.raw_code) - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): update_persistent_hash(hash_, HLAType) update_persistent_hash(hash_, self.raw_code) @@ -41,7 +41,7 @@ class HLAPerGroup(PersistentlyHashable): hla_group: HLAGroup hla_types: List[HLAType] - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): update_persistent_hash(hash_, HLAPerGroup) update_persistent_hash(hash_, self.hla_group) update_persistent_hash(hash_, sorted(self.hla_types, key=lambda hla_type: hla_type.raw_code)) @@ -57,7 +57,7 @@ def __post_init__(self): hla_types = [hla_type for hla_type in self.hla_types_list if hla_type.code] self.hla_per_groups = _split_hla_types_to_groups(hla_types) - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): update_persistent_hash(hash_, HLATyping) update_persistent_hash(hash_, self.hla_per_groups) @@ -82,7 +82,7 @@ def __eq__(self, other): def __hash__(self): return hash((self.raw_code, self.mfi, self.cutoff)) - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): update_persistent_hash(hash_, HLAAntibody) update_persistent_hash(hash_, self.raw_code) update_persistent_hash(hash_, self.mfi) @@ -94,7 +94,7 @@ class AntibodiesPerGroup(PersistentlyHashable): hla_group: HLAGroup hla_antibody_list: List[HLAAntibody] - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): update_persistent_hash(hash_, AntibodiesPerGroup) update_persistent_hash(hash_, self.hla_group) update_persistent_hash( @@ -150,7 +150,7 @@ def _group_key(hla_antibody: HLAAntibody) -> str: self.hla_antibodies_per_groups = _split_antibodies_to_groups(hla_antibodies_over_cutoff_list) - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): update_persistent_hash(hash_, HLAAntibodies) update_persistent_hash(hash_, self.hla_antibodies_per_groups) diff --git a/txmatching/patients/patient.py b/txmatching/patients/patient.py index aefb528fa..d286af7ab 100644 --- a/txmatching/patients/patient.py +++ b/txmatching/patients/patient.py @@ -8,7 +8,7 @@ from txmatching.patients.patient_types import DonorDbId, RecipientDbId from txmatching.utils.blood_groups import BloodGroup from txmatching.utils.hla_system.hla_transformations import parse_hla_raw_code -from txmatching.utils.persistent_hash import (PersistentlyHashable, +from txmatching.utils.persistent_hash import (HashType, PersistentlyHashable, update_persistent_hash) DEFAULT_CUTOFF = 2000 @@ -32,7 +32,7 @@ def is_recipient(self) -> bool: def is_donor(self) -> bool: return isinstance(self, Donor) - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): update_persistent_hash(hash_, Patient) update_persistent_hash(hash_, self.medical_id) update_persistent_hash(hash_, self.parameters) @@ -44,7 +44,7 @@ class Donor(Patient, PersistentlyHashable): donor_type: DonorType = DonorType.DONOR active: bool = True - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): super().update_persistent_hash(hash_) update_persistent_hash(hash_, Donor) update_persistent_hash(hash_, self.related_recipient_db_id) @@ -65,7 +65,7 @@ class RecipientRequirements(PersistentlyHashable): require_better_match_in_compatibility_index_or_blood_group: Optional[bool] = None require_compatible_blood_group: Optional[bool] = None - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): update_persistent_hash(hash_, RecipientRequirements) # Treat None as False update_persistent_hash(hash_, bool(self.require_better_match_in_compatibility_index_or_blood_group)) @@ -87,7 +87,7 @@ def __post_init__(self): if self.recipient_cutoff is None: self.recipient_cutoff = calculate_cutoff(self.hla_antibodies.hla_antibodies_list) - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): super().update_persistent_hash(hash_) update_persistent_hash(hash_, Recipient) update_persistent_hash(hash_, self.related_donor_db_id) diff --git a/txmatching/patients/patient_parameters.py b/txmatching/patients/patient_parameters.py index dce24807e..1ce2c7875 100644 --- a/txmatching/patients/patient_parameters.py +++ b/txmatching/patients/patient_parameters.py @@ -4,7 +4,7 @@ from txmatching.patients.hla_model import HLATyping from txmatching.utils.blood_groups import BloodGroup from txmatching.utils.enums import Country, Sex -from txmatching.utils.persistent_hash import (PersistentlyHashable, +from txmatching.utils.persistent_hash import (HashType, PersistentlyHashable, update_persistent_hash) Kilograms = float @@ -21,7 +21,7 @@ class PatientParameters(PersistentlyHashable): weight: Optional[Kilograms] = None year_of_birth: Optional[int] = None - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): update_persistent_hash(hash_, PatientParameters) update_persistent_hash(hash_, self.blood_group) update_persistent_hash(hash_, self.country_code) diff --git a/txmatching/utils/persistent_hash.py b/txmatching/utils/persistent_hash.py index 34b3ce0f7..3d5216092 100644 --- a/txmatching/utils/persistent_hash.py +++ b/txmatching/utils/persistent_hash.py @@ -1,11 +1,16 @@ +from __future__ import annotations + import hashlib from abc import ABC, abstractmethod from datetime import datetime +# pylint: disable=undefined-variable) +HashType: TypeAlias = 'hashlib._Hash' + class PersistentlyHashable(ABC): @abstractmethod - def update_persistent_hash(self, hash_): + def update_persistent_hash(self, hash_: HashType): pass def persistent_hash(self): @@ -14,16 +19,16 @@ def persistent_hash(self): return get_hash_digest(hash_) -def initialize_persistent_hash(): +def initialize_persistent_hash() -> HashType: return hashlib.md5() -def get_hash_digest(hash_) -> int: +def get_hash_digest(hash_: HashType) -> int: # Decrease hash size so that it would fit to postgres BIGINT (INT8) return int(hash_.hexdigest(), 16) % 2**63 -def update_persistent_hash(hash_, value): +def update_persistent_hash(hash_: HashType, value: any): """ Create hash for a given value that is persistent across app sessions. Contrary to this function, a hash created using `hash()` function is not persistent (see https://docs.python.org/3/using/cmdline.html#envvar-PYTHONHASHSEED)