From 2bd506267699540be9710919c0628d8923df560a Mon Sep 17 00:00:00 2001 From: Juliana Karoline Date: Mon, 21 Oct 2024 13:12:58 -0300 Subject: [PATCH 1/4] Add validation for non-existing items to update method on CRUD --- fastcrud/crud/fast_crud.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fastcrud/crud/fast_crud.py b/fastcrud/crud/fast_crud.py index 49a57f9..8f839f3 100644 --- a/fastcrud/crud/fast_crud.py +++ b/fastcrud/crud/fast_crud.py @@ -2145,6 +2145,7 @@ async def update( Raises: MultipleResultsFound: If `allow_multiple` is `False` and more than one record matches the filters. + NoResultFound: If no record matches the filters. ValueError: If extra fields not present in the model are provided in the update data. ValueError: If `return_as_model` is `True` but `schema_to_select` is not provided. @@ -2199,7 +2200,10 @@ async def update( ) ``` """ - if not allow_multiple and (total_count := await self.count(db, **kwargs)) > 1: + total_count = await self.count(db, **kwargs) + if total_count == 0: + raise NoResultFound("No record found to update.") + if not allow_multiple and total_count > 1: raise MultipleResultsFound( f"Expected exactly one record to update, found {total_count}." ) From 1c01f6814953e17b236a0d559fd0151d6bcab705 Mon Sep 17 00:00:00 2001 From: Juliana Karoline Date: Mon, 21 Oct 2024 14:48:32 -0300 Subject: [PATCH 2/4] Update unit tests --- .../crud/test_get_multi_by_cursor.py | 2 +- tests/sqlalchemy/crud/test_update.py | 22 ++++++++----------- .../sqlmodel/crud/test_get_multi_by_cursor.py | 2 +- tests/sqlmodel/crud/test_update.py | 22 ++++++++----------- 4 files changed, 20 insertions(+), 28 deletions(-) diff --git a/tests/sqlalchemy/crud/test_get_multi_by_cursor.py b/tests/sqlalchemy/crud/test_get_multi_by_cursor.py index adec46f..ffa1fb1 100644 --- a/tests/sqlalchemy/crud/test_get_multi_by_cursor.py +++ b/tests/sqlalchemy/crud/test_get_multi_by_cursor.py @@ -124,7 +124,7 @@ async def test_get_multi_by_cursor_pagination_integrity(async_session, test_data db=async_session, object={"name": "Updated Name"}, allow_multiple=True, - name="SpecificName", + name=test_data[0]["name"], ) second_batch = await crud.get_multi_by_cursor( diff --git a/tests/sqlalchemy/crud/test_update.py b/tests/sqlalchemy/crud/test_update.py index b917f44..34bc444 100644 --- a/tests/sqlalchemy/crud/test_update.py +++ b/tests/sqlalchemy/crud/test_update.py @@ -2,7 +2,8 @@ import pytest from sqlalchemy import select -from sqlalchemy.exc import MultipleResultsFound +from sqlalchemy.exc import MultipleResultsFound, NoResultFound + from fastcrud.crud.fast_crud import FastCRUD from ...sqlalchemy.conftest import ModelTest, UpdateSchemaTest, ModelTestWithTimestamp @@ -51,12 +52,11 @@ async def test_update_non_existent_record(async_session, test_data): crud = FastCRUD(ModelTest) non_existent_id = 99999 updated_data = {"name": "New Name"} - await crud.update(db=async_session, object=updated_data, id=non_existent_id) - record = await async_session.execute( - select(ModelTest).where(ModelTest.id == non_existent_id) - ) - assert record.scalar_one_or_none() is None + with pytest.raises(NoResultFound) as exc_info: + await crud.update(db=async_session, object=updated_data, id=non_existent_id) + + assert "No record found to update" in str(exc_info.value) @pytest.mark.asyncio @@ -69,14 +69,10 @@ async def test_update_invalid_filters(async_session, test_data): updated_data = {"name": "New Name"} non_matching_filter = {"name": "NonExistingName"} - await crud.update(db=async_session, object=updated_data, **non_matching_filter) + with pytest.raises(NoResultFound) as exc_info: + await crud.update(db=async_session, object=updated_data, **non_matching_filter) - for item in test_data: - record = await async_session.execute( - select(ModelTest).where(ModelTest.id == item["id"]) - ) - fetched_record = record.scalar_one() - assert fetched_record.name != "New Name" + assert "No record found to update" in str(exc_info.value) @pytest.mark.asyncio diff --git a/tests/sqlmodel/crud/test_get_multi_by_cursor.py b/tests/sqlmodel/crud/test_get_multi_by_cursor.py index 764c285..7301c14 100644 --- a/tests/sqlmodel/crud/test_get_multi_by_cursor.py +++ b/tests/sqlmodel/crud/test_get_multi_by_cursor.py @@ -124,7 +124,7 @@ async def test_get_multi_by_cursor_pagination_integrity(async_session, test_data db=async_session, object={"name": "Updated Name"}, allow_multiple=True, - name="SpecificName", + name=test_data[0]["name"], ) second_batch = await crud.get_multi_by_cursor( diff --git a/tests/sqlmodel/crud/test_update.py b/tests/sqlmodel/crud/test_update.py index 9a9878d..0209f05 100644 --- a/tests/sqlmodel/crud/test_update.py +++ b/tests/sqlmodel/crud/test_update.py @@ -2,7 +2,7 @@ import pytest from sqlalchemy import select -from sqlalchemy.exc import MultipleResultsFound +from sqlalchemy.exc import MultipleResultsFound, NoResultFound from fastcrud.crud.fast_crud import FastCRUD from ...sqlmodel.conftest import ModelTest, UpdateSchemaTest, ModelTestWithTimestamp @@ -51,12 +51,11 @@ async def test_update_non_existent_record(async_session, test_data): crud = FastCRUD(ModelTest) non_existent_id = 99999 updated_data = {"name": "New Name"} - await crud.update(db=async_session, object=updated_data, id=non_existent_id) - record = await async_session.execute( - select(ModelTest).where(ModelTest.id == non_existent_id) - ) - assert record.scalar_one_or_none() is None + with pytest.raises(NoResultFound) as exc_info: + await crud.update(db=async_session, object=updated_data, id=non_existent_id) + + assert "No record found to update" in str(exc_info.value) @pytest.mark.asyncio @@ -69,14 +68,11 @@ async def test_update_invalid_filters(async_session, test_data): updated_data = {"name": "New Name"} non_matching_filter = {"name": "NonExistingName"} - await crud.update(db=async_session, object=updated_data, **non_matching_filter) - for item in test_data: - record = await async_session.execute( - select(ModelTest).where(ModelTest.id == item["id"]) - ) - fetched_record = record.scalar_one() - assert fetched_record.name != "New Name" + with pytest.raises(NoResultFound) as exc_info: + await crud.update(db=async_session, object=updated_data, **non_matching_filter) + + assert "No record found to update" in str(exc_info.value) @pytest.mark.asyncio From 0cb5cb80d54c163e178811acc61b598d7632b51c Mon Sep 17 00:00:00 2001 From: Juliana Karoline Date: Mon, 21 Oct 2024 15:46:40 -0300 Subject: [PATCH 3/4] Add deprecation warning until the next 2 releases --- fastcrud/crud/fast_crud.py | 8 ++++++- tests/sqlalchemy/crud/test_update.py | 32 ++++++++++++++++++++++++++-- tests/sqlmodel/crud/test_update.py | 30 +++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/fastcrud/crud/fast_crud.py b/fastcrud/crud/fast_crud.py index 8f839f3..10b1885 100644 --- a/fastcrud/crud/fast_crud.py +++ b/fastcrud/crud/fast_crud.py @@ -1,5 +1,6 @@ from typing import Any, Dict, Generic, Union, Optional, Callable from datetime import datetime, timezone +import warnings from pydantic import BaseModel, ValidationError from sqlalchemy import ( @@ -2202,7 +2203,12 @@ async def update( """ total_count = await self.count(db, **kwargs) if total_count == 0: - raise NoResultFound("No record found to update.") + warnings.warn( + "Passing non-existing records to `update` will raise NoResultFound on version 0.15.3.", + DeprecationWarning, + stacklevel=2, + ) + # raise NoResultFound("No record found to update.") if not allow_multiple and total_count > 1: raise MultipleResultsFound( f"Expected exactly one record to update, found {total_count}." diff --git a/tests/sqlalchemy/crud/test_update.py b/tests/sqlalchemy/crud/test_update.py index 34bc444..9680320 100644 --- a/tests/sqlalchemy/crud/test_update.py +++ b/tests/sqlalchemy/crud/test_update.py @@ -2,8 +2,7 @@ import pytest from sqlalchemy import select -from sqlalchemy.exc import MultipleResultsFound, NoResultFound - +from sqlalchemy.exc import MultipleResultsFound from fastcrud.crud.fast_crud import FastCRUD from ...sqlalchemy.conftest import ModelTest, UpdateSchemaTest, ModelTestWithTimestamp @@ -52,11 +51,24 @@ async def test_update_non_existent_record(async_session, test_data): crud = FastCRUD(ModelTest) non_existent_id = 99999 updated_data = {"name": "New Name"} + """ + In version 0.15.3, the `update` method will raise a `NoResultFound` exception: + ``` with pytest.raises(NoResultFound) as exc_info: await crud.update(db=async_session, object=updated_data, id=non_existent_id) assert "No record found to update" in str(exc_info.value) + ``` + + For 0.15.2, the test will check if the record is not updated. + """ + await crud.update(db=async_session, object=updated_data, id=non_existent_id) + + record = await async_session.execute( + select(ModelTest).where(ModelTest.id == non_existent_id) + ) + assert record.scalar_one_or_none() is None @pytest.mark.asyncio @@ -69,10 +81,26 @@ async def test_update_invalid_filters(async_session, test_data): updated_data = {"name": "New Name"} non_matching_filter = {"name": "NonExistingName"} + """ + In version 0.15.3, the `update` method will raise a `NoResultFound` exception: + + ``` with pytest.raises(NoResultFound) as exc_info: await crud.update(db=async_session, object=updated_data, **non_matching_filter) assert "No record found to update" in str(exc_info.value) + ``` + + For 0.15.2, the test will check if the record is not updated. + """ + await crud.update(db=async_session, object=updated_data, **non_matching_filter) + + for item in test_data: + record = await async_session.execute( + select(ModelTest).where(ModelTest.id == item["id"]) + ) + fetched_record = record.scalar_one() + assert fetched_record.name != "New Name" @pytest.mark.asyncio diff --git a/tests/sqlmodel/crud/test_update.py b/tests/sqlmodel/crud/test_update.py index 0209f05..2968674 100644 --- a/tests/sqlmodel/crud/test_update.py +++ b/tests/sqlmodel/crud/test_update.py @@ -2,7 +2,7 @@ import pytest from sqlalchemy import select -from sqlalchemy.exc import MultipleResultsFound, NoResultFound +from sqlalchemy.exc import MultipleResultsFound from fastcrud.crud.fast_crud import FastCRUD from ...sqlmodel.conftest import ModelTest, UpdateSchemaTest, ModelTestWithTimestamp @@ -51,11 +51,24 @@ async def test_update_non_existent_record(async_session, test_data): crud = FastCRUD(ModelTest) non_existent_id = 99999 updated_data = {"name": "New Name"} + """ + In version 0.15.3, the `update` method will raise a `NoResultFound` exception: + ``` with pytest.raises(NoResultFound) as exc_info: await crud.update(db=async_session, object=updated_data, id=non_existent_id) assert "No record found to update" in str(exc_info.value) + ``` + + For 0.15.2, the test will check if the record is not updated. + """ + await crud.update(db=async_session, object=updated_data, id=non_existent_id) + + record = await async_session.execute( + select(ModelTest).where(ModelTest.id == non_existent_id) + ) + assert record.scalar_one_or_none() is None @pytest.mark.asyncio @@ -68,11 +81,26 @@ async def test_update_invalid_filters(async_session, test_data): updated_data = {"name": "New Name"} non_matching_filter = {"name": "NonExistingName"} + """ + In version 0.15.3, the `update` method will raise a `NoResultFound` exception: + ``` with pytest.raises(NoResultFound) as exc_info: await crud.update(db=async_session, object=updated_data, **non_matching_filter) assert "No record found to update" in str(exc_info.value) + ``` + + For 0.15.2, the test will check if the record is not updated. + """ + await crud.update(db=async_session, object=updated_data, **non_matching_filter) + + for item in test_data: + record = await async_session.execute( + select(ModelTest).where(ModelTest.id == item["id"]) + ) + fetched_record = record.scalar_one() + assert fetched_record.name != "New Name" @pytest.mark.asyncio From 46bac8a9139ad00cc451de939afc11a7545a3eff Mon Sep 17 00:00:00 2001 From: Juliana Karoline Date: Mon, 21 Oct 2024 15:47:42 -0300 Subject: [PATCH 4/4] Update docstring --- fastcrud/crud/fast_crud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fastcrud/crud/fast_crud.py b/fastcrud/crud/fast_crud.py index 10b1885..4c46291 100644 --- a/fastcrud/crud/fast_crud.py +++ b/fastcrud/crud/fast_crud.py @@ -2146,7 +2146,7 @@ async def update( Raises: MultipleResultsFound: If `allow_multiple` is `False` and more than one record matches the filters. - NoResultFound: If no record matches the filters. + NoResultFound: If no record matches the filters. (on version 0.15.3) ValueError: If extra fields not present in the model are provided in the update data. ValueError: If `return_as_model` is `True` but `schema_to_select` is not provided.