diff --git a/.proofline/lines/line-0004/evidence/implementation-progress.md b/.proofline/lines/line-0004/evidence/implementation-progress.md new file mode 100644 index 0000000..cdd429e --- /dev/null +++ b/.proofline/lines/line-0004/evidence/implementation-progress.md @@ -0,0 +1,40 @@ +## 구현 진행 상황 + +- **대상:** 포트폴리오 추가 시 연결 데몬을 설정해도 지정되지 않음 (GitHub Issue #27) +- **브랜치:** `line/line-0004-implementation` +- **기준:** `main` (`bb3bb5d3f0ce57358f7385084fba1343f7ff06e1`) +- **상태:** 로컬 구현·검증 완료 (미커밋) +- **최종 업데이트:** 2026-08-15 15:20 KST + +### 구현 계획 및 작업 목록 + +- [x] 공유 daemon validation 도메인 오류 정의 + - `DaemonNotFoundError`(HTTP 404)·`DaemonInactiveError`(HTTP 409)를 `app/domain/errors.py`에 추가해 전 계층에서 같은 오류 계약을 사용한다. +- [x] `create_portfolio` 저장소 계약·구현에 `managed_by_daemon_id` 전달 + - 프로토콜 시그니처(`portfolio_repository.py`)와 PostgreSQL 구현(`postgres.py`)에 `managed_by_daemon_id` 파라미터를 추가하고 INSERT 컬럼까지 반영해 생성 persistence 경계에 누락을 제거한다. +- [x] 생성·수정 트랜잭션 안의 원자적 daemon 검증 경계 + - 기존 `_lock_active_strategy` 패턴처럼 daemon row를 `FOR SHARE` lock하고 존재·소유권·active를 같은 트랜잭션에서 검증해, portfolio write와 하나의 원자적 판정으로 묶어 TOCTOU를 차단한다. +- [x] `POST /v1/portfolios` 생성 handler에서 비-null daemon 전달·오류 매핑 + - UUID 형식이 아니면 422, 미존재·foreign(타인 소유)이면 404, inactive면 409로 매핑하고 admin의 `user_id` override 생성은 최종 target owner 기준으로 검증한다. +- [x] `PATCH /v1/portfolios/{portfolio_id}` 비-null 지정·재지정에도 동일 검증 적용 + - 기존 비-null PATCH 지정·재지정 경로에 생성과 동일한 daemon 검증·오류 계약을 적용하되, 포트폴리오 소유권·disabled 보호·service/client 거부 등 기존 mutation 계약은 유지한다. +- [x] 단위/API 테스트 추가 + - 성공 생성 응답의 `managed_by_daemon_id` 일치, 미선택 생성 시 null 유지, malformed 422, 미존재·foreign 404, inactive 409, admin target-owner 허용/거부, PATCH 지정·재지정을 검증한다. +- [x] 실제 PostgreSQL 통합 테스트 추가 + - `POST → GET` persistence 보존, 검증 실패 시 portfolio 무변경, assignment와 daemon 비활성화가 경합해도 inactive daemon에 새 assignment가 확정되지 않는 동시성 race를 검증한다. +- [x] 문서 갱신 + - `docs/interface-contract.md`, `docs/data-model.md`에 daemon 배정 검증 계약·오류 코드·원자적 판정 경계를 반영한다. +- [x] 회귀·정적 검사 및 진행 상황 업데이트 + - 기존 portfolio 생성·수정의 인증·소유권·strategy·asset·disabled 계약 회귀, 전체 관련 테스트, compile·`git diff --check`·Ruff 검사를 실행하고 진행 상황 파일을 갱신한다. + +### 진행 기록 + +- 2026-08-15 13:00 KST — Issue #27의 승인된 Discovery(`dcy-0004`) 범위를 기준으로 구현 계획을 확정했다. 근본 원인(`v1_portfolios.py` 생성 handler가 `repo.create_portfolio(...)`에 `managed_by_daemon_id`를 전달하지 않음)과 승인된 결정(존재·active·동일 소유자, 404/409/422 오류 계약, 원자적 판정 경계)을 반영해 작업 목록을 구성했다. +- 2026-08-15 15:10 KST — `app/domain/errors.py`에 `DaemonNotFoundError`·`DaemonInactiveError`를 추가하고, `create_portfolio` 프로토콜·구현과 INSERT 컬럼에 `managed_by_daemon_id`를 반영했다. `postgres.py`에 `_lock_active_daemon`(daemon row `FOR SHARE` + 존재·동일 소유자·active 검증)을 추가해 생성·수정 트랜잭션 안에서 원자적 판정 경계를 구성했다. +- 2026-08-15 15:10 KST — `POST /v1/portfolios`와 `PATCH /v1/portfolios/{id}` handler에 UUID 형식(422)·미존재/foreign(404)·inactive(409) 검증과 오류 매핑을 적용했다. admin target-user 생성은 최종 target owner 기준으로 daemon 소유권을 검증한다. +- 2026-08-15 15:12 KST — 기존 `test_v1_portfolios_crud.py`의 `_FakeRepo.create_portfolio` 시그니처를 새 파라미터에 맞춰 보완했다. +- 2026-08-15 15:15 KST — 전용 API 테스트 `tests/test_v1_portfolios_daemon_assignment.py`(13개)를 추가했다. POST 성공·null 유지·malformed 422·미존재/foreign 404·inactive 409·admin target-owner 허용/거부, PATCH 지정·재지정·malformed·foreign·inactive를 검증해 통과했다. +- 2026-08-15 15:18 KST — 실제 PostgreSQL 통합 테스트 `tests/test_v1_portfolios_daemon_postgres_integration.py`(4개)를 추가했다. HTTP `POST → GET` persistence, 검증 실패 시 portfolio 무변경, assignment vs deactivation race의 허용 outcome, PATCH 재배정 persistence를 실제 DB에서 검증했다. +- 2026-08-15 15:20 KST — 회귀 검증: 전체 단위 스위트 `625 passed, 11 skipped`, 실제 PostgreSQL 통합(daemon 4 + strategy 4) `8 passed`. Python compile, `git diff --check`, 변경 핵심 파일 Ruff 검사 통과(Ruff 이슈 44건은 기준 커밋과 동일한 기존 항목이며 신규 도입 없음). + +> 각 작업이 실제로 완료되고 검증될 때마다 이 댓글의 체크박스와 진행 기록을 갱신한다. 체크 표시는 구현 완료 또는 해당 검증의 실제 통과를 의미하며, 배포·릴리스 완료를 의미하지 않는다. 현재 변경은 기준 HEAD 위 미커밋 상태로 보존되어 있으며 커밋·push·PR·main 반영·배포는 수행하지 않았다. \ No newline at end of file diff --git a/app/api/v1_portfolios.py b/app/api/v1_portfolios.py index bb263c8..5cd7a26 100644 --- a/app/api/v1_portfolios.py +++ b/app/api/v1_portfolios.py @@ -15,6 +15,7 @@ from datetime import date, datetime, timedelta, timezone from fastapi import APIRouter, Depends, HTTPException, Query, Response, status from app.core.auth import AuthContext, Permission, require_read, require_trade_write +from app.domain.errors import DaemonInactiveError, DaemonNotFoundError from app.domain.models import Portfolio, PortfolioComment from app.domain.pnl import calculate_current_wac, calculate_realized_pnl from app.repositories.factory import get_portfolio_repository_from_settings @@ -67,6 +68,18 @@ def _require_write_token(auth: AuthContext) -> None: ) +def _require_valid_daemon_uuid(daemon_id: str | None) -> None: + if daemon_id is None: + return + try: + uuid.UUID(daemon_id) + except (ValueError, AttributeError, TypeError): + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"managed_by_daemon_id는 UUID 형식이어야 합니다: {daemon_id}", + ) + + def _portfolio_to_item( portfolio: Portfolio, *, @@ -167,6 +180,8 @@ def create_portfolio_v1( if strategy_set.status != "active": raise StrategyInactiveError(f"strategy is inactive: {body.strategy_id}") + _require_valid_daemon_uuid(body.managed_by_daemon_id) + portfolio = repo.create_portfolio( str(uuid.uuid4()), owner_user_id=owner_user_id, @@ -180,6 +195,7 @@ def create_portfolio_v1( profit_buffer=body.profit_buffer, active_strategy_id=strategy_set.id, avabile_strategies=[strategy_set.id], + managed_by_daemon_id=body.managed_by_daemon_id, ) except HTTPException: raise @@ -187,6 +203,10 @@ def create_portfolio_v1( raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc except StrategyInactiveError as exc: raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc + except DaemonNotFoundError as exc: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc + except DaemonInactiveError as exc: + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc except ValueError as exc: raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(exc)) from exc finally: @@ -230,6 +250,7 @@ def update_portfolio_v1( raise StrategyInactiveError(f"strategy is inactive: {strategy_id}") fields["active_strategy_id"] = strategy_set.id fields["avabile_strategies"] = [strategy_set.id] + _require_valid_daemon_uuid(fields.get("managed_by_daemon_id")) repo.update_portfolio(portfolio_id, **fields) portfolio = repo.get(portfolio_id) @@ -239,6 +260,10 @@ def update_portfolio_v1( raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc except StrategyInactiveError as exc: raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc + except DaemonNotFoundError as exc: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc + except DaemonInactiveError as exc: + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc except ValueError as exc: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc finally: diff --git a/app/domain/errors.py b/app/domain/errors.py index 849c2fb..332f5b8 100644 --- a/app/domain/errors.py +++ b/app/domain/errors.py @@ -1,4 +1,4 @@ -"""Shared strategy-catalog domain errors used across API, service, and persistence layers.""" +"""Shared domain errors used across API, service, and persistence layers.""" class StrategyNotFoundError(ValueError): @@ -7,3 +7,11 @@ class StrategyNotFoundError(ValueError): class StrategyInactiveError(ValueError): """Raised when a new portfolio selection targets an inactive strategy.""" + + +class DaemonNotFoundError(ValueError): + """Raised when a referenced daemon ID does not exist or is not owned by the portfolio owner.""" + + +class DaemonInactiveError(ValueError): + """Raised when a new portfolio assignment targets an inactive daemon.""" diff --git a/app/repositories/portfolio_repository.py b/app/repositories/portfolio_repository.py index 72661df..bbba237 100644 --- a/app/repositories/portfolio_repository.py +++ b/app/repositories/portfolio_repository.py @@ -162,6 +162,7 @@ class PortfolioRepository(Protocol): profit_buffer: float, active_strategy_id: str | None, avabile_strategies: list[str] | None, + managed_by_daemon_id: str | None = None, ) -> Portfolio: ... def get_asset_by_id(self, asset_id: str) -> Asset | None: ... diff --git a/app/repositories/postgres.py b/app/repositories/postgres.py index b9755b3..3470ef5 100644 --- a/app/repositories/postgres.py +++ b/app/repositories/postgres.py @@ -13,7 +13,7 @@ from typing import Any import psycopg from psycopg import sql -from app.domain.errors import StrategyInactiveError, StrategyNotFoundError +from app.domain.errors import DaemonInactiveError, DaemonNotFoundError, StrategyInactiveError, StrategyNotFoundError from app.domain.models import Asset, AuthToken, BrokerCredential, Daemon, LedgerEntry, Portfolio, PortfolioComment, PortfolioTag, StrategySet, Tag, TokenPrincipal, User logger = logging.getLogger(__name__) @@ -159,6 +159,26 @@ class PostgresPortfolioRepository: if row[0] != "active": raise StrategyInactiveError(f"strategy is inactive: {strategy_id}") + @staticmethod + def _lock_active_daemon(cur: psycopg.Cursor, daemon_id: str, owner_user_id: str) -> None: + """Serialize new portfolio assignments with daemon deactivation/owner transfer. + + Locks the daemon row FOR SHARE inside the portfolio write transaction and + enforces existence, same-owner, and active status so validation and write + form a single atomic decision boundary. + """ + cur.execute( + "SELECT owner_user_id, is_active FROM daemons WHERE id = %s FOR SHARE", + (daemon_id,), + ) + row = cur.fetchone() + if row is None: + raise DaemonNotFoundError(f"daemon not found: {daemon_id}") + if row[0] != owner_user_id: + raise DaemonNotFoundError(f"daemon not found: {daemon_id}") + if not row[1]: + raise DaemonInactiveError(f"daemon is inactive: {daemon_id}") + def ensure_schema(self) -> None: conn = self._connect() with conn.cursor() as cur: @@ -1128,12 +1148,15 @@ class PostgresPortfolioRepository: profit_buffer: float, active_strategy_id: str | None, avabile_strategies: list[str] | None, + managed_by_daemon_id: str | None = None, ) -> Portfolio: self.ensure_schema() conn = self._connect() with conn.transaction(), conn.cursor() as cur: if active_strategy_id is not None: self._lock_active_strategy(cur, active_strategy_id) + if managed_by_daemon_id is not None: + self._lock_active_daemon(cur, managed_by_daemon_id, owner_user_id) cur.execute("SELECT 1 FROM portfolios WHERE portfolio_id = %s", (portfolio_id,)) if cur.fetchone() is not None: raise ValueError(f"이미 존재하는 포트폴리오입니다: {portfolio_id}") @@ -1142,8 +1165,8 @@ class PostgresPortfolioRepository: INSERT INTO portfolios ( portfolio_id, broker, status, name, asset_id, owner_user_id, cash_balance, asset_quantity, asset_price, profit_buffer, - active_strategy_id, avabile_strategies - ) VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) + active_strategy_id, avabile_strategies, managed_by_daemon_id + ) VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) """, ( portfolio_id, @@ -1158,6 +1181,7 @@ class PostgresPortfolioRepository: float(profit_buffer), active_strategy_id, avabile_strategies, + managed_by_daemon_id, ), ) portfolio = self.get(portfolio_id) @@ -1195,6 +1219,16 @@ class PostgresPortfolioRepository: strategy_id = fields.get("active_strategy_id") if isinstance(strategy_id, str): self._lock_active_strategy(cur, strategy_id) + daemon_id = fields.get("managed_by_daemon_id") + if isinstance(daemon_id, str): + cur.execute( + "SELECT owner_user_id FROM portfolios WHERE portfolio_id = %s", + (portfolio_id,), + ) + owner_row = cur.fetchone() + if owner_row is None: + raise ValueError(f"portfolio를 찾을 수 없습니다: {portfolio_id}") + self._lock_active_daemon(cur, daemon_id, owner_row[0]) cur.execute( f""" UPDATE portfolios diff --git a/docs/data-model.md b/docs/data-model.md index 5c47198..7495be9 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -166,6 +166,7 @@ - tag_ids stores UUID references to active tags; duplicates are prevented by conditional array_append; comments are retrieved in created_at descending order. - `status="disabled"` (soft delete): `managed_by_daemon_id` is set to NULL; portfolio is excluded from `GET /v1/portfolios` by default. Dependent data (`portfolio_comments`, `tag_ids`) is preserved — never deleted on soft delete. READ operations on sub-resources (GET tags, GET comments) remain allowed; write mutations raise HTTP 409. - Status restoration: PATCH with `{"status": "enabled"}` re-activates a disabled portfolio; no other fields may be set in the same request. +- Daemon assignment: a non-null `managed_by_daemon_id` on create or PATCH must reference an existing, active daemon owned by the target portfolio owner (admin `user_id` override uses the final target owner). The daemon `FOR SHARE` row lock and the portfolio INSERT/UPDATE share one transaction, so validation and write form a single atomic decision boundary; a concurrently deactivated daemon is rejected (HTTP 409) rather than committed. Deactivation after an assignment commits preserves the portfolio reference and follows the existing token-revocation contract. ## Entity: `User` @@ -255,7 +256,8 @@ ### State Rules - A daemon must exist and be active before a service token can be bound to it at issuance. -- Deactivating a daemon (is_active=false) immediately revokes all bound service tokens via `revoke_tokens_by_daemon_id` (REQ-022/BAT-007) and blocks new token issuance. +- A daemon must exist, be active, and be owned by the target portfolio owner before a portfolio can be assigned to it (create or PATCH). Assignment validation locks the daemon row `FOR SHARE` inside the portfolio write transaction (Issue #27 / dcy-0004). +- Deactivating a daemon (is_active=false) immediately revokes all bound service tokens via `revoke_tokens_by_daemon_id` (REQ-022/BAT-007) and blocks new token issuance. It does not auto-remove existing portfolio references; deactivation racing a new assignment is rejected with HTTP 409 when the deactivation commits first. - daemon_slug is immutable after creation; only display_name and is_active can be updated. ## Entity: `BrokerCredential` diff --git a/docs/interface-contract.md b/docs/interface-contract.md index de1b318..7c0a115 100644 --- a/docs/interface-contract.md +++ b/docs/interface-contract.md @@ -304,7 +304,8 @@ - Body: `{"asset_id": "", "broker": "", "name": "", "cash_balance": , "asset_quantity": , "asset_price": , "profit_buffer": , "strategy_id": "", "managed_by_daemon_id": "", "user_id": " (admin only)"}` - Response: HTTP 201 `PortfolioItem {"portfolio_id": "", ..., "active_strategy_id": "", "strategy_name": "", "strategy_params": {}, "strategy_revision": , ...}` - Note: `symbol` is populated from the server-side joined asset row (`portfolios.asset_id -> assets.asset_id -> assets.symbol`), not from client-side catalog reconstruction. -- Error: HTTP 403 if service/client token; HTTP 404 if `strategy_id` is unknown; HTTP 409 if the strategy is inactive; HTTP 422 for a missing/invalid `strategy_id` or an unknown/inactive asset +- Note: a non-null `managed_by_daemon_id` must be a daemon server-generated UUID that exists, is active, and is owned by the target portfolio owner (admin `user_id` override uses the final target owner, not the calling admin). Daemon validation and the portfolio write share one atomic transaction (`FOR SHARE` daemon lock); a selected daemon that is deactivated concurrently is rejected rather than committed. +- Error: HTTP 403 if service/client token; HTTP 404 if `strategy_id` is unknown or the daemon does not exist/is not owned by the portfolio owner; HTTP 409 if the strategy is inactive or the daemon is inactive; HTTP 422 for a missing/invalid `strategy_id`, an unknown/inactive asset, or a non-UUID `managed_by_daemon_id` ### Endpoint: `PATCH /v1/portfolios/{portfolio_id}` @@ -312,7 +313,8 @@ - Body: `{"broker": "", "status": "", "name": "", "strategy_id": "", "asset_price": , "managed_by_daemon_id": ""}` - Response: HTTP 200 `PortfolioItem {"portfolio_id": "", ..., "active_strategy_id": "", "strategy_name": "", "strategy_params": {}, "strategy_revision": , ...}` - Note: response `symbol` is emitted from the joined asset row referenced by `asset_id`. -- Error: HTTP 403 if service/client or ownership mismatch; HTTP 404 if portfolio or strategy is not found; HTTP 409 if the portfolio is disabled for the mutation or the selected strategy is inactive; HTTP 422 for an invalid request body +- Note: a non-null `managed_by_daemon_id` assignment/reassignment follows the same daemon validation contract as create: it must be a UUID that exists, is active, and is owned by the portfolio owner. The daemon `FOR SHARE` lock and the portfolio UPDATE share one atomic transaction, so a concurrently deactivated daemon is rejected with HTTP 409. +- Error: HTTP 403 if service/client or ownership mismatch; HTTP 404 if portfolio, strategy, or daemon is not found (foreign daemon is hidden as 404); HTTP 409 if the portfolio is disabled for the mutation, the selected strategy is inactive, or the daemon is inactive; HTTP 422 for an invalid request body or a non-UUID `managed_by_daemon_id` ## Interface: `v1 Strategy Catalog API` diff --git a/tests/test_v1_portfolios_crud.py b/tests/test_v1_portfolios_crud.py index ed5145c..a88c701 100644 --- a/tests/test_v1_portfolios_crud.py +++ b/tests/test_v1_portfolios_crud.py @@ -97,6 +97,7 @@ class _FakeRepo: profit_buffer: float, active_strategy_id: str | None, avabile_strategies: list[str] | None, + managed_by_daemon_id: str | None = None, ) -> Portfolio: p = Portfolio( portfolio_id=portfolio_id, @@ -111,6 +112,7 @@ class _FakeRepo: price=asset_price, profit_buffer=profit_buffer, tag_ids=[], + managed_by_daemon_id=managed_by_daemon_id, ) self._portfolios[portfolio_id] = p return p diff --git a/tests/test_v1_portfolios_daemon_assignment.py b/tests/test_v1_portfolios_daemon_assignment.py new file mode 100644 index 0000000..7d094a6 --- /dev/null +++ b/tests/test_v1_portfolios_daemon_assignment.py @@ -0,0 +1,415 @@ +"""Tests for daemon assignment validation on POST/PATCH /v1/portfolios (Issue #27 / dcy-0004). + +Covers the approved discovery contract: non-null managed_by_daemon_id is validated +(exists, active, same-owner as the portfolio owner) with HTTP 422/404/409 mapping, +admin target-owner semantics, and persistence of the selected daemon. +""" + +from __future__ import annotations + +from contextlib import ExitStack +from typing import Any +from unittest.mock import patch + +from fastapi.testclient import TestClient + +from app.core.config import Settings, get_settings +from app.domain.errors import DaemonInactiveError, DaemonNotFoundError +from app.domain.models import Asset, Daemon, Portfolio, StrategySet +from app.main import create_app + +_ASSET_UUID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" +_PORTFOLIO_UUID = "cccccccc-cccc-cccc-cccc-cccccccccccc" +_USER_ID = "user-001" +_OTHER_USER_ID = "user-002" +_DAEMON_UUID = "dddddddd-dddd-dddd-dddd-dddddddddddd" +_DAEMON2_UUID = "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee" + + +def _make_settings(**kwargs) -> Settings: + return Settings(allowed_origins_raw="http://localhost:3000", **kwargs) + + +def _make_asset(asset_id: str = _ASSET_UUID, status: str = "active") -> Asset: + return Asset(asset_id=asset_id, symbol="005930", markets=["KRX"], status=status) + + +def _make_daemon( + daemon_id: str = _DAEMON_UUID, + owner: str = _USER_ID, + is_active: bool = True, +) -> Daemon: + return Daemon(id=daemon_id, owner_user_id=owner, daemon_slug="d-slug", is_active=is_active) + + +def _make_portfolio(owner: str = _USER_ID) -> Portfolio: + return Portfolio( + portfolio_id=_PORTFOLIO_UUID, + broker="kiwoom", + status="enabled", + name="test-portfolio", + symbol="005930", + asset_id=_ASSET_UUID, + owner_user_id=owner, + cash_balance=1000.0, + asset_quantity=10.0, + price=50000.0, + profit_buffer=0.0, + tag_ids=[], + managed_by_daemon_id=None, + ) + + +def _make_strategy_set() -> StrategySet: + return StrategySet(id="strat-001", name="target_value", config="{}", nick="tv") + + +class _FakeRepo: + """Fake repository that models the repository-side atomic daemon validation.""" + + def __init__( + self, + asset: Asset | None = None, + portfolios: dict[str, Portfolio] | None = None, + daemons: dict[str, Daemon] | None = None, + ) -> None: + self._asset = asset or _make_asset() + self._portfolios: dict[str, Portfolio] = portfolios or {} + self._daemons: dict[str, Daemon] = daemons or {} + self._strategy_set = _make_strategy_set() + + def close(self) -> None: + pass + + def get_asset_by_id(self, asset_id: str) -> Asset | None: + if self._asset and self._asset.asset_id == asset_id: + return self._asset + return None + + def get_strategy_set(self, id: str) -> StrategySet | None: + return self._strategy_set if id == self._strategy_set.id else None + + def get_daemon_by_id(self, daemon_id: str) -> Daemon | None: + return self._daemons.get(daemon_id) + + def get(self, portfolio_id: str) -> Portfolio: + p = self._portfolios.get(portfolio_id) + if p is None: + raise ValueError(f"포트폴리오를 찾을 수 없습니다: {portfolio_id}") + return p + + def _validate_daemon(self, daemon_id: str, owner_user_id: str) -> None: + daemon = self._daemons.get(daemon_id) + if daemon is None or daemon.owner_user_id != owner_user_id: + raise DaemonNotFoundError(f"daemon not found: {daemon_id}") + if not daemon.is_active: + raise DaemonInactiveError(f"daemon is inactive: {daemon_id}") + + def create_portfolio( + self, + portfolio_id: str, + *, + owner_user_id: str, + broker: str, + status: str, + name: str, + asset_id: str, + cash_balance: float, + asset_quantity: float, + asset_price: float, + profit_buffer: float, + active_strategy_id: str | None, + avabile_strategies: list[str] | None, + managed_by_daemon_id: str | None = None, + ) -> Portfolio: + if managed_by_daemon_id is not None: + self._validate_daemon(managed_by_daemon_id, owner_user_id) + p = Portfolio( + portfolio_id=portfolio_id, + broker=broker, + status=status, + name=name, + symbol="005930", + asset_id=asset_id, + owner_user_id=owner_user_id, + cash_balance=cash_balance, + asset_quantity=asset_quantity, + price=asset_price, + profit_buffer=profit_buffer, + tag_ids=[], + managed_by_daemon_id=managed_by_daemon_id, + ) + self._portfolios[portfolio_id] = p + return p + + def update_portfolio(self, portfolio_id: str, **fields: Any) -> Portfolio: + p = self.get(portfolio_id) + daemon_id = fields.get("managed_by_daemon_id") + if isinstance(daemon_id, str): + self._validate_daemon(daemon_id, p.owner_user_id) + for k, v in fields.items(): + if v is not None and hasattr(p, k): + setattr(p, k, v) + return p + + +def _make_client() -> TestClient: + get_settings.cache_clear() + settings = _make_settings( + admin_tokens_raw="admin-tok", + trade_write_tokens_raw="user-tok", + ) + app = create_app(settings_factory=lambda: settings) + app.dependency_overrides[get_settings] = lambda: settings + return TestClient(app) + + +def _with_repo(repo: _FakeRepo) -> ExitStack: + stack = ExitStack() + stack.enter_context( + patch("app.api.v1_portfolios.get_portfolio_repository_from_settings", return_value=repo) + ) + return stack + + +def _auth(token: str) -> dict[str, str]: + return {"Authorization": f"Bearer {token}"} + + +# --------------------------------------------------------------------------- +# POST /v1/portfolios — daemon assignment +# --------------------------------------------------------------------------- + +def test_post_with_owner_active_daemon_returns_201_with_daemon_id() -> None: + repo = _FakeRepo(daemons={_DAEMON_UUID: _make_daemon(owner="system")}) + client = _make_client() + + with _with_repo(repo): + resp = client.post( + "/v1/portfolios", + json={ + "asset_id": _ASSET_UUID, + "broker": "kiwoom", + "strategy_id": "strat-001", + "managed_by_daemon_id": _DAEMON_UUID, + }, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 201 + assert resp.json()["managed_by_daemon_id"] == _DAEMON_UUID + + +def test_post_without_daemon_keeps_null() -> None: + repo = _FakeRepo(daemons={_DAEMON_UUID: _make_daemon(owner=_USER_ID)}) + client = _make_client() + + with _with_repo(repo): + resp = client.post( + "/v1/portfolios", + json={"asset_id": _ASSET_UUID, "broker": "kiwoom", "strategy_id": "strat-001"}, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 201 + assert resp.json()["managed_by_daemon_id"] is None + + +def test_post_malformed_daemon_id_returns_422() -> None: + repo = _FakeRepo(daemons={_DAEMON_UUID: _make_daemon(owner=_USER_ID)}) + client = _make_client() + + with _with_repo(repo): + resp = client.post( + "/v1/portfolios", + json={ + "asset_id": _ASSET_UUID, + "broker": "kiwoom", + "strategy_id": "strat-001", + "managed_by_daemon_id": "not-a-uuid", + }, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 422 + assert len(repo._portfolios) == 0 + + +def test_post_missing_daemon_returns_404() -> None: + repo = _FakeRepo(daemons={_DAEMON_UUID: _make_daemon(owner=_USER_ID)}) + client = _make_client() + + with _with_repo(repo): + resp = client.post( + "/v1/portfolios", + json={ + "asset_id": _ASSET_UUID, + "broker": "kiwoom", + "strategy_id": "strat-001", + "managed_by_daemon_id": "99999999-9999-9999-9999-999999999999", + }, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 404 + assert len(repo._portfolios) == 0 + + +def test_post_foreign_daemon_returns_404() -> None: + repo = _FakeRepo(daemons={_DAEMON_UUID: _make_daemon(owner=_OTHER_USER_ID)}) + client = _make_client() + + with _with_repo(repo): + resp = client.post( + "/v1/portfolios", + json={ + "asset_id": _ASSET_UUID, + "broker": "kiwoom", + "strategy_id": "strat-001", + "managed_by_daemon_id": _DAEMON_UUID, + }, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 404 + assert len(repo._portfolios) == 0 + + +def test_post_inactive_daemon_returns_409() -> None: + repo = _FakeRepo(daemons={_DAEMON_UUID: _make_daemon(owner="system", is_active=False)}) + client = _make_client() + + with _with_repo(repo): + resp = client.post( + "/v1/portfolios", + json={ + "asset_id": _ASSET_UUID, + "broker": "kiwoom", + "strategy_id": "strat-001", + "managed_by_daemon_id": _DAEMON_UUID, + }, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 409 + assert len(repo._portfolios) == 0 + + +def test_post_admin_target_user_accepts_target_owner_daemon() -> None: + repo = _FakeRepo(daemons={_DAEMON_UUID: _make_daemon(owner=_OTHER_USER_ID)}) + client = _make_client() + + with _with_repo(repo): + resp = client.post( + "/v1/portfolios", + json={ + "asset_id": _ASSET_UUID, + "broker": "kiwoom", + "strategy_id": "strat-001", + "user_id": _OTHER_USER_ID, + "managed_by_daemon_id": _DAEMON_UUID, + }, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 201 + assert resp.json()["owner_user_id"] == _OTHER_USER_ID + assert resp.json()["managed_by_daemon_id"] == _DAEMON_UUID + + +def test_post_admin_target_user_rejects_admin_owned_daemon() -> None: + repo = _FakeRepo(daemons={_DAEMON_UUID: _make_daemon(owner=_USER_ID)}) + client = _make_client() + + with _with_repo(repo): + resp = client.post( + "/v1/portfolios", + json={ + "asset_id": _ASSET_UUID, + "broker": "kiwoom", + "strategy_id": "strat-001", + "user_id": _OTHER_USER_ID, + "managed_by_daemon_id": _DAEMON_UUID, + }, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 404 + assert len(repo._portfolios) == 0 + + +# --------------------------------------------------------------------------- +# PATCH /v1/portfolios/{portfolio_id} — daemon assignment / reassignment +# --------------------------------------------------------------------------- + +def _repo_with_portfolio_and_daemons(**daemon_kwargs: Any) -> _FakeRepo: + repo = _FakeRepo( + daemons={_DAEMON_UUID: _make_daemon(**daemon_kwargs)}, + ) + repo._portfolios[_PORTFOLIO_UUID] = _make_portfolio() + return repo + + +def test_patch_assign_daemon_success() -> None: + repo = _repo_with_portfolio_and_daemons(owner=_USER_ID) + client = _make_client() + + with _with_repo(repo): + resp = client.patch( + f"/v1/portfolios/{_PORTFOLIO_UUID}", + json={"managed_by_daemon_id": _DAEMON_UUID}, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 200 + assert resp.json()["managed_by_daemon_id"] == _DAEMON_UUID + + +def test_patch_reassign_daemon_success() -> None: + repo = _FakeRepo( + daemons={ + _DAEMON_UUID: _make_daemon(owner=_USER_ID), + _DAEMON2_UUID: _make_daemon(daemon_id=_DAEMON2_UUID, owner=_USER_ID), + } + ) + repo._portfolios[_PORTFOLIO_UUID] = _make_portfolio() + client = _make_client() + + with _with_repo(repo): + resp = client.patch( + f"/v1/portfolios/{_PORTFOLIO_UUID}", + json={"managed_by_daemon_id": _DAEMON2_UUID}, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 200 + assert resp.json()["managed_by_daemon_id"] == _DAEMON2_UUID + + +def test_patch_malformed_daemon_id_returns_422() -> None: + repo = _repo_with_portfolio_and_daemons(owner=_USER_ID) + client = _make_client() + + with _with_repo(repo): + resp = client.patch( + f"/v1/portfolios/{_PORTFOLIO_UUID}", + json={"managed_by_daemon_id": "not-a-uuid"}, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 422 + + +def test_patch_foreign_daemon_returns_404() -> None: + repo = _repo_with_portfolio_and_daemons(owner=_OTHER_USER_ID) + client = _make_client() + + with _with_repo(repo): + resp = client.patch( + f"/v1/portfolios/{_PORTFOLIO_UUID}", + json={"managed_by_daemon_id": _DAEMON_UUID}, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 404 + + +def test_patch_inactive_daemon_returns_409() -> None: + repo = _repo_with_portfolio_and_daemons(owner=_USER_ID, is_active=False) + client = _make_client() + + with _with_repo(repo): + resp = client.patch( + f"/v1/portfolios/{_PORTFOLIO_UUID}", + json={"managed_by_daemon_id": _DAEMON_UUID}, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 409 \ No newline at end of file diff --git a/tests/test_v1_portfolios_daemon_postgres_integration.py b/tests/test_v1_portfolios_daemon_postgres_integration.py new file mode 100644 index 0000000..150ff78 --- /dev/null +++ b/tests/test_v1_portfolios_daemon_postgres_integration.py @@ -0,0 +1,325 @@ +"""Real-PostgreSQL persistence and concurrency verification for daemon assignment (Issue #27 / dcy-0004). + +Run with PORTFOLIOHUB_RUN_POSTGRES_INTEGRATION=1 against a disposable/local +PostgreSQL endpoint. Normal unit-test environments skip this module. +""" + +from __future__ import annotations + +import os +import threading +import uuid +from contextlib import ExitStack +from unittest.mock import patch + +import pytest +from fastapi.testclient import TestClient + +from app.core.config import Settings, get_settings +from app.domain.errors import DaemonInactiveError, DaemonNotFoundError +from app.main import create_app +from app.repositories.postgres import PortfolioDBConfig, PostgresPortfolioRepository + +pytestmark = pytest.mark.skipif( + os.getenv("PORTFOLIOHUB_RUN_POSTGRES_INTEGRATION") != "1", + reason="set PORTFOLIOHUB_RUN_POSTGRES_INTEGRATION=1 to run real PostgreSQL tests", +) + + +def _db_values(dbname: str | None = None) -> dict[str, object]: + return { + "host": os.getenv("TIMESCALEDB_HOST", "127.0.0.1"), + "port": int(os.getenv("TIMESCALEDB_PORT", "5432")), + "dbname": dbname or os.getenv("TIMESCALEDB_NAME", "portfolio_hub_migration_test"), + "user": os.getenv("TIMESCALEDB_USER", "portfolio_hub_user"), + "password": os.getenv("TIMESCALEDB_PASSWORD", "change-me"), + "sslmode": os.getenv("TIMESCALEDB_SSLMODE", "prefer"), + } + + +def _repo(dbname: str | None = None) -> PostgresPortfolioRepository: + values = _db_values(dbname) + return PostgresPortfolioRepository( + PortfolioDBConfig( + host=str(values["host"]), + port=int(values["port"]), + dbname=str(values["dbname"]), + user=str(values["user"]), + password=str(values["password"]), + sslmode=str(values["sslmode"]), + ) + ) + + +def _prepare_daemon_fixture(repo: PostgresPortfolioRepository) -> tuple[str, str, str, str]: + repo.ensure_schema() + strategy = repo.create_strategy_set(f"daemon-{uuid.uuid4().hex}", "{}") + asset_id = str(uuid.uuid4()) + owner_user_id = f"daemon-owner-{uuid.uuid4().hex}" + with repo._connect().cursor() as cur: + cur.execute( + "INSERT INTO users (id, username, permission, is_active) VALUES (%s, %s, 'trade-write', TRUE)", + (owner_user_id, owner_user_id), + ) + cur.execute( + """ + INSERT INTO assets (asset_id, symbol, markets, status) + VALUES (%s, %s, ARRAY['KRX'], 'active') + """, + (asset_id, f"DA{uuid.uuid4().hex[:10]}"), + ) + daemon = repo.create_daemon(owner_user_id, daemon_slug=f"d-{uuid.uuid4().hex[:10]}") + return strategy.id, asset_id, owner_user_id, daemon.id + + +def _cleanup( + repo: PostgresPortfolioRepository, + portfolio_ids: list[str], + strategy_id: str, + asset_id: str, + owner_user_id: str, + daemon_id: str, +) -> None: + with repo._connect().cursor() as cur: + for portfolio_id in portfolio_ids: + cur.execute("DELETE FROM portfolio_ledger WHERE portfolio_id = %s", (portfolio_id,)) + cur.execute("DELETE FROM portfolios WHERE portfolio_id = %s", (portfolio_id,)) + cur.execute("DELETE FROM auth_tokens WHERE daemon_id = %s", (daemon_id,)) + cur.execute("DELETE FROM daemons WHERE id = %s", (daemon_id,)) + cur.execute("DELETE FROM strategy_sets WHERE id = %s", (strategy_id,)) + cur.execute("DELETE FROM assets WHERE asset_id = %s", (asset_id,)) + cur.execute("DELETE FROM users WHERE id = %s", (owner_user_id,)) + repo.close() + + +def _make_client() -> TestClient: + get_settings.cache_clear() + settings = Settings( + allowed_origins_raw="http://localhost:3000", + admin_tokens_raw="admin-tok", + trade_write_tokens_raw="user-tok", + ) + app = create_app(settings_factory=lambda: settings) + app.dependency_overrides[get_settings] = lambda: settings + return TestClient(app) + + +def _with_repo(repo: PostgresPortfolioRepository) -> ExitStack: + stack = ExitStack() + stack.enter_context( + patch("app.api.v1_portfolios.get_portfolio_repository_from_settings", return_value=repo) + ) + return stack + + +def _auth(token: str) -> dict[str, str]: + return {"Authorization": f"Bearer {token}"} + + +def test_http_post_returns_daemon_and_list_get_preserves_it() -> None: + repo = _repo() + strategy_id, asset_id, owner_user_id, daemon_id = _prepare_daemon_fixture(repo) + client = _make_client() + + try: + with _with_repo(repo): + resp = client.post( + "/v1/portfolios", + json={ + "asset_id": asset_id, + "broker": "kis", + "strategy_id": strategy_id, + "name": "daemon-assignment-check", + "user_id": owner_user_id, + "managed_by_daemon_id": daemon_id, + }, + headers=_auth("admin-tok"), + ) + assert resp.status_code == 201 + body = resp.json() + assert body["managed_by_daemon_id"] == daemon_id + portfolio_id = body["portfolio_id"] + + stored = repo.get(portfolio_id) + assert stored.managed_by_daemon_id == daemon_id + + with _with_repo(repo): + list_resp = client.get("/v1/portfolios", headers=_auth("admin-tok")) + assert list_resp.status_code == 200 + matched = [p for p in list_resp.json()["portfolios"] if p["portfolio_id"] == portfolio_id] + assert matched and matched[0]["managed_by_daemon_id"] == daemon_id + + _cleanup(repo, [portfolio_id], strategy_id, asset_id, owner_user_id, daemon_id) + finally: + pass + + +def test_validation_failure_leaves_no_partial_portfolio() -> None: + repo = _repo() + strategy_id, asset_id, owner_user_id, daemon_id = _prepare_daemon_fixture(repo) + portfolio_id = str(uuid.uuid4()) + inactive = _repo() + try: + inactive.update_daemon(daemon_id, is_active=False) + + with pytest.raises(DaemonInactiveError): + repo.create_portfolio( + portfolio_id, + owner_user_id=owner_user_id, + broker="kis", + status="enabled", + name="no-partial-inactive", + asset_id=asset_id, + cash_balance=0, + asset_quantity=0, + asset_price=0, + profit_buffer=0, + active_strategy_id=strategy_id, + avabile_strategies=[strategy_id], + managed_by_daemon_id=daemon_id, + ) + with repo._connect().cursor() as cur: + cur.execute("SELECT COUNT(*) FROM portfolios WHERE portfolio_id = %s", (portfolio_id,)) + assert cur.fetchone()[0] == 0 + + foreign_user_id = f"foreign-owner-{uuid.uuid4().hex}" + with repo._connect().cursor() as cur: + cur.execute( + "INSERT INTO users (id, username, permission, is_active) VALUES (%s, %s, 'trade-write', TRUE)", + (foreign_user_id, foreign_user_id), + ) + foreign_daemon = repo.create_daemon(foreign_user_id, daemon_slug=f"foreign-{uuid.uuid4().hex[:10]}") + with pytest.raises(DaemonNotFoundError): + repo.create_portfolio( + portfolio_id, + owner_user_id=owner_user_id, + broker="kis", + status="enabled", + name="no-partial-foreign", + asset_id=asset_id, + cash_balance=0, + asset_quantity=0, + asset_price=0, + profit_buffer=0, + active_strategy_id=strategy_id, + avabile_strategies=[strategy_id], + managed_by_daemon_id=foreign_daemon.id, + ) + with repo._connect().cursor() as cur: + cur.execute("SELECT COUNT(*) FROM portfolios WHERE portfolio_id = %s", (portfolio_id,)) + assert cur.fetchone()[0] == 0 + + with repo._connect().cursor() as cur: + cur.execute("DELETE FROM auth_tokens WHERE daemon_id = %s", (foreign_daemon.id,)) + cur.execute("DELETE FROM daemons WHERE id = %s", (foreign_daemon.id,)) + cur.execute("DELETE FROM users WHERE id = %s", (foreign_user_id,)) + _cleanup(repo, [], strategy_id, asset_id, owner_user_id, daemon_id) + finally: + inactive.close() + + +def test_assignment_and_deactivation_race_never_commits_inactive_assignment() -> None: + repo = _repo() + strategy_id, asset_id, owner_user_id, daemon_id = _prepare_daemon_fixture(repo) + portfolio_id = str(uuid.uuid4()) + barrier = threading.Barrier(3) + outcomes: list[str] = [] + + def assign_daemon() -> None: + worker = _repo() + try: + barrier.wait() + worker.create_portfolio( + portfolio_id, + owner_user_id=owner_user_id, + broker="kis", + status="enabled", + name="daemon-race-check", + asset_id=asset_id, + cash_balance=0, + asset_quantity=0, + asset_price=0, + profit_buffer=0, + active_strategy_id=strategy_id, + avabile_strategies=[strategy_id], + managed_by_daemon_id=daemon_id, + ) + outcomes.append("assigned") + except DaemonInactiveError: + outcomes.append("assignment-rejected") + finally: + worker.close() + + def deactivate_daemon() -> None: + worker = _repo() + try: + barrier.wait() + worker.update_daemon(daemon_id, is_active=False) + outcomes.append("deactivated") + finally: + worker.close() + + threads = [threading.Thread(target=assign_daemon), threading.Thread(target=deactivate_daemon)] + for thread in threads: + thread.start() + barrier.wait() + for thread in threads: + thread.join(timeout=10) + assert not thread.is_alive() + + assert sorted(outcomes) in ( + ["assigned", "deactivated"], + ["assignment-rejected", "deactivated"], + ) + with repo._connect().cursor() as cur: + cur.execute("SELECT COUNT(*) FROM portfolios WHERE portfolio_id = %s", (portfolio_id,)) + portfolio_exists = cur.fetchone()[0] == 1 + if portfolio_exists: + assert repo.get(portfolio_id).managed_by_daemon_id == daemon_id + + with repo._connect().cursor() as cur: + if portfolio_exists: + cur.execute("DELETE FROM portfolio_ledger WHERE portfolio_id = %s", (portfolio_id,)) + cur.execute("DELETE FROM portfolios WHERE portfolio_id = %s", (portfolio_id,)) + cur.execute("DELETE FROM auth_tokens WHERE daemon_id = %s", (daemon_id,)) + cur.execute("DELETE FROM daemons WHERE id = %s", (daemon_id,)) + cur.execute("DELETE FROM strategy_sets WHERE id = %s", (strategy_id,)) + cur.execute("DELETE FROM assets WHERE asset_id = %s", (asset_id,)) + cur.execute("DELETE FROM users WHERE id = %s", (owner_user_id,)) + repo.close() + + +def test_patch_reassignment_persists_new_daemon() -> None: + repo = _repo() + strategy_id, asset_id, owner_user_id, daemon_a = _prepare_daemon_fixture(repo) + daemon_b = repo.create_daemon(owner_user_id, daemon_slug=f"d-{uuid.uuid4().hex[:10]}") + portfolio_id = str(uuid.uuid4()) + try: + repo.create_portfolio( + portfolio_id, + owner_user_id=owner_user_id, + broker="kis", + status="enabled", + name="reassign-check", + asset_id=asset_id, + cash_balance=0, + asset_quantity=0, + asset_price=0, + profit_buffer=0, + active_strategy_id=strategy_id, + avabile_strategies=[strategy_id], + managed_by_daemon_id=daemon_a, + ) + assert repo.get(portfolio_id).managed_by_daemon_id == daemon_a + + repo.update_portfolio(portfolio_id, managed_by_daemon_id=daemon_b.id) + assert repo.get(portfolio_id).managed_by_daemon_id == daemon_b.id + + with repo._connect().cursor() as cur: + cur.execute("DELETE FROM portfolio_ledger WHERE portfolio_id = %s", (portfolio_id,)) + cur.execute("DELETE FROM portfolios WHERE portfolio_id = %s", (portfolio_id,)) + cur.execute("DELETE FROM auth_tokens WHERE daemon_id = %s", (daemon_b.id,)) + cur.execute("DELETE FROM daemons WHERE id = %s", (daemon_b.id,)) + _cleanup(repo, [], strategy_id, asset_id, owner_user_id, daemon_a) + finally: + pass \ No newline at end of file