From 3449e4273a8d6709c2856eec300f4752f2c085ac Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 8 Dec 2025 09:21:05 -0700 Subject: [PATCH 1/7] refactor: use eager loading at endpoints only, not in models Because eager loading was defined in the models, it was being applied every time those models were queried, leading to unnecessary data being loaded in many cases. This change moves the eager loading logic to the service layer, applying it only at the endpoints that require it. This optimizes performance by reducing redundant data fetching and ensures that only the necessary related data is loaded when needed. --- db/location.py | 4 +-- db/thing.py | 16 ++++++------ services/thing_helper.py | 54 +++++++++++++++++++++++++++++++++------- 3 files changed, 54 insertions(+), 20 deletions(-) diff --git a/db/location.py b/db/location.py index fda4611f9..6376988f6 100644 --- a/db/location.py +++ b/db/location.py @@ -118,9 +118,7 @@ class LocationThingAssociation(Base, AutoBaseMixin): ) # --- Relationship Definitions --- - location: Mapped["Location"] = relationship( - back_populates="thing_associations", lazy="joined" - ) + location: Mapped["Location"] = relationship(back_populates="thing_associations") thing: Mapped["Thing"] = relationship(back_populates="location_associations") diff --git a/db/thing.py b/db/thing.py index 92c7bd942..3730a87fa 100644 --- a/db/thing.py +++ b/db/thing.py @@ -209,7 +209,7 @@ class Thing( cascade="all, delete-orphan", passive_deletes=True, order_by="LocationThingAssociation.effective_start.desc()", - lazy="joined", + # lazy="joined", ) contact_associations = relationship( @@ -256,7 +256,7 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - lazy="joined", + # lazy="joined", ) well_casing_materials: Mapped[List["WellCasingMaterial"]] = relationship( @@ -264,7 +264,7 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - lazy="joined", + # lazy="joined", ) links: Mapped[List["ThingIdLink"]] = relationship( @@ -272,7 +272,7 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - lazy="joined", + # lazy="joined", ) # One-To-Many: A Thing (well) can have multiple measuring points over time. @@ -281,7 +281,7 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - lazy="joined", + # lazy="joined", ) monitoring_frequencies: Mapped[List["MonitoringFrequencyHistory"]] = relationship( @@ -289,7 +289,7 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - lazy="joined", + # lazy="joined", ) # One-To-Many: A Thing can be associated with many AquiferSystems via the ThingAquiferAssociation join table. @@ -298,7 +298,7 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - lazy="joined", + # lazy="joined", ) # Many-To-Many: A Thing can penetrate many GeologicFormations. @@ -308,7 +308,7 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - lazy="joined", + # lazy="joined", ) ) diff --git a/services/thing_helper.py b/services/thing_helper.py index fdd0424db..731db8429 100644 --- a/services/thing_helper.py +++ b/services/thing_helper.py @@ -16,13 +16,13 @@ from datetime import datetime from zoneinfo import ZoneInfo -from fastapi import Request +from fastapi import Request, HTTPException from fastapi_pagination.ext.sqlalchemy import paginate from pydantic import BaseModel from shapely import wkb from shapely.geometry import mapping from sqlalchemy import select, func -from sqlalchemy.orm import Session, aliased +from sqlalchemy.orm import Session, aliased, selectinload from starlette.status import HTTP_404_NOT_FOUND, HTTP_409_CONFLICT from db import ( @@ -33,9 +33,11 @@ WellScreen, WellPurpose, WellCasingMaterial, + ThingAquiferAssociation, + GroupThingAssociation, + MeasuringPointHistory, ) -from db.group import GroupThingAssociation -from db.measuring_point_history import MeasuringPointHistory + from services.audit_helper import audit_add from services.crud_helper import model_patcher from services.exceptions_helper import PydanticStyleException @@ -47,6 +49,22 @@ "well_casing_materials": (WellCasingMaterial, "material"), } +WELL_LOADER_OPTIONS = [ + selectinload(Thing.location_associations).selectinload( + LocationThingAssociation.location + ), + selectinload(Thing.well_purposes), + selectinload(Thing.well_casing_materials), + selectinload(Thing.links), + selectinload(Thing.measuring_points), + selectinload(Thing.monitoring_frequencies), + selectinload(Thing.aquifer_associations).selectinload( + ThingAquiferAssociation.aquifer_system + ), +] + +WELL_THING_TYPE = "water well" + def wkb_to_geojson(wkb_element): if wkb_element is None: @@ -74,6 +92,12 @@ def get_db_things( if thing_type: sql = sql.where(Thing.thing_type == thing_type) + if thing_type == WELL_THING_TYPE: + sql = sql.options(*WELL_LOADER_OPTIONS) + else: + # add all eager loads for generic thing query until/unless GET /thing is deprecated + sql = sql.options(*WELL_LOADER_OPTIONS) + if name: sql = sql.where(Thing.name == name) @@ -118,8 +142,7 @@ def get_thing_type_from_request(request: Request) -> str: return thing_type -def verify_thing_type_correspondence(thing: Thing, request: Request): - thing_type = get_thing_type_from_request(request) +def verify_thing_type_correspondence(thing: Thing, thing_type: str): if thing.thing_type != thing_type: raise PydanticStyleException( status_code=HTTP_404_NOT_FOUND, @@ -135,9 +158,21 @@ def verify_thing_type_correspondence(thing: Thing, request: Request): def get_thing_of_a_thing_type_by_id(session: Session, request: Request, thing_id: int): - thing = simple_get_by_id(session, Thing, thing_id) + thing_type = get_thing_type_from_request(request) + sql = select(Thing).where(Thing.id == thing_id) + + if thing_type == WELL_THING_TYPE: + sql = sql.options(*WELL_LOADER_OPTIONS) - verify_thing_type_correspondence(thing, request) + thing = session.execute(sql).scalar_one_or_none() + + if not thing: + raise HTTPException( + status_code=HTTP_404_NOT_FOUND, + detail=f"Thing with ID {thing_id} not found.", + ) + + verify_thing_type_correspondence(thing, thing_type) return thing @@ -261,7 +296,8 @@ def patch_thing( ): thing = simple_get_by_id(session, Thing, thing_id) - verify_thing_type_correspondence(thing, request) + thing_type = get_thing_type_from_request(request) + verify_thing_type_correspondence(thing, thing_type) thing = model_patcher(session, Thing, thing_id, payload, user) return thing From f3c1af3b0d62637c15b85693b476ef0645a3959d Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 8 Dec 2025 09:32:41 -0700 Subject: [PATCH 2/7] refactor: Add ThingResponseForContact schema and update ContactResponse If a full ThingResponse is used in ContactResponse, it can lead to performance issues due to lazy loading of unneeded fields. To preclude this from happening, a new ThingResponseForContact schema has been created since all that is needed in this context are the id and name of the Things related to a Contact. --- schemas/contact.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/schemas/contact.py b/schemas/contact.py index d43cd4aaf..d20af7981 100644 --- a/schemas/contact.py +++ b/schemas/contact.py @@ -22,7 +22,6 @@ from core.enums import Role, ContactType, PhoneType, EmailType, AddressType from schemas import BaseResponseModel, BaseCreateModel, BaseUpdateModel -from schemas.thing import ThingResponse # -------- VALIDATORS ---------- @@ -199,6 +198,16 @@ class AddressResponse(BaseItemResponse): address_type: AddressType +class ThingResponseForContact(BaseModel): + """ + Response schema for thing details related to a contact. All that is needed + are the id and name + """ + + id: int + name: str | None = None + + class ContactResponse(BaseResponseModel): """ Response schema for contact details. @@ -212,7 +221,7 @@ class ContactResponse(BaseResponseModel): emails: List[EmailResponse] = [] phones: List[PhoneResponse] = [] addresses: List[AddressResponse] = [] - things: List[ThingResponse] = [] # List of related things + things: List[ThingResponseForContact] = [] @field_validator("incomplete_nma_phones", mode="before") def make_incomplete_nma_phone_str(cls, v: list) -> list: From 9f9f91e4bfad14eda06699b5b7bbaf1be8986753 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 8 Dec 2025 09:40:22 -0700 Subject: [PATCH 3/7] refactor: remove non-polymorphic eager loading moving forward all eager loading will be defined at endpoints, not in the model definitions (unless they are in polymorphic models) --- db/thing_aquifer_association.py | 5 ++--- db/thing_geologic_formation_association.py | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/db/thing_aquifer_association.py b/db/thing_aquifer_association.py index cca5758a9..d9a3e6ac4 100644 --- a/db/thing_aquifer_association.py +++ b/db/thing_aquifer_association.py @@ -34,12 +34,12 @@ class ThingAquiferAssociation(Base, AutoBaseMixin, ReleaseMixin): # --- Relationship Definitions --- # Many-To-One: This association links to one Thing. thing: Mapped["Thing"] = relationship( - "Thing", back_populates="aquifer_associations", lazy="joined" + "Thing", back_populates="aquifer_associations" ) # Many-To-One: This association links to one AquiferSystem. aquifer_system: Mapped["AquiferSystem"] = relationship( - "AquiferSystem", back_populates="thing_associations", lazy="joined" + "AquiferSystem", back_populates="thing_associations" ) # One-To-Many: An association can have multiple aquifer types. aquifer_types: Mapped[list["AquiferType"]] = relationship( @@ -47,5 +47,4 @@ class ThingAquiferAssociation(Base, AutoBaseMixin, ReleaseMixin): back_populates="thing_aquifer_association", cascade="all, delete-orphan", passive_deletes=True, - lazy="joined", ) diff --git a/db/thing_geologic_formation_association.py b/db/thing_geologic_formation_association.py index 0707df269..71d393404 100644 --- a/db/thing_geologic_formation_association.py +++ b/db/thing_geologic_formation_association.py @@ -51,10 +51,10 @@ class ThingGeologicFormationAssociation(Base, AutoBaseMixin, ReleaseMixin): # --- Relationship Definitions --- # Many-To-One: This association links to one Thing. thing: Mapped["Thing"] = relationship( - "Thing", back_populates="formation_associations", lazy="joined" + "Thing", back_populates="formation_associations" ) # Many-To-One: This association links to one GeologicFormation. geologic_formation: Mapped["GeologicFormation"] = relationship( - "GeologicFormation", back_populates="thing_associations", lazy="joined" + "GeologicFormation", back_populates="thing_associations" ) From c481cbd007ff8b31750a8c2b79dd5892da110388 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 8 Dec 2025 09:48:46 -0700 Subject: [PATCH 4/7] refactor: keep parameter as only eager load for obseration every observation is linked to a parameter, so eager loading of parameters when loading observations will always be needed --- api/thing.py | 2 ++ db/deployment.py | 4 +--- db/observation.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/thing.py b/api/thing.py index b65672880..367237f58 100644 --- a/api/thing.py +++ b/api/thing.py @@ -17,6 +17,7 @@ from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select from sqlalchemy.exc import ProgrammingError +from sqlalchemy.orm import selectinload from starlette.status import ( HTTP_200_OK, HTTP_201_CREATED, @@ -361,6 +362,7 @@ async def get_thing_deployments( """ thing = simple_get_by_id(session, Thing, thing_id) sql = select(Deployment).where(Deployment.thing_id == thing.id) + sql = sql.options(selectinload(Deployment.sensor)) return paginate(query=sql, conn=session) diff --git a/db/deployment.py b/db/deployment.py index 8f4aaf84e..0b2dc61df 100644 --- a/db/deployment.py +++ b/db/deployment.py @@ -50,6 +50,4 @@ class Deployment(Base, AutoBaseMixin, ReleaseMixin): # Many-To-One: A Deployment is for one Thing. thing: Mapped["Thing"] = relationship("Thing", back_populates="deployments") # Many-To-One: A Deployment is of one piece of equipment (sensor). - sensor: Mapped["Sensor"] = relationship( - "Sensor", back_populates="deployments", lazy="joined" - ) + sensor: Mapped["Sensor"] = relationship("Sensor", back_populates="deployments") diff --git a/db/observation.py b/db/observation.py index 90971bc52..27fe70458 100644 --- a/db/observation.py +++ b/db/observation.py @@ -94,7 +94,7 @@ class Observation(Base, AutoBaseMixin, ReleaseMixin): # Many-To-One: An Observation measures one Parameter. parameter: Mapped["Parameter"] = relationship( - "Parameter", back_populates="observations", lazy="joined" + "Parameter", back_populates="observations", lazy="selectin" ) From 72d16f3aa54944dc922b4e4fd47f54061d2bf43a Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 8 Dec 2025 10:08:49 -0700 Subject: [PATCH 5/7] fix: use BaseResponseModel for ThingResponseForContact --- schemas/contact.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/schemas/contact.py b/schemas/contact.py index d20af7981..eeecd6bfd 100644 --- a/schemas/contact.py +++ b/schemas/contact.py @@ -198,14 +198,13 @@ class AddressResponse(BaseItemResponse): address_type: AddressType -class ThingResponseForContact(BaseModel): +class ThingResponseForContact(BaseResponseModel): """ Response schema for thing details related to a contact. All that is needed are the id and name """ - id: int - name: str | None = None + name: str class ContactResponse(BaseResponseModel): From e13355ccbbd79864dd336625e1089893cee612e0 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 8 Dec 2025 11:33:15 -0700 Subject: [PATCH 6/7] fix: remove commented out eager loads --- db/thing.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/db/thing.py b/db/thing.py index 3730a87fa..1f043eae4 100644 --- a/db/thing.py +++ b/db/thing.py @@ -209,7 +209,6 @@ class Thing( cascade="all, delete-orphan", passive_deletes=True, order_by="LocationThingAssociation.effective_start.desc()", - # lazy="joined", ) contact_associations = relationship( @@ -256,7 +255,6 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - # lazy="joined", ) well_casing_materials: Mapped[List["WellCasingMaterial"]] = relationship( @@ -264,7 +262,6 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - # lazy="joined", ) links: Mapped[List["ThingIdLink"]] = relationship( @@ -272,7 +269,6 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - # lazy="joined", ) # One-To-Many: A Thing (well) can have multiple measuring points over time. @@ -281,7 +277,6 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - # lazy="joined", ) monitoring_frequencies: Mapped[List["MonitoringFrequencyHistory"]] = relationship( @@ -289,7 +284,6 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - # lazy="joined", ) # One-To-Many: A Thing can be associated with many AquiferSystems via the ThingAquiferAssociation join table. @@ -298,7 +292,6 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - # lazy="joined", ) # Many-To-Many: A Thing can penetrate many GeologicFormations. @@ -308,7 +301,6 @@ class Thing( back_populates="thing", cascade="all, delete-orphan", passive_deletes=True, - # lazy="joined", ) ) From c9b87ede093dda18b4fb3c0953ac41bd9a5059cd Mon Sep 17 00:00:00 2001 From: jacob-a-brown Date: Mon, 8 Dec 2025 15:12:09 -0700 Subject: [PATCH 7/7] refactor: remove deprecate well_construction_notes field --- db/thing.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/db/thing.py b/db/thing.py index 6ceacc337..9d73a8f98 100644 --- a/db/thing.py +++ b/db/thing.py @@ -340,9 +340,6 @@ class Thing( # Full-text search vector search_vector = Column(TSVectorType("name")) - # for temporary backwards compatibility - well_construction_notes = mapped_column(String(1000), nullable=True) - @property def current_location(self): """