fix: lazy attributes getting accessed on repository update method (#553)

This fixes #552 by moving the guard condition up preventing the `getattr` from triggering the issue.

Author: Glinte

advanced_alchemy/repository/_async.py

@@ -1510,17 +1510,17 @@ async def update(
 
                     # Handle relationships by merging objects into session first
                     for relationship in mapper.mapper.relationships:
+                        if relationship.viewonly or relationship.lazy in {  # pragma: no cover
+                            "write_only",
+                            "dynamic",
+                            "raise",
+                            "raise_on_sql",
+                        }:
+                            # Skip relationships with incompatible lazy loading strategies
+                            continue
+
                         if (new_value := getattr(data, relationship.key, MISSING)) is not MISSING:
                             # Skip relationships that cannot be handled by generic merge operations
-                            if relationship.viewonly or relationship.lazy in {  # pragma: no cover
-                                "write_only",
-                                "dynamic",
-                                "raise",
-                                "raise_on_sql",
-                            }:
-                                # Skip relationships with incompatible lazy loading strategies
-                                continue
-
                             if isinstance(new_value, list):
                                 merged_values = [  # pyright: ignore
                                     await self.session.merge(item, load=False)  # pyright: ignore

advanced_alchemy/repository/_sync.py

@@ -1511,17 +1511,17 @@ def update(
 
                     # Handle relationships by merging objects into session first
                     for relationship in mapper.mapper.relationships:
+                        if relationship.viewonly or relationship.lazy in {  # pragma: no cover
+                            "write_only",
+                            "dynamic",
+                            "raise",
+                            "raise_on_sql",
+                        }:
+                            # Skip relationships with incompatible lazy loading strategies
+                            continue
+
                         if (new_value := getattr(data, relationship.key, MISSING)) is not MISSING:
                             # Skip relationships that cannot be handled by generic merge operations
-                            if relationship.viewonly or relationship.lazy in {  # pragma: no cover
-                                "write_only",
-                                "dynamic",
-                                "raise",
-                                "raise_on_sql",
-                            }:
-                                # Skip relationships with incompatible lazy loading strategies
-                                continue
-
                             if isinstance(new_value, list):
                                 merged_values = [  # pyright: ignore
                                     self.session.merge(item, load=False)  # pyright: ignore

tests/unit/test_repository.py

@@ -6,15 +6,15 @@
 import decimal
 from collections.abc import AsyncGenerator, Collection, Generator
 from typing import TYPE_CHECKING, Any, Union, cast
-from unittest.mock import AsyncMock, MagicMock
+from unittest.mock import AsyncMock, MagicMock, PropertyMock
 from uuid import uuid4
 
 import pytest
 from msgspec import Struct
 from pydantic import BaseModel
 from pytest_lazy_fixtures import lf
 from sqlalchemy import Integer, String
-from sqlalchemy.exc import SQLAlchemyError
+from sqlalchemy.exc import InvalidRequestError, SQLAlchemyError
 from sqlalchemy.ext.asyncio import AsyncSession
 from sqlalchemy.orm import InstrumentedAttribute, Mapped, Session, mapped_column
 from sqlalchemy.types import TypeEngine
@@ -1509,11 +1509,12 @@ async def test_update_skips_raise_lazy_relationships(
     mock_mapper.mapper.columns = []
     mock_mapper.mapper.relationships = [mock_relationship]
 
-    # Mock the data object to have the raise relationship attribute
-    mock_instance.items = MagicMock()
+    # Mock the data object to raise an error when accessing the relationship
+    type(mock_instance).items = PropertyMock(side_effect=InvalidRequestError)
 
     mocker.patch.object(mock_repo, "get_id_attribute_value", return_value=id_)
     mocker.patch.object(mock_repo, "get", return_value=existing_instance)
+    mocker.patch("advanced_alchemy.repository._sync.inspect", return_value=mock_mapper)
     mocker.patch("advanced_alchemy.repository._async.inspect", return_value=mock_mapper)
     mock_repo.session.merge.return_value = existing_instance  # pyright: ignore[reportFunctionMemberAccess]