fix: default relationship fields to required=False to avoid missing data errors on load (#664)

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.

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,

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:

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

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: