diff --git a/.changelog/_unreleased.toml b/.changelog/_unreleased.toml new file mode 100644 index 0000000..e9fc08d --- /dev/null +++ b/.changelog/_unreleased.toml @@ -0,0 +1,5 @@ +[[entries]] +id = "ab7f2766-e6f5-4236-946d-bddedcd73433" +type = "fix" +description = "Fix ClassDecoratorSetting and get_class_settings to correctly handle __databind_settings__ on subclasses" +author = "@NiklasRosenstein" diff --git a/.github/workflows/python.yaml b/.github/workflows/python.yaml index 822f1f7..7a4e76e 100644 --- a/.github/workflows/python.yaml +++ b/.github/workflows/python.yaml @@ -6,25 +6,25 @@ on: jobs: test: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest strategy: matrix: python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.x"] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - uses: NiklasRosenstein/slap@gha/install/v1 - - uses: actions/setup-python@v2 + - uses: actions/setup-python@v5 with: { python-version: "${{ matrix.python-version }}" } - run: slap install --link --no-venv-check - run: slap test - run: slap publish --dry documentation: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - uses: NiklasRosenstein/slap@gha/install/v1 - - uses: actions/setup-python@v2 + - uses: actions/setup-python@v5 with: { python-version: "3.10" } - run: slap --version && slap install --only-extras docs --no-venv-check - run: slap run --no-venv-check docs:build diff --git a/databind/src/databind/core/settings.py b/databind/src/databind/core/settings.py index 5777529..da8b562 100644 --- a/databind/src/databind/core/settings.py +++ b/databind/src/databind/core/settings.py @@ -176,7 +176,7 @@ def __call__(self, type_: t.Type[T]) -> t.Type[T]: raise RuntimeError("cannot decorate multiple types with the same setting instance") self.bound_to = type_ - settings = getattr(type_, "__databind_settings__", None) + settings = vars(type_).get("__databind_settings__", None) if settings is None: settings = [] setattr(type_, "__databind_settings__", settings) @@ -197,11 +197,14 @@ def get_highest_setting(settings: t.Iterable[T_Setting]) -> "T_Setting | None": def get_class_settings( type_: type, setting_type: t.Type[T_ClassDecoratorSetting] ) -> t.Iterable[T_ClassDecoratorSetting]: - """Returns all matching settings on *type_*.""" - - for item in vars(type_).get("__databind_settings__", []): - if isinstance(item, setting_type): - yield item + """Returns all matching settings on *type_*, traversing the MRO so that parent class settings + are inherited by subclasses. Settings defined on the class itself are yielded before those + inherited from parent classes, giving them priority when priority levels are equal.""" + + for klass in type_.__mro__: + for item in vars(klass).get("__databind_settings__", []): + if isinstance(item, setting_type): + yield item def get_class_setting(type_: type, setting_type: t.Type[T_ClassDecoratorSetting]) -> "T_ClassDecoratorSetting | None": diff --git a/databind/src/databind/core/union.py b/databind/src/databind/core/union.py index 95bb90d..e39f53e 100644 --- a/databind/src/databind/core/union.py +++ b/databind/src/databind/core/union.py @@ -12,7 +12,7 @@ from databind.core.utils import T if sys.version_info[:2] < (3, 10): - from pkg_resources import EntryPoint, iter_entry_points + from pkg_resources import EntryPoint, iter_entry_points # type: ignore[import-not-found,unused-ignore] else: from importlib.metadata import EntryPoint, entry_points @@ -148,7 +148,7 @@ def _entrypoints(self) -> t.Dict[str, EntryPoint]: def get_type_id(self, type_: t.Any) -> str: for ep in self._entrypoints.values(): if ep.load() == type_: - return ep.name + return ep.name # type: ignore[no-any-return,unused-ignore] raise ValueError(f"unable to resolve type {type_!r} to a type ID for {self}") def get_type_by_id(self, type_id: str) -> t.Any: diff --git a/databind/src/databind/json/tests/converters_test.py b/databind/src/databind/json/tests/converters_test.py index 9ef970f..fe14e4d 100644 --- a/databind/src/databind/json/tests/converters_test.py +++ b/databind/src/databind/json/tests/converters_test.py @@ -106,7 +106,7 @@ def test_enum_converter(direction: Direction) -> None: class Pet(enum.Enum): CAT = enum.auto() DOG = enum.auto() - LION: te.Annotated[int, Alias("KITTY")] = enum.auto() + LION: te.Annotated[int, Alias("KITTY")] = enum.auto() # type: ignore[misc,assignment] if direction == Direction.SERIALIZE: assert mapper.convert(direction, Pet.CAT, Pet) == "CAT" @@ -717,3 +717,79 @@ def of(cls, v: str) -> "MyCls": mapper = make_mapper([JsonConverterSupport()]) assert mapper.serialize(MyCls(), MyCls) == "MyCls" assert mapper.deserialize("MyCls", MyCls) == MyCls() + + +def test_extra_keys_on_subclass_creates_own_settings() -> None: + """Regression test: ExtraKeys() applied to a subclass must create its own __databind_settings__, + not append to the parent's list via MRO traversal.""" + from databind.core.settings import get_class_settings + + @ExtraKeys() + @dataclasses.dataclass + class Parent: + a: int + + @ExtraKeys() + @dataclasses.dataclass + class Child(Parent): + b: str = "" + + # Each class must have its own independent __databind_settings__ list. + assert "__databind_settings__" in vars(Parent) + assert "__databind_settings__" in vars(Child) + assert vars(Parent)["__databind_settings__"] is not vars(Child)["__databind_settings__"] + + # get_class_settings must find ExtraKeys on each class independently. + assert list(get_class_settings(Parent, ExtraKeys)) != [] + assert list(get_class_settings(Child, ExtraKeys)) != [] + + # Parent's settings list must not be polluted with Child's decorator. + assert len(vars(Parent)["__databind_settings__"]) == 1 + + +def test_extra_keys_subclass_deserialization_allows_extra_keys() -> None: + """Regression test: ExtraKeys() on a subclass must allow extra keys during deserialization.""" + mapper = make_mapper([SchemaConverter(), PlainDatatypeConverter()]) + + @ExtraKeys() + @dataclasses.dataclass + class Parent: + a: int + + @ExtraKeys() + @dataclasses.dataclass + class Child(Parent): + b: str = "" + + # Child should allow extra keys because it is decorated with ExtraKeys(). + result = mapper.deserialize({"a": 1, "b": "hello", "extra": "ignored"}, Child) + assert result == Child(a=1, b="hello") + + +def test_extra_keys_parent_decorated_child_inherits_and_can_override() -> None: + """When only the parent has ExtraKeys(), the child inherits that permission via MRO traversal. + The child can override it by decorating with @ExtraKeys(allow=False).""" + mapper = make_mapper([SchemaConverter(), PlainDatatypeConverter()]) + + @ExtraKeys() + @dataclasses.dataclass + class Parent: + a: int + + @dataclasses.dataclass + class ChildInheriting(Parent): + b: str = "" + + @ExtraKeys(allow=False) + @dataclasses.dataclass + class ChildOverriding(Parent): + b: str = "" + + # Child inherits ExtraKeys() from parent, so extra keys are allowed. + result = mapper.deserialize({"a": 1, "b": "hello", "extra": "ignored"}, ChildInheriting) + assert result == ChildInheriting(a=1, b="hello") + + # Child explicitly overrides with ExtraKeys(allow=False), so extra keys raise an error. + with pytest.raises(ConversionError) as excinfo: + mapper.deserialize({"a": 1, "b": "hello", "extra": "ignored"}, ChildOverriding) + assert "extra" in str(excinfo.value)