diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f3b6ce9f..bff50b65d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ and this project adheres to ### Fixed - Fix type of `statement.result.score.scaled` from `int` to `Decimal` +- Fix XAPI definitions extensions not accepting empty strings as values. + + +### Changed + +- Refactor statements' ExtensionMap ## [5.0.1] - 2024-07-11 diff --git a/src/ralph/models/xapi/base/common.py b/src/ralph/models/xapi/base/common.py index 0c0b62485..6d16d5e19 100644 --- a/src/ralph/models/xapi/base/common.py +++ b/src/ralph/models/xapi/base/common.py @@ -3,7 +3,7 @@ from typing import Dict, Type, Union from langcodes import tag_is_valid -from pydantic import RootModel, model_validator, validate_email +from pydantic import ConfigDict, RootModel, model_validator, validate_email from rfc3987 import parse from ralph.conf import NonEmptyStrictStr @@ -47,6 +47,12 @@ def validate_language_tag(cls, tag): LanguageMap = Dict[LanguageTag, NonEmptyStrictStr] +class ExtensionMap(RootModel[Dict[IRI, Union[str, int, bool, list, dict, None]]]): + """Pydantic custom data type for XAPI context and object definitions extensions.""" + + model_config = ConfigDict(str_min_length=0, coerce_numbers_to_str=True) + + class MailtoEmail(RootModel[str]): """Pydantic custom data type validating `mailto:email` format.""" diff --git a/src/ralph/models/xapi/base/contexts.py b/src/ralph/models/xapi/base/contexts.py index 2e3456c10..9a2ae4e31 100644 --- a/src/ralph/models/xapi/base/contexts.py +++ b/src/ralph/models/xapi/base/contexts.py @@ -1,13 +1,13 @@ """Base xAPI `Context` definitions.""" -from typing import Dict, List, Optional, Union +from typing import List, Optional, Union from uuid import UUID from ralph.conf import NonEmptyStrictStr from ..config import BaseModelWithConfig from .agents import BaseXapiAgent -from .common import IRI, LanguageTag +from .common import ExtensionMap, LanguageTag from .groups import BaseXapiGroup from .unnested_objects import BaseXapiActivity, BaseXapiStatementRef @@ -54,4 +54,4 @@ class BaseXapiContext(BaseModelWithConfig): platform: Optional[NonEmptyStrictStr] = None language: Optional[LanguageTag] = None statement: Optional[BaseXapiStatementRef] = None - extensions: Optional[Dict[IRI, Union[str, int, bool, list, dict, None]]] = None + extensions: Optional[ExtensionMap] = None diff --git a/src/ralph/models/xapi/base/results.py b/src/ralph/models/xapi/base/results.py index 8e99583fa..0097b1f01 100644 --- a/src/ralph/models/xapi/base/results.py +++ b/src/ralph/models/xapi/base/results.py @@ -2,7 +2,7 @@ from datetime import timedelta from decimal import Decimal -from typing import Any, Dict, Optional, Union +from typing import Any, Optional from pydantic import Field, StrictBool, model_validator from typing_extensions import Annotated @@ -10,7 +10,7 @@ from ralph.conf import NonEmptyStrictStr from ..config import BaseModelWithConfig -from .common import IRI +from .common import ExtensionMap class BaseXapiResultScore(BaseModelWithConfig): @@ -60,4 +60,4 @@ class BaseXapiResult(BaseModelWithConfig): completion: Optional[StrictBool] = None response: Optional[NonEmptyStrictStr] = None duration: Optional[timedelta] = None - extensions: Optional[Dict[IRI, Union[str, int, bool, list, dict, None]]] = None + extensions: Optional[ExtensionMap] = None diff --git a/src/ralph/models/xapi/base/unnested_objects.py b/src/ralph/models/xapi/base/unnested_objects.py index 847264b15..fa0ac08d8 100644 --- a/src/ralph/models/xapi/base/unnested_objects.py +++ b/src/ralph/models/xapi/base/unnested_objects.py @@ -1,6 +1,6 @@ """Base xAPI `Object` definitions (1).""" -from typing import Any, Dict, List, Literal, Optional, Union +from typing import Any, List, Literal, Optional, Union from uuid import UUID from pydantic import AnyUrl, StringConstraints, field_validator @@ -9,7 +9,7 @@ from ralph.conf import NonEmptyStrictStr from ..config import BaseModelWithConfig -from .common import IRI, LanguageMap +from .common import IRI, ExtensionMap, LanguageMap class BaseXapiActivityDefinition(BaseModelWithConfig): @@ -27,7 +27,7 @@ class BaseXapiActivityDefinition(BaseModelWithConfig): description: Optional[LanguageMap] = None type: Optional[IRI] = None moreInfo: Optional[AnyUrl] = None - extensions: Optional[Dict[IRI, Union[str, int, bool, list, dict, None]]] = None + extensions: Optional[ExtensionMap] = None class BaseXapiInteractionComponent(BaseModelWithConfig): diff --git a/tests/models/xapi/base/test_common.py b/tests/models/xapi/base/test_common.py index 40e5db1d7..be5b7d86b 100644 --- a/tests/models/xapi/base/test_common.py +++ b/tests/models/xapi/base/test_common.py @@ -3,7 +3,7 @@ import pytest from pydantic import BaseModel, ValidationError -from ralph.models.xapi.base.common import IRI, LanguageMap, LanguageTag +from ralph.models.xapi.base.common import IRI, ExtensionMap, LanguageMap, LanguageTag @pytest.mark.parametrize( @@ -92,7 +92,7 @@ class DummyLanguageTagModel(BaseModel): DummyLanguageTagModel(**values) -@pytest.mark.parametrize("values", [({"map": {"en": "Hello"}})]) +@pytest.mark.parametrize("values", [{"map": {"en": "Hello"}}]) def test_models_xapi_base_common_field_language_map_with_valid_data(values): """Test that a valid verb field does not raise a `ValidationError`.""" @@ -136,3 +136,82 @@ class DummyLanguageTagModel(BaseModel): with pytest.raises(exception, match=error): DummyLanguageTagModel(**values) + + +@pytest.mark.parametrize( + "values", + [ + ({"extensions": {}}), + ({"extensions": {"http://localhost/foo/bar": None}}), + ({"extensions": {"http://localhost/foo/bar": None}}), + ({"extensions": {"http://localhost/foo/bar": 42}}), + ({"extensions": {"http://localhost/foo/bar": []}}), + ({"extensions": {"http://localhost/foo/bar": {}}}), + ({"extensions": {"http://localhost/foo/bar": ""}}), + ( + { + "extensions": { + "http://localhost/foo/bar": "An explanation", + "http://localhost/foost/barst": "Another explanation", + } + } + ), + ( + { + "extensions": { + "http://localhost/foo/bar": { + "http://localhost/food/bard": "An explanation", + "whatever": "that_is", + }, + } + } + ), + ( + { + "extensions": { + "http://localhost/foo/bar": { + "http://localhost/food/bard": "An explanation", + "nope": "", + }, + } + } + ), + ], +) +def test_models_xapi_base_common_field_extensions_with_valid_data(values): + """Test that a valid Extensions field does not raise a `ValidationError`.""" + + class DummyExtensionsModel(BaseModel): + """A dummy pydantic model with an Extensions field.""" + + extensions: ExtensionMap + + try: + DummyExtensionsModel(**values) + except ValidationError as err: + pytest.fail(f"Valid Extensions should not raise exceptions: {err}") + + +@pytest.mark.parametrize( + "values,exception,error", + [ + ( + {"extensions": []}, + ValidationError, + "extensions\n Input should be a valid dictionary", + ), + ({"extensions": {"localhost": 42}}, ValidationError, "not a valid 'IRI'"), + ], +) +def test_models_xapi_base_common_field_extensions_with_invalid_data( + values, exception, error +): + """Test that an invalid Extensions field raises a `ValidationError`.""" + + class DummyExtensionsModel(BaseModel): + """A dummy pydantic model with a extensions field.""" + + extensions: ExtensionMap + + with pytest.raises(exception, match=error): + DummyExtensionsModel(**values)