diff --git a/CHANGELOG.md b/CHANGELOG.md index f496d2afb..404345c72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ and this project adheres to `test-helm` CI job to fix flaky "no matching resources"/"status not found" - Drop the stale `add_ssh_keys` entry from the `deploy-docs` CI job so it pushes to `gh-pages` with the read-write checkout key +- 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 5fb94805c..269d5bf86 100644 --- a/src/ralph/models/xapi/base/common.py +++ b/src/ralph/models/xapi/base/common.py @@ -1,9 +1,9 @@ """Common for xAPI base definitions.""" -from typing import Dict, Type, Union +from typing import Dict, Type, Union, Annotated from langcodes import tag_is_valid -from pydantic import RootModel, model_validator, validate_email +from pydantic import ConfigDict, RootModel, StringConstraints, model_validator, validate_email from rfc3987 import parse from ralph.conf import NonEmptyStrictStr @@ -46,6 +46,10 @@ def validate_language_tag(cls, tag): LanguageMap = Dict[LanguageTag, NonEmptyStrictStr] +ExtensionValue = Union[Annotated[str, StringConstraints(min_length=0)], bool, int, list, dict, None] + +class ExtensionMap(RootModel[Dict[IRI, ExtensionValue]]): + """Pydantic custom data type for XAPI context and object definitions extensions.""" 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 aa4e10143..94ec46f49 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": 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'"), + ({"extensions": {"": 43}}, 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)