From 018069d2cecf6ab2a6d9070ee3a89ed5dbc555a3 Mon Sep 17 00:00:00 2001 From: Yuval Yaron Date: Wed, 22 Feb 2023 13:25:29 +0000 Subject: [PATCH 1/7] use unique_identifier_suffix instead of short workspace ID in airlock --- .../StatusChangedQueueTrigger/__init__.py | 40 +++++++++---------- api_app/event_grid/event_sender.py | 5 +-- api_app/models/domain/events.py | 2 +- api_app/services/airlock.py | 19 ++++----- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/airlock_processor/StatusChangedQueueTrigger/__init__.py b/airlock_processor/StatusChangedQueueTrigger/__init__.py index 5946a9b4ec..df087f1540 100644 --- a/airlock_processor/StatusChangedQueueTrigger/__init__.py +++ b/airlock_processor/StatusChangedQueueTrigger/__init__.py @@ -18,7 +18,7 @@ class RequestProperties(BaseModel): new_status: str previous_status: Optional[str] type: str - workspace_id: str + unique_identifier_suffix: str class ContainersCopyMetadata: @@ -48,18 +48,18 @@ def handle_status_changed(request_properties: RequestProperties, stepResultEvent new_status = request_properties.new_status previous_status = request_properties.previous_status req_id = request_properties.request_id - ws_id = request_properties.workspace_id + unique_suffix = request_properties.unique_identifier_suffix request_type = request_properties.type logging.info('Processing request with id %s. new status is "%s", type is "%s"', req_id, new_status, request_type) if new_status == constants.STAGE_DRAFT: - account_name = get_storage_account(status=constants.STAGE_DRAFT, request_type=request_type, short_workspace_id=ws_id) + account_name = get_storage_account(status=constants.STAGE_DRAFT, request_type=request_type, unique_identifier_suffix=unique_suffix) blob_operations.create_container(account_name, req_id) return if new_status == constants.STAGE_CANCELLED: - storage_account_name = get_storage_account(previous_status, request_type, ws_id) + storage_account_name = get_storage_account(previous_status, request_type, unique_suffix) container_to_delete_url = blob_operations.get_blob_url(account_name=storage_account_name, container_name=req_id) set_output_event_to_trigger_container_deletion(dataDeletionEvent, request_properties, container_url=container_to_delete_url) return @@ -69,7 +69,7 @@ def handle_status_changed(request_properties: RequestProperties, stepResultEvent if (is_require_data_copy(new_status)): logging.info('Request with id %s. requires data copy between storage accounts', req_id) - containers_metadata = get_source_dest_for_copy(new_status=new_status, previous_status=previous_status, request_type=request_type, short_workspace_id=ws_id) + containers_metadata = get_source_dest_for_copy(new_status=new_status, previous_status=previous_status, request_type=request_type, unique_identifier_suffix=unique_suffix) blob_operations.create_container(containers_metadata.dest_account_name, req_id) blob_operations.copy_data(containers_metadata.source_account_name, containers_metadata.dest_account_name, req_id) @@ -102,7 +102,7 @@ def is_require_data_copy(new_status: str): return False -def get_source_dest_for_copy(new_status: str, previous_status: str, request_type: str, short_workspace_id: str) -> ContainersCopyMetadata: +def get_source_dest_for_copy(new_status: str, previous_status: str, request_type: str, unique_identifier_suffix: str) -> ContainersCopyMetadata: # sanity if is_require_data_copy(new_status) is False: raise Exception("Given new status is not supported") @@ -114,19 +114,19 @@ def get_source_dest_for_copy(new_status: str, previous_status: str, request_type logging.error(msg) raise Exception(msg) - source_account_name = get_storage_account(previous_status, request_type, short_workspace_id) - dest_account_name = get_storage_account_destination_for_copy(new_status, request_type, short_workspace_id) + source_account_name = get_storage_account(previous_status, request_type, unique_identifier_suffix) + dest_account_name = get_storage_account_destination_for_copy(new_status, request_type, unique_identifier_suffix) return ContainersCopyMetadata(source_account_name, dest_account_name) -def get_storage_account(status: str, request_type: str, short_workspace_id: str) -> str: +def get_storage_account(status: str, request_type: str, unique_identifier_suffix: str) -> str: tre_id = _get_tre_id() if request_type == constants.IMPORT_TYPE: if status == constants.STAGE_DRAFT: return constants.STORAGE_ACCOUNT_NAME_IMPORT_EXTERNAL + tre_id elif status == constants.STAGE_APPROVED: - return constants.STORAGE_ACCOUNT_NAME_IMPORT_APPROVED + short_workspace_id + return constants.STORAGE_ACCOUNT_NAME_IMPORT_APPROVED + unique_identifier_suffix elif status == constants.STAGE_REJECTED: return constants.STORAGE_ACCOUNT_NAME_IMPORT_REJECTED + tre_id elif status == constants.STAGE_BLOCKED_BY_SCAN: @@ -136,29 +136,29 @@ def get_storage_account(status: str, request_type: str, short_workspace_id: str) if request_type == constants.EXPORT_TYPE: if status == constants.STAGE_DRAFT: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_INTERNAL + short_workspace_id + return constants.STORAGE_ACCOUNT_NAME_EXPORT_INTERNAL + unique_identifier_suffix elif status == constants.STAGE_APPROVED: return constants.STORAGE_ACCOUNT_NAME_EXPORT_APPROVED + tre_id elif status == constants.STAGE_REJECTED: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_REJECTED + short_workspace_id + return constants.STORAGE_ACCOUNT_NAME_EXPORT_REJECTED + unique_identifier_suffix elif status == constants.STAGE_BLOCKED_BY_SCAN: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_BLOCKED + short_workspace_id + return constants.STORAGE_ACCOUNT_NAME_EXPORT_BLOCKED + unique_identifier_suffix elif status in [constants.STAGE_IN_REVIEW, constants.STAGE_SUBMITTED, constants.STAGE_APPROVAL_INPROGRESS, constants.STAGE_REJECTION_INPROGRESS, constants.STAGE_BLOCKING_INPROGRESS]: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS + short_workspace_id + return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS + unique_identifier_suffix error_message = f"Missing current storage account definition for status '{status}' and request type '{request_type}'." logging.error(error_message) raise Exception(error_message) -def get_storage_account_destination_for_copy(new_status: str, request_type: str, short_workspace_id: str) -> str: +def get_storage_account_destination_for_copy(new_status: str, request_type: str, unique_identifier_suffix: str) -> str: tre_id = _get_tre_id() if request_type == constants.IMPORT_TYPE: if new_status == constants.STAGE_SUBMITTED: return constants.STORAGE_ACCOUNT_NAME_IMPORT_INPROGRESS + tre_id elif new_status == constants.STAGE_APPROVAL_INPROGRESS: - return constants.STORAGE_ACCOUNT_NAME_IMPORT_APPROVED + short_workspace_id + return constants.STORAGE_ACCOUNT_NAME_IMPORT_APPROVED + unique_identifier_suffix elif new_status == constants.STAGE_REJECTION_INPROGRESS: return constants.STORAGE_ACCOUNT_NAME_IMPORT_REJECTED + tre_id elif new_status == constants.STAGE_BLOCKING_INPROGRESS: @@ -166,13 +166,13 @@ def get_storage_account_destination_for_copy(new_status: str, request_type: str, if request_type == constants.EXPORT_TYPE: if new_status == constants.STAGE_SUBMITTED: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS + short_workspace_id + return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS + unique_identifier_suffix elif new_status == constants.STAGE_APPROVAL_INPROGRESS: return constants.STORAGE_ACCOUNT_NAME_EXPORT_APPROVED + tre_id elif new_status == constants.STAGE_REJECTION_INPROGRESS: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_REJECTED + short_workspace_id + return constants.STORAGE_ACCOUNT_NAME_EXPORT_REJECTED + unique_identifier_suffix elif new_status == constants.STAGE_BLOCKING_INPROGRESS: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_BLOCKED + short_workspace_id + return constants.STORAGE_ACCOUNT_NAME_EXPORT_BLOCKED + unique_identifier_suffix error_message = f"Missing copy destination storage account definition for status '{new_status}' and request type '{request_type}'." logging.error(error_message) @@ -218,7 +218,7 @@ def set_output_event_to_trigger_container_deletion(dataDeletionEvent, request_pr def get_request_files(request_properties: RequestProperties): - storage_account_name = get_storage_account(request_properties.previous_status, request_properties.type, request_properties.workspace_id) + storage_account_name = get_storage_account(request_properties.previous_status, request_properties.type, request_properties.unique_identifier_suffix) return blob_operations.get_request_files(account_name=storage_account_name, request_id=request_properties.request_id) diff --git a/api_app/event_grid/event_sender.py b/api_app/event_grid/event_sender.py index 6799ac7895..dc1214da34 100644 --- a/api_app/event_grid/event_sender.py +++ b/api_app/event_grid/event_sender.py @@ -11,16 +11,15 @@ from models.domain.workspace import Workspace -async def send_status_changed_event(airlock_request: AirlockRequest, previous_status: Optional[AirlockRequestStatus]): +async def send_status_changed_event(airlock_request: AirlockRequest, unique_identifier_suffix: str, previous_status: Optional[AirlockRequestStatus]): request_id = airlock_request.id new_status = airlock_request.status.value previous_status = previous_status.value if previous_status else None request_type = airlock_request.type.value - short_workspace_id = airlock_request.workspaceId[-4:] status_changed_event = EventGridEvent( event_type="statusChanged", - data=StatusChangedData(request_id=request_id, new_status=new_status, previous_status=previous_status, type=request_type, workspace_id=short_workspace_id).__dict__, + data=StatusChangedData(request_id=request_id, new_status=new_status, previous_status=previous_status, type=request_type, unique_identifier_suffix=unique_identifier_suffix).__dict__, subject=f"{request_id}/statusChanged", data_version="2.0" ) diff --git a/api_app/models/domain/events.py b/api_app/models/domain/events.py index 76d7c557c9..4fe64ebd2d 100644 --- a/api_app/models/domain/events.py +++ b/api_app/models/domain/events.py @@ -39,4 +39,4 @@ class StatusChangedData(AzureTREModel): new_status: str previous_status: Optional[str] type: str - workspace_id: str + unique_identifier_suffix: str diff --git a/api_app/services/airlock.py b/api_app/services/airlock.py index a0d4ed40f4..24f0c1b574 100644 --- a/api_app/services/airlock.py +++ b/api_app/services/airlock.py @@ -36,7 +36,8 @@ def get_account_by_request(airlock_request: AirlockRequest, workspace: Workspace) -> str: tre_id = config.TRE_ID - short_workspace_id = workspace.id[-4:] + unique_suffix = workspace.properties['unique_identifier_suffix'] + if airlock_request.type == constants.IMPORT_TYPE: if airlock_request.status == AirlockRequestStatus.Draft: return constants.STORAGE_ACCOUNT_NAME_IMPORT_EXTERNAL.format(tre_id) @@ -45,24 +46,24 @@ def get_account_by_request(airlock_request: AirlockRequest, workspace: Workspace elif airlock_request.status == AirlockRequestStatus.InReview: return constants.STORAGE_ACCOUNT_NAME_IMPORT_INPROGRESS.format(tre_id) elif airlock_request.status == AirlockRequestStatus.Approved: - return constants.STORAGE_ACCOUNT_NAME_IMPORT_APPROVED.format(short_workspace_id) + return constants.STORAGE_ACCOUNT_NAME_IMPORT_APPROVED.format(unique_suffix) elif airlock_request.status == AirlockRequestStatus.Rejected: return constants.STORAGE_ACCOUNT_NAME_IMPORT_REJECTED.format(tre_id) elif airlock_request.status == AirlockRequestStatus.Blocked: return constants.STORAGE_ACCOUNT_NAME_IMPORT_BLOCKED.format(tre_id) else: if airlock_request.status == AirlockRequestStatus.Draft: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_INTERNAL.format(short_workspace_id) + return constants.STORAGE_ACCOUNT_NAME_EXPORT_INTERNAL.format(unique_suffix) elif airlock_request.status in AirlockRequestStatus.Submitted: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS.format(short_workspace_id) + return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS.format(unique_suffix) elif airlock_request.status == AirlockRequestStatus.InReview: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS.format(short_workspace_id) + return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS.format(unique_suffix) elif airlock_request.status == AirlockRequestStatus.Approved: return constants.STORAGE_ACCOUNT_NAME_EXPORT_APPROVED.format(tre_id) elif airlock_request.status == AirlockRequestStatus.Rejected: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_REJECTED.format(short_workspace_id) + return constants.STORAGE_ACCOUNT_NAME_EXPORT_REJECTED.format(unique_suffix) elif airlock_request.status == AirlockRequestStatus.Blocked: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_BLOCKED.format(short_workspace_id) + return constants.STORAGE_ACCOUNT_NAME_EXPORT_BLOCKED.format(unique_suffix) def validate_user_allowed_to_access_storage_account(user: User, airlock_request: AirlockRequest): @@ -278,7 +279,7 @@ async def save_and_publish_event_airlock_request(airlock_request: AirlockRequest try: logging.debug(f"Sending status changed event for airlock request item: {airlock_request.id}") - await send_status_changed_event(airlock_request=airlock_request, previous_status=None) + await send_status_changed_event(airlock_request=airlock_request, unique_identifier_suffix=workspace.properties['unique_identifier_suffix'], previous_status=None) await send_airlock_notification_event(airlock_request, workspace, role_assignment_details) except Exception: await airlock_request_repo.delete_item(airlock_request.id) @@ -320,7 +321,7 @@ async def update_and_publish_event_airlock_request( try: logging.debug(f"Sending status changed event for airlock request item: {airlock_request.id}") - await send_status_changed_event(airlock_request=updated_airlock_request, previous_status=airlock_request.status) + await send_status_changed_event(airlock_request=updated_airlock_request, unique_identifier_suffix=workspace.properties['unique_identifier_suffix'], previous_status=airlock_request.status) access_service = get_access_service() role_assignment_details = access_service.get_workspace_role_assignment_details(workspace) await send_airlock_notification_event(updated_airlock_request, workspace, role_assignment_details) From a6fd24485b42a9deacf50f208e6b8328ec785949 Mon Sep 17 00:00:00 2001 From: Yuval Yaron Date: Wed, 22 Feb 2023 15:19:14 +0000 Subject: [PATCH 2/7] add migration to add unique_identifier_suffix to resources that were created before the field was introduced --- api_app/api/routes/migrations.py | 4 ++++ api_app/db/migrations/resources.py | 9 +++++++++ api_app/tests_ma/test_api/test_routes/test_migrations.py | 9 ++++++--- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/api_app/api/routes/migrations.py b/api_app/api/routes/migrations.py index b434649084..30394967f8 100644 --- a/api_app/api/routes/migrations.py +++ b/api_app/api/routes/migrations.py @@ -79,6 +79,10 @@ async def migrate_database(resources_repo=Depends(get_repository(ResourceReposit num_updated = await airlock_migration.update_review_decision_values() migrations.append(Migration(issueNumber="3152", status=f'Updated {num_updated} airlock requests with new reviewDecision value')) + logging.info("PR - Migrate reviewDecision of Airlock Reviews") + num_updated = await resource_migration.add_unique_identifier_suffix() + migrations.append(Migration(issueNumber="", status=f'Added the unique_identifier_suffix field to {num_updated} resources')) + return MigrationOutList(migrations=migrations) except Exception as e: logging.exception("Failed to migrate database") diff --git a/api_app/db/migrations/resources.py b/api_app/db/migrations/resources.py index 3302c9746e..12f1bd4aee 100644 --- a/api_app/db/migrations/resources.py +++ b/api_app/db/migrations/resources.py @@ -41,3 +41,12 @@ async def archive_history(self, resource_history_repository: ResourceHistoryRepo num_updated = num_updated + 1 return num_updated + + async def add_unique_identifier_suffix(self) -> int: + num_updated = 0 + for resource in await self.query("SELECT * from c WHERE NOT IS_DEFINED(c.properties.unique_identifier_suffix)"): + resource['properties']['unique_identifier_suffix'] = resource['id'][-4:] + await self.update_item_dict(resource) + num_updated = num_updated + 1 + + return num_updated diff --git a/api_app/tests_ma/test_api/test_routes/test_migrations.py b/api_app/tests_ma/test_api/test_routes/test_migrations.py index 88f4dc345d..41831cb100 100644 --- a/api_app/tests_ma/test_api/test_routes/test_migrations.py +++ b/api_app/tests_ma/test_api/test_routes/test_migrations.py @@ -45,9 +45,11 @@ def _prepare(self, app, admin_user): @ patch("api.routes.migrations.AirlockMigration.rename_field_name") @ patch("api.routes.migrations.AirlockMigration.change_review_resources_to_dict") @ patch("api.routes.migrations.AirlockMigration.update_review_decision_values") - async def test_post_migrations_returns_202_on_successful(self, update_review_decision_values, change_review_resources_to_dict, airlock_rename_field, add_created_by_and_rename_in_history, - check_min_firewall_version, workspace_migration, shared_services_migration, - rename_field, add_deployment_field, archive_history, _, logging, client, app): + @ patch("api.routes.migrations.ResourceMigration.add_unique_identifier_suffix") + async def test_post_migrations_returns_202_on_successful(self, add_unique_identifier_suffix, update_review_decision_values, + change_review_resources_to_dict, airlock_rename_field, add_created_by_and_rename_in_history, + check_min_firewall_version, workspace_migration, shared_services_migration, rename_field, + add_deployment_field, archive_history, _, logging, client, app): response = await client.post(app.url_path_for(strings.API_MIGRATE_DATABASE)) check_min_firewall_version.assert_called_once() @@ -60,6 +62,7 @@ async def test_post_migrations_returns_202_on_successful(self, update_review_dec change_review_resources_to_dict.assert_called_once() update_review_decision_values.assert_called_once() archive_history.assert_called_once() + add_unique_identifier_suffix.assert_called_once() logging.assert_called() assert response.status_code == status.HTTP_202_ACCEPTED From 56edcbf18635fc8d8ba084416a0dbd4b93325b1d Mon Sep 17 00:00:00 2001 From: Yuval Yaron Date: Wed, 22 Feb 2023 15:40:04 +0000 Subject: [PATCH 3/7] fix unit tests --- .../tests/test_status_change_queue_trigger.py | 20 +++++++++---------- .../test_api/test_routes/test_airlock.py | 5 ++--- .../test_airlock_request_status_update.py | 1 + .../tests_ma/test_services/test_airlock.py | 3 ++- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/airlock_processor/tests/test_status_change_queue_trigger.py b/airlock_processor/tests/test_status_change_queue_trigger.py index 65bac890a5..745d348443 100644 --- a/airlock_processor/tests/test_status_change_queue_trigger.py +++ b/airlock_processor/tests/test_status_change_queue_trigger.py @@ -11,25 +11,25 @@ class TestPropertiesExtraction(): def test_extract_prop_valid_body_return_all_values(self): - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"456\" ,\"previous_status\":\"789\" , \"type\":\"101112\", \"workspace_id\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"456\" ,\"previous_status\":\"789\" , \"type\":\"101112\", \"unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) req_prop = extract_properties(message) assert req_prop.request_id == "123" assert req_prop.new_status == "456" assert req_prop.previous_status == "789" assert req_prop.type == "101112" - assert req_prop.workspace_id == "ws1" + assert req_prop.unique_identifier_suffix == "ws1" def test_extract_prop_missing_arg_throws(self): - message_body = "{ \"data\": { \"status\":\"456\" , \"type\":\"789\", \"workspace_id\":\"ws1\" }}" + message_body = "{ \"data\": { \"status\":\"456\" , \"type\":\"789\", \"unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) pytest.raises(ValidationError, extract_properties, message) - message_body = "{ \"data\": { \"request_id\":\"123\", \"type\":\"789\", \"workspace_id\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\", \"type\":\"789\", \"unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) pytest.raises(ValidationError, extract_properties, message) - message_body = "{ \"data\": { \"request_id\":\"123\",\"status\":\"456\" , \"workspace_id\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"status\":\"456\" , \"unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) pytest.raises(ValidationError, extract_properties, message) @@ -72,7 +72,7 @@ class TestFileEnumeration(): @patch("StatusChangedQueueTrigger.is_require_data_copy", return_value=False) @patch.dict(os.environ, {"TRE_ID": "tre-id"}, clear=True) def test_get_request_files_should_be_called_on_submit_stage(self, _, mock_get_request_files, mock_set_output_event_to_report_request_files): - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"workspace_id\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) main(msg=message, stepResultEvent=MagicMock(), dataDeletionEvent=MagicMock()) assert mock_get_request_files.called @@ -82,7 +82,7 @@ def test_get_request_files_should_be_called_on_submit_stage(self, _, mock_get_re @patch("StatusChangedQueueTrigger.get_request_files") @patch("StatusChangedQueueTrigger.handle_status_changed") def test_get_request_files_should_not_be_called_if_new_status_is_not_submit(self, _, mock_get_request_files, mock_set_output_event_to_report_failure): - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"fake-status\" ,\"previous_status\":\"None\" , \"type\":\"export\", \"workspace_id\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"fake-status\" ,\"previous_status\":\"None\" , \"type\":\"export\", \"unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) main(msg=message, stepResultEvent=MagicMock(), dataDeletionEvent=MagicMock()) assert not mock_get_request_files.called @@ -92,7 +92,7 @@ def test_get_request_files_should_not_be_called_if_new_status_is_not_submit(self @patch("StatusChangedQueueTrigger.get_request_files") @patch("StatusChangedQueueTrigger.handle_status_changed", side_effect=Exception) def test_get_request_files_should_be_called_when_failing_during_submit_stage(self, _, mock_get_request_files, mock_set_output_event_to_report_failure): - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"workspace_id\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) main(msg=message, stepResultEvent=MagicMock(), dataDeletionEvent=MagicMock()) assert mock_get_request_files.called @@ -102,7 +102,7 @@ def test_get_request_files_should_be_called_when_failing_during_submit_stage(sel @patch.dict(os.environ, {"TRE_ID": "tre-id"}, clear=True) def test_get_request_files_called_with_correct_storage_account(self, mock_get_request_files): source_storage_account_for_submitted_stage = constants.STORAGE_ACCOUNT_NAME_EXPORT_INTERNAL + 'ws1' - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"workspace_id\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) request_properties = extract_properties(message) get_request_files(request_properties) @@ -113,7 +113,7 @@ class TestFilesDeletion(): @patch("StatusChangedQueueTrigger.set_output_event_to_trigger_container_deletion") @patch.dict(os.environ, {"TRE_ID": "tre-id"}, clear=True) def test_delete_request_files_should_be_called_on_cancel_stage(self, mock_set_output_event_to_trigger_container_deletion): - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"cancelled\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"workspace_id\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"cancelled\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) main(msg=message, stepResultEvent=MagicMock(), dataDeletionEvent=MagicMock()) assert mock_set_output_event_to_trigger_container_deletion.called diff --git a/api_app/tests_ma/test_api/test_routes/test_airlock.py b/api_app/tests_ma/test_api/test_routes/test_airlock.py index 592b04fea9..b0ada76904 100644 --- a/api_app/tests_ma/test_api/test_routes/test_airlock.py +++ b/api_app/tests_ma/test_api/test_routes/test_airlock.py @@ -283,8 +283,7 @@ async def test_get_airlock_container_link_cancelled_request_returns_400(self, _, airlock_request_id=AIRLOCK_REQUEST_ID)) assert response.status_code == status.HTTP_400_BAD_REQUEST - @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", - return_value=sample_workspace(WORKSPACE_ID)) + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace(WORKSPACE_ID, {'unique_identifier_suffix': '123456'})) @patch("api.routes.airlock.AirlockRequestRepository.read_item_by_id", return_value=sample_airlock_request_object(status=AirlockRequestStatus.Approved)) @patch("services.airlock.validate_user_allowed_to_access_storage_account") @patch("services.airlock.get_airlock_request_container_sas_token", return_value="valid-sas-token") @@ -429,7 +428,7 @@ def inner(user): @pytest.mark.parametrize("role", (role for role in get_required_roles(endpoint=create_draft_request))) @patch("api.routes.workspaces.OperationRepository.resource_has_deployed_operation") - @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace(WORKSPACE_ID)) + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace(WORKSPACE_ID, {'unique_identifier_suffix': '123456'})) @patch("api.routes.airlock.AirlockRequestRepository.read_item_by_id", return_value=sample_airlock_request_object(status=AirlockRequestStatus.Draft)) @patch("services.airlock.get_airlock_request_container_sas_token", return_value="valid-sas-token") async def test_get_airlock_container_link_is_accessible_to_every_role_that_can_create_request(self, _, __, ___, ____, app, client, log_in_with_user, role): diff --git a/api_app/tests_ma/test_service_bus/test_airlock_request_status_update.py b/api_app/tests_ma/test_service_bus/test_airlock_request_status_update.py index 6ee7ac4214..dd6ab6a923 100644 --- a/api_app/tests_ma/test_service_bus/test_airlock_request_status_update.py +++ b/api_app/tests_ma/test_service_bus/test_airlock_request_status_update.py @@ -26,6 +26,7 @@ def sample_workspace(): properties={ "display_name": "research workspace", "description": "research workspace", + "unique_identifier_suffix": "123456", "client_id": "12345" }, resourcePath="test") diff --git a/api_app/tests_ma/test_services/test_airlock.py b/api_app/tests_ma/test_services/test_airlock.py index 5dee85f719..98c1eef7ef 100644 --- a/api_app/tests_ma/test_services/test_airlock.py +++ b/api_app/tests_ma/test_services/test_airlock.py @@ -45,6 +45,7 @@ def sample_workspace(): properties={ "client_id": "12345", "display_name": "my research workspace", + "unique_identifier_suffix": WORKSPACE_ID[-6:], "description": "for science!"}, resourcePath="test") @@ -86,7 +87,7 @@ def sample_airlock_user_resource_object(): def sample_status_changed_event(new_status="draft", previous_status=None): status_changed_event = EventGridEvent( event_type="statusChanged", - data=StatusChangedData(request_id=AIRLOCK_REQUEST_ID, new_status=new_status, previous_status=previous_status, type=AirlockRequestType.Import, workspace_id=WORKSPACE_ID[-4:]).__dict__, + data=StatusChangedData(request_id=AIRLOCK_REQUEST_ID, new_status=new_status, previous_status=previous_status, type=AirlockRequestType.Import, unique_identifier_suffix=WORKSPACE_ID[-6:]).__dict__, subject=f"{AIRLOCK_REQUEST_ID}/statusChanged", data_version="2.0" ) From 37b67d1bd191d9ac7ea52c5324972fc14a4c7954 Mon Sep 17 00:00:00 2001 From: Yuval Yaron Date: Wed, 22 Feb 2023 17:06:26 +0000 Subject: [PATCH 4/7] update versions --- airlock_processor/StatusChangedQueueTrigger/__init__.py | 2 +- airlock_processor/_version.py | 2 +- api_app/_version.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/airlock_processor/StatusChangedQueueTrigger/__init__.py b/airlock_processor/StatusChangedQueueTrigger/__init__.py index df087f1540..4793243655 100644 --- a/airlock_processor/StatusChangedQueueTrigger/__init__.py +++ b/airlock_processor/StatusChangedQueueTrigger/__init__.py @@ -51,7 +51,7 @@ def handle_status_changed(request_properties: RequestProperties, stepResultEvent unique_suffix = request_properties.unique_identifier_suffix request_type = request_properties.type - logging.info('Processing request with id %s. new status is "%s", type is "%s"', req_id, new_status, request_type) + logging.info(f'Processing request with id {req_id}. new status is {new_status}, previous status is {previous_status}, unique_suffix is {unique_suffix} type is {request_type}') if new_status == constants.STAGE_DRAFT: account_name = get_storage_account(status=constants.STAGE_DRAFT, request_type=request_type, unique_identifier_suffix=unique_suffix) diff --git a/airlock_processor/_version.py b/airlock_processor/_version.py index 4b2ce7df3d..33c949b73b 100644 --- a/airlock_processor/_version.py +++ b/airlock_processor/_version.py @@ -1 +1 @@ -__version__ = "0.4.13" +__version__ = "0.5.0w1832" diff --git a/api_app/_version.py b/api_app/_version.py index 7e0dc0e843..9e78220f94 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.13.1" +__version__ = "0.14.0" From ee1669af709d3e2936d9216839fd6e5cbc785845 Mon Sep 17 00:00:00 2001 From: Yuval Yaron Date: Wed, 22 Feb 2023 17:09:59 +0000 Subject: [PATCH 5/7] fix version --- airlock_processor/_version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airlock_processor/_version.py b/airlock_processor/_version.py index 33c949b73b..3d187266f1 100644 --- a/airlock_processor/_version.py +++ b/airlock_processor/_version.py @@ -1 +1 @@ -__version__ = "0.5.0w1832" +__version__ = "0.5.0" From d6e4caf1a984e00a2f778ee7610c10dec80ca99c Mon Sep 17 00:00:00 2001 From: Yuval Yaron Date: Wed, 22 Feb 2023 17:22:25 +0000 Subject: [PATCH 6/7] update the PR number in migration --- api_app/api/routes/migrations.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api_app/api/routes/migrations.py b/api_app/api/routes/migrations.py index 30394967f8..4ff141fe0d 100644 --- a/api_app/api/routes/migrations.py +++ b/api_app/api/routes/migrations.py @@ -79,9 +79,9 @@ async def migrate_database(resources_repo=Depends(get_repository(ResourceReposit num_updated = await airlock_migration.update_review_decision_values() migrations.append(Migration(issueNumber="3152", status=f'Updated {num_updated} airlock requests with new reviewDecision value')) - logging.info("PR - Migrate reviewDecision of Airlock Reviews") + logging.info("PR 3243 - Migrate reviewDecision of Airlock Reviews") num_updated = await resource_migration.add_unique_identifier_suffix() - migrations.append(Migration(issueNumber="", status=f'Added the unique_identifier_suffix field to {num_updated} resources')) + migrations.append(Migration(issueNumber="3243", status=f'Added the unique_identifier_suffix field to {num_updated} resources')) return MigrationOutList(migrations=migrations) except Exception as e: From 5f57547b125e049ba568d524ef6de079b1dd31d4 Mon Sep 17 00:00:00 2001 From: Yuval Yaron Date: Thu, 23 Feb 2023 08:48:38 +0000 Subject: [PATCH 7/7] rename unique_identifier_suffix to workspace_unique_identifier_suffix where appropriate --- .../StatusChangedQueueTrigger/__init__.py | 38 +++++++++---------- .../tests/test_status_change_queue_trigger.py | 20 +++++----- api_app/event_grid/event_sender.py | 4 +- api_app/models/domain/events.py | 2 +- api_app/services/airlock.py | 4 +- .../tests_ma/test_services/test_airlock.py | 2 +- 6 files changed, 35 insertions(+), 35 deletions(-) diff --git a/airlock_processor/StatusChangedQueueTrigger/__init__.py b/airlock_processor/StatusChangedQueueTrigger/__init__.py index 4793243655..879c0b134b 100644 --- a/airlock_processor/StatusChangedQueueTrigger/__init__.py +++ b/airlock_processor/StatusChangedQueueTrigger/__init__.py @@ -18,7 +18,7 @@ class RequestProperties(BaseModel): new_status: str previous_status: Optional[str] type: str - unique_identifier_suffix: str + workspace_unique_identifier_suffix: str class ContainersCopyMetadata: @@ -48,13 +48,13 @@ def handle_status_changed(request_properties: RequestProperties, stepResultEvent new_status = request_properties.new_status previous_status = request_properties.previous_status req_id = request_properties.request_id - unique_suffix = request_properties.unique_identifier_suffix + unique_suffix = request_properties.workspace_unique_identifier_suffix request_type = request_properties.type logging.info(f'Processing request with id {req_id}. new status is {new_status}, previous status is {previous_status}, unique_suffix is {unique_suffix} type is {request_type}') if new_status == constants.STAGE_DRAFT: - account_name = get_storage_account(status=constants.STAGE_DRAFT, request_type=request_type, unique_identifier_suffix=unique_suffix) + account_name = get_storage_account(status=constants.STAGE_DRAFT, request_type=request_type, workspace_unique_identifier_suffix=unique_suffix) blob_operations.create_container(account_name, req_id) return @@ -69,7 +69,7 @@ def handle_status_changed(request_properties: RequestProperties, stepResultEvent if (is_require_data_copy(new_status)): logging.info('Request with id %s. requires data copy between storage accounts', req_id) - containers_metadata = get_source_dest_for_copy(new_status=new_status, previous_status=previous_status, request_type=request_type, unique_identifier_suffix=unique_suffix) + containers_metadata = get_source_dest_for_copy(new_status=new_status, previous_status=previous_status, request_type=request_type, workspace_unique_identifier_suffix=unique_suffix) blob_operations.create_container(containers_metadata.dest_account_name, req_id) blob_operations.copy_data(containers_metadata.source_account_name, containers_metadata.dest_account_name, req_id) @@ -102,7 +102,7 @@ def is_require_data_copy(new_status: str): return False -def get_source_dest_for_copy(new_status: str, previous_status: str, request_type: str, unique_identifier_suffix: str) -> ContainersCopyMetadata: +def get_source_dest_for_copy(new_status: str, previous_status: str, request_type: str, workspace_unique_identifier_suffix: str) -> ContainersCopyMetadata: # sanity if is_require_data_copy(new_status) is False: raise Exception("Given new status is not supported") @@ -114,19 +114,19 @@ def get_source_dest_for_copy(new_status: str, previous_status: str, request_type logging.error(msg) raise Exception(msg) - source_account_name = get_storage_account(previous_status, request_type, unique_identifier_suffix) - dest_account_name = get_storage_account_destination_for_copy(new_status, request_type, unique_identifier_suffix) + source_account_name = get_storage_account(previous_status, request_type, workspace_unique_identifier_suffix) + dest_account_name = get_storage_account_destination_for_copy(new_status, request_type, workspace_unique_identifier_suffix) return ContainersCopyMetadata(source_account_name, dest_account_name) -def get_storage_account(status: str, request_type: str, unique_identifier_suffix: str) -> str: +def get_storage_account(status: str, request_type: str, workspace_unique_identifier_suffix: str) -> str: tre_id = _get_tre_id() if request_type == constants.IMPORT_TYPE: if status == constants.STAGE_DRAFT: return constants.STORAGE_ACCOUNT_NAME_IMPORT_EXTERNAL + tre_id elif status == constants.STAGE_APPROVED: - return constants.STORAGE_ACCOUNT_NAME_IMPORT_APPROVED + unique_identifier_suffix + return constants.STORAGE_ACCOUNT_NAME_IMPORT_APPROVED + workspace_unique_identifier_suffix elif status == constants.STAGE_REJECTED: return constants.STORAGE_ACCOUNT_NAME_IMPORT_REJECTED + tre_id elif status == constants.STAGE_BLOCKED_BY_SCAN: @@ -136,29 +136,29 @@ def get_storage_account(status: str, request_type: str, unique_identifier_suffix if request_type == constants.EXPORT_TYPE: if status == constants.STAGE_DRAFT: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_INTERNAL + unique_identifier_suffix + return constants.STORAGE_ACCOUNT_NAME_EXPORT_INTERNAL + workspace_unique_identifier_suffix elif status == constants.STAGE_APPROVED: return constants.STORAGE_ACCOUNT_NAME_EXPORT_APPROVED + tre_id elif status == constants.STAGE_REJECTED: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_REJECTED + unique_identifier_suffix + return constants.STORAGE_ACCOUNT_NAME_EXPORT_REJECTED + workspace_unique_identifier_suffix elif status == constants.STAGE_BLOCKED_BY_SCAN: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_BLOCKED + unique_identifier_suffix + return constants.STORAGE_ACCOUNT_NAME_EXPORT_BLOCKED + workspace_unique_identifier_suffix elif status in [constants.STAGE_IN_REVIEW, constants.STAGE_SUBMITTED, constants.STAGE_APPROVAL_INPROGRESS, constants.STAGE_REJECTION_INPROGRESS, constants.STAGE_BLOCKING_INPROGRESS]: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS + unique_identifier_suffix + return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS + workspace_unique_identifier_suffix error_message = f"Missing current storage account definition for status '{status}' and request type '{request_type}'." logging.error(error_message) raise Exception(error_message) -def get_storage_account_destination_for_copy(new_status: str, request_type: str, unique_identifier_suffix: str) -> str: +def get_storage_account_destination_for_copy(new_status: str, request_type: str, workspace_unique_identifier_suffix: str) -> str: tre_id = _get_tre_id() if request_type == constants.IMPORT_TYPE: if new_status == constants.STAGE_SUBMITTED: return constants.STORAGE_ACCOUNT_NAME_IMPORT_INPROGRESS + tre_id elif new_status == constants.STAGE_APPROVAL_INPROGRESS: - return constants.STORAGE_ACCOUNT_NAME_IMPORT_APPROVED + unique_identifier_suffix + return constants.STORAGE_ACCOUNT_NAME_IMPORT_APPROVED + workspace_unique_identifier_suffix elif new_status == constants.STAGE_REJECTION_INPROGRESS: return constants.STORAGE_ACCOUNT_NAME_IMPORT_REJECTED + tre_id elif new_status == constants.STAGE_BLOCKING_INPROGRESS: @@ -166,13 +166,13 @@ def get_storage_account_destination_for_copy(new_status: str, request_type: str, if request_type == constants.EXPORT_TYPE: if new_status == constants.STAGE_SUBMITTED: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS + unique_identifier_suffix + return constants.STORAGE_ACCOUNT_NAME_EXPORT_INPROGRESS + workspace_unique_identifier_suffix elif new_status == constants.STAGE_APPROVAL_INPROGRESS: return constants.STORAGE_ACCOUNT_NAME_EXPORT_APPROVED + tre_id elif new_status == constants.STAGE_REJECTION_INPROGRESS: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_REJECTED + unique_identifier_suffix + return constants.STORAGE_ACCOUNT_NAME_EXPORT_REJECTED + workspace_unique_identifier_suffix elif new_status == constants.STAGE_BLOCKING_INPROGRESS: - return constants.STORAGE_ACCOUNT_NAME_EXPORT_BLOCKED + unique_identifier_suffix + return constants.STORAGE_ACCOUNT_NAME_EXPORT_BLOCKED + workspace_unique_identifier_suffix error_message = f"Missing copy destination storage account definition for status '{new_status}' and request type '{request_type}'." logging.error(error_message) @@ -218,7 +218,7 @@ def set_output_event_to_trigger_container_deletion(dataDeletionEvent, request_pr def get_request_files(request_properties: RequestProperties): - storage_account_name = get_storage_account(request_properties.previous_status, request_properties.type, request_properties.unique_identifier_suffix) + storage_account_name = get_storage_account(request_properties.previous_status, request_properties.type, request_properties.workspace_unique_identifier_suffix) return blob_operations.get_request_files(account_name=storage_account_name, request_id=request_properties.request_id) diff --git a/airlock_processor/tests/test_status_change_queue_trigger.py b/airlock_processor/tests/test_status_change_queue_trigger.py index 745d348443..284010a00f 100644 --- a/airlock_processor/tests/test_status_change_queue_trigger.py +++ b/airlock_processor/tests/test_status_change_queue_trigger.py @@ -11,25 +11,25 @@ class TestPropertiesExtraction(): def test_extract_prop_valid_body_return_all_values(self): - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"456\" ,\"previous_status\":\"789\" , \"type\":\"101112\", \"unique_identifier_suffix\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"456\" ,\"previous_status\":\"789\" , \"type\":\"101112\", \"workspace_unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) req_prop = extract_properties(message) assert req_prop.request_id == "123" assert req_prop.new_status == "456" assert req_prop.previous_status == "789" assert req_prop.type == "101112" - assert req_prop.unique_identifier_suffix == "ws1" + assert req_prop.workspace_unique_identifier_suffix == "ws1" def test_extract_prop_missing_arg_throws(self): - message_body = "{ \"data\": { \"status\":\"456\" , \"type\":\"789\", \"unique_identifier_suffix\":\"ws1\" }}" + message_body = "{ \"data\": { \"status\":\"456\" , \"type\":\"789\", \"workspace_unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) pytest.raises(ValidationError, extract_properties, message) - message_body = "{ \"data\": { \"request_id\":\"123\", \"type\":\"789\", \"unique_identifier_suffix\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\", \"type\":\"789\", \"workspace_unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) pytest.raises(ValidationError, extract_properties, message) - message_body = "{ \"data\": { \"request_id\":\"123\",\"status\":\"456\" , \"unique_identifier_suffix\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"status\":\"456\" , \"workspace_unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) pytest.raises(ValidationError, extract_properties, message) @@ -72,7 +72,7 @@ class TestFileEnumeration(): @patch("StatusChangedQueueTrigger.is_require_data_copy", return_value=False) @patch.dict(os.environ, {"TRE_ID": "tre-id"}, clear=True) def test_get_request_files_should_be_called_on_submit_stage(self, _, mock_get_request_files, mock_set_output_event_to_report_request_files): - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"unique_identifier_suffix\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"workspace_unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) main(msg=message, stepResultEvent=MagicMock(), dataDeletionEvent=MagicMock()) assert mock_get_request_files.called @@ -82,7 +82,7 @@ def test_get_request_files_should_be_called_on_submit_stage(self, _, mock_get_re @patch("StatusChangedQueueTrigger.get_request_files") @patch("StatusChangedQueueTrigger.handle_status_changed") def test_get_request_files_should_not_be_called_if_new_status_is_not_submit(self, _, mock_get_request_files, mock_set_output_event_to_report_failure): - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"fake-status\" ,\"previous_status\":\"None\" , \"type\":\"export\", \"unique_identifier_suffix\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"fake-status\" ,\"previous_status\":\"None\" , \"type\":\"export\", \"workspace_unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) main(msg=message, stepResultEvent=MagicMock(), dataDeletionEvent=MagicMock()) assert not mock_get_request_files.called @@ -92,7 +92,7 @@ def test_get_request_files_should_not_be_called_if_new_status_is_not_submit(self @patch("StatusChangedQueueTrigger.get_request_files") @patch("StatusChangedQueueTrigger.handle_status_changed", side_effect=Exception) def test_get_request_files_should_be_called_when_failing_during_submit_stage(self, _, mock_get_request_files, mock_set_output_event_to_report_failure): - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"unique_identifier_suffix\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"workspace_unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) main(msg=message, stepResultEvent=MagicMock(), dataDeletionEvent=MagicMock()) assert mock_get_request_files.called @@ -102,7 +102,7 @@ def test_get_request_files_should_be_called_when_failing_during_submit_stage(sel @patch.dict(os.environ, {"TRE_ID": "tre-id"}, clear=True) def test_get_request_files_called_with_correct_storage_account(self, mock_get_request_files): source_storage_account_for_submitted_stage = constants.STORAGE_ACCOUNT_NAME_EXPORT_INTERNAL + 'ws1' - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"unique_identifier_suffix\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"submitted\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"workspace_unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) request_properties = extract_properties(message) get_request_files(request_properties) @@ -113,7 +113,7 @@ class TestFilesDeletion(): @patch("StatusChangedQueueTrigger.set_output_event_to_trigger_container_deletion") @patch.dict(os.environ, {"TRE_ID": "tre-id"}, clear=True) def test_delete_request_files_should_be_called_on_cancel_stage(self, mock_set_output_event_to_trigger_container_deletion): - message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"cancelled\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"unique_identifier_suffix\":\"ws1\" }}" + message_body = "{ \"data\": { \"request_id\":\"123\",\"new_status\":\"cancelled\" ,\"previous_status\":\"draft\" , \"type\":\"export\", \"workspace_unique_identifier_suffix\":\"ws1\" }}" message = _mock_service_bus_message(body=message_body) main(msg=message, stepResultEvent=MagicMock(), dataDeletionEvent=MagicMock()) assert mock_set_output_event_to_trigger_container_deletion.called diff --git a/api_app/event_grid/event_sender.py b/api_app/event_grid/event_sender.py index dc1214da34..d46cddb2cf 100644 --- a/api_app/event_grid/event_sender.py +++ b/api_app/event_grid/event_sender.py @@ -11,7 +11,7 @@ from models.domain.workspace import Workspace -async def send_status_changed_event(airlock_request: AirlockRequest, unique_identifier_suffix: str, previous_status: Optional[AirlockRequestStatus]): +async def send_status_changed_event(airlock_request: AirlockRequest, workspace_unique_identifier_suffix: str, previous_status: Optional[AirlockRequestStatus]): request_id = airlock_request.id new_status = airlock_request.status.value previous_status = previous_status.value if previous_status else None @@ -19,7 +19,7 @@ async def send_status_changed_event(airlock_request: AirlockRequest, unique_iden status_changed_event = EventGridEvent( event_type="statusChanged", - data=StatusChangedData(request_id=request_id, new_status=new_status, previous_status=previous_status, type=request_type, unique_identifier_suffix=unique_identifier_suffix).__dict__, + data=StatusChangedData(request_id=request_id, new_status=new_status, previous_status=previous_status, type=request_type, workspace_unique_identifier_suffix=workspace_unique_identifier_suffix).__dict__, subject=f"{request_id}/statusChanged", data_version="2.0" ) diff --git a/api_app/models/domain/events.py b/api_app/models/domain/events.py index 4fe64ebd2d..4b0690abe6 100644 --- a/api_app/models/domain/events.py +++ b/api_app/models/domain/events.py @@ -39,4 +39,4 @@ class StatusChangedData(AzureTREModel): new_status: str previous_status: Optional[str] type: str - unique_identifier_suffix: str + workspace_unique_identifier_suffix: str diff --git a/api_app/services/airlock.py b/api_app/services/airlock.py index 24f0c1b574..bad650c5f5 100644 --- a/api_app/services/airlock.py +++ b/api_app/services/airlock.py @@ -279,7 +279,7 @@ async def save_and_publish_event_airlock_request(airlock_request: AirlockRequest try: logging.debug(f"Sending status changed event for airlock request item: {airlock_request.id}") - await send_status_changed_event(airlock_request=airlock_request, unique_identifier_suffix=workspace.properties['unique_identifier_suffix'], previous_status=None) + await send_status_changed_event(airlock_request=airlock_request, workspace_unique_identifier_suffix=workspace.properties['unique_identifier_suffix'], previous_status=None) await send_airlock_notification_event(airlock_request, workspace, role_assignment_details) except Exception: await airlock_request_repo.delete_item(airlock_request.id) @@ -321,7 +321,7 @@ async def update_and_publish_event_airlock_request( try: logging.debug(f"Sending status changed event for airlock request item: {airlock_request.id}") - await send_status_changed_event(airlock_request=updated_airlock_request, unique_identifier_suffix=workspace.properties['unique_identifier_suffix'], previous_status=airlock_request.status) + await send_status_changed_event(airlock_request=updated_airlock_request, workspace_unique_identifier_suffix=workspace.properties['unique_identifier_suffix'], previous_status=airlock_request.status) access_service = get_access_service() role_assignment_details = access_service.get_workspace_role_assignment_details(workspace) await send_airlock_notification_event(updated_airlock_request, workspace, role_assignment_details) diff --git a/api_app/tests_ma/test_services/test_airlock.py b/api_app/tests_ma/test_services/test_airlock.py index 98c1eef7ef..16e0882fbe 100644 --- a/api_app/tests_ma/test_services/test_airlock.py +++ b/api_app/tests_ma/test_services/test_airlock.py @@ -87,7 +87,7 @@ def sample_airlock_user_resource_object(): def sample_status_changed_event(new_status="draft", previous_status=None): status_changed_event = EventGridEvent( event_type="statusChanged", - data=StatusChangedData(request_id=AIRLOCK_REQUEST_ID, new_status=new_status, previous_status=previous_status, type=AirlockRequestType.Import, unique_identifier_suffix=WORKSPACE_ID[-6:]).__dict__, + data=StatusChangedData(request_id=AIRLOCK_REQUEST_ID, new_status=new_status, previous_status=previous_status, type=AirlockRequestType.Import, workspace_unique_identifier_suffix=WORKSPACE_ID[-6:]).__dict__, subject=f"{AIRLOCK_REQUEST_ID}/statusChanged", data_version="2.0" )