From 4a1934b7b4545ee1700f5a9669f1ad8543e54fca Mon Sep 17 00:00:00 2001 From: Fire162 Date: Tue, 6 Oct 2026 10:04:36 +0000 Subject: [PATCH] fix: default relationship fields to required=False and avoid SAWarning on null keys (#664) - Set default required=False on relationship fields generated by ModelConverter while keeping allow_none matching relationship nullability. - Prevent ValidationError ('Missing data for required field') when deserializing models with include_relationships=True using foreign key values or nested data. - Avoid SAWarning in Related._get_existing_instance when all lookup values are None. - Add regression tests for schema.load with non-nullable relationships and warning prevention. --- CHANGELOG.rst | 6 +++ src/marshmallow_sqlalchemy/convert.py | 2 +- src/marshmallow_sqlalchemy/fields.py | 2 + tests/test_conversion.py | 2 +- tests/test_sqlalchemy_schema.py | 56 +++++++++++++++++++++++++++ 5 files changed, 66 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5b5be9d..e966728 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,6 +4,12 @@ Changelog unreleased ++++++++++ +Bug fixes: + +* Ensure relationship fields default to ``required=False`` while maintaining ``allow_none=False`` for non-nullable relationships, allowing deserialization via foreign keys or nested instances (:issue:`664`). + Thanks :user:`Curiouspaul1` and :user:`n8harmon` for reporting. +* Avoid ``SAWarning`` when querying related instances with all-None lookup values (:issue:`664`). + Other changes: * Drop support for marshmallow 3, which is EOL. diff --git a/src/marshmallow_sqlalchemy/convert.py b/src/marshmallow_sqlalchemy/convert.py index 2b7c670..e2a1d06 100644 --- a/src/marshmallow_sqlalchemy/convert.py +++ b/src/marshmallow_sqlalchemy/convert.py @@ -465,7 +465,7 @@ def _add_relationship_kwargs( ): nullable = False break - kwargs.update({"allow_none": nullable, "required": not nullable}) + kwargs.update({"allow_none": nullable, "required": False}) def _should_exclude_field( self, diff --git a/src/marshmallow_sqlalchemy/fields.py b/src/marshmallow_sqlalchemy/fields.py index 35c6f54..4021280 100644 --- a/src/marshmallow_sqlalchemy/fields.py +++ b/src/marshmallow_sqlalchemy/fields.py @@ -130,6 +130,8 @@ def _get_existing_instance(self, related_model, value): else: # Use a faster path if the related key is the primary key. lookup_values = [value.get(prop.key) for prop in self.related_keys] + if all(v is None for v in lookup_values): + raise NoResultFound try: result = self.session.get(related_model, lookup_values) except TypeError as error: diff --git a/tests/test_conversion.py b/tests/test_conversion.py index dd467cc..092bbd9 100644 --- a/tests/test_conversion.py +++ b/tests/test_conversion.py @@ -100,7 +100,7 @@ def test_many_to_one_relationship(self, models): current_school_field = student_fields["current_school"] assert type(current_school_field) is Related assert current_school_field.allow_none is False - assert current_school_field.required is True + assert current_school_field.required is False school_fields = fields_for_model(models.School, include_relationships=True) assert type(school_fields["students"]) is RelatedList diff --git a/tests/test_sqlalchemy_schema.py b/tests/test_sqlalchemy_schema.py index 386e89c..30be1ec 100644 --- a/tests/test_sqlalchemy_schema.py +++ b/tests/test_sqlalchemy_schema.py @@ -1,5 +1,6 @@ from __future__ import annotations +import warnings from importlib.metadata import version import marshmallow @@ -438,6 +439,61 @@ def test_load(self, teacher, SchemaClass, models): assert isinstance(load_data, models.Teacher) + def test_load_student_with_non_nullable_relationship(self, models, session, school): + class StudentSchema(SQLAlchemyAutoSchema): + class Meta: + model = models.Student + include_relationships = True + include_fk = True + load_instance = True + sqla_session = session + + schema = StudentSchema() + student = schema.load( + {"full_name": "Harry Potter", "current_school_id": school.id} + ) + assert student.full_name == "Harry Potter" + assert student.current_school_id == school.id + + def test_load_student_with_non_nullable_relationship_disallows_none( + self, models, session + ): + class StudentSchema(SQLAlchemyAutoSchema): + class Meta: + model = models.Student + include_relationships = True + include_fk = True + load_instance = True + sqla_session = session + + schema = StudentSchema() + with pytest.raises(marshmallow.ValidationError) as excinfo: + schema.load({"full_name": "Harry Potter", "current_school": None}) + assert "current_school" in excinfo.value.messages + assert "Field may not be null." in excinfo.value.messages["current_school"] + + def test_related_field_all_none_lookup_values_no_warning(self, models, session): + class StudentSchema(SQLAlchemyAutoSchema): + class Meta: + model = models.Student + include_relationships = True + include_fk = False + load_instance = True + sqla_session = session + + schema = StudentSchema() + with warnings.catch_warnings(): + warnings.simplefilter("error", sa.exc.SAWarning) + # Providing related dict without primary key should not raise SAWarning + student = schema.load( + { + "full_name": "Ron Weasley", + "current_school": {"name": "Hogwarts School"}, + } + ) + assert student.full_name == "Ron Weasley" + assert student.current_school.name == "Hogwarts School" + def test_load_transient(self, models, teacher): class TeacherSchema(SQLAlchemyAutoSchema): class Meta: