diff --git a/docs/boost-endpoint-api.md b/docs/boost-endpoint-api.md index 4385562..7a3cbd5 100644 --- a/docs/boost-endpoint-api.md +++ b/docs/boost-endpoint-api.md @@ -141,25 +141,24 @@ See [Request reference](#request-reference) for the full body schema. ```json { + "type": "validation_error", "errors": [ { "code": "required_field", - "message": "This field is required.", - "metadata": {"field": "organization", "drf_code": "required"} + "detail": "This field is required.", + "attr": "organization" }, { "code": "invalid_submodule_list", - "message": "Expected a list of items but got type \"str\".", - "metadata": { - "field": "add_or_update", - "language": "zh_Hans", - "drf_code": "not_a_list" - } + "detail": "Expected a list of items but got type \"str\".", + "attr": "add_or_update.zh_Hans" } ] } ``` +HTTP validation errors follow Weblate's [DRF standardized error format](https://drf-standardized-errors.readthedocs.io/en/latest/error_response.html): top-level `type` is `"validation_error"`, and each entry has `code`, `detail`, and `attr` (the serializer field path, or `null` for non-field errors). Stable `BoostEndpointErrorCode` values appear in `code`. + --- ## Request reference @@ -214,7 +213,8 @@ This processes the `json` and `unordered` submodules for Simplified Chinese, and | Field | Type | Description | |-------|------|-------------| -| `errors` | array | Unified list of structured error objects (see [Error handling](#error-handling)) | +| `type` | string | Always `"validation_error"` for request-body validation failures | +| `errors` | array | DRF standardized validation entries (`code`, `detail`, `attr`); see [Error handling](#error-handling) | ### 401 Unauthorized @@ -250,7 +250,8 @@ The Celery task (`boost_add_or_update_task`) returns a dictionary keyed by langu "errors": [ { "code": "clone_failed", - "message": "Failed to clone repository for unordered", + "detail": "Failed to clone repository for unordered", + "attr": null, "metadata": { "submodule": "unordered", "organization": "boostorg", @@ -284,7 +285,7 @@ The Celery task (`boost_add_or_update_task`) returns a dictionary keyed by langu | `components_updated` | integer | Existing components whose push branch was refreshed | | `components_failed` | integer | Components where `create_or_update_component` returned `None` | | `components_deleted` | integer | Components removed because they were no longer found in the repo scan | -| `errors` | array of objects | Non-fatal structured errors (`code`, `message`, `metadata`); see [Error handling](#error-handling) | +| `errors` | array of objects | Non-fatal structured errors (`code`, `detail`, `attr`, optional `metadata`); see [Error handling](#error-handling) | --- @@ -381,24 +382,38 @@ For each submodule the following steps run in order: ## Error handling -All Boost endpoint errors share one JSON-serializable shape (`src/boost_weblate/endpoint/errors.py`): +Boost endpoint errors use stable `BoostEndpointErrorCode` values in `code`. The JSON shape depends on the layer: + +### HTTP validation (`400 Bad Request`) + +Produced by Weblate's DRF exception handler when `AddOrUpdateRequestSerializer` rejects the request body. + +| Field | Type | Description | +|-------|------|-------------| +| `type` | string | `"validation_error"` | +| `errors` | array | Entries with `code`, `detail`, and `attr` (field path such as `organization` or `add_or_update.zh_Hans`, or `null`) | + +No `metadata` bag on HTTP validation entries. Field location is expressed via `attr`. + +### Service / submodule results + +Recoverable failures inside `BoostComponentService` are appended to per-submodule `errors` lists in the Celery task return value via `to_error_dict()` (`src/boost_weblate/endpoint/errors.py`). | Field | Type | Description | |-------|------|-------------| | `code` | string | Stable machine-readable identifier (see table below) | -| `message` | string | Human-readable description | -| `metadata` | object | Context for monitoring, retry logic, and client handling | +| `detail` | string | Human-readable description | +| `attr` | string or null | Related request field when applicable; usually `null` for service failures | +| `metadata` | object | Optional context (e.g. `submodule`, `organization`, `permission`, `timeout_seconds`) | ### Where errors appear | Layer | HTTP status / Celery state | Shape | |-------|---------------------------|-------| -| Request validation | `400 Bad Request` | `{"errors": [, ...]}` | -| Recoverable submodule failure | `202` accepted; task `SUCCESS` with partial failures | `submodule_results[].errors: [, ...]` | +| Request validation | `400 Bad Request` | `{"type": "validation_error", "errors": [{"code", "detail", "attr"}, ...]}` | +| Recoverable submodule failure | `202` accepted; task `SUCCESS` with partial failures | `submodule_results[].errors: [{"code", "detail", "attr", "metadata"?}, ...]` | | Fatal task failure | Task `FAILURE` | `BoostEndpointError` exception with `.code` and `.metadata` | -HTTP `400` responses and submodule `errors` lists use the same object schema. Validation errors may include `metadata.field`, `metadata.language`, and `metadata.drf_code` (the original DRF `ErrorDetail.code` when applicable). - ### Error codes | Code | Typical source | diff --git a/src/boost_weblate/endpoint/errors.py b/src/boost_weblate/endpoint/errors.py index d6ec0f8..1881118 100644 --- a/src/boost_weblate/endpoint/errors.py +++ b/src/boost_weblate/endpoint/errors.py @@ -50,39 +50,132 @@ def __init__( self.metadata = dict(metadata or {}) def to_dict(self) -> dict[str, Any]: - return { - "code": self.code.value, - "message": str(self.args[0]), - "metadata": self.metadata, - } + field = self.metadata.get("field") + language = self.metadata.get("language") + return to_error_entry( + self.code.value, + str(self.args[0]), + attr=attr_for_field(field, language=language), + ) + + +def attr_for_field( + field: str | None, + *, + language: str | None = None, +) -> str | None: + """Map request field / language metadata to a DRF ``attr`` path.""" + if field is None: + return None + if language: + return f"{field}.{language}" + return field + + +def _error_code_value(code: BoostEndpointErrorCode | str) -> str: + code_str = code.value if isinstance(code, BoostEndpointErrorCode) else str(code) + try: + return BoostEndpointErrorCode(code_str).value + except ValueError: + return code_str + + +def to_error_entry( + code: BoostEndpointErrorCode | str, + detail: str, + *, + attr: str | None = None, +) -> dict[str, Any]: + """Build a strict DRF standardized error entry (no metadata bag).""" + return { + "code": _error_code_value(code), + "detail": detail, + "attr": attr, + } def to_error_dict( code: BoostEndpointErrorCode | str, - message: str, + detail: str, + *, + attr: str | None = None, **metadata: Any, ) -> dict[str, Any]: - """Build a JSON-serializable error dict without raising.""" - return BoostEndpointError(message, code=code, metadata=metadata).to_dict() + """Build a JSON-serializable service-layer error dict without raising.""" + entry = to_error_entry(code, detail, attr=attr) + if metadata: + entry["metadata"] = metadata + return entry def append_error( result: dict[str, Any], code: BoostEndpointErrorCode | str, - message: str, + detail: str, **metadata: Any, ) -> None: """Append a structured error to a service result's ``errors`` list.""" - result.setdefault("errors", []).append(to_error_dict(code, message, **metadata)) + field = metadata.pop("field", None) + language = metadata.pop("language", None) + result.setdefault("errors", []).append( + to_error_dict( + code, + detail, + attr=attr_for_field(field, language=language), + **metadata, + ) + ) def boost_validation_errors( items: list[tuple[BoostEndpointErrorCode | str, str, dict[str, Any]]], ) -> list[dict[str, Any]]: """Build a unified error list from validation failure tuples.""" - return [ - to_error_dict(code, message, **metadata) for code, message, metadata in items - ] + errors: list[dict[str, Any]] = [] + for code, detail, item_metadata in items: + meta = dict(item_metadata) + field = meta.pop("field", None) + language = meta.pop("language", None) + errors.append( + to_error_dict( + code, + detail, + attr=attr_for_field(field, language=language), + **meta, + ) + ) + return errors + + +def _message_and_code(err: Any) -> tuple[str, str]: + code = getattr(err, "code", BoostEndpointErrorCode.REQUIRED_FIELD.value) + return str(err), str(code) + + +def flatten_validation_detail(detail: dict[str, Any]) -> list[dict[str, Any]]: + """Flatten field-keyed validation detail into DRF standardized error entries.""" + results: list[dict[str, Any]] = [] + + def walk(messages: Any, attr: str | None = None) -> None: + if isinstance(messages, dict) or hasattr(messages, "items"): + for key, value in messages.items(): + key_str = str(key) + nested_attr = f"{attr}.{key_str}" if attr else key_str + walk(value, nested_attr) + return + if isinstance(messages, (list, tuple)): + for msg in messages: + if isinstance(msg, dict) or hasattr(msg, "items"): + walk(msg, attr) + else: + message, code = _message_and_code(msg) + results.append(to_error_entry(code, message, attr=attr)) + return + message, code = _message_and_code(messages) + results.append(to_error_entry(code, message, attr=attr)) + + walk(detail) + return results def wrap_task_error(exc: BaseException) -> BoostEndpointError: diff --git a/src/boost_weblate/endpoint/serializers.py b/src/boost_weblate/endpoint/serializers.py index b4eca5e..6c1278a 100644 --- a/src/boost_weblate/endpoint/serializers.py +++ b/src/boost_weblate/endpoint/serializers.py @@ -11,11 +11,11 @@ from django.core.exceptions import ValidationError from rest_framework import serializers +from rest_framework.exceptions import ErrorDetail from boost_weblate.endpoint.errors import ( BoostEndpointErrorCode, - boost_validation_errors, - to_error_dict, + flatten_validation_detail, ) from boost_weblate.endpoint.validators import ( MAX_ADD_OR_UPDATE_LANGS, @@ -24,6 +24,8 @@ validate_repo_segment, ) +_BOOST_ERROR_CODES = frozenset(code.value for code in BoostEndpointErrorCode) + class DrfValidationCode(StrEnum): """DRF ``ErrorDetail.code`` values mapped by this serializer.""" @@ -77,40 +79,49 @@ class AddOrUpdateRequestSerializer(serializers.Serializer): def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) - self._custom_validation_errors: list[dict[str, Any]] = [] - self._custom_error_fields: set[str] = set() - self._structured_errors: list[dict[str, Any]] = [] + self._validation_detail: dict[str, Any] = {} @property - def structured_errors(self) -> list[dict[str, Any]]: - return self._structured_errors + def standardized_errors(self) -> list[dict[str, Any]]: + return flatten_validation_detail(self._validation_detail) def is_valid(self, *, raise_exception: bool = False) -> bool: - self._custom_validation_errors = [] - self._custom_error_fields = set() + self._validation_detail = {} valid = super().is_valid(raise_exception=False) if not valid: - self._structured_errors = self._to_structured_errors() - else: - self._structured_errors = [] + self._build_validation_detail() if not valid and raise_exception: - raise serializers.ValidationError(self._structured_errors) + raise serializers.ValidationError(self._validation_detail) return valid - def _to_structured_errors(self) -> list[dict[str, Any]]: - structured = list(self._custom_validation_errors) + def _build_validation_detail(self) -> None: for field, messages in self.errors.items(): - if field in self._custom_error_fields: - continue - for subfield, message, drf_code in self._flatten_field_errors(messages): - code = self._code_for_drf_error(field, drf_code, subfield=subfield) - metadata: dict[str, Any] = {"field": field} - if drf_code is not None: - metadata["drf_code"] = drf_code - if subfield and field == RequestField.ADD_OR_UPDATE: - metadata["language"] = subfield - structured.append(to_error_dict(code, message, **metadata)) - return structured + self._merge_field_errors(str(field), messages) + + def _merge_field_errors(self, field: str, messages: Any) -> None: + if isinstance(messages, dict) or ( + hasattr(messages, "items") and not isinstance(messages, (list, tuple)) + ): + nested: dict[str, list[ErrorDetail]] = {} + for subfield, submessages in messages.items(): + subfield_str = str(subfield) + for _sub, message, drf_code in self._flatten_field_errors(submessages): + code = self._boost_code_for_drf_error( + field, drf_code, subfield=subfield_str + ) + nested.setdefault(subfield_str, []).append( + ErrorDetail(message, code=code.value) + ) + if nested: + self._validation_detail[field] = nested + return + + entries: list[ErrorDetail] = [] + for _subfield, message, drf_code in self._flatten_field_errors(messages): + code = self._boost_code_for_drf_error(field, drf_code) + entries.append(ErrorDetail(message, code=code.value)) + if entries: + self._validation_detail[field] = entries @staticmethod def _message_and_drf_code(err: Any) -> tuple[str, str | None]: @@ -169,50 +180,50 @@ def _code_for_drf_error( if drf_code == DrfValidationCode.REQUIRED: return BoostEndpointErrorCode.REQUIRED_FIELD if drf_code == DrfValidationCode.NOT_A_LIST: - return BoostEndpointErrorCode.INVALID_SUBMODULE_LIST + if field == RequestField.ADD_OR_UPDATE: + return BoostEndpointErrorCode.INVALID_SUBMODULE_LIST + return BoostEndpointErrorCode.REQUIRED_FIELD if drf_code == DrfValidationCode.EMPTY: if field == RequestField.ADD_OR_UPDATE and subfield: return BoostEndpointErrorCode.INVALID_SUBMODULE_LIST return BoostEndpointErrorCode.REQUIRED_FIELD return BoostEndpointErrorCode.REQUIRED_FIELD + @classmethod + def _boost_code_for_drf_error( + cls, + field: str, + drf_code: str | None, + *, + subfield: str | None = None, + ) -> BoostEndpointErrorCode: + if drf_code in _BOOST_ERROR_CODES: + return BoostEndpointErrorCode(drf_code) + return cls._code_for_drf_error(field, drf_code, subfield=subfield) + def validate_organization(self, value: str) -> str: """Reject organization names that would produce unsafe clone URLs.""" try: return validate_repo_segment(value, field="organization") except ValidationError as exc: - self._custom_error_fields.add(RequestField.ORGANIZATION) - self._custom_validation_errors.extend( - boost_validation_errors( - [ - ( - BoostEndpointErrorCode.INVALID_CLONE_URL, - str(exc), - {"field": RequestField.ORGANIZATION}, - ) - ] + raise serializers.ValidationError( + ErrorDetail( + str(exc), + code=BoostEndpointErrorCode.INVALID_CLONE_URL.value, ) - ) - raise serializers.ValidationError(str(exc)) from exc + ) from exc def validate_version(self, value: str) -> str: """Reject version strings with unsafe characters or excessive length.""" try: return validate_repo_segment(value, field="version") except ValidationError as exc: - self._custom_error_fields.add(RequestField.VERSION) - self._custom_validation_errors.extend( - boost_validation_errors( - [ - ( - BoostEndpointErrorCode.INVALID_CLONE_URL, - str(exc), - {"field": RequestField.VERSION}, - ) - ] + raise serializers.ValidationError( + ErrorDetail( + str(exc), + code=BoostEndpointErrorCode.INVALID_CLONE_URL.value, ) - ) - raise serializers.ValidationError(str(exc)) from exc + ) from exc def validate_extensions(self, value: list[str] | None) -> list[str] | None: """Strip entries and remove blanks so all-empty input does not filter files.""" @@ -222,8 +233,10 @@ def validate_extensions(self, value: list[str] | None) -> list[str] | None: for entry in value: if not isinstance(entry, str): raise serializers.ValidationError( - "Each extension must be a string.", - code=DrfValidationCode.NOT_A_LIST, + ErrorDetail( + "Each extension must be a string.", + code=BoostEndpointErrorCode.REQUIRED_FIELD.value, + ) ) stripped = entry.strip() if stripped: @@ -232,121 +245,103 @@ def validate_extensions(self, value: list[str] | None) -> list[str] | None: def validate_add_or_update(self, value: dict[str, Any]) -> dict[str, Any]: """Require non-empty string language keys and non-empty submodule lists.""" - items: list[tuple[BoostEndpointErrorCode, str, dict[str, Any]]] = [] + field_errors: list[ErrorDetail] = [] + nested_errors: dict[str, list[ErrorDetail]] = {} + + def add_lang_error( + lang_code: str, + message: str, + code: BoostEndpointErrorCode, + ) -> None: + nested_errors.setdefault(lang_code, []).append( + ErrorDetail(message, code=code.value) + ) + if len(value) > MAX_ADD_OR_UPDATE_LANGS: - items.append( - ( - BoostEndpointErrorCode.INVALID_LANGUAGE_CODE, + field_errors.append( + ErrorDetail( ( f"add_or_update: exceeds maximum of " f"{MAX_ADD_OR_UPDATE_LANGS} language keys " f"(got {len(value)})." ), - {"field": RequestField.ADD_OR_UPDATE}, + code=BoostEndpointErrorCode.INVALID_LANGUAGE_CODE.value, ) ) - self._custom_error_fields.add(RequestField.ADD_OR_UPDATE) - self._custom_validation_errors.extend(boost_validation_errors(items)) - raise serializers.ValidationError({RequestField.ADD_OR_UPDATE: "invalid"}) - for lang_code, submodules in value.items(): - if not isinstance(lang_code, str) or lang_code.strip() == "": - items.append( - ( - BoostEndpointErrorCode.INVALID_LANGUAGE_CODE, + else: + for lang_code, submodules in value.items(): + if not isinstance(lang_code, str) or lang_code.strip() == "": + add_lang_error( + str(lang_code), ( "add_or_update: each key must be a non-empty language " f"code; got {repr(lang_code)}" ), - { - "field": RequestField.ADD_OR_UPDATE, - "language": str(lang_code), - }, - ) - ) - continue - try: - validate_language_code(lang_code) - except ValidationError as exc: - items.append( - ( BoostEndpointErrorCode.INVALID_LANGUAGE_CODE, + ) + continue + try: + validate_language_code(lang_code) + except ValidationError as exc: + add_lang_error( + lang_code, str(exc), - { - "field": RequestField.ADD_OR_UPDATE, - "language": lang_code, - }, + BoostEndpointErrorCode.INVALID_LANGUAGE_CODE, ) - ) - continue - if not isinstance(submodules, list): - items.append( - ( - BoostEndpointErrorCode.INVALID_SUBMODULE_LIST, + continue + if not isinstance(submodules, list): + add_lang_error( + lang_code, ( "add_or_update: each value must be a non-empty list of " f"submodule names; key {lang_code!r} is not a list " f"(got {type(submodules).__name__})." ), - {"field": RequestField.ADD_OR_UPDATE, "language": lang_code}, - ) - ) - elif len(submodules) == 0: - items.append( - ( BoostEndpointErrorCode.INVALID_SUBMODULE_LIST, + ) + elif len(submodules) == 0: + add_lang_error( + lang_code, ( "add_or_update: each value must be a non-empty list of " f"submodule names; key {lang_code!r} has an empty list." ), - {"field": RequestField.ADD_OR_UPDATE, "language": lang_code}, - ) - ) - elif len(submodules) > MAX_SUBMODULES_PER_LANG: - items.append( - ( BoostEndpointErrorCode.INVALID_SUBMODULE_LIST, + ) + elif len(submodules) > MAX_SUBMODULES_PER_LANG: + add_lang_error( + lang_code, ( f"add_or_update: key {lang_code!r} exceeds maximum of " f"{MAX_SUBMODULES_PER_LANG} submodules " f"(got {len(submodules)})." ), - {"field": RequestField.ADD_OR_UPDATE, "language": lang_code}, + BoostEndpointErrorCode.INVALID_SUBMODULE_LIST, ) - ) - else: - for submodule in submodules: - if not isinstance(submodule, str): - items.append( - ( - BoostEndpointErrorCode.INVALID_SUBMODULE_LIST, + else: + for submodule in submodules: + if not isinstance(submodule, str): + add_lang_error( + lang_code, ( "add_or_update: each submodule name must be a " f"string; key {lang_code!r} has " f"{type(submodule).__name__}." ), - { - "field": RequestField.ADD_OR_UPDATE, - "language": lang_code, - }, + BoostEndpointErrorCode.INVALID_SUBMODULE_LIST, ) - ) - break - try: - validate_repo_segment(submodule, field="submodule") - except ValidationError as exc: - items.append( - ( - BoostEndpointErrorCode.INVALID_SUBMODULE, + break + try: + validate_repo_segment(submodule, field="submodule") + except ValidationError as exc: + add_lang_error( + lang_code, str(exc), - { - "field": RequestField.ADD_OR_UPDATE, - "language": lang_code, - "submodule": submodule, - }, + BoostEndpointErrorCode.INVALID_SUBMODULE, ) - ) - if items: - self._custom_error_fields.add(RequestField.ADD_OR_UPDATE) - self._custom_validation_errors.extend(boost_validation_errors(items)) - raise serializers.ValidationError({RequestField.ADD_OR_UPDATE: "invalid"}) + + if field_errors or nested_errors: + if nested_errors: + raise serializers.ValidationError(nested_errors) + raise serializers.ValidationError(field_errors) return value diff --git a/src/boost_weblate/endpoint/tasks.py b/src/boost_weblate/endpoint/tasks.py index e9d66f6..f50f9c8 100644 --- a/src/boost_weblate/endpoint/tasks.py +++ b/src/boost_weblate/endpoint/tasks.py @@ -2,7 +2,13 @@ # # SPDX-License-Identifier: BSL-1.0 -"""Celery tasks for Boost documentation add-or-update (async HTTP handling).""" +"""Celery tasks for Boost documentation add-or-update (async HTTP handling). + +HTTP validation failures are returned as DRF standardized errors (``type`` + +``errors`` with ``code``, ``detail``, ``attr``). Celery task failures raise +``BoostEndpointError`` with ``.code`` and ``.metadata``; per-submodule service +results use ``to_error_dict`` entries that include a ``metadata`` bag. +""" from __future__ import annotations diff --git a/src/boost_weblate/endpoint/views.py b/src/boost_weblate/endpoint/views.py index 536884f..af437e8 100644 --- a/src/boost_weblate/endpoint/views.py +++ b/src/boost_weblate/endpoint/views.py @@ -86,11 +86,7 @@ def post(self, request, format=None): # noqa: A002 task_id so clients can validate the request without waiting for completion. """ serializer = AddOrUpdateRequestSerializer(data=request.data) - if not serializer.is_valid(): - return Response( - {"errors": serializer.structured_errors}, - status=status.HTTP_400_BAD_REQUEST, - ) + serializer.is_valid(raise_exception=True) data = serializer.validated_data async_result = boost_add_or_update_task.delay( diff --git a/tests/endpoint/test_errors.py b/tests/endpoint/test_errors.py index 77f4711..a99bf6c 100644 --- a/tests/endpoint/test_errors.py +++ b/tests/endpoint/test_errors.py @@ -5,14 +5,18 @@ from __future__ import annotations import pytest +from rest_framework.exceptions import ErrorDetail from weblate.trans.exceptions import WeblateError from boost_weblate.endpoint.errors import ( BoostEndpointError, BoostEndpointErrorCode, append_error, + attr_for_field, boost_validation_errors, + flatten_validation_detail, to_error_dict, + to_error_entry, wrap_task_error, ) @@ -39,15 +43,45 @@ def test_boost_endpoint_error_to_dict_shape() -> None: ) assert err.to_dict() == { "code": "clone_failed", - "message": "Failed to clone repository for json", - "metadata": {"submodule": "json", "organization": "boostorg"}, + "detail": "Failed to clone repository for json", + "attr": None, } -def test_boost_endpoint_error_empty_metadata_defaults_to_dict() -> None: - err = BoostEndpointError("msg", code=BoostEndpointErrorCode.REQUIRED_FIELD) - assert err.metadata == {} - assert err.to_dict()["metadata"] == {} +def test_boost_endpoint_error_to_dict_includes_attr_from_field_metadata() -> None: + err = BoostEndpointError( + "This field is required.", + code=BoostEndpointErrorCode.REQUIRED_FIELD, + metadata={"field": "organization"}, + ) + assert err.to_dict() == { + "code": "required_field", + "detail": "This field is required.", + "attr": "organization", + } + + +def test_attr_for_field_maps_language_subkeys() -> None: + assert ( + attr_for_field("add_or_update", language="zh_Hans") == "add_or_update.zh_Hans" + ) + assert attr_for_field("add_or_update", language="") == "add_or_update" + assert attr_for_field("organization") == "organization" + assert attr_for_field(None) is None + + +def test_to_error_entry_has_no_metadata() -> None: + payload = to_error_entry( + BoostEndpointErrorCode.REQUIRED_FIELD, + "This field is required.", + attr="organization", + ) + assert payload == { + "code": "required_field", + "detail": "This field is required.", + "attr": "organization", + } + assert "metadata" not in payload def test_to_error_dict_without_raising() -> None: @@ -59,7 +93,8 @@ def test_to_error_dict_without_raising() -> None: ) assert payload == { "code": "permission_denied", - "message": "Can not create project (missing project.add)", + "detail": "Can not create project (missing project.add)", + "attr": None, "metadata": { "permission": "project.add", "project_slug": "boost-json-documentation-zh_Hans", @@ -78,7 +113,8 @@ def test_append_error_creates_errors_list() -> None: assert result["errors"] == [ { "code": "clone_failed", - "message": "Failed to clone repository for json", + "detail": "Failed to clone repository for json", + "attr": None, "metadata": {"submodule": "json"}, } ] @@ -89,7 +125,8 @@ def test_append_error_appends_to_existing_list() -> None: "errors": [ { "code": "invalid_submodule", - "message": "Invalid submodule name: ../evil", + "detail": "Invalid submodule name: ../evil", + "attr": None, "metadata": {"submodule": "../evil"}, } ] @@ -123,8 +160,43 @@ def test_boost_validation_errors_builds_list() -> None: ) assert len(errors) == 2 assert errors[0]["code"] == "invalid_language_code" + assert errors[0]["attr"] == "add_or_update" assert errors[1]["code"] == "invalid_submodule_list" - assert errors[1]["metadata"]["language"] == "zh_Hans" + assert errors[1]["attr"] == "add_or_update.zh_Hans" + + +def test_flatten_validation_detail_matches_drf_shape() -> None: + detail = { + "organization": [ + ErrorDetail("This field is required.", code="required_field"), + ], + "version": [ + ErrorDetail("This field is required.", code="required"), + ], + "add_or_update": { + "zh_Hans": [ + ErrorDetail("empty list", code="invalid_submodule_list"), + ], + }, + } + flattened = flatten_validation_detail(detail) + assert flattened == [ + { + "code": "required_field", + "detail": "This field is required.", + "attr": "organization", + }, + { + "code": "required", + "detail": "This field is required.", + "attr": "version", + }, + { + "code": "invalid_submodule_list", + "detail": "empty list", + "attr": "add_or_update.zh_Hans", + }, + ] def test_wrap_task_error_wraps_generic_exception() -> None: @@ -133,6 +205,11 @@ def test_wrap_task_error_wraps_generic_exception() -> None: assert wrapped.code == BoostEndpointErrorCode.TASK_INTERNAL_ERROR assert wrapped.metadata == {"exception_type": "RuntimeError"} assert str(wrapped) == "db unavailable" + assert wrapped.to_dict() == { + "code": "task_internal_error", + "detail": "db unavailable", + "attr": None, + } def test_wrap_task_error_passthrough_boost_endpoint_error() -> None: diff --git a/tests/endpoint/test_serializers.py b/tests/endpoint/test_serializers.py index e12a97e..2883a6c 100644 --- a/tests/endpoint/test_serializers.py +++ b/tests/endpoint/test_serializers.py @@ -4,6 +4,8 @@ from __future__ import annotations +import pytest +from rest_framework import serializers from rest_framework.exceptions import ErrorDetail from boost_weblate.endpoint.errors import BoostEndpointErrorCode @@ -18,6 +20,10 @@ def _error_codes(errors: list[dict]) -> list[str]: return [e["code"] for e in errors] +def _attrs(errors: list[dict]) -> set[str | None]: + return {e.get("attr") for e in errors} + + def test_add_or_update_serializer_valid_minimal() -> None: ser = AddOrUpdateRequestSerializer( data={ @@ -26,7 +32,7 @@ def test_add_or_update_serializer_valid_minimal() -> None: "add_or_update": {"zh_Hans": ["json"]}, } ) - assert ser.is_valid(), ser.structured_errors + assert ser.is_valid(), ser.standardized_errors assert ser.validated_data["organization"] == "CppDigest" assert ser.validated_data["version"] == "boost-1.90.0" assert ser.validated_data["add_or_update"] == {"zh_Hans": ["json"]} @@ -42,7 +48,7 @@ def test_add_or_update_serializer_accepts_extensions() -> None: "extensions": [".adoc", ".md"], } ) - assert ser.is_valid(), ser.structured_errors + assert ser.is_valid(), ser.standardized_errors assert ser.validated_data["extensions"] == [".adoc", ".md"] @@ -56,13 +62,12 @@ def test_add_or_update_serializer_rejects_empty_map() -> None: ) assert not ser.is_valid() assert BoostEndpointErrorCode.REQUIRED_FIELD.value in _error_codes( - ser.structured_errors + ser.standardized_errors ) add_or_update_errors = [ - e for e in ser.structured_errors if e["metadata"]["field"] == "add_or_update" + e for e in ser.standardized_errors if e["attr"] == "add_or_update" ] assert len(add_or_update_errors) == 1 - assert add_or_update_errors[0]["metadata"]["drf_code"] == "empty" assert ( add_or_update_errors[0]["code"] == BoostEndpointErrorCode.REQUIRED_FIELD.value ) @@ -78,11 +83,10 @@ def test_add_or_update_serializer_rejects_empty_submodule_list() -> None: ) assert not ser.is_valid() assert BoostEndpointErrorCode.INVALID_SUBMODULE_LIST.value in _error_codes( - ser.structured_errors + ser.standardized_errors ) assert any( - e.get("metadata", {}).get("language") == "zh_Hans" - for e in ser.structured_errors + e.get("attr") == "add_or_update.zh_Hans" for e in ser.standardized_errors ) @@ -96,15 +100,13 @@ def test_add_or_update_serializer_rejects_non_list_submodules() -> None: ) assert not ser.is_valid() assert BoostEndpointErrorCode.INVALID_SUBMODULE_LIST.value in _error_codes( - ser.structured_errors + ser.standardized_errors ) lang_errors = [ - e - for e in ser.structured_errors - if e.get("metadata", {}).get("language") == "zh_Hans" + e for e in ser.standardized_errors if e.get("attr") == "add_or_update.zh_Hans" ] assert lang_errors - assert lang_errors[0]["metadata"]["drf_code"] == "not_a_list" + assert lang_errors[0]["code"] == BoostEndpointErrorCode.INVALID_SUBMODULE_LIST.value def test_flatten_field_errors_propagates_error_detail_code() -> None: @@ -153,17 +155,24 @@ def test_code_for_drf_error_maps_drf_codes() -> None: ) == BoostEndpointErrorCode.REQUIRED_FIELD ) + assert ( + AddOrUpdateRequestSerializer._code_for_drf_error( + RequestField.EXTENSIONS, DrfValidationCode.NOT_A_LIST + ) + == BoostEndpointErrorCode.REQUIRED_FIELD + ) def test_add_or_update_serializer_missing_required_fields() -> None: ser = AddOrUpdateRequestSerializer(data={}) assert not ser.is_valid() - codes = _error_codes(ser.structured_errors) + codes = _error_codes(ser.standardized_errors) assert codes.count(BoostEndpointErrorCode.REQUIRED_FIELD.value) == 3 - fields = {e["metadata"]["field"] for e in ser.structured_errors} + fields = _attrs(ser.standardized_errors) assert fields == {"organization", "version", "add_or_update"} assert all( - e["metadata"].get("drf_code") == "required" for e in ser.structured_errors + e["code"] == BoostEndpointErrorCode.REQUIRED_FIELD.value + for e in ser.standardized_errors ) @@ -177,7 +186,7 @@ def test_add_or_update_serializer_rejects_invalid_organization() -> None: ) assert not ser.is_valid() assert BoostEndpointErrorCode.INVALID_CLONE_URL.value in _error_codes( - ser.structured_errors + ser.standardized_errors ) @@ -191,7 +200,7 @@ def test_add_or_update_serializer_rejects_invalid_submodule_segment() -> None: ) assert not ser.is_valid() assert BoostEndpointErrorCode.INVALID_SUBMODULE.value in _error_codes( - ser.structured_errors + ser.standardized_errors ) @@ -204,11 +213,11 @@ def test_add_or_update_serializer_accumulates_custom_errors() -> None: } ) assert not ser.is_valid() - codes = _error_codes(ser.structured_errors) + codes = _error_codes(ser.standardized_errors) assert BoostEndpointErrorCode.INVALID_CLONE_URL.value in codes assert BoostEndpointErrorCode.INVALID_SUBMODULE.value in codes - fields = {e["metadata"]["field"] for e in ser.structured_errors} - assert fields == {"organization", "add_or_update"} + fields = _attrs(ser.standardized_errors) + assert fields == {"organization", "add_or_update.zh_Hans"} def test_invalid_organization_still_flattens_other_drf_errors() -> None: @@ -219,19 +228,15 @@ def test_invalid_organization_still_flattens_other_drf_errors() -> None: } ) assert not ser.is_valid() - codes = _error_codes(ser.structured_errors) + codes = _error_codes(ser.standardized_errors) assert BoostEndpointErrorCode.INVALID_CLONE_URL.value in codes assert BoostEndpointErrorCode.REQUIRED_FIELD.value in codes - org_errors = [ - e for e in ser.structured_errors if e["metadata"]["field"] == "organization" - ] - version_errors = [ - e for e in ser.structured_errors if e["metadata"]["field"] == "version" - ] + org_errors = [e for e in ser.standardized_errors if e["attr"] == "organization"] + version_errors = [e for e in ser.standardized_errors if e["attr"] == "version"] assert len(org_errors) == 1 assert org_errors[0]["code"] == BoostEndpointErrorCode.INVALID_CLONE_URL.value assert len(version_errors) == 1 - assert version_errors[0]["metadata"]["drf_code"] == "required" + assert version_errors[0]["code"] == BoostEndpointErrorCode.REQUIRED_FIELD.value def test_add_or_update_serializer_rejects_invalid_version() -> None: @@ -244,11 +249,9 @@ def test_add_or_update_serializer_rejects_invalid_version() -> None: ) assert not ser.is_valid() assert BoostEndpointErrorCode.INVALID_CLONE_URL.value in _error_codes( - ser.structured_errors + ser.standardized_errors ) - version_errors = [ - e for e in ser.structured_errors if e["metadata"]["field"] == "version" - ] + version_errors = [e for e in ser.standardized_errors if e["attr"] == "version"] assert len(version_errors) == 1 @@ -262,7 +265,7 @@ def test_add_or_update_serializer_rejects_sql_injection_lang_code() -> None: ) assert not ser.is_valid() assert BoostEndpointErrorCode.INVALID_LANGUAGE_CODE.value in _error_codes( - ser.structured_errors + ser.standardized_errors ) @@ -276,7 +279,7 @@ def test_add_or_update_serializer_rejects_whitespace_lang_code() -> None: ) assert not ser.is_valid() assert BoostEndpointErrorCode.INVALID_LANGUAGE_CODE.value in _error_codes( - ser.structured_errors + ser.standardized_errors ) @@ -289,7 +292,9 @@ def test_add_or_update_serializer_rejects_non_string_submodule() -> None: } ) assert not ser.is_valid() - assert any(e["metadata"]["field"] == "add_or_update" for e in ser.structured_errors) + assert any( + e.get("attr") == "add_or_update.zh_Hans" for e in ser.standardized_errors + ) def test_add_or_update_serializer_rejects_oversized_organization() -> None: @@ -302,7 +307,7 @@ def test_add_or_update_serializer_rejects_oversized_organization() -> None: ) assert not ser.is_valid() assert BoostEndpointErrorCode.INVALID_CLONE_URL.value in _error_codes( - ser.structured_errors + ser.standardized_errors ) @@ -319,7 +324,7 @@ def test_add_or_update_serializer_rejects_too_many_languages() -> None: ) assert not ser.is_valid() assert BoostEndpointErrorCode.INVALID_LANGUAGE_CODE.value in _error_codes( - ser.structured_errors + ser.standardized_errors ) @@ -336,7 +341,7 @@ def test_add_or_update_serializer_rejects_too_many_submodules() -> None: ) assert not ser.is_valid() assert BoostEndpointErrorCode.INVALID_SUBMODULE_LIST.value in _error_codes( - ser.structured_errors + ser.standardized_errors ) @@ -350,4 +355,35 @@ def test_add_or_update_serializer_rejects_extensions_dict() -> None: } ) assert not ser.is_valid() - assert any(e["metadata"]["field"] == "extensions" for e in ser.structured_errors) + ext_errors = [e for e in ser.standardized_errors if e["attr"] == "extensions"] + assert len(ext_errors) == 1 + assert ext_errors[0]["code"] == BoostEndpointErrorCode.REQUIRED_FIELD.value + + +def test_add_or_update_serializer_rejects_non_string_extension_elements() -> None: + ser = AddOrUpdateRequestSerializer() + with pytest.raises(serializers.ValidationError) as exc_info: + ser.validate_extensions([".md", 123]) + detail = exc_info.value.detail + assert detail[0].code == BoostEndpointErrorCode.REQUIRED_FIELD.value + assert str(detail[0]) == "Each extension must be a string." + + +def test_add_or_update_serializer_rejects_mixed_extensions_list() -> None: + ser = AddOrUpdateRequestSerializer( + data={ + "organization": "o", + "version": "v", + "add_or_update": {"zh_Hans": ["json"]}, + "extensions": [".md", {}], + } + ) + assert not ser.is_valid() + ext_errors = [ + e + for e in ser.standardized_errors + if str(e.get("attr", "")).startswith("extensions") + ] + assert len(ext_errors) == 1 + assert ext_errors[0]["attr"] == "extensions.1" + assert ext_errors[0]["code"] == BoostEndpointErrorCode.REQUIRED_FIELD.value diff --git a/tests/endpoint/test_views.py b/tests/endpoint/test_views.py index 8b48026..7c3f121 100644 --- a/tests/endpoint/test_views.py +++ b/tests/endpoint/test_views.py @@ -186,10 +186,10 @@ def test_add_or_update_validation_error() -> None: force_authenticate(request, user=user) response = AddOrUpdateView.as_view()(request) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "errors" in response.data + assert response.data["type"] == "validation_error" errors = response.data["errors"] assert isinstance(errors, list) - assert all("code" in e and "message" in e and "metadata" in e for e in errors) + assert all("code" in e and "detail" in e and "attr" in e for e in errors) assert BoostEndpointErrorCode.REQUIRED_FIELD.value in [e["code"] for e in errors] @@ -356,6 +356,11 @@ def process_all(self, _submodules, *, user, request=None): # noqa: ANN001 user_id=1, ) assert exc_info.value.code == BoostEndpointErrorCode.TASK_INTERNAL_ERROR + assert ( + exc_info.value.to_dict()["code"] + == BoostEndpointErrorCode.TASK_INTERNAL_ERROR.value + ) + assert exc_info.value.to_dict()["detail"] == "fail" def test_boost_add_or_update_task_duplicate_raises_task_duplicate( diff --git a/tests/postgres/test_endpoint_trust_boundary.py b/tests/postgres/test_endpoint_trust_boundary.py index 0c00e33..e093e32 100644 --- a/tests/postgres/test_endpoint_trust_boundary.py +++ b/tests/postgres/test_endpoint_trust_boundary.py @@ -13,6 +13,8 @@ from rest_framework import status from weblate.trans.models import Project +from boost_weblate.endpoint.errors import BoostEndpointErrorCode + pytestmark = [pytest.mark.postgres, pytest.mark.django_db] _VALID_ADD_OR_UPDATE_BODY = { @@ -41,7 +43,17 @@ def test_adversarial_payload_rejected_without_db_rows_or_enqueue( format="json", ) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "errors" in response.data + assert response.data["type"] == "validation_error" + errors = response.data["errors"] + assert isinstance(errors, list) + assert all("code" in e and "detail" in e and "attr" in e for e in errors) + org_errors = [ + e + for e in errors + if e["code"] == BoostEndpointErrorCode.INVALID_CLONE_URL.value + ] + assert len(org_errors) == 1 + assert org_errors[0]["attr"] == "organization" assert Project.objects.count() == baseline_count mock_add_or_update_delay.assert_not_called()