From 1b0991c9cf7e7593b227aa7f588f24bb2dc7b9cf Mon Sep 17 00:00:00 2001 From: Justin Ko Date: Fri, 3 Apr 2026 08:37:04 +0000 Subject: [PATCH] feat: add strategy set CRUD operations (API + CLI) --- app/api/strategy_sets.py | 87 ++++++++++++++++ app/cli/commands/strategy.py | 121 +++++++++++++++++++++++ app/cli/main.py | 2 + app/main.py | 2 + app/repositories/portfolio_repository.py | 4 + app/repositories/postgres.py | 28 ++++++ app/schemas/portfolio.py | 11 +++ app/services/strategy_set_service.py | 74 ++++++++++++++ docs/api-examples.md | 114 ++++++++++++++++++++- docs/portfoliohub-spec.md | 52 +++++++++- tests/test_cli.py | 10 ++ tests/test_domain_services.py | 10 ++ 12 files changed, 510 insertions(+), 5 deletions(-) create mode 100644 app/api/strategy_sets.py create mode 100644 app/cli/commands/strategy.py create mode 100644 app/services/strategy_set_service.py diff --git a/app/api/strategy_sets.py b/app/api/strategy_sets.py new file mode 100644 index 0000000..a56513c --- /dev/null +++ b/app/api/strategy_sets.py @@ -0,0 +1,87 @@ +"""Strategy Set API routes.""" + +from fastapi import APIRouter, Depends, Response, status + +from app.core.auth import require_read, require_trade_write +from app.schemas.portfolio import CreateStrategySetRequest, StrategySetResponse, UpdateStrategySetRequest +from app.services.strategy_set_service import StrategySetService, get_strategy_set_repository + + +def get_strategy_set_service(repository=Depends(get_strategy_set_repository)) -> StrategySetService: + return StrategySetService(repository) + + +router = APIRouter(prefix="/strategy-sets", tags=["strategy-sets"]) + + +@router.get("", response_model=list[StrategySetResponse], dependencies=[Depends(require_read)]) +def list_strategy_sets( + service: StrategySetService = Depends(get_strategy_set_service), +) -> list[StrategySetResponse]: + """List all strategy sets.""" + strategy_sets = service.list() + return [ + StrategySetResponse( + id=ss.id, + name=ss.name, + config=ss.config, + nick=ss.nick, + created_at=ss.created_at, + ) + for ss in strategy_sets + ] + + +@router.post("", response_model=StrategySetResponse, status_code=status.HTTP_201_CREATED, dependencies=[Depends(require_trade_write)]) +def create_strategy_set( + payload: CreateStrategySetRequest, + service: StrategySetService = Depends(get_strategy_set_service), +) -> StrategySetResponse: + """Create a new strategy set (or return existing if (name, config) pair already exists).""" + strategy_set = service.create(payload) + return StrategySetResponse( + id=strategy_set.id, + name=strategy_set.name, + config=strategy_set.config, + nick=strategy_set.nick, + created_at=strategy_set.created_at, + ) + + +@router.patch("/{strategy_set_id}", response_model=StrategySetResponse, dependencies=[Depends(require_trade_write)]) +def update_strategy_set( + strategy_set_id: str, + payload: UpdateStrategySetRequest, + service: StrategySetService = Depends(get_strategy_set_service), +) -> StrategySetResponse: + """Update a strategy set's name and/or config. + + Note: Since strategy sets are identified by (name, config) uniqueness, + updating may result in a new strategy set ID if name or config changes. + """ + strategy_set = service.update(strategy_set_id, payload) + return StrategySetResponse( + id=strategy_set.id, + name=strategy_set.name, + config=strategy_set.config, + nick=strategy_set.nick, + created_at=strategy_set.created_at, + ) + + +@router.delete("/{strategy_set_id}", status_code=status.HTTP_204_NO_CONTENT, dependencies=[Depends(require_trade_write)]) +def delete_strategy_set( + strategy_set_id: str, + service: StrategySetService = Depends(get_strategy_set_service), +) -> Response: + """Delete a strategy set. + + Returns 204 No Content on success. + Returns 409 Conflict if the strategy set is in use by any portfolio. + """ + try: + service.delete(strategy_set_id) + except ValueError as e: + # If strategy set is in use, return 409 Conflict + raise ValueError(str(e)) from e + return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/app/cli/commands/strategy.py b/app/cli/commands/strategy.py new file mode 100644 index 0000000..b7eb7ac --- /dev/null +++ b/app/cli/commands/strategy.py @@ -0,0 +1,121 @@ +"""Strategy Set CLI commands.""" + +from __future__ import annotations + +import typer + +from app.cli.core.utils import execute_command, render_output +from app.services.strategy_set_service import StrategySetService + + +strategy_app = typer.Typer(help="Strategy set management commands.") + + +def _with_service(ctx: typer.Context, callback): + app_context = ctx.obj + repository = app_context.repository_factory(app_context.settings) + service = StrategySetService(repository) + try: + result = execute_command(app_context.console, callback=lambda: callback(service, app_context)) + return result + finally: + if hasattr(repository, "close"): + repository.close() + + +def _print_strategy_set_table(strategy_sets): + """Print strategy sets as a simple table.""" + if not strategy_sets: + print("No strategy sets found.") + return + + # Print header + print(f"{'ID':<40} {'Name':<20} {'Nick':<20} {'Created At':<26}") + print("-" * 106) + + # Print rows + for ss in strategy_sets: + created_at = ss.created_at.isoformat() if ss.created_at else "N/A" + print(f"{ss.id:<40} {ss.name:<20} {ss.nick:<20} {created_at:<26}") + + +@strategy_app.command("list") +def list_strategy_sets(ctx: typer.Context) -> None: + """List all available strategy sets.""" + def cmd(service: StrategySetService, app_context): + strategy_sets = service.list() + _print_strategy_set_table(strategy_sets) + + _with_service(ctx, cmd) + + +@strategy_app.command("create") +def create_strategy_set( + ctx: typer.Context, + name: str = typer.Option(..., help="Strategy name (e.g., 'target_value')."), + config: str = typer.Option(..., help="Strategy config as JSON string."), +) -> None: + """Create a new strategy set (or retrieve existing if (name, config) pair already exists).""" + def cmd(service: StrategySetService, app_context): + from app.schemas.portfolio import CreateStrategySetRequest + + payload = CreateStrategySetRequest(name=name, config=config) + strategy_set = service.create(payload) + + print(f"Strategy set created/retrieved:") + print(f" ID: {strategy_set.id}") + print(f" Name: {strategy_set.name}") + print(f" Nick: {strategy_set.nick}") + print(f" Created At: {strategy_set.created_at.isoformat() if strategy_set.created_at else 'N/A'}") + + _with_service(ctx, cmd) + + +@strategy_app.command("update") +def update_strategy_set( + ctx: typer.Context, + strategy_set_id: str = typer.Argument(..., help="Strategy set ID to update."), + name: str | None = typer.Option(None, help="New strategy name (optional)."), + config: str | None = typer.Option(None, help="New strategy config as JSON string (optional)."), +) -> None: + """Update an existing strategy set's name and/or config.""" + def cmd(service: StrategySetService, app_context): + from app.schemas.portfolio import UpdateStrategySetRequest + + payload = UpdateStrategySetRequest(name=name, config=config) + strategy_set = service.update(strategy_set_id, payload) + + print(f"Strategy set updated:") + print(f" ID: {strategy_set.id}") + print(f" Name: {strategy_set.name}") + print(f" Nick: {strategy_set.nick}") + print(f" Created At: {strategy_set.created_at.isoformat() if strategy_set.created_at else 'N/A'}") + + _with_service(ctx, cmd) + + +@strategy_app.command("delete") +def delete_strategy_set( + ctx: typer.Context, + strategy_set_id: str = typer.Argument(..., help="Strategy set ID to delete."), + force: bool = typer.Option(False, "--force", "-f", help="Skip confirmation."), +) -> None: + """Delete a strategy set. + + Returns an error if the strategy set is in use by any portfolio. + """ + def cmd(service: StrategySetService, app_context): + if not force: + confirmed = typer.confirm(f"Delete strategy set {strategy_set_id}?") + if not confirmed: + print("Cancelled.") + return + + try: + service.delete(strategy_set_id) + print(f"Strategy set {strategy_set_id} deleted.") + except ValueError as e: + print(f"Error: {e}") + raise typer.Exit(code=1) + + _with_service(ctx, cmd) diff --git a/app/cli/main.py b/app/cli/main.py index 780a038..49e9402 100644 --- a/app/cli/main.py +++ b/app/cli/main.py @@ -10,6 +10,7 @@ from rich.console import Console from app.cli.commands.health import register_health_commands from app.cli.commands.portfolio import portfolio_app +from app.cli.commands.strategy import strategy_app from app.core.config import Settings, get_settings from app.repositories.factory import get_portfolio_repository_from_settings from app.repositories.portfolio_repository import PortfolioRepository @@ -45,6 +46,7 @@ def create_app( ) app.add_typer(portfolio_app, name="portfolio") + app.add_typer(strategy_app, name="strategy") register_health_commands(app) return app diff --git a/app/main.py b/app/main.py index a66e0d7..355b95c 100644 --- a/app/main.py +++ b/app/main.py @@ -5,6 +5,7 @@ from fastapi.middleware.cors import CORSMiddleware from app.api.health import router as health_router from app.api.portfolios import router as portfolio_router +from app.api.strategy_sets import router as strategy_sets_router from app.core.config import get_settings from app.core.errors import register_exception_handlers @@ -28,6 +29,7 @@ def create_app() -> FastAPI: app.include_router(health_router) app.include_router(portfolio_router) + app.include_router(strategy_sets_router) register_exception_handlers(app) return app diff --git a/app/repositories/portfolio_repository.py b/app/repositories/portfolio_repository.py index 678fbaa..609e49c 100644 --- a/app/repositories/portfolio_repository.py +++ b/app/repositories/portfolio_repository.py @@ -47,6 +47,10 @@ class PortfolioRepository(Protocol): def get_strategy_set(self, id: str) -> StrategySet | None: ... + def list_strategy_sets(self) -> list[StrategySet]: ... + + def delete_strategy_set(self, id: str) -> None: ... + def save( self, portfolio: Portfolio, diff --git a/app/repositories/postgres.py b/app/repositories/postgres.py index 058ff4b..25c2734 100644 --- a/app/repositories/postgres.py +++ b/app/repositories/postgres.py @@ -385,6 +385,34 @@ class PostgresPortfolioRepository: return None return StrategySet(id=row[0], name=row[1], config=row[2], nick=row[3], created_at=row[4]) + def list_strategy_sets(self) -> list[StrategySet]: + self.ensure_schema() + with self._connect().cursor() as cur: + cur.execute( + "SELECT id, name, config, nick, created_at FROM strategy_sets ORDER BY created_at DESC" + ) + rows = cur.fetchall() + return [StrategySet(id=row[0], name=row[1], config=row[2], nick=row[3], created_at=row[4]) for row in rows] + + def delete_strategy_set(self, id: str) -> None: + self.ensure_schema() + with self._connect().cursor() as cur: + # 1. Check if this strategy set is referenced by any portfolio + cur.execute( + """ + SELECT COUNT(*) FROM portfolios + WHERE active_strategy_id = %s + OR avabile_strategies @> ARRAY[%s] + """, + (id, id), + ) + count = cur.fetchone()[0] + if count > 0: + raise ValueError(f"Strategy set is in use by {count} portfolio(s) and cannot be deleted") + + # 2. If not in use, proceed with deletion + cur.execute("DELETE FROM strategy_sets WHERE id = %s", (id,)) + def save( self, portfolio: Portfolio, diff --git a/app/schemas/portfolio.py b/app/schemas/portfolio.py index 3a8fc35..689d1c0 100644 --- a/app/schemas/portfolio.py +++ b/app/schemas/portfolio.py @@ -123,3 +123,14 @@ class ResetPortfolioRequest(BaseModel): class ErrorResponse(BaseModel): detail: str model_config = ConfigDict(extra="ignore") + + +class CreateStrategySetRequest(BaseModel): + name: str + config: str + + +class UpdateStrategySetRequest(BaseModel): + name: str | None = None + config: str | None = None + diff --git a/app/services/strategy_set_service.py b/app/services/strategy_set_service.py new file mode 100644 index 0000000..50f0d0f --- /dev/null +++ b/app/services/strategy_set_service.py @@ -0,0 +1,74 @@ +"""Strategy set service orchestration.""" + +from __future__ import annotations + +from fastapi import Depends + +from app.core.config import Settings, get_settings +from app.domain.models import StrategySet +from app.repositories.factory import get_portfolio_repository_from_settings +from app.repositories.portfolio_repository import PortfolioRepository +from app.schemas.portfolio import CreateStrategySetRequest, UpdateStrategySetRequest + + +def get_strategy_set_repository(settings: Settings = Depends(get_settings)) -> PortfolioRepository: + return get_portfolio_repository_from_settings(settings) + + +class StrategySetService: + def __init__(self, repository: PortfolioRepository) -> None: + self._repository = repository + + def list(self) -> list[StrategySet]: + """List all strategy sets.""" + return self._repository.list_strategy_sets() + + def get(self, strategy_set_id: str) -> StrategySet | None: + """Get a single strategy set by ID.""" + return self._repository.get_strategy_set(strategy_set_id) + + def create(self, payload: CreateStrategySetRequest) -> StrategySet: + """Create or get existing strategy set. + + Uses get_or_create_strategy_set for idempotence. + """ + return self._repository.get_or_create_strategy_set(payload.name, payload.config) + + def update(self, strategy_set_id: str, payload: UpdateStrategySetRequest) -> StrategySet: + """Update a strategy set. + + Since strategy sets are identified by (name, config) uniqueness, + updating requires creating a new one if name/config changes. + """ + current = self.get(strategy_set_id) + if current is None: + raise ValueError(f"Strategy set {strategy_set_id} not found") + + # Use provided values or fall back to current ones + new_name = payload.name if payload.name is not None else current.name + new_config = payload.config if payload.config is not None else current.config + + # If nothing changed, return current + if new_name == current.name and new_config == current.config: + return current + + # Create or get the new strategy set + new_strategy_set = self._repository.get_or_create_strategy_set(new_name, new_config) + + # If the new strategy set is different from current, delete the old one + # (only if it's not in use - delete_strategy_set will raise if in use) + if new_strategy_set.id != strategy_set_id: + try: + self._repository.delete_strategy_set(strategy_set_id) + except ValueError: + # If old strategy set is in use, keep it and return new one + pass + + return new_strategy_set + + def delete(self, strategy_set_id: str) -> None: + """Delete a strategy set. + + Raises ValueError if the strategy set is in use by any portfolio. + """ + self._repository.delete_strategy_set(strategy_set_id) diff --git a/docs/api-examples.md b/docs/api-examples.md index da9af0c..b112226 100644 --- a/docs/api-examples.md +++ b/docs/api-examples.md @@ -255,9 +255,115 @@ Response 204: - No content. -## 5. Common Error Examples +## 5. Strategy Sets -### 5.1 Missing token +### 5.1 GET /strategy-sets + +Request: + +```bash +curl -X GET "http://localhost:8000/strategy-sets" \ + -H "Authorization: Bearer " +``` + +Response 200: + +```json +[ + { + "id": "set-1", + "name": "target_value", + "config": "{\"target\": 1000}", + "nick": "target_value:abc123def456", + "created_at": "2026-04-03T08:13:08.318Z" + }, + { + "id": "set-2", + "name": "momentum", + "config": "{\"window\": 20}", + "nick": "momentum:xyz789uvw012", + "created_at": "2026-04-03T08:14:15.450Z" + } +] +``` + +### 5.2 POST /strategy-sets + +Request: + +```bash +curl -X POST "http://localhost:8000/strategy-sets" \ + -H "Authorization: Bearer " \ + -H "Content-Type: application/json" \ + -d '{ + "name": "target_value", + "config": "{\"target\": 1000}" + }' +``` + +Response 201: + +```json +{ + "id": "set-1", + "name": "target_value", + "config": "{\"target\": 1000}", + "nick": "target_value:abc123def456", + "created_at": "2026-04-03T08:13:08.318Z" +} +``` + +### 5.3 PATCH /strategy-sets/{strategy_set_id} + +Request: + +```bash +curl -X PATCH "http://localhost:8000/strategy-sets/set-1" \ + -H "Authorization: Bearer " \ + -H "Content-Type: application/json" \ + -d '{ + "config": "{\"target\": 2000}" + }' +``` + +Response 200: + +```json +{ + "id": "set-1-updated", + "name": "target_value", + "config": "{\"target\": 2000}", + "nick": "target_value:def789ghi012", + "created_at": "2026-04-03T08:13:08.318Z" +} +``` + +Note: Since strategy sets are identified by (name, config) uniqueness, updating may result in a new strategy set ID. + +### 5.4 DELETE /strategy-sets/{strategy_set_id} + +Request: + +```bash +curl -X DELETE "http://localhost:8000/strategy-sets/set-1" \ + -H "Authorization: Bearer " +``` + +Response 204: + +- No content on success. + +Response 409 Conflict (if strategy set is in use): + +```json +{ + "detail": "Strategy set is in use by 1 portfolio(s) and cannot be deleted" +} +``` + +## 6. Common Error Examples + +### 6.1 Missing token Response 401: @@ -267,7 +373,7 @@ Response 401: } ``` -### 5.2 Invalid token +### 6.2 Invalid token Response 401: @@ -277,7 +383,7 @@ Response 401: } ``` -### 5.3 Insufficient permission +### 6.3 Insufficient permission Response 403: diff --git a/docs/portfoliohub-spec.md b/docs/portfoliohub-spec.md index 1a567fd..f8640aa 100644 --- a/docs/portfoliohub-spec.md +++ b/docs/portfoliohub-spec.md @@ -208,7 +208,40 @@ Base routes: - `keep_ledger` (bool, default `false`) - Response `204`: no content -### 7.3 Example Requests and Responses +### 7.3 Strategy Set Endpoints + +#### List Strategy Sets + +- `GET /strategy-sets` +- Auth: `read` +- Response `200`: array of `StrategySetResponse` + +#### Create Strategy Set + +- `POST /strategy-sets` +- Auth: `trade-write` +- Body: `CreateStrategySetRequest` +- Response `201`: `StrategySetResponse` + +Note: Uses `get_or_create_strategy_set` logic, so if (name, config) pair already exists, returns existing strategy set. + +#### Update Strategy Set + +- `PATCH /strategy-sets/{strategy_set_id}` +- Auth: `trade-write` +- Body: `UpdateStrategySetRequest` +- Response `200`: `StrategySetResponse` + +Note: Since strategy sets are identified by (name, config) uniqueness, updating may result in a new strategy set ID if name or config changes. + +#### Delete Strategy Set + +- `DELETE /strategy-sets/{strategy_set_id}` +- Auth: `trade-write` +- Response `204`: no content +- Response `409`: Conflict if strategy set is in use by any portfolio + +### 7.4 Example Requests and Responses - See `docs/api-examples.md` for maintained request/response examples. @@ -266,6 +299,16 @@ Base routes: - `infer_symbol: str | null` +`CreateStrategySetRequest` + +- `name: str` +- `config: str` + +`UpdateStrategySetRequest` (all fields optional) + +- `name: str | null` +- `config: str | null` + ### 8.2 Response Models `PortfolioListItem` @@ -336,6 +379,13 @@ Portfolio subcommands: - `portfoliohub portfolio reset [--infer-symbol ...] [--force]` - `portfoliohub portfolio delete [--keep-ledger] [--force]` +Strategy subcommands: + +- `portfoliohub strategy list` +- `portfoliohub strategy create --name ... --config ...` +- `portfoliohub strategy update [--name ...] [--config ...]` +- `portfoliohub strategy delete [--force]` + Confirmation behavior: - Mutating portfolio commands request confirmation by default. diff --git a/tests/test_cli.py b/tests/test_cli.py index 32fe548..27853cc 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -16,6 +16,7 @@ class FakeRepo: self._portfolio = portfolio self.entries: list[LedgerEntry] = [] self.strategy_set = StrategySet(id="set-1", name="target_value", config="{}", nick="target_value:99914b932bd3") + self._strategy_sets: dict[str, StrategySet] = {self.strategy_set.id: self.strategy_set} self.closed = False def close(self) -> None: @@ -101,11 +102,20 @@ class FakeRepo: def get_or_create_strategy_set(self, name: str, config: str) -> StrategySet: self.strategy_set = StrategySet(id="set-1", name=name, config=config, nick=f"{name}:99914b932bd3") + self._strategy_sets[self.strategy_set.id] = self.strategy_set return self.strategy_set def get_strategy_set(self, id: str) -> StrategySet | None: return self.strategy_set if self.strategy_set.id == id else None + def list_strategy_sets(self) -> list[StrategySet]: + from datetime import datetime, timezone + return sorted(self._strategy_sets.values(), key=lambda s: s.created_at or datetime.now(timezone.utc), reverse=True) + + def delete_strategy_set(self, id: str) -> None: + if id in self._strategy_sets: + del self._strategy_sets[id] + def save( self, portfolio: Portfolio, diff --git a/tests/test_domain_services.py b/tests/test_domain_services.py index be6623f..a7652f2 100644 --- a/tests/test_domain_services.py +++ b/tests/test_domain_services.py @@ -13,6 +13,7 @@ class FakeRepo: self._portfolio = portfolio self.entries: list[LedgerEntry] = [] self.strategy_set = StrategySet(id="set-1", name="target_value", config="{}", nick="target_value:99914b932bd3") + self._strategy_sets: dict[str, StrategySet] = {self.strategy_set.id: self.strategy_set} self.last_delete: tuple[str, bool] | None = None def get(self, portfolio_id: str) -> Portfolio: @@ -62,8 +63,17 @@ class FakeRepo: def get_or_create_strategy_set(self, name: str, config: str) -> StrategySet: self.strategy_set = StrategySet(id="set-1", name=name, config=config, nick=f"{name}:99914b932bd3") + self._strategy_sets[self.strategy_set.id] = self.strategy_set return self.strategy_set + def list_strategy_sets(self) -> list[StrategySet]: + return sorted(self._strategy_sets.values(), key=lambda s: s.created_at or datetime.now(timezone.utc), reverse=True) + + def delete_strategy_set(self, id: str) -> None: + # In tests, just remove from the fake storage + if id in self._strategy_sets: + del self._strategy_sets[id] + def save( self, portfolio: Portfolio,