Skip to content

Commit

Permalink
Mb/staging release 20240307 (#131)
Browse files Browse the repository at this point in the history
* 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 <zzhan@indocresearch.org>

* 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 <zzhan@indocresearch.org>

* bumpup to next preminor

---------

Co-authored-by: Daniel <dfelipe@indocresearch.org>
Co-authored-by: zhiren <zzhan@indocresearch.org>
Co-authored-by: Dušan Andrić <Dusan.Andric@gmail.com>
  • Loading branch information
4 people authored Mar 7, 2024
1 parent e529357 commit 2129488
Show file tree
Hide file tree
Showing 11 changed files with 154 additions and 8 deletions.
14 changes: 10 additions & 4 deletions app/commands/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion app/resources/custom_error.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 '
Expand All @@ -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': (
Expand Down
10 changes: 9 additions & 1 deletion app/services/file_manager/file_metadata/file_metadata_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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', {})
Expand Down
6 changes: 6 additions & 0 deletions app/services/file_manager/file_move/file_move_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/services/output_manager/error_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion app/utils/aggregated.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -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"]

Expand Down
29 changes: 29 additions & 0 deletions tests/app/commands/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')
15 changes: 15 additions & 0 deletions tests/app/services/file_manager/file_move/test_file_move_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand Down
7 changes: 7 additions & 0 deletions tests/app/utils/test_aggregated.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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

1 comment on commit 2129488

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage

Coverage Report
FileStmtsMissCoverMissing
__init__.py00100% 
pilotcli.py0310%5, 7–8, 10–14, 17–21, 23–28, 31–39, 42–44
commands
   __init__.py00100% 
   container_registry.py01959%16, 22–24, 31–33, 41–44, 50–53, 62–65
   dataset.py04844%19, 36–38, 41–48, 50–60, 78–80, 83–91, 93–103, 123, 127
   entry_point.py01085%39, 45, 89–91, 93–97
   file.py04084%40, 152–154, 158, 163, 166–168, 185–186, 228, 313–320, 322, 324, 338–344, 346, 375, 377, 380, 426, 429, 446–448, 454–455
   project.py0197%17
   user.py0685%23, 40, 48, 56, 72–73
configs
   __init__.py00100% 
   app_config.py00100% 
   config.py00100% 
   user_config.py01784%61, 70, 114, 124, 131, 135, 139, 143, 147, 151, 155, 159, 163, 167, 171, 175, 179
models
   __init__.py00100% 
   enums.py00100% 
   service_meta_class.py0180%9
   singleton.py00100% 
   upload_form.py0625%30, 42–46
resources
   custom_error.py00100% 
   custom_help.py00100% 
services
   __init__.py00100% 
services/container_registry_manager
   container_registry_manager.py010817%19–20, 23–26, 29–33, 36–44, 48–60, 62–64, 68–82, 84–86, 90–99, 101–103, 107–110, 125–136, 140–143, 147–162, 164, 166–169
services/crypto
   __init__.py00100% 
   crypto.py01940%35, 43–47, 49, 59–61, 69–75, 77, 79
services/dataset_manager
   dataset_detail.py02370%42, 44, 61, 68–73, 75–77, 79–88, 90
   dataset_download.py01983%56, 72–73, 84, 96, 98–99, 101–102, 104, 106–108, 116–119, 153, 165
   dataset_list.py0878%37, 43–46, 48–50
   model.py00100% 
services/file_manager
   __init__.py00100% 
   file_list.py01775%29, 54, 56, 70–72, 80–83, 85, 91–96
   file_manifests.py09525%19–24, 39–43, 47–48, 51–59, 61, 65–68, 71–75, 77, 81–82, 85–87, 91–92, 95–102, 104, 107, 109–110, 112–113, 115–118, 123–129, 134–139, 142–148, 150–152, 155–161, 163–165, 167–170
   file_tag.py03931%23, 27–31, 33, 36–48, 50–52, 54–55, 59–61, 64–68, 70–73, 75–76
services/file_manager/file_download
   __init__.py00100% 
   download_client.py017423%44–45, 47–53, 56–57, 59–60, 69, 71–72, 76–79, 86, 91–94, 96, 98–106, 108, 110, 114–119, 121, 124–125, 128–130, 132–134, 136–137, 139–141, 145–153, 156–161, 165–174, 182–184, 186–193, 195–199, 204–214, 216–217, 220–221, 226–227, 231–236, 240–241, 244–246, 248–259, 263–266, 269–273, 275–277, 279–281, 283–284, 286, 290–292, 294–307, 309
   model.py0187%15
services/file_manager/file_metadata
   __init__.py00100% 
   file_metadata_client.py0395%97–99
services/file_manager/file_move
   __init__.py00100% 
   file_move_client.py0790%67, 76, 85–87, 89, 116
services/file_manager/file_upload
   __init__.py00100% 
   exception.py0175%10
   file_upload.py02884%35–42, 99–101, 113, 144–145, 149, 154, 191, 211–213, 254–255, 257–259, 333, 336, 340
   models.py0591%24, 44, 150–152
   upload_client.py07760%101–104, 207–209, 223–229, 231–235, 237–249, 251, 295, 298, 302, 304–306, 308–311, 315–319, 321, 325, 327, 329, 331, 350, 380–381, 386, 390–393, 397, 413, 415–417, 424, 429–430, 432, 434–435, 437–440, 442
   upload_validator.py02156%29–31, 34–39, 42–48, 51–52, 57, 59, 61
services/logger_services
   __init__.py00100% 
   log_functions.py00100% 
services/output_manager
   __init__.py00100% 
   error_handler.py00100% 
   help_page.py00100% 
   message_handler.py05770%23, 45, 50, 65–67, 72, 82, 87, 100, 105, 112, 117, 122, 126, 142, 177, 187, 196, 214, 224, 243, 265–276, 287–292, 294–300, 311, 315–316, 320–321, 323–324, 328, 332, 336
services/project_manager
   __init__.py00100% 
   project.py0878%38, 44–47, 49–51
services/user_authentication
   __init__.py00100% 
   decorator.py0293%27, 30
   token_manager.py0986%26, 41–42, 73, 76, 91, 96, 100, 105
   user_login_logout.py04266%30–31, 41, 122, 126–132, 134–136, 140–141, 145–152, 154–156, 160–161, 168–173, 175–177, 181–184
utils
   __init__.py00100% 
   aggregated.py03572%50, 62, 102, 115–117, 119–120, 127–128, 130, 139, 154–161, 163–164, 168–175, 177–179, 194–195
TOTAL309597768% 

Please sign in to comment.