From 4dd94c8320e08c0d1059a83d2c4cf2a2f1edabc9 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Wed, 21 Aug 2024 16:09:57 +0200 Subject: [PATCH 01/28] initial commit --- cg/services/file_delivery/abstract_classes.py | 11 -- .../fetch_analysis_files_service.py | 16 ++- .../fetch_fastq_analysis_files_service.py | 12 +- .../fetch_fastq_files_service.py | 15 ++- .../fetch_file_service/models.py | 6 + .../filter_delivery_files_service.py | 14 --- .../move_delivery_files_service.py | 107 ++++++++++++++++++ 7 files changed, 149 insertions(+), 32 deletions(-) delete mode 100644 cg/services/file_delivery/filter_delivery_files/filter_delivery_files_service.py create mode 100644 cg/services/file_delivery/move_files_service/move_delivery_files_service.py diff --git a/cg/services/file_delivery/abstract_classes.py b/cg/services/file_delivery/abstract_classes.py index 0f0ffc3fca..e125be3180 100644 --- a/cg/services/file_delivery/abstract_classes.py +++ b/cg/services/file_delivery/abstract_classes.py @@ -16,17 +16,6 @@ def format_files(self, case_id: str) -> None: pass -class MoveDeliveryFilesService(ABC): - """ - Abstract class that encapsulates the logic required for moving files to the customer folder. including formatting and renaming. - """ - - @abstractmethod - def move_files(self, case_id: str) -> None: - """Move the files to the customer folder.""" - pass - - class DeliverFilesService(ABC): """ Abstract class that encapsulates the logic required for delivering files to the customer. diff --git a/cg/services/file_delivery/fetch_file_service/fetch_analysis_files_service.py b/cg/services/file_delivery/fetch_file_service/fetch_analysis_files_service.py index df881d189b..691b9c705b 100644 --- a/cg/services/file_delivery/fetch_file_service/fetch_analysis_files_service.py +++ b/cg/services/file_delivery/fetch_file_service/fetch_analysis_files_service.py @@ -8,7 +8,12 @@ ) from housekeeper.store.models import File -from cg.services.file_delivery.fetch_file_service.models import SampleFile, CaseFile, DeliveryFiles +from cg.services.file_delivery.fetch_file_service.models import ( + SampleFile, + CaseFile, + DeliveryFiles, + DeliveryMetaData, +) from cg.store.models import Case from cg.store.store import Store @@ -31,7 +36,14 @@ def get_files_to_deliver(self, case_id: str) -> DeliveryFiles: case: Case = self.status_db.get_case_by_internal_id(internal_id=case_id) analysis_case_files: list[CaseFile] = self.get_analysis_case_delivery_files(case) analysis_sample_files: list[SampleFile] = self._get_analysis_sample_delivery_files(case) - return DeliveryFiles(case_files=analysis_case_files, sample_files=analysis_sample_files) + delivery_data = DeliveryMetaData( + customer_internal_id=case.customer.internal_id, ticket_id=case.latest_ticket + ) + return DeliveryFiles( + delivery_data=delivery_data, + case_files=analysis_case_files, + sample_files=analysis_sample_files, + ) def _get_sample_files_from_case_bundle( self, workflow: Workflow, sample_id: str, case_id: str diff --git a/cg/services/file_delivery/fetch_file_service/fetch_fastq_analysis_files_service.py b/cg/services/file_delivery/fetch_file_service/fetch_fastq_analysis_files_service.py index a894de630a..9a1c811469 100644 --- a/cg/services/file_delivery/fetch_file_service/fetch_fastq_analysis_files_service.py +++ b/cg/services/file_delivery/fetch_file_service/fetch_fastq_analysis_files_service.py @@ -11,7 +11,8 @@ from cg.services.file_delivery.fetch_file_service.fetch_fastq_files_service import ( FetchFastqDeliveryFilesService, ) -from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles, DeliveryMetaData +from cg.store.models import Case from cg.store.store import Store @@ -21,6 +22,7 @@ def __init__(self, status_db: Store, hk_api: HousekeeperAPI): self.hk_api = hk_api def get_files_to_deliver(self, case_id: str) -> DeliveryFiles: + case: Case = self.status_db.get_case_by_internal_id(internal_id=case_id) fetch_fastq_service = FetchFastqDeliveryFilesService( self.status_db, self.hk_api, @@ -33,7 +35,11 @@ def get_files_to_deliver(self, case_id: str) -> DeliveryFiles: ) fastq_files = fetch_fastq_service.get_files_to_deliver(case_id) analysis_files = fetch_analysis_service.get_files_to_deliver(case_id) + delivery_data = DeliveryMetaData( + customer_internal_id=case.customer.internal_id, ticket_id=case.latest_ticket + ) return DeliveryFiles( - case_files=analysis_files.case_files, - sample_files=fastq_files.sample_files + analysis_files.sample_files, + delivery_data=delivery_data, + case_files=analysis_case_files, + sample_files=analysis_sample_files, ) diff --git a/cg/services/file_delivery/fetch_file_service/fetch_fastq_files_service.py b/cg/services/file_delivery/fetch_file_service/fetch_fastq_files_service.py index 71c53b7f8e..e029f5623c 100644 --- a/cg/services/file_delivery/fetch_file_service/fetch_fastq_files_service.py +++ b/cg/services/file_delivery/fetch_file_service/fetch_fastq_files_service.py @@ -7,7 +7,11 @@ from cg.services.file_delivery.fetch_file_service.fetch_delivery_files_service import ( FetchDeliveryFilesService, ) -from cg.services.file_delivery.fetch_file_service.models import SampleFile, DeliveryFiles +from cg.services.file_delivery.fetch_file_service.models import ( + SampleFile, + DeliveryFiles, + DeliveryMetaData, +) from cg.store.models import Case from cg.store.store import Store from housekeeper.store.models import File @@ -39,7 +43,14 @@ def get_files_to_deliver(self, case_id: str) -> DeliveryFiles: ): fastq_files.extend(sample_fastq_files) - return DeliveryFiles(case_files=None, sample_files=fastq_files) + delivery_data = DeliveryMetaData( + customer_internal_id=case.customer.internal_id, ticket_id=case.latest_ticket + ) + return DeliveryFiles( + delivery_data=delivery_data, + case_files=None, + sample_files=fastq_files, + ) @handle_missing_bundle_errors def _get_fastq_files_for_sample(self, case_id: str, sample_id: str) -> list[SampleFile]: diff --git a/cg/services/file_delivery/fetch_file_service/models.py b/cg/services/file_delivery/fetch_file_service/models.py index 0320d93c9a..99380e69f7 100644 --- a/cg/services/file_delivery/fetch_file_service/models.py +++ b/cg/services/file_delivery/fetch_file_service/models.py @@ -3,6 +3,11 @@ from pydantic import BaseModel +class DeliveryMetaData(BaseModel): + customer_internal_id: str + ticket_id: str + + class CaseFile(BaseModel): case_id: str file_path: Path @@ -15,5 +20,6 @@ class SampleFile(BaseModel): class DeliveryFiles(BaseModel): + delivery_data: DeliveryMetaData case_files: list[CaseFile] | None sample_files: list[SampleFile] diff --git a/cg/services/file_delivery/filter_delivery_files/filter_delivery_files_service.py b/cg/services/file_delivery/filter_delivery_files/filter_delivery_files_service.py deleted file mode 100644 index aeb6feb88d..0000000000 --- a/cg/services/file_delivery/filter_delivery_files/filter_delivery_files_service.py +++ /dev/null @@ -1,14 +0,0 @@ -from abc import abstractmethod, ABC - -from cg.services.delivery.fetch_file_service.models import DeliveryFiles - - -class FilterDeliveryFilesService(ABC): - """ - Abstract class that encapsulates the logic required for filtering files to deliver. - """ - - @abstractmethod - def filter_files(self, deliver_files: DeliveryFiles) -> None: - """Filter the files to deliver.""" - pass diff --git a/cg/services/file_delivery/move_files_service/move_delivery_files_service.py b/cg/services/file_delivery/move_files_service/move_delivery_files_service.py new file mode 100644 index 0000000000..dde89ac8d1 --- /dev/null +++ b/cg/services/file_delivery/move_files_service/move_delivery_files_service.py @@ -0,0 +1,107 @@ +from abc import abstractmethod, ABC +from pathlib import Path + +# Exchan + +from cg.constants.delivery import INBOX_NAME +from cg.services.file_delivery.fetch_file_service.models import ( + DeliveryFiles, + DeliveryMetaData, + CaseFile, + SampleFile, +) +from cg.utils.files import link_or_overwrite_file + + +class MoveDeliveryFilesService: + """ + Class that encapsulates the logic required for moving files to the customer folder. + """ + + def move_files(self, delivery_files: DeliveryFiles, delivery_base_path: Path) -> DeliveryFiles: + """Move the files to the customer folder.""" + inbox_path: Path = self._create_ticket_inbox_path( + delivery_base_path, delivery_files.delivery_data + ) + self._create_ticket_inbox_folder(inbox_path) + self._create_hard_links_for_delivery_files( + delivery_files=delivery_files, inbox_path=inbox_path + ) + return self._replace_file_paths_with_inbox_paths( + delivery_files=delivery_files, inbox_path=inbox_path + ) + + @staticmethod + def _create_ticket_inbox_folder( + inbox_path: Path, + ) -> Path: + """Create a ticket inbox folder in the customer folder.""" + if not inbox_path.exists(): + inbox_path.mkdir(parents=True) + return inbox_path + raise FileExistsError(f"Ticket inbox folder {inbox_path} already exists for ticket.") + + @staticmethod + def _create_ticket_inbox_path( + delivery_base_path: Path, delivery_data: DeliveryMetaData + ) -> Path: + """Create the path to the ticket inbox folder.""" + return Path( + delivery_base_path, + delivery_data.customer_internal_id, + INBOX_NAME, + delivery_data.ticket_id, + ) + + @staticmethod + def _create_inbox_file_path(file_path: Path, inbox_path: Path) -> Path: + """Create the path to the inbox file.""" + return Path(inbox_path, file_path.name) + + def _create_hard_link_file_paths( + self, file_models: list[SampleFile | CaseFile], inbox_path: Path + ) -> None: + """Create hard links to the sample files in the customer folder.""" + for file_model in file_models: + inbox_file_path: Path = self._create_inbox_file_path( + file_path=file_model.file_path, inbox_path=inbox_path + ) + link_or_overwrite_file(src=file_model.file_path, dst=inbox_file_path) + + def _create_hard_links_for_delivery_files( + self, delivery_files: DeliveryFiles, inbox_path: Path + ) -> None: + """Create hard links to the files in the customer folder.""" + if delivery_files.case_files: + self._create_hard_link_file_paths( + file_models=delivery_files.case_files, inbox_path=inbox_path + ) + self._create_hard_link_file_paths( + file_models=delivery_files.sample_files, inbox_path=inbox_path + ) + + def _replace_file_path_with_inbox_path( + self, file_models: list[SampleFile | CaseFile], inbox_path: Path + ) -> list[SampleFile | CaseFile]: + """Replace the file path with the inbox path.""" + for file_model in file_models: + inbox_file_path: Path = self._create_inbox_file_path( + file_path=file_model.file_path, inbox_path=inbox_path + ) + file_model.file_path = inbox_file_path + return file_models + + def _replace_file_paths_with_inbox_paths( + self, + delivery_files: DeliveryFiles, + inbox_path: Path, + ) -> DeliveryFiles: + """Replace the file paths with the inbox paths.""" + if delivery_files.case_files: + delivery_files.case_files = self._replace_file_path_with_inbox_path( + file_models=delivery_files.case_files, inbox_path=inbox_path + ) + delivery_files.sample_files = self._replace_file_path_with_inbox_path( + file_models=delivery_files.sample_files, inbox_path=inbox_path + ) + return delivery_files From c09db2fed13121a1a96abc20ab746ce1169b70e5 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Wed, 21 Aug 2024 16:20:22 +0200 Subject: [PATCH 02/28] fixes --- cg/services/file_delivery/abstract_classes.py | 3 +++ .../move_files_service/move_delivery_files_service.py | 8 +++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cg/services/file_delivery/abstract_classes.py b/cg/services/file_delivery/abstract_classes.py index e125be3180..69eb9c2e24 100644 --- a/cg/services/file_delivery/abstract_classes.py +++ b/cg/services/file_delivery/abstract_classes.py @@ -3,6 +3,9 @@ from cg.services.file_delivery.fetch_file_service.fetch_delivery_files_service import ( FetchDeliveryFilesService, ) +from cg.services.file_delivery.move_files_service.move_delivery_files_service import ( + MoveDeliveryFilesService, +) class FormatDeliveryFilesService(ABC): diff --git a/cg/services/file_delivery/move_files_service/move_delivery_files_service.py b/cg/services/file_delivery/move_files_service/move_delivery_files_service.py index dde89ac8d1..44ba51160b 100644 --- a/cg/services/file_delivery/move_files_service/move_delivery_files_service.py +++ b/cg/services/file_delivery/move_files_service/move_delivery_files_service.py @@ -1,8 +1,4 @@ -from abc import abstractmethod, ABC from pathlib import Path - -# Exchan - from cg.constants.delivery import INBOX_NAME from cg.services.file_delivery.fetch_file_service.models import ( DeliveryFiles, @@ -96,7 +92,9 @@ def _replace_file_paths_with_inbox_paths( delivery_files: DeliveryFiles, inbox_path: Path, ) -> DeliveryFiles: - """Replace the file paths with the inbox paths.""" + """ + Replace to original file paths in the delivery files with the customer inbox file paths. + """ if delivery_files.case_files: delivery_files.case_files = self._replace_file_path_with_inbox_path( file_models=delivery_files.case_files, inbox_path=inbox_path From f449da483d22360a31260061e4db538ad358589d Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Wed, 21 Aug 2024 16:29:32 +0200 Subject: [PATCH 03/28] fix --- .../delivery_files_models_fixtures.py | 31 +++++++++++++++++-- .../test_move_delivery_file_service.py | 4 +++ 2 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py index de7113591a..25b39c011b 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py @@ -6,7 +6,14 @@ AlignmentFileTag, SequencingFileTag, ) -from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles, SampleFile, CaseFile +from cg.services.file_delivery.fetch_file_service.models import ( + DeliveryFiles, + SampleFile, + CaseFile, + DeliveryMetaData, +) +from cg.store.models import Case +from cg.store.store import Store @pytest.fixture @@ -15,6 +22,7 @@ def expected_fastq_delivery_files( case_id: str, sample_id: str, another_sample_id: str, + delivery_store_microsalt: Store, ) -> DeliveryFiles: """Return the expected fastq delivery files.""" hk_bundle_names: list[str] = [sample_id, another_sample_id] @@ -28,7 +36,11 @@ def expected_fastq_delivery_files( ) for sample in hk_bundle_names ] - return DeliveryFiles(case_files=None, sample_files=sample_files) + case: Case = delivery_store_microsalt.get_case_by_internal_id(case_id) + delivery_data = DeliveryMetaData( + customer_internal_id=case.customer.internal_id, ticket_id=case.latest_ticket + ) + return DeliveryFiles(delivery_data=delivery_data, case_files=None, sample_files=sample_files) @pytest.fixture @@ -37,6 +49,7 @@ def expected_analysis_delivery_files( case_id: str, sample_id: str, another_sample_id: str, + delivery_store_balsamic: Store, ) -> DeliveryFiles: """Return the expected analysis delivery files.""" hk_bundle_names: list[str] = [sample_id, another_sample_id] @@ -62,4 +75,16 @@ def expected_analysis_delivery_files( )[0].full_path, ) ] - return DeliveryFiles(case_files=case_files, sample_files=sample_files) + case: Case = delivery_store_balsamic.get_case_by_internal_id(case_id) + delivery_data = DeliveryMetaData( + customer_internal_id=case.customer.internal_id, ticket_id=case.latest_ticket + ) + return DeliveryFiles( + delivery_data=delivery_data, case_files=case_files, sample_files=sample_files + ) + + +@pytest.fixture +def moved_fastq_delivery_files(expected_fastq_delivery_files: DeliveryFiles) -> DeliveryFiles: + """Return the moved fastq delivery files.""" + return expected_fastq_delivery_files diff --git a/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py new file mode 100644 index 0000000000..fc13345118 --- /dev/null +++ b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py @@ -0,0 +1,4 @@ +"""Test the move delivery files service.""" + + +def test_move_files(expected_analysis_delivery_files): From 33456ef8bd55fba68510906c2f16cb92e928151c Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Wed, 21 Aug 2024 16:48:16 +0200 Subject: [PATCH 04/28] add test --- .../delivery_files_models_fixtures.py | 33 ++++++++++++++++++- .../test_move_delivery_file_service.py | 32 +++++++++++++++++- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py index 25b39c011b..dad33c6661 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py @@ -1,6 +1,9 @@ +from pathlib import Path + import pytest from cg.apps.housekeeper.hk import HousekeeperAPI +from cg.constants.delivery import INBOX_NAME from cg.constants.housekeeper_tags import ( HK_DELIVERY_REPORT_TAG, AlignmentFileTag, @@ -85,6 +88,34 @@ def expected_analysis_delivery_files( @pytest.fixture -def moved_fastq_delivery_files(expected_fastq_delivery_files: DeliveryFiles) -> DeliveryFiles: +def expected_moved_fastq_delivery_files( + expected_fastq_delivery_files: DeliveryFiles, tmp_path +) -> DeliveryFiles: """Return the moved fastq delivery files.""" + inbox_path: Path = Path( + tmp_path, + expected_fastq_delivery_files.delivery_data.customer_internal_id, + INBOX_NAME, + expected_fastq_delivery_files.delivery_data.ticket_id, + ) + for sample_file in expected_fastq_delivery_files.sample_files: + sample_file.file_path = Path(inbox_path, sample_file.file_path.name) return expected_fastq_delivery_files + + +@pytest.fixture +def expected_moved_analysis_delivery_files( + expected_analysis_delivery_files: DeliveryFiles, tmp_path +) -> DeliveryFiles: + """Return the moved analysis delivery files.""" + inbox_path: Path = Path( + tmp_path, + expected_analysis_delivery_files.delivery_data.customer_internal_id, + INBOX_NAME, + expected_analysis_delivery_files.delivery_data.ticket_id, + ) + for sample_file in expected_analysis_delivery_files.sample_files: + sample_file.file_path = Path(inbox_path, sample_file.file_path.name) + for case_file in expected_analysis_delivery_files.case_files: + case_file.file_path = Path(inbox_path, case_file.file_path.name) + return expected_analysis_delivery_files diff --git a/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py index fc13345118..98e3d08765 100644 --- a/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py +++ b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py @@ -1,4 +1,34 @@ """Test the move delivery files service.""" +import pytest -def test_move_files(expected_analysis_delivery_files): +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from cg.services.file_delivery.move_files_service.move_delivery_files_service import ( + MoveDeliveryFilesService, +) + + +@pytest.mark.parametrize( + "expected_moved_delivery_files,delivery_files", + [ + ("expected_moved_fastq_delivery_files", "expected_fastq_delivery_files"), + ("expected_moved_analysis_delivery_files", "expected_analysis_delivery_files"), + ], +) +def test_move_files( + expected_moved_delivery_files: DeliveryFiles, delivery_files: DeliveryFiles, tmp_path, request +): + # GIVEN a delivery files to move + expected_moved_delivery_files: DeliveryFiles = request.getfixturevalue( + expected_moved_delivery_files + ) + delivery_files: DeliveryFiles = request.getfixturevalue(delivery_files) + + # WHEN moving the delivery files + file_mover = MoveDeliveryFilesService() + moved_delivery_files: DeliveryFiles = file_mover.move_files( + delivery_files=delivery_files, delivery_base_path=tmp_path + ) + + # THEN assert that the delivery files are moved + assert moved_delivery_files == expected_moved_delivery_files From f17554028148d20cf99c674013df9e204a2e79fa Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Thu, 22 Aug 2024 09:01:08 +0200 Subject: [PATCH 05/28] fix test --- .../delivery_files_models_fixtures.py | 49 ++++++++++++++----- .../test_move_delivery_file_service.py | 6 ++- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py index dad33c6661..16f1dcd432 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py @@ -92,15 +92,21 @@ def expected_moved_fastq_delivery_files( expected_fastq_delivery_files: DeliveryFiles, tmp_path ) -> DeliveryFiles: """Return the moved fastq delivery files.""" + delivery_files = DeliveryFiles(**expected_fastq_delivery_files.model_dump()) inbox_path: Path = Path( tmp_path, - expected_fastq_delivery_files.delivery_data.customer_internal_id, + delivery_files.delivery_data.customer_internal_id, INBOX_NAME, - expected_fastq_delivery_files.delivery_data.ticket_id, + delivery_files.delivery_data.ticket_id, + ) + new_sample_files: list[SampleFile] = swap_file_paths_with_inbox_paths( + delivery_files.sample_files, inbox_path + ) + return DeliveryFiles( + delivery_data=expected_fastq_delivery_files.delivery_data, + case_files=None, + sample_files=new_sample_files, ) - for sample_file in expected_fastq_delivery_files.sample_files: - sample_file.file_path = Path(inbox_path, sample_file.file_path.name) - return expected_fastq_delivery_files @pytest.fixture @@ -108,14 +114,33 @@ def expected_moved_analysis_delivery_files( expected_analysis_delivery_files: DeliveryFiles, tmp_path ) -> DeliveryFiles: """Return the moved analysis delivery files.""" + delivery_files = DeliveryFiles(**expected_analysis_delivery_files.model_dump()) inbox_path: Path = Path( tmp_path, - expected_analysis_delivery_files.delivery_data.customer_internal_id, + delivery_files.delivery_data.customer_internal_id, INBOX_NAME, - expected_analysis_delivery_files.delivery_data.ticket_id, + delivery_files.delivery_data.ticket_id, + ) + new_case_files: list[CaseFile] = swap_file_paths_with_inbox_paths( + delivery_files.case_files, inbox_path + ) + new_sample_files: list[SampleFile] = swap_file_paths_with_inbox_paths( + delivery_files.sample_files, inbox_path ) - for sample_file in expected_analysis_delivery_files.sample_files: - sample_file.file_path = Path(inbox_path, sample_file.file_path.name) - for case_file in expected_analysis_delivery_files.case_files: - case_file.file_path = Path(inbox_path, case_file.file_path.name) - return expected_analysis_delivery_files + return DeliveryFiles( + delivery_data=delivery_files.delivery_data, + case_files=new_case_files, + sample_files=new_sample_files, + ) + + +def swap_file_paths_with_inbox_paths( + file_models: list[CaseFile | SampleFile], inbox_path: Path +) -> list[CaseFile | SampleFile]: + """Swap the file paths with the inbox paths.""" + new_file_models: list[SampleFile | CaseFile] = [] + for file_model in file_models: + new_file_model: SampleFile = file_model + new_file_model.file_path = Path(inbox_path, file_model.file_path.name) + new_file_models.append(new_file_model) + return new_file_models diff --git a/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py index 98e3d08765..da70b82a39 100644 --- a/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py +++ b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py @@ -16,14 +16,16 @@ ], ) def test_move_files( - expected_moved_delivery_files: DeliveryFiles, delivery_files: DeliveryFiles, tmp_path, request + expected_moved_delivery_files: DeliveryFiles, + delivery_files: DeliveryFiles, + tmp_path, + request, ): # GIVEN a delivery files to move expected_moved_delivery_files: DeliveryFiles = request.getfixturevalue( expected_moved_delivery_files ) delivery_files: DeliveryFiles = request.getfixturevalue(delivery_files) - # WHEN moving the delivery files file_mover = MoveDeliveryFilesService() moved_delivery_files: DeliveryFiles = file_mover.move_files( From 511ed7f2c9b93658d661d5f539c1fa5d1a86bdc7 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Thu, 22 Aug 2024 09:53:41 +0200 Subject: [PATCH 06/28] add exists assertion --- .../test_move_delivery_file_service.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py index da70b82a39..3521bf3439 100644 --- a/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py +++ b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py @@ -34,3 +34,8 @@ def test_move_files( # THEN assert that the delivery files are moved assert moved_delivery_files == expected_moved_delivery_files + if case_files := moved_delivery_files.case_files: + for case_file in case_files: + assert case_file.file_path.exists + for sample_file in moved_delivery_files.sample_files: + assert sample_file.file_path.exists() From 9063fc86c37a39e3a5df24048d23247fa404f1ba Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Thu, 22 Aug 2024 09:54:13 +0200 Subject: [PATCH 07/28] add func call --- .../test_move_delivery_file_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py index 3521bf3439..f01ece3415 100644 --- a/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py +++ b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py @@ -36,6 +36,6 @@ def test_move_files( assert moved_delivery_files == expected_moved_delivery_files if case_files := moved_delivery_files.case_files: for case_file in case_files: - assert case_file.file_path.exists + assert case_file.file_path.exists() for sample_file in moved_delivery_files.sample_files: assert sample_file.file_path.exists() From 8a64d3b041a336e6e859adb3fd383f560ebac173 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Thu, 22 Aug 2024 13:38:47 +0200 Subject: [PATCH 08/28] initial commit --- cg/services/file_delivery/abstract_classes.py | 16 +-- .../fetch_analysis_files_service.py | 15 ++- .../fetch_fastq_analysis_files_service.py | 8 +- .../fetch_fastq_files_service.py | 8 +- .../fetch_file_service/models.py | 2 + .../file_formatter_service/__init__.py | 0 .../delivery_file_formatting_service.py | 15 +++ ...generic_delivery_file_formatter_service.py | 119 ++++++++++++++++++ ...crosalt_delivery_file_formatter_service.py | 24 ++++ .../file_formatter_service/models.py | 11 ++ .../mutant_delivery_file_formatter_service.py | 14 +++ .../delivery_files_models_fixtures.py | 30 +++-- 12 files changed, 235 insertions(+), 27 deletions(-) create mode 100644 cg/services/file_delivery/file_formatter_service/__init__.py create mode 100644 cg/services/file_delivery/file_formatter_service/delivery_file_formatting_service.py create mode 100644 cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py create mode 100644 cg/services/file_delivery/file_formatter_service/microsalt_delivery_file_formatter_service.py create mode 100644 cg/services/file_delivery/file_formatter_service/models.py create mode 100644 cg/services/file_delivery/file_formatter_service/mutant_delivery_file_formatter_service.py diff --git a/cg/services/file_delivery/abstract_classes.py b/cg/services/file_delivery/abstract_classes.py index 69eb9c2e24..0aec7ad806 100644 --- a/cg/services/file_delivery/abstract_classes.py +++ b/cg/services/file_delivery/abstract_classes.py @@ -3,22 +3,14 @@ from cg.services.file_delivery.fetch_file_service.fetch_delivery_files_service import ( FetchDeliveryFilesService, ) +from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( + DeliveryFileFormattingService, +) from cg.services.file_delivery.move_files_service.move_delivery_files_service import ( MoveDeliveryFilesService, ) -class FormatDeliveryFilesService(ABC): - """ - Abstract class that encapsulates the logic required for formatting files to deliver. - """ - - @abstractmethod - def format_files(self, case_id: str) -> None: - """Format the files to deliver.""" - pass - - class DeliverFilesService(ABC): """ Abstract class that encapsulates the logic required for delivering files to the customer. @@ -35,9 +27,11 @@ def __init__( self, delivery_file_manager_service: FetchDeliveryFilesService, move_file_service: MoveDeliveryFilesService, + file_formatter_service: DeliveryFileFormattingService, ): self.file_manager = delivery_file_manager_service self.file_mover = move_file_service + self.file_formatter = file_formatter_service @abstractmethod def deliver_files_for_case(self, case_id: str) -> None: diff --git a/cg/services/file_delivery/fetch_file_service/fetch_analysis_files_service.py b/cg/services/file_delivery/fetch_file_service/fetch_analysis_files_service.py index 691b9c705b..ef74b86268 100644 --- a/cg/services/file_delivery/fetch_file_service/fetch_analysis_files_service.py +++ b/cg/services/file_delivery/fetch_file_service/fetch_analysis_files_service.py @@ -51,12 +51,17 @@ def _get_sample_files_from_case_bundle( """Return a list of files from a case bundle with a sample id as tag.""" sample_tags: list[set[str]] = self.tags_fetcher.fetch_tags(workflow).sample_tags sample_tags_with_sample_id: list[set[str]] = [tag | {sample_id} for tag in sample_tags] - sample_files: list[File] = self.hk_api.get_files_from_latest_version_containing_tags( bundle_name=case_id, tags=sample_tags_with_sample_id ) + sample_name: str = self.status_db.get_sample_by_internal_id(sample_id).name return [ - SampleFile(case_id=case_id, sample_id=sample_id, file_path=sample_file.full_path) + SampleFile( + case_id=case_id, + sample_id=sample_id, + sample_name=sample_name, + file_path=sample_file.full_path, + ) for sample_file in sample_files ] @@ -82,6 +87,10 @@ def get_analysis_case_delivery_files(self, case: Case) -> list[CaseFile]: bundle_name=case.internal_id, tags=case_tags, excluded_tags=sample_id_tags ) return [ - CaseFile(case_id=case.internal_id, file_path=case_file.full_path) + CaseFile( + case_id=case.internal_id, + case_name=case.name, + file_path=case_file.full_path, + ) for case_file in case_files ] diff --git a/cg/services/file_delivery/fetch_file_service/fetch_fastq_analysis_files_service.py b/cg/services/file_delivery/fetch_file_service/fetch_fastq_analysis_files_service.py index 9a1c811469..3dab5ddb71 100644 --- a/cg/services/file_delivery/fetch_file_service/fetch_fastq_analysis_files_service.py +++ b/cg/services/file_delivery/fetch_file_service/fetch_fastq_analysis_files_service.py @@ -33,13 +33,13 @@ def get_files_to_deliver(self, case_id: str) -> DeliveryFiles: self.hk_api, tags_fetcher=FetchSampleAndCaseDeliveryFileTagsService(), ) - fastq_files = fetch_fastq_service.get_files_to_deliver(case_id) - analysis_files = fetch_analysis_service.get_files_to_deliver(case_id) + fastq_files: DeliveryFiles = fetch_fastq_service.get_files_to_deliver(case_id) + analysis_files: DeliveryFiles = fetch_analysis_service.get_files_to_deliver(case_id) delivery_data = DeliveryMetaData( customer_internal_id=case.customer.internal_id, ticket_id=case.latest_ticket ) return DeliveryFiles( delivery_data=delivery_data, - case_files=analysis_case_files, - sample_files=analysis_sample_files, + case_files=analysis_files.case_files, + sample_files=analysis_files.sample_files + fastq_files.sample_files, ) diff --git a/cg/services/file_delivery/fetch_file_service/fetch_fastq_files_service.py b/cg/services/file_delivery/fetch_file_service/fetch_fastq_files_service.py index e029f5623c..ef64ccc200 100644 --- a/cg/services/file_delivery/fetch_file_service/fetch_fastq_files_service.py +++ b/cg/services/file_delivery/fetch_file_service/fetch_fastq_files_service.py @@ -59,7 +59,13 @@ def _get_fastq_files_for_sample(self, case_id: str, sample_id: str) -> list[Samp fastq_files: list[File] = self.hk_api.get_files_from_latest_version_containing_tags( bundle_name=sample_id, tags=fastq_tags ) + sample_name: str = self.status_db.get_sample_by_internal_id(sample_id).name return [ - SampleFile(case_id=case_id, sample_id=sample_id, file_path=fastq_file.full_path) + SampleFile( + case_id=case_id, + sample_id=sample_id, + sample_name=sample_name, + file_path=fastq_file.full_path, + ) for fastq_file in fastq_files ] diff --git a/cg/services/file_delivery/fetch_file_service/models.py b/cg/services/file_delivery/fetch_file_service/models.py index 99380e69f7..aa75c12b08 100644 --- a/cg/services/file_delivery/fetch_file_service/models.py +++ b/cg/services/file_delivery/fetch_file_service/models.py @@ -10,12 +10,14 @@ class DeliveryMetaData(BaseModel): class CaseFile(BaseModel): case_id: str + case_name: str file_path: Path class SampleFile(BaseModel): case_id: str sample_id: str + sample_name: str file_path: Path diff --git a/cg/services/file_delivery/file_formatter_service/__init__.py b/cg/services/file_delivery/file_formatter_service/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cg/services/file_delivery/file_formatter_service/delivery_file_formatting_service.py b/cg/services/file_delivery/file_formatter_service/delivery_file_formatting_service.py new file mode 100644 index 0000000000..2a772e167e --- /dev/null +++ b/cg/services/file_delivery/file_formatter_service/delivery_file_formatting_service.py @@ -0,0 +1,15 @@ +from abc import abstractmethod, ABC + +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from cg.store.store import Store + + +class DeliveryFileFormattingService(ABC): + """ + Abstract class that encapsulates the logic required for formatting files to deliver. + """ + + @abstractmethod + def format_files(self, delivery_files: DeliveryFiles) -> None: + """Format the files to deliver.""" + pass diff --git a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py new file mode 100644 index 0000000000..9038667472 --- /dev/null +++ b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py @@ -0,0 +1,119 @@ +import os +from pathlib import Path + +from cg.constants import FileExtensions +from cg.constants.delivery import INBOX_NAME +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles, CaseFile, SampleFile +from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( + DeliveryFileFormattingService, +) +from cg.services.file_delivery.file_formatter_service.models import FormattedFiles, FormattedFile + + +class GenericDeliveryFileFormatter(DeliveryFileFormattingService): + """ + Format the files to be delivered in the generic format. + Expected structure: + /inbox/// + /inbox//// + /inbox/// (i.e. fastq files) + """ + + def format_files(self, delivery_files: DeliveryFiles) -> None: + """Format the files to be delivered in the generic format.""" + case_name: str = delivery_files.case_files[0].case_name + samples_names: list[set[str]] = self._get_sample_names(delivery_files.sample_files) + self._create_folder_structure( + case_name=case_name, sample_names=samples_names, delivery_files=delivery_files + ) + formatted_files: FormattedFiles = self._get_formatted_files(delivery_files) + for formatted_file in formatted_files.files: + os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) + + @staticmethod + def _get_sample_names(sample_files: list[SampleFile]) -> list[set[str]]: + return [set(sample_file.sample_name for sample_file in sample_files)] + + @staticmethod + def _is_fastq_file(file_path: Path) -> bool: + return FileExtensions.FASTQ in str(file_path) + + @staticmethod + def _is_inbox_path(file_path: Path): + return INBOX_NAME in str(file_path) + + def _get_ticket_dir_path(self, file_path) -> Path: + if self._is_inbox_path(file_path): + return file_path.parent + + @staticmethod + def _create_case_name_folder(ticket_path: Path, case_name: str) -> Path: + case_dir_path = Path(ticket_path, case_name) + case_dir_path.mkdir(exist_ok=True) + return case_dir_path + + @staticmethod + def _create_sample_folders(parent_path: Path, sample_names: list[str]): + for sample_name in sample_names: + sample_dir_path = Path(parent_path, sample_name) + sample_dir_path.mkdir(exist_ok=True) + + def _create_folder_structure( + self, case_name: str, sample_names: list[str], delivery_files: DeliveryFiles + ): + ticket_dir_path: Path = self._get_ticket_dir_path(delivery_files.sample_files[0].file_path) + case_dir_path: Path = self._create_case_name_folder( + ticket_path=ticket_dir_path, case_name=case_name + ) + self._create_sample_folders(parent_path=ticket_dir_path, sample_names=sample_names) + self._create_sample_folders(parent_path=case_dir_path, sample_names=sample_names) + + def _get_formatted_files(self, deliver_files: DeliveryFiles) -> FormattedFiles: + formatted_case_files: list[FormattedFile] = self._format_case_files( + deliver_files.case_files + ) + formatted_sample_files: list[FormattedFile] = self._format_sample_files( + sample_files=deliver_files.sample_files, case_name=deliver_files.case_files[0].case_name + ) + return FormattedFiles(files=formatted_case_files + formatted_sample_files) + + @staticmethod + def _format_case_files(case_files: list[CaseFile]) -> list[FormattedFile]: + formatted_files: list[FormattedFile] = [] + for case_file in case_files: + replaced_case_file_name: str = case_file.file_path.name.replace( + case_file.case_id, case_file.case_name + ) + formatted_file_path = Path( + case_file.file_path.parent, case_file.case_name, replaced_case_file_name + ) + formatted_files.append( + FormattedFile(original_path=case_file.file_path, formatted_path=formatted_file_path) + ) + return formatted_files + + def _format_sample_files( + self, sample_files: list[SampleFile], case_name: str + ) -> list[FormattedFile]: + formatted_files: list[FormattedFile] = [] + for sample_file in sample_files: + replaced_sample_file_name: str = sample_file.file_path.name.replace( + sample_file.sample_id, sample_file.sample_name + ) + if self._is_fastq_file(sample_file.file_path): + formatted_file_path = Path( + sample_file.file_path.parent, sample_file.sample_name, replaced_sample_file_name + ) + else: + formatted_file_path = Path( + sample_file.file_path.parent, + case_name, + sample_file.sample_name, + replaced_sample_file_name, + ) + formatted_files.append( + FormattedFile( + original_path=sample_file.file_path, formatted_path=formatted_file_path + ) + ) + return formatted_files diff --git a/cg/services/file_delivery/file_formatter_service/microsalt_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/microsalt_delivery_file_formatter_service.py new file mode 100644 index 0000000000..6af3e55567 --- /dev/null +++ b/cg/services/file_delivery/file_formatter_service/microsalt_delivery_file_formatter_service.py @@ -0,0 +1,24 @@ +from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( + FastqConcatenationService, +) +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( + DeliveryFileFormattingService, +) + + +class MicrosaltDeliveryFileFormatter(DeliveryFileFormattingService): + """ + Reformat the files to be delivered in the microsalt format. + Expected structure: + /inbox/// + /inbox/// (i.e. fastq files) + Extra rule: + fastq files are concatenated into a single file per read direction. + """ + + def __init__(self, concatenation_service: FastqConcatenationService): + self.concatenation_service = concatenation_service + + def format_files(self, delivery_files: DeliveryFiles) -> None: + """Format the files to be delivered in the generic format.""" diff --git a/cg/services/file_delivery/file_formatter_service/models.py b/cg/services/file_delivery/file_formatter_service/models.py new file mode 100644 index 0000000000..d4be95ff73 --- /dev/null +++ b/cg/services/file_delivery/file_formatter_service/models.py @@ -0,0 +1,11 @@ +from pathlib import Path +from pydantic import BaseModel + + +class FormattedFile(BaseModel): + original_path: Path + formatted_path: Path + + +class FormattedFiles(BaseModel): + files: list[FormattedFile] diff --git a/cg/services/file_delivery/file_formatter_service/mutant_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/mutant_delivery_file_formatter_service.py new file mode 100644 index 0000000000..e70fe9f282 --- /dev/null +++ b/cg/services/file_delivery/file_formatter_service/mutant_delivery_file_formatter_service.py @@ -0,0 +1,14 @@ +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( + DeliveryFileFormattingService, +) + + +class MutantDeliveryFileFormatter(DeliveryFileFormattingService): + """ + Reformat the files to be delivered in the mutant format. + + """ + + def format_files(self, delivery_files: DeliveryFiles) -> None: + """Format the files to be delivered in the generic format.""" diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py index 16f1dcd432..601a72e7c5 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py @@ -24,20 +24,26 @@ def expected_fastq_delivery_files( delivery_housekeeper_api: HousekeeperAPI, case_id: str, sample_id: str, + sample_name: str, another_sample_id: str, + another_sample_name: str, delivery_store_microsalt: Store, ) -> DeliveryFiles: """Return the expected fastq delivery files.""" - hk_bundle_names: list[str] = [sample_id, another_sample_id] + sample_info: list[tuple[str, str]] = [ + (sample_id, sample_name), + (another_sample_id, another_sample_name), + ] sample_files: list[SampleFile] = [ SampleFile( case_id=case_id, - sample_id=sample, + sample_id=sample[0], + sample_name=sample[1], file_path=delivery_housekeeper_api.get_files_from_latest_version( - bundle_name=sample, tags=[SequencingFileTag.FASTQ] + bundle_name=sample[0], tags=[SequencingFileTag.FASTQ] )[0].full_path, ) - for sample in hk_bundle_names + for sample in sample_info ] case: Case = delivery_store_microsalt.get_case_by_internal_id(case_id) delivery_data = DeliveryMetaData( @@ -50,29 +56,37 @@ def expected_fastq_delivery_files( def expected_analysis_delivery_files( delivery_housekeeper_api: HousekeeperAPI, case_id: str, + case_name: str, sample_id: str, + sample_name: str, another_sample_id: str, + another_sample_name: str, delivery_store_balsamic: Store, ) -> DeliveryFiles: """Return the expected analysis delivery files.""" - hk_bundle_names: list[str] = [sample_id, another_sample_id] + sample_info: list[tuple[str, str]] = [ + (sample_id, sample_name), + (another_sample_id, another_sample_name), + ] sample_files: list[SampleFile] = [] - for sample in hk_bundle_names: + for sample in sample_info: sample_files.extend( [ SampleFile( case_id=case_id, - sample_id=sample, + sample_id=sample[0], + sample_name=sample[1], file_path=file.full_path, ) for file in delivery_housekeeper_api.get_files_from_latest_version( - bundle_name=case_id, tags=[AlignmentFileTag.CRAM, sample] + bundle_name=case_id, tags=[AlignmentFileTag.CRAM, sample[0]] ) ] ) case_files: list[CaseFile] = [ CaseFile( case_id=case_id, + case_name=case_name, file_path=delivery_housekeeper_api.get_files_from_latest_version( bundle_name=case_id, tags=[HK_DELIVERY_REPORT_TAG] )[0].full_path, From 83d6c9ff0e61050b04311fb5fa7968bc994d7cdd Mon Sep 17 00:00:00 2001 From: ChristianOertlin Date: Thu, 22 Aug 2024 13:52:29 +0200 Subject: [PATCH 09/28] Update tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py Co-authored-by: Henrik Stranneheim --- .../delivery_fixtures/delivery_files_models_fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py index 16f1dcd432..6e7036f630 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py @@ -91,7 +91,7 @@ def expected_analysis_delivery_files( def expected_moved_fastq_delivery_files( expected_fastq_delivery_files: DeliveryFiles, tmp_path ) -> DeliveryFiles: - """Return the moved fastq delivery files.""" + """Return the moved FASTQ delivery files.""" delivery_files = DeliveryFiles(**expected_fastq_delivery_files.model_dump()) inbox_path: Path = Path( tmp_path, From bacaa111f81c2eadd7fac0d15baab0c51b801581 Mon Sep 17 00:00:00 2001 From: ChristianOertlin Date: Thu, 22 Aug 2024 13:52:40 +0200 Subject: [PATCH 10/28] Update tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py Co-authored-by: Henrik Stranneheim --- .../delivery_fixtures/delivery_files_models_fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py index 6e7036f630..1005deb9f6 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py @@ -135,7 +135,7 @@ def expected_moved_analysis_delivery_files( def swap_file_paths_with_inbox_paths( - file_models: list[CaseFile | SampleFile], inbox_path: Path + file_models: list[CaseFile | SampleFile], inbox_dir_path: Path ) -> list[CaseFile | SampleFile]: """Swap the file paths with the inbox paths.""" new_file_models: list[SampleFile | CaseFile] = [] From 05330718ce1f43016575ae5a59aaa36df8e8b010 Mon Sep 17 00:00:00 2001 From: ChristianOertlin Date: Thu, 22 Aug 2024 13:53:12 +0200 Subject: [PATCH 11/28] Apply suggestions from code review Co-authored-by: Henrik Stranneheim --- .../delivery_files_models_fixtures.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py index 1005deb9f6..9e0ad5610c 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py @@ -40,7 +40,7 @@ def expected_fastq_delivery_files( for sample in hk_bundle_names ] case: Case = delivery_store_microsalt.get_case_by_internal_id(case_id) - delivery_data = DeliveryMetaData( + delivery_meta_data = DeliveryMetaData( customer_internal_id=case.customer.internal_id, ticket_id=case.latest_ticket ) return DeliveryFiles(delivery_data=delivery_data, case_files=None, sample_files=sample_files) @@ -79,7 +79,7 @@ def expected_analysis_delivery_files( ) ] case: Case = delivery_store_balsamic.get_case_by_internal_id(case_id) - delivery_data = DeliveryMetaData( + delivery_meta_data = DeliveryMetaData( customer_internal_id=case.customer.internal_id, ticket_id=case.latest_ticket ) return DeliveryFiles( @@ -93,7 +93,7 @@ def expected_moved_fastq_delivery_files( ) -> DeliveryFiles: """Return the moved FASTQ delivery files.""" delivery_files = DeliveryFiles(**expected_fastq_delivery_files.model_dump()) - inbox_path: Path = Path( + inbox_dir_path = Path( tmp_path, delivery_files.delivery_data.customer_internal_id, INBOX_NAME, @@ -111,11 +111,11 @@ def expected_moved_fastq_delivery_files( @pytest.fixture def expected_moved_analysis_delivery_files( - expected_analysis_delivery_files: DeliveryFiles, tmp_path + expected_analysis_delivery_files: DeliveryFiles, tmp_path: Path ) -> DeliveryFiles: """Return the moved analysis delivery files.""" delivery_files = DeliveryFiles(**expected_analysis_delivery_files.model_dump()) - inbox_path: Path = Path( + inbox_dir_path = Path( tmp_path, delivery_files.delivery_data.customer_internal_id, INBOX_NAME, From 3fbc9efd904327d8e537ae652d9c2f5d236eb6e5 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Thu, 22 Aug 2024 13:56:53 +0200 Subject: [PATCH 12/28] review --- .../move_delivery_files_service.py | 54 +++++++++---------- .../delivery_files_models_fixtures.py | 16 +++--- .../test_move_delivery_file_service.py | 5 +- 3 files changed, 39 insertions(+), 36 deletions(-) diff --git a/cg/services/file_delivery/move_files_service/move_delivery_files_service.py b/cg/services/file_delivery/move_files_service/move_delivery_files_service.py index 44ba51160b..0e5ef200eb 100644 --- a/cg/services/file_delivery/move_files_service/move_delivery_files_service.py +++ b/cg/services/file_delivery/move_files_service/move_delivery_files_service.py @@ -16,29 +16,29 @@ class MoveDeliveryFilesService: def move_files(self, delivery_files: DeliveryFiles, delivery_base_path: Path) -> DeliveryFiles: """Move the files to the customer folder.""" - inbox_path: Path = self._create_ticket_inbox_path( + inbox_dir_path: Path = self._create_ticket_inbox_dir_path( delivery_base_path, delivery_files.delivery_data ) - self._create_ticket_inbox_folder(inbox_path) + self._create_ticket_inbox_folder(inbox_dir_path) self._create_hard_links_for_delivery_files( - delivery_files=delivery_files, inbox_path=inbox_path + delivery_files=delivery_files, inbox_dir_path=inbox_dir_path ) - return self._replace_file_paths_with_inbox_paths( - delivery_files=delivery_files, inbox_path=inbox_path + return self._replace_file_paths_with_inbox_dir_paths( + delivery_files=delivery_files, inbox_dir_path=inbox_dir_path ) @staticmethod def _create_ticket_inbox_folder( - inbox_path: Path, + inbox_dir_path: Path, ) -> Path: """Create a ticket inbox folder in the customer folder.""" - if not inbox_path.exists(): - inbox_path.mkdir(parents=True) - return inbox_path - raise FileExistsError(f"Ticket inbox folder {inbox_path} already exists for ticket.") + if not inbox_dir_path.exists(): + inbox_dir_path.mkdir(parents=True) + return inbox_dir_path + raise FileExistsError(f"Ticket inbox folder {inbox_dir_path} already exists for ticket.") @staticmethod - def _create_ticket_inbox_path( + def _create_ticket_inbox_dir_path( delivery_base_path: Path, delivery_data: DeliveryMetaData ) -> Path: """Create the path to the ticket inbox folder.""" @@ -50,56 +50,56 @@ def _create_ticket_inbox_path( ) @staticmethod - def _create_inbox_file_path(file_path: Path, inbox_path: Path) -> Path: + def _create_inbox_file_path(file_path: Path, inbox_dir_path: Path) -> Path: """Create the path to the inbox file.""" - return Path(inbox_path, file_path.name) + return Path(inbox_dir_path, file_path.name) def _create_hard_link_file_paths( - self, file_models: list[SampleFile | CaseFile], inbox_path: Path + self, file_models: list[SampleFile | CaseFile], inbox_dir_path: Path ) -> None: """Create hard links to the sample files in the customer folder.""" for file_model in file_models: inbox_file_path: Path = self._create_inbox_file_path( - file_path=file_model.file_path, inbox_path=inbox_path + file_path=file_model.file_path, inbox_dir_path=inbox_dir_path ) link_or_overwrite_file(src=file_model.file_path, dst=inbox_file_path) def _create_hard_links_for_delivery_files( - self, delivery_files: DeliveryFiles, inbox_path: Path + self, delivery_files: DeliveryFiles, inbox_dir_path: Path ) -> None: """Create hard links to the files in the customer folder.""" if delivery_files.case_files: self._create_hard_link_file_paths( - file_models=delivery_files.case_files, inbox_path=inbox_path + file_models=delivery_files.case_files, inbox_dir_path=inbox_dir_path ) self._create_hard_link_file_paths( - file_models=delivery_files.sample_files, inbox_path=inbox_path + file_models=delivery_files.sample_files, inbox_dir_path=inbox_dir_path ) - def _replace_file_path_with_inbox_path( - self, file_models: list[SampleFile | CaseFile], inbox_path: Path + def _replace_file_path_with_inbox_dir_path( + self, file_models: list[SampleFile | CaseFile], inbox_dir_path: Path ) -> list[SampleFile | CaseFile]: """Replace the file path with the inbox path.""" for file_model in file_models: inbox_file_path: Path = self._create_inbox_file_path( - file_path=file_model.file_path, inbox_path=inbox_path + file_path=file_model.file_path, inbox_dir_path=inbox_dir_path ) file_model.file_path = inbox_file_path return file_models - def _replace_file_paths_with_inbox_paths( + def _replace_file_paths_with_inbox_dir_paths( self, delivery_files: DeliveryFiles, - inbox_path: Path, + inbox_dir_path: Path, ) -> DeliveryFiles: """ Replace to original file paths in the delivery files with the customer inbox file paths. """ if delivery_files.case_files: - delivery_files.case_files = self._replace_file_path_with_inbox_path( - file_models=delivery_files.case_files, inbox_path=inbox_path + delivery_files.case_files = self._replace_file_path_with_inbox_dir_path( + file_models=delivery_files.case_files, inbox_dir_path=inbox_dir_path ) - delivery_files.sample_files = self._replace_file_path_with_inbox_path( - file_models=delivery_files.sample_files, inbox_path=inbox_path + delivery_files.sample_files = self._replace_file_path_with_inbox_dir_path( + file_models=delivery_files.sample_files, inbox_dir_path=inbox_dir_path ) return delivery_files diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py index 9e0ad5610c..b093946c53 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py @@ -43,7 +43,9 @@ def expected_fastq_delivery_files( delivery_meta_data = DeliveryMetaData( customer_internal_id=case.customer.internal_id, ticket_id=case.latest_ticket ) - return DeliveryFiles(delivery_data=delivery_data, case_files=None, sample_files=sample_files) + return DeliveryFiles( + delivery_data=delivery_meta_data, case_files=None, sample_files=sample_files + ) @pytest.fixture @@ -83,13 +85,13 @@ def expected_analysis_delivery_files( customer_internal_id=case.customer.internal_id, ticket_id=case.latest_ticket ) return DeliveryFiles( - delivery_data=delivery_data, case_files=case_files, sample_files=sample_files + delivery_data=delivery_meta_data, case_files=case_files, sample_files=sample_files ) @pytest.fixture def expected_moved_fastq_delivery_files( - expected_fastq_delivery_files: DeliveryFiles, tmp_path + expected_fastq_delivery_files: DeliveryFiles, tmp_path: Path ) -> DeliveryFiles: """Return the moved FASTQ delivery files.""" delivery_files = DeliveryFiles(**expected_fastq_delivery_files.model_dump()) @@ -100,7 +102,7 @@ def expected_moved_fastq_delivery_files( delivery_files.delivery_data.ticket_id, ) new_sample_files: list[SampleFile] = swap_file_paths_with_inbox_paths( - delivery_files.sample_files, inbox_path + file_models=delivery_files.sample_files, inbox_dir_path=inbox_dir_path ) return DeliveryFiles( delivery_data=expected_fastq_delivery_files.delivery_data, @@ -122,10 +124,10 @@ def expected_moved_analysis_delivery_files( delivery_files.delivery_data.ticket_id, ) new_case_files: list[CaseFile] = swap_file_paths_with_inbox_paths( - delivery_files.case_files, inbox_path + file_models=delivery_files.case_files, inbox_dir_path=inbox_dir_path ) new_sample_files: list[SampleFile] = swap_file_paths_with_inbox_paths( - delivery_files.sample_files, inbox_path + file_models=delivery_files.sample_files, inbox_dir_path=inbox_dir_path ) return DeliveryFiles( delivery_data=delivery_files.delivery_data, @@ -141,6 +143,6 @@ def swap_file_paths_with_inbox_paths( new_file_models: list[SampleFile | CaseFile] = [] for file_model in file_models: new_file_model: SampleFile = file_model - new_file_model.file_path = Path(inbox_path, file_model.file_path.name) + new_file_model.file_path = Path(inbox_dir_path, file_model.file_path.name) new_file_models.append(new_file_model) return new_file_models diff --git a/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py index f01ece3415..5c31f8e959 100644 --- a/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py +++ b/tests/services/file_delivery/move_delivery_files_service/test_move_delivery_file_service.py @@ -26,9 +26,10 @@ def test_move_files( expected_moved_delivery_files ) delivery_files: DeliveryFiles = request.getfixturevalue(delivery_files) + # WHEN moving the delivery files - file_mover = MoveDeliveryFilesService() - moved_delivery_files: DeliveryFiles = file_mover.move_files( + move_files_service = MoveDeliveryFilesService() + moved_delivery_files: DeliveryFiles = move_files_service.move_files( delivery_files=delivery_files, delivery_base_path=tmp_path ) From 652938e662efc2af9a19f74c923a5abff911ad00 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Mon, 26 Aug 2024 11:22:28 +0200 Subject: [PATCH 13/28] refactor logic --- .../case_file_formatter.py | 36 ++++++ ...nation_delivery_file_formatter_service.py} | 17 ++- .../delivery_file_formatting_service.py | 3 +- ...generic_delivery_file_formatter_service.py | 116 ++++-------------- .../mutant_delivery_file_formatter_service.py | 14 --- .../sample_concatenation_file_formatter.py | 8 ++ .../sample_file_formatter.py | 41 +++++++ .../test_reformat_delivery_files_service.py | 32 +++++ 8 files changed, 160 insertions(+), 107 deletions(-) create mode 100644 cg/services/file_delivery/file_formatter_service/case_file_formatter.py rename cg/services/file_delivery/file_formatter_service/{microsalt_delivery_file_formatter_service.py => concatenation_delivery_file_formatter_service.py} (52%) delete mode 100644 cg/services/file_delivery/file_formatter_service/mutant_delivery_file_formatter_service.py create mode 100644 cg/services/file_delivery/file_formatter_service/sample_concatenation_file_formatter.py create mode 100644 cg/services/file_delivery/file_formatter_service/sample_file_formatter.py create mode 100644 tests/services/file_delivery/format_deliver_files_service/test_reformat_delivery_files_service.py diff --git a/cg/services/file_delivery/file_formatter_service/case_file_formatter.py b/cg/services/file_delivery/file_formatter_service/case_file_formatter.py new file mode 100644 index 0000000000..53bb62b1e9 --- /dev/null +++ b/cg/services/file_delivery/file_formatter_service/case_file_formatter.py @@ -0,0 +1,36 @@ +from pathlib import Path + +from cg.services.file_delivery.fetch_file_service.models import CaseFile +from cg.services.file_delivery.file_formatter_service.models import FormattedFiles, FormattedFile + + +class CaseFileFormatter: + + def format_case_files( + self, case_files: list[CaseFile], ticket_dir_path: Path + ) -> list[FormattedFile]: + """Format the case files to deliver.""" + self._create_case_name_folder( + ticket_path=ticket_dir_path, case_name=case_files[0].case_name + ) + return self._rename_case_files(case_files=case_files) + + @staticmethod + def _rename_case_files(case_files: list[CaseFile]) -> list[FormattedFile]: + formatted_files: list[FormattedFile] = [] + for case_file in case_files: + replaced_case_file_name: str = case_file.file_path.name.replace( + case_file.case_id, case_file.case_name + ) + formatted_file_path = Path( + case_file.file_path.parent, case_file.case_name, replaced_case_file_name + ) + formatted_files.append( + FormattedFile(original_path=case_file.file_path, formatted_path=formatted_file_path) + ) + return formatted_files + + @staticmethod + def _create_case_name_folder(ticket_path: Path, case_name: str) -> None: + case_dir_path = Path(ticket_path, case_name) + case_dir_path.mkdir(exist_ok=True) diff --git a/cg/services/file_delivery/file_formatter_service/microsalt_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py similarity index 52% rename from cg/services/file_delivery/file_formatter_service/microsalt_delivery_file_formatter_service.py rename to cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py index 6af3e55567..78838af237 100644 --- a/cg/services/file_delivery/file_formatter_service/microsalt_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py @@ -2,22 +2,33 @@ FastqConcatenationService, ) from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from cg.services.file_delivery.file_formatter_service.case_file_formatter import CaseFileFormatter from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( DeliveryFileFormattingService, ) +from cg.services.file_delivery.file_formatter_service.sample_concatenation_file_formatter import ( + SampleConcatenationFileFormatter, +) -class MicrosaltDeliveryFileFormatter(DeliveryFileFormattingService): +class ConcatenationDeliveryFileFormatter(DeliveryFileFormattingService): """ Reformat the files to be delivered in the microsalt format. Expected structure: /inbox/// - /inbox/// (i.e. fastq files) + /inbox/// Extra rule: fastq files are concatenated into a single file per read direction. """ - def __init__(self, concatenation_service: FastqConcatenationService): + def __init__( + self, + case_file_formatter: CaseFileFormatter, + sample_file_formatter: SampleConcatenationFileFormatter, + concatenation_service: FastqConcatenationService, + ): + self.case_file_formatter = case_file_formatter + self.sample_file_formatter = sample_file_formatter self.concatenation_service = concatenation_service def format_files(self, delivery_files: DeliveryFiles) -> None: diff --git a/cg/services/file_delivery/file_formatter_service/delivery_file_formatting_service.py b/cg/services/file_delivery/file_formatter_service/delivery_file_formatting_service.py index 2a772e167e..b74d513c0c 100644 --- a/cg/services/file_delivery/file_formatter_service/delivery_file_formatting_service.py +++ b/cg/services/file_delivery/file_formatter_service/delivery_file_formatting_service.py @@ -1,6 +1,7 @@ from abc import abstractmethod, ABC from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from cg.services.file_delivery.file_formatter_service.models import FormattedFiles from cg.store.store import Store @@ -10,6 +11,6 @@ class DeliveryFileFormattingService(ABC): """ @abstractmethod - def format_files(self, delivery_files: DeliveryFiles) -> None: + def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: """Format the files to deliver.""" pass diff --git a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py index 9038667472..d9e551c7b3 100644 --- a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py @@ -1,13 +1,15 @@ import os from pathlib import Path - -from cg.constants import FileExtensions from cg.constants.delivery import INBOX_NAME -from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles, CaseFile, SampleFile +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from cg.services.file_delivery.file_formatter_service.case_file_formatter import CaseFileFormatter from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( DeliveryFileFormattingService, ) from cg.services.file_delivery.file_formatter_service.models import FormattedFiles, FormattedFile +from cg.services.file_delivery.file_formatter_service.sample_file_formatter import ( + SampleFileFormatter, +) class GenericDeliveryFileFormatter(DeliveryFileFormattingService): @@ -15,28 +17,21 @@ class GenericDeliveryFileFormatter(DeliveryFileFormattingService): Format the files to be delivered in the generic format. Expected structure: /inbox/// - /inbox//// - /inbox/// (i.e. fastq files) + /inbox/// """ - def format_files(self, delivery_files: DeliveryFiles) -> None: + def __init__( + self, case_file_formatter: CaseFileFormatter, sample_file_formatter: SampleFileFormatter + ): + self.case_file_formatter = case_file_formatter + self.sample_file_formatter = sample_file_formatter + + def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: """Format the files to be delivered in the generic format.""" - case_name: str = delivery_files.case_files[0].case_name - samples_names: list[set[str]] = self._get_sample_names(delivery_files.sample_files) - self._create_folder_structure( - case_name=case_name, sample_names=samples_names, delivery_files=delivery_files - ) - formatted_files: FormattedFiles = self._get_formatted_files(delivery_files) + formatted_files: FormattedFiles = self._get_files_to_format(delivery_files) for formatted_file in formatted_files.files: os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) - - @staticmethod - def _get_sample_names(sample_files: list[SampleFile]) -> list[set[str]]: - return [set(sample_file.sample_name for sample_file in sample_files)] - - @staticmethod - def _is_fastq_file(file_path: Path) -> bool: - return FileExtensions.FASTQ in str(file_path) + return formatted_files @staticmethod def _is_inbox_path(file_path: Path): @@ -46,74 +41,17 @@ def _get_ticket_dir_path(self, file_path) -> Path: if self._is_inbox_path(file_path): return file_path.parent - @staticmethod - def _create_case_name_folder(ticket_path: Path, case_name: str) -> Path: - case_dir_path = Path(ticket_path, case_name) - case_dir_path.mkdir(exist_ok=True) - return case_dir_path - - @staticmethod - def _create_sample_folders(parent_path: Path, sample_names: list[str]): - for sample_name in sample_names: - sample_dir_path = Path(parent_path, sample_name) - sample_dir_path.mkdir(exist_ok=True) - - def _create_folder_structure( - self, case_name: str, sample_names: list[str], delivery_files: DeliveryFiles - ): - ticket_dir_path: Path = self._get_ticket_dir_path(delivery_files.sample_files[0].file_path) - case_dir_path: Path = self._create_case_name_folder( - ticket_path=ticket_dir_path, case_name=case_name - ) - self._create_sample_folders(parent_path=ticket_dir_path, sample_names=sample_names) - self._create_sample_folders(parent_path=case_dir_path, sample_names=sample_names) - - def _get_formatted_files(self, deliver_files: DeliveryFiles) -> FormattedFiles: - formatted_case_files: list[FormattedFile] = self._format_case_files( - deliver_files.case_files - ) - formatted_sample_files: list[FormattedFile] = self._format_sample_files( - sample_files=deliver_files.sample_files, case_name=deliver_files.case_files[0].case_name - ) - return FormattedFiles(files=formatted_case_files + formatted_sample_files) - - @staticmethod - def _format_case_files(case_files: list[CaseFile]) -> list[FormattedFile]: + def _get_files_to_format(self, delivery_files: DeliveryFiles): formatted_files: list[FormattedFile] = [] - for case_file in case_files: - replaced_case_file_name: str = case_file.file_path.name.replace( - case_file.case_id, case_file.case_name - ) - formatted_file_path = Path( - case_file.file_path.parent, case_file.case_name, replaced_case_file_name - ) - formatted_files.append( - FormattedFile(original_path=case_file.file_path, formatted_path=formatted_file_path) + if delivery_files.case_files: + formatter_case_file = self.case_file_formatter.format_case_files( + case_files=delivery_files.case_files, + ticket_dir_path=self._get_ticket_dir_path(delivery_files.case_files[0].file_path), ) - return formatted_files - - def _format_sample_files( - self, sample_files: list[SampleFile], case_name: str - ) -> list[FormattedFile]: - formatted_files: list[FormattedFile] = [] - for sample_file in sample_files: - replaced_sample_file_name: str = sample_file.file_path.name.replace( - sample_file.sample_id, sample_file.sample_name - ) - if self._is_fastq_file(sample_file.file_path): - formatted_file_path = Path( - sample_file.file_path.parent, sample_file.sample_name, replaced_sample_file_name - ) - else: - formatted_file_path = Path( - sample_file.file_path.parent, - case_name, - sample_file.sample_name, - replaced_sample_file_name, - ) - formatted_files.append( - FormattedFile( - original_path=sample_file.file_path, formatted_path=formatted_file_path - ) - ) - return formatted_files + formatted_files.extend(formatter_case_file) + formatter_sample_files = self.sample_file_formatter.format_sample_files( + sample_files=delivery_files.sample_files, + ticket_dir_path=self._get_ticket_dir_path(delivery_files.sample_files[0].file_path), + ) + formatted_files.extend(formatter_sample_files) + return FormattedFiles(files=formatted_files) diff --git a/cg/services/file_delivery/file_formatter_service/mutant_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/mutant_delivery_file_formatter_service.py deleted file mode 100644 index e70fe9f282..0000000000 --- a/cg/services/file_delivery/file_formatter_service/mutant_delivery_file_formatter_service.py +++ /dev/null @@ -1,14 +0,0 @@ -from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles -from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( - DeliveryFileFormattingService, -) - - -class MutantDeliveryFileFormatter(DeliveryFileFormattingService): - """ - Reformat the files to be delivered in the mutant format. - - """ - - def format_files(self, delivery_files: DeliveryFiles) -> None: - """Format the files to be delivered in the generic format.""" diff --git a/cg/services/file_delivery/file_formatter_service/sample_concatenation_file_formatter.py b/cg/services/file_delivery/file_formatter_service/sample_concatenation_file_formatter.py new file mode 100644 index 0000000000..c8af0cbef5 --- /dev/null +++ b/cg/services/file_delivery/file_formatter_service/sample_concatenation_file_formatter.py @@ -0,0 +1,8 @@ +from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( + FastqConcatenationService, +) + + +class SampleConcatenationFileFormatter: + def __init__(self, concatenation_service: FastqConcatenationService): + self.concatenation_service = concatenation_service diff --git a/cg/services/file_delivery/file_formatter_service/sample_file_formatter.py b/cg/services/file_delivery/file_formatter_service/sample_file_formatter.py new file mode 100644 index 0000000000..64cc5b28fb --- /dev/null +++ b/cg/services/file_delivery/file_formatter_service/sample_file_formatter.py @@ -0,0 +1,41 @@ +from pathlib import Path + +from cg.services.file_delivery.fetch_file_service.models import SampleFile +from cg.services.file_delivery.file_formatter_service.models import FormattedFile + + +class SampleFileFormatter: + def format_sample_files( + self, sample_files: list[SampleFile], ticket_dir_path: Path + ) -> list[FormattedFile]: + """Format the sample files to deliver.""" + sample_names: list[str] = self._get_sample_names(sample_files=sample_files) + self._create_sample_folders(ticket_dir_path=ticket_dir_path, sample_names=sample_names) + return self._format_sample_files(sample_files=sample_files) + + @staticmethod + def _get_sample_names(sample_files: list[SampleFile]) -> list[set[str]]: + return [set(sample_file.sample_name for sample_file in sample_files)] + + @staticmethod + def _create_sample_folders(ticket_dir_path: Path, sample_names: list[str]): + for sample_name in sample_names: + sample_dir_path = Path(ticket_dir_path, sample_name) + sample_dir_path.mkdir(exist_ok=True) + + @staticmethod + def _format_sample_files(sample_files: list[SampleFile]) -> list[FormattedFile]: + formatted_files: list[FormattedFile] = [] + for sample_file in sample_files: + replaced_sample_file_name: str = sample_file.file_path.name.replace( + sample_file.sample_id, sample_file.sample_name + ) + formatted_file_path = Path( + sample_file.file_path.parent, sample_file.sample_name, replaced_sample_file_name + ) + formatted_files.append( + FormattedFile( + original_path=sample_file.file_path, formatted_path=formatted_file_path + ) + ) + return formatted_files diff --git a/tests/services/file_delivery/format_deliver_files_service/test_reformat_delivery_files_service.py b/tests/services/file_delivery/format_deliver_files_service/test_reformat_delivery_files_service.py new file mode 100644 index 0000000000..37c588f7ad --- /dev/null +++ b/tests/services/file_delivery/format_deliver_files_service/test_reformat_delivery_files_service.py @@ -0,0 +1,32 @@ +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( + DeliveryFileFormattingService, +) +from cg.services.file_delivery.file_formatter_service.generic_delivery_file_formatter_service import ( + GenericDeliveryFileFormatter, +) +import pytest + +from cg.services.file_delivery.file_formatter_service.models import FormattedFiles + + +@pytest.mark.parametrize( + "delivery_files,expected_formatted_files,formatter_service", + [ + "moved_fastq_delivery_files", + "expected_formatted_fastq_files", + GenericDeliveryFileFormatter(), + ], +) +def test_reformat_files( + delivery_files: DeliveryFiles, + expected_formatted_files: FormattedFiles, + formatter_service: DeliveryFileFormattingService, +): + # GIVEN a delivery file formatter and moved delivery files + + # WHEN reformatting the delivery files + formatted_files: FormattedFiles = formatter_service.format_files(delivery_files) + # THEN the delivery files should be reformatted + + assert formatted_files == expected_formatted_files From 1aad85b0240c113b1abc9fea55f18b1c0692be23 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Mon, 26 Aug 2024 11:23:31 +0200 Subject: [PATCH 14/28] fix formatter --- .../concatenation_delivery_file_formatter_service.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py index 78838af237..c9c7f7592c 100644 --- a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py @@ -25,11 +25,9 @@ def __init__( self, case_file_formatter: CaseFileFormatter, sample_file_formatter: SampleConcatenationFileFormatter, - concatenation_service: FastqConcatenationService, ): self.case_file_formatter = case_file_formatter self.sample_file_formatter = sample_file_formatter - self.concatenation_service = concatenation_service def format_files(self, delivery_files: DeliveryFiles) -> None: """Format the files to be delivered in the generic format.""" From 6cabd68abbabee97e3c25053e95ef30fff7fa5e1 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Mon, 26 Aug 2024 14:21:45 +0200 Subject: [PATCH 15/28] fix concatenation --- .../fastq_concatenation_service/utils.py | 8 +- ...enation_delivery_file_formatter_service.py | 35 +++++-- ...generic_delivery_file_formatter_service.py | 29 ++---- .../sample_concatenation_file_formatter.py | 8 -- .../file_formatter_service/utils/__init__.py | 0 .../{ => utils}/case_file_formatter.py | 6 +- .../sample_file_concatenation_formatter.py | 97 +++++++++++++++++++ .../{ => utils}/sample_file_formatter.py | 17 +++- .../file_formatter_service/utils/utils.py | 12 +++ 9 files changed, 171 insertions(+), 41 deletions(-) delete mode 100644 cg/services/file_delivery/file_formatter_service/sample_concatenation_file_formatter.py create mode 100644 cg/services/file_delivery/file_formatter_service/utils/__init__.py rename cg/services/file_delivery/file_formatter_service/{ => utils}/case_file_formatter.py (84%) create mode 100644 cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py rename cg/services/file_delivery/file_formatter_service/{ => utils}/sample_file_formatter.py (72%) create mode 100644 cg/services/file_delivery/file_formatter_service/utils/utils.py diff --git a/cg/services/fastq_concatenation_service/utils.py b/cg/services/fastq_concatenation_service/utils.py index 01de27a199..49582a5716 100644 --- a/cg/services/fastq_concatenation_service/utils.py +++ b/cg/services/fastq_concatenation_service/utils.py @@ -4,7 +4,7 @@ import uuid from cg.services.fastq_concatenation_service.exceptions import ConcatenationError -from cg.constants.constants import ReadDirection +from cg.constants.constants import ReadDirection, FileFormat from cg.constants import FileExtensions @@ -73,7 +73,11 @@ def sort_files_by_name(files: list[Path]) -> list[Path]: def file_can_be_removed(file: Path, forward_file: Path, reverse_file: Path) -> bool: - return file.suffix == FileExtensions.GZIP and file != forward_file and file != reverse_file + return ( + f"{FileFormat.FASTQ}{FileExtensions.GZIP}" in file.name + and file != forward_file + and file != reverse_file + ) def remove_raw_fastqs(fastq_directory: Path, forward_file: Path, reverse_file: Path) -> None: diff --git a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py index c9c7f7592c..76f79c814b 100644 --- a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py @@ -1,14 +1,24 @@ +from pathlib import Path + +from cg.constants.delivery import INBOX_NAME from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( FastqConcatenationService, ) from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles -from cg.services.file_delivery.file_formatter_service.case_file_formatter import CaseFileFormatter +from cg.services.file_delivery.file_formatter_service.models import FormattedFiles, FormattedFile +from cg.services.file_delivery.file_formatter_service.utils.case_file_formatter import ( + CaseFileFormatter, +) from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( DeliveryFileFormattingService, ) -from cg.services.file_delivery.file_formatter_service.sample_concatenation_file_formatter import ( - SampleConcatenationFileFormatter, +from cg.services.file_delivery.file_formatter_service.utils.sample_file_concatenation_formatter import ( + SampleFileConcatenationFormatter, +) +from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( + SampleFileFormatter, ) +from cg.services.file_delivery.file_formatter_service.utils.utils import get_ticket_dir_path class ConcatenationDeliveryFileFormatter(DeliveryFileFormattingService): @@ -24,10 +34,23 @@ class ConcatenationDeliveryFileFormatter(DeliveryFileFormattingService): def __init__( self, case_file_formatter: CaseFileFormatter, - sample_file_formatter: SampleConcatenationFileFormatter, + sample_file_formatter: SampleFileConcatenationFormatter, ): self.case_file_formatter = case_file_formatter self.sample_file_formatter = sample_file_formatter - def format_files(self, delivery_files: DeliveryFiles) -> None: - """Format the files to be delivered in the generic format.""" + def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: + """Format the files to be delivered in the concatenated format.""" + formatted_files: list[FormattedFile] = [] + if delivery_files.case_files: + formatter_case_file = self.case_file_formatter.format_files( + case_files=delivery_files.case_files, + ticket_dir_path=get_ticket_dir_path(delivery_files.case_files[0].file_path), + ) + formatted_files.extend(formatter_case_file) + formatter_sample_files = self.sample_file_formatter.format_files( + sample_files=delivery_files.sample_files, + ticket_dir_path=get_ticket_dir_path(delivery_files.sample_files[0].file_path), + ) + formatted_files.extend(formatter_sample_files) + return FormattedFiles(files=formatted_files) diff --git a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py index d9e551c7b3..5d27c85eff 100644 --- a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py @@ -2,14 +2,17 @@ from pathlib import Path from cg.constants.delivery import INBOX_NAME from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles -from cg.services.file_delivery.file_formatter_service.case_file_formatter import CaseFileFormatter +from cg.services.file_delivery.file_formatter_service.utils.case_file_formatter import ( + CaseFileFormatter, +) from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( DeliveryFileFormattingService, ) from cg.services.file_delivery.file_formatter_service.models import FormattedFiles, FormattedFile -from cg.services.file_delivery.file_formatter_service.sample_file_formatter import ( +from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( SampleFileFormatter, ) +from cg.services.file_delivery.file_formatter_service.utils.utils import get_ticket_dir_path class GenericDeliveryFileFormatter(DeliveryFileFormattingService): @@ -28,30 +31,16 @@ def __init__( def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: """Format the files to be delivered in the generic format.""" - formatted_files: FormattedFiles = self._get_files_to_format(delivery_files) - for formatted_file in formatted_files.files: - os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) - return formatted_files - - @staticmethod - def _is_inbox_path(file_path: Path): - return INBOX_NAME in str(file_path) - - def _get_ticket_dir_path(self, file_path) -> Path: - if self._is_inbox_path(file_path): - return file_path.parent - - def _get_files_to_format(self, delivery_files: DeliveryFiles): formatted_files: list[FormattedFile] = [] if delivery_files.case_files: - formatter_case_file = self.case_file_formatter.format_case_files( + formatter_case_file = self.case_file_formatter.format_files( case_files=delivery_files.case_files, - ticket_dir_path=self._get_ticket_dir_path(delivery_files.case_files[0].file_path), + ticket_dir_path=get_ticket_dir_path(delivery_files.case_files[0].file_path), ) formatted_files.extend(formatter_case_file) - formatter_sample_files = self.sample_file_formatter.format_sample_files( + formatter_sample_files = self.sample_file_formatter.format_files( sample_files=delivery_files.sample_files, - ticket_dir_path=self._get_ticket_dir_path(delivery_files.sample_files[0].file_path), + ticket_dir_path=get_ticket_dir_path(delivery_files.sample_files[0].file_path), ) formatted_files.extend(formatter_sample_files) return FormattedFiles(files=formatted_files) diff --git a/cg/services/file_delivery/file_formatter_service/sample_concatenation_file_formatter.py b/cg/services/file_delivery/file_formatter_service/sample_concatenation_file_formatter.py deleted file mode 100644 index c8af0cbef5..0000000000 --- a/cg/services/file_delivery/file_formatter_service/sample_concatenation_file_formatter.py +++ /dev/null @@ -1,8 +0,0 @@ -from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( - FastqConcatenationService, -) - - -class SampleConcatenationFileFormatter: - def __init__(self, concatenation_service: FastqConcatenationService): - self.concatenation_service = concatenation_service diff --git a/cg/services/file_delivery/file_formatter_service/utils/__init__.py b/cg/services/file_delivery/file_formatter_service/utils/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cg/services/file_delivery/file_formatter_service/case_file_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py similarity index 84% rename from cg/services/file_delivery/file_formatter_service/case_file_formatter.py rename to cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py index 53bb62b1e9..8843fa92d7 100644 --- a/cg/services/file_delivery/file_formatter_service/case_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py @@ -1,3 +1,4 @@ +import os from pathlib import Path from cg.services.file_delivery.fetch_file_service.models import CaseFile @@ -6,13 +7,16 @@ class CaseFileFormatter: - def format_case_files( + def format_files( self, case_files: list[CaseFile], ticket_dir_path: Path ) -> list[FormattedFile]: """Format the case files to deliver.""" self._create_case_name_folder( ticket_path=ticket_dir_path, case_name=case_files[0].case_name ) + formatted_files: list[FormattedFile] = self._rename_case_files(case_files=case_files) + for formatted_file in formatted_files: + os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) return self._rename_case_files(case_files=case_files) @staticmethod diff --git a/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py new file mode 100644 index 0000000000..38b889421b --- /dev/null +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py @@ -0,0 +1,97 @@ +from pathlib import Path + +from cg.constants.constants import ReadDirection, FileFormat, FileExtensions +from cg.meta.deliver.fastq_path_generator import generate_concatenated_fastq_delivery_path +from cg.services.file_delivery.fetch_file_service.models import SampleFile +from cg.services.file_delivery.file_formatter_service.models import FormattedFile +from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( + SampleFileFormatter, +) + + +class SampleFileConcatenationFormatter(SampleFileFormatter): + + def format_files( + self, sample_files: list[SampleFile], ticket_dir_path: Path + ) -> list[FormattedFile]: + formatted_files: list[FormattedFile] = super().format_files( + sample_files=sample_files, ticket_dir_path=ticket_dir_path + ) + forward_paths, reverse_path = self._concatenate_fastq_files(formatted_files=formatted_files) + return self._replace_fastq_paths( + reverse_paths=reverse_path, + forward_paths=forward_paths, + formatted_files=formatted_files, + ) + + def _concatenate_fastq_files( + self, formatted_files: list[FormattedFile] + ) -> tuple[list[Path], list[Path]]: + unique_sample_dir_paths: set[Path] = self._get_unique_sample_paths( + sample_files=formatted_files + ) + forward_paths: list[Path] = [] + reverse_paths: list[Path] = [] + for sample_dir_path in unique_sample_dir_paths: + sample_name: str = sample_dir_path.name + forward_path: Path = generate_concatenated_fastq_delivery_path( + fastq_directory=sample_dir_path, + sample_name=sample_name, + direction=ReadDirection.FORWARD, + ) + forward_paths.append(forward_path) + reverse_path: Path = generate_concatenated_fastq_delivery_path( + fastq_directory=sample_dir_path, + sample_name=sample_name, + direction=ReadDirection.REVERSE, + ) + reverse_paths.append(reverse_path) + self.concatenation_service.concatenate( + fastq_directory=sample_dir_path, + forward_output_path=forward_path, + reverse_output_path=reverse_path, + remove_raw=True, + ) + return forward_paths, reverse_paths + + @staticmethod + def _get_unique_sample_paths(sample_files: list[FormattedFile]) -> set[Path]: + sample_paths: list[Path] = [] + for sample_file in sample_files: + sample_paths.append(sample_file.formatted_path) + return set(sample_paths) + + @staticmethod + def _replace_fastq_formatted_file_path( + formatted_files: list[FormattedFile], + direction: ReadDirection, + new_path: Path, + ): + for formatted_file in formatted_files: + if ( + formatted_file.formatted_path.parent == new_path.parent + and f"{FileFormat.FASTQ}{FileExtensions.GZIP}" in formatted_file.formatted_path.name + and f"R{direction}" in formatted_file.formatted_path.name + ): + formatted_file.formatted_path = new_path + return formatted_files + + def _replace_fastq_paths( + self, + forward_paths: list[Path], + reverse_paths: list[Path], + formatted_files: list[FormattedFile], + ) -> list[FormattedFile]: + for forward_path in forward_paths: + formatted_files = self._replace_fastq_formatted_file_path( + formatted_files=formatted_files, + direction=ReadDirection.FORWARD, + new_path=forward_path, + ) + for reverse_path in reverse_paths: + formatted_files = self._replace_fastq_formatted_file_path( + formatted_files=formatted_files, + direction=ReadDirection.REVERSE, + new_path=reverse_path, + ) + return formatted_files diff --git a/cg/services/file_delivery/file_formatter_service/sample_file_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py similarity index 72% rename from cg/services/file_delivery/file_formatter_service/sample_file_formatter.py rename to cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py index 64cc5b28fb..cba76588ab 100644 --- a/cg/services/file_delivery/file_formatter_service/sample_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py @@ -1,17 +1,26 @@ +import os from pathlib import Path - +from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( + FastqConcatenationService, +) from cg.services.file_delivery.fetch_file_service.models import SampleFile from cg.services.file_delivery.file_formatter_service.models import FormattedFile class SampleFileFormatter: - def format_sample_files( + def __init__(self, concatenation_service: FastqConcatenationService): + self.concatenation_service = concatenation_service + + def format_files( self, sample_files: list[SampleFile], ticket_dir_path: Path ) -> list[FormattedFile]: """Format the sample files to deliver.""" sample_names: list[str] = self._get_sample_names(sample_files=sample_files) self._create_sample_folders(ticket_dir_path=ticket_dir_path, sample_names=sample_names) - return self._format_sample_files(sample_files=sample_files) + formatted_files: list[FormattedFile] = self._rename_sample_files(sample_files=sample_files) + for formatted_file in formatted_files: + os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) + return formatted_files @staticmethod def _get_sample_names(sample_files: list[SampleFile]) -> list[set[str]]: @@ -24,7 +33,7 @@ def _create_sample_folders(ticket_dir_path: Path, sample_names: list[str]): sample_dir_path.mkdir(exist_ok=True) @staticmethod - def _format_sample_files(sample_files: list[SampleFile]) -> list[FormattedFile]: + def _rename_sample_files(sample_files: list[SampleFile]) -> list[FormattedFile]: formatted_files: list[FormattedFile] = [] for sample_file in sample_files: replaced_sample_file_name: str = sample_file.file_path.name.replace( diff --git a/cg/services/file_delivery/file_formatter_service/utils/utils.py b/cg/services/file_delivery/file_formatter_service/utils/utils.py new file mode 100644 index 0000000000..ba0ea50086 --- /dev/null +++ b/cg/services/file_delivery/file_formatter_service/utils/utils.py @@ -0,0 +1,12 @@ +from pathlib import Path + +from cg.constants.delivery import INBOX_NAME + + +def is_inbox_path(file_path: Path): + return INBOX_NAME in str(file_path) + + +def get_ticket_dir_path(file_path) -> Path: + if is_inbox_path(file_path): + return file_path.parent From 56d7ec16947a7249573f86b34b523f243acfa199 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Mon, 26 Aug 2024 16:37:55 +0200 Subject: [PATCH 16/28] tests --- ...enation_delivery_file_formatter_service.py | 19 ++-- ...generic_delivery_file_formatter_service.py | 17 ++-- .../utils/case_file_formatter.py | 7 +- .../sample_file_concatenation_formatter.py | 18 +++- .../utils/sample_file_formatter.py | 20 ++-- .../file_formatter_service/utils/utils.py | 4 + tests/conftest.py | 1 + .../delivery_formatted_files_fixtures.py | 98 +++++++++++++++++++ .../delivery_fixtures/path_fixtures.py | 5 + ... => test_format_delivery_files_service.py} | 0 .../utils/test_case_file_formatter.py | 34 +++++++ ...est_sample_file_concatenation_formatter.py | 40 ++++++++ .../utils/test_sample_file_formatter.py | 33 +++++++ 13 files changed, 261 insertions(+), 35 deletions(-) create mode 100644 tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py rename tests/services/file_delivery/format_deliver_files_service/{test_reformat_delivery_files_service.py => test_format_delivery_files_service.py} (100%) create mode 100644 tests/services/file_delivery/format_deliver_files_service/utils/test_case_file_formatter.py create mode 100644 tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_concatenation_formatter.py create mode 100644 tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_formatter.py diff --git a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py index 76f79c814b..39d68bd02a 100644 --- a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py @@ -18,7 +18,10 @@ from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( SampleFileFormatter, ) -from cg.services.file_delivery.file_formatter_service.utils.utils import get_ticket_dir_path +from cg.services.file_delivery.file_formatter_service.utils.utils import ( + get_ticket_dir_path, + create_ticket_dir, +) class ConcatenationDeliveryFileFormatter(DeliveryFileFormattingService): @@ -41,16 +44,18 @@ def __init__( def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: """Format the files to be delivered in the concatenated format.""" + ticket_dir_path: Path = get_ticket_dir_path(delivery_files.sample_files[0].file_path) + create_ticket_dir(ticket_dir_path) formatted_files: list[FormattedFile] = [] + formatter_sample_files = self.sample_file_formatter.format_files( + sample_files=delivery_files.sample_files, + ticket_dir_path=ticket_dir_path, + ) + formatted_files.extend(formatter_sample_files) if delivery_files.case_files: formatter_case_file = self.case_file_formatter.format_files( case_files=delivery_files.case_files, - ticket_dir_path=get_ticket_dir_path(delivery_files.case_files[0].file_path), + ticket_dir_path=ticket_dir_path, ) formatted_files.extend(formatter_case_file) - formatter_sample_files = self.sample_file_formatter.format_files( - sample_files=delivery_files.sample_files, - ticket_dir_path=get_ticket_dir_path(delivery_files.sample_files[0].file_path), - ) - formatted_files.extend(formatter_sample_files) return FormattedFiles(files=formatted_files) diff --git a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py index 5d27c85eff..41895b2dc7 100644 --- a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py @@ -12,7 +12,10 @@ from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( SampleFileFormatter, ) -from cg.services.file_delivery.file_formatter_service.utils.utils import get_ticket_dir_path +from cg.services.file_delivery.file_formatter_service.utils.utils import ( + get_ticket_dir_path, + create_ticket_dir, +) class GenericDeliveryFileFormatter(DeliveryFileFormattingService): @@ -31,16 +34,18 @@ def __init__( def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: """Format the files to be delivered in the generic format.""" + ticket_dir_path: Path = get_ticket_dir_path(delivery_files.sample_files[0].file_path) + create_ticket_dir(ticket_dir_path) formatted_files: list[FormattedFile] = [] + formatter_sample_files = self.sample_file_formatter.format_files( + sample_files=delivery_files.sample_files, + ticket_dir_path=get_ticket_dir_path(delivery_files.sample_files[0].file_path), + ) + formatted_files.extend(formatter_sample_files) if delivery_files.case_files: formatter_case_file = self.case_file_formatter.format_files( case_files=delivery_files.case_files, ticket_dir_path=get_ticket_dir_path(delivery_files.case_files[0].file_path), ) formatted_files.extend(formatter_case_file) - formatter_sample_files = self.sample_file_formatter.format_files( - sample_files=delivery_files.sample_files, - ticket_dir_path=get_ticket_dir_path(delivery_files.sample_files[0].file_path), - ) - formatted_files.extend(formatter_sample_files) return FormattedFiles(files=formatted_files) diff --git a/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py index 8843fa92d7..c9e947b48c 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py @@ -14,10 +14,7 @@ def format_files( self._create_case_name_folder( ticket_path=ticket_dir_path, case_name=case_files[0].case_name ) - formatted_files: list[FormattedFile] = self._rename_case_files(case_files=case_files) - for formatted_file in formatted_files: - os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) - return self._rename_case_files(case_files=case_files) + return self._rename_case_files(case_files) @staticmethod def _rename_case_files(case_files: list[CaseFile]) -> list[FormattedFile]: @@ -32,6 +29,8 @@ def _rename_case_files(case_files: list[CaseFile]) -> list[FormattedFile]: formatted_files.append( FormattedFile(original_path=case_file.file_path, formatted_path=formatted_file_path) ) + for formatted_file in formatted_files: + os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) return formatted_files @staticmethod diff --git a/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py index 38b889421b..b716f8dead 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py @@ -2,6 +2,9 @@ from cg.constants.constants import ReadDirection, FileFormat, FileExtensions from cg.meta.deliver.fastq_path_generator import generate_concatenated_fastq_delivery_path +from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( + FastqConcatenationService, +) from cg.services.file_delivery.fetch_file_service.models import SampleFile from cg.services.file_delivery.file_formatter_service.models import FormattedFile from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( @@ -10,6 +13,8 @@ class SampleFileConcatenationFormatter(SampleFileFormatter): + def __init__(self, concatenation_service: FastqConcatenationService): + self.concatenation_service = concatenation_service def format_files( self, sample_files: list[SampleFile], ticket_dir_path: Path @@ -18,6 +23,7 @@ def format_files( sample_files=sample_files, ticket_dir_path=ticket_dir_path ) forward_paths, reverse_path = self._concatenate_fastq_files(formatted_files=formatted_files) + return self._replace_fastq_paths( reverse_paths=reverse_path, forward_paths=forward_paths, @@ -33,21 +39,23 @@ def _concatenate_fastq_files( forward_paths: list[Path] = [] reverse_paths: list[Path] = [] for sample_dir_path in unique_sample_dir_paths: - sample_name: str = sample_dir_path.name + fastq_directory: Path = sample_dir_path + sample_name: str = fastq_directory.name + forward_path: Path = generate_concatenated_fastq_delivery_path( - fastq_directory=sample_dir_path, + fastq_directory=fastq_directory, sample_name=sample_name, direction=ReadDirection.FORWARD, ) forward_paths.append(forward_path) reverse_path: Path = generate_concatenated_fastq_delivery_path( - fastq_directory=sample_dir_path, + fastq_directory=fastq_directory, sample_name=sample_name, direction=ReadDirection.REVERSE, ) reverse_paths.append(reverse_path) self.concatenation_service.concatenate( - fastq_directory=sample_dir_path, + fastq_directory=fastq_directory, forward_output_path=forward_path, reverse_output_path=reverse_path, remove_raw=True, @@ -58,7 +66,7 @@ def _concatenate_fastq_files( def _get_unique_sample_paths(sample_files: list[FormattedFile]) -> set[Path]: sample_paths: list[Path] = [] for sample_file in sample_files: - sample_paths.append(sample_file.formatted_path) + sample_paths.append(sample_file.formatted_path.parent) return set(sample_paths) @staticmethod diff --git a/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py index cba76588ab..46b98a1f8b 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py @@ -1,33 +1,25 @@ import os from pathlib import Path -from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( - FastqConcatenationService, -) from cg.services.file_delivery.fetch_file_service.models import SampleFile from cg.services.file_delivery.file_formatter_service.models import FormattedFile class SampleFileFormatter: - def __init__(self, concatenation_service: FastqConcatenationService): - self.concatenation_service = concatenation_service def format_files( self, sample_files: list[SampleFile], ticket_dir_path: Path ) -> list[FormattedFile]: """Format the sample files to deliver.""" - sample_names: list[str] = self._get_sample_names(sample_files=sample_files) + sample_names: set[str] = self._get_sample_names(sample_files=sample_files) self._create_sample_folders(ticket_dir_path=ticket_dir_path, sample_names=sample_names) - formatted_files: list[FormattedFile] = self._rename_sample_files(sample_files=sample_files) - for formatted_file in formatted_files: - os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) - return formatted_files + return self._rename_sample_files(sample_files=sample_files) @staticmethod - def _get_sample_names(sample_files: list[SampleFile]) -> list[set[str]]: - return [set(sample_file.sample_name for sample_file in sample_files)] + def _get_sample_names(sample_files: list[SampleFile]) -> set[str]: + return set(sample_file.sample_name for sample_file in sample_files) @staticmethod - def _create_sample_folders(ticket_dir_path: Path, sample_names: list[str]): + def _create_sample_folders(ticket_dir_path: Path, sample_names: set[str]): for sample_name in sample_names: sample_dir_path = Path(ticket_dir_path, sample_name) sample_dir_path.mkdir(exist_ok=True) @@ -47,4 +39,6 @@ def _rename_sample_files(sample_files: list[SampleFile]) -> list[FormattedFile]: original_path=sample_file.file_path, formatted_path=formatted_file_path ) ) + for formatted_file in formatted_files: + os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) return formatted_files diff --git a/cg/services/file_delivery/file_formatter_service/utils/utils.py b/cg/services/file_delivery/file_formatter_service/utils/utils.py index ba0ea50086..28595c9a26 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/utils.py +++ b/cg/services/file_delivery/file_formatter_service/utils/utils.py @@ -10,3 +10,7 @@ def is_inbox_path(file_path: Path): def get_ticket_dir_path(file_path) -> Path: if is_inbox_path(file_path): return file_path.parent + + +def create_ticket_dir(ticket_dir_path: Path) -> None: + ticket_dir_path.mkdir(exist_ok=True) diff --git a/tests/conftest.py b/tests/conftest.py index fa9ce37d75..38b17354e7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -95,6 +95,7 @@ "tests.fixture_plugins.delivery_fixtures.path_fixtures", "tests.fixture_plugins.delivery_fixtures.delivery_files_models_fixtures", "tests.fixture_plugins.delivery_fixtures.delivery_services_fixtures", + "tests.fixture_plugins.delivery_fixtures.delivery_formatted_files_fixtures", "tests.fixture_plugins.demultiplex_fixtures.flow_cell_fixtures", "tests.fixture_plugins.demultiplex_fixtures.housekeeper_fixtures", "tests.fixture_plugins.demultiplex_fixtures.metrics_fixtures", diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py new file mode 100644 index 0000000000..0b712b53e3 --- /dev/null +++ b/tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py @@ -0,0 +1,98 @@ +from pathlib import Path + +import pytest + +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles, SampleFile +from cg.services.file_delivery.file_formatter_service.models import FormattedFile + + +@pytest.fixture +def expected_formatted_analysis_case_files( + expected_moved_analysis_delivery_files: DeliveryFiles, +) -> list[FormattedFile]: + formatted_files: list[FormattedFile] = [] + for case_file in expected_moved_analysis_delivery_files.case_files: + replaced_case_file_name: str = case_file.file_path.name.replace( + case_file.case_id, case_file.case_name + ) + formatted_file_path = Path( + case_file.file_path.parent, case_file.case_name, replaced_case_file_name + ) + formatted_files.append( + FormattedFile(original_path=case_file.file_path, formatted_path=formatted_file_path) + ) + return formatted_files + + +@pytest.fixture +def expected_formatted_analysis_sample_files( + expected_moved_analysis_delivery_files: DeliveryFiles, +) -> list[FormattedFile]: + formatted_files: list[FormattedFile] = [] + for sample_file in expected_moved_analysis_delivery_files.sample_files: + replaced_sample_file_name: str = sample_file.file_path.name.replace( + sample_file.sample_id, sample_file.sample_name + ) + formatted_file_path = Path( + sample_file.file_path.parent, sample_file.sample_name, replaced_sample_file_name + ) + formatted_files.append( + FormattedFile(original_path=sample_file.file_path, formatted_path=formatted_file_path) + ) + return formatted_files + + +@pytest.fixture +def expected_formatted_fastq_sample_files( + expected_moved_fastq_delivery_files: DeliveryFiles, +) -> list[FormattedFile]: + formatted_files: list[FormattedFile] = [] + for sample_file in expected_moved_fastq_delivery_files.sample_files: + replaced_sample_file_name: str = sample_file.file_path.name.replace( + sample_file.sample_id, sample_file.sample_name + ) + formatted_file_path = Path( + sample_file.file_path.parent, sample_file.sample_name, replaced_sample_file_name + ) + formatted_files.append( + FormattedFile(original_path=sample_file.file_path, formatted_path=formatted_file_path) + ) + return formatted_files + + +@pytest.fixture +def fastq_sample_files(tmp_path: Path) -> list[SampleFile]: + some_ticket: str = "some_ticket" + fastq_paths: list[Path] = [ + Path(tmp_path, some_ticket, "S1_1_R1.fastq.gz"), + Path(tmp_path, some_ticket, "S1_2_R1.fastq.gz"), + Path(tmp_path, some_ticket, "S1_1_R2.fastq.gz"), + Path(tmp_path, some_ticket, "S1_2_R2.fastq.gz"), + ] + return [ + SampleFile( + sample_id="S1", + case_id="Case1", + sample_name="Sample1", + file_path=fastq_path, + ) + for fastq_path in fastq_paths + ] + + +@pytest.fixture +def expected_concatenated_fastq_formatted_files( + fastq_sample_files: list[SampleFile], +) -> list[FormattedFile]: + formatted_files: list[FormattedFile] = [] + for sample_file in fastq_sample_files: + replaced_sample_file_name: str = sample_file.file_path.name.replace( + sample_file.sample_id, sample_file.sample_name + ) + formatted_file_path = Path( + sample_file.file_path.parent, sample_file.sample_name, replaced_sample_file_name + ) + formatted_files.append( + FormattedFile(original_path=sample_file.file_path, formatted_path=formatted_file_path) + ) + return formatted_files diff --git a/tests/fixture_plugins/delivery_fixtures/path_fixtures.py b/tests/fixture_plugins/delivery_fixtures/path_fixtures.py index f9300b42f8..ad0e0bff1b 100644 --- a/tests/fixture_plugins/delivery_fixtures/path_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/path_fixtures.py @@ -54,3 +54,8 @@ def delivery_another_cram_file(tmp_path: Path, another_sample_id: str) -> Path: file = Path(tmp_path, f"{another_sample_id}{FileExtensions.CRAM}") file.touch() return file + + +@pytest.fixture +def delivery_ticket_dir_path(tmp_path: Path, ticket_id: str) -> Path: + return Path(tmp_path, ticket_id) diff --git a/tests/services/file_delivery/format_deliver_files_service/test_reformat_delivery_files_service.py b/tests/services/file_delivery/format_deliver_files_service/test_format_delivery_files_service.py similarity index 100% rename from tests/services/file_delivery/format_deliver_files_service/test_reformat_delivery_files_service.py rename to tests/services/file_delivery/format_deliver_files_service/test_format_delivery_files_service.py diff --git a/tests/services/file_delivery/format_deliver_files_service/utils/test_case_file_formatter.py b/tests/services/file_delivery/format_deliver_files_service/utils/test_case_file_formatter.py new file mode 100644 index 0000000000..fe671fcd42 --- /dev/null +++ b/tests/services/file_delivery/format_deliver_files_service/utils/test_case_file_formatter.py @@ -0,0 +1,34 @@ +import os +from pathlib import Path + +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles, CaseFile +from cg.services.file_delivery.file_formatter_service.models import FormattedFile +from cg.services.file_delivery.file_formatter_service.utils.case_file_formatter import ( + CaseFileFormatter, +) +from cg.services.file_delivery.file_formatter_service.utils.utils import create_ticket_dir + + +def test_case_file_formatter( + expected_moved_analysis_delivery_files: DeliveryFiles, + expected_formatted_analysis_case_files: list[FormattedFile], +): + # GIVEN existing case files, a case file formatter and a ticket directory path and a customer inbox + case_file_formatter = CaseFileFormatter() + ticket_dir_path: Path = expected_moved_analysis_delivery_files.case_files[0].file_path.parent + os.makedirs(ticket_dir_path, exist_ok=True) + case_files: list[CaseFile] = expected_moved_analysis_delivery_files.case_files + for case_file in case_files: + case_file.file_path.touch() + + # WHEN formatting the case files + formatted_files: list[FormattedFile] = case_file_formatter.format_files( + case_files=case_files, + ticket_dir_path=ticket_dir_path, + ) + + # THEN the case files should be formatted + assert formatted_files == expected_formatted_analysis_case_files + for file in formatted_files: + assert file.formatted_path.exists() + assert not file.original_path.exists() diff --git a/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_concatenation_formatter.py b/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_concatenation_formatter.py new file mode 100644 index 0000000000..8618272e30 --- /dev/null +++ b/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_concatenation_formatter.py @@ -0,0 +1,40 @@ +import os +from pathlib import Path + +from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( + FastqConcatenationService, +) +from cg.services.file_delivery.fetch_file_service.models import SampleFile +from cg.services.file_delivery.file_formatter_service.models import FormattedFile +from cg.services.file_delivery.file_formatter_service.utils.sample_file_concatenation_formatter import ( + SampleFileConcatenationFormatter, +) +from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( + SampleFileFormatter, +) + + +def test_sample_file_concatenation_formatter( + fastq_sample_files: list[SampleFile], + expected_concatenated_fastq_formatted_files: list[FormattedFile], +): + # GIVEN existing case files, a case file formatter and a ticket directory path and a customer inbox + sample_file_formatter = SampleFileConcatenationFormatter( + concatenation_service=FastqConcatenationService() + ) + ticket_dir_path: Path = fastq_sample_files[0].file_path.parent + os.makedirs(ticket_dir_path, exist_ok=True) + for sample_file in fastq_sample_files: + sample_file.file_path.touch() + + # WHEN formatting the case files + formatted_files: list[FormattedFile] = sample_file_formatter.format_files( + sample_files=fastq_sample_files, + ticket_dir_path=ticket_dir_path, + ) + + # THEN the case files should be formatted + assert formatted_files == expected_concatenated_fastq_formatted_files + for file in formatted_files: + assert file.formatted_path.exists() + assert not file.original_path.exists() diff --git a/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_formatter.py b/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_formatter.py new file mode 100644 index 0000000000..cdeaccb880 --- /dev/null +++ b/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_formatter.py @@ -0,0 +1,33 @@ +import os +from pathlib import Path + +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles, SampleFile +from cg.services.file_delivery.file_formatter_service.models import FormattedFile +from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( + SampleFileFormatter, +) + + +def test_sample_file_formatter( + expected_moved_analysis_delivery_files: DeliveryFiles, + expected_formatted_analysis_sample_files: list[FormattedFile], +): + # GIVEN existing case files, a case file formatter and a ticket directory path and a customer inbox + sample_file_formatter = SampleFileFormatter() + ticket_dir_path: Path = expected_moved_analysis_delivery_files.sample_files[0].file_path.parent + os.makedirs(ticket_dir_path, exist_ok=True) + sample_files: list[SampleFile] = expected_moved_analysis_delivery_files.sample_files + for sample_file in sample_files: + sample_file.file_path.touch() + + # WHEN formatting the case files + formatted_files: list[FormattedFile] = sample_file_formatter.format_files( + sample_files=sample_files, + ticket_dir_path=ticket_dir_path, + ) + + # THEN the case files should be formatted + assert formatted_files == expected_formatted_analysis_sample_files + for file in formatted_files: + assert file.formatted_path.exists() + assert not file.original_path.exists() From 2845d053592b61ec3e3533e4441ad20f401fa029 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Tue, 27 Aug 2024 10:00:41 +0200 Subject: [PATCH 17/28] add tests for formatters --- ...enation_delivery_file_formatter_service.py | 4 +- ...generic_delivery_file_formatter_service.py | 4 +- .../utils/case_file_formatter.py | 6 +- .../sample_file_concatenation_formatter.py | 4 +- .../utils/sample_file_formatter.py | 6 +- .../delivery_files_models_fixtures.py | 34 +++++++++ .../delivery_formatted_files_fixtures.py | 28 ++------ .../utils/test_case_file_formatter.py | 34 --------- .../utils/test_formatter_utils.py | 69 +++++++++++++++++++ ...est_sample_file_concatenation_formatter.py | 40 ----------- .../utils/test_sample_file_formatter.py | 33 --------- 11 files changed, 121 insertions(+), 141 deletions(-) delete mode 100644 tests/services/file_delivery/format_deliver_files_service/utils/test_case_file_formatter.py create mode 100644 tests/services/file_delivery/format_deliver_files_service/utils/test_formatter_utils.py delete mode 100644 tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_concatenation_formatter.py delete mode 100644 tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_formatter.py diff --git a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py index 39d68bd02a..189637594c 100644 --- a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py @@ -48,13 +48,13 @@ def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: create_ticket_dir(ticket_dir_path) formatted_files: list[FormattedFile] = [] formatter_sample_files = self.sample_file_formatter.format_files( - sample_files=delivery_files.sample_files, + moved_files=delivery_files.sample_files, ticket_dir_path=ticket_dir_path, ) formatted_files.extend(formatter_sample_files) if delivery_files.case_files: formatter_case_file = self.case_file_formatter.format_files( - case_files=delivery_files.case_files, + moved_files=delivery_files.case_files, ticket_dir_path=ticket_dir_path, ) formatted_files.extend(formatter_case_file) diff --git a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py index 41895b2dc7..0c1d334d78 100644 --- a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py @@ -38,13 +38,13 @@ def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: create_ticket_dir(ticket_dir_path) formatted_files: list[FormattedFile] = [] formatter_sample_files = self.sample_file_formatter.format_files( - sample_files=delivery_files.sample_files, + moved_files=delivery_files.sample_files, ticket_dir_path=get_ticket_dir_path(delivery_files.sample_files[0].file_path), ) formatted_files.extend(formatter_sample_files) if delivery_files.case_files: formatter_case_file = self.case_file_formatter.format_files( - case_files=delivery_files.case_files, + moved_files=delivery_files.case_files, ticket_dir_path=get_ticket_dir_path(delivery_files.case_files[0].file_path), ) formatted_files.extend(formatter_case_file) diff --git a/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py index c9e947b48c..43f24a7ada 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py @@ -8,13 +8,13 @@ class CaseFileFormatter: def format_files( - self, case_files: list[CaseFile], ticket_dir_path: Path + self, moved_files: list[CaseFile], ticket_dir_path: Path ) -> list[FormattedFile]: """Format the case files to deliver.""" self._create_case_name_folder( - ticket_path=ticket_dir_path, case_name=case_files[0].case_name + ticket_path=ticket_dir_path, case_name=moved_files[0].case_name ) - return self._rename_case_files(case_files) + return self._rename_case_files(moved_files) @staticmethod def _rename_case_files(case_files: list[CaseFile]) -> list[FormattedFile]: diff --git a/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py index b716f8dead..96f76b8e73 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py @@ -17,10 +17,10 @@ def __init__(self, concatenation_service: FastqConcatenationService): self.concatenation_service = concatenation_service def format_files( - self, sample_files: list[SampleFile], ticket_dir_path: Path + self, moved_files: list[SampleFile], ticket_dir_path: Path ) -> list[FormattedFile]: formatted_files: list[FormattedFile] = super().format_files( - sample_files=sample_files, ticket_dir_path=ticket_dir_path + moved_files=moved_files, ticket_dir_path=ticket_dir_path ) forward_paths, reverse_path = self._concatenate_fastq_files(formatted_files=formatted_files) diff --git a/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py index 46b98a1f8b..591d18533b 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py @@ -7,12 +7,12 @@ class SampleFileFormatter: def format_files( - self, sample_files: list[SampleFile], ticket_dir_path: Path + self, moved_files: list[SampleFile], ticket_dir_path: Path ) -> list[FormattedFile]: """Format the sample files to deliver.""" - sample_names: set[str] = self._get_sample_names(sample_files=sample_files) + sample_names: set[str] = self._get_sample_names(moved_files) self._create_sample_folders(ticket_dir_path=ticket_dir_path, sample_names=sample_names) - return self._rename_sample_files(sample_files=sample_files) + return self._rename_sample_files(moved_files) @staticmethod def _get_sample_names(sample_files: list[SampleFile]) -> set[str]: diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py index 60dbc45191..7414a87b58 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_files_models_fixtures.py @@ -150,6 +150,40 @@ def expected_moved_analysis_delivery_files( ) +@pytest.fixture +def expected_moved_analysis_sample_delivery_files( + expected_moved_analysis_delivery_files: DeliveryFiles, +) -> list[SampleFile]: + return expected_moved_analysis_delivery_files.sample_files + + +@pytest.fixture +def expected_moved_analysis_case_delivery_files( + expected_moved_analysis_delivery_files: DeliveryFiles, +) -> list[CaseFile]: + return expected_moved_analysis_delivery_files.case_files + + +@pytest.fixture +def fastq_concatenation_sample_files(tmp_path: Path) -> list[SampleFile]: + some_ticket: str = "some_ticket" + fastq_paths: list[Path] = [ + Path(tmp_path, some_ticket, "S1_1_R1_1.fastq.gz"), + Path(tmp_path, some_ticket, "S1_2_R1_1.fastq.gz"), + Path(tmp_path, some_ticket, "S1_1_R2_1.fastq.gz"), + Path(tmp_path, some_ticket, "S1_2_R2_1.fastq.gz"), + ] + return [ + SampleFile( + sample_id="S1", + case_id="Case1", + sample_name="Sample1", + file_path=fastq_path, + ) + for fastq_path in fastq_paths + ] + + def swap_file_paths_with_inbox_paths( file_models: list[CaseFile | SampleFile], inbox_dir_path: Path ) -> list[CaseFile | SampleFile]: diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py index 0b712b53e3..a3878ff4eb 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py @@ -60,35 +60,19 @@ def expected_formatted_fastq_sample_files( return formatted_files -@pytest.fixture -def fastq_sample_files(tmp_path: Path) -> list[SampleFile]: - some_ticket: str = "some_ticket" - fastq_paths: list[Path] = [ - Path(tmp_path, some_ticket, "S1_1_R1.fastq.gz"), - Path(tmp_path, some_ticket, "S1_2_R1.fastq.gz"), - Path(tmp_path, some_ticket, "S1_1_R2.fastq.gz"), - Path(tmp_path, some_ticket, "S1_2_R2.fastq.gz"), - ] - return [ - SampleFile( - sample_id="S1", - case_id="Case1", - sample_name="Sample1", - file_path=fastq_path, - ) - for fastq_path in fastq_paths - ] - - @pytest.fixture def expected_concatenated_fastq_formatted_files( - fastq_sample_files: list[SampleFile], + fastq_concatenation_sample_files, ) -> list[FormattedFile]: formatted_files: list[FormattedFile] = [] - for sample_file in fastq_sample_files: + for sample_file in fastq_concatenation_sample_files: replaced_sample_file_name: str = sample_file.file_path.name.replace( sample_file.sample_id, sample_file.sample_name ) + replaced_sample_file_name = replaced_sample_file_name.replace("1_R1_1", "1") + replaced_sample_file_name = replaced_sample_file_name.replace("2_R1_1", "1") + replaced_sample_file_name = replaced_sample_file_name.replace("1_R2_1", "2") + replaced_sample_file_name = replaced_sample_file_name.replace("2_R2_1", "2") formatted_file_path = Path( sample_file.file_path.parent, sample_file.sample_name, replaced_sample_file_name ) diff --git a/tests/services/file_delivery/format_deliver_files_service/utils/test_case_file_formatter.py b/tests/services/file_delivery/format_deliver_files_service/utils/test_case_file_formatter.py deleted file mode 100644 index fe671fcd42..0000000000 --- a/tests/services/file_delivery/format_deliver_files_service/utils/test_case_file_formatter.py +++ /dev/null @@ -1,34 +0,0 @@ -import os -from pathlib import Path - -from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles, CaseFile -from cg.services.file_delivery.file_formatter_service.models import FormattedFile -from cg.services.file_delivery.file_formatter_service.utils.case_file_formatter import ( - CaseFileFormatter, -) -from cg.services.file_delivery.file_formatter_service.utils.utils import create_ticket_dir - - -def test_case_file_formatter( - expected_moved_analysis_delivery_files: DeliveryFiles, - expected_formatted_analysis_case_files: list[FormattedFile], -): - # GIVEN existing case files, a case file formatter and a ticket directory path and a customer inbox - case_file_formatter = CaseFileFormatter() - ticket_dir_path: Path = expected_moved_analysis_delivery_files.case_files[0].file_path.parent - os.makedirs(ticket_dir_path, exist_ok=True) - case_files: list[CaseFile] = expected_moved_analysis_delivery_files.case_files - for case_file in case_files: - case_file.file_path.touch() - - # WHEN formatting the case files - formatted_files: list[FormattedFile] = case_file_formatter.format_files( - case_files=case_files, - ticket_dir_path=ticket_dir_path, - ) - - # THEN the case files should be formatted - assert formatted_files == expected_formatted_analysis_case_files - for file in formatted_files: - assert file.formatted_path.exists() - assert not file.original_path.exists() diff --git a/tests/services/file_delivery/format_deliver_files_service/utils/test_formatter_utils.py b/tests/services/file_delivery/format_deliver_files_service/utils/test_formatter_utils.py new file mode 100644 index 0000000000..9890cce0bc --- /dev/null +++ b/tests/services/file_delivery/format_deliver_files_service/utils/test_formatter_utils.py @@ -0,0 +1,69 @@ +import os +import pytest +from pathlib import Path + +from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( + FastqConcatenationService, +) +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles, CaseFile, SampleFile +from cg.services.file_delivery.file_formatter_service.models import FormattedFile +from cg.services.file_delivery.file_formatter_service.utils.case_file_formatter import ( + CaseFileFormatter, +) +from cg.services.file_delivery.file_formatter_service.utils.sample_file_concatenation_formatter import ( + SampleFileConcatenationFormatter, +) +from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( + SampleFileFormatter, +) + + +@pytest.mark.parametrize( + "moved_files,expected_formatted_files,file_formatter", + [ + ( + "expected_moved_analysis_case_delivery_files", + "expected_formatted_analysis_case_files", + CaseFileFormatter(), + ), + ( + "expected_moved_analysis_sample_delivery_files", + "expected_formatted_analysis_sample_files", + SampleFileFormatter(), + ), + ( + "fastq_concatenation_sample_files", + "expected_concatenated_fastq_formatted_files", + SampleFileConcatenationFormatter(FastqConcatenationService()), + ), + ], +) +def test_file_formatter_utils( + moved_files: list[CaseFile | SampleFile], + expected_formatted_files: list[FormattedFile], + file_formatter: CaseFileFormatter | SampleFileFormatter | SampleFileConcatenationFormatter, + request, +): + # GIVEN existing case files, a case file formatter and a ticket directory path and a customer inbox + moved_files: list[CaseFile | SampleFile] = request.getfixturevalue(moved_files) + expected_formatted_files: list[FormattedFile] = request.getfixturevalue( + expected_formatted_files + ) + ticket_dir_path: Path = moved_files[0].file_path.parent + + os.makedirs(ticket_dir_path, exist_ok=True) + + for moved_file in moved_files: + moved_file.file_path.touch() + + # WHEN formatting the case files + formatted_files: list[FormattedFile] = file_formatter.format_files( + moved_files=moved_files, + ticket_dir_path=ticket_dir_path, + ) + + # THEN the case files should be formatted + assert formatted_files == expected_formatted_files + for file in formatted_files: + assert file.formatted_path.exists() + assert not file.original_path.exists() diff --git a/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_concatenation_formatter.py b/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_concatenation_formatter.py deleted file mode 100644 index 8618272e30..0000000000 --- a/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_concatenation_formatter.py +++ /dev/null @@ -1,40 +0,0 @@ -import os -from pathlib import Path - -from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( - FastqConcatenationService, -) -from cg.services.file_delivery.fetch_file_service.models import SampleFile -from cg.services.file_delivery.file_formatter_service.models import FormattedFile -from cg.services.file_delivery.file_formatter_service.utils.sample_file_concatenation_formatter import ( - SampleFileConcatenationFormatter, -) -from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( - SampleFileFormatter, -) - - -def test_sample_file_concatenation_formatter( - fastq_sample_files: list[SampleFile], - expected_concatenated_fastq_formatted_files: list[FormattedFile], -): - # GIVEN existing case files, a case file formatter and a ticket directory path and a customer inbox - sample_file_formatter = SampleFileConcatenationFormatter( - concatenation_service=FastqConcatenationService() - ) - ticket_dir_path: Path = fastq_sample_files[0].file_path.parent - os.makedirs(ticket_dir_path, exist_ok=True) - for sample_file in fastq_sample_files: - sample_file.file_path.touch() - - # WHEN formatting the case files - formatted_files: list[FormattedFile] = sample_file_formatter.format_files( - sample_files=fastq_sample_files, - ticket_dir_path=ticket_dir_path, - ) - - # THEN the case files should be formatted - assert formatted_files == expected_concatenated_fastq_formatted_files - for file in formatted_files: - assert file.formatted_path.exists() - assert not file.original_path.exists() diff --git a/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_formatter.py b/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_formatter.py deleted file mode 100644 index cdeaccb880..0000000000 --- a/tests/services/file_delivery/format_deliver_files_service/utils/test_sample_file_formatter.py +++ /dev/null @@ -1,33 +0,0 @@ -import os -from pathlib import Path - -from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles, SampleFile -from cg.services.file_delivery.file_formatter_service.models import FormattedFile -from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( - SampleFileFormatter, -) - - -def test_sample_file_formatter( - expected_moved_analysis_delivery_files: DeliveryFiles, - expected_formatted_analysis_sample_files: list[FormattedFile], -): - # GIVEN existing case files, a case file formatter and a ticket directory path and a customer inbox - sample_file_formatter = SampleFileFormatter() - ticket_dir_path: Path = expected_moved_analysis_delivery_files.sample_files[0].file_path.parent - os.makedirs(ticket_dir_path, exist_ok=True) - sample_files: list[SampleFile] = expected_moved_analysis_delivery_files.sample_files - for sample_file in sample_files: - sample_file.file_path.touch() - - # WHEN formatting the case files - formatted_files: list[FormattedFile] = sample_file_formatter.format_files( - sample_files=sample_files, - ticket_dir_path=ticket_dir_path, - ) - - # THEN the case files should be formatted - assert formatted_files == expected_formatted_analysis_sample_files - for file in formatted_files: - assert file.formatted_path.exists() - assert not file.original_path.exists() From 481c518032bd17c4528f1d6d67b4cc9050cb5915 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Tue, 27 Aug 2024 11:32:10 +0200 Subject: [PATCH 18/28] add test --- .../file_formatter_service/utils/utils.py | 3 +- .../delivery_formatted_files_fixtures.py | 5 ++ .../delivery_services_fixtures.py | 35 +++++++++ .../test_format_delivery_files_service.py | 72 +++++++++++++++---- 4 files changed, 99 insertions(+), 16 deletions(-) diff --git a/cg/services/file_delivery/file_formatter_service/utils/utils.py b/cg/services/file_delivery/file_formatter_service/utils/utils.py index 28595c9a26..47f442d8c0 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/utils.py +++ b/cg/services/file_delivery/file_formatter_service/utils/utils.py @@ -1,3 +1,4 @@ +import os from pathlib import Path from cg.constants.delivery import INBOX_NAME @@ -13,4 +14,4 @@ def get_ticket_dir_path(file_path) -> Path: def create_ticket_dir(ticket_dir_path: Path) -> None: - ticket_dir_path.mkdir(exist_ok=True) + os.makedirs(ticket_dir_path) diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py index a3878ff4eb..45aacbefe9 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_formatted_files_fixtures.py @@ -80,3 +80,8 @@ def expected_concatenated_fastq_formatted_files( FormattedFile(original_path=sample_file.file_path, formatted_path=formatted_file_path) ) return formatted_files + + +@pytest.fixture +def empty_case_files() -> None: + return None diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_services_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_services_fixtures.py index 9f89406f6b..ab69ca661e 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_services_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_services_fixtures.py @@ -1,6 +1,9 @@ import pytest from cg.apps.housekeeper.hk import HousekeeperAPI +from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( + FastqConcatenationService, +) from cg.services.file_delivery.fetch_delivery_files_tags.fetch_sample_and_case_delivery_file_tags_service import ( FetchSampleAndCaseDeliveryFileTagsService, ) @@ -10,6 +13,21 @@ from cg.services.file_delivery.fetch_file_service.fetch_fastq_files_service import ( FetchFastqDeliveryFilesService, ) +from cg.services.file_delivery.file_formatter_service.concatenation_delivery_file_formatter_service import ( + ConcatenationDeliveryFileFormatter, +) +from cg.services.file_delivery.file_formatter_service.generic_delivery_file_formatter_service import ( + GenericDeliveryFileFormatter, +) +from cg.services.file_delivery.file_formatter_service.utils.case_file_formatter import ( + CaseFileFormatter, +) +from cg.services.file_delivery.file_formatter_service.utils.sample_file_concatenation_formatter import ( + SampleFileConcatenationFormatter, +) +from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( + SampleFileFormatter, +) from cg.store.store import Store @@ -39,3 +57,20 @@ def analysis_delivery_service( status_db=delivery_store_balsamic, tags_fetcher=tag_service, ) + + +@pytest.fixture +def generic_delivery_file_formatter() -> GenericDeliveryFileFormatter: + """Fixture to get an instance of GenericDeliveryFileFormatter.""" + return GenericDeliveryFileFormatter( + sample_file_formatter=SampleFileFormatter(), case_file_formatter=CaseFileFormatter() + ) + + +@pytest.fixture +def concatenation_delivery_file_formatter() -> ConcatenationDeliveryFileFormatter: + """Fixture to get an instance of ConcatenationDeliveryFileFormatter.""" + return ConcatenationDeliveryFileFormatter( + sample_file_formatter=SampleFileConcatenationFormatter(FastqConcatenationService()), + case_file_formatter=CaseFileFormatter(), + ) diff --git a/tests/services/file_delivery/format_deliver_files_service/test_format_delivery_files_service.py b/tests/services/file_delivery/format_deliver_files_service/test_format_delivery_files_service.py index 37c588f7ad..f1c64fe77d 100644 --- a/tests/services/file_delivery/format_deliver_files_service/test_format_delivery_files_service.py +++ b/tests/services/file_delivery/format_deliver_files_service/test_format_delivery_files_service.py @@ -1,32 +1,74 @@ -from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from unittest.mock import Mock + +import mock + +from cg.services.file_delivery.fetch_file_service.models import ( + DeliveryFiles, + SampleFile, + CaseFile, + DeliveryMetaData, +) from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( DeliveryFileFormattingService, ) -from cg.services.file_delivery.file_formatter_service.generic_delivery_file_formatter_service import ( - GenericDeliveryFileFormatter, -) import pytest -from cg.services.file_delivery.file_formatter_service.models import FormattedFiles +from cg.services.file_delivery.file_formatter_service.models import FormattedFiles, FormattedFile @pytest.mark.parametrize( - "delivery_files,expected_formatted_files,formatter_service", + "formatter_service, formatted_case_files, formatted_sample_files, case_files, sample_files", [ - "moved_fastq_delivery_files", - "expected_formatted_fastq_files", - GenericDeliveryFileFormatter(), + ( + "generic_delivery_file_formatter", + "empty_case_files", + "expected_formatted_analysis_sample_files", + "empty_case_files", + "expected_moved_analysis_sample_delivery_files", + ), + ( + "generic_delivery_file_formatter", + "expected_formatted_analysis_case_files", + "expected_formatted_analysis_sample_files", + "expected_moved_analysis_case_delivery_files", + "expected_moved_analysis_sample_delivery_files", + ), ], ) def test_reformat_files( - delivery_files: DeliveryFiles, - expected_formatted_files: FormattedFiles, formatter_service: DeliveryFileFormattingService, + formatted_case_files: list[FormattedFile], + formatted_sample_files: list[FormattedFile], + case_files: list[CaseFile], + sample_files: list[SampleFile], + request, ): - # GIVEN a delivery file formatter and moved delivery files + # GIVEN a delivery file formatter, mocked delivery files and expected formatted files + formatter_service = request.getfixturevalue(formatter_service) + formatted_case_files = request.getfixturevalue(formatted_case_files) + formatted_sample_files = request.getfixturevalue(formatted_sample_files) + case_files = request.getfixturevalue(case_files) + sample_files = request.getfixturevalue(sample_files) - # WHEN reformatting the delivery files - formatted_files: FormattedFiles = formatter_service.format_files(delivery_files) - # THEN the delivery files should be reformatted + delivery_data = DeliveryMetaData(customer_internal_id="cust_id", ticket_id="ticket_id") + mock_delivery_files = DeliveryFiles( + delivery_data=delivery_data, case_files=case_files, sample_files=sample_files + ) + files = [] + files.extend(formatted_sample_files) + if case_files: + files.extend(formatted_case_files) + + expected_formatted_files = FormattedFiles(files=files) + with mock.patch( + "cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter.SampleFileFormatter.format_files", + return_value=formatted_sample_files, + ), mock.patch( + "cg.services.file_delivery.file_formatter_service.utils.case_file_formatter.CaseFileFormatter.format_files", + return_value=formatted_case_files, + ): + # WHEN reformatting the delivery files + formatted_files: FormattedFiles = formatter_service.format_files(mock_delivery_files) + # THEN the delivery files should be reformatted assert formatted_files == expected_formatted_files From 293ee09de82ea15e94d5e79692cd7d0e3df5e302 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Tue, 27 Aug 2024 12:45:57 +0200 Subject: [PATCH 19/28] review comments --- ...atenation_delivery_file_formatter_service.py | 12 ++++++------ .../generic_delivery_file_formatter_service.py | 10 +++++----- .../utils/case_file_formatter.py | 2 +- .../sample_file_concatenation_formatter.py | 17 +++++++++-------- .../utils/sample_file_formatter.py | 2 +- 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py index 189637594c..3846f5551e 100644 --- a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py @@ -26,7 +26,7 @@ class ConcatenationDeliveryFileFormatter(DeliveryFileFormattingService): """ - Reformat the files to be delivered in the microsalt format. + Reformat the files to be delivered and concatenate fastq files. Expected structure: /inbox/// /inbox/// @@ -43,19 +43,19 @@ def __init__( self.sample_file_formatter = sample_file_formatter def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: - """Format the files to be delivered in the concatenated format.""" + """Format the files to be delivered and reutrn the formatted in the concatenated format.""" ticket_dir_path: Path = get_ticket_dir_path(delivery_files.sample_files[0].file_path) create_ticket_dir(ticket_dir_path) formatted_files: list[FormattedFile] = [] - formatter_sample_files = self.sample_file_formatter.format_files( + formatted_sample_files: list[FormattedFile] = self.sample_file_formatter.format_files( moved_files=delivery_files.sample_files, ticket_dir_path=ticket_dir_path, ) - formatted_files.extend(formatter_sample_files) + formatted_files.extend(formatted_sample_files) if delivery_files.case_files: - formatter_case_file = self.case_file_formatter.format_files( + formatted_case_file: list[FormattedFile] = self.case_file_formatter.format_files( moved_files=delivery_files.case_files, ticket_dir_path=ticket_dir_path, ) - formatted_files.extend(formatter_case_file) + formatted_files.extend(formatted_case_file) return FormattedFiles(files=formatted_files) diff --git a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py index 0c1d334d78..ee50219f58 100644 --- a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py @@ -33,19 +33,19 @@ def __init__( self.sample_file_formatter = sample_file_formatter def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: - """Format the files to be delivered in the generic format.""" + """Format the files to be delivered and return the formatted files in the generic format.""" ticket_dir_path: Path = get_ticket_dir_path(delivery_files.sample_files[0].file_path) create_ticket_dir(ticket_dir_path) formatted_files: list[FormattedFile] = [] - formatter_sample_files = self.sample_file_formatter.format_files( + formatted_sample_files: list[FormattedFile] = self.sample_file_formatter.format_files( moved_files=delivery_files.sample_files, ticket_dir_path=get_ticket_dir_path(delivery_files.sample_files[0].file_path), ) - formatted_files.extend(formatter_sample_files) + formatted_files.extend(formatted_sample_files) if delivery_files.case_files: - formatter_case_file = self.case_file_formatter.format_files( + formatted_case_file: list[FormattedFile] = self.case_file_formatter.format_files( moved_files=delivery_files.case_files, ticket_dir_path=get_ticket_dir_path(delivery_files.case_files[0].file_path), ) - formatted_files.extend(formatter_case_file) + formatted_files.extend(formatted_case_file) return FormattedFiles(files=formatted_files) diff --git a/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py index 43f24a7ada..64ae1d00e7 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py @@ -10,7 +10,7 @@ class CaseFileFormatter: def format_files( self, moved_files: list[CaseFile], ticket_dir_path: Path ) -> list[FormattedFile]: - """Format the case files to deliver.""" + """Format the case files to deliver and return the formatted files..""" self._create_case_name_folder( ticket_path=ticket_dir_path, case_name=moved_files[0].case_name ) diff --git a/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py index 96f76b8e73..90cb27df7b 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py @@ -19,16 +19,17 @@ def __init__(self, concatenation_service: FastqConcatenationService): def format_files( self, moved_files: list[SampleFile], ticket_dir_path: Path ) -> list[FormattedFile]: + """Format the sample files to deliver, concatenate fastq files and return the formatted files.""" formatted_files: list[FormattedFile] = super().format_files( moved_files=moved_files, ticket_dir_path=ticket_dir_path ) forward_paths, reverse_path = self._concatenate_fastq_files(formatted_files=formatted_files) - - return self._replace_fastq_paths( + self._replace_fastq_paths( reverse_paths=reverse_path, forward_paths=forward_paths, formatted_files=formatted_files, ) + return formatted_files def _concatenate_fastq_files( self, formatted_files: list[FormattedFile] @@ -74,7 +75,8 @@ def _replace_fastq_formatted_file_path( formatted_files: list[FormattedFile], direction: ReadDirection, new_path: Path, - ): + ) -> None: + """Replace the formatted file path with the new path.""" for formatted_file in formatted_files: if ( formatted_file.formatted_path.parent == new_path.parent @@ -82,24 +84,23 @@ def _replace_fastq_formatted_file_path( and f"R{direction}" in formatted_file.formatted_path.name ): formatted_file.formatted_path = new_path - return formatted_files def _replace_fastq_paths( self, forward_paths: list[Path], reverse_paths: list[Path], formatted_files: list[FormattedFile], - ) -> list[FormattedFile]: + ) -> None: + """Replace the fastq file paths with the new concatenated fastq file paths.""" for forward_path in forward_paths: - formatted_files = self._replace_fastq_formatted_file_path( + self._replace_fastq_formatted_file_path( formatted_files=formatted_files, direction=ReadDirection.FORWARD, new_path=forward_path, ) for reverse_path in reverse_paths: - formatted_files = self._replace_fastq_formatted_file_path( + self._replace_fastq_formatted_file_path( formatted_files=formatted_files, direction=ReadDirection.REVERSE, new_path=reverse_path, ) - return formatted_files diff --git a/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py index 591d18533b..83b90ffca4 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py @@ -9,7 +9,7 @@ class SampleFileFormatter: def format_files( self, moved_files: list[SampleFile], ticket_dir_path: Path ) -> list[FormattedFile]: - """Format the sample files to deliver.""" + """Format the sample files to deliver and return the formatted files.""" sample_names: set[str] = self._get_sample_names(moved_files) self._create_sample_folders(ticket_dir_path=ticket_dir_path, sample_names=sample_names) return self._rename_sample_files(moved_files) From db99c052fa4324cc888b6cd2bf8fed9923a49e39 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Tue, 27 Aug 2024 14:11:33 +0200 Subject: [PATCH 20/28] make super call --- ...enation_delivery_file_formatter_service.py | 37 +++---------------- 1 file changed, 6 insertions(+), 31 deletions(-) diff --git a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py index 3846f5551e..36b21035ff 100644 --- a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py @@ -1,30 +1,22 @@ from pathlib import Path - -from cg.constants.delivery import INBOX_NAME -from cg.services.fastq_concatenation_service.fastq_concatenation_service import ( - FastqConcatenationService, -) from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from cg.services.file_delivery.file_formatter_service.generic_delivery_file_formatter_service import ( + GenericDeliveryFileFormatter, +) from cg.services.file_delivery.file_formatter_service.models import FormattedFiles, FormattedFile from cg.services.file_delivery.file_formatter_service.utils.case_file_formatter import ( CaseFileFormatter, ) -from cg.services.file_delivery.file_formatter_service.delivery_file_formatting_service import ( - DeliveryFileFormattingService, -) from cg.services.file_delivery.file_formatter_service.utils.sample_file_concatenation_formatter import ( SampleFileConcatenationFormatter, ) -from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( - SampleFileFormatter, -) from cg.services.file_delivery.file_formatter_service.utils.utils import ( get_ticket_dir_path, create_ticket_dir, ) -class ConcatenationDeliveryFileFormatter(DeliveryFileFormattingService): +class ConcatenationDeliveryFileFormatter(GenericDeliveryFileFormatter): """ Reformat the files to be delivered and concatenate fastq files. Expected structure: @@ -39,23 +31,6 @@ def __init__( case_file_formatter: CaseFileFormatter, sample_file_formatter: SampleFileConcatenationFormatter, ): - self.case_file_formatter = case_file_formatter - self.sample_file_formatter = sample_file_formatter - - def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: - """Format the files to be delivered and reutrn the formatted in the concatenated format.""" - ticket_dir_path: Path = get_ticket_dir_path(delivery_files.sample_files[0].file_path) - create_ticket_dir(ticket_dir_path) - formatted_files: list[FormattedFile] = [] - formatted_sample_files: list[FormattedFile] = self.sample_file_formatter.format_files( - moved_files=delivery_files.sample_files, - ticket_dir_path=ticket_dir_path, + super().__init__( + case_file_formatter=case_file_formatter, sample_file_formatter=sample_file_formatter ) - formatted_files.extend(formatted_sample_files) - if delivery_files.case_files: - formatted_case_file: list[FormattedFile] = self.case_file_formatter.format_files( - moved_files=delivery_files.case_files, - ticket_dir_path=ticket_dir_path, - ) - formatted_files.extend(formatted_case_file) - return FormattedFiles(files=formatted_files) From 93056cd3b60f2859604509b9a3532e44943b7742 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Tue, 27 Aug 2024 14:25:38 +0200 Subject: [PATCH 21/28] simplifiy formatter --- ...enation_delivery_file_formatter_service.py | 36 ------------------- ..._service.py => delivery_file_formatter.py} | 9 +++-- .../delivery_services_fixtures.py | 20 +++-------- 3 files changed, 11 insertions(+), 54 deletions(-) delete mode 100644 cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py rename cg/services/file_delivery/file_formatter_service/{generic_delivery_file_formatter_service.py => delivery_file_formatter.py} (86%) diff --git a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py deleted file mode 100644 index 36b21035ff..0000000000 --- a/cg/services/file_delivery/file_formatter_service/concatenation_delivery_file_formatter_service.py +++ /dev/null @@ -1,36 +0,0 @@ -from pathlib import Path -from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles -from cg.services.file_delivery.file_formatter_service.generic_delivery_file_formatter_service import ( - GenericDeliveryFileFormatter, -) -from cg.services.file_delivery.file_formatter_service.models import FormattedFiles, FormattedFile -from cg.services.file_delivery.file_formatter_service.utils.case_file_formatter import ( - CaseFileFormatter, -) -from cg.services.file_delivery.file_formatter_service.utils.sample_file_concatenation_formatter import ( - SampleFileConcatenationFormatter, -) -from cg.services.file_delivery.file_formatter_service.utils.utils import ( - get_ticket_dir_path, - create_ticket_dir, -) - - -class ConcatenationDeliveryFileFormatter(GenericDeliveryFileFormatter): - """ - Reformat the files to be delivered and concatenate fastq files. - Expected structure: - /inbox/// - /inbox/// - Extra rule: - fastq files are concatenated into a single file per read direction. - """ - - def __init__( - self, - case_file_formatter: CaseFileFormatter, - sample_file_formatter: SampleFileConcatenationFormatter, - ): - super().__init__( - case_file_formatter=case_file_formatter, sample_file_formatter=sample_file_formatter - ) diff --git a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py b/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py similarity index 86% rename from cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py rename to cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py index ee50219f58..4efbe70817 100644 --- a/cg/services/file_delivery/file_formatter_service/generic_delivery_file_formatter_service.py +++ b/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py @@ -9,6 +9,9 @@ DeliveryFileFormattingService, ) from cg.services.file_delivery.file_formatter_service.models import FormattedFiles, FormattedFile +from cg.services.file_delivery.file_formatter_service.utils.sample_file_concatenation_formatter import ( + SampleFileConcatenationFormatter, +) from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( SampleFileFormatter, ) @@ -18,7 +21,7 @@ ) -class GenericDeliveryFileFormatter(DeliveryFileFormattingService): +class DeliveryFileFormatter(DeliveryFileFormattingService): """ Format the files to be delivered in the generic format. Expected structure: @@ -27,7 +30,9 @@ class GenericDeliveryFileFormatter(DeliveryFileFormattingService): """ def __init__( - self, case_file_formatter: CaseFileFormatter, sample_file_formatter: SampleFileFormatter + self, + case_file_formatter: CaseFileFormatter, + sample_file_formatter: SampleFileFormatter | SampleFileConcatenationFormatter, ): self.case_file_formatter = case_file_formatter self.sample_file_formatter = sample_file_formatter diff --git a/tests/fixture_plugins/delivery_fixtures/delivery_services_fixtures.py b/tests/fixture_plugins/delivery_fixtures/delivery_services_fixtures.py index ab69ca661e..15f30d6169 100644 --- a/tests/fixture_plugins/delivery_fixtures/delivery_services_fixtures.py +++ b/tests/fixture_plugins/delivery_fixtures/delivery_services_fixtures.py @@ -13,11 +13,8 @@ from cg.services.file_delivery.fetch_file_service.fetch_fastq_files_service import ( FetchFastqDeliveryFilesService, ) -from cg.services.file_delivery.file_formatter_service.concatenation_delivery_file_formatter_service import ( - ConcatenationDeliveryFileFormatter, -) -from cg.services.file_delivery.file_formatter_service.generic_delivery_file_formatter_service import ( - GenericDeliveryFileFormatter, +from cg.services.file_delivery.file_formatter_service.delivery_file_formatter import ( + DeliveryFileFormatter, ) from cg.services.file_delivery.file_formatter_service.utils.case_file_formatter import ( CaseFileFormatter, @@ -60,17 +57,8 @@ def analysis_delivery_service( @pytest.fixture -def generic_delivery_file_formatter() -> GenericDeliveryFileFormatter: +def generic_delivery_file_formatter() -> DeliveryFileFormatter: """Fixture to get an instance of GenericDeliveryFileFormatter.""" - return GenericDeliveryFileFormatter( + return DeliveryFileFormatter( sample_file_formatter=SampleFileFormatter(), case_file_formatter=CaseFileFormatter() ) - - -@pytest.fixture -def concatenation_delivery_file_formatter() -> ConcatenationDeliveryFileFormatter: - """Fixture to get an instance of ConcatenationDeliveryFileFormatter.""" - return ConcatenationDeliveryFileFormatter( - sample_file_formatter=SampleFileConcatenationFormatter(FastqConcatenationService()), - case_file_formatter=CaseFileFormatter(), - ) From c13ce349b19567a68778b898d85040b48a0118ab Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Tue, 27 Aug 2024 14:31:30 +0200 Subject: [PATCH 22/28] add docstring --- .../utils/sample_file_concatenation_formatter.py | 5 +++++ .../file_formatter_service/utils/sample_file_formatter.py | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py index 90cb27df7b..626ec274ee 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py @@ -13,6 +13,11 @@ class SampleFileConcatenationFormatter(SampleFileFormatter): + """ + Format the sample files to deliver, concatenate fastq files and return the formatted files. + Used for workflows: Microsalt and Mutant. + """ + def __init__(self, concatenation_service: FastqConcatenationService): self.concatenation_service = concatenation_service diff --git a/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py index 83b90ffca4..c0dc2a1ed8 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py @@ -5,6 +5,10 @@ class SampleFileFormatter: + """ + Format the sample files to deliver. + Used for all workflows except Microsalt and Mutant. + """ def format_files( self, moved_files: list[SampleFile], ticket_dir_path: Path From 5d1ddb9a49d6262d574e527f2ae86a4d81cbdfe2 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Tue, 27 Aug 2024 14:34:22 +0200 Subject: [PATCH 23/28] solve imports --- .../test_format_delivery_files_service.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/services/file_delivery/format_deliver_files_service/test_format_delivery_files_service.py b/tests/services/file_delivery/format_deliver_files_service/test_format_delivery_files_service.py index f1c64fe77d..3dbfd7d37d 100644 --- a/tests/services/file_delivery/format_deliver_files_service/test_format_delivery_files_service.py +++ b/tests/services/file_delivery/format_deliver_files_service/test_format_delivery_files_service.py @@ -1,5 +1,3 @@ -from unittest.mock import Mock - import mock from cg.services.file_delivery.fetch_file_service.models import ( From 3d605acf3b6bf5528bc0f9ffc90534803d2037c0 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Wed, 28 Aug 2024 12:58:39 +0200 Subject: [PATCH 24/28] review pt1 --- .../delivery_file_formatter.py | 55 +++++++++++++------ .../utils/case_file_formatter.py | 29 ++++++---- .../utils/sample_file_formatter.py | 17 ++++-- .../file_formatter_service/utils/utils.py | 17 ------ 4 files changed, 70 insertions(+), 48 deletions(-) delete mode 100644 cg/services/file_delivery/file_formatter_service/utils/utils.py diff --git a/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py b/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py index 4efbe70817..e86a706ce5 100644 --- a/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py @@ -15,10 +15,6 @@ from cg.services.file_delivery.file_formatter_service.utils.sample_file_formatter import ( SampleFileFormatter, ) -from cg.services.file_delivery.file_formatter_service.utils.utils import ( - get_ticket_dir_path, - create_ticket_dir, -) class DeliveryFileFormatter(DeliveryFileFormattingService): @@ -39,18 +35,43 @@ def __init__( def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: """Format the files to be delivered and return the formatted files in the generic format.""" - ticket_dir_path: Path = get_ticket_dir_path(delivery_files.sample_files[0].file_path) - create_ticket_dir(ticket_dir_path) - formatted_files: list[FormattedFile] = [] - formatted_sample_files: list[FormattedFile] = self.sample_file_formatter.format_files( - moved_files=delivery_files.sample_files, - ticket_dir_path=get_ticket_dir_path(delivery_files.sample_files[0].file_path), + ticket_dir_path: Path = self.get_folder_under_inbox( + delivery_files.sample_files[0].file_path + ) + self._create_ticket_dir(ticket_dir_path) + formatted_files = self._format_sample_and_case_files( + sample_files=delivery_files.sample_files, + case_files=delivery_files.case_files, + ticket_dir_path=ticket_dir_path, ) - formatted_files.extend(formatted_sample_files) - if delivery_files.case_files: - formatted_case_file: list[FormattedFile] = self.case_file_formatter.format_files( - moved_files=delivery_files.case_files, - ticket_dir_path=get_ticket_dir_path(delivery_files.case_files[0].file_path), - ) - formatted_files.extend(formatted_case_file) return FormattedFiles(files=formatted_files) + + def _format_sample_and_case_files( + self, sample_files, case_files, ticket_dir_path + ) -> list[FormattedFile]: + """Helper method to format both sample and case files.""" + formatted_sample_files = self.sample_file_formatter.format_files( + moved_files=sample_files, + ticket_dir_path=ticket_dir_path, + ) + formatted_files = formatted_sample_files + if case_files: + formatted_case_files = self.case_file_formatter.format_files( + moved_files=case_files, + ticket_dir_path=ticket_dir_path, + ) + formatted_files.extend(formatted_case_files) + + return formatted_files + + @staticmethod + def get_folder_under_inbox(file_path: Path) -> Path: + try: + inbox_index = file_path.parts.index(INBOX_NAME) + return Path(*file_path.parts[: inbox_index + 2]) + except ValueError: + raise ValueError(f"Could not find the inbox directory in the path: {file_path}") + + @staticmethod + def _create_ticket_dir(ticket_dir_path: Path) -> None: + os.makedirs(ticket_dir_path, exist_ok=True) diff --git a/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py index 64ae1d00e7..eeb522c1d3 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/case_file_formatter.py @@ -2,7 +2,7 @@ from pathlib import Path from cg.services.file_delivery.fetch_file_service.models import CaseFile -from cg.services.file_delivery.file_formatter_service.models import FormattedFiles, FormattedFile +from cg.services.file_delivery.file_formatter_service.models import FormattedFile class CaseFileFormatter: @@ -14,10 +14,26 @@ def format_files( self._create_case_name_folder( ticket_path=ticket_dir_path, case_name=moved_files[0].case_name ) - return self._rename_case_files(moved_files) + return self._format_case_files(moved_files) + + def _format_case_files(self, case_files: list[CaseFile]) -> list[FormattedFile]: + formatted_files: list[FormattedFile] = self._get_formatted_files(case_files) + for formatted_file in formatted_files: + os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) + return formatted_files + + @staticmethod + def _create_case_name_folder(ticket_path: Path, case_name: str) -> None: + case_dir_path = Path(ticket_path, case_name) + case_dir_path.mkdir(exist_ok=True) @staticmethod - def _rename_case_files(case_files: list[CaseFile]) -> list[FormattedFile]: + def _get_formatted_files(case_files: list[CaseFile]) -> list[FormattedFile]: + """ + Returns formatted files: + 1. Adds a folder with case name to the path of the case files. + 2. Replaces case id by case name. + """ formatted_files: list[FormattedFile] = [] for case_file in case_files: replaced_case_file_name: str = case_file.file_path.name.replace( @@ -29,11 +45,4 @@ def _rename_case_files(case_files: list[CaseFile]) -> list[FormattedFile]: formatted_files.append( FormattedFile(original_path=case_file.file_path, formatted_path=formatted_file_path) ) - for formatted_file in formatted_files: - os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) return formatted_files - - @staticmethod - def _create_case_name_folder(ticket_path: Path, case_name: str) -> None: - case_dir_path = Path(ticket_path, case_name) - case_dir_path.mkdir(exist_ok=True) diff --git a/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py index c0dc2a1ed8..e1e70058b1 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_formatter.py @@ -16,7 +16,7 @@ def format_files( """Format the sample files to deliver and return the formatted files.""" sample_names: set[str] = self._get_sample_names(moved_files) self._create_sample_folders(ticket_dir_path=ticket_dir_path, sample_names=sample_names) - return self._rename_sample_files(moved_files) + return self._format_sample_files(moved_files) @staticmethod def _get_sample_names(sample_files: list[SampleFile]) -> set[str]: @@ -28,8 +28,19 @@ def _create_sample_folders(ticket_dir_path: Path, sample_names: set[str]): sample_dir_path = Path(ticket_dir_path, sample_name) sample_dir_path.mkdir(exist_ok=True) + def _format_sample_files(self, sample_files: list[SampleFile]) -> list[FormattedFile]: + formatted_files: list[FormattedFile] = self._get_formatted_files(sample_files) + for formatted_file in formatted_files: + os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) + return formatted_files + @staticmethod - def _rename_sample_files(sample_files: list[SampleFile]) -> list[FormattedFile]: + def _get_formatted_files(sample_files: list[SampleFile]) -> list[FormattedFile]: + """ + Returns formatted files: + 1. Adds a folder with sample name to the path of the sample files. + 2. Replaces sample id by sample name. + """ formatted_files: list[FormattedFile] = [] for sample_file in sample_files: replaced_sample_file_name: str = sample_file.file_path.name.replace( @@ -43,6 +54,4 @@ def _rename_sample_files(sample_files: list[SampleFile]) -> list[FormattedFile]: original_path=sample_file.file_path, formatted_path=formatted_file_path ) ) - for formatted_file in formatted_files: - os.rename(src=formatted_file.original_path, dst=formatted_file.formatted_path) return formatted_files diff --git a/cg/services/file_delivery/file_formatter_service/utils/utils.py b/cg/services/file_delivery/file_formatter_service/utils/utils.py deleted file mode 100644 index 47f442d8c0..0000000000 --- a/cg/services/file_delivery/file_formatter_service/utils/utils.py +++ /dev/null @@ -1,17 +0,0 @@ -import os -from pathlib import Path - -from cg.constants.delivery import INBOX_NAME - - -def is_inbox_path(file_path: Path): - return INBOX_NAME in str(file_path) - - -def get_ticket_dir_path(file_path) -> Path: - if is_inbox_path(file_path): - return file_path.parent - - -def create_ticket_dir(ticket_dir_path: Path) -> None: - os.makedirs(ticket_dir_path) From 1694efefa48a119a722cc93169f6d96783bdf4d4 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Wed, 28 Aug 2024 13:01:21 +0200 Subject: [PATCH 25/28] review pt2 --- .../utils/sample_file_concatenation_formatter.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py index 626ec274ee..fe3701668c 100644 --- a/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/utils/sample_file_concatenation_formatter.py @@ -44,8 +44,7 @@ def _concatenate_fastq_files( ) forward_paths: list[Path] = [] reverse_paths: list[Path] = [] - for sample_dir_path in unique_sample_dir_paths: - fastq_directory: Path = sample_dir_path + for fastq_directory in unique_sample_dir_paths: sample_name: str = fastq_directory.name forward_path: Path = generate_concatenated_fastq_delivery_path( From 1c6e67c34f1374769235be894cacf52ac8876e34 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Wed, 28 Aug 2024 14:00:22 +0200 Subject: [PATCH 26/28] small fix --- .../file_formatter_service/delivery_file_formatter.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py b/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py index e86a706ce5..7981b2c677 100644 --- a/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py @@ -50,18 +50,16 @@ def _format_sample_and_case_files( self, sample_files, case_files, ticket_dir_path ) -> list[FormattedFile]: """Helper method to format both sample and case files.""" - formatted_sample_files = self.sample_file_formatter.format_files( + formatted_files = self.sample_file_formatter.format_files( moved_files=sample_files, ticket_dir_path=ticket_dir_path, ) - formatted_files = formatted_sample_files if case_files: formatted_case_files = self.case_file_formatter.format_files( moved_files=case_files, ticket_dir_path=ticket_dir_path, ) formatted_files.extend(formatted_case_files) - return formatted_files @staticmethod From 45f8777301c684d35be4d659bd1dfe535251b777 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Wed, 28 Aug 2024 14:02:13 +0200 Subject: [PATCH 27/28] missing typehints --- .../file_formatter_service/delivery_file_formatter.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py b/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py index 7981b2c677..8c004eaa2c 100644 --- a/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py @@ -39,7 +39,7 @@ def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: delivery_files.sample_files[0].file_path ) self._create_ticket_dir(ticket_dir_path) - formatted_files = self._format_sample_and_case_files( + formatted_files: list[FormattedFile] = self._format_sample_and_case_files( sample_files=delivery_files.sample_files, case_files=delivery_files.case_files, ticket_dir_path=ticket_dir_path, @@ -50,12 +50,12 @@ def _format_sample_and_case_files( self, sample_files, case_files, ticket_dir_path ) -> list[FormattedFile]: """Helper method to format both sample and case files.""" - formatted_files = self.sample_file_formatter.format_files( + formatted_files: list[FormattedFile] = self.sample_file_formatter.format_files( moved_files=sample_files, ticket_dir_path=ticket_dir_path, ) if case_files: - formatted_case_files = self.case_file_formatter.format_files( + formatted_case_files: list[FormattedFile] = self.case_file_formatter.format_files( moved_files=case_files, ticket_dir_path=ticket_dir_path, ) @@ -65,7 +65,7 @@ def _format_sample_and_case_files( @staticmethod def get_folder_under_inbox(file_path: Path) -> Path: try: - inbox_index = file_path.parts.index(INBOX_NAME) + inbox_index: int = file_path.parts.index(INBOX_NAME) return Path(*file_path.parts[: inbox_index + 2]) except ValueError: raise ValueError(f"Could not find the inbox directory in the path: {file_path}") From 729de7a678dc4fd7df6c206880a6cf75822532f7 Mon Sep 17 00:00:00 2001 From: Christian Oertlin Date: Thu, 29 Aug 2024 08:33:10 +0200 Subject: [PATCH 28/28] typehint --- .../file_formatter_service/delivery_file_formatter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py b/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py index 8c004eaa2c..552635b10b 100644 --- a/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py +++ b/cg/services/file_delivery/file_formatter_service/delivery_file_formatter.py @@ -1,7 +1,7 @@ import os from pathlib import Path from cg.constants.delivery import INBOX_NAME -from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles +from cg.services.file_delivery.fetch_file_service.models import DeliveryFiles, SampleFile, CaseFile from cg.services.file_delivery.file_formatter_service.utils.case_file_formatter import ( CaseFileFormatter, ) @@ -47,7 +47,7 @@ def format_files(self, delivery_files: DeliveryFiles) -> FormattedFiles: return FormattedFiles(files=formatted_files) def _format_sample_and_case_files( - self, sample_files, case_files, ticket_dir_path + self, sample_files: list[SampleFile], case_files: list[CaseFile], ticket_dir_path: Path ) -> list[FormattedFile]: """Helper method to format both sample and case files.""" formatted_files: list[FormattedFile] = self.sample_file_formatter.format_files(