diff --git a/migrations/versions/2026-07-26-00-00-00_a3f1c8d92b47_location_geography_index.py b/migrations/versions/2026-07-26-00-00-00_a3f1c8d92b47_location_geography_index.py new file mode 100644 index 00000000..5c52a3b5 --- /dev/null +++ b/migrations/versions/2026-07-26-00-00-00_a3f1c8d92b47_location_geography_index.py @@ -0,0 +1,35 @@ +"""location geography index + +Adds a gist index on (geometry::geography) so the radius search in fetch_locations() can use ST_DWithin instead of +ST_DistanceSphere(...) < radius. The latter is a plain function comparison that the planner cannot index, so it +seq-scanned the whole location table on every ?lat&lon&radius request. + +PostGIS only, the index is skipped on other dialects. + +Revision ID: a3f1c8d92b47 +Revises: 76e048ecbc17 +Create Date: 2026-07-26 00:00:00.000000 + +""" + +from alembic import op + +# revision identifiers, used by Alembic. +revision = 'a3f1c8d92b47' +down_revision = '76e048ecbc17' +branch_labels = None +depends_on = None + + +def upgrade(): + if op.get_bind().dialect.name != 'postgresql': + return + + op.execute('CREATE INDEX geography_index ON location USING gist ((geometry::geography))') + + +def downgrade(): + if op.get_bind().dialect.name != 'postgresql': + return + + op.execute('DROP INDEX geography_index') diff --git a/tests/unit/models/capability_bits_test.py b/tests/unit/models/capability_bits_test.py new file mode 100644 index 00000000..fed89a35 --- /dev/null +++ b/tests/unit/models/capability_bits_test.py @@ -0,0 +1,80 @@ +""" +Open ChargePoint DataBase OCPDB +Copyright (C) 2026 binary butterfly GmbH + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . +""" + +from webapp.models.charging_station import CAPABILITY_BIT_BY_MEMBER, CAPABILITY_BITS, Capability + +# The bit value each capability is stored as in charging_station.capabilities. Written out literally so that any change +# to the persisted encoding has to be made here too, where it is obvious that existing database rows are affected. +EXPECTED_BITS: dict[str, int] = { + 'CHARGING_PROFILE_CAPABLE': 1, + 'CHARGING_PREFERENCES_CAPABLE': 2, + 'CHIP_CARD_SUPPORT': 4, + 'CONTACTLESS_CARD_SUPPORT': 8, + 'CREDIT_CARD_PAYABLE': 16, + 'DEBIT_CARD_PAYABLE': 32, + 'PED_TERMINAL': 64, + 'REMOTE_START_STOP_CAPABLE': 128, + 'RESERVABLE': 256, + 'RFID_READER': 512, + 'TOKEN_GROUP_CAPABLE': 1024, + 'UNLOCK_CAPABLE': 2048, + 'PUBLIC': 4096, + 'LOCAL_KEY': 8192, + 'CASH': 16384, + 'IEC15118': 32768, + 'DIRECT_REMOTE': 65536, +} + + +class CapabilityBitsTest: + """ + Guards the bitmask encoding of ChargingStation.capabilities. The bit values are persisted, so changing one + reinterprets every existing row without any migration or error. + """ + + @staticmethod + def test_bit_values_are_unchanged(): + """Every capability must keep the bit value it was stored with.""" + assert {item.name: bit for bit, item in CAPABILITY_BITS} == EXPECTED_BITS + + @staticmethod + def test_every_capability_has_a_bit(): + """A capability missing from the table would silently never be stored or returned.""" + assert {item for _bit, item in CAPABILITY_BITS} == set(Capability) + + @staticmethod + def test_bits_are_unique_and_single(): + """Each entry must be a distinct power of two, otherwise capabilities alias each other.""" + bits = [bit for bit, _item in CAPABILITY_BITS] + + assert len(set(bits)) == len(bits) + for bit in bits: + assert bit > 0 + assert bit & (bit - 1) == 0 + + @staticmethod + def test_bit_by_member_matches_the_table(): + """The lookup used by the setter must agree with the table used by the getter.""" + assert CAPABILITY_BIT_BY_MEMBER == {item: bit for bit, item in CAPABILITY_BITS} + + @staticmethod + def test_table_is_ordered_by_bit_value(): + """The getter returns capabilities in table order, which callers expect to be ascending by bit.""" + bits = [bit for bit, _item in CAPABILITY_BITS] + + assert bits == sorted(bits) diff --git a/tests/unit/models/parking_restriction_bits_test.py b/tests/unit/models/parking_restriction_bits_test.py new file mode 100644 index 00000000..5127ebc2 --- /dev/null +++ b/tests/unit/models/parking_restriction_bits_test.py @@ -0,0 +1,82 @@ +""" +Open ChargePoint DataBase OCPDB +Copyright (C) 2026 binary butterfly GmbH + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . +""" + +from webapp.models.evse import ( + PARKING_RESTRICTION_BIT_BY_MEMBER, + PARKING_RESTRICTION_BITS, + ParkingRestriction, +) + +# The bit value each parking restriction is stored as in evse.parking_restrictions. Written out literally so that any +# change to the persisted encoding has to be made here too, where it is obvious that existing rows are affected. +EXPECTED_BITS: dict[str, int] = { + 'EV_ONLY': 1, + 'PLUGGED': 2, + 'DISABLED': 4, + 'CUSTOMERS': 8, + 'MOTORCYCLES': 16, + 'CARSHARING': 32, + 'BICYCLE_ONLY': 64, +} + + +class ParkingRestrictionBitsTest: + """ + Guards the bitmask encoding of Evse.parking_restrictions. The bit values are persisted, so changing one + reinterprets every existing row without any migration or error. + """ + + @staticmethod + def test_bit_values_are_unchanged(): + """Every parking restriction must keep the bit value it was stored with.""" + assert {item.name: bit for bit, item in PARKING_RESTRICTION_BITS} == EXPECTED_BITS + + @staticmethod + def test_every_parking_restriction_has_a_bit(): + """A restriction missing from the table would silently never be stored or returned.""" + assert {item for _bit, item in PARKING_RESTRICTION_BITS} == set(ParkingRestriction) + + @staticmethod + def test_bits_are_unique_and_single(): + """Each entry must be a distinct power of two, otherwise restrictions alias each other.""" + bits = [bit for bit, _item in PARKING_RESTRICTION_BITS] + + assert len(set(bits)) == len(bits) + for bit in bits: + assert bit > 0 + assert bit & (bit - 1) == 0 + + @staticmethod + def test_bit_by_member_matches_the_table(): + """The lookup used by the setter must agree with the table used by the getter.""" + assert PARKING_RESTRICTION_BIT_BY_MEMBER == {item: bit for bit, item in PARKING_RESTRICTION_BITS} + + @staticmethod + def test_table_is_ordered_by_bit_value(): + """The getter returns restrictions in table order, which callers expect to be ascending by bit.""" + bits = [bit for bit, _item in PARKING_RESTRICTION_BITS] + + assert bits == sorted(bits) + + @staticmethod + def test_bicycle_only_bit_used_by_the_tile_summary_query(): + """ + fetch_locations_summary_by_bounds() counts bike chargepoints with a raw SQL bitmask test built from this + constant. It used to be a hardcoded 64. + """ + assert PARKING_RESTRICTION_BIT_BY_MEMBER[ParkingRestriction.BICYCLE_ONLY] == 64 diff --git a/webapp/models/charging_station.py b/webapp/models/charging_station.py index 440269f5..927b6df0 100644 --- a/webapp/models/charging_station.py +++ b/webapp/models/charging_station.py @@ -69,6 +69,33 @@ class Capability(Enum): DIRECT_REMOTE = 'DIRECT_REMOTE' +# Bit value per capability, as stored in charging_station.capabilities. +# +# These values are persisted in the database, so they MUST NOT change: reordering, inserting or renumbering an entry +# silently reinterprets every stored bitmask. Add new capabilities at the end with the next free bit, and never reuse +# the bit of a removed one. capability_bits_test.py guards this. +CAPABILITY_BITS: list[tuple[int, Capability]] = [ + (1 << 0, Capability.CHARGING_PROFILE_CAPABLE), + (1 << 1, Capability.CHARGING_PREFERENCES_CAPABLE), + (1 << 2, Capability.CHIP_CARD_SUPPORT), + (1 << 3, Capability.CONTACTLESS_CARD_SUPPORT), + (1 << 4, Capability.CREDIT_CARD_PAYABLE), + (1 << 5, Capability.DEBIT_CARD_PAYABLE), + (1 << 6, Capability.PED_TERMINAL), + (1 << 7, Capability.REMOTE_START_STOP_CAPABLE), + (1 << 8, Capability.RESERVABLE), + (1 << 9, Capability.RFID_READER), + (1 << 10, Capability.TOKEN_GROUP_CAPABLE), + (1 << 11, Capability.UNLOCK_CAPABLE), + (1 << 12, Capability.PUBLIC), + (1 << 13, Capability.LOCAL_KEY), + (1 << 14, Capability.CASH), + (1 << 15, Capability.IEC15118), + (1 << 16, Capability.DIRECT_REMOTE), +] +CAPABILITY_BIT_BY_MEMBER: dict[Capability, int] = {item: bit for bit, item in CAPABILITY_BITS} + + class ChargingStation(BaseModel): __tablename__ = 'charging_station' parking_spaces_list_validator: list[ParkingSpace] = ListValidator(DataclassValidator(ParkingSpace)) @@ -130,10 +157,7 @@ def user_interface_languages(self, languages: list[str] | None) -> None: def capabilities(self) -> list[Capability]: if self._capabilities is None: return [] - return sorted( - [item for item in list(Capability) if (1 << list(Capability).index(item)) & self._capabilities], - key=lambda item: 1 << list(Capability).index(item), - ) + return [item for bit, item in CAPABILITY_BITS if bit & self._capabilities] @capabilities.setter def capabilities(self, capabilities: list[Capability] | None) -> None: @@ -141,7 +165,7 @@ def capabilities(self, capabilities: list[Capability] | None) -> None: if capabilities is None: return for capability in capabilities: - self._capabilities = self._capabilities | (1 << list(Capability).index(capability)) + self._capabilities = self._capabilities | CAPABILITY_BIT_BY_MEMBER[capability] @hybrid_property def directions(self) -> list[dict[str, str]] | None: diff --git a/webapp/models/evse.py b/webapp/models/evse.py index fd1149b8..9a11ddff 100644 --- a/webapp/models/evse.py +++ b/webapp/models/evse.py @@ -69,6 +69,23 @@ class ParkingRestriction(Enum): BICYCLE_ONLY = 'BICYCLE_ONLY' +# Bit value per parking restriction, as stored in evse.parking_restrictions. +# +# These values are persisted in the database, so they MUST NOT change: reordering, inserting or renumbering an entry +# silently reinterprets every stored bitmask. Add new restrictions at the end with the next free bit, and never reuse +# the bit of a removed one. parking_restriction_bits_test.py guards this. +PARKING_RESTRICTION_BITS: list[tuple[int, ParkingRestriction]] = [ + (1 << 0, ParkingRestriction.EV_ONLY), + (1 << 1, ParkingRestriction.PLUGGED), + (1 << 2, ParkingRestriction.DISABLED), + (1 << 3, ParkingRestriction.CUSTOMERS), + (1 << 4, ParkingRestriction.MOTORCYCLES), + (1 << 5, ParkingRestriction.CARSHARING), + (1 << 6, ParkingRestriction.BICYCLE_ONLY), +] +PARKING_RESTRICTION_BIT_BY_MEMBER: dict[ParkingRestriction, int] = {item: bit for bit, item in PARKING_RESTRICTION_BITS} + + class Evse(BaseModel): __tablename__ = 'evse' @@ -126,14 +143,7 @@ class Evse(BaseModel): def parking_restrictions(self) -> list[ParkingRestriction]: if self._parking_restrictions is None: return [] - return sorted( - [ - item - for item in list(ParkingRestriction) - if (1 << list(ParkingRestriction).index(item)) & self._parking_restrictions - ], - key=lambda item: 1 << list(ParkingRestriction).index(item), - ) + return [item for bit, item in PARKING_RESTRICTION_BITS if bit & self._parking_restrictions] @parking_restrictions.setter def parking_restrictions(self, parking_restrictions: list[ParkingRestriction] | None) -> None: @@ -141,8 +151,8 @@ def parking_restrictions(self, parking_restrictions: list[ParkingRestriction] | if parking_restrictions is None: return for parking_restriction in parking_restrictions: - self._parking_restrictions = self._parking_restrictions | ( - 1 << list(ParkingRestriction).index(parking_restriction) + self._parking_restrictions = ( + self._parking_restrictions | PARKING_RESTRICTION_BIT_BY_MEMBER[parking_restriction] ) @hybrid_property diff --git a/webapp/models/location.py b/webapp/models/location.py index a1b81dd9..379e8810 100644 --- a/webapp/models/location.py +++ b/webapp/models/location.py @@ -33,6 +33,7 @@ Text, event, func, + text, ) from sqlalchemy import ( Enum as SqlalchemyEnum, @@ -125,6 +126,9 @@ class Location(BaseModel): __table_args__ = ( Index('uid_source', 'uid', 'source'), Index('geometry_index', 'geometry', postgresql_using='gist'), + # Used by the ST_DWithin radius search in LocationRepository. Declared here so that autogenerated migrations + # do not drop it again; it is created by migration a3f1c8d92b47. + Index('geography_index', text('(geometry::geography)'), postgresql_using='gist'), Index('ix_country_official_region_code', 'country', 'official_region_code'), ) diff --git a/webapp/public_api/location_api/location_handler.py b/webapp/public_api/location_api/location_handler.py index 1cc7bebb..f2a4e8a9 100644 --- a/webapp/public_api/location_api/location_handler.py +++ b/webapp/public_api/location_api/location_handler.py @@ -34,15 +34,19 @@ def __init__(self, *args, location_repository: LocationRepository, **kwargs): def get_locations(self, search_query: LocationApiSearchQuery, strict: bool = False) -> PaginatedResult[dict]: locations = self.location_repository.fetch_locations( search_query=search_query, - include_operators=False, + # Eager-load everything _map_location_to_ocpi() renders, otherwise each operator, connector and EVSE image + # is lazy-loaded per row, causing hundreds of N+1 queries per page. + include_operators=True, include_logos=False, include_location_images=False, include_charging_stations=False, include_charging_station_images=False, include_evses=False, - include_evse_images=False, - include_connectors=False, - include_tariffs=True, + include_evse_images=True, + include_connectors=True, + # No tariffs: _map_location_to_ocpi() never renders them, and eager-loading them cost a third of the + # fetch time. + include_tariffs=False, ) return locations.map(lambda location: self._map_location_to_ocpi(location, strict=strict)) diff --git a/webapp/repositories/business_repository.py b/webapp/repositories/business_repository.py index 696dbf0e..9d1e4c40 100644 --- a/webapp/repositories/business_repository.py +++ b/webapp/repositories/business_repository.py @@ -16,6 +16,7 @@ along with this program. If not, see . """ +from sqlalchemy.orm import joinedload from validataclass_search_queries.pagination import PaginatedResult from validataclass_search_queries.search_queries import BaseSearchQuery @@ -28,10 +29,13 @@ class BusinessRepository(BaseRepository[Business]): model_cls = Business def fetch_by_id(self, business_id: int) -> Business: - return self.fetch_resource_by_id(business_id) + # _map_business_to_ocpi() reads business.logo, so eager-load it to avoid a lazy round-trip. + return self.fetch_resource_by_id(business_id, load_options=[joinedload(Business.logo)]) def fetch_businesses(self, search_query: BaseSearchQuery | None = None) -> PaginatedResult[Business]: - query = self.session.query(Business) + # _map_business_to_ocpi() reads business.logo per row; without this eager load every business in the page + # triggers an N+1 logo lookup. + query = self.session.query(Business).options(joinedload(Business.logo)) return self._search_and_paginate(query, search_query) def fetch_business_by_name(self, name: str) -> Business: diff --git a/webapp/repositories/evse_repository.py b/webapp/repositories/evse_repository.py index 1f7bfbe6..924e5c2f 100644 --- a/webapp/repositories/evse_repository.py +++ b/webapp/repositories/evse_repository.py @@ -18,7 +18,7 @@ from dataclasses import dataclass -from sqlalchemy.orm import selectinload +from sqlalchemy.orm import joinedload, selectinload from validataclass_search_queries.pagination import PaginatedResult from validataclass_search_queries.search_queries import BaseSearchQuery @@ -55,6 +55,9 @@ def fetch_evse_by_id(self, evse_id: int, *, include_children: bool = False) -> E if include_children: query = query.options( + # _map_evse_to_ocpi() reads evse.charging_station (capabilities, floor_level, coordinates), so eager-load + # it to avoid a lazy round-trip. + joinedload(Evse.charging_station), selectinload(Evse.connectors), selectinload(Evse.images), ) @@ -68,6 +71,9 @@ def fetch_evse_by_id(self, evse_id: int, *, include_children: bool = False) -> E def fetch_evses(self, search_query: BaseSearchQuery | None = None) -> PaginatedResult[Evse]: options = [ + # _map_evse_to_ocpi() reads evse.charging_station (capabilities, floor_level, coordinates) per row; + # without this eager load every EVSE in the page triggers an N+1 charging_station lookup. + joinedload(Evse.charging_station), selectinload(Evse.connectors), selectinload(Evse.images), ] diff --git a/webapp/repositories/location_repository.py b/webapp/repositories/location_repository.py index 37468ab1..631458c3 100644 --- a/webapp/repositories/location_repository.py +++ b/webapp/repositories/location_repository.py @@ -27,6 +27,7 @@ from webapp.common.sqlalchemy import Query from webapp.models import Business, Evse, Location, TariffAssociation from webapp.models.charging_station import ChargingStation +from webapp.models.evse import PARKING_RESTRICTION_BIT_BY_MEMBER, ParkingRestriction from .base_repository import BaseRepository @@ -53,9 +54,17 @@ def fetch_location_ids_by_source(self, source: str) -> list[int]: def fetch_location_by_id(self, location_id: int, *, include_children: bool = False) -> Location: load_options: list[LoaderOption] = [] if include_children: + # The OCPI location mappers render operator/suboperator/owner and walk the whole + # charging_pool -> evses -> connectors/images tree plus charging_station images. Eager-load all of it so a + # single location response does not fan out into N+1 queries across its stations, EVSEs and images. + cs_load = selectinload(Location.charging_pool) load_options += [ joinedload(Location.operator), - selectinload(Location.charging_pool).selectinload(ChargingStation.evses).selectinload(Evse.connectors), + joinedload(Location.suboperator), + joinedload(Location.owner), + cs_load.selectinload(ChargingStation.images), + cs_load.selectinload(ChargingStation.evses).selectinload(Evse.connectors), + cs_load.selectinload(ChargingStation.evses).selectinload(Evse.images), ] return self.fetch_resource_by_id(location_id, load_options=load_options) @@ -99,7 +108,8 @@ def fetch_locations_summary_by_bounds( " SUM(CASE WHEN evse.status = 'AVAILABLE' THEN 1 ELSE 0 END) as chargepoint_available_count, " " SUM(CASE WHEN evse.status = 'UNKNOWN' THEN 1 ELSE 0 END) as chargepoint_unknown_count, " " SUM(CASE WHEN evse.status = 'STATIC' THEN 1 ELSE 0 END) as chargepoint_static_count, " - ' SUM(CASE WHEN evse.parking_restrictions & 64 = 64 THEN 1 ELSE 0 END) as chargepoint_bike_count ' + ' SUM(CASE WHEN evse.parking_restrictions & :bicycle_only_bit = :bicycle_only_bit THEN 1 ELSE 0 END) ' + ' as chargepoint_bike_count ' 'FROM location ' 'LEFT JOIN charging_station ON charging_station.location_id = location.id ' 'LEFT JOIN evse ON evse.charging_station_id = charging_station.id ' @@ -114,7 +124,12 @@ def fetch_locations_summary_by_bounds( query += f'{additional_where} GROUP BY location.id' - return list(self.session.execute(text(query))) + return list( + self.session.execute( + text(query), + {'bicycle_only_bit': PARKING_RESTRICTION_BIT_BY_MEMBER[ParkingRestriction.BICYCLE_ONLY]}, + ) + ) def fetch_locations_by_bounds(self, bbox: LngLatBbox) -> list[Location]: locations = self.session.query(Location) @@ -256,12 +271,22 @@ def _filter_by_search_query(self, query: Query, search_query: BaseSearchQuery | and getattr(search_query, 'radius', None) ): if self.session.connection().dialect.name == 'postgresql': + # ST_DWithin on geography instead of ST_DistanceSphere(...) < radius: the latter is a plain function + # comparison the planner cannot index, so it seq-scanned every location. ST_DWithin adds the bounding + # box operator that hits the gist index on (geometry::geography), see geography_index. + # use_spheroid=False keeps this a sphere calculation, i.e. exactly the ST_DistanceSphere result. query = query.filter( - func.ST_DistanceSphere( - Location.geometry, - func.ST_GeomFromText(f'POINT({float(search_query.lon)} {float(search_query.lat)})'), + func.ST_DWithin( + func.geography(Location.geometry), + func.geography( + func.ST_SetSRID( + func.ST_MakePoint(float(search_query.lon), float(search_query.lat)), + 4326, + ), + ), + search_query.radius, + False, ) - < search_query.radius ) else: query = query.filter(