diff --git a/src/app/components/bulk/BulkResultStep.vue b/src/app/components/bulk/BulkResultStep.vue index 0d1d1f6..6489cf5 100644 --- a/src/app/components/bulk/BulkResultStep.vue +++ b/src/app/components/bulk/BulkResultStep.vue @@ -9,12 +9,11 @@ const { query, makePageInfo, makeIndicators } = useBulk() const toast = useToast() const { handleFetchError } = useErrorHandling() -const { data: status, execute: executeStatus } +const { data: status, refresh: refreshStatus } = await useFetch(`/api/bulk/execute/status/${properties.taskId}`, { method: 'GET', lazy: true, - immediate: false, onResponseError({ response }) { switch (response.status) { case 404: { @@ -33,7 +32,7 @@ const isPolling = ref(false) const pollExecuteStatus = async () => { isPolling.value = true for (let index = 0; index < maxAttempts; index++) { - await executeStatus() + await refreshStatus() const st = (status.value?.status) if (st === 'SUCCESS') { isPolling.value = false @@ -60,13 +59,12 @@ const pollExecuteStatus = async () => { isPolling.value = false } -const { data: executeResult, execute: fetchExecuteResult, status: getResultStatus } +const { data: executeResult, refresh: refreshExecuteResult, status: getResultStatus } = await useFetch(`/api/bulk/result/${properties.historyId}`, { method: 'GET', query, lazy: true, server: false, - immediate: false, onResponseError({ response }) { switch (response.status) { case 403: { @@ -95,7 +93,7 @@ const { data: executeResult, execute: fetchExecuteResult, status: getResultStatu onMounted(async () => { if (properties.taskId) await pollExecuteStatus() - await fetchExecuteResult() + await refreshExecuteResult() }) const indicators = computed(() => makeIndicators(executeResult.value)) diff --git a/src/app/components/bulk/BulkValidationStep.vue b/src/app/components/bulk/BulkValidationStep.vue index ed45ca3..0555a2e 100644 --- a/src/app/components/bulk/BulkValidationStep.vue +++ b/src/app/components/bulk/BulkValidationStep.vue @@ -18,20 +18,19 @@ const { const { makePageInfo, makeIndicators } = useBulk() const { polling: { interval, maxAttempts } } = useAppConfig() const { handleFetchError } = useErrorHandling() -const { data: status, execute: getValidateStatus } +const { data: status, refresh: refreshValidateStatus } = await useFetch(`/api/bulk/validate/status/${taskId.value}`, { method: 'GET', lazy: true, server: false, - immediate: false, onResponseError({ response }) { switch (response.status) { case 404: { showError({ status: 404, statusText: 'Not Found', - message: $t('error-page.not-found.bulk-validation'), + message: $t('bulk.validation.fetch_failed'), }) break } @@ -46,7 +45,7 @@ const isPolling = ref(false) const pollValidationStatus = async () => { isPolling.value = true for (let index = 0; index < maxAttempts; index++) { - await getValidateStatus() + await refreshValidateStatus() const st = (status.value?.status) if (st === 'SUCCESS') { isPolling.value = false @@ -74,13 +73,12 @@ const pollValidationStatus = async () => { isPolling.value = false } -const { data: validationResults, execute: getValidateResult, status: getResultStatus } +const { data: validationResults, refresh: refreshValidateResult, status: getResultStatus } = await useFetch(`/api/bulk/validate/result/${taskId.value}`, { method: 'GET', query, lazy: true, server: false, - immediate: false, onResponseError({ response }) { switch (response.status) { case 400: { @@ -118,7 +116,7 @@ const { data: validationResults, execute: getValidateResult, status: getResultSt onMounted(async () => { await pollValidationStatus() - getValidateResult() + refreshValidateResult() }) const offset = computed(() => (validationResults.value?.offset ?? 1)) diff --git a/src/app/composables/useHistory.ts b/src/app/composables/useHistory.ts index dccfdfe..9d20b4f 100644 --- a/src/app/composables/useHistory.ts +++ b/src/app/composables/useHistory.ts @@ -225,7 +225,9 @@ const useHistory = () => { function sortableHeader(key: 'timestamp') { const label = key === 'timestamp' - ? (tab.value === 'download' ? $t('history.download.date') : $t('history.upload.date')) + ? (tab.value === 'download' + ? $t('history.table.column.download-date') + : $t('history.table.column.upload-date')) : '' const iconSet = { asc: 'i-lucide-arrow-down-0-1', diff --git a/src/app/i18n/locales/en.json b/src/app/i18n/locales/en.json index bc2d203..49a51db 100644 --- a/src/app/i18n/locales/en.json +++ b/src/app/i18n/locales/en.json @@ -6,7 +6,6 @@ "about-description3": "You can choose to delete users who are not included in the file.", "about-description4": "Supported formats: TSV, CSV, Excel.", "column": { - "email": "E-mail", "eppn": "ePPN", "groups": "Group ID", "row": "row", @@ -21,13 +20,10 @@ }, "execute": { "failed": "User update failed.", - "result_failed": "Failed to get execution results.", "timeout": "Bulk update process timed out." }, "file-format-error": "The file format is not supported. \nChoose TSV, CSV, or Excel file.", "file-required": "Please select file.", - "file-validate-error": "File validation failed.", - "filter": "filter", "import": { }, "import-results": "Batch operation results", @@ -58,7 +54,6 @@ "validate": "data validation", "validate_description": "Confirm changes" }, - "target-repository": "Operation target repository", "title": "Bulk user operations", "upload": { "execute": "Execute upload" @@ -74,8 +69,7 @@ "fix_error": "Please fix the error.", "results": "data preview", "timeout": "File validation timed out." - }, - "view-all": "Show all" + } }, "button": { "add": "Add", @@ -109,7 +103,6 @@ }, "error-page": { "failed": { - "bulk-validation": "File validation failed", "load-more": "Failed to retrieve download history again" }, "forbidden": { @@ -218,8 +211,8 @@ "public": "Public Status", "users-count": "Users" }, - "search-placeholder": "Search by group name...", "no-groupss-description": "Your repository does not have any groups. Please create a group first.", + "search-placeholder": "Search by group name...", "title": "Groups List" }, "title": "Groups" @@ -236,28 +229,16 @@ "logoAlt": "Header App Logo" }, "history": { - "available": "Re-download", - "cancel": "Number of cancellations", "date": "Execution date and time", "description": "You can check the history of file uploads and downloads.", - "download": { - "date": "Download date and time" - }, - "empty": "No data matching your search criteria was found.", "empty-data": "No data.", "expired": "Expired", "failed": "failure", - "failed-count": "Number of failures", - "file": "file", "file_not_available": "File download failed.", "filter": { - "options_not_found": "Failed to get filter.", "title": "filter" }, - "first-download": "First download", "group-count": "Number of groups", - "more": "Show more", - "operation": "Execution conditions", "operator": "Execution user", "private": "make private", "progress": "Processing", @@ -268,8 +249,6 @@ "show-detail": "Show details", "status": "status", "success": "success", - "success-count": "Number of successes", - "sum": "Total number of downloads | Total number of uploads", "tab": { "download": "Download history", "upload": "Upload history" @@ -277,24 +256,14 @@ "table": { "column": { "download-date": "Download date and time", - "eppns": "eppn", - "groups": "group", - "operator": "Execution user", - "re-download": "re-download", - "users": "user" + "upload-date": "Upload date and time" } }, "table-title": "History list", "target": "Download target | Upload target", "title": "History", "toggle_public_status_failed": "Failed to update publishing status.", - "unavailable": "Expired", - "upload": { - "date": "Upload date and time" - }, - "uploaded-at": "Upload date and time", - "user-count": "Number of users", - "view": "Number of items displayed" + "user-count": "Number of users" }, "login": { "dev-account": "Account", diff --git a/src/app/i18n/locales/ja.json b/src/app/i18n/locales/ja.json index 9c21aef..c49dcb1 100644 --- a/src/app/i18n/locales/ja.json +++ b/src/app/i18n/locales/ja.json @@ -6,7 +6,6 @@ "about-description3": "ファイルに含まれていないユーザーの削除を選択できます。", "about-description4": "対応形式: TSV, CSV, Excel", "column": { - "email": "E-maile", "eppn": "ePPN", "groups": "グループ ID", "row": "行", @@ -21,13 +20,10 @@ }, "execute": { "failed": "ユーザーの更新に失敗しました。", - "result_failed": "実行結果の取得に失敗しました。", "timeout": "一括更新処理がタイムアウトしました。" }, "file-format-error": "対応していないファイル形式です。TSV、CSV、またはExcelファイルを選択してください。", "file-required": "ファイルを選択してください。", - "file-validate-error": "ファイルの検証に失敗しました。", - "filter": "フィルター", "import": { }, "import-results": "一括操作結果", @@ -58,7 +54,6 @@ "validate": "データ検証", "validate_description": "変更内容の確認" }, - "target-repository": "操作対象リポジトリ", "title": "ユーザー一括操作", "upload": { "execute": "アップロード実行" @@ -74,8 +69,7 @@ "fix_error": "エラーを修正してください。", "results": "データプレビュー", "timeout": "ファイルの検証がタイムアウトしました。" - }, - "view-all": "すべて表示" + } }, "button": { "add": "追加", @@ -99,7 +93,6 @@ }, "error-page": { "failed": { - "bulk-validation": "ファイルの検証に失敗しました", "load-more": "再ダウンロード履歴の取得に失敗しました" }, "forbidden": { @@ -222,27 +215,16 @@ "logoAlt": "ヘッダーロゴ" }, "history": { - "available": "再ダウンロード", "date": "実行日時", "description": "ファイルのアップロード・ダウウンロードの履歴を確認できます。", - "download": { - "date": "ダウンロード日時" - }, - "empty": "検索条件に一致するデータが見つかりませんでした。", "empty-data": "データがありません。", "expired": "期限切れ", "failed": "失敗", - "failed-count": "失敗件数", - "file": "ファイル", "file_not_available": "ファイルのダウンロードに失敗しました。", "filter": { - "options_not_found": "フィルターの取得に失敗しました。", "title": "フィルター" }, - "first-download": "初回ダウンロード", "group-count": "グループ数", - "more": "さらに表示", - "operation": "実行条件", "operator": "実行ユーザー", "private": "非公開にする", "progress": "処理中", @@ -253,8 +235,6 @@ "show-detail": "詳細を表示", "status": "ステータス", "success": "成功", - "success-count": "成功件数", - "sum": "総ダウンロード回数 | 総アップロード回数", "tab": { "download": "ダウンロード履歴", "upload": "アップロード履歴" @@ -262,24 +242,14 @@ "table": { "column": { "download-date": "ダウンロード日時", - "eppns": "eppn", - "groups": "グループ", - "operator": "実行ユーザー", - "re-download": "再ダウンロード", - "users": "ユーザー" + "upload-date": "アップロード日時" } }, "table-title": "履歴一覧", "target": "ダウンロード対象 | アップロード対象", "title": "履歴", "toggle_public_status_failed": "公開状態の更新に失敗しました。", - "unavailable": "期限切れ", - "upload": { - "date": "アップロード日時" - }, - "uploaded-at": "アップロード日時", - "user-count": "ユーザー数", - "view": "表示件数" + "user-count": "ユーザー数" }, "login": { "dev-account": "アカウント", diff --git a/src/server/api/history.py b/src/server/api/history.py index 6f1bbf2..b45023a 100644 --- a/src/server/api/history.py +++ b/src/server/api/history.py @@ -138,25 +138,3 @@ def files(file_id: UUID) -> Response | tuple[ErrorResponse, int]: current_app.logger.error(E.FILE_NOT_FOUND, {"path": file_path}) return ErrorResponse(message=E.FILE_NOT_FOUND % {"path": file_path}), 404 return send_file(path_or_file=file_path) - - -@bp.get("/files//exists") -@login_required -@roles_required(USER_ROLES.SYSTEM_ADMIN, USER_ROLES.REPOSITORY_ADMIN) -@validate(response_by_alias=True) -def is_exist_files(file_id: UUID) -> tuple[bool | ErrorResponse, int]: - """Check if the file exists. - - Args: - file_id (UUID): Unique identifier of the file - - Returns: - bool:Whether to check if the file exists - """ - try: - file_path = Path(history.get_file_path(file_id)) - except RecordNotFound as exc: - return ErrorResponse(message=exc.message), 404 - if not Path(file_path).exists(): - return False, 200 - return True, 200 diff --git a/src/server/api/schemas.py b/src/server/api/schemas.py index 0c81c54..0331b4a 100644 --- a/src/server/api/schemas.py +++ b/src/server/api/schemas.py @@ -364,13 +364,3 @@ class FileQuery(UsersQuery): model_config = ignore_extra_config """Configure to ignore extra fields.""" - - -class ExportBody(BaseModel): - """Body for user export request.""" - - user_ids: list[str] - """List of user IDs to export.""" - - model_config = camel_case_config - """Configure to use camelCase aliasing.""" diff --git a/src/server/entities/bulk.py b/src/server/entities/bulk.py index d5a1493..acf10cb 100644 --- a/src/server/entities/bulk.py +++ b/src/server/entities/bulk.py @@ -78,7 +78,7 @@ class ResultSummary(BaseModel): class EachResult(BaseModel): """Model for result of validation check for each user.""" - id: str | None + id: str | None = None """The unique identifier for the user.""" eppn: list[str] @@ -96,9 +96,12 @@ class EachResult(BaseModel): status: t.Literal["create", "update", "delete", "skip", "error"] """The status of the validation check.""" - code: str | None + code: str | None = None """The code representing the result of the validation check.""" + message: str | None = None + """The message describing the result of the validation check.""" + model_config = camel_case_config | forbid_extra_config """Configure camelCase aliasing and forbid extra fields.""" diff --git a/src/server/entities/history_detail.py b/src/server/entities/history_detail.py index 5831257..f159daa 100644 --- a/src/server/entities/history_detail.py +++ b/src/server/entities/history_detail.py @@ -11,7 +11,7 @@ from pydantic import BaseModel, ConfigDict -from server.entities.summaries import GroupSummary, RepositorySummary, UserSummary +from server.entities.summaries import UserSummary from .common import camel_case_config @@ -86,9 +86,6 @@ class UploadHistoryData(BaseModel): status: t.Literal["S", "F", "P"] """Status of the upload operation.""" - summary: HistorySummary | None = None - """Summary of the upload operation.""" - file_path: str """Path of the uploaded file.""" @@ -108,53 +105,6 @@ class UploadHistoryData(BaseModel): """Configure to use camelCase aliasing.""" -class Results(BaseModel): - """Result of the upload operation.""" - - user_id: str - """User ID.""" - - eppn: list[str] - """User EPPN.""" - - emails: list[str] - """ User emails""" - - user_name: str - """User name.""" - - group: list[str] - """Group list.""" - - status: t.Literal["C", "U", "S", "D", "E"] - """Status of the upload operation.""" - - code: str | None - """Error code if the upload failed.""" - - model_config = camel_case_config - """Configure to use camelCase aliasing.""" - - -class HistorySummary(BaseModel): - """Summary of the history operation.""" - - create: int - """Number of created items.""" - - update: int - """Number of updated items.""" - - delete: int - """Number of deleted items.""" - - skip: int - """Number of skipped items.""" - - error: int - """Number of error items.""" - - class HistoryQuery(BaseModel): """Query parameters for searching history data.""" @@ -193,19 +143,3 @@ class HistoryQuery(BaseModel): model_config = ignore_extra_config """Configure to ignore extra fields.""" - - -class HistoryDataFilter(BaseModel): - """Available filters for history data.""" - - operators: list[UserSummary] - """List of operators.""" - - target_repositories: list[RepositorySummary] - """List of target repositories.""" - - target_groups: list[GroupSummary] - """List of target groups.""" - - target_users: list[UserSummary] - """List of target users.""" diff --git a/src/server/messages/error.py b/src/server/messages/error.py index 91e84a4..fbb17e0 100644 --- a/src/server/messages/error.py +++ b/src/server/messages/error.py @@ -605,6 +605,11 @@ "Logged-in user does not have permission to perform this operation.", ) +VALIDATION_ERROR_BLOCK = LogMessage( + "E603", + "Execution is not allowed because there is one or more validation errors.", +) + FAILED_GET_UPLOAD_HISTORY_RECORD = LogMessage( "E610", "Failed to get upload history (id: %(history_id)s) from database.", @@ -674,6 +679,11 @@ "File validation failed for task: %(task_id)s. Please check the file.", ) +FILE_NOT_ACTIVE_SHEET = LogMessage( + "E625", + "No active sheet found in the Excel file (path: %(path)s).", +) + TASK_NOT_FOUND = LogMessage( "E634", "Task (id: %(task_id)s) not found. It may have been expired.", diff --git a/src/server/services/bulks.py b/src/server/services/bulks.py index 23e5f7d..6c530c5 100644 --- a/src/server/services/bulks.py +++ b/src/server/services/bulks.py @@ -205,7 +205,7 @@ def build_user_from_file( header_row_index = 1 for i, row in enumerate(it): - if i == header_row_index + 1 or row is None: + if i == header_row_index + 1 or not row: continue if i == 0: @@ -226,6 +226,8 @@ def build_user_from_file( exclude = {"id"} else: user_name_value = r[user_name_idx] if user_name_idx else None + if not user_name_value: + continue bucket = new_data[user_name_value] exclude = {"user_name"} for col, j in idx_of.items(): @@ -263,8 +265,7 @@ def _read_file(file_path: str) -> t.Generator: if ws: iterator = ws.iter_rows(values_only=True) else: - error = f"{path}: No active sheet found in the Excel file." - current_app.logger.error(error) + current_app.logger.error(E.FILE_NOT_ACTIVE_SHEET, {"path": path}) raise FileValidationError(E.INVALID_FILE_STRUCTURE) if iterator is None: raise FileFormatError(E.FILE_FORMAT_UNSUPPORTED % {"suffix": path.suffix}) @@ -499,7 +500,8 @@ def _build_check_results( groups=user_group_ids, email=u.emails or [], status="error", - code=exc.message, + code=exc.code, + message=exc.string, ) ) count_error += 1 @@ -666,7 +668,7 @@ def update_users( UUID: The ID of the upload history record. Raises: - FileNotFound: If the upload history does not exist. + RecordNotFound: If the upload history does not exist. requests.RequestException: If there is an error communicating with mAP Core API. ValidationError: If there is an error parsing the response from mAP Core API. OAuthTokenError: If there is an issue with the access token. @@ -675,9 +677,12 @@ def update_users( """ upload_data = history_table.get_upload_by_id(history_id) if not upload_data: - error = f"History not found: {history_id}" - current_app.logger.error(error) - raise FileNotFound(error) + current_app.logger.error( + E.FAILED_GET_UPLOAD_HISTORY_RECORD, {"history_id": history_id} + ) + raise RecordNotFound( + E.FAILED_GET_UPLOAD_HISTORY_RECORD % {"history_id": history_id} + ) # file content must contain at least one repository. repository_id = upload_data.file.file_content["repositories"][0]["id"] @@ -685,9 +690,8 @@ def update_users( summary = upload_data.results.get("summary", {}) if summary.get("error", 1) > 0: - error = "There are errors in the validation results." - current_app.logger.error(error) - raise FileValidationError(error) + current_app.logger.error(E.VALIDATION_ERROR_BLOCK) + raise FileValidationError(E.VALIDATION_ERROR_BLOCK) bulk_ops, count_delete = _build_bulk_operations_from_check_results( repository_id, check_results, remove_users @@ -716,6 +720,7 @@ def update_users( UnexpectedResponseError, ) as exc: history_table.update_upload_status(history_id=history_id, status="F") + db.session.commit() current_app.logger.error(exc) raise @@ -740,11 +745,13 @@ def update_users( status="F", new_results={"results": check_results, "summary": summary}, ) + db.session.commit() else: history_table.update_upload_status( history_id=history_id, status="S", ) + db.session.commit() current_app.logger.info(I.SUCCESS_BULK_OPERATION, {"history_id": history_id}) return history_id @@ -766,16 +773,15 @@ def save_file(temp_file_id: UUID) -> UUID: try: files = history_table.get_file_by_id(temp_file_id) repository_id = files.file_content["repositories"][0]["id"] - except (KeyError, AttributeError) as e: - current_app.logger.error("Failed to retrieve temporary file: %s", temp_file_id) - raise FileNotFound(str(e)) from e + except (KeyError, AttributeError) as exc: + current_app.logger.error(exc) + raise FileNotFound( + E.FAILED_GET_FILE_RECORD % {"file_id": temp_file_id} + ) from exc file_path = Path(files.file_path) - if file_path.parent != Path(config.STORAGE.local.temporary): - return files.id if not file_path.exists(): - error_msg = f"File not found: {file_path}" - raise FileNotFound(error_msg) + raise FileNotFound(E.FILE_EXPIRED % {"path": file_path}) if file_path.suffix not in {".csv", ".tsv", ".xlsx"}: raise FileFormatError(E.FILE_FORMAT_UNSUPPORTED % {"suffix": file_path.suffix}) diff --git a/src/server/services/groups.py b/src/server/services/groups.py index e1c49b0..b4d2f96 100644 --- a/src/server/services/groups.py +++ b/src/server/services/groups.py @@ -472,7 +472,7 @@ def delete_multiple(group_ids: set[str]) -> set[str] | None: CredentialsError: If the client credentials are invalid. UnexpectedResponseError: If response from mAP Core API is unexpected. """ - if config.FEATURES.enable_bulk_operation: + if not config.FEATURES.enable_bulk_operation: return delete_multiple_sequentially(group_ids) operations = [ BulkOperation(method="DELETE", path=f"/Groups/{group_id}") @@ -540,6 +540,8 @@ def delete_multiple_sequentially(group_ids: set[str]) -> set[str] | None: OAuthTokenError: If the access token is invalid or expired. CredentialsError: If the client credentials are invalid. """ + if config.FEATURES.enable_bulk_operation: + return delete_multiple(group_ids) failed_list: set[str] = set() for group_id in group_ids: try: diff --git a/src/server/services/history.py b/src/server/services/history.py index 7e9e6a0..7401128 100644 --- a/src/server/services/history.py +++ b/src/server/services/history.py @@ -80,7 +80,6 @@ def get_upload_history_data( { **history.__dict__, "operator": {"id": history.operator_id, "user_name": history.operator_name}, - "summary": history.results.get("summary"), "file_id": file.id, "file_path": file.file_path, "repository_count": len(file.file_content.get("repositories", [])), diff --git a/src/server/services/users.py b/src/server/services/users.py index cfc7b7e..656d789 100644 --- a/src/server/services/users.py +++ b/src/server/services/users.py @@ -17,6 +17,7 @@ from flask import current_app from pydantic_core import ValidationError +from server.api.schemas import FileQuery from server.clients import users from server.config import config from server.const import ( @@ -64,7 +65,6 @@ if t.TYPE_CHECKING: - from server.api.schemas import FileQuery from server.clients.users import UsersSearchResponse from server.entities.map_user import Group, MapUser from server.entities.patch_request import PatchOperation diff --git a/src/server/services/utils/transformers.py b/src/server/services/utils/transformers.py index f3ca70f..7fc3951 100644 --- a/src/server/services/utils/transformers.py +++ b/src/server/services/utils/transformers.py @@ -357,7 +357,7 @@ def validate_group_to_map_group( ) -> MapGroup: ... -def validate_group_to_map_group( +def validate_group_to_map_group( # noqa: C901, PLR0912 group: GroupDetail, *, mode: t.Literal["create", "update"] ) -> tuple[MapGroup, str] | MapGroup: """Validate the GroupDetail instance and convert it to a MapGroup instance. @@ -402,11 +402,11 @@ def validate_group_to_map_group( from server.services import repositories # noqa: PLC0415 if repositories.get_by_id(repository_id) is None: - error = E.GROUP_REQUIRES_EXISTING_REPOSITORY % {"id": repository_id} + error = E.GROUP_REQUIRES_EXISTING_REPOSITORY % {"rid": repository_id} raise InvalidFormError(error) if not is_super() and repository_id not in get_permitted_repository_ids(): - error = E.GROUP_FORBIDDEN_REPOSITORY % {"id": repository_id} + error = E.GROUP_FORBIDDEN_REPOSITORY % {"rid": repository_id} raise InvalidFormError(error) user_defined_id = group.user_defined_id diff --git a/tests/unit/api/test_auth.py b/tests/unit/api/test_auth.py index 57f06c9..534537b 100644 --- a/tests/unit/api/test_auth.py +++ b/tests/unit/api/test_auth.py @@ -6,12 +6,15 @@ from flask import session from flask_login import current_user, login_user +from redis import RedisError from server.api import auth from server.const import USER_ROLES from server.entities.login_user import LoginUser from server.entities.summaries import GroupSummary from server.entities.user_detail import UserDetail +from server.exc import DatastoreError +from server.messages import E from server.services.utils.affiliations import Affiliations, _RoleGroup @@ -79,7 +82,7 @@ def test_login_not_is_member_of(app, mocker: MockerFixture): mock_affiliations = Affiliations( roles=[_RoleGroup(repository_id="test", role=USER_ROLES.REPOSITORY_ADMIN)], groups=[] ) - excepted_is_member_of = "https://cg.gakunin.jp/gr/jc_test_roles_repoadm;https://cg.gakunin.jp/gr/group1" + excepted_is_member_of = "/gr/jc_test_roles_repoadm;/gr/group1" with app.test_request_context( "/api/auth/login", headers={ @@ -171,6 +174,29 @@ def test_login_next(app, mocker: MockerFixture): assert resp.location.endswith("/?next=users") +def test_login_with_redis_error(app, datastore, mocker: MockerFixture): + mock_affiliations = Affiliations(roles=[_RoleGroup(repository_id=None, role=USER_ROLES.SYSTEM_ADMIN)], groups=[]) + with app.test_request_context( + "/api/auth/login?next=users", + headers={ + "eppn": "test_eppn", + "IsMemberOf": "https://cg.gakunin.jp/gr/group1;https://cg.gakunin.jp/gr/jc_roles_sysadm", + "DisplayName": "Test User", + }, + ): + _, account_store, _ = datastore + account_store.hset.side_effect = RedisError + mocker.patch("server.services.users.get_by_eppn", return_value=mock_repoadmin_user_detail) + mocker.patch("server.api.auth.extract_group_ids", return_value=["group1", "jc_roles_sysadm"]) + mocker.patch("server.api.auth.detect_affiliations", return_value=mock_affiliations) + expected_code = (E.FAILED_SET_LOGIN_SESSION % {"eppn": "test_eppn"}).code + expected_message = (E.FAILED_SET_LOGIN_SESSION % {"eppn": "test_eppn"}).data + with pytest.raises(DatastoreError) as exc_info: + auth.login() + assert str(exc_info.value.code) == expected_code + assert str(exc_info.value.string) == expected_message + + @pytest.mark.parametrize("session_id", ["test_eppn", None]) def test_logout(app, session_id): app.secret_key = "test-secret" @@ -180,3 +206,14 @@ def test_logout(app, session_id): resp, code = auth.logout() assert resp == "" # noqa: PLC1901 assert code == HTTPStatus.NO_CONTENT + + +def test_logout_with_redis_error(app, datastore, mocker: MockerFixture): + _, account_store, _ = datastore + account_store.delete.side_effect = RedisError + with app.test_request_context("/api/auth/logout"): + login_user(mock_repoadmin_login_user) + session["_id"] = "test_session_id" + resp, code = auth.logout() + assert resp == "" # noqa: PLC1901 + assert code == HTTPStatus.NO_CONTENT diff --git a/tests/unit/api/test_bulk.py b/tests/unit/api/test_bulk.py index 3bb0649..b6ae24e 100644 --- a/tests/unit/api/test_bulk.py +++ b/tests/unit/api/test_bulk.py @@ -3,16 +3,16 @@ from datetime import UTC, datetime from http import HTTPStatus -from uuid import UUID, uuid7 +from uuid import uuid7 from flask_login import login_user -from redis.exceptions import ConnectionError as RedisConnectionError from server.api import bulk -from server.api.schemas import BulkBody, ErrorResponse, ExcuteRequest, TargetRepository, UploadQuery -from server.entities.bulk import HistorySummary, ResultSummary, ValidateSummary +from server.api.schemas import BulkBody, ErrorResponse, ExcuteRequest, TargetRepositoryForm, UploadQuery +from server.entities.bulk import ExecuteResults, ResultSummary, ValidateResults from server.entities.login_user import LoginUser -from server.exc import InvalidRecordError, RecordNotFound +from server.exc import FileNotFound, FileValidationError, RecordNotFound, TaskExcutionError +from server.messages import E if t.TYPE_CHECKING: @@ -21,17 +21,17 @@ def test_upload_file(app, mocker: MockerFixture): test_func = inspect.unwrap(bulk.upload_file) - repository_id = "repo123" - operator_id = "user123" + repository_id = "repo1" + operator_id = "user1" operator_name = "test_user" + temp_file_id = uuid7() mock_user = LoginUser(eppn="test_eppn", is_member_of="", user_name=operator_name, map_id=operator_id, session_id="") - form = TargetRepository(repository_id=repository_id) + form = TargetRepositoryForm(repository_id=repository_id) dummy_file = mocker.Mock(bulk_file=mocker.Mock(filename="test.csv", save=mocker.Mock())) mock_task = mocker.Mock(id="task_id") - mock_create_file = mocker.patch("server.services.history_table.create_file", return_value=None) - mock_delete_temporary_file = mocker.patch( - "server.services.bulks.delete_temporary_file.apply_async", return_value=None - ) + mocker.patch("server.services.repositories.get_by_id", return_value=mocker.Mock()) + mocker.patch("server.api.bulk.get_permitted_repository_ids", return_value=[repository_id]) + mock_upload_file = mocker.patch("server.services.bulks.upload_file", return_value=temp_file_id) mock_validate_upload_data = mocker.patch( "server.services.bulks.validate_upload_data.apply_async", return_value=mock_task ) @@ -41,26 +41,46 @@ def test_upload_file(app, mocker: MockerFixture): assert result[0].task_id == mock_task.id assert isinstance(result[0], BulkBody) assert result[1] == HTTPStatus.OK - _, kwargs = mock_create_file.call_args - assert isinstance(kwargs["file_id"], UUID) - assert isinstance(kwargs["file_path"], str) - assert kwargs["file_content"] == {"repositories": [{"id": repository_id}]} - args, kwargs = mock_delete_temporary_file.call_args - assert isinstance(args[0][0], str) - assert isinstance(kwargs["countdown"], int) - args, kwargs = mock_validate_upload_data.call_args + args, _ = mock_upload_file.call_args + assert args[0] == form.repository_id + assert args[1] == dummy_file.bulk_file + args, _ = mock_validate_upload_data.call_args assert args[0][0] == operator_id assert args[0][1] == operator_name - assert isinstance(args[0][2], UUID) + assert args[0][2] == temp_file_id + + +def test_upload_file_repository_not_found(app, mocker: MockerFixture): + test_func = inspect.unwrap(bulk.upload_file) + mocker.patch("server.services.repositories.get_by_id", return_value=None) + repository_id = "repo1" + form = TargetRepositoryForm(repository_id=repository_id) + dummy_file = mocker.Mock(bulk_file=mocker.Mock(filename="test.csv", save=mocker.Mock())) + expected_message = E.REPOSITORY_NOT_FOUND % {"id": form.repository_id} + result = test_func(form=form, files=dummy_file) + assert result[0] == ErrorResponse(message=expected_message) + assert result[1] == HTTPStatus.NOT_FOUND + + +def test_upload_file_repository_forbidden(app, mocker: MockerFixture): + test_func = inspect.unwrap(bulk.upload_file) + mocker.patch("server.services.repositories.get_by_id", return_value=mocker.Mock()) + repository_id = "repo1" + form = TargetRepositoryForm(repository_id=repository_id) + dummy_file = mocker.Mock(bulk_file=mocker.Mock(filename="test.csv", save=mocker.Mock())) + expected_message = E.REPOSITORY_FORBIDDEN % {"id": form.repository_id} + with app.test_request_context(): + login_user(LoginUser(eppn="test_eppn", is_member_of="", user_name="test_user", map_id="test_id", session_id="")) + result = test_func(form=form, files=dummy_file) + assert result[0] == ErrorResponse(message=expected_message) + assert result[1] == HTTPStatus.FORBIDDEN def test_validate_status(app, mocker: MockerFixture): task_id = "task_id" test_func = inspect.unwrap(bulk.validate_status) - mock_task = mocker.Mock() - mock_task.state = "SUCCESS" - mocker.patch("server.services.bulks.validate_upload_data.AsyncResult", return_value=mock_task) - expected = BulkBody(status=mock_task.state) + mocker.patch("server.services.bulks.get_validate_task_result", return_value=mocker.Mock(state="SUCCESS")) + expected = BulkBody(status="SUCCESS") with app.test_request_context(): result = test_func(task_id) assert result[0] == expected @@ -70,88 +90,103 @@ def test_validate_status(app, mocker: MockerFixture): def test_validate_status_not_found(app, mocker: MockerFixture): task_id = "task_id" test_func = inspect.unwrap(bulk.validate_status) - mocker.patch("server.services.bulks.validate_upload_data.AsyncResult", return_value=None) + expected_message = E.TASK_NOT_FOUND % {"task_id": task_id} + mocker.patch("server.services.bulks.get_validate_task_result", side_effect=TaskExcutionError(expected_message)) + expected = ErrorResponse(message=expected_message) with app.test_request_context(): result = test_func(task_id) - assert result[0] == ErrorResponse(code="", message="Task not found: task_id") + assert result[0] == expected assert result[1] == HTTPStatus.NOT_FOUND -def test_validate_status_redis_error(app, mocker: MockerFixture): - task_id = "task_id" - test_func = inspect.unwrap(bulk.validate_status) - mocker.patch("server.services.bulks.validate_upload_data.AsyncResult", side_effect=RedisConnectionError) - with app.test_request_context(): - result = test_func(task_id) - assert result[0] == ErrorResponse(code="", message=f"Failed to connect to Redis: {task_id}") - assert result[1] == HTTPStatus.INTERNAL_SERVER_ERROR - - def test_validate_result(app, mocker: MockerFixture): task_id = "task_id" history_id = uuid7() test_func = inspect.unwrap(bulk.validate_result) - mock_task = mocker.Mock() - mock_task.result = history_id - mocker.patch("server.services.bulks.validate_upload_data.AsyncResult", return_value=mock_task) - expected = ValidateSummary( + mocker.patch("server.services.bulks.get_validate_task_result", return_value=mocker.Mock(result=history_id)) + mocker.patch("server.api.bulk.is_user_logged_in", return_value=True) + mocker.patch("server.services.bulks.chack_permission_to_operation", return_value=True) + expected = ValidateResults( results=[], - summary=HistorySummary(create=0, delete=0, error=0, skip=0, update=0), + summary=ResultSummary(create=0, delete=0, error=0, skip=0, update=0), missing_user=[], offset=1, page_size=50, ) mock_validate_result = mocker.patch("server.services.bulks.get_validate_result", return_value=expected) with app.test_request_context(): + login_user(LoginUser(eppn="test_eppn", is_member_of="", user_name="test_user", map_id="test_id", session_id="")) result = test_func(query=UploadQuery(f=[3], p=2, l=50), task_id=task_id) assert result[0] == expected assert result[1] == HTTPStatus.OK mock_validate_result.assert_called_once_with(history_id=history_id, status_filter=["skip"], offset=2, size=50) +def test_validate_result_not_permission(app, mocker: MockerFixture): + task_id = "task_id" + history_id = uuid7() + test_func = inspect.unwrap(bulk.validate_result) + mocker.patch("server.services.bulks.get_validate_task_result", return_value=mocker.Mock(result=history_id)) + mocker.patch("server.api.bulk.is_user_logged_in", return_value=False) + expected = ErrorResponse(message=E.OPERATION_FORBIDDEN) + with app.test_request_context(): + result = test_func(query=UploadQuery(), task_id=task_id) + assert result[0] == expected + assert result[1] == HTTPStatus.FORBIDDEN + + def test_validate_result_failed(app, mocker: MockerFixture): task_id = "task_id" test_func = inspect.unwrap(bulk.validate_result) - mock_task = mocker.Mock() - mock_task.successful.return_value = False - mocker.patch("server.services.bulks.validate_upload_data.AsyncResult", return_value=mock_task) + mocker.patch("server.services.bulks.get_validate_task_result", return_value=mocker.Mock(successful=False)) + expected_message = E.UNEXPECTED_SERVER_ERROR with app.test_request_context(): result = test_func(query=UploadQuery(), task_id=task_id) - assert result[0] == ErrorResponse(code="", message=f"Task not successful: {task_id}") - assert result[1] == HTTPStatus.BAD_REQUEST + assert result[0] == ErrorResponse(message=expected_message) + assert result[1] == HTTPStatus.INTERNAL_SERVER_ERROR -def test_validate_result_not_found(app, mocker: MockerFixture): +def test_validate_result_with_exception(app, mocker: MockerFixture): task_id = "task_id" test_func = inspect.unwrap(bulk.validate_result) - mocker.patch("server.services.bulks.validate_upload_data.AsyncResult", return_value=None) + history_id = uuid7() + mocker.patch("server.services.bulks.get_validate_task_result", return_value=mocker.Mock(result=history_id)) + mocker.patch("server.api.bulk.is_user_logged_in", return_value=True) + mocker.patch( + "server.services.bulks.chack_permission_to_operation", + side_effect=RecordNotFound(E.UPDATE_HISTORY_RECORD_NOT_FOUND % {"id": history_id}), + ) with app.test_request_context(): + login_user(LoginUser(eppn="test_eppn", is_member_of="", user_name="test_user", map_id="test_id", session_id="")) result = test_func(query=UploadQuery(), task_id=task_id) - assert result[0] == ErrorResponse(code="", message=f"Task not found: {task_id}") + assert result[0] == ErrorResponse(message=E.UPDATE_HISTORY_RECORD_NOT_FOUND % {"id": history_id}) assert result[1] == HTTPStatus.NOT_FOUND -def test_validate_result_redis_error(app, mocker: MockerFixture): +def test_validate_result_not_found(app, mocker: MockerFixture): task_id = "task_id" test_func = inspect.unwrap(bulk.validate_result) - mock_return_value = InvalidRecordError("Results must include 'summary' and 'results' keys") + expected_message = E.FILE_EXPIRED % {"path": "file_path"} mocker.patch( - "server.services.bulks.validate_upload_data.AsyncResult", return_value=mocker.Mock(result=mock_return_value) + "server.services.bulks.get_validate_task_result", + return_value=mocker.Mock(result=FileNotFound(expected_message)), ) with app.test_request_context(): result = test_func(query=UploadQuery(), task_id=task_id) - assert result[0] == ErrorResponse(code="", message=f"Task resulted in an exception: {mock_return_value}") - assert result[1] == HTTPStatus.BAD_REQUEST + assert result[0] == ErrorResponse(message=expected_message) + assert result[1] == HTTPStatus.NOT_FOUND -def test_validate_result_redis_connection_error(app, mocker: MockerFixture): +def test_validate_result_file_error(app, mocker: MockerFixture): task_id = "task_id" test_func = inspect.unwrap(bulk.validate_result) - mocker.patch("server.services.bulks.validate_upload_data.AsyncResult", side_effect=RedisConnectionError) + expected_message = E.INVALID_FILE_STRUCTURE + mock_return_value = FileValidationError(expected_message) + mocker.patch("server.services.bulks.get_validate_task_result", return_value=mocker.Mock(result=mock_return_value)) with app.test_request_context(): result = test_func(query=UploadQuery(), task_id=task_id) - assert result[0] == ErrorResponse(code="", message=f"Failed to connect to Redis: {task_id}") - assert result[1] == HTTPStatus.INTERNAL_SERVER_ERROR + assert result[0] == ErrorResponse(message=expected_message) + assert result[1] == HTTPStatus.BAD_REQUEST def test_execute(app, mocker: MockerFixture): @@ -165,6 +200,8 @@ def test_execute(app, mocker: MockerFixture): delete_users=["user1", "user2"], ) history_id = uuid7() + mocker.patch("server.api.bulk.is_user_logged_in", return_value=True) + mocker.patch("server.services.bulks.chack_permission_to_operation", return_value=True) mock_get_history_by_file_id = mocker.patch( "server.services.history_table.get_history_by_file_id", return_value=mocker.Mock(id=history_id) ) @@ -173,6 +210,7 @@ def test_execute(app, mocker: MockerFixture): ) expected = BulkBody(task_id=task_id, history_id=history_id) with app.test_request_context(): + login_user(LoginUser(eppn="test_eppn", is_member_of="", user_name="test_user", map_id="test_id", session_id="")) result = test_func(body) assert result[0] == expected assert result[1] == HTTPStatus.OK @@ -187,15 +225,35 @@ def test_execute_history_not_found(app, mocker: MockerFixture): temp_id = uuid7() body = ExcuteRequest(temp_file_id=temp_id) mocker.patch( - "server.services.history_table.get_history_by_file_id", side_effect=RecordNotFound("History not found") + "server.services.history_table.get_history_by_file_id", + side_effect=RecordNotFound(E.UPDATE_HISTORY_RECORD_NOT_FOUND % {"id": temp_id}), ) - expected_message = "History not found" + expected_message = E.UPDATE_HISTORY_RECORD_NOT_FOUND % {"id": temp_id} with app.test_request_context(): result = test_func(body) - assert result[0] == ErrorResponse(code="", message=expected_message) + assert result[0] == ErrorResponse(message=expected_message) assert result[1] == HTTPStatus.NOT_FOUND +def test_execute_not_permission(app, mocker: MockerFixture): + test_func = inspect.unwrap(bulk.execute) + temp_id = uuid7() + repository_id = "repo1" + body = ExcuteRequest(temp_file_id=temp_id, repository_id=repository_id) + expected_message = E.OPERATION_FORBIDDEN + mocker.patch( + "server.services.history_table.get_history_by_file_id", + return_value=mocker.Mock(id=uuid7()), + ) + mocker.patch("server.api.bulk.is_user_logged_in", return_value=True) + mocker.patch("server.services.bulks.chack_permission_to_operation", return_value=False) + with app.test_request_context(): + login_user(LoginUser(eppn="test_eppn", is_member_of="", user_name="test_user", map_id="test_id", session_id="")) + result = test_func(body) + assert result[0] == ErrorResponse(message=expected_message) + assert result[1] == HTTPStatus.FORBIDDEN + + def test_execute_status(app, mocker: MockerFixture): test_func = inspect.unwrap(bulk.execute_status) task_id = "task_id" @@ -210,31 +268,20 @@ def test_execute_status(app, mocker: MockerFixture): def test_execute_status_not_found(app, mocker: MockerFixture): test_func = inspect.unwrap(bulk.execute_status) task_id = "task_id" - mocker.patch("server.services.bulks.update_users.AsyncResult", return_value=None) - expected_message = "Task not found: task_id" + expected_message = E.TASK_NOT_FOUND % {"task_id": task_id} + mocker.patch("server.services.bulks.get_execute_task_result", side_effect=TaskExcutionError(expected_message)) with app.test_request_context(): result = test_func(task_id) - assert result[0] == ErrorResponse(code="", message=expected_message) + assert result[0] == ErrorResponse(message=expected_message) assert result[1] == HTTPStatus.NOT_FOUND -def test_execute_status_redis_error(app, mocker: MockerFixture): - test_func = inspect.unwrap(bulk.execute_status) - task_id = "task_id" - mocker.patch("server.services.bulks.update_users.AsyncResult", side_effect=RedisConnectionError) - expected_message = f"Failed to connect to Redis: {task_id}" - with app.test_request_context(): - result = test_func(task_id) - assert result[0] == ErrorResponse(code="", message=expected_message) - assert result[1] == HTTPStatus.INTERNAL_SERVER_ERROR - - def test_result(app, mocker: MockerFixture): test_func = inspect.unwrap(bulk.result) history_id = uuid7() - expected_summary = ResultSummary( + expected_summary = ExecuteResults( items=[], - summary=HistorySummary(create=0, delete=0, error=0, skip=0, update=0), + summary=ResultSummary(create=0, delete=0, error=0, skip=0, update=0), file_id=uuid7(), file_name="", operator="", @@ -244,6 +291,7 @@ def test_result(app, mocker: MockerFixture): offset=2, page_size=50, ) + mocker.patch("server.services.bulks.chack_permission_to_view", return_value=True) mock_get_upload_result = mocker.patch("server.services.bulks.get_upload_result", return_value=expected_summary) with app.test_request_context(): result = test_func(history_id, UploadQuery(f=[0, 1], p=2, l=50)) @@ -257,9 +305,23 @@ def test_result(app, mocker: MockerFixture): def test_result_not_found(app, mocker: MockerFixture): test_func = inspect.unwrap(bulk.result) history_id = uuid7() - mocker.patch("server.services.bulks.get_upload_result", side_effect=RecordNotFound("History not found")) - expected_message = "History not found" + expected_message = E.UPDATE_HISTORY_RECORD_NOT_FOUND % {"id": history_id} + mocker.patch("server.services.bulks.chack_permission_to_view", return_value=True) + mocker.patch( + "server.services.bulks.get_upload_result", + side_effect=RecordNotFound(expected_message), + ) with app.test_request_context(): result = test_func(history_id, UploadQuery()) - assert result[0] == ErrorResponse(code="", message=expected_message) + assert result[0] == ErrorResponse(message=expected_message) assert result[1] == HTTPStatus.NOT_FOUND + + +def test_result_not_permission(app, mocker: MockerFixture): + test_func = inspect.unwrap(bulk.result) + history_id = uuid7() + expected_message = E.OPERATION_FORBIDDEN + mocker.patch("server.services.bulks.chack_permission_to_view", return_value=False) + result = test_func(history_id, UploadQuery()) + assert result[0] == ErrorResponse(message=expected_message) + assert result[1] == HTTPStatus.FORBIDDEN diff --git a/tests/unit/api/test_groups.py b/tests/unit/api/test_groups.py index a8e3f24..771d779 100644 --- a/tests/unit/api/test_groups.py +++ b/tests/unit/api/test_groups.py @@ -237,7 +237,7 @@ def test_id_get_not_found(app: Flask, gen_group_id, mocker: MockerFixture) -> No """Tests id_get returns ErrorResponse and 404 when group does not exist.""" group_id = gen_group_id("g4") expected_status = 404 - not_found_message = "Group resource (id: jc_repo_id_groups_g4_test) not found." + not_found_message = "Group resource (id: jc_repo_id_gr_g4_test) not found." mocker.patch("server.api.groups.has_permission", return_value=True) mocker.patch("server.services.groups.get_by_id", return_value=None) @@ -310,7 +310,7 @@ def test_id_put_forbidden_no_permission(app: Flask, gen_group_id, mocker: Mocker type="group", ) expected_status = 403 - expected_message = "Logged-in user does not have permission to access Group (id: jc_repo_id_groups_g3_test)." + expected_message = "Logged-in user does not have permission to access Group (id: jc_repo_id_gr_g3_test)." mocker.patch("server.api.groups.has_permission", return_value=False) mocker.patch("server.services.groups.update", return_value=group) diff --git a/tests/unit/api/test_history.py b/tests/unit/api/test_history.py index 9bd9784..22f0352 100644 --- a/tests/unit/api/test_history.py +++ b/tests/unit/api/test_history.py @@ -11,7 +11,8 @@ from server.api.schemas import ErrorResponse, HistoryPublic, OperatorQuery from server.entities.history_detail import DownloadHistoryData, HistoryQuery, UploadHistoryData from server.entities.search_request import FilterOption, SearchResult -from server.exc import DatabaseError, InvalidQueryError, RecordNotFound +from server.exc import InvalidQueryError, RecordNotFound +from server.messages import E if t.TYPE_CHECKING: @@ -67,7 +68,7 @@ def test_filter_options_operators_invalid_query(app, mocker: MockerFixture): side_effect=InvalidQueryError(exception_message), ) resp = test_func(tab="download", query=OperatorQuery(p=0, l=20)) - assert resp[0] == ErrorResponse(code="", message=exception_message) + assert resp[0] == ErrorResponse(message=exception_message) assert resp[1] == HTTPStatus.BAD_REQUEST mock_get_filter_option.assert_called_once_with("download", key="o", criteria=OperatorQuery(q=None, p=0, l=20)) @@ -103,22 +104,6 @@ def test_get(app, mocker: MockerFixture, tab, expected): mock_get_download_history.assert_not_called() -def test_get_datebase_error(app, mocker: MockerFixture): - test_func = inspect.unwrap(history.get) - with ( - app.test_request_context(), - ): - exception_message = "download table connection error" - mock_get_download_history = mocker.patch( - "server.services.history.get_download_history_data", - side_effect=DatabaseError(""), - ) - resp = test_func(tab="download", query=HistoryQuery(p=1, l=20)) - assert resp[0] == ErrorResponse(code="", message=exception_message) - assert resp[1] == HTTPStatus.SERVICE_UNAVAILABLE - mock_get_download_history.assert_called_once_with(HistoryQuery(q=None, p=1, l=20)) - - def test_public_status(app, mocker: MockerFixture): test_func = inspect.unwrap(history.public_status) request_body = HistoryPublic(public=True) @@ -148,7 +133,7 @@ def test_public_status_record_not_found(app, mocker: MockerFixture): side_effect=RecordNotFound(exception_message), ) resp = test_func(tab="download", history_id=UUID(history_id), body=request_body) - assert resp[0] == ErrorResponse(code="", message=exception_message) + assert resp[0] == ErrorResponse(message=exception_message) assert resp[1] == HTTPStatus.NOT_FOUND mock_update_public_status.assert_called_once_with( tab="download", history_id=UUID(history_id), public=request_body.public @@ -175,7 +160,7 @@ def test_files_not_found(app, mocker: MockerFixture): mock_get_file_path = mocker.patch("server.services.history.get_file_path", return_value=file_path) with app.test_request_context(): resp = test_func(file_id=UUID(file_id)) - assert resp[0] == ErrorResponse(code="", message=f"File not found: {file_id}") + assert resp[0] == ErrorResponse(message=E.FILE_NOT_FOUND % {"path": file_path}) assert resp[1] == HTTPStatus.NOT_FOUND mock_get_file_path.assert_called_once_with(UUID(file_id)) @@ -190,38 +175,6 @@ def test_files_record_not_found(app, mocker: MockerFixture): ) with app.test_request_context(): resp = test_func(file_id=UUID(file_id)) - assert resp[0] == ErrorResponse(code="", message=exception_message) - assert resp[1] == HTTPStatus.NOT_FOUND - mock_get_file_path.assert_called_once_with(UUID(file_id)) - - -@pytest.mark.parametrize( - ("file_path", "expected"), - [ - (__file__, True), - ("/non/existent/file/path", False), - ], -) -def test_is_exist_files(app, mocker: MockerFixture, file_path, expected): - test_func = inspect.unwrap(history.is_exist_files) - file_id = "019c794e-bb17-758f-8f0b-63fc3c40a1d1" - mock_get_file_path = mocker.patch("server.services.history.get_file_path", return_value=file_path) - with app.test_request_context(): - resp = test_func(file_id=UUID(file_id)) - assert resp == (expected, HTTPStatus.OK) - mock_get_file_path.assert_called_once_with(UUID(file_id)) - - -def test_is_exist_files_record_not_found(app, mocker: MockerFixture): - test_func = inspect.unwrap(history.is_exist_files) - file_id = "019c794e-bc5e-75a4-a8a6-9e4fd32f62e9" - exception_message = f"{file_id} is not found" - mock_get_file_path = mocker.patch( - "server.services.history.get_file_path", - side_effect=RecordNotFound(exception_message), - ) - with app.test_request_context(): - resp = test_func(file_id=UUID(file_id)) - assert resp[0] == ErrorResponse(code="", message=exception_message) + assert resp[0] == ErrorResponse(message=exception_message) assert resp[1] == HTTPStatus.NOT_FOUND mock_get_file_path.assert_called_once_with(UUID(file_id)) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index e2c9a28..0c29ad5 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -74,6 +74,18 @@ def test_config(): "max_id_length": "50 - len('jc_') - len('_gr_')", }, "POSTGRES": {"db": "jctest", "host": db_host}, + "USERS": { + "export_fields": [ + "id", + "user_name", + "groups[].id", + "groups[].name", + "role", + "edu_person_principal_names[]", + "preferred_language", + "emails[]", + ] + }, "REDIS": { "cache_type": "RedisCache", "single": {"base_url": f"redis://{redis_host}:6379/0"}, @@ -81,6 +93,7 @@ def test_config(): }, "RABBITMQ": {"url": f"amqp://guest:guest@{amqp_host}:5672//"}, "STORAGE": {"local": {"temporary": "/var/tmp/jcgroups"}}, # noqa: S108 + "FEATURES": {"search_only_username": False, "enable_bulk_operation": True}, }) diff --git a/tests/unit/services/test_bulk.py b/tests/unit/services/test_bulk.py index 6a78abe..3d090ab 100644 --- a/tests/unit/services/test_bulk.py +++ b/tests/unit/services/test_bulk.py @@ -6,14 +6,17 @@ import pytest -from server.db.history import Files, _FileContent +from redis.exceptions import ConnectionError as RedisConnectionError + +from server.const import USER_ROLES +from server.db.history import Files, UploadHistory, _FileContent from server.entities.bulk import ( - CheckResult, - HistorySummary, + EachResult, + ExecuteResults, RepositoryMember, ResultSummary, UserAggregated, - ValidateSummary, + ValidateResults, ) from server.entities.bulk_request import BulkOperation, BulkResponse from server.entities.map_error import MapError @@ -22,8 +25,20 @@ from server.entities.patch_request import AddOperation, RemoveOperation from server.entities.search_request import SearchResponse from server.entities.summaries import GroupSummary -from server.entities.user_detail import UserDetail -from server.exc import FileValidationError, OAuthTokenError, RecordNotFound, ResourceInvalid, ResourceNotFound +from server.entities.user_detail import RepositoryRole, UserDetail +from server.exc import ( + DatastoreError, + FileFormatError, + FileNotFound, + FileUploadError, + FileValidationError, + InvalidFormError, + OAuthTokenError, + RecordNotFound, + TaskExcutionError, + UnexpectedResponseError, +) +from server.messages import E from server.services import bulks from server.services.utils.affiliations import Affiliations, _Group @@ -46,6 +61,33 @@ def deep_tuple(obj): assert set(map(to_tuple, list1)) == set(map(to_tuple, list2)) +def test_upload_file(app, mocker: MockerFixture): + repository_id = "repo1" + mock_file = mocker.MagicMock() + mock_file.save.return_value = None + mock_commit = mocker.patch("server.db.db.session.commit") + mocker.patch("server.services.history_table.create_file", return_value=None) + mock_task = mocker.patch("server.services.bulks.delete_temporary_file.apply_async", return_value=None) + result = bulks.upload_file(repository_id, mock_file) + assert isinstance(result, UUID) + mock_commit.assert_called_once() + mock_task.assert_called_once() + + +def test_upload_file_with_exception(app, mocker: MockerFixture): + repository_id = "repo1" + mock_file = mocker.MagicMock() + mock_file.save.side_effect = PermissionError + mocker.patch("server.services.history_table.create_file", return_value=None) + mock_commit = mocker.patch("server.db.db.session.commit") + mock_task = mocker.patch("server.services.bulks.delete_temporary_file.apply_async", return_value=None) + with pytest.raises(FileUploadError) as exc: + bulks.upload_file(repository_id, mock_file) + assert str(exc.value.code) == (E.FAILED_SAVE_UPLOADED_FILE % {"file_path": "dummy_path"}).code + mock_commit.assert_not_called() + mock_task.assert_not_called() + + def test_validate_upload_data(app, mocker: MockerFixture): operator_id = "test_user_id" operator_name = "test_user_name" @@ -72,7 +114,7 @@ def test_validate_upload_data(app, mocker: MockerFixture): mocker.patch("server.services.bulks._get_missing_users", return_value=missing_users) mocker.patch("server.services.bulks._get_repo_user_by_id", return_value=None) check_results = [ - CheckResult( + EachResult( id="user1", eppn=["eppn1"], email=["user1@example.com"], @@ -81,7 +123,7 @@ def test_validate_upload_data(app, mocker: MockerFixture): status="update", code=None, ), - CheckResult( + EachResult( id="user2", eppn=["eppn2"], email=["user2@example.com"], @@ -90,7 +132,7 @@ def test_validate_upload_data(app, mocker: MockerFixture): status="skip", code=None, ), - CheckResult( + EachResult( id="user3", eppn=["eppn3"], email=["user3@example.com"], @@ -100,7 +142,7 @@ def test_validate_upload_data(app, mocker: MockerFixture): code=None, ), ] - summary = HistorySummary(create=1, update=1, delete=0, skip=1, error=0) + summary = ResultSummary(create=1, update=1, delete=0, skip=1, error=0) mocker.patch( "server.services.bulks._build_check_results", return_value=( @@ -108,11 +150,13 @@ def test_validate_upload_data(app, mocker: MockerFixture): summary, ), ) + mock_history = UploadHistory() + mock_history.id = uuid7() mock_create_upload = mocker.patch( "server.services.history_table.create_upload", - return_value=uuid7(), + return_value=mock_history, ) - validate_summary = ValidateSummary( + validate_summary = ValidateResults( results=check_results, summary=summary, missing_user=missing_users, offset=0, page_size=3 ) result = bulks.validate_upload_data(operator_id, operator_name, temp_file_id) @@ -157,29 +201,50 @@ def test_get_repository_member(app, mocker: MockerFixture): def test_build_user_from_file(app, mocker: MockerFixture): + repo = ["id", "name", "ver"] header = [ "id", "user_name", "groups[].id", "groups[].name", + "role", "edu_person_principal_names[]", "emails[]", "preferred_language", ] - meta = ["readonly", "readonly", "writable", "readonly", "readonly", "readonly", "readonly"] - data1 = ["user1", "User 1", "jc_repo1_gr_test_group1", "Group 1", "test@eppn", "user1@example.com", "en"] - data2 = ["user1", "User 1", "jc_repo1_gr_test_group2", "Group 2", "test@eppn", "user1@example.com", "en"] - data3 = ["", "User 2", "jc_repo1_gr_test_group1", "Group 1", "test@eppn", "user2@example.com", "ja"] - data4 = ["", "User 3", "jc_repo1_gr_test_group1", "Group 1", "test@eppn", "user2@example.com", "ja"] + meta = ["readonly", "readonly", "writable", "readonly", "writeble", "readonly", "readonly", "readonly"] + data1 = [ + "user1", + "User 1", + "jc_repo1_gr_test_group1", + "Group 1", + "repository_admin", + "test@eppn", + "user1@example.com", + "en", + ] + data2 = [ + "user1", + "User 1", + "jc_repo1_gr_test_group2", + "Group 2", + "community_admin", + "test@eppn", + "user1@example.com", + "en", + ] + data3 = ["", "User 2", "jc_repo1_gr_test_group1", "Group 1", "contributor", "test@eppn", "user2@example.com", "ja"] + data4 = ["", "User 3", "jc_repo1_gr_test_group1", "Group 1", "general_user", "test@eppn", "user3@example.com", "ja"] data5 = [] - data6 = ["", "", "jc_repo1_gr_test_group1", "Group 1", "test@eppn", "user2@example.com", "ja"] - rows = [iter([header, meta, data1, data2, data3, data4, data5, data6])] + data6 = ["", "", "jc_repo1_gr_test_group1", "Group 1", "repository_admin", "test@eppn", "user4@example.com", "ja"] + rows = [iter([repo, header, meta, data1, data2, data3, data4, data5, data6])] mock_read_file = mocker.patch("server.services.bulks._read_file", return_value=iter(rows)) expected_data = { "user1": { "user_name": ["User 1", "User 1"], "groups[].id": ["jc_repo1_gr_test_group1", "jc_repo1_gr_test_group2"], "groups[].name": ["Group 1", "Group 2"], + "role": ["repository_admin", "community_admin"], "edu_person_principal_names[]": ["test@eppn", "test@eppn"], "emails[]": ["user1@example.com", "user1@example.com"], "preferred_language": ["en", "en"], @@ -190,6 +255,7 @@ def test_build_user_from_file(app, mocker: MockerFixture): "id": [""], "groups[].id": ["jc_repo1_gr_test_group1"], "groups[].name": ["Group 1"], + "role": ["contributor"], "edu_person_principal_names[]": ["test@eppn"], "emails[]": ["user2@example.com"], "preferred_language": ["ja"], @@ -198,8 +264,9 @@ def test_build_user_from_file(app, mocker: MockerFixture): "id": [""], "groups[].id": ["jc_repo1_gr_test_group1"], "groups[].name": ["Group 1"], + "role": ["general_user"], "edu_person_principal_names[]": ["test@eppn"], - "emails[]": ["user2@example.com"], + "emails[]": ["user3@example.com"], "preferred_language": ["ja"], }, } @@ -210,19 +277,6 @@ def test_build_user_from_file(app, mocker: MockerFixture): assert new_data == expected_new_data -def test_build_user_from_file_with_exception(app, mocker: MockerFixture): - dummy_file_path = "/var/tmp/test_file.css" # noqa: S108 - expected_error_message = f"{Path(dummy_file_path).suffix}: Unsupported file format." - mock_read_file = mocker.patch( - "server.services.bulks._read_file", - side_effect=ResourceInvalid(expected_error_message), - ) - with pytest.raises(ResourceNotFound) as exc: - bulks.build_user_from_file(dummy_file_path) - mock_read_file.assert_called_once_with(dummy_file_path) - assert str(exc.value) == expected_error_message - - @pytest.mark.parametrize( ("file_path", "expected"), [ @@ -254,17 +308,17 @@ def test__read_file_not_ws(app, mocker: MockerFixture): mock_wb = mocker.MagicMock() mock_wb.active = None mocker.patch("openpyxl.load_workbook", return_value=mock_wb) - with pytest.raises(ResourceInvalid) as exc: # noqa: PT012 + with pytest.raises(FileValidationError) as exc: # noqa: PT012 result = bulks._read_file(file_path) # noqa: SLF001 next(result) - assert str(exc.value) == f"{file_path}: No active sheet found in the Excel file." + assert str(exc.value) == str(E.INVALID_FILE_STRUCTURE) @pytest.mark.parametrize( ("file_path", "file_exist", "expected", "expected_exception"), [ - ("/var/tmp/test_file.csv", False, "/var/tmp/test_file.csv: File not found.", ResourceNotFound), # noqa: S108 - ("/var/tmp/test_file.css", True, ".css: Unsupported file format.", ResourceInvalid), # noqa: S108 + ("/var/tmp/test_file.csv", False, str(E.FILE_EXPIRED % {"path": "/var/tmp/test_file.csv"}), FileNotFound), # noqa: S108 + ("/var/tmp/test_file.css", True, str(E.FILE_FORMAT_UNSUPPORTED % {"suffix": ".css"}), FileFormatError), # noqa: S108 ], ) def test__read_file_with_exception(app, mocker: MockerFixture, file_path, file_exist, expected, expected_exception): @@ -281,6 +335,7 @@ def test_build_user_detail_from_dict(app, mocker: MockerFixture): "user_name": ["User 1", "User 1"], "groups[].id": ["jc_repo1_gr_test_group1", "jc_repo1_gr_test_group2", "jc_repo1_gr_test_group3"], "groups[].name": ["Group 1", "Group 2"], + "role": ["repository_admin"], "edu_person_principal_names[]": ["test@eppn", "test@eppn"], "emails[]": ["user1@example.com", "user1@example.com"], "preferred_language": ["en", "en"], @@ -289,6 +344,7 @@ def test_build_user_detail_from_dict(app, mocker: MockerFixture): "user_name": ["User 2", "User 2"], "groups[].id": ["jc_repo1_gr_test_group1"], "groups[].name": ["Group 1", "Group 2"], + "role": ["contributor"], "edu_person_principal_names[]": ["test@eppn", "test@eppn"], "emails[]": ["user2@example.com", "user2@example.com"], "preferred_language": ["ja", "ja"], @@ -297,6 +353,7 @@ def test_build_user_detail_from_dict(app, mocker: MockerFixture): "user_name": [""], "groups[].id": [""], "groups[].name": [""], + "role": ["contributor"], "edu_person_principal_names[]": [""], "emails[]": [""], "preferred_language": [""], @@ -315,6 +372,7 @@ def test_build_user_detail_from_dict(app, mocker: MockerFixture): eppns=["test@eppn"], emails=["user1@example.com"], preferred_language="en", + repository_roles=[RepositoryRole(id="repo1", service_name=None, user_role=USER_ROLES.REPOSITORY_ADMIN)], ), UserDetail( id="user2", @@ -325,10 +383,12 @@ def test_build_user_detail_from_dict(app, mocker: MockerFixture): eppns=["test@eppn"], emails=["user2@example.com"], preferred_language="ja", + repository_roles=[RepositoryRole(id="repo1", service_name=None, user_role=USER_ROLES.CONTRIBUTOR)], ), ] ) - assert bulks.build_user_detail_from_dict(data) == expected + repository_id = "repo1" + assert bulks.build_user_detail_from_dict(data, repository_id) == expected def test_build_user_detail_from_dict_by_name(app, mocker: MockerFixture): @@ -337,6 +397,7 @@ def test_build_user_detail_from_dict_by_name(app, mocker: MockerFixture): "id": [""], "groups[].id": ["jc_repo1_gr_test_group1"], "groups[].name": ["Group 1"], + "role": ["repository_admin"], "edu_person_principal_names[]": ["test@eppn"], "emails[]": ["user2@example.com"], "preferred_language": ["ja"], @@ -345,6 +406,7 @@ def test_build_user_detail_from_dict_by_name(app, mocker: MockerFixture): "id": [""], "groups[].id": ["jc_repo1_gr_test_group1"], "groups[].name": ["Group 1", ""], + "role": ["contributor"], "edu_person_principal_names[]": ["test@eppn"], "emails[]": ["user3@example.com"], "preferred_language": ["ja"], @@ -353,6 +415,7 @@ def test_build_user_detail_from_dict_by_name(app, mocker: MockerFixture): "id": [""], "groups[].id": [""], "groups[].name": [""], + "role": ["contributor"], "edu_person_principal_names[]": [""], "emails[]": [""], "preferred_language": [""], @@ -368,6 +431,7 @@ def test_build_user_detail_from_dict_by_name(app, mocker: MockerFixture): eppns=["test@eppn"], emails=["user2@example.com"], preferred_language="ja", + repository_roles=[RepositoryRole(id="repo1", service_name=None, user_role=USER_ROLES.REPOSITORY_ADMIN)], ), UserDetail( user_name="User 3", @@ -377,10 +441,12 @@ def test_build_user_detail_from_dict_by_name(app, mocker: MockerFixture): preferred_language="ja", eppns=["test@eppn"], emails=["user3@example.com"], + repository_roles=[RepositoryRole(id="repo1", service_name=None, user_role=USER_ROLES.CONTRIBUTOR)], ), ] ) - assert bulks.build_user_detail_from_dict_by_name(data) == expected + repository_id = "repo1" + assert bulks.build_user_detail_from_dict_by_name(data, repository_id) == expected @pytest.mark.parametrize( @@ -465,7 +531,7 @@ def test__build_check_results(app, mocker: MockerFixture): emails=["user2@example.com"], groups=[GroupSummary(id="group1", display_name="Group 1")], ), - UserDetail(id="user3", user_name="User 1", eppns=["test@eppn"], emails=["user1@example.com"]), + UserDetail(id="user3", user_name="User 3", eppns=["test@eppn"], emails=["user3@example.com"]), UserDetail(id="not_repo_user", user_name="User 1", eppns=["test@eppn"], emails=["user1@example.com"]), ] create_user = [ @@ -478,10 +544,9 @@ def test__build_check_results(app, mocker: MockerFixture): GroupSummary(id="group4", display_name="Group 4"), ], ), - UserDetail(user_name="User 5", eppns=["test@eppn"], emails=["user5@example.com"]), - UserDetail(id="user6", user_name="User 6", eppns=["test@eppn"], emails=["user6@example.com"]), + UserDetail(id="user5", user_name="User 5", eppns=["test@eppn"], emails=["user5@example.com"]), ] - repository_member = RepositoryMember(groups={"group1", "group2"}, users={"user1", "user2"}) + repository_member = RepositoryMember(groups={"group1", "group2"}, users={"user1", "user2", "user3"}) repo_user_by_id = { "user1": UserDetail(id="user1", user_name="User 1", eppns=["test@eppn"], emails=["user1@example.com"]), "user2": UserDetail( @@ -494,7 +559,7 @@ def test__build_check_results(app, mocker: MockerFixture): "user3": UserDetail(id="user3", user_name="User 3", eppns=["test@eppn"], emails=["user3@example.com"]), } expected_check_results = [ - CheckResult( + EachResult( id="user4", eppn=["test@eppn"], email=["user4@example.com"], @@ -503,44 +568,67 @@ def test__build_check_results(app, mocker: MockerFixture): status="error", code="Group ID does not exist", ), - CheckResult( - id=None, + EachResult( + id="user5", eppn=["test@eppn"], email=["user5@example.com"], user_name="User 5", groups=set(), - status="error", - code="Invalid user data", - ), - CheckResult( - id="user6", - eppn=["test@eppn"], - email=["user6@example.com"], - user_name="User 6", - groups=set(), status="create", - code=None, ), - CheckResult( + EachResult( id="user2", eppn=["test@eppn"], email=["user2@example.com"], user_name="User 2", groups={"group1"}, status="update", - code=None, ), - CheckResult( + EachResult( id="user3", eppn=["test@eppn"], - email=["user1@example.com"], + email=["user3@example.com"], user_name="User 3", groups=set(), status="skip", - code=None, ), ] - expected_summary = HistorySummary(create=1, update=1, delete=0, skip=1, error=2) + expected_summary = ResultSummary(create=1, update=1, delete=0, skip=1, error=1) + mocker.patch("server.services.bulks.validate_user_to_map_user", return_value=MapUser()) + result = bulks._build_check_results(update_user, create_user, repository_member, repo_user_by_id) # noqa: SLF001 + assert result == (expected_check_results, expected_summary) + + +def test__build_check_results_invalid_form(app, mocker: MockerFixture): + update_user = [] + create_user = [ + UserDetail(user_name="User 1"), + ] + repository_member = RepositoryMember(groups={"group1", "group2"}, users={"user1", "user2"}) + repo_user_by_id = { + "user1": UserDetail(id="user1", user_name="User 1", eppns=["test@eppn"], emails=["user1@example.com"]), + "user2": UserDetail( + id="user2", + user_name="User 2", + eppns=["test@eppn"], + emails=["user2@example.com"], + groups=[GroupSummary(id="group2", display_name="Group 2")], + ), + } + expected_check_results = [ + EachResult( + id=None, + eppn=[], + email=[], + user_name="User 1", + groups=set(), + status="error", + code=E.USER_REQUIRES_EPPN.code, + message=E.USER_REQUIRES_EPPN.data, + ), + ] + expected_summary = ResultSummary(create=0, update=0, delete=0, skip=0, error=1) + mocker.patch("server.services.bulks.validate_user_to_map_user", side_effect=InvalidFormError(E.USER_REQUIRES_EPPN)) result = bulks._build_check_results(update_user, create_user, repository_member, repo_user_by_id) # noqa: SLF001 assert result == (expected_check_results, expected_summary) @@ -674,6 +762,31 @@ def test__check_immutable_attributes(app, mocker: MockerFixture, original, updat assert bulks._check_immutable_attributes(original, update_user) == expected # noqa: SLF001 +def test_get_validate_task_result(app, mocker: MockerFixture): + task_id = "test_task_id" + expected = mocker.MagicMock() + mocker.patch("server.services.bulks.validate_upload_data.AsyncResult", return_value=expected) + assert bulks.get_validate_task_result(task_id) == expected + + +def test_get_validate_task_result_none(app, mocker: MockerFixture): + task_id = "test_task_id" + mocker.patch("server.services.bulks.validate_upload_data.AsyncResult", return_value=None) + expected = E.TASK_NOT_FOUND % {"task_id": "test_task_id"} + with pytest.raises(TaskExcutionError) as exc: + bulks.get_validate_task_result(task_id) + assert str(exc.value) == str(expected) + + +def test_get_validate_task_result_with_exception(app, mocker: MockerFixture): + task_id = "test_task_id" + expected_exception = RedisConnectionError("Failed to connect to Redis") + mocker.patch("server.services.bulks.validate_upload_data.AsyncResult", side_effect=expected_exception) + with pytest.raises(DatastoreError) as exc: + bulks.get_validate_task_result(task_id) + assert str(exc.value) == str(E.FAILED_CONNECT_REDIS % {"error": str(expected_exception)}) + + def test_get_validate_result(app, mocker: MockerFixture): result = [ { @@ -687,7 +800,7 @@ def test_get_validate_result(app, mocker: MockerFixture): } ] mocker.patch("server.services.history_table.get_paginated_upload_results", return_value=result) - summary = HistorySummary(create=1, update=0, delete=0, skip=0, error=0) + summary = ResultSummary(create=1, update=0, delete=0, skip=0, error=0) missing_user = [] mocker.patch("server.services.history_table.get_upload_results", side_effect=[summary, missing_user]) repository_member = RepositoryMember(groups={"group1"}, users={"user1"}) @@ -696,9 +809,9 @@ def test_get_validate_result(app, mocker: MockerFixture): status_filter = ["create"] offset = 1 limit = 20 - expected = ValidateSummary( + expected = ValidateResults( results=[ - CheckResult( + EachResult( id="user1", eppn=["test@eppn"], email=["user1@example.com"], @@ -720,7 +833,7 @@ def test_update_users_success(app, mocker): history_id = uuid7() temp_file_id = uuid7() check_results = [ - CheckResult(id="user1", eppn=[], email=[], user_name="User 1", groups=set(), status="create", code=None) + EachResult(id="user1", eppn=[], email=[], user_name="User 1", groups=set(), status="create", code=None) ] summary = {"error": 0} upload_data = mocker.MagicMock() @@ -738,7 +851,7 @@ def test_update_users_success(app, mocker): return_value=BulkResponse(operations=[BulkOperation(method="POST", path="/Users/user1", status="201")]), ) - result = bulks.update_users(history_id, temp_file_id, delete_users=None) + result = bulks.update_users(history_id, temp_file_id, remove_users=None) assert result == history_id mock_update_status.assert_any_call( @@ -757,9 +870,9 @@ def test_update_users_history_not_found(app, mocker): history_id = uuid7() temp_file_id = uuid7() mocker.patch("server.services.history_table.get_upload_by_id", return_value=None) - with pytest.raises(ResourceNotFound) as exc: - bulks.update_users(history_id, temp_file_id, delete_users=None) - assert str(exc.value) == f"History not found: {history_id}" + with pytest.raises(RecordNotFound) as exc: + bulks.update_users(history_id, temp_file_id, remove_users=None) + assert str(exc.value) == str(E.FAILED_GET_UPLOAD_HISTORY_RECORD % {"history_id": history_id}) def test_update_users_error_in_summary(app, mocker): @@ -770,15 +883,15 @@ def test_update_users_error_in_summary(app, mocker): upload_data.results = {"results": [], "summary": {"error": 1}} mocker.patch("server.services.history_table.get_upload_by_id", return_value=upload_data) with pytest.raises(FileValidationError) as exc: - bulks.update_users(history_id, temp_file_id, delete_users=None) - assert str(exc.value) == "There are errors in the validation results." + bulks.update_users(history_id, temp_file_id, remove_users=None) + assert str(exc.value) == str(E.VALIDATION_ERROR_BLOCK) def test_update_users_bulk_oauth_token_error(app, mocker): history_id = uuid7() temp_file_id = uuid7() check_results = [ - CheckResult(id="user1", eppn=[], email=[], user_name="User 1", groups=set(), status="create", code=None) + EachResult(id="user1", eppn=[], email=[], user_name="User 1", groups=set(), status="create", code=None) ] summary = {"error": 0} upload_data = mocker.MagicMock() @@ -792,7 +905,7 @@ def test_update_users_bulk_oauth_token_error(app, mocker): expected_error_message = "OAuth tokens are not stored on the server." mocker.patch("server.services.bulks.get_access_token", side_effect=OAuthTokenError(expected_error_message)) with pytest.raises(OAuthTokenError) as exc: - bulks.update_users(history_id, temp_file_id, delete_users=None) + bulks.update_users(history_id, temp_file_id, remove_users=None) assert str(exc.value) == expected_error_message @@ -800,7 +913,7 @@ def test_update_users_map_error(app, mocker): history_id = uuid7() temp_file_id = uuid7() check_results = [ - CheckResult(id="user1", eppn=[], email=[], user_name="User 1", groups=set(), status="create", code=None) + EachResult(id="user1", eppn=[], email=[], user_name="User 1", groups=set(), status="create", code=None) ] summary = {"error": 0} upload_data = mocker.MagicMock() @@ -817,16 +930,16 @@ def test_update_users_map_error(app, mocker): "server.services.bulks.bulks.post", return_value=MapError(status="400", scim_type="invalidValue", detail="error"), ) - with pytest.raises(ResourceInvalid): - bulks.update_users(history_id, temp_file_id, delete_users=None) + with pytest.raises(UnexpectedResponseError): + bulks.update_users(history_id, temp_file_id, remove_users=None) def test_update_users_failed(app, mocker): history_id = uuid7() temp_file_id = uuid7() check_results = [ - CheckResult(id="user1", eppn=[], email=[], user_name="User 1", groups=set(), status="create", code=None), - CheckResult(id="user2", eppn=[], email=[], user_name="User 2", groups=set(), status="update", code=None), + EachResult(id="user1", eppn=[], email=[], user_name="User 1", groups=set(), status="create", code=None), + EachResult(id="user2", eppn=[], email=[], user_name="User 2", groups=set(), status="update", code=None), ] summary = {"error": 0} upload_data = mocker.MagicMock() @@ -849,7 +962,7 @@ def test_update_users_failed(app, mocker): ), ) - result = bulks.update_users(history_id, temp_file_id, delete_users=None) + result = bulks.update_users(history_id, temp_file_id, remove_users=None) assert result == history_id @@ -868,26 +981,19 @@ def test_save_file(app, mocker: MockerFixture): mocker.patch("pathlib.Path.exists", return_value=True) mocker.patch("server.services.history_table.create_file", return_value=None) mocker.patch("pathlib.Path.rename") - return_fnc = mocker.patch("server.services.history_table.create_file", return_value=None) file_id = uuid7() - bulks.save_file(file_id) - mock_get_file_by_id.assert_called_once_with(file_id) - return_fnc.assert_called_once() - - -def test_save_file_saved(app, mocker: MockerFixture): - file_content = _FileContent( - repositories=[{"id": "repo1", "serviceName": "test_service"}], - groups=[{"id": "group1", "displayName": "Group 1"}, {"id": "group2", "displayName": "Group 2"}], - users=[{"id": "user1", "userName": "User 1"}, {"id": "user2", "userName": "User 2"}], + new_file_id = uuid7() + mock_files = Files() + mock_files.id = new_file_id + mock_files.file_content = file_content + mock_files.file_path = file_path + return_fnc = mocker.patch( + "server.services.history_table.create_file", + return_value=mock_files, ) - mock_get_file_by_id = mocker.patch( - "server.services.history_table.get_file_by_id", return_value=mocker.MagicMock(file_content=file_content) - ) - mocker.patch("server.services.history_table.create_file", return_value=None) - file_id = uuid7() - bulks.save_file(file_id) + assert bulks.save_file(file_id) == new_file_id mock_get_file_by_id.assert_called_once_with(file_id) + return_fnc.assert_called_once() def test_save_file_with_exception(app, mocker: MockerFixture): @@ -900,7 +1006,7 @@ def test_save_file_with_exception(app, mocker: MockerFixture): mocker.patch( "server.services.history_table.get_file_by_id", return_value=mocker.MagicMock(file_content=file_content) ) - with pytest.raises(ResourceNotFound) as exc: + with pytest.raises(FileNotFound) as exc: bulks.save_file(file_id) assert str(exc.value) is not None @@ -918,10 +1024,10 @@ def test_save_file_not_found(app, mocker: MockerFixture): ) mocker.patch("server.services.history_table.create_file", return_value=None) file_id = uuid7() - with pytest.raises(ResourceNotFound) as exc: + with pytest.raises(FileNotFound) as exc: bulks.save_file(file_id) mock_get_file_by_id.assert_called_once_with(file_id) - assert str(exc.value) == f"File not found: {file_path}" + assert str(exc.value) == str(E.FILE_EXPIRED % {"path": file_path}) def test_save_file_invalid_suffix(app, mocker: MockerFixture): @@ -938,14 +1044,14 @@ def test_save_file_invalid_suffix(app, mocker: MockerFixture): mocker.patch("pathlib.Path.exists", return_value=True) mocker.patch("server.services.history_table.create_file", return_value=None) file_id = uuid7() - with pytest.raises(ResourceInvalid) as exc: + with pytest.raises(FileFormatError) as exc: bulks.save_file(file_id) mock_get_file_by_id.assert_called_once_with(file_id) - assert str(exc.value) == "not supported file format." + assert str(exc.value) == str(E.FILE_FORMAT_UNSUPPORTED % {"suffix": Path(file_path).suffix}) def test_build_map_user_from_check_result(app, mocker: MockerFixture): - check_result = CheckResult( + check_result = EachResult( id="user1", eppn=["test@eppn"], email=["test@example.com"], @@ -995,7 +1101,7 @@ def test_build_remove_user_path(app, mocker: MockerFixture): def test__build_bulk_operations_from_check_results(app, mocker: MockerFixture): repository_id = "repo1" check_results = [ - CheckResult( + EachResult( id="user1", eppn=["test@eppn"], email=["test@example.com"], @@ -1004,7 +1110,7 @@ def test__build_bulk_operations_from_check_results(app, mocker: MockerFixture): status="create", code=None, ), - CheckResult( + EachResult( id="user2", eppn=["test@eppn"], email=["test2@example.com"], @@ -1013,7 +1119,7 @@ def test__build_bulk_operations_from_check_results(app, mocker: MockerFixture): status="update", code=None, ), - CheckResult( + EachResult( id="user3", eppn=["test@eppn"], email=["test3@example.com"], @@ -1023,7 +1129,7 @@ def test__build_bulk_operations_from_check_results(app, mocker: MockerFixture): code=None, ), ] - delete_users = ["user1"] + remove_users = ["user1"] expected = ( [ BulkOperation( @@ -1096,7 +1202,7 @@ def test__build_bulk_operations_from_check_results(app, mocker: MockerFixture): groups=[_Group(repository_id=repository_id, group_id="jc_repo1_gr_group1", user_defined_id="group1")], ), ) - result = bulks._build_bulk_operations_from_check_results(repository_id, check_results, delete_users) # noqa: SLF001 + result = bulks._build_bulk_operations_from_check_results(repository_id, check_results, remove_users) # noqa: SLF001 assert len(result[0]) == len(expected[0]) for r, e in zip(result[0], expected[0], strict=False): assert r.method == e.method @@ -1137,6 +1243,31 @@ def test__build_groups_update_bulk_operations_no_group(app, mocker: MockerFixtur ] +def test_get_execute_task_result(app, mocker: MockerFixture): + task_id = "test_task_id" + expected = mocker.MagicMock() + mocker.patch("server.services.bulks.update_users.AsyncResult", return_value=expected) + assert bulks.get_execute_task_result(task_id) == expected + + +def test_get_execute_task_result_none(app, mocker: MockerFixture): + task_id = "test_task_id" + mocker.patch("server.services.bulks.update_users.AsyncResult", return_value=None) + expected = E.TASK_NOT_FOUND % {"task_id": "test_task_id"} + with pytest.raises(TaskExcutionError) as exc: + bulks.get_execute_task_result(task_id) + assert str(exc.value) == str(expected) + + +def test_get_execute_task_result_with_exception(app, mocker: MockerFixture): + task_id = "test_task_id" + expected_exception = RedisConnectionError("Failed to connect to Redis") + mocker.patch("server.services.bulks.update_users.AsyncResult", side_effect=expected_exception) + with pytest.raises(DatastoreError) as exc: + bulks.get_execute_task_result(task_id) + assert str(exc.value) == str(E.FAILED_CONNECT_REDIS % {"error": str(expected_exception)}) + + def test_get_upload_result(app, mocker: MockerFixture): history_id = uuid7() status_filter = ["create"] @@ -1147,7 +1278,7 @@ def test_get_upload_result(app, mocker: MockerFixture): operator = "test_operator" start_timestamp = datetime(2026, 1, 1, 0, 0, 0, tzinfo=UTC) items = [ - CheckResult( + EachResult( id="user1", eppn=["test@eppn"], email=["test@example.com"], @@ -1162,14 +1293,14 @@ def test_get_upload_result(app, mocker: MockerFixture): file_id=file_id, file=mock_file, operator_name=operator, timestamp=start_timestamp, end_timestamp=None ) raw_results = [i.model_dump() for i in items] - summary = HistorySummary(create=1, update=1, delete=0, skip=0, error=0) + summary = ResultSummary(create=1, update=1, delete=0, skip=0, error=0) mocker.patch("server.services.history_table.get_upload_by_id", return_value=upload) mocker.patch("server.services.history_table.get_paginated_upload_results", return_value=raw_results) mocker.patch("server.services.history_table.get_upload_results", return_value=summary) result = bulks.get_upload_result(history_id, status_filter, offset, size) - expected = ResultSummary( + expected = ExecuteResults( items=items, summary=summary, file_id=file_id, @@ -1185,11 +1316,11 @@ def test_get_upload_result(app, mocker: MockerFixture): def test_get_upload_result_history_not_found(app, mocker: MockerFixture): history_id = uuid7() - expected_error_message = f"upload history not found: {history_id}" + expected = E.UPDATE_HISTORY_RECORD_NOT_FOUND % {"id": history_id} mocker.patch("server.services.history_table.get_upload_by_id", return_value=None) with pytest.raises(RecordNotFound) as exc: bulks.get_upload_result(history_id, status_filter=["create"], offset=1, size=20) - assert str(exc.value) == expected_error_message + assert str(exc.value) == str(expected) @pytest.mark.parametrize( @@ -1212,3 +1343,47 @@ def test_delete_temporary_file_with_exception(app, mocker: MockerFixture): file_id = uuid7() mocker.patch("server.services.history_table.get_file_by_id", side_effect=RecordNotFound("")) assert bulks.delete_temporary_file(str(file_id)) is None + + +def test_chack_permission_to_operation(app, mocker: MockerFixture): + mock_upload_record = UploadHistory() + mock_upload_record.operator_id = "user1" + history_id = uuid7() + mock_get_upload = mocker.patch("server.services.history_table.get_upload_by_id", return_value=mock_upload_record) + assert bulks.chack_permission_to_operation(history_id, "user1") is True + mock_get_upload.assert_called_once_with(history_id) + + +def test_chack_permission_to_operation_no_history(app, mocker: MockerFixture): + history_id = uuid7() + expected = E.UPDATE_HISTORY_RECORD_NOT_FOUND % {"id": history_id} + mocker.patch("server.services.history_table.get_upload_by_id", return_value=None) + with pytest.raises(RecordNotFound) as exc: + bulks.chack_permission_to_operation(history_id, "user1") + assert str(exc.value) == str(expected) + + +def test_chack_permission_to_view(app, mocker: MockerFixture): + history_id = uuid7() + mock_upload_record = mocker.MagicMock() + mock_upload_record.file.file_content["repositories"][0]["id"] = "repo1" + mock_upload_record.public = False + mocker.patch("server.services.bulks.get_permitted_repository_ids", return_value=set()) + mocker.patch("server.services.history_table.get_upload_by_id", return_value=mock_upload_record) + assert bulks.chack_permission_to_view(history_id) is False + + +def test_chack_permission_to_view_is_system_admin(app, mocker: MockerFixture): + history_id = uuid7() + mocker.patch("server.services.bulks.is_current_user_system_admin", return_value=True) + assert bulks.chack_permission_to_view(history_id) is True + + +def test_chack_permission_to_view_history_none(app, mocker: MockerFixture): + history_id = uuid7() + mocker.patch("server.services.bulks.get_permitted_repository_ids", return_value=set()) + mocker.patch("server.services.history_table.get_upload_by_id", return_value=None) + expected = E.UPDATE_HISTORY_RECORD_NOT_FOUND % {"id": history_id} + with pytest.raises(RecordNotFound) as exc: + bulks.chack_permission_to_view(history_id) + assert str(exc.value) == str(expected) diff --git a/tests/unit/services/test_groups.py b/tests/unit/services/test_groups.py index 4b62d3f..9fa7fb6 100644 --- a/tests/unit/services/test_groups.py +++ b/tests/unit/services/test_groups.py @@ -1005,7 +1005,7 @@ def test_update_raises_resource_not_found(app: Flask, gen_group_id, mocker: Mock mocker.patch("server.services.groups.get_by_id", return_value=None) mocker.patch("server.clients.groups.patch_by_id", return_value=None) - msg: str = "E204 | Group resource (id: jc_repo_id_groups_g202_test) not found." + msg: str = "E204 | Group resource (id: jc_repo_id_gr_g202_test) not found." with pytest.raises(ResourceNotFound, match=msg): groups.update(updated_group) @@ -1983,6 +1983,7 @@ def test_delete_multiple_all_failure(app, gen_group_id, mocker: MockerFixture) - def test_delete_multiple_raises_resource_invalid_and_logs(app: Flask, gen_group_id, mocker: MockerFixture) -> None: """Test delete_multiple raises ResourceInvalid and logs when MapError is returned.""" + mocker.patch("server.config.config.FEATURES.enable_bulk_operation", return_value=True) group_ids: set[str] = {gen_group_id("g1"), gen_group_id("g2")} mocker.patch("server.services.groups.get_access_token", return_value="token") mocker.patch("server.services.groups.get_client_secret", return_value="secret") @@ -2286,7 +2287,7 @@ def test_delete_by_id_map_error_not_found(app, gen_group_id, mocker): "server.clients.groups.delete_by_id", return_value=MapError(detail=f"Group '{group_id}' Not Found", status="404", scim_type="noTarget"), ) - msg: str = "E204 | Group resource (id: jc_repo_id_groups_g200_test) not found." + msg: str = "E204 | Group resource (id: jc_repo_id_gr_g200_test) not found." with pytest.raises(ResourceNotFound, match=msg): groups.delete_by_id(group_id) @@ -2335,7 +2336,7 @@ def test_update_member_add_and_remove(app: Flask, gen_group_id, mocker: MockerFi group_id: str = gen_group_id("g113") same_user: str = "user12" - msg: str = "E260 | Conflict in updating Group members (id: jc_repo_id_groups_g113_test, users: user12)." + msg: str = "E260 | Conflict in updating Group members (id: jc_repo_id_gr_g113_test, users: user12)." with pytest.raises(RequestConflict, match=msg): groups.update_member(group_id, add={same_user}, remove={same_user}) @@ -2508,7 +2509,7 @@ def test_update_member_get_by_id_none(app, gen_group_id, mocker): group_id = gen_group_id("g301") mocker.patch.object(groups.config.MAP_CORE, "update_strategy", new="patch") mocker.patch("server.services.groups.get_by_id", return_value=None) - msg: str = "E204 | Group resource (id: jc_repo_id_groups_g301_test) not found." + msg: str = "E204 | Group resource (id: jc_repo_id_gr_g301_test) not found." with pytest.raises(ResourceNotFound, match=msg): groups.update_member(group_id, add={"u1"}, remove={"u2"}) @@ -2606,7 +2607,7 @@ def test_update_member_put_direct_success(app, gen_group_id, mocker): def test_update_member_put_not_found(app, gen_group_id, mocker): group_id = gen_group_id("g401") mocker.patch("server.services.groups.get_by_id", return_value=None) - msg: str = "E204 | Group resource (id: jc_repo_id_groups_g401_test) not found." + msg: str = "E204 | Group resource (id: jc_repo_id_gr_g401_test) not found." with pytest.raises(ResourceNotFound, match=msg): groups.update_member_put(group_id, {"u1"}, {"u2"}) @@ -2817,7 +2818,7 @@ def test_update_member_put_request_conflict(app, gen_group_id, mocker): add = {"user1", "user2"} remove = {"user2", "user3"} mocker.patch.object(groups.config.MAP_CORE, "update_strategy", new="put") - msg: str = "E260 | Conflict in updating Group members (id: jc_repo_id_groups_g_mem_conflict_test, users: user2)." + msg: str = "E260 | Conflict in updating Group members (id: jc_repo_id_gr_g_mem_conflict_test, users: user2)." with pytest.raises(RequestConflict, match=msg): groups.update_member_put(group_id, add, remove) @@ -2842,7 +2843,7 @@ def test_update_member_put_map_error_resource_not_found(app, gen_group_id, mocke mocker.patch("server.services.groups.get_client_secret", return_value="secret") logger_mock = mocker.patch("flask.current_app.logger.error") map_error = MapError( - detail="E204 | Group resource (id: jc_repo_id_groups_g_mem_nf_test) not found.", + detail="E204 | Group resource (id: jc_repo_id_gr_g_mem_nf_test) not found.", status="404", scim_type="invalidValue", ) @@ -2878,7 +2879,7 @@ def test_update_member_map_not_found_pattern_raises_resource_not_found(app, gen_ mocker.patch("server.services.utils.build_update_member_operations") map_error = MapError(detail=r"'(.*)' Not Found", status="404", scim_type="noTarget") mocker.patch("server.clients.groups.patch_by_id", return_value=map_error) - msg = "E204 | Group resource (id: jc_repo_id_groups_g_map_nf_test) not found." + msg = "E204 | Group resource (id: jc_repo_id_gr_g_map_nf_test) not found." with pytest.raises(ResourceNotFound, match=msg): groups.update_member(group_id, add, remove) @@ -2936,13 +2937,13 @@ def test_update_member_put_http_error_branch(app, gen_group_id, mocker, status_c "401", ResourceNotFound, r"'(.*)' Not Found", - "E204 | Group resource (id: jc_repo_id_groups_g_mem_http_test) not found.", + "E204 | Group resource (id: jc_repo_id_gr_g_mem_http_test) not found.", ), ( "500", OAuthTokenError, r"No update rights for '(.*)'", - "E223 | No update rights for Group (id: jc_repo_id_groups_g_mem_http_test) with current access token.", + "E223 | No update rights for Group (id: jc_repo_id_gr_g_mem_http_test) with current access token.", ), ("409", UnexpectedResponseError, "", "E051 | Received unexpected response from mAP Core API."), ], @@ -3375,7 +3376,7 @@ def test_update_member_map_no_rights_update_pattern_raises_oauth_token_error(app def test_update_member_put_map_no_rights_update_pattern_raises_oauth_token_error(app, gen_group_id, mocker): """Test update_member_put raises ResourceNotFound when MapError.detail matches MAP_NOT_FOUND_PATTERN.""" group_id = gen_group_id("g_mem_nf") - error_msg = "E223 | No update rights for Group (id: jc_repo_id_groups_g_mem_nf_test) with current access token." + error_msg = "E223 | No update rights for Group (id: jc_repo_id_gr_g_mem_nf_test) with current access token." add = {"user1"} remove = set() dummy_group = MapGroup( diff --git a/tests/unit/services/test_history.py b/tests/unit/services/test_history.py index 8f2daf5..b304c07 100644 --- a/tests/unit/services/test_history.py +++ b/tests/unit/services/test_history.py @@ -1,7 +1,7 @@ import typing as t from datetime import UTC, date, datetime -from uuid import UUID +from uuid import UUID, uuid7 import pytest @@ -9,10 +9,11 @@ from sqlalchemy.exc import SQLAlchemyError from server.db.history import DownloadHistory, Files, UploadHistory, _FileContent, _ResultData -from server.entities.history_detail import DownloadHistoryData, HistoryQuery, HistorySummary, UploadHistoryData +from server.entities.history_detail import DownloadHistoryData, HistoryQuery, UploadHistoryData from server.entities.search_request import SearchResult from server.entities.summaries import UserSummary from server.exc import DatabaseError, RecordNotFound +from server.messages import E from server.services import history @@ -104,7 +105,6 @@ last_modified=None, ), status="P", - summary=HistorySummary(create=0, update=0, delete=0, skip=0, error=0), file_path="ver/tmp/2026/01/test_file.csv", file_id=UUID("019c794d-fb97-70f2-aba2-c3888973190d"), repository_count=1, @@ -334,18 +334,20 @@ def test_update_public_status_not_found(app, mocker: MockerFixture): db = mocker.MagicMock() mocker.patch("server.services.history.db", db) db.session.get.return_value = None + history_id = uuid7() with pytest.raises(RecordNotFound) as exc: - history.update_public_status(tab="upload", history_id=UUID("019c794e-04c0-7403-8621-b04c40698107"), public=True) - assert str(exc.value) == "019c794e-04c0-7403-8621-b04c40698107 is not found" + history.update_public_status(tab="upload", history_id=history_id, public=True) + assert str(exc.value) == str(E.FAILED_GET_HISTORY_RECORD % {"history_id": history_id, "table": "upload"}) def test_update_public_status_db_error(app, mocker: MockerFixture): db = mocker.MagicMock() mocker.patch("server.services.history.db", db) db.session.get.side_effect = SQLAlchemyError + history_id = uuid7() with pytest.raises(DatabaseError) as exc: - history.update_public_status(tab="upload", history_id=UUID("019c794e-04c0-7403-8621-b04c40698107"), public=True) - assert str(exc.value) == "Failed to update the public status due to a database error." + history.update_public_status(tab="upload", history_id=history_id, public=True) + assert str(exc.value) == str(E.FAILED_UPDATE_PUBLIC % {"history_id": history_id}) def test_update_public_status(app, mocker: MockerFixture): @@ -371,18 +373,20 @@ def test_get_file_path_not_found(app, mocker: MockerFixture): db = mocker.MagicMock() mocker.patch("server.services.history.db", db) db.session.get.return_value = None + file_id = uuid7() with pytest.raises(RecordNotFound) as exc: - history.get_file_path(UUID("019c794e-04c0-7403-8621-b04c40698107")) - assert str(exc.value) == "File with ID 019c794e-04c0-7403-8621-b04c40698107 not found." + history.get_file_path(file_id) + assert str(exc.value) == str(E.FAILED_GET_FILE_PATH % {"file_id": file_id}) def test_get_file_path_db_error(app, mocker: MockerFixture): db = mocker.MagicMock() mocker.patch("server.services.history.db", db) db.session.get.side_effect = SQLAlchemyError + file_id = uuid7() with pytest.raises(DatabaseError) as exc: - history.get_file_path(UUID("019c794e-04c0-7403-8621-b04c40698107")) - assert str(exc.value) == "Failed to retrieve the file path due to a database error." + history.get_file_path(file_id) + assert str(exc.value) == str(E.FAILED_GET_FILE_PATH % {"file_id": file_id}) def test_empty_history_criteria(): diff --git a/tests/unit/services/test_history_table.py b/tests/unit/services/test_history_table.py index 0f04321..fd2f7eb 100644 --- a/tests/unit/services/test_history_table.py +++ b/tests/unit/services/test_history_table.py @@ -4,7 +4,11 @@ import pytest -from server.exc import InvalidQueryError, InvalidRecordError, RecordNotFound +from sqlalchemy.exc import SQLAlchemyError + +from server.db.history import UploadHistory +from server.exc import DatabaseError, InvalidQueryError, InvalidRecordError, RecordNotFound +from server.messages import E from server.services import history_table @@ -20,6 +24,15 @@ def test_get_upload_by_id(app, mocker: MockerFixture): assert result_fnc.call_args[0][1] == history_id +def test_get_upload_by_id_with_exception(app, mocker: MockerFixture): + history_id = uuid7() + result_fnc = mocker.patch("server.db.db.session.get", side_effect=SQLAlchemyError) + with pytest.raises(DatabaseError) as exc: + history_table.get_upload_by_id(history_id) + result_fnc.assert_called_once() + assert str(exc.value) == str(E.FAILED_GET_UPLOAD_HISTORY_RECORD % {"history_id": history_id}) + + def test_get_upload_results(app, mocker: MockerFixture): history_id = uuid7() attribute = "results" @@ -31,6 +44,20 @@ def test_get_upload_results(app, mocker: MockerFixture): result_fnc.assert_called_once() +def test_get_upload_results_with_exception(app, mocker: MockerFixture): + history_id = uuid7() + attribute = "results" + result_fnc = mocker.patch( + "server.db.db.session.query", + return_value=mocker.MagicMock(filter=mocker.MagicMock(first=None)), + side_effect=SQLAlchemyError, + ) + with pytest.raises(DatabaseError) as exc: + history_table.get_upload_results(history_id, attribute) + result_fnc.assert_called_once() + assert str(exc.value) == str(E.FAILED_GET_UPLOAD_HISTORY_RECORD % {"history_id": history_id}) + + @pytest.mark.parametrize( ("status_filter"), [(["S", "F"]), ([])], @@ -47,14 +74,30 @@ def test_get_paginated_upload_results(app, mocker: MockerFixture, status_filter) mock_db.assert_called_once() -def test_get_paginated_upload_results_with_exceptions(app, mocker: MockerFixture): +def test_get_paginated_upload_results_invalid_query(app, mocker: MockerFixture): history_id = uuid7() offset = 0 size = 10 status_filter = ["S", "F"] with pytest.raises(InvalidQueryError) as exc: history_table.get_paginated_upload_results(history_id, offset, size, status_filter) - assert str(exc.value) == "Invalid offset or size" + assert str(exc.value) == str(E.INVALID_QUERY % {"offset": offset, "size": size}) + + +def test_get_paginated_upload_results_with_exception(app, mocker: MockerFixture): + history_id = uuid7() + offset = 2 + size = 10 + status_filter = ["S", "F"] + mock_db = mocker.patch( + "server.db.db.session.query", + return_value=mocker.MagicMock(filter=mocker.MagicMock(first=None)), + side_effect=SQLAlchemyError, + ) + with pytest.raises(DatabaseError) as exc: + history_table.get_paginated_upload_results(history_id, offset, size, status_filter) + mock_db.assert_called_once() + assert str(exc.value) == str(E.FAILED_GET_UPLOAD_HISTORY_RECORD % {"history_id": history_id}) def test_create_upload(app, mocker: MockerFixture): @@ -65,12 +108,10 @@ def test_create_upload(app, mocker: MockerFixture): mock_add = mocker.patch("server.db.db.session.add") mock_commit = mocker.patch("server.db.db.session.commit") mock_upload = mocker.patch("server.services.history_table.UploadHistory", autospec=True) - instance = mock_upload.return_value - instance.id = "dummy-id" ret = history_table.create_upload(file_id, results, operator_id, operator_name) - mock_add.assert_called_once_with(instance) + mock_add.assert_called_once_with(mock_upload.return_value) mock_commit.assert_called_once() - assert ret == "dummy-id" + assert isinstance(ret, UploadHistory) def test_create_upload_not_summary(app): @@ -80,24 +121,31 @@ def test_create_upload_not_summary(app): operator_name = "opname" with pytest.raises(InvalidRecordError) as exc: history_table.create_upload(file_id, results, operator_id, operator_name) - assert str(exc.value) == "Results must include 'summary' and 'results' keys" + assert str(exc.value) == str(E.INVALID_UPLOAD_HISTORY_RECORD_ATTRIBUTES) -def test_update_upload_status(app, mocker: MockerFixture): - class Dummy: - pass +def test_create_upload_with_exception(app, mocker: MockerFixture): + file_id = uuid7() + results = {"summary": {"a": 1}, "results": [1, 2], "missing_users": ["x"]} + operator_id = "opid" + operator_name = "opname" + mock_add = mocker.patch("server.db.db.session.add", side_effect=SQLAlchemyError) + with pytest.raises(DatabaseError) as exc: + history_table.create_upload(file_id, results, operator_id, operator_name) + mock_add.assert_called_once() + assert str(exc.value) == str(E.FAILED_CREATE_UPLOAD_HISTORY_RECORD % {"file_id": file_id}) + +def test_update_upload_status(app, mocker: MockerFixture): history_id = uuid7() status = "S" new_results = {"summary": {}, "results": [], "missing_users": []} file_id = uuid7() mock_get = mocker.patch("server.db.db.session.get") - obj = Dummy() + obj = UploadHistory() mock_get.return_value = obj - mock_commit = mocker.patch("server.db.db.session.commit") history_table.update_upload_status(history_id, status, new_results, file_id) mock_get.assert_called_once_with(history_table.UploadHistory, history_id) - mock_commit.assert_called_once() assert obj.status == status assert obj.file_id == file_id assert obj.results["summary"] == {} @@ -114,39 +162,29 @@ def test_update_upload_status_object_not_found(app, mocker: MockerFixture): def test_update_upload_status_no_new_results(app, mocker: MockerFixture): - class Dummy: - pass - history_id = uuid7() status = "P" new_results = None file_id = uuid7() mock_get = mocker.patch("server.db.db.session.get") - obj = Dummy() + obj = UploadHistory() mock_get.return_value = obj - mock_commit = mocker.patch("server.db.db.session.commit") history_table.update_upload_status(history_id, status, new_results, file_id) mock_get.assert_called_once_with(history_table.UploadHistory, history_id) - mock_commit.assert_called_once() assert obj.status == status assert obj.file_id == file_id def test_update_upload_status_no_file_id(app, mocker: MockerFixture): - class Dummy: - pass - history_id = uuid7() status = "S" new_results = None file_id = None mock_get = mocker.patch("server.db.db.session.get") - obj = Dummy() + obj = UploadHistory() mock_get.return_value = obj - mock_commit = mocker.patch("server.db.db.session.commit") history_table.update_upload_status(history_id, status, new_results, file_id) mock_get.assert_called_once_with(history_table.UploadHistory, history_id) - mock_commit.assert_called_once() assert obj.status == status @@ -172,7 +210,7 @@ def test_get_history_by_file_id_not_found(app, mocker: MockerFixture): mocker.patch("server.db.db.session.query", return_value=mock_query) with pytest.raises(RecordNotFound) as exc: history_table.get_history_by_file_id(file_id) - assert str(exc.value) == f"History not found for file_id: {file_id}" + assert str(exc.value) == str(E.FAILED_GET_UPLOAD_HISTORY_RECORD_BY_FILE_ID % {"file_id": file_id}) def test_get_file_by_id(app, mocker): @@ -197,7 +235,7 @@ def test_get_file_by_id_not_found(app, mocker): mocker.patch("server.db.db.session.query", return_value=mock_query) with pytest.raises(RecordNotFound) as exc: history_table.get_file_by_id(file_id) - assert str(exc.value) == f"File not found for file_id: {file_id}" + assert str(exc.value) == str(E.FAILED_GET_FILE_RECORD % {"file_id": file_id}) def test_delete_file_by_id(app, mocker: MockerFixture): @@ -206,10 +244,8 @@ def test_delete_file_by_id(app, mocker: MockerFixture): "server.services.history_table.Files", return_value=mocker.MagicMock(query=mocker.MagicMock(filter=mocker.MagicMock(delete=mocker.MagicMock()))), ) - mock_commit = mocker.patch("server.db.db.session.commit") history_table.delete_file_by_id(file_id) mock_files.query.filter.assert_called_once() - mock_commit.assert_called_once() def test_create_file(app, mocker: MockerFixture): @@ -220,11 +256,11 @@ def test_create_file(app, mocker: MockerFixture): instance = mock_files.return_value instance.id = file_id mock_add = mocker.patch("server.db.db.session.add") - mock_commit = mocker.patch("server.db.db.session.commit") - ret = history_table.create_file(file_path, file_content, file_id) + result = history_table.create_file(file_path, file_content, file_id) mock_add.assert_called_once_with(instance) - mock_commit.assert_called_once() - assert ret == file_id + assert result.id == file_id + assert result.file_path == file_path + assert result.file_content == file_content def test_create_file_without_id(app, mocker: MockerFixture): @@ -235,8 +271,8 @@ def test_create_file_without_id(app, mocker: MockerFixture): instance = mock_files.return_value instance.id = file_id mock_add = mocker.patch("server.db.db.session.add") - mock_commit = mocker.patch("server.db.db.session.commit") - ret = history_table.create_file(file_path, file_content, file_id) + result = history_table.create_file(file_path, file_content, file_id) mock_add.assert_called_once_with(instance) - mock_commit.assert_called_once() - assert ret == file_id + assert result.id == file_id + assert result.file_path == file_path + assert result.file_content == file_content diff --git a/tests/unit/services/test_permissions.py b/tests/unit/services/test_permissions.py index b05cfb0..282be3a 100644 --- a/tests/unit/services/test_permissions.py +++ b/tests/unit/services/test_permissions.py @@ -1,11 +1,8 @@ import typing as t -import pytest - from flask_login import login_user from server.const import USER_ROLES -from server.entities import map_error, map_group, map_service, map_user from server.entities.login_user import LoginUser from server.services.utils import permissions from server.services.utils.affiliations import Affiliations, _Group, _RoleGroup @@ -97,7 +94,7 @@ def test_get_permitted_repository_ids_not_logged_in(mocker: MockerFixture): return_value=mock_affiliations_no_logged_in, ) permitted_ids = permissions.get_permitted_repository_ids() - assert permitted_ids == set() + assert permitted_ids == {"*"} def test_filter_permitted_group_ids(mocker: MockerFixture): @@ -127,98 +124,3 @@ def test_get_current_user_affiliations_not_logged_in(mocker: MockerFixture): mocker.patch("server.services.utils.permissions.is_user_logged_in", return_value=False) affiliations = permissions.get_current_user_affiliations() assert affiliations == mock_affiliations_no_logged_in - - -@pytest.mark.parametrize( - ("test_user", "expected_groups"), - [ - ( - test_sys_admin_user, - [ - map_service.Group(value="jc_repo2_groups_test_group2"), - map_service.Group(value="jc_repo3_groups_test_group3"), - ], - ), - ( - test_repo_admin_user, - [ - map_service.Group(value="jc_repo2_groups_test_group2"), - ], - ), - ], -) -def test_remove_info_outside_system_map_service(app, mocker: MockerFixture, test_user, expected_groups): - entity = map_service.MapService( - id="service1", - groups=[ - map_service.Group(value="jc_repo2_groups_test_group2"), - map_service.Group(value="external_group"), - map_service.Group(value="jc_repo3_groups_test_group3"), - ], - ) - mocker.patch( - "server.services.utils.permissions.detect_affiliations", - return_value=mock_affiliations, - ) - mocker.patch("server.services.utils.permissions.get_permitted_repository_ids", return_value={"repo1", "repo2"}) - with app.test_request_context("/"): - login_user(test_user) - filtered_entity = permissions.remove_info_outside_system(entity) - assert isinstance(filtered_entity, map_service.MapService) - if filtered_entity.groups is None: - filtered_entity.groups = [] - assert filtered_entity.groups == expected_groups - - -@pytest.mark.parametrize(("test_user", "expected_member_count"), [(test_sys_admin_user, 2), (test_repo_admin_user, 1)]) -def test_remove_info_outside_system_map_group(app, mocker: MockerFixture, test_user, expected_member_count): - entity = map_group.MapGroup( - id="jc_repo1_groups_test_group1", - members=[ - map_group.MemberGroup(value="jc_repo2_groups_test_group2"), - map_group.MemberGroup(value="jc_repo3_groups_test_group3"), - map_group.MemberGroup(value="external_group"), - ], - ) - mocker.patch( - "server.services.utils.permissions.detect_affiliations", - return_value=mock_affiliations, - ) - mocker.patch("server.services.utils.permissions.get_permitted_repository_ids", return_value={"repo1", "repo2"}) - with app.test_request_context("/"): - login_user(test_user) - filtered_entity = permissions.remove_info_outside_system(entity) - assert isinstance(filtered_entity, map_group.MapGroup) - if filtered_entity.members is None: - filtered_entity.members = [] - assert len(filtered_entity.members) == expected_member_count - - -@pytest.mark.parametrize(("test_user", "expected_group_count"), [(test_sys_admin_user, 2), (test_repo_admin_user, 1)]) -def test_remove_info_outside_system_map_user(app, mocker: MockerFixture, test_user, expected_group_count): - entity = map_user.MapUser( - id="user1", - groups=[ - map_user.Group(value="jc_repo2_groups_test_group2"), - map_user.Group(value="external_group"), - map_user.Group(value="jc_repo3_groups_test_group3"), - ], - ) - mocker.patch( - "server.services.utils.permissions.detect_affiliations", - return_value=mock_affiliations, - ) - mocker.patch("server.services.utils.permissions.get_permitted_repository_ids", return_value={"repo1", "repo2"}) - with app.test_request_context("/"): - login_user(test_user) - filtered_entity = permissions.remove_info_outside_system(entity) - assert isinstance(filtered_entity, map_user.MapUser) - if filtered_entity.groups is None: - filtered_entity.groups = [] - assert len(filtered_entity.groups) == expected_group_count - - -def test_remove_info_outside_system_other_entity(): - entity = map_error.MapError(status="404", scim_type="noTarget", detail="Not found") - filtered_entity = permissions.remove_info_outside_system(entity) - assert filtered_entity == entity diff --git a/tests/unit/services/test_search_queries.py b/tests/unit/services/test_search_queries.py index ffd6547..0990140 100644 --- a/tests/unit/services/test_search_queries.py +++ b/tests/unit/services/test_search_queries.py @@ -9,7 +9,8 @@ from server.api.schemas import GroupsQuery, RepositoriesQuery, UsersQuery from server.entities.search_request import SearchRequestParameter -from server.exc import InvalidQueryError +from server.exc import ConfigurationError, InvalidQueryError +from server.messages import E from server.services.utils import search_queries from server.services.utils.affiliations import Affiliations, _Group @@ -38,7 +39,7 @@ class DummyCriteria: with pytest.raises(InvalidQueryError) as exc: search_queries.build_search_query(DummyCriteria()) # pyright: ignore[reportArgumentType] - assert str(exc.value) == f"Unsupported criteria type: {type(DummyCriteria())}" + assert str(exc.value) == str(E.UNRECOGNIZED_SEARCH_CRITERIA) @pytest.mark.parametrize( @@ -67,7 +68,7 @@ class DummyCriteria: SearchRequestParameter( filter='(groups.value eq "jc_roles_sysadm_test") and (groups.value eq "jc_repo1_ro_radm_test")', start_index=None, - count=None, + count=20, sort_by="entityIds.value", sort_order="descending", ), @@ -110,7 +111,7 @@ def test_build_repositories_search_query( @pytest.mark.parametrize( - ("search_query", "is_system_admin", "permitted", "affiliations", "expected"), + ("search_query", "is_system_admin", "permitted", "affiliations", "affiliation", "expected"), [ ( GroupsQuery( @@ -128,6 +129,7 @@ def test_build_repositories_search_query( False, set(), Affiliations(roles=[], groups=[]), + _Group(repository_id="repo1", group_id="jc_repo1_gr_test1", user_defined_id="test1"), SearchRequestParameter( filter=( '((displayName co "test")) and (id eq "") and ' @@ -148,6 +150,7 @@ def test_build_repositories_search_query( roles=[], groups=[_Group(repository_id="repo1", group_id="jc_repo1_gr_test1", user_defined_id="test1")], ), + _Group(repository_id="repo1", group_id="jc_repo1_gr_test1", user_defined_id="test1"), SearchRequestParameter( filter='(id eq "jc_repo1_gr_test1") and (public eq false) and (memberListVisibility eq Private)', start_index=None, @@ -157,13 +160,25 @@ def test_build_repositories_search_query( ), ), ( - GroupsQuery(q=None, i=["jc_repo1_gr_test1"], r=None, u=None, s=None, v=2, k=None, d=None, p=None, l=None), + GroupsQuery( + q=None, + i=["jc_repo1_gr_test1"], + r=None, + u=None, + s=None, + v=2, + k=None, + d=None, + p=None, + l=None, + ), True, {"repo1"}, Affiliations( roles=[], groups=[_Group(repository_id="repo1", group_id="jc_repo1_gr_test1", user_defined_id="test1")], ), + _Group(repository_id="repo1", group_id="jc_repo1_gr_test1", user_defined_id="test1"), SearchRequestParameter( filter='(id eq "jc_repo1_gr_test1") and (memberListVisibility eq Hidden)', start_index=None, @@ -173,15 +188,16 @@ def test_build_repositories_search_query( ), ), ( - GroupsQuery(q=None, i=None, r=None, u=None, s=None, v=None, k=None, d=None, p=None, l=None), + GroupsQuery(q=None, i=None, r=None, u=None, s=None, v=None, k=None, d=None, p=-1, l=-1), False, {"repo1"}, Affiliations( roles=[], groups=[_Group(repository_id="repo1", group_id="jc_repo1_gr_test1", user_defined_id="test1")], ), + _Group(repository_id="repo1", group_id="jc_repo1_gr_test1", user_defined_id="test1"), SearchRequestParameter( - filter='id sw "jc_repo1"', start_index=None, count=20, sort_by=None, sort_order=None + filter='id sw "jc_repo1"', start_index=None, count=None, sort_by=None, sort_order=None ), ), ( @@ -189,20 +205,22 @@ def test_build_repositories_search_query( True, set(), Affiliations(roles=[], groups=[]), + _Group(repository_id="repo1", group_id="jc_repo1_gr_test1", user_defined_id="test1"), SearchRequestParameter(filter='id sw "jc_"', start_index=None, count=20, sort_by=None, sort_order=None), ), ], ) def test_build_groups_search_query( - app, mocker: MockerFixture, search_query, is_system_admin, permitted, affiliations, expected + app, mocker: MockerFixture, search_query, is_system_admin, permitted, affiliations, affiliation, expected ): mocker.patch("server.services.utils.search_queries.is_current_user_system_admin", return_value=is_system_admin) mocker.patch("server.services.utils.search_queries.get_permitted_repository_ids", return_value=permitted) mocker.patch("server.services.utils.search_queries.detect_affiliations", return_value=affiliations) + mocker.patch("server.services.utils.search_queries.detect_affiliation", return_value=affiliation) assert search_queries.build_groups_search_query(search_query) == expected -def test_build_users_search_query_invalid_query(app, mocker: MockerFixture): +def test_build_groups_search_query_invalid_query(app, mocker: MockerFixture): mocker.patch("server.services.utils.search_queries.is_current_user_system_admin", return_value=False) mocker.patch("server.services.utils.search_queries.get_permitted_repository_ids", return_value={"repo1"}) mocker.patch( @@ -210,7 +228,7 @@ def test_build_users_search_query_invalid_query(app, mocker: MockerFixture): ) with pytest.raises(InvalidQueryError) as exc: search_queries.build_groups_search_query(GroupsQuery(i=["jc_repo1_gr_test1"])) - assert str(exc.value) == "Invalid group filter criteria" + assert str(exc.value) == str(E.UNRECOGNIZED_SEARCH_CRITERIA) @pytest.mark.parametrize( @@ -585,6 +603,6 @@ def test__path_generator(app, mocker: MockerFixture, model, expected): def test__get_id_prefix_not_match(app, test_config, mocker: MockerFixture): mocker.patch("server.config.config.REPOSITORIES.id_patterns.sp_connector", "invalid_pattern") test_func = inspect.unwrap(search_queries._get_id_prefix) # noqa: SLF001 - with pytest.raises(InvalidQueryError) as exc: + with pytest.raises(ConfigurationError) as exc: test_func() - assert str(exc.value) == "Invalid user-defined group ID pattern" + assert str(exc.value) == str(E.INVALID_SERVER_CONFIG) diff --git a/tests/unit/services/test_transformers.py b/tests/unit/services/test_transformers.py index d9247db..bfd0892 100644 --- a/tests/unit/services/test_transformers.py +++ b/tests/unit/services/test_transformers.py @@ -10,7 +10,6 @@ from server.entities.group_detail import ( GroupDetail, Repository as GroupRepository, - Service as GroupService_, ) from server.entities.map_group import ( Administrator as GroupAdministrator, @@ -28,9 +27,10 @@ from server.entities.map_user import EPPN, Email, Group as UserGroup, MapUser, Meta as UserMeta from server.entities.repository_detail import RepositoryDetail from server.entities.search_request import SearchResult -from server.entities.summaries import GroupSummary, RepositorySummary, UserSummary +from server.entities.summaries import GroupSummary, RepositorySummary from server.entities.user_detail import RepositoryRole, UserDetail from server.exc import InvalidFormError, SystemAdminNotFound +from server.messages import E from server.services.utils import transformers from server.services.utils.affiliations import Affiliations, _Group, _RoleGroup @@ -90,7 +90,9 @@ def test_prepare_service(app, mocker: MockerFixture): expected_repository_id = "repo1" mocker.patch("server.services.utils.transformers.resolve_repository_id", return_value=expected_repository_id) administrators = {"admin1", "admin2"} - map_service, repository_id = transformers.prepare_service(RepositoryDetail(), administrators) + map_service, repository_id = transformers.prepare_service( + RepositoryDetail(id=expected_repository_id), administrators + ) assert repository_id == expected_repository_id assert map_service.id == expected.id assert map_service.service_name == expected.service_name @@ -120,7 +122,7 @@ def test_prepare_service_no_administrators(app, mocker: MockerFixture): administrators = set() with pytest.raises(SystemAdminNotFound) as exc: transformers.prepare_service(RepositoryDetail(), administrators) - assert str(exc.value) == "At least one administrator is required to create a repository." + assert str(exc.value) == str(E.REPOSITORY_REQUIRES_SYSTEM_ADMIN) def test_prepare_role_groups(app, test_config, mocker: MockerFixture): @@ -185,7 +187,7 @@ def test_prepare_role_groups_no_administrators(app, mocker: MockerFixture): mocker.patch("server.services.utils.transformers.resolve_service_id", return_value="jc_repo1_test") with pytest.raises(SystemAdminNotFound) as exc: transformers.prepare_role_groups(repository_id, service_name, administrators) - assert str(exc.value) == "At least one administrator is required to create a repository." + assert str(exc.value) == str(E.REPOSITORY_REQUIRES_SYSTEM_ADMIN) def test_make_repository_detail(app, mocker: MockerFixture): @@ -290,14 +292,14 @@ def test_make_repository_detail_more(app, mocker: MockerFixture): service_url=HttpUrl("https://FQDN/example.com/"), entity_ids=["https:///shibboleth-sp"], ), - "Service name is required to create a repository.", + str(E.REPOSITORY_REQUIRES_SERVICE_NAME), InvalidFormError, ), ( RepositoryDetail( id="test", service_name="test", entity_ids=["https:///shibboleth-sp"], service_url=None ), - "Service URL is required to create a repository.", + str(E.REPOSITORY_REQUIRES_SERVICE_URL), InvalidFormError, ), ( @@ -307,7 +309,7 @@ def test_make_repository_detail_more(app, mocker: MockerFixture): service_url=HttpUrl("https://FQDN/example.com/"), entity_ids=["https:///shibboleth-sp"], ), - "Service URL must contain a valid host.", + str(E.REPOSITORY_INVALID_SERVICE_URL), InvalidFormError, ), ( @@ -317,14 +319,14 @@ def test_make_repository_detail_more(app, mocker: MockerFixture): service_url=HttpUrl("https://example.com/very/very/very/very/very/very/very/long/url"), entity_ids=["https:///shibboleth-sp"], ), - "Service URL is too long.", + str(E.REPOSITORY_TOO_LONG_URL % {"max": 50}), InvalidFormError, ), ( RepositoryDetail( id="test", service_name="test", service_url=HttpUrl("https://FQDN/example.com/"), entity_ids=[] ), - "At least one entity ID is required to create a repository.", + str(E.REPOSITORY_REQUIRES_ENTITY_ID), InvalidFormError, ), ], @@ -384,11 +386,11 @@ def test_prepare_group(app, test_config, mocker: MockerFixture): def test_prepare_group_no_administrators(app, mocker: MockerFixture): detail = GroupDetail(type="group") administrators = set() - expected = "At least one administrator is required to create a repository." + expected = E.GROUP_REQUIRES_SYSTEM_ADMIN mocker.patch("server.services.utils.transformers.validate_group_to_map_group", return_value=(MapGroup(), "repo1")) with pytest.raises(SystemAdminNotFound) as exc: transformers.prepare_group(detail, administrators) - assert str(exc.value) == expected + assert str(exc.value) == str(expected) def test_make_group_detail(app, mocker: MockerFixture): @@ -412,9 +414,9 @@ def test_make_group_detail(app, mocker: MockerFixture): created=datetime(2026, 1, 1, 0, 0, 0, tzinfo=UTC), last_modified=datetime(2026, 1, 2, 0, 0, 0, tzinfo=UTC), ) - expected._users = [UserSummary(id="user1"), UserSummary(id="user2")] # noqa: SLF001 - expected._admins = [UserSummary(id="admin1"), UserSummary(id="admin2")] # noqa: SLF001 - expected._services = [GroupService_(id="jc_repo1_test")] # noqa: SLF001 + expected._users = ["user1", "user2"] # noqa: SLF001 + expected._admins = ["admin1", "admin2"] # noqa: SLF001 + expected._services = ["jc_repo1_test"] # noqa: SLF001 group_detail = transformers.make_group_detail(group) assert group_detail == expected @@ -478,10 +480,15 @@ def test_make_group_detail_more(app, mocker: MockerFixture, group, affiliation, ("group", "mode", "expected", "expectedarg"), [ ( - GroupDetail(display_name="Test Group", id="jc_repo1_gr_test_group", type="group"), + GroupDetail(display_name="Test Group", id="jc_repo1_gr_test_group_test", type="group"), "update", MapGroup(), - GroupDetail(display_name="Test Group", id="jc_repo1_gr_test_group", type="group"), + GroupDetail( + display_name="Test Group", + id="jc_repo1_gr_test_group_test", + repository=GroupRepository(id="repo1"), + type="group", + ), ), ( GroupDetail( @@ -529,6 +536,7 @@ def test_make_group_detail_more(app, mocker: MockerFixture, group, affiliation, ) def test_validate_group_to_map_group(app, mocker: MockerFixture, group, mode, expected, expectedarg): mocker.patch("server.services.repositories.get_by_id", return_value=RepositoryDetail(id="repo1")) + mocker.patch("server.services.utils.transformers.get_permitted_repository_ids", return_value={"repo1"}) mock_make_map_group = mocker.patch("server.services.utils.transformers.make_map_group", return_value=MapGroup()) assert transformers.validate_group_to_map_group(group=group, mode=mode) == expected mock_make_map_group.assert_called_once_with(expectedarg) @@ -537,24 +545,24 @@ def test_validate_group_to_map_group(app, mocker: MockerFixture, group, mode, ex @pytest.mark.parametrize( ("group", "mode", "repository_exist", "expected"), [ - (GroupDetail(display_name=None, type="group"), "create", False, "Display name is required to create a group."), + (GroupDetail(display_name=None, type="group"), "create", False, E.GROUP_REQUIRES_DISPLAY_NAME), ( GroupDetail(display_name="Test Group", id=None, type="group"), "update", False, - "Group ID is required to update a group.", + E.GROUP_REQUIRES_ID, ), ( GroupDetail(display_name="Test Group", repository=None, type="group"), "create", False, - "Repository ID is required to create a group.", + E.GROUP_REQUIRES_REPOSITORY, ), ( GroupDetail(display_name="Test Group", repository=GroupRepository(id="repo1"), type="group"), "create", False, - "Repository with ID 'repo1' does not exist.", + E.GROUP_REQUIRES_EXISTING_REPOSITORY % {"rid": "repo1"}, ), ( GroupDetail( @@ -562,7 +570,7 @@ def test_validate_group_to_map_group(app, mocker: MockerFixture, group, mode, ex ), "create", True, - "Group ID is required to create a group.", + E.GROUP_REQUIRES_USER_DEFINED_ID, ), ( GroupDetail( @@ -573,16 +581,17 @@ def test_validate_group_to_map_group(app, mocker: MockerFixture, group, mode, ex ), "create", True, - "Group ID is too long.", + E.GROUP_TOO_LONG_ID % {"rid": "repo1", "max": 50 - len("jc_") - len("_gr_") - len("repo1")}, ), ], ) def test_validate_group_to_map_group_error(app, mocker: MockerFixture, group, mode, repository_exist, expected): repository = RepositoryDetail(id="repo1") if repository_exist else None mocker.patch("server.services.repositories.get_by_id", return_value=repository) + mocker.patch("server.services.utils.transformers.get_permitted_repository_ids", return_value={"repo1"}) with pytest.raises(InvalidFormError) as exc: transformers.validate_group_to_map_group(group=group, mode=mode) - assert str(exc.value) == expected + assert str(exc.value) == str(expected) @pytest.mark.parametrize( @@ -602,7 +611,7 @@ def test_validate_group_to_map_group_error(app, mocker: MockerFixture, group, mo ), ), ( - ([UserSummary(id="user1")], [UserSummary(id="admin1")], [GroupService_(id="service1")]), + (["user1"], ["admin1"], ["service1"]), MapGroup( id="jc_repo1_gr_test_group_test", display_name="Test Group", @@ -701,7 +710,7 @@ def test_prepare_user(app, mocker: MockerFixture): def test_make_user_detail(app, mocker: MockerFixture, map_user, affiliations, is_system_admin, expected): mocker.patch("server.services.utils.transformers.get_permitted_repository_ids", return_value={"repo1"}) mocker.patch("server.services.utils.transformers.detect_affiliations", return_value=affiliations) - mocker.patch("server.services.utils.transformers.is_current_user_system_admin", return_value=is_system_admin) + mocker.patch("server.services.utils.transformers.is_super", return_value=is_system_admin) mocker.patch("server.services.utils.transformers.make_criteria_object", return_value=None) user_detail = transformers.make_user_detail(map_user) assert user_detail == expected @@ -743,7 +752,7 @@ def test_make_user_detail_more(app, mocker: MockerFixture, map_user, permitted_r "server.services.utils.transformers.get_permitted_repository_ids", return_value=permitted_repository_ids ) mocker.patch("server.services.utils.transformers.detect_affiliations", return_value=affiliations) - mocker.patch("server.services.utils.transformers.is_current_user_system_admin", return_value=False) + mocker.patch("server.services.utils.transformers.is_super", return_value=False) mocker.patch("server.services.utils.transformers.make_criteria_object", return_value=None) mocker.patch( "server.services.groups.search", @@ -766,17 +775,17 @@ def test_make_user_detail_more(app, mocker: MockerFixture, map_user, permitted_r [ ( UserDetail(id="user1", user_name=""), - "Username is required to create a user.", + E.USER_REQUIRES_USERNAME, InvalidFormError, ), ( UserDetail(id=None, user_name="Test User", eppns=[]), - "At least one eduPersonPrincipalName is required to create a user.", + E.USER_REQUIRES_EPPN, InvalidFormError, ), ( UserDetail(id="user1", user_name="Test User", eppns=["test_eppn"], emails=[]), - "At least one email is required to create a user.", + E.USER_REQUIRES_EMAIL, InvalidFormError, ), ( @@ -787,7 +796,7 @@ def test_make_user_detail_more(app, mocker: MockerFixture, map_user, permitted_r emails=["test@email.com"], groups=[GroupSummary(id="jc_not_repo_gr_test_group")], ), - "Cannot specify groups that do not exist.", + E.USER_REQUIRES_REPOSITORY, InvalidFormError, ), ( @@ -800,7 +809,7 @@ def test_make_user_detail_more(app, mocker: MockerFixture, map_user, permitted_r is_system_admin=True, repository_roles=[RepositoryRole(id="repo1", user_role=USER_ROLES.REPOSITORY_ADMIN)], ), - "System administrator cannot be affiliated with any repository.", + E.USER_NO_CREATE_SYSTEM_ADMIN, InvalidFormError, ), ], @@ -814,7 +823,7 @@ def test_validate_user_to_map_user_create(app, mocker: MockerFixture, user_detai ) with pytest.raises(expected_exception) as exc: transformers.validate_user_to_map_user(user_detail, mode="create") - assert str(exc.value) == expectedarg + assert str(exc.value) == str(expectedarg) @pytest.mark.parametrize( @@ -827,11 +836,11 @@ def test_validate_user_to_map_user_create(app, mocker: MockerFixture, user_detai eppns=["test_eppn"], emails=["test@email.com"], groups=[], - is_system_admin=False, - repository_roles=None, + is_system_admin=True, + repository_roles=[RepositoryRole(id="repo1", user_role=USER_ROLES.REPOSITORY_ADMIN)], ), None, - "At least one repository and role is required.", + str(E.USER_REQUIRES_NO_REPOSITORY), InvalidFormError, ), ( @@ -840,12 +849,12 @@ def test_validate_user_to_map_user_create(app, mocker: MockerFixture, user_detai user_name="Test User", eppns=["test_eppn"], emails=["test@email.com"], - groups=[], - is_system_admin=False, - repository_roles=[RepositoryRole(id="repo1", user_role=USER_ROLES.REPOSITORY_ADMIN)], + groups=[GroupSummary(id="jc_repo1_gr_test_group")], + is_system_admin=True, + repository_roles=None, ), None, - "Cannot specify repositories that do not exist.", + str(E.USER_REQUIRES_NO_GROUP), InvalidFormError, ), ( @@ -889,7 +898,7 @@ def test_validate_user_to_map_user_create(app, mocker: MockerFixture, user_detai user_name="Test User", eppns=["test_eppn"], emails=["test@email.com"], - groups=[GroupSummary(id="jc_repo1_ro_radm_test")], + groups=[GroupSummary(id="jc_repo1_gr_test_group")], is_system_admin=False, repository_roles=[ RepositoryRole(id="repo1", user_role=USER_ROLES.REPOSITORY_ADMIN), @@ -907,6 +916,16 @@ def test_validate_user_to_map_user_update( "server.services.repositories.get_by_id", return_value=exist_repository, ) + mocker.patch("server.services.utils.transformers.get_permitted_repository_ids", return_value={"repo1"}) + mocker.patch( + "server.services.utils.transformers.validate_user_roles", + return_value=[], + ) + mocker.patch( + "server.services.utils.transformers.is_super", + return_value=True, + ) + mocker.patch("server.services.utils.transformers.validate_user_groups", return_value=["jc_repo1_gr_test_group"]) if expected_exception: with pytest.raises(expected_exception) as exc: transformers.validate_user_to_map_user(user_detail, mode="update") diff --git a/tests/unit/test_auth.py b/tests/unit/test_auth.py index a375477..8f37027 100644 --- a/tests/unit/test_auth.py +++ b/tests/unit/test_auth.py @@ -1,6 +1,6 @@ import typing as t -from datetime import UTC, datetime, timedelta +from datetime import UTC, datetime from flask import session from flask_login import current_user, login_user @@ -36,19 +36,9 @@ def test_refresh_session_not_logged_in(app, datastore): _, account_store, _ = datastore with app.test_request_context("/"): res = auth.refresh_session() - assert res is None - account_store.expire.assert_not_called() - account_store.delete.assert_not_called() - - -def test_refresh_session_no_data(app, datastore): - _, account_store, _ = datastore - with app.test_request_context("/"): - login_user(mock_repoadmin_login_user) - res = auth.refresh_session() - assert res is None - account_store.expire.assert_not_called() - account_store.delete.assert_not_called() + assert res is None + account_store.expire.assert_not_called() + account_store.delete.assert_not_called() def test_refresh_session(app, datastore, mocker: MockerFixture): @@ -60,45 +50,42 @@ def test_refresh_session(app, datastore, mocker: MockerFixture): login_user(mock_repoadmin_login_user) session["_id"] = test_session_id res = auth.refresh_session() - assert res is None - account_store.expire.assert_called_once() + assert res is None + account_store.expire.assert_called_once() def test_refresh_session_over(app, datastore, mocker: MockerFixture): _, account_store, _ = datastore test_session_id = "test_session_id" - expired_login_date = (datetime.now(UTC) - timedelta(seconds=60 * 60 * 24)).isoformat() - account_store.hget.return_value = expired_login_date.encode("utf-8") with app.test_request_context("/"): + mocker.patch("server.config.config.SESSION.absolute_lifetime", 0) login_user(mock_repoadmin_login_user) session["_id"] = test_session_id res = auth.refresh_session() - assert res is None - account_store.delete.assert_called_once() + assert res is None + account_store.delete.assert_called_once() def test_refresh_session_absolute(app, datastore, mocker: MockerFixture): - mocker.patch("server.config.config.SESSION.strategy", "absolute") _, account_store, _ = datastore with app.test_request_context("/"): + mocker.patch("server.config.config.SESSION.strategy", "absolute") res = auth.refresh_session() - assert res is None - account_store.expire.assert_not_called() - account_store.delete.assert_not_called() + assert res is None + account_store.expire.assert_not_called() + account_store.delete.assert_not_called() def test_refresh_session_invalid_ttl(app, datastore, mocker: MockerFixture): _, account_store, _ = datastore test_session_id = "test_session_id" - test_login_date = datetime.now(UTC).isoformat() - account_store.hget.return_value = test_login_date.encode("utf-8") - mocker.patch("server.config.config.SESSION.sliding_lifetime", -1) with app.test_request_context("/"): + mocker.patch("server.config.config.SESSION.sliding_lifetime", -1) login_user(mock_repoadmin_login_user) session["_id"] = test_session_id res = auth.refresh_session() - assert res is None - account_store.expire.assert_not_called() + assert res is None + account_store.expire.assert_not_called() def test_load_user_not_eppn(): @@ -121,7 +108,7 @@ def test_load_user_invalid_eppn(app, mocker: MockerFixture): mocker.patch("server.auth.get_user_from_store", return_value=mock_repoadmin_login_user) session["_id"] = test_session_id user = auth.load_user(test_invalid_eppn) - assert user is None + assert user is None def test_load_user(app, mocker: MockerFixture): @@ -131,7 +118,7 @@ def test_load_user(app, mocker: MockerFixture): mocker.patch("server.auth.get_user_from_store", return_value=mock_repoadmin_login_user) session["_id"] = test_session_id user = auth.load_user(test_eppn) - assert user == mock_repoadmin_login_user + assert user == mock_repoadmin_login_user def test_get_user_from_store_valid(app, datastore): @@ -146,9 +133,9 @@ def test_get_user_from_store_valid(app, datastore): account_store.hgetall.return_value = user_dict with app.test_request_context("/"): user = auth.get_user_from_store(test_session_id) - assert isinstance(user, LoginUser) - assert user.eppn == "test_eppn" - assert user.session_id == test_session_id + assert isinstance(user, LoginUser) + assert user.eppn == "test_eppn" + assert user.session_id == test_session_id def test_get_user_from_store_none(app, datastore): @@ -157,12 +144,12 @@ def test_get_user_from_store_none(app, datastore): account_store.hgetall.return_value = None with app.test_request_context("/"): user = auth.get_user_from_store(test_session_id) - assert user is None + assert user is None def test_build_account_store_key(app, test_config): session_id = "test_session_id" with app.test_request_context("/"): key = auth.build_account_store_key(session_id) - prefix = test_config.REDIS.key_prefix - assert key == f"{prefix}login-{session_id}" + prefix = test_config.REDIS.key_prefix + assert key == f"{prefix}login-{session_id}"