From 27b18a43b46bcb19b933e818bd97341e8affd874 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 23 Oct 2026 10:34:52 +0200 Subject: [PATCH 1/3] typing: wrap SQLModel columns in col() and drop 99 ty: ignore suppressions Closes #591 Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- modules/api_tokens/api_tokens/resolver.py | 5 +++-- modules/api_tokens/api_tokens/service.py | 14 +++++++++----- modules/audit_log/audit_log/service.py | 7 +++++-- .../background_tasks/_signal_support.py | 3 ++- .../background_tasks/endpoints/api_status.py | 3 ++- .../background_tasks/background_tasks/filters.py | 7 ++++--- .../background_tasks/log_purge.py | 3 ++- .../background_tasks/log_service.py | 13 ++++++++----- .../background_tasks/outbox_claim.py | 11 ++++++----- .../background_tasks/outbox_service.py | 11 ++++++----- .../background_tasks/retry_service.py | 5 +++-- .../background_tasks/task_heartbeat.py | 5 +++-- .../background_tasks/background_tasks/tasks.py | 5 +++-- modules/feature_flags/feature_flags/service.py | 7 ++++--- modules/file_storage/file_storage/service.py | 3 ++- modules/file_storage/file_storage/tasks.py | 9 +++++---- .../file_storage/file_storage/upload_guards.py | 9 +++++---- modules/file_storage/file_storage/uploads.py | 7 ++++--- modules/file_storage/file_storage/visibility.py | 3 ++- modules/keycloak/keycloak/provider.py | 3 ++- modules/permissions/permissions/service.py | 5 +++-- modules/settings/settings/_listing.py | 3 ++- modules/settings/settings/service.py | 7 ++++--- modules/tenants/tenants/_deletion.py | 3 ++- modules/tenants/tenants/_membership_ops.py | 15 ++++++++++----- modules/tenants/tenants/_owner_guard.py | 11 ++++++----- modules/tenants/tenants/invitations.py | 11 ++++++----- modules/tenants/tenants/service.py | 3 ++- modules/users/users/admin/queries.py | 5 +++-- modules/users/users/admin/service.py | 9 +++++---- modules/users/users/auth_local/lockout.py | 3 ++- modules/users/users/auth_local/self_account.py | 9 +++++---- modules/users/users/auth_local/token_api.py | 10 +++++----- modules/users/users/bootstrap.py | 13 +++++++------ modules/users/users/demo.py | 9 ++++++--- modules/users/users/provider.py | 3 ++- modules/users/users/registration.py | 3 ++- modules/users/users/token_strategy.py | 9 +++++---- 38 files changed, 157 insertions(+), 107 deletions(-) diff --git a/modules/api_tokens/api_tokens/resolver.py b/modules/api_tokens/api_tokens/resolver.py index c7f0d8649..66ebd1558 100644 --- a/modules/api_tokens/api_tokens/resolver.py +++ b/modules/api_tokens/api_tokens/resolver.py @@ -24,6 +24,7 @@ from simple_module_core.user_directory import UserDirectory, get_user_directory from simple_module_db import ALL_TENANTS_OPTION, independent_session from sqlalchemy import or_, select, update +from sqlmodel import col from starlette.requests import Request from api_tokens.constants import ( @@ -112,7 +113,7 @@ async def touch_last_used(app: Any, token: ApiToken, now: datetime) -> None: stmt = ( update(ApiToken) .where( - ApiToken.id == token.id, # ty: ignore[invalid-argument-type] + col(ApiToken.id) == token.id, or_(ApiToken.last_used_at.is_(None), ApiToken.last_used_at <= cutoff), ) .values(last_used_at=now, updated_at=ApiToken.updated_at) @@ -136,7 +137,7 @@ async def resolve_api_token(request: Request) -> UserContext | None: async with app.state.sm.db.session_factory() as db: stmt = ( select(ApiToken) - .where(ApiToken.token_hash == hash_token(credential)) # ty: ignore[invalid-argument-type] + .where(col(ApiToken.token_hash) == hash_token(credential)) .execution_options(**{ALL_TENANTS_OPTION: True}) ) token = (await db.execute(stmt)).scalar_one_or_none() diff --git a/modules/api_tokens/api_tokens/service.py b/modules/api_tokens/api_tokens/service.py index 4aa0e36e8..8fe2acb04 100644 --- a/modules/api_tokens/api_tokens/service.py +++ b/modules/api_tokens/api_tokens/service.py @@ -17,6 +17,7 @@ from simple_module_db import ALL_TENANTS_OPTION, all_tenants from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from api_tokens.constants import FORBIDDEN_SERVICE_ROLES, TokenKind from api_tokens.contracts.schemas import TokenView @@ -115,7 +116,10 @@ async def list_for_owner(self, owner_user_id: uuid.UUID) -> list[ApiToken]: """ stmt = ( select(ApiToken) - .where(ApiToken.owner_user_id == owner_user_id, ApiToken.kind == TokenKind.PERSONAL) # ty: ignore[invalid-argument-type] + .where( + col(ApiToken.owner_user_id) == owner_user_id, + col(ApiToken.kind) == TokenKind.PERSONAL, + ) .order_by(ApiToken.created_at.desc()) .execution_options(**{ALL_TENANTS_OPTION: True}) ) @@ -130,16 +134,16 @@ async def get_owned(self, token_id: uuid.UUID, owner_user_id: uuid.UUID) -> ApiT stmt = ( select(ApiToken) .where( - ApiToken.id == token_id, # ty: ignore[invalid-argument-type] - ApiToken.owner_user_id == owner_user_id, # ty: ignore[invalid-argument-type] - ApiToken.kind == TokenKind.PERSONAL, # ty: ignore[invalid-argument-type] + col(ApiToken.id) == token_id, + col(ApiToken.owner_user_id) == owner_user_id, + col(ApiToken.kind) == TokenKind.PERSONAL, ) .execution_options(**{ALL_TENANTS_OPTION: True}) ) return (await self.db.execute(stmt)).scalar_one_or_none() async def get_in_tenant(self, token_id: uuid.UUID) -> ApiToken | None: - stmt = select(ApiToken).where(ApiToken.id == token_id) # ty: ignore[invalid-argument-type] + stmt = select(ApiToken).where(col(ApiToken.id) == token_id) return (await self.db.execute(stmt)).scalar_one_or_none() async def revoke(self, token: ApiToken, *, cross_tenant: bool = False) -> ApiToken: diff --git a/modules/audit_log/audit_log/service.py b/modules/audit_log/audit_log/service.py index 0dd7153ef..8cf0cda53 100644 --- a/modules/audit_log/audit_log/service.py +++ b/modules/audit_log/audit_log/service.py @@ -8,6 +8,7 @@ from simple_module_db.provider import DatabaseProvider from sqlalchemy import Select, and_, func, literal_column, or_, select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from audit_log.constants import DEFAULT_PAGE_SIZE, MAX_PAGE, MAX_PAGE_SIZE from audit_log.contracts.schemas import AuditEntryList, AuditEntryRead @@ -158,8 +159,10 @@ async def iter_entries( # is what stops a batch boundary swallowing the rest of them. stmt = stmt.where( or_( - AuditEntry.created_at < last_created, # ty: ignore[invalid-argument-type] - and_(AuditEntry.created_at == last_created, AuditEntry.id > last_id), # ty: ignore[invalid-argument-type] + col(AuditEntry.created_at) < last_created, + and_( + col(AuditEntry.created_at) == last_created, col(AuditEntry.id) > last_id + ), ) ) stmt = stmt.order_by(AuditEntry.created_at.desc(), AuditEntry.id).limit(batch_size) diff --git a/modules/background_tasks/background_tasks/_signal_support.py b/modules/background_tasks/background_tasks/_signal_support.py index 37be8af1c..0e67077af 100644 --- a/modules/background_tasks/background_tasks/_signal_support.py +++ b/modules/background_tasks/background_tasks/_signal_support.py @@ -15,6 +15,7 @@ from sqlalchemy import select from sqlalchemy.orm import Session +from sqlmodel import col from background_tasks.constants import INTERNAL_TASK_SWEEP_OUTBOX, TaskStatus from background_tasks.models import TaskExecution @@ -53,7 +54,7 @@ def upsert_by_celery_id( """ row: TaskExecution | None = None if celery_task_id: - stmt = select(TaskExecution).where(TaskExecution.celery_task_id == celery_task_id) # ty: ignore[invalid-argument-type] + stmt = select(TaskExecution).where(col(TaskExecution.celery_task_id) == celery_task_id) row = session.execute(stmt).scalar_one_or_none() if row is None: row = TaskExecution(celery_task_id=celery_task_id, **defaults) diff --git a/modules/background_tasks/background_tasks/endpoints/api_status.py b/modules/background_tasks/background_tasks/endpoints/api_status.py index c95bc55a0..6d7e0c1b7 100644 --- a/modules/background_tasks/background_tasks/endpoints/api_status.py +++ b/modules/background_tasks/background_tasks/endpoints/api_status.py @@ -18,6 +18,7 @@ from simple_module_hosting.permissions import ensure_resolved_permissions from simple_module_hosting.token_guard import Authenticated from sqlalchemy import select +from sqlmodel import col from background_tasks.constants import MODULE_NAME, PERM_VIEW, STATUS_ROUTER_PREFIX from background_tasks.contracts.schemas import TaskStatusDTO @@ -56,7 +57,7 @@ async def get_task_status( row = ( await db.execute( select(TaskExecution) - .where(TaskExecution.celery_task_id == celery_task_id) # ty: ignore[invalid-argument-type] + .where(col(TaskExecution.celery_task_id) == celery_task_id) .order_by(TaskExecution.queued_at.desc()) .limit(1) ) diff --git a/modules/background_tasks/background_tasks/filters.py b/modules/background_tasks/background_tasks/filters.py index 12b3c7c0e..0dc6e8f8c 100644 --- a/modules/background_tasks/background_tasks/filters.py +++ b/modules/background_tasks/background_tasks/filters.py @@ -10,6 +10,7 @@ from simple_module_db import LIKE_ESCAPE_CHAR, like_contains_pattern from sqlalchemy import ColumnElement, select +from sqlmodel import col from background_tasks.constants import PLATFORM_TENANT_FILTER, RETRYABLE_STATUSES, TaskStatus from background_tasks.models import TaskExecution @@ -34,15 +35,15 @@ def execution_filters( if tenant_id == PLATFORM_TENANT_FILTER: conditions.append(TaskExecution.tenant_id.is_(None)) elif tenant_id: - conditions.append(TaskExecution.tenant_id == tenant_id) # ty: ignore[invalid-argument-type] + conditions.append(col(TaskExecution.tenant_id) == tenant_id) if status is not None: - conditions.append(TaskExecution.status == status) # ty: ignore[invalid-argument-type] + conditions.append(col(TaskExecution.status) == status) if task_name: conditions.append( TaskExecution.task_name.ilike(like_contains_pattern(task_name), escape=LIKE_ESCAPE_CHAR) ) if queue: - conditions.append(TaskExecution.queue == queue) # ty: ignore[invalid-argument-type] + conditions.append(col(TaskExecution.queue) == queue) return conditions diff --git a/modules/background_tasks/background_tasks/log_purge.py b/modules/background_tasks/background_tasks/log_purge.py index 406b597b5..f5fbe3f1a 100644 --- a/modules/background_tasks/background_tasks/log_purge.py +++ b/modules/background_tasks/background_tasks/log_purge.py @@ -6,11 +6,12 @@ from sqlalchemy import delete from sqlalchemy.orm import Session +from sqlmodel import col from background_tasks.models import RetainedLog def purge_log_records(session: Session, cutoff: datetime) -> int: """Delete log lines older than ``cutoff``; returns the count.""" - result = session.execute(delete(RetainedLog).where(RetainedLog.ts < cutoff)) # ty: ignore[invalid-argument-type] + result = session.execute(delete(RetainedLog).where(col(RetainedLog.ts) < cutoff)) return result.rowcount or 0 diff --git a/modules/background_tasks/background_tasks/log_service.py b/modules/background_tasks/background_tasks/log_service.py index 7ea46a9ab..8555f3819 100644 --- a/modules/background_tasks/background_tasks/log_service.py +++ b/modules/background_tasks/background_tasks/log_service.py @@ -9,6 +9,7 @@ from simple_module_db import RequestSession from simple_module_db.pagination import PageParams, paginate from sqlalchemy import false, select +from sqlmodel import col from background_tasks.constants import LOG_LEVEL_NAMES from background_tasks.contracts.logs import LogItem, LogListResponse @@ -60,16 +61,18 @@ async def search( ) -> LogListResponse: where = [*self.scope.conditions()] if task_id is not None: - where.append(RetainedLog.task_id == task_id) + where.append(col(RetainedLog.task_id) == task_id) if (level := level_number(min_level)) is not None: - where.append(RetainedLog.level >= level) + where.append(col(RetainedLog.level) >= level) if q: - where.append(RetainedLog.message.ilike(_like(q), escape="\\")) + where.append(col(RetainedLog.message).ilike(_like(q), escape="\\")) if logger_name: - where.append(RetainedLog.logger.startswith(logger_name)) + where.append(col(RetainedLog.logger).startswith(logger_name)) found = await paginate( self.db, - select(RetainedLog).where(*where).order_by(RetainedLog.ts.desc(), RetainedLog.id), # ty: ignore[invalid-argument-type] + select(RetainedLog) + .where(*where) + .order_by(col(RetainedLog.ts).desc(), col(RetainedLog.id)), PageParams(page=page, per_page=per_page), ) return LogListResponse( diff --git a/modules/background_tasks/background_tasks/outbox_claim.py b/modules/background_tasks/background_tasks/outbox_claim.py index f468d17b3..65c820dc5 100644 --- a/modules/background_tasks/background_tasks/outbox_claim.py +++ b/modules/background_tasks/background_tasks/outbox_claim.py @@ -20,6 +20,7 @@ from sqlalchemy import or_, select, update from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker +from sqlmodel import col from background_tasks.constants import ( OUTBOX_BACKOFF_BASE_SECONDS, @@ -103,12 +104,12 @@ async def reclaim_expired( async with factory() as session: now = now or await db_now(session) expired = ( - OutboxMessage.status == OutboxStatus.RUNNING, - or_(OutboxMessage.locked_until < now, OutboxMessage.locked_until.is_(None)), + col(OutboxMessage.status) == OutboxStatus.RUNNING, + or_(col(OutboxMessage.locked_until) < now, col(OutboxMessage.locked_until).is_(None)), ) dead = await session.execute( update(OutboxMessage) - .where(*expired, OutboxMessage.attempts >= max_attempts) # ty: ignore[invalid-argument-type] + .where(*expired, col(OutboxMessage.attempts) >= max_attempts) .values( status=OutboxStatus.DEAD, locked_until=None, @@ -118,7 +119,7 @@ async def reclaim_expired( ) back = await session.execute( update(OutboxMessage) - .where(*expired) # ty: ignore[invalid-argument-type] + .where(*expired) .values(status=OutboxStatus.PENDING, locked_until=None, next_attempt_at=now) .execution_options(synchronize_session=False) ) @@ -174,7 +175,7 @@ async def claim( update(OutboxMessage) .where( OutboxMessage.id.in_(select(claimable.c.id)), - OutboxMessage.status == OutboxStatus.PENDING, # ty: ignore[invalid-argument-type] + col(OutboxMessage.status) == OutboxStatus.PENDING, ) .values( status=OutboxStatus.RUNNING, diff --git a/modules/background_tasks/background_tasks/outbox_service.py b/modules/background_tasks/background_tasks/outbox_service.py index 28137b0a5..e60d7f1b2 100644 --- a/modules/background_tasks/background_tasks/outbox_service.py +++ b/modules/background_tasks/background_tasks/outbox_service.py @@ -9,6 +9,7 @@ from fastapi import HTTPException, status from simple_module_db.pagination import PageParams, paginate from sqlalchemy import delete, false, func, select, update +from sqlmodel import col from background_tasks.constants import OutboxStatus from background_tasks.contracts.outbox import OutboxItem, OutboxListResponse @@ -66,7 +67,7 @@ async def list( self.db, select(OutboxMessage) .where(*where) - .order_by(OutboxMessage.created_at.desc(), OutboxMessage.id), # ty: ignore[invalid-argument-type] + .order_by(col(OutboxMessage.created_at).desc(), col(OutboxMessage.id)), PageParams(page=page, per_page=per_page), ) return OutboxListResponse( @@ -93,7 +94,7 @@ async def _get(self, message_id: uuid.UUID) -> OutboxMessage: row = ( await self.db.execute( select(OutboxMessage) - .where(OutboxMessage.id == message_id, *self.scope.conditions()) # ty: ignore[invalid-argument-type] + .where(col(OutboxMessage.id) == message_id, *self.scope.conditions()) .execution_options(populate_existing=True) ) ).scalar_one_or_none() @@ -114,8 +115,8 @@ async def retry(self, message_id: uuid.UUID) -> OutboxItem: result = await self.db.execute( update(OutboxMessage) .where( - OutboxMessage.id == message_id, # ty: ignore[invalid-argument-type] - OutboxMessage.status == OutboxStatus.DEAD, # ty: ignore[invalid-argument-type] + col(OutboxMessage.id) == message_id, + col(OutboxMessage.status) == OutboxStatus.DEAD, *self.scope.conditions(), ) .values( @@ -141,7 +142,7 @@ async def discard(self, message_id: uuid.UUID) -> None: result = await self.db.execute( delete(OutboxMessage) .where( - OutboxMessage.id == message_id, # ty: ignore[invalid-argument-type] + col(OutboxMessage.id) == message_id, OutboxMessage.status.in_(DISCARDABLE), *self.scope.conditions(), ) diff --git a/modules/background_tasks/background_tasks/retry_service.py b/modules/background_tasks/background_tasks/retry_service.py index d7d5743f9..d3e5d4fb6 100644 --- a/modules/background_tasks/background_tasks/retry_service.py +++ b/modules/background_tasks/background_tasks/retry_service.py @@ -16,6 +16,7 @@ from simple_module_core.events import EventBus from simple_module_db import RequestSession, all_tenants, tenant_context from sqlalchemy import Select, func, select, update +from sqlmodel import col from background_tasks._signal_support import now_utc from background_tasks.after_commit import after_commit @@ -271,8 +272,8 @@ def _fail_unsent(task_id: str, exc: Exception) -> None: session.execute( update(TaskExecution) .where( - TaskExecution.celery_task_id == task_id, # ty: ignore[invalid-argument-type] - TaskExecution.status == TaskStatus.PENDING, # ty: ignore[invalid-argument-type] + col(TaskExecution.celery_task_id) == task_id, + col(TaskExecution.status) == TaskStatus.PENDING, ) .values( status=TaskStatus.FAILED, diff --git a/modules/background_tasks/background_tasks/task_heartbeat.py b/modules/background_tasks/background_tasks/task_heartbeat.py index f52d49b89..f94655bc3 100644 --- a/modules/background_tasks/background_tasks/task_heartbeat.py +++ b/modules/background_tasks/background_tasks/task_heartbeat.py @@ -24,6 +24,7 @@ from celery import current_task from sqlalchemy import update +from sqlmodel import col from background_tasks._signal_support import now_utc from background_tasks.constants import ( @@ -122,11 +123,11 @@ def heartbeat( session.execute( update(TaskExecution) .where( - TaskExecution.celery_task_id == task_id, # ty: ignore[invalid-argument-type] + col(TaskExecution.celery_task_id) == task_id, # Only a row still ``running``: a heartbeat must not revive # a row the sweep already declared stuck (it may have been # retried since) or rewrite a terminal one. - TaskExecution.status == TaskStatus.RUNNING, # ty: ignore[invalid-argument-type] + col(TaskExecution.status) == TaskStatus.RUNNING, ) .values(heartbeat_at=now_utc(), **pending) ) diff --git a/modules/background_tasks/background_tasks/tasks.py b/modules/background_tasks/background_tasks/tasks.py index 816fb7e9d..cc195b0a0 100644 --- a/modules/background_tasks/background_tasks/tasks.py +++ b/modules/background_tasks/background_tasks/tasks.py @@ -21,6 +21,7 @@ from celery import Task, shared_task from sqlalchemy import ColumnElement, and_, delete, or_, update +from sqlmodel import col from background_tasks.constants import ( DEMO_ECHO_TASK, @@ -119,7 +120,7 @@ def sweep_stuck_tasks(self: Task) -> int: stmt = ( update(TaskExecution) .where( - TaskExecution.status == TaskStatus.RUNNING, # ty: ignore[invalid-argument-type] + col(TaskExecution.status) == TaskStatus.RUNNING, _stale(now, stuck_after, overrides), ) .values(status=TaskStatus.STUCK, finished_at=now) @@ -149,7 +150,7 @@ def purge_old_executions() -> int: count = result.rowcount or 0 delivered = session.execute( delete(OutboxMessage) - .where(OutboxMessage.status == OutboxStatus.DELIVERED) # ty: ignore[invalid-argument-type] + .where(col(OutboxMessage.status) == OutboxStatus.DELIVERED) .where(OutboxMessage.delivered_at < cutoff) ) count += delivered.rowcount or 0 diff --git a/modules/feature_flags/feature_flags/service.py b/modules/feature_flags/feature_flags/service.py index a21e9334f..842576b4a 100644 --- a/modules/feature_flags/feature_flags/service.py +++ b/modules/feature_flags/feature_flags/service.py @@ -12,6 +12,7 @@ from simple_module_core.feature_flags import FeatureFlagDefinition, FeatureFlagRegistry from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from feature_flags.constants import SCOPE_SYSTEM, SCOPE_TENANT, SYSTEM_SCOPE_ID from feature_flags.contracts.schemas import FeatureFlagOverrideOut, FeatureFlagView @@ -151,9 +152,9 @@ def build_view( async def _find(self, scope: str, scope_id: str, name: str) -> FeatureFlagOverride | None: result = await self.db.execute( select(FeatureFlagOverride).where( - FeatureFlagOverride.scope == scope, # ty: ignore[invalid-argument-type] - FeatureFlagOverride.scope_id == scope_id, # ty: ignore[invalid-argument-type] - FeatureFlagOverride.name == name, # ty: ignore[invalid-argument-type] + col(FeatureFlagOverride.scope) == scope, + col(FeatureFlagOverride.scope_id) == scope_id, + col(FeatureFlagOverride.name) == name, ) ) return result.scalar_one_or_none() diff --git a/modules/file_storage/file_storage/service.py b/modules/file_storage/file_storage/service.py index 5ac0809ff..10bdd15b1 100644 --- a/modules/file_storage/file_storage/service.py +++ b/modules/file_storage/file_storage/service.py @@ -18,6 +18,7 @@ from fastapi import UploadFile from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from file_storage import constants, queries from file_storage.backend_guard import require_backend @@ -167,7 +168,7 @@ async def get( ``owner`` (``deps.get_owner_scope``) narrows it to one uploader's rows the same way: someone else's file misses like an unknown id (#544). """ - stmt = queries.owned_by(select(StoredFile).where(StoredFile.id == file_id), owner) # ty: ignore[invalid-argument-type] + stmt = queries.owned_by(select(StoredFile).where(col(StoredFile.id) == file_id), owner) if platform: stmt = stmt.where(StoredFile.tenant_id == PLATFORM_TENANT_ID) async with platform_scope(self.db, platform): diff --git a/modules/file_storage/file_storage/tasks.py b/modules/file_storage/file_storage/tasks.py index e91093593..522f99a5e 100644 --- a/modules/file_storage/file_storage/tasks.py +++ b/modules/file_storage/file_storage/tasks.py @@ -19,6 +19,7 @@ from datetime import UTC, datetime, timedelta from celery import shared_task +from sqlmodel import col from file_storage import constants @@ -71,8 +72,8 @@ def reap_stale_uploads() -> dict[str, int]: # they only linger for support questions, so drop them after a window. purged = db.execute( delete(UploadSession).where( - UploadSession.status != constants.UploadStatus.ACTIVE, # ty: ignore[invalid-argument-type] - UploadSession.expires_at # ty: ignore[invalid-argument-type] + col(UploadSession.status) != constants.UploadStatus.ACTIVE, + col(UploadSession.expires_at) < now - timedelta(days=settings.upload_session_retention_days), ) ).rowcount @@ -80,8 +81,8 @@ def reap_stale_uploads() -> dict[str, int]: db.execute( select(UploadSession) .where( - UploadSession.status == constants.UploadStatus.ACTIVE, # ty: ignore[invalid-argument-type] - UploadSession.expires_at <= now, # ty: ignore[invalid-argument-type] + col(UploadSession.status) == constants.UploadStatus.ACTIVE, + col(UploadSession.expires_at) <= now, ) .limit(_REAP_BATCH) ).scalars() diff --git a/modules/file_storage/file_storage/upload_guards.py b/modules/file_storage/file_storage/upload_guards.py index 217b1df42..42df15438 100644 --- a/modules/file_storage/file_storage/upload_guards.py +++ b/modules/file_storage/file_storage/upload_guards.py @@ -8,6 +8,7 @@ from typing import TYPE_CHECKING, cast from sqlalchemy import delete, func, select, text +from sqlmodel import col from file_storage import constants from file_storage.contracts.service import StorageError @@ -66,8 +67,8 @@ async def _check_quota(self, size: int, *, reserve: bool, platform: bool = False await self._serialize_reservations(platform=platform) stored_query = select(func.coalesce(func.sum(StoredFile.size_bytes), 0)) reserved_query = select(func.coalesce(func.sum(UploadSession.total_size), 0)).where( - UploadSession.status == constants.UploadStatus.ACTIVE, # ty: ignore[invalid-argument-type] - UploadSession.expires_at > datetime.now(UTC), # ty: ignore[invalid-argument-type] + col(UploadSession.status) == constants.UploadStatus.ACTIVE, + col(UploadSession.expires_at) > datetime.now(UTC), ) if platform: stored_query = stored_query.where(StoredFile.tenant_id == PLATFORM_TENANT_ID) @@ -117,8 +118,8 @@ async def purge_finished(self, *, owner: str | None) -> int: result = await self.db.execute( delete(UploadSession).where( UploadSession.created_by == owner, - UploadSession.status != constants.UploadStatus.ACTIVE, # ty: ignore[invalid-argument-type] - UploadSession.expires_at < cutoff, # ty: ignore[invalid-argument-type] + col(UploadSession.status) != constants.UploadStatus.ACTIVE, + col(UploadSession.expires_at) < cutoff, ) ) return result.rowcount or 0 diff --git a/modules/file_storage/file_storage/uploads.py b/modules/file_storage/file_storage/uploads.py index 341250b1d..d367f3c7d 100644 --- a/modules/file_storage/file_storage/uploads.py +++ b/modules/file_storage/file_storage/uploads.py @@ -22,6 +22,7 @@ from simple_module_db.listeners import current_user_id from sqlalchemy import select +from sqlmodel import col from file_storage import constants, queries from file_storage.contracts.schemas import StoredFileOut @@ -109,7 +110,7 @@ async def get(self, upload_id: uuid.UUID) -> UploadSession: """The caller's own session. Others' ids miss exactly like unknown ones.""" owner = current_user_id.get() stmt = select(UploadSession).where( - UploadSession.id == upload_id, # ty: ignore[invalid-argument-type] + col(UploadSession.id) == upload_id, UploadSession.created_by == owner, ) session = (await self.db.execute(stmt)).scalar_one_or_none() if owner else None @@ -160,8 +161,8 @@ async def abort_expired(self, *, owner: str | None) -> int: return 0 stmt = select(UploadSession).where( UploadSession.created_by == owner, - UploadSession.status == constants.UploadStatus.ACTIVE, # ty: ignore[invalid-argument-type] - UploadSession.expires_at <= datetime.now(UTC), # ty: ignore[invalid-argument-type] + col(UploadSession.status) == constants.UploadStatus.ACTIVE, + col(UploadSession.expires_at) <= datetime.now(UTC), ) expired = list((await self.db.execute(stmt)).scalars()) for session in expired: diff --git a/modules/file_storage/file_storage/visibility.py b/modules/file_storage/file_storage/visibility.py index 6975aa9d3..28f35037e 100644 --- a/modules/file_storage/file_storage/visibility.py +++ b/modules/file_storage/file_storage/visibility.py @@ -12,6 +12,7 @@ from simple_module_db.listeners import SESSION_HAS_WRITES_KEY from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from file_storage import thumbnails from file_storage.backend_guard import require_backend @@ -47,7 +48,7 @@ async def get_public(self, file_id: uuid.UUID) -> StoredFile: """ stmt = ( select(StoredFile) - .where(StoredFile.id == file_id, StoredFile.public.is_(True)) # ty: ignore[invalid-argument-type] + .where(col(StoredFile.id) == file_id, col(StoredFile.public).is_(True)) .execution_options(all_tenants=True) ) row = (await self.db.execute(stmt)).scalar_one_or_none() diff --git a/modules/keycloak/keycloak/provider.py b/modules/keycloak/keycloak/provider.py index 4a473fd0f..c5b377ada 100644 --- a/modules/keycloak/keycloak/provider.py +++ b/modules/keycloak/keycloak/provider.py @@ -10,6 +10,7 @@ from auth.contracts.tokens import AUTH_KIND_BEARER, is_api_token, parse_bearer from simple_module_core.tenancy import is_tenant_role from simple_module_db import is_valid_tenant_id +from sqlmodel import col from starlette.requests import Request if TYPE_CHECKING: @@ -153,7 +154,7 @@ async def _upsert_user_cache(self, request: Request, claims: dict) -> str: session_factory = request.app.state.sm.db.session_factory sub = claims["sub"] async with session_factory() as db: - stmt = select(KeycloakUserCache).where(KeycloakUserCache.keycloak_sub == sub) # ty: ignore[invalid-argument-type] + stmt = select(KeycloakUserCache).where(col(KeycloakUserCache.keycloak_sub) == sub) row = (await db.execute(stmt)).scalar_one_or_none() if row is None: import uuid as uuid_mod diff --git a/modules/permissions/permissions/service.py b/modules/permissions/permissions/service.py index 597a671e7..345f64af7 100644 --- a/modules/permissions/permissions/service.py +++ b/modules/permissions/permissions/service.py @@ -9,6 +9,7 @@ from simple_module_core.user_directory import DirectoryUser, NullUserDirectory, UserDirectory from sqlalchemy import delete, func, select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from permissions._registry_sync import RegistrySyncMixin, _role_names from permissions.contracts.schemas import ( @@ -115,7 +116,7 @@ async def set_role_permissions( if to_remove: await self.db.execute( delete(RolePermission).where( - RolePermission.role_name == role.name, # ty: ignore[invalid-argument-type] + col(RolePermission.role_name) == role.name, RolePermission.permission_key.in_(to_remove), ) ) @@ -193,7 +194,7 @@ async def set_user_permissions( if to_remove: await self.db.execute( delete(UserPermission).where( - UserPermission.user_id == user_id, # ty: ignore[invalid-argument-type] + col(UserPermission.user_id) == user_id, UserPermission.permission_key.in_(to_remove), ) ) diff --git a/modules/settings/settings/_listing.py b/modules/settings/settings/_listing.py index 908a224f9..f1b368a58 100644 --- a/modules/settings/settings/_listing.py +++ b/modules/settings/settings/_listing.py @@ -6,6 +6,7 @@ from simple_module_db.stored_settings import SYSTEM_SCOPE_ID, Setting from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from settings._row_masking import out from settings.constants import ALL_SCOPES, DEFAULT_PER_PAGE, SCOPE_ALL @@ -111,6 +112,6 @@ async def list_by_scope_unmasked( def _scope_stmt(scope: SettingScope, scope_id: str): return ( select(Setting) - .where(Setting.scope == scope.value, Setting.scope_id == scope_id) # ty: ignore[invalid-argument-type] + .where(col(Setting.scope) == scope.value, col(Setting.scope_id) == scope_id) .order_by(Setting.key) ) diff --git a/modules/settings/settings/service.py b/modules/settings/settings/service.py index 8eb80df95..1d201f6ec 100644 --- a/modules/settings/settings/service.py +++ b/modules/settings/settings/service.py @@ -7,6 +7,7 @@ from simple_module_db.stored_settings import SYSTEM_SCOPE_ID, VALUE_TYPE_STRING, Setting from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from settings._announce import announce from settings._listing import SettingListing @@ -204,9 +205,9 @@ async def _remove(self, entity: Setting, as_owner: bool) -> None: async def _find(self, scope: SettingScope, scope_id: str, key: str) -> Setting | None: stmt = select(Setting).where( - Setting.scope == scope.value, # ty: ignore[invalid-argument-type] - Setting.scope_id == scope_id, # ty: ignore[invalid-argument-type] - Setting.key == key, # ty: ignore[invalid-argument-type] + col(Setting.scope) == scope.value, + col(Setting.scope_id) == scope_id, + col(Setting.key) == key, ) result = await self.db.execute(stmt) return result.scalar_one_or_none() diff --git a/modules/tenants/tenants/_deletion.py b/modules/tenants/tenants/_deletion.py index bb54e7741..2474aae57 100644 --- a/modules/tenants/tenants/_deletion.py +++ b/modules/tenants/tenants/_deletion.py @@ -5,6 +5,7 @@ from simple_module_core.events import Event from sqlalchemy import delete, select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from tenants.contracts.events import MembershipRemoved, TenantDeleted from tenants.models import Invitation, Membership, Tenant @@ -28,7 +29,7 @@ async def delete_tenant_rows(db: AsyncSession, tenant: Tenant) -> list[Event]: ) ).all() for model in (Membership, Invitation): - await db.execute(delete(model).where(model.tenant_id == tenant.id)) # ty: ignore[invalid-argument-type] + await db.execute(delete(model).where(col(model.tenant_id) == tenant.id)) events: list[Event] = [MembershipRemoved(tenant.id, user_id) for user_id in user_ids] events.append(TenantDeleted(tenant.id, tenant.slug, tenant.name)) await db.delete(tenant) diff --git a/modules/tenants/tenants/_membership_ops.py b/modules/tenants/tenants/_membership_ops.py index 6f2cf0836..a5198f5e1 100644 --- a/modules/tenants/tenants/_membership_ops.py +++ b/modules/tenants/tenants/_membership_ops.py @@ -13,6 +13,7 @@ from sqlalchemy import func, select from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from tenants._owner_guard import demote_owner, remove_owner from tenants.constants import ENTITLEMENT_SEATS, MembershipRole @@ -53,15 +54,15 @@ async def has_member_email(self, tenant_id: str, email: str) -> bool: async def get_membership(self, tenant_id: str, user_id: str) -> Membership | None: stmt = select(Membership).where( - Membership.tenant_id == tenant_id, # ty: ignore[invalid-argument-type] - Membership.user_id == user_id, # ty: ignore[invalid-argument-type] + col(Membership.tenant_id) == tenant_id, + col(Membership.user_id) == user_id, ) return (await self.db.execute(stmt)).scalar_one_or_none() async def list_members(self, tenant_id: str) -> list[Membership]: stmt = ( select(Membership) - .where(Membership.tenant_id == tenant_id) # ty: ignore[invalid-argument-type] + .where(col(Membership.tenant_id) == tenant_id) .order_by(Membership.created_at) ) return list((await self.db.execute(stmt)).scalars().all()) @@ -71,7 +72,9 @@ async def seats_used(self, tenant_id: str) -> int: from tenants.invitations import pending_invitation_count members = await self.db.scalar( - select(func.count()).select_from(Membership).where(Membership.tenant_id == tenant_id) # ty: ignore[invalid-argument-type] + select(func.count()) + .select_from(Membership) + .where(col(Membership.tenant_id) == tenant_id) ) return int(members or 0) + await pending_invitation_count(self.db, tenant_id) @@ -109,7 +112,9 @@ async def _owner_count(self, tenant_id: str) -> int: stmt = ( select(func.count()) .select_from(Membership) - .where(Membership.tenant_id == tenant_id, Membership.role == MembershipRole.OWNER) # ty: ignore[invalid-argument-type] + .where( + col(Membership.tenant_id) == tenant_id, col(Membership.role) == MembershipRole.OWNER + ) ) return int(await self.db.scalar(stmt) or 0) diff --git a/modules/tenants/tenants/_owner_guard.py b/modules/tenants/tenants/_owner_guard.py index 2923107a9..a60b9f8e0 100644 --- a/modules/tenants/tenants/_owner_guard.py +++ b/modules/tenants/tenants/_owner_guard.py @@ -19,6 +19,7 @@ from sqlalchemy import delete, exists, update from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import aliased +from sqlmodel import col from tenants.constants import MembershipRole from tenants.models import Membership @@ -27,9 +28,9 @@ def _another_owner(membership: Membership): other = aliased(Membership) return exists().where( - other.tenant_id == membership.tenant_id, # ty: ignore[invalid-argument-type] - other.role == MembershipRole.OWNER, # ty: ignore[invalid-argument-type] - other.id != membership.id, # ty: ignore[invalid-argument-type] + col(other.tenant_id) == membership.tenant_id, + col(other.role) == MembershipRole.OWNER, + col(other.id) != membership.id, ) @@ -37,7 +38,7 @@ async def demote_owner(db: AsyncSession, membership: Membership, role: str) -> b """Set ``role`` on an owner's membership unless they are the last owner.""" stmt = ( update(Membership) - .where(Membership.id == membership.id, _another_owner(membership)) # ty: ignore[invalid-argument-type] + .where(col(Membership.id) == membership.id, _another_owner(membership)) .values(role=role) .execution_options(synchronize_session=False) ) @@ -52,7 +53,7 @@ async def remove_owner(db: AsyncSession, membership: Membership) -> bool: """Delete an owner's membership unless they are the last owner.""" stmt = ( delete(Membership) - .where(Membership.id == membership.id, _another_owner(membership)) # ty: ignore[invalid-argument-type] + .where(col(Membership.id) == membership.id, _another_owner(membership)) .execution_options(synchronize_session=False) ) applied = (await db.execute(stmt)).rowcount == 1 diff --git a/modules/tenants/tenants/invitations.py b/modules/tenants/tenants/invitations.py index 16f890470..6da9e2339 100644 --- a/modules/tenants/tenants/invitations.py +++ b/modules/tenants/tenants/invitations.py @@ -14,6 +14,7 @@ from sqlalchemy import delete, func, select from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from tenants.constants import MembershipRole, TenantStatus from tenants.contracts.events import InvitationCreated @@ -69,10 +70,10 @@ async def create( # still occupy the unique index; drop it so the address can be re-invited. await self.db.execute( delete(Invitation).where( - Invitation.tenant_id == tenant_id, # ty: ignore[invalid-argument-type] - Invitation.email == data.email, # ty: ignore[invalid-argument-type] + col(Invitation.tenant_id) == tenant_id, + col(Invitation.email) == data.email, Invitation.accepted_at.is_(None), - Invitation.expires_at <= _now(), # ty: ignore[invalid-argument-type] + col(Invitation.expires_at) <= _now(), ) ) duplicate = await self.db.scalar( @@ -129,8 +130,8 @@ async def lookup( return None stmt = ( select(Invitation, Tenant) - .join(Tenant, Tenant.id == Invitation.tenant_id) # ty: ignore[invalid-argument-type] - .where(Invitation.token_hash == hash_token(token)) # ty: ignore[invalid-argument-type] + .join(Tenant, col(Tenant.id) == Invitation.tenant_id) + .where(col(Invitation.token_hash) == hash_token(token)) ) if for_update: # two concurrent accepts of one link must not both pass stmt = stmt.with_for_update(of=Invitation) diff --git a/modules/tenants/tenants/service.py b/modules/tenants/tenants/service.py index 007e6700d..e18052409 100644 --- a/modules/tenants/tenants/service.py +++ b/modules/tenants/tenants/service.py @@ -23,6 +23,7 @@ from sqlalchemy import func, select from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from tenants._deletion import delete_tenant_rows from tenants._membership_ops import MembershipOpsMixin @@ -192,7 +193,7 @@ async def lock(self, tenant_id: str) -> Tenant: compiles away; there the database's own writer lock serialises the two transactions (see ``tests/test_owner_race.py``). """ - stmt = select(Tenant).where(Tenant.id == tenant_id).with_for_update() # ty: ignore[invalid-argument-type] + stmt = select(Tenant).where(col(Tenant.id) == tenant_id).with_for_update() tenant = (await self.db.execute(stmt)).scalar_one_or_none() if tenant is None: raise TenantError("not_found", status_code=404) diff --git a/modules/users/users/admin/queries.py b/modules/users/users/admin/queries.py index f8e7b30fb..f4c1082d5 100644 --- a/modules/users/users/admin/queries.py +++ b/modules/users/users/admin/queries.py @@ -12,6 +12,7 @@ from sqlalchemy import func, or_, select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload +from sqlmodel import col from users.admin.user_state import STATUS_CONDITIONS, user_state from users.contracts.schemas import RoleListItem, UserListItem @@ -98,7 +99,7 @@ def to_list_item(self, user: User) -> UserListItem: async def list_roles(self) -> list[RoleListItem]: stmt = ( select(Role, func.count(UserRole.user_id)) # ty: ignore[invalid-argument-type] - .outerjoin(UserRole, UserRole.role_id == Role.id) # ty: ignore[invalid-argument-type] + .outerjoin(UserRole, col(UserRole.role_id) == Role.id) .group_by(Role.id) # ty: ignore[invalid-argument-type] .order_by(Role.name) ) @@ -115,7 +116,7 @@ async def list_roles(self) -> list[RoleListItem]: async def _get_user_with_roles(self, user_id: uuid.UUID) -> User | None: result = await self._db.execute( - select(User).where(User.id == user_id).options(selectinload(User.roles)) # ty: ignore[invalid-argument-type] + select(User).where(col(User.id) == user_id).options(selectinload(User.roles)) # ty: ignore[invalid-argument-type] ) return result.scalar_one_or_none() diff --git a/modules/users/users/admin/service.py b/modules/users/users/admin/service.py index d0ee7c8b0..484050c41 100644 --- a/modules/users/users/admin/service.py +++ b/modules/users/users/admin/service.py @@ -8,6 +8,7 @@ from sqlalchemy import delete, func, select, update from sqlalchemy.orm import noload +from sqlmodel import col from users.admin.queries import _UserServiceBase from users.contracts.schemas import UserCreate @@ -122,14 +123,14 @@ async def delete_user(self, user_id: uuid.UUID) -> None: would be rolled back as a read-only request.""" result = await self._db.execute( select(User) - .where(User.id == user_id) # ty: ignore[invalid-argument-type] + .where(col(User.id) == user_id) .options(noload(User.roles), noload(User.oauth_accounts)) # ty: ignore[invalid-argument-type] ) user = result.scalar_one_or_none() if user is None: raise UserNotFoundError(user_id) for model in (UserRole, UserAccessToken, OAuthAccount, RefreshToken): - await self._db.execute(delete(model).where(model.user_id == user_id)) # ty: ignore[invalid-argument-type] + await self._db.execute(delete(model).where(col(model.user_id) == user_id)) await self._db.delete(user) await self._db.flush() @@ -216,7 +217,7 @@ async def disable(self, user_id: uuid.UUID) -> User: # "sign out everywhere" does). await self._db.execute( update(RefreshToken) - .where(RefreshToken.user_id == user.id, RefreshToken.revoked_at.is_(None)) # ty: ignore[invalid-argument-type] + .where(col(RefreshToken.user_id) == user.id, col(RefreshToken.revoked_at).is_(None)) .values(revoked_at=datetime.now(UTC)) ) await self._db.flush() @@ -256,7 +257,7 @@ async def set_roles( user = await self._require_user(user_id) # Delete all existing role assignments for this user - await self._db.execute(delete(UserRole).where(UserRole.user_id == user_id)) # ty: ignore[invalid-argument-type] + await self._db.execute(delete(UserRole).where(col(UserRole.user_id) == user_id)) # Insert new role assignments roles = await self._resolve_roles(role_names) diff --git a/modules/users/users/auth_local/lockout.py b/modules/users/users/auth_local/lockout.py index 60937be85..6af3feacb 100644 --- a/modules/users/users/auth_local/lockout.py +++ b/modules/users/users/auth_local/lockout.py @@ -38,6 +38,7 @@ from fastapi import HTTPException, Request, status from simple_module_core.rate_limit import RateSpec, WindowStore from sqlalchemy import case, func, select, update +from sqlmodel import col from users.auth_local.rate_limit import client_ip from users.models import User @@ -282,7 +283,7 @@ async def release(session: AsyncSession, attempt: LoginAttempt, admission: Admis if admission.user_ids: await session.execute( update(User) - .where(User.id.in_(admission.user_ids), User.failed_login_count > 0) # ty: ignore[invalid-argument-type] + .where(col(User.id).in_(admission.user_ids), col(User.failed_login_count) > 0) .values(failed_login_count=User.failed_login_count - 1, updated_at=User.updated_at) .execution_options(synchronize_session=False) ) diff --git a/modules/users/users/auth_local/self_account.py b/modules/users/users/auth_local/self_account.py index c236fd31b..9dd4fb09d 100644 --- a/modules/users/users/auth_local/self_account.py +++ b/modules/users/users/auth_local/self_account.py @@ -20,6 +20,7 @@ from simple_module_hosting.session import SESSION_REMEMBER_KEY from simple_module_hosting.token_guard import Authenticated from sqlalchemy import delete, update +from sqlmodel import col from users.auth_local.rate_limit import enforce_auth_throughput_limit from users.backend import cookie_login_response @@ -113,10 +114,10 @@ async def change_my_password( # what "this password is no longer the one that works" has to mean. await db.execute( update(RefreshToken) - .where(RefreshToken.user_id == user.id, RefreshToken.revoked_at.is_(None)) # ty: ignore[invalid-argument-type] + .where(col(RefreshToken.user_id) == user.id, col(RefreshToken.revoked_at).is_(None)) .values(revoked_at=datetime.now(UTC)) ) - await db.execute(delete(UserAccessToken).where(UserAccessToken.user_id == user.id)) # ty: ignore[invalid-argument-type] + await db.execute(delete(UserAccessToken).where(col(UserAccessToken.user_id) == user.id)) await db.flush() return await _reissue_browser_cookie(request, user, access_token_db) @@ -173,10 +174,10 @@ async def revoke_all_my_sessions( db.on_commit(lambda: publish_revocation(request.app, user.id)) await db.execute( update(RefreshToken) - .where(RefreshToken.user_id == user.id, RefreshToken.revoked_at.is_(None)) # ty: ignore[invalid-argument-type] + .where(col(RefreshToken.user_id) == user.id, col(RefreshToken.revoked_at).is_(None)) .values(revoked_at=now) ) - await db.execute(delete(UserAccessToken).where(UserAccessToken.user_id == user.id)) # ty: ignore[invalid-argument-type] + await db.execute(delete(UserAccessToken).where(col(UserAccessToken.user_id) == user.id)) await db.flush() request.session.clear() diff --git a/modules/users/users/auth_local/token_api.py b/modules/users/users/auth_local/token_api.py index 4b6386a73..9a718ddd3 100644 --- a/modules/users/users/auth_local/token_api.py +++ b/modules/users/users/auth_local/token_api.py @@ -14,7 +14,7 @@ from simple_module_db.deps import get_db from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession -from sqlmodel import SQLModel +from sqlmodel import SQLModel, col from users.auth_local.lockout import ( admit, @@ -128,9 +128,9 @@ async def token_refresh( now = datetime.now(UTC) stmt = select(RefreshToken).where( - RefreshToken.token == token_uuid, # ty: ignore[invalid-argument-type] + col(RefreshToken.token) == token_uuid, RefreshToken.revoked_at.is_(None), - RefreshToken.expires_at > now, # ty: ignore[invalid-argument-type] + col(RefreshToken.expires_at) > now, ) rt = (await db.execute(stmt)).scalar_one_or_none() if rt is None: @@ -144,7 +144,7 @@ async def token_refresh( # have been deactivated, or signed everything out. Minting from a stale # ``user_id`` alone would hand back a live credential to an account that no # longer has one. - user = (await db.execute(select(User).where(User.id == rt.user_id))).scalar_one_or_none() # ty: ignore[invalid-argument-type] + user = (await db.execute(select(User).where(col(User.id) == rt.user_id))).scalar_one_or_none() if user is None or not user.is_active or user.disabled_at is not None: raise HTTPException(status_code=401, detail="Invalid or expired refresh token") @@ -163,7 +163,7 @@ async def token_revoke( except (ValueError, TypeError): raise HTTPException(status_code=400, detail="Invalid token format") from None - stmt = select(RefreshToken).where(RefreshToken.token == token_uuid) # ty: ignore[invalid-argument-type] + stmt = select(RefreshToken).where(col(RefreshToken.token) == token_uuid) rt = (await db.execute(stmt)).scalar_one_or_none() if rt and rt.revoked_at is None: rt.revoked_at = datetime.now(UTC) diff --git a/modules/users/users/bootstrap.py b/modules/users/users/bootstrap.py index addd25378..5bb2876c2 100644 --- a/modules/users/users/bootstrap.py +++ b/modules/users/users/bootstrap.py @@ -11,6 +11,7 @@ from simple_module_core.dotenv import parse_dotenv from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from users.constants import ( ADMIN_ROLE_DESCRIPTION, @@ -74,11 +75,11 @@ async def create_admin( # Look up by name first (works on both Postgres and SQLite regardless of UUID # storage format), then fall back to id-based lookup in case name was changed. admin_role = ( - await db.execute(select(Role).where(Role.name == ADMIN_ROLE_NAME)) # ty: ignore[invalid-argument-type] + await db.execute(select(Role).where(col(Role.name) == ADMIN_ROLE_NAME)) ).scalar_one_or_none() if admin_role is None: admin_role = ( - await db.execute(select(Role).where(Role.id == ADMIN_ROLE_ID)) # ty: ignore[invalid-argument-type] + await db.execute(select(Role).where(col(Role.id) == ADMIN_ROLE_ID)) ).scalar_one_or_none() if admin_role is None: # The seed migration (e3ce9754e6dc) inserts this row, so in a real @@ -120,8 +121,8 @@ async def create_admin( existing_link = ( await db.execute( select(UserRole).where( - UserRole.user_id == existing.id, # ty: ignore[invalid-argument-type] - UserRole.role_id == admin_role.id, # ty: ignore[invalid-argument-type] + col(UserRole.user_id) == existing.id, + col(UserRole.role_id) == admin_role.id, ) ) ).scalar_one_or_none() @@ -143,11 +144,11 @@ async def ensure_user_role(db: AsyncSession) -> Role: not assume it exists. """ user_role = ( - await db.execute(select(Role).where(Role.name == USER_ROLE_NAME)) # ty: ignore[invalid-argument-type] + await db.execute(select(Role).where(col(Role.name) == USER_ROLE_NAME)) ).scalar_one_or_none() if user_role is None: user_role = ( - await db.execute(select(Role).where(Role.id == USER_ROLE_ID)) # ty: ignore[invalid-argument-type] + await db.execute(select(Role).where(col(Role.id) == USER_ROLE_ID)) ).scalar_one_or_none() if user_role is None: user_role = Role(id=USER_ROLE_ID, name=USER_ROLE_NAME, description=USER_ROLE_DESCRIPTION) diff --git a/modules/users/users/demo.py b/modules/users/users/demo.py index fcb201937..b63d452be 100644 --- a/modules/users/users/demo.py +++ b/modules/users/users/demo.py @@ -31,6 +31,7 @@ from fastapi_users.password import PasswordHelper from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from users.constants import ( ADMIN_ROLE_DESCRIPTION, @@ -127,11 +128,11 @@ async def _role_row(db: AsyncSession, name: str) -> Role: both, so this only fires for tests built with ``create_all`` and for databases downgraded past the seed revision. """ - role = (await db.execute(select(Role).where(Role.name == name))).scalar_one_or_none() # ty: ignore[invalid-argument-type] + role = (await db.execute(select(Role).where(col(Role.name) == name))).scalar_one_or_none() if role is not None: return role role_id, description = _ROLE_SEEDS[name] - role = (await db.execute(select(Role).where(Role.id == role_id))).scalar_one_or_none() # ty: ignore[invalid-argument-type] + role = (await db.execute(select(Role).where(col(Role.id) == role_id))).scalar_one_or_none() if role is None: role = Role(id=role_id, name=name, description=description) db.add(role) @@ -147,7 +148,9 @@ async def _sync_role(db: AsyncSession, user: User, role_name: str) -> None: sync that only ever added rows would leave the admin grant in place. """ wanted = await _role_row(db, role_name) - links = (await db.execute(select(UserRole).where(UserRole.user_id == user.id))).scalars().all() # ty: ignore[invalid-argument-type] + links = ( + (await db.execute(select(UserRole).where(col(UserRole.user_id) == user.id))).scalars().all() + ) managed = {rid for rid, _ in _ROLE_SEEDS.values()} if not any(link.role_id == wanted.id for link in links): db.add(UserRole(user_id=user.id, role_id=wanted.id)) diff --git a/modules/users/users/provider.py b/modules/users/users/provider.py index 58c371349..85fec660e 100644 --- a/modules/users/users/provider.py +++ b/modules/users/users/provider.py @@ -22,6 +22,7 @@ ensure_session_expiry, session_has_expired, ) +from sqlmodel import col from starlette.requests import Request from users.constants import SESSION_DEMO_KEY, SESSION_USER_ID_KEY, SESSION_VERSION_KEY @@ -254,7 +255,7 @@ async def _load_user(self, scope, user_id: uuid_mod.UUID, session=None) -> UserC # otherwise fire an extra query the UserContext never reads. stmt = ( select(User) - .where(User.id == user_id) # ty: ignore[invalid-argument-type] + .where(col(User.id) == user_id) .options(selectinload(User.roles), noload(User.oauth_accounts)) # ty: ignore[invalid-argument-type] ) user = (await db_session.execute(stmt)).scalar_one_or_none() diff --git a/modules/users/users/registration.py b/modules/users/users/registration.py index 6800883b9..6f61d1e86 100644 --- a/modules/users/users/registration.py +++ b/modules/users/users/registration.py @@ -6,6 +6,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel import col from users.constants import USER_ROLE_NAME from users.models import Role, User, UserRole @@ -28,7 +29,7 @@ async def attach_default_role(session: AsyncSession, user: User) -> None: from racing to insert it. """ role = ( - await session.execute(select(Role).where(Role.name == USER_ROLE_NAME)) # ty: ignore[invalid-argument-type] + await session.execute(select(Role).where(col(Role.name) == USER_ROLE_NAME)) ).scalar_one_or_none() if role is None: logger.warning("No '%s' role exists; %s registered with no role", USER_ROLE_NAME, user.id) diff --git a/modules/users/users/token_strategy.py b/modules/users/users/token_strategy.py index da46d5122..e871e2676 100644 --- a/modules/users/users/token_strategy.py +++ b/modules/users/users/token_strategy.py @@ -27,6 +27,7 @@ from fastapi_users import exceptions, models from fastapi_users.authentication.strategy.db import AccessTokenDatabase, DatabaseStrategy from fastapi_users.manager import BaseUserManager +from sqlmodel import col from users.models import UserAccessToken @@ -135,9 +136,9 @@ async def resolve_bearer(scope, token: str) -> UserContext | None: now = datetime.now(UTC) cutoff = now - timedelta(seconds=_TOKEN_LIFETIME_SECONDS) stmt = select(UserAccessToken).where( - UserAccessToken.token == token, # ty: ignore[invalid-argument-type] - UserAccessToken.created_at > cutoff, # ty: ignore[invalid-argument-type] - UserAccessToken.expires_at > now, # ty: ignore[invalid-argument-type] + col(UserAccessToken.token) == token, + col(UserAccessToken.created_at) > cutoff, + col(UserAccessToken.expires_at) > now, ) access = (await db_session.execute(stmt)).scalar_one_or_none() if access is None: @@ -146,7 +147,7 @@ async def resolve_bearer(scope, token: str) -> UserContext | None: # otherwise fire an extra query the UserContext never reads. stmt = ( select(User) - .where(User.id == access.user_id) # ty: ignore[invalid-argument-type] + .where(col(User.id) == access.user_id) .options(selectinload(User.roles), noload(User.oauth_accounts)) # ty: ignore[invalid-argument-type] ) user = (await db_session.execute(stmt)).scalar_one_or_none() From 245785b1ecf499e846587eb98652321285032dc5 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 23 Oct 2026 10:34:52 +0200 Subject: [PATCH 2/3] users/keycloak: share the user-directory SQL helpers from simple_module_db Closes #596 Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../db/simple_module_db/user_directory_sql.py | 41 +++++++++++++++++ .../db/tests/test_db_user_directory_sql.py | 29 ++++++++++++ modules/keycloak/keycloak/directory.py | 31 +++++-------- modules/users/users/directory.py | 44 ++++++++----------- 4 files changed, 100 insertions(+), 45 deletions(-) create mode 100644 framework/db/simple_module_db/user_directory_sql.py create mode 100644 framework/db/tests/test_db_user_directory_sql.py diff --git a/framework/db/simple_module_db/user_directory_sql.py b/framework/db/simple_module_db/user_directory_sql.py new file mode 100644 index 000000000..fe9f8efd0 --- /dev/null +++ b/framework/db/simple_module_db/user_directory_sql.py @@ -0,0 +1,41 @@ +"""SQL-backed user directory helpers shared by account providers.""" + +from __future__ import annotations + +from typing import Any + +from simple_module_core.user_directory import DirectoryUser +from sqlalchemy import or_ +from sqlalchemy.sql.elements import ColumnElement + +from simple_module_db.search import LIKE_ESCAPE_CHAR, like_contains_pattern + + +def name_or_email_contains(full_name: Any, email: Any, term: str) -> ColumnElement[bool]: + """Match a literal substring in either provider column, ignoring case.""" + pattern = like_contains_pattern(term) + return or_( + full_name.ilike(pattern, escape=LIKE_ESCAPE_CHAR), + email.ilike(pattern, escape=LIKE_ESCAPE_CHAR), + ) + + +def to_directory_user( + row: Any, + key: str, + *, + roles: tuple[str, ...] | None = None, + is_active: bool = True, +) -> DirectoryUser: + """Convert identity columns, preserving unknown versus empty roles. + + Providers supply account availability and roles when known. ``None`` means + unknown roles; an empty tuple means no assigned roles. + """ + return DirectoryUser( + id=key, + email=row.email, + full_name=row.full_name, + is_active=is_active, + roles=roles, + ) diff --git a/framework/db/tests/test_db_user_directory_sql.py b/framework/db/tests/test_db_user_directory_sql.py new file mode 100644 index 000000000..7734e0f7c --- /dev/null +++ b/framework/db/tests/test_db_user_directory_sql.py @@ -0,0 +1,29 @@ +"""Shared directory predicates treat SQL LIKE metacharacters as literal text.""" + +from types import SimpleNamespace + +import pytest +from simple_module_db.user_directory_sql import name_or_email_contains, to_directory_user +from sqlalchemy import literal, select +from sqlmodel import Session, create_engine + + +@pytest.mark.parametrize("term", ["%", "_", "\\"]) +def test_name_or_email_contains_escapes_literal_metacharacters(term: str): + with Session(create_engine("sqlite://")) as db: + for full_name, email in [(f"Ada{term}Lovelace", "other"), (None, f"ada{term}@test")]: + predicate = name_or_email_contains(literal(full_name), literal(email), term) + assert db.exec(select(predicate)).scalar_one() + predicate = name_or_email_contains(literal("Ada Lovelace"), literal("ada@test"), term) + assert not db.exec(select(predicate)).scalar_one() + + +@pytest.mark.parametrize("roles", [None, (), ("editor",)]) +def test_conversion_preserves_known_and_unknown_roles(roles: tuple[str, ...] | None): + row = SimpleNamespace(email="ada@test", full_name=None) + user = to_directory_user(row, "caller-id", roles=roles, is_active=False) + assert user.id == "caller-id" + assert user.email == row.email + assert user.full_name is None + assert user.roles == roles + assert not user.is_active diff --git a/modules/keycloak/keycloak/directory.py b/modules/keycloak/keycloak/directory.py index 358e17fca..adeaffb76 100644 --- a/modules/keycloak/keycloak/directory.py +++ b/modules/keycloak/keycloak/directory.py @@ -19,8 +19,8 @@ chunked, parse_user_ids, ) -from simple_module_db import LIKE_ESCAPE_CHAR, like_contains_pattern -from sqlalchemy import String, func, or_, select +from simple_module_db.user_directory_sql import name_or_email_contains, to_directory_user +from sqlalchemy import String, func, select from sqlalchemy.ext.compiler import compiles from sqlalchemy.sql.expression import FunctionElement @@ -54,19 +54,6 @@ def _uuid_text_sqlite(element: Any, compiler: Any, **kw: Any) -> str: _COLUMNS = (KeycloakUserCache.id, KeycloakUserCache.email, KeycloakUserCache.full_name) -def _to_directory_user(row: Any, key: str) -> DirectoryUser: - # ``roles`` stays ``None``: unknown, which callers must not read as "none". - return DirectoryUser(id=key, email=row.email, full_name=row.full_name) - - -def _name_or_email_contains(term: str) -> Any: - pattern = like_contains_pattern(term) - return or_( - KeycloakUserCache.full_name.ilike(pattern, escape=LIKE_ESCAPE_CHAR), - KeycloakUserCache.email.ilike(pattern, escape=LIKE_ESCAPE_CHAR), - ) - - class KeycloakDirectory: """Directory over ``keycloak_user_cache``.""" @@ -83,17 +70,23 @@ async def get_many( stmt = select(*_COLUMNS).where(KeycloakUserCache.id.in_(chunk)) for row in (await db.execute(stmt)).all(): for key in parsed[row.id]: - found[key] = _to_directory_user(row, key) + found[key] = to_directory_user(row, key) return found async def search(self, db: Any, term: str | None, *, limit: int) -> list[DirectoryUser]: stmt = select(*_COLUMNS).order_by(KeycloakUserCache.email).limit(limit) if term and term.strip(): - stmt = stmt.where(_name_or_email_contains(term.strip())) - return [_to_directory_user(row, str(row.id)) for row in (await db.execute(stmt)).all()] + stmt = stmt.where( + name_or_email_contains( + KeycloakUserCache.full_name, KeycloakUserCache.email, term.strip() + ) + ) + return [to_directory_user(row, str(row.id)) for row in (await db.execute(stmt)).all()] def match_ids(self, term: str) -> Any | None: - return select(_UuidText(KeycloakUserCache.id)).where(_name_or_email_contains(term)) + return select(_UuidText(KeycloakUserCache.id)).where( + name_or_email_contains(KeycloakUserCache.full_name, KeycloakUserCache.email, term) + ) async def stats(self, db: Any, *, active_window: timedelta) -> DirectoryStats: cutoff = datetime.now(UTC) - active_window diff --git a/modules/users/users/directory.py b/modules/users/users/directory.py index 658bc3f94..0a3e9fa9e 100644 --- a/modules/users/users/directory.py +++ b/modules/users/users/directory.py @@ -20,8 +20,8 @@ chunked, parse_user_ids, ) -from simple_module_db import LIKE_ESCAPE_CHAR, like_contains_pattern -from sqlalchemy import String, cast, func, or_, select +from simple_module_db.user_directory_sql import name_or_email_contains, to_directory_user +from sqlalchemy import String, cast, func, select from users.models import Role, User, UserRole @@ -30,26 +30,6 @@ _COLUMNS = (User.id, User.email, User.full_name, User.is_active, User.disabled_at) -def _to_directory_user(row: Any, roles: tuple[str, ...] | None, key: str) -> DirectoryUser: - return DirectoryUser( - id=key, - email=row.email, - full_name=row.full_name, - # A disabled account is still ``is_active`` in fastapi-users' sense; - # to every other module it is simply not usable. - is_active=bool(row.is_active) and row.disabled_at is None, - roles=roles, - ) - - -def _name_or_email_contains(term: str) -> Any: - pattern = like_contains_pattern(term) - return or_( - User.full_name.ilike(pattern, escape=LIKE_ESCAPE_CHAR), - User.email.ilike(pattern, escape=LIKE_ESCAPE_CHAR), - ) - - class UsersDirectory: """Directory over the local ``users_user`` table.""" @@ -68,7 +48,12 @@ async def get_many( for row in rows: user_roles = tuple(sorted(roles.get(row.id, ()))) if with_roles else None for key in parsed[row.id]: - found[key] = _to_directory_user(row, user_roles, key) + found[key] = to_directory_user( + row, + key, + roles=user_roles, + is_active=bool(row.is_active) and row.disabled_at is None, + ) return found async def _roles_for( @@ -87,15 +72,22 @@ async def _roles_for( async def search(self, db: Any, term: str | None, *, limit: int) -> list[DirectoryUser]: stmt = select(*_COLUMNS).order_by(User.email).limit(limit) if term and term.strip(): - stmt = stmt.where(_name_or_email_contains(term.strip())) + stmt = stmt.where(name_or_email_contains(User.full_name, User.email, term.strip())) rows = (await db.execute(stmt)).all() - return [_to_directory_user(row, None, str(row.id)) for row in rows] + return [ + to_directory_user( + row, str(row.id), is_active=bool(row.is_active) and row.disabled_at is None + ) + for row in rows + ] def match_ids(self, term: str) -> Any | None: # ``CAST`` renders the canonical lowercase hyphenated form on both # dialects (Postgres from ``uuid``, SQLite from GUID's ``CHAR(36)``), # which is the form the audit trail and every other text id stores. - return select(cast(User.id, String)).where(_name_or_email_contains(term)) + return select(cast(User.id, String)).where( + name_or_email_contains(User.full_name, User.email, term) + ) async def stats(self, db: Any, *, active_window: timedelta) -> DirectoryStats: now = datetime.now(UTC) From cfdcede2f565a38da14b8b35efc40cee0dc321ec Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 23 Oct 2026 11:05:42 +0200 Subject: [PATCH 3/3] users: name the usable-account predicate in the directory Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- modules/users/users/directory.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/modules/users/users/directory.py b/modules/users/users/directory.py index 0a3e9fa9e..0a80604ff 100644 --- a/modules/users/users/directory.py +++ b/modules/users/users/directory.py @@ -30,6 +30,12 @@ _COLUMNS = (User.id, User.email, User.full_name, User.is_active, User.disabled_at) +def _is_usable(row: Any) -> bool: + # A disabled account is still ``is_active`` in fastapi-users' sense; + # to every other module it is simply not usable. + return bool(row.is_active) and row.disabled_at is None + + class UsersDirectory: """Directory over the local ``users_user`` table.""" @@ -49,10 +55,7 @@ async def get_many( user_roles = tuple(sorted(roles.get(row.id, ()))) if with_roles else None for key in parsed[row.id]: found[key] = to_directory_user( - row, - key, - roles=user_roles, - is_active=bool(row.is_active) and row.disabled_at is None, + row, key, roles=user_roles, is_active=_is_usable(row) ) return found @@ -74,12 +77,7 @@ async def search(self, db: Any, term: str | None, *, limit: int) -> list[Directo if term and term.strip(): stmt = stmt.where(name_or_email_contains(User.full_name, User.email, term.strip())) rows = (await db.execute(stmt)).all() - return [ - to_directory_user( - row, str(row.id), is_active=bool(row.is_active) and row.disabled_at is None - ) - for row in rows - ] + return [to_directory_user(row, str(row.id), is_active=_is_usable(row)) for row in rows] def match_ids(self, term: str) -> Any | None: # ``CAST`` renders the canonical lowercase hyphenated form on both