feat: add Metrics API endpoints (GET/POST/DELETE) with admin auth
- New app/api/metrics.py: GET /api/metrics, GET /api/metrics/{id:path},
POST /api/admin/metrics (201), DELETE /api/admin/metrics/{id:path}
- Add require_admin dependency to app/auth/dependencies.py
- Register metrics_router in app/main.py before web_router
- Deprecate GET /api/catalog/metrics/{path} with 301 redirect to new endpoint
- 7 new tests in TestMetricsAPI covering CRUD, 404, RBAC, category filter
This commit is contained in:
parent
5cddb5573a
commit
5cf0df77fc
5 changed files with 221 additions and 22 deletions
|
|
@ -1,11 +1,9 @@
|
||||||
"""Catalog endpoints — table profiles, metrics."""
|
"""Catalog endpoints — table profiles, metrics."""
|
||||||
|
|
||||||
import json
|
import json
|
||||||
import re
|
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException
|
from fastapi import APIRouter, Depends, HTTPException
|
||||||
import duckdb
|
import duckdb
|
||||||
import yaml
|
|
||||||
|
|
||||||
from app.auth.dependencies import get_current_user, _get_db
|
from app.auth.dependencies import get_current_user, _get_db
|
||||||
from app.utils import get_data_dir as _get_data_dir
|
from app.utils import get_data_dir as _get_data_dir
|
||||||
|
|
@ -70,30 +68,15 @@ async def list_catalog_tables(
|
||||||
return {"tables": tables, "count": len(tables)}
|
return {"tables": tables, "count": len(tables)}
|
||||||
|
|
||||||
|
|
||||||
@router.get("/metrics/{metric_path:path}")
|
@router.get("/metrics/{metric_path:path}", deprecated=True)
|
||||||
async def get_metric(
|
async def get_metric(
|
||||||
metric_path: str,
|
metric_path: str,
|
||||||
user: dict = Depends(get_current_user),
|
user: dict = Depends(get_current_user),
|
||||||
):
|
):
|
||||||
"""Get a metric YAML definition parsed as structured JSON."""
|
"""Deprecated: use GET /api/metrics/{metric_id} instead."""
|
||||||
if not re.match(r"^[a-z_]+/[a-z_]+\.yml$", metric_path):
|
from fastapi.responses import RedirectResponse
|
||||||
raise HTTPException(status_code=400, detail="Invalid metric path")
|
metric_id = metric_path.replace(".yml", "")
|
||||||
|
return RedirectResponse(url=f"/api/metrics/{metric_id}", status_code=301)
|
||||||
docs_dir = _get_data_dir() / "docs" / "metrics"
|
|
||||||
file_path = docs_dir / metric_path
|
|
||||||
|
|
||||||
if not file_path.is_file():
|
|
||||||
raise HTTPException(status_code=404, detail="Metric file not found")
|
|
||||||
|
|
||||||
# Security: ensure path doesn't escape docs dir
|
|
||||||
if not file_path.resolve().is_relative_to(docs_dir.resolve()):
|
|
||||||
raise HTTPException(status_code=400, detail="Invalid path")
|
|
||||||
|
|
||||||
try:
|
|
||||||
content = yaml.safe_load(file_path.read_text())
|
|
||||||
return content
|
|
||||||
except Exception as e:
|
|
||||||
raise HTTPException(status_code=500, detail=f"Error parsing metric: {e}")
|
|
||||||
|
|
||||||
|
|
||||||
@router.post("/profile/{table_name}/refresh")
|
@router.post("/profile/{table_name}/refresh")
|
||||||
|
|
|
||||||
108
app/api/metrics.py
Normal file
108
app/api/metrics.py
Normal file
|
|
@ -0,0 +1,108 @@
|
||||||
|
"""Metrics API endpoints — CRUD for metric definitions stored in DuckDB."""
|
||||||
|
|
||||||
|
from typing import List, Optional
|
||||||
|
|
||||||
|
import duckdb
|
||||||
|
from fastapi import APIRouter, Depends, HTTPException
|
||||||
|
from pydantic import BaseModel
|
||||||
|
|
||||||
|
from app.auth.dependencies import get_current_user, require_admin, _get_db
|
||||||
|
from src.repositories.metrics import MetricRepository
|
||||||
|
|
||||||
|
router = APIRouter(tags=["metrics"])
|
||||||
|
|
||||||
|
|
||||||
|
class MetricCreate(BaseModel):
|
||||||
|
id: str
|
||||||
|
name: str
|
||||||
|
display_name: str
|
||||||
|
category: str
|
||||||
|
sql: str
|
||||||
|
description: Optional[str] = None
|
||||||
|
type: str = "sum"
|
||||||
|
unit: Optional[str] = None
|
||||||
|
grain: str = "monthly"
|
||||||
|
table_name: Optional[str] = None
|
||||||
|
tables: Optional[List[str]] = None
|
||||||
|
expression: Optional[str] = None
|
||||||
|
time_column: Optional[str] = None
|
||||||
|
dimensions: Optional[List[str]] = None
|
||||||
|
filters: Optional[List[str]] = None
|
||||||
|
synonyms: Optional[List[str]] = None
|
||||||
|
notes: Optional[List[str]] = None
|
||||||
|
sql_variants: Optional[dict] = None
|
||||||
|
validation: Optional[dict] = None
|
||||||
|
source: str = "manual"
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/api/metrics")
|
||||||
|
async def list_metrics(
|
||||||
|
category: Optional[str] = None,
|
||||||
|
user: dict = Depends(get_current_user),
|
||||||
|
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
|
||||||
|
):
|
||||||
|
"""List all metric definitions, optionally filtered by category."""
|
||||||
|
repo = MetricRepository(conn)
|
||||||
|
metrics = repo.list(category=category)
|
||||||
|
return {"metrics": metrics, "count": len(metrics)}
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/api/metrics/{metric_id:path}")
|
||||||
|
async def get_metric(
|
||||||
|
metric_id: str,
|
||||||
|
user: dict = Depends(get_current_user),
|
||||||
|
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
|
||||||
|
):
|
||||||
|
"""Get a single metric definition by ID."""
|
||||||
|
repo = MetricRepository(conn)
|
||||||
|
metric = repo.get(metric_id)
|
||||||
|
if metric is None:
|
||||||
|
raise HTTPException(status_code=404, detail=f"Metric '{metric_id}' not found")
|
||||||
|
return metric
|
||||||
|
|
||||||
|
|
||||||
|
@router.post("/api/admin/metrics", status_code=201)
|
||||||
|
async def create_or_update_metric(
|
||||||
|
body: MetricCreate,
|
||||||
|
user: dict = Depends(require_admin),
|
||||||
|
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
|
||||||
|
):
|
||||||
|
"""Create or update a metric definition (admin only)."""
|
||||||
|
repo = MetricRepository(conn)
|
||||||
|
metric = repo.create(
|
||||||
|
id=body.id,
|
||||||
|
name=body.name,
|
||||||
|
display_name=body.display_name,
|
||||||
|
category=body.category,
|
||||||
|
sql=body.sql,
|
||||||
|
description=body.description,
|
||||||
|
type=body.type,
|
||||||
|
unit=body.unit,
|
||||||
|
grain=body.grain,
|
||||||
|
table_name=body.table_name,
|
||||||
|
tables=body.tables,
|
||||||
|
expression=body.expression,
|
||||||
|
time_column=body.time_column,
|
||||||
|
dimensions=body.dimensions,
|
||||||
|
filters=body.filters,
|
||||||
|
synonyms=body.synonyms,
|
||||||
|
notes=body.notes,
|
||||||
|
sql_variants=body.sql_variants,
|
||||||
|
validation=body.validation,
|
||||||
|
source=body.source,
|
||||||
|
)
|
||||||
|
return metric
|
||||||
|
|
||||||
|
|
||||||
|
@router.delete("/api/admin/metrics/{metric_id:path}")
|
||||||
|
async def delete_metric(
|
||||||
|
metric_id: str,
|
||||||
|
user: dict = Depends(require_admin),
|
||||||
|
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
|
||||||
|
):
|
||||||
|
"""Delete a metric definition by ID (admin only)."""
|
||||||
|
repo = MetricRepository(conn)
|
||||||
|
deleted = repo.delete(metric_id)
|
||||||
|
if not deleted:
|
||||||
|
raise HTTPException(status_code=404, detail=f"Metric '{metric_id}' not found")
|
||||||
|
return {"status": "deleted", "id": metric_id}
|
||||||
|
|
@ -80,3 +80,13 @@ def require_role(minimum_role: Role):
|
||||||
)
|
)
|
||||||
return user
|
return user
|
||||||
return _check
|
return _check
|
||||||
|
|
||||||
|
|
||||||
|
async def require_admin(user: dict = Depends(get_current_user)) -> dict:
|
||||||
|
"""Dependency: require user is an admin. Raises 403 otherwise."""
|
||||||
|
if user.get("role") != "admin":
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_403_FORBIDDEN,
|
||||||
|
detail="Admin access required",
|
||||||
|
)
|
||||||
|
return user
|
||||||
|
|
|
||||||
|
|
@ -27,6 +27,7 @@ from app.api.admin import router as admin_router
|
||||||
from app.api.permissions import router as permissions_router
|
from app.api.permissions import router as permissions_router
|
||||||
from app.api.access_requests import router as access_requests_router
|
from app.api.access_requests import router as access_requests_router
|
||||||
from app.api.jira_webhooks import router as jira_webhooks_router
|
from app.api.jira_webhooks import router as jira_webhooks_router
|
||||||
|
from app.api.metrics import router as metrics_router
|
||||||
from app.web.router import router as web_router
|
from app.web.router import router as web_router
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
@ -133,6 +134,7 @@ def create_app() -> FastAPI:
|
||||||
app.include_router(permissions_router)
|
app.include_router(permissions_router)
|
||||||
app.include_router(access_requests_router)
|
app.include_router(access_requests_router)
|
||||||
app.include_router(jira_webhooks_router)
|
app.include_router(jira_webhooks_router)
|
||||||
|
app.include_router(metrics_router)
|
||||||
|
|
||||||
# Web UI router (must be last — has catch-all routes)
|
# Web UI router (must be last — has catch-all routes)
|
||||||
app.include_router(web_router)
|
app.include_router(web_router)
|
||||||
|
|
|
||||||
|
|
@ -233,3 +233,99 @@ class TestUpload:
|
||||||
)
|
)
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
assert resp.json()["user"] == "analyst@acme.com"
|
assert resp.json()["user"] == "analyst@acme.com"
|
||||||
|
|
||||||
|
|
||||||
|
# ---- Metrics API ----
|
||||||
|
|
||||||
|
SAMPLE_METRIC = {
|
||||||
|
"id": "finance/mrr",
|
||||||
|
"name": "mrr",
|
||||||
|
"display_name": "Monthly Recurring Revenue",
|
||||||
|
"category": "finance",
|
||||||
|
"sql": "SELECT SUM(amount) FROM subscriptions WHERE active = true",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
class TestMetricsAPI:
|
||||||
|
def test_list_metrics_empty(self, seeded_client):
|
||||||
|
client, admin_token, _ = seeded_client
|
||||||
|
headers = {"Authorization": f"Bearer {admin_token}"}
|
||||||
|
resp = client.get("/api/metrics", headers=headers)
|
||||||
|
assert resp.status_code == 200
|
||||||
|
data = resp.json()
|
||||||
|
assert data["count"] == 0
|
||||||
|
assert data["metrics"] == []
|
||||||
|
|
||||||
|
def test_create_and_list_metric(self, seeded_client):
|
||||||
|
client, admin_token, _ = seeded_client
|
||||||
|
headers = {"Authorization": f"Bearer {admin_token}"}
|
||||||
|
|
||||||
|
resp = client.post("/api/admin/metrics", json=SAMPLE_METRIC, headers=headers)
|
||||||
|
assert resp.status_code == 201
|
||||||
|
|
||||||
|
resp = client.get("/api/metrics", headers=headers)
|
||||||
|
assert resp.status_code == 200
|
||||||
|
data = resp.json()
|
||||||
|
assert data["count"] == 1
|
||||||
|
assert data["metrics"][0]["id"] == "finance/mrr"
|
||||||
|
|
||||||
|
def test_get_metric_detail(self, seeded_client):
|
||||||
|
client, admin_token, _ = seeded_client
|
||||||
|
headers = {"Authorization": f"Bearer {admin_token}"}
|
||||||
|
|
||||||
|
client.post("/api/admin/metrics", json=SAMPLE_METRIC, headers=headers)
|
||||||
|
|
||||||
|
resp = client.get("/api/metrics/finance/mrr", headers=headers)
|
||||||
|
assert resp.status_code == 200
|
||||||
|
data = resp.json()
|
||||||
|
assert data["id"] == "finance/mrr"
|
||||||
|
assert data["display_name"] == "Monthly Recurring Revenue"
|
||||||
|
assert data["category"] == "finance"
|
||||||
|
|
||||||
|
def test_get_metric_not_found(self, seeded_client):
|
||||||
|
client, admin_token, _ = seeded_client
|
||||||
|
headers = {"Authorization": f"Bearer {admin_token}"}
|
||||||
|
|
||||||
|
resp = client.get("/api/metrics/nonexistent/metric", headers=headers)
|
||||||
|
assert resp.status_code == 404
|
||||||
|
|
||||||
|
def test_delete_metric(self, seeded_client):
|
||||||
|
client, admin_token, _ = seeded_client
|
||||||
|
headers = {"Authorization": f"Bearer {admin_token}"}
|
||||||
|
|
||||||
|
client.post("/api/admin/metrics", json=SAMPLE_METRIC, headers=headers)
|
||||||
|
|
||||||
|
resp = client.delete("/api/admin/metrics/finance/mrr", headers=headers)
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert resp.json()["status"] == "deleted"
|
||||||
|
|
||||||
|
resp = client.get("/api/metrics/finance/mrr", headers=headers)
|
||||||
|
assert resp.status_code == 404
|
||||||
|
|
||||||
|
def test_analyst_cannot_create_metric(self, seeded_client):
|
||||||
|
client, _, analyst_token = seeded_client
|
||||||
|
headers = {"Authorization": f"Bearer {analyst_token}"}
|
||||||
|
|
||||||
|
resp = client.post("/api/admin/metrics", json=SAMPLE_METRIC, headers=headers)
|
||||||
|
assert resp.status_code == 403
|
||||||
|
|
||||||
|
def test_list_metrics_filter_by_category(self, seeded_client):
|
||||||
|
client, admin_token, _ = seeded_client
|
||||||
|
headers = {"Authorization": f"Bearer {admin_token}"}
|
||||||
|
|
||||||
|
finance_metric = {**SAMPLE_METRIC}
|
||||||
|
support_metric = {
|
||||||
|
"id": "support/tickets",
|
||||||
|
"name": "tickets",
|
||||||
|
"display_name": "Open Tickets",
|
||||||
|
"category": "support",
|
||||||
|
"sql": "SELECT COUNT(*) FROM tickets WHERE status = 'open'",
|
||||||
|
}
|
||||||
|
client.post("/api/admin/metrics", json=finance_metric, headers=headers)
|
||||||
|
client.post("/api/admin/metrics", json=support_metric, headers=headers)
|
||||||
|
|
||||||
|
resp = client.get("/api/metrics?category=finance", headers=headers)
|
||||||
|
assert resp.status_code == 200
|
||||||
|
data = resp.json()
|
||||||
|
assert data["count"] == 1
|
||||||
|
assert data["metrics"][0]["category"] == "finance"
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue