From 21294888bf3bc7727ce36889d2f04e3ebe1c8687 Mon Sep 17 00:00:00 2001 From: Color Zhan Date: Thu, 7 Mar 2024 11:34:52 -0500 Subject: [PATCH 1/5] Mb/staging release 20240307 (#131) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * PILOT-3962: Port over changes from 2.7.3 to 2.7.4 * prepare the release branch * use correct version * bumup version * bump up to correct version * Hotfix20240228: hotfix to staging (#129) * update folder name restriction to 100 char * add error handling when uploading with wrong format of tag/attribute files * fixup the metadata download command cannt download item metadata/tags/attributes from project folder * bumpup version --------- Co-authored-by: zhiren * PILOT-4745: fixup the move command fails when creating a new folder under project folder (#130) * fixup the move command fails when creating a new folder under project folder * bumpup the version --------- Co-authored-by: zhiren * bumpup to next preminor --------- Co-authored-by: Daniel Co-authored-by: zhiren Co-authored-by: Dušan Andrić --- app/commands/file.py | 14 +++- app/resources/custom_error.py | 3 +- .../file_metadata/file_metadata_client.py | 10 ++- .../file_move/file_move_client.py | 6 ++ app/services/output_manager/error_handler.py | 1 + app/utils/aggregated.py | 2 +- pyproject.toml | 2 +- tests/app/commands/test_file.py | 29 ++++++++ .../test_file_metadata_client.py | 73 +++++++++++++++++++ .../file_move/test_file_move_client.py | 15 ++++ tests/app/utils/test_aggregated.py | 7 ++ 11 files changed, 154 insertions(+), 8 deletions(-) diff --git a/app/commands/file.py b/app/commands/file.py index f38b0f13..b3590f3d 100644 --- a/app/commands/file.py +++ b/app/commands/file.py @@ -132,10 +132,16 @@ def file_put(**kwargs): # noqa: C901 output_path = kwargs.get('output_path') # load tag json file to list, and attribute file to dict - tag = [] - for t_f in tag_files: - tag.extend(json.load(t_f)) - attribute = json.load(attribute_file) if attribute_file else None + try: + tag = [] + for t_f in tag_files: + tag.extend(json.load(t_f)) + except Exception: + SrvErrorHandler.customized_handle(ECustomizedError.INVALID_TAG_FILE, True) + try: + attribute = json.load(attribute_file) if attribute_file else None + except Exception: + SrvErrorHandler.customized_handle(ECustomizedError.INVALID_TEMPLATE, True) # Check zone and upload-message zone = get_zone(zone) if zone else AppConfig.Env.green_zone.lower() diff --git a/app/resources/custom_error.py b/app/resources/custom_error.py index 8f862a6b..893d676a 100644 --- a/app/resources/custom_error.py +++ b/app/resources/custom_error.py @@ -23,6 +23,7 @@ class Error: "Attribute validation failed. Please ensure mandatory attribute '%s' have value and try again." ), 'INVALID_TEMPLATE': 'Attribute validation failed. Please correct JSON format and try again.', + 'INVALID_TAG_FILE': 'Tag files validation failed. Please correct JSON format and try again.', 'LIMIT_TAG_ERROR': 'Tag limit has been reached. A maximum of 10 tags are allowed per file.', 'INVALID_TAG_ERROR': ( 'Invalid tag format. Tags must be between 1 and 32 characters long ' @@ -45,7 +46,7 @@ class Error: 'INVALID_FOLDERNAME': ( 'The input folder name is not valid. Please follow the rule:\n' ' - cannot contains special characters.\n' - ' - the length should be smaller than 20 characters.' + ' - the length should be smaller than or equal to 100 characters.' ), 'INVALID_TOKEN': 'Your login session has expired. Please try again or log in again.', 'PERMISSION_DENIED': ( diff --git a/app/services/file_manager/file_metadata/file_metadata_client.py b/app/services/file_manager/file_metadata/file_metadata_client.py index 8fdbd77f..ad3833a3 100644 --- a/app/services/file_manager/file_metadata/file_metadata_client.py +++ b/app/services/file_manager/file_metadata/file_metadata_client.py @@ -109,7 +109,15 @@ def download_file_metadata(self) -> List[Dict[str, Any]]: """ project_code, object_path = self.file_path.split('/', 1) - item_res = search_item(project_code, self.zone, object_path).get('result', {}) + item_res = search_item(project_code, self.zone, object_path) + # double check if the file is in shared folder + if item_res.get('code') == 404: + item_res = search_item(project_code, self.zone, f'shared/{object_path}') + if item_res.get('code') == 404: + logger.error(f'Cannot find item {self.file_path} at {self.zone}.') + exit(1) + + item_res = item_res.get('result', {}) extra_info = item_res.pop('extended', {}).get('extra') tags = extra_info.get('tags', []) attributes = extra_info.get('attributes', {}) diff --git a/app/services/file_manager/file_move/file_move_client.py b/app/services/file_manager/file_move/file_move_client.py index 41833891..17478598 100644 --- a/app/services/file_manager/file_move/file_move_client.py +++ b/app/services/file_manager/file_move/file_move_client.py @@ -60,6 +60,12 @@ def create_object_path_if_not_exist(self, folder_path: str) -> dict: """ path_list = folder_path.split('/') + # first get the root folder to check if it is name folder + # or project folder + root_item = search_item(self.project_code, self.zone, path_list[0]).get('result') + if root_item.get('type') == 'project_folder': + path_list[0] = '/'.join([root_item.get('parent_path'), path_list[0]]) + # first check every folder in path exist or not # the loop start with index 1 since we assume cli will not # create any name folder or project folder diff --git a/app/services/output_manager/error_handler.py b/app/services/output_manager/error_handler.py index a7e2c79f..53e5d480 100644 --- a/app/services/output_manager/error_handler.py +++ b/app/services/output_manager/error_handler.py @@ -22,6 +22,7 @@ class ECustomizedError(enum.Enum): TEXT_TOO_LONG = 'TEXT_TOO_LONG' FIELD_REQUIRED = 'FIELD_REQUIRED' INVALID_TEMPLATE = 'INVALID_TEMPLATE' + INVALID_TAG_FILE = 'INVALID_TAG_FILE' LIMIT_TAG_ERROR = 'LIMIT_TAG_ERROR' INVALID_TAG_ERROR = 'INVALID_TAG_ERROR' RESERVED_TAG = 'RESERVED_TAG' diff --git a/app/utils/aggregated.py b/app/utils/aggregated.py index 95ac1a97..83bd7520 100644 --- a/app/utils/aggregated.py +++ b/app/utils/aggregated.py @@ -133,7 +133,7 @@ def get_zone(zone): def validate_folder_name(folder_name): regex = re.compile('[/:?.\\*<>|”\']') contain_invalid_char = regex.search(folder_name) - if contain_invalid_char or len(folder_name) > 20 or not folder_name: + if contain_invalid_char or len(folder_name) > 100 or not folder_name: valid = False else: valid = True diff --git a/pyproject.toml b/pyproject.toml index aba8fe43..f04e7c59 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "app" -version = "2.9.8" +version = "2.10.0a0" description = "This service is designed to support pilot platform" authors = ["Indoc Systems"] diff --git a/tests/app/commands/test_file.py b/tests/app/commands/test_file.py index 3b6adb0e..d90912d1 100644 --- a/tests/app/commands/test_file.py +++ b/tests/app/commands/test_file.py @@ -55,6 +55,35 @@ def test_file_upload_command_success_with_attribute(mocker, cli_runner): attribute_mock.assert_called_once() +def test_file_upload_failed_with_invalid_tag_file(cli_runner): + # create invalid tag file with wrong format + runner = click.testing.CliRunner() + with runner.isolated_filesystem(): + with open('wrong_tag.json', 'w') as f: + f.write('wrong_tag.json') + + result = cli_runner.invoke( + file_put, ['--project-path', 'test', '--thread', 1, '--tag', 'wrong_tag.json', 'wrong_tag.json'] + ) + assert result.exit_code == 0 + assert result.output == customized_error_msg(ECustomizedError.INVALID_TAG_FILE) + '\n' + + +def test_file_upload_failed_with_invalid_attribute_file(cli_runner): + # create invalid attribute file with wrong format + runner = click.testing.CliRunner() + with runner.isolated_filesystem(): + with open('wrong_attribute.json', 'w') as f: + f.write('wrong_attribute.json') + + result = cli_runner.invoke( + file_put, + ['--project-path', 'test', '--thread', 1, '--attribute', 'wrong_attribute.json', 'wrong_attribute.json'], + ) + assert result.exit_code == 0 + assert result.output == customized_error_msg(ECustomizedError.INVALID_TEMPLATE) + '\n' + + def test_resumable_upload_command_success(mocker, cli_runner): mocker.patch('os.path.exists', return_value=True) # mock the open function diff --git a/tests/app/services/file_manager/file_metadata/test_file_metadata_client.py b/tests/app/services/file_manager/file_metadata/test_file_metadata_client.py index 48c16216..dafeeaac 100644 --- a/tests/app/services/file_manager/file_metadata/test_file_metadata_client.py +++ b/tests/app/services/file_manager/file_metadata/test_file_metadata_client.py @@ -84,3 +84,76 @@ def test_file_metadata_client_get_detail_success_with_no_tag_attributes(mocker, assert item_info == item_info assert res_attributes == {} assert tags == tags + + +def test_metadata_download_from_project_folder(mocker, httpx_mock): + item_info = { + 'id': 'test', + 'parent_id': 'test_parent', + 'parent_path': 'shared/path', + 'name': 'admin', + 'zone': 0, + 'status': 'ACTIVE', + } + tags = ['test'] + attri_template_uid = 'template_uid' + attri_template_name = 'template_name' + attributes = {attri_template_uid: {'attr_1': 'value'}} + + mocker.patch( + 'app.services.user_authentication.token_manager.SrvTokenManager.decode_access_token', + return_value=decoded_token(), + ) + + search_mock = mocker.patch( + 'app.services.file_manager.file_metadata.file_metadata_client.search_item', + ) + search_mock.side_effect = [ + {'result': {}, 'code': 404}, + {'result': {**item_info, 'extended': {'extra': {'tags': tags, 'attributes': attributes}}}}, + ] + httpx_mock.add_response( + url=AppConfig.Connections.url_portal + f'/v1/data/manifest/{attri_template_uid}', + method='GET', + json={'result': {'id': attri_template_uid, 'name': attri_template_name}}, + ) + + mocker.patch( + 'app.services.file_manager.file_metadata.file_metadata_client.FileMetaClient.save_file_metadata', + return_value=None, + ) + + file_meta_client = FileMetaClient('zone', 'project_code/object_path', 'general', 'attr', 'tag') + assert file_meta_client.project_code == 'project_code' + assert file_meta_client.object_path == 'object_path' + + item_info, res_attributes, tags = file_meta_client.download_file_metadata() + assert item_info == item_info + assert res_attributes == {attri_template_name: attributes.get(attri_template_uid)} + assert tags == tags + assert search_mock.call_count == 2 + + +def test_metadata_download_fail_when_file_doesnot_exist(mocker, capfd): + mocker.patch( + 'app.services.user_authentication.token_manager.SrvTokenManager.decode_access_token', + return_value=decoded_token(), + ) + + search_mock = mocker.patch( + 'app.services.file_manager.file_metadata.file_metadata_client.search_item', + ) + search_mock.side_effect = [{'result': {}, 'code': 404}, {'result': {}, 'code': 404}] + + file_meta_client = FileMetaClient('zone', 'project_code/object_path', 'general', 'attr', 'tag') + + try: + file_meta_client.download_file_metadata() + except SystemExit: + assert search_mock.call_count == 2 + out, _ = capfd.readouterr() + + expect = 'Cannot find item project_code/object_path at zone.\n' + assert out == expect + else: + AssertionError('SystemExit not raised') diff --git a/tests/app/services/file_manager/file_move/test_file_move_client.py b/tests/app/services/file_manager/file_move/test_file_move_client.py index e22776e6..59d8cab5 100644 --- a/tests/app/services/file_manager/file_move/test_file_move_client.py +++ b/tests/app/services/file_manager/file_move/test_file_move_client.py @@ -18,6 +18,11 @@ def test_file_move_success(mocker, httpx_mock): return_value=decoded_token(), ) + mocker.patch( + 'app.services.file_manager.file_move.file_move_client.FileMoveClient.create_object_path_if_not_exist', + return_value=[], + ) + httpx_mock.add_response( url=AppConfig.Connections.url_bff + f'/v1/{project_code}/files', method='PATCH', @@ -37,6 +42,11 @@ def test_file_move_error_with_permission_denied_403(mocker, httpx_mock, capfd): return_value=decoded_token(), ) + mocker.patch( + 'app.services.file_manager.file_move.file_move_client.FileMoveClient.create_object_path_if_not_exist', + return_value=[], + ) + httpx_mock.add_response( url=AppConfig.Connections.url_bff + f'/v1/{project_code}/files', method='PATCH', @@ -60,6 +70,11 @@ def test_file_move_error_with_wrong_input_422(mocker, httpx_mock, capfd): return_value=decoded_token(), ) + mocker.patch( + 'app.services.file_manager.file_move.file_move_client.FileMoveClient.create_object_path_if_not_exist', + return_value=[], + ) + httpx_mock.add_response( url=AppConfig.Connections.url_bff + f'/v1/{project_code}/files', method='PATCH', diff --git a/tests/app/utils/test_aggregated.py b/tests/app/utils/test_aggregated.py index 34a02646..00ec31e5 100644 --- a/tests/app/utils/test_aggregated.py +++ b/tests/app/utils/test_aggregated.py @@ -7,6 +7,7 @@ from app.configs.app_config import AppConfig from app.utils.aggregated import check_item_duplication from app.utils.aggregated import search_item +from app.utils.aggregated import validate_folder_name from tests.conftest import decoded_token test_project_code = 'testproject' @@ -122,3 +123,9 @@ def test_check_duplicate_fail_with_error_code(httpx_mock, mocker, capsys): check_item_duplication(['test_path'], 0, 'test_project_code') out, _ = capsys.readouterr() assert out.rstrip() == '{"error": "internal server error"}' + + +@pytest.mark.parametrize('folder_name', ['/:?.\\*<>|”\'', ''.join(['1' for _ in range(101)])]) +def test_validate_folder_name(folder_name): + valid = validate_folder_name(folder_name) + assert valid is False From 873d29075375807e4877a51cfad8b90bb84e37ca Mon Sep 17 00:00:00 2001 From: Color Zhan Date: Thu, 21 Mar 2024 10:22:16 -0400 Subject: [PATCH 2/5] Pilot 4730: update upload command to distinguish project folder and name folder (#132) * add new folder type for namefolder and project folder * update upload logic to use projectfolder as the keyword to distinguish between project folder and name folder uploading * add more test cases for folder type * bump up to next version * update help message for project folder --------- Co-authored-by: zhiren --- app/commands/file.py | 5 +- app/models/folder.py | 22 ++++++++ app/resources/custom_help.py | 6 ++- .../file_manager/file_upload/file_upload.py | 16 +++--- app/utils/aggregated.py | 52 +++++++++++++++---- pyproject.toml | 2 +- tests/app/commands/test_file.py | 8 ++- .../file_upload/test_file_upload.py | 13 ++--- tests/app/utils/test_aggregated.py | 23 ++++++++ 9 files changed, 110 insertions(+), 37 deletions(-) create mode 100644 app/models/folder.py diff --git a/app/commands/file.py b/app/commands/file.py index b3590f3d..e83baa27 100644 --- a/app/commands/file.py +++ b/app/commands/file.py @@ -167,8 +167,7 @@ def file_put(**kwargs): # noqa: C901 message_handler.SrvOutPutHandler.cancel_upload() exit(1) - project_path = click.prompt('ProjectCode') if not project_path else project_path - project_code, target_folder = identify_target_folder(project_path) + project_code, folder_type, target_folder = identify_target_folder(project_path) srv_manifest = SrvFileManifests() upload_val_event = { 'zone': zone, @@ -207,8 +206,8 @@ def file_put(**kwargs): # noqa: C901 f, target_folder, project_code, + folder_type, zone, - zipping, ) upload_event = { diff --git a/app/models/folder.py b/app/models/folder.py new file mode 100644 index 00000000..180c337c --- /dev/null +++ b/app/models/folder.py @@ -0,0 +1,22 @@ +# Copyright (C) 2023-2024 Indoc Systems +# +# Contact Indoc Systems for any questions regarding the use of this source code. + +from enum import Enum + + +class FolderType(str, Enum): + """Available folder types.""" + + NAMEFOLDER = 'namefolder' + PROJECTFOLDER = 'projectfolder' + + def get_prefix(self) -> str: + """Get the prefix for the folder type.""" + + prefix = { + 'namefolder': '', + 'projectfolder': 'shared/', + } + + return prefix.get(self.value) diff --git a/app/resources/custom_help.py b/app/resources/custom_help.py index 75e77563..3e4b17e6 100644 --- a/app/resources/custom_help.py +++ b/app/resources/custom_help.py @@ -43,7 +43,11 @@ class HelpPage: 'FILE_SYNC_ZIP': 'Download files as a zip.', 'FILE_SYNC_I': 'Enable downloading by geid.', 'FILE_SYNC_Z': 'Target Zone (i.e., core/greenroom).', - 'FILE_UPLOAD_P': 'Project folder path starting from Project Code. (i.e., indoctestproject/user/folder)', + 'FILE_UPLOAD_P': ( + 'Project folder path starting from Project Code(i.e. /user/folder). ' + 'A new key word `projectfolder` is required to specify project folder(i.e. ' + '/projectfolder/folder1)' + ), 'FILE_UPLOAD_A': 'Add attributes to the file using a File Attribute Template.', 'FILE_UPLOAD_T': 'Add tags to the file using a Tag file.', 'FILE_UPLOAD_M': 'The message used to comment on the purpose of uploading your processed file.', diff --git a/app/services/file_manager/file_upload/file_upload.py b/app/services/file_manager/file_upload/file_upload.py index 15cd5219..4dda080b 100644 --- a/app/services/file_manager/file_upload/file_upload.py +++ b/app/services/file_manager/file_upload/file_upload.py @@ -18,6 +18,7 @@ import app.services.logger_services.log_functions as logger import app.services.output_manager.message_handler as mhandler from app.configs.app_config import AppConfig +from app.models.folder import FolderType from app.services.file_manager.file_upload.models import FileObject from app.services.file_manager.file_upload.models import ItemStatus from app.services.file_manager.file_upload.models import UploadType @@ -43,7 +44,7 @@ def compress_folder_to_zip(path): def assemble_path( - f: str, target_folder: str, project_code: str, zone: str, zipping: bool = False + f: str, target_folder: str, project_code: str, folder_type: FolderType, zone: str ) -> Tuple[str, Dict, bool, str]: ''' Summary: @@ -61,7 +62,6 @@ def assemble_path( - target_folder(str): the folder on the platform - project_code(str): the unique identifier of project - zone(str): the zone label eg.greenroom/core - - zipping(bool): default False. The flag to indicate if upload as a zip Return: - current_file_path: the format file path on platform - parent_folder: the item information of longest parent folder @@ -72,18 +72,16 @@ def assemble_path( current_file_path = target_folder + '/' + f.rstrip('/').split('/')[-1] # set name folder as first parent folder - name_folder = target_folder.split('/')[0] - parent_folder = search_item(project_code, zone, name_folder).get('result', {}) + root_folder = target_folder.split('/')[0] + parent_folder = search_item(project_code, zone, root_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 + # add prefix to folder + current_folder_node = folder_type.get_prefix() + current_folder_node + target_folder = folder_type.get_prefix() + target_folder if len(current_file_path.split('/')) > 2: sub_path = target_folder.split('/') diff --git a/app/utils/aggregated.py b/app/utils/aggregated.py index 83bd7520..662db673 100644 --- a/app/utils/aggregated.py +++ b/app/utils/aggregated.py @@ -8,6 +8,7 @@ from typing import Any from typing import Dict from typing import List +from typing import Tuple import httpx import requests @@ -16,6 +17,7 @@ from app.configs.app_config import AppConfig from app.configs.config import ConfigClass from app.configs.user_config import UserConfig +from app.models.folder import FolderType from app.services.output_manager.error_handler import ECustomizedError from app.services.output_manager.error_handler import SrvErrorHandler from app.services.user_authentication.decorator import require_valid_token @@ -164,19 +166,49 @@ def get_file_in_folder(path): return files_list -def identify_target_folder(project_path): - project_code = project_path.split('/')[0] - if len(project_path.split('/')) > 1: - target_folder = '/'.join(project_path.split('/')[1:]) - for f in target_folder.split('/'): - f = f.strip(' ') - valid = validate_folder_name(f) - if not valid: - SrvErrorHandler.customized_handle(ECustomizedError.INVALID_FOLDERNAME, True) +def identify_target_folder(project_path: str) -> Tuple[str, FolderType, str]: + ''' + Summary: + the function will validate if input folder path doesn't + contain invalid characters and return the project code and target folder + Parameters: + - project_path: + - for project folder the input folder path (eg. /projectfolder/) + - for name folder the input folder path will be (eg. /) + Return: + - project_code: the project code + - folder_type: the folder type + - target_folder: the target folder + ''' + # split into project_code, folder_type, folder + temp_paths = project_path.split('/', 2) + project_code, folder_type, folder_name = temp_paths[0], '', '' + + # check folder type if is project folder or name folder + # there will be a extra string for project folder between project code and folder name + if len(temp_paths) == 2: + folder_type = FolderType.NAMEFOLDER + folder_name = temp_paths[1] + elif len(temp_paths) >= 3: + if temp_paths[1] == FolderType.PROJECTFOLDER.value: + folder_type = FolderType.PROJECTFOLDER + folder_name = temp_paths[2] + else: + folder_type = FolderType.NAMEFOLDER + folder_name = os.path.join(temp_paths[1], temp_paths[2]) else: SrvErrorHandler.customized_handle(ECustomizedError.INVALID_NAMEFOLDER, True) target_folder = '' - return project_code, target_folder + + # first check if folder names are valid + target_folder = '/'.join(folder_name.split('/')) + for f in target_folder.split('/'): + f = f.strip(' ') + valid = validate_folder_name(f) + if not valid: + SrvErrorHandler.customized_handle(ECustomizedError.INVALID_FOLDERNAME, True) + + return project_code, folder_type, target_folder def batch_generator(iterable: List[Any], batch_size=1): diff --git a/pyproject.toml b/pyproject.toml index f04e7c59..07a4eeb4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "app" -version = "2.10.0a0" +version = "2.10.0" description = "This service is designed to support pilot platform" authors = ["Indoc Systems"] diff --git a/tests/app/commands/test_file.py b/tests/app/commands/test_file.py index d90912d1..3abe2cab 100644 --- a/tests/app/commands/test_file.py +++ b/tests/app/commands/test_file.py @@ -24,10 +24,6 @@ def test_file_upload_command_success_with_attribute(mocker, cli_runner): - project_code = 'test_project' - target_folder = 'admin' - - mocker.patch('app.commands.file.identify_target_folder', return_value=(project_code, target_folder)) mocker.patch('app.commands.file.validate_upload_event', return_value={'source_file': '', 'attribute': 'test'}) mocker.patch('app.commands.file.assemble_path', return_value=('test', {'id': 'id'}, True, 'test')) @@ -48,8 +44,10 @@ def test_file_upload_command_success_with_attribute(mocker, cli_runner): json.dump({'template': {'attr1': 'value'}}, f) result = cli_runner.invoke( - file_put, ['--project-path', 'test', '--thread', 1, '--attribute', 'template.json', 'test.txt'] + file_put, + ['--project-path', 'test_project/admin', '--thread', 1, '--attribute', 'template.json', 'test.txt'], ) + assert result.exit_code == 0 simple_upload_mock.assert_called_once() attribute_mock.assert_called_once() 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 17b3e474..b4a9ffb2 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 @@ -3,6 +3,7 @@ # Contact Indoc Systems for any questions regarding the use of this source code. from app.configs.app_config import AppConfig +from app.models.folder import FolderType from app.services.file_manager.file_upload.file_upload import assemble_path from app.services.file_manager.file_upload.file_upload import resume_upload from app.services.file_manager.file_upload.file_upload import simple_upload @@ -17,7 +18,6 @@ def test_assemble_path_at_name_folder(mocker): target_folder = 'admin' project_code = 'test_project' zone = 0 - resumable_id = None mocker.patch( 'app.services.file_manager.file_upload.file_upload.search_item', @@ -34,7 +34,7 @@ def test_assemble_path_at_name_folder(mocker): ) current_file_path, parent_folder, create_folder_flag, _ = assemble_path( - local_file_path, target_folder, project_code, zone, resumable_id + local_file_path, target_folder, project_code, FolderType.NAMEFOLDER, zone ) assert current_file_path == 'admin/file.txt' assert parent_folder.get('name') == 'admin' @@ -46,7 +46,6 @@ def test_assemble_path_at_exsting_folder(mocker): target_folder = 'admin/test_folder_exist' project_code = 'test_project' zone = 0 - resumable_id = None node_list = [ { @@ -74,7 +73,7 @@ def test_assemble_path_at_exsting_folder(mocker): mocker.patch('app.services.file_manager.file_upload.file_upload.search_item', side_effect=node_list) current_file_path, parent_folder, create_folder_flag, _ = assemble_path( - local_file_path, target_folder, project_code, zone, resumable_id + local_file_path, target_folder, project_code, FolderType.NAMEFOLDER, zone ) assert current_file_path == 'admin/test_folder_exist/file.txt' assert parent_folder.get('name') == 'test_folder_exist' @@ -86,7 +85,6 @@ def test_assemble_path_at_non_existing_folder(mocker): target_folder = 'admin/test_folder_not_exist' project_code = 'test_project' zone = 0 - resumable_id = None node_list = [ { @@ -106,7 +104,7 @@ def test_assemble_path_at_non_existing_folder(mocker): mocker.patch('app.services.file_manager.file_upload.file_upload.click.confirm', return_value=None) current_file_path, parent_folder, create_folder_flag, _ = assemble_path( - local_file_path, target_folder, project_code, zone, resumable_id + local_file_path, target_folder, project_code, FolderType.NAMEFOLDER, zone ) assert current_file_path == 'admin/test_folder_not_exist' assert parent_folder.get('name') == 'admin' @@ -118,7 +116,6 @@ def test_assemble_path_at_project_folder(mocker): 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', @@ -135,7 +132,7 @@ def test_assemble_path_at_project_folder(mocker): ) current_file_path, parent_folder, create_folder_flag, target_folder = assemble_path( - local_file_path, target_folder, project_code, zone, resumable_id + local_file_path, target_folder, project_code, FolderType.PROJECTFOLDER, zone ) assert current_file_path == 'shared/project_folder/file.txt' assert parent_folder.get('name') == 'project_folder' diff --git a/tests/app/utils/test_aggregated.py b/tests/app/utils/test_aggregated.py index 00ec31e5..88194ca2 100644 --- a/tests/app/utils/test_aggregated.py +++ b/tests/app/utils/test_aggregated.py @@ -5,7 +5,9 @@ import pytest from app.configs.app_config import AppConfig +from app.models.folder import FolderType from app.utils.aggregated import check_item_duplication +from app.utils.aggregated import identify_target_folder from app.utils.aggregated import search_item from app.utils.aggregated import validate_folder_name from tests.conftest import decoded_token @@ -129,3 +131,24 @@ def test_check_duplicate_fail_with_error_code(httpx_mock, mocker, capsys): def test_validate_folder_name(folder_name): valid = validate_folder_name(folder_name) assert valid is False + + +@pytest.mark.parametrize( + 'input_path,expected_result', + [ + ('project_code/username', ('project_code', FolderType.NAMEFOLDER, 'username')), + ('project_code/username/folder1', ('project_code', FolderType.NAMEFOLDER, 'username/folder1')), + ('project_code/projectfolder/folder1', ('project_code', FolderType.PROJECTFOLDER, 'folder1')), + ('project_code/projectfolder/folder1/folder2', ('project_code', FolderType.PROJECTFOLDER, 'folder1/folder2')), + ], +) +def test_identify_target_folder_success_with_different_path(mocker, input_path, expected_result): + mocker.patch('app.utils.aggregated.validate_folder_name', return_value=True) + result = identify_target_folder(input_path) + assert result == expected_result + + +def test_identify_target_folder_fail_with_invalid_input(mocker): + mocker.patch('app.utils.aggregated.validate_folder_name', return_value=False) + with pytest.raises(SystemExit): + identify_target_folder('project_code') From 8b3766673fc417db7ed36947c4d3d5c763b819c5 Mon Sep 17 00:00:00 2001 From: Color Zhan Date: Thu, 4 Apr 2024 11:05:33 -0400 Subject: [PATCH 3/5] Pilot 4734: distinguish project folders and name folders when listing (#133) * add [p] prefix for project folder when listing items under project * group itemprefix class with itemtype class * add double quotation when item name contains space * change enum type PROJECTFOLDER to SHAREDFOLDER * bumpup versions --------- Co-authored-by: zhiren --- app/models/folder.py | 22 ----------- app/models/item.py | 38 +++++++++++++++++++ app/services/file_manager/file_list.py | 15 +++++++- .../file_manager/file_upload/file_upload.py | 8 ++-- app/utils/aggregated.py | 11 +++--- pyproject.toml | 2 +- tests/app/commands/test_file.py | 7 +++- .../file_upload/test_file_upload.py | 11 +++--- tests/app/utils/test_aggregated.py | 10 ++--- 9 files changed, 76 insertions(+), 48 deletions(-) delete mode 100644 app/models/folder.py create mode 100644 app/models/item.py diff --git a/app/models/folder.py b/app/models/folder.py deleted file mode 100644 index 180c337c..00000000 --- a/app/models/folder.py +++ /dev/null @@ -1,22 +0,0 @@ -# Copyright (C) 2023-2024 Indoc Systems -# -# Contact Indoc Systems for any questions regarding the use of this source code. - -from enum import Enum - - -class FolderType(str, Enum): - """Available folder types.""" - - NAMEFOLDER = 'namefolder' - PROJECTFOLDER = 'projectfolder' - - def get_prefix(self) -> str: - """Get the prefix for the folder type.""" - - prefix = { - 'namefolder': '', - 'projectfolder': 'shared/', - } - - return prefix.get(self.value) diff --git a/app/models/item.py b/app/models/item.py new file mode 100644 index 00000000..a7a91029 --- /dev/null +++ b/app/models/item.py @@ -0,0 +1,38 @@ +# Copyright (C) 2023-2024 Indoc Systems +# +# Contact Indoc Systems for any questions regarding the use of this source code. + +from enum import Enum + + +class ItemType(str, Enum): + """The class to reflect the type of item in database.""" + + FILE = 'file' + Folder = 'folder' + NAMEFOLDER = 'name_folder' + SHAREDFOLDER = 'project_folder' + + @classmethod + def get_type_from_keyword(self, keyword: str): + """The function will return the type of the item based on the keyword. + + - name folder will have keyword 'namefolder' as input + - project folder will not have any keyword + """ + + alternative_mapping = { + 'projectfolder': self.SHAREDFOLDER, + } + + return alternative_mapping.get(keyword, self.NAMEFOLDER) + + def get_prefix_by_type(self) -> str: + """Get the prefix for the folder type.""" + + prefix = { + self.NAMEFOLDER: '', + self.SHAREDFOLDER: 'shared/', + } + + return prefix.get(self.value, '') diff --git a/app/services/file_manager/file_list.py b/app/services/file_manager/file_list.py index 39982cd9..2131903a 100644 --- a/app/services/file_manager/file_list.py +++ b/app/services/file_manager/file_list.py @@ -9,6 +9,7 @@ import app.services.logger_services.log_functions as logger from app.configs.app_config import AppConfig from app.configs.user_config import UserConfig +from app.models.item import ItemType from app.models.service_meta_class import MetaService from app.services.output_manager.error_handler import ECustomizedError from app.services.output_manager.error_handler import SrvErrorHandler @@ -59,10 +60,20 @@ def list_files(self, paths, zone, page, page_size): # then format the console output files, folders = '', '' for f in res: - if 'file' == f.get('type'): + item_type = ItemType(f.get('type')) + # if there is space within the nane add double quotation to aviod confusion + if ' ' in f.get('name'): + f['name'] = f'"{f.get("name")}"' + + if item_type == ItemType.FILE: files = files + f.get('name') + ' ...' - elif f.get('type') in ['folder', 'name_folder', 'project_folder']: + else: + # add [p] in front of the project folder + if item_type == ItemType.SHAREDFOLDER: + f['name'] = f'[p]{f.get("name")}' + 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 4dda080b..952ec259 100644 --- a/app/services/file_manager/file_upload/file_upload.py +++ b/app/services/file_manager/file_upload/file_upload.py @@ -18,7 +18,7 @@ import app.services.logger_services.log_functions as logger import app.services.output_manager.message_handler as mhandler from app.configs.app_config import AppConfig -from app.models.folder import FolderType +from app.models.item import ItemType from app.services.file_manager.file_upload.models import FileObject from app.services.file_manager.file_upload.models import ItemStatus from app.services.file_manager.file_upload.models import UploadType @@ -44,7 +44,7 @@ def compress_folder_to_zip(path): def assemble_path( - f: str, target_folder: str, project_code: str, folder_type: FolderType, zone: str + f: str, target_folder: str, project_code: str, folder_type: ItemType, zone: str ) -> Tuple[str, Dict, bool, str]: ''' Summary: @@ -80,8 +80,8 @@ def assemble_path( current_folder_node = target_folder if os.path.isfile(f) else current_file_path create_folder_flag = False # add prefix to folder - current_folder_node = folder_type.get_prefix() + current_folder_node - target_folder = folder_type.get_prefix() + target_folder + current_folder_node = folder_type.get_prefix_by_type() + current_folder_node + target_folder = folder_type.get_prefix_by_type() + target_folder if len(current_file_path.split('/')) > 2: sub_path = target_folder.split('/') diff --git a/app/utils/aggregated.py b/app/utils/aggregated.py index 662db673..e338ae5f 100644 --- a/app/utils/aggregated.py +++ b/app/utils/aggregated.py @@ -17,7 +17,7 @@ from app.configs.app_config import AppConfig from app.configs.config import ConfigClass from app.configs.user_config import UserConfig -from app.models.folder import FolderType +from app.models.item import ItemType from app.services.output_manager.error_handler import ECustomizedError from app.services.output_manager.error_handler import SrvErrorHandler from app.services.user_authentication.decorator import require_valid_token @@ -166,7 +166,7 @@ def get_file_in_folder(path): return files_list -def identify_target_folder(project_path: str) -> Tuple[str, FolderType, str]: +def identify_target_folder(project_path: str) -> Tuple[str, ItemType, str]: ''' Summary: the function will validate if input folder path doesn't @@ -187,14 +187,13 @@ def identify_target_folder(project_path: str) -> Tuple[str, FolderType, str]: # check folder type if is project folder or name folder # there will be a extra string for project folder between project code and folder name if len(temp_paths) == 2: - folder_type = FolderType.NAMEFOLDER + folder_type = ItemType.NAMEFOLDER folder_name = temp_paths[1] elif len(temp_paths) >= 3: - if temp_paths[1] == FolderType.PROJECTFOLDER.value: - folder_type = FolderType.PROJECTFOLDER + folder_type = ItemType.get_type_from_keyword(temp_paths[1]) + if folder_type == ItemType.SHAREDFOLDER: folder_name = temp_paths[2] else: - folder_type = FolderType.NAMEFOLDER folder_name = os.path.join(temp_paths[1], temp_paths[2]) else: SrvErrorHandler.customized_handle(ECustomizedError.INVALID_NAMEFOLDER, True) diff --git a/pyproject.toml b/pyproject.toml index 07a4eeb4..0ad2804c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "app" -version = "2.10.0" +version = "2.10.1" description = "This service is designed to support pilot platform" authors = ["Indoc Systems"] diff --git a/tests/app/commands/test_file.py b/tests/app/commands/test_file.py index 3abe2cab..ab9e64bc 100644 --- a/tests/app/commands/test_file.py +++ b/tests/app/commands/test_file.py @@ -185,7 +185,9 @@ def test_file_list_with_pagination_with_name_project_folder(requests_mock, mocke 'result': [ {'type': 'folder', 'name': 'folder1'}, {'type': 'name_folder', 'name': 'name_folder1'}, - {'type': 'project_folder', 'name': 'project_folder1'}, + {'type': 'project_folder', 'name': 'project folder1'}, + {'type': 'folder', 'name': 'test folder2'}, + {'type': 'project_folder', 'name': 'project folder2'}, ], }, ) @@ -193,7 +195,8 @@ def test_file_list_with_pagination_with_name_project_folder(requests_mock, mocke 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 ' + assert outputs[0] == 'folder1 name_folder1 [p]"project folder1" ' + assert outputs[1] == '"test folder2" [p]"project folder2" ' def test_empty_file_list_with_pagination(requests_mock, mocker, cli_runner): 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 b4a9ffb2..fb385057 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 @@ -3,7 +3,7 @@ # Contact Indoc Systems for any questions regarding the use of this source code. from app.configs.app_config import AppConfig -from app.models.folder import FolderType +from app.models.item import ItemType from app.services.file_manager.file_upload.file_upload import assemble_path from app.services.file_manager.file_upload.file_upload import resume_upload from app.services.file_manager.file_upload.file_upload import simple_upload @@ -34,7 +34,7 @@ def test_assemble_path_at_name_folder(mocker): ) current_file_path, parent_folder, create_folder_flag, _ = assemble_path( - local_file_path, target_folder, project_code, FolderType.NAMEFOLDER, zone + local_file_path, target_folder, project_code, ItemType.NAMEFOLDER, zone ) assert current_file_path == 'admin/file.txt' assert parent_folder.get('name') == 'admin' @@ -71,9 +71,8 @@ def test_assemble_path_at_exsting_folder(mocker): ] mocker.patch('app.services.file_manager.file_upload.file_upload.search_item', side_effect=node_list) - current_file_path, parent_folder, create_folder_flag, _ = assemble_path( - local_file_path, target_folder, project_code, FolderType.NAMEFOLDER, zone + local_file_path, target_folder, project_code, ItemType.NAMEFOLDER, zone ) assert current_file_path == 'admin/test_folder_exist/file.txt' assert parent_folder.get('name') == 'test_folder_exist' @@ -104,7 +103,7 @@ def test_assemble_path_at_non_existing_folder(mocker): mocker.patch('app.services.file_manager.file_upload.file_upload.click.confirm', return_value=None) current_file_path, parent_folder, create_folder_flag, _ = assemble_path( - local_file_path, target_folder, project_code, FolderType.NAMEFOLDER, zone + local_file_path, target_folder, project_code, ItemType.NAMEFOLDER, zone ) assert current_file_path == 'admin/test_folder_not_exist' assert parent_folder.get('name') == 'admin' @@ -132,7 +131,7 @@ def test_assemble_path_at_project_folder(mocker): ) current_file_path, parent_folder, create_folder_flag, target_folder = assemble_path( - local_file_path, target_folder, project_code, FolderType.PROJECTFOLDER, zone + local_file_path, target_folder, project_code, ItemType.SHAREDFOLDER, zone ) assert current_file_path == 'shared/project_folder/file.txt' assert parent_folder.get('name') == 'project_folder' diff --git a/tests/app/utils/test_aggregated.py b/tests/app/utils/test_aggregated.py index 88194ca2..91454cbb 100644 --- a/tests/app/utils/test_aggregated.py +++ b/tests/app/utils/test_aggregated.py @@ -5,7 +5,7 @@ import pytest from app.configs.app_config import AppConfig -from app.models.folder import FolderType +from app.models.item import ItemType from app.utils.aggregated import check_item_duplication from app.utils.aggregated import identify_target_folder from app.utils.aggregated import search_item @@ -136,10 +136,10 @@ def test_validate_folder_name(folder_name): @pytest.mark.parametrize( 'input_path,expected_result', [ - ('project_code/username', ('project_code', FolderType.NAMEFOLDER, 'username')), - ('project_code/username/folder1', ('project_code', FolderType.NAMEFOLDER, 'username/folder1')), - ('project_code/projectfolder/folder1', ('project_code', FolderType.PROJECTFOLDER, 'folder1')), - ('project_code/projectfolder/folder1/folder2', ('project_code', FolderType.PROJECTFOLDER, 'folder1/folder2')), + ('project_code/username', ('project_code', ItemType.NAMEFOLDER, 'username')), + ('project_code/username/folder1', ('project_code', ItemType.NAMEFOLDER, 'username/folder1')), + ('project_code/projectfolder/folder1', ('project_code', ItemType.SHAREDFOLDER, 'folder1')), + ('project_code/projectfolder/folder1/folder2', ('project_code', ItemType.SHAREDFOLDER, 'folder1/folder2')), ], ) def test_identify_target_folder_success_with_different_path(mocker, input_path, expected_result): From 834fcbbf2226bcde181b9e778b108b0731f5a0ae Mon Sep 17 00:00:00 2001 From: zhiren Date: Mon, 8 Apr 2024 15:49:58 -0400 Subject: [PATCH 4/5] staging release --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 0ad2804c..61fd3306 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "app" -version = "2.10.1" +version = "2.10.2" description = "This service is designed to support pilot platform" authors = ["Indoc Systems"] From edd762e5b60f19d035a620dd223c29f5c57993a9 Mon Sep 17 00:00:00 2001 From: zhiren Date: Tue, 9 Apr 2024 15:45:53 -0400 Subject: [PATCH 5/5] fixup precommit --- tests/app/utils/test_aggregated.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/app/utils/test_aggregated.py b/tests/app/utils/test_aggregated.py index b6ef8933..91454cbb 100644 --- a/tests/app/utils/test_aggregated.py +++ b/tests/app/utils/test_aggregated.py @@ -132,6 +132,7 @@ def test_validate_folder_name(folder_name): valid = validate_folder_name(folder_name) assert valid is False + @pytest.mark.parametrize( 'input_path,expected_result', [