From d7d167217bc0aea1187a76463725669490f2bd85 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 5 Jul 2026 20:43:54 +0900 Subject: [PATCH 01/10] fix(alembic): repair broken revision graph 0003_revoked_token pointed at nonexistent down_revision "0002" and the tree had two heads. Repoint to 0002_auth_share and add 0004_merge_heads merging 0003 + 0003_validate_project_space_fk. Verified: alembic upgrade head runs the full chain against Postgres. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AxU2xaupAjp912oDNFuWyd --- .../alembic/versions/0003_revoked_token.py | 4 +-- backend/alembic/versions/0004_merge_heads.py | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 backend/alembic/versions/0004_merge_heads.py diff --git a/backend/alembic/versions/0003_revoked_token.py b/backend/alembic/versions/0003_revoked_token.py index b7bcce29..88296542 100644 --- a/backend/alembic/versions/0003_revoked_token.py +++ b/backend/alembic/versions/0003_revoked_token.py @@ -1,7 +1,7 @@ """revoked_token Revision ID: 0003 -Revises: 0002 +Revises: 0002_auth_share Create Date: 2026-06-22 10:00:00.000000 """ @@ -12,7 +12,7 @@ # revision identifiers, used by Alembic. revision = "0003" -down_revision = "0002" +down_revision = "0002_auth_share" branch_labels = None depends_on = None diff --git a/backend/alembic/versions/0004_merge_heads.py b/backend/alembic/versions/0004_merge_heads.py new file mode 100644 index 00000000..14a457f9 --- /dev/null +++ b/backend/alembic/versions/0004_merge_heads.py @@ -0,0 +1,28 @@ +"""merge revoked_token and validate_project_space_fk into a single head + +Two migrations branched independently from 0002_auth_share: + - 0003 (revoked_token): creates the revoked_token table + - 0003_validate_project_space_fk: validates a project_space FK +They are unrelated and safe to apply in any order, so this is a no-op merge +that unifies the graph to a single head (``alembic upgrade head`` was ambiguous +with two heads). + +Revision ID: 0004_merge_heads +Revises: 0003, 0003_validate_project_space_fk +Create Date: 2026-07-05 00:00:00.000000 + +""" + +# revision identifiers, used by Alembic. +revision = "0004_merge_heads" +down_revision = ("0003", "0003_validate_project_space_fk") +branch_labels = None +depends_on = None + + +def upgrade() -> None: + """No schema change; this revision only merges two heads.""" + + +def downgrade() -> None: + """No schema change; splitting back into two heads requires no work.""" From 6aaf2b52d26d24ec373809119183722ad610138d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 5 Jul 2026 20:43:54 +0900 Subject: [PATCH 02/10] test(modals): add RTL tests for AddTable/EditEdge/Cardinality dialogs Cover render, dialog a11y (role=dialog/aria-modal), source-backed labels, handler wiring and input changes for the three dialogs lacking direct component tests. +13 tests. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AxU2xaupAjp912oDNFuWyd --- .../components/modals/AddTableModal.test.tsx | 63 +++++++++++++++ .../modals/CardinalityModal.test.tsx | 79 +++++++++++++++++++ .../components/modals/EditEdgeModal.test.tsx | 65 +++++++++++++++ 3 files changed, 207 insertions(+) create mode 100644 frontend/src/components/modals/AddTableModal.test.tsx create mode 100644 frontend/src/components/modals/CardinalityModal.test.tsx create mode 100644 frontend/src/components/modals/EditEdgeModal.test.tsx diff --git a/frontend/src/components/modals/AddTableModal.test.tsx b/frontend/src/components/modals/AddTableModal.test.tsx new file mode 100644 index 00000000..6e30bb9e --- /dev/null +++ b/frontend/src/components/modals/AddTableModal.test.tsx @@ -0,0 +1,63 @@ +import '@testing-library/jest-dom/vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; + +import { AddTableModal } from './AddTableModal'; + +const baseProps = { + isOpen: true, + newTableName: 'users', + setNewTableName: vi.fn(), + onAddTableCancel: vi.fn(), + onAddTableSubmit: vi.fn(), +}; + +afterEach(() => cleanup()); + +describe('AddTableModal', () => { + it('renders nothing when closed', () => { + render(); + expect(screen.queryByRole('dialog')).toBeNull(); + }); + + it('renders an accessible dialog with the name field', () => { + render(); + const dialog = screen.getByRole('dialog'); + expect(dialog).toHaveAttribute('aria-modal', 'true'); + expect(dialog).toHaveAttribute('aria-labelledby', 'add-table-title'); + expect(screen.getByText('테이블 추가')).toBeInTheDocument(); + expect(screen.getByLabelText('테이블 이름')).toHaveValue('users'); + }); + + it('disables save while the name is blank and enables it once typed', () => { + const { rerender } = render(); + expect(screen.getByRole('button', { name: '저장' })).toBeDisabled(); + rerender(); + expect(screen.getByRole('button', { name: '저장' })).toBeEnabled(); + }); + + it('submits a non-empty name and cancels through the handlers', () => { + const onAddTableSubmit = vi.fn(); + const onAddTableCancel = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByRole('button', { name: '저장' })); + expect(onAddTableSubmit).toHaveBeenCalledOnce(); + fireEvent.click(screen.getByRole('button', { name: '취소' })); + expect(onAddTableCancel).toHaveBeenCalledOnce(); + }); + + it('reports name edits through setNewTableName', () => { + const setNewTableName = vi.fn(); + render(); + fireEvent.change(screen.getByLabelText('테이블 이름'), { + target: { value: 'products' }, + }); + expect(setNewTableName).toHaveBeenCalledWith('products'); + }); +}); diff --git a/frontend/src/components/modals/CardinalityModal.test.tsx b/frontend/src/components/modals/CardinalityModal.test.tsx new file mode 100644 index 00000000..990a9868 --- /dev/null +++ b/frontend/src/components/modals/CardinalityModal.test.tsx @@ -0,0 +1,79 @@ +import '@testing-library/jest-dom/vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; + +import type { Node } from '@xyflow/react'; + +import { CardinalityModal } from './CardinalityModal'; +import type { TableNodeData } from '../../erd/convert'; + +const node = { + id: 't1', + type: 'tableNode', + position: { x: 0, y: 0 }, + data: { + title: 'users', + columns: [{ column_name: 'email', data_type: 'text' }], + badges: { pk: false, fk: false }, + }, +} as unknown as Node; + +const baseProps = { + isOpen: true, + cardinalityNode: node, + nodes: [node], + cardinalityRowCount: '', + setCardinalityRowCount: vi.fn(), + cardinalityRowCountNumber: null, + cardinalityDistinctCounts: {}, + cardinalityColumnSelections: { email: false }, + cardinalityRecommendations: [], + appliedCardinalitySignatures: { names: new Set(), columns: new Set() }, + onCloseCardinalityWizard: vi.fn(), + onCardinalityTableChange: vi.fn(), + onCardinalityColumnToggle: vi.fn(), + onCardinalityDistinctCountChange: vi.fn(), + onApplyCardinalityRecommendation: vi.fn(), + parsePositiveInteger: (value: string) => { + const n = Number(value); + return Number.isFinite(n) && n > 0 ? n : null; + }, + calculateCardinalityRatio: () => 0, + formatPercent: () => '0%', + strengthLabel: () => '—', +}; + +afterEach(() => cleanup()); + +describe('CardinalityModal', () => { + it('renders nothing when closed or without a target node', () => { + const { rerender } = render(); + expect(screen.queryByRole('dialog')).toBeNull(); + rerender(); + expect(screen.queryByRole('dialog')).toBeNull(); + }); + + it('renders an accessible wizard with the column row', () => { + render(); + const dialog = screen.getByRole('dialog'); + expect(dialog).toHaveAttribute('aria-modal', 'true'); + expect(dialog).toHaveAttribute('aria-labelledby', 'cardinality-title'); + expect(screen.getByText('인덱스 카디널리티')).toBeInTheDocument(); + expect(screen.getByLabelText('테이블')).toBeInTheDocument(); + expect(screen.getByLabelText('email 사용')).toBeInTheDocument(); + }); + + it('closes through the handler', () => { + const onCloseCardinalityWizard = vi.fn(); + render(); + fireEvent.click(screen.getByRole('button', { name: '카디널리티 계산 닫기' })); + expect(onCloseCardinalityWizard).toHaveBeenCalledOnce(); + }); + + it('toggles a column selection through the handler', () => { + const onCardinalityColumnToggle = vi.fn(); + render(); + fireEvent.click(screen.getByLabelText('email 사용')); + expect(onCardinalityColumnToggle).toHaveBeenCalledWith('email', true); + }); +}); diff --git a/frontend/src/components/modals/EditEdgeModal.test.tsx b/frontend/src/components/modals/EditEdgeModal.test.tsx new file mode 100644 index 00000000..9b590c72 --- /dev/null +++ b/frontend/src/components/modals/EditEdgeModal.test.tsx @@ -0,0 +1,65 @@ +import '@testing-library/jest-dom/vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; + +import { EditEdgeModal } from './EditEdgeModal'; + +const edge = { id: 'e1', source: 'users', target: 'orders' }; + +const baseProps = { + editingEdge: edge, + relLabel: 'fk_users_orders', + setRelLabel: vi.fn(), + onRelDelete: vi.fn(), + onRelCancel: vi.fn(), + onRelSubmit: vi.fn(), +}; + +afterEach(() => cleanup()); + +describe('EditEdgeModal', () => { + it('renders nothing when there is no editing edge', () => { + render(); + expect(screen.queryByRole('dialog')).toBeNull(); + }); + + it('renders an accessible dialog with the relation source/target', () => { + render(); + const dialog = screen.getByRole('dialog'); + expect(dialog).toHaveAttribute('aria-modal', 'true'); + expect(dialog).toHaveAttribute('aria-labelledby', 'edit-rel-title'); + expect(screen.getByText('관계 설정')).toBeInTheDocument(); + expect(screen.getByText(/users/)).toBeInTheDocument(); + expect(screen.getByText(/orders/)).toBeInTheDocument(); + expect(screen.getByLabelText('제약조건 이름 (Label)')).toHaveValue('fk_users_orders'); + }); + + it('wires delete, cancel and submit buttons to their handlers', () => { + const onRelDelete = vi.fn(); + const onRelCancel = vi.fn(); + const onRelSubmit = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByRole('button', { name: '삭제' })); + fireEvent.click(screen.getByRole('button', { name: '취소' })); + fireEvent.click(screen.getByRole('button', { name: '저장' })); + expect(onRelDelete).toHaveBeenCalledOnce(); + expect(onRelCancel).toHaveBeenCalledOnce(); + expect(onRelSubmit).toHaveBeenCalledOnce(); + }); + + it('reports label edits through setRelLabel', () => { + const setRelLabel = vi.fn(); + render(); + fireEvent.change(screen.getByLabelText('제약조건 이름 (Label)'), { + target: { value: 'fk_new' }, + }); + expect(setRelLabel).toHaveBeenCalledWith('fk_new'); + }); +}); From bc26f62709deb2ff8e00446b7609ae94cabf7a36 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 5 Jul 2026 20:43:54 +0900 Subject: [PATCH 03/10] feat: schema snapshot diff + saved diagram views Schema Diff: pure diff_snapshots() matching tables by (schema,name) not volatile relation_oid; GET /api/snapshots/{uuid}/diff (IDOR-safe, uniform not_found); DiffModal + demo. Saved Views: DiagramView model + 0005 migration + IDOR-guarded CRUD; captureLayout/applyLayout + ViewsModal. Verified: frontend 130 tests, backend 235 tests, tsc --noEmit clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AxU2xaupAjp912oDNFuWyd --- backend/alembic/versions/0005_diagram_view.py | 48 ++++ backend/app/api/diagram_views.py | 156 +++++++++++++ backend/app/api/snapshots.py | 47 +++- backend/app/diff/__init__.py | 1 + backend/app/diff/schema_diff.py | 207 ++++++++++++++++++ backend/app/main.py | 2 + backend/app/models.py | 28 +++ backend/app/schemas.py | 39 ++++ backend/tests/test_api_diagram_views.py | 95 ++++++++ backend/tests/test_api_snapshot_diff.py | 104 +++++++++ backend/tests/test_schema_diff.py | 205 +++++++++++++++++ frontend/src/App.tsx | 161 +++++++++++++- frontend/src/api.ts | 150 ++++++++++++- .../src/components/modals/DiffModal.test.tsx | 120 ++++++++++ frontend/src/components/modals/DiffModal.tsx | 185 ++++++++++++++++ .../src/components/modals/ViewsModal.test.tsx | 66 ++++++ frontend/src/components/modals/ViewsModal.tsx | 115 ++++++++++ frontend/src/components/modals/index.ts | 2 + frontend/src/diagramViews.test.ts | 49 +++++ frontend/src/diagramViews.ts | 32 +++ frontend/src/types.ts | 56 +++++ 21 files changed, 1864 insertions(+), 4 deletions(-) create mode 100644 backend/alembic/versions/0005_diagram_view.py create mode 100644 backend/app/api/diagram_views.py create mode 100644 backend/app/diff/__init__.py create mode 100644 backend/app/diff/schema_diff.py create mode 100644 backend/tests/test_api_diagram_views.py create mode 100644 backend/tests/test_api_snapshot_diff.py create mode 100644 backend/tests/test_schema_diff.py create mode 100644 frontend/src/components/modals/DiffModal.test.tsx create mode 100644 frontend/src/components/modals/DiffModal.tsx create mode 100644 frontend/src/components/modals/ViewsModal.test.tsx create mode 100644 frontend/src/components/modals/ViewsModal.tsx create mode 100644 frontend/src/diagramViews.test.ts create mode 100644 frontend/src/diagramViews.ts diff --git a/backend/alembic/versions/0005_diagram_view.py b/backend/alembic/versions/0005_diagram_view.py new file mode 100644 index 00000000..53f61b2a --- /dev/null +++ b/backend/alembic/versions/0005_diagram_view.py @@ -0,0 +1,48 @@ +"""diagram_view: saved ERD canvas views + +Revision ID: 0005_diagram_view +Revises: 0004_merge_heads +Create Date: 2026-07-05 + +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op + +revision = "0005_diagram_view" +down_revision = "0004_merge_heads" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "diagram_view", + sa.Column("diagram_view_uuid", sa.Uuid(), primary_key=True, nullable=False), + sa.Column("project_space_uuid", sa.Uuid(), nullable=False), + sa.Column("name", sa.Text(), nullable=False), + sa.Column("layout_json", sa.JSON(), nullable=False), + sa.Column("created_by", sa.Uuid(), nullable=True), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), + sa.ForeignKeyConstraint( + ["project_space_uuid"], + ["project_space.project_space_uuid"], + ondelete="CASCADE", + ), + sa.PrimaryKeyConstraint("diagram_view_uuid"), + ) + op.create_index( + "ix_diagram_view__project_space_uuid", + "diagram_view", + ["project_space_uuid"], + ) + + +def downgrade() -> None: + op.drop_index( + "ix_diagram_view__project_space_uuid", table_name="diagram_view" + ) + op.drop_table("diagram_view") diff --git a/backend/app/api/diagram_views.py b/backend/app/api/diagram_views.py new file mode 100644 index 00000000..f57782bc --- /dev/null +++ b/backend/app/api/diagram_views.py @@ -0,0 +1,156 @@ +from __future__ import annotations + +import datetime as dt +import json +import uuid + +from fastapi import APIRouter, Depends, HTTPException +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.auth import CurrentUser, get_current_user +from app.db import get_read_session, get_session +from app.models import DiagramView +from app.permissions import require_project_member +from app.schemas import ( + DiagramViewCreateIn, + DiagramViewDetailOut, + DiagramViewOut, +) + +router = APIRouter(prefix="/api/diagram-views", tags=["diagram-views"]) + +# Saved layouts are small (positions for a few hundred tables). Bound the +# serialized payload so a client cannot store arbitrarily large blobs. +MAX_LAYOUT_BYTES = 512 * 1024 + + +def _bound_layout_size(layout: dict) -> None: + encoded = json.dumps(layout, separators=(",", ":")).encode("utf-8") + if len(encoded) > MAX_LAYOUT_BYTES: + raise HTTPException(status_code=413, detail="layout payload too large") + + +async def _get_authorized_view( + session: AsyncSession, + diagram_view_uuid: uuid.UUID, + user: CurrentUser, + minimum_role: str | None = None, +) -> DiagramView | None: + """Fetch a view only after project membership has been checked. + + Returns ``None`` for both missing and unauthorized so callers can respond + with a uniform 404 (no existence enumeration). + """ + + project_space_uuid = await session.scalar( + select(DiagramView.project_space_uuid).where( + DiagramView.diagram_view_uuid == diagram_view_uuid + ) + ) + if project_space_uuid is None: + return None + try: + await require_project_member( + session, + project_space_uuid, + user.user_account_uuid, + minimum_role=minimum_role, + ) + except HTTPException as exc: + if exc.status_code == 403: + return None + raise + return await session.get(DiagramView, diagram_view_uuid) + + +@router.post("/by-project/{project_space_uuid}", response_model=DiagramViewOut) +async def create_view( + project_space_uuid: uuid.UUID, + body: DiagramViewCreateIn, + user: CurrentUser = Depends(get_current_user), + session: AsyncSession = Depends(get_session), +) -> DiagramViewOut: + """Save a new ERD canvas view for a project.""" + await require_project_member( + session, project_space_uuid, user.user_account_uuid, minimum_role="editor" + ) + _bound_layout_size(body.layout_json) + now = dt.datetime.now(dt.timezone.utc) + view = DiagramView( + diagram_view_uuid=uuid.uuid4(), + project_space_uuid=project_space_uuid, + name=body.name, + layout_json=body.layout_json, + created_by=user.user_account_uuid, + created_at=now, + updated_at=now, + ) + session.add(view) + await session.commit() + return DiagramViewOut( + diagram_view_uuid=view.diagram_view_uuid, + name=view.name, + created_at=view.created_at, + updated_at=view.updated_at, + ) + + +@router.get("/by-project/{project_space_uuid}", response_model=list[DiagramViewOut]) +async def list_views( + project_space_uuid: uuid.UUID, + user: CurrentUser = Depends(get_current_user), + session: AsyncSession = Depends(get_read_session), +) -> list[DiagramViewOut]: + """List saved views for a project (newest first).""" + await require_project_member(session, project_space_uuid, user.user_account_uuid) + rows = await session.execute( + select(DiagramView) + .where(DiagramView.project_space_uuid == project_space_uuid) + .order_by(DiagramView.updated_at.desc()) + ) + return [ + DiagramViewOut( + diagram_view_uuid=v.diagram_view_uuid, + name=v.name, + created_at=v.created_at, + updated_at=v.updated_at, + ) + for v in rows.scalars().all() + ] + + +@router.get("/{diagram_view_uuid}", response_model=DiagramViewDetailOut) +async def get_view( + diagram_view_uuid: uuid.UUID, + user: CurrentUser = Depends(get_current_user), + session: AsyncSession = Depends(get_read_session), +) -> DiagramViewDetailOut: + """Get one saved view including its layout payload.""" + view = await _get_authorized_view(session, diagram_view_uuid, user) + if view is None: + raise HTTPException(status_code=404, detail="diagram view not found") + return DiagramViewDetailOut( + diagram_view_uuid=view.diagram_view_uuid, + name=view.name, + layout_json=view.layout_json, + created_at=view.created_at, + updated_at=view.updated_at, + ) + + +@router.delete("/{diagram_view_uuid}") +async def delete_view( + diagram_view_uuid: uuid.UUID, + user: CurrentUser = Depends(get_current_user), + session: AsyncSession = Depends(get_session), +) -> dict[str, bool]: + """Delete a saved view (requires editor membership on its project).""" + view = await _get_authorized_view( + session, diagram_view_uuid, user, minimum_role="editor" + ) + if view is None: + raise HTTPException(status_code=404, detail="diagram view not found") + await session.delete(view) + await session.commit() + return {"ok": True} diff --git a/backend/app/api/snapshots.py b/backend/app/api/snapshots.py index 79f2515f..77e9c19a 100644 --- a/backend/app/api/snapshots.py +++ b/backend/app/api/snapshots.py @@ -17,8 +17,14 @@ SchemaSnapshotData, ) from app.permissions import require_project_member -from app.schemas import SnapshotCreateIn, SnapshotDetailOut, SnapshotOut +from app.schemas import ( + SnapshotCreateIn, + SnapshotDetailOut, + SnapshotDiffOut, + SnapshotOut, +) from app.ddl.export import snapshot_json_to_sql +from app.diff.schema_diff import diff_snapshots from app.jobs.valkey_queue import enqueue_job_signal from app.spec.llm import ( LlmConfigurationError, @@ -144,6 +150,45 @@ async def get_snapshot( ) +@router.get("/{schema_snapshot_uuid}/diff", response_model=SnapshotDiffOut) +async def diff_snapshot( + schema_snapshot_uuid: uuid.UUID, + against: uuid.UUID = Query( + ..., description="Base snapshot UUID to compare this snapshot against" + ), + user: CurrentUser = Depends(get_current_user), + session: AsyncSession = Depends(get_read_session), +) -> SnapshotDiffOut: + """Diff this snapshot (target) against another (base). + + Both snapshots are authorized independently via project membership, so a + caller can only diff snapshots they may already read. If either is missing + or unauthorized, a uniform ``not_found`` response is returned so existence + cannot be enumerated. + """ + target_snap = await _get_authorized_snapshot(session, schema_snapshot_uuid, user) + base_snap = await _get_authorized_snapshot(session, against, user) + if target_snap is None or base_snap is None: + return SnapshotDiffOut( + base_snapshot_uuid=against, + target_snapshot_uuid=schema_snapshot_uuid, + status="not_found", + diff=None, + ) + base_data = await session.get(SchemaSnapshotData, against) + target_data = await session.get(SchemaSnapshotData, schema_snapshot_uuid) + diff = diff_snapshots( + base_data.snapshot_json if base_data else None, + target_data.snapshot_json if target_data else None, + ) + return SnapshotDiffOut( + base_snapshot_uuid=against, + target_snapshot_uuid=schema_snapshot_uuid, + status="ok", + diff=diff, + ) + + @router.get("/{schema_snapshot_uuid}/export.sql", response_class=PlainTextResponse) async def export_snapshot_sql( schema_snapshot_uuid: uuid.UUID, diff --git a/backend/app/diff/__init__.py b/backend/app/diff/__init__.py new file mode 100644 index 00000000..793ebd10 --- /dev/null +++ b/backend/app/diff/__init__.py @@ -0,0 +1 @@ +"""Schema snapshot diffing (pure, DB-independent transforms).""" diff --git a/backend/app/diff/schema_diff.py b/backend/app/diff/schema_diff.py new file mode 100644 index 00000000..a499933e --- /dev/null +++ b/backend/app/diff/schema_diff.py @@ -0,0 +1,207 @@ +"""Diff two schema snapshots into a structured change set. + +Snapshots are the JSON payloads captured by reverse-engineering a database +(see ``SchemaSnapshotData.snapshot_json``): ``relations``, ``columns``, +``pk_columns`` and ``fk_edges``, all keyed internally by ``relation_oid``. + +IMPORTANT: ``relation_oid`` (and ``fk_constraint_oid``) are database-internal +identifiers that are re-assigned on every introspection run. Two snapshots of +the *same* database therefore have *different* oids for the *same* table, so we +must never diff by oid. Instead we key tables by ``(schema_name, relation_name)`` +and columns by name, and we resolve every FK's oids to table names *within its +own snapshot* before comparing. Diffing by oid would report every table as +changed on each run — a silent, high-impact bug. +""" + +from __future__ import annotations + +from typing import Any + + +def _table_key(schema_name: object, relation_name: object) -> str: + schema = str(schema_name) if schema_name is not None else "" + name = str(relation_name) if relation_name is not None else "" + return f"{schema}.{name}" if schema else name + + +def _index_snapshot(snapshot: dict[str, Any] | None) -> dict[str, Any]: + """Build name-keyed lookups for one snapshot (oid-independent).""" + + snapshot = snapshot or {} + relations = snapshot.get("relations") or [] + columns = snapshot.get("columns") or [] + pk_columns = snapshot.get("pk_columns") or [] + fk_edges = snapshot.get("fk_edges") or [] + + oid_to_table: dict[Any, str] = {} + tables: dict[str, dict[str, Any]] = {} + for rel in relations: + key = _table_key(rel.get("schema_name"), rel.get("relation_name")) + oid_to_table[rel.get("relation_oid")] = key + tables[key] = { + "comment": rel.get("relation_comment"), + "kind": rel.get("relation_kind"), + "columns": {}, + "pk": [], + } + + for col in columns: + table = oid_to_table.get(col.get("relation_oid")) + if table is None or table not in tables: + continue + name = col.get("column_name") + if name is None: + continue + tables[table]["columns"][str(name)] = { + "data_type": col.get("data_type"), + "is_not_null": bool(col.get("is_not_null")), + } + + # Preserve primary-key column order via column_ordinal when present. + pk_tmp: dict[str, list[tuple[int, str]]] = {} + for pk in pk_columns: + table = oid_to_table.get(pk.get("relation_oid")) + if table is None or table not in tables: + continue + name = pk.get("column_name") + if name is None: + continue + ordinal = pk.get("column_ordinal") + ordinal = int(ordinal) if isinstance(ordinal, int) else len(pk_tmp.get(table, [])) + pk_tmp.setdefault(table, []).append((ordinal, str(name))) + for table, items in pk_tmp.items(): + items.sort(key=lambda pair: pair[0]) + tables[table]["pk"] = [name for _, name in items] + + # Resolve FK oids to (child_table, child_col -> parent_table, parent_col). + # Group multi-column FKs by constraint via a stable resolved signature. + fk_parts: dict[tuple[str, str, str], list[tuple[int, str, str]]] = {} + for edge in fk_edges: + child = oid_to_table.get(edge.get("child_relation_oid")) + parent = oid_to_table.get(edge.get("parent_relation_oid")) + if child is None or parent is None: + continue + name = str(edge.get("fk_constraint_name") or "") + ordinal = edge.get("column_ordinal") + ordinal = int(ordinal) if isinstance(ordinal, int) else 0 + fk_parts.setdefault((child, parent, name), []).append( + ( + ordinal, + str(edge.get("child_column_name") or ""), + str(edge.get("parent_column_name") or ""), + ) + ) + + fks: dict[str, dict[str, Any]] = {} + for (child, parent, name), parts in fk_parts.items(): + parts.sort(key=lambda p: p[0]) + child_cols = [c for _, c, _ in parts] + parent_cols = [p for _, _, p in parts] + # Signature is oid-independent: names + column pairing, not the FK name + # (constraint names may auto-generate differently between runs). + signature = ( + f"{child}({','.join(child_cols)})->" + f"{parent}({','.join(parent_cols)})" + ) + fks[signature] = { + "name": name or None, + "child_table": child, + "child_columns": child_cols, + "parent_table": parent, + "parent_columns": parent_cols, + } + + return {"tables": tables, "fks": fks} + + +def _diff_columns( + base_cols: dict[str, Any], target_cols: dict[str, Any] +) -> dict[str, Any]: + base_names = set(base_cols) + target_names = set(target_cols) + added = sorted(target_names - base_names) + removed = sorted(base_names - target_names) + changed: list[dict[str, Any]] = [] + for name in sorted(base_names & target_names): + before = base_cols[name] + after = target_cols[name] + if before != after: + changed.append({"column": name, "from": before, "to": after}) + return {"added": added, "removed": removed, "changed": changed} + + +def diff_snapshots( + base: dict[str, Any] | None, target: dict[str, Any] | None +) -> dict[str, Any]: + """Compute a structured diff from ``base`` to ``target`` snapshot JSON. + + Both arguments are snapshot ``snapshot_json`` payloads (or ``None``). The + result reports table/column/primary-key/foreign-key changes plus a summary. + All matching is by name, so it is stable across introspection runs. + """ + + b = _index_snapshot(base) + t = _index_snapshot(target) + + base_tables = set(b["tables"]) + target_tables = set(t["tables"]) + + tables_added = sorted(target_tables - base_tables) + tables_removed = sorted(base_tables - target_tables) + + changed_tables: list[dict[str, Any]] = [] + columns_added = columns_removed = columns_changed = 0 + for table in sorted(base_tables & target_tables): + bt = b["tables"][table] + tt = t["tables"][table] + col_diff = _diff_columns(bt["columns"], tt["columns"]) + pk_changed = bt["pk"] != tt["pk"] + comment_changed = bt["comment"] != tt["comment"] + if ( + col_diff["added"] + or col_diff["removed"] + or col_diff["changed"] + or pk_changed + or comment_changed + ): + columns_added += len(col_diff["added"]) + columns_removed += len(col_diff["removed"]) + columns_changed += len(col_diff["changed"]) + entry: dict[str, Any] = {"table": table, "columns": col_diff} + if pk_changed: + entry["primary_key"] = {"from": bt["pk"], "to": tt["pk"]} + if comment_changed: + entry["comment"] = {"from": bt["comment"], "to": tt["comment"]} + changed_tables.append(entry) + + base_fks = set(b["fks"]) + target_fks = set(t["fks"]) + fks_added = sorted(target_fks - base_fks) + fks_removed = sorted(base_fks - target_fks) + + summary = { + "tables_added": len(tables_added), + "tables_removed": len(tables_removed), + "tables_changed": len(changed_tables), + "columns_added": columns_added, + "columns_removed": columns_removed, + "columns_changed": columns_changed, + "fks_added": len(fks_added), + "fks_removed": len(fks_removed), + } + summary["has_changes"] = any(v for k, v in summary.items() if k != "has_changes") + + return { + "base_table_count": len(base_tables), + "target_table_count": len(target_tables), + "tables": { + "added": tables_added, + "removed": tables_removed, + "changed": changed_tables, + }, + "foreign_keys": { + "added": [t["fks"][s] for s in fks_added], + "removed": [b["fks"][s] for s in fks_removed], + }, + "summary": summary, + } diff --git a/backend/app/main.py b/backend/app/main.py index 41f802c7..bfa30d02 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -10,6 +10,7 @@ from app.api.connections import router as connections_router from app.api.auth_routes import router as auth_router +from app.api.diagram_views import router as diagram_views_router from app.api.me import router as me_router from app.api.projects import router as projects_router from app.api.share import router as share_router @@ -165,6 +166,7 @@ async def csrf_token() -> dict[str, str]: app.include_router(projects_router) app.include_router(connections_router) app.include_router(snapshots_router) +app.include_router(diagram_views_router) app.include_router(me_router) app.include_router(share_router) app.include_router(auth_router) diff --git a/backend/app/models.py b/backend/app/models.py index 2a772ac3..cb535fa6 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -185,6 +185,34 @@ class JobQueue(Base): __table_args__ = (Index("ix_job_queue__status_run_after", "status", "run_after"),) +class DiagramView(Base): + """A saved ERD canvas view (node layout + hidden tables) for a project.""" + + __tablename__ = "diagram_view" + + diagram_view_uuid: Mapped[uuid.UUID] = mapped_column( + UUID(as_uuid=True), primary_key=True, default=uuid.uuid4 + ) + project_space_uuid: Mapped[uuid.UUID] = mapped_column( + UUID(as_uuid=True), + ForeignKey("project_space.project_space_uuid", ondelete="CASCADE"), + index=True, + ) + name: Mapped[str] = mapped_column(Text()) + # Opaque, client-defined layout payload (node positions, hidden tables, + # viewport). Stored as JSONB; the API bounds its size. + layout_json: Mapped[dict] = mapped_column(JSONB()) + created_by: Mapped[uuid.UUID | None] = mapped_column( + UUID(as_uuid=True), nullable=True + ) + created_at: Mapped[dt.datetime] = mapped_column( + DateTime(timezone=True), default=utcnow + ) + updated_at: Mapped[dt.datetime] = mapped_column( + DateTime(timezone=True), default=utcnow, onupdate=utcnow + ) + + class ShareLink(Base): """Public share link granting read access to a project's snapshots.""" diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 0359799c..692aff7a 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -1,5 +1,6 @@ from __future__ import annotations +import datetime as dt import uuid from typing import Literal @@ -91,6 +92,44 @@ class SnapshotDetailOut(BaseModel): snapshot_json: dict | None +class SnapshotDiffOut(BaseModel): + """Structured diff between two schema snapshots. + + ``status`` is ``"not_found"`` when either snapshot is missing or the caller + is not authorized for it (uniform response avoids existence enumeration); + ``"ok"`` with a populated ``diff`` otherwise. + """ + + base_snapshot_uuid: uuid.UUID + target_snapshot_uuid: uuid.UUID + status: str + diff: dict | None + + +class DiagramViewCreateIn(BaseModel): + """Request body for saving an ERD canvas view.""" + + name: str = Field(min_length=1, max_length=200) + # Opaque client layout (node positions, hidden tables, viewport). The API + # bounds the serialized size in the endpoint to prevent abuse. + layout_json: dict + + +class DiagramViewOut(BaseModel): + """Diagram view summary.""" + + diagram_view_uuid: uuid.UUID + name: str + created_at: dt.datetime + updated_at: dt.datetime + + +class DiagramViewDetailOut(DiagramViewOut): + """Diagram view including its layout payload.""" + + layout_json: dict + + class MeOut(BaseModel): """Current user payload returned by /me.""" diff --git a/backend/tests/test_api_diagram_views.py b/backend/tests/test_api_diagram_views.py new file mode 100644 index 00000000..44d29087 --- /dev/null +++ b/backend/tests/test_api_diagram_views.py @@ -0,0 +1,95 @@ +import datetime as dt +import uuid +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +import pytest +from fastapi import HTTPException + +from app.api.diagram_views import create_view, delete_view, get_view +from app.auth import CurrentUser +from app.schemas import DiagramViewCreateIn + + +def _user(): + return CurrentUser( + user_account_uuid=uuid.uuid4(), subject="test", display_name="Test" + ) + + +@pytest.mark.asyncio +async def test_get_view_returns_404_when_missing_or_unauthorized(): + # Uniform 404 for both missing and unauthorized (no enumeration). + session = AsyncMock() + with patch( + "app.api.diagram_views._get_authorized_view", + new_callable=AsyncMock, + return_value=None, + ): + with pytest.raises(HTTPException) as exc: + await get_view( + diagram_view_uuid=uuid.uuid4(), user=_user(), session=session + ) + assert exc.value.status_code == 404 + + +@pytest.mark.asyncio +async def test_get_view_returns_detail_when_authorized(): + session = AsyncMock() + now = dt.datetime.now(dt.timezone.utc) + view_id = uuid.uuid4() + view = SimpleNamespace( + diagram_view_uuid=view_id, + name="my view", + layout_json={"positions": {"public.member": {"x": 10, "y": 20}}}, + created_at=now, + updated_at=now, + ) + with patch( + "app.api.diagram_views._get_authorized_view", + new_callable=AsyncMock, + return_value=view, + ): + out = await get_view( + diagram_view_uuid=view_id, user=_user(), session=session + ) + assert out.diagram_view_uuid == view_id + assert out.name == "my view" + assert out.layout_json["positions"]["public.member"] == {"x": 10, "y": 20} + + +@pytest.mark.asyncio +async def test_create_view_rejects_oversized_layout(): + session = AsyncMock() + huge = {"blob": "a" * (600 * 1024)} # > 512KB serialized + body = DiagramViewCreateIn(name="big", layout_json=huge) + with patch( + "app.api.diagram_views.require_project_member", new_callable=AsyncMock + ): + with pytest.raises(HTTPException) as exc: + await create_view( + project_space_uuid=uuid.uuid4(), + body=body, + user=_user(), + session=session, + ) + assert exc.value.status_code == 413 + # Nothing should have been persisted. + session.add.assert_not_called() + session.commit.assert_not_called() + + +@pytest.mark.asyncio +async def test_delete_view_returns_404_when_unauthorized(): + session = AsyncMock() + with patch( + "app.api.diagram_views._get_authorized_view", + new_callable=AsyncMock, + return_value=None, + ): + with pytest.raises(HTTPException) as exc: + await delete_view( + diagram_view_uuid=uuid.uuid4(), user=_user(), session=session + ) + assert exc.value.status_code == 404 + session.delete.assert_not_called() diff --git a/backend/tests/test_api_snapshot_diff.py b/backend/tests/test_api_snapshot_diff.py new file mode 100644 index 00000000..0c588cff --- /dev/null +++ b/backend/tests/test_api_snapshot_diff.py @@ -0,0 +1,104 @@ +import uuid +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +import pytest + +from app.api.snapshots import diff_snapshot +from app.auth import CurrentUser + + +def _user(): + return CurrentUser( + user_account_uuid=uuid.uuid4(), subject="test", display_name="Test" + ) + + +@pytest.mark.asyncio +async def test_diff_returns_not_found_when_a_snapshot_is_unauthorized(): + # If either snapshot is missing/unauthorized, respond uniformly (no + # existence enumeration) — mirrors get_snapshot's not_found contract. + session = AsyncMock() + target = uuid.uuid4() + base = uuid.uuid4() + + with patch( + "app.api.snapshots._get_authorized_snapshot", + new_callable=AsyncMock, + return_value=None, + ): + out = await diff_snapshot( + schema_snapshot_uuid=target, against=base, user=_user(), session=session + ) + + assert out.status == "not_found" + assert out.diff is None + assert out.base_snapshot_uuid == base + assert out.target_snapshot_uuid == target + # Must not read snapshot data for an unauthorized request. + session.get.assert_not_called() + + +@pytest.mark.asyncio +async def test_diff_computes_changes_when_both_authorized(): + session = AsyncMock() + target = uuid.uuid4() + base = uuid.uuid4() + + base_json = { + "relations": [ + {"relation_oid": 1, "schema_name": "public", "relation_name": "member"} + ], + "columns": [ + { + "relation_oid": 1, + "column_name": "email", + "data_type": "varchar(100)", + "is_not_null": False, + } + ], + "pk_columns": [], + "fk_edges": [], + } + # Same table (different oid), email widened + NOT NULL, new table added. + target_json = { + "relations": [ + {"relation_oid": 50, "schema_name": "public", "relation_name": "member"}, + {"relation_oid": 51, "schema_name": "public", "relation_name": "orders"}, + ], + "columns": [ + { + "relation_oid": 50, + "column_name": "email", + "data_type": "varchar(255)", + "is_not_null": True, + }, + { + "relation_oid": 51, + "column_name": "order_id", + "data_type": "bigint", + "is_not_null": True, + }, + ], + "pk_columns": [], + "fk_edges": [], + } + + # Both snapshots authorized. + authorized = AsyncMock(return_value=SimpleNamespace(schema_snapshot_uuid=target)) + # session.get(SchemaSnapshotData, against) then (…, target). + session.get.side_effect = [ + SimpleNamespace(snapshot_json=base_json), + SimpleNamespace(snapshot_json=target_json), + ] + + with patch("app.api.snapshots._get_authorized_snapshot", authorized): + out = await diff_snapshot( + schema_snapshot_uuid=target, against=base, user=_user(), session=session + ) + + assert out.status == "ok" + assert out.diff is not None + assert out.diff["tables"]["added"] == ["public.orders"] + assert out.diff["summary"]["columns_changed"] == 1 + assert out.diff["summary"]["has_changes"] is True diff --git a/backend/tests/test_schema_diff.py b/backend/tests/test_schema_diff.py new file mode 100644 index 00000000..402a41e2 --- /dev/null +++ b/backend/tests/test_schema_diff.py @@ -0,0 +1,205 @@ +from __future__ import annotations + +from app.diff.schema_diff import diff_snapshots + + +def _snap(relations, columns, pk_columns=None, fk_edges=None): + return { + "relations": relations, + "columns": columns, + "pk_columns": pk_columns or [], + "fk_edges": fk_edges or [], + } + + +def _base(): + return _snap( + relations=[ + {"relation_oid": 1, "schema_name": "public", "relation_name": "member"}, + {"relation_oid": 2, "schema_name": "public", "relation_name": "orders"}, + ], + columns=[ + {"relation_oid": 1, "column_name": "member_id", "data_type": "bigint", "is_not_null": True}, + {"relation_oid": 1, "column_name": "email", "data_type": "varchar(100)", "is_not_null": False}, + {"relation_oid": 2, "column_name": "order_id", "data_type": "bigint", "is_not_null": True}, + {"relation_oid": 2, "column_name": "member_id", "data_type": "bigint", "is_not_null": True}, + ], + pk_columns=[ + {"relation_oid": 1, "column_name": "member_id", "column_ordinal": 1}, + {"relation_oid": 2, "column_name": "order_id", "column_ordinal": 1}, + ], + fk_edges=[ + { + "fk_constraint_oid": 10, + "fk_constraint_name": "fk_orders_member", + "child_relation_oid": 2, + "parent_relation_oid": 1, + "child_column_name": "member_id", + "parent_column_name": "member_id", + "column_ordinal": 1, + } + ], + ) + + +def test_identical_snapshots_report_no_changes_even_with_different_oids(): + # Same logical schema, but re-introspection assigned different oids. + # A correct diff keys by name and must report zero changes. + base = _base() + target = _snap( + relations=[ + {"relation_oid": 900, "schema_name": "public", "relation_name": "member"}, + {"relation_oid": 901, "schema_name": "public", "relation_name": "orders"}, + ], + columns=[ + {"relation_oid": 900, "column_name": "member_id", "data_type": "bigint", "is_not_null": True}, + {"relation_oid": 900, "column_name": "email", "data_type": "varchar(100)", "is_not_null": False}, + {"relation_oid": 901, "column_name": "order_id", "data_type": "bigint", "is_not_null": True}, + {"relation_oid": 901, "column_name": "member_id", "data_type": "bigint", "is_not_null": True}, + ], + pk_columns=[ + {"relation_oid": 900, "column_name": "member_id", "column_ordinal": 1}, + {"relation_oid": 901, "column_name": "order_id", "column_ordinal": 1}, + ], + fk_edges=[ + { + "fk_constraint_oid": 77, + "fk_constraint_name": "fk_orders_member", + "child_relation_oid": 901, + "parent_relation_oid": 900, + "child_column_name": "member_id", + "parent_column_name": "member_id", + "column_ordinal": 1, + } + ], + ) + d = diff_snapshots(base, target) + assert d["summary"]["has_changes"] is False + assert d["tables"]["added"] == [] + assert d["tables"]["removed"] == [] + assert d["tables"]["changed"] == [] + assert d["foreign_keys"]["added"] == [] + assert d["foreign_keys"]["removed"] == [] + + +def test_table_added_and_removed(): + base = _base() + target = _snap( + relations=[ + {"relation_oid": 1, "schema_name": "public", "relation_name": "member"}, + {"relation_oid": 3, "schema_name": "public", "relation_name": "audit_log"}, + ], + columns=[ + {"relation_oid": 1, "column_name": "member_id", "data_type": "bigint", "is_not_null": True}, + {"relation_oid": 1, "column_name": "email", "data_type": "varchar(100)", "is_not_null": False}, + {"relation_oid": 3, "column_name": "id", "data_type": "bigint", "is_not_null": True}, + ], + pk_columns=[{"relation_oid": 1, "column_name": "member_id", "column_ordinal": 1}], + ) + d = diff_snapshots(base, target) + assert d["tables"]["added"] == ["public.audit_log"] + assert d["tables"]["removed"] == ["public.orders"] + assert d["summary"]["tables_added"] == 1 + assert d["summary"]["tables_removed"] == 1 + # The removed FK (only existed in base) is reported. + assert d["summary"]["fks_removed"] == 1 + + +def test_column_added_removed_type_and_nullability_changed(): + base = _base() + target = _snap( + relations=[ + {"relation_oid": 1, "schema_name": "public", "relation_name": "member"}, + {"relation_oid": 2, "schema_name": "public", "relation_name": "orders"}, + ], + columns=[ + {"relation_oid": 1, "column_name": "member_id", "data_type": "bigint", "is_not_null": True}, + # email: type widened AND became NOT NULL (a change) + {"relation_oid": 1, "column_name": "email", "data_type": "varchar(255)", "is_not_null": True}, + # new column added + {"relation_oid": 1, "column_name": "phone", "data_type": "varchar(20)", "is_not_null": False}, + {"relation_oid": 2, "column_name": "order_id", "data_type": "bigint", "is_not_null": True}, + # orders.member_id removed + ], + pk_columns=[ + {"relation_oid": 1, "column_name": "member_id", "column_ordinal": 1}, + {"relation_oid": 2, "column_name": "order_id", "column_ordinal": 1}, + ], + ) + d = diff_snapshots(base, target) + changed = {c["table"]: c for c in d["tables"]["changed"]} + assert set(changed) == {"public.member", "public.orders"} + + member = changed["public.member"] + assert member["columns"]["added"] == ["phone"] + email_change = next(c for c in member["columns"]["changed"] if c["column"] == "email") + assert email_change["from"] == {"data_type": "varchar(100)", "is_not_null": False} + assert email_change["to"] == {"data_type": "varchar(255)", "is_not_null": True} + + orders = changed["public.orders"] + assert orders["columns"]["removed"] == ["member_id"] + assert d["summary"]["columns_added"] == 1 + assert d["summary"]["columns_removed"] == 1 + assert d["summary"]["columns_changed"] == 1 + + +def test_primary_key_change_detected(): + base = _base() + target = _snap( + relations=[{"relation_oid": 1, "schema_name": "public", "relation_name": "member"}], + columns=[ + {"relation_oid": 1, "column_name": "member_id", "data_type": "bigint", "is_not_null": True}, + {"relation_oid": 1, "column_name": "email", "data_type": "varchar(100)", "is_not_null": False}, + ], + # PK moved from member_id to email + pk_columns=[{"relation_oid": 1, "column_name": "email", "column_ordinal": 1}], + ) + d = diff_snapshots(base, target) + member = next(c for c in d["tables"]["changed"] if c["table"] == "public.member") + assert member["primary_key"] == {"from": ["member_id"], "to": ["email"]} + + +def test_fk_added(): + base = _snap( + relations=[ + {"relation_oid": 1, "schema_name": "public", "relation_name": "member"}, + {"relation_oid": 2, "schema_name": "public", "relation_name": "orders"}, + ], + columns=[ + {"relation_oid": 1, "column_name": "member_id", "data_type": "bigint", "is_not_null": True}, + {"relation_oid": 2, "column_name": "member_id", "data_type": "bigint", "is_not_null": True}, + ], + ) + target = _snap( + relations=[ + {"relation_oid": 1, "schema_name": "public", "relation_name": "member"}, + {"relation_oid": 2, "schema_name": "public", "relation_name": "orders"}, + ], + columns=[ + {"relation_oid": 1, "column_name": "member_id", "data_type": "bigint", "is_not_null": True}, + {"relation_oid": 2, "column_name": "member_id", "data_type": "bigint", "is_not_null": True}, + ], + fk_edges=[ + { + "fk_constraint_name": "fk_orders_member", + "child_relation_oid": 2, + "parent_relation_oid": 1, + "child_column_name": "member_id", + "parent_column_name": "member_id", + "column_ordinal": 1, + } + ], + ) + d = diff_snapshots(base, target) + assert d["summary"]["fks_added"] == 1 + added = d["foreign_keys"]["added"][0] + assert added["child_table"] == "public.orders" + assert added["parent_table"] == "public.member" + assert added["child_columns"] == ["member_id"] + + +def test_none_and_empty_snapshots_are_safe(): + assert diff_snapshots(None, None)["summary"]["has_changes"] is False + d = diff_snapshots(None, _base()) + assert d["summary"]["tables_added"] == 2 + assert diff_snapshots(_base(), None)["summary"]["tables_removed"] == 2 diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index e99700c5..112e9834 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -17,10 +17,12 @@ import { useEffect, useMemo, useRef, useState, useCallback } from "react"; import { AddTableModal, CardinalityModal, + DiffModal, EditEdgeModal, EditTableModal, ExportModal, GroupModal, + ViewsModal, } from "./components/modals"; import { @@ -29,11 +31,17 @@ import { createConnection, createProject, createSnapshot, + createView, + deleteView, + diffSnapshots, getSnapshot, + getView, listConnections, listProjects, listSnapshots, + listViews, } from "./api"; +import { applyLayout, captureLayout } from "./diagramViews"; import TableNode from "./erd/TableNode"; import { BUSINESS_GROUP_COLORS, @@ -58,7 +66,7 @@ import { } from "./erd/export"; import { exportMermaid } from "./erd/mermaid"; import { GRID_COLUMNS, GRID_X_GAP, GRID_Y_GAP } from "./erd/layoutConstants"; -import type { Connection, Project, Snapshot, SnapshotDetail } from "./types"; +import type { Connection, DiagramView, Project, SchemaDiff, Snapshot, SnapshotDetail } from "./types"; const TERMINAL_SNAPSHOT_STATUSES = new Set([ "succeeded", @@ -150,6 +158,19 @@ export default function App() { const [isShareLinkCopied, setIsShareLinkCopied] = useState(false); const [shareLinkError, setShareLinkError] = useState(null); + const [isDiffModalOpen, setIsDiffModalOpen] = useState(false); + const [diffBaseId, setDiffBaseId] = useState(null); + const [diffResult, setDiffResult] = useState(null); + const [diffNotFound, setDiffNotFound] = useState(false); + const [isDiffLoading, setIsDiffLoading] = useState(false); + const [diffError, setDiffError] = useState(null); + + const [isViewsModalOpen, setIsViewsModalOpen] = useState(false); + const [savedViews, setSavedViews] = useState([]); + const [newViewName, setNewViewName] = useState(""); + const [isSavingView, setIsSavingView] = useState(false); + const [viewsError, setViewsError] = useState(null); + const [editingEdge, setEditingEdge] = useState(null); const [editingNode, setEditingNode] = useState | null>(null); const [isEditTableModalOpen, setIsEditTableModalOpen] = useState(false); @@ -537,6 +558,87 @@ export default function App() { setIsExportModalOpen(true); } + function onOpenDiff() { + setDiffBaseId(null); + setDiffResult(null); + setDiffNotFound(false); + setDiffError(null); + setIsDiffModalOpen(true); + } + + function onCloseDiff() { + setIsDiffModalOpen(false); + } + + function onOpenViews() { + setViewsError(null); + setNewViewName(""); + setIsViewsModalOpen(true); + if (!selectedProjectId) return; + listViews(selectedProjectId) + .then(setSavedViews) + .catch(() => setViewsError("저장된 뷰를 불러오지 못했습니다.")); + } + + function onCloseViews() { + setIsViewsModalOpen(false); + } + + function onSaveCurrentView() { + if (!selectedProjectId || nodes.length === 0) return; + const name = newViewName.trim(); + if (!name) return; + setIsSavingView(true); + setViewsError(null); + createView(selectedProjectId, name, captureLayout(nodes)) + .then((view) => { + setSavedViews((prev) => [view, ...prev]); + setNewViewName(""); + }) + .catch(() => setViewsError("뷰 저장에 실패했습니다.")) + .finally(() => setIsSavingView(false)); + } + + function onLoadView(viewId: string) { + setViewsError(null); + getView(viewId) + .then((detail) => { + setNodes((prev) => applyLayout(prev, detail.layout_json)); + setIsViewsModalOpen(false); + }) + .catch(() => setViewsError("뷰를 불러오지 못했습니다.")); + } + + function onDeleteView(viewId: string) { + setViewsError(null); + deleteView(viewId) + .then(() => { + setSavedViews((prev) => + prev.filter((v) => v.diagram_view_uuid !== viewId), + ); + }) + .catch(() => setViewsError("뷰 삭제에 실패했습니다.")); + } + + function onSelectDiffBase(baseSnapshotId: string) { + setDiffBaseId(baseSnapshotId); + if (!snapshotId) return; + setIsDiffLoading(true); + setDiffError(null); + setDiffNotFound(false); + setDiffResult(null); + diffSnapshots(snapshotId, baseSnapshotId) + .then((res) => { + if (res.status === "not_found") { + setDiffNotFound(true); + } else { + setDiffResult(res.diff); + } + }) + .catch(() => setDiffError("스키마 비교를 불러오지 못했습니다.")) + .finally(() => setIsDiffLoading(false)); + } + function onCloseExport() { setIsExportModalOpen(false); setIsCopied(false); @@ -1423,6 +1525,34 @@ export default function App() { > ↗ + + + + +
+ + +
+ + {isLoading ? ( + 비교 중… + ) : null} + + {error ? ( +
{error}
+ ) : null} + + {notFound ? ( + + 스냅샷을 찾을 수 없거나 접근 권한이 없습니다. + + ) : null} + + {!isLoading && !error && !notFound && !selectedBaseId ? ( + 비교할 기준 스냅샷을 선택하세요. + ) : null} + + {diff && summary ? ( + <> +
+ {!summary.has_changes ? ( + 변경 없음 + ) : ( +
    +
  • 테이블 +{summary.tables_added} / -{summary.tables_removed} / 변경 {summary.tables_changed}
    • +
  • 컬럼 +{summary.columns_added} / -{summary.columns_removed} / 변경 {summary.columns_changed}
    • +
  • 외래키 +{summary.fks_added} / -{summary.fks_removed}
    • +
+ )} +
+ + {diff.tables.added.length > 0 ? ( +
+

추가된 테이블

+
    + {diff.tables.added.map((t) => ( +
  • + {t}
    • + ))} +
+
+ ) : null} + + {diff.tables.removed.length > 0 ? ( +
+

삭제된 테이블

+
    + {diff.tables.removed.map((t) => ( +
  • - {t}
    • + ))} +
+
+ ) : null} + + {diff.tables.changed.length > 0 ? ( +
+

변경된 테이블

+ {diff.tables.changed.map((t) => ( +
+ {t.table} + {t.primary_key ? ( +
+ PK: {t.primary_key.from.join(', ') || '—'} → {t.primary_key.to.join(', ') || '—'} +
+ ) : null} + {t.columns.added.map((c) => ( +
+ 컬럼 {c}
+ ))} + {t.columns.removed.map((c) => ( +
- 컬럼 {c}
+ ))} + {t.columns.changed.map((c) => ( +
+ ~ {c.column}: {c.from.data_type ?? '?'} + {c.from.is_not_null ? ' NOT NULL' : ''} → {c.to.data_type ?? '?'} + {c.to.is_not_null ? ' NOT NULL' : ''} +
+ ))} +
+ ))} +
+ ) : null} + + {diff.foreign_keys.added.length > 0 || diff.foreign_keys.removed.length > 0 ? ( +
+

외래키 변경

+ {diff.foreign_keys.added.map((fk, i) => ( +
+ + {fk.child_table}({fk.child_columns.join(', ')}) → {fk.parent_table}({fk.parent_columns.join(', ')}) +
+ ))} + {diff.foreign_keys.removed.map((fk, i) => ( +
+ - {fk.child_table}({fk.child_columns.join(', ')}) → {fk.parent_table}({fk.parent_columns.join(', ')}) +
+ ))} +
+ ) : null} + + ) : null} + + + ); +} diff --git a/frontend/src/components/modals/ViewsModal.test.tsx b/frontend/src/components/modals/ViewsModal.test.tsx new file mode 100644 index 00000000..62600821 --- /dev/null +++ b/frontend/src/components/modals/ViewsModal.test.tsx @@ -0,0 +1,66 @@ +import '@testing-library/jest-dom/vitest'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { ViewsModal } from './ViewsModal'; +import type { DiagramView } from '../../types'; + +const views: DiagramView[] = [ + { diagram_view_uuid: 'v1', name: '전체 개요', created_at: '', updated_at: '' }, + { diagram_view_uuid: 'v2', name: '주문 영역', created_at: '', updated_at: '' }, +]; + +const baseProps = { + isOpen: true, + views, + newViewName: '', + setNewViewName: vi.fn(), + isSaving: false, + error: null, + canSave: true, + onSaveCurrent: vi.fn(), + onLoadView: vi.fn(), + onDeleteView: vi.fn(), + onClose: vi.fn(), +}; + +afterEach(() => { + cleanup(); +}); + +describe('ViewsModal', () => { + it('lists saved views and loads one', () => { + const onLoadView = vi.fn(); + render(); + expect(screen.getByText('전체 개요')).toBeInTheDocument(); + expect(screen.getByText('주문 영역')).toBeInTheDocument(); + fireEvent.click(screen.getAllByRole('button', { name: '불러오기' })[1]); + expect(onLoadView).toHaveBeenCalledWith('v2'); + }); + + it('deletes a view', () => { + const onDeleteView = vi.fn(); + render(); + fireEvent.click(screen.getByRole('button', { name: '전체 개요 삭제' })); + expect(onDeleteView).toHaveBeenCalledWith('v1'); + }); + + it('saves the current layout when a name is entered', () => { + const onSaveCurrent = vi.fn(); + render(); + fireEvent.click(screen.getByRole('button', { name: '현재 배치 저장' })); + expect(onSaveCurrent).toHaveBeenCalledOnce(); + }); + + it('disables save when name is empty or nothing to save', () => { + const { rerender } = render(); + expect(screen.getByRole('button', { name: '현재 배치 저장' })).toBeDisabled(); + rerender(); + expect(screen.getByRole('button', { name: '현재 배치 저장' })).toBeDisabled(); + }); + + it('shows an empty-state hint when there are no views', () => { + render(); + expect(screen.getByText('저장된 뷰가 없습니다.')).toBeInTheDocument(); + }); +}); diff --git a/frontend/src/components/modals/ViewsModal.tsx b/frontend/src/components/modals/ViewsModal.tsx new file mode 100644 index 00000000..a7b58a7c --- /dev/null +++ b/frontend/src/components/modals/ViewsModal.tsx @@ -0,0 +1,115 @@ +import React from 'react'; +import type { DiagramView } from '../../types'; +import { useDialogAccessibility } from './useDialogAccessibility'; + +interface ViewsModalProps { + isOpen: boolean; + views: DiagramView[]; + newViewName: string; + setNewViewName: (value: string) => void; + isSaving: boolean; + error: string | null; + /** True when there is a layout worth saving (project selected + tables on canvas). */ + canSave: boolean; + onSaveCurrent: () => void; + onLoadView: (viewId: string) => void; + onDeleteView: (viewId: string) => void; + onClose: () => void; +} + +export function ViewsModal({ + isOpen, + views, + newViewName, + setNewViewName, + isSaving, + error, + canSave, + onSaveCurrent, + onLoadView, + onDeleteView, + onClose, +}: ViewsModalProps) { + const dialogRef = useDialogAccessibility(isOpen, onClose); + + if (!isOpen) return null; + + return ( +
+
+
+
+

저장된 뷰

+

+ 현재 캔버스 배치를 이름을 지정해 저장하고, 저장한 배치를 다시 불러옵니다. +

+
+ +
+ +
+ +
+ setNewViewName(e.target.value)} + placeholder="뷰 이름" + aria-label="뷰 이름" + /> + +
+ {!canSave ? ( + + 프로젝트를 선택하고 캔버스에 테이블이 있어야 저장할 수 있습니다. + + ) : null} +
+ + {error ? ( +
{error}
+ ) : null} + +
+ {views.length === 0 ? ( + 저장된 뷰가 없습니다. + ) : ( +
    + {views.map((view) => ( +
  • + {view.name} + + + + +
    • + ))} +
+ )} +
+
+
+ ); +} diff --git a/frontend/src/components/modals/index.ts b/frontend/src/components/modals/index.ts index 0cf1f546..9c50dda2 100644 --- a/frontend/src/components/modals/index.ts +++ b/frontend/src/components/modals/index.ts @@ -1,6 +1,8 @@ export { AddTableModal } from './AddTableModal'; export { CardinalityModal } from './CardinalityModal'; +export { DiffModal } from './DiffModal'; export { EditEdgeModal } from './EditEdgeModal'; export { EditTableModal } from './EditTableModal'; export { ExportModal } from './ExportModal'; export { GroupModal } from './GroupModal'; +export { ViewsModal } from './ViewsModal'; diff --git a/frontend/src/diagramViews.test.ts b/frontend/src/diagramViews.test.ts new file mode 100644 index 00000000..ac4c4eef --- /dev/null +++ b/frontend/src/diagramViews.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from 'vitest' + +import { applyLayout, captureLayout } from './diagramViews' + +const nodes = [ + { id: 'public.member', position: { x: 10, y: 20 }, data: {} }, + { id: 'public.orders', position: { x: 300, y: 140 }, data: {} }, +] + +describe('captureLayout', () => { + it('captures node positions keyed by id', () => { + expect(captureLayout(nodes)).toEqual({ + positions: { + 'public.member': { x: 10, y: 20 }, + 'public.orders': { x: 300, y: 140 }, + }, + }) + }) + + it('round-trips through applyLayout', () => { + const layout = captureLayout(nodes) + const moved = nodes.map((n) => ({ ...n, position: { x: 0, y: 0 } })) + const restored = applyLayout(moved, layout) + expect(restored[0].position).toEqual({ x: 10, y: 20 }) + expect(restored[1].position).toEqual({ x: 300, y: 140 }) + }) +}) + +describe('applyLayout', () => { + it('updates only nodes present in the layout, leaving others unchanged', () => { + const layout = { positions: { 'public.member': { x: 99, y: 88 } } } + const result = applyLayout(nodes, layout) + expect(result[0].position).toEqual({ x: 99, y: 88 }) + // orders not in layout → unchanged (same reference is fine) + expect(result[1].position).toEqual({ x: 300, y: 140 }) + }) + + it('ignores layout entries for nodes that no longer exist', () => { + const layout = { positions: { 'public.deleted': { x: 1, y: 2 }, 'public.member': { x: 5, y: 6 } } } + const result = applyLayout(nodes, layout) + expect(result.map((n) => n.id)).toEqual(['public.member', 'public.orders']) + expect(result[0].position).toEqual({ x: 5, y: 6 }) + }) + + it('is a no-op for null/empty layout', () => { + expect(applyLayout(nodes, null)).toEqual(nodes) + expect(applyLayout(nodes, { positions: {} })).toEqual(nodes) + }) +}) diff --git a/frontend/src/diagramViews.ts b/frontend/src/diagramViews.ts new file mode 100644 index 00000000..799ffe5b --- /dev/null +++ b/frontend/src/diagramViews.ts @@ -0,0 +1,32 @@ +import type { ViewLayout } from './types' + +// A structural node shape — anything with an id and a position. Kept minimal so +// these helpers stay decoupled from @xyflow/react's full Node type and easy to test. +type PositionedNode = { id: string; position: { x: number; y: number } } + +/** Capture the current canvas layout (node positions) into a saveable payload. */ +export function captureLayout(nodes: T[]): ViewLayout { + const positions: ViewLayout['positions'] = {} + for (const node of nodes) { + positions[node.id] = { x: node.position.x, y: node.position.y } + } + return { positions } +} + +/** + * Apply a saved layout to the current nodes, returning a new array with updated + * positions. Nodes not present in the layout are returned unchanged (so a view + * saved before a table existed still loads cleanly), and layout entries for + * nodes that no longer exist are ignored. + */ +export function applyLayout( + nodes: T[], + layout: ViewLayout | null | undefined, +): T[] { + const positions = layout?.positions ?? {} + return nodes.map((node) => { + const saved = positions[node.id] + if (!saved) return node + return { ...node, position: { x: saved.x, y: saved.y } } + }) +} diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 6926aa6b..186015fc 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -94,6 +94,62 @@ export type SnapshotDetail = { snapshot_json: SnapshotJson | null } +// Schema diff (see backend app/diff/schema_diff.py). All keys are name-based so +// the diff is stable across introspection runs (relation_oids are re-assigned). +export type DiffColumnShape = { data_type: string | null; is_not_null: boolean } +export type DiffColumnChange = { column: string; from: DiffColumnShape; to: DiffColumnShape } +export type DiffTableChange = { + table: string + columns: { added: string[]; removed: string[]; changed: DiffColumnChange[] } + primary_key?: { from: string[]; to: string[] } + comment?: { from: string | null; to: string | null } +} +export type DiffFkRef = { + name: string | null + child_table: string + child_columns: string[] + parent_table: string + parent_columns: string[] +} +export type SchemaDiff = { + base_table_count: number + target_table_count: number + tables: { added: string[]; removed: string[]; changed: DiffTableChange[] } + foreign_keys: { added: DiffFkRef[]; removed: DiffFkRef[] } + summary: { + tables_added: number + tables_removed: number + tables_changed: number + columns_added: number + columns_removed: number + columns_changed: number + fks_added: number + fks_removed: number + has_changes: boolean + } +} +export type SnapshotDiffResult = { + base_snapshot_uuid: string + target_snapshot_uuid: string + status: string + diff: SchemaDiff | null +} + +// Saved diagram views (see backend app/api/diagram_views.py). layout_json is an +// opaque client payload; captureLayout/applyLayout in diagramViews.ts own its shape. +export type ViewLayout = { + positions: Record +} +export type DiagramView = { + diagram_view_uuid: string + name: string + created_at: string + updated_at: string +} +export type DiagramViewDetail = DiagramView & { + layout_json: ViewLayout +} + export type SnapshotDetailResponse = Omit & { error_message: unknown } From c17831ed8b25ba1679fef0d389f7eff1d3c435b0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 5 Jul 2026 20:50:41 +0900 Subject: [PATCH 04/10] test(dsn-guard): cover SSRF guard reject/dedup branches Close untested branches in the DSN SSRF guard: non-integer query port, host resolving to no usable address (empty sockaddr), and host dedup. dsn_guard.py coverage 96% -> 100%. Full backend suite 238 passed. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AxU2xaupAjp912oDNFuWyd --- backend/tests/test_dsn_guard.py | 41 +++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/backend/tests/test_dsn_guard.py b/backend/tests/test_dsn_guard.py index c8d2b3ee..cfb7bc4a 100644 --- a/backend/tests/test_dsn_guard.py +++ b/backend/tests/test_dsn_guard.py @@ -274,3 +274,44 @@ def raise_gaierror(*args, **kwargs): with pytest.raises(DsnTargetError, match="database host could not be resolved"): await validate_postgres_dsn_target("postgresql://user:pass@db.example.com/app") + + +def test_unique_hosts_deduplicates_repeated_hosts() -> None: + assert _unique_hosts(["1.2.3.4", "1.2.3.4", "5.6.7.8"]) == ( + "1.2.3.4", + "5.6.7.8", + ) + + +@pytest.mark.asyncio +async def test_dsn_guard_rejects_non_integer_query_port( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + settings, "db_introspection_allowed_hosts", "db.example.com" + ) + with pytest.raises(DsnTargetError, match="query port is invalid"): + await validate_postgres_dsn_target( + "postgresql://user:pass@db.example.com/app?port=abc" + ) + + +@pytest.mark.asyncio +async def test_dsn_guard_rejects_host_resolving_to_no_usable_address( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # getaddrinfo yields an entry whose sockaddr is empty -> skipped -> no IPs left. + monkeypatch.setattr( + socket, + "getaddrinfo", + lambda *_args, **_kwargs: [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ())], + ) + monkeypatch.setattr( + settings, "db_introspection_allowed_hosts", "db.example.com" + ) + with pytest.raises( + DsnTargetError, match="did not resolve to an IP address" + ): + await validate_postgres_dsn_target( + "postgresql://user:pass@db.example.com/app" + ) From 364c183a0c241fee6314087db049500b2e8572e3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 5 Jul 2026 20:53:05 +0900 Subject: [PATCH 05/10] test: cover storage-sanitize and schema-diff edge branches sanitize_for_storage: memoryview/bytes/bytearray -> text, invalid-utf8 base64 fallback, tuple recursion (75% -> 100%). schema_diff: orphan/nameless column/pk/fk rows are skipped and table-comment changes detected (94% -> 100%). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AxU2xaupAjp912oDNFuWyd --- backend/tests/test_sanitize.py | 16 +++++++++ backend/tests/test_schema_diff.py | 57 +++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/backend/tests/test_sanitize.py b/backend/tests/test_sanitize.py index 6f8002c3..54f577f9 100644 --- a/backend/tests/test_sanitize.py +++ b/backend/tests/test_sanitize.py @@ -13,3 +13,19 @@ def test_strip_nul_removes_nul_characters_from_strings() -> None: assert strip_nul("hello\x00world") == "helloworld" assert strip_nul("\x00") == "" assert strip_nul("no nulls here") == "no nulls here" + + +def test_sanitize_converts_memoryview_and_bytes_to_text() -> None: + import base64 + + assert sanitize_for_storage(memoryview(b"hi")) == "hi" + assert sanitize_for_storage(b"ok\x00") == "ok" # utf-8 decode + NUL strip + assert sanitize_for_storage(bytearray(b"a")) == "a" + raw = b"\xff\xfe" # invalid utf-8 -> base64 fallback + assert sanitize_for_storage(raw) == base64.b64encode(raw).decode("ascii") + + +def test_sanitize_preserves_tuples_and_passes_through_scalars() -> None: + assert sanitize_for_storage(("a\x00", ["b\x00"])) == ("a", ["b"]) + assert sanitize_for_storage(5) == 5 + assert sanitize_for_storage(None) is None diff --git a/backend/tests/test_schema_diff.py b/backend/tests/test_schema_diff.py index 402a41e2..57765916 100644 --- a/backend/tests/test_schema_diff.py +++ b/backend/tests/test_schema_diff.py @@ -203,3 +203,60 @@ def test_none_and_empty_snapshots_are_safe(): d = diff_snapshots(None, _base()) assert d["summary"]["tables_added"] == 2 assert diff_snapshots(_base(), None)["summary"]["tables_removed"] == 2 + + +def test_diff_skips_orphan_nameless_rows_and_detects_comment_change(): + # Rows referencing an unknown relation_oid or missing a name must be skipped, + # not treated as spurious changes; a table-comment change is a real change. + base = _snap( + relations=[ + { + "relation_oid": 1, + "schema_name": "public", + "relation_name": "t", + "relation_comment": "old note", + } + ], + columns=[ + {"relation_oid": 1, "column_name": "id", "data_type": "int", "is_not_null": True}, + {"relation_oid": 999, "column_name": "ghost", "data_type": "int"}, # orphan oid + {"relation_oid": 1, "column_name": None, "data_type": "int"}, # no name + ], + pk_columns=[ + {"relation_oid": 999, "column_name": "id", "column_ordinal": 1}, # orphan oid + {"relation_oid": 1, "column_name": None, "column_ordinal": 1}, # no name + ], + fk_edges=[ + { + "fk_constraint_name": "x", + "child_relation_oid": 999, # orphan child -> skipped + "parent_relation_oid": 1, + "child_column_name": "a", + "parent_column_name": "b", + "column_ordinal": 1, + } + ], + ) + target = _snap( + relations=[ + { + "relation_oid": 1, + "schema_name": "public", + "relation_name": "t", + "relation_comment": "new note", # comment changed + } + ], + columns=[ + {"relation_oid": 1, "column_name": "id", "data_type": "int", "is_not_null": True} + ], + ) + d = diff_snapshots(base, target) + changed = d["tables"]["changed"] + assert len(changed) == 1 + assert changed[0]["table"] == "public.t" + assert changed[0]["comment"] == {"from": "old note", "to": "new note"} + # orphan/nameless rows and the orphan FK produced no spurious changes + assert changed[0]["columns"]["added"] == [] + assert changed[0]["columns"]["removed"] == [] + assert d["summary"]["fks_added"] == 0 + assert d["summary"]["fks_removed"] == 0 From 1fa5d9bd1a05a3bcace7512049841100882b7adf Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 5 Jul 2026 21:30:10 +0900 Subject: [PATCH 06/10] feat(annotations): per-project table annotations (by table name) TableAnnotation model + 0006 migration + IDOR-guarded CRUD (list/upsert/delete). Notes are keyed by (project, schema_name, relation_name) -- never the volatile relation_oid -- with a unique constraint (one note per table). Editor role for writes, uniform 404 for missing/unauthorized. Migration verified against Postgres; full backend suite 245 passing. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AxU2xaupAjp912oDNFuWyd --- .../alembic/versions/0006_table_annotation.py | 55 +++++++ backend/app/api/annotations.py | 139 ++++++++++++++++++ backend/app/main.py | 2 + backend/app/models.py | 51 ++++++- backend/app/schemas.py | 19 +++ backend/tests/test_api_annotations.py | 119 +++++++++++++++ 6 files changed, 384 insertions(+), 1 deletion(-) create mode 100644 backend/alembic/versions/0006_table_annotation.py create mode 100644 backend/app/api/annotations.py create mode 100644 backend/tests/test_api_annotations.py diff --git a/backend/alembic/versions/0006_table_annotation.py b/backend/alembic/versions/0006_table_annotation.py new file mode 100644 index 00000000..333e1310 --- /dev/null +++ b/backend/alembic/versions/0006_table_annotation.py @@ -0,0 +1,55 @@ +"""table_annotation: per-project notes attached to tables by name + +Revision ID: 0006_table_annotation +Revises: 0005_diagram_view +Create Date: 2026-07-05 + +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op + +revision = "0006_table_annotation" +down_revision = "0005_diagram_view" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "table_annotation", + sa.Column("table_annotation_uuid", sa.Uuid(), primary_key=True, nullable=False), + sa.Column("project_space_uuid", sa.Uuid(), nullable=False), + sa.Column("schema_name", sa.Text(), nullable=False), + sa.Column("relation_name", sa.Text(), nullable=False), + sa.Column("body", sa.Text(), nullable=False), + sa.Column("created_by", sa.Uuid(), nullable=True), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), + sa.ForeignKeyConstraint( + ["project_space_uuid"], + ["project_space.project_space_uuid"], + ondelete="CASCADE", + ), + sa.PrimaryKeyConstraint("table_annotation_uuid"), + sa.UniqueConstraint( + "project_space_uuid", + "schema_name", + "relation_name", + name="uq_table_annotation__project_table", + ), + ) + op.create_index( + "ix_table_annotation__project_space_uuid", + "table_annotation", + ["project_space_uuid"], + ) + + +def downgrade() -> None: + op.drop_index( + "ix_table_annotation__project_space_uuid", table_name="table_annotation" + ) + op.drop_table("table_annotation") diff --git a/backend/app/api/annotations.py b/backend/app/api/annotations.py new file mode 100644 index 00000000..2f47c7af --- /dev/null +++ b/backend/app/api/annotations.py @@ -0,0 +1,139 @@ +from __future__ import annotations + +import datetime as dt +import uuid + +from fastapi import APIRouter, Depends, HTTPException +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.auth import CurrentUser, get_current_user +from app.db import get_read_session, get_session +from app.models import TableAnnotation +from app.permissions import require_project_member +from app.schemas import TableAnnotationOut, TableAnnotationUpsertIn + +router = APIRouter(prefix="/api/annotations", tags=["annotations"]) + + +def _to_out(ann: TableAnnotation) -> TableAnnotationOut: + return TableAnnotationOut( + table_annotation_uuid=ann.table_annotation_uuid, + schema_name=ann.schema_name, + relation_name=ann.relation_name, + body=ann.body, + created_at=ann.created_at, + updated_at=ann.updated_at, + ) + + +async def _get_authorized_annotation( + session: AsyncSession, + table_annotation_uuid: uuid.UUID, + user: CurrentUser, + minimum_role: str | None = None, +) -> TableAnnotation | None: + """Fetch an annotation only after project membership has been checked. + + Returns ``None`` for both missing and unauthorized so callers can respond + with a uniform 404 (no existence enumeration / IDOR). + """ + project_space_uuid = await session.scalar( + select(TableAnnotation.project_space_uuid).where( + TableAnnotation.table_annotation_uuid == table_annotation_uuid + ) + ) + if project_space_uuid is None: + return None + try: + await require_project_member( + session, + project_space_uuid, + user.user_account_uuid, + minimum_role=minimum_role, + ) + except HTTPException as exc: + if exc.status_code == 403: + return None + raise + return await session.get(TableAnnotation, table_annotation_uuid) + + +@router.get( + "/by-project/{project_space_uuid}", response_model=list[TableAnnotationOut] +) +async def list_annotations( + project_space_uuid: uuid.UUID, + user: CurrentUser = Depends(get_current_user), + session: AsyncSession = Depends(get_read_session), +) -> list[TableAnnotationOut]: + """List table annotations for a project.""" + await require_project_member(session, project_space_uuid, user.user_account_uuid) + rows = await session.execute( + select(TableAnnotation) + .where(TableAnnotation.project_space_uuid == project_space_uuid) + .order_by(TableAnnotation.schema_name, TableAnnotation.relation_name) + ) + return [_to_out(a) for a in rows.scalars().all()] + + +@router.put( + "/by-project/{project_space_uuid}", response_model=TableAnnotationOut +) +async def upsert_annotation( + project_space_uuid: uuid.UUID, + body: TableAnnotationUpsertIn, + user: CurrentUser = Depends(get_current_user), + session: AsyncSession = Depends(get_session), +) -> TableAnnotationOut: + """Create or update the note for one table in a project (editor role). + + Keyed by (project, schema_name, relation_name); a unique constraint keeps + it to a single note per table. + """ + await require_project_member( + session, project_space_uuid, user.user_account_uuid, minimum_role="editor" + ) + now = dt.datetime.now(dt.timezone.utc) + existing = await session.scalar( + select(TableAnnotation).where( + TableAnnotation.project_space_uuid == project_space_uuid, + TableAnnotation.schema_name == body.schema_name, + TableAnnotation.relation_name == body.relation_name, + ) + ) + if existing is not None: + existing.body = body.body + existing.updated_at = now + ann = existing + else: + ann = TableAnnotation( + table_annotation_uuid=uuid.uuid4(), + project_space_uuid=project_space_uuid, + schema_name=body.schema_name, + relation_name=body.relation_name, + body=body.body, + created_by=user.user_account_uuid, + created_at=now, + updated_at=now, + ) + session.add(ann) + await session.commit() + return _to_out(ann) + + +@router.delete("/{table_annotation_uuid}") +async def delete_annotation( + table_annotation_uuid: uuid.UUID, + user: CurrentUser = Depends(get_current_user), + session: AsyncSession = Depends(get_session), +) -> dict[str, bool]: + """Delete a table annotation (requires editor membership on its project).""" + ann = await _get_authorized_annotation( + session, table_annotation_uuid, user, minimum_role="editor" + ) + if ann is None: + raise HTTPException(status_code=404, detail="annotation not found") + await session.delete(ann) + await session.commit() + return {"ok": True} diff --git a/backend/app/main.py b/backend/app/main.py index bfa30d02..c191880a 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -8,6 +8,7 @@ from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware +from app.api.annotations import router as annotations_router from app.api.connections import router as connections_router from app.api.auth_routes import router as auth_router from app.api.diagram_views import router as diagram_views_router @@ -167,6 +168,7 @@ async def csrf_token() -> dict[str, str]: app.include_router(connections_router) app.include_router(snapshots_router) app.include_router(diagram_views_router) +app.include_router(annotations_router) app.include_router(me_router) app.include_router(share_router) app.include_router(auth_router) diff --git a/backend/app/models.py b/backend/app/models.py index cb535fa6..9c0fd845 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -3,7 +3,15 @@ import datetime as dt import uuid -from sqlalchemy import DateTime, ForeignKey, Index, Integer, LargeBinary, Text +from sqlalchemy import ( + DateTime, + ForeignKey, + Index, + Integer, + LargeBinary, + Text, + UniqueConstraint, +) from sqlalchemy.dialects.postgresql import JSONB, UUID from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column @@ -213,6 +221,47 @@ class DiagramView(Base): ) +class TableAnnotation(Base): + """A user note attached to a table within a project. + + Tables are identified by ``(schema_name, relation_name)`` -- never by the + volatile ``relation_oid``, which is reassigned on every introspection run. + At most one annotation exists per (project, schema, table). + """ + + __tablename__ = "table_annotation" + + table_annotation_uuid: Mapped[uuid.UUID] = mapped_column( + UUID(as_uuid=True), primary_key=True, default=uuid.uuid4 + ) + project_space_uuid: Mapped[uuid.UUID] = mapped_column( + UUID(as_uuid=True), + ForeignKey("project_space.project_space_uuid", ondelete="CASCADE"), + index=True, + ) + schema_name: Mapped[str] = mapped_column(Text()) + relation_name: Mapped[str] = mapped_column(Text()) + body: Mapped[str] = mapped_column(Text()) + created_by: Mapped[uuid.UUID | None] = mapped_column( + UUID(as_uuid=True), nullable=True + ) + created_at: Mapped[dt.datetime] = mapped_column( + DateTime(timezone=True), default=utcnow + ) + updated_at: Mapped[dt.datetime] = mapped_column( + DateTime(timezone=True), default=utcnow, onupdate=utcnow + ) + + __table_args__ = ( + UniqueConstraint( + "project_space_uuid", + "schema_name", + "relation_name", + name="uq_table_annotation__project_table", + ), + ) + + class ShareLink(Base): """Public share link granting read access to a project's snapshots.""" diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 692aff7a..d7969d5b 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -130,6 +130,25 @@ class DiagramViewDetailOut(DiagramViewOut): layout_json: dict +class TableAnnotationUpsertIn(BaseModel): + """Request body for creating/updating a table annotation.""" + + schema_name: str = Field(min_length=1, max_length=255) + relation_name: str = Field(min_length=1, max_length=255) + body: str = Field(min_length=1, max_length=10_000) + + +class TableAnnotationOut(BaseModel): + """A table annotation.""" + + table_annotation_uuid: uuid.UUID + schema_name: str + relation_name: str + body: str + created_at: dt.datetime + updated_at: dt.datetime + + class MeOut(BaseModel): """Current user payload returned by /me.""" diff --git a/backend/tests/test_api_annotations.py b/backend/tests/test_api_annotations.py new file mode 100644 index 00000000..2987a2a7 --- /dev/null +++ b/backend/tests/test_api_annotations.py @@ -0,0 +1,119 @@ +import datetime as dt +import uuid +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +import pytest +from fastapi import HTTPException + +from app.api.annotations import ( + delete_annotation, + list_annotations, + upsert_annotation, +) +from app.auth import CurrentUser +from app.schemas import TableAnnotationUpsertIn + + +def _user(): + return CurrentUser( + user_account_uuid=uuid.uuid4(), subject="test", display_name="Test" + ) + + +@pytest.mark.asyncio +async def test_delete_annotation_returns_404_when_missing_or_unauthorized(): + # Uniform 404 for both missing and unauthorized (no enumeration / IDOR). + session = AsyncMock() + with patch( + "app.api.annotations._get_authorized_annotation", + new_callable=AsyncMock, + return_value=None, + ): + with pytest.raises(HTTPException) as exc: + await delete_annotation( + table_annotation_uuid=uuid.uuid4(), user=_user(), session=session + ) + assert exc.value.status_code == 404 + session.delete.assert_not_called() + + +@pytest.mark.asyncio +async def test_upsert_creates_new_annotation_when_absent(): + session = AsyncMock() + session.scalar = AsyncMock(return_value=None) # no existing row + body = TableAnnotationUpsertIn( + schema_name="public", relation_name="orders", body="핵심 주문 테이블" + ) + with patch( + "app.api.annotations.require_project_member", new_callable=AsyncMock + ): + out = await upsert_annotation( + project_space_uuid=uuid.uuid4(), + body=body, + user=_user(), + session=session, + ) + assert out.schema_name == "public" + assert out.relation_name == "orders" + assert out.body == "핵심 주문 테이블" + session.add.assert_called_once() + session.commit.assert_awaited() + + +@pytest.mark.asyncio +async def test_upsert_updates_existing_annotation_without_insert(): + session = AsyncMock() + now = dt.datetime.now(dt.timezone.utc) + existing = SimpleNamespace( + table_annotation_uuid=uuid.uuid4(), + schema_name="public", + relation_name="orders", + body="old", + created_at=now, + updated_at=now, + ) + session.scalar = AsyncMock(return_value=existing) + body = TableAnnotationUpsertIn( + schema_name="public", relation_name="orders", body="new note" + ) + with patch( + "app.api.annotations.require_project_member", new_callable=AsyncMock + ): + out = await upsert_annotation( + project_space_uuid=uuid.uuid4(), + body=body, + user=_user(), + session=session, + ) + assert out.body == "new note" + assert existing.body == "new note" # updated in place, not re-inserted + session.add.assert_not_called() + session.commit.assert_awaited() + + +@pytest.mark.asyncio +async def test_list_annotations_returns_project_notes(): + session = AsyncMock() + now = dt.datetime.now(dt.timezone.utc) + ann = SimpleNamespace( + table_annotation_uuid=uuid.uuid4(), + schema_name="public", + relation_name="member", + body="회원 마스터", + created_at=now, + updated_at=now, + ) + result = SimpleNamespace( + scalars=lambda: SimpleNamespace(all=lambda: [ann]) + ) + session.execute = AsyncMock(return_value=result) + with patch( + "app.api.annotations.require_project_member", new_callable=AsyncMock + ): + out = await list_annotations( + project_space_uuid=uuid.uuid4(), user=_user(), session=session + ) + assert len(out) == 1 + assert out[0].relation_name == "member" + assert out[0].body == "회원 마스터" From 3c52e3ffa75ad7bfe92ae9ea14cd7b1aa0672eff Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 5 Jul 2026 21:35:32 +0900 Subject: [PATCH 07/10] feat(connections): connection health test endpoint POST /api/connections/{uuid}/test probes a stored connection and reports ok/server_version/error. IDOR-safe (project membership required, uniform 404). Reuses the introspectors' SSRF guard: probe_postgres pins to validate_postgres_dsn_target's IPs, probe_snowflake goes through _parse_snowflake_dsn; errors are DSN-redacted and an unreachable DB is ok=false (not an error status). Backend suite green. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AxU2xaupAjp912oDNFuWyd --- backend/app/api/connections.py | 46 ++++++- backend/app/db_introspect.py | 20 ++- backend/app/pg_introspect/introspect.py | 23 ++++ backend/app/schemas.py | 8 ++ .../app/snowflake_introspect/introspect.py | 19 +++ backend/tests/test_api_connection_test.py | 120 ++++++++++++++++++ 6 files changed, 232 insertions(+), 4 deletions(-) create mode 100644 backend/tests/test_api_connection_test.py diff --git a/backend/app/api/connections.py b/backend/app/api/connections.py index 5689ce62..b34423b6 100644 --- a/backend/app/api/connections.py +++ b/backend/app/api/connections.py @@ -3,16 +3,17 @@ import datetime as dt import uuid -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, HTTPException from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from app.auth import CurrentUser, get_current_user from app.db import get_read_session, get_session +from app.db_introspect import probe_database from app.models import DbConnection from app.permissions import require_project_member -from app.schemas import ConnectionCreateIn, ConnectionOut -from app.security import encrypt_text +from app.schemas import ConnectionCreateIn, ConnectionOut, ConnectionTestOut +from app.security import decrypt_text, encrypt_text from app.sanitize import sanitize_for_storage router = APIRouter(prefix="/api/connections", tags=["connections"]) @@ -62,3 +63,42 @@ async def create_connection( session.add(c) await session.commit() return ConnectionOut(db_connection_uuid=c.db_connection_uuid, conn_name=c.conn_name) + + +@router.post("/{db_connection_uuid}/test", response_model=ConnectionTestOut) +async def test_connection( + db_connection_uuid: uuid.UUID, + user: CurrentUser = Depends(get_current_user), + session: AsyncSession = Depends(get_read_session), +) -> ConnectionTestOut: + """Probe a stored connection's DSN and report reachability. + + IDOR-safe: the connection's project is resolved first and membership is + required, with a uniform 404 for missing/unauthorized. The DSN is decrypted + only in memory; the live probe reuses the introspectors' SSRF guard, and any + failure message is DSN-redacted (``ok=false`` rather than an error status, + since an unreachable database is a normal result). + """ + project_space_uuid = await session.scalar( + select(DbConnection.project_space_uuid).where( + DbConnection.db_connection_uuid == db_connection_uuid + ) + ) + if project_space_uuid is None: + raise HTTPException(status_code=404, detail="connection not found") + try: + await require_project_member( + session, project_space_uuid, user.user_account_uuid + ) + except HTTPException as exc: + if exc.status_code == 403: + raise HTTPException(status_code=404, detail="connection not found") + raise + + conn = await session.get(DbConnection, db_connection_uuid) + dsn = decrypt_text(conn.dsn_ciphertext, conn.dsn_nonce) + try: + version = await probe_database(dsn) + return ConnectionTestOut(ok=True, server_version=version, error=None) + except Exception as exc: # noqa: BLE001 - message is already DSN-redacted + return ConnectionTestOut(ok=False, server_version=None, error=str(exc)) diff --git a/backend/app/db_introspect.py b/backend/app/db_introspect.py index 934a5cb5..6b41776e 100644 --- a/backend/app/db_introspect.py +++ b/backend/app/db_introspect.py @@ -4,8 +4,9 @@ from urllib.parse import urlparse from app.dsn_redaction import redact_dsn_error_message -from app.pg_introspect.introspect import introspect_postgres +from app.pg_introspect.introspect import introspect_postgres, probe_postgres from app.snowflake_introspect import introspect_snowflake +from app.snowflake_introspect.introspect import probe_snowflake DatabaseDialect = Literal["postgresql", "snowflake"] @@ -32,3 +33,20 @@ async def introspect_database(dsn: str, schema_filter: str | None) -> dict: except Exception as exc: message = str(exc) or type(exc).__name__ raise RuntimeError(redact_dsn_error_message(message, dsn)) from None + + +async def probe_database(dsn: str) -> str: + """Lightweight connectivity probe; returns the server version string. + + Reuses the dialect introspectors' SSRF-guarded connection setup. Errors are + DSN-redacted so credentials never surface in an API response. + """ + + try: + dialect = detect_dsn_dialect(dsn) + if dialect == "snowflake": + return await probe_snowflake(dsn) + return await probe_postgres(dsn) + except Exception as exc: + message = str(exc) or type(exc).__name__ + raise RuntimeError(redact_dsn_error_message(message, dsn)) from None diff --git a/backend/app/pg_introspect/introspect.py b/backend/app/pg_introspect/introspect.py index 4fb1ebc4..02d9cb5d 100644 --- a/backend/app/pg_introspect/introspect.py +++ b/backend/app/pg_introspect/introspect.py @@ -10,6 +10,29 @@ from app.sanitize import sanitize_for_storage +async def probe_postgres(dsn: str) -> str: + """SSRF-guarded connectivity check: connect and return the server version. + + Reuses ``validate_postgres_dsn_target`` and connects to the pinned IP, so + the same anti-SSRF guarantees as full introspection apply. + """ + target = await validate_postgres_dsn_target(dsn) + connect_host: str | list[str] = ( + target.hosts[0] if len(target.hosts) == 1 else list(target.hosts) + ) + if target.port is not None: + conn = await asyncpg.connect( + dsn, host=connect_host, port=target.port, timeout=10 + ) + else: + conn = await asyncpg.connect(dsn, host=connect_host, timeout=10) + try: + await conn.fetchval("SELECT 1") + return str(await conn.fetchval("SHOW server_version")) + finally: + await conn.close() + + async def introspect_postgres(dsn: str, schema_filter: str | None) -> dict: """Introspect a PostgreSQL database and return a snapshot JSON.""" diff --git a/backend/app/schemas.py b/backend/app/schemas.py index d7969d5b..9540c560 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -59,6 +59,14 @@ class ConnectionOut(BaseModel): conn_name: str +class ConnectionTestOut(BaseModel): + """Result of a connection health probe (DSN-redacted on failure).""" + + ok: bool + server_version: str | None = None + error: str | None = None + + class SnapshotCreateIn(BaseModel): """Request body for creating a schema snapshot.""" diff --git a/backend/app/snowflake_introspect/introspect.py b/backend/app/snowflake_introspect/introspect.py index dd0e3767..59ec20c7 100644 --- a/backend/app/snowflake_introspect/introspect.py +++ b/backend/app/snowflake_introspect/introspect.py @@ -702,6 +702,25 @@ def _introspect_snowflake_sync_with_config( ) +def _probe_snowflake_sync(config: SnowflakeDsnConfig) -> str: + conn = _connect(**config.connect_kwargs()) + cursor = conn.cursor() + try: + rows = _fetch_dicts(cursor, "SELECT CURRENT_VERSION() AS version") + return str(rows[0].get("version") or "") if rows else "" + finally: + try: + cursor.close() + finally: + conn.close() + + +async def probe_snowflake(dsn: str) -> str: + """SSRF-guarded connectivity check for Snowflake; returns the server version.""" + config = await _parse_snowflake_dsn(dsn) + return await asyncio.to_thread(_probe_snowflake_sync, config) + + async def introspect_snowflake(dsn: str, schema_filter: str | None) -> dict: """Introspect Snowflake metadata into the common snapshot JSON shape.""" diff --git a/backend/tests/test_api_connection_test.py b/backend/tests/test_api_connection_test.py new file mode 100644 index 00000000..5c2104e9 --- /dev/null +++ b/backend/tests/test_api_connection_test.py @@ -0,0 +1,120 @@ +import uuid +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +import pytest +from fastapi import HTTPException + +from app.api.connections import test_connection as run_connection_test +from app.auth import CurrentUser +from app.db_introspect import probe_database + + +def _user(): + return CurrentUser( + user_account_uuid=uuid.uuid4(), subject="t", display_name="T" + ) + + +def _conn(): + return SimpleNamespace(dsn_ciphertext=b"ciphertext", dsn_nonce=b"nonce") + + +@pytest.mark.asyncio +async def test_test_connection_returns_404_when_missing(): + session = AsyncMock() + session.scalar = AsyncMock(return_value=None) # no such connection + with pytest.raises(HTTPException) as exc: + await run_connection_test( + db_connection_uuid=uuid.uuid4(), user=_user(), session=session + ) + assert exc.value.status_code == 404 + + +@pytest.mark.asyncio +async def test_test_connection_masks_forbidden_as_404(): + # A non-member must not learn the connection exists (IDOR). + session = AsyncMock() + session.scalar = AsyncMock(return_value=uuid.uuid4()) + with patch( + "app.api.connections.require_project_member", + new_callable=AsyncMock, + side_effect=HTTPException(status_code=403, detail="denied"), + ): + with pytest.raises(HTTPException) as exc: + await run_connection_test( + db_connection_uuid=uuid.uuid4(), user=_user(), session=session + ) + assert exc.value.status_code == 404 + + +@pytest.mark.asyncio +async def test_test_connection_reports_ok_true_on_success(): + session = AsyncMock() + session.scalar = AsyncMock(return_value=uuid.uuid4()) + session.get = AsyncMock(return_value=_conn()) + with patch( + "app.api.connections.require_project_member", new_callable=AsyncMock + ), patch( + "app.api.connections.decrypt_text", + return_value="postgresql://u@db.example.com/app", + ), patch( + "app.api.connections.probe_database", + new_callable=AsyncMock, + return_value="16.2", + ): + out = await run_connection_test( + db_connection_uuid=uuid.uuid4(), user=_user(), session=session + ) + assert out.ok is True + assert out.server_version == "16.2" + assert out.error is None + + +@pytest.mark.asyncio +async def test_test_connection_reports_ok_false_on_probe_failure(): + session = AsyncMock() + session.scalar = AsyncMock(return_value=uuid.uuid4()) + session.get = AsyncMock(return_value=_conn()) + with patch( + "app.api.connections.require_project_member", new_callable=AsyncMock + ), patch( + "app.api.connections.decrypt_text", + return_value="postgresql://u@db.example.com/app", + ), patch( + "app.api.connections.probe_database", + new_callable=AsyncMock, + side_effect=RuntimeError( + "database host is not in the introspection allowlist" + ), + ): + out = await run_connection_test( + db_connection_uuid=uuid.uuid4(), user=_user(), session=session + ) + assert out.ok is False + assert out.server_version is None + assert "allowlist" in (out.error or "") + + +@pytest.mark.asyncio +async def test_probe_database_routes_to_postgres(): + with patch( + "app.db_introspect.probe_postgres", + new_callable=AsyncMock, + return_value="16.0", + ): + assert await probe_database("postgresql://u@db.example.com/app") == "16.0" + + +@pytest.mark.asyncio +async def test_probe_database_wraps_and_redacts_errors(): + dsn = "postgresql://user:s3cret@db.example.com/app" + with patch( + "app.db_introspect.probe_postgres", + new_callable=AsyncMock, + side_effect=OSError(f"could not connect to {dsn}"), + ): + with pytest.raises(RuntimeError) as exc: + await probe_database(dsn) + # Credentials must never surface in the wrapped error. + assert "s3cret" not in str(exc.value) From d20f9e2a7a7313e8adb762ef13a60c1755dd53d9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 5 Jul 2026 21:43:05 +0900 Subject: [PATCH 08/10] feat(frontend): table annotations modal + connection test UI AnnotationsModal (list/add/edit/delete table notes) wired to a toolbar button; api client listAnnotations/upsertAnnotation/deleteAnnotation + demo store. Connection test button + status pill next to the connection picker; api testConnection + demo. types ConnectionTestResult/TableAnnotation. typecheck clean, frontend suite 23 files / 136 tests green. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AxU2xaupAjp912oDNFuWyd --- frontend/src/App.tsx | 123 +++++++++++++++- frontend/src/api.ts | 87 ++++++++++- .../modals/AnnotationsModal.test.tsx | 74 ++++++++++ .../components/modals/AnnotationsModal.tsx | 135 ++++++++++++++++++ frontend/src/components/modals/index.ts | 1 + frontend/src/types.ts | 15 ++ 6 files changed, 433 insertions(+), 2 deletions(-) create mode 100644 frontend/src/components/modals/AnnotationsModal.test.tsx create mode 100644 frontend/src/components/modals/AnnotationsModal.tsx diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 112e9834..6d8675ce 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -17,6 +17,7 @@ import { useEffect, useMemo, useRef, useState, useCallback } from "react"; import { AddTableModal, CardinalityModal, + AnnotationsModal, DiffModal, EditEdgeModal, EditTableModal, @@ -32,14 +33,18 @@ import { createProject, createSnapshot, createView, + deleteAnnotation, deleteView, diffSnapshots, getSnapshot, getView, + listAnnotations, listConnections, listProjects, listSnapshots, listViews, + testConnection, + upsertAnnotation, } from "./api"; import { applyLayout, captureLayout } from "./diagramViews"; import TableNode from "./erd/TableNode"; @@ -66,7 +71,7 @@ import { } from "./erd/export"; import { exportMermaid } from "./erd/mermaid"; import { GRID_COLUMNS, GRID_X_GAP, GRID_Y_GAP } from "./erd/layoutConstants"; -import type { Connection, DiagramView, Project, SchemaDiff, Snapshot, SnapshotDetail } from "./types"; +import type { Connection, ConnectionTestResult, DiagramView, Project, SchemaDiff, Snapshot, SnapshotDetail, TableAnnotation } from "./types"; const TERMINAL_SNAPSHOT_STATUSES = new Set([ "succeeded", @@ -171,6 +176,14 @@ export default function App() { const [isSavingView, setIsSavingView] = useState(false); const [viewsError, setViewsError] = useState(null); + const [isAnnotationsModalOpen, setIsAnnotationsModalOpen] = useState(false); + const [annotations, setAnnotations] = useState([]); + const [isSavingAnnotation, setIsSavingAnnotation] = useState(false); + const [annotationsError, setAnnotationsError] = useState(null); + + const [connTestResult, setConnTestResult] = useState(null); + const [isTestingConn, setIsTestingConn] = useState(false); + const [editingEdge, setEditingEdge] = useState(null); const [editingNode, setEditingNode] = useState | null>(null); const [isEditTableModalOpen, setIsEditTableModalOpen] = useState(false); @@ -620,6 +633,72 @@ export default function App() { .catch(() => setViewsError("뷰 삭제에 실패했습니다.")); } + function onOpenAnnotations() { + setAnnotationsError(null); + setIsAnnotationsModalOpen(true); + if (!selectedProjectId) return; + listAnnotations(selectedProjectId) + .then(setAnnotations) + .catch(() => setAnnotationsError("주석을 불러오지 못했습니다.")); + } + + function onCloseAnnotations() { + setIsAnnotationsModalOpen(false); + } + + function onSaveAnnotation( + schemaName: string, + relationName: string, + body: string, + ) { + if (!selectedProjectId) return; + setIsSavingAnnotation(true); + setAnnotationsError(null); + upsertAnnotation(selectedProjectId, schemaName, relationName, body) + .then((saved) => { + setAnnotations((prev) => [ + saved, + ...prev.filter( + (a) => + !( + a.schema_name === saved.schema_name && + a.relation_name === saved.relation_name + ), + ), + ]); + }) + .catch(() => setAnnotationsError("주석 저장에 실패했습니다.")) + .finally(() => setIsSavingAnnotation(false)); + } + + function onDeleteAnnotation(annotationId: string) { + if (!selectedProjectId) return; + setAnnotationsError(null); + deleteAnnotation(selectedProjectId, annotationId) + .then(() => { + setAnnotations((prev) => + prev.filter((a) => a.table_annotation_uuid !== annotationId), + ); + }) + .catch(() => setAnnotationsError("주석 삭제에 실패했습니다.")); + } + + function onTestConnection() { + if (!selectedConnId) return; + setIsTestingConn(true); + setConnTestResult(null); + testConnection(selectedConnId) + .then(setConnTestResult) + .catch(() => + setConnTestResult({ + ok: false, + server_version: null, + error: "연결 테스트에 실패했습니다.", + }), + ) + .finally(() => setIsTestingConn(false)); + } + function onSelectDiffBase(baseSnapshotId: string) { setDiffBaseId(baseSnapshotId); if (!snapshotId) return; @@ -1180,6 +1259,25 @@ export default function App() { ))} + + {connTestResult ? ( + + {connTestResult.ok + ? `연결 성공${connTestResult.server_version ? ` · ${connTestResult.server_version}` : ""}` + : `연결 실패${connTestResult.error ? ` · ${connTestResult.error}` : ""}`} + + ) : null}
@@ -1553,6 +1651,19 @@ export default function App() { > ⧉ + +
+ +
+
+ setSchemaName(e.target.value)} + placeholder="스키마" + /> + setRelationName(e.target.value)} + placeholder="테이블" + /> +
+