From d6339d0dc66afd0a4ae4ebc48c9a948b7332c86a Mon Sep 17 00:00:00 2001 From: Valentin Ambroise <113367796+vaamb@users.noreply.github.com> Date: Fri, 31 Jan 2025 19:58:24 +0100 Subject: [PATCH] Improve `CRUDMixin.{create, update}` to receive empty values (#165) * Allow `CRUDMixin.create`'s `values` to be empty in the case where `lookup_keys` represent the whole data * Use `gaia_validators`'s `missing` to mark an unchanged field in `CRUDMixin.update`'s `values` * Use `gaia_validators`'s `missing` in update validation models * Add a missing values removal step to `CalendarEvent.update()` method * Use `payload.model_dump(exclude_defaults=True)` in update routes --- src/ouranos/core/database/models/abc.py | 13 +++++-- src/ouranos/core/database/models/app.py | 8 +++- src/ouranos/core/database/models/caching.py | 2 +- src/ouranos/core/database/models/gaia.py | 2 +- .../web_server/routes/gaia/ecosystem.py | 8 ++-- .../web_server/routes/gaia/hardware.py | 2 +- .../web_server/routes/services/calendar.py | 5 +-- src/ouranos/web_server/routes/user.py | 4 +- src/ouranos/web_server/validate/calendar.py | 12 +++--- .../web_server/validate/gaia/ecosystem.py | 39 ++++++++++--------- .../web_server/validate/gaia/hardware.py | 18 ++++----- src/ouranos/web_server/validate/user.py | 8 ++-- src/ouranos/web_server/validate/wiki.py | 6 ++- tests/web_server/routes/hardware.py | 4 +- 14 files changed, 73 insertions(+), 58 deletions(-) diff --git a/src/ouranos/core/database/models/abc.py b/src/ouranos/core/database/models/abc.py index df206ce6..e40f6f56 100644 --- a/src/ouranos/core/database/models/abc.py +++ b/src/ouranos/core/database/models/abc.py @@ -9,6 +9,8 @@ from sqlalchemy import and_, delete, insert, inspect, Select, select, update from sqlalchemy.ext.asyncio import AsyncSession +from gaia_validators import missing + from ouranos import db @@ -51,10 +53,11 @@ async def create( cls, session: AsyncSession, /, - values: dict, + values: dict | None = None, **lookup_keys: lookup_keys_type, ) -> None: cls._check_lookup_keys(*lookup_keys.keys()) + values = values or {} stmt = insert(cls).values(**lookup_keys, **values) await session.execute(stmt) @@ -147,7 +150,11 @@ async def update( for key, value in lookup_keys.items() ) ) - .values(**values) + .values({ + key: value + for key, value in values.items() + if value is not missing + }) ) await session.execute(stmt) @@ -187,7 +194,7 @@ async def update_or_create( cls, session: AsyncSession, /, - values: dict, + values: dict | None = None, **lookup_keys: lookup_keys_type, ) -> None: #cls._check_lookup_keys(*lookup_keys.keys()) diff --git a/src/ouranos/core/database/models/app.py b/src/ouranos/core/database/models/app.py index 1b99b7d0..160349d4 100644 --- a/src/ouranos/core/database/models/app.py +++ b/src/ouranos/core/database/models/app.py @@ -16,7 +16,7 @@ from sqlalchemy.sql import func import gaia_validators as gv -from gaia_validators import safe_enum_from_name +from gaia_validators import missing, safe_enum_from_name from ouranos import current_app from ouranos.core.config.consts import ( @@ -754,7 +754,11 @@ async def update( stmt = ( update(cls) .where(cls.id == event_id) - .values(values) + .values({ + key: value + for key, value in values.items() + if value is not missing + }) ) await session.execute(stmt) diff --git a/src/ouranos/core/database/models/caching.py b/src/ouranos/core/database/models/caching.py index ce3d16ab..d6f66e17 100644 --- a/src/ouranos/core/database/models/caching.py +++ b/src/ouranos/core/database/models/caching.py @@ -275,7 +275,7 @@ async def create( cls, session: AsyncSession, /, - values: dict, + values: dict | None = None, **lookup_keys: lookup_keys_type, ) -> Self | None: return await super().create(session, values=values, **lookup_keys) diff --git a/src/ouranos/core/database/models/gaia.py b/src/ouranos/core/database/models/gaia.py index ab2f7fc5..9d92cd52 100644 --- a/src/ouranos/core/database/models/gaia.py +++ b/src/ouranos/core/database/models/gaia.py @@ -718,7 +718,7 @@ async def create( cls, session: AsyncSession, /, - values: gv.HardwareConfigDict, + values: gv.HardwareConfigDict | None = None, **lookup_keys: str | Enum | UUID, ) -> None: measures: list[gv.Measure | gv.MeasureDict] = values.pop("measures", []) diff --git a/src/ouranos/web_server/routes/gaia/ecosystem.py b/src/ouranos/web_server/routes/gaia/ecosystem.py index fbb53431..4a64b122 100644 --- a/src/ouranos/web_server/routes/gaia/ecosystem.py +++ b/src/ouranos/web_server/routes/gaia/ecosystem.py @@ -147,7 +147,7 @@ async def update_ecosystem( session: Annotated[AsyncSession, Depends(get_session)], ): ecosystem = await ecosystem_or_abort(session, ecosystem_uid) - ecosystem_dict = payload.model_dump() + ecosystem_dict = payload.model_dump(exclude_defaults=True) try: await emit_crud_event( ecosystem, gv.CrudAction.update, "ecosystem", @@ -247,7 +247,7 @@ async def update_management( session: Annotated[AsyncSession, Depends(get_session)], ): ecosystem = await ecosystem_or_abort(session, ecosystem_uid) - management_dict = payload.model_dump() + management_dict = payload.model_dump(exclude_defaults=True) try: await emit_crud_event( ecosystem, gv.CrudAction.update, "management", management_dict) @@ -328,7 +328,7 @@ async def update_ecosystem_lighting( session: Annotated[AsyncSession, Depends(get_session)], ): ecosystem = await ecosystem_or_abort(session, ecosystem_uid) - lighting_dict = payload.model_dump() + lighting_dict = payload.model_dump(exclude_defaults=True) try: await emit_crud_event( ecosystem, gv.CrudAction.update, "nycthemeral_config", lighting_dict) @@ -462,7 +462,7 @@ async def update_environment_parameter( ): ecosystem = await ecosystem_or_abort(session, ecosystem_uid) await environment_parameter_or_abort(session, ecosystem.uid, parameter) - environment_parameter_dict = payload.model_dump() + environment_parameter_dict = payload.model_dump(exclude_defaults=True) environment_parameter_dict["parameter"] = parameter try: await emit_crud_event( diff --git a/src/ouranos/web_server/routes/gaia/hardware.py b/src/ouranos/web_server/routes/gaia/hardware.py index 6b5cc3d3..80c82bcd 100644 --- a/src/ouranos/web_server/routes/gaia/hardware.py +++ b/src/ouranos/web_server/routes/gaia/hardware.py @@ -167,7 +167,7 @@ async def update_hardware( ): ecosystem = await ecosystem_or_abort(session, ecosystem_uid) hardware = await hardware_or_abort(session, hardware_uid) - hardware_dict = payload.model_dump() + hardware_dict = payload.model_dump(exclude_defaults=True) try: await emit_crud_event( ecosystem, gv.CrudAction.update, "hardware", hardware_dict) diff --git a/src/ouranos/web_server/routes/services/calendar.py b/src/ouranos/web_server/routes/services/calendar.py index 22307dce..69758699 100644 --- a/src/ouranos/web_server/routes/services/calendar.py +++ b/src/ouranos/web_server/routes/services/calendar.py @@ -95,10 +95,7 @@ async def update_event( event = await CalendarEvent.get(session, event_id=event_id) if event.created_by != current_user.id and not current_user.can(Permission.ADMIN): raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) - values = { - key: value for key, value in payload.model_dump().items() - if value is not None - } + values = payload.model_dump(exclude_defaults=True) await CalendarEvent.update(session, event_id=event_id, values=values) return ResultResponse( msg=f"Updated event with id '{event_id}'", diff --git a/src/ouranos/web_server/routes/user.py b/src/ouranos/web_server/routes/user.py index dd460895..59bdb783 100644 --- a/src/ouranos/web_server/routes/user.py +++ b/src/ouranos/web_server/routes/user.py @@ -122,10 +122,10 @@ async def update_user( status_code=status.HTTP_403_FORBIDDEN, detail="You can only update profiles with permissions lower than yours.", ) - user_dict = payload.model_dump() + user_dict = payload.model_dump(exclude_defaults=True) user_dict = { key: value for key, value in user_dict.items() - if value is not None and value != getattr(user, key) + if value != getattr(user, key) } try: await User.update(session, user_id=user.id, values=user_dict) diff --git a/src/ouranos/web_server/validate/calendar.py b/src/ouranos/web_server/validate/calendar.py index 5bdfec9c..a12d1da6 100644 --- a/src/ouranos/web_server/validate/calendar.py +++ b/src/ouranos/web_server/validate/calendar.py @@ -5,7 +5,7 @@ from pydantic import field_validator import gaia_validators as gv -from gaia_validators import safe_enum_from_name +from gaia_validators import MissingValue, missing, safe_enum_from_name from ouranos.core.validate.base import BaseModel @@ -31,11 +31,11 @@ def parse_datetime(cls, value): class EventUpdatePayload(BaseModel): - level: gv.WarningLevel | None = None - title: str | None = None - description: str | None = None - start_time: datetime | None = None - end_time: datetime | None = None + level: gv.WarningLevel | MissingValue = missing + title: str | MissingValue = missing + description: str | None | MissingValue = missing + start_time: datetime | MissingValue = missing + end_time: datetime | MissingValue = missing @field_validator("level", mode="before") def parse_level(cls, value): diff --git a/src/ouranos/web_server/validate/gaia/ecosystem.py b/src/ouranos/web_server/validate/gaia/ecosystem.py index 05532d5c..1e7b89ce 100644 --- a/src/ouranos/web_server/validate/gaia/ecosystem.py +++ b/src/ouranos/web_server/validate/gaia/ecosystem.py @@ -6,7 +6,7 @@ from pydantic import ConfigDict, Field, field_serializer, field_validator import gaia_validators as gv -from gaia_validators import safe_enum_from_name +from gaia_validators import MissingValue, missing, safe_enum_from_name from ouranos.core.database.models.gaia import Ecosystem, ActuatorState from ouranos.core.validate.base import BaseModel @@ -47,8 +47,8 @@ def parse_time(cls, value): class EcosystemBaseInfoUpdatePayload(BaseModel): - name: str | None = None - status: bool | None = None + name: str | MissingValue = missing + status: bool | MissingValue = missing EcosystemInfo = sqlalchemy_to_pydantic( @@ -67,14 +67,14 @@ class EcosystemBaseInfoUpdatePayload(BaseModel): # Ecosystem management # --------------------------------------------------------------------------- class EcosystemManagementUpdatePayload(BaseModel): - sensors: bool | None = None - light: bool | None = None - climate: bool | None = None - watering: bool | None = None - health: bool | None = None - alarms: bool | None = None - pictures: bool | None = None - database: bool | None = None + sensors: bool | MissingValue = missing + light: bool | MissingValue = missing + climate: bool | MissingValue = missing + watering: bool | MissingValue = missing + health: bool | MissingValue = missing + alarms: bool | MissingValue = missing + pictures: bool | MissingValue = missing + database: bool | MissingValue = missing class ManagementInfo(BaseModel): @@ -118,11 +118,11 @@ class EcosystemLightInfo(gv.LightingHours, _EcosystemLightInfo): class NycthemeralCycleUpdatePayload(BaseModel): - span: gv.NycthemeralSpanMethod | None = None - lighting: gv.LightingMethod | None = None - target: str | None = None - day: time | None = None - night: time | None = None + span: gv.NycthemeralSpanMethod | MissingValue = missing + lighting: gv.LightingMethod | MissingValue = missing + target: str | None | MissingValue = missing + day: time | MissingValue = missing + night: time | MissingValue = missing @field_validator("span", mode="before") def parse_span(cls, value): @@ -153,9 +153,10 @@ class EnvironmentParameterCreationPayload(gv.ClimateConfig): class EnvironmentParameterUpdatePayload(BaseModel): - day: float | None = None - night: float | None = None - hysteresis: float | None = None + day: float | MissingValue = missing + night: float | MissingValue = missing + hysteresis: float | MissingValue = missing + alarm: float | None | MissingValue = missing class EnvironmentParameterInfo(BaseModel): diff --git a/src/ouranos/web_server/validate/gaia/hardware.py b/src/ouranos/web_server/validate/gaia/hardware.py index b2c74b04..d72c546d 100644 --- a/src/ouranos/web_server/validate/gaia/hardware.py +++ b/src/ouranos/web_server/validate/gaia/hardware.py @@ -7,7 +7,7 @@ from pydantic import ConfigDict, field_serializer, field_validator import gaia_validators as gv -from gaia_validators import safe_enum_from_name +from gaia_validators import MissingValue, missing, safe_enum_from_name from ouranos.core.validate.base import BaseModel @@ -30,14 +30,14 @@ class HardwareType(BaseModel): class HardwareUpdatePayload(gv.AnonymousHardwareConfig): - name: str | None = None - level: gv.HardwareLevel | None = None - address: str | None = None - type: gv.HardwareType | None = None - model: str | None = None - status: bool | None = None - measures: list[str] | None = None - plant_uid: list[str] | None = None + name: str | MissingValue = missing + level: gv.HardwareLevel | MissingValue = missing + address: str | MissingValue = missing + type: gv.HardwareType | MissingValue = missing + model: str | MissingValue = missing + status: bool | MissingValue = missing + measures: list[str] | MissingValue = missing + plant_uid: list[str] | MissingValue = missing @field_validator("type", mode="before") def parse_level(cls, value): diff --git a/src/ouranos/web_server/validate/user.py b/src/ouranos/web_server/validate/user.py index 4e73ac5b..4e95239f 100644 --- a/src/ouranos/web_server/validate/user.py +++ b/src/ouranos/web_server/validate/user.py @@ -2,6 +2,8 @@ from datetime import datetime +from gaia_validators import MissingValue, missing + from ouranos.core.database.models.app import RoleName from ouranos.core.validate.base import BaseModel @@ -18,6 +20,6 @@ class UserDescription(BaseModel): class UserUpdatePayload(BaseModel): - email: str | None = None - firstname: str | None = None - lastname: str | None = None + email: str | MissingValue = missing + firstname: str | None | MissingValue = missing + lastname: str | None | MissingValue = missing diff --git a/src/ouranos/web_server/validate/wiki.py b/src/ouranos/web_server/validate/wiki.py index c9786143..aec13480 100644 --- a/src/ouranos/web_server/validate/wiki.py +++ b/src/ouranos/web_server/validate/wiki.py @@ -6,6 +6,8 @@ from anyio import Path as ioPath from pydantic import Field, field_validator +from gaia_validators import MissingValue, missing + from ouranos.core.database.models.app import ModificationType from ouranos.core.validate.base import BaseModel @@ -51,8 +53,8 @@ class WikiArticleCreationPayload(BaseModel): class WikiArticleUpdatePayload(BaseModel): # topic is provided by the route - # name is provided by the route - content: str + name : str | MissingValue = missing + content: str | MissingValue = missing # author_id is provided by the route diff --git a/tests/web_server/routes/hardware.py b/tests/web_server/routes/hardware.py index 3b4c1444..43fc7abe 100644 --- a/tests/web_server/routes/hardware.py +++ b/tests/web_server/routes/hardware.py @@ -1,4 +1,5 @@ from fastapi.testclient import TestClient +import pytest import gaia_validators as gv @@ -121,7 +122,8 @@ def test_hardware_update_request_success( assert dispatched["data"]["action"] == gv.CrudAction.update assert dispatched["data"]["target"] == "hardware" assert dispatched["data"]["data"]["name"] == payload["name"] - assert dispatched["data"]["data"]["level"] is None + with pytest.raises(KeyError): + dispatched["data"]["data"]["level"] # Not in the payload, should be missing def test_hardware_delete_request_failure_user(client_user: TestClient):