From 063248267b9fa8c47dae9b3a89052ce88f70dce1 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 21:05:23 +0300 Subject: [PATCH 01/30] feat(schedules): add guided cron builder --- .../schedules/CronScheduleBuilder.test.tsx | 34 ++++ .../schedules/CronScheduleBuilder.tsx | 176 ++++++++++++++++++ .../components/schedules/cronSchedule.test.ts | 56 ++++++ .../src/components/schedules/cronSchedule.ts | 120 ++++++++++++ frontend/src/pages/SchedulesPage.tsx | 63 ++++--- 5 files changed, 427 insertions(+), 22 deletions(-) create mode 100644 frontend/src/components/schedules/CronScheduleBuilder.test.tsx create mode 100644 frontend/src/components/schedules/CronScheduleBuilder.tsx create mode 100644 frontend/src/components/schedules/cronSchedule.test.ts create mode 100644 frontend/src/components/schedules/cronSchedule.ts diff --git a/frontend/src/components/schedules/CronScheduleBuilder.test.tsx b/frontend/src/components/schedules/CronScheduleBuilder.test.tsx new file mode 100644 index 0000000..0fb8446 --- /dev/null +++ b/frontend/src/components/schedules/CronScheduleBuilder.test.tsx @@ -0,0 +1,34 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +import { CronScheduleBuilder } from "./CronScheduleBuilder"; +import { INITIAL_CRON_BUILDER } from "./cronSchedule"; + +describe("CronScheduleBuilder", () => { + it("starts with a plain-language daily schedule", () => { + render(); + + expect(screen.getByLabelText("Frequency")).toHaveValue("daily"); + expect(screen.getByLabelText("Time")).toHaveValue("00:00"); + expect(screen.getByText("Every day at 00:00")).toBeInTheDocument(); + expect(screen.queryByLabelText("Cron expression")).not.toBeInTheDocument(); + }); + + it("offers raw cron only when Custom cron is selected", () => { + const onChange = vi.fn(); + const { rerender } = render( + , + ); + + fireEvent.change(screen.getByLabelText("Frequency"), { target: { value: "custom" } }); + expect(onChange).toHaveBeenCalledWith({ ...INITIAL_CRON_BUILDER, frequency: "custom" }); + + rerender( + , + ); + expect(screen.getByLabelText("Cron expression")).toHaveValue("0 */6 * * *"); + }); +}); diff --git a/frontend/src/components/schedules/CronScheduleBuilder.tsx b/frontend/src/components/schedules/CronScheduleBuilder.tsx new file mode 100644 index 0000000..ad0787a --- /dev/null +++ b/frontend/src/components/schedules/CronScheduleBuilder.tsx @@ -0,0 +1,176 @@ +import { Clock3 } from "lucide-react"; + +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { Select } from "@/components/ui/select"; +import { + buildCronExpression, + describeCronBuilder, + isValidCronExpression, + WEEKDAYS, + type CronBuilderValue, + type CronFrequency, + type CronWeekday, +} from "./cronSchedule"; + +interface CronScheduleBuilderProps { + value: CronBuilderValue; + onChange: (value: CronBuilderValue) => void; +} + +export function CronScheduleBuilder({ value, onChange }: CronScheduleBuilderProps) { + const update = (patch: Partial) => onChange({ ...value, ...patch }); + const expression = buildCronExpression(value); + const customIsInvalid = + value.frequency === "custom" && !isValidCronExpression(value.customExpression); + + return ( +
+
+ + +
+ + {value.frequency === "hourly" && ( +
+ + update({ minute: event.target.value })} + /> +

+ For example, 15 runs at 09:15, 10:15, and so on. +

+
+ )} + + {value.frequency === "daily" && ( +
+ + update({ time: event.target.value })} + /> +
+ )} + + {value.frequency === "weekly" && ( +
+
+ + +
+
+ + update({ time: event.target.value })} + /> +
+
+ )} + + {value.frequency === "monthly" && ( +
+
+ + update({ monthDay: event.target.value })} + /> +
+
+ + update({ time: event.target.value })} + /> +
+

+ Months without the selected date are skipped. +

+
+ )} + + {value.frequency === "custom" && ( +
+ + update({ customExpression: event.target.value })} + placeholder="0 */6 * * *" + aria-invalid={customIsInvalid} + /> +

+ {customIsInvalid + ? "Enter exactly five fields: minute hour day month weekday." + : "Advanced: minute hour day month weekday."} +

+
+ )} + +
+
+
+ ); +} diff --git a/frontend/src/components/schedules/cronSchedule.test.ts b/frontend/src/components/schedules/cronSchedule.test.ts new file mode 100644 index 0000000..03a8408 --- /dev/null +++ b/frontend/src/components/schedules/cronSchedule.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from "vitest"; + +import { + buildCronExpression, + describeCronExpression, + INITIAL_CRON_BUILDER, + isValidCronExpression, +} from "./cronSchedule"; + +describe("cron schedule builder", () => { + it("builds hourly, daily, weekly, and monthly expressions", () => { + expect( + buildCronExpression({ ...INITIAL_CRON_BUILDER, frequency: "hourly", minute: "15" }), + ).toBe("15 * * * *"); + expect( + buildCronExpression({ ...INITIAL_CRON_BUILDER, frequency: "daily", time: "08:30" }), + ).toBe("30 8 * * *"); + expect( + buildCronExpression({ + ...INITIAL_CRON_BUILDER, + frequency: "weekly", + weekday: "sun", + time: "21:05", + }), + ).toBe("5 21 * * sun"); + expect( + buildCronExpression({ + ...INITIAL_CRON_BUILDER, + frequency: "monthly", + monthDay: "12", + time: "06:00", + }), + ).toBe("0 6 12 * *"); + }); + + it("keeps custom cron available as an advanced option", () => { + expect( + buildCronExpression({ + ...INITIAL_CRON_BUILDER, + frequency: "custom", + customExpression: " 0 */6 * * * ", + }), + ).toBe("0 */6 * * *"); + expect(isValidCronExpression("0 */6 * * *")).toBe(true); + expect(isValidCronExpression("0 */6 * *")).toBe(false); + }); + + it("describes generated expressions in plain language", () => { + expect(describeCronExpression("15 * * * *")).toBe("Every hour at minute 15"); + expect(describeCronExpression("30 8 * * *")).toBe("Every day at 08:30"); + expect(describeCronExpression("5 21 * * sun")).toBe("Every Sunday at 21:05"); + expect(describeCronExpression("0 6 12 * *")).toBe("Every month on day 12 at 06:00"); + expect(describeCronExpression("*/10 * * * *")).toBe("Every 10 minutes"); + expect(describeCronExpression("0 */6 * * *")).toBe("Every 6 hours"); + }); +}); diff --git a/frontend/src/components/schedules/cronSchedule.ts b/frontend/src/components/schedules/cronSchedule.ts new file mode 100644 index 0000000..939e419 --- /dev/null +++ b/frontend/src/components/schedules/cronSchedule.ts @@ -0,0 +1,120 @@ +export type CronFrequency = "hourly" | "daily" | "weekly" | "monthly" | "custom"; + +export type CronWeekday = "mon" | "tue" | "wed" | "thu" | "fri" | "sat" | "sun"; + +export interface CronBuilderValue { + frequency: CronFrequency; + minute: string; + time: string; + weekday: CronWeekday; + monthDay: string; + customExpression: string; +} + +export const INITIAL_CRON_BUILDER: CronBuilderValue = { + frequency: "daily", + minute: "0", + time: "00:00", + weekday: "mon", + monthDay: "1", + customExpression: "0 */6 * * *", +}; + +export const WEEKDAYS: Array<{ value: CronWeekday; label: string }> = [ + { value: "mon", label: "Monday" }, + { value: "tue", label: "Tuesday" }, + { value: "wed", label: "Wednesday" }, + { value: "thu", label: "Thursday" }, + { value: "fri", label: "Friday" }, + { value: "sat", label: "Saturday" }, + { value: "sun", label: "Sunday" }, +]; + +function normalizeInteger(value: string, min: number, max: number): string { + const parsed = Number.parseInt(value, 10); + if (Number.isNaN(parsed)) return String(min); + return String(Math.min(max, Math.max(min, parsed))); +} + +function parseTime(value: string): [string, string] { + const match = /^(\d{2}):(\d{2})$/.exec(value); + if (!match) return ["0", "0"]; + return [normalizeInteger(match[2], 0, 59), normalizeInteger(match[1], 0, 23)]; +} + +export function buildCronExpression(value: CronBuilderValue): string { + const [minute, hour] = parseTime(value.time); + + switch (value.frequency) { + case "hourly": + return `${normalizeInteger(value.minute, 0, 59)} * * * *`; + case "daily": + return `${minute} ${hour} * * *`; + case "weekly": + return `${minute} ${hour} * * ${value.weekday}`; + case "monthly": + return `${minute} ${hour} ${normalizeInteger(value.monthDay, 1, 31)} * *`; + case "custom": + return value.customExpression.trim(); + } +} + +export function isValidCronExpression(expression: string): boolean { + return expression.trim().split(/\s+/).length === 5; +} + +function formatTime(hour: string, minute: string): string { + return `${hour.padStart(2, "0")}:${minute.padStart(2, "0")}`; +} + +export function describeCronExpression(expression: string | null | undefined): string { + if (!expression) return "Calendar schedule"; + + const [minute, hour, day, month, weekday] = expression.trim().split(/\s+/); + if (!minute || !hour || !day || !month || !weekday) return "Custom calendar schedule"; + + const minuteInterval = /^\*\/(\d+)$/.exec(minute); + if (minuteInterval && hour === "*" && day === "*" && month === "*" && weekday === "*") { + return `Every ${minuteInterval[1]} minutes`; + } + + const hourInterval = /^\*\/(\d+)$/.exec(hour); + if (hourInterval && /^\d+$/.test(minute) && day === "*" && month === "*" && weekday === "*") { + return minute === "0" + ? `Every ${hourInterval[1]} hours` + : `Every ${hourInterval[1]} hours at minute ${minute}`; + } + + if (/^\d+$/.test(minute) && hour === "*" && day === "*" && month === "*" && weekday === "*") { + return `Every hour at minute ${minute}`; + } + + if (/^\d+$/.test(minute) && /^\d+$/.test(hour) && day === "*" && month === "*") { + const time = formatTime(hour, minute); + if (weekday === "*") return `Every day at ${time}`; + + const weekdayLabel = WEEKDAYS.find((item) => item.value === weekday)?.label; + if (weekdayLabel) return `Every ${weekdayLabel} at ${time}`; + } + + if ( + /^\d+$/.test(minute) && + /^\d+$/.test(hour) && + /^\d+$/.test(day) && + month === "*" && + weekday === "*" + ) { + return `Every month on day ${day} at ${formatTime(hour, minute)}`; + } + + return "Custom calendar schedule"; +} + +export function describeCronBuilder(value: CronBuilderValue): string { + if (value.frequency === "custom") { + return isValidCronExpression(value.customExpression) + ? describeCronExpression(value.customExpression) + : "Enter a complete five-field cron expression."; + } + return describeCronExpression(buildCronExpression(value)); +} diff --git a/frontend/src/pages/SchedulesPage.tsx b/frontend/src/pages/SchedulesPage.tsx index d6d9efc..ff8711f 100644 --- a/frontend/src/pages/SchedulesPage.tsx +++ b/frontend/src/pages/SchedulesPage.tsx @@ -4,6 +4,14 @@ import { Pause, Play, Plus, RefreshCw, Trash2 } from "lucide-react"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; +import { CronScheduleBuilder } from "@/components/schedules/CronScheduleBuilder"; +import { + buildCronExpression, + describeCronExpression, + INITIAL_CRON_BUILDER, + isValidCronExpression, + type CronBuilderValue, +} from "@/components/schedules/cronSchedule"; import { Dialog, DialogContent, @@ -43,6 +51,7 @@ export function SchedulesPage() { schedule_type: "interval", interval_seconds: 300, }); + const [cronBuilder, setCronBuilder] = useState(INITIAL_CRON_BUILDER); const [createError, setCreateError] = useState(""); const { @@ -74,6 +83,7 @@ export function SchedulesPage() { setShowCreate(false); setCreateError(""); setCreateForm({ endpoint_id: "", schedule_type: "interval", interval_seconds: 300 }); + setCronBuilder(INITIAL_CRON_BUILDER); }, onError: (err) => setCreateError(getApiError(err)), }); @@ -179,9 +189,16 @@ export function SchedulesPage() { - {s.schedule_type === "cron" - ? s.cron_expression - : `Every ${s.interval_seconds}s`} + {s.schedule_type === "cron" ? ( +
+

{describeCronExpression(s.cron_expression)}

+ + {s.cron_expression} + +
+ ) : ( + `Every ${s.interval_seconds}s` + )} @@ -273,8 +290,9 @@ export function SchedulesPage() {
- +
{createForm.schedule_type === "interval" && ( @@ -310,20 +328,16 @@ export function SchedulesPage() { )} {createForm.schedule_type === "cron" && ( -
- - - setCreateForm((f) => ({ ...f, cron_expression: e.target.value })) - } - placeholder="0 */6 * * *" - /> -

- Standard 5-field cron: minute hour day month weekday -

-
+ { + setCronBuilder(value); + setCreateForm((form) => ({ + ...form, + cron_expression: buildCronExpression(value), + })); + }} + /> )} @@ -332,7 +346,12 @@ export function SchedulesPage() { From 053c685f0d8d166197aab93293c4d8776336ee9b Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 21:14:03 +0300 Subject: [PATCH 02/30] fix(schedules): preserve job history on deletion --- ...eserve_job_runs_when_deleting_schedules.py | 60 ++++++++++++++ backend/app/models/job_run.py | 4 +- backend/app/routers/schedules.py | 2 + backend/app/schemas/schedule.py | 2 +- backend/app/services/schedule.py | 3 - backend/tests/test_migration.py | 8 ++ backend/tests/test_schedules.py | 79 +++++++++++++++++++ frontend/src/pages/SchedulesPage.tsx | 27 ++++++- frontend/src/types/schedule.ts | 2 +- 9 files changed, 178 insertions(+), 9 deletions(-) create mode 100644 backend/alembic/versions/b2d18f4a6c73_preserve_job_runs_when_deleting_schedules.py diff --git a/backend/alembic/versions/b2d18f4a6c73_preserve_job_runs_when_deleting_schedules.py b/backend/alembic/versions/b2d18f4a6c73_preserve_job_runs_when_deleting_schedules.py new file mode 100644 index 0000000..a6f0f4f --- /dev/null +++ b/backend/alembic/versions/b2d18f4a6c73_preserve_job_runs_when_deleting_schedules.py @@ -0,0 +1,60 @@ +"""Preserve job runs when deleting schedules. + +Revision ID: b2d18f4a6c73 +Revises: a8307fb20816 +Create Date: 2026-08-30 +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +revision: str = "b2d18f4a6c73" +down_revision: str | None = "a8307fb20816" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + op.drop_constraint( + "job_runs_schedule_id_fkey", "job_runs", type_="foreignkey" + ) + op.alter_column( + "job_runs", + "schedule_id", + existing_type=sa.UUID(), + nullable=True, + ) + op.create_foreign_key( + "job_runs_schedule_id_fkey", + "job_runs", + "schedules", + ["schedule_id"], + ["id"], + ondelete="SET NULL", + ) + + +def downgrade() -> None: + op.drop_constraint( + "job_runs_schedule_id_fkey", "job_runs", type_="foreignkey" + ) + # Schedules deleted after this migration cannot be reconstructed. Remove + # only their orphaned audit rows so the original NOT NULL contract can be + # restored; snapshots remain and their job_run_id becomes NULL. + op.execute("DELETE FROM job_runs WHERE schedule_id IS NULL") + op.alter_column( + "job_runs", + "schedule_id", + existing_type=sa.UUID(), + nullable=False, + ) + op.create_foreign_key( + "job_runs_schedule_id_fkey", + "job_runs", + "schedules", + ["schedule_id"], + ["id"], + ondelete="RESTRICT", + ) diff --git a/backend/app/models/job_run.py b/backend/app/models/job_run.py index 63c0ebe..5e97c78 100644 --- a/backend/app/models/job_run.py +++ b/backend/app/models/job_run.py @@ -30,8 +30,8 @@ class JobRun(UUIDPrimaryKeyMixin, Base): id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4) - schedule_id: Mapped[uuid.UUID] = mapped_column( - ForeignKey("schedules.id", ondelete="RESTRICT"), nullable=False + schedule_id: Mapped[uuid.UUID | None] = mapped_column( + ForeignKey("schedules.id", ondelete="SET NULL"), nullable=True ) endpoint_id: Mapped[uuid.UUID] = mapped_column( ForeignKey("endpoints.id", ondelete="RESTRICT"), nullable=False diff --git a/backend/app/routers/schedules.py b/backend/app/routers/schedules.py index 07f2bc7..d8dc745 100644 --- a/backend/app/routers/schedules.py +++ b/backend/app/routers/schedules.py @@ -38,6 +38,7 @@ SnapshotResponse, ) from app.services.schedule import ScheduleService +from app.services.scheduler import remove_schedule_job log = structlog.get_logger() @@ -151,6 +152,7 @@ async def delete_schedule( status_code=status.HTTP_404_NOT_FOUND, detail="Schedule not found." ) await db.commit() + remove_schedule_job(schedule_id) # ── Control actions ────────────────────────────────────────────────────────── diff --git a/backend/app/schemas/schedule.py b/backend/app/schemas/schedule.py index 3a110ac..384cee9 100644 --- a/backend/app/schemas/schedule.py +++ b/backend/app/schemas/schedule.py @@ -69,7 +69,7 @@ class ScheduleResponse(BaseModel): class JobRunResponse(BaseModel): id: uuid.UUID - schedule_id: uuid.UUID + schedule_id: uuid.UUID | None endpoint_id: uuid.UUID started_at: datetime finished_at: datetime | None diff --git a/backend/app/services/schedule.py b/backend/app/services/schedule.py index b7b835e..2655acb 100644 --- a/backend/app/services/schedule.py +++ b/backend/app/services/schedule.py @@ -172,9 +172,6 @@ async def delete_schedule( if obj is None: return False - # Remove from APScheduler - remove_schedule_job(obj.id) - await self._repo.delete(obj) log.info( diff --git a/backend/tests/test_migration.py b/backend/tests/test_migration.py index 53e72af..014cf50 100644 --- a/backend/tests/test_migration.py +++ b/backend/tests/test_migration.py @@ -132,6 +132,14 @@ def test_allow_unauthenticated_migration(self) -> None: assert "drop_column" in src, f"{f.name} must drop the column (downgrade)" assert "endpoints" in src, f"{f.name} must target the endpoints table" + def test_schedule_delete_preserves_job_runs_migration(self) -> None: + migration = MIGRATION_DIR / "b2d18f4a6c73_preserve_job_runs_when_deleting_schedules.py" + source = migration.read_text() + assert migration.is_file() + assert 'ondelete="SET NULL"' in source + assert '"schedule_id"' in source + assert "nullable=True" in source + def test_migration_chain_is_linear(self) -> None: """The revision graph must be a single linear chain: exactly one base (down_revision=None), exactly one head, and every down_revision known.""" diff --git a/backend/tests/test_schedules.py b/backend/tests/test_schedules.py index 8c60ef9..5348f44 100644 --- a/backend/tests/test_schedules.py +++ b/backend/tests/test_schedules.py @@ -7,6 +7,7 @@ """ import uuid +from datetime import UTC, datetime import pytest from app.schemas.schedule import ( @@ -115,6 +116,22 @@ def test_job_run_response_fields() -> None: assert "error_detail" in fields +def test_job_run_response_allows_deleted_schedule_history() -> None: + now = datetime.now(UTC) + response = JobRunResponse( + id=uuid.uuid4(), + schedule_id=None, + endpoint_id=uuid.uuid4(), + started_at=now, + finished_at=now, + status="success", + row_count=1, + error_detail=None, + created_at=now, + ) + assert response.schedule_id is None + + def test_snapshot_response_fields() -> None: fields = SnapshotResponse.model_fields assert "id" in fields @@ -332,6 +349,68 @@ async def test_delete_schedule(async_client: object) -> None: assert r_get.status_code == 404 +@pytest.mark.integration +async def test_delete_schedule_preserves_job_run_history( + async_client: object, db_session: object +) -> None: + from app.models.job_run import JobRun, JobRunStatus + from httpx import AsyncClient + from sqlalchemy import select + from sqlalchemy.ext.asyncio import AsyncSession + + client: AsyncClient = async_client # type: ignore[assignment] + db: AsyncSession = db_session # type: ignore[assignment] + + connection = await client.post( + "/api/v1/admin/connections/", + json={ + "name": f"test-conn-history-{uuid.uuid4().hex[:8]}", + "host": "oracle.example.com", + "service_name": "SVC", + "username": "scott", + "password": "tiger", + }, + ) + endpoint = await client.post( + "/api/v1/admin/endpoints/", + json={ + "name": f"history-ep-{uuid.uuid4().hex[:8]}", + "path": f"history-path-{uuid.uuid4().hex[:8]}", + "connection_id": connection.json()["id"], + "allow_unauthenticated": True, + "sql_text": "SELECT 1 FROM dual", + "data_strategy": "snapshot", + }, + ) + schedule = await client.post( + "/api/v1/admin/schedules/", + json={ + "endpoint_id": endpoint.json()["id"], + "schedule_type": "interval", + "interval_seconds": 300, + }, + ) + schedule_id = uuid.UUID(schedule.json()["id"]) + job_run = JobRun( + schedule_id=schedule_id, + endpoint_id=uuid.UUID(endpoint.json()["id"]), + started_at=datetime.now(UTC), + finished_at=datetime.now(UTC), + status=JobRunStatus.success, + row_count=1, + ) + db.add(job_run) + await db.commit() + + deleted = await client.delete(f"/api/v1/admin/schedules/{schedule_id}") + assert deleted.status_code == 204 + + preserved = await db.scalar(select(JobRun).where(JobRun.id == job_run.id)) + assert preserved is not None + await db.refresh(preserved) + assert preserved.schedule_id is None + + @pytest.mark.integration async def test_list_job_runs(async_client: object) -> None: from httpx import AsyncClient diff --git a/frontend/src/pages/SchedulesPage.tsx b/frontend/src/pages/SchedulesPage.tsx index ff8711f..3bfe0a1 100644 --- a/frontend/src/pages/SchedulesPage.tsx +++ b/frontend/src/pages/SchedulesPage.tsx @@ -53,6 +53,7 @@ export function SchedulesPage() { }); const [cronBuilder, setCronBuilder] = useState(INITIAL_CRON_BUILDER); const [createError, setCreateError] = useState(""); + const [deleteError, setDeleteError] = useState(""); const { data: schedules = [], @@ -93,7 +94,9 @@ export function SchedulesPage() { onSuccess: () => { qc.invalidateQueries({ queryKey: queryKeys.schedules.all }); setDeleteSchedule(null); + setDeleteError(""); }, + onError: (err) => setDeleteError(getApiError(err)), }); const runNowMutation = useMutation({ @@ -246,7 +249,14 @@ export function SchedulesPage() { > - @@ -360,7 +370,15 @@ export function SchedulesPage() { {/* Delete Dialog */} - setDeleteSchedule(null)}> + { + if (!open) { + setDeleteSchedule(null); + setDeleteError(""); + } + }} + > Delete Schedule @@ -372,6 +390,11 @@ export function SchedulesPage() { . Existing snapshots will be preserved. + {deleteError && ( +
+ {deleteError} +
+ )} - @@ -281,30 +293,27 @@ export function EndpointsPage() { - {/* Delete Dialog */} - setDeleteEndpoint(null)}> - - - Delete Endpoint - - This will permanently delete {deleteEndpoint?.name}. The URL{" "} - /api/v1/data/{deleteEndpoint?.path} will stop serving data. - - - - - - - - + { + if (!open) { + setDeleteEndpoint(null); + setDeleteError(""); + } + }} + title="Delete Endpoint" + description={ + <> + This will permanently delete {deleteEndpoint?.name}. The URL{" "} + /api/v1/data/{deleteEndpoint?.path} will stop serving data. Its schedule + and cached snapshots will also be removed; historical job-run audit records are + retained. + + } + isDeleting={deleteMutation.isPending} + onConfirm={() => deleteEndpoint && deleteMutation.mutate(deleteEndpoint.id)} + error={deleteError} + /> ); } diff --git a/frontend/src/types/schedule.ts b/frontend/src/types/schedule.ts index cf21789..e072c72 100644 --- a/frontend/src/types/schedule.ts +++ b/frontend/src/types/schedule.ts @@ -31,7 +31,7 @@ export interface ScheduleUpdate { export interface JobRun { id: string; schedule_id: string | null; - endpoint_id: string; + endpoint_id: string | null; started_at: string; finished_at: string | null; status: "running" | "success" | "failed" | "timeout"; From 6ba4a0ea92cecee0881166fbf1ed64b1bc4ea965 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 21:18:22 +0300 Subject: [PATCH 04/30] test: harden destructive database guard --- backend/tests/conftest.py | 22 ++++++++++--------- backend/tests/test_test_database_guard.py | 26 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 10 deletions(-) create mode 100644 backend/tests/test_test_database_guard.py diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 186c4be..5034c87 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -61,6 +61,15 @@ ADMIN_TEST_PASSWORD = "admin-password-do-not-use-in-prod" +def _assert_safe_test_database(database_url: str, app_env: str) -> None: + """Require both an explicit test environment and a test-named database.""" + if app_env != "test" or "test" not in database_url.lower(): + raise RuntimeError( + "Refusing to run destructive test schema setup: APP_ENV must be " + "'test' and DATABASE_URL must contain 'test'." + ) + + def _mint_admin_token() -> str: """Mint a valid admin JWT using the test-time settings. @@ -98,16 +107,9 @@ async def engine() -> AsyncGenerator[AsyncEngine]: so the database is left empty between tests. """ database_url = settings.database_url - # Hard safety guard: never run destructive schema ops against a DB that - # isn't clearly marked as a test database. Without this, a developer - # invoking pytest with the default DATABASE_URL would have their app - # database wiped on every test run. - if settings.app_env != "test" and "test" not in database_url.lower(): - raise RuntimeError( - "Refusing to run drop_all/create_all: APP_ENV is not 'test' and " - "DATABASE_URL does not contain 'test'. Set APP_ENV=test or point " - "DATABASE_URL at a test database." - ) + # Hard safety guard: both signals are mandatory. Requiring only one means + # APP_ENV=test can accidentally target and wipe the development database. + _assert_safe_test_database(database_url, settings.app_env) test_engine = create_async_engine(database_url, echo=False) async with test_engine.begin() as conn: diff --git a/backend/tests/test_test_database_guard.py b/backend/tests/test_test_database_guard.py new file mode 100644 index 0000000..8d0d99b --- /dev/null +++ b/backend/tests/test_test_database_guard.py @@ -0,0 +1,26 @@ +"""Regression coverage for the destructive test-database safety guard.""" + +import pytest + +from tests.conftest import _assert_safe_test_database + + +def test_database_guard_accepts_explicit_test_database() -> None: + _assert_safe_test_database( + "postgresql+asyncpg://user:pass@db:5432/db2api_test", "test" + ) + + +@pytest.mark.parametrize( + ("database_url", "app_env"), + [ + ("postgresql+asyncpg://user:pass@db:5432/db2api", "test"), + ("postgresql+asyncpg://user:pass@db:5432/db2api_test", "development"), + ("postgresql+asyncpg://user:pass@db:5432/db2api", "development"), + ], +) +def test_database_guard_rejects_any_non_test_signal( + database_url: str, app_env: str +) -> None: + with pytest.raises(RuntimeError, match="APP_ENV must be 'test'"): + _assert_safe_test_database(database_url, app_env) From 407a9235571a9815ab6a6b14007d86797f5c6de1 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 21:20:05 +0300 Subject: [PATCH 05/30] fix(scheduler): restore active jobs on startup --- backend/app/main.py | 2 +- backend/app/services/scheduler.py | 32 ++++++++++++-- backend/tests/test_scheduler_restore.py | 58 +++++++++++++++++++++++++ docs/deployment.md | 8 ++-- 4 files changed, 93 insertions(+), 7 deletions(-) create mode 100644 backend/tests/test_scheduler_restore.py diff --git a/backend/app/main.py b/backend/app/main.py index a30e5c7..5ac12cf 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -101,7 +101,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None]: debug=settings.debug, ) _init_oracle_client() - start_scheduler() + await start_scheduler() yield stop_scheduler() log.info("application_shutdown") diff --git a/backend/app/services/scheduler.py b/backend/app/services/scheduler.py index 595b350..abc3384 100644 --- a/backend/app/services/scheduler.py +++ b/backend/app/services/scheduler.py @@ -6,8 +6,8 @@ - Persist job runs and snapshots. - Update schedule metadata (last_run_at, next_run_at). -The scheduler currently relies on APScheduler's in-memory job store; -scheduled jobs themselves do not persist across process restarts. +The scheduler uses APScheduler's in-memory job store. Active schedules are +rehydrated from the application database whenever the process starts. """ import uuid @@ -207,7 +207,32 @@ async def execute_scheduled_job(schedule_id: str, endpoint_id: str) -> None: ) -def start_scheduler() -> None: +async def restore_active_schedules() -> int: + """Register all active database schedules in the in-memory scheduler.""" + restored = 0 + async with AsyncSessionLocal() as db: + schedules = await ScheduleRepository(db).get_all(active_only=True) + for schedule in schedules: + try: + add_schedule_job( + schedule_id=schedule.id, + endpoint_id=schedule.endpoint_id, + schedule_type=schedule.schedule_type, + cron_expression=schedule.cron_expression, + interval_seconds=schedule.interval_seconds, + ) + restored += 1 + except Exception as exc: # noqa: BLE001 + log.error( + "scheduler_job_restore_failed", + schedule_id=str(schedule.id), + error=str(exc), + ) + log.info("scheduler_jobs_restored", restored_count=restored) + return restored + + +async def start_scheduler() -> None: """Initialize and start the APScheduler instance.""" global _scheduler # noqa: PLW0603 @@ -224,6 +249,7 @@ def start_scheduler() -> None: scheduler.start() _scheduler = scheduler log.info("scheduler_started") + await restore_active_schedules() except Exception as exc: # noqa: BLE001 log.error("scheduler_start_failed", error=str(exc)) diff --git a/backend/tests/test_scheduler_restore.py b/backend/tests/test_scheduler_restore.py new file mode 100644 index 0000000..3102ef9 --- /dev/null +++ b/backend/tests/test_scheduler_restore.py @@ -0,0 +1,58 @@ +"""Unit coverage for restoring in-memory scheduler jobs after restart.""" + +import uuid +from types import SimpleNamespace +from typing import cast +from unittest.mock import MagicMock + +import pytest + + +@pytest.mark.asyncio +async def test_restore_active_schedules_registers_each_row( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from app.services import scheduler as scheduler_service + + schedules = [ + SimpleNamespace( + id=uuid.uuid4(), + endpoint_id=uuid.uuid4(), + schedule_type="cron", + cron_expression="0 6 * * *", + interval_seconds=None, + ), + SimpleNamespace( + id=uuid.uuid4(), + endpoint_id=uuid.uuid4(), + schedule_type="interval", + cron_expression=None, + interval_seconds=300, + ), + ] + + class FakeSessionContext: + async def __aenter__(self) -> object: + return object() + + async def __aexit__(self, *args: object) -> None: + return None + + class FakeScheduleRepository: + def __init__(self, db: object) -> None: + self.db = db + + async def get_all(self, *, active_only: bool = False) -> list[object]: + assert active_only is True + return cast(list[object], schedules) + + add_job = MagicMock() + monkeypatch.setattr(scheduler_service, "AsyncSessionLocal", FakeSessionContext) + monkeypatch.setattr(scheduler_service, "ScheduleRepository", FakeScheduleRepository) + monkeypatch.setattr(scheduler_service, "add_schedule_job", add_job) + + restored = await scheduler_service.restore_active_schedules() + + assert restored == 2 + assert add_job.call_count == 2 + assert add_job.call_args_list[0].kwargs["schedule_id"] == schedules[0].id diff --git a/docs/deployment.md b/docs/deployment.md index fab2ffb..fe7a6d2 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -86,7 +86,7 @@ pip install -r requirements.txt alembic upgrade head # Start the server -uvicorn app.main:app --host 0.0.0.0 --port 8000 --workers 4 +uvicorn app.main:app --host 0.0.0.0 --port 8000 --workers 1 ``` For production, use a process manager: @@ -96,12 +96,14 @@ For production, use a process manager: pip install gunicorn gunicorn app.main:app \ --worker-class uvicorn.workers.UvicornWorker \ - --workers 4 \ + --workers 1 \ --bind 0.0.0.0:8000 \ --access-logfile - \ --error-logfile - ``` +Keep exactly one API worker while APScheduler uses its in-process job store. Each worker restores active schedules during startup, so multiple workers or replicas would execute duplicate snapshot refreshes until distributed scheduler coordination is implemented. + #### Frontend Setup ```bash @@ -156,7 +158,7 @@ psql -c "CREATE DATABASE db2api_db OWNER db2api_user;" Use the Docker images as a starting point. Key considerations: -- Deploy the API as a `Deployment` with `replicas: 2+` +- Deploy the API as a `Deployment` with `replicas: 1` while the scheduler is in-process - Use a `Service` for internal routing and an `Ingress` for external access - PostgreSQL: use a managed service (e.g., AWS RDS, GCP Cloud SQL) or a StatefulSet - Store `ENCRYPTION_KEY` and `DATABASE_URL` as Kubernetes Secrets From 7b4eaf044130ab5ac63653cb675dfeaab0cf7848 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 21:29:52 +0300 Subject: [PATCH 06/30] feat(snapshots): add validated parameter defaults --- backend/app/routers/endpoints.py | 17 +-- backend/app/routers/schedules.py | 5 + backend/app/schemas/endpoint.py | 54 +++++++- backend/app/services/endpoint.py | 12 ++ backend/app/services/schedule.py | 2 + backend/app/services/scheduler.py | 12 +- backend/app/sql/param_models.py | 34 ++++- backend/tests/test_endpoints.py | 126 ++++++++++++++++++ backend/tests/test_param_models.py | 32 +++++ .../components/endpoints/EndpointWizard.tsx | 35 +++-- .../endpoints/wizard/ConfigStep.test.tsx | 40 ++++++ .../endpoints/wizard/ConfigStep.tsx | 13 ++ .../endpoints/wizard/ParamsStep.test.tsx | 62 +++++++-- .../endpoints/wizard/ParamsStep.tsx | 68 +++++++--- .../endpoints/wizard/ReviewStep.tsx | 17 ++- .../wizard/parameterDefaults.test.ts | 41 ++++++ .../endpoints/wizard/parameterDefaults.ts | 25 ++++ frontend/src/types/endpoint.ts | 2 + 18 files changed, 529 insertions(+), 68 deletions(-) create mode 100644 frontend/src/components/endpoints/wizard/parameterDefaults.test.ts create mode 100644 frontend/src/components/endpoints/wizard/parameterDefaults.ts diff --git a/backend/app/routers/endpoints.py b/backend/app/routers/endpoints.py index 77c9f1e..71beac0 100644 --- a/backend/app/routers/endpoints.py +++ b/backend/app/routers/endpoints.py @@ -27,6 +27,7 @@ EndpointResponse, EndpointUpdate, PublicEndpointError, + SnapshotConfigurationError, SqlPreviewRequest, SqlPreviewResponse, ) @@ -74,14 +75,12 @@ async def create_endpoint( ) -> EndpointResponse: try: result = await svc.create_endpoint(payload) - except PublicEndpointError as exc: + except (PublicEndpointError, SnapshotConfigurationError) as exc: raise HTTPException( status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(exc) ) from exc except ValueError as exc: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail=str(exc) - ) from exc + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc await db.commit() return result @@ -116,18 +115,14 @@ async def update_endpoint( ) -> EndpointResponse: try: result = await svc.update_endpoint(endpoint_id, payload) - except PublicEndpointError as exc: + except (PublicEndpointError, SnapshotConfigurationError) as exc: raise HTTPException( status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(exc) ) from exc except ValueError as exc: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail=str(exc) - ) from exc + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc if result is None: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail="Endpoint not found." - ) + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Endpoint not found.") await db.commit() return result diff --git a/backend/app/routers/schedules.py b/backend/app/routers/schedules.py index d8dc745..90ce0dd 100644 --- a/backend/app/routers/schedules.py +++ b/backend/app/routers/schedules.py @@ -29,6 +29,7 @@ from app.repositories.job_run import JobRunRepository from app.repositories.schedule import ScheduleRepository from app.repositories.snapshot import SnapshotRepository +from app.schemas.endpoint import SnapshotConfigurationError from app.schemas.schedule import ( JobRunResponse, ScheduleCreate, @@ -86,6 +87,10 @@ async def create_schedule( ) -> ScheduleResponse: try: result = await svc.create_schedule(payload) + except SnapshotConfigurationError as exc: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(exc) + ) from exc except ValueError as exc: raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail=str(exc) diff --git a/backend/app/schemas/endpoint.py b/backend/app/schemas/endpoint.py index c4e5b96..42329db 100644 --- a/backend/app/schemas/endpoint.py +++ b/backend/app/schemas/endpoint.py @@ -10,7 +10,7 @@ import re import uuid from datetime import datetime -from typing import Self +from typing import Literal, Self from pydantic import BaseModel, Field, field_validator, model_validator @@ -47,6 +47,10 @@ class PublicEndpointError(ValueError): explicit ``allow_unauthenticated`` opt-in. Routers surface this as 422.""" +class SnapshotConfigurationError(ValueError): + """Raised when a snapshot endpoint cannot execute without request inputs.""" + + def extract_bind_params(sql: str) -> list[str]: """Return deduplicated bind parameter names from SQL text.""" # Exclude matches inside single-quoted string literals. @@ -75,6 +79,13 @@ class ParamDescriptor(BaseModel): ) required: bool = True default: str | int | float | bool | None = None + default_expression: Literal["today", "yesterday"] | None = Field( + None, + description=( + "Dynamic date default evaluated from the application server date " + "when the query executes." + ), + ) description: str | None = None max_length: int | None = Field( None, @@ -84,11 +95,49 @@ class ParamDescriptor(BaseModel): @model_validator(mode="after") def optional_must_have_default(self) -> Self: - if not self.required and self.default is None: + if self.default is not None and self.default_expression is not None: + raise ValueError("Declare either default or default_expression, not both.") + if self.default_expression is not None and self.type != "date": + raise ValueError("default_expression is supported only for date parameters.") + if not self.required and self.default is None and self.default_expression is None: raise ValueError("Optional parameters must declare a default value.") return self +def missing_snapshot_defaults( + param_schema: dict[str, ParamDescriptor] | dict[str, object], +) -> list[str]: + """Return snapshot bind names that cannot be resolved without a request.""" + missing: list[str] = [] + for name, raw_descriptor in param_schema.items(): + if isinstance(raw_descriptor, ParamDescriptor): + descriptor = raw_descriptor + elif isinstance(raw_descriptor, dict): + descriptor = ParamDescriptor.model_validate(raw_descriptor) + else: + missing.append(name) + continue + + if descriptor.default is None and descriptor.default_expression is None: + missing.append(name) + return sorted(missing) + + +def require_snapshot_defaults( + data_strategy: DataStrategy, + param_schema: dict[str, ParamDescriptor] | dict[str, object], +) -> None: + """Reject snapshot endpoints whose binds require caller-supplied values.""" + if data_strategy != DataStrategy.snapshot: + return + missing = missing_snapshot_defaults(param_schema) + if missing: + names = ", ".join(f":{name}" for name in missing) + raise SnapshotConfigurationError( + f"Snapshot endpoints require a default for every parameter. Missing: {names}." + ) + + class EndpointCreate(BaseModel): """Payload for POST /api/v1/admin/endpoints.""" @@ -163,6 +212,7 @@ def bind_params_match_schema(self) -> Self: raise ValueError( f"Schema declares params not referenced in SQL: {sorted(unused)}" ) + require_snapshot_defaults(self.data_strategy, self.param_schema) return self diff --git a/backend/app/services/endpoint.py b/backend/app/services/endpoint.py index 4d93896..a08c805 100644 --- a/backend/app/services/endpoint.py +++ b/backend/app/services/endpoint.py @@ -24,9 +24,11 @@ EndpointUpdate, ParamDescriptor, PublicEndpointError, + SnapshotConfigurationError, SqlPreviewRequest, SqlPreviewResponse, extract_bind_params, + require_snapshot_defaults, ) from app.sql.executor import SqlExecutionError, execute_query @@ -181,6 +183,16 @@ async def update_endpoint( if effective_auth is None and not effective_public: raise PublicEndpointError(PUBLIC_OPT_IN_MESSAGE) + if "data_strategy" in payload.model_fields_set and payload.data_strategy is None: + raise SnapshotConfigurationError("data_strategy cannot be null.") + effective_strategy = payload.data_strategy or obj.data_strategy + effective_param_schema: dict[str, ParamDescriptor] | dict[str, object] + if "param_schema" in payload.model_fields_set and payload.param_schema is not None: + effective_param_schema = payload.param_schema + else: + effective_param_schema = obj.param_schema_json or {} + require_snapshot_defaults(effective_strategy, effective_param_schema) + # Uniqueness check on name change if payload.name is not None and payload.name != obj.name: conflict = await self._repo.get_by_name(payload.name) diff --git a/backend/app/services/schedule.py b/backend/app/services/schedule.py index 2655acb..43d273d 100644 --- a/backend/app/services/schedule.py +++ b/backend/app/services/schedule.py @@ -18,6 +18,7 @@ from app.repositories.job_run import JobRunRepository from app.repositories.schedule import ScheduleRepository from app.repositories.snapshot import SnapshotRepository +from app.schemas.endpoint import require_snapshot_defaults from app.schemas.schedule import ( JobRunResponse, ScheduleCreate, @@ -85,6 +86,7 @@ async def create_schedule( ep = await self._ep_repo.get_by_id(payload.endpoint_id) if ep is None: raise ValueError(f"Endpoint '{payload.endpoint_id}' not found.") + require_snapshot_defaults(ep.data_strategy, ep.param_schema_json or {}) # Check uniqueness — one schedule per endpoint existing = await self._repo.get_by_endpoint_id(payload.endpoint_id) diff --git a/backend/app/services/scheduler.py b/backend/app/services/scheduler.py index abc3384..e5c586f 100644 --- a/backend/app/services/scheduler.py +++ b/backend/app/services/scheduler.py @@ -26,6 +26,7 @@ from app.repositories.schedule import ScheduleRepository from app.repositories.snapshot import SnapshotRepository from app.sql.executor import SqlExecutionError, execute_query +from app.sql.param_models import build_param_model log = structlog.get_logger() @@ -95,13 +96,12 @@ async def execute_scheduled_job(schedule_id: str, endpoint_id: str) -> None: if connection is None or not connection.is_active: raise ValueError("Data source connection is unavailable.") - # Execute SQL (no parameters for scheduled jobs — snapshot queries - # should be parameterless or use defaults) - params: dict[str, object] = {} + # Scheduled snapshots have no request inputs, so validate and + # resolve every configured static or dynamic default through the + # same typed model used by live data requests. param_schema = endpoint.param_schema_json or {} - for param_name, descriptor in param_schema.items(): - if isinstance(descriptor, dict) and descriptor.get("default") is not None: - params[param_name] = descriptor["default"] + ParamModel = build_param_model(param_schema) + params = ParamModel.model_validate({}).model_dump(exclude_none=True) columns, rows, duration_ms = await execute_query( connection=connection, diff --git a/backend/app/sql/param_models.py b/backend/app/sql/param_models.py index bbc6614..9717184 100644 --- a/backend/app/sql/param_models.py +++ b/backend/app/sql/param_models.py @@ -9,10 +9,11 @@ The descriptor format mirrors ``app.schemas.endpoint.ParamDescriptor`` — ``{"type": "string|integer|float|boolean|date", "required": bool, -"default": , "max_length": int | None}``. +"default": , "default_expression": "today|yesterday", +"max_length": int | None}``. """ -from datetime import date +from datetime import date, timedelta from typing import Annotated, Any, Literal, get_args import structlog @@ -49,7 +50,19 @@ def _coerce_bool(value: object) -> object: return value -def _build_field(descriptor: dict[str, Any]) -> tuple[type, Any]: +def resolve_default_expression(expression: str, *, current_date: date | None = None) -> date: + """Resolve a supported dynamic date default using the server's local date.""" + resolved_date = current_date or date.today() + if expression == "today": + return resolved_date + if expression == "yesterday": + return resolved_date - timedelta(days=1) + raise ValueError(f"Unsupported default expression: {expression}") + + +def _build_field( + descriptor: dict[str, Any], *, current_date: date | None = None +) -> tuple[type, Any]: """Map one descriptor to a ``(annotation, default)`` pair for ``create_model``.""" raw_type = descriptor.get("type", "string") if raw_type not in _VALID_PARAM_TYPES: @@ -59,6 +72,15 @@ def _build_field(descriptor: dict[str, Any]) -> tuple[type, Any]: required = bool(descriptor.get("required", True)) default = descriptor.get("default") + default_expression = descriptor.get("default_expression") + if default_expression is not None: + if raw_type != "date": + raise ValueError("Dynamic defaults are supported only for date parameters.") + if default is not None: + raise ValueError("Declare either default or default_expression, not both.") + default = resolve_default_expression( + str(default_expression), current_date=current_date + ) max_length = descriptor.get("max_length") annotation: type @@ -100,7 +122,9 @@ def _build_field(descriptor: dict[str, Any]) -> tuple[type, Any]: return annotation, field_default -def build_param_model(param_schema: dict[str, Any]) -> type[BaseModel]: +def build_param_model( + param_schema: dict[str, Any], *, current_date: date | None = None +) -> type[BaseModel]: """Construct a Pydantic model that validates ``param_schema`` payloads. Non-dict values in ``param_schema`` are ignored to match the legacy @@ -121,7 +145,7 @@ def build_param_model(param_schema: dict[str, Any]) -> type[BaseModel]: descriptor_type=type(descriptor).__name__, ) continue - fields[name] = _build_field(descriptor) + fields[name] = _build_field(descriptor, current_date=current_date) model = create_model( "EndpointParams", diff --git a/backend/tests/test_endpoints.py b/backend/tests/test_endpoints.py index c223cd0..b5d245c 100644 --- a/backend/tests/test_endpoints.py +++ b/backend/tests/test_endpoints.py @@ -13,6 +13,7 @@ EndpointCreate, EndpointResponse, EndpointUpdate, + ParamDescriptor, SqlPreviewRequest, extract_bind_params, validate_sql_safety, @@ -87,6 +88,78 @@ def test_endpoint_create_valid() -> None: assert payload.connection_id == conn_id +def test_date_parameter_accepts_dynamic_default() -> None: + descriptor = ParamDescriptor(type="date", required=True, default_expression="today") + assert descriptor.default_expression == "today" + + +def test_dynamic_default_rejected_for_non_date_parameter() -> None: + with pytest.raises(ValueError, match="only for date"): + ParamDescriptor(type="string", required=True, default_expression="today") + + +def test_static_and_dynamic_defaults_are_mutually_exclusive() -> None: + with pytest.raises(ValueError, match="only one of"): + ParamDescriptor( + type="date", + required=True, + default="2026-08-30", + default_expression="today", + ) + + +def test_snapshot_endpoint_requires_defaults_for_all_parameters() -> None: + with pytest.raises(ValueError, match=r"Missing: :end_date, :start_date"): + EndpointCreate( + name="snapshot-without-defaults", + path="snapshot-without-defaults", + connection_id=uuid.uuid4(), + sql_text=("SELECT * FROM orders WHERE business_date BETWEEN :start_date AND :end_date"), + param_schema={ + "start_date": {"type": "date", "required": True}, + "end_date": {"type": "date", "required": True}, + }, + allow_unauthenticated=True, + data_strategy="snapshot", + ) + + +def test_snapshot_endpoint_accepts_dynamic_defaults() -> None: + payload = EndpointCreate( + name="snapshot-with-dynamic-defaults", + path="snapshot-with-dynamic-defaults", + connection_id=uuid.uuid4(), + sql_text=("SELECT * FROM orders WHERE business_date BETWEEN :start_date AND :end_date"), + param_schema={ + "start_date": { + "type": "date", + "required": True, + "default_expression": "yesterday", + }, + "end_date": { + "type": "date", + "required": True, + "default_expression": "today", + }, + }, + allow_unauthenticated=True, + data_strategy="snapshot", + ) + assert payload.param_schema["start_date"].default_expression == "yesterday" + + +def test_parameterless_snapshot_endpoint_is_valid() -> None: + payload = EndpointCreate( + name="parameterless-snapshot", + path="parameterless-snapshot", + connection_id=uuid.uuid4(), + sql_text="SELECT 1 FROM dual", + allow_unauthenticated=True, + data_strategy="snapshot", + ) + assert payload.param_schema == {} + + def test_endpoint_create_normalizes_path() -> None: conn_id = uuid.uuid4() payload = EndpointCreate( @@ -341,6 +414,59 @@ async def test_update_endpoint(async_client: object) -> None: assert data["is_deprecated"] is True +@pytest.mark.integration +async def test_update_live_endpoint_to_snapshot_requires_merged_defaults( + async_client: object, +) -> None: + from httpx import AsyncClient + + client: AsyncClient = async_client # type: ignore[assignment] + conn_payload = { + "name": f"test-conn-snapshot-update-{uuid.uuid4().hex[:8]}", + "host": "oracle.example.com", + "service_name": "SVC", + "username": "scott", + "password": "tiger", + } + connection = await client.post("/api/v1/admin/connections/", json=conn_payload) + endpoint = await client.post( + "/api/v1/admin/endpoints/", + json={ + "name": f"snapshot-update-{uuid.uuid4().hex[:8]}", + "path": f"snapshot-update-{uuid.uuid4().hex[:8]}", + "connection_id": connection.json()["id"], + "sql_text": "SELECT * FROM orders WHERE business_date = :business_date", + "param_schema": {"business_date": {"type": "date", "required": True}}, + "allow_unauthenticated": True, + "data_strategy": "live", + }, + ) + assert endpoint.status_code == 201 + + invalid = await client.put( + f"/api/v1/admin/endpoints/{endpoint.json()['id']}", + json={"data_strategy": "snapshot"}, + ) + assert invalid.status_code == 422 + assert ":business_date" in invalid.json()["detail"] + + valid = await client.put( + f"/api/v1/admin/endpoints/{endpoint.json()['id']}", + json={ + "data_strategy": "snapshot", + "param_schema": { + "business_date": { + "type": "date", + "required": True, + "default_expression": "today", + } + }, + }, + ) + assert valid.status_code == 200 + assert valid.json()["param_schema"]["business_date"]["default_expression"] == "today" + + @pytest.mark.integration async def test_delete_endpoint(async_client: object) -> None: from httpx import AsyncClient diff --git a/backend/tests/test_param_models.py b/backend/tests/test_param_models.py index 7fedfcb..92fd271 100644 --- a/backend/tests/test_param_models.py +++ b/backend/tests/test_param_models.py @@ -181,3 +181,35 @@ def test_build_param_model_optional_without_default_accepts_value() -> None: Model = build_param_model({"p": {"type": "integer", "required": False}}) instance = Model.model_validate({"p": "42"}) assert instance.model_dump()["p"] == 42 + + +def test_build_param_model_resolves_today_expression() -> None: + Model = build_param_model( + {"p": {"type": "date", "required": True, "default_expression": "today"}}, + current_date=date(2026, 8, 30), + ) + assert Model.model_validate({}).model_dump()["p"] == date(2026, 8, 30) + + +def test_build_param_model_resolves_yesterday_expression() -> None: + Model = build_param_model( + { + "p": { + "type": "date", + "required": True, + "default_expression": "yesterday", + } + }, + current_date=date(2026, 8, 30), + ) + assert Model.model_validate({}).model_dump()["p"] == date(2026, 8, 29) + + +def test_build_param_model_explicit_value_overrides_dynamic_default() -> None: + Model = build_param_model( + {"p": {"type": "date", "required": True, "default_expression": "today"}}, + current_date=date(2026, 8, 30), + ) + assert Model.model_validate({"p": "2026-01-15"}).model_dump()["p"] == date( + 2026, 1, 15 + ) diff --git a/frontend/src/components/endpoints/EndpointWizard.tsx b/frontend/src/components/endpoints/EndpointWizard.tsx index 83f591b..7da62c9 100644 --- a/frontend/src/components/endpoints/EndpointWizard.tsx +++ b/frontend/src/components/endpoints/EndpointWizard.tsx @@ -22,6 +22,7 @@ import { ParamsStep } from "./wizard/ParamsStep"; import { ReviewStep } from "./wizard/ReviewStep"; import { SqlStep } from "./wizard/SqlStep"; import { extractBindParams, reconcileParamSchema } from "./wizard/bindParams"; +import { missingSnapshotDefaults } from "./wizard/parameterDefaults"; import { INITIAL_WIZARD_STATE, WIZARD_STEPS, @@ -118,15 +119,27 @@ export function EndpointWizard({ onSuccess, onCancel }: EndpointWizardProps) { setState((s) => ({ ...s, ...patch })); }, []); - const updateParam = useCallback((name: string, field: keyof ParamDescriptor, value: unknown) => { - setState((s) => ({ - ...s, - param_schema: { - ...s.param_schema, - [name]: { ...s.param_schema[name], [field]: value }, - }, - })); - }, []); + const updateParam = useCallback( + (name: string, field: keyof ParamDescriptor, value: unknown) => { + setState((s) => { + const descriptor = { ...s.param_schema[name], [field]: value }; + if (field === "default" && value !== null && value !== undefined) { + descriptor.default_expression = null; + } + if (field === "default_expression" && value !== null && value !== undefined) { + descriptor.default = null; + } + if (field === "type" && value !== "date") { + descriptor.default_expression = null; + } + return { + ...s, + param_schema: { ...s.param_schema, [name]: descriptor }, + }; + }); + }, + [], + ); const canNext = (): boolean => { if (step === 0) return !!state.connection_id; @@ -136,7 +149,9 @@ export function EndpointWizard({ onSuccess, onCancel }: EndpointWizardProps) { // mirroring the server-side 422 so the admin can't reach Review with // an invalid configuration. const authOk = !!state.auth_method_id || state.allow_unauthenticated; - return !!state.name.trim() && !!state.path.trim() && authOk; + const snapshotDefaultsOk = + state.data_strategy !== "snapshot" || missingSnapshotDefaults(state.param_schema).length === 0; + return !!state.name.trim() && !!state.path.trim() && authOk && snapshotDefaultsOk; } return true; }; diff --git a/frontend/src/components/endpoints/wizard/ConfigStep.test.tsx b/frontend/src/components/endpoints/wizard/ConfigStep.test.tsx index a1040da..0e417a7 100644 --- a/frontend/src/components/endpoints/wizard/ConfigStep.test.tsx +++ b/frontend/src/components/endpoints/wizard/ConfigStep.test.tsx @@ -45,3 +45,43 @@ describe("ConfigStep public-endpoint warning (M1)", () => { expect(screen.queryByText(/This endpoint is PUBLIC/i)).not.toBeInTheDocument(); }); }); + +describe("ConfigStep snapshot defaults", () => { + it("lists snapshot parameters that have no default", () => { + render( + , + ); + + expect(screen.getByText(/Snapshot defaults required/i)).toBeInTheDocument(); + expect(screen.getByText(/:end_date/)).toBeInTheDocument(); + expect(screen.queryByText(/:start_date/)).not.toBeInTheDocument(); + }); + + it("accepts false and zero as configured snapshot defaults", () => { + render( + , + ); + + expect(screen.queryByText(/Snapshot defaults required/i)).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/components/endpoints/wizard/ConfigStep.tsx b/frontend/src/components/endpoints/wizard/ConfigStep.tsx index 00e1c19..c7b03f0 100644 --- a/frontend/src/components/endpoints/wizard/ConfigStep.tsx +++ b/frontend/src/components/endpoints/wizard/ConfigStep.tsx @@ -7,6 +7,7 @@ import type { AuthMethod } from "@/types/auth_method"; import type { DataStrategy } from "@/types/endpoint"; import type { WizardState, WizardUpdate } from "./types"; +import { missingSnapshotDefaults } from "./parameterDefaults"; interface ConfigStepProps { state: WizardState; @@ -15,6 +16,8 @@ interface ConfigStepProps { } export function ConfigStep({ state, update, authMethods }: ConfigStepProps) { + const missingDefaults = missingSnapshotDefaults(state.param_schema); + return (

Endpoint Configuration

@@ -92,6 +95,16 @@ export function ConfigStep({ state, update, authMethods }: ConfigStepProps) {
+ {state.data_strategy === "snapshot" && missingDefaults.length > 0 && ( + + Snapshot defaults required + + Add a fixed or dynamic default for {missingDefaults.map((name) => `:${name}`).join(", ")} + {" "}in the Parameters step. Scheduled snapshots have no request values to bind. + + + )} + {!state.auth_method_id && ( ⚠ This endpoint is PUBLIC diff --git a/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx b/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx index 4296dfb..f567e49 100644 --- a/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx +++ b/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx @@ -139,37 +139,37 @@ describe("ParamsStep", () => { }); describe("boolean type", () => { - it("renders a checkbox", () => { + it("renders a default selector", () => { render( , ); - expect(screen.getByRole("checkbox")).toBeInTheDocument(); + expect(screen.getByLabelText("Default value for active")).toBeInTheDocument(); }); - it("is unchecked when default is null", () => { + it("selects no default when default is null", () => { render( , ); - expect(screen.getByRole("checkbox")).not.toBeChecked(); + expect(screen.getByLabelText("Default value for active")).toHaveValue("none"); }); - it("is checked when default is true", () => { + it("selects true when default is true", () => { render( , ); - expect(screen.getByRole("checkbox")).toBeChecked(); + expect(screen.getByLabelText("Default value for active")).toHaveValue("true"); }); - it("sends true when checked", () => { + it("sends true when selected", () => { const onUpdateParam = vi.fn(); render( { onUpdateParam={onUpdateParam} />, ); - // fireEvent.click toggles the native checked state and fires the change - // event, which React maps to the synthetic onChange handler. - fireEvent.click(screen.getByRole("checkbox")); + fireEvent.change(screen.getByLabelText("Default value for active"), { + target: { value: "true" }, + }); expect(onUpdateParam).toHaveBeenCalledWith("active", "default", true); }); - it("sends null when unchecked", () => { + it("supports false as an explicit default", () => { const onUpdateParam = vi.fn(); render( { onUpdateParam={onUpdateParam} />, ); - fireEvent.click(screen.getByRole("checkbox")); - expect(onUpdateParam).toHaveBeenCalledWith("active", "default", null); + fireEvent.change(screen.getByLabelText("Default value for active"), { + target: { value: "false" }, + }); + expect(onUpdateParam).toHaveBeenCalledWith("active", "default", false); }); }); @@ -233,5 +235,39 @@ describe("ParamsStep", () => { fireEvent.change(input, { target: { value: "" } }); expect(onUpdateParam).toHaveBeenCalledWith("since", "default", null); }); + + it("offers today and yesterday dynamic defaults", () => { + const onUpdateParam = vi.fn(); + render( + , + ); + + fireEvent.change(screen.getByLabelText("Default mode for since"), { + target: { value: "yesterday" }, + }); + expect(onUpdateParam).toHaveBeenCalledWith("since", "default_expression", "yesterday"); + }); + + it("shows the selected dynamic default without a fixed date input", () => { + render( + , + ); + + expect(screen.getByLabelText("Default mode for since")).toHaveValue("today"); + expect(screen.queryByLabelText("Fixed default date for since")).not.toBeInTheDocument(); + }); }); }); diff --git a/frontend/src/components/endpoints/wizard/ParamsStep.tsx b/frontend/src/components/endpoints/wizard/ParamsStep.tsx index e725cd6..fe00d8e 100644 --- a/frontend/src/components/endpoints/wizard/ParamsStep.tsx +++ b/frontend/src/components/endpoints/wizard/ParamsStep.tsx @@ -20,15 +20,22 @@ function DefaultValueControl({ desc, name, onUpdateParam }: DefaultValueControlP switch (desc.type) { case "boolean": return ( -
- onUpdateParam(name, "default", e.target.checked ? true : null)} - /> -
+ ); case "integer": return ( @@ -59,13 +66,36 @@ function DefaultValueControl({ desc, name, onUpdateParam }: DefaultValueControlP ); case "date": return ( - onUpdateParam(name, "default", e.target.value || null)} - placeholder="(none)" - /> +
+ + {desc.default_expression == null && ( + onUpdateParam(name, "default", e.target.value || null)} + /> + )} +
); default: return ( @@ -88,6 +118,12 @@ export function ParamsStep({ state, onUpdateParam }: ParamsStepProps) {

Define types and defaults for bind parameters detected in your query.

+ {state.data_strategy === "snapshot" && entries.length > 0 && ( +

+ Snapshot endpoints require a default for every parameter. Dynamic dates are evaluated + from the application server date whenever the snapshot runs. +

+ )} {entries.length === 0 ? (

No bind parameters detected. You can proceed or go back to modify your query. diff --git a/frontend/src/components/endpoints/wizard/ReviewStep.tsx b/frontend/src/components/endpoints/wizard/ReviewStep.tsx index d185afc..0e56af4 100644 --- a/frontend/src/components/endpoints/wizard/ReviewStep.tsx +++ b/frontend/src/components/endpoints/wizard/ReviewStep.tsx @@ -2,6 +2,7 @@ import type { AuthMethod } from "@/types/auth_method"; import type { Connection } from "@/types/connection"; import type { WizardState } from "./types"; +import { describeParameterDefault } from "./parameterDefaults"; interface ReviewStepProps { state: WizardState; @@ -32,11 +33,17 @@ export function ReviewStep({ state, connections, authMethods }: ReviewStepProps) Strategy: {state.data_strategy} Parameters: - - {Object.keys(state.param_schema).length > 0 - ? Object.keys(state.param_schema).join(", ") - : "None"} - + {Object.keys(state.param_schema).length > 0 ? ( + + {Object.entries(state.param_schema).map(([name, descriptor]) => ( + + :{name} — {descriptor.type}; default: {describeParameterDefault(descriptor)} + + ))} + + ) : ( + None + )}

SQL:

diff --git a/frontend/src/components/endpoints/wizard/parameterDefaults.test.ts b/frontend/src/components/endpoints/wizard/parameterDefaults.test.ts new file mode 100644 index 0000000..36c93fb --- /dev/null +++ b/frontend/src/components/endpoints/wizard/parameterDefaults.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; + +import { + describeParameterDefault, + hasParameterDefault, + missingSnapshotDefaults, +} from "./parameterDefaults"; + +describe("parameter default helpers", () => { + it("counts false, zero, empty strings, and dynamic expressions as defaults", () => { + expect(hasParameterDefault({ type: "boolean", required: true, default: false })).toBe(true); + expect(hasParameterDefault({ type: "integer", required: true, default: 0 })).toBe(true); + expect(hasParameterDefault({ type: "string", required: true, default: "" })).toBe(true); + expect( + hasParameterDefault({ + type: "date", + required: true, + default_expression: "today", + }), + ).toBe(true); + }); + + it("returns only parameter names with no default", () => { + expect( + missingSnapshotDefaults({ + start_date: { type: "date", required: true, default_expression: "yesterday" }, + end_date: { type: "date", required: true, default: null }, + }), + ).toEqual(["end_date"]); + }); + + it("describes dynamic defaults in server-date terms", () => { + expect( + describeParameterDefault({ + type: "date", + required: true, + default_expression: "today", + }), + ).toBe("Today (server date)"); + }); +}); diff --git a/frontend/src/components/endpoints/wizard/parameterDefaults.ts b/frontend/src/components/endpoints/wizard/parameterDefaults.ts new file mode 100644 index 0000000..c9e6a91 --- /dev/null +++ b/frontend/src/components/endpoints/wizard/parameterDefaults.ts @@ -0,0 +1,25 @@ +import type { ParamDescriptor } from "@/types/endpoint"; + +export function hasParameterDefault(descriptor: ParamDescriptor): boolean { + return descriptor.default !== null && descriptor.default !== undefined + ? true + : descriptor.default_expression === "today" || descriptor.default_expression === "yesterday"; +} + +export function missingSnapshotDefaults( + paramSchema: Record, +): string[] { + return Object.entries(paramSchema) + .filter(([, descriptor]) => !hasParameterDefault(descriptor)) + .map(([name]) => name) + .sort(); +} + +export function describeParameterDefault(descriptor: ParamDescriptor): string { + if (descriptor.default_expression === "today") return "Today (server date)"; + if (descriptor.default_expression === "yesterday") return "Yesterday (server date)"; + if (descriptor.default !== null && descriptor.default !== undefined) { + return String(descriptor.default); + } + return "No default"; +} diff --git a/frontend/src/types/endpoint.ts b/frontend/src/types/endpoint.ts index 2555df1..27cb4f2 100644 --- a/frontend/src/types/endpoint.ts +++ b/frontend/src/types/endpoint.ts @@ -1,11 +1,13 @@ /** TypeScript interfaces for API endpoint management (Phase 4). */ export type DataStrategy = "live" | "snapshot"; +export type DateDefaultExpression = "today" | "yesterday"; export interface ParamDescriptor { type: "string" | "integer" | "float" | "date" | "boolean"; required: boolean; default?: string | number | boolean | null; + default_expression?: DateDefaultExpression | null; description?: string; } From d342b9fb01b9a0d9e01bf244df3ed8d11ce5e589 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 21:32:47 +0300 Subject: [PATCH 07/30] feat(parameters): support explicit null defaults --- backend/app/schemas/endpoint.py | 30 +++- backend/app/services/scheduler.py | 12 +- backend/app/sql/param_models.py | 30 ++-- backend/tests/test_endpoints.py | 30 ++++ backend/tests/test_param_models.py | 12 ++ backend/tests/test_scheduler_restore.py | 16 +++ .../components/endpoints/EndpointWizard.tsx | 49 +++---- .../endpoints/wizard/ParamsStep.test.tsx | 43 ++++++ .../endpoints/wizard/ParamsStep.tsx | 135 ++++++++++++------ .../endpoints/wizard/SqlStep.test.tsx | 24 ++++ .../components/endpoints/wizard/SqlStep.tsx | 4 +- .../wizard/parameterDefaults.test.ts | 32 +++++ .../endpoints/wizard/parameterDefaults.ts | 55 ++++++- frontend/src/types/endpoint.ts | 1 + 14 files changed, 379 insertions(+), 94 deletions(-) diff --git a/backend/app/schemas/endpoint.py b/backend/app/schemas/endpoint.py index 42329db..0b21b1e 100644 --- a/backend/app/schemas/endpoint.py +++ b/backend/app/schemas/endpoint.py @@ -79,6 +79,13 @@ class ParamDescriptor(BaseModel): ) required: bool = True default: str | int | float | bool | None = None + default_is_null: bool = Field( + False, + description=( + "Use an explicit SQL NULL when this optional parameter is omitted. " + "This is distinct from having no configured default." + ), + ) default_expression: Literal["today", "yesterday"] | None = Field( None, description=( @@ -95,11 +102,22 @@ class ParamDescriptor(BaseModel): @model_validator(mode="after") def optional_must_have_default(self) -> Self: - if self.default is not None and self.default_expression is not None: - raise ValueError("Declare either default or default_expression, not both.") + configured_defaults = sum( + ( + self.default is not None, + self.default_is_null, + self.default_expression is not None, + ) + ) + if configured_defaults > 1: + raise ValueError( + "Declare only one of default, default_is_null, or default_expression." + ) + if self.default_is_null and self.required: + raise ValueError("A NULL default is supported only for optional parameters.") if self.default_expression is not None and self.type != "date": raise ValueError("default_expression is supported only for date parameters.") - if not self.required and self.default is None and self.default_expression is None: + if not self.required and configured_defaults == 0: raise ValueError("Optional parameters must declare a default value.") return self @@ -118,7 +136,11 @@ def missing_snapshot_defaults( missing.append(name) continue - if descriptor.default is None and descriptor.default_expression is None: + if ( + descriptor.default is None + and not descriptor.default_is_null + and descriptor.default_expression is None + ): missing.append(name) return sorted(missing) diff --git a/backend/app/services/scheduler.py b/backend/app/services/scheduler.py index e5c586f..db12038 100644 --- a/backend/app/services/scheduler.py +++ b/backend/app/services/scheduler.py @@ -47,6 +47,12 @@ def get_scheduler() -> Any: return _scheduler +def resolve_scheduled_params(param_schema: dict[str, Any]) -> dict[str, Any]: + """Resolve every scheduled bind default, retaining explicit SQL NULLs.""" + ParamModel = build_param_model(param_schema) + return ParamModel.model_validate({}).model_dump() + + async def execute_scheduled_job(schedule_id: str, endpoint_id: str) -> None: """Execute a single scheduled job — query Oracle, save snapshot. @@ -100,8 +106,10 @@ async def execute_scheduled_job(schedule_id: str, endpoint_id: str) -> None: # resolve every configured static or dynamic default through the # same typed model used by live data requests. param_schema = endpoint.param_schema_json or {} - ParamModel = build_param_model(param_schema) - params = ParamModel.model_validate({}).model_dump(exclude_none=True) + # Keep explicit NULL defaults in the bind dictionary. Omitting a + # None-valued entry makes Oracle see a missing bind variable rather + # than a bind whose value is SQL NULL. + params = resolve_scheduled_params(param_schema) columns, rows, duration_ms = await execute_query( connection=connection, diff --git a/backend/app/sql/param_models.py b/backend/app/sql/param_models.py index 9717184..d5fb285 100644 --- a/backend/app/sql/param_models.py +++ b/backend/app/sql/param_models.py @@ -9,7 +9,8 @@ The descriptor format mirrors ``app.schemas.endpoint.ParamDescriptor`` — ``{"type": "string|integer|float|boolean|date", "required": bool, -"default": , "default_expression": "today|yesterday", +"default": , "default_is_null": bool, +"default_expression": "today|yesterday", "max_length": int | None}``. """ @@ -61,7 +62,9 @@ def resolve_default_expression(expression: str, *, current_date: date | None = N def _build_field( - descriptor: dict[str, Any], *, current_date: date | None = None + descriptor: dict[str, Any], + *, + current_date: date | None = None, ) -> tuple[type, Any]: """Map one descriptor to a ``(annotation, default)`` pair for ``create_model``.""" raw_type = descriptor.get("type", "string") @@ -72,15 +75,14 @@ def _build_field( required = bool(descriptor.get("required", True)) default = descriptor.get("default") + default_is_null = bool(descriptor.get("default_is_null", False)) default_expression = descriptor.get("default_expression") if default_expression is not None: if raw_type != "date": raise ValueError("Dynamic defaults are supported only for date parameters.") if default is not None: raise ValueError("Declare either default or default_expression, not both.") - default = resolve_default_expression( - str(default_expression), current_date=current_date - ) + default = resolve_default_expression(str(default_expression), current_date=current_date) max_length = descriptor.get("max_length") annotation: type @@ -97,14 +99,10 @@ def _build_field( if isinstance(max_length, int) and max_length >= 1: annotation = Annotated[str, Field(max_length=max_length)] # type: ignore[assignment] - # Legacy ``_coerce_param`` semantics: if a default is configured, - # apply it whenever the query param is missing — regardless of the - # ``required`` flag. The previous Pydantic-based draft made every - # required field unconditionally mandatory, which would 422 endpoints - # whose stored schema combined ``required=true`` with a non-null - # ``default``. ``ParamDescriptor`` still allows that combination, so - # honor it here. - if default is not None: + if default_is_null: + annotation = annotation | None # type: ignore[assignment] + field_default = None + elif default is not None: field_default = default elif required: field_default = ... @@ -123,7 +121,9 @@ def _build_field( def build_param_model( - param_schema: dict[str, Any], *, current_date: date | None = None + param_schema: dict[str, Any], + *, + current_date: date | None = None, ) -> type[BaseModel]: """Construct a Pydantic model that validates ``param_schema`` payloads. @@ -147,7 +147,7 @@ def build_param_model( continue fields[name] = _build_field(descriptor, current_date=current_date) - model = create_model( + model: type[BaseModel] = create_model( "EndpointParams", # ``validate_default=True`` makes Pydantic coerce/validate the # ``default`` we feed each field. Without it, a corrupted stored diff --git a/backend/tests/test_endpoints.py b/backend/tests/test_endpoints.py index b5d245c..90dee0e 100644 --- a/backend/tests/test_endpoints.py +++ b/backend/tests/test_endpoints.py @@ -108,6 +108,17 @@ def test_static_and_dynamic_defaults_are_mutually_exclusive() -> None: ) +def test_optional_parameter_accepts_explicit_null_default() -> None: + descriptor = ParamDescriptor(type="string", required=False, default_is_null=True) + assert descriptor.default_is_null is True + assert descriptor.default is None + + +def test_required_parameter_rejects_explicit_null_default() -> None: + with pytest.raises(ValueError, match="only for optional parameters"): + ParamDescriptor(type="string", required=True, default_is_null=True) + + def test_snapshot_endpoint_requires_defaults_for_all_parameters() -> None: with pytest.raises(ValueError, match=r"Missing: :end_date, :start_date"): EndpointCreate( @@ -148,6 +159,25 @@ def test_snapshot_endpoint_accepts_dynamic_defaults() -> None: assert payload.param_schema["start_date"].default_expression == "yesterday" +def test_snapshot_endpoint_accepts_explicit_null_default() -> None: + payload = EndpointCreate( + name="snapshot-with-null-default", + path="snapshot-with-null-default", + connection_id=uuid.uuid4(), + sql_text="SELECT * FROM stores WHERE :str_id IS NULL OR id = :str_id", + param_schema={ + "str_id": { + "type": "string", + "required": False, + "default_is_null": True, + } + }, + allow_unauthenticated=True, + data_strategy="snapshot", + ) + assert payload.param_schema["str_id"].default_is_null is True + + def test_parameterless_snapshot_endpoint_is_valid() -> None: payload = EndpointCreate( name="parameterless-snapshot", diff --git a/backend/tests/test_param_models.py b/backend/tests/test_param_models.py index 92fd271..e1907d0 100644 --- a/backend/tests/test_param_models.py +++ b/backend/tests/test_param_models.py @@ -183,6 +183,18 @@ def test_build_param_model_optional_without_default_accepts_value() -> None: assert instance.model_dump()["p"] == 42 +def test_build_param_model_explicit_null_default_binds_none() -> None: + Model = build_param_model({"p": {"type": "string", "required": False, "default_is_null": True}}) + instance = Model.model_validate({}) + assert instance.model_dump() == {"p": None} + + +def test_build_param_model_explicit_value_overrides_null_default() -> None: + Model = build_param_model({"p": {"type": "string", "required": False, "default_is_null": True}}) + instance = Model.model_validate({"p": "10105"}) + assert instance.model_dump() == {"p": "10105"} + + def test_build_param_model_resolves_today_expression() -> None: Model = build_param_model( {"p": {"type": "date", "required": True, "default_expression": "today"}}, diff --git a/backend/tests/test_scheduler_restore.py b/backend/tests/test_scheduler_restore.py index 3102ef9..576b550 100644 --- a/backend/tests/test_scheduler_restore.py +++ b/backend/tests/test_scheduler_restore.py @@ -8,6 +8,22 @@ import pytest +def test_scheduled_params_retain_explicit_null_bind() -> None: + from app.services.scheduler import resolve_scheduled_params + + params = resolve_scheduled_params( + { + "str_id": { + "type": "string", + "required": False, + "default_is_null": True, + } + } + ) + + assert params == {"str_id": None} + + @pytest.mark.asyncio async def test_restore_active_schedules_registers_each_row( monkeypatch: pytest.MonkeyPatch, diff --git a/frontend/src/components/endpoints/EndpointWizard.tsx b/frontend/src/components/endpoints/EndpointWizard.tsx index 7da62c9..ff84b8c 100644 --- a/frontend/src/components/endpoints/EndpointWizard.tsx +++ b/frontend/src/components/endpoints/EndpointWizard.tsx @@ -22,7 +22,7 @@ import { ParamsStep } from "./wizard/ParamsStep"; import { ReviewStep } from "./wizard/ReviewStep"; import { SqlStep } from "./wizard/SqlStep"; import { extractBindParams, reconcileParamSchema } from "./wizard/bindParams"; -import { missingSnapshotDefaults } from "./wizard/parameterDefaults"; +import { missingSnapshotDefaults, updateParameterDescriptor } from "./wizard/parameterDefaults"; import { INITIAL_WIZARD_STATE, WIZARD_STEPS, @@ -58,10 +58,16 @@ export function EndpointWizard({ onSuccess, onCancel }: EndpointWizardProps) { connection_id: state.connection_id, sql_text: state.sql_text, params: Object.fromEntries( - Object.entries(state.param_schema).map(([name, descriptor]) => [ - name, - previewParams[name] ?? descriptor.default ?? "", - ]), + Object.entries(state.param_schema).map(([name, descriptor]) => { + const previewValue = previewParams[name]; + const value = + previewValue !== undefined && previewValue.trim().length > 0 + ? previewValue + : descriptor.default_is_null + ? null + : (descriptor.default ?? previewValue ?? ""); + return [name, value]; + }), ), max_rows: 10, }), @@ -119,27 +125,15 @@ export function EndpointWizard({ onSuccess, onCancel }: EndpointWizardProps) { setState((s) => ({ ...s, ...patch })); }, []); - const updateParam = useCallback( - (name: string, field: keyof ParamDescriptor, value: unknown) => { - setState((s) => { - const descriptor = { ...s.param_schema[name], [field]: value }; - if (field === "default" && value !== null && value !== undefined) { - descriptor.default_expression = null; - } - if (field === "default_expression" && value !== null && value !== undefined) { - descriptor.default = null; - } - if (field === "type" && value !== "date") { - descriptor.default_expression = null; - } - return { - ...s, - param_schema: { ...s.param_schema, [name]: descriptor }, - }; - }); - }, - [], - ); + const updateParam = useCallback((name: string, field: keyof ParamDescriptor, value: unknown) => { + setState((s) => { + const descriptor = updateParameterDescriptor(s.param_schema[name], field, value); + return { + ...s, + param_schema: { ...s.param_schema, [name]: descriptor }, + }; + }); + }, []); const canNext = (): boolean => { if (step === 0) return !!state.connection_id; @@ -150,7 +144,8 @@ export function EndpointWizard({ onSuccess, onCancel }: EndpointWizardProps) { // an invalid configuration. const authOk = !!state.auth_method_id || state.allow_unauthenticated; const snapshotDefaultsOk = - state.data_strategy !== "snapshot" || missingSnapshotDefaults(state.param_schema).length === 0; + state.data_strategy !== "snapshot" || + missingSnapshotDefaults(state.param_schema).length === 0; return !!state.name.trim() && !!state.path.trim() && authOk && snapshotDefaultsOk; } return true; diff --git a/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx b/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx index f567e49..8b8d006 100644 --- a/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx +++ b/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx @@ -60,6 +60,30 @@ describe("ParamsStep", () => { fireEvent.change(screen.getByDisplayValue("hello"), { target: { value: "" } }); expect(onUpdateParam).toHaveBeenCalledWith("q", "default", null); }); + + it("allows an optional string to explicitly pass SQL NULL", () => { + const onUpdateParam = vi.fn(); + render( + , + ); + + expect(screen.getByLabelText("Default mode for q")).toHaveValue("null"); + expect(screen.getByText(/Passes SQL NULL/i)).toBeInTheDocument(); + fireEvent.change(screen.getByLabelText("Default mode for q"), { + target: { value: "fixed" }, + }); + expect(onUpdateParam).toHaveBeenCalledWith("q", "default_is_null", false); + }); }); describe("integer type", () => { @@ -196,6 +220,25 @@ describe("ParamsStep", () => { }); expect(onUpdateParam).toHaveBeenCalledWith("active", "default", false); }); + + it("supports NULL as an optional boolean default", () => { + const onUpdateParam = vi.fn(); + render( + , + ); + + expect(screen.getByLabelText("Default value for active")).toHaveValue("null"); + }); }); describe("date type", () => { diff --git a/frontend/src/components/endpoints/wizard/ParamsStep.tsx b/frontend/src/components/endpoints/wizard/ParamsStep.tsx index fe00d8e..3aba790 100644 --- a/frontend/src/components/endpoints/wizard/ParamsStep.tsx +++ b/frontend/src/components/endpoints/wizard/ParamsStep.tsx @@ -1,3 +1,5 @@ +import type { ReactNode } from "react"; + import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Select } from "@/components/ui/select"; @@ -16,6 +18,33 @@ interface DefaultValueControlProps { onUpdateParam: (name: string, field: keyof ParamDescriptor, value: unknown) => void; } +interface ScalarDefaultControlProps extends DefaultValueControlProps { + children: ReactNode; +} + +function ScalarDefaultControl({ desc, name, onUpdateParam, children }: ScalarDefaultControlProps) { + if (desc.required) return
{children}
; + + return ( +
+ + {desc.default_is_null ? ( +

Passes SQL NULL when omitted.

+ ) : ( + children + )} +
+ ); +} + function DefaultValueControl({ desc, name, onUpdateParam }: DefaultValueControlProps) { switch (desc.type) { case "boolean": @@ -23,46 +52,61 @@ function DefaultValueControl({ desc, name, onUpdateParam }: DefaultValueControlP ); case "integer": return ( - { - const parsed = parseInt(e.target.value, 10); - onUpdateParam(name, "default", e.target.value && !isNaN(parsed) ? parsed : null); - }} - placeholder="(none)" - /> + + { + const parsed = parseInt(e.target.value, 10); + onUpdateParam(name, "default", e.target.value && !isNaN(parsed) ? parsed : null); + }} + placeholder="(none)" + /> + ); case "float": return ( - { - const parsed = parseFloat(e.target.value); - onUpdateParam(name, "default", e.target.value && !isNaN(parsed) ? parsed : null); - }} - placeholder="(none)" - /> + + { + const parsed = parseFloat(e.target.value); + onUpdateParam(name, "default", e.target.value && !isNaN(parsed) ? parsed : null); + }} + placeholder="(none)" + /> + ); case "date": return ( @@ -70,23 +114,28 @@ function DefaultValueControl({ desc, name, onUpdateParam }: DefaultValueControlP - {desc.default_expression == null && ( + {desc.default_is_null ? ( +

Passes SQL NULL when omitted.

+ ) : desc.default_expression == null ? ( onUpdateParam(name, "default", e.target.value || null)} /> - )} + ) : null}
); default: return ( - onUpdateParam(name, "default", e.target.value || null)} - placeholder="(none)" - /> + + onUpdateParam(name, "default", e.target.value || null)} + placeholder="(none)" + /> + ); } } @@ -120,8 +171,8 @@ export function ParamsStep({ state, onUpdateParam }: ParamsStepProps) {

{state.data_strategy === "snapshot" && entries.length > 0 && (

- Snapshot endpoints require a default for every parameter. Dynamic dates are evaluated - from the application server date whenever the snapshot runs. + Snapshot endpoints require a fixed, NULL, or dynamic default for every parameter. Dynamic + dates are evaluated from the application server date whenever the snapshot runs.

)} {entries.length === 0 ? ( diff --git a/frontend/src/components/endpoints/wizard/SqlStep.test.tsx b/frontend/src/components/endpoints/wizard/SqlStep.test.tsx index 95ab7f4..4a69bc9 100644 --- a/frontend/src/components/endpoints/wizard/SqlStep.test.tsx +++ b/frontend/src/components/endpoints/wizard/SqlStep.test.tsx @@ -77,4 +77,28 @@ describe("SqlStep preview parameters", () => { fireEvent.click(screen.getByRole("button", { name: "Preview Query" })); expect(onPreview).toHaveBeenCalledOnce(); }); + + it("allows preview when an optional parameter has an explicit NULL default", () => { + const state = makeState(); + state.param_schema.customer_id = { + type: "string", + required: false, + default: null, + default_is_null: true, + }; + + render( + , + ); + + expect(screen.getByRole("button", { name: "Preview Query" })).toBeEnabled(); + }); }); diff --git a/frontend/src/components/endpoints/wizard/SqlStep.tsx b/frontend/src/components/endpoints/wizard/SqlStep.tsx index cd8628d..78c576b 100644 --- a/frontend/src/components/endpoints/wizard/SqlStep.tsx +++ b/frontend/src/components/endpoints/wizard/SqlStep.tsx @@ -28,7 +28,9 @@ export function SqlStep({ const bindParams = Object.entries(state.param_schema); const hasPreviewValues = bindParams.every(([name, descriptor]) => { const value = previewParams[name]; - return value !== undefined ? value.trim().length > 0 : descriptor.default != null; + return value !== undefined && value.trim().length > 0 + ? true + : descriptor.default_is_null === true || descriptor.default != null; }); return ( diff --git a/frontend/src/components/endpoints/wizard/parameterDefaults.test.ts b/frontend/src/components/endpoints/wizard/parameterDefaults.test.ts index 36c93fb..e9f6ed5 100644 --- a/frontend/src/components/endpoints/wizard/parameterDefaults.test.ts +++ b/frontend/src/components/endpoints/wizard/parameterDefaults.test.ts @@ -4,6 +4,7 @@ import { describeParameterDefault, hasParameterDefault, missingSnapshotDefaults, + updateParameterDescriptor, } from "./parameterDefaults"; describe("parameter default helpers", () => { @@ -18,6 +19,13 @@ describe("parameter default helpers", () => { default_expression: "today", }), ).toBe(true); + expect( + hasParameterDefault({ + type: "string", + required: false, + default_is_null: true, + }), + ).toBe(true); }); it("returns only parameter names with no default", () => { @@ -38,4 +46,28 @@ describe("parameter default helpers", () => { }), ).toBe("Today (server date)"); }); + + it("describes an explicit null default", () => { + expect( + describeParameterDefault({ + type: "string", + required: false, + default_is_null: true, + }), + ).toBe("NULL"); + }); + + it("turns an empty optional default into an explicit null bind", () => { + const descriptor = updateParameterDescriptor( + { type: "string", required: true, default: null }, + "required", + false, + ); + + expect(descriptor).toMatchObject({ + required: false, + default: null, + default_is_null: true, + }); + }); }); diff --git a/frontend/src/components/endpoints/wizard/parameterDefaults.ts b/frontend/src/components/endpoints/wizard/parameterDefaults.ts index c9e6a91..46aa76c 100644 --- a/frontend/src/components/endpoints/wizard/parameterDefaults.ts +++ b/frontend/src/components/endpoints/wizard/parameterDefaults.ts @@ -1,14 +1,13 @@ import type { ParamDescriptor } from "@/types/endpoint"; export function hasParameterDefault(descriptor: ParamDescriptor): boolean { + if (descriptor.default_is_null) return true; return descriptor.default !== null && descriptor.default !== undefined ? true : descriptor.default_expression === "today" || descriptor.default_expression === "yesterday"; } -export function missingSnapshotDefaults( - paramSchema: Record, -): string[] { +export function missingSnapshotDefaults(paramSchema: Record): string[] { return Object.entries(paramSchema) .filter(([, descriptor]) => !hasParameterDefault(descriptor)) .map(([name]) => name) @@ -16,6 +15,7 @@ export function missingSnapshotDefaults( } export function describeParameterDefault(descriptor: ParamDescriptor): string { + if (descriptor.default_is_null) return "NULL"; if (descriptor.default_expression === "today") return "Today (server date)"; if (descriptor.default_expression === "yesterday") return "Yesterday (server date)"; if (descriptor.default !== null && descriptor.default !== undefined) { @@ -23,3 +23,52 @@ export function describeParameterDefault(descriptor: ParamDescriptor): string { } return "No default"; } + +export function updateParameterDescriptor( + current: ParamDescriptor, + field: keyof ParamDescriptor, + value: unknown, +): ParamDescriptor { + const descriptor: ParamDescriptor = { ...current, [field]: value }; + + if (field === "default") { + if (value !== null && value !== undefined) { + descriptor.default_expression = null; + descriptor.default_is_null = false; + } else if (!descriptor.required) { + descriptor.default_is_null = true; + } + } + + if (field === "default_expression" && value !== null && value !== undefined) { + descriptor.default = null; + descriptor.default_is_null = false; + } + + if (field === "default_is_null") { + descriptor.default_is_null = value === true; + if (descriptor.default_is_null) { + descriptor.required = false; + descriptor.default = null; + descriptor.default_expression = null; + } + } + + if (field === "required") { + descriptor.required = value === true; + if (descriptor.required) { + descriptor.default_is_null = false; + } else if (descriptor.default == null && descriptor.default_expression == null) { + descriptor.default_is_null = true; + } + } + + if (field === "type" && value !== "date") { + descriptor.default_expression = null; + if (!descriptor.required && descriptor.default == null) { + descriptor.default_is_null = true; + } + } + + return descriptor; +} diff --git a/frontend/src/types/endpoint.ts b/frontend/src/types/endpoint.ts index 27cb4f2..51fdac8 100644 --- a/frontend/src/types/endpoint.ts +++ b/frontend/src/types/endpoint.ts @@ -7,6 +7,7 @@ export interface ParamDescriptor { type: "string" | "integer" | "float" | "date" | "boolean"; required: boolean; default?: string | number | boolean | null; + default_is_null?: boolean; default_expression?: DateDefaultExpression | null; description?: string; } From f4343bdf72cfb2f127215843eeb210bc76372f8a Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 21:38:38 +0300 Subject: [PATCH 08/30] fix(data): enforce required live parameters --- backend/app/services/data.py | 6 +++- backend/app/sql/param_models.py | 16 +++++++-- backend/tests/test_param_models.py | 55 ++++++++++++++++++++++++------ backend/tests/test_security.py | 42 +++++++++++++++++++++++ docs/architecture.md | 2 ++ 5 files changed, 108 insertions(+), 13 deletions(-) diff --git a/backend/app/services/data.py b/backend/app/services/data.py index c75da10..f17368b 100644 --- a/backend/app/services/data.py +++ b/backend/app/services/data.py @@ -367,7 +367,11 @@ async def _serve_live( @staticmethod def _coerce_params(endpoint: ApiEndpoint, request: Request) -> dict[str, Any]: param_schema = endpoint.param_schema_json or {} - Model = build_param_model(param_schema) + # Scheduler defaults make snapshot refreshes autonomous, but they do + # not make required HTTP query parameters optional. Live callers must + # supply every descriptor marked required; configured defaults remain + # available to the scheduler and to omitted optional request fields. + Model = build_param_model(param_schema, enforce_required=True) # Pull only declared params from the query string; ignore unknowns # so the legacy loop's behavior is preserved. Filter on # ``isinstance(descriptor, dict)`` so a corrupted non-dict diff --git a/backend/app/sql/param_models.py b/backend/app/sql/param_models.py index d5fb285..b4a56e9 100644 --- a/backend/app/sql/param_models.py +++ b/backend/app/sql/param_models.py @@ -65,6 +65,7 @@ def _build_field( descriptor: dict[str, Any], *, current_date: date | None = None, + enforce_required: bool = False, ) -> tuple[type, Any]: """Map one descriptor to a ``(annotation, default)`` pair for ``create_model``.""" raw_type = descriptor.get("type", "string") @@ -99,7 +100,13 @@ def _build_field( if isinstance(max_length, int) and max_length >= 1: annotation = Annotated[str, Field(max_length=max_length)] # type: ignore[assignment] - if default_is_null: + # Scheduled execution must resolve every configured default without + # request input. Live requests use ``enforce_required=True`` so a + # scheduler default never weakens their public required-parameter + # contract. Optional request fields continue to use their defaults. + if enforce_required and required: + field_default = ... + elif default_is_null: annotation = annotation | None # type: ignore[assignment] field_default = None elif default is not None: @@ -124,6 +131,7 @@ def build_param_model( param_schema: dict[str, Any], *, current_date: date | None = None, + enforce_required: bool = False, ) -> type[BaseModel]: """Construct a Pydantic model that validates ``param_schema`` payloads. @@ -145,7 +153,11 @@ def build_param_model( descriptor_type=type(descriptor).__name__, ) continue - fields[name] = _build_field(descriptor, current_date=current_date) + fields[name] = _build_field( + descriptor, + current_date=current_date, + enforce_required=enforce_required, + ) model: type[BaseModel] = create_model( "EndpointParams", diff --git a/backend/tests/test_param_models.py b/backend/tests/test_param_models.py index e1907d0..26f7a2b 100644 --- a/backend/tests/test_param_models.py +++ b/backend/tests/test_param_models.py @@ -69,9 +69,7 @@ def test_build_param_model_coerces_value( @pytest.mark.parametrize(("descriptor", "raw"), _GOLDEN_FAILURES) -def test_build_param_model_rejects_value( - descriptor: dict[str, object], raw: str -) -> None: +def test_build_param_model_rejects_value(descriptor: dict[str, object], raw: str) -> None: Model = build_param_model({"p": descriptor}) with pytest.raises(ValidationError): Model.model_validate({"p": raw}) @@ -140,10 +138,7 @@ def test_build_param_model_boolean_default_round_trips() -> None: def test_build_param_model_default_applies_when_required_and_missing() -> None: - """Legacy code applied a configured ``default`` whenever the param was - missing — regardless of the ``required`` flag. Pin that contract here - so the Pydantic-based path doesn't 422 endpoints whose stored schema - combined ``required=true`` with a non-null default.""" + """Default-resolution mode lets scheduled jobs resolve required binds.""" Model = build_param_model( {"p": {"type": "integer", "required": True, "default": 42}}, ) @@ -183,6 +178,48 @@ def test_build_param_model_optional_without_default_accepts_value() -> None: assert instance.model_dump()["p"] == 42 +def test_build_param_model_enforces_required_despite_static_default() -> None: + Model = build_param_model( + {"p": {"type": "integer", "required": True, "default": 42}}, + enforce_required=True, + ) + with pytest.raises(ValidationError) as exc_info: + Model.model_validate({}) + assert any(err["loc"] == ("p",) for err in exc_info.value.errors()) + + +def test_build_param_model_enforces_required_despite_dynamic_default() -> None: + Model = build_param_model( + { + "p": { + "type": "date", + "required": True, + "default_expression": "today", + } + }, + current_date=date(2026, 8, 30), + enforce_required=True, + ) + with pytest.raises(ValidationError) as exc_info: + Model.model_validate({}) + assert any(err["loc"] == ("p",) for err in exc_info.value.errors()) + + +def test_build_param_model_required_value_overrides_scheduler_default() -> None: + Model = build_param_model( + { + "p": { + "type": "date", + "required": True, + "default_expression": "today", + } + }, + current_date=date(2026, 8, 30), + enforce_required=True, + ) + assert Model.model_validate({"p": "2026-01-15"}).model_dump()["p"] == date(2026, 1, 15) + + def test_build_param_model_explicit_null_default_binds_none() -> None: Model = build_param_model({"p": {"type": "string", "required": False, "default_is_null": True}}) instance = Model.model_validate({}) @@ -222,6 +259,4 @@ def test_build_param_model_explicit_value_overrides_dynamic_default() -> None: {"p": {"type": "date", "required": True, "default_expression": "today"}}, current_date=date(2026, 8, 30), ) - assert Model.model_validate({"p": "2026-01-15"}).model_dump()["p"] == date( - 2026, 1, 15 - ) + assert Model.model_validate({"p": "2026-01-15"}).model_dump()["p"] == date(2026, 1, 15) diff --git a/backend/tests/test_security.py b/backend/tests/test_security.py index b800fe7..413cffa 100644 --- a/backend/tests/test_security.py +++ b/backend/tests/test_security.py @@ -468,6 +468,48 @@ async def test_missing_required_param(self, async_client: object) -> None: assert r.status_code == 422 assert "id" in r.json()["detail"] + async def test_required_live_param_does_not_fall_back_to_scheduler_default( + self, async_client: object + ) -> None: + client: AsyncClient = async_client # type: ignore[assignment] + + connection = await client.post( + "/api/v1/admin/connections/", + json={ + "name": _unique("required-default-conn"), + "host": "oracle.example.com", + "service_name": "SVC", + "username": "hr", + "password": "secret", + }, + ) + assert connection.status_code == 201 + + ep_path = _unique("required-default-data") + endpoint = await client.post( + "/api/v1/admin/endpoints/", + json={ + "name": _unique("required-default-ep"), + "path": ep_path, + "connection_id": connection.json()["id"], + "allow_unauthenticated": True, + "sql_text": "SELECT * FROM t WHERE business_date = :business_date", + "param_schema": { + "business_date": { + "type": "date", + "required": True, + "default_expression": "yesterday", + } + }, + }, + ) + assert endpoint.status_code == 201 + + response = await client.get(f"/api/v1/data/{ep_path}") + assert response.status_code == 422 + assert "business_date" in response.json()["detail"] + assert "Field required" in response.json()["detail"] + async def test_invalid_param_type(self, async_client: object) -> None: client: AsyncClient = async_client # type: ignore[assignment] diff --git a/docs/architecture.md b/docs/architecture.md index 7a01749..d03d36c 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -41,6 +41,8 @@ ### SQL Safety All user-defined SQL is executed via SQLAlchemy `text()` with named bind parameters (`:param_name`). String interpolation of user input is prohibited at all layers. Bind values are validated through typed Pydantic schemas before reaching the query executor. +Live data requests enforce every parameter marked `required`, even when that parameter also has a scheduler default. Defaults exist so scheduled execution can run without request input and so optional live parameters can be omitted. + ### Authentication - Admin API: session-based or JWT Bearer (TBD per Phase 1). - Data endpoints: per-endpoint configurable auth — Bearer token, Basic Auth, or API key. Middleware resolves the policy from endpoint metadata at request time and enforces it when an auth method is attached. Endpoints published without an auth method are served publicly, so one must be assigned to any endpoint that should be protected. From ff6ab59238e28d5a9f452c3894104f2a983f92e6 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 21:39:39 +0300 Subject: [PATCH 09/30] fix(parameters): accept day-first dates --- backend/app/sql/param_models.py | 20 ++++++++- backend/tests/test_param_models.py | 5 ++- backend/tests/test_security.py | 67 ++++++++++++++++++++++++++++++ docs/architecture.md | 2 +- 4 files changed, 89 insertions(+), 5 deletions(-) diff --git a/backend/app/sql/param_models.py b/backend/app/sql/param_models.py index b4a56e9..3461a63 100644 --- a/backend/app/sql/param_models.py +++ b/backend/app/sql/param_models.py @@ -14,7 +14,7 @@ "max_length": int | None}``. """ -from datetime import date, timedelta +from datetime import date, datetime, timedelta from typing import Annotated, Any, Literal, get_args import structlog @@ -51,6 +51,22 @@ def _coerce_bool(value: object) -> object: return value +def _coerce_date(value: object) -> object: + """Accept the documented HTTP date formats and return a date value.""" + if isinstance(value, datetime): + return value.date() + if isinstance(value, date): + return value + if isinstance(value, str): + for date_format in ("%Y-%m-%d", "%d-%m-%Y"): + try: + return datetime.strptime(value, date_format).date() + except ValueError: + continue + raise ValueError("expected YYYY-MM-DD or DD-MM-YYYY") + return value + + def resolve_default_expression(expression: str, *, current_date: date | None = None) -> date: """Resolve a supported dynamic date default using the server's local date.""" resolved_date = current_date or date.today() @@ -94,7 +110,7 @@ def _build_field( elif raw_type == "boolean": annotation = Annotated[bool, BeforeValidator(_coerce_bool)] # type: ignore[assignment] elif raw_type == "date": - annotation = date + annotation = Annotated[date, BeforeValidator(_coerce_date)] # type: ignore[assignment] else: # "string" annotation = str if isinstance(max_length, int) and max_length >= 1: diff --git a/backend/tests/test_param_models.py b/backend/tests/test_param_models.py index 26f7a2b..b247e01 100644 --- a/backend/tests/test_param_models.py +++ b/backend/tests/test_param_models.py @@ -42,8 +42,9 @@ ({"type": "boolean", "required": True}, "false", False), ({"type": "boolean", "required": True}, "0", False), ({"type": "boolean", "required": True}, "NO", False), - # date — must be ISO format + # date — public API accepts ISO and the Oracle-oriented DD-MM-YYYY format ({"type": "date", "required": True}, "2024-01-15", date(2024, 1, 15)), + ({"type": "date", "required": True}, "15-01-2024", date(2024, 1, 15)), ] _GOLDEN_FAILURES: list[tuple[dict[str, object], str]] = [ @@ -217,7 +218,7 @@ def test_build_param_model_required_value_overrides_scheduler_default() -> None: current_date=date(2026, 8, 30), enforce_required=True, ) - assert Model.model_validate({"p": "2026-01-15"}).model_dump()["p"] == date(2026, 1, 15) + assert Model.model_validate({"p": "15-01-2026"}).model_dump()["p"] == date(2026, 1, 15) def test_build_param_model_explicit_null_default_binds_none() -> None: diff --git a/backend/tests/test_security.py b/backend/tests/test_security.py index 413cffa..927080e 100644 --- a/backend/tests/test_security.py +++ b/backend/tests/test_security.py @@ -11,6 +11,7 @@ """ import uuid +from datetime import date import pytest from app.schemas.endpoint import EndpointCreate, validate_sql_safety @@ -510,6 +511,72 @@ async def test_required_live_param_does_not_fall_back_to_scheduler_default( assert "business_date" in response.json()["detail"] assert "Field required" in response.json()["detail"] + async def test_live_date_params_accept_dd_mm_yyyy( + self, + async_client: object, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + client: AsyncClient = async_client # type: ignore[assignment] + captured_params: dict[str, object] = {} + + async def fake_execute_query( + **kwargs: object, + ) -> tuple[list[str], list[dict[str, object]], float]: + params = kwargs["params"] + assert isinstance(params, dict) + captured_params.update(params) + return ["ok"], [{"ok": True}], 1.0 + + monkeypatch.setattr("app.services.data.execute_query", fake_execute_query) + + connection = await client.post( + "/api/v1/admin/connections/", + json={ + "name": _unique("date-format-conn"), + "host": "oracle.example.com", + "service_name": "SVC", + "username": "hr", + "password": "secret", + }, + ) + assert connection.status_code == 201 + + ep_path = _unique("date-format-data") + endpoint = await client.post( + "/api/v1/admin/endpoints/", + json={ + "name": _unique("date-format-ep"), + "path": ep_path, + "connection_id": connection.json()["id"], + "allow_unauthenticated": True, + "sql_text": ( + "SELECT * FROM t WHERE business_date BETWEEN :start_date AND :end_date" + ), + "param_schema": { + "start_date": { + "type": "date", + "required": True, + "default_expression": "yesterday", + }, + "end_date": { + "type": "date", + "required": True, + "default_expression": "yesterday", + }, + }, + }, + ) + assert endpoint.status_code == 201 + + response = await client.get( + f"/api/v1/data/{ep_path}?start_date=08-08-2026&end_date=15-08-2026" + ) + assert response.status_code == 200 + assert captured_params == { + "start_date": date(2026, 8, 8), + "end_date": date(2026, 8, 15), + } + async def test_invalid_param_type(self, async_client: object) -> None: client: AsyncClient = async_client # type: ignore[assignment] diff --git a/docs/architecture.md b/docs/architecture.md index d03d36c..a8b22f3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -41,7 +41,7 @@ ### SQL Safety All user-defined SQL is executed via SQLAlchemy `text()` with named bind parameters (`:param_name`). String interpolation of user input is prohibited at all layers. Bind values are validated through typed Pydantic schemas before reaching the query executor. -Live data requests enforce every parameter marked `required`, even when that parameter also has a scheduler default. Defaults exist so scheduled execution can run without request input and so optional live parameters can be omitted. +Live data requests enforce every parameter marked `required`, even when that parameter also has a scheduler default. Defaults exist so scheduled execution can run without request input and so optional live parameters can be omitted. Date query parameters accept `YYYY-MM-DD` and `DD-MM-YYYY`; both formats are normalized to Python `date` values before database binding. ### Authentication - Admin API: session-based or JWT Bearer (TBD per Phase 1). From 2eb8ffb2ab441afbb53b8e5cfefe815f466fdca6 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 21:40:13 +0300 Subject: [PATCH 10/30] docs: document scheduler and parameter fixes --- README.md | 6 +++--- docs/architecture.md | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index da7afee..39d04a4 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ Define connection ─▶ Author SQL (with :bind params) ─▶ Attach auth ─ ``` 1. **Connect** to your Oracle database with securely stored, encrypted credentials. -2. **Author** a `SELECT` query using named bind parameters (`:param_name`) in a rich SQL editor. The wizard detects parameters as you type and requests temporary sample values before previewing the results. +2. **Author** a `SELECT` query using named bind parameters (`:param_name`) in a rich SQL editor. The wizard detects parameters as you type, requests temporary sample values before previewing, and supports fixed or dynamic date defaults (`today` and `yesterday`). Snapshot endpoints require a default for every bind so scheduled refreshes can execute without request inputs. Scheduler defaults do not make required parameters optional for live HTTP requests; callers must supply required dates as either `YYYY-MM-DD` or `DD-MM-YYYY`. 3. **Secure** the endpoint by attaching a Bearer token, Basic Auth, or API key policy. 4. **Choose** a data strategy: serve results **live** on each request, or from a **scheduled snapshot** cache. 5. **Publish** a versioned endpoint under `/api/v1/data/*` that resolves dynamically — no service restart needed. @@ -73,9 +73,9 @@ QueryGateway is organized into five admin modules, all driven from the React adm | Module | What it does | |--------|--------------| | **Connections** | Create, edit, test, and delete Oracle database connections. Credentials are encrypted at rest; pool sizing and timeouts are configurable. Uses `python-oracledb`; thin mode needs no native client, while the backend Docker image includes Oracle Instant Client 19.32 for thick mode. | -| **API Creation Wizard** | A multi-step wizard that turns a parameterized SQL query into a deployable GET endpoint: pick a connection, author SQL with a rich editor, supply preview-only sample values for detected bind parameters, preview sample rows and inferred schema, map/rename output columns, attach an auth method, and select a data strategy. | +| **API Creation Wizard** | A multi-step wizard that turns a parameterized SQL query into a deployable GET endpoint: pick a connection, author SQL with a rich editor, supply preview-only sample values for detected bind parameters, configure fixed values, explicit SQL `NULL` values for optional binds, or dynamic date defaults, preview sample rows and inferred schema, map/rename output columns, attach an auth method, and select a data strategy. | | **Authentication** | Manage per-endpoint auth methods — Bearer token (JWT), Basic Auth, and API key. Tokens are issued/verified with `PyJWT`; credentials are hashed with `bcrypt`. When an auth method is attached to an endpoint, middleware enforces it on every request; endpoints with no auth method attached are served publicly. | -| **Scheduling & Snapshots** | Schedule query refreshes with the in-process APScheduler (cron or interval). Run now, pause/resume, and enable/disable jobs. Results are cached as PostgreSQL JSONB snapshots and served with freshness metadata. Schedule definitions are persisted in the app database; the active APScheduler jobs run in-memory and are (re)registered when a schedule is created, updated, or resumed. | +| **Scheduling & Snapshots** | Schedule query refreshes with friendly hourly, daily, weekly, or monthly calendar controls, an advanced custom-cron option, or a fixed interval. Run now, pause/resume, and enable/disable jobs. Results are cached as PostgreSQL JSONB snapshots and served with freshness metadata. Schedule definitions are persisted in the app database, active APScheduler jobs are restored on API startup, and deleting a schedule preserves its job-run and snapshot history. Deleting an endpoint removes its schedule and snapshots while retaining orphaned job-run audit records. | | **Settings & Health** | Configure runtime settings (base URL/port, logging level, query timeouts, CORS/rate-limit inputs) and view a health dashboard covering API, PostgreSQL, Oracle connectivity, scheduler status, and recent job outcomes. | ### Security by Default diff --git a/docs/architecture.md b/docs/architecture.md index a8b22f3..8e54693 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -49,10 +49,12 @@ Live data requests enforce every parameter marked `required`, even when that par - Credentials are hashed with `bcrypt`; tokens are issued/verified with `PyJWT`. ### Scheduler -APScheduler 3.x runs in-process with an in-memory job store. Schedule definitions are persisted in the app database (PostgreSQL), and the corresponding APScheduler jobs are (re)registered when a schedule is created, updated, or resumed. The in-memory jobs are not automatically reloaded on process restart — active schedule rows remain in the database, but their jobs are re-registered on the next create/update/resume. Execution telemetry (start time, duration, row count, status, errors) is written to the app DB and exposed through admin APIs. +APScheduler 3.x runs in-process with an in-memory job store. Schedule definitions are persisted in PostgreSQL, and every active schedule is registered when it is created, updated, resumed, or restored during API startup. Execution telemetry (start time, duration, row count, status, errors) is written to the app DB and exposed through admin APIs. Deleting a schedule sets the historical job run's `schedule_id` to `NULL`, preserving both audit history and snapshots. Deleting an endpoint removes its schedule and cached snapshots, unregisters its in-memory job after the database commit, and sets the historical job run's `endpoint_id` (and cascaded `schedule_id`) to `NULL` so the audit record remains available. + +The scheduler is intentionally single-process. Run one API process/replica unless distributed scheduler coordination is added; otherwise each process would register and execute the same persisted schedules. ### Snapshot Cache -Scheduled endpoints can serve results from a PostgreSQL JSONB snapshot rather than executing live queries. Freshness metadata is returned in responses. Stale snapshot fallback behavior is configurable per endpoint. +Scheduled endpoints can serve results from a PostgreSQL JSONB snapshot rather than executing live queries. Every bind parameter on a snapshot endpoint must define a fixed default, an explicit SQL `NULL` default for an optional bind, or a dynamic date expression (`today` or `yesterday`) so refreshes never depend on request inputs. Dynamic defaults are resolved from the application server date at execution time, while explicit null defaults remain present in the bind dictionary with a `None` value so the database receives SQL `NULL`. Freshness metadata is returned in responses. Stale snapshot fallback behavior is configurable per endpoint. ### Configuration Pydantic Settings v2 loads all configuration from environment variables (`.env` in development, injected secrets in production). No hardcoded configuration values in source code. From ec18fe2a8d201c3d55b91607aa8e6c0bd928df76 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 21:40:28 +0300 Subject: [PATCH 11/30] style(backend): format touched code --- backend/app/models/job_run.py | 4 +--- backend/app/routers/endpoints.py | 8 ++------ backend/app/schemas/schedule.py | 4 +--- backend/tests/test_migration.py | 4 +--- 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/backend/app/models/job_run.py b/backend/app/models/job_run.py index 94eae4f..27d9f4f 100644 --- a/backend/app/models/job_run.py +++ b/backend/app/models/job_run.py @@ -38,9 +38,7 @@ class JobRun(UUIDPrimaryKeyMixin, Base): ) started_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) - finished_at: Mapped[datetime | None] = mapped_column( - DateTime(timezone=True), nullable=True - ) + finished_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) status: Mapped[JobRunStatus] = mapped_column( SAEnum(JobRunStatus, name="job_run_status"), nullable=False, diff --git a/backend/app/routers/endpoints.py b/backend/app/routers/endpoints.py index 71beac0..1a26dad 100644 --- a/backend/app/routers/endpoints.py +++ b/backend/app/routers/endpoints.py @@ -96,9 +96,7 @@ async def get_endpoint( ) -> EndpointResponse: result = await svc.get_endpoint(endpoint_id) if result is None: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail="Endpoint not found." - ) + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Endpoint not found.") return result @@ -163,6 +161,4 @@ async def preview_sql( try: return await svc.preview_sql(payload) except ValueError as exc: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc) - ) from exc + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc diff --git a/backend/app/schemas/schedule.py b/backend/app/schemas/schedule.py index 6aac698..cb22429 100644 --- a/backend/app/schemas/schedule.py +++ b/backend/app/schemas/schedule.py @@ -37,9 +37,7 @@ def model_post_init(self, __context: object) -> None: if self.schedule_type == "cron" and not self.cron_expression: raise ValueError("cron_expression is required when schedule_type is 'cron'.") if self.schedule_type == "interval" and not self.interval_seconds: - raise ValueError( - "interval_seconds is required when schedule_type is 'interval'." - ) + raise ValueError("interval_seconds is required when schedule_type is 'interval'.") class ScheduleUpdate(BaseModel): diff --git a/backend/tests/test_migration.py b/backend/tests/test_migration.py index afc3217..3d77481 100644 --- a/backend/tests/test_migration.py +++ b/backend/tests/test_migration.py @@ -249,9 +249,7 @@ async def test_tables_created(self, async_client: object) -> None: r = await client.get("/api/v1/admin/health/ready") assert r.status_code == 200 - async def test_connection_crud_after_fresh_schema( - self, async_client: object - ) -> None: + async def test_connection_crud_after_fresh_schema(self, async_client: object) -> None: """Verify CRUD operations work on fresh schema.""" from httpx import AsyncClient From 27aab9a8968715ba97b7d1705b04e47c5d456400 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 22:35:43 +0300 Subject: [PATCH 12/30] fix(deps): update vulnerable frontend packages --- frontend/package-lock.json | 83 ++++++++++++++++++++++---------------- 1 file changed, 49 insertions(+), 34 deletions(-) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 78f3f78..a32a298 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -121,6 +121,7 @@ "integrity": "sha512-RgHBCvtjbOK2gXSNBNIkNoEc9qoVEtau3hj8gEqKQuL3HZAibKarWFEI3Lfm6EYKkLalOh8eSrj9b+ch9H/VBA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.29.7", "@babel/generator": "^7.29.7", @@ -481,6 +482,7 @@ "resolved": "https://registry.npmjs.org/@codemirror/view/-/view-6.39.16.tgz", "integrity": "sha512-m6S22fFpKtOWhq8HuhzsI1WzUP/hB9THbDj0Tl5KX4gbO6Y91hwBl7Yky33NdvB6IffuRFiBxf1R8kJMyXmA4Q==", "license": "MIT", + "peer": true, "dependencies": { "@codemirror/state": "^6.5.0", "crelt": "^1.0.6", @@ -576,6 +578,7 @@ } ], "license": "MIT", + "peer": true, "engines": { "node": ">=18" }, @@ -599,6 +602,7 @@ } ], "license": "MIT", + "peer": true, "engines": { "node": ">=18" } @@ -1903,8 +1907,7 @@ "resolved": "https://registry.npmjs.org/@types/aria-query/-/aria-query-5.0.4.tgz", "integrity": "sha512-rfT93uj5s0PRL7EzccGMs3brplhcrghnDoV26NqKhCAS1hVo+WdNsPvE/yb6ilfr5hi2MEk6d5EWJTKdxg8jVw==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@types/babel__core": { "version": "7.20.5", @@ -1989,6 +1992,7 @@ "integrity": "sha512-akNQMv0wW5uyRpD2v2IEyRSZiR+BeGuoB6L310EgGObO44HSMNT8z1xzio28V8qOrgYaopIDNA18YgdXd+qTiw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~6.21.0" } @@ -2006,6 +2010,7 @@ "integrity": "sha512-z9VXpC7MWrhfWipitjNdgCauoMLRdIILQsAEV+ZesIzBq/oUlxk0m3ApZuMFCXdnS4U7KrI+l3WRUEGQ8K1QKw==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "@types/prop-types": "*", "csstype": "^3.2.2" @@ -2017,6 +2022,7 @@ "integrity": "sha512-MEe3UeoENYVFXzoXEWsvcpg6ZvlrFNlOQ7EOsvhI3CfAXwzPfO8Qwuxd40nepsYKqyyVQnTdEfv68q91yLcKrQ==", "dev": true, "license": "MIT", + "peer": true, "peerDependencies": { "@types/react": "^18.0.0" } @@ -2066,6 +2072,7 @@ "integrity": "sha512-klQbnPAAiGYFyI02+znpBRLyjL4/BrBd0nyWkdC0s/6xFLkXYQ8OoRrSkqacS1ddVxf/LDyODIKbQ5TgKAf/Fg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.56.1", "@typescript-eslint/types": "8.56.1", @@ -2220,16 +2227,16 @@ } }, "node_modules/@typescript-eslint/typescript-estree/node_modules/brace-expansion": { - "version": "5.0.6", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", - "integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==", + "version": "5.0.9", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.9.tgz", + "integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/@typescript-eslint/typescript-estree/node_modules/minimatch": { @@ -2509,6 +2516,7 @@ "integrity": "sha512-UVJyE9MttOsBQIDKw1skb9nAwQuR5wuGD3+82K6JgJlm/Y+KI92oNsMNGZCYdDsVtRHSak0pcV5Dno5+4jh9sw==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2559,7 +2567,6 @@ "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=8" } @@ -2763,9 +2770,9 @@ } }, "node_modules/brace-expansion": { - "version": "1.1.15", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.15.tgz", - "integrity": "sha512-EwOCDEex4quD37XhqM3omwtMoJjr//isUZz1JopUNWms+4Z2ViyM/k1YIRePpoVNnQhENnxtFjLaxNHrT7xIUg==", + "version": "1.1.18", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", + "integrity": "sha512-Edep/X9fGqVNmzKBVsDYIOtD+z1tuezV70LBjdCst9Tqu76lsnvRiZ6oTic1n+/BIwX6QDGAO94PN4N2SADvtw==", "dev": true, "license": "MIT", "dependencies": { @@ -2806,6 +2813,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.9.0", "caniuse-lite": "^1.0.30001759", @@ -3196,8 +3204,7 @@ "resolved": "https://registry.npmjs.org/dom-accessibility-api/-/dom-accessibility-api-0.5.16.tgz", "integrity": "sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/dunder-proto": { "version": "1.0.1", @@ -3356,6 +3363,7 @@ "integrity": "sha512-VmQ+sifHUbI/IcSopBCF/HO3YiHQx/AVd3UVyYL6weuwW+HvON9VYn5l6Zl1WZzPWXPNZrSQpxwkkZ/VuvJZzg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -4087,6 +4095,7 @@ "integrity": "sha512-/imKNG4EbWNrVjoNC/1H5/9GFy+tqjGBHCaSsN+P2RnPqjsLmv6UD3Ej+Kj8nBWaRAwyk7kK5ZUc+OEatnTR3A==", "dev": true, "license": "MIT", + "peer": true, "bin": { "jiti": "bin/jiti.js" } @@ -4098,9 +4107,9 @@ "license": "MIT" }, "node_modules/js-yaml": { - "version": "4.3.0", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.0.tgz", - "integrity": "sha512-1td788aAnnZ5qs7V2QIRl1owjtYpbKt749Y3xauqQgwIIGF/xXWz1wMTEBx5O3LK3lXLVuqXPdPxj2BoFHaW9Q==", + "version": "4.3.2", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.2.tgz", + "integrity": "sha512-SFNOvSJ+Dgf/9An904Yx+CgSlIPCkIpao4qo51lpee25TIRejdH3rhR4EZMGoNx3/TP3O+wzWuiTFl4sqbltzA==", "dev": true, "funding": [ { @@ -4126,6 +4135,7 @@ "integrity": "sha512-8i7LzZj7BF8uplX+ZyOlIz86V6TAsSs+np6m1kpW9u0JWi4z/1t+FzcK1aek+ybTnAC4KhBL4uXCNT0wcUIeCw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "cssstyle": "^4.1.0", "data-urls": "^5.0.0", @@ -4312,7 +4322,6 @@ "integrity": "sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ==", "dev": true, "license": "MIT", - "peer": true, "bin": { "lz-string": "bin/bin.js" } @@ -4423,9 +4432,9 @@ } }, "node_modules/nanoid": { - "version": "3.3.15", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.15.tgz", - "integrity": "sha512-y7Wygv/7mEOvxTuEQDB8StXdMRBWf1kR/tlhAzBRUFkB2jfcLOAxO/SHmOO2zgz1pVgK29/kyupn059/bCHdjA==", + "version": "3.3.18", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz", + "integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==", "dev": true, "funding": [ { @@ -4657,9 +4666,9 @@ } }, "node_modules/postcss": { - "version": "8.5.15", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.15.tgz", - "integrity": "sha512-FfR8sjd4em2T6fb3I2MwAJU7HWVMr9zba+enmQeeWFfCbm+UOC/0X4DS8XtpUTMwWMGbjKYP7xjfNekzyGmB3A==", + "version": "8.5.26", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.26.tgz", + "integrity": "sha512-u82N74LFzG8ca+dD8puPnplTXoGH4fTPpVGuIbt36G3qvNlkvfD0lEAZSxaly3KX8TS/L1A1gsCEmvKmBcVbkQ==", "dev": true, "funding": [ { @@ -4676,8 +4685,9 @@ } ], "license": "MIT", + "peer": true, "dependencies": { - "nanoid": "^3.3.12", + "nanoid": "^3.3.17", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, @@ -4849,6 +4859,7 @@ "integrity": "sha512-UOnG6LftzbdaHZcKoPFtOcCKztrQ57WkHDeRD9t/PTQtmT0NHSeWWepj6pS0z/N7+08BHFDQVUrfmfMRcZwbMg==", "dev": true, "license": "MIT", + "peer": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -4952,7 +4963,6 @@ "integrity": "sha512-Qb1gy5OrP5+zDf2Bvnzdl3jsTf1qXVMazbvCoKhtKqVs4/YK4ozX4gKQJJVyNe+cajNPn0KoC0MC3FUmaHWEmQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "ansi-regex": "^5.0.1", "ansi-styles": "^5.0.0", @@ -4968,7 +4978,6 @@ "integrity": "sha512-Cxwpt2SfTzTtXcfOlzGEee8O+c+MmUgGrNiBcXnuWxuFJHe6a5Hz7qwhwe5OgaSYI0IJvkLqWX1ASG+cJOkEiA==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=10" }, @@ -5021,6 +5030,7 @@ "resolved": "https://registry.npmjs.org/react/-/react-18.3.1.tgz", "integrity": "sha512-wS+hAgJShR0KhEvPJArfuPVN1+Hz1t0Y6n5jLrGQbkb4urgPE/0Rve+1kMB1v/oWgHgm4WIcV+i7F2pTVj+2iQ==", "license": "MIT", + "peer": true, "dependencies": { "loose-envify": "^1.1.0" }, @@ -5033,6 +5043,7 @@ "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-18.3.1.tgz", "integrity": "sha512-5m4nQKp+rZRb09LNH59GM4BxTh9251/ylbKIbpe7TpGxfJ+9kv6BLkLBXIjjspbgbnIBNqlI23tRnTWT0snUIw==", "license": "MIT", + "peer": true, "dependencies": { "loose-envify": "^1.1.0", "scheduler": "^0.23.2" @@ -5046,8 +5057,7 @@ "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", "integrity": "sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/react-refresh": { "version": "0.17.0", @@ -5060,9 +5070,9 @@ } }, "node_modules/react-router": { - "version": "7.18.0", - "resolved": "https://registry.npmjs.org/react-router/-/react-router-7.18.0.tgz", - "integrity": "sha512-pTTGt8J+ji1NOmYnjzT+bAJy/1zD+Jp4ziO6cL7T3ZLvXKtusO7BpFqlRXitqpcPVqllsIXFHRMt+2/k3Xn6HQ==", + "version": "7.18.3", + "resolved": "https://registry.npmjs.org/react-router/-/react-router-7.18.3.tgz", + "integrity": "sha512-gyXgtdr5uACJ5b1Q4udzjVV+tb/rlHIMJKuJ0e89R4Kzgz47z/rgP0dIKxktqIEUhDHluGTPJJH/wRha7CyqsA==", "license": "MIT", "dependencies": { "cookie": "^1.0.1", @@ -5082,12 +5092,12 @@ } }, "node_modules/react-router-dom": { - "version": "7.18.0", - "resolved": "https://registry.npmjs.org/react-router-dom/-/react-router-dom-7.18.0.tgz", - "integrity": "sha512-Fi0yY6kgtKae/Th2xibdWK0KSdYZ4B53Gyf6wRtomOKWgpNm7H7+DyfDhncdz9FKbpS+1jmDhg3F4WoGJ+yFOA==", + "version": "7.18.3", + "resolved": "https://registry.npmjs.org/react-router-dom/-/react-router-dom-7.18.3.tgz", + "integrity": "sha512-ytVbyBBM7vMfRCam25r0WMhSVSom909A8p+8m0/f1w853dz/xfFu6etAT2SEbVoSnI+ZoPRDqIsQXVT89gp7kg==", "license": "MIT", "dependencies": { - "react-router": "7.18.0" + "react-router": "7.18.3" }, "engines": { "node": ">=20.0.0" @@ -5455,6 +5465,7 @@ "integrity": "sha512-3ofp+LL8E+pK/JuPLPggVAIaEuhvIz4qNcf3nA1Xn2o/7fb7s/TYpHhwGDv1ZU3PkBluUVaF8PyCHcm48cKLWQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@alloc/quick-lru": "^5.2.0", "arg": "^5.0.2", @@ -5582,6 +5593,7 @@ "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -5697,6 +5709,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -5790,6 +5803,7 @@ "integrity": "sha512-NTKlcQjlAK7MlQoyb6LgaqHc8sso/pVyUJYWMws3jg21uTJw/LddqIFPcPqP6PzpgbIcZyKI85sFE4HBrQDA8A==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.4.4", @@ -5883,6 +5897,7 @@ "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, From b3ce137e86b5cb10f42cba23d0286e3423e8162c Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 22:38:37 +0300 Subject: [PATCH 13/30] style(frontend): format endpoint wizard changes --- frontend/src/components/endpoints/wizard/ConfigStep.tsx | 5 +++-- frontend/src/components/endpoints/wizard/ReviewStep.tsx | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/endpoints/wizard/ConfigStep.tsx b/frontend/src/components/endpoints/wizard/ConfigStep.tsx index c7b03f0..a505e91 100644 --- a/frontend/src/components/endpoints/wizard/ConfigStep.tsx +++ b/frontend/src/components/endpoints/wizard/ConfigStep.tsx @@ -99,8 +99,9 @@ export function ConfigStep({ state, update, authMethods }: ConfigStepProps) { Snapshot defaults required - Add a fixed or dynamic default for {missingDefaults.map((name) => `:${name}`).join(", ")} - {" "}in the Parameters step. Scheduled snapshots have no request values to bind. + Add a fixed or dynamic default for{" "} + {missingDefaults.map((name) => `:${name}`).join(", ")} in the Parameters step. Scheduled + snapshots have no request values to bind. )} diff --git a/frontend/src/components/endpoints/wizard/ReviewStep.tsx b/frontend/src/components/endpoints/wizard/ReviewStep.tsx index 0e56af4..b7cb7f5 100644 --- a/frontend/src/components/endpoints/wizard/ReviewStep.tsx +++ b/frontend/src/components/endpoints/wizard/ReviewStep.tsx @@ -37,7 +37,8 @@ export function ReviewStep({ state, connections, authMethods }: ReviewStepProps) {Object.entries(state.param_schema).map(([name, descriptor]) => ( - :{name} — {descriptor.type}; default: {describeParameterDefault(descriptor)} + :{name} — {descriptor.type}; default:{" "} + {describeParameterDefault(descriptor)} ))} From 396ba7467eb57fbe1bb06daefcd68663e33baeee Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 22:53:46 +0300 Subject: [PATCH 14/30] fix(scheduler): fail startup when job restoration fails --- backend/app/services/scheduler.py | 77 ++++++++++++++++--------- backend/tests/test_scheduler_restore.py | 61 +++++++++++++++++++- 2 files changed, 111 insertions(+), 27 deletions(-) diff --git a/backend/app/services/scheduler.py b/backend/app/services/scheduler.py index db12038..561d94c 100644 --- a/backend/app/services/scheduler.py +++ b/backend/app/services/scheduler.py @@ -147,28 +147,30 @@ async def execute_scheduled_job(schedule_id: str, endpoint_id: str) -> None: from app.repositories.settings import SettingsRepository # noqa: PLC0415 settings_repo = SettingsRepository(db) - retention_setting = await settings_repo.get_by_key( - "snapshot_retention_count" - ) - retention_count = ( - int(retention_setting.value) if retention_setting else 5 - ) + retention_setting = await settings_repo.get_by_key("snapshot_retention_count") + retention_count = int(retention_setting.value) if retention_setting else 5 await snap_repo.delete_old(eid, keep=retention_count) # Mark job success finished_at = datetime.now(UTC) - await job_repo.update(job_run, { - "finished_at": finished_at, - "status": JobRunStatus.success, - "row_count": len(rows), - }) + await job_repo.update( + job_run, + { + "finished_at": finished_at, + "status": JobRunStatus.success, + "row_count": len(rows), + }, + ) # Update schedule last_run_at schedule = await sched_repo.get_by_id(sid) if schedule: - await sched_repo.update(schedule, { - "last_run_at": finished_at, - }) + await sched_repo.update( + schedule, + { + "last_run_at": finished_at, + }, + ) await db.commit() @@ -190,18 +192,24 @@ async def execute_scheduled_job(schedule_id: str, endpoint_id: str) -> None: if "timeout" in error_detail.lower(): status = JobRunStatus.timeout - await job_repo.update(job_run, { - "finished_at": finished_at, - "status": status, - "error_detail": error_detail, - }) + await job_repo.update( + job_run, + { + "finished_at": finished_at, + "status": status, + "error_detail": error_detail, + }, + ) # Update schedule last_run_at even on failure schedule = await sched_repo.get_by_id(sid) if schedule: - await sched_repo.update(schedule, { - "last_run_at": finished_at, - }) + await sched_repo.update( + schedule, + { + "last_run_at": finished_at, + }, + ) await db.commit() @@ -218,24 +226,31 @@ async def execute_scheduled_job(schedule_id: str, endpoint_id: str) -> None: async def restore_active_schedules() -> int: """Register all active database schedules in the in-memory scheduler.""" restored = 0 + failed_schedule_ids: list[str] = [] async with AsyncSessionLocal() as db: schedules = await ScheduleRepository(db).get_all(active_only=True) for schedule in schedules: try: - add_schedule_job( + registered = add_schedule_job( schedule_id=schedule.id, endpoint_id=schedule.endpoint_id, schedule_type=schedule.schedule_type, cron_expression=schedule.cron_expression, interval_seconds=schedule.interval_seconds, ) + if not registered: + raise ValueError("Schedule configuration could not be registered.") restored += 1 except Exception as exc: # noqa: BLE001 + failed_schedule_ids.append(str(schedule.id)) log.error( "scheduler_job_restore_failed", schedule_id=str(schedule.id), error=str(exc), ) + if failed_schedule_ids: + failed = ", ".join(failed_schedule_ids) + raise RuntimeError(f"Failed to restore active schedules: {failed}") log.info("scheduler_jobs_restored", restored_count=restored) return restored @@ -244,6 +259,7 @@ async def start_scheduler() -> None: """Initialize and start the APScheduler instance.""" global _scheduler # noqa: PLW0603 + scheduler = None try: from apscheduler.schedulers.asyncio import AsyncIOScheduler # noqa: PLC0415 @@ -259,7 +275,15 @@ async def start_scheduler() -> None: log.info("scheduler_started") await restore_active_schedules() except Exception as exc: # noqa: BLE001 + if _scheduler is not None: + stop_scheduler() + elif scheduler is not None: + try: + scheduler.shutdown(wait=False) + except Exception as shutdown_exc: # noqa: BLE001 + log.warning("scheduler_cleanup_failed", error=str(shutdown_exc)) log.error("scheduler_start_failed", error=str(exc)) + raise def stop_scheduler() -> None: @@ -280,11 +304,11 @@ def add_schedule_job( schedule_type: str, cron_expression: str | None = None, interval_seconds: int | None = None, -) -> None: +) -> bool: """Register a job in APScheduler.""" if _scheduler is None: log.warning("scheduler_not_running", action="add_job") - return + return False from apscheduler.schedulers.asyncio import AsyncIOScheduler # noqa: PLC0415 @@ -318,7 +342,7 @@ def add_schedule_job( kwargs["seconds"] = interval_seconds else: log.warning("invalid_schedule_config", schedule_id=str(schedule_id)) - return + return False scheduler.add_job(**kwargs) log.info( @@ -326,6 +350,7 @@ def add_schedule_job( job_id=job_id, schedule_type=schedule_type, ) + return True def remove_schedule_job(schedule_id: uuid.UUID) -> None: diff --git a/backend/tests/test_scheduler_restore.py b/backend/tests/test_scheduler_restore.py index 576b550..8c5ed30 100644 --- a/backend/tests/test_scheduler_restore.py +++ b/backend/tests/test_scheduler_restore.py @@ -3,7 +3,7 @@ import uuid from types import SimpleNamespace from typing import cast -from unittest.mock import MagicMock +from unittest.mock import AsyncMock, MagicMock import pytest @@ -72,3 +72,62 @@ async def get_all(self, *, active_only: bool = False) -> list[object]: assert restored == 2 assert add_job.call_count == 2 assert add_job.call_args_list[0].kwargs["schedule_id"] == schedules[0].id + + +@pytest.mark.asyncio +async def test_restore_active_schedules_fails_when_a_job_is_not_registered( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from app.services import scheduler as scheduler_service + + schedule = SimpleNamespace( + id=uuid.uuid4(), + endpoint_id=uuid.uuid4(), + schedule_type="cron", + cron_expression=None, + interval_seconds=None, + ) + + class FakeSessionContext: + async def __aenter__(self) -> object: + return object() + + async def __aexit__(self, *args: object) -> None: + return None + + class FakeScheduleRepository: + def __init__(self, db: object) -> None: + self.db = db + + async def get_all(self, *, active_only: bool = False) -> list[object]: + assert active_only is True + return [schedule] + + monkeypatch.setattr(scheduler_service, "AsyncSessionLocal", FakeSessionContext) + monkeypatch.setattr(scheduler_service, "ScheduleRepository", FakeScheduleRepository) + + with pytest.raises(RuntimeError, match=str(schedule.id)): + await scheduler_service.restore_active_schedules() + + +@pytest.mark.asyncio +async def test_start_scheduler_cleans_up_and_propagates_restore_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from app.services import scheduler as scheduler_service + from apscheduler.schedulers import asyncio as apscheduler_asyncio + + scheduler = MagicMock() + monkeypatch.setattr(apscheduler_asyncio, "AsyncIOScheduler", MagicMock(return_value=scheduler)) + monkeypatch.setattr( + scheduler_service, + "restore_active_schedules", + AsyncMock(side_effect=RuntimeError("database unavailable")), + ) + monkeypatch.setattr(scheduler_service, "_scheduler", None) + + with pytest.raises(RuntimeError, match="database unavailable"): + await scheduler_service.start_scheduler() + + scheduler.shutdown.assert_called_once_with(wait=False) + assert scheduler_service._scheduler is None From c50a56c84c14a313f88bebfa055a2808f86be1ae Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 22:55:30 +0300 Subject: [PATCH 15/30] test: validate destructive database target name --- backend/tests/conftest.py | 6 ++++-- backend/tests/test_test_database_guard.py | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 5034c87..ebc2e26 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -49,6 +49,7 @@ from app.main import app from app.models.base import Base from httpx import ASGITransport, AsyncClient +from sqlalchemy.engine import make_url from sqlalchemy.ext.asyncio import ( AsyncEngine, AsyncSession, @@ -63,10 +64,11 @@ def _assert_safe_test_database(database_url: str, app_env: str) -> None: """Require both an explicit test environment and a test-named database.""" - if app_env != "test" or "test" not in database_url.lower(): + database_name = make_url(database_url).database or "" + if app_env != "test" or "test" not in database_name.lower(): raise RuntimeError( "Refusing to run destructive test schema setup: APP_ENV must be " - "'test' and DATABASE_URL must contain 'test'." + "'test' and the parsed database name must contain 'test'." ) diff --git a/backend/tests/test_test_database_guard.py b/backend/tests/test_test_database_guard.py index 8d0d99b..e1b7d62 100644 --- a/backend/tests/test_test_database_guard.py +++ b/backend/tests/test_test_database_guard.py @@ -24,3 +24,18 @@ def test_database_guard_rejects_any_non_test_signal( ) -> None: with pytest.raises(RuntimeError, match="APP_ENV must be 'test'"): _assert_safe_test_database(database_url, app_env) + + +@pytest.mark.parametrize( + "database_url", + [ + "postgresql+asyncpg://test_runner:pass@db:5432/customer_data", + "postgresql+asyncpg://user:pass@test-db:5432/customer_data", + "postgresql+asyncpg://user:pass@db:5432/customer_data?application_name=test", + ], +) +def test_database_guard_ignores_test_marker_outside_database_name( + database_url: str, +) -> None: + with pytest.raises(RuntimeError, match="database name must contain 'test'"): + _assert_safe_test_database(database_url, "test") From 479a124c17ac482876110626e32a1c39d4d74bed Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 22:58:54 +0300 Subject: [PATCH 16/30] fix(parameters): validate configured defaults --- backend/app/schemas/endpoint.py | 12 +++++++-- backend/tests/test_endpoints.py | 44 +++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/backend/app/schemas/endpoint.py b/backend/app/schemas/endpoint.py index 0b21b1e..fbabb98 100644 --- a/backend/app/schemas/endpoint.py +++ b/backend/app/schemas/endpoint.py @@ -117,8 +117,16 @@ def optional_must_have_default(self) -> Self: raise ValueError("A NULL default is supported only for optional parameters.") if self.default_expression is not None and self.type != "date": raise ValueError("default_expression is supported only for date parameters.") - if not self.required and configured_defaults == 0: - raise ValueError("Optional parameters must declare a default value.") + if configured_defaults: + from app.sql.param_models import build_param_model # noqa: PLC0415 + + try: + model = build_param_model({"value": self.model_dump()}) + model.model_validate({}) + except (TypeError, ValueError) as exc: + raise ValueError( + f"Invalid default for parameter type '{self.type}'." + ) from exc return self diff --git a/backend/tests/test_endpoints.py b/backend/tests/test_endpoints.py index 90dee0e..fb2d0f0 100644 --- a/backend/tests/test_endpoints.py +++ b/backend/tests/test_endpoints.py @@ -9,6 +9,7 @@ import uuid import pytest +from app.models.endpoint import DataStrategy from app.schemas.endpoint import ( EndpointCreate, EndpointResponse, @@ -16,6 +17,7 @@ ParamDescriptor, SqlPreviewRequest, extract_bind_params, + require_snapshot_defaults, validate_sql_safety, ) @@ -119,6 +121,48 @@ def test_required_parameter_rejects_explicit_null_default() -> None: ParamDescriptor(type="string", required=True, default_is_null=True) +def test_optional_parameter_accepts_no_default() -> None: + descriptor = ParamDescriptor(type="integer", required=False) + assert descriptor.default is None + assert descriptor.default_is_null is False + + +def test_parameter_rejects_incompatible_static_default() -> None: + with pytest.raises(ValueError, match="Invalid default"): + ParamDescriptor(type="integer", required=True, default="abc") + + +def test_endpoint_create_rejects_incompatible_static_default() -> None: + with pytest.raises(ValueError, match="Invalid default"): + EndpointCreate( + name="invalid-default", + path="invalid-default", + connection_id=uuid.uuid4(), + sql_text="SELECT * FROM stores WHERE id = :store_id", + param_schema={ + "store_id": {"type": "integer", "required": True, "default": "abc"} + }, + allow_unauthenticated=True, + ) + + +def test_endpoint_update_rejects_incompatible_static_default() -> None: + with pytest.raises(ValueError, match="Invalid default"): + EndpointUpdate( + param_schema={ + "store_id": {"type": "integer", "required": True, "default": "abc"} + } + ) + + +def test_snapshot_default_validation_rejects_incompatible_stored_default() -> None: + with pytest.raises(ValueError, match="Invalid default"): + require_snapshot_defaults( + DataStrategy.snapshot, + {"store_id": {"type": "integer", "required": True, "default": "abc"}}, + ) + + def test_snapshot_endpoint_requires_defaults_for_all_parameters() -> None: with pytest.raises(ValueError, match=r"Missing: :end_date, :start_date"): EndpointCreate( From 38b87f43e30c57f3b7f6bb1214cf33f0202dd396 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 22:59:30 +0300 Subject: [PATCH 17/30] test: clarify non-sensitive schedule credential --- backend/tests/test_schedules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/tests/test_schedules.py b/backend/tests/test_schedules.py index 269f68c..13d4a45 100644 --- a/backend/tests/test_schedules.py +++ b/backend/tests/test_schedules.py @@ -384,7 +384,7 @@ async def test_delete_schedule_preserves_job_run_history( "host": "oracle.example.com", "service_name": "SVC", "username": "scott", - "password": "tiger", + "password": "test-password", }, ) endpoint = await client.post( From 231a6fbaf50b486d29e14461fa9c78e339bf4071 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 23:00:49 +0300 Subject: [PATCH 18/30] docs: fix architecture heading spacing --- docs/architecture.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/architecture.md b/docs/architecture.md index 8e54693..3a94acb 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -39,27 +39,33 @@ ## Key Design Decisions ### SQL Safety + All user-defined SQL is executed via SQLAlchemy `text()` with named bind parameters (`:param_name`). String interpolation of user input is prohibited at all layers. Bind values are validated through typed Pydantic schemas before reaching the query executor. Live data requests enforce every parameter marked `required`, even when that parameter also has a scheduler default. Defaults exist so scheduled execution can run without request input and so optional live parameters can be omitted. Date query parameters accept `YYYY-MM-DD` and `DD-MM-YYYY`; both formats are normalized to Python `date` values before database binding. ### Authentication + - Admin API: session-based or JWT Bearer (TBD per Phase 1). - Data endpoints: per-endpoint configurable auth — Bearer token, Basic Auth, or API key. Middleware resolves the policy from endpoint metadata at request time and enforces it when an auth method is attached. Endpoints published without an auth method are served publicly, so one must be assigned to any endpoint that should be protected. - Credentials are hashed with `bcrypt`; tokens are issued/verified with `PyJWT`. ### Scheduler + APScheduler 3.x runs in-process with an in-memory job store. Schedule definitions are persisted in PostgreSQL, and every active schedule is registered when it is created, updated, resumed, or restored during API startup. Execution telemetry (start time, duration, row count, status, errors) is written to the app DB and exposed through admin APIs. Deleting a schedule sets the historical job run's `schedule_id` to `NULL`, preserving both audit history and snapshots. Deleting an endpoint removes its schedule and cached snapshots, unregisters its in-memory job after the database commit, and sets the historical job run's `endpoint_id` (and cascaded `schedule_id`) to `NULL` so the audit record remains available. The scheduler is intentionally single-process. Run one API process/replica unless distributed scheduler coordination is added; otherwise each process would register and execute the same persisted schedules. ### Snapshot Cache + Scheduled endpoints can serve results from a PostgreSQL JSONB snapshot rather than executing live queries. Every bind parameter on a snapshot endpoint must define a fixed default, an explicit SQL `NULL` default for an optional bind, or a dynamic date expression (`today` or `yesterday`) so refreshes never depend on request inputs. Dynamic defaults are resolved from the application server date at execution time, while explicit null defaults remain present in the bind dictionary with a `None` value so the database receives SQL `NULL`. Freshness metadata is returned in responses. Stale snapshot fallback behavior is configurable per endpoint. ### Configuration + Pydantic Settings v2 loads all configuration from environment variables (`.env` in development, injected secrets in production). No hardcoded configuration values in source code. ### Logging + `structlog` emits structured JSON with mandatory correlation fields (`request_id`, `user`, `endpoint`, `status`, `duration_ms`, `event`). Sensitive fields are redacted at middleware level before emission. ## Directory Layout From 5bdd4305d812b2c474790f6f45363bbbd5b26998 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 23:01:12 +0300 Subject: [PATCH 19/30] docs(deployment): prevent overlapping schedulers --- docs/deployment.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/deployment.md b/docs/deployment.md index fe7a6d2..c9b4f7a 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -158,13 +158,22 @@ psql -c "CREATE DATABASE db2api_db OWNER db2api_user;" Use the Docker images as a starting point. Key considerations: -- Deploy the API as a `Deployment` with `replicas: 1` while the scheduler is in-process +- Deploy the API as a `Deployment` with `replicas: 1` and `strategy.type: Recreate` + while the scheduler is in-process. A rolling update can overlap the old and new + Pods even with one replica, causing both schedulers to execute the same jobs. +- Add distributed scheduler coordination before using rolling updates or multiple + API replicas. - Use a `Service` for internal routing and an `Ingress` for external access - PostgreSQL: use a managed service (e.g., AWS RDS, GCP Cloud SQL) or a StatefulSet - Store `ENCRYPTION_KEY` and `DATABASE_URL` as Kubernetes Secrets - Configure liveness and readiness probes: ```yaml +spec: + replicas: 1 + strategy: + type: Recreate + livenessProbe: httpGet: path: /api/v1/admin/health/live From 352b7db14aac4de2cf66c85f5961033de6116bb2 Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 23:04:24 +0300 Subject: [PATCH 20/30] fix(parameters): clear boolean defaults in wizard --- .../endpoints/wizard/ParamsStep.test.tsx | 17 +++++++++++++++++ .../components/endpoints/wizard/ParamsStep.tsx | 1 + 2 files changed, 18 insertions(+) diff --git a/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx b/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx index 8b8d006..eadb065 100644 --- a/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx +++ b/frontend/src/components/endpoints/wizard/ParamsStep.test.tsx @@ -221,6 +221,23 @@ describe("ParamsStep", () => { expect(onUpdateParam).toHaveBeenCalledWith("active", "default", false); }); + it("clears an existing boolean value when no default is selected", () => { + const onUpdateParam = vi.fn(); + render( + , + ); + + fireEvent.change(screen.getByLabelText("Default value for active"), { + target: { value: "none" }, + }); + + expect(onUpdateParam).toHaveBeenNthCalledWith(1, "active", "default", null); + expect(onUpdateParam).toHaveBeenNthCalledWith(2, "active", "default_is_null", false); + }); + it("supports NULL as an optional boolean default", () => { const onUpdateParam = vi.fn(); render( diff --git a/frontend/src/components/endpoints/wizard/ParamsStep.tsx b/frontend/src/components/endpoints/wizard/ParamsStep.tsx index 3aba790..74f1352 100644 --- a/frontend/src/components/endpoints/wizard/ParamsStep.tsx +++ b/frontend/src/components/endpoints/wizard/ParamsStep.tsx @@ -65,6 +65,7 @@ function DefaultValueControl({ desc, name, onUpdateParam }: DefaultValueControlP if (e.target.value === "null") { onUpdateParam(name, "default_is_null", true); } else if (e.target.value === "none") { + onUpdateParam(name, "default", null); onUpdateParam(name, "default_is_null", false); } else { onUpdateParam(name, "default", e.target.value === "true"); From 19e2034b1d84dc35360bd58bd6ede538ab4cf18f Mon Sep 17 00:00:00 2001 From: Badry Date: Sun, 30 Aug 2026 23:08:16 +0300 Subject: [PATCH 21/30] fix(endpoints): resolve defaults in SQL preview --- .../endpoints/EndpointWizard.test.tsx | 108 ++++++++++++++++++ .../components/endpoints/EndpointWizard.tsx | 13 ++- .../endpoints/wizard/SqlStep.test.tsx | 24 ++++ .../components/endpoints/wizard/SqlStep.tsx | 5 +- .../wizard/parameterDefaults.test.ts | 31 +++++ .../endpoints/wizard/parameterDefaults.ts | 29 +++++ 6 files changed, 203 insertions(+), 7 deletions(-) create mode 100644 frontend/src/components/endpoints/EndpointWizard.test.tsx diff --git a/frontend/src/components/endpoints/EndpointWizard.test.tsx b/frontend/src/components/endpoints/EndpointWizard.test.tsx new file mode 100644 index 0000000..ea49a15 --- /dev/null +++ b/frontend/src/components/endpoints/EndpointWizard.test.tsx @@ -0,0 +1,108 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const listConnectionsMock = vi.fn(); +const listAuthMethodsMock = vi.fn(); +const previewMock = vi.fn(); +const createMock = vi.fn(); + +vi.mock("@/components/endpoints/SqlEditor", () => ({ + SqlEditor: ({ value, onChange }: { value: string; onChange: (value: string) => void }) => ( +