From e553f94c6dc17672ff865b780c4a7db9814ea8c0 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 5 Nov 2025 15:42:55 +0530 Subject: [PATCH 01/20] Add configuration management models and migration scripts --- .../bbf6f525d6a3_config_management_tables.py | 63 ++++++++++++++++ backend/app/models/__init__.py | 11 +++ backend/app/models/config/__init__.py | 19 +++++ backend/app/models/config/config.py | 72 +++++++++++++++++++ backend/app/models/config/version.py | 70 ++++++++++++++++++ 5 files changed, 235 insertions(+) create mode 100644 backend/app/alembic/versions/bbf6f525d6a3_config_management_tables.py create mode 100644 backend/app/models/config/__init__.py create mode 100644 backend/app/models/config/config.py create mode 100644 backend/app/models/config/version.py diff --git a/backend/app/alembic/versions/bbf6f525d6a3_config_management_tables.py b/backend/app/alembic/versions/bbf6f525d6a3_config_management_tables.py new file mode 100644 index 000000000..0c68a321a --- /dev/null +++ b/backend/app/alembic/versions/bbf6f525d6a3_config_management_tables.py @@ -0,0 +1,63 @@ +"""Config management tables + +Revision ID: bbf6f525d6a3 +Revises: 219033c644de +Create Date: 2025-11-05 15:38:49.629076 + +""" +from alembic import op +import sqlalchemy as sa +import sqlmodel.sql.sqltypes +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = 'bbf6f525d6a3' +down_revision = '219033c644de' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('config', + sa.Column('name', sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), + sa.Column('description', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), + sa.Column('id', sa.Uuid(), nullable=False), + sa.Column('project_id', sa.Integer(), nullable=False), + sa.Column('inserted_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=False), + sa.Column('deleted_at', sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(['project_id'], ['project.id'], ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('name', 'project_id') + ) + op.create_index(op.f('ix_config_name'), 'config', ['name'], unique=False) + op.create_index(op.f('ix_config_project_id'), 'config', ['project_id'], unique=False) + op.create_table('config_version', + sa.Column('config_json', postgresql.JSON(astext_type=sa.Text()), nullable=False), + sa.Column('commit_message', sqlmodel.sql.sqltypes.AutoString(), nullable=True), + sa.Column('id', sa.Uuid(), nullable=False), + sa.Column('config_id', sa.Uuid(), nullable=False), + sa.Column('version', sa.Integer(), nullable=False), + sa.Column('inserted_at', sa.DateTime(), nullable=False), + sa.Column('deleted_at', sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(['config_id'], ['config.id'], ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('config_id', 'version') + ) + op.create_index(op.f('ix_config_version_config_id'), 'config_version', ['config_id'], unique=False) + op.drop_constraint(op.f('openai_conversation_organization_id_fkey1'), 'openai_conversation', type_='foreignkey') + op.drop_constraint(op.f('openai_conversation_project_id_fkey1'), 'openai_conversation', type_='foreignkey') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_foreign_key(op.f('openai_conversation_project_id_fkey1'), 'openai_conversation', 'project', ['project_id'], ['id']) + op.create_foreign_key(op.f('openai_conversation_organization_id_fkey1'), 'openai_conversation', 'organization', ['organization_id'], ['id']) + op.drop_index(op.f('ix_config_version_config_id'), table_name='config_version') + op.drop_table('config_version') + op.drop_index(op.f('ix_config_project_id'), table_name='config') + op.drop_index(op.f('ix_config_name'), table_name='config') + op.drop_table('config') + # ### end Alembic commands ### diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index b2f294025..09ddd7246 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -21,6 +21,17 @@ CollectionJobCreate, CollectionJobImmediatePublic, ) +from .config import ( + Config, + ConfigBase, + ConfigCreate, + ConfigUpdate, + ConfigPublic, + ConfigVersion, + ConfigVersionBase, + ConfigVersionCreate, + ConfigVersionPublic, +) from .credentials import ( Credential, CredsBase, diff --git a/backend/app/models/config/__init__.py b/backend/app/models/config/__init__.py new file mode 100644 index 000000000..ae27df584 --- /dev/null +++ b/backend/app/models/config/__init__.py @@ -0,0 +1,19 @@ +from .config import Config, ConfigBase, ConfigCreate, ConfigPublic, ConfigUpdate +from .version import ( + ConfigVersion, + ConfigVersionBase, + ConfigVersionCreate, + ConfigVersionPublic, +) + +__all__ = [ + "Config", + "ConfigBase", + "ConfigCreate", + "ConfigPublic", + "ConfigUpdate", + "ConfigVersion", + "ConfigVersionBase", + "ConfigVersionCreate", + "ConfigVersionPublic", +] diff --git a/backend/app/models/config/config.py b/backend/app/models/config/config.py new file mode 100644 index 000000000..e4f38e9d3 --- /dev/null +++ b/backend/app/models/config/config.py @@ -0,0 +1,72 @@ +from uuid import UUID, uuid4 +from datetime import datetime +from typing import TYPE_CHECKING, Any + +from sqlmodel import Field, SQLModel, UniqueConstraint + +from app.core.util import now + + +class ConfigBase(SQLModel): + """Base model for LLM configuration metadata""" + + name: str = Field(index=True, min_length=1, max_length=128, description="Config name") + description: str | None = Field(default=None, max_length=512, description="Optional description") + + +class Config(ConfigBase, table=True): + """Database model for LLM configuration storage""" + + __tablename__ = "config" + __table_args__ = ( + UniqueConstraint( + "name", + "project_id", + ), + ) + + id: UUID = Field(default=uuid4, primary_key=True) + + project_id: int = Field( + foreign_key="project.id", + index=True, + nullable=False, + ondelete="CASCADE", + ) + + inserted_at: datetime = Field(default_factory=now, nullable=False) + updated_at: datetime = Field(default_factory=now, nullable=False) + + deleted_at: datetime | None = Field(default=None, nullable=True) + + +class ConfigCreate(SQLModel): + """Create new configuration""" + + name: str = Field(max_length=255) + description: str | None = Field(default=None, max_length=500) + + # Initial version data + config_json: dict[str, Any] = Field(description="Provider-specific parameters") + commit_message: str | None = Field( + default=None, + max_length=512, + description="Optional message describing the changes in this version", + ) + + + +class ConfigUpdate(SQLModel): + name: str | None = Field(default=None, max_length=128) + description: str | None = Field(default=None, max_length=512, description="Optional description") + + +class ConfigPublic(ConfigBase): + + id: int + project_id: int + organization_id: int + latest_version_id: int | None + created_by_user_id: int | None + inserted_at: datetime + updated_at: datetime diff --git a/backend/app/models/config/version.py b/backend/app/models/config/version.py new file mode 100644 index 000000000..e1cdb109b --- /dev/null +++ b/backend/app/models/config/version.py @@ -0,0 +1,70 @@ +from datetime import datetime +from uuid import UUID, uuid4 +from typing import Any + +import sqlalchemy as sa +from sqlalchemy.dialects.postgresql import JSON +from sqlmodel import Field, SQLModel, UniqueConstraint + +from app.core.util import now + + +class ConfigVersionBase(SQLModel): + + config_json: dict[str, Any] = Field( + sa_column=sa.Column(JSON, nullable=False), + description="Provider-specific configuration parameters (temperature, max_tokens, etc.)", + ) + commit_message: str | None = Field( + default=None, + max_length=512, + description="Optional message describing the changes in this version", + ) + + +class ConfigVersion(ConfigVersionBase, table=True): + + __tablename__ = "config_version" + __table_args__ = ( + UniqueConstraint( + "config_id", + "version", + ), + ) + + id: UUID = Field(default=uuid4, primary_key=True) + + # Parent config reference + config_id: UUID = Field( + foreign_key="config.id", + index=True, + nullable=False, + ondelete="CASCADE", + ) + + # Version number (immutable, auto-increments per config) + version: int = Field(nullable=False, description="Version number starting at 1") + + inserted_at: datetime = Field(default_factory=now, nullable=False) + + deleted_at: datetime | None = Field(default=None, nullable=True) + + + +class ConfigVersionCreate(SQLModel): + + config_json: dict[str, Any] + commit_message: str | None = Field( + default=None, + max_length=512, + description="Optional message describing the changes in this version", + ) + + +class ConfigVersionPublic(ConfigVersionBase): + + id: int + config_id: int + version: int + created_by_user_id: int | None + inserted_at: datetime From b69fb8353804c461b745c99537725b6af3440b76 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 5 Nov 2025 16:31:47 +0530 Subject: [PATCH 02/20] implement create endpoint for config management --- ... e51b082aa9b3_config_management_tables.py} | 18 ++-- backend/app/api/main.py | 2 + backend/app/api/routes/config/__init__.py | 10 +++ backend/app/api/routes/config/config.py | 43 +++++++++ backend/app/api/routes/config/version.py | 27 ++++++ backend/app/crud/config/__init__.py | 3 + backend/app/crud/config/config.py | 89 +++++++++++++++++++ backend/app/models/__init__.py | 1 + backend/app/models/config/__init__.py | 9 +- backend/app/models/config/config.py | 39 ++++---- backend/app/models/config/version.py | 19 ++-- 11 files changed, 216 insertions(+), 44 deletions(-) rename backend/app/alembic/versions/{bbf6f525d6a3_config_management_tables.py => e51b082aa9b3_config_management_tables.py} (92%) create mode 100644 backend/app/api/routes/config/__init__.py create mode 100644 backend/app/api/routes/config/config.py create mode 100644 backend/app/api/routes/config/version.py create mode 100644 backend/app/crud/config/__init__.py create mode 100644 backend/app/crud/config/config.py diff --git a/backend/app/alembic/versions/bbf6f525d6a3_config_management_tables.py b/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py similarity index 92% rename from backend/app/alembic/versions/bbf6f525d6a3_config_management_tables.py rename to backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py index 0c68a321a..2267ce05d 100644 --- a/backend/app/alembic/versions/bbf6f525d6a3_config_management_tables.py +++ b/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py @@ -1,8 +1,8 @@ """Config management tables -Revision ID: bbf6f525d6a3 +Revision ID: e51b082aa9b3 Revises: 219033c644de -Create Date: 2025-11-05 15:38:49.629076 +Create Date: 2025-11-05 16:19:20.423398 """ from alembic import op @@ -11,7 +11,7 @@ from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. -revision = 'bbf6f525d6a3' +revision = 'e51b082aa9b3' down_revision = '219033c644de' branch_labels = None depends_on = None @@ -28,33 +28,33 @@ def upgrade(): sa.Column('updated_at', sa.DateTime(), nullable=False), sa.Column('deleted_at', sa.DateTime(), nullable=True), sa.ForeignKeyConstraint(['project_id'], ['project.id'], ondelete='CASCADE'), - sa.PrimaryKeyConstraint('id'), - sa.UniqueConstraint('name', 'project_id') + sa.PrimaryKeyConstraint('id') ) op.create_index(op.f('ix_config_name'), 'config', ['name'], unique=False) op.create_index(op.f('ix_config_project_id'), 'config', ['project_id'], unique=False) op.create_table('config_version', sa.Column('config_json', postgresql.JSON(astext_type=sa.Text()), nullable=False), - sa.Column('commit_message', sqlmodel.sql.sqltypes.AutoString(), nullable=True), + sa.Column('commit_message', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), + sa.Column('version', sa.Integer(), nullable=False), sa.Column('id', sa.Uuid(), nullable=False), sa.Column('config_id', sa.Uuid(), nullable=False), - sa.Column('version', sa.Integer(), nullable=False), sa.Column('inserted_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=False), sa.Column('deleted_at', sa.DateTime(), nullable=True), sa.ForeignKeyConstraint(['config_id'], ['config.id'], ondelete='CASCADE'), sa.PrimaryKeyConstraint('id'), sa.UniqueConstraint('config_id', 'version') ) op.create_index(op.f('ix_config_version_config_id'), 'config_version', ['config_id'], unique=False) - op.drop_constraint(op.f('openai_conversation_organization_id_fkey1'), 'openai_conversation', type_='foreignkey') op.drop_constraint(op.f('openai_conversation_project_id_fkey1'), 'openai_conversation', type_='foreignkey') + op.drop_constraint(op.f('openai_conversation_organization_id_fkey1'), 'openai_conversation', type_='foreignkey') # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.create_foreign_key(op.f('openai_conversation_project_id_fkey1'), 'openai_conversation', 'project', ['project_id'], ['id']) op.create_foreign_key(op.f('openai_conversation_organization_id_fkey1'), 'openai_conversation', 'organization', ['organization_id'], ['id']) + op.create_foreign_key(op.f('openai_conversation_project_id_fkey1'), 'openai_conversation', 'project', ['project_id'], ['id']) op.drop_index(op.f('ix_config_version_config_id'), table_name='config_version') op.drop_table('config_version') op.drop_index(op.f('ix_config_project_id'), table_name='config') diff --git a/backend/app/api/main.py b/backend/app/api/main.py index e2b473f92..ac6fd2863 100644 --- a/backend/app/api/main.py +++ b/backend/app/api/main.py @@ -4,6 +4,7 @@ api_keys, assistants, collections, + config, documents, doc_transformation_job, login, @@ -31,6 +32,7 @@ api_router.include_router(assistants.router) api_router.include_router(collections.router) api_router.include_router(collection_job.router) +api_router.include_router(config.router) api_router.include_router(credentials.router) api_router.include_router(cron.router) api_router.include_router(documents.router) diff --git a/backend/app/api/routes/config/__init__.py b/backend/app/api/routes/config/__init__.py new file mode 100644 index 000000000..4816a8a40 --- /dev/null +++ b/backend/app/api/routes/config/__init__.py @@ -0,0 +1,10 @@ +from fastapi import APIRouter + +from app.api.routes.config import config, version + +router = APIRouter(prefix="/configs", tags=["config management"]) + +router.include_router(config.router) +router.include_router(version.router) + +__all__ = ["router"] diff --git a/backend/app/api/routes/config/config.py b/backend/app/api/routes/config/config.py new file mode 100644 index 000000000..aad18ad98 --- /dev/null +++ b/backend/app/api/routes/config/config.py @@ -0,0 +1,43 @@ +from uuid import UUID +from fastapi import APIRouter, Depends, Query, HTTPException + +from app.api.deps import SessionDep, AuthContextDep +from app.crud.config import ConfigCrud +from app.models import ( + Config, + ConfigCreate, + ConfigUpdate, + ConfigPublic, + ConfigWithVersion, + ConfigVersion, + Message, +) +from app.utils import APIResponse +from app.api.permissions import Permission, require_permission + +router = APIRouter() + + +@router.post( + "/", + response_model=APIResponse[ConfigWithVersion], + status_code=201, + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def create_config_route( + config_create: ConfigCreate, + current_user: AuthContextDep, + session: SessionDep, +): + """ + create new config along with initial version + """ + config_crud = ConfigCrud(session=session, project_id=current_user.project.id) + config, version = config_crud.create(config_create) + + response = ConfigWithVersion(**config.model_dump(), version=version) + + return APIResponse.success_response( + data=response, + ) + diff --git a/backend/app/api/routes/config/version.py b/backend/app/api/routes/config/version.py new file mode 100644 index 000000000..08cb9e791 --- /dev/null +++ b/backend/app/api/routes/config/version.py @@ -0,0 +1,27 @@ +from uuid import UUID +from fastapi import APIRouter, Depends, Query, HTTPException + +from app.api.deps import SessionDep, AuthContextDep +from app.models import ( + ConfigVersionCreate, + ConfigVersionPublic, +) +from app.utils import APIResponse +from app.api.permissions import Permission, require_permission + +router = APIRouter() + + +@router.post( + "/{config_id}/versions", + response_model=APIResponse[ConfigVersionPublic], + status_code=201, + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def create_version_route( + config_id: UUID, + version_create: ConfigVersionCreate, + current_user: AuthContextDep, + session: SessionDep, +): + pass \ No newline at end of file diff --git a/backend/app/crud/config/__init__.py b/backend/app/crud/config/__init__.py new file mode 100644 index 000000000..6545f2fb6 --- /dev/null +++ b/backend/app/crud/config/__init__.py @@ -0,0 +1,3 @@ +from app.crud.config.config import ConfigCrud + +__all__ = ["ConfigCrud", "ConfigVersionCrud"] diff --git a/backend/app/crud/config/config.py b/backend/app/crud/config/config.py new file mode 100644 index 000000000..09f8552c4 --- /dev/null +++ b/backend/app/crud/config/config.py @@ -0,0 +1,89 @@ +import logging +from uuid import UUID +from typing import Tuple + +from sqlmodel import Session, select, and_ +from fastapi import HTTPException + +from app.models import ( + Config, + ConfigCreate, + ConfigUpdate, + ConfigVersion, +) +from app.crud.project import get_project_by_id +from app.core.util import now + +logger = logging.getLogger(__name__) + + +class ConfigCrud: + """ + CRUD operations for configurations scoped to a project. + """ + + def __init__(self, session: Session, project_id: int): + self.session = session + self.project_id = project_id + + def create(self, config_create: ConfigCreate) -> Tuple[Config, ConfigVersion]: + """ + Create a new configuration with an initial version. + """ + existing = self._get_by_name(config_create.name) + if existing: + raise HTTPException( + status_code=409, + detail=f"Configuration with name '{config_create.name}' already exists in this project", + ) + + try: + config = Config( + name=config_create.name, + description=config_create.description, + project_id=self.project_id, + ) + + self.session.add(config) + self.session.flush() # Flush to get the config.id + + # Create the initial version + version = ConfigVersion( + config_id=config.id, + version=1, + config_json=config_create.config_json, + commit_message=config_create.commit_message, + ) + + self.session.add(version) + self.session.commit() + self.session.refresh(config) + self.session.refresh(version) + + logger.info( + f"[ConfigCrud.create] Configuration created successfully | " + f"{{'config_id': '{config.id}', 'config_version_id': '{version.id}', 'project_id': {self.project_id}}}" + ) + + return config, version + + except Exception as e: + self.session.rollback() + logger.error( + f"[ConfigCrud.create] Failed to create configuration | " + f"{{'name': '{config_create.name}', 'project_id': {self.project_id}, 'error': '{str(e)}'}}", + exc_info=True, + ) + raise HTTPException( + status_code=500, detail=f"Unexpected error occurred: failed to create config" + ) + + def _get_by_name(self, name: str) -> Config | None: + statement = select(Config).where( + and_( + Config.name == name, + Config.project_id == self.project_id, + Config.deleted_at.is_(None), + ) + ) + return self.session.exec(statement).one_or_none() diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 09ddd7246..ba16741da 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -27,6 +27,7 @@ ConfigCreate, ConfigUpdate, ConfigPublic, + ConfigWithVersion, ConfigVersion, ConfigVersionBase, ConfigVersionCreate, diff --git a/backend/app/models/config/__init__.py b/backend/app/models/config/__init__.py index ae27df584..b88c4b91c 100644 --- a/backend/app/models/config/__init__.py +++ b/backend/app/models/config/__init__.py @@ -1,4 +1,11 @@ -from .config import Config, ConfigBase, ConfigCreate, ConfigPublic, ConfigUpdate +from .config import ( + Config, + ConfigBase, + ConfigCreate, + ConfigPublic, + ConfigUpdate, + ConfigWithVersion, +) from .version import ( ConfigVersion, ConfigVersionBase, diff --git a/backend/app/models/config/config.py b/backend/app/models/config/config.py index e4f38e9d3..ae3afea68 100644 --- a/backend/app/models/config/config.py +++ b/backend/app/models/config/config.py @@ -2,30 +2,29 @@ from datetime import datetime from typing import TYPE_CHECKING, Any -from sqlmodel import Field, SQLModel, UniqueConstraint +from sqlmodel import Field, SQLModel from app.core.util import now +from .version import ConfigVersionPublic class ConfigBase(SQLModel): """Base model for LLM configuration metadata""" - name: str = Field(index=True, min_length=1, max_length=128, description="Config name") - description: str | None = Field(default=None, max_length=512, description="Optional description") + name: str = Field( + index=True, min_length=1, max_length=128, description="Config name" + ) + description: str | None = Field( + default=None, max_length=512, description="Optional description" + ) class Config(ConfigBase, table=True): """Database model for LLM configuration storage""" __tablename__ = "config" - __table_args__ = ( - UniqueConstraint( - "name", - "project_id", - ), - ) - id: UUID = Field(default=uuid4, primary_key=True) + id: UUID = Field(default_factory=uuid4, primary_key=True) project_id: int = Field( foreign_key="project.id", @@ -40,12 +39,9 @@ class Config(ConfigBase, table=True): deleted_at: datetime | None = Field(default=None, nullable=True) -class ConfigCreate(SQLModel): +class ConfigCreate(ConfigBase): """Create new configuration""" - name: str = Field(max_length=255) - description: str | None = Field(default=None, max_length=500) - # Initial version data config_json: dict[str, Any] = Field(description="Provider-specific parameters") commit_message: str | None = Field( @@ -55,18 +51,19 @@ class ConfigCreate(SQLModel): ) - class ConfigUpdate(SQLModel): name: str | None = Field(default=None, max_length=128) - description: str | None = Field(default=None, max_length=512, description="Optional description") + description: str | None = Field( + default=None, max_length=512, description="Optional description" + ) class ConfigPublic(ConfigBase): - - id: int + id: UUID project_id: int - organization_id: int - latest_version_id: int | None - created_by_user_id: int | None inserted_at: datetime updated_at: datetime + + +class ConfigWithVersion(ConfigPublic): + version: ConfigVersionPublic diff --git a/backend/app/models/config/version.py b/backend/app/models/config/version.py index e1cdb109b..be1e6e8d7 100644 --- a/backend/app/models/config/version.py +++ b/backend/app/models/config/version.py @@ -10,7 +10,6 @@ class ConfigVersionBase(SQLModel): - config_json: dict[str, Any] = Field( sa_column=sa.Column(JSON, nullable=False), description="Provider-specific configuration parameters (temperature, max_tokens, etc.)", @@ -23,7 +22,6 @@ class ConfigVersionBase(SQLModel): class ConfigVersion(ConfigVersionBase, table=True): - __tablename__ = "config_version" __table_args__ = ( UniqueConstraint( @@ -32,27 +30,23 @@ class ConfigVersion(ConfigVersionBase, table=True): ), ) - id: UUID = Field(default=uuid4, primary_key=True) + id: UUID = Field(default_factory=uuid4, primary_key=True) - # Parent config reference config_id: UUID = Field( foreign_key="config.id", index=True, nullable=False, ondelete="CASCADE", ) - - # Version number (immutable, auto-increments per config) version: int = Field(nullable=False, description="Version number starting at 1") inserted_at: datetime = Field(default_factory=now, nullable=False) + updated_at: datetime = Field(default_factory=now, nullable=False) deleted_at: datetime | None = Field(default=None, nullable=True) - class ConfigVersionCreate(SQLModel): - config_json: dict[str, Any] commit_message: str | None = Field( default=None, @@ -62,9 +56,8 @@ class ConfigVersionCreate(SQLModel): class ConfigVersionPublic(ConfigVersionBase): - - id: int - config_id: int - version: int - created_by_user_id: int | None + id: UUID = Field(description="Unique id for the configuration version") + config_id: UUID = Field(description="Id of the parent configuration") + version: int = Field(nullable=False, description="Version number starting at 1") inserted_at: datetime + updated_at: datetime From 3e01b5913a8462e8eddebaf6bf4c4644027a69d9 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 5 Nov 2025 17:53:58 +0530 Subject: [PATCH 03/20] Implement CRUD operations for configuration and version management, including listing, retrieving, and deleting configurations and their versions. --- .../e51b082aa9b3_config_management_tables.py | 113 +++++++++++------ backend/app/api/routes/config/config.py | 67 ++++++++++ backend/app/api/routes/config/version.py | 40 +++++- backend/app/crud/config/__init__.py | 1 + backend/app/crud/config/config.py | 44 ++++++- backend/app/crud/config/version.py | 118 ++++++++++++++++++ backend/app/models/config/config.py | 4 + backend/app/models/config/version.py | 6 +- 8 files changed, 351 insertions(+), 42 deletions(-) create mode 100644 backend/app/crud/config/version.py diff --git a/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py b/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py index 2267ce05d..eaa503eda 100644 --- a/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py +++ b/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py @@ -11,53 +11,90 @@ from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. -revision = 'e51b082aa9b3' -down_revision = '219033c644de' +revision = "e51b082aa9b3" +down_revision = "219033c644de" branch_labels = None depends_on = None def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.create_table('config', - sa.Column('name', sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), - sa.Column('description', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), - sa.Column('id', sa.Uuid(), nullable=False), - sa.Column('project_id', sa.Integer(), nullable=False), - sa.Column('inserted_at', sa.DateTime(), nullable=False), - sa.Column('updated_at', sa.DateTime(), nullable=False), - sa.Column('deleted_at', sa.DateTime(), nullable=True), - sa.ForeignKeyConstraint(['project_id'], ['project.id'], ondelete='CASCADE'), - sa.PrimaryKeyConstraint('id') - ) - op.create_index(op.f('ix_config_name'), 'config', ['name'], unique=False) - op.create_index(op.f('ix_config_project_id'), 'config', ['project_id'], unique=False) - op.create_table('config_version', - sa.Column('config_json', postgresql.JSON(astext_type=sa.Text()), nullable=False), - sa.Column('commit_message', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), - sa.Column('version', sa.Integer(), nullable=False), - sa.Column('id', sa.Uuid(), nullable=False), - sa.Column('config_id', sa.Uuid(), nullable=False), - sa.Column('inserted_at', sa.DateTime(), nullable=False), - sa.Column('updated_at', sa.DateTime(), nullable=False), - sa.Column('deleted_at', sa.DateTime(), nullable=True), - sa.ForeignKeyConstraint(['config_id'], ['config.id'], ondelete='CASCADE'), - sa.PrimaryKeyConstraint('id'), - sa.UniqueConstraint('config_id', 'version') - ) - op.create_index(op.f('ix_config_version_config_id'), 'config_version', ['config_id'], unique=False) - op.drop_constraint(op.f('openai_conversation_project_id_fkey1'), 'openai_conversation', type_='foreignkey') - op.drop_constraint(op.f('openai_conversation_organization_id_fkey1'), 'openai_conversation', type_='foreignkey') + op.create_table( + "config", + sa.Column("name", sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), + sa.Column( + "description", sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True + ), + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("project_id", sa.Integer(), nullable=False), + sa.Column("inserted_at", sa.DateTime(), nullable=False), + sa.Column("updated_at", sa.DateTime(), nullable=False), + sa.Column("deleted_at", sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(["project_id"], ["project.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index(op.f("ix_config_name"), "config", ["name"], unique=False) + op.create_index( + op.f("ix_config_project_id"), "config", ["project_id"], unique=False + ) + op.create_table( + "config_version", + sa.Column( + "config_json", postgresql.JSON(astext_type=sa.Text()), nullable=False + ), + sa.Column( + "commit_message", + sqlmodel.sql.sqltypes.AutoString(length=512), + nullable=True, + ), + sa.Column("version", sa.Integer(), nullable=False), + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("config_id", sa.Uuid(), nullable=False), + sa.Column("inserted_at", sa.DateTime(), nullable=False), + sa.Column("updated_at", sa.DateTime(), nullable=False), + sa.Column("deleted_at", sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(["config_id"], ["config.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("config_id", "version"), + ) + op.create_index( + op.f("ix_config_version_config_id"), + "config_version", + ["config_id"], + unique=False, + ) + op.drop_constraint( + op.f("openai_conversation_project_id_fkey1"), + "openai_conversation", + type_="foreignkey", + ) + op.drop_constraint( + op.f("openai_conversation_organization_id_fkey1"), + "openai_conversation", + type_="foreignkey", + ) # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.create_foreign_key(op.f('openai_conversation_organization_id_fkey1'), 'openai_conversation', 'organization', ['organization_id'], ['id']) - op.create_foreign_key(op.f('openai_conversation_project_id_fkey1'), 'openai_conversation', 'project', ['project_id'], ['id']) - op.drop_index(op.f('ix_config_version_config_id'), table_name='config_version') - op.drop_table('config_version') - op.drop_index(op.f('ix_config_project_id'), table_name='config') - op.drop_index(op.f('ix_config_name'), table_name='config') - op.drop_table('config') + op.create_foreign_key( + op.f("openai_conversation_organization_id_fkey1"), + "openai_conversation", + "organization", + ["organization_id"], + ["id"], + ) + op.create_foreign_key( + op.f("openai_conversation_project_id_fkey1"), + "openai_conversation", + "project", + ["project_id"], + ["id"], + ) + op.drop_index(op.f("ix_config_version_config_id"), table_name="config_version") + op.drop_table("config_version") + op.drop_index(op.f("ix_config_project_id"), table_name="config") + op.drop_index(op.f("ix_config_name"), table_name="config") + op.drop_table("config") # ### end Alembic commands ### diff --git a/backend/app/api/routes/config/config.py b/backend/app/api/routes/config/config.py index aad18ad98..afe0a4903 100644 --- a/backend/app/api/routes/config/config.py +++ b/backend/app/api/routes/config/config.py @@ -41,3 +41,70 @@ def create_config_route( data=response, ) + +@router.get( + "/", + response_model=APIResponse[list[ConfigPublic]], + status_code=200, + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def list_configs_route( + current_user: AuthContextDep, + session: SessionDep, + skip: int = Query(0, ge=0, description="Number of records to skip"), + limit: int = Query(100, ge=1, le=100, description="Maximum records to return"), +): + """ + List all configurations for the current project. + """ + + # Decide how to handle pagination effectively + config_crud = ConfigCrud(session=session, project_id=current_user.project.id) + configs = config_crud.read_all(skip=skip, limit=limit) + return APIResponse.success_response( + data=configs, + ) + + +@router.get( + "/{config_id}", + response_model=APIResponse[ConfigWithVersion], + status_code=200, + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def get_config_route( + config_id: UUID, + current_user: AuthContextDep, + session: SessionDep, +): + """ + Get a specific configuration by ID. + """ + # Decide how to handle fetching versions + # should we fetch all versions at /{config_id}/versions and donot include here? + config_crud = ConfigCrud(session=session, project_id=current_user.project.id) + config = config_crud.read_one(config_id=config_id) + + pass + + +@router.delete( + "/{config_id}", + response_model=APIResponse[Message], + status_code=200, + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def delete_config_route( + config_id: UUID, + current_user: AuthContextDep, + session: SessionDep, +): + """ + Delete a specific configuration. + """ + config_crud = ConfigCrud(session=session, project_id=current_user.project.id) + config_crud.delete(config_id=config_id) + + return APIResponse.success_response( + data=Message(message="Config deleted successfully"), + ) diff --git a/backend/app/api/routes/config/version.py b/backend/app/api/routes/config/version.py index 08cb9e791..d4cb49551 100644 --- a/backend/app/api/routes/config/version.py +++ b/backend/app/api/routes/config/version.py @@ -2,9 +2,11 @@ from fastapi import APIRouter, Depends, Query, HTTPException from app.api.deps import SessionDep, AuthContextDep +from app.crud.config import ConfigCrud, ConfigVersionCrud from app.models import ( ConfigVersionCreate, ConfigVersionPublic, + Message, ) from app.utils import APIResponse from app.api.permissions import Permission, require_permission @@ -24,4 +26,40 @@ def create_version_route( current_user: AuthContextDep, session: SessionDep, ): - pass \ No newline at end of file + """ + Create a new version for an existing configuration. + The version number is automatically incremented. + """ + version_crud = ConfigVersionCrud( + session=session, project_id=current_user.project.id, config_id=config_id + ) + version = version_crud.create(config_id=config_id, version_create=version_create) + + return APIResponse.success_response( + data=ConfigVersionPublic(**version.model_dump()), + ) + + +@router.delete( + "/{config_id}/versions/{version_id}", + response_model=APIResponse[Message], + status_code=200, + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def delete_version_route( + config_id: UUID, + version_id: UUID, + current_user: AuthContextDep, + session: SessionDep, +): + """ + Delete a specific version of a config. + """ + version_crud = ConfigVersionCrud( + session=session, project_id=current_user.project.id, config_id=config_id + ) + version_crud.delete(version_id=version_id) + + return APIResponse.success_response( + data=Message(message="Config Version deleted successfully"), + ) diff --git a/backend/app/crud/config/__init__.py b/backend/app/crud/config/__init__.py index 6545f2fb6..490a89d1d 100644 --- a/backend/app/crud/config/__init__.py +++ b/backend/app/crud/config/__init__.py @@ -1,3 +1,4 @@ from app.crud.config.config import ConfigCrud +from app.crud.config.version import ConfigVersionCrud __all__ = ["ConfigCrud", "ConfigVersionCrud"] diff --git a/backend/app/crud/config/config.py b/backend/app/crud/config/config.py index 09f8552c4..e31cf9916 100644 --- a/backend/app/crud/config/config.py +++ b/backend/app/crud/config/config.py @@ -75,9 +75,51 @@ def create(self, config_create: ConfigCreate) -> Tuple[Config, ConfigVersion]: exc_info=True, ) raise HTTPException( - status_code=500, detail=f"Unexpected error occurred: failed to create config" + status_code=500, + detail=f"Unexpected error occurred: failed to create config", ) + def read_one(self, config_id: UUID) -> Config | None: + statement = select(Config).where( + and_( + Config.id == config_id, + Config.project_id == self.project_id, + Config.deleted_at.is_(None), + ) + ) + return self.session.exec(statement).one_or_none() + + def read_all(self, skip: int = 0, limit: int = 100) -> list[Config]: + statement = ( + select(Config) + .where( + and_( + Config.project_id == self.project_id, + Config.deleted_at.is_(None), + ) + ) + .offset(skip) + .limit(limit) + ) + return self.session.exec(statement).all() + + def delete(self, config_id: UUID) -> None: + config = self.read_one(config_id) + if config is None: + raise HTTPException( + status_code=404, + detail=f"config with id '{config_id}' not found", + ) + + config.deleted_at = now() + self.session.add(config) + self.session.commit() + self.session.refresh(config) + + def exists(self, config_id: UUID) -> bool: + config = self.read_one(config_id) + return config is not None + def _get_by_name(self, name: str) -> Config | None: statement = select(Config).where( and_( diff --git a/backend/app/crud/config/version.py b/backend/app/crud/config/version.py new file mode 100644 index 000000000..41085fd76 --- /dev/null +++ b/backend/app/crud/config/version.py @@ -0,0 +1,118 @@ +import logging +from uuid import UUID + +from sqlmodel import Session, select, and_, func +from fastapi import HTTPException + +from .config import ConfigCrud +from app.core.util import now +from app.models import Config, ConfigVersion, ConfigVersionCreate + +logger = logging.getLogger(__name__) + + +class ConfigVersionCrud: + """ + CRUD operations for configuration versions scoped to a project. + """ + + def __init__(self, session: Session, config_id: UUID, project_id: int): + self.session = session + self.project_id = project_id + self.config_id = config_id + + def create( + self, version_create: ConfigVersionCreate + ) -> ConfigVersion: + """ + Create a new version for an existing configuration. + Automatically increments the version number. + """ + try: + self._config_exists(self.config_id) + next_version = self._get_next_version(self.config_id) + + # Create the new version + version = ConfigVersion( + config_id=self.config_id, + version=next_version, + config_json=version_create.config_json, + commit_message=version_create.commit_message, + ) + + self.session.add(version) + self.session.commit() + self.session.refresh(version) + + logger.info( + f"[ConfigVersionCrud.create] Version created successfully | " + f"{{'config_id': '{self.config_id}', 'version_id': '{version.id}'}}" + ) + + return version + + except Exception as e: + self.session.rollback() + logger.error( + f"[ConfigVersionCrud.create] Failed to create version | " + f"{{'config_id': '{self.config_id}', 'error': '{str(e)}'}}", + exc_info=True, + ) + raise HTTPException( + status_code=500, + detail="Unexpected error occurred: failed to create version", + ) + + + def read_one(self, version_id: UUID) -> ConfigVersion | None: + """ + Read a specific configuration version by its ID. + """ + self._config_exists(self.config_id) + statement = select(ConfigVersion).where( + and_( + ConfigVersion.id == version_id, + ConfigVersion.config_id == self.config_id, + ConfigVersion.deleted_at.is_(None), + ) + ) + return self.session.exec(statement).one_or_none() + + + def delete(self, version_id: UUID) -> None: + """ + Soft delete a configuration version by setting its deleted_at timestamp. + """ + version = self.read_one(version_id) + if version is None: + raise HTTPException( + status_code=404, + detail=f"Version with id '{version_id}' not found'", + ) + + version.deleted_at = now() + self.session.add(version) + self.session.commit() + self.session.refresh(version) + + + def _get_next_version(self, config_id: UUID) -> int | None: + """Get the next version number for a config.""" + statement = select(func.max(ConfigVersion.version)).where( + and_( + ConfigVersion.config_id == config_id, + ) + ) + return self.session.exec(statement).one() + 1 + + def _config_exists(self, config_id: UUID) -> Config: + """Check if a config exists in the project.""" + config_crud = ConfigCrud(session=self.session, project_id=self.project_id) + config = config_crud.read_one(config_id) + + if config is None: + raise HTTPException( + status_code=404, + detail="Config with id '{config_id}' not found in this project" + ) + return config diff --git a/backend/app/models/config/config.py b/backend/app/models/config/config.py index ae3afea68..7d36143bd 100644 --- a/backend/app/models/config/config.py +++ b/backend/app/models/config/config.py @@ -67,3 +67,7 @@ class ConfigPublic(ConfigBase): class ConfigWithVersion(ConfigPublic): version: ConfigVersionPublic + + +class ConfigWithVersions(ConfigPublic): + versions: list[ConfigVersionPublic] diff --git a/backend/app/models/config/version.py b/backend/app/models/config/version.py index be1e6e8d7..b2586b8b0 100644 --- a/backend/app/models/config/version.py +++ b/backend/app/models/config/version.py @@ -38,7 +38,9 @@ class ConfigVersion(ConfigVersionBase, table=True): nullable=False, ondelete="CASCADE", ) - version: int = Field(nullable=False, description="Version number starting at 1") + version: int = Field( + nullable=False, description="Version number starting at 1", ge=1 + ) inserted_at: datetime = Field(default_factory=now, nullable=False) updated_at: datetime = Field(default_factory=now, nullable=False) @@ -46,7 +48,7 @@ class ConfigVersion(ConfigVersionBase, table=True): deleted_at: datetime | None = Field(default=None, nullable=True) -class ConfigVersionCreate(SQLModel): +class ConfigVersionCreate(ConfigVersionBase): config_json: dict[str, Any] commit_message: str | None = Field( default=None, From c8c36653ad077e027d16f2768f3ec986b95ec4b5 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 5 Nov 2025 19:24:42 +0530 Subject: [PATCH 04/20] Add a list versions endpoint --- backend/app/api/routes/config/__init__.py | 2 +- backend/app/api/routes/config/config.py | 22 ---------------- backend/app/api/routes/config/version.py | 31 ++++++++++++++++++++++- backend/app/crud/config/version.py | 31 ++++++++++++++++------- 4 files changed, 53 insertions(+), 33 deletions(-) diff --git a/backend/app/api/routes/config/__init__.py b/backend/app/api/routes/config/__init__.py index 4816a8a40..39fbf9203 100644 --- a/backend/app/api/routes/config/__init__.py +++ b/backend/app/api/routes/config/__init__.py @@ -2,7 +2,7 @@ from app.api.routes.config import config, version -router = APIRouter(prefix="/configs", tags=["config management"]) +router = APIRouter(prefix="/configs", tags=["Config Management"]) router.include_router(config.router) router.include_router(version.router) diff --git a/backend/app/api/routes/config/config.py b/backend/app/api/routes/config/config.py index afe0a4903..24c18da97 100644 --- a/backend/app/api/routes/config/config.py +++ b/backend/app/api/routes/config/config.py @@ -66,28 +66,6 @@ def list_configs_route( ) -@router.get( - "/{config_id}", - response_model=APIResponse[ConfigWithVersion], - status_code=200, - dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], -) -def get_config_route( - config_id: UUID, - current_user: AuthContextDep, - session: SessionDep, -): - """ - Get a specific configuration by ID. - """ - # Decide how to handle fetching versions - # should we fetch all versions at /{config_id}/versions and donot include here? - config_crud = ConfigCrud(session=session, project_id=current_user.project.id) - config = config_crud.read_one(config_id=config_id) - - pass - - @router.delete( "/{config_id}", response_model=APIResponse[Message], diff --git a/backend/app/api/routes/config/version.py b/backend/app/api/routes/config/version.py index d4cb49551..4115dda2b 100644 --- a/backend/app/api/routes/config/version.py +++ b/backend/app/api/routes/config/version.py @@ -33,13 +33,42 @@ def create_version_route( version_crud = ConfigVersionCrud( session=session, project_id=current_user.project.id, config_id=config_id ) - version = version_crud.create(config_id=config_id, version_create=version_create) + version = version_crud.create(version_create=version_create) return APIResponse.success_response( data=ConfigVersionPublic(**version.model_dump()), ) +@router.get( + "/{config_id}/versions", + response_model=APIResponse[list[ConfigVersionPublic]], + status_code=200, + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def list_versions_route( + config_id: UUID, + current_user: AuthContextDep, + session: SessionDep, + skip: int = Query(0, ge=0, description="Number of records to skip"), + limit: int = Query(100, ge=1, le=100, description="Maximum records to return"), +): + """ + List all versions for a specific configuration. + """ + version_crud = ConfigVersionCrud( + session=session, project_id=current_user.project.id, config_id=config_id + ) + versions = version_crud.read_all( + skip=skip, + limit=limit, + ) + + return APIResponse.success_response( + data=versions, + ) + + @router.delete( "/{config_id}/versions/{version_id}", response_model=APIResponse[Message], diff --git a/backend/app/crud/config/version.py b/backend/app/crud/config/version.py index 41085fd76..5500c1390 100644 --- a/backend/app/crud/config/version.py +++ b/backend/app/crud/config/version.py @@ -21,15 +21,13 @@ def __init__(self, session: Session, config_id: UUID, project_id: int): self.project_id = project_id self.config_id = config_id - def create( - self, version_create: ConfigVersionCreate - ) -> ConfigVersion: + def create(self, version_create: ConfigVersionCreate) -> ConfigVersion: """ Create a new version for an existing configuration. Automatically increments the version number. """ + self._config_exists(self.config_id) try: - self._config_exists(self.config_id) next_version = self._get_next_version(self.config_id) # Create the new version @@ -62,7 +60,6 @@ def create( status_code=500, detail="Unexpected error occurred: failed to create version", ) - def read_one(self, version_id: UUID) -> ConfigVersion | None: """ @@ -77,7 +74,24 @@ def read_one(self, version_id: UUID) -> ConfigVersion | None: ) ) return self.session.exec(statement).one_or_none() - + + def read_all(self, skip: int = 0, limit: int = 100) -> list[ConfigVersion]: + """ + Read all versions for a specific configuration with pagination. + """ + self._config_exists(self.config_id) + statement = ( + select(ConfigVersion) + .where( + and_( + ConfigVersion.config_id == self.config_id, + ConfigVersion.deleted_at.is_(None), + ) + ) + .offset(skip) + .limit(limit) + ) + return self.session.exec(statement).all() def delete(self, version_id: UUID) -> None: """ @@ -95,7 +109,6 @@ def delete(self, version_id: UUID) -> None: self.session.commit() self.session.refresh(version) - def _get_next_version(self, config_id: UUID) -> int | None: """Get the next version number for a config.""" statement = select(func.max(ConfigVersion.version)).where( @@ -104,7 +117,7 @@ def _get_next_version(self, config_id: UUID) -> int | None: ) ) return self.session.exec(statement).one() + 1 - + def _config_exists(self, config_id: UUID) -> Config: """Check if a config exists in the project.""" config_crud = ConfigCrud(session=self.session, project_id=self.project_id) @@ -113,6 +126,6 @@ def _config_exists(self, config_id: UUID) -> Config: if config is None: raise HTTPException( status_code=404, - detail="Config with id '{config_id}' not found in this project" + detail=f"Config with id '{config_id}' not found in this project", ) return config From f13ab7218c14cb7d37957a2231dc4cdd526376a9 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 6 Nov 2025 10:22:51 +0530 Subject: [PATCH 05/20] Implement GET and PATCH endpoints for configuration management, including unique name validation and version retrieval. --- backend/app/api/routes/config/config.py | 44 +++++++++++++++++++ backend/app/api/routes/config/version.py | 30 +++++++++++++ backend/app/crud/config/config.py | 55 +++++++++++++++++------- backend/app/crud/config/version.py | 9 +--- backend/app/models/config/config.py | 11 ++++- 5 files changed, 124 insertions(+), 25 deletions(-) diff --git a/backend/app/api/routes/config/config.py b/backend/app/api/routes/config/config.py index 24c18da97..60cf776ff 100644 --- a/backend/app/api/routes/config/config.py +++ b/backend/app/api/routes/config/config.py @@ -66,6 +66,50 @@ def list_configs_route( ) +@router.get( + "/{config_id}", + response_model=APIResponse[ConfigPublic], + status_code=200, + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def get_config_route( + config_id: UUID, + current_user: AuthContextDep, + session: SessionDep, +): + """ + Get a specific configuration by its ID. + """ + config_crud = ConfigCrud(session=session, project_id=current_user.project.id) + config = config_crud.exists(config_id=config_id) + return APIResponse.success_response( + data=config, + ) + + +@router.patch( + "/{config_id}", + response_model=APIResponse[ConfigPublic], + status_code=200, + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def update_config_route( + config_id: UUID, + config_update: ConfigUpdate, + current_user: AuthContextDep, + session: SessionDep, +): + """ + Update a specific configuration. + """ + config_crud = ConfigCrud(session=session, project_id=current_user.project.id) + config = config_crud.update(config_id=config_id, config_update=config_update) + + return APIResponse.success_response( + data=config, + ) + + @router.delete( "/{config_id}", response_model=APIResponse[Message], diff --git a/backend/app/api/routes/config/version.py b/backend/app/api/routes/config/version.py index 4115dda2b..1ff7ab92b 100644 --- a/backend/app/api/routes/config/version.py +++ b/backend/app/api/routes/config/version.py @@ -69,6 +69,36 @@ def list_versions_route( ) +@router.get( + "/{config_id}/versions/{version_id}", + response_model=APIResponse[ConfigVersionPublic], + status_code=200, + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def get_version_route( + config_id: UUID, + version_id: UUID, + current_user: AuthContextDep, + session: SessionDep, +): + """ + Get a specific version of a config. + """ + version_crud = ConfigVersionCrud( + session=session, project_id=current_user.project.id, config_id=config_id + ) + version = version_crud.read_one(version_id=version_id) + if version is None: + raise HTTPException( + status_code=404, + detail=f"Config Version with id '{version_id}' not found for config '{config_id}'", + ) + + return APIResponse.success_response( + data=version, + ) + + @router.delete( "/{config_id}/versions/{version_id}", response_model=APIResponse[Message], diff --git a/backend/app/crud/config/config.py b/backend/app/crud/config/config.py index e31cf9916..3cb9a6567 100644 --- a/backend/app/crud/config/config.py +++ b/backend/app/crud/config/config.py @@ -11,7 +11,6 @@ ConfigUpdate, ConfigVersion, ) -from app.crud.project import get_project_by_id from app.core.util import now logger = logging.getLogger(__name__) @@ -30,12 +29,7 @@ def create(self, config_create: ConfigCreate) -> Tuple[Config, ConfigVersion]: """ Create a new configuration with an initial version. """ - existing = self._get_by_name(config_create.name) - if existing: - raise HTTPException( - status_code=409, - detail=f"Configuration with name '{config_create.name}' already exists in this project", - ) + self._check_unique_name(config_create.name) try: config = Config( @@ -103,7 +97,38 @@ def read_all(self, skip: int = 0, limit: int = 100) -> list[Config]: ) return self.session.exec(statement).all() + def update(self, config_id: UUID, config_update: ConfigUpdate) -> Config: + config = self.exists(config_id) + + config_update = config_update.model_dump(exclude_none=True) + + if config_update.get("name"): + self._check_unique_name(config_update["name"]) + + for key, value in config_update.items(): + setattr(config, key, value) + + config.updated_at = now() + + self.session.add(config) + self.session.commit() + self.session.refresh(config) + + logger.info( + f"[ConfigCrud.update] Config updated successfully | " + f"{{'config_id': '{config.id}', 'project_id': {self.project_id}}}" + ) + return config + def delete(self, config_id: UUID) -> None: + config = self.exists(config_id) + + config.deleted_at = now() + self.session.add(config) + self.session.commit() + self.session.refresh(config) + + def exists(self, config_id: UUID) -> Config: config = self.read_one(config_id) if config is None: raise HTTPException( @@ -111,16 +136,16 @@ def delete(self, config_id: UUID) -> None: detail=f"config with id '{config_id}' not found", ) - config.deleted_at = now() - self.session.add(config) - self.session.commit() - self.session.refresh(config) + return config - def exists(self, config_id: UUID) -> bool: - config = self.read_one(config_id) - return config is not None + def _check_unique_name(self, name: str) -> None: + if self._read_by_name(name): + raise HTTPException( + status_code=409, + detail=f"Config with name '{name}' already exists in this project", + ) - def _get_by_name(self, name: str) -> Config | None: + def _read_by_name(self, name: str) -> Config | None: statement = select(Config).where( and_( Config.name == name, diff --git a/backend/app/crud/config/version.py b/backend/app/crud/config/version.py index 5500c1390..0f0fa6abc 100644 --- a/backend/app/crud/config/version.py +++ b/backend/app/crud/config/version.py @@ -121,11 +121,4 @@ def _get_next_version(self, config_id: UUID) -> int | None: def _config_exists(self, config_id: UUID) -> Config: """Check if a config exists in the project.""" config_crud = ConfigCrud(session=self.session, project_id=self.project_id) - config = config_crud.read_one(config_id) - - if config is None: - raise HTTPException( - status_code=404, - detail=f"Config with id '{config_id}' not found in this project", - ) - return config + config_crud.exists(config_id) diff --git a/backend/app/models/config/config.py b/backend/app/models/config/config.py index 7d36143bd..1a006d25b 100644 --- a/backend/app/models/config/config.py +++ b/backend/app/models/config/config.py @@ -2,7 +2,7 @@ from datetime import datetime from typing import TYPE_CHECKING, Any -from sqlmodel import Field, SQLModel +from sqlmodel import Field, SQLModel, UniqueConstraint from app.core.util import now from .version import ConfigVersionPublic @@ -23,6 +23,13 @@ class Config(ConfigBase, table=True): """Database model for LLM configuration storage""" __tablename__ = "config" + __table_args__ = ( + UniqueConstraint( + "project_id", + "name", + "deleted_at", + ), + ) id: UUID = Field(default_factory=uuid4, primary_key=True) @@ -52,7 +59,7 @@ class ConfigCreate(ConfigBase): class ConfigUpdate(SQLModel): - name: str | None = Field(default=None, max_length=128) + name: str | None = Field(default=None, min_length=1, max_length=128) description: str | None = Field( default=None, max_length=512, description="Optional description" ) From 33961b0902d4152b0dc04e71340aa9755cdbb7b4 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 6 Nov 2025 13:22:29 +0530 Subject: [PATCH 06/20] Refactor config management to replace 'config_json' with 'config_blob' across models and CRUD operations, and update version retrieval to use version numbers instead of IDs. --- .../e51b082aa9b3_config_management_tables.py | 2 +- backend/app/api/routes/config/version.py | 23 +++++++----- backend/app/crud/config/config.py | 2 +- backend/app/crud/config/version.py | 36 +++++++++++-------- backend/app/models/__init__.py | 1 + backend/app/models/config/__init__.py | 1 + backend/app/models/config/config.py | 9 ++++- backend/app/models/config/version.py | 31 ++++++++++++---- 8 files changed, 71 insertions(+), 34 deletions(-) diff --git a/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py b/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py index eaa503eda..4089139f9 100644 --- a/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py +++ b/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py @@ -40,7 +40,7 @@ def upgrade(): op.create_table( "config_version", sa.Column( - "config_json", postgresql.JSON(astext_type=sa.Text()), nullable=False + "config_blob", postgresql.JSON(astext_type=sa.Text()), nullable=False ), sa.Column( "commit_message", diff --git a/backend/app/api/routes/config/version.py b/backend/app/api/routes/config/version.py index 1ff7ab92b..f36f20a1a 100644 --- a/backend/app/api/routes/config/version.py +++ b/backend/app/api/routes/config/version.py @@ -1,5 +1,5 @@ from uuid import UUID -from fastapi import APIRouter, Depends, Query, HTTPException +from fastapi import APIRouter, Depends, Query, HTTPException, Path from app.api.deps import SessionDep, AuthContextDep from app.crud.config import ConfigCrud, ConfigVersionCrud @@ -7,6 +7,7 @@ ConfigVersionCreate, ConfigVersionPublic, Message, + ConfigVersionItems, ) from app.utils import APIResponse from app.api.permissions import Permission, require_permission @@ -42,7 +43,7 @@ def create_version_route( @router.get( "/{config_id}/versions", - response_model=APIResponse[list[ConfigVersionPublic]], + response_model=APIResponse[list[ConfigVersionItems]], status_code=200, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) @@ -70,16 +71,18 @@ def list_versions_route( @router.get( - "/{config_id}/versions/{version_id}", + "/{config_id}/versions/{version_number}", response_model=APIResponse[ConfigVersionPublic], status_code=200, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) def get_version_route( config_id: UUID, - version_id: UUID, current_user: AuthContextDep, session: SessionDep, + version_number: int = Path( + ..., ge=1, description="The version number of the config" + ), ): """ Get a specific version of a config. @@ -87,11 +90,11 @@ def get_version_route( version_crud = ConfigVersionCrud( session=session, project_id=current_user.project.id, config_id=config_id ) - version = version_crud.read_one(version_id=version_id) + version = version_crud.read_one(version_number=version_number) if version is None: raise HTTPException( status_code=404, - detail=f"Config Version with id '{version_id}' not found for config '{config_id}'", + detail=f"Config Version with number '{version_number}' not found for config '{config_id}'", ) return APIResponse.success_response( @@ -100,16 +103,18 @@ def get_version_route( @router.delete( - "/{config_id}/versions/{version_id}", + "/{config_id}/versions/{version_number}", response_model=APIResponse[Message], status_code=200, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) def delete_version_route( config_id: UUID, - version_id: UUID, current_user: AuthContextDep, session: SessionDep, + version_number: int = Path( + ..., ge=1, description="The version number of the config" + ), ): """ Delete a specific version of a config. @@ -117,7 +122,7 @@ def delete_version_route( version_crud = ConfigVersionCrud( session=session, project_id=current_user.project.id, config_id=config_id ) - version_crud.delete(version_id=version_id) + version_crud.delete(version_number=version_number) return APIResponse.success_response( data=Message(message="Config Version deleted successfully"), diff --git a/backend/app/crud/config/config.py b/backend/app/crud/config/config.py index 3cb9a6567..eeadf25a8 100644 --- a/backend/app/crud/config/config.py +++ b/backend/app/crud/config/config.py @@ -45,7 +45,7 @@ def create(self, config_create: ConfigCreate) -> Tuple[Config, ConfigVersion]: version = ConfigVersion( config_id=config.id, version=1, - config_json=config_create.config_json, + config_blob=config_create.config_blob, commit_message=config_create.commit_message, ) diff --git a/backend/app/crud/config/version.py b/backend/app/crud/config/version.py index 0f0fa6abc..a6725964d 100644 --- a/backend/app/crud/config/version.py +++ b/backend/app/crud/config/version.py @@ -6,7 +6,7 @@ from .config import ConfigCrud from app.core.util import now -from app.models import Config, ConfigVersion, ConfigVersionCreate +from app.models import Config, ConfigVersion, ConfigVersionCreate, ConfigVersionItems logger = logging.getLogger(__name__) @@ -30,11 +30,10 @@ def create(self, version_create: ConfigVersionCreate) -> ConfigVersion: try: next_version = self._get_next_version(self.config_id) - # Create the new version version = ConfigVersion( config_id=self.config_id, version=next_version, - config_json=version_create.config_json, + config_blob=version_create.config_blob, commit_message=version_create.commit_message, ) @@ -61,27 +60,27 @@ def create(self, version_create: ConfigVersionCreate) -> ConfigVersion: detail="Unexpected error occurred: failed to create version", ) - def read_one(self, version_id: UUID) -> ConfigVersion | None: + def read_one(self, version_number: int) -> ConfigVersion | None: """ - Read a specific configuration version by its ID. + Read a specific configuration version by its version number. """ self._config_exists(self.config_id) statement = select(ConfigVersion).where( and_( - ConfigVersion.id == version_id, + ConfigVersion.version == version_number, ConfigVersion.config_id == self.config_id, ConfigVersion.deleted_at.is_(None), ) ) return self.session.exec(statement).one_or_none() - def read_all(self, skip: int = 0, limit: int = 100) -> list[ConfigVersion]: + def read_all(self, skip: int = 0, limit: int = 100) -> list[ConfigVersionItems]: """ Read all versions for a specific configuration with pagination. """ self._config_exists(self.config_id) statement = ( - select(ConfigVersion) + select(ConfigVersionItems) .where( and_( ConfigVersion.config_id == self.config_id, @@ -93,22 +92,29 @@ def read_all(self, skip: int = 0, limit: int = 100) -> list[ConfigVersion]: ) return self.session.exec(statement).all() - def delete(self, version_id: UUID) -> None: + def delete(self, version_number: int) -> None: """ Soft delete a configuration version by setting its deleted_at timestamp. """ - version = self.read_one(version_id) - if version is None: - raise HTTPException( - status_code=404, - detail=f"Version with id '{version_id}' not found'", - ) + version = self.exists(version_number) version.deleted_at = now() self.session.add(version) self.session.commit() self.session.refresh(version) + def exists(self, version_number: int) -> ConfigVersion: + """ + Check if a configuration version exists; raise 404 if not found. + """ + version = self.read_one(version_number=version_number) + if version is None: + raise HTTPException( + status_code=404, + detail=f"Version with number '{version_number}' not found for config '{self.config_id}'", + ) + return version + def _get_next_version(self, config_id: UUID) -> int | None: """Get the next version number for a config.""" statement = select(func.max(ConfigVersion.version)).where( diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index ba16741da..06081464a 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -32,6 +32,7 @@ ConfigVersionBase, ConfigVersionCreate, ConfigVersionPublic, + ConfigVersionItems, ) from .credentials import ( Credential, diff --git a/backend/app/models/config/__init__.py b/backend/app/models/config/__init__.py index b88c4b91c..e71b95759 100644 --- a/backend/app/models/config/__init__.py +++ b/backend/app/models/config/__init__.py @@ -11,6 +11,7 @@ ConfigVersionBase, ConfigVersionCreate, ConfigVersionPublic, + ConfigVersionItems, ) __all__ = [ diff --git a/backend/app/models/config/config.py b/backend/app/models/config/config.py index 1a006d25b..2e972e84b 100644 --- a/backend/app/models/config/config.py +++ b/backend/app/models/config/config.py @@ -3,6 +3,7 @@ from typing import TYPE_CHECKING, Any from sqlmodel import Field, SQLModel, UniqueConstraint +from pydantic import field_validator from app.core.util import now from .version import ConfigVersionPublic @@ -50,13 +51,19 @@ class ConfigCreate(ConfigBase): """Create new configuration""" # Initial version data - config_json: dict[str, Any] = Field(description="Provider-specific parameters") + config_blob: dict[str, Any] = Field(description="Provider-specific parameters") commit_message: str | None = Field( default=None, max_length=512, description="Optional message describing the changes in this version", ) + @field_validator("config_blob") + def validate_blob_not_empty(cls, value): + if not value: + raise ValueError("config_blob cannot be empty") + return value + class ConfigUpdate(SQLModel): name: str | None = Field(default=None, min_length=1, max_length=128) diff --git a/backend/app/models/config/version.py b/backend/app/models/config/version.py index b2586b8b0..6a1bdc2ed 100644 --- a/backend/app/models/config/version.py +++ b/backend/app/models/config/version.py @@ -2,6 +2,7 @@ from uuid import UUID, uuid4 from typing import Any +from pydantic import field_validator import sqlalchemy as sa from sqlalchemy.dialects.postgresql import JSON from sqlmodel import Field, SQLModel, UniqueConstraint @@ -10,7 +11,7 @@ class ConfigVersionBase(SQLModel): - config_json: dict[str, Any] = Field( + config_blob: dict[str, Any] = Field( sa_column=sa.Column(JSON, nullable=False), description="Provider-specific configuration parameters (temperature, max_tokens, etc.)", ) @@ -20,6 +21,12 @@ class ConfigVersionBase(SQLModel): description="Optional message describing the changes in this version", ) + @field_validator("config_blob") + def validate_blob_not_empty(cls, value): + if not value: + raise ValueError("config_blob cannot be empty") + return value + class ConfigVersion(ConfigVersionBase, table=True): __tablename__ = "config_version" @@ -49,12 +56,7 @@ class ConfigVersion(ConfigVersionBase, table=True): class ConfigVersionCreate(ConfigVersionBase): - config_json: dict[str, Any] - commit_message: str | None = Field( - default=None, - max_length=512, - description="Optional message describing the changes in this version", - ) + pass class ConfigVersionPublic(ConfigVersionBase): @@ -63,3 +65,18 @@ class ConfigVersionPublic(ConfigVersionBase): version: int = Field(nullable=False, description="Version number starting at 1") inserted_at: datetime updated_at: datetime + + +class ConfigVersionItems(SQLModel): + """Lightweight version for lists (without large config_blob)""" + + id: UUID = Field(description="Unique id for the configuration version") + version: int = Field(nullable=False, description="Version number starting at 1") + config_id: UUID = Field(description="Id of the parent configuration") + commit_message: str | None = Field( + default=None, + max_length=512, + description="Optional message describing the changes in this version", + ) + inserted_at: datetime + updated_at: datetime From 52a6c8649c8fda2da2f3d4d345e830f706b1bb77 Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Fri, 7 Nov 2025 13:50:28 +0530 Subject: [PATCH 07/20] Enhance configuration and version listing by adding ordering: - List configurations ordered by updated_at in descending order. - List versions ordered by version number in descending order. --- backend/app/api/routes/config/config.py | 1 + backend/app/api/routes/config/version.py | 2 +- backend/app/crud/config/config.py | 1 + backend/app/crud/config/version.py | 11 +++++++++-- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/backend/app/api/routes/config/config.py b/backend/app/api/routes/config/config.py index 60cf776ff..4cbe43e67 100644 --- a/backend/app/api/routes/config/config.py +++ b/backend/app/api/routes/config/config.py @@ -56,6 +56,7 @@ def list_configs_route( ): """ List all configurations for the current project. + Ordered by updated_at in descending order. """ # Decide how to handle pagination effectively diff --git a/backend/app/api/routes/config/version.py b/backend/app/api/routes/config/version.py index f36f20a1a..4a3a703d6 100644 --- a/backend/app/api/routes/config/version.py +++ b/backend/app/api/routes/config/version.py @@ -56,6 +56,7 @@ def list_versions_route( ): """ List all versions for a specific configuration. + Ordered by version number in descending order. """ version_crud = ConfigVersionCrud( session=session, project_id=current_user.project.id, config_id=config_id @@ -64,7 +65,6 @@ def list_versions_route( skip=skip, limit=limit, ) - return APIResponse.success_response( data=versions, ) diff --git a/backend/app/crud/config/config.py b/backend/app/crud/config/config.py index eeadf25a8..214951c9e 100644 --- a/backend/app/crud/config/config.py +++ b/backend/app/crud/config/config.py @@ -94,6 +94,7 @@ def read_all(self, skip: int = 0, limit: int = 100) -> list[Config]: ) .offset(skip) .limit(limit) + .order_by(Config.updated_at.desc()) ) return self.session.exec(statement).all() diff --git a/backend/app/crud/config/version.py b/backend/app/crud/config/version.py index a6725964d..6f35d6d00 100644 --- a/backend/app/crud/config/version.py +++ b/backend/app/crud/config/version.py @@ -3,6 +3,7 @@ from sqlmodel import Session, select, and_, func from fastapi import HTTPException +from sqlalchemy.orm import defer from .config import ConfigCrud from app.core.util import now @@ -79,18 +80,24 @@ def read_all(self, skip: int = 0, limit: int = 100) -> list[ConfigVersionItems]: Read all versions for a specific configuration with pagination. """ self._config_exists(self.config_id) + statement = ( - select(ConfigVersionItems) + select(ConfigVersion) .where( and_( ConfigVersion.config_id == self.config_id, ConfigVersion.deleted_at.is_(None), ) ) + .options( + defer(ConfigVersion.config_blob), + ) .offset(skip) .limit(limit) + .order_by(ConfigVersion.version.desc()) ) - return self.session.exec(statement).all() + results = self.session.exec(statement).all() + return [ConfigVersionItems.model_validate(item) for item in results] def delete(self, version_number: int) -> None: """ From 1b510fdc7dc9bab0948bb3abeca7ca3a5eb9bcbf Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 12 Nov 2025 11:51:15 +0530 Subject: [PATCH 08/20] test for config routes --- .../tests/api/routes/configs/test_config.py | 466 ++++++++++++++++++ backend/app/tests/utils/test_data.py | 42 ++ 2 files changed, 508 insertions(+) create mode 100644 backend/app/tests/api/routes/configs/test_config.py diff --git a/backend/app/tests/api/routes/configs/test_config.py b/backend/app/tests/api/routes/configs/test_config.py new file mode 100644 index 000000000..631f746e3 --- /dev/null +++ b/backend/app/tests/api/routes/configs/test_config.py @@ -0,0 +1,466 @@ +from uuid import uuid4 + +from fastapi.testclient import TestClient +from sqlmodel import Session + +from app.core.config import settings +from app.tests.utils.auth import TestAuthContext +from app.tests.utils.test_data import create_test_config, create_test_project + + +def test_create_config_success( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test creating a config successfully with API key authentication.""" + config_data = { + "name": "test-llm-config", + "description": "A test LLM configuration", + "config_blob": { + "model": "gpt-4", + "temperature": 0.8, + "max_tokens": 2000, + }, + "commit_message": "Initial configuration", + } + + response = client.post( + f"{settings.API_V1_STR}/configs/", + headers={"X-API-KEY": user_api_key.key}, + json=config_data, + ) + assert response.status_code == 201 + data = response.json() + assert data["success"] is True + assert "data" in data + assert data["data"]["name"] == config_data["name"] + assert data["data"]["description"] == config_data["description"] + assert data["data"]["project_id"] == user_api_key.project_id + assert "id" in data["data"] + assert "version" in data["data"] + assert data["data"]["version"]["version"] == 1 + assert data["data"]["version"]["config_blob"] == config_data["config_blob"] + + +def test_create_config_empty_blob_fails( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that creating a config with empty config_blob fails validation.""" + config_data = { + "name": "test-config", + "description": "Test", + "config_blob": {}, + "commit_message": "Initial", + } + + response = client.post( + f"{settings.API_V1_STR}/configs/", + headers={"X-API-KEY": user_api_key.key}, + json=config_data, + ) + assert response.status_code == 422 + + +def test_create_config_duplicate_name_fails( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that creating a config with duplicate name in same project fails.""" + # Create first config + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="duplicate-config", + ) + + # Try to create another with same name + config_data = { + "name": "duplicate-config", + "description": "Should fail", + "config_blob": {"model": "gpt-4"}, + "commit_message": "Initial", + } + + response = client.post( + f"{settings.API_V1_STR}/configs/", + headers={"X-API-KEY": user_api_key.key}, + json=config_data, + ) + assert response.status_code == 409 + response_data = response.json() + error = response_data.get("error", response_data.get("detail", "")) + assert "already exists" in error.lower() + + +def test_list_configs( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test listing configs for a project.""" + created_configs = [] + for i in range(3): + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name=f"list-test-config-{i}", + ) + created_configs.append(config) + + response = client.get( + f"{settings.API_V1_STR}/configs/", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert isinstance(data["data"], list) + assert len(data["data"]) >= 3 + + config_names = [c["name"] for c in data["data"]] + for config in created_configs: + assert config.name in config_names + + +def test_list_configs_with_pagination( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test listing configs with pagination parameters.""" + for i in range(5): + create_test_config( + db=db, + project_id=user_api_key.project_id, + name=f"pagination-test-{i}", + ) + + # Test with limit + response = client.get( + f"{settings.API_V1_STR}/configs/", + headers={"X-API-KEY": user_api_key.key}, + params={"skip": 0, "limit": 2}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert len(data["data"]) == 2 + + # Test with skip + response = client.get( + f"{settings.API_V1_STR}/configs/", + headers={"X-API-KEY": user_api_key.key}, + params={"skip": 2, "limit": 2}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert len(data["data"]) >= 2 + + +def test_get_config_by_id( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test retrieving a specific config by ID.""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="get-by-id-test", + description="Test config for retrieval", + ) + + response = client.get( + f"{settings.API_V1_STR}/configs/{config.id}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert data["data"]["id"] == str(config.id) + assert data["data"]["name"] == config.name + assert data["data"]["description"] == config.description + assert data["data"]["project_id"] == user_api_key.project_id + + +def test_get_config_nonexistent( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test retrieving a non-existent config returns 404.""" + fake_uuid = uuid4() + response = client.get( + f"{settings.API_V1_STR}/configs/{fake_uuid}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 404 + + +def test_get_config_from_different_project_fails( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that users cannot access configs from other projects.""" + # Create config in different project + other_project = create_test_project(db) + config = create_test_config( + db=db, + project_id=other_project.id, + name="other-project-config", + ) + + # Try to access it with user_api_key from different project + response = client.get( + f"{settings.API_V1_STR}/configs/{config.id}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 404 + + +def test_update_config_name( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test updating a config's name.""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="original-name", + ) + + update_data = { + "name": "updated-name", + } + + response = client.patch( + f"{settings.API_V1_STR}/configs/{config.id}", + headers={"X-API-KEY": user_api_key.key}, + json=update_data, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert data["data"]["name"] == "updated-name" + assert data["data"]["id"] == str(config.id) + + +def test_update_config_description( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test updating a config's description.""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="test-config", + description="Original description", + ) + + update_data = { + "description": "Updated description", + } + + response = client.patch( + f"{settings.API_V1_STR}/configs/{config.id}", + headers={"X-API-KEY": user_api_key.key}, + json=update_data, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert data["data"]["description"] == "Updated description" + + +def test_update_config_to_duplicate_name_fails( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that updating a config to a duplicate name fails.""" + # Create two configs + config1 = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="config-one", + ) + config2 = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="config-two", + ) + + # Try to update config2 to have the same name as config1 + update_data = { + "name": "config-one", + } + + response = client.patch( + f"{settings.API_V1_STR}/configs/{config2.id}", + headers={"X-API-KEY": user_api_key.key}, + json=update_data, + ) + assert response.status_code == 409 + response_data = response.json() + error = response_data.get("error", response_data.get("detail", "")) + assert "already exists" in error.lower() + + +def test_update_config_nonexistent( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test updating a non-existent config returns 404.""" + fake_uuid = uuid4() + update_data = { + "name": "new-name", + } + + response = client.patch( + f"{settings.API_V1_STR}/configs/{fake_uuid}", + headers={"X-API-KEY": user_api_key.key}, + json=update_data, + ) + assert response.status_code == 404 + + +def test_delete_config( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test deleting a config (soft delete).""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="config-to-delete", + ) + + response = client.delete( + f"{settings.API_V1_STR}/configs/{config.id}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert "deleted successfully" in data["data"]["message"].lower() + + # Verify the config is no longer accessible + get_response = client.get( + f"{settings.API_V1_STR}/configs/{config.id}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert get_response.status_code == 404 + + +def test_delete_config_nonexistent( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test deleting a non-existent config returns 404.""" + fake_uuid = uuid4() + response = client.delete( + f"{settings.API_V1_STR}/configs/{fake_uuid}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 404 + + +def test_delete_config_from_different_project_fails( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that users cannot delete configs from other projects.""" + # Create config in different project + other_project = create_test_project(db) + config = create_test_config( + db=db, + project_id=other_project.id, + name="other-project-config", + ) + + # Try to delete it with user_api_key from different project + response = client.delete( + f"{settings.API_V1_STR}/configs/{config.id}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 404 + + +def test_create_config_requires_authentication( + db: Session, + client: TestClient, +) -> None: + """Test that creating a config without authentication fails.""" + config_data = { + "name": "test-config", + "description": "Test", + "config_blob": {"model": "gpt-4"}, + "commit_message": "Initial", + } + + response = client.post( + f"{settings.API_V1_STR}/configs/", + json=config_data, + ) + assert response.status_code == 401 + + +def test_list_configs_requires_authentication( + db: Session, + client: TestClient, +) -> None: + """Test that listing configs without authentication fails.""" + response = client.get( + f"{settings.API_V1_STR}/configs/", + ) + assert response.status_code == 401 + + +def test_configs_isolated_by_project( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that configs are properly isolated between projects.""" + # Create configs in user's project + user_configs = [] + for i in range(2): + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name=f"user-config-{i}", + ) + user_configs.append(config) + + # Create configs in different project + other_project = create_test_project(db) + for i in range(3): + create_test_config( + db=db, + project_id=other_project.id, + name=f"other-config-{i}", + ) + + # User should only see their project's configs + response = client.get( + f"{settings.API_V1_STR}/configs/", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 200 + data = response.json() + + # Verify we only get configs from user's project + for config_data in data["data"]: + assert config_data["project_id"] == user_api_key.project_id diff --git a/backend/app/tests/utils/test_data.py b/backend/app/tests/utils/test_data.py index c560bbca3..269aef2fb 100644 --- a/backend/app/tests/utils/test_data.py +++ b/backend/app/tests/utils/test_data.py @@ -14,6 +14,8 @@ ModelEvaluation, ModelEvaluationBase, ModelEvaluationStatus, + Config, + ConfigCreate, ) from app.crud import ( create_organization, @@ -23,6 +25,7 @@ create_model_evaluation, APIKeyCrud, ) +from app.crud.config import ConfigCrud from app.core.providers import Provider from app.tests.utils.user import create_random_user from app.tests.utils.utils import ( @@ -226,3 +229,42 @@ def create_test_model_evaluation(db) -> list[ModelEvaluation]: model_evaluations.append(model_eval) return model_evaluations + + +def create_test_config( + db: Session, + project_id: int | None = None, + name: str | None = None, + description: str | None = None, + config_blob: dict | None = None, +) -> Config: + """ + Creates and returns a test configuration with an initial version. + + Persists the config and version to the database. + """ + if project_id is None: + project = create_test_project(db) + project_id = project.id + + if name is None: + name = f"test-config-{random_lower_string()}" + + if config_blob is None: + config_blob = { + "model": "gpt-4", + "temperature": 0.7, + "max_tokens": 1000, + } + + config_create = ConfigCreate( + name=name, + description=description or "Test configuration description", + config_blob=config_blob, + commit_message="Initial version", + ) + + config_crud = ConfigCrud(session=db, project_id=project_id) + config, version = config_crud.create(config_create) + + return config From 87a3150e5df4c01e10be091390115206625e785e Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 12 Nov 2025 12:02:39 +0530 Subject: [PATCH 09/20] test for routes version --- .../tests/api/routes/configs/test_version.py | 517 ++++++++++++++++++ backend/app/tests/utils/test_data.py | 36 +- 2 files changed, 552 insertions(+), 1 deletion(-) create mode 100644 backend/app/tests/api/routes/configs/test_version.py diff --git a/backend/app/tests/api/routes/configs/test_version.py b/backend/app/tests/api/routes/configs/test_version.py new file mode 100644 index 000000000..d534f67dd --- /dev/null +++ b/backend/app/tests/api/routes/configs/test_version.py @@ -0,0 +1,517 @@ +from uuid import uuid4 + +from fastapi.testclient import TestClient +from sqlmodel import Session + +from app.core.config import settings +from app.tests.utils.auth import TestAuthContext +from app.tests.utils.test_data import ( + create_test_config, + create_test_project, + create_test_version, +) + + +def test_create_version_success( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test creating a new version for a config successfully.""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="test-config", + ) + + version_data = { + "config_blob": { + "model": "gpt-4-turbo", + "temperature": 0.9, + "max_tokens": 3000, + }, + "commit_message": "Updated model to gpt-4-turbo", + } + + response = client.post( + f"{settings.API_V1_STR}/configs/{config.id}/versions", + headers={"X-API-KEY": user_api_key.key}, + json=version_data, + ) + assert response.status_code == 201 + data = response.json() + assert data["success"] is True + assert "data" in data + assert data["data"]["version"] == 2 # First version created with config, this is second + assert data["data"]["config_blob"] == version_data["config_blob"] + assert data["data"]["commit_message"] == version_data["commit_message"] + assert data["data"]["config_id"] == str(config.id) + + +def test_create_version_empty_blob_fails( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that creating a version with empty config_blob fails validation.""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="test-config", + ) + + version_data = { + "config_blob": {}, + "commit_message": "Empty blob", + } + + response = client.post( + f"{settings.API_V1_STR}/configs/{config.id}/versions", + headers={"X-API-KEY": user_api_key.key}, + json=version_data, + ) + assert response.status_code == 422 + + +def test_create_version_nonexistent_config( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test creating a version for a non-existent config returns 404.""" + fake_uuid = uuid4() + version_data = { + "config_blob": {"model": "gpt-4"}, + "commit_message": "Test", + } + + response = client.post( + f"{settings.API_V1_STR}/configs/{fake_uuid}/versions", + headers={"X-API-KEY": user_api_key.key}, + json=version_data, + ) + assert response.status_code == 404 + + +def test_create_version_different_project_fails( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that creating a version for a config in a different project fails.""" + other_project = create_test_project(db) + config = create_test_config( + db=db, + project_id=other_project.id, + name="other-project-config", + ) + + version_data = { + "config_blob": {"model": "gpt-4"}, + "commit_message": "Should fail", + } + + response = client.post( + f"{settings.API_V1_STR}/configs/{config.id}/versions", + headers={"X-API-KEY": user_api_key.key}, + json=version_data, + ) + assert response.status_code == 404 + + +def test_create_version_auto_increments( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that version numbers are automatically incremented.""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="test-config", + ) + + # Create multiple versions and verify they increment + for i in range(2, 5): + version_data = { + "config_blob": {"model": f"gpt-4-version-{i}"}, + "commit_message": f"Version {i}", + } + + response = client.post( + f"{settings.API_V1_STR}/configs/{config.id}/versions", + headers={"X-API-KEY": user_api_key.key}, + json=version_data, + ) + assert response.status_code == 201 + data = response.json() + assert data["data"]["version"] == i + + +def test_list_versions( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test listing all versions for a config.""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="test-config", + ) + + # Create additional versions + for i in range(3): + create_test_version( + db=db, + config_id=config.id, + project_id=user_api_key.project_id, + commit_message=f"Version {i + 2}", + ) + + response = client.get( + f"{settings.API_V1_STR}/configs/{config.id}/versions", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert isinstance(data["data"], list) + assert len(data["data"]) == 4 # 1 initial + 3 created + + # Verify versions are ordered by version number descending + versions = data["data"] + for i in range(len(versions) - 1): + assert versions[i]["version"] > versions[i + 1]["version"] + + +def test_list_versions_with_pagination( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test listing versions with pagination parameters.""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="test-config", + ) + + # Create 5 additional versions (6 total including initial) + for i in range(5): + create_test_version( + db=db, + config_id=config.id, + project_id=user_api_key.project_id, + ) + + # Test with limit + response = client.get( + f"{settings.API_V1_STR}/configs/{config.id}/versions", + headers={"X-API-KEY": user_api_key.key}, + params={"skip": 0, "limit": 3}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert len(data["data"]) == 3 + + # Test with skip + response = client.get( + f"{settings.API_V1_STR}/configs/{config.id}/versions", + headers={"X-API-KEY": user_api_key.key}, + params={"skip": 3, "limit": 3}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert len(data["data"]) == 3 + + +def test_list_versions_nonexistent_config( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test listing versions for a non-existent config returns 404.""" + fake_uuid = uuid4() + + response = client.get( + f"{settings.API_V1_STR}/configs/{fake_uuid}/versions", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 404 + + +def test_list_versions_different_project_fails( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that listing versions for a config in a different project fails.""" + other_project = create_test_project(db) + config = create_test_config( + db=db, + project_id=other_project.id, + name="other-project-config", + ) + + response = client.get( + f"{settings.API_V1_STR}/configs/{config.id}/versions", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 404 + + +def test_get_version_by_number( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test retrieving a specific version by version number.""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="test-config", + ) + + # Create additional version + version = create_test_version( + db=db, + config_id=config.id, + project_id=user_api_key.project_id, + config_blob={"model": "gpt-4-turbo", "temperature": 0.5}, + commit_message="Updated config", + ) + + response = client.get( + f"{settings.API_V1_STR}/configs/{config.id}/versions/{version.version}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert data["data"]["version"] == version.version + assert data["data"]["config_blob"] == version.config_blob + assert data["data"]["commit_message"] == version.commit_message + assert data["data"]["config_id"] == str(config.id) + + +def test_get_version_nonexistent( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test retrieving a non-existent version returns 404.""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="test-config", + ) + + response = client.get( + f"{settings.API_V1_STR}/configs/{config.id}/versions/999", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 404 + + +def test_get_version_from_different_project_fails( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that users cannot access versions from configs in other projects.""" + other_project = create_test_project(db) + config = create_test_config( + db=db, + project_id=other_project.id, + name="other-project-config", + ) + + # The config has version 1 by default + response = client.get( + f"{settings.API_V1_STR}/configs/{config.id}/versions/1", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 404 + + +def test_delete_version( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test deleting a version (soft delete).""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="test-config", + ) + + # Create a version to delete + version = create_test_version( + db=db, + config_id=config.id, + project_id=user_api_key.project_id, + ) + + response = client.delete( + f"{settings.API_V1_STR}/configs/{config.id}/versions/{version.version}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert "deleted successfully" in data["data"]["message"].lower() + + # Verify the version is no longer accessible + get_response = client.get( + f"{settings.API_V1_STR}/configs/{config.id}/versions/{version.version}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert get_response.status_code == 404 + + +def test_delete_version_nonexistent( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test deleting a non-existent version returns 404.""" + config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="test-config", + ) + + response = client.delete( + f"{settings.API_V1_STR}/configs/{config.id}/versions/999", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 404 + + +def test_delete_version_from_different_project_fails( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that users cannot delete versions from configs in other projects.""" + other_project = create_test_project(db) + config = create_test_config( + db=db, + project_id=other_project.id, + name="other-project-config", + ) + + # Try to delete the initial version + response = client.delete( + f"{settings.API_V1_STR}/configs/{config.id}/versions/1", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 404 + + +def test_create_version_requires_authentication( + db: Session, + client: TestClient, +) -> None: + """Test that creating a version without authentication fails.""" + version_data = { + "config_blob": {"model": "gpt-4"}, + "commit_message": "Test", + } + + fake_uuid = uuid4() + response = client.post( + f"{settings.API_V1_STR}/configs/{fake_uuid}/versions", + json=version_data, + ) + assert response.status_code == 401 + + +def test_list_versions_requires_authentication( + db: Session, + client: TestClient, +) -> None: + """Test that listing versions without authentication fails.""" + fake_uuid = uuid4() + response = client.get( + f"{settings.API_V1_STR}/configs/{fake_uuid}/versions", + ) + assert response.status_code == 401 + + +def test_get_version_requires_authentication( + db: Session, + client: TestClient, +) -> None: + """Test that getting a version without authentication fails.""" + fake_uuid = uuid4() + response = client.get( + f"{settings.API_V1_STR}/configs/{fake_uuid}/versions/1", + ) + assert response.status_code == 401 + + +def test_delete_version_requires_authentication( + db: Session, + client: TestClient, +) -> None: + """Test that deleting a version without authentication fails.""" + fake_uuid = uuid4() + response = client.delete( + f"{settings.API_V1_STR}/configs/{fake_uuid}/versions/1", + ) + assert response.status_code == 401 + + +def test_versions_isolated_by_project( + db: Session, + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test that versions are properly isolated between projects.""" + # Create config in user's project with additional versions + user_config = create_test_config( + db=db, + project_id=user_api_key.project_id, + name="user-config", + ) + for i in range(2): + create_test_version( + db=db, + config_id=user_config.id, + project_id=user_api_key.project_id, + ) + + # Create config in different project with versions + other_project = create_test_project(db) + other_config = create_test_config( + db=db, + project_id=other_project.id, + name="other-config", + ) + for i in range(3): + create_test_version( + db=db, + config_id=other_config.id, + project_id=other_project.id, + ) + + # User should only see versions from their project's config + response = client.get( + f"{settings.API_V1_STR}/configs/{user_config.id}/versions", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 200 + data = response.json() + assert len(data["data"]) == 3 # 1 initial + 2 created + + # User should NOT be able to access other project's versions + response = client.get( + f"{settings.API_V1_STR}/configs/{other_config.id}/versions", + headers={"X-API-KEY": user_api_key.key}, + ) + assert response.status_code == 404 diff --git a/backend/app/tests/utils/test_data.py b/backend/app/tests/utils/test_data.py index 269aef2fb..c663cf014 100644 --- a/backend/app/tests/utils/test_data.py +++ b/backend/app/tests/utils/test_data.py @@ -16,6 +16,8 @@ ModelEvaluationStatus, Config, ConfigCreate, + ConfigVersion, + ConfigVersionCreate, ) from app.crud import ( create_organization, @@ -25,7 +27,7 @@ create_model_evaluation, APIKeyCrud, ) -from app.crud.config import ConfigCrud +from app.crud.config import ConfigCrud, ConfigVersionCrud from app.core.providers import Provider from app.tests.utils.user import create_random_user from app.tests.utils.utils import ( @@ -268,3 +270,35 @@ def create_test_config( config, version = config_crud.create(config_create) return config + + +def create_test_version( + db: Session, + config_id, + project_id: int, + config_blob: dict | None = None, + commit_message: str | None = None, +) -> ConfigVersion: + """ + Creates and returns a test version for an existing configuration. + + Persists the version to the database. + """ + if config_blob is None: + config_blob = { + "model": "gpt-4", + "temperature": 0.8, + "max_tokens": 1500, + } + + version_create = ConfigVersionCreate( + config_blob=config_blob, + commit_message=commit_message or "Test version commit", + ) + + version_crud = ConfigVersionCrud( + session=db, project_id=project_id, config_id=config_id + ) + version = version_crud.create(version_create=version_create) + + return version From 409576ad892e864d3465895ee10248d500bcdaae Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 12 Nov 2025 12:58:12 +0530 Subject: [PATCH 10/20] Add tests for configuration and version management, including creation, reading, and deletion scenarios --- backend/app/crud/config/config.py | 2 +- backend/app/tests/crud/config/test_config.py | 482 ++++++++++++++++++ backend/app/tests/crud/config/test_version.py | 420 +++++++++++++++ 3 files changed, 903 insertions(+), 1 deletion(-) create mode 100644 backend/app/tests/crud/config/test_config.py create mode 100644 backend/app/tests/crud/config/test_version.py diff --git a/backend/app/crud/config/config.py b/backend/app/crud/config/config.py index 214951c9e..03016987e 100644 --- a/backend/app/crud/config/config.py +++ b/backend/app/crud/config/config.py @@ -103,7 +103,7 @@ def update(self, config_id: UUID, config_update: ConfigUpdate) -> Config: config_update = config_update.model_dump(exclude_none=True) - if config_update.get("name"): + if config_update.get("name") and config_update["name"] != config.name: self._check_unique_name(config_update["name"]) for key, value in config_update.items(): diff --git a/backend/app/tests/crud/config/test_config.py b/backend/app/tests/crud/config/test_config.py new file mode 100644 index 000000000..a354d3484 --- /dev/null +++ b/backend/app/tests/crud/config/test_config.py @@ -0,0 +1,482 @@ +import pytest +from uuid import uuid4 +from sqlmodel import Session +from fastapi import HTTPException + +from app.models import ( + Config, + ConfigCreate, + ConfigUpdate, +) +from app.crud.config import ConfigCrud +from app.tests.utils.test_data import create_test_project, create_test_config +from app.tests.utils.utils import random_lower_string + + +def test_create_config(db: Session) -> None: + """Test creating a new configuration with initial version.""" + project = create_test_project(db) + config_crud = ConfigCrud(session=db, project_id=project.id) + + config_name = f"test-config-{random_lower_string()}" + config_blob = { + "model": "gpt-4", + "temperature": 0.7, + "max_tokens": 1000, + } + config_create = ConfigCreate( + name=config_name, + description="Test configuration", + config_blob=config_blob, + commit_message="Initial version", + ) + + config, version = config_crud.create(config_create) + + assert config.id is not None + assert config.name == config_name + assert config.description == "Test configuration" + assert config.project_id == project.id + assert config.deleted_at is None + + # Verify initial version was created + assert version.id is not None + assert version.config_id == config.id + assert version.version == 1 + assert version.config_blob == config_blob + assert version.commit_message == "Initial version" + + +def test_create_config_duplicate_name(db: Session) -> None: + """Test creating a configuration with a duplicate name raises HTTPException.""" + project = create_test_project(db) + config_crud = ConfigCrud(session=db, project_id=project.id) + + config_name = f"test-config-{random_lower_string()}" + config_create = ConfigCreate( + name=config_name, + description="Test configuration", + config_blob={"model": "gpt-4"}, + commit_message="Initial version", + ) + + # Create first config + config_crud.create(config_create) + + # Attempt to create second config with same name + with pytest.raises( + HTTPException, match=f"Config with name '{config_name}' already exists" + ): + config_crud.create(config_create) + + +def test_create_config_different_projects_same_name(db: Session) -> None: + """Test creating configs with same name in different projects succeeds.""" + project1 = create_test_project(db) + project2 = create_test_project(db) + + config_name = f"test-config-{random_lower_string()}" + config_blob = {"model": "gpt-4"} + + # Create config in project1 + config_crud1 = ConfigCrud(session=db, project_id=project1.id) + config_create = ConfigCreate( + name=config_name, + description="Test configuration", + config_blob=config_blob, + commit_message="Initial version", + ) + config1, _ = config_crud1.create(config_create) + + # Create config with same name in project2 + config_crud2 = ConfigCrud(session=db, project_id=project2.id) + config2, _ = config_crud2.create(config_create) + + assert config1.id != config2.id + assert config1.name == config2.name == config_name + assert config1.project_id == project1.id + assert config2.project_id == project2.id + + +def test_read_one_config(db: Session) -> None: + """Test reading a single configuration by ID.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + fetched_config = config_crud.read_one(config.id) + + assert fetched_config is not None + assert fetched_config.id == config.id + assert fetched_config.name == config.name + assert fetched_config.project_id == config.project_id + + +def test_read_one_config_not_found(db: Session) -> None: + """Test reading a non-existent configuration returns None.""" + project = create_test_project(db) + config_crud = ConfigCrud(session=db, project_id=project.id) + + non_existent_id = uuid4() + fetched_config = config_crud.read_one(non_existent_id) + + assert fetched_config is None + + +def test_read_one_config_different_project(db: Session) -> None: + """Test reading a config from a different project returns None.""" + # Create config in project1 + config = create_test_config(db) + + # Try to read from project2 + project2 = create_test_project(db) + config_crud = ConfigCrud(session=db, project_id=project2.id) + + fetched_config = config_crud.read_one(config.id) + + assert fetched_config is None + + +def test_read_one_deleted_config(db: Session) -> None: + """Test reading a deleted configuration returns None.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + # Delete the config + config_crud.delete(config.id) + + # Try to read deleted config + fetched_config = config_crud.read_one(config.id) + + assert fetched_config is None + + +def test_read_all_configs(db: Session) -> None: + """Test reading all configurations for a project.""" + project = create_test_project(db) + + # Create multiple configs + config1 = create_test_config(db, project_id=project.id, name="config-1") + config2 = create_test_config(db, project_id=project.id, name="config-2") + config3 = create_test_config(db, project_id=project.id, name="config-3") + + config_crud = ConfigCrud(session=db, project_id=project.id) + configs = config_crud.read_all() + + config_ids = [c.id for c in configs] + assert config1.id in config_ids + assert config2.id in config_ids + assert config3.id in config_ids + + +def test_read_all_configs_pagination(db: Session) -> None: + """Test reading configurations with pagination.""" + project = create_test_project(db) + + # Create 5 configs + for i in range(5): + create_test_config(db, project_id=project.id, name=f"config-{i}") + + config_crud = ConfigCrud(session=db, project_id=project.id) + + # Test skip and limit + configs_page1 = config_crud.read_all(skip=0, limit=2) + configs_page2 = config_crud.read_all(skip=2, limit=2) + + assert len(configs_page1) == 2 + assert len(configs_page2) == 2 + assert configs_page1[0].id != configs_page2[0].id + + +def test_read_all_configs_ordered_by_updated_at(db: Session) -> None: + """Test that configurations are ordered by updated_at in descending order.""" + project = create_test_project(db) + config_crud = ConfigCrud(session=db, project_id=project.id) + + # Create configs (they will have different updated_at timestamps) + config1 = create_test_config(db, project_id=project.id, name="config-1") + config2 = create_test_config(db, project_id=project.id, name="config-2") + config3 = create_test_config(db, project_id=project.id, name="config-3") + + # Update config1 to make it the most recently updated + config_crud.update( + config1.id, ConfigUpdate(description="Updated description") + ) + + configs = config_crud.read_all() + + # config1 should be first because it was most recently updated + assert configs[0].id == config1.id + + +def test_read_all_configs_excludes_deleted(db: Session) -> None: + """Test that read_all excludes deleted configurations.""" + project = create_test_project(db) + + config1 = create_test_config(db, project_id=project.id, name="config-1") + config2 = create_test_config(db, project_id=project.id, name="config-2") + + config_crud = ConfigCrud(session=db, project_id=project.id) + + # Delete config1 + config_crud.delete(config1.id) + + configs = config_crud.read_all() + + config_ids = [c.id for c in configs] + assert config1.id not in config_ids + assert config2.id in config_ids + + +def test_read_all_configs_different_projects(db: Session) -> None: + """Test that read_all only returns configs for the specific project.""" + project1 = create_test_project(db) + project2 = create_test_project(db) + + config1 = create_test_config(db, project_id=project1.id, name="config-1") + config2 = create_test_config(db, project_id=project2.id, name="config-2") + + config_crud = ConfigCrud(session=db, project_id=project1.id) + configs = config_crud.read_all() + + config_ids = [c.id for c in configs] + assert config1.id in config_ids + assert config2.id not in config_ids + + +def test_update_config_name(db: Session) -> None: + """Test updating a configuration's name.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + new_name = f"updated-config-{random_lower_string()}" + config_update = ConfigUpdate(name=new_name) + + updated_config = config_crud.update(config.id, config_update) + + assert updated_config.name == new_name + assert updated_config.id == config.id + + +def test_update_config_description(db: Session) -> None: + """Test updating a configuration's description.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + new_description = "Updated description" + config_update = ConfigUpdate(description=new_description) + + updated_config = config_crud.update(config.id, config_update) + + assert updated_config.description == new_description + assert updated_config.id == config.id + + +def test_update_config_multiple_fields(db: Session) -> None: + """Test updating multiple fields of a configuration.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + new_name = f"updated-config-{random_lower_string()}" + new_description = "Updated description" + config_update = ConfigUpdate(name=new_name, description=new_description) + + updated_config = config_crud.update(config.id, config_update) + + assert updated_config.name == new_name + assert updated_config.description == new_description + + +def test_update_config_duplicate_name(db: Session) -> None: + """Test updating a config to a duplicate name raises HTTPException.""" + project = create_test_project(db) + + config1 = create_test_config(db, project_id=project.id, name="config-1") + config2 = create_test_config(db, project_id=project.id, name="config-2") + + config_crud = ConfigCrud(session=db, project_id=project.id) + + # Try to update config2 to have config1's name + config_update = ConfigUpdate(name=config1.name) + + with pytest.raises( + HTTPException, match=f"Config with name '{config1.name}' already exists" + ): + config_crud.update(config2.id, config_update) + + +def test_update_config_same_name(db: Session) -> None: + """Test updating a config to its own name succeeds.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + # Update to same name should succeed + config_update = ConfigUpdate(name=config.name, description="Updated") + + updated_config = config_crud.update(config.id, config_update) + + assert updated_config.name == config.name + assert updated_config.description == "Updated" + + +def test_update_config_not_found(db: Session) -> None: + """Test updating a non-existent configuration raises HTTPException.""" + project = create_test_project(db) + config_crud = ConfigCrud(session=db, project_id=project.id) + + non_existent_id = uuid4() + config_update = ConfigUpdate(name="new-name") + + with pytest.raises( + HTTPException, match=f"config with id '{non_existent_id}' not found" + ): + config_crud.update(non_existent_id, config_update) + + +def test_update_config_updates_timestamp(db: Session) -> None: + """Test that updating a config updates the updated_at timestamp.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + original_updated_at = config.updated_at + + config_update = ConfigUpdate(description="Updated description") + updated_config = config_crud.update(config.id, config_update) + + assert updated_config.updated_at > original_updated_at + + +def test_delete_config(db: Session) -> None: + """Test soft deleting a configuration.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + config_crud.delete(config.id) + + # Verify soft delete (deleted_at is set) + db.refresh(config) + assert config.deleted_at is not None + + +def test_delete_config_not_found(db: Session) -> None: + """Test deleting a non-existent configuration raises HTTPException.""" + project = create_test_project(db) + config_crud = ConfigCrud(session=db, project_id=project.id) + + non_existent_id = uuid4() + + with pytest.raises( + HTTPException, match=f"config with id '{non_existent_id}' not found" + ): + config_crud.delete(non_existent_id) + + +def test_delete_config_different_project(db: Session) -> None: + """Test deleting a config from a different project raises HTTPException.""" + # Create config in project1 + config = create_test_config(db) + + # Try to delete from project2 + project2 = create_test_project(db) + config_crud = ConfigCrud(session=db, project_id=project2.id) + + with pytest.raises( + HTTPException, match=f"config with id '{config.id}' not found" + ): + config_crud.delete(config.id) + + +def test_exists_config(db: Session) -> None: + """Test that exists returns the config when it exists.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + existing_config = config_crud.exists(config.id) + + assert existing_config.id == config.id + assert existing_config.name == config.name + + +def test_exists_config_not_found(db: Session) -> None: + """Test that exists raises HTTPException when config doesn't exist.""" + project = create_test_project(db) + config_crud = ConfigCrud(session=db, project_id=project.id) + + non_existent_id = uuid4() + + with pytest.raises( + HTTPException, match=f"config with id '{non_existent_id}' not found" + ): + config_crud.exists(non_existent_id) + + +def test_exists_deleted_config(db: Session) -> None: + """Test that exists raises HTTPException for deleted configs.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + # Delete the config + config_crud.delete(config.id) + + # exists should raise HTTPException + with pytest.raises( + HTTPException, match=f"config with id '{config.id}' not found" + ): + config_crud.exists(config.id) + + +def test_check_unique_name_with_existing_name(db: Session) -> None: + """Test that _check_unique_name raises HTTPException for duplicate names.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + with pytest.raises( + HTTPException, match=f"Config with name '{config.name}' already exists" + ): + config_crud._check_unique_name(config.name) + + +def test_check_unique_name_with_new_name(db: Session) -> None: + """Test that _check_unique_name passes for unique names.""" + project = create_test_project(db) + config_crud = ConfigCrud(session=db, project_id=project.id) + + # Should not raise exception + unique_name = f"unique-name-{random_lower_string()}" + config_crud._check_unique_name(unique_name) + + +def test_read_by_name(db: Session) -> None: + """Test reading a configuration by name.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + fetched_config = config_crud._read_by_name(config.name) + + assert fetched_config is not None + assert fetched_config.id == config.id + assert fetched_config.name == config.name + + +def test_read_by_name_not_found(db: Session) -> None: + """Test that _read_by_name returns None for non-existent names.""" + project = create_test_project(db) + config_crud = ConfigCrud(session=db, project_id=project.id) + + non_existent_name = f"non-existent-{random_lower_string()}" + fetched_config = config_crud._read_by_name(non_existent_name) + + assert fetched_config is None + + +def test_read_by_name_deleted_config(db: Session) -> None: + """Test that _read_by_name returns None for deleted configs.""" + config = create_test_config(db) + config_crud = ConfigCrud(session=db, project_id=config.project_id) + + # Delete the config + config_crud.delete(config.id) + + # Should return None + fetched_config = config_crud._read_by_name(config.name) + + assert fetched_config is None diff --git a/backend/app/tests/crud/config/test_version.py b/backend/app/tests/crud/config/test_version.py new file mode 100644 index 000000000..54fb31a2f --- /dev/null +++ b/backend/app/tests/crud/config/test_version.py @@ -0,0 +1,420 @@ +import pytest +from uuid import uuid4 +from sqlmodel import Session +from fastapi import HTTPException + +from app.models import ConfigVersionCreate +from app.crud.config import ConfigVersionCrud +from app.tests.utils.test_data import ( + create_test_project, + create_test_config, + create_test_version, +) + + +def test_create_version(db: Session) -> None: + """Test creating a new version for an existing configuration.""" + config = create_test_config(db) + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + config_blob = { + "model": "gpt-4-turbo", + "temperature": 0.8, + "max_tokens": 2000, + } + version_create = ConfigVersionCreate( + config_blob=config_blob, + commit_message="Updated model and parameters", + ) + + version = version_crud.create(version_create) + + assert version.id is not None + assert version.config_id == config.id + assert version.version == 2 # Should be 2 since config creation creates version 1 + assert version.config_blob == config_blob + assert version.commit_message == "Updated model and parameters" + assert version.deleted_at is None + + +def test_create_version_auto_increment(db: Session) -> None: + """Test that version numbers auto-increment correctly.""" + config = create_test_config(db) + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + # Create multiple versions + version2 = version_crud.create( + ConfigVersionCreate( + config_blob={"model": "gpt-4"}, commit_message="Version 2" + ) + ) + version3 = version_crud.create( + ConfigVersionCreate( + config_blob={"model": "gpt-4"}, commit_message="Version 3" + ) + ) + version4 = version_crud.create( + ConfigVersionCreate( + config_blob={"model": "gpt-4"}, commit_message="Version 4" + ) + ) + + assert version2.version == 2 + assert version3.version == 3 + assert version4.version == 4 + + +def test_create_version_config_not_found(db: Session) -> None: + """Test creating a version for a non-existent config raises HTTPException.""" + project = create_test_project(db) + non_existent_config_id = uuid4() + + version_crud = ConfigVersionCrud( + session=db, project_id=project.id, config_id=non_existent_config_id + ) + + version_create = ConfigVersionCreate( + config_blob={"model": "gpt-4"}, commit_message="Test" + ) + + with pytest.raises( + HTTPException, match=f"config with id '{non_existent_config_id}' not found" + ): + version_crud.create(version_create) + + +def test_read_one_version(db: Session) -> None: + """Test reading a specific version by its version number.""" + config = create_test_config(db) + version = create_test_version( + db, + config_id=config.id, + project_id=config.project_id, + config_blob={"model": "gpt-4-turbo"}, + commit_message="Test version", + ) + + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + fetched_version = version_crud.read_one(version.version) + + assert fetched_version is not None + assert fetched_version.id == version.id + assert fetched_version.version == version.version + assert fetched_version.config_id == config.id + assert fetched_version.config_blob == {"model": "gpt-4-turbo"} + + +def test_read_one_version_not_found(db: Session) -> None: + """Test reading a non-existent version returns None.""" + config = create_test_config(db) + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + non_existent_version = 999 + fetched_version = version_crud.read_one(non_existent_version) + + assert fetched_version is None + + +def test_read_one_version_deleted(db: Session) -> None: + """Test reading a deleted version returns None.""" + config = create_test_config(db) + version = create_test_version( + db, config_id=config.id, project_id=config.project_id + ) + + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + # Delete the version + version_crud.delete(version.version) + + # Try to read deleted version + fetched_version = version_crud.read_one(version.version) + + assert fetched_version is None + + +def test_read_all_versions(db: Session) -> None: + """Test reading all versions for a configuration.""" + config = create_test_config(db) + + # Create additional versions (config already has version 1) + version2 = create_test_version( + db, + config_id=config.id, + project_id=config.project_id, + commit_message="Version 2", + ) + version3 = create_test_version( + db, + config_id=config.id, + project_id=config.project_id, + commit_message="Version 3", + ) + + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + versions = version_crud.read_all() + + assert len(versions) == 3 + version_numbers = [v.version for v in versions] + assert 1 in version_numbers + assert version2.version in version_numbers + assert version3.version in version_numbers + + +def test_read_all_versions_pagination(db: Session) -> None: + """Test reading versions with pagination.""" + config = create_test_config(db) + + # Create 4 additional versions + for i in range(4): + create_test_version( + db, + config_id=config.id, + project_id=config.project_id, + commit_message=f"Version {i + 2}", + ) + + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + # Test skip and limit + versions_page1 = version_crud.read_all(skip=0, limit=2) + versions_page2 = version_crud.read_all(skip=2, limit=2) + + assert len(versions_page1) == 2 + assert len(versions_page2) == 2 + assert versions_page1[0].id != versions_page2[0].id + + +def test_read_all_versions_ordered_by_version_desc(db: Session) -> None: + """Test that versions are ordered by version number in descending order.""" + config = create_test_config(db) + + # Create additional versions + version2 = create_test_version( + db, + config_id=config.id, + project_id=config.project_id, + commit_message="Version 2", + ) + version3 = create_test_version( + db, + config_id=config.id, + project_id=config.project_id, + commit_message="Version 3", + ) + + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + versions = version_crud.read_all() + + # Versions should be in descending order (3, 2, 1) + assert versions[0].version == version3.version + assert versions[1].version == version2.version + assert versions[2].version == 1 + + +def test_read_all_versions_excludes_blob(db: Session) -> None: + """Test that read_all returns ConfigVersionItems without config_blob.""" + config = create_test_config(db) + create_test_version( + db, + config_id=config.id, + project_id=config.project_id, + config_blob={"model": "gpt-4-turbo"}, + ) + + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + versions = version_crud.read_all() + + # Verify versions are ConfigVersionItems (should not have config_blob field) + for version in versions: + assert hasattr(version, "id") + assert hasattr(version, "version") + assert hasattr(version, "commit_message") + # ConfigVersionItems should not include config_blob + assert not hasattr(version, "config_blob") + + +def test_read_all_versions_excludes_deleted(db: Session) -> None: + """Test that read_all excludes deleted versions.""" + config = create_test_config(db) + + version2 = create_test_version( + db, + config_id=config.id, + project_id=config.project_id, + commit_message="Version 2", + ) + version3 = create_test_version( + db, + config_id=config.id, + project_id=config.project_id, + commit_message="Version 3", + ) + + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + # Delete version 2 + version_crud.delete(version2.version) + + versions = version_crud.read_all() + + version_numbers = [v.version for v in versions] + assert version2.version not in version_numbers + assert 1 in version_numbers + assert version3.version in version_numbers + + +def test_delete_version(db: Session) -> None: + """Test soft deleting a version.""" + config = create_test_config(db) + version = create_test_version( + db, config_id=config.id, project_id=config.project_id + ) + + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + version_crud.delete(version.version) + + # Verify soft delete (deleted_at is set) + db.refresh(version) + assert version.deleted_at is not None + + +def test_delete_version_not_found(db: Session) -> None: + """Test deleting a non-existent version raises HTTPException.""" + config = create_test_config(db) + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + non_existent_version = 999 + + with pytest.raises( + HTTPException, + match=f"Version with number '{non_existent_version}' not found for config '{config.id}'", + ): + version_crud.delete(non_existent_version) + + +def test_exists_version(db: Session) -> None: + """Test that exists returns the version when it exists.""" + config = create_test_config(db) + version = create_test_version( + db, config_id=config.id, project_id=config.project_id + ) + + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + existing_version = version_crud.exists(version.version) + + assert existing_version.id == version.id + assert existing_version.version == version.version + + +def test_exists_version_not_found(db: Session) -> None: + """Test that exists raises HTTPException when version doesn't exist.""" + config = create_test_config(db) + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + non_existent_version = 999 + + with pytest.raises( + HTTPException, + match=f"Version with number '{non_existent_version}' not found for config '{config.id}'", + ): + version_crud.exists(non_existent_version) + + +def test_exists_version_deleted(db: Session) -> None: + """Test that exists raises HTTPException for deleted versions.""" + config = create_test_config(db) + version = create_test_version( + db, config_id=config.id, project_id=config.project_id + ) + + version_crud = ConfigVersionCrud( + session=db, project_id=config.project_id, config_id=config.id + ) + + # Delete the version + version_crud.delete(version.version) + + # exists should raise HTTPException + with pytest.raises( + HTTPException, + match=f"Version with number '{version.version}' not found for config '{config.id}'", + ): + version_crud.exists(version.version) + + +def test_create_version_different_configs(db: Session) -> None: + """Test that version numbers are independent across different configs.""" + project = create_test_project(db) + + # Create two configs + config1 = create_test_config(db, project_id=project.id, name="config-1") + config2 = create_test_config(db, project_id=project.id, name="config-2") + + # Create versions for config1 + version_crud1 = ConfigVersionCrud( + session=db, project_id=project.id, config_id=config1.id + ) + version2_config1 = version_crud1.create( + ConfigVersionCreate(config_blob={"model": "gpt-4"}, commit_message="V2") + ) + + # Create versions for config2 + version_crud2 = ConfigVersionCrud( + session=db, project_id=project.id, config_id=config2.id + ) + version2_config2 = version_crud2.create( + ConfigVersionCreate(config_blob={"model": "gpt-4"}, commit_message="V2") + ) + + # Both should have version 2 (independent numbering) + assert version2_config1.version == 2 + assert version2_config2.version == 2 + assert version2_config1.config_id == config1.id + assert version2_config2.config_id == config2.id + + +def test_read_all_versions_config_not_found(db: Session) -> None: + """Test reading versions for a non-existent config raises HTTPException.""" + project = create_test_project(db) + non_existent_config_id = uuid4() + + version_crud = ConfigVersionCrud( + session=db, project_id=project.id, config_id=non_existent_config_id + ) + + with pytest.raises( + HTTPException, match=f"config with id '{non_existent_config_id}' not found" + ): + version_crud.read_all() From 6349af148a13f956f1904d3e87c2f470c18d1aea Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 12 Nov 2025 14:09:42 +0530 Subject: [PATCH 11/20] precommit --- .../tests/api/routes/configs/test_version.py | 4 ++- backend/app/tests/crud/config/test_config.py | 12 ++------ backend/app/tests/crud/config/test_version.py | 28 +++++-------------- 3 files changed, 13 insertions(+), 31 deletions(-) diff --git a/backend/app/tests/api/routes/configs/test_version.py b/backend/app/tests/api/routes/configs/test_version.py index d534f67dd..882e57fcf 100644 --- a/backend/app/tests/api/routes/configs/test_version.py +++ b/backend/app/tests/api/routes/configs/test_version.py @@ -42,7 +42,9 @@ def test_create_version_success( data = response.json() assert data["success"] is True assert "data" in data - assert data["data"]["version"] == 2 # First version created with config, this is second + assert ( + data["data"]["version"] == 2 + ) # First version created with config, this is second assert data["data"]["config_blob"] == version_data["config_blob"] assert data["data"]["commit_message"] == version_data["commit_message"] assert data["data"]["config_id"] == str(config.id) diff --git a/backend/app/tests/crud/config/test_config.py b/backend/app/tests/crud/config/test_config.py index a354d3484..febaeb896 100644 --- a/backend/app/tests/crud/config/test_config.py +++ b/backend/app/tests/crud/config/test_config.py @@ -198,9 +198,7 @@ def test_read_all_configs_ordered_by_updated_at(db: Session) -> None: config3 = create_test_config(db, project_id=project.id, name="config-3") # Update config1 to make it the most recently updated - config_crud.update( - config1.id, ConfigUpdate(description="Updated description") - ) + config_crud.update(config1.id, ConfigUpdate(description="Updated description")) configs = config_crud.read_all() @@ -379,9 +377,7 @@ def test_delete_config_different_project(db: Session) -> None: project2 = create_test_project(db) config_crud = ConfigCrud(session=db, project_id=project2.id) - with pytest.raises( - HTTPException, match=f"config with id '{config.id}' not found" - ): + with pytest.raises(HTTPException, match=f"config with id '{config.id}' not found"): config_crud.delete(config.id) @@ -418,9 +414,7 @@ def test_exists_deleted_config(db: Session) -> None: config_crud.delete(config.id) # exists should raise HTTPException - with pytest.raises( - HTTPException, match=f"config with id '{config.id}' not found" - ): + with pytest.raises(HTTPException, match=f"config with id '{config.id}' not found"): config_crud.exists(config.id) diff --git a/backend/app/tests/crud/config/test_version.py b/backend/app/tests/crud/config/test_version.py index 54fb31a2f..dada05ce9 100644 --- a/backend/app/tests/crud/config/test_version.py +++ b/backend/app/tests/crud/config/test_version.py @@ -48,19 +48,13 @@ def test_create_version_auto_increment(db: Session) -> None: # Create multiple versions version2 = version_crud.create( - ConfigVersionCreate( - config_blob={"model": "gpt-4"}, commit_message="Version 2" - ) + ConfigVersionCreate(config_blob={"model": "gpt-4"}, commit_message="Version 2") ) version3 = version_crud.create( - ConfigVersionCreate( - config_blob={"model": "gpt-4"}, commit_message="Version 3" - ) + ConfigVersionCreate(config_blob={"model": "gpt-4"}, commit_message="Version 3") ) version4 = version_crud.create( - ConfigVersionCreate( - config_blob={"model": "gpt-4"}, commit_message="Version 4" - ) + ConfigVersionCreate(config_blob={"model": "gpt-4"}, commit_message="Version 4") ) assert version2.version == 2 @@ -127,9 +121,7 @@ def test_read_one_version_not_found(db: Session) -> None: def test_read_one_version_deleted(db: Session) -> None: """Test reading a deleted version returns None.""" config = create_test_config(db) - version = create_test_version( - db, config_id=config.id, project_id=config.project_id - ) + version = create_test_version(db, config_id=config.id, project_id=config.project_id) version_crud = ConfigVersionCrud( session=db, project_id=config.project_id, config_id=config.id @@ -288,9 +280,7 @@ def test_read_all_versions_excludes_deleted(db: Session) -> None: def test_delete_version(db: Session) -> None: """Test soft deleting a version.""" config = create_test_config(db) - version = create_test_version( - db, config_id=config.id, project_id=config.project_id - ) + version = create_test_version(db, config_id=config.id, project_id=config.project_id) version_crud = ConfigVersionCrud( session=db, project_id=config.project_id, config_id=config.id @@ -322,9 +312,7 @@ def test_delete_version_not_found(db: Session) -> None: def test_exists_version(db: Session) -> None: """Test that exists returns the version when it exists.""" config = create_test_config(db) - version = create_test_version( - db, config_id=config.id, project_id=config.project_id - ) + version = create_test_version(db, config_id=config.id, project_id=config.project_id) version_crud = ConfigVersionCrud( session=db, project_id=config.project_id, config_id=config.id @@ -355,9 +343,7 @@ def test_exists_version_not_found(db: Session) -> None: def test_exists_version_deleted(db: Session) -> None: """Test that exists raises HTTPException for deleted versions.""" config = create_test_config(db) - version = create_test_version( - db, config_id=config.id, project_id=config.project_id - ) + version = create_test_version(db, config_id=config.id, project_id=config.project_id) version_crud = ConfigVersionCrud( session=db, project_id=config.project_id, config_id=config.id From 62886300dabe46c58b94cecc53f063b30cc85630 Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 13 Nov 2025 11:59:48 +0530 Subject: [PATCH 12/20] update migration --- .../4523c8d1253d_config_management_tables.py | 57 ++++++++++ .../e51b082aa9b3_config_management_tables.py | 100 ------------------ 2 files changed, 57 insertions(+), 100 deletions(-) create mode 100644 backend/app/alembic/versions/4523c8d1253d_config_management_tables.py delete mode 100644 backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py diff --git a/backend/app/alembic/versions/4523c8d1253d_config_management_tables.py b/backend/app/alembic/versions/4523c8d1253d_config_management_tables.py new file mode 100644 index 000000000..fba06f15e --- /dev/null +++ b/backend/app/alembic/versions/4523c8d1253d_config_management_tables.py @@ -0,0 +1,57 @@ +"""Config management tables + +Revision ID: 4523c8d1253d +Revises: 6fe772038a5a +Create Date: 2025-11-13 11:56:57.233241 + +""" +from alembic import op +import sqlalchemy as sa +import sqlmodel.sql.sqltypes +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '4523c8d1253d' +down_revision = '6fe772038a5a' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('config', + sa.Column('name', sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), + sa.Column('description', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), + sa.Column('id', sa.Uuid(), nullable=False), + sa.Column('project_id', sa.Integer(), nullable=False), + sa.Column('inserted_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=False), + sa.Column('deleted_at', sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(['project_id'], ['project.id'], ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('project_id', 'name', 'deleted_at') + ) + op.create_index(op.f('ix_config_name'), 'config', ['name'], unique=False) + op.create_index(op.f('ix_config_project_id'), 'config', ['project_id'], unique=False) + op.create_table('config_version', + sa.Column('config_blob', postgresql.JSON(astext_type=sa.Text()), nullable=False), + sa.Column('commit_message', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), + sa.Column('id', sa.Uuid(), nullable=False), + sa.Column('config_id', sa.Uuid(), nullable=False), + sa.Column('version', sa.Integer(), nullable=False), + sa.Column('inserted_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=False), + sa.Column('deleted_at', sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(['config_id'], ['config.id'], ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('config_id', 'version') + ) + op.create_index(op.f('ix_config_version_config_id'), 'config_version', ['config_id'], unique=False) + + +def downgrade(): + op.drop_index(op.f('ix_config_version_config_id'), table_name='config_version') + op.drop_table('config_version') + op.drop_index(op.f('ix_config_project_id'), table_name='config') + op.drop_index(op.f('ix_config_name'), table_name='config') + op.drop_table('config') diff --git a/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py b/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py deleted file mode 100644 index 4089139f9..000000000 --- a/backend/app/alembic/versions/e51b082aa9b3_config_management_tables.py +++ /dev/null @@ -1,100 +0,0 @@ -"""Config management tables - -Revision ID: e51b082aa9b3 -Revises: 219033c644de -Create Date: 2025-11-05 16:19:20.423398 - -""" -from alembic import op -import sqlalchemy as sa -import sqlmodel.sql.sqltypes -from sqlalchemy.dialects import postgresql - -# revision identifiers, used by Alembic. -revision = "e51b082aa9b3" -down_revision = "219033c644de" -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.create_table( - "config", - sa.Column("name", sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), - sa.Column( - "description", sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True - ), - sa.Column("id", sa.Uuid(), nullable=False), - sa.Column("project_id", sa.Integer(), nullable=False), - sa.Column("inserted_at", sa.DateTime(), nullable=False), - sa.Column("updated_at", sa.DateTime(), nullable=False), - sa.Column("deleted_at", sa.DateTime(), nullable=True), - sa.ForeignKeyConstraint(["project_id"], ["project.id"], ondelete="CASCADE"), - sa.PrimaryKeyConstraint("id"), - ) - op.create_index(op.f("ix_config_name"), "config", ["name"], unique=False) - op.create_index( - op.f("ix_config_project_id"), "config", ["project_id"], unique=False - ) - op.create_table( - "config_version", - sa.Column( - "config_blob", postgresql.JSON(astext_type=sa.Text()), nullable=False - ), - sa.Column( - "commit_message", - sqlmodel.sql.sqltypes.AutoString(length=512), - nullable=True, - ), - sa.Column("version", sa.Integer(), nullable=False), - sa.Column("id", sa.Uuid(), nullable=False), - sa.Column("config_id", sa.Uuid(), nullable=False), - sa.Column("inserted_at", sa.DateTime(), nullable=False), - sa.Column("updated_at", sa.DateTime(), nullable=False), - sa.Column("deleted_at", sa.DateTime(), nullable=True), - sa.ForeignKeyConstraint(["config_id"], ["config.id"], ondelete="CASCADE"), - sa.PrimaryKeyConstraint("id"), - sa.UniqueConstraint("config_id", "version"), - ) - op.create_index( - op.f("ix_config_version_config_id"), - "config_version", - ["config_id"], - unique=False, - ) - op.drop_constraint( - op.f("openai_conversation_project_id_fkey1"), - "openai_conversation", - type_="foreignkey", - ) - op.drop_constraint( - op.f("openai_conversation_organization_id_fkey1"), - "openai_conversation", - type_="foreignkey", - ) - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.create_foreign_key( - op.f("openai_conversation_organization_id_fkey1"), - "openai_conversation", - "organization", - ["organization_id"], - ["id"], - ) - op.create_foreign_key( - op.f("openai_conversation_project_id_fkey1"), - "openai_conversation", - "project", - ["project_id"], - ["id"], - ) - op.drop_index(op.f("ix_config_version_config_id"), table_name="config_version") - op.drop_table("config_version") - op.drop_index(op.f("ix_config_project_id"), table_name="config") - op.drop_index(op.f("ix_config_name"), table_name="config") - op.drop_table("config") - # ### end Alembic commands ### From d8f2733b15e59a8016485b2aa61d1c45f025bd37 Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 13 Nov 2025 12:00:04 +0530 Subject: [PATCH 13/20] precomit --- .../4523c8d1253d_config_management_tables.py | 83 +++++++++++-------- 1 file changed, 50 insertions(+), 33 deletions(-) diff --git a/backend/app/alembic/versions/4523c8d1253d_config_management_tables.py b/backend/app/alembic/versions/4523c8d1253d_config_management_tables.py index fba06f15e..ac80699ce 100644 --- a/backend/app/alembic/versions/4523c8d1253d_config_management_tables.py +++ b/backend/app/alembic/versions/4523c8d1253d_config_management_tables.py @@ -11,47 +11,64 @@ from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. -revision = '4523c8d1253d' -down_revision = '6fe772038a5a' +revision = "4523c8d1253d" +down_revision = "6fe772038a5a" branch_labels = None depends_on = None def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.create_table('config', - sa.Column('name', sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), - sa.Column('description', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), - sa.Column('id', sa.Uuid(), nullable=False), - sa.Column('project_id', sa.Integer(), nullable=False), - sa.Column('inserted_at', sa.DateTime(), nullable=False), - sa.Column('updated_at', sa.DateTime(), nullable=False), - sa.Column('deleted_at', sa.DateTime(), nullable=True), - sa.ForeignKeyConstraint(['project_id'], ['project.id'], ondelete='CASCADE'), - sa.PrimaryKeyConstraint('id'), - sa.UniqueConstraint('project_id', 'name', 'deleted_at') + op.create_table( + "config", + sa.Column("name", sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), + sa.Column( + "description", sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True + ), + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("project_id", sa.Integer(), nullable=False), + sa.Column("inserted_at", sa.DateTime(), nullable=False), + sa.Column("updated_at", sa.DateTime(), nullable=False), + sa.Column("deleted_at", sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(["project_id"], ["project.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("project_id", "name", "deleted_at"), ) - op.create_index(op.f('ix_config_name'), 'config', ['name'], unique=False) - op.create_index(op.f('ix_config_project_id'), 'config', ['project_id'], unique=False) - op.create_table('config_version', - sa.Column('config_blob', postgresql.JSON(astext_type=sa.Text()), nullable=False), - sa.Column('commit_message', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), - sa.Column('id', sa.Uuid(), nullable=False), - sa.Column('config_id', sa.Uuid(), nullable=False), - sa.Column('version', sa.Integer(), nullable=False), - sa.Column('inserted_at', sa.DateTime(), nullable=False), - sa.Column('updated_at', sa.DateTime(), nullable=False), - sa.Column('deleted_at', sa.DateTime(), nullable=True), - sa.ForeignKeyConstraint(['config_id'], ['config.id'], ondelete='CASCADE'), - sa.PrimaryKeyConstraint('id'), - sa.UniqueConstraint('config_id', 'version') + op.create_index(op.f("ix_config_name"), "config", ["name"], unique=False) + op.create_index( + op.f("ix_config_project_id"), "config", ["project_id"], unique=False + ) + op.create_table( + "config_version", + sa.Column( + "config_blob", postgresql.JSON(astext_type=sa.Text()), nullable=False + ), + sa.Column( + "commit_message", + sqlmodel.sql.sqltypes.AutoString(length=512), + nullable=True, + ), + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("config_id", sa.Uuid(), nullable=False), + sa.Column("version", sa.Integer(), nullable=False), + sa.Column("inserted_at", sa.DateTime(), nullable=False), + sa.Column("updated_at", sa.DateTime(), nullable=False), + sa.Column("deleted_at", sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(["config_id"], ["config.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("config_id", "version"), + ) + op.create_index( + op.f("ix_config_version_config_id"), + "config_version", + ["config_id"], + unique=False, ) - op.create_index(op.f('ix_config_version_config_id'), 'config_version', ['config_id'], unique=False) def downgrade(): - op.drop_index(op.f('ix_config_version_config_id'), table_name='config_version') - op.drop_table('config_version') - op.drop_index(op.f('ix_config_project_id'), table_name='config') - op.drop_index(op.f('ix_config_name'), table_name='config') - op.drop_table('config') + op.drop_index(op.f("ix_config_version_config_id"), table_name="config_version") + op.drop_table("config_version") + op.drop_index(op.f("ix_config_project_id"), table_name="config") + op.drop_index(op.f("ix_config_name"), table_name="config") + op.drop_table("config") From 59037515529b54c903acd75dd953b1771c9bc390 Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 13 Nov 2025 12:19:04 +0530 Subject: [PATCH 14/20] add init --- backend/app/tests/api/routes/configs/__init__.py | 0 backend/app/tests/crud/config/__init__.py | 0 2 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 backend/app/tests/api/routes/configs/__init__.py create mode 100644 backend/app/tests/crud/config/__init__.py diff --git a/backend/app/tests/api/routes/configs/__init__.py b/backend/app/tests/api/routes/configs/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/app/tests/crud/config/__init__.py b/backend/app/tests/crud/config/__init__.py new file mode 100644 index 000000000..e69de29bb From 4316763be2be92fba3b750400095e4373b48590a Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 13 Nov 2025 16:04:04 +0530 Subject: [PATCH 15/20] Review queries and index --- .../4523c8d1253d_config_management_tables.py | 74 ------------------- .../f245ccb27ab6_config_management_tables.py | 56 ++++++++++++++ backend/app/crud/config/config.py | 2 +- backend/app/crud/config/version.py | 14 ++-- backend/app/models/config/config.py | 19 +++-- backend/app/models/config/version.py | 12 ++- backend/uv.lock | 2 +- 7 files changed, 86 insertions(+), 93 deletions(-) delete mode 100644 backend/app/alembic/versions/4523c8d1253d_config_management_tables.py create mode 100644 backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py diff --git a/backend/app/alembic/versions/4523c8d1253d_config_management_tables.py b/backend/app/alembic/versions/4523c8d1253d_config_management_tables.py deleted file mode 100644 index ac80699ce..000000000 --- a/backend/app/alembic/versions/4523c8d1253d_config_management_tables.py +++ /dev/null @@ -1,74 +0,0 @@ -"""Config management tables - -Revision ID: 4523c8d1253d -Revises: 6fe772038a5a -Create Date: 2025-11-13 11:56:57.233241 - -""" -from alembic import op -import sqlalchemy as sa -import sqlmodel.sql.sqltypes -from sqlalchemy.dialects import postgresql - -# revision identifiers, used by Alembic. -revision = "4523c8d1253d" -down_revision = "6fe772038a5a" -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.create_table( - "config", - sa.Column("name", sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), - sa.Column( - "description", sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True - ), - sa.Column("id", sa.Uuid(), nullable=False), - sa.Column("project_id", sa.Integer(), nullable=False), - sa.Column("inserted_at", sa.DateTime(), nullable=False), - sa.Column("updated_at", sa.DateTime(), nullable=False), - sa.Column("deleted_at", sa.DateTime(), nullable=True), - sa.ForeignKeyConstraint(["project_id"], ["project.id"], ondelete="CASCADE"), - sa.PrimaryKeyConstraint("id"), - sa.UniqueConstraint("project_id", "name", "deleted_at"), - ) - op.create_index(op.f("ix_config_name"), "config", ["name"], unique=False) - op.create_index( - op.f("ix_config_project_id"), "config", ["project_id"], unique=False - ) - op.create_table( - "config_version", - sa.Column( - "config_blob", postgresql.JSON(astext_type=sa.Text()), nullable=False - ), - sa.Column( - "commit_message", - sqlmodel.sql.sqltypes.AutoString(length=512), - nullable=True, - ), - sa.Column("id", sa.Uuid(), nullable=False), - sa.Column("config_id", sa.Uuid(), nullable=False), - sa.Column("version", sa.Integer(), nullable=False), - sa.Column("inserted_at", sa.DateTime(), nullable=False), - sa.Column("updated_at", sa.DateTime(), nullable=False), - sa.Column("deleted_at", sa.DateTime(), nullable=True), - sa.ForeignKeyConstraint(["config_id"], ["config.id"], ondelete="CASCADE"), - sa.PrimaryKeyConstraint("id"), - sa.UniqueConstraint("config_id", "version"), - ) - op.create_index( - op.f("ix_config_version_config_id"), - "config_version", - ["config_id"], - unique=False, - ) - - -def downgrade(): - op.drop_index(op.f("ix_config_version_config_id"), table_name="config_version") - op.drop_table("config_version") - op.drop_index(op.f("ix_config_project_id"), table_name="config") - op.drop_index(op.f("ix_config_name"), table_name="config") - op.drop_table("config") diff --git a/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py b/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py new file mode 100644 index 000000000..7f0c1ba15 --- /dev/null +++ b/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py @@ -0,0 +1,56 @@ +"""Config management tables + +Revision ID: f245ccb27ab6 +Revises: 6fe772038a5a +Create Date: 2025-11-13 16:02:08.465124 + +""" +from alembic import op +import sqlalchemy as sa +import sqlmodel.sql.sqltypes +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = 'f245ccb27ab6' +down_revision = '6fe772038a5a' +branch_labels = None +depends_on = None + + +def upgrade(): + op.create_table('config', + sa.Column('name', sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), + sa.Column('description', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), + sa.Column('id', sa.Uuid(), nullable=False), + sa.Column('project_id', sa.Integer(), nullable=False), + sa.Column('inserted_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=False), + sa.Column('deleted_at', sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(['project_id'], ['project.id'], ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id') + ) + op.create_index('idx_config_project_id_updated_at_active', 'config', ['project_id', 'updated_at'], unique=False, postgresql_where=sa.text('deleted_at IS NULL')) + op.create_index('uq_config_project_id_name_active', 'config', ['project_id', 'name'], unique=True, postgresql_where=sa.text('deleted_at IS NULL')) + op.create_table('config_version', + sa.Column('config_blob', postgresql.JSONB(astext_type=sa.Text()), nullable=False), + sa.Column('commit_message', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), + sa.Column('id', sa.Uuid(), nullable=False), + sa.Column('config_id', sa.Uuid(), nullable=False), + sa.Column('version', sa.Integer(), nullable=False), + sa.Column('inserted_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=False), + sa.Column('deleted_at', sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(['config_id'], ['config.id'], ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('config_id', 'version', name='uq_config_version_config_id_version') + ) + op.create_index('idx_config_version_config_id_version_active', 'config_version', ['config_id', 'version'], unique=False, postgresql_where=sa.text('deleted_at IS NULL')) + + +def downgrade(): + op.drop_index('idx_config_version_config_id_version_active', table_name='config_version', postgresql_where=sa.text('deleted_at IS NULL')) + op.drop_table('config_version') + op.drop_index('uq_config_project_id_name_active', table_name='config', postgresql_where=sa.text('deleted_at IS NULL')) + op.drop_index(op.f('ix_config_name'), table_name='config') + op.drop_index('idx_config_project_id_updated_at_active', table_name='config', postgresql_where=sa.text('deleted_at IS NULL')) + op.drop_table('config') diff --git a/backend/app/crud/config/config.py b/backend/app/crud/config/config.py index 03016987e..292f0dd56 100644 --- a/backend/app/crud/config/config.py +++ b/backend/app/crud/config/config.py @@ -92,9 +92,9 @@ def read_all(self, skip: int = 0, limit: int = 100) -> list[Config]: Config.deleted_at.is_(None), ) ) + .order_by(Config.updated_at.desc()) .offset(skip) .limit(limit) - .order_by(Config.updated_at.desc()) ) return self.session.exec(statement).all() diff --git a/backend/app/crud/config/version.py b/backend/app/crud/config/version.py index 6f35d6d00..030bfe8c4 100644 --- a/backend/app/crud/config/version.py +++ b/backend/app/crud/config/version.py @@ -92,9 +92,9 @@ def read_all(self, skip: int = 0, limit: int = 100) -> list[ConfigVersionItems]: .options( defer(ConfigVersion.config_blob), ) + .order_by(ConfigVersion.version.desc()) .offset(skip) .limit(limit) - .order_by(ConfigVersion.version.desc()) ) results = self.session.exec(statement).all() return [ConfigVersionItems.model_validate(item) for item in results] @@ -124,12 +124,14 @@ def exists(self, version_number: int) -> ConfigVersion: def _get_next_version(self, config_id: UUID) -> int | None: """Get the next version number for a config.""" - statement = select(func.max(ConfigVersion.version)).where( - and_( - ConfigVersion.config_id == config_id, - ) + stmt = ( + select(ConfigVersion.version) + .where(ConfigVersion.config_id == config_id) + .order_by(ConfigVersion.version.desc()) + .limit(1) ) - return self.session.exec(statement).one() + 1 + latest = self.session.exec(stmt).first() + return latest + 1 def _config_exists(self, config_id: UUID) -> Config: """Check if a config exists in the project.""" diff --git a/backend/app/models/config/config.py b/backend/app/models/config/config.py index 2e972e84b..f13789802 100644 --- a/backend/app/models/config/config.py +++ b/backend/app/models/config/config.py @@ -2,7 +2,7 @@ from datetime import datetime from typing import TYPE_CHECKING, Any -from sqlmodel import Field, SQLModel, UniqueConstraint +from sqlmodel import Field, SQLModel, UniqueConstraint, Index, text from pydantic import field_validator from app.core.util import now @@ -12,9 +12,7 @@ class ConfigBase(SQLModel): """Base model for LLM configuration metadata""" - name: str = Field( - index=True, min_length=1, max_length=128, description="Config name" - ) + name: str = Field(min_length=1, max_length=128, description="Config name") description: str | None = Field( default=None, max_length=512, description="Optional description" ) @@ -25,10 +23,18 @@ class Config(ConfigBase, table=True): __tablename__ = "config" __table_args__ = ( - UniqueConstraint( + Index( + "uq_config_project_id_name_active", "project_id", "name", - "deleted_at", + unique=True, + postgresql_where=text("deleted_at IS NULL"), + ), + Index( + "idx_config_project_id_updated_at_active", + "project_id", + "updated_at", + postgresql_where=text("deleted_at IS NULL"), ), ) @@ -36,7 +42,6 @@ class Config(ConfigBase, table=True): project_id: int = Field( foreign_key="project.id", - index=True, nullable=False, ondelete="CASCADE", ) diff --git a/backend/app/models/config/version.py b/backend/app/models/config/version.py index 6a1bdc2ed..0169b048f 100644 --- a/backend/app/models/config/version.py +++ b/backend/app/models/config/version.py @@ -4,15 +4,15 @@ from pydantic import field_validator import sqlalchemy as sa -from sqlalchemy.dialects.postgresql import JSON -from sqlmodel import Field, SQLModel, UniqueConstraint +from sqlalchemy.dialects.postgresql import JSONB +from sqlmodel import Field, SQLModel, UniqueConstraint, Index, text from app.core.util import now class ConfigVersionBase(SQLModel): config_blob: dict[str, Any] = Field( - sa_column=sa.Column(JSON, nullable=False), + sa_column=sa.Column(JSONB, nullable=False), description="Provider-specific configuration parameters (temperature, max_tokens, etc.)", ) commit_message: str | None = Field( @@ -32,8 +32,13 @@ class ConfigVersion(ConfigVersionBase, table=True): __tablename__ = "config_version" __table_args__ = ( UniqueConstraint( + "config_id", "version", name="uq_config_version_config_id_version" + ), + Index( + "idx_config_version_config_id_version_active", "config_id", "version", + postgresql_where=text("deleted_at IS NULL"), ), ) @@ -41,7 +46,6 @@ class ConfigVersion(ConfigVersionBase, table=True): config_id: UUID = Field( foreign_key="config.id", - index=True, nullable=False, ondelete="CASCADE", ) diff --git a/backend/uv.lock b/backend/uv.lock index 45f551a1a..b1da74249 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -240,7 +240,7 @@ requires-dist = [ { name = "langfuse", specifier = "==2.60.3" }, { name = "moto", extras = ["s3"], specifier = ">=5.1.1" }, { name = "numpy", specifier = ">=1.24.0" }, - { name = "openai", specifier = ">=1.67.0" }, + { name = "openai", specifier = ">=1.100.1" }, { name = "openai-responses" }, { name = "pandas", specifier = ">=2.3.2" }, { name = "passlib", extras = ["bcrypt"], specifier = ">=1.7.4,<2.0.0" }, From 6b460e21c6f7eff77a00a03a823568efaa99d49e Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 13 Nov 2025 16:04:23 +0530 Subject: [PATCH 16/20] precommit --- .../f245ccb27ab6_config_management_tables.py | 112 ++++++++++++------ 1 file changed, 77 insertions(+), 35 deletions(-) diff --git a/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py b/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py index 7f0c1ba15..bd313e202 100644 --- a/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py +++ b/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py @@ -11,46 +11,88 @@ from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. -revision = 'f245ccb27ab6' -down_revision = '6fe772038a5a' +revision = "f245ccb27ab6" +down_revision = "6fe772038a5a" branch_labels = None depends_on = None def upgrade(): - op.create_table('config', - sa.Column('name', sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), - sa.Column('description', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), - sa.Column('id', sa.Uuid(), nullable=False), - sa.Column('project_id', sa.Integer(), nullable=False), - sa.Column('inserted_at', sa.DateTime(), nullable=False), - sa.Column('updated_at', sa.DateTime(), nullable=False), - sa.Column('deleted_at', sa.DateTime(), nullable=True), - sa.ForeignKeyConstraint(['project_id'], ['project.id'], ondelete='CASCADE'), - sa.PrimaryKeyConstraint('id') - ) - op.create_index('idx_config_project_id_updated_at_active', 'config', ['project_id', 'updated_at'], unique=False, postgresql_where=sa.text('deleted_at IS NULL')) - op.create_index('uq_config_project_id_name_active', 'config', ['project_id', 'name'], unique=True, postgresql_where=sa.text('deleted_at IS NULL')) - op.create_table('config_version', - sa.Column('config_blob', postgresql.JSONB(astext_type=sa.Text()), nullable=False), - sa.Column('commit_message', sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True), - sa.Column('id', sa.Uuid(), nullable=False), - sa.Column('config_id', sa.Uuid(), nullable=False), - sa.Column('version', sa.Integer(), nullable=False), - sa.Column('inserted_at', sa.DateTime(), nullable=False), - sa.Column('updated_at', sa.DateTime(), nullable=False), - sa.Column('deleted_at', sa.DateTime(), nullable=True), - sa.ForeignKeyConstraint(['config_id'], ['config.id'], ondelete='CASCADE'), - sa.PrimaryKeyConstraint('id'), - sa.UniqueConstraint('config_id', 'version', name='uq_config_version_config_id_version') - ) - op.create_index('idx_config_version_config_id_version_active', 'config_version', ['config_id', 'version'], unique=False, postgresql_where=sa.text('deleted_at IS NULL')) + op.create_table( + "config", + sa.Column("name", sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), + sa.Column( + "description", sqlmodel.sql.sqltypes.AutoString(length=512), nullable=True + ), + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("project_id", sa.Integer(), nullable=False), + sa.Column("inserted_at", sa.DateTime(), nullable=False), + sa.Column("updated_at", sa.DateTime(), nullable=False), + sa.Column("deleted_at", sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(["project_id"], ["project.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index( + "idx_config_project_id_updated_at_active", + "config", + ["project_id", "updated_at"], + unique=False, + postgresql_where=sa.text("deleted_at IS NULL"), + ) + op.create_index( + "uq_config_project_id_name_active", + "config", + ["project_id", "name"], + unique=True, + postgresql_where=sa.text("deleted_at IS NULL"), + ) + op.create_table( + "config_version", + sa.Column( + "config_blob", postgresql.JSONB(astext_type=sa.Text()), nullable=False + ), + sa.Column( + "commit_message", + sqlmodel.sql.sqltypes.AutoString(length=512), + nullable=True, + ), + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("config_id", sa.Uuid(), nullable=False), + sa.Column("version", sa.Integer(), nullable=False), + sa.Column("inserted_at", sa.DateTime(), nullable=False), + sa.Column("updated_at", sa.DateTime(), nullable=False), + sa.Column("deleted_at", sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(["config_id"], ["config.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint( + "config_id", "version", name="uq_config_version_config_id_version" + ), + ) + op.create_index( + "idx_config_version_config_id_version_active", + "config_version", + ["config_id", "version"], + unique=False, + postgresql_where=sa.text("deleted_at IS NULL"), + ) def downgrade(): - op.drop_index('idx_config_version_config_id_version_active', table_name='config_version', postgresql_where=sa.text('deleted_at IS NULL')) - op.drop_table('config_version') - op.drop_index('uq_config_project_id_name_active', table_name='config', postgresql_where=sa.text('deleted_at IS NULL')) - op.drop_index(op.f('ix_config_name'), table_name='config') - op.drop_index('idx_config_project_id_updated_at_active', table_name='config', postgresql_where=sa.text('deleted_at IS NULL')) - op.drop_table('config') + op.drop_index( + "idx_config_version_config_id_version_active", + table_name="config_version", + postgresql_where=sa.text("deleted_at IS NULL"), + ) + op.drop_table("config_version") + op.drop_index( + "uq_config_project_id_name_active", + table_name="config", + postgresql_where=sa.text("deleted_at IS NULL"), + ) + op.drop_index(op.f("ix_config_name"), table_name="config") + op.drop_index( + "idx_config_project_id_updated_at_active", + table_name="config", + postgresql_where=sa.text("deleted_at IS NULL"), + ) + op.drop_table("config") From 07f2440671217d71d819d9282c0127442c8c971d Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 13 Nov 2025 16:18:50 +0530 Subject: [PATCH 17/20] fix migration --- .../alembic/versions/f245ccb27ab6_config_management_tables.py | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py b/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py index bd313e202..3caf0ff8c 100644 --- a/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py +++ b/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py @@ -89,7 +89,6 @@ def downgrade(): table_name="config", postgresql_where=sa.text("deleted_at IS NULL"), ) - op.drop_index(op.f("ix_config_name"), table_name="config") op.drop_index( "idx_config_project_id_updated_at_active", table_name="config", From 0483759ac6c52456e354595f017ed586cc35ff49 Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 19 Nov 2025 13:18:30 +0530 Subject: [PATCH 18/20] Update migration and resolve comments --- ... => ecda6b144627_config_management_tables.py} | 14 +++++++++----- backend/app/api/routes/config/config.py | 12 +++++------- backend/app/api/routes/config/version.py | 16 +++++----------- backend/app/crud/config/version.py | 3 +++ backend/app/models/config/__init__.py | 2 ++ 5 files changed, 24 insertions(+), 23 deletions(-) rename backend/app/alembic/versions/{f245ccb27ab6_config_management_tables.py => ecda6b144627_config_management_tables.py} (89%) diff --git a/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py b/backend/app/alembic/versions/ecda6b144627_config_management_tables.py similarity index 89% rename from backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py rename to backend/app/alembic/versions/ecda6b144627_config_management_tables.py index 3caf0ff8c..378d232c9 100644 --- a/backend/app/alembic/versions/f245ccb27ab6_config_management_tables.py +++ b/backend/app/alembic/versions/ecda6b144627_config_management_tables.py @@ -1,8 +1,8 @@ """Config management tables -Revision ID: f245ccb27ab6 -Revises: 6fe772038a5a -Create Date: 2025-11-13 16:02:08.465124 +Revision ID: ecda6b144627 +Revises: 633e69806207 +Create Date: 2025-11-19 13:16:50.954576 """ from alembic import op @@ -11,13 +11,14 @@ from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. -revision = "f245ccb27ab6" -down_revision = "6fe772038a5a" +revision = "ecda6b144627" +down_revision = "633e69806207" branch_labels = None depends_on = None def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### op.create_table( "config", sa.Column("name", sqlmodel.sql.sqltypes.AutoString(length=128), nullable=False), @@ -75,9 +76,11 @@ def upgrade(): unique=False, postgresql_where=sa.text("deleted_at IS NULL"), ) + # ### end Alembic commands ### def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### op.drop_index( "idx_config_version_config_id_version_active", table_name="config_version", @@ -95,3 +98,4 @@ def downgrade(): postgresql_where=sa.text("deleted_at IS NULL"), ) op.drop_table("config") + # ### end Alembic commands ### diff --git a/backend/app/api/routes/config/config.py b/backend/app/api/routes/config/config.py index 4cbe43e67..78bb7fe13 100644 --- a/backend/app/api/routes/config/config.py +++ b/backend/app/api/routes/config/config.py @@ -24,7 +24,7 @@ status_code=201, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) -def create_config_route( +def create_config( config_create: ConfigCreate, current_user: AuthContextDep, session: SessionDep, @@ -48,7 +48,7 @@ def create_config_route( status_code=200, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) -def list_configs_route( +def list_configs( current_user: AuthContextDep, session: SessionDep, skip: int = Query(0, ge=0, description="Number of records to skip"), @@ -58,8 +58,6 @@ def list_configs_route( List all configurations for the current project. Ordered by updated_at in descending order. """ - - # Decide how to handle pagination effectively config_crud = ConfigCrud(session=session, project_id=current_user.project.id) configs = config_crud.read_all(skip=skip, limit=limit) return APIResponse.success_response( @@ -73,7 +71,7 @@ def list_configs_route( status_code=200, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) -def get_config_route( +def get_config( config_id: UUID, current_user: AuthContextDep, session: SessionDep, @@ -94,7 +92,7 @@ def get_config_route( status_code=200, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) -def update_config_route( +def update_config( config_id: UUID, config_update: ConfigUpdate, current_user: AuthContextDep, @@ -117,7 +115,7 @@ def update_config_route( status_code=200, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) -def delete_config_route( +def delete_config( config_id: UUID, current_user: AuthContextDep, session: SessionDep, diff --git a/backend/app/api/routes/config/version.py b/backend/app/api/routes/config/version.py index 4a3a703d6..b65840ad7 100644 --- a/backend/app/api/routes/config/version.py +++ b/backend/app/api/routes/config/version.py @@ -21,7 +21,7 @@ status_code=201, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) -def create_version_route( +def create_version( config_id: UUID, version_create: ConfigVersionCreate, current_user: AuthContextDep, @@ -47,7 +47,7 @@ def create_version_route( status_code=200, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) -def list_versions_route( +def list_versions( config_id: UUID, current_user: AuthContextDep, session: SessionDep, @@ -76,7 +76,7 @@ def list_versions_route( status_code=200, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) -def get_version_route( +def get_version( config_id: UUID, current_user: AuthContextDep, session: SessionDep, @@ -90,13 +90,7 @@ def get_version_route( version_crud = ConfigVersionCrud( session=session, project_id=current_user.project.id, config_id=config_id ) - version = version_crud.read_one(version_number=version_number) - if version is None: - raise HTTPException( - status_code=404, - detail=f"Config Version with number '{version_number}' not found for config '{config_id}'", - ) - + version = version_crud.exists(version_number=version_number) return APIResponse.success_response( data=version, ) @@ -108,7 +102,7 @@ def get_version_route( status_code=200, dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) -def delete_version_route( +def delete_version( config_id: UUID, current_user: AuthContextDep, session: SessionDep, diff --git a/backend/app/crud/config/version.py b/backend/app/crud/config/version.py index 030bfe8c4..941aa1e0a 100644 --- a/backend/app/crud/config/version.py +++ b/backend/app/crud/config/version.py @@ -131,6 +131,9 @@ def _get_next_version(self, config_id: UUID) -> int | None: .limit(1) ) latest = self.session.exec(stmt).first() + if latest is None: + return 1 + return latest + 1 def _config_exists(self, config_id: UUID) -> Config: diff --git a/backend/app/models/config/__init__.py b/backend/app/models/config/__init__.py index e71b95759..fa34aa1d6 100644 --- a/backend/app/models/config/__init__.py +++ b/backend/app/models/config/__init__.py @@ -23,5 +23,7 @@ "ConfigVersion", "ConfigVersionBase", "ConfigVersionCreate", + "ConfigVersionItems", "ConfigVersionPublic", + "ConfigWithVersion", ] From c248f05637b346e050217e07f37031f79f277097 Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Fri, 21 Nov 2025 09:56:40 +0530 Subject: [PATCH 19/20] Refactor config and version CRUD methods to use 'or_raise' for better error handling --- backend/app/api/routes/config/config.py | 10 ++-- backend/app/api/routes/config/version.py | 6 +-- backend/app/crud/config/config.py | 18 +++---- backend/app/crud/config/version.py | 18 +++---- backend/app/tests/crud/config/test_config.py | 48 ++++++++++--------- backend/app/tests/crud/config/test_version.py | 30 ++++++------ backend/app/tests/utils/test_data.py | 4 +- 7 files changed, 70 insertions(+), 64 deletions(-) diff --git a/backend/app/api/routes/config/config.py b/backend/app/api/routes/config/config.py index 78bb7fe13..cdb71e981 100644 --- a/backend/app/api/routes/config/config.py +++ b/backend/app/api/routes/config/config.py @@ -33,7 +33,7 @@ def create_config( create new config along with initial version """ config_crud = ConfigCrud(session=session, project_id=current_user.project.id) - config, version = config_crud.create(config_create) + config, version = config_crud.create_or_raise(config_create) response = ConfigWithVersion(**config.model_dump(), version=version) @@ -80,7 +80,7 @@ def get_config( Get a specific configuration by its ID. """ config_crud = ConfigCrud(session=session, project_id=current_user.project.id) - config = config_crud.exists(config_id=config_id) + config = config_crud.exists_or_raise(config_id=config_id) return APIResponse.success_response( data=config, ) @@ -102,7 +102,9 @@ def update_config( Update a specific configuration. """ config_crud = ConfigCrud(session=session, project_id=current_user.project.id) - config = config_crud.update(config_id=config_id, config_update=config_update) + config = config_crud.update_or_raise( + config_id=config_id, config_update=config_update + ) return APIResponse.success_response( data=config, @@ -124,7 +126,7 @@ def delete_config( Delete a specific configuration. """ config_crud = ConfigCrud(session=session, project_id=current_user.project.id) - config_crud.delete(config_id=config_id) + config_crud.delete_or_raise(config_id=config_id) return APIResponse.success_response( data=Message(message="Config deleted successfully"), diff --git a/backend/app/api/routes/config/version.py b/backend/app/api/routes/config/version.py index b65840ad7..aef49ae41 100644 --- a/backend/app/api/routes/config/version.py +++ b/backend/app/api/routes/config/version.py @@ -34,7 +34,7 @@ def create_version( version_crud = ConfigVersionCrud( session=session, project_id=current_user.project.id, config_id=config_id ) - version = version_crud.create(version_create=version_create) + version = version_crud.create_or_raise(version_create=version_create) return APIResponse.success_response( data=ConfigVersionPublic(**version.model_dump()), @@ -90,7 +90,7 @@ def get_version( version_crud = ConfigVersionCrud( session=session, project_id=current_user.project.id, config_id=config_id ) - version = version_crud.exists(version_number=version_number) + version = version_crud.exists_or_raise(version_number=version_number) return APIResponse.success_response( data=version, ) @@ -116,7 +116,7 @@ def delete_version( version_crud = ConfigVersionCrud( session=session, project_id=current_user.project.id, config_id=config_id ) - version_crud.delete(version_number=version_number) + version_crud.delete_or_raise(version_number=version_number) return APIResponse.success_response( data=Message(message="Config Version deleted successfully"), diff --git a/backend/app/crud/config/config.py b/backend/app/crud/config/config.py index 292f0dd56..51927d8af 100644 --- a/backend/app/crud/config/config.py +++ b/backend/app/crud/config/config.py @@ -25,11 +25,13 @@ def __init__(self, session: Session, project_id: int): self.session = session self.project_id = project_id - def create(self, config_create: ConfigCreate) -> Tuple[Config, ConfigVersion]: + def create_or_raise( + self, config_create: ConfigCreate + ) -> Tuple[Config, ConfigVersion]: """ Create a new configuration with an initial version. """ - self._check_unique_name(config_create.name) + self._check_unique_name_or_raise(config_create.name) try: config = Config( @@ -98,8 +100,8 @@ def read_all(self, skip: int = 0, limit: int = 100) -> list[Config]: ) return self.session.exec(statement).all() - def update(self, config_id: UUID, config_update: ConfigUpdate) -> Config: - config = self.exists(config_id) + def update_or_raise(self, config_id: UUID, config_update: ConfigUpdate) -> Config: + config = self.exists_or_raise(config_id) config_update = config_update.model_dump(exclude_none=True) @@ -121,15 +123,15 @@ def update(self, config_id: UUID, config_update: ConfigUpdate) -> Config: ) return config - def delete(self, config_id: UUID) -> None: - config = self.exists(config_id) + def delete_or_raise(self, config_id: UUID) -> None: + config = self.exists_or_raise(config_id) config.deleted_at = now() self.session.add(config) self.session.commit() self.session.refresh(config) - def exists(self, config_id: UUID) -> Config: + def exists_or_raise(self, config_id: UUID) -> Config: config = self.read_one(config_id) if config is None: raise HTTPException( @@ -139,7 +141,7 @@ def exists(self, config_id: UUID) -> Config: return config - def _check_unique_name(self, name: str) -> None: + def _check_unique_name_or_raise(self, name: str) -> None: if self._read_by_name(name): raise HTTPException( status_code=409, diff --git a/backend/app/crud/config/version.py b/backend/app/crud/config/version.py index 941aa1e0a..cf4a3ae20 100644 --- a/backend/app/crud/config/version.py +++ b/backend/app/crud/config/version.py @@ -22,12 +22,12 @@ def __init__(self, session: Session, config_id: UUID, project_id: int): self.project_id = project_id self.config_id = config_id - def create(self, version_create: ConfigVersionCreate) -> ConfigVersion: + def create_or_raise(self, version_create: ConfigVersionCreate) -> ConfigVersion: """ Create a new version for an existing configuration. Automatically increments the version number. """ - self._config_exists(self.config_id) + self._config_exists_or_raise(self.config_id) try: next_version = self._get_next_version(self.config_id) @@ -65,7 +65,7 @@ def read_one(self, version_number: int) -> ConfigVersion | None: """ Read a specific configuration version by its version number. """ - self._config_exists(self.config_id) + self._config_exists_or_raise(self.config_id) statement = select(ConfigVersion).where( and_( ConfigVersion.version == version_number, @@ -79,7 +79,7 @@ def read_all(self, skip: int = 0, limit: int = 100) -> list[ConfigVersionItems]: """ Read all versions for a specific configuration with pagination. """ - self._config_exists(self.config_id) + self._config_exists_or_raise(self.config_id) statement = ( select(ConfigVersion) @@ -99,18 +99,18 @@ def read_all(self, skip: int = 0, limit: int = 100) -> list[ConfigVersionItems]: results = self.session.exec(statement).all() return [ConfigVersionItems.model_validate(item) for item in results] - def delete(self, version_number: int) -> None: + def delete_or_raise(self, version_number: int) -> None: """ Soft delete a configuration version by setting its deleted_at timestamp. """ - version = self.exists(version_number) + version = self.exists_or_raise(version_number) version.deleted_at = now() self.session.add(version) self.session.commit() self.session.refresh(version) - def exists(self, version_number: int) -> ConfigVersion: + def exists_or_raise(self, version_number: int) -> ConfigVersion: """ Check if a configuration version exists; raise 404 if not found. """ @@ -136,7 +136,7 @@ def _get_next_version(self, config_id: UUID) -> int | None: return latest + 1 - def _config_exists(self, config_id: UUID) -> Config: + def _config_exists_or_raise(self, config_id: UUID) -> Config: """Check if a config exists in the project.""" config_crud = ConfigCrud(session=self.session, project_id=self.project_id) - config_crud.exists(config_id) + config_crud.exists_or_raise(config_id) diff --git a/backend/app/tests/crud/config/test_config.py b/backend/app/tests/crud/config/test_config.py index febaeb896..e63567a6d 100644 --- a/backend/app/tests/crud/config/test_config.py +++ b/backend/app/tests/crud/config/test_config.py @@ -31,7 +31,7 @@ def test_create_config(db: Session) -> None: commit_message="Initial version", ) - config, version = config_crud.create(config_create) + config, version = config_crud.create_or_raise(config_create) assert config.id is not None assert config.name == config_name @@ -61,13 +61,13 @@ def test_create_config_duplicate_name(db: Session) -> None: ) # Create first config - config_crud.create(config_create) + config_crud.create_or_raise(config_create) # Attempt to create second config with same name with pytest.raises( HTTPException, match=f"Config with name '{config_name}' already exists" ): - config_crud.create(config_create) + config_crud.create_or_raise(config_create) def test_create_config_different_projects_same_name(db: Session) -> None: @@ -86,11 +86,11 @@ def test_create_config_different_projects_same_name(db: Session) -> None: config_blob=config_blob, commit_message="Initial version", ) - config1, _ = config_crud1.create(config_create) + config1, _ = config_crud1.create_or_raise(config_create) # Create config with same name in project2 config_crud2 = ConfigCrud(session=db, project_id=project2.id) - config2, _ = config_crud2.create(config_create) + config2, _ = config_crud2.create_or_raise(config_create) assert config1.id != config2.id assert config1.name == config2.name == config_name @@ -142,7 +142,7 @@ def test_read_one_deleted_config(db: Session) -> None: config_crud = ConfigCrud(session=db, project_id=config.project_id) # Delete the config - config_crud.delete(config.id) + config_crud.delete_or_raise(config.id) # Try to read deleted config fetched_config = config_crud.read_one(config.id) @@ -198,7 +198,9 @@ def test_read_all_configs_ordered_by_updated_at(db: Session) -> None: config3 = create_test_config(db, project_id=project.id, name="config-3") # Update config1 to make it the most recently updated - config_crud.update(config1.id, ConfigUpdate(description="Updated description")) + config_crud.update_or_raise( + config1.id, ConfigUpdate(description="Updated description") + ) configs = config_crud.read_all() @@ -216,7 +218,7 @@ def test_read_all_configs_excludes_deleted(db: Session) -> None: config_crud = ConfigCrud(session=db, project_id=project.id) # Delete config1 - config_crud.delete(config1.id) + config_crud.delete_or_raise(config1.id) configs = config_crud.read_all() @@ -249,7 +251,7 @@ def test_update_config_name(db: Session) -> None: new_name = f"updated-config-{random_lower_string()}" config_update = ConfigUpdate(name=new_name) - updated_config = config_crud.update(config.id, config_update) + updated_config = config_crud.update_or_raise(config.id, config_update) assert updated_config.name == new_name assert updated_config.id == config.id @@ -263,7 +265,7 @@ def test_update_config_description(db: Session) -> None: new_description = "Updated description" config_update = ConfigUpdate(description=new_description) - updated_config = config_crud.update(config.id, config_update) + updated_config = config_crud.update_or_raise(config.id, config_update) assert updated_config.description == new_description assert updated_config.id == config.id @@ -278,7 +280,7 @@ def test_update_config_multiple_fields(db: Session) -> None: new_description = "Updated description" config_update = ConfigUpdate(name=new_name, description=new_description) - updated_config = config_crud.update(config.id, config_update) + updated_config = config_crud.update_or_raise(config.id, config_update) assert updated_config.name == new_name assert updated_config.description == new_description @@ -299,7 +301,7 @@ def test_update_config_duplicate_name(db: Session) -> None: with pytest.raises( HTTPException, match=f"Config with name '{config1.name}' already exists" ): - config_crud.update(config2.id, config_update) + config_crud.update_or_raise(config2.id, config_update) def test_update_config_same_name(db: Session) -> None: @@ -310,7 +312,7 @@ def test_update_config_same_name(db: Session) -> None: # Update to same name should succeed config_update = ConfigUpdate(name=config.name, description="Updated") - updated_config = config_crud.update(config.id, config_update) + updated_config = config_crud.update_or_raise(config.id, config_update) assert updated_config.name == config.name assert updated_config.description == "Updated" @@ -327,7 +329,7 @@ def test_update_config_not_found(db: Session) -> None: with pytest.raises( HTTPException, match=f"config with id '{non_existent_id}' not found" ): - config_crud.update(non_existent_id, config_update) + config_crud.update_or_raise(non_existent_id, config_update) def test_update_config_updates_timestamp(db: Session) -> None: @@ -338,7 +340,7 @@ def test_update_config_updates_timestamp(db: Session) -> None: original_updated_at = config.updated_at config_update = ConfigUpdate(description="Updated description") - updated_config = config_crud.update(config.id, config_update) + updated_config = config_crud.update_or_raise(config.id, config_update) assert updated_config.updated_at > original_updated_at @@ -348,7 +350,7 @@ def test_delete_config(db: Session) -> None: config = create_test_config(db) config_crud = ConfigCrud(session=db, project_id=config.project_id) - config_crud.delete(config.id) + config_crud.delete_or_raise(config.id) # Verify soft delete (deleted_at is set) db.refresh(config) @@ -365,7 +367,7 @@ def test_delete_config_not_found(db: Session) -> None: with pytest.raises( HTTPException, match=f"config with id '{non_existent_id}' not found" ): - config_crud.delete(non_existent_id) + config_crud.delete_or_raise(non_existent_id) def test_delete_config_different_project(db: Session) -> None: @@ -378,7 +380,7 @@ def test_delete_config_different_project(db: Session) -> None: config_crud = ConfigCrud(session=db, project_id=project2.id) with pytest.raises(HTTPException, match=f"config with id '{config.id}' not found"): - config_crud.delete(config.id) + config_crud.delete_or_raise(config.id) def test_exists_config(db: Session) -> None: @@ -386,7 +388,7 @@ def test_exists_config(db: Session) -> None: config = create_test_config(db) config_crud = ConfigCrud(session=db, project_id=config.project_id) - existing_config = config_crud.exists(config.id) + existing_config = config_crud.exists_or_raise(config.id) assert existing_config.id == config.id assert existing_config.name == config.name @@ -402,7 +404,7 @@ def test_exists_config_not_found(db: Session) -> None: with pytest.raises( HTTPException, match=f"config with id '{non_existent_id}' not found" ): - config_crud.exists(non_existent_id) + config_crud.exists_or_raise(non_existent_id) def test_exists_deleted_config(db: Session) -> None: @@ -411,11 +413,11 @@ def test_exists_deleted_config(db: Session) -> None: config_crud = ConfigCrud(session=db, project_id=config.project_id) # Delete the config - config_crud.delete(config.id) + config_crud.delete_or_raise(config.id) # exists should raise HTTPException with pytest.raises(HTTPException, match=f"config with id '{config.id}' not found"): - config_crud.exists(config.id) + config_crud.exists_or_raise(config.id) def test_check_unique_name_with_existing_name(db: Session) -> None: @@ -468,7 +470,7 @@ def test_read_by_name_deleted_config(db: Session) -> None: config_crud = ConfigCrud(session=db, project_id=config.project_id) # Delete the config - config_crud.delete(config.id) + config_crud.delete_or_raise(config.id) # Should return None fetched_config = config_crud._read_by_name(config.name) diff --git a/backend/app/tests/crud/config/test_version.py b/backend/app/tests/crud/config/test_version.py index dada05ce9..d62265d5e 100644 --- a/backend/app/tests/crud/config/test_version.py +++ b/backend/app/tests/crud/config/test_version.py @@ -29,7 +29,7 @@ def test_create_version(db: Session) -> None: commit_message="Updated model and parameters", ) - version = version_crud.create(version_create) + version = version_crud.create_or_raise(version_create) assert version.id is not None assert version.config_id == config.id @@ -47,13 +47,13 @@ def test_create_version_auto_increment(db: Session) -> None: ) # Create multiple versions - version2 = version_crud.create( + version2 = version_crud.create_or_raise( ConfigVersionCreate(config_blob={"model": "gpt-4"}, commit_message="Version 2") ) - version3 = version_crud.create( + version3 = version_crud.create_or_raise( ConfigVersionCreate(config_blob={"model": "gpt-4"}, commit_message="Version 3") ) - version4 = version_crud.create( + version4 = version_crud.create_or_raise( ConfigVersionCreate(config_blob={"model": "gpt-4"}, commit_message="Version 4") ) @@ -78,7 +78,7 @@ def test_create_version_config_not_found(db: Session) -> None: with pytest.raises( HTTPException, match=f"config with id '{non_existent_config_id}' not found" ): - version_crud.create(version_create) + version_crud.create_or_raise(version_create) def test_read_one_version(db: Session) -> None: @@ -128,7 +128,7 @@ def test_read_one_version_deleted(db: Session) -> None: ) # Delete the version - version_crud.delete(version.version) + version_crud.delete_or_raise(version.version) # Try to read deleted version fetched_version = version_crud.read_one(version.version) @@ -267,7 +267,7 @@ def test_read_all_versions_excludes_deleted(db: Session) -> None: ) # Delete version 2 - version_crud.delete(version2.version) + version_crud.delete_or_raise(version2.version) versions = version_crud.read_all() @@ -286,7 +286,7 @@ def test_delete_version(db: Session) -> None: session=db, project_id=config.project_id, config_id=config.id ) - version_crud.delete(version.version) + version_crud.delete_or_raise(version.version) # Verify soft delete (deleted_at is set) db.refresh(version) @@ -306,7 +306,7 @@ def test_delete_version_not_found(db: Session) -> None: HTTPException, match=f"Version with number '{non_existent_version}' not found for config '{config.id}'", ): - version_crud.delete(non_existent_version) + version_crud.delete_or_raise(non_existent_version) def test_exists_version(db: Session) -> None: @@ -318,7 +318,7 @@ def test_exists_version(db: Session) -> None: session=db, project_id=config.project_id, config_id=config.id ) - existing_version = version_crud.exists(version.version) + existing_version = version_crud.exists_or_raise(version.version) assert existing_version.id == version.id assert existing_version.version == version.version @@ -337,7 +337,7 @@ def test_exists_version_not_found(db: Session) -> None: HTTPException, match=f"Version with number '{non_existent_version}' not found for config '{config.id}'", ): - version_crud.exists(non_existent_version) + version_crud.exists_or_raise(non_existent_version) def test_exists_version_deleted(db: Session) -> None: @@ -350,14 +350,14 @@ def test_exists_version_deleted(db: Session) -> None: ) # Delete the version - version_crud.delete(version.version) + version_crud.delete_or_raise(version.version) # exists should raise HTTPException with pytest.raises( HTTPException, match=f"Version with number '{version.version}' not found for config '{config.id}'", ): - version_crud.exists(version.version) + version_crud.exists_or_raise(version.version) def test_create_version_different_configs(db: Session) -> None: @@ -372,7 +372,7 @@ def test_create_version_different_configs(db: Session) -> None: version_crud1 = ConfigVersionCrud( session=db, project_id=project.id, config_id=config1.id ) - version2_config1 = version_crud1.create( + version2_config1 = version_crud1.create_or_raise( ConfigVersionCreate(config_blob={"model": "gpt-4"}, commit_message="V2") ) @@ -380,7 +380,7 @@ def test_create_version_different_configs(db: Session) -> None: version_crud2 = ConfigVersionCrud( session=db, project_id=project.id, config_id=config2.id ) - version2_config2 = version_crud2.create( + version2_config2 = version_crud2.create_or_raise( ConfigVersionCreate(config_blob={"model": "gpt-4"}, commit_message="V2") ) diff --git a/backend/app/tests/utils/test_data.py b/backend/app/tests/utils/test_data.py index c663cf014..abb62f542 100644 --- a/backend/app/tests/utils/test_data.py +++ b/backend/app/tests/utils/test_data.py @@ -267,7 +267,7 @@ def create_test_config( ) config_crud = ConfigCrud(session=db, project_id=project_id) - config, version = config_crud.create(config_create) + config, version = config_crud.create_or_raise(config_create) return config @@ -299,6 +299,6 @@ def create_test_version( version_crud = ConfigVersionCrud( session=db, project_id=project_id, config_id=config_id ) - version = version_crud.create(version_create=version_create) + version = version_crud.create_or_raise(version_create=version_create) return version From de674bb5e65c5d27d40ee814a77047d3155fcb5f Mon Sep 17 00:00:00 2001 From: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Date: Fri, 21 Nov 2025 10:12:31 +0530 Subject: [PATCH 20/20] Refactor unique name checks to use 'or_raise' for improved error handling --- backend/app/crud/config/config.py | 2 +- backend/app/tests/crud/config/test_config.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/app/crud/config/config.py b/backend/app/crud/config/config.py index 51927d8af..00ac3b927 100644 --- a/backend/app/crud/config/config.py +++ b/backend/app/crud/config/config.py @@ -106,7 +106,7 @@ def update_or_raise(self, config_id: UUID, config_update: ConfigUpdate) -> Confi config_update = config_update.model_dump(exclude_none=True) if config_update.get("name") and config_update["name"] != config.name: - self._check_unique_name(config_update["name"]) + self._check_unique_name_or_raise(config_update["name"]) for key, value in config_update.items(): setattr(config, key, value) diff --git a/backend/app/tests/crud/config/test_config.py b/backend/app/tests/crud/config/test_config.py index e63567a6d..6b753e0ba 100644 --- a/backend/app/tests/crud/config/test_config.py +++ b/backend/app/tests/crud/config/test_config.py @@ -428,7 +428,7 @@ def test_check_unique_name_with_existing_name(db: Session) -> None: with pytest.raises( HTTPException, match=f"Config with name '{config.name}' already exists" ): - config_crud._check_unique_name(config.name) + config_crud._check_unique_name_or_raise(config.name) def test_check_unique_name_with_new_name(db: Session) -> None: @@ -438,7 +438,7 @@ def test_check_unique_name_with_new_name(db: Session) -> None: # Should not raise exception unique_name = f"unique-name-{random_lower_string()}" - config_crud._check_unique_name(unique_name) + config_crud._check_unique_name_or_raise(unique_name) def test_read_by_name(db: Session) -> None: