refactor: fix code review issues across routes and services

- Extract shared route_utils.py (validate_symbol, safe decorator)
  removing duplication from 6 route files
- Extract shared obb_utils.py (to_list, extract_single, safe_last)
  removing duplication from calendar_service and market_service
- Fix _to_list dict mutation during iteration (use comprehension)
- Fix double vars() call and live __dict__ mutation risk
- Fix route ordering: /etf/search and /crypto/search now registered
  before /{symbol} path params to prevent shadowing
- Add date format validation (YYYY-MM-DD pattern) on calendar routes
- Use timezone-aware datetime.now(tz=timezone.utc) in all services
- Add explicit type annotation for asyncio.gather results
This commit is contained in:
Yaojia Wang
2026-03-09 10:56:21 +01:00
parent 507194397e
commit 003c1d6ffc
12 changed files with 271 additions and 428 deletions

View File

@@ -1,9 +1,4 @@
import functools
import logging
from collections.abc import Awaitable, Callable
from typing import ParamSpec, TypeVar
from fastapi import APIRouter, HTTPException, Path, Query
from fastapi import APIRouter, Path, Query
from mappers import (
discover_items_from_list,
@@ -12,7 +7,6 @@ from mappers import (
quote_from_dict,
)
from models import (
SYMBOL_PATTERN,
ApiResponse,
FinancialsResponse,
HistoricalBar,
@@ -21,87 +15,60 @@ from models import (
PortfolioResponse,
SummaryResponse,
)
from route_utils import safe, validate_symbol
import openbb_service
import analysis_service
logger = logging.getLogger(__name__)
router = APIRouter(prefix="/api/v1")
P = ParamSpec("P")
R = TypeVar("R")
def _validate_symbol(symbol: str) -> str:
if not SYMBOL_PATTERN.match(symbol):
raise HTTPException(status_code=400, detail="Invalid symbol format")
return symbol.upper()
def _safe(fn: Callable[P, Awaitable[R]]) -> Callable[P, Awaitable[R]]:
"""Decorator to catch OpenBB errors and return 502."""
@functools.wraps(fn)
async def wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
try:
return await fn(*args, **kwargs)
except HTTPException:
raise
except Exception:
logger.exception("Upstream data error")
raise HTTPException(
status_code=502,
detail="Data provider error. Check server logs.",
)
return wrapper # type: ignore[return-value]
# --- Stock Data ---
@router.get("/stock/{symbol}/quote", response_model=ApiResponse)
@_safe
@safe
async def stock_quote(symbol: str = Path(..., min_length=1, max_length=20)):
"""Get current quote for a stock."""
symbol = _validate_symbol(symbol)
symbol = validate_symbol(symbol)
data = await openbb_service.get_quote(symbol)
return ApiResponse(data=quote_from_dict(symbol, data).model_dump())
@router.get("/stock/{symbol}/profile", response_model=ApiResponse)
@_safe
@safe
async def stock_profile(symbol: str = Path(..., min_length=1, max_length=20)):
"""Get company profile."""
symbol = _validate_symbol(symbol)
symbol = validate_symbol(symbol)
data = await openbb_service.get_profile(symbol)
return ApiResponse(data=profile_from_dict(symbol, data).model_dump())
@router.get("/stock/{symbol}/metrics", response_model=ApiResponse)
@_safe
@safe
async def stock_metrics(symbol: str = Path(..., min_length=1, max_length=20)):
"""Get key financial metrics (PE, PB, ROE, etc.)."""
symbol = _validate_symbol(symbol)
symbol = validate_symbol(symbol)
data = await openbb_service.get_metrics(symbol)
return ApiResponse(data=metrics_from_dict(symbol, data).model_dump())
@router.get("/stock/{symbol}/financials", response_model=ApiResponse)
@_safe
@safe
async def stock_financials(symbol: str = Path(..., min_length=1, max_length=20)):
"""Get income statement, balance sheet, and cash flow."""
symbol = _validate_symbol(symbol)
symbol = validate_symbol(symbol)
data = await openbb_service.get_financials(symbol)
return ApiResponse(data=FinancialsResponse(**data).model_dump())
@router.get("/stock/{symbol}/historical", response_model=ApiResponse)
@_safe
@safe
async def stock_historical(
symbol: str = Path(..., min_length=1, max_length=20),
days: int = Query(default=365, ge=1, le=3650),
):
"""Get historical price data."""
symbol = _validate_symbol(symbol)
symbol = validate_symbol(symbol)
data = await openbb_service.get_historical(symbol, days=days)
bars = [
HistoricalBar(
@@ -118,10 +85,10 @@ async def stock_historical(
@router.get("/stock/{symbol}/news", response_model=ApiResponse)
@_safe
@safe
async def stock_news(symbol: str = Path(..., min_length=1, max_length=20)):
"""Get recent company news."""
symbol = _validate_symbol(symbol)
symbol = validate_symbol(symbol)
data = await openbb_service.get_news(symbol)
news = [
NewsItem(
@@ -136,10 +103,10 @@ async def stock_news(symbol: str = Path(..., min_length=1, max_length=20)):
@router.get("/stock/{symbol}/summary", response_model=ApiResponse)
@_safe
@safe
async def stock_summary(symbol: str = Path(..., min_length=1, max_length=20)):
"""Get aggregated stock data: quote + profile + metrics + financials."""
symbol = _validate_symbol(symbol)
symbol = validate_symbol(symbol)
data = await openbb_service.get_summary(symbol)
summary = SummaryResponse(
quote=quote_from_dict(symbol, data.get("quote", {})),
@@ -156,7 +123,7 @@ async def stock_summary(symbol: str = Path(..., min_length=1, max_length=20)):
@router.post("/portfolio/analyze", response_model=ApiResponse)
@_safe
@safe
async def portfolio_analyze(request: PortfolioRequest):
"""Analyze portfolio holdings with rule-based engine."""
result: PortfolioResponse = await analysis_service.analyze_portfolio(
@@ -169,7 +136,7 @@ async def portfolio_analyze(request: PortfolioRequest):
@router.get("/discover/gainers", response_model=ApiResponse)
@_safe
@safe
async def discover_gainers():
"""Get top gainers (US market)."""
data = await openbb_service.get_gainers()
@@ -177,7 +144,7 @@ async def discover_gainers():
@router.get("/discover/losers", response_model=ApiResponse)
@_safe
@safe
async def discover_losers():
"""Get top losers (US market)."""
data = await openbb_service.get_losers()
@@ -185,7 +152,7 @@ async def discover_losers():
@router.get("/discover/active", response_model=ApiResponse)
@_safe
@safe
async def discover_active():
"""Get most active stocks (US market)."""
data = await openbb_service.get_active()
@@ -193,7 +160,7 @@ async def discover_active():
@router.get("/discover/undervalued", response_model=ApiResponse)
@_safe
@safe
async def discover_undervalued():
"""Get undervalued large cap stocks."""
data = await openbb_service.get_undervalued()
@@ -201,7 +168,7 @@ async def discover_undervalued():
@router.get("/discover/growth", response_model=ApiResponse)
@_safe
@safe
async def discover_growth():
"""Get growth tech stocks."""
data = await openbb_service.get_growth()