diff --git a/doc/changelog.rst b/doc/changelog.rst index 86912b66..1b159390 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -65,6 +65,17 @@ Added Changed ^^^^^^^ +- :meth:`Resource.from_schema ` and + :meth:`Extension.from_schema ` refuse a schema declaring + two attributes whose names only differ by case, and report both. + :rfc:`RFC7643 §2.1 <7643#section-2.1>` makes them one attribute, so such a schema describes + it twice. :issue:`166` +- Attribute names are matched case-insensitively, and nothing else. The ``nameChar`` rule of + :rfc:`RFC7643 §2.1 <7643#section-2.1>` makes ``$``, ``-`` and ``_`` part of a name, so + ``{"user-name": "x"}``, which 0.7 read as ``userName``, is now an unknown attribute that + :attr:`~scim2_models.ScimPolicy.unknown` governs. Paths, filters and ``sortBy`` resolve the + same way. A model whose fields answer to one attribute name raises a :class:`TypeError` where + it is defined. :issue:`166` - The bulk models take the resource type their operations carry, as in ``BulkRequest[User]`` or ``BulkRequest[User | Group]``, and raise a :class:`TypeError` when used bare. A payload the type parameter does not cover is now refused, and a bulk response no longer dumps ``path``. @@ -124,6 +135,28 @@ Deprecated Fixed ^^^^^ +- A PATCH operation carrying no ``path`` that unassigns an extension declared + :attr:`Required.true ` is refused. The extension was named by its + URN, which the constraint checks did not resolve, so they found no constraint to answer for. + :issue:`166` +- A filter or a path accepts a ``$`` anywhere in an attribute name, as ``nameChar`` allows. Only + a leading one went through. :issue:`166` +- A schema declaring several attributes that yield one Python name builds a field for each of + them: the attribute already spelled as that name keeps it, and the others are held under their + SCIM name. ``employee_id`` and ``employeeId`` used to share one field, so a dump reported one + value twice and lost the other. An attribute is read under the name SCIM gives it, as in + ``resource["employeeId"]``, so a field name that is no Python identifier costs nothing. + :issue:`166` +- A pydantic error spells the attribute as SCIM does, ``userName`` and ``$ref`` where it used to + report ``username`` and ``ref``, and so does the JSON schema a model publishes, which FastAPI + reads to document a request body. :issue:`166` +- A field declaring its own ``alias`` is read under it, and an unknown attribute is refused under + the spelling the peer used. ``Field(alias="string_field")`` used to answer ``extra_forbidden`` + quoting a spelling nobody had sent. :issue:`166` +- An extension is read under its class name as well as under its URN, so + ``User[EnterpriseUser](EnterpriseUser=extension)`` is accepted and a resource carrying an + extension survives a dump without aliases read back. Both used to answer ``extra_forbidden``. + :issue:`166` - A bulk model indexed with something other than a resource type names itself in the error. The rules of the :class:`~scim2_models.PatchOp` its operations carry used to answer for it, so ``BulkRequest[str]`` told the caller to write ``PatchOp[User]``. diff --git a/doc/how-to/define-custom-models.rst b/doc/how-to/define-custom-models.rst index 9b44df55..ed49d6c6 100644 --- a/doc/how-to/define-custom-models.rst +++ b/doc/how-to/define-custom-models.rst @@ -41,6 +41,47 @@ as optional because SCIM can omit an attribute in a valid request or response. >>> pet.model_dump()["details"] {'color': 'ginger', 'weightKg': 4.2} +Name an attribute +----------------- + +A field named in Python takes the camel-case spelling of its name as its SCIM attribute name: +``weight_kg`` becomes ``weightKg``. Reading a payload goes the other way, and +:rfc:`RFC7643 §2.1 <7643#section-2.1>` makes attribute names case-insensitive, so ``weightKg``, +``weightkg`` and ``WEIGHTKG`` all reach ``weight_kg``. Nothing else is folded: the ABNF of that +section makes ``$``, ``-`` and ``_`` part of a name, so ``weight-kg`` is another attribute +altogether, refused as an unknown one. See :doc:`tolerate-a-nonconformant-peer` to accept what a peer +spells its own way. + +When the attribute name is not what camel-casing a Python name yields, declare it with a +``serialization_alias``: + +.. doctest:: + + >>> from pydantic import Field + >>> class Pet(Resource): + ... __schema__ = URN("urn:example:schemas:Pet") + ... vet_ref: str | None = Field(None, serialization_alias="$vetRef") + ... + >>> Pet.model_validate({"$vetRef": "https://example.com/Vets/1"}).vet_ref + 'https://example.com/Vets/1' + +An alias applies to reading as well as to writing, and it wins over the Python name of any other +field. A field whose alias is the Python name of its neighbour therefore takes the +keyword that spells it, in a payload and in the constructor alike: + +.. doctest:: + + >>> class Pet(Resource): + ... __schema__ = URN("urn:example:schemas:Pet") + ... pet_name: str | None = None + ... legacy: str | None = Field(None, serialization_alias="pet_name") + ... + >>> Pet(pet_name="Mochi").legacy + 'Mochi' + +Two fields cannot answer to one attribute name. Such a model raises a :class:`TypeError` where it +is defined, no payload key being able to reach both. + Apply SCIM attribute metadata ----------------------------- diff --git a/scim2_models/attributes.py b/scim2_models/attributes.py index ee2f5d6a..eb5aa3cf 100644 --- a/scim2_models/attributes.py +++ b/scim2_models/attributes.py @@ -71,10 +71,7 @@ def _get_attribute_urn(self, field_name: str) -> str: See RFC7644 §3.10. """ - alias = ( - self.__class__.model_fields[field_name].serialization_alias or field_name - ) - return f"{self._attribute_urn}.{alias}" + return f"{self._attribute_urn}.{self._scim_name(field_name)}" class MultiValuedComplexAttribute(ComplexAttribute): @@ -95,7 +92,9 @@ class MultiValuedComplexAttribute(ComplexAttribute): value: Any | None = None """The value of an entitlement.""" - ref: Reference[Any] | None = Field(None, serialization_alias="$ref") + ref: Reference[Any] | None = Field( + None, serialization_alias="$ref", validation_alias="$ref" + ) """The reference URI of a target resource, if the attribute is a reference.""" diff --git a/scim2_models/base.py b/scim2_models/base.py index 9550b79b..e60c9a27 100644 --- a/scim2_models/base.py +++ b/scim2_models/base.py @@ -11,6 +11,7 @@ from typing import get_args from typing import get_origin +from pydantic import AliasChoices from pydantic import AliasGenerator from pydantic import Base64Bytes from pydantic import BaseModel as PydanticBaseModel @@ -23,6 +24,7 @@ from pydantic import ValidatorFunctionWrapHandler from pydantic import model_serializer from pydantic import model_validator +from pydantic.fields import FieldInfo from pydantic_core import InitErrorDetails from pydantic_core import PydanticCustomError from typing_extensions import Self @@ -118,9 +120,10 @@ class _SCIMClassInfo(NamedTuple): """SCIM metadata for BaseModel.""" alias_to_field: Mapping[str, str] = MappingProxyType({}) - """Alias -> Python field name. + """Serialization alias -> Python field name. - Holds both validation and serialization aliases. + Keyed by the spelling a dump carries, so a serializer can walk back from a + key it produced to the field that holds it. """ attribute_urns: Mapping[str, str] = MappingProxyType({}) @@ -132,12 +135,92 @@ class _SCIMClassInfo(NamedTuple): extensions: frozenset[str] = frozenset() """Field names whose root type is a ``Extension`` subclass.""" - known_keys: frozenset[str] = frozenset() - """Every payload key the class accepts, normalized. + validation_names: Mapping[str, str] = MappingProxyType({}) + """""Python field name -> the name pydantic reads that field under. - Field names and aliases alike: an extension is named by its URN in a - payload and by its class name as a field, and both name the same thing. + An attribute reaches pydantic under its SCIM spelling, which a validation + error and a published JSON schema then carry. + """ "" + + field_by_name: Mapping[str, str] = MappingProxyType({}) + """Lowercased attribute name -> Python field name. + + Every spelling a payload may use for a field: the name it is serialized + under, the aliases it declares for itself, and its Python name. RFC7643 + §2.1 makes attribute names case-insensitive and nothing else — its + ``nameChar`` rule makes ``$``, ``-`` and ``_`` part of a name — so the keys + are lowercased and keep their punctuation. + """ + + +_DECLARED_ALIAS_PRIORITY = 2 +"""The ``alias_priority`` pydantic gives an alias the field itself declares, an +alias generator filling the slots left empty with a priority of 1.""" + + +def _declared_validation_names(field: FieldInfo) -> list[str]: + """Return the names a field declares for itself. + + Only the field itself declares an attribute name: what the alias generator + derived from a Python name is how pydantic reads the field, not a spelling + a peer may use. An AliasChoices holds several spellings of one attribute, + each of them usable. An AliasPath points at a place inside the payload + rather than at an attribute, so it indexes nothing: the key it starts from + is no attribute name, and reaches pydantic as the peer spelled it. + """ + if field.alias_priority != _DECLARED_ALIAS_PRIORITY: + return [] + + alias = field.validation_alias + if isinstance(alias, str): + return [alias] + if isinstance(alias, AliasChoices): + return [choice for choice in alias.choices if isinstance(choice, str)] + return [] + + +def _validation_name(field: FieldInfo, field_name: str) -> str: + """Return the name pydantic reads a field under. + + The alias generator gives every field its SCIM attribute name, which an + error and a published JSON schema then carry. A field declaring + several spellings of its own names none of them in particular, and is read + under its Python name. """ + alias = field.validation_alias + return alias if isinstance(alias, str) else field_name + + +def _claim_attribute_name( + index: dict[str, str], name: str, field_name: str, owner: type +) -> None: + """Record that a field answers to an attribute name. + + Two fields answering to one name leave a payload key reaching both, which + the class cannot be built with, so such a class is refused where it is + written. + """ + key = name.lower() + claimed = index.get(key) + if claimed is not None and claimed != field_name: + raise TypeError( + f"{owner.__name__} has two fields answering to the SCIM attribute " + f"name {name!r}: {claimed!r} and {field_name!r}. Attribute names are " + f"case-insensitive (RFC7643 §2.1), so one payload key would reach both." + ) + index[key] = field_name + + +def _claim_python_name(index: dict[str, str], field_name: str) -> None: + """Record that a field answers to its own Python name. + + That name is a convenience rather than an attribute name, so two fields + whose names only differ by case take it from each other instead of making + the class impossible to build: the key then designates neither, leaving the + SCIM name of each of them the only way to reach it. + """ + key = field_name.lower() + index[key] = "" if key in index and index[key] != field_name else field_name def _holds_reference(model: type["BaseModel"], field_name: str) -> bool: @@ -172,11 +255,12 @@ class BaseModel(PydanticBaseModel): model_config = ConfigDict( alias_generator=AliasGenerator( - validation_alias=_normalize_attribute_name, + validation_alias=_to_camel, serialization_alias=_to_camel, ), validate_assignment=True, - populate_by_name=True, + validate_by_name=True, + validate_by_alias=True, use_attribute_docstrings=True, extra="forbid", ) @@ -329,6 +413,11 @@ def get_field_multiplicity(cls, attribute_name: str) -> bool: origin = get_origin(attribute_type) return isinstance(origin, type) and issubclass(origin, list) + @classmethod + def _scim_name(cls, field_name: str) -> str: + """Return the name a field is serialized under, ``$ref`` included.""" + return cls.model_fields[field_name].serialization_alias or _to_camel(field_name) + @classmethod def __pydantic_on_complete__(cls) -> None: """Build the per-class SCIM metadata table on ``cls.__scim_info__``. @@ -342,6 +431,9 @@ def __pydantic_on_complete__(cls) -> None: attribute_urns: dict[str, str] = {} complex_fields: set[str] = set() extensions: set[str] = set() + scim_names: dict[str, str] = {} + python_names: dict[str, str] = {} + validation_names: dict[str, str] = {} main_schema = getattr(cls, "__schema__", None) extension_cls: type | None = None @@ -352,10 +444,16 @@ def __pydantic_on_complete__(cls) -> None: for field_name, field in cls.model_fields.items(): # Alias -> field name mapping - serialization_alias = field.serialization_alias or field_name + serialization_alias = cls._scim_name(field_name) alias_to_field[serialization_alias] = field_name - if isinstance(field.validation_alias, str): - alias_to_field[field.validation_alias] = field_name + + # The names this field answers to, the SCIM ones winning over the + # Python one, which is only the spelling pydantic offers. + _claim_attribute_name(scim_names, serialization_alias, field_name, cls) + for declared in _declared_validation_names(field): + _claim_attribute_name(scim_names, declared, field_name, cls) + _claim_python_name(python_names, field_name) + validation_names[field_name] = _validation_name(field, field_name) root_type = cls.get_field_root_type(field_name) @@ -385,41 +483,42 @@ def __pydantic_on_complete__(cls) -> None: attribute_urns=attribute_urns, complex_fields=frozenset(complex_fields), extensions=frozenset(extensions), - known_keys=frozenset( - _normalize_attribute_name(key) - for key in (*cls.model_fields, *alias_to_field) - ), + field_by_name={ + **{key: name for key, name in python_names.items() if name}, + **scim_names, + }, + validation_names=validation_names, ) @model_validator(mode="wrap") @classmethod - def _normalize_attribute_names( + def _resolve_attribute_names( cls, value: Any, handler: ValidatorFunctionWrapHandler, info: ValidationInfo ) -> Self: - """Normalize payload attribute names, and set aside the ones no field declares. + """Rewrite each payload key to the field holding it, and set aside the rest. - RFC7643 §2.1 indicate that attribute names should be case-insensitive. - Any attribute name is transformed in lowercase so any case is handled - the same way. + RFC7643 §2.1 makes attribute names case-insensitive, so a key is looked + up folded. What it resolves to is the Python name of the field, which + is the one spelling pydantic accepts for every field. - Unless the policy forbids them, unknown keys are taken out of the - payload with the spelling the peer used. Pydantic never sees them, so - the ``extra="forbid"`` of the class has nothing to refuse. + A key no field answers to is taken out of the payload with the spelling + the peer used, unless the policy forbids unknown attributes: it is then + left in place, so that the error pydantic raises quotes what was sent. """ unknown: dict[str, Any] = {} if isinstance(value, dict): - if _policy(info).unknown == ScimPolicy.Unknown.forbid: - value = {_normalize_attribute_name(k): v for k, v in value.items()} - else: - known = cls.__scim_info__.known_keys - normalized = {} - for key, item in value.items(): - name = _normalize_attribute_name(key) - if name in known: - normalized[name] = item - else: - unknown[key] = item - value = normalized + scim_info = cls.__scim_info__ + tolerated = _policy(info).unknown != ScimPolicy.Unknown.forbid + resolved: dict[Any, Any] = {} + for key, item in value.items(): + field_name = scim_info.field_by_name.get(_normalize_attribute_name(key)) + if field_name is not None: + resolved[scim_info.validation_names[field_name]] = item + elif tolerated: + unknown[key] = item + else: + resolved[key] = item + value = resolved obj = cast(Self, handler(value)) if unknown: @@ -667,7 +766,7 @@ def _set_complex_attribute_urns(self) -> None: ``_attribute_urn`` is later read by _get_attribute_urn. """ - for field_name in self.__class__.__scim_info__.complex_fields: + for field_name in self.__scim_info__.complex_fields: attr_value = getattr(self, field_name) if not attr_value: continue diff --git a/scim2_models/messages/patch_op.py b/scim2_models/messages/patch_op.py index 183df53a..6c8b6e27 100644 --- a/scim2_models/messages/patch_op.py +++ b/scim2_models/messages/patch_op.py @@ -70,24 +70,6 @@ def _targeted_attributes(value: Any) -> dict[str, Any]: return value if isinstance(value, dict) else {} -def _names_a_declared_target( - resource_class: type[Resource[Any]], attr_name: str -) -> bool: - """Whether a pathless operation names something the model declares. - - The ``value`` of a pathless operation names attributes of the resource, and - an extension by its schema URN, under which it names the attributes of that - extension. - """ - if _resolved_field(resource_class, attr_name) is not None: - return True - - lowered = attr_name.lower() - return any( - schema.lower() == lowered for schema in resource_class.get_extension_models() - ) - - def _resolved_field(resource_class: type[BaseModel], attr_name: str) -> str | None: """Return the Python field a SCIM attribute name designates. @@ -102,11 +84,6 @@ def _resolved_field(resource_class: type[BaseModel], attr_name: str) -> str | No """Fields that carry the payload rather than the state it describes.""" -def _attribute_name(model: type[BaseModel], field_name: str) -> str: - """Return the SCIM spelling of a field, as a path segment.""" - return model.model_fields[field_name].serialization_alias or field_name - - def _asserted_sub_attributes(entries: Any) -> set[str]: """Return the sub-attributes the entries of a wanted state name.""" asserted: set[str] = set() @@ -212,7 +189,7 @@ def _diff( old = getattr(before, field_name, None) if before is not None else None new = getattr(after, field_name, None) - path = f"{prefix}{_attribute_name(model, field_name)}" + path = f"{prefix}{model._scim_name(field_name)}" if model.get_field_multiplicity(field_name): yield from _diff_multi_valued(path, old, new, mutability) @@ -259,10 +236,7 @@ def _validate_mutability( resource instance and is enforced at runtime in PatchOp._check_immutable. """ - if (field := _resolved_field(resource_class, field_name)) is None: - return - - mutability = resource_class.get_field_annotation(field, Mutability) + mutability = resource_class.get_field_annotation(field_name, Mutability) if mutability == Mutability.read_only: raise MutabilityException( @@ -293,12 +267,7 @@ def _validate_required_attribute( else: return - # An extension is named by its schema URN, which is no field of the - # resource and carries no annotation of its own to check. - if (field := _resolved_field(resource_class, field_name)) is None: - return - - required = resource_class.get_field_annotation(field, Required) + required = resource_class.get_field_annotation(field_name, Required) # RFC7644 §3.5.2.2 has a server answer "mutability" when a required # attribute is removed or becomes unassigned. @@ -431,7 +400,8 @@ def _validate_operations(self, info: ValidationInfo) -> Self: # the attributes to write. Each of them is a target of its own, # and answers to §3.5.2 as a named path does. for attr_name, written in _targeted_attributes(operation.value).items(): - if not _names_a_declared_target(resource_class, attr_name): + field_name = _resolved_field(resource_class, attr_name) + if field_name is None: # §3.5.2 has an operation that is not compatible with an # attribute's schema return an error, and §3.12 defines # invalidValue for a value "not compatible with [...] the @@ -439,9 +409,9 @@ def _validate_operations(self, info: ValidationInfo) -> Self: raise InvalidValueException( detail=f"attribute '{attr_name}' is not declared by the resource schema" ).as_pydantic_error() - operation._validate_mutability(resource_class, attr_name) + operation._validate_mutability(resource_class, field_name) operation._validate_required_attribute( - resource_class, attr_name, written + resource_class, field_name, written ) continue diff --git a/scim2_models/path/expressions.py b/scim2_models/path/expressions.py index bde35ace..6546ed04 100644 --- a/scim2_models/path/expressions.py +++ b/scim2_models/path/expressions.py @@ -16,8 +16,8 @@ else: # pragma: no cover Template = None -_ATTR_NAME = r"\$?[A-Za-z][A-Za-z0-9_-]*" -"""The ``ATTRNAME`` rule, with the leading ``$`` of ``$ref`` that errata 8924 adds.""" +_ATTR_NAME = r"\$?[A-Za-z][A-Za-z0-9$_-]*" +"""The ``ATTRNAME`` rule, ``nameChar`` and the leading ``$`` of ``$ref`` that errata 8924 adds.""" _URN = r"urn:[A-Za-z0-9][A-Za-z0-9._-]*(?::[A-Za-z0-9][A-Za-z0-9._-]*)*" """A schema URN, the only ``URI`` an attribute path accepts.""" diff --git a/scim2_models/path/grammar.py b/scim2_models/path/grammar.py index 8cd13c64..4b16060a 100644 --- a/scim2_models/path/grammar.py +++ b/scim2_models/path/grammar.py @@ -89,14 +89,14 @@ // Trailing word boundaries are mandatory: without them an attribute named // "never" lexes as the "ne" operator followed by "ver". -COMPARE_OP.2: /(?:eq|ne|co|sw|ew|gt|lt|ge|le)(?![A-Za-z0-9_-])/i -PR.2: /pr(?![A-Za-z0-9_-])/i -TRUE.2: /true(?![A-Za-z0-9_-])/i -FALSE.2: /false(?![A-Za-z0-9_-])/i -NULL.2: /null(?![A-Za-z0-9_-])/i -_AND.3: /and(?![A-Za-z0-9_-])/i -_OR.3: /or(?![A-Za-z0-9_-])/i -_NOT.3: /not(?![A-Za-z0-9_-])/i +COMPARE_OP.2: /(?:eq|ne|co|sw|ew|gt|lt|ge|le)(?![A-Za-z0-9$_-])/i +PR.2: /pr(?![A-Za-z0-9$_-])/i +TRUE.2: /true(?![A-Za-z0-9$_-])/i +FALSE.2: /false(?![A-Za-z0-9$_-])/i +NULL.2: /null(?![A-Za-z0-9$_-])/i +_AND.3: /and(?![A-Za-z0-9$_-])/i +_OR.3: /or(?![A-Za-z0-9$_-])/i +_NOT.3: /not(?![A-Za-z0-9$_-])/i // Both literals follow the JSON rules the ABNF refers to, escapes included, so // that an invalid one is a syntax error located by the lexer rather than a diff --git a/scim2_models/path/path.py b/scim2_models/path/path.py index be46d2d2..446c628d 100644 --- a/scim2_models/path/path.py +++ b/scim2_models/path/path.py @@ -10,7 +10,6 @@ from ..base import BaseModel from ..urn import URN -from ..utils import _to_camel from .access import _delete_value from .access import _get_value from .access import _set_value @@ -51,11 +50,6 @@ def _node_attr_path(node: PathNode) -> AttrPath: return node.attr_path -def _scim_name(model: type[BaseModel], field_name: str) -> str: - """Return the name a field is serialized under, ``$ref`` included.""" - return model.model_fields[field_name].serialization_alias or _to_camel(field_name) - - class Path(_BoundToModels, _Expression, Generic[ResourceT]): """A SCIM attribute path, as defined at :rfc:`RFC7644 §3.5.2 <7644#section-3.5.2>`. @@ -506,7 +500,7 @@ def iter_model_paths( elif isclass(target_model) and issubclass(target_model, Extension): urn = target_model()._get_attribute_urn(field_name) else: - urn = _scim_name(target_model, field_name) + urn = target_model._scim_name(field_name) yield cls(urn) @@ -519,7 +513,7 @@ def iter_model_paths( for sub_field_name in field_type.model_fields: # type: ignore[union-attr] if not matches_filters(field_type, sub_field_name): # type: ignore[arg-type] continue - sub_urn = f"{urn}.{_scim_name(field_type, sub_field_name)}" # type: ignore[arg-type] + sub_urn = f"{urn}.{field_type._scim_name(sub_field_name)}" # type: ignore[union-attr] yield cls(sub_urn) yield from iter_model_paths(model) # type: ignore[arg-type] diff --git a/scim2_models/resources/enterprise_user.py b/scim2_models/resources/enterprise_user.py index 227ab098..34a65e54 100644 --- a/scim2_models/resources/enterprise_user.py +++ b/scim2_models/resources/enterprise_user.py @@ -22,7 +22,7 @@ class Manager(ComplexAttribute): ref: Annotated[ # type: ignore[type-arg] Reference["User"] | None, Required.true, - ] = Field(None, serialization_alias="$ref") + ] = Field(None, serialization_alias="$ref", validation_alias="$ref") """The URI of the SCIM resource representing the User's manager.""" display_name: Annotated[str | None, Mutability.read_only] = None diff --git a/scim2_models/resources/group.py b/scim2_models/resources/group.py index 328293c6..187a9c34 100644 --- a/scim2_models/resources/group.py +++ b/scim2_models/resources/group.py @@ -25,7 +25,7 @@ class GroupMember(ComplexAttribute): ref: Annotated[ # type: ignore[type-arg] Reference[Union["User", "Group"]] | None, Mutability.immutable, - ] = Field(None, serialization_alias="$ref") + ] = Field(None, serialization_alias="$ref", validation_alias="$ref") """The reference URI of a target resource, if the attribute is a reference.""" diff --git a/scim2_models/resources/resource.py b/scim2_models/resources/resource.py index 13159850..f451e4d5 100644 --- a/scim2_models/resources/resource.py +++ b/scim2_models/resources/resource.py @@ -38,7 +38,6 @@ from ..policy import _policy from ..scim_object import ScimObject from ..utils import UNION_TYPES -from ..utils import _normalize_attribute_name if TYPE_CHECKING: from .schema import Attribute @@ -248,7 +247,7 @@ def __class_getitem__(cls, item: Any) -> type["Resource[Any]"]: class_attrs[extension.__name__] = Field( default=None, # type: ignore[arg-type] serialization_alias=schema, - validation_alias=_normalize_attribute_name(schema), + validation_alias=schema, ) new_annotations = { @@ -539,7 +538,7 @@ def _model_attribute_to_scim_attribute( ) kwargs: dict[str, Any] = { - "name": field_info.serialization_alias or attribute_name, + "name": model._scim_name(attribute_name), "type": Attribute.Type(attribute_type), "multi_valued": model.get_field_multiplicity(attribute_name), "description": field_info.description, diff --git a/scim2_models/resources/schema.py b/scim2_models/resources/schema.py index 6bffeefa..378bca10 100644 --- a/scim2_models/resources/schema.py +++ b/scim2_models/resources/schema.py @@ -1,4 +1,5 @@ import re +from collections import Counter from datetime import datetime from enum import Enum from typing import Annotated @@ -30,7 +31,6 @@ from ..reference import External from ..reference import Reference from ..urn import URN -from ..utils import _normalize_attribute_name from .resource import Resource T = TypeVar("T", bound=BaseModel) @@ -47,32 +47,78 @@ def _make_python_identifier(identifier: str) -> str: return sanitized +def _field_names(attributes: "list[Attribute]") -> list[str]: + """Return the Python name each attribute is held under. + + Two SCIM names may yield one Python name, as ``employee_id`` and + ``employeeId`` both yield ``employee_id``. The one already spelled as that + name keeps it and the others are held under their SCIM name, so that no + attribute is dropped and the order the schema declares them in changes + nothing. Such a name is no Python identifier when it carries a dash, which + costs nothing: an attribute is read under the name SCIM gives it, as in + ``resource["employee-Id"]``. + """ + natural = [ + to_snake(_make_python_identifier(attr.name or "")) for attr in attributes + ] + shared = Counter(natural) + return [ + name if shared[name] == 1 or name == attr.name else attr.name or "" + for attr, name in zip(attributes, natural, strict=True) + ] + + +def _python_attributes( + attributes: "list[Attribute] | None", declared_by: str +) -> dict[str, Any]: + """Return the fields a schema or a complex attribute declares. + + ``declared_by`` is what declares them, quoted by the error. Two + attributes whose names only differ by case are refused: RFC7643 §2.1 makes + them one attribute, so a schema declaring both describes it twice. + """ + declared = [] + named: dict[str, str] = {} + for attr in attributes or []: + if not attr.name: + continue + + claimed = named.get(attr.name.lower()) + if claimed is not None: + raise ValueError( + f"{declared_by} declares {claimed!r} and {attr.name!r}, " + f"which name the same attribute" + ) + named[attr.name.lower()] = attr.name + declared.append(attr) + + return { + field_name: attr._to_python() + for field_name, attr in zip(_field_names(declared), declared, strict=True) + } + + def _make_python_model( obj: Union["Schema", "Attribute"], base: type[T], ) -> type[T]: """Build a Python model from a Schema or an Attribute object.""" - if isinstance(obj, Attribute): - pydantic_attributes = { - to_snake(_make_python_identifier(attr.name)): attr._to_python() - for attr in (obj.sub_attributes or []) - if attr.name - } - - else: - pydantic_attributes = { - to_snake(_make_python_identifier(attr.name)): attr._to_python() - for attr in (obj.attributes or []) - if attr.name - } - if not obj.name: raise ValueError("Schema or Attribute 'name' must be defined") + if isinstance(obj, Attribute): + pydantic_attributes = _python_attributes( + obj.sub_attributes, f"the attribute {obj.name!r}" + ) + else: + pydantic_attributes = _python_attributes( + obj.attributes, f"the schema {obj.id or obj.name}" + ) + model_name = to_pascal(to_snake(obj.name)) model = cast( type[T], - create_model(model_name, __base__=base, **pydantic_attributes), # type: ignore[call-overload] + create_model(model_name, __base__=base, **pydantic_attributes), ) if isinstance(obj, Schema) and obj.id: @@ -236,7 +282,7 @@ def _to_python(self) -> tuple[Any, Any] | None: description=self.description, examples=self.canonical_values, serialization_alias=self.name, - validation_alias=_normalize_attribute_name(self.name), + validation_alias=self.name, default=None, ) diff --git a/scim2_models/resources/user.py b/scim2_models/resources/user.py index fcb75394..4423c745 100644 --- a/scim2_models/resources/user.py +++ b/scim2_models/resources/user.py @@ -200,7 +200,7 @@ class GroupMembership(ComplexAttribute): ref: Annotated[ Reference["Group"] | None, Mutability.read_only, - ] = Field(None, serialization_alias="$ref") + ] = Field(None, serialization_alias="$ref", validation_alias="$ref") """The reference URI of a target resource, if the attribute is a reference.""" diff --git a/scim2_models/utils.py b/scim2_models/utils.py index 953d63de..c7576358 100644 --- a/scim2_models/utils.py +++ b/scim2_models/utils.py @@ -1,5 +1,4 @@ import re -from functools import lru_cache from inspect import isclass from typing import TYPE_CHECKING from typing import Any @@ -42,7 +41,6 @@ def _model_union(annotation: Any) -> "tuple[type[BaseModel], ...] | None": _UNDERSCORE_ALPHANUMERIC = re.compile(r"_+([0-9A-Za-z]+)") -_NON_WORD_UNDERSCORE = re.compile(r"[\W_]+") def _int_to_str(status: int | None) -> str | None: @@ -62,28 +60,22 @@ def _to_camel(string: str) -> str: return camel -@lru_cache(maxsize=256) def _normalize_attribute_name(attribute_name: str) -> str: - """Remove all non-alphabetical characters and lowerise a string. + """Fold the case of an attribute name. - This method is used for attribute name validation. + RFC7643 §2.1 makes attribute names case-insensitive, and its ``nameChar`` + rule makes ``$``, ``-`` and ``_`` part of a name, so the case is all there + is to fold. """ - is_extension_attribute = ":" in attribute_name - if not is_extension_attribute: - attribute_name = _NON_WORD_UNDERSCORE.sub("", attribute_name) - return attribute_name.lower() def _find_field_name(model_class: type["BaseModel"], attr_name: str) -> str | None: """Return the field a SCIM attribute name designates, or None. - ``nickName`` designates the ``nick_name`` field. + ``nickName`` designates the ``nick_name`` field, and ``$ref`` the ``ref`` + one. """ - normalized_attr_name = _normalize_attribute_name(attr_name) - - for field_key in model_class.model_fields: - if _normalize_attribute_name(field_key) == normalized_attr_name: - return field_key - - return None + return model_class.__scim_info__.field_by_name.get( + _normalize_attribute_name(attr_name) + ) diff --git a/tests/test_dynamic_resources.py b/tests/test_dynamic_resources.py index e00e7787..39e0f613 100644 --- a/tests/test_dynamic_resources.py +++ b/tests/test_dynamic_resources.py @@ -3,6 +3,7 @@ import weakref from typing import Union +import pytest from pydantic import Base64Bytes from scim2_models.annotations import CaseExact @@ -2904,3 +2905,213 @@ def test_a_model_built_at_runtime_is_collected_once_it_is_dropped(): gc.collect() assert reference() is None + + +def test_a_schema_declaring_two_attributes_that_differ_by_case_is_refused(): + """RFC7643 §2.1 makes two cases of one name the same attribute, so a schema declaring both describes one attribute twice.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + {"name": "userName", "type": "string", "multiValued": False}, + {"name": "username", "type": "string", "multiValued": False}, + ], + } + ) + + with pytest.raises(ValueError, match="name the same attribute"): + Resource.from_schema(schema) + + +@pytest.mark.parametrize( + "declared", [["employee_id", "employeeId"], ["employeeId", "employee_id"]] +) +def test_two_attributes_yielding_one_python_name_each_get_a_field(declared): + """The attribute already spelled as that name keeps it, whichever order the schema declares them in, and the other is held under its SCIM name.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + {"name": name, "type": "string", "multiValued": False} + for name in declared + ], + } + ) + Model = Resource.from_schema(schema) + + obj = Model.model_validate( + { + "schemas": ["urn:example:2.0:Ambiguous"], + "employee_id": "snake", + "employeeId": "camel", + } + ) + + assert obj["employee_id"] == "snake" + assert obj["employeeId"] == "camel" + assert obj.model_dump() == { + "schemas": ["urn:example:2.0:Ambiguous"], + "employee_id": "snake", + "employeeId": "camel", + } + + +def test_attributes_none_of_which_is_spelled_as_its_python_name_keep_their_scim_name(): + """A SCIM name carrying a dash is no Python identifier, which costs nothing since an attribute is read under the name SCIM gives it.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + {"name": "employeeId", "type": "string", "multiValued": False}, + {"name": "employee-Id", "type": "string", "multiValued": False}, + ], + } + ) + Model = Resource.from_schema(schema) + + obj = Model.model_validate( + { + "schemas": ["urn:example:2.0:Ambiguous"], + "employeeId": "camel", + "employee-Id": "dashed", + } + ) + + assert obj["employeeId"] == "camel" + assert obj["employee-Id"] == "dashed" + + +def test_a_complex_attribute_declaring_two_sub_attributes_that_differ_by_case_is_refused(): + """A sub-attribute answers for the same rules as an attribute of the schema itself.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + { + "name": "address", + "type": "complex", + "multiValued": False, + "subAttributes": [ + {"name": "postalCode", "type": "string"}, + {"name": "postalcode", "type": "string"}, + ], + } + ], + } + ) + + with pytest.raises(ValueError, match="the attribute 'address' declares"): + Resource.from_schema(schema) + + +def test_two_sub_attributes_yielding_one_python_name_each_get_a_field(): + """A sub-attribute is held under its SCIM name where an attribute would be.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + { + "name": "address", + "type": "complex", + "multiValued": False, + "subAttributes": [ + {"name": "postal_code", "type": "string"}, + {"name": "postalCode", "type": "string"}, + ], + } + ], + } + ) + Model = Resource.from_schema(schema) + + obj = Model.model_validate( + { + "schemas": ["urn:example:2.0:Ambiguous"], + "address": {"postal_code": "snake", "postalCode": "camel"}, + } + ) + + assert obj["address.postal_code"] == "snake" + assert obj["address.postalCode"] == "camel" + + +def test_attribute_names_keep_the_punctuation_they_carry(): + """The nameChar rule of RFC7643 §2.1 makes - and $ part of a name, so two names that differ by one are two attributes.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Punctuated", + "name": "Punctuated", + "attributes": [ + {"name": "employee-id", "type": "string", "multiValued": False}, + {"name": "employeeId", "type": "string", "multiValued": False}, + ], + } + ) + Model = Resource.from_schema(schema) + + obj = Model.model_validate( + { + "schemas": ["urn:example:2.0:Punctuated"], + "employee-id": "dashed", + "employeeId": "camel", + } + ) + + assert obj.model_dump() == { + "schemas": ["urn:example:2.0:Punctuated"], + "employee-id": "dashed", + "employeeId": "camel", + } + + +def test_an_attribute_without_a_name_builds_no_field(): + """RFC7643 §7 makes the name mandatory, and nothing can be built from an attribute missing it.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Nameless", + "name": "Nameless", + "attributes": [{"type": "string", "multiValued": False}], + } + ) + Model = Resource.from_schema(schema) + + assert set(Model.model_fields) == set(Resource.model_fields) + + +def test_the_three_spellings_of_one_python_name_each_get_a_field(): + """A schema may declare names that a dash, an underscore and a capital tell apart, and each of them keeps its value.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + {"name": name, "type": "string", "multiValued": False} + for name in ("employee-id", "employee_id", "employeeId") + ], + } + ) + Model = Resource.from_schema(schema) + + obj = Model.model_validate( + { + "schemas": ["urn:example:2.0:Ambiguous"], + "employee-id": "dashed", + "employee_id": "snake", + "employeeId": "camel", + } + ) + + assert obj.model_dump() == { + "schemas": ["urn:example:2.0:Ambiguous"], + "employee-id": "dashed", + "employee_id": "snake", + "employeeId": "camel", + } + # The Python name of the dashed attribute is no attribute name, so the + # spelling that only differs from employeeId by its case reaches that one. + assert obj["EMPLOYEEID"] == "camel" diff --git a/tests/test_filter_grammar.py b/tests/test_filter_grammar.py index 457750b1..9540d01c 100644 --- a/tests/test_filter_grammar.py +++ b/tests/test_filter_grammar.py @@ -522,7 +522,7 @@ def test_quoting_a_value_yields_a_literal_the_grammar_reads_back(value, literal) @pytest.mark.parametrize( "name", - ['userName eq "admin" or userName', "", "1abc", "name.given", 'a"b', "foo$bar"], + ['userName eq "admin" or userName', "", "1abc", "name.given", 'a"b', "foo bar"], ) def test_an_attribute_path_refuses_what_is_not_a_name(name): """A node built by hand carries a name the grammar reads as one, or nothing.""" @@ -530,6 +530,12 @@ def test_an_attribute_path_refuses_what_is_not_a_name(name): AttrPath(name) +@pytest.mark.parametrize("name", ["$ref", "foo$bar", "foo-bar", "foo_bar", "a$"]) +def test_an_attribute_path_takes_every_name_char(name): + """The nameChar rule of RFC7643 §2.1 makes $, - and _ part of a name.""" + assert AttrPath(name).attr == name + + def test_an_attribute_path_refuses_a_sub_attribute_that_is_not_a_name(): with pytest.raises(ValueError, match="is not an attribute name"): AttrPath("emails", sub_attr='type eq "work"') diff --git a/tests/test_model_attributes.py b/tests/test_model_attributes.py index 79d487e3..d1442948 100644 --- a/tests/test_model_attributes.py +++ b/tests/test_model_attributes.py @@ -1,9 +1,12 @@ import uuid from typing import Annotated +import pytest from pydantic import AliasChoices +from pydantic import AliasPath from pydantic import Base64Bytes from pydantic import Field +from pydantic import ValidationError from scim2_models import URN from scim2_models import ResponseParameters @@ -457,7 +460,7 @@ def test_extension_excluded_by_full_urn(): def test_field_with_custom_validation_aliases(): - """A field may bring its own validation aliases, which are not indexed as names.""" + """A field is read under every name it declares for itself.""" class AliasedResource(Resource): __schema__ = URN("urn:example:2.0:AliasedResource") @@ -466,9 +469,7 @@ class AliasedResource(Resource): None, validation_alias=AliasChoices("value", "legacyvalue") ) - assert all( - isinstance(alias, str) for alias in AliasedResource.__scim_info__.alias_to_field - ) + assert AliasedResource.__scim_info__.field_by_name["legacyvalue"] == "value" obj = AliasedResource.model_validate({"legacyValue": "x"}) @@ -485,3 +486,135 @@ def test_short_attr_path_with_plain_name(): assert _short_attr_path("userName") == "userName" assert _short_attr_path("name.familyName") == "name.familyName" + + +@pytest.mark.parametrize( + "spelling", ["userName", "username", "USERNAME", "UserName", "user_name"] +) +def test_an_attribute_name_is_read_whatever_its_case(spelling): + """RFC7643 §2.1 makes attribute names case-insensitive.""" + user = User.model_validate({"schemas": [str(User.__schema__)], spelling: "bjensen"}) + + assert user.user_name == "bjensen" + + +@pytest.mark.parametrize( + "spelling", ["user-name", "u.s.e.r.n.a.m.e", "user$name", "username "] +) +def test_a_name_differing_by_punctuation_is_another_attribute(spelling): + """The nameChar rule of RFC7643 §2.1 makes $, - and _ part of a name, so dropping them would merge two attributes into one.""" + with pytest.raises(ValidationError) as exc_info: + User.model_validate({"schemas": [str(User.__schema__)], spelling: "bjensen"}) + + assert exc_info.value.errors()[0]["loc"] == (spelling,) + + +def test_an_unknown_attribute_is_named_as_the_peer_spelled_it(): + """A refusal quotes what was sent, so that the peer can find it in its own payload.""" + with pytest.raises(ValidationError) as exc_info: + User.model_validate({"schemas": [str(User.__schema__)], "usr_Name": "bjensen"}) + + assert exc_info.value.errors()[0]["loc"] == ("usr_Name",) + + +def test_a_field_is_read_under_the_alias_it_declares(): + """An alias naming an attribute the camel-cased field name would not spell is honoured.""" + + class Aliased(Resource): + __schema__ = URN("urn:example:2.0:Aliased") + + string_field: str | None = Field(None, alias="string_field") + + obj = Aliased.model_validate({"string_field": "x"}) + + assert obj.string_field == "x" + + +def test_an_alias_wins_over_the_python_name_of_another_field(): + """The name an attribute is serialized under is the one SCIM names it by, where a Python field name is only the spelling pydantic offers.""" + + class Aliased(Resource): + __schema__ = URN("urn:example:2.0:Aliased") + + user_name: str | None = None + legacy: str | None = Field(None, serialization_alias="user_name") + + obj = Aliased.model_validate( + {"schemas": [str(Aliased.__schema__)], "userName": "a", "user_name": "b"} + ) + + assert obj.user_name == "a" + assert obj.legacy == "b" + + +def test_a_constructor_keyword_reaches_the_field_the_attribute_name_designates(): + """A keyword is resolved as a payload key is, so an alias covering it takes it.""" + + class Aliased(Resource): + __schema__ = URN("urn:example:2.0:Aliased") + + user_name: str | None = None + legacy: str | None = Field(None, serialization_alias="user_name") + + obj = Aliased(user_name="x") + + assert obj.legacy == "x" + assert obj.user_name is None + + +def test_two_fields_cannot_answer_to_one_attribute_name(): + """A model whose fields share an attribute name is refused where it is written, no payload key being able to reach both.""" + with pytest.raises(TypeError, match="two fields answering"): + + class Ambiguous(Resource): + __schema__ = URN("urn:example:2.0:Ambiguous") + + display_name: str | None = None + legacy: str | None = Field(None, serialization_alias="displayName") + + +def test_an_extension_is_named_by_its_field_as_well_as_by_its_urn(): + """An extension answers to its class name, which is what a dump without aliases carries.""" + extended = User[EnterpriseUser]( + user_name="bjensen", EnterpriseUser=EnterpriseUser(department="Sales") + ) + + assert extended[EnterpriseUser].department == "Sales" + + revalidated = User[EnterpriseUser].model_validate( + extended.model_dump(scim_ctx=None) + ) + + assert revalidated[EnterpriseUser].department == "Sales" + + +def test_a_payload_naming_one_attribute_twice_keeps_the_last_spelling(): + """RFC7643 §2.1 makes two cases of one name the same attribute, so the payload assigns it twice.""" + user = User.model_validate( + {"schemas": [str(User.__schema__)], "userName": "first", "USERNAME": "last"} + ) + + assert user.user_name == "last" + + +def test_the_name_a_field_is_serialized_under_falls_back_on_its_camel_case(): + """Every model carries an alias generator, so the fallback answers for a field defined without one.""" + + class Bare(BaseModel): + model_config = {} + + user_name: str | None = None + + assert Bare._scim_name("user_name") == "userName" + + +def test_an_alias_naming_a_place_in_the_payload_names_no_attribute(): + """An AliasPath reaches into a payload rather than naming an attribute, so it adds no name a peer may use.""" + + class Nested(Resource): + __schema__ = URN("urn:example:2.0:Nested") + + value: str | None = Field(None, validation_alias=AliasPath("outer", "inner")) + + assert "outer" not in Nested.__scim_info__.field_by_name + assert Nested.__scim_info__.validation_names["value"] == "value" diff --git a/tests/test_reference.py b/tests/test_reference.py index d3466667..bd741652 100644 --- a/tests/test_reference.py +++ b/tests/test_reference.py @@ -171,7 +171,7 @@ def test_reference_json_schema_generation(): """Test that models with Reference fields can generate JSON Schema.""" schema = ReferenceTestModel.model_json_schema() assert schema["type"] == "object" - assert "uriref" in schema["properties"] - assert "extref" in schema["properties"] - assert "resourceref" in schema["properties"] - assert "multiref" in schema["properties"] + assert "uriRef" in schema["properties"] + assert "extRef" in schema["properties"] + assert "resourceRef" in schema["properties"] + assert "multiRef" in schema["properties"]