From febe57924e9b104fce904d464da453f6ee1ce6eb Mon Sep 17 00:00:00 2001 From: zhiren Date: Thu, 9 Nov 2023 17:39:35 -0500 Subject: [PATCH 1/8] remove the unnecessary item type in search function --- app/commands/file.py | 2 +- app/services/file_manager/file_list.py | 2 +- app/services/file_manager/file_upload/file_upload.py | 4 ++-- app/services/file_manager/file_upload/upload_validator.py | 2 +- app/utils/aggregated.py | 3 +-- tests/app/utils/test_aggregated.py | 8 ++++---- 6 files changed, 10 insertions(+), 11 deletions(-) diff --git a/app/commands/file.py b/app/commands/file.py index 58871f25..c0a8f590 100644 --- a/app/commands/file.py +++ b/app/commands/file.py @@ -399,7 +399,7 @@ def file_download(**kwargs): for path in paths: project_code = path.strip('/').split('/')[0] target_path = '/'.join(path.split('/')[1::]) - item = search_item(project_code, zone, target_path, '') + item = search_item(project_code, zone, target_path) if item.get('code') == 200 and item.get('result'): item_status = 'success' item_result = item.get('result') diff --git a/app/services/file_manager/file_list.py b/app/services/file_manager/file_list.py index 7c178e5b..bebc0e4d 100644 --- a/app/services/file_manager/file_list.py +++ b/app/services/file_manager/file_list.py @@ -29,7 +29,7 @@ def list_files(self, paths, zone, page, page_size): source_type = 'project' else: source_type = 'project' - res = search_item(project_code, zone, folder_rel_path, 'folder') + res = search_item(project_code, zone, folder_rel_path) get_url = AppConfig.Connections.url_bff + f'/v1/{project_code}/files/query' headers = { 'Authorization': 'Bearer ' + self.user.access_token, diff --git a/app/services/file_manager/file_upload/file_upload.py b/app/services/file_manager/file_upload/file_upload.py index d663eee0..a647dbac 100644 --- a/app/services/file_manager/file_upload/file_upload.py +++ b/app/services/file_manager/file_upload/file_upload.py @@ -77,7 +77,7 @@ def assemble_path( # set name folder as first parent folder name_folder = target_folder.split('/')[0] - parent_folder = search_item(project_code, zone, name_folder, 'name_folder') + parent_folder = search_item(project_code, zone, name_folder) parent_folder = parent_folder.get('result') # if f input is a file then current_folder_node is target_folder @@ -88,7 +88,7 @@ def assemble_path( sub_path = target_folder.split('/') for index in range(len(sub_path) - 1): folder_path = '/'.join(sub_path[0 : 2 + index]) - res = search_item(project_code, zone, folder_path, 'folder') + res = search_item(project_code, zone, folder_path) # find the longest existing folder as parent folder # if user input a path that need to create some folders diff --git a/app/services/file_manager/file_upload/upload_validator.py b/app/services/file_manager/file_upload/upload_validator.py index 3e48dcde..d2ed75a2 100644 --- a/app/services/file_manager/file_upload/upload_validator.py +++ b/app/services/file_manager/file_upload/upload_validator.py @@ -29,7 +29,7 @@ def validate_zone(self): ECustomizedError.INVALID_UPLOAD_REQUEST, True, value='upload-message is required' ) if self.source: - source_file_info = search_item(self.project_code, AppConfig.Env.core_zone.lower(), self.source, 'file') + source_file_info = search_item(self.project_code, AppConfig.Env.core_zone.lower(), self.source) source_file_info = source_file_info['result'] if not source_file_info: SrvErrorHandler.customized_handle(ECustomizedError.INVALID_SOURCE_FILE, True, value=self.source) diff --git a/app/utils/aggregated.py b/app/utils/aggregated.py index 725cb753..44c507ee 100644 --- a/app/utils/aggregated.py +++ b/app/utils/aggregated.py @@ -27,14 +27,13 @@ def resilient_session(): @require_valid_token() -def search_item(project_code, zone, folder_relative_path, item_type, container_type='project'): +def search_item(project_code, zone, folder_relative_path, container_type='project'): token = UserConfig().access_token url = AppConfig.Connections.url_bff + '/v1/project/{}/search'.format(project_code) params = { 'zone': zone, 'project_code': project_code, 'path': folder_relative_path, - 'item_type': item_type, 'container_type': container_type, } headers = {'Authorization': 'Bearer ' + token} diff --git a/tests/app/utils/test_aggregated.py b/tests/app/utils/test_aggregated.py index 9ddadd2f..e648ad27 100644 --- a/tests/app/utils/test_aggregated.py +++ b/tests/app/utils/test_aggregated.py @@ -55,7 +55,7 @@ def test_search_file_should_return_200(requests_mock, mocker): 'storage': {'id': 'storage-id', 'location_uri': 'minio-path', 'version': 'version-id'}, 'extended': {'id': 'extended-id', 'extra': {'tags': [], 'system_tags': [], 'attributes': {}}}, } - res = search_item(test_project_code, 'zone', 'folder_relative_path', 'file', 'project') + res = search_item(test_project_code, 'zone', 'folder_relative_path', 'project') assert res['result'] == expected_result @@ -68,7 +68,7 @@ def test_search_item_returns_response_when_status_code_is_404(requests_mock, moc status_code=404, ) - response = search_item(test_project_code, 'zone', 'folder_relative_path', 'file', 'project') + response = search_item(test_project_code, 'zone', 'folder_relative_path', 'project') assert response == expected_response @@ -81,7 +81,7 @@ def test_search_file_error_handling_with_403(requests_mock, mocker, capsys): status_code=403, ) with pytest.raises(SystemExit): - search_item(test_project_code, 'zone', 'folder_relative_path', 'file', 'project') + search_item(test_project_code, 'zone', 'folder_relative_path', 'project') out, _ = capsys.readouterr() assert ( out.rstrip() @@ -97,6 +97,6 @@ def test_search_file_error_handling_with_401(requests_mock, mocker, capsys): status_code=401, ) with pytest.raises(SystemExit): - search_item(test_project_code, 'zone', 'folder_relative_path', 'file', 'project') + search_item(test_project_code, 'zone', 'folder_relative_path', 'project') out, _ = capsys.readouterr() assert out.rstrip() == 'Authentication failed.' From 4ef20e5e67097ee396c4fbcee62da3fcdcbfb0e5 Mon Sep 17 00:00:00 2001 From: zhiren Date: Tue, 5 Dec 2023 09:03:01 -0500 Subject: [PATCH 2/8] testing --- app/commands/file.py | 2 ++ app/configs/config.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/commands/file.py b/app/commands/file.py index c0a8f590..5463e5e9 100644 --- a/app/commands/file.py +++ b/app/commands/file.py @@ -181,6 +181,8 @@ def file_put(**kwargs): # noqa: C901 zipping, ) + raise Exception('Not Implemented') + upload_event = { 'project_code': project_code, 'target_folder': target_folder, diff --git a/app/configs/config.py b/app/configs/config.py index 7b61c438..e468a2eb 100644 --- a/app/configs/config.py +++ b/app/configs/config.py @@ -39,7 +39,8 @@ def base_url(self) -> str: @computed_field def url_bff(self) -> str: - return f'{self.base_url}/cli' + # return f'{self.base_url}/cli' + return 'http://localhost:5080' @computed_field def url_keycloak_realm(self) -> str: From 6daf9fbfa9b2717ab516d9fd13c35110e7e5f0f8 Mon Sep 17 00:00:00 2001 From: zhiren Date: Thu, 7 Dec 2023 16:46:44 -0500 Subject: [PATCH 3/8] update logic for project folder --- app/commands/file.py | 4 +--- app/services/file_manager/file_list.py | 13 ++++++++++--- .../file_manager/file_upload/file_upload.py | 17 +++++++++-------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/app/commands/file.py b/app/commands/file.py index 5463e5e9..2dcc498b 100644 --- a/app/commands/file.py +++ b/app/commands/file.py @@ -173,7 +173,7 @@ def file_put(**kwargs): # noqa: C901 # and process them one by one for f in paths: # so this function will always return the furthest folder node as current_folder_node+parent_folder_id - current_folder_node, parent_folder, create_folder_flag, result_file = assemble_path( + current_folder_node, parent_folder, create_folder_flag, target_folder = assemble_path( f, target_folder, project_code, @@ -181,8 +181,6 @@ def file_put(**kwargs): # noqa: C901 zipping, ) - raise Exception('Not Implemented') - upload_event = { 'project_code': project_code, 'target_folder': target_folder, diff --git a/app/services/file_manager/file_list.py b/app/services/file_manager/file_list.py index bebc0e4d..705bcd1d 100644 --- a/app/services/file_manager/file_list.py +++ b/app/services/file_manager/file_list.py @@ -30,6 +30,12 @@ def list_files(self, paths, zone, page, page_size): else: source_type = 'project' res = search_item(project_code, zone, folder_rel_path) + parent_folder = res.get('result') + # if the target folder is project folder add the default path + if parent_folder.get('type') == 'project_folder': + folder_rel_path = 'shared/' + folder_rel_path + + # now query the backend to get the file list get_url = AppConfig.Connections.url_bff + f'/v1/{project_code}/files/query' headers = { 'Authorization': 'Bearer ' + self.user.access_token, @@ -49,12 +55,13 @@ def list_files(self, paths, zone, page, page_size): elif res_json.get('error_msg') == 'Folder not exist': SrvErrorHandler.customized_handle(ECustomizedError.INVALID_FOLDER, True) res = res_json.get('result') - files = '' - folders = '' + + # then format the console output + files, folders = '', '' for f in res: if 'file' == f.get('type'): files = files + f.get('name') + ' ...' - elif f.get('type') in ['folder', 'name_folder']: + elif f.get('type') in ['folder', 'name_folder', 'project_folder']: folders = folders + f"\033[34m{f.get('name')}\033[0m ..." f_string = folders + files return f_string diff --git a/app/services/file_manager/file_upload/file_upload.py b/app/services/file_manager/file_upload/file_upload.py index a647dbac..7252bb6d 100644 --- a/app/services/file_manager/file_upload/file_upload.py +++ b/app/services/file_manager/file_upload/file_upload.py @@ -66,24 +66,25 @@ def assemble_path( - current_file_path: the format file path on platform - parent_folder: the item information of longest parent folder - create_folder_flag: the flag to indicate if need to create new folder - - result_file: the result file if zipping + - target_folder: result object path on platform ''' current_file_path = target_folder + '/' + f.rstrip('/').split('/')[-1] - result_file = current_file_path - if zipping: - result_file = result_file + '.zip' - # set name folder as first parent folder name_folder = target_folder.split('/')[0] - parent_folder = search_item(project_code, zone, name_folder) - parent_folder = parent_folder.get('result') + parent_folder = search_item(project_code, zone, name_folder).get('result', {}) # if f input is a file then current_folder_node is target_folder # otherwise it is target_folder + f input name current_folder_node = target_folder if os.path.isfile(f) else current_file_path create_folder_flag = False + # always add `shared/` as prefix to folder/file if + # they directly under the project root folder + if parent_folder.get('type') == 'project_folder': + current_folder_node = 'shared/' + current_folder_node + target_folder = 'shared/' + target_folder + if len(current_file_path.split('/')) > 2: sub_path = target_folder.split('/') for index in range(len(sub_path) - 1): @@ -105,7 +106,7 @@ def assemble_path( if not parent_folder: SrvErrorHandler.customized_handle(ECustomizedError.PERMISSION_DENIED, True) - return current_folder_node, parent_folder, create_folder_flag, result_file + return current_folder_node, parent_folder, create_folder_flag, target_folder def simple_upload( # noqa: C901 From 97c1033e1f49f446e0ef43670318065814ab3175 Mon Sep 17 00:00:00 2001 From: zhiren Date: Thu, 7 Dec 2023 17:08:57 -0500 Subject: [PATCH 4/8] add the test case for project folder in list/upload api --- tests/app/commands/test_file.py | 64 +++++++++++++++++-- .../file_upload/test_file_upload.py | 34 ++++++++++ 2 files changed, 93 insertions(+), 5 deletions(-) diff --git a/tests/app/commands/test_file.py b/tests/app/commands/test_file.py index 0e01a49b..82dae12b 100644 --- a/tests/app/commands/test_file.py +++ b/tests/app/commands/test_file.py @@ -1,8 +1,8 @@ # Copyright (C) 2022-2023 Indoc Systems # # Contact Indoc Systems for any questions regarding the use of this source code. - import click +import pytest import questionary from app.commands.file import file_list @@ -87,19 +87,30 @@ def test_resumable_upload_command_failed_with_file_not_exists(mocker, cli_runner assert result.output == customized_error_msg(ECustomizedError.INVALID_RESUMABLE) + '\n' -def test_file_list_with_pagination(requests_mock, mocker, cli_runner): +def test_file_list_with_pagination_with_folder_success(requests_mock, mocker, cli_runner): mocker.patch( 'app.services.user_authentication.token_manager.SrvTokenManager.decode_access_token', return_value=decoded_token(), ) - mocker.patch('app.services.file_manager.file_list.search_item', return_value=None) + mocker.patch( + 'app.services.file_manager.file_list.search_item', + return_value={ + 'result': { + 'type': 'folder', + 'id': 'id', + } + }, + ) requests_mock.get( 'http://bff_cli' + '/v1/testproject/files/query', json={ 'code': 200, 'error_msg': '', - 'result': [{'type': 'file', 'name': 'file1'}, {'type': 'file', 'name': 'file2'}], + 'result': [ + {'type': 'file', 'name': 'file1'}, + {'type': 'file', 'name': 'file2'}, + ], }, ) mocker.patch.object(questionary, 'select') @@ -109,13 +120,56 @@ def test_file_list_with_pagination(requests_mock, mocker, cli_runner): assert outputs[0] == 'file1 file2 ' +@pytest.mark.parametrize('parent_folder_type', ['name_folder', 'project_folder']) +def test_file_list_with_pagination_with_name_project_folder(requests_mock, mocker, cli_runner, parent_folder_type): + mocker.patch( + 'app.services.user_authentication.token_manager.SrvTokenManager.decode_access_token', + return_value=decoded_token(), + ) + + mocker.patch( + 'app.services.file_manager.file_list.search_item', + return_value={ + 'result': { + 'type': parent_folder_type, + 'id': 'id', + } + }, + ) + requests_mock.get( + 'http://bff_cli' + '/v1/testproject/files/query', + json={ + 'code': 200, + 'error_msg': '', + 'result': [ + {'type': 'folder', 'name': 'folder1'}, + {'type': 'name_folder', 'name': 'name_folder1'}, + {'type': 'project_folder', 'name': 'project_folder1'}, + ], + }, + ) + mocker.patch.object(questionary, 'select') + questionary.select.return_value.ask.return_value = 'exit' + result = cli_runner.invoke(file_list, ['testproject/admin', '-z', 'greenroom']) + outputs = result.output.split('\n') + assert outputs[0] == 'folder1 name_folder1 project_folder1 ' + + def test_empty_file_list_with_pagination(requests_mock, mocker, cli_runner): mocker.patch( 'app.services.user_authentication.token_manager.SrvTokenManager.decode_access_token', return_value=decoded_token(), ) - mocker.patch('app.services.file_manager.file_list.search_item', return_value=None) + mocker.patch( + 'app.services.file_manager.file_list.search_item', + return_value={ + 'result': { + 'type': 'folder', + 'id': 'id', + } + }, + ) requests_mock.get( 'http://bff_cli' + '/v1/testproject/files/query', json={'code': 200, 'error_msg': '', 'result': []}, diff --git a/tests/app/services/file_manager/file_upload/test_file_upload.py b/tests/app/services/file_manager/file_upload/test_file_upload.py index c8eddaa4..cff579fa 100644 --- a/tests/app/services/file_manager/file_upload/test_file_upload.py +++ b/tests/app/services/file_manager/file_upload/test_file_upload.py @@ -28,6 +28,7 @@ def test_assemble_path_at_name_folder(mocker): 'parent_path': '', 'name': 'admin', 'zone': 0, + 'type': 'name_folder', } }, ) @@ -55,6 +56,7 @@ def test_assemble_path_at_exsting_folder(mocker): 'parent_path': '', 'name': 'admin', 'zone': 0, + 'type': 'folder', } }, { @@ -64,6 +66,7 @@ def test_assemble_path_at_exsting_folder(mocker): 'parent_path': 'admin', 'name': 'test_folder_exist', 'zone': 0, + 'type': 'folder', } }, ] @@ -93,6 +96,7 @@ def test_assemble_path_at_non_existing_folder(mocker): 'parent_path': '', 'name': 'admin', 'zone': 0, + 'type': 'folder', } }, {'result': {}}, @@ -109,6 +113,36 @@ def test_assemble_path_at_non_existing_folder(mocker): assert create_folder_flag is True +def test_assemble_path_at_project_folder(mocker): + local_file_path = './test/file.txt' + target_folder = 'project_folder' + project_code = 'test_project' + zone = 0 + resumable_id = None + + mocker.patch( + 'app.services.file_manager.file_upload.file_upload.search_item', + return_value={ + 'result': { + 'id': 'test', + 'parent_id': 'test_parent', + 'parent_path': '', + 'name': 'project_folder', + 'zone': 0, + 'type': 'project_folder', + } + }, + ) + + current_file_path, parent_folder, create_folder_flag, target_folder = assemble_path( + local_file_path, target_folder, project_code, zone, resumable_id + ) + assert current_file_path == 'shared/project_folder/file.txt' + assert parent_folder.get('name') == 'project_folder' + assert target_folder == 'shared/project_folder' + assert create_folder_flag is False + + def test_file_upload_skip_empty_file(mocker, tmp_path, capfd): file_name = 'test' upload_event = { From 17f719a2397bf0c172cdcc210173c2faa643fa0e Mon Sep 17 00:00:00 2001 From: zhiren Date: Fri, 8 Dec 2023 11:24:19 -0500 Subject: [PATCH 5/8] update download logic for project folder --- app/commands/file.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/commands/file.py b/app/commands/file.py index 2dcc498b..da5583eb 100644 --- a/app/commands/file.py +++ b/app/commands/file.py @@ -397,17 +397,18 @@ def file_download(**kwargs): else: item_res = [] for path in paths: - project_code = path.strip('/').split('/')[0] + project_code, root_folder = path.strip('/').split('/')[:2] target_path = '/'.join(path.split('/')[1::]) + # search the root to check for name folder or project folder + root_item = search_item(project_code, zone, root_folder).get('result', {}) + target_path = 'shared/' + target_path if root_item.get('type') == 'project_folder' else target_path + + # search the target item and download to local item = search_item(project_code, zone, target_path) if item.get('code') == 200 and item.get('result'): item_status = 'success' item_result = item.get('result') item_geid = item.get('result').get('id') - elif item.get('code') == 403 and item.get('error_msg'): - item_status = item.get('error_msg') - item_result = {} - item_geid = path else: item_status = 'File Not Exist' item_result = {} From c944b772abfc7097a8b4aad2ba6a13d12ae2a791 Mon Sep 17 00:00:00 2001 From: zhiren Date: Fri, 8 Dec 2023 11:56:04 -0500 Subject: [PATCH 6/8] add the more test case for file download --- tests/app/commands/test_file.py | 44 +++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/app/commands/test_file.py b/tests/app/commands/test_file.py index 82dae12b..66b5d819 100644 --- a/tests/app/commands/test_file.py +++ b/tests/app/commands/test_file.py @@ -5,6 +5,7 @@ import pytest import questionary +from app.commands.file import file_download from app.commands.file import file_list from app.commands.file import file_put from app.commands.file import file_resume @@ -179,3 +180,46 @@ def test_empty_file_list_with_pagination(requests_mock, mocker, cli_runner): result = cli_runner.invoke(file_list, ['testproject/admin', '-z', 'greenroom']) outputs = result.output.split('\n') assert outputs[0] == ' ' + + +@pytest.mark.parametrize('parent_folder_type', ['name_folder', 'project_folder']) +def test_file_download_success(requests_mock, mocker, cli_runner, parent_folder_type): + mocker.patch( + 'app.services.user_authentication.token_manager.SrvTokenManager.decode_access_token', + return_value=decoded_token(), + ) + + search_mock = mocker.patch( + 'app.commands.file.search_item', + side_effect=[ + { + 'code': 200, + 'result': { + 'type': parent_folder_type, + 'name': 'test', + 'id': 'id', + }, + }, + { + 'code': 200, + 'result': { + 'type': 'file', + 'id': 'id', + }, + }, + ], + ) + + download_mock = mocker.patch( + 'app.services.file_manager.file_download.download_client.SrvFileDownload.simple_download_file', + return_value=None, + ) + + project_code, target_folder = 'testproject', 'test/test.txt' + result = cli_runner.invoke(file_download, [f'{project_code}/{target_folder}', './']) + outputs = result.output.split('\n') + assert outputs[0] == '' + + except_target_folder = 'test/test.txt' if parent_folder_type == 'name_folder' else 'shared/test/test.txt' + search_mock.assert_called_with(project_code, 'greenroom', except_target_folder) + download_mock.assert_called_once() From aed850229235f0de4080b4c1c38f4f3313242324 Mon Sep 17 00:00:00 2001 From: zhiren Date: Fri, 8 Dec 2023 12:05:33 -0500 Subject: [PATCH 7/8] use correct default config --- app/configs/config.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/configs/config.py b/app/configs/config.py index 31f73cd5..e4a36b4d 100644 --- a/app/configs/config.py +++ b/app/configs/config.py @@ -39,8 +39,7 @@ def base_url(self) -> str: @computed_field def url_bff(self) -> str: - # return f'{self.base_url}/cli' - return 'http://localhost:5080' + return f'{self.base_url}/cli' @computed_field def url_portal(self) -> str: From 3844e4db9d7f90f50af7236c3aeb204640902746 Mon Sep 17 00:00:00 2001 From: zhiren Date: Mon, 11 Dec 2023 17:02:04 -0500 Subject: [PATCH 8/8] bumup version --- app/configs/config.py | 5 +---- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/app/configs/config.py b/app/configs/config.py index e4a36b4d..04bbf98c 100644 --- a/app/configs/config.py +++ b/app/configs/config.py @@ -17,10 +17,7 @@ class Settings(BaseSettings): project: str = 'pilot' app_name: str = 'pilotcli' - @computed_field - def config_path(self) -> str: - return str(Path.home() / f'.{self.app_name}') - + config_path: str = str(Path.home() / f'.{app_name}') config_file: str = 'config.ini' keycloak_device_client_id: str = 'cli' diff --git a/pyproject.toml b/pyproject.toml index 85770675..9154869f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "app" -version = "2.9.2" +version = "2.9.3" description = "This service is designed to support pilot platform" authors = ["Indoc Systems"]