From 6e02d92282b6a371113b907d44fb4839e4518b60 Mon Sep 17 00:00:00 2001 From: Justin Ko Date: Sat, 15 Aug 2026 11:18:31 +0900 Subject: [PATCH] Add ProofLine criteria documentation and Strategy Catalog v1 implementation - Add acceptance criteria documentation (AC-0013 through AC-0023) - Add Line-0003 discovery documentation - Implement Strategy Catalog v1 with new database migration - Update portfolio API endpoints and schemas - Refactor strategy and portfolio services - Add strategy management CLI commands - Update domain models with strategy catalog support - Add comprehensive tests for v1 strategies - Update design documentation (data model, interfaces, runtime flows) --- .proofline/criteria/ac-0013.md | 17 + .proofline/criteria/ac-0014.md | 17 + .proofline/criteria/ac-0015.md | 17 + .proofline/criteria/ac-0016.md | 17 + .proofline/criteria/ac-0017.md | 17 + .proofline/criteria/ac-0018.md | 17 + .proofline/criteria/ac-0019.md | 17 + .proofline/criteria/ac-0020.md | 17 + .proofline/criteria/ac-0021.md | 17 + .proofline/criteria/ac-0022.md | 17 + .proofline/criteria/ac-0023.md | 17 + .proofline/identities.json | 4 +- .proofline/lines/line-0003/dcy-0003.md | 184 ++++++++++ .../evidence/implementation-complete.md | 61 ++++ .../evidence/implementation-progress.md | 65 ++++ .proofline/lines/line-0003/line-0003.md | 3 + .proofline/lines/line-0003/req-0003.md | 65 ++++ app/api/portfolios.py | 11 +- app/api/strategy_sets.py | 115 ------ app/api/v1_portfolios.py | 35 +- app/api/v1_strategies.py | 186 ++++++++++ app/cli/commands/portfolio.py | 14 +- app/cli/commands/strategy.py | 134 ++++--- app/domain/errors.py | 9 + app/domain/models.py | 4 + app/main.py | 6 +- app/repositories/portfolio_repository.py | 13 +- app/repositories/postgres.py | 173 +++++++-- app/schemas/portfolio.py | 26 +- app/schemas/v1.py | 50 ++- app/services/portfolio_service.py | 66 ++-- app/services/strategy_set_service.py | 108 +++--- docs/data-model.md | 26 +- docs/interface-contract.md | 27 +- docs/project-structure.md | 2 +- docs/runtime-flows.md | 13 +- .../20260814_0029_strategy_catalog_v1.py | 121 +++++++ tests/test_asset_catalog.py | 14 +- tests/test_cli.py | 72 +++- tests/test_domain_services.py | 2 +- tests/test_portfolio_comments.py | 2 +- tests/test_portfolio_ownership.py | 2 +- tests/test_portfolio_uuid.py | 34 +- tests/test_strategy_catalog_migration.py | 74 ++++ ...t_strategy_catalog_postgres_integration.py | 328 ++++++++++++++++++ tests/test_v1_portfolios_crud.py | 62 +++- tests/test_v1_portfolios_list.py | 4 + tests/test_v1_strategies.py | 205 +++++++++++ 48 files changed, 2114 insertions(+), 393 deletions(-) create mode 100644 .proofline/criteria/ac-0013.md create mode 100644 .proofline/criteria/ac-0014.md create mode 100644 .proofline/criteria/ac-0015.md create mode 100644 .proofline/criteria/ac-0016.md create mode 100644 .proofline/criteria/ac-0017.md create mode 100644 .proofline/criteria/ac-0018.md create mode 100644 .proofline/criteria/ac-0019.md create mode 100644 .proofline/criteria/ac-0020.md create mode 100644 .proofline/criteria/ac-0021.md create mode 100644 .proofline/criteria/ac-0022.md create mode 100644 .proofline/criteria/ac-0023.md create mode 100644 .proofline/lines/line-0003/dcy-0003.md create mode 100644 .proofline/lines/line-0003/evidence/implementation-complete.md create mode 100644 .proofline/lines/line-0003/evidence/implementation-progress.md create mode 100644 .proofline/lines/line-0003/line-0003.md create mode 100644 .proofline/lines/line-0003/req-0003.md delete mode 100644 app/api/strategy_sets.py create mode 100644 app/api/v1_strategies.py create mode 100644 app/domain/errors.py create mode 100644 migrations/versions/20260814_0029_strategy_catalog_v1.py create mode 100644 tests/test_strategy_catalog_migration.py create mode 100644 tests/test_strategy_catalog_postgres_integration.py create mode 100644 tests/test_v1_strategies.py diff --git a/.proofline/criteria/ac-0013.md b/.proofline/criteria/ac-0013.md new file mode 100644 index 0000000..2bed9bb --- /dev/null +++ b/.proofline/criteria/ac-0013.md @@ -0,0 +1,17 @@ +--- +id: "ac-0013" +status: active +--- + +# active 전략 일반 조회 계약 + +## Criterion + +일반 사용자와 admin은 `/v1/strategies`의 목록·상세 조회에서 active 전략만 받을 수 있어야 하며, inactive 전략은 목록에서 제외하고 상세에서 전략 본문 대신 `409`를 반환하고 미존재 전략은 `404`를 반환해야 한다. + +## Verification + +- 성공 조건: active 전략은 목록과 상세에서 안정적인 ID, 이름, config, status와 revision을 반환한다. +- 실패·경계 조건: inactive 전략은 목록에서 제외되고 일반 상세 조회는 전략 본문 대신 `409`를 반환하며, 미존재 ID는 `404`다. +- 검사 방법: 일반·admin token의 목록/상세 API 회귀 테스트로 active/inactive/missing을 각각 검사한다. +- 현재 구현 평가: working tree의 `app/api/v1_strategies.py`와 `tests/test_v1_strategies.py`가 이 계약을 실행 가능하게 검증하며 scoped 재실행에 포함되어 PASS했다. diff --git a/.proofline/criteria/ac-0014.md b/.proofline/criteria/ac-0014.md new file mode 100644 index 0000000..74ef418 --- /dev/null +++ b/.proofline/criteria/ac-0014.md @@ -0,0 +1,17 @@ +--- +id: "ac-0014" +status: active +--- + +# 관리자 전략 카탈로그 권한과 관리 계약 + +## Criterion + +admin은 `/v1/admin/strategies`에서 active·inactive 전체 전략을 조회하고 생성·수정·상태 변경·미참조 삭제할 수 있어야 하며, 비-admin 사용자의 모든 관리 mutation은 `403`이어야 한다. + +## Verification + +- 성공 조건: admin CRUD와 상태 변경이 영속화되고 관리 조회가 active·inactive를 모두 반환한다. +- 실패·경계 조건: 일반 사용자 또는 권한 부족 token의 create/update/delete는 변경 없이 `403`이다. +- 검사 방법: 역할별 token으로 admin 목록·생성·PATCH·DELETE와 비-admin 거부를 API 테스트로 검사한다. +- 현재 구현 평가: working tree의 모든 admin route가 공통 `require_admin` dependency를 사용하고 admin CRUD tests 및 비-admin 관리 GET `403` test가 scoped 재실행에서 PASS했다. 비-admin POST/PATCH/DELETE 각각의 독립 negative test는 현재 없어 mutation별 executable coverage는 `PARTIAL`이다. diff --git a/.proofline/criteria/ac-0015.md b/.proofline/criteria/ac-0015.md new file mode 100644 index 0000000..b5fbbae --- /dev/null +++ b/.proofline/criteria/ac-0015.md @@ -0,0 +1,17 @@ +--- +id: "ac-0015" +status: active +--- + +# 공유 전략 identity와 revision 계약 + +## Criterion + +admin PATCH는 기존 전략 row의 ID를 유지하면서 요청된 `name`·`config`·`status`의 실질적 변경을 원자적으로 반영하고 revision을 증가시켜야 하며, 모든 참조 포트폴리오의 이후 portfolio-hub API 조회는 commit된 최신 설정과 revision을 반환해야 한다. + +## Verification + +- 성공 조건: 실질적 PATCH 뒤 ID는 동일하고 revision은 단조 증가하며 derived `nick`과 name/config가 같은 committed state에서 계산되고 참조 포트폴리오 조회에 최신 값이 나타난다. +- 실패·경계 조건: no-op PATCH는 revision을 올리지 않고, 동시 partial PATCH도 서로의 최신 필드를 잃거나 불일치한 `nick`을 만들지 않는다. +- 검사 방법: API/repository 회귀와 실제 PostgreSQL 동시 partial PATCH 시험으로 ID, revision, name/config/nick 및 포트폴리오 응답을 검사한다. +- 현재 구현 평가: `FOR UPDATE` 기반 update와 revision 응답 테스트가 존재하며, 2026-08-15 실제 PostgreSQL integration 재실행 `4 passed`에 동시 partial PATCH 검증이 포함됐다. diff --git a/.proofline/criteria/ac-0016.md b/.proofline/criteria/ac-0016.md new file mode 100644 index 0000000..fd14763 --- /dev/null +++ b/.proofline/criteria/ac-0016.md @@ -0,0 +1,17 @@ +--- +id: "ac-0016" +status: active +--- + +# 전략 상태와 포트폴리오 선택 계약 + +## Criterion + +신규 포트폴리오 생성·수정은 전역 카탈로그의 기존 active `strategy_id`만 선택할 수 있어야 하며, 전략을 inactive로 전환해도 기존 포트폴리오 참조는 보존되고 신규 선택만 차단되어야 한다. + +## Verification + +- 성공 조건: active ID 선택은 저장되고 inactive 전환 전의 기존 참조는 조회와 저장에서 유지된다. +- 실패·경계 조건: inactive ID 신규 선택은 `409`, 미존재 ID는 `404`이며 legacy `avabile_strategies`만으로 선택 권한을 얻지 못한다. +- 검사 방법: create/update 및 inactive 전환 전후 포트폴리오 회귀 테스트와 선택·상태 변경 경합 테스트로 확인한다. +- 현재 구현 평가: portfolio service/repository와 API 회귀가 scoped 재실행에서 PASS했고, 2026-08-15 실제 PostgreSQL integration `4 passed`에 선택-vs-delete 경합 검증이 포함됐다. diff --git a/.proofline/criteria/ac-0017.md b/.proofline/criteria/ac-0017.md new file mode 100644 index 0000000..8e99ee2 --- /dev/null +++ b/.proofline/criteria/ac-0017.md @@ -0,0 +1,17 @@ +--- +id: "ac-0017" +status: active +--- + +# 포트폴리오 ID-only 요청과 응답 호환성 계약 + +## Criterion + +`/v1/portfolios`와 legacy `/portfolios`의 생성에는 active `strategy_id`가 필수이고 수정도 ID-only여야 하며, 요청의 `strategy_name`·`strategy_params`는 `422`로 거부하는 동시에 응답의 `active_strategy_id`·`strategy_name`·`strategy_params`·`strategy_revision`은 유지되어야 한다. + +## Verification + +- 성공 조건: 명시한 active ID로만 포트폴리오가 생성·수정되고 응답에 현재 전략 identity·이름·config·revision이 나타난다. +- 실패·경계 조건: strategy ID 누락, 제거 필드 전달 또는 malformed 요청은 새 전략 row를 만들지 않고 `422`다. +- 검사 방법: v1·legacy create/update schema와 저장소 row count, 응답 호환 필드를 API/CLI 회귀로 검사한다. +- 현재 구현 평가: schemas와 양쪽 portfolio API가 ID-only 입력 및 `extra="forbid"` 계약을 구현하고 scoped portfolio/CLI 회귀는 PASS했다. 다만 제거된 `strategy_name`·`strategy_params` 각각의 `422`와 응답 호환 필드 전체를 직접 단언하는 전용 negative/response test는 현재 없어 해당 executable coverage는 `PARTIAL`이다. diff --git a/.proofline/criteria/ac-0018.md b/.proofline/criteria/ac-0018.md new file mode 100644 index 0000000..a1af016 --- /dev/null +++ b/.proofline/criteria/ac-0018.md @@ -0,0 +1,17 @@ +--- +id: "ac-0018" +status: active +--- + +# 전략 API 오류 분류 계약 + +## Criterion + +전략 조회·선택·관리 API는 미존재 `404`, inactive 신규 선택과 참조 중 삭제 `409`, 제거 필드·malformed 입력 `422`, 권한 부족 `403`을 서로 구분해 반환해야 한다. + +## Verification + +- 성공 조건: 각 오류 조건이 승인된 상태 코드와 안정적인 비민감 오류 응답으로 구분된다. +- 실패·경계 조건: 권한 부족을 `404`로, inactive를 `422`로, malformed 입력을 묵시적으로 무시하는 등 오류 클래스가 합쳐지지 않는다. +- 검사 방법: 조건별 독립 API negative test로 status code와 무변경을 검사한다. +- 현재 구현 평가: `tests/test_v1_strategies.py`와 `tests/test_v1_portfolios_crud.py`의 negative matrix가 scoped 재실행에서 PASS했다. diff --git a/.proofline/criteria/ac-0019.md b/.proofline/criteria/ac-0019.md new file mode 100644 index 0000000..c6358d7 --- /dev/null +++ b/.proofline/criteria/ac-0019.md @@ -0,0 +1,17 @@ +--- +id: "ac-0019" +status: active +--- + +# 참조 안전 전략 삭제 계약 + +## Criterion + +전략 hard delete는 어떤 포트폴리오의 현재 `active_strategy_id` 또는 현재 선택 참조에도 없는 경우에만 허용되어야 하며, 참조 중이면 자동 이전·참조 삭제 없이 `409`를 반환해야 한다. + +## Verification + +- 성공 조건: 미참조 전략은 admin이 삭제할 수 있고 삭제 후 조회되지 않는다. +- 실패·경계 조건: 어떤 지원 참조 표현이라도 전략을 가리키면 `409`이며 포트폴리오 참조는 그대로다; 동시 신규 선택과 삭제에서도 orphan이 생기지 않는다. +- 검사 방법: 두 참조 형태별 삭제, retirement 대안, 선택-vs-delete 실제 PostgreSQL 경합을 검사한다. +- 현재 구현 평가: `FOR UPDATE`와 참조 검사·삭제의 단일 transaction 및 회귀 테스트가 있으며, 2026-08-15 실제 PostgreSQL integration `4 passed`에서 선택-vs-delete orphan 방지를 재검증했다. diff --git a/.proofline/criteria/ac-0020.md b/.proofline/criteria/ac-0020.md new file mode 100644 index 0000000..24b90fa --- /dev/null +++ b/.proofline/criteria/ac-0020.md @@ -0,0 +1,17 @@ +--- +id: "ac-0020" +status: active +--- + +# v1 전략 API 전환과 운영자 CLI 무결성 계약 + +## Criterion + +전략 HTTP API는 `/v1/strategies`와 `/v1/admin/strategies`만 제공하고 비버전 `/strategy-sets` alias를 제공하지 않아야 하며, 신뢰된 운영자 CLI도 상태·revision·참조 삭제 무결성을 우회해서는 안 된다. + +## Verification + +- 성공 조건: OpenAPI/router inventory에 새 v1 경로가 있고 CLI 관리 결과가 HTTP와 같은 row identity·status·revision·delete guard를 따른다. +- 실패·경계 조건: `/strategy-sets` 요청은 기존 CRUD로 동작하지 않고, CLI가 참조 중 삭제나 잘못된 상태 전이를 강행하지 않는다. +- 검사 방법: route inventory, legacy path negative test, CLI create/list/update/delete 회귀와 문서 경로 sweep을 수행한다. +- 현재 구현 평가: legacy router 제거, v1 router 등록 및 CLI tests가 존재하며 scoped 재실행에서 PASS했다. diff --git a/.proofline/criteria/ac-0021.md b/.proofline/criteria/ac-0021.md new file mode 100644 index 0000000..c384abe --- /dev/null +++ b/.proofline/criteria/ac-0021.md @@ -0,0 +1,17 @@ +--- +id: "ac-0021" +status: active +--- + +# 기본 전략 migration 멱등 계약 + +## Criterion + +Alembic data migration은 active `name="target_value"`, `config={}` 전략을 중복 없이 보장하고, 동일 `(name, config)` row가 있으면 재사용해야 하며 포트폴리오 요청의 `strategy_id` 필수 조건을 완화하지 않아야 한다. + +## Verification + +- 성공 조건: 빈 DB upgrade 후 기본 전략 한 건이 active, revision 1과 runtime 일치 derived identity로 존재한다. +- 실패·경계 조건: migration 재실행이나 기존 동일 row가 중복 row를 만들지 않고, 기본 전략 존재가 implicit portfolio selection을 허용하지 않는다. +- 검사 방법: migration unit/idempotency test와 신규 PostgreSQL DB full upgrade/head 재실행, portfolio create negative test를 수행한다. +- 현재 구현 평가: migration unit test가 scoped 재실행에서 PASS했고, 2026-08-15 실제 PostgreSQL integration `4 passed`에서 신규 DB full upgrade와 head 재실행을 재검증했다. diff --git a/.proofline/criteria/ac-0022.md b/.proofline/criteria/ac-0022.md new file mode 100644 index 0000000..7187653 --- /dev/null +++ b/.proofline/criteria/ac-0022.md @@ -0,0 +1,17 @@ +--- +id: "ac-0022" +status: active +--- + +# downstream 슬롯 적용 시점 경계 + +## Criterion + +portfolio-hub에 commit된 전략 변경은 이후 portfolio-hub API 조회에 즉시 보이되 실행 중 trex-core 슬롯·기존 주문·완료 거래에 소급 적용되지 않아야 하며, trex-core는 다음 슬롯 재생성 또는 정기 갱신에서 최신 revision을 가져와 이후 동작에 적용해야 한다. + +## Verification + +- 성공 조건: portfolio-hub 조회는 최신 revision을 제공하고 portfolio-hub가 daemon reload/restart 또는 config push를 발생시키지 않으며, trex-core의 다음 refresh가 최신 revision을 선택한다. +- 실패·경계 조건: PATCH가 실행 중 슬롯을 강제 변경하거나 기존 주문·완료 거래를 재해석하지 않고, 정확한 일일 scheduler 시각은 이 Line이 정하지 않는다. +- 검사 방법: portfolio-hub API/runtime-flow 문서와 outbound side-effect 부재를 검사하고, 별도 trex-core integration에서 refresh 전/후 revision 적용을 검증한다. +- 현재 구현 평가: portfolio-hub 측 최신 revision 노출과 강제 push/reload 부재는 working tree에 반영되어 있다. trex-core refresh end-to-end 검증은 이 저장소에 executable seam이 없어 현재 `UNVERIFIED`이며, `active` 상태를 satisfaction으로 해석하지 않는다. diff --git a/.proofline/criteria/ac-0023.md b/.proofline/criteria/ac-0023.md new file mode 100644 index 0000000..321a593 --- /dev/null +++ b/.proofline/criteria/ac-0023.md @@ -0,0 +1,17 @@ +--- +id: "ac-0023" +status: active +--- + +# 동시성 및 stale persistence 무결성 계약 + +## Criterion + +전략 PATCH, 포트폴리오 전략 선택, 전략 상태 변경·삭제와 거래 저장이 경합해도 최신 전략 참조와 revision은 직렬화 가능하게 보존되고, stale portfolio snapshot은 현재 전략 선택을 되살리거나 덮어쓰지 않으며 portfolio mutation과 ledger 기록은 원자적이어야 한다. + +## Verification + +- 성공 조건: 동시 partial PATCH는 모든 변경과 단조 revision을 보존하고, A snapshot 후 B 선택·A 삭제·stale 거래 저장에서도 B 참조와 A 삭제가 유지되며 portfolio UPDATE와 ledger INSERT가 함께 commit된다. +- 실패·경계 조건: stale write가 삭제된 A를 복원하거나 B를 잃지 않고, transaction 실패 시 portfolio와 ledger가 부분 commit되지 않는다. +- 검사 방법: 실제 PostgreSQL에서 동시 PATCH, select-vs-inactivate/delete, stale BUY/SELL snapshot과 transaction rollback 시나리오를 실행한다. +- 현재 구현 평가: repository transaction/row-lock 경계와 integration tests가 존재하며, 2026-08-15 실제 PostgreSQL integration 재실행 `4 passed`에서 동시 PATCH, 선택-vs-delete, stale BUY snapshot과 migration 시나리오를 확인했다. 독립 source-time QC도 PASS를 기록한다. 다만 portfolio UPDATE 성공 후 ledger INSERT 실패를 의도적으로 주입해 rollback을 관찰하는 fault-injection test는 현재 없어 그 세부 검증은 `PARTIAL`이다. diff --git a/.proofline/identities.json b/.proofline/identities.json index d7e4631..bae8b70 100644 --- a/.proofline/identities.json +++ b/.proofline/identities.json @@ -1,5 +1,5 @@ { "schema_version": 1, - "next_line_number": 3, - "next_ac_number": 13 + "next_line_number": 4, + "next_ac_number": 24 } diff --git a/.proofline/lines/line-0003/dcy-0003.md b/.proofline/lines/line-0003/dcy-0003.md new file mode 100644 index 0000000..5a58054 --- /dev/null +++ b/.proofline/lines/line-0003/dcy-0003.md @@ -0,0 +1,184 @@ +--- +id: "dcy-0003" +status: confirmed +--- + +# 전략 API v1 이전 및 관리자 전역 카탈로그 권한 분리 + +## Problem + +`strategy_sets`는 전역 공유 카탈로그이지만 비버전 `/strategy-sets` 쓰기 API와 포트폴리오의 `get_or_create_strategy_set()` 간접 생성 경로가 일반 `trade-write` 사용자에게 열려 있었다. 전략 정의의 관리자 관리 책임, 일반 사용자의 기존 active 전략 선택 책임, 공유 설정 저장 시점과 downstream 실행 슬롯 적용 시점이 분리되어야 한다. + +본 문서는 이미 요청자 승인을 받은 GitHub 이슈 #26을 ProofLine에 사후 포착한다. 아래 `Scope`의 **승인된 이슈 원문 전체**는 구조적 heading만 canonical H2 충돌을 피하도록 한 단계 낮췄으며, 원문 비어 있지 않은 모든 행을 순서대로 보존한다. + +## Evidence + +### 원문·승인·사후 문서화 경계 + +- 원문: https://github.com/fourblocks2020/portfolio-hub/issues/26 +- 원문 제목: `전략 API v1 이전 및 관리자 전역 카탈로그 권한 분리` +- 원문 상태: `OPEN` +- 원문 body SHA-256: `011ece78747cac8e0c38ad07b644ffef080ea1fb7480eef77380c44724b9727e` +- 원문 자체 승인 기록: `APPROVED`, 승인자 `genichin`, 승인 시각 `2026-08-14T08:14:31Z`, unresolved Open Questions 없음. +- 요청자는 이슈 #26 본문을 최대한 포함한 `confirmed` Discovery와 `approved` REQ, 구현 상태를 반영한 `active` AC를 새 Line에 사후 작성하도록 명시했다. +- 이 Line은 이미 수행된 로컬 구현을 승인된 지속 사양으로 포착한다. #26 구현이 ProofLine 정식 시퀀스로 수행됐다는 주장, ProofLine implementation/delivery closure, push, PR, 원격 main 반영, release 또는 deployment 완료를 소급 생성하지 않는다. + +### 현재 구현·검증 증거 + +- 구현 기준: `main` `5fc389dea8f05c6f711ecfe874f055979af5e58d` +- 구현 형태: 기준 HEAD 위 로컬 `main` working tree의 미커밋 product/test/doc 변경. +- 구현 진행 댓글: https://github.com/fourblocks2020/portfolio-hub/issues/26#issuecomment-5291151212 +- 구현 완료 댓글: https://github.com/fourblocks2020/portfolio-hub/issues/26#issuecomment-5291927258 +- 댓글이 기록한 최종 검증: 이슈 집중 회귀 `99 passed`; 실제 PostgreSQL 동시성·migration 통합 `4 passed`; 관련 회귀 `75 passed`; 알려진 기존 외부 오염 파일 제외 광범위 회귀 `608 passed, 3 skipped`; 독립 최종 QC `PASS`. +- 2026-08-15 현재 재검증: scoped API·migration·portfolio·CLI suite `70 passed, 4 skipped`; 별도 실제 PostgreSQL 동시성·migration integration `4 passed`. +- 두 댓글의 exact body는 이 Line의 `evidence/implementation-progress.md`와 `evidence/implementation-complete.md`에 비정식 증적으로 보존한다. +- 위 결과는 source-time 구현 증거다. `active` AC는 normative 상태이며, 그 자체가 현재 commit-bound satisfaction이나 ProofLine delivery closure를 뜻하지 않는다. + +## Scope + +### 승인된 이슈 원문 전체 + +### 배경 + +현재 전략 설정 CRUD는 비버전 경로인 `/strategy-sets`에 있고, 포트폴리오·자산·태그·데몬 등 주요 제품 API는 `/v1` 아래에 있다. `strategy_sets`는 사용자 소유 리소스가 아닌 전역 공유 카탈로그지만, 현재 쓰기 API는 `trade-write` 권한을 허용한다. + +`/v1/portfolios`의 생성·수정 요청은 `strategy_name`과 `strategy_params`를 받아 `get_or_create_strategy_set()`을 호출하므로, 전략 관리 API만 제한하면 일반 사용자의 간접 생성 경로가 남는다. + +### 확인 근거 + +- `app/api/strategy_sets.py`: 비버전 `/strategy-sets`와 `trade-write` 쓰기 권한 +- `app/api/v1_portfolios.py`: 생성·수정 시 `get_or_create_strategy_set()`을 통한 간접 생성 +- `app/repositories/postgres.py`: 전역 `strategy_sets` 저장과 포트폴리오 참조·삭제 방어 +- `app/schemas/v1.py`: 포트폴리오 요청의 `strategy_name`·`strategy_params`, 응답의 `active_strategy_id`·`strategy_name`·`strategy_params` +- `app/cli/commands/strategy.py`: 운영자 CLI의 직접 저장소 관리 경로 +- 워크스페이스 소비자 조사: trex-core와 전략 daemon은 `/v1/portfolios` 응답의 `strategy_name`·`strategy_params`를 소비한다. `/strategy-sets`를 직접 호출하는 다른 repository 코드는 발견되지 않았다. + +### 문제 + +1. 전략 API만 비버전 경로에 남아 있어 API 계약 체계가 일관되지 않는다. +2. 일반 `trade-write` 사용자가 전역 전략 설정을 생성·수정·삭제할 수 있다. +3. 포트폴리오 생성·수정 API를 통한 전략 설정의 간접 생성이 가능하다. +4. 전략 정의 관리와 일반 사용자의 기존 전략 선택 책임이 분리되어 있지 않다. +5. 공유 설정 변경의 저장 시점과 trex-core 실행 슬롯의 실제 적용 시점이 구분되어 있지 않다. + +### 승인된 범위 + +#### 읽기·선택 API + +- 일반 사용자와 admin은 `GET /v1/strategies` 및 상세 조회 API로 active 전략을 조회할 수 있다. +- 일반 사용자는 자기 포트폴리오 생성·수정 시 기존 active `strategy_id`만 선택할 수 있다. +- 생성 시 `strategy_id`는 필수다. +- 전역 active 카탈로그가 선택 가능 여부의 권위이며, legacy `avabile_strategies`는 신규 API의 선택 권한 근거로 사용하지 않는다. + +#### 관리자 카탈로그 API + +- `GET /v1/admin/strategies` +- `POST /v1/admin/strategies` +- `PATCH /v1/admin/strategies/{strategy_id}` +- `DELETE /v1/admin/strategies/{strategy_id}` +- 생성·수정·삭제는 admin만 허용한다. +- admin 조회는 active와 inactive 전략을 모두 다룰 수 있다. + +#### 공유 설정과 revision + +- 전략은 안정적인 `strategy_id`로 식별되는 전역 공유 설정이다. +- admin PATCH는 기존 row의 `name/config`를 제자리 수정하고 동일 ID의 `revision`을 원자적으로 증가시킨다. +- 같은 ID를 참조하는 모든 포트폴리오의 이후 portfolio-hub API 조회에는 commit된 최신 config/revision이 반영된다. +- 실행 중 trex-core 슬롯에는 즉시 적용하지 않는다. trex-core가 재시작되거나 하루 한 번 정해진 시각에 슬롯을 갱신할 때 최신 revision을 가져와 그때부터 적용한다. +- 이미 생성된 슬롯·제출된 주문·완료된 거래에는 소급 적용하지 않는다. +- portfolio-hub는 downstream daemon을 강제로 재시작하거나 설정을 push하지 않는다. + +#### 상태·삭제 + +- 전략에 `active/inactive` 상태를 둔다. +- 일반 조회와 신규 포트폴리오 선택에는 active 전략만 노출·허용한다. +- 기존 포트폴리오가 참조하는 전략을 inactive로 바꿔도 참조는 보존한다. +- DELETE는 어떤 포트폴리오에서도 참조하지 않는 전략만 hard delete한다. +- 참조 중인 전략 DELETE는 HTTP 409를 반환한다. 신규 선택을 막는 retirement는 PATCH inactive로 처리한다. + +#### 포트폴리오 계약과 호환성 + +- `/v1/portfolios`와 legacy `/portfolios`의 생성·수정 모두 ID-only 선택으로 통일한다. +- 요청의 `strategy_name`·`strategy_params`는 제거하며 전달 시 HTTP 422로 거부한다. +- 응답의 `active_strategy_id`·`strategy_name`·`strategy_params`는 trex-core 및 전략 daemon 소비 계약이므로 유지한다. +- 비버전 `/strategy-sets` 경로는 새 `/v1` API 도입과 동시에 제거하며 deprecated alias를 유지하지 않는다. +- 운영자 CLI의 직접 저장소 쓰기는 신뢰된 운영자 전용 관리 경로로 유지하고 일반 사용자 API로 간주하지 않는다. HTTP와 동일한 상태·참조 무결성을 우회해서는 안 된다. + +#### 기본 전략 migration + +- Alembic data migration은 `name="target_value"`, `config={}`, `active`인 기본 전략을 idempotent하게 생성한다. +- 동일 `(name, config)` row가 이미 존재하면 해당 row를 재사용한다. +- 기본 전략이 존재하더라도 포트폴리오 생성 요청의 `strategy_id` 필수 조건은 완화하지 않는다. 호출자는 카탈로그 조회로 ID를 얻어 명시한다. + +#### 오류 계약 + +- 미존재 `strategy_id`: HTTP 404 +- inactive 전략 신규 선택: HTTP 409 +- 제거된 `strategy_name`·`strategy_params` 또는 malformed 입력: HTTP 422 +- 권한 부족: HTTP 403 +- 참조 중 전략 DELETE: HTTP 409 + +### 승인된 결정 사항 + +1. 외부 API 명칭은 `/v1/strategies`, `/v1/admin/strategies`를 사용한다. +2. `active/inactive` 상태를 도입하고 inactive 참조는 보존하되 신규 선택은 금지한다. +3. PATCH는 기존 ID의 공유 설정을 제자리 수정하여 모든 참조 포트폴리오의 이후 API 조회에 반영한다. +4. 포트폴리오는 active `strategy_id`를 필수로 명시하고 전역 active 카탈로그를 선택 권위로 사용한다. +5. 포트폴리오 요청의 `strategy_name`·`strategy_params`는 제거하고 422로 거부한다. +6. 비버전 `/strategy-sets`는 새 API 도입과 동시에 제거한다. +7. 기본 `target_value`/`{}` 전략은 Alembic migration으로 idempotent하게 생성한다. +8. legacy `/portfolios`도 ID-only로 통일하고 CLI는 신뢰된 운영자 경로로 유지한다. +9. portfolio-hub 저장은 즉시 보이지만 실행 슬롯 적용은 trex-core 슬롯 재생성·정기 갱신 시점으로 지연한다. +10. 기본 전략은 active 상태로 생성하거나 기존 동일 row를 재사용한다. +11. 미존재 404, inactive 409, legacy/malformed 422, 권한 부족 403으로 구분한다. +12. 참조 중 전략은 삭제하지 않고 409를 반환하며 retirement는 inactive로 처리한다. + +### 승인된 수용 기준 + +- [ ] 일반 사용자 토큰으로 active 전략 카탈로그 목록·상세를 조회할 수 있다. +- [ ] 일반 사용자 토큰으로 전략 설정 생성·수정·삭제 시 HTTP 403을 반환한다. +- [ ] admin 토큰으로 전략 설정 생성·조회·수정·상태 변경·미참조 삭제가 가능하다. +- [ ] admin이 공유 설정을 수정하면 ID는 유지되고 revision이 증가하며 모든 참조 포트폴리오의 이후 API 조회에 최신 설정이 나타난다. +- [ ] inactive 전략은 일반 카탈로그와 신규 선택에서 제외되지만 기존 포트폴리오 참조는 보존된다. +- [ ] 포트폴리오 생성 시 active `strategy_id`가 필수이며 수정 시에도 active ID만 허용한다. +- [ ] `/v1/portfolios`와 legacy `/portfolios`가 새로운 전략 설정을 간접 생성하지 않는다. +- [ ] 포트폴리오 요청의 `strategy_name`·`strategy_params`는 HTTP 422로 거부하지만 응답의 전략 필드는 유지된다. +- [ ] 미존재·inactive·권한 부족·malformed 요청이 승인된 HTTP 상태 코드로 구분된다. +- [ ] 새 API 도입 시 비버전 `/strategy-sets` 경로가 제거되고 관련 문서·테스트가 갱신된다. +- [ ] migration이 기본 active `target_value`/`{}` 전략을 중복 없이 생성 또는 재사용한다. +- [ ] 참조 중 전략 DELETE는 409이며 포트폴리오 참조를 자동 이전하거나 삭제하지 않는다. +- [ ] trex-core 슬롯은 설정 PATCH만으로 즉시 변경되지 않고 다음 슬롯 재생성·정기 갱신에서 최신 revision을 반영한다. +- [ ] 권한·간접 생성·상태·revision·오류·삭제·migration·호환성에 대한 회귀 테스트가 추가된다. + +### 범위 밖 + +- trex-core의 정확한 일일 슬롯 갱신 시각 및 scheduler 구현 변경 +- portfolio-hub가 downstream daemon을 강제로 reload·restart하거나 설정을 push하는 기능 +- 이미 생성된 슬롯·제출된 주문·완료된 거래의 소급 재해석 +- 내부 DB 테이블명 `strategy_sets` 변경 +- 운영 배포·데이터 migration 실행·release 승인 + +### 위험 및 의존성 + +- `/strategy-sets` 즉시 제거는 breaking change다. 현재 워크스페이스에서 외부 코드 소비자는 발견되지 않았지만 문서와 운영 호출자는 배포 전에 새 경로로 전환해야 한다. +- 공유 config는 mutable하므로 revision 증가와 원자적 update가 필요하다. +- trex-core의 실제 반영은 슬롯 refresh 계약에 의존한다. #26은 최신 revision을 제공하는 portfolio-hub 경계까지만 소유한다. +- legacy 요청 필드가 framework 기본 동작으로 조용히 무시되지 않도록 명시적으로 422를 보장해야 한다. + +### Approval State + +- **Status:** APPROVED +- **Approved by:** genichin +- **Approved at:** 2026-08-14T08:14:31Z +- **Approval basis:** 이슈 #26의 OQ-001~OQ-012 단일 선택 완료와 요청자의 명시적 Discovery 승인 지시 +- **Unresolved Open Questions:** 없음 +- **Authority boundary:** 이 승인은 위 Discovery 범위를 승인하며 구현 완료, main 반영, 배포 또는 release 승인을 의미하지 않는다. + +## Out of Scope + +### 사후 ProofLine 경계 + +- 이 사후 사양 포착은 구현이 ProofLine 정식 시퀀스로 수행됐다는 주장이나 implementation/delivery/release/deployment lifecycle closure를 만들지 않는다. +- 구현 진행·완료 댓글은 canonical lifecycle artifact가 아니라 출처가 명시된 비정식 evidence다. +- 현재 구현은 미커밋 working tree에 있으므로 exact implementation commit, 원격 main 통합, push, PR, 게시, 운영 배포 및 release는 확인되거나 승인된 것으로 간주하지 않는다. +- 이슈 원문의 `범위 밖` 항목과 authority boundary는 위 승인 원문 전체에 보존되며 `req-0003` 및 연결 AC가 이를 축소하거나 확장하지 않는다. diff --git a/.proofline/lines/line-0003/evidence/implementation-complete.md b/.proofline/lines/line-0003/evidence/implementation-complete.md new file mode 100644 index 0000000..a67b931 --- /dev/null +++ b/.proofline/lines/line-0003/evidence/implementation-complete.md @@ -0,0 +1,61 @@ +# 구현 완료 — GitHub Issue #26 비정식 증적 + +> 이 문서는 ProofLine canonical lifecycle artifact가 아니다. #26 구현 당시 GitHub 댓글을 출처와 함께 원문 보존한 비정식 증적이다. +> 이 증거는 로컬 구현·검증 상태를 기록하지만 ProofLine 정식 implementation chronology, delivery closure, push, PR, 원격 main 반영, release 또는 deployment 완료를 소급 증명하지 않는다. +> Source: https://github.com/fourblocks2020/portfolio-hub/issues/26#issuecomment-5291927258 +> Comment ID: `IC_kwDOR4l8PM8AAAABO2xm2g` +> Author: `genichin` +> Created: `2026-08-14T09:48:42Z` +> Original body SHA-256: `c3bc9a3ad6c9cc6678ed700a11add03b1f5abe71645a21e157dc091faec28f10` +> Original body boundary: 아래 `---` 다음 byte부터 파일 끝까지가 GitHub 원문 body다. + +--- + +## 구현 완료 + +이슈 #26 **전략 API v1 이전 및 관리자 전역 카탈로그 권한 분리**의 승인 범위를 로컬 `main` 작업 트리에 구현하고 실제 PostgreSQL 검증 및 독립 QC까지 완료했습니다. + +### 구현 내용 + +- 비버전 `/strategy-sets` 제거 및 전략 API `/v1` 이전 + - 일반 사용자: active 전략 조회 전용 `/v1/strategies` + - 관리자: 전체 전략 카탈로그 CRUD `/v1/admin/strategies` +- 전략 카탈로그에 `active/inactive` 상태와 revision 도입 +- 신규 전략 revision 1, 실질적 PATCH만 revision 증가, no-op 유지 +- 동시 PATCH를 `FOR UPDATE`로 직렬화하고 최신 name/config 기준으로 derived `nick` 계산 +- 포트폴리오 create/update에서 기존 active `strategy_id`만 선택 +- `strategy_name`·`strategy_params`를 통한 간접 전략 생성 차단 +- 전략 선택의 `FOR SHARE`와 포트폴리오 저장을 같은 트랜잭션으로 묶어 inactivate/delete race 차단 +- 전략 DELETE의 `FOR UPDATE`·두 참조 형태 검사·삭제를 한 트랜잭션으로 통합 +- stale BUY/SELL snapshot이 전략 선택 컬럼을 덮어쓰지 못하도록 거래 저장 책임 분리 +- 거래 portfolio UPDATE와 ledger INSERT 원자화 +- 포트폴리오 응답에 최신 `strategy_revision` 노출 +- 기본 `target_value`/`{}` 전략 migration과 runtime nick 파생 규칙 통일 +- CLI, 문서, migration 및 동시성 회귀 테스트 갱신 + +### 실제 검증 결과 + +- 이슈 집중 회귀: `99 passed` +- 실제 PostgreSQL 동시성·migration 통합 테스트: 최종 `4 passed` + - 동시 partial PATCH의 nick/revision 일관성 + - 전략 선택 대 DELETE race의 orphan 방지 + - stale A snapshot → B 선택 → A 삭제 → stale BUY 저장 후 B 유지 + - 신규 DB 전체 Alembic upgrade 및 head 재실행 +- 관련 최종 회귀: `75 passed` +- 알려진 기존 문제 파일 제외 광범위 회귀: `608 passed, 3 skipped` +- 독립 최종 QC 재실행: PostgreSQL `4 passed`, focused `37 passed, 4 skipped` +- Python compile, `git diff --check`, 변경 핵심 파일 Ruff 검사: 통과 +- 독립 QC 최종 판정: **PASS** + +전체 broad 실행에 남은 3건은 저장소 밖 `temp/deleted_20260813/.worktrees`를 스캔하는 기존 환경 오염 실패로, 이번 변경에 의한 회귀가 아닙니다. + +### 비차단 후속 권고 + +- PostgreSQL 통합 테스트 cleanup을 fixture/`finally` 기반으로 강화하면 테스트가 중간 실패·중단될 때의 잔여 fixture 방지를 개선할 수 있습니다. 현재 성공 경로 cleanup은 QC 전후 잔여 레코드 수가 동일해 정상입니다. + +### 완료 경계 + +- 완료: 로컬 구현, 실제 테스트, migration 검증, 문서 갱신, 독립 QC +- 미수행: 커밋, push, PR, 원격 `main` 반영, 이미지 게시, 운영 설정 변경, 배포, 릴리스 + +현재 변경은 기준 HEAD `5fc389dea8f05c6f711ecfe874f055979af5e58d`의 로컬 `main` 작업 트리에 **미커밋 상태**로 보존되어 있습니다. 이 댓글은 제품 구현·검증 완료를 뜻하며 배포·릴리스 또는 이슈 lifecycle closure를 뜻하지 않습니다. diff --git a/.proofline/lines/line-0003/evidence/implementation-progress.md b/.proofline/lines/line-0003/evidence/implementation-progress.md new file mode 100644 index 0000000..4333de0 --- /dev/null +++ b/.proofline/lines/line-0003/evidence/implementation-progress.md @@ -0,0 +1,65 @@ +# 구현 진행 상황 — GitHub Issue #26 비정식 증적 + +> 이 문서는 ProofLine canonical lifecycle artifact가 아니다. #26 구현 당시 GitHub 댓글을 출처와 함께 원문 보존한 비정식 증적이다. +> 이 증거는 로컬 구현·검증 상태를 기록하지만 ProofLine 정식 implementation chronology, delivery closure, push, PR, 원격 main 반영, release 또는 deployment 완료를 소급 증명하지 않는다. +> Source: https://github.com/fourblocks2020/portfolio-hub/issues/26#issuecomment-5291151212 +> Comment ID: `IC_kwDOR4l8PM8AAAABO2CPbA` +> Author: `genichin` +> Created: `2026-08-14T08:21:45Z` +> Original body SHA-256: `539cfe82e6f8281c5821c52f484a4078b116e52f5696ee0f9ea502f27b42cfeb` +> Original body boundary: 아래 `---` 다음 byte부터 파일 끝까지가 GitHub 원문 body다. + +--- + +## 구현 진행 상황 + +- **대상:** 전략 API v1 이전 및 관리자 전역 카탈로그 권한 분리 +- **브랜치:** `main` +- **기준:** `main` (`5fc389dea8f05c6f711ecfe874f055979af5e58d`) +- **상태:** 로컬 구현·검증·독립 QC 완료 (PASS) +- **최종 업데이트:** 2026-08-14 18:47 KST + +### 구현 계획 및 작업 목록 + +- [x] 전략 카탈로그 데이터 모델과 저장소 기능 확장 + - 안정적인 전략 ID를 유지하면서 `active/inactive` 상태와 원자적으로 증가하는 revision을 저장·조회하고, 동시 PATCH에서도 `name/config/nick` 일관성을 보장한다. +- [x] `/v1/strategies` 일반 조회 API 구현 + - 일반 사용자는 active 전략의 목록과 상세만 조회할 수 있도록 한다. +- [x] `/v1/admin/strategies` 관리자 관리 API 구현 + - admin만 전체 목록 조회, 생성, 제자리 수정·상태 변경, 미참조 전략 삭제를 수행할 수 있도록 한다. +- [x] 포트폴리오 전략 선택을 기존 active `strategy_id` 전용으로 변경 + - `/v1/portfolios`와 legacy `/portfolios` 모두 생성 시 전략 ID를 필수로 받고, 수정 시에도 기존 active 전략만 선택하며 선택·저장·삭제 race를 DB 트랜잭션과 row lock으로 차단한다. +- [x] 동시성·참조 무결성 회귀 검증 + - 동시 전략 PATCH와 포트폴리오 선택·전략 삭제 경합에서도 derived `nick`, revision, active 선택, orphan 방지가 유지되는지 실제 PostgreSQL로 검증한다. +- [x] 일반 사용자의 간접 전략 생성 및 비버전 API 제거 + - 포트폴리오 요청의 `strategy_name`·`strategy_params`를 422로 거부하고 비버전 `/strategy-sets` 라우트를 제거한다. +- [x] 기본 전략 데이터 migration과 운영자 CLI 무결성 반영 + - 기본 active `target_value`/`{}` 전략을 중복 없이 생성·재사용하고, CLI가 상태·revision·참조 무결성을 우회하지 않도록 한다. +- [x] 권한·상태·revision·오류·삭제·호환성 회귀 테스트 추가 및 실행 + - 승인된 HTTP 403/404/409/422 계약, 참조 보존, 간접 생성 차단과 migration idempotency를 실제 테스트로 검증한다. +- [x] 인터페이스·런타임·데이터 모델 문서 갱신 및 전체 변경 검수 + - 새 API 계약과 비버전 경로 제거, downstream 적용 시점의 경계를 문서화하고 전체 diff와 회귀를 점검한다. + +### 진행 기록 + +- 2026-08-14 17:21 KST — 이슈 #26의 승인된 범위와 수용 기준을 기준으로 구현 계획을 확정했다. +- 2026-08-14 17:21 KST — 별도 구현 브랜치 없이 `main` (`5fc389d`)에서 구현·검증을 시작했다. +- 2026-08-14 17:32 KST — v1 전략 API, 전략 상태·revision 저장소, ID 전용 포트폴리오 선택, 비버전 라우트 제거 및 migration의 초기 구현을 작업트리에 반영했다. 아직 검증 전이므로 체크하지 않았다. +- 2026-08-14 17:33 KST — 첫 전체 회귀 실행 결과 `571 passed, 27 failed, 6 skipped`였다. 새 필수 `strategy_id` 계약에 맞지 않는 기존 테스트·CLI와 미완료 전용 테스트·문서를 보완한 뒤 다시 검증한다. +- 2026-08-14 17:44 KST — CLI·기존 테스트·전용 API/migration 테스트·문서를 보완했다. 이슈 집중 회귀는 `166 passed`, 전체 제품 테스트는 기존 stale worktree 정적검사 3건을 제외하고 `606 passed, 3 skipped`였다. +- 2026-08-14 17:49 KST — 실제 PostgreSQL 신규 DB에서 Alembic 전체 upgrade와 head 재실행을 검증했다. 기본 `target_value`/`{}` 전략은 active, revision 1로 한 건 생성됐다. +- 2026-08-14 17:50 KST — 실제 PostgreSQL 기반 HTTP 흐름에서 admin 생성·PATCH·삭제, 일반 조회·권한 거부, ID 유지와 revision 증가를 검증했다. +- 2026-08-14 17:51 KST — 검증 중 발견한 inactive 신규 전략이 revision 2로 시작하는 결함을 수정해 실제 DB와 회귀 테스트에서 revision 1을 확인했다. +- 2026-08-14 17:52 KST — 독립 QC가 동시 PATCH의 derived `nick` 불일치 가능성과 전략 선택·삭제 사이의 참조 race를 HIGH로 판정해 반려했다. 특히 참조 race 수정은 포트폴리오 mutation과 전략 삭제를 같은 DB 트랜잭션/row lock 경계로 묶는 작업이 추가로 필요해, 승인된 구현 계획 보완이 필요하므로 구현을 중지했다. +- 2026-08-14 18:17 KST — 요청자가 트랜잭션·row lock·동시성 테스트를 구현 계획에 추가하고 QC 지적 전체를 수정하도록 명시 승인해 구현을 재개했다. +- 2026-08-14 18:34 KST — 전략 PATCH를 `FOR UPDATE` 트랜잭션으로 직렬화하고, active 전략 선택의 `FOR SHARE` lock과 포트폴리오 INSERT/UPDATE를 같은 트랜잭션으로 묶었다. 전략 DELETE도 `FOR UPDATE`·참조 검사·삭제를 한 트랜잭션으로 통합했다. +- 2026-08-14 18:34 KST — 포트폴리오 응답에 최신 `strategy_revision`을 추가하고, migration 기본 전략 nick을 runtime SHA-1 규칙과 일치시켰다. +- 2026-08-14 18:34 KST — 집중 회귀 `99 passed`, 실제 PostgreSQL 동시 PATCH·선택/삭제 race·신규 DB migration 재실행 통합 테스트 `3 passed`, 알려진 외부 정적 sweep 및 기존 ledger fixture 파일을 제외한 광범위 회귀 `607 passed, 3 skipped`를 확인했다. Python compile, `git diff --check`, 변경 핵심 파일 Ruff 검사도 통과했다. +- 2026-08-14 18:34 KST — 전체 원문 실행은 `609 passed, 3 failed, 3 errors, 3 skipped`였다. 실패 3건은 저장소 밖 `temp/deleted_20260813/.worktrees`를 스캔하는 기존 정적 테스트이고, error 3건은 기존 ledger integration fixture가 현재 DB의 필수 `owner_user_id`를 넣지 않는 문제로 이번 전략 변경과 분리했다. 독립 재-QC를 요청했다. +- 2026-08-14 18:40 KST — 독립 재-QC는 거래 저장 `save()`가 stale 포트폴리오 snapshot의 전략 A를 다시 써, B로 선택 변경 후 삭제된 A 참조를 되살릴 수 있음을 HIGH로 재현해 FAIL 판정했다. 이전 findings는 모두 해소된 것으로 확인됐다. +- 2026-08-14 18:43 KST — `save()`에서 전략 선택 컬럼 쓰기를 제거하고 balance/price/buffer/timestamp UPDATE와 ledger INSERT를 단일 트랜잭션으로 묶었다. A snapshot→B 선택→A 삭제→stale BUY 저장 후에도 B가 유지되고 A가 없는 실제 PostgreSQL 회귀 테스트를 추가했다. +- 2026-08-14 18:43 KST — 실제 PostgreSQL 통합 테스트 `4 passed`, 관련 회귀 `75 passed`, 알려진 기존 문제 파일 제외 광범위 회귀 `608 passed, 3 skipped`, 핵심 Ruff·compile·diff 검사를 통과해 최종 독립 재-QC를 요청했다. +- 2026-08-14 18:47 KST — 최종 독립 재-QC가 PASS를 판정했다. stale BUY/SELL snapshot의 삭제 전략 참조 복원 문제가 완전히 해소됐고, 거래 UPDATE와 ledger INSERT의 단일 트랜잭션, selection/strategy lock 상호작용, 실제 PostgreSQL 회귀 시나리오를 확인했다. 독립 재실행은 PostgreSQL `4 passed`, focused `37 passed, 4 skipped`, compile·diff 검사 통과였다. +- 2026-08-14 18:47 KST — 비차단 테스트 위생 권고: PostgreSQL 통합 테스트 cleanup을 향후 fixture/finally로 강화하면 테스트 중단 시 잔여 fixture를 더 확실히 방지할 수 있다. 현재 성공 경로 cleanup은 전후 잔여 레코드 수가 동일해 정상이다. + +> 모든 체크는 로컬 작업 트리의 구현·테스트·독립 QC 완료를 뜻한다. 커밋·push·PR·main 원격 반영·게시·배포·릴리스는 수행하지 않았으며 별도 지시와 게이트가 필요하다. diff --git a/.proofline/lines/line-0003/line-0003.md b/.proofline/lines/line-0003/line-0003.md new file mode 100644 index 0000000..27b5075 --- /dev/null +++ b/.proofline/lines/line-0003/line-0003.md @@ -0,0 +1,3 @@ +--- +id: "line-0003" +--- diff --git a/.proofline/lines/line-0003/req-0003.md b/.proofline/lines/line-0003/req-0003.md new file mode 100644 index 0000000..e28387d --- /dev/null +++ b/.proofline/lines/line-0003/req-0003.md @@ -0,0 +1,65 @@ +--- +id: "req-0003" +status: approved +discovery: "dcy-0003" +criteria: + create: + - "ac-0013" + - "ac-0014" + - "ac-0015" + - "ac-0016" + - "ac-0017" + - "ac-0018" + - "ac-0019" + - "ac-0020" + - "ac-0021" + - "ac-0022" + - "ac-0023" + update: [] + retire: [] + satisfy: [] +--- + +# 전략 API v1 이전 및 관리자 전역 카탈로그 권한 분리 + +## Objective + +전역 전략 카탈로그의 일반 조회·선택과 관리자 관리를 versioned API에서 분리하고, 안정적인 전략 identity·revision·상태·참조 무결성을 유지하면서 포트폴리오의 ID-only 선택 계약과 downstream 적용 시점 경계를 제공한다. + +## Scope + +- 일반 사용자와 admin의 active 전략 목록·상세 조회를 `/v1/strategies`로 제공한다. +- active·inactive 전체 전략의 생성·조회·제자리 수정·상태 변경·미참조 삭제를 `/v1/admin/strategies`의 admin 전용 권한으로 제공한다. +- 전략의 안정적인 ID, 양의 revision, active/inactive 상태와 동시 변경 시 원자적 일관성을 보장한다. +- `/v1/portfolios`와 legacy `/portfolios`의 생성·수정을 기존 active `strategy_id` 전용으로 통일하고 간접 전략 생성을 제거한다. +- 요청의 제거된 전략 이름·파라미터 필드는 거부하되 기존 downstream 응답 필드는 최신 공유 설정과 revision으로 유지한다. +- 참조 중 전략의 hard delete를 거부하고 inactive retirement 및 기존 참조 보존을 지원한다. +- 비버전 `/strategy-sets`를 alias 없이 제거하고 운영자 CLI가 HTTP와 같은 상태·참조 무결성을 우회하지 않게 한다. +- 기본 active `target_value`/`{}` 전략을 idempotent Alembic data migration으로 생성하거나 기존 동일 row를 재사용한다. +- portfolio-hub의 commit된 최신 revision 제공 시점과 trex-core 슬롯의 재생성·정기 갱신 적용 시점을 구분한다. +- 동시 PATCH, 선택·상태 변경·삭제 경합 및 stale 거래 snapshot 저장에서도 derived field, revision, 현재 참조와 ledger 원자성을 보존한다. + +## Constraints + +- 전역 active 카탈로그가 신규 선택 권위이며 legacy `avabile_strategies`는 선택 권한 근거가 아니다. +- 외부 오류 계약은 미존재 `404`, inactive 신규 선택과 참조 중 삭제 `409`, 제거 필드·malformed 입력 `422`, 권한 부족 `403`으로 구분한다. +- 기존 inactive 전략 참조는 보존하며 자동 이전하거나 삭제하지 않는다. +- portfolio-hub는 downstream daemon을 강제 reload/restart하거나 설정을 push하지 않고 이미 생성된 슬롯·주문·완료 거래를 소급 재해석하지 않는다. +- 내부 DB 테이블명 `strategy_sets`는 유지한다. + +### Current Implementation Assessment + +- 이 REQ는 요청자 지시에 따라 사후 승인 상태로 포착됐다. `approved`/`active`는 normative authority이며 ProofLine 구현 chronology나 satisfaction marker가 아니다. +- 현재 `main` 기준 HEAD `5fc389dea8f05c6f711ecfe874f055979af5e58d` 위 미커밋 working tree에 portfolio-hub 범위의 구현·테스트·문서 변경이 존재한다. +- 2026-08-15 현재 scoped 재실행 `uv run python -m pytest -q tests/test_v1_strategies.py tests/test_strategy_catalog_migration.py tests/test_v1_portfolios_crud.py tests/test_strategy_catalog_postgres_integration.py tests/test_cli.py`는 `70 passed, 4 skipped`였다. 이어 `PORTFOLIOHUB_RUN_POSTGRES_INTEGRATION=1 uv run python -m pytest -q tests/test_strategy_catalog_postgres_integration.py`를 실행해 실제 PostgreSQL integration `4 passed`를 확인했다. +- `ac-0013`~`ac-0021`, `ac-0023`은 현재 working-tree 구현과 executable tests가 대응한다. 다만 `ac-0014`의 비-admin mutation별 독립 negative test, `ac-0017`의 제거 요청 필드·응답 호환 필드 전용 test, `ac-0023`의 ledger INSERT 실패 fault-injection rollback test는 `PARTIAL`이며, exact implementation commit과 ProofLine delivery closure도 없다. +- `ac-0022`의 portfolio-hub 경계(최신 revision 조회 제공, 강제 push/reload 부재)는 구현되어 있으나, trex-core가 다음 슬롯 재생성·정기 갱신에서 최신 revision을 실제 반영한다는 cross-repository end-to-end seam은 이 저장소 증거만으로 확인되지 않아 `UNVERIFIED`다. + +## Non-Goals + +- trex-core의 정확한 일일 슬롯 갱신 시각 또는 scheduler 구현 변경 +- downstream daemon 강제 reload·restart 또는 설정 push +- 기존 슬롯·주문·완료 거래의 소급 재해석 +- 내부 `strategy_sets` 테이블명 변경 +- 운영 배포, production data migration 실행, push, PR, 원격 main 반영 또는 release 승인 +- 이 사후 문서로 과거 구현에 ProofLine 정식 implementation/delivery chronology나 terminal closure를 소급 부여하는 행위 diff --git a/app/api/portfolios.py b/app/api/portfolios.py index 5fe01f7..34aa7cb 100644 --- a/app/api/portfolios.py +++ b/app/api/portfolios.py @@ -25,12 +25,17 @@ from app.schemas.portfolio import ( UpdatePortfolioRequest, ) from app.services.portfolio_service import PortfolioService +from app.services.strategy_set_service import StrategyInactiveError, StrategyNotFoundError router = APIRouter(prefix="/portfolios", tags=["portfolios"]) -def _raise_portfolio_error(exc: ValueError) -> None: +def _raise_portfolio_error(exc: ValueError) -> NoReturn: detail = str(exc) + if isinstance(exc, StrategyNotFoundError): + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=detail) from exc + if isinstance(exc, StrategyInactiveError): + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=detail) from exc if "이미 연결된 태그입니다" in detail: raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=detail) from exc if "찾을 수 없습니다" in detail: @@ -151,6 +156,8 @@ def create_portfolio( if _ownership_required(settings): owner_user_id = _require_owner_context(auth_context) return service.create_portfolio(payload, owner_user_id=owner_user_id) + except ValueError as exc: + _raise_portfolio_error(exc) finally: _close_repository(repository) @@ -169,6 +176,8 @@ def update_portfolio( if _ownership_required(settings): _ensure_portfolio_owned(repository, portfolio_id, auth_context) return service.update_portfolio(portfolio_id, payload) + except ValueError as exc: + _raise_portfolio_error(exc) finally: _close_repository(repository) diff --git a/app/api/strategy_sets.py b/app/api/strategy_sets.py deleted file mode 100644 index 73826a1..0000000 --- a/app/api/strategy_sets.py +++ /dev/null @@ -1,115 +0,0 @@ -"""Strategy Set API routes.""" - -from fastapi import APIRouter, Depends, Response, status - -from app.core.auth import AuthContext, require_read, require_trade_write -from app.core.config import Settings, get_settings -from app.repositories.factory import get_portfolio_repository_from_settings -from app.schemas.portfolio import CreateStrategySetRequest, StrategySetResponse, UpdateStrategySetRequest -from app.services.strategy_set_service import StrategySetService - - -router = APIRouter(prefix="/strategy-sets", tags=["strategy-sets"]) - - -def _get_service(settings: Settings) -> tuple[StrategySetService, object]: - repository = get_portfolio_repository_from_settings(settings) - return StrategySetService(repository), repository - - -def _close_repository(repository: object) -> None: - if callable(getattr(type(repository), "close", None)): - repository.close() - - -@router.get("", response_model=list[StrategySetResponse]) -def list_strategy_sets( - _auth: AuthContext = Depends(require_read), - settings: Settings = Depends(get_settings), -) -> list[StrategySetResponse]: - """List all strategy sets.""" - service, repository = _get_service(settings) - try: - strategy_sets = service.list() - finally: - _close_repository(repository) - 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) -def create_strategy_set( - payload: CreateStrategySetRequest, - _auth: AuthContext = Depends(require_trade_write), - settings: Settings = Depends(get_settings), -) -> StrategySetResponse: - """Create a new strategy set (or return existing if (name, config) pair already exists).""" - service, repository = _get_service(settings) - try: - strategy_set = service.create(payload) - finally: - _close_repository(repository) - 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) -def update_strategy_set( - strategy_set_id: str, - payload: UpdateStrategySetRequest, - _auth: AuthContext = Depends(require_trade_write), - settings: Settings = Depends(get_settings), -) -> 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. - """ - service, repository = _get_service(settings) - try: - strategy_set = service.update(strategy_set_id, payload) - finally: - _close_repository(repository) - 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) -def delete_strategy_set( - strategy_set_id: str, - _auth: AuthContext = Depends(require_trade_write), - settings: Settings = Depends(get_settings), -) -> 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, repository = _get_service(settings) - try: - service.delete(strategy_set_id) - finally: - _close_repository(repository) - 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/api/v1_portfolios.py b/app/api/v1_portfolios.py index 4ba168e..bb263c8 100644 --- a/app/api/v1_portfolios.py +++ b/app/api/v1_portfolios.py @@ -8,7 +8,6 @@ REQ-043: GET /v1/portfolios/{id}/holdings/cost-basis — current holding WAC cos from __future__ import annotations -import json import logging import uuid from datetime import date, datetime, timedelta, timezone @@ -44,6 +43,7 @@ from app.schemas.v1 import ( TradeHistoryItem, TradeHistoryResponse, ) +from app.services.strategy_set_service import StrategyInactiveError, StrategyNotFoundError logger = logging.getLogger(__name__) router = APIRouter(prefix="/v1/portfolios", tags=["v1"]) @@ -96,6 +96,7 @@ def _portfolio_to_item( active_strategy_id=portfolio.active_strategy_id, strategy_name=portfolio.strategy_name, strategy_params=portfolio.strategy_params, + strategy_revision=portfolio.strategy_revision, markets=portfolio.markets, ) @@ -160,10 +161,11 @@ def create_portfolio_v1( ) name = body.name or f"{body.broker}-{body.asset_id[:8]}" - strategy_set = repo.get_or_create_strategy_set( - body.strategy_name or "target_value", - json.dumps(body.strategy_params or {}, sort_keys=True, ensure_ascii=True), - ) + strategy_set = repo.get_strategy_set(body.strategy_id) + if strategy_set is None: + raise StrategyNotFoundError(f"strategy not found: {body.strategy_id}") + if strategy_set.status != "active": + raise StrategyInactiveError(f"strategy is inactive: {body.strategy_id}") portfolio = repo.create_portfolio( str(uuid.uuid4()), @@ -181,6 +183,10 @@ def create_portfolio_v1( ) except HTTPException: raise + except StrategyNotFoundError as exc: + 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 ValueError as exc: raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(exc)) from exc finally: @@ -215,21 +221,24 @@ def update_portfolio_v1( fields = body.model_dump(exclude_none=True) if fields: - if "strategy_name" in fields or "strategy_params" in fields: - strategy_set = repo.get_or_create_strategy_set( - fields.pop("strategy_name", None) or "target_value", - json.dumps(fields.pop("strategy_params", None) or {}, sort_keys=True, ensure_ascii=True), - ) + if "strategy_id" in fields: + strategy_id = str(fields.pop("strategy_id")) + strategy_set = repo.get_strategy_set(strategy_id) + if strategy_set is None: + raise StrategyNotFoundError(f"strategy not found: {strategy_id}") + if strategy_set.status != "active": + raise StrategyInactiveError(f"strategy is inactive: {strategy_id}") fields["active_strategy_id"] = strategy_set.id fields["avabile_strategies"] = [strategy_set.id] - else: - fields.pop("strategy_name", None) - fields.pop("strategy_params", None) repo.update_portfolio(portfolio_id, **fields) portfolio = repo.get(portfolio_id) except HTTPException: raise + except StrategyNotFoundError as exc: + 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 ValueError as exc: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc finally: diff --git a/app/api/v1_strategies.py b/app/api/v1_strategies.py new file mode 100644 index 0000000..1b224f2 --- /dev/null +++ b/app/api/v1_strategies.py @@ -0,0 +1,186 @@ +"""Versioned strategy catalog APIs.""" + +from __future__ import annotations + +import json + +from fastapi import APIRouter, Depends, HTTPException, Response, status + +from app.core.auth import AuthContext, require_admin, require_read +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.schemas.portfolio import CreateStrategySetRequest, UpdateStrategySetRequest +from app.schemas.v1 import StrategyCreateRequest, StrategyResponse, StrategyUpdateRequest +from app.services.strategy_set_service import ( + StrategyConflictError, + StrategyInactiveError, + StrategyInUseError, + StrategyNotFoundError, + StrategySetService, +) + +router = APIRouter(tags=["v1-strategies"]) + + +def _get_service(settings: Settings) -> tuple[StrategySetService, object]: + repository = get_portfolio_repository_from_settings(settings) + return StrategySetService(repository), repository + + +def _close_repository(repository: object) -> None: + if callable(getattr(type(repository), "close", None)): + repository.close() + + +def _canonical_config(config: dict[str, object]) -> str: + return json.dumps(config, sort_keys=True, ensure_ascii=True, separators=(",", ":")) + + +def _response(strategy: StrategySet) -> StrategyResponse: + try: + parsed = json.loads(strategy.config) if strategy.config else {} + except (TypeError, ValueError, json.JSONDecodeError): + parsed = {} + return StrategyResponse( + strategy_id=strategy.id, + name=strategy.name, + config=parsed if isinstance(parsed, dict) else {}, + status=strategy.status, + revision=strategy.revision, + created_at=strategy.created_at, + updated_at=strategy.updated_at, + ) + + +def _raise_strategy_error(exc: ValueError) -> None: + if isinstance(exc, StrategyNotFoundError): + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc + if isinstance(exc, (StrategyInactiveError, StrategyInUseError, StrategyConflictError)): + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc + raise exc + + +@router.get("/v1/strategies", response_model=list[StrategyResponse]) +def list_active_strategies( + _auth: AuthContext = Depends(require_read), + settings: Settings = Depends(get_settings), +) -> list[StrategyResponse]: + service, repository = _get_service(settings) + try: + return [_response(item) for item in service.list(include_inactive=False)] + finally: + _close_repository(repository) + + +@router.get("/v1/strategies/{strategy_id}", response_model=StrategyResponse) +def get_active_strategy( + strategy_id: str, + _auth: AuthContext = Depends(require_read), + settings: Settings = Depends(get_settings), +) -> StrategyResponse: + service, repository = _get_service(settings) + try: + try: + return _response(service.get(strategy_id, active_only=True)) + except ValueError as exc: + _raise_strategy_error(exc) + finally: + _close_repository(repository) + + +@router.get("/v1/admin/strategies", response_model=list[StrategyResponse]) +def list_admin_strategies( + _auth: AuthContext = Depends(require_admin), + settings: Settings = Depends(get_settings), +) -> list[StrategyResponse]: + service, repository = _get_service(settings) + try: + return [_response(item) for item in service.list(include_inactive=True)] + finally: + _close_repository(repository) + + +@router.get("/v1/admin/strategies/{strategy_id}", response_model=StrategyResponse) +def get_admin_strategy( + strategy_id: str, + _auth: AuthContext = Depends(require_admin), + settings: Settings = Depends(get_settings), +) -> StrategyResponse: + service, repository = _get_service(settings) + try: + try: + return _response(service.get(strategy_id)) + except ValueError as exc: + _raise_strategy_error(exc) + finally: + _close_repository(repository) + + +@router.post( + "/v1/admin/strategies", + response_model=StrategyResponse, + status_code=status.HTTP_201_CREATED, +) +def create_admin_strategy( + body: StrategyCreateRequest, + _auth: AuthContext = Depends(require_admin), + settings: Settings = Depends(get_settings), +) -> StrategyResponse: + service, repository = _get_service(settings) + try: + strategy = service.create( + CreateStrategySetRequest( + name=body.name, + config=_canonical_config(body.config), + status=body.status, + ) + ) + return _response(strategy) + finally: + _close_repository(repository) + + +@router.patch("/v1/admin/strategies/{strategy_id}", response_model=StrategyResponse) +def update_admin_strategy( + strategy_id: str, + body: StrategyUpdateRequest, + _auth: AuthContext = Depends(require_admin), + settings: Settings = Depends(get_settings), +) -> StrategyResponse: + service, repository = _get_service(settings) + try: + try: + strategy = service.update( + strategy_id, + UpdateStrategySetRequest( + name=body.name, + config=_canonical_config(body.config) if body.config is not None else None, + status=body.status, + ), + ) + return _response(strategy) + except ValueError as exc: + _raise_strategy_error(exc) + finally: + _close_repository(repository) + + +@router.delete( + "/v1/admin/strategies/{strategy_id}", + status_code=status.HTTP_204_NO_CONTENT, +) +def delete_admin_strategy( + strategy_id: str, + _auth: AuthContext = Depends(require_admin), + settings: Settings = Depends(get_settings), +) -> Response: + service, repository = _get_service(settings) + try: + try: + service.delete(strategy_id) + except ValueError as exc: + _raise_strategy_error(exc) + finally: + _close_repository(repository) + return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/app/cli/commands/portfolio.py b/app/cli/commands/portfolio.py index 07e0a19..83e157a 100644 --- a/app/cli/commands/portfolio.py +++ b/app/cli/commands/portfolio.py @@ -150,8 +150,7 @@ def create_portfolio( asset_quantity: float = typer.Option(0.0, min=0.0, help="Starting asset quantity."), asset_price: float = typer.Option(0.0, min=0.0, help="Starting asset price."), profit_buffer: float = typer.Option(0.0, min=0.0, help="Initial profit buffer."), - strategy_name: str = typer.Option("target_value", help="Strategy name."), - strategy_params: str = typer.Option("{}", help="Strategy params as JSON object."), + strategy_id: str = typer.Option(..., "--strategy-id", help="Active strategy catalog ID."), force: bool = typer.Option(False, "--force", help="Skip confirmation prompt."), ) -> None: _validate_portfolio_name(name, ctx.obj.console) @@ -179,8 +178,7 @@ def create_portfolio( asset_quantity=asset_quantity, asset_price=asset_price, profit_buffer=profit_buffer, - strategy_name=strategy_name, - strategy_params=_parse_strategy_params(strategy_params), + strategy_id=strategy_id, ), owner_user_id=owner.id, ) @@ -198,8 +196,7 @@ def update_portfolio( broker: str | None = typer.Option(None, help="Broker code."), status: str | None = typer.Option(None, help="Portfolio status."), new_name: str | None = typer.Option(None, "--new-name", help="New portfolio name (requires --force or confirm)."), - strategy_name: str | None = typer.Option(None, help="Strategy name."), - strategy_params: str | None = typer.Option(None, help="Strategy params as JSON object."), + strategy_id: str | None = typer.Option(None, "--strategy-id", help="New active strategy catalog ID."), asset_price: float | None = typer.Option(None, min=0.0, help="Current asset price."), daemon_id: str | None = typer.Option(None, "--daemon-id", help="Assign portfolio to this daemon (managed_by_daemon_id)."), force: bool = typer.Option(False, "--force", help="Skip confirmation prompt."), @@ -211,8 +208,6 @@ def update_portfolio( else: _confirm(force, f"Update portfolio {name!r} (user: {user})?") - parsed_strategy_params = None if strategy_params is None else _parse_strategy_params(strategy_params) - def _callback(service, repository, app_context): portfolio_id = _resolve_portfolio_id(repository, name, user) return service.update_portfolio( @@ -221,8 +216,7 @@ def update_portfolio( broker=broker, status=status, name=new_name, - strategy_name=strategy_name, - strategy_params=parsed_strategy_params, + strategy_id=strategy_id, asset_price=asset_price, managed_by_daemon_id=daemon_id, ), diff --git a/app/cli/commands/strategy.py b/app/cli/commands/strategy.py index 9cef3c2..0f82a3c 100644 --- a/app/cli/commands/strategy.py +++ b/app/cli/commands/strategy.py @@ -1,14 +1,16 @@ -"""Strategy Set CLI commands.""" +"""Strategy catalog CLI commands.""" from __future__ import annotations import typer +from typing import Literal -from app.cli.core.utils import execute_command, render_output +from app.cli.core.utils import execute_command +from app.schemas.portfolio import CreateStrategySetRequest, UpdateStrategySetRequest from app.services.strategy_set_service import StrategySetService -strategy_app = typer.Typer(help="Strategy set management commands.") +strategy_app = typer.Typer(help="Strategy catalog management commands.") def _with_service(ctx: typer.Context, callback): @@ -16,37 +18,43 @@ def _with_service(ctx: typer.Context, callback): 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 + return execute_command(app_context.console, callback=lambda: callback(service)) finally: if callable(getattr(type(repository), "close", None)): repository.close() -def _print_strategy_set_table(strategy_sets): - """Print strategy sets as a simple table.""" +def _print_strategy_set_table(strategy_sets) -> None: if not strategy_sets: - print("No strategy sets found.") + print("No strategies 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}") + + print(f"{'ID':<40} {'Name':<20} {'Status':<10} {'Revision':<8} {'Nick':<28} {'Updated At':<26}") + print("-" * 138) + for strategy in strategy_sets: + updated_at = strategy.updated_at.isoformat() if strategy.updated_at else "N/A" + print( + f"{strategy.id:<40} {strategy.name:<20} {strategy.status:<10} " + f"{strategy.revision:<8} {strategy.nick:<28} {updated_at:<26}" + ) + + +def _print_strategy(strategy, *, action: str) -> None: + print(f"Strategy {action}:") + print(f" ID: {strategy.id}") + print(f" Name: {strategy.name}") + print(f" Nick: {strategy.nick}") + print(f" Status: {strategy.status}") + print(f" Revision: {strategy.revision}") + print(f" Created At: {strategy.created_at.isoformat() if strategy.created_at else 'N/A'}") + print(f" Updated At: {strategy.updated_at.isoformat() if strategy.updated_at else 'N/A'}") @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) + """List active and inactive catalog strategies.""" + + _with_service(ctx, lambda service: _print_strategy_set_table(service.list(include_inactive=True))) @strategy_app.command("create") @@ -54,68 +62,52 @@ 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."), + status: Literal["active", "inactive"] = typer.Option("active", help="Strategy status: active or inactive."), ) -> 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'}") - + """Create a strategy or return the existing matching catalog entry.""" + + def cmd(service: StrategySetService) -> None: + strategy = service.create(CreateStrategySetRequest(name=name, config=config, status=status)) + _print_strategy(strategy, action="created/retrieved") + _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)."), + strategy_set_id: str = typer.Argument(..., help="Strategy ID to update."), + name: str | None = typer.Option(None, help="New strategy name."), + config: str | None = typer.Option(None, help="New strategy config as JSON string."), + status: Literal["active", "inactive"] | None = typer.Option(None, help="New strategy status: active or inactive."), ) -> 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'}") - + """Update catalog fields while preserving the strategy ID.""" + + def cmd(service: StrategySetService) -> None: + if name is None and config is None and status is None: + raise ValueError("at least one of --name, --config, or --status is required") + strategy = service.update( + strategy_set_id, + UpdateStrategySetRequest(name=name, config=config, status=status), + ) + _print_strategy(strategy, action="updated") + _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."), + strategy_set_id: str = typer.Argument(..., help="Strategy 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) - + """Delete an unreferenced strategy.""" + + def cmd(service: StrategySetService) -> None: + if not force and not typer.confirm(f"Delete strategy {strategy_set_id}?"): + print("Cancelled.") + return + service.delete(strategy_set_id) + print(f"Strategy {strategy_set_id} deleted.") + _with_service(ctx, cmd) diff --git a/app/domain/errors.py b/app/domain/errors.py new file mode 100644 index 0000000..849c2fb --- /dev/null +++ b/app/domain/errors.py @@ -0,0 +1,9 @@ +"""Shared strategy-catalog domain errors used across API, service, and persistence layers.""" + + +class StrategyNotFoundError(ValueError): + """Raised when a referenced strategy ID does not exist.""" + + +class StrategyInactiveError(ValueError): + """Raised when a new portfolio selection targets an inactive strategy.""" diff --git a/app/domain/models.py b/app/domain/models.py index cad5b8d..2d81ba8 100644 --- a/app/domain/models.py +++ b/app/domain/models.py @@ -46,7 +46,10 @@ class StrategySet: name: str config: str nick: str + status: str = "active" + revision: int = 1 created_at: datetime | None = None + updated_at: datetime | None = None @dataclass @@ -118,6 +121,7 @@ class Portfolio: active_strategy_id: str | None = None strategy_name: str | None = None # REQ-055: strategy_sets.name JOIN 결과 strategy_params: dict[str, Any] = field(default_factory=dict) # REQ-055: strategy_sets.config 파싱 결과 + strategy_revision: int | None = None markets: list[str] = field(default_factory=list) # REQ-055: assets.markets JOIN 결과 avabile_strategies: list[str] | None = None created_at: datetime | None = None diff --git a/app/main.py b/app/main.py index 66511d9..e2b60cb 100644 --- a/app/main.py +++ b/app/main.py @@ -9,13 +9,14 @@ from fastapi.middleware.cors import CORSMiddleware from app.api.health import router as health_router from app.api.auth_routes import router as auth_router from app.api.portfolios import router as portfolio_router -from app.api.strategy_sets import router as strategy_sets_router + from app.api.users import router as users_router from app.api.v1_admin_daemons import router as v1_admin_daemons_router from app.api.v1_assets import router as v1_assets_router from app.api.v1_daemon_credential import router as v1_daemon_credential_router from app.api.v1_daemons import router as v1_daemons_router from app.api.v1_portfolios import router as v1_portfolios_router +from app.api.v1_strategies import router as v1_strategies_router from app.api.v1_tags import router as v1_tags_router from app.api.v1_user_info import router as v1_user_info_router from app.core.config import Settings, get_settings @@ -81,13 +82,14 @@ def create_app( app.include_router(health_router) app.include_router(auth_router) app.include_router(portfolio_router) - app.include_router(strategy_sets_router) + app.include_router(users_router) app.include_router(v1_admin_daemons_router) app.include_router(v1_assets_router) app.include_router(v1_daemon_credential_router) app.include_router(v1_daemons_router) app.include_router(v1_portfolios_router) + app.include_router(v1_strategies_router) app.include_router(v1_tags_router) app.include_router(v1_user_info_router) register_exception_handlers(app) diff --git a/app/repositories/portfolio_repository.py b/app/repositories/portfolio_repository.py index d37d715..72661df 100644 --- a/app/repositories/portfolio_repository.py +++ b/app/repositories/portfolio_repository.py @@ -242,9 +242,20 @@ class PortfolioRepository(Protocol): def get_or_create_strategy_set(self, name: str, config: str) -> StrategySet: ... + def create_strategy_set(self, name: str, config: str, *, status: str = "active") -> StrategySet: ... + def get_strategy_set(self, id: str) -> StrategySet | None: ... - def list_strategy_sets(self) -> list[StrategySet]: ... + def list_strategy_sets(self, *, include_inactive: bool = True) -> list[StrategySet]: ... + + def update_strategy_set( + self, + id: str, + *, + name: str | None = None, + config: str | None = None, + status: str | None = None, + ) -> StrategySet: ... def delete_strategy_set(self, id: str) -> None: ... diff --git a/app/repositories/postgres.py b/app/repositories/postgres.py index 24335ec..b9755b3 100644 --- a/app/repositories/postgres.py +++ b/app/repositories/postgres.py @@ -10,13 +10,14 @@ from dataclasses import dataclass from datetime import datetime, timezone from typing import Any -logger = logging.getLogger(__name__) - import psycopg from psycopg import sql +from app.domain.errors import StrategyInactiveError, StrategyNotFoundError from app.domain.models import Asset, AuthToken, BrokerCredential, Daemon, LedgerEntry, Portfolio, PortfolioComment, PortfolioTag, StrategySet, Tag, TokenPrincipal, User +logger = logging.getLogger(__name__) + _SELECT_USER = """ SELECT id, @@ -71,7 +72,8 @@ _SELECT_PORTFOLIO = """ a.display_name, ss.name, ss.config, - a.markets + a.markets, + ss.revision FROM portfolios p JOIN assets a ON p.asset_id = a.asset_id LEFT JOIN strategy_sets ss ON p.active_strategy_id = ss.id @@ -144,6 +146,19 @@ class PostgresPortfolioRepository: cur.execute("SELECT 1") cur.fetchone() + @staticmethod + def _lock_active_strategy(cur: psycopg.Cursor, strategy_id: str) -> None: + """Serialize new selections with strategy PATCH/DELETE and enforce active status.""" + cur.execute( + "SELECT status FROM strategy_sets WHERE id = %s FOR SHARE", + (strategy_id,), + ) + row = cur.fetchone() + if row is None: + raise StrategyNotFoundError(f"strategy not found: {strategy_id}") + if row[0] != "active": + raise StrategyInactiveError(f"strategy is inactive: {strategy_id}") + def ensure_schema(self) -> None: conn = self._connect() with conn.cursor() as cur: @@ -208,11 +223,17 @@ class PostgresPortfolioRepository: name TEXT NOT NULL, config TEXT NOT NULL DEFAULT '', nick TEXT NOT NULL UNIQUE, + status TEXT NOT NULL DEFAULT 'active', + revision INTEGER NOT NULL DEFAULT 1, created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), UNIQUE (name, config) ) """ ) + cur.execute("ALTER TABLE strategy_sets ADD COLUMN IF NOT EXISTS status TEXT NOT NULL DEFAULT 'active'") + cur.execute("ALTER TABLE strategy_sets ADD COLUMN IF NOT EXISTS revision INTEGER NOT NULL DEFAULT 1") + cur.execute("ALTER TABLE strategy_sets ADD COLUMN IF NOT EXISTS updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW()") cur.execute( """ CREATE TABLE IF NOT EXISTS assets ( @@ -530,6 +551,7 @@ class PostgresPortfolioRepository: # REQ-055: markets (row[21] = a.markets from assets JOIN) _markets_raw = row[21] if len(row) > 21 else None markets: list[str] = list(_markets_raw) if _markets_raw else [] + strategy_revision = int(row[22]) if len(row) > 22 and row[22] is not None else None return Portfolio( portfolio_id=row[0], @@ -554,6 +576,7 @@ class PostgresPortfolioRepository: display_name=row[18] if len(row) > 18 else None, strategy_name=strategy_name, strategy_params=strategy_params, + strategy_revision=strategy_revision, markets=markets, ) @@ -578,6 +601,21 @@ class PostgresPortfolioRepository: tag_ids=[str(t) for t in row[7]] if len(row) > 7 and row[7] is not None else [], ) + @staticmethod + def _row_to_strategy_set(row: Any) -> StrategySet: + if row is None: + raise ValueError("전략을 찾을 수 없습니다") + return StrategySet( + id=str(row[0]), + name=str(row[1]), + config=str(row[2]), + nick=str(row[3]), + status=str(row[4]), + revision=int(row[5]), + created_at=row[6], + updated_at=row[7], + ) + @staticmethod def _row_to_tag(row: Any) -> Tag: if row is None: @@ -1037,7 +1075,10 @@ class PostgresPortfolioRepository: initial_cash: float = 0.0, ) -> Portfolio: self.ensure_schema() - with self._connect().cursor() as cur: + 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) cur.execute("SELECT asset_id FROM assets WHERE symbol = %s", (symbol,)) row = cur.fetchone() if row is None: @@ -1089,7 +1130,10 @@ class PostgresPortfolioRepository: avabile_strategies: list[str] | None, ) -> Portfolio: self.ensure_schema() - with self._connect().cursor() as cur: + 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) cur.execute("SELECT 1 FROM portfolios WHERE portfolio_id = %s", (portfolio_id,)) if cur.fetchone() is not None: raise ValueError(f"이미 존재하는 포트폴리오입니다: {portfolio_id}") @@ -1146,7 +1190,11 @@ class PostgresPortfolioRepository: return self.get(portfolio_id) assignments.append("updated_at = NOW()") - with self._connect().cursor() as cur: + conn = self._connect() + with conn.transaction(), conn.cursor() as cur: + strategy_id = fields.get("active_strategy_id") + if isinstance(strategy_id, str): + self._lock_active_strategy(cur, strategy_id) cur.execute( f""" UPDATE portfolios @@ -1276,54 +1324,120 @@ class PostgresPortfolioRepository: raise ValueError("태그를 찾을 수 없습니다") def get_or_create_strategy_set(self, name: str, config: str) -> StrategySet: + return self.create_strategy_set(name, config) + + def create_strategy_set(self, name: str, config: str, *, status: str = "active") -> StrategySet: self.ensure_schema() nick = f"{name}:{hashlib.sha1(config.encode('utf-8')).hexdigest()[:12]}" with self._connect().cursor() as cur: new_id = str(uuid.uuid4()) cur.execute( """ - INSERT INTO strategy_sets (id, name, config, nick) - VALUES (%s, %s, %s, %s) + INSERT INTO strategy_sets (id, name, config, nick, status) + VALUES (%s, %s, %s, %s, %s) ON CONFLICT (name, config) DO NOTHING """, - (new_id, name, config, nick), + (new_id, name, config, nick, status), ) cur.execute( """ - SELECT id, name, config, nick, created_at + SELECT id, name, config, nick, status, revision, created_at, updated_at FROM strategy_sets WHERE name = %s AND config = %s """, (name, config), ) row = cur.fetchone() - return StrategySet(id=row[0], name=row[1], config=row[2], nick=row[3], created_at=row[4]) + return self._row_to_strategy_set(row) def get_strategy_set(self, id: str) -> StrategySet | None: self.ensure_schema() with self._connect().cursor() as cur: cur.execute( - "SELECT id, name, config, nick, created_at FROM strategy_sets WHERE id = %s", + "SELECT id, name, config, nick, status, revision, created_at, updated_at FROM strategy_sets WHERE id = %s", (id,), ) row = cur.fetchone() if row is None: return None - return StrategySet(id=row[0], name=row[1], config=row[2], nick=row[3], created_at=row[4]) + return self._row_to_strategy_set(row) - def list_strategy_sets(self) -> list[StrategySet]: + def list_strategy_sets(self, *, include_inactive: bool = True) -> 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" - ) + where = "" if include_inactive else "WHERE status = 'active'" + cur.execute(f""" + SELECT id, name, config, nick, status, revision, created_at, updated_at + FROM strategy_sets + {where} + ORDER BY created_at DESC, id ASC + """) 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] + return [self._row_to_strategy_set(row) for row in rows] + + def update_strategy_set( + self, + id: str, + *, + name: str | None = None, + config: str | None = None, + status: str | None = None, + ) -> StrategySet: + self.ensure_schema() + conn = self._connect() + try: + with conn.transaction(), conn.cursor() as cur: + cur.execute( + """ + SELECT id, name, config, nick, status, revision, created_at, updated_at + FROM strategy_sets + WHERE id = %s + FOR UPDATE + """, + (id,), + ) + row = cur.fetchone() + if row is None: + raise ValueError("전략을 찾을 수 없습니다") + current = self._row_to_strategy_set(row) + final_name = name if name is not None else current.name + final_config = config if config is not None else current.config + final_status = status if status is not None else current.status + if ( + final_name == current.name + and final_config == current.config + and final_status == current.status + ): + return current + nick = f"{final_name}:{hashlib.sha1(final_config.encode('utf-8')).hexdigest()[:12]}" + cur.execute( + """ + UPDATE strategy_sets + SET name = %s, + config = %s, + nick = %s, + status = %s, + revision = revision + 1, + updated_at = NOW() + WHERE id = %s + RETURNING id, name, config, nick, status, revision, created_at, updated_at + """, + (final_name, final_config, nick, final_status, id), + ) + updated = cur.fetchone() + except psycopg.errors.UniqueViolation as exc: + raise ValueError("동일한 name/config 전략이 이미 존재합니다") from exc + if updated is None: + raise ValueError("전략을 찾을 수 없습니다") + return self._row_to_strategy_set(updated) 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 + conn = self._connect() + with conn.transaction(), conn.cursor() as cur: + cur.execute("SELECT id FROM strategy_sets WHERE id = %s FOR UPDATE", (id,)) + if cur.fetchone() is None: + raise ValueError("전략을 찾을 수 없습니다") cur.execute( """ SELECT COUNT(*) FROM portfolios @@ -1335,9 +1449,9 @@ class PostgresPortfolioRepository: 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,)) + cur.execute("DELETE FROM strategy_sets WHERE id = %s RETURNING id", (id,)) + if cur.fetchone() is None: + raise ValueError("전략을 찾을 수 없습니다") def save( self, @@ -1355,31 +1469,24 @@ class PostgresPortfolioRepository: ) -> None: self.ensure_schema() trade_time = datetime.now(timezone.utc) - with self._connect().cursor() as cur: + conn = self._connect() + with conn.transaction(), conn.cursor() as cur: cur.execute( """ UPDATE portfolios - SET broker = %s, - status = %s, - cash_balance = %s, + SET cash_balance = %s, asset_quantity = %s, asset_price = %s, profit_buffer = %s, - active_strategy_id = %s, - avabile_strategies = %s, updated_at = %s, last_trade_at = CASE WHEN %s IN ('BUY', 'SELL') THEN %s ELSE last_trade_at END WHERE portfolio_id = %s """, ( - portfolio.broker, - portfolio.status, float(portfolio.cash_balance), float(portfolio.asset_quantity), float(current_price), float(portfolio.profit_buffer), - portfolio.active_strategy_id, - portfolio.avabile_strategies, trade_time, action, trade_time, diff --git a/app/schemas/portfolio.py b/app/schemas/portfolio.py index f04d15f..c238f10 100644 --- a/app/schemas/portfolio.py +++ b/app/schemas/portfolio.py @@ -17,7 +17,10 @@ class StrategySetResponse(BaseModel): name: str config: str nick: str + status: Literal["active", "inactive"] = "active" + revision: int = 1 created_at: datetime | None = None + updated_at: datetime | None = None class LedgerEntryResponse(BaseModel): @@ -55,6 +58,9 @@ class PortfolioSummary(BaseModel): price: float = 0.0 profit_buffer: float = 0.0 active_strategy_id: str | None = None + strategy_name: str | None = None + strategy_params: dict[str, Any] = Field(default_factory=dict) + strategy_revision: int | None = Field(default=None, ge=1) avabile_strategies: list[str] | None = None managed_by_daemon_id: str | None = None created_at: datetime | None = None @@ -74,12 +80,11 @@ class PortfolioDetailResponse(BaseModel): class CreatePortfolioRequest(PortfolioBase): + model_config = ConfigDict(extra="forbid") + name: str # required, overrides Optional from PortfolioBase; used as (owner_user_id, name) identifier asset_id: str - strategy_name: str = "target_value" - strategy_params: dict[str, Any] = Field(default_factory=dict) - active_strategy_id: str | None = None - avabile_strategies: list[str] | None = None + strategy_id: str = Field(..., min_length=1) cash_balance: float = Field(default=0.0, ge=0.0) asset_quantity: float = Field(default=0.0, ge=0.0) asset_price: float = Field(default=0.0, ge=0.0) @@ -96,13 +101,12 @@ class CreatePortfolioRequest(PortfolioBase): class UpdatePortfolioRequest(BaseModel): + model_config = ConfigDict(extra="forbid") + broker: str | None = None status: str | None = None name: str | None = None - strategy_name: str | None = None - strategy_params: dict[str, Any] | None = None - active_strategy_id: str | None = None - avabile_strategies: list[str] | None = None + strategy_id: str | None = Field(default=None, min_length=1) asset_price: float | None = Field(default=None, ge=0.0) managed_by_daemon_id: str | None = None @@ -179,11 +183,17 @@ class ErrorResponse(BaseModel): class CreateStrategySetRequest(BaseModel): + model_config = ConfigDict(extra="forbid") + name: str config: str + status: Literal["active", "inactive"] = "active" class UpdateStrategySetRequest(BaseModel): + model_config = ConfigDict(extra="forbid") + name: str | None = None config: str | None = None + status: Literal["active", "inactive"] | None = None diff --git a/app/schemas/v1.py b/app/schemas/v1.py index 23ed964..caf1c99 100644 --- a/app/schemas/v1.py +++ b/app/schemas/v1.py @@ -233,6 +233,8 @@ class PortfolioReturnSummaryResponse(BaseModel): # --------------------------------------------------------------------------- class PortfolioCreateRequest(BaseModel): + model_config = ConfigDict(extra="forbid") + asset_id: str = Field(..., min_length=1) broker: str = Field(..., min_length=1) name: str | None = None @@ -240,18 +242,18 @@ class PortfolioCreateRequest(BaseModel): asset_quantity: float = Field(default=0.0, ge=0.0) asset_price: float = Field(default=0.0, ge=0.0) profit_buffer: float = Field(default=0.0, ge=0.0) - strategy_name: str | None = None - strategy_params: dict[str, Any] = Field(default_factory=dict) + strategy_id: str = Field(..., min_length=1) managed_by_daemon_id: str | None = None user_id: str | None = None # admin-only: override owner class PortfolioUpdateRequest(BaseModel): + model_config = ConfigDict(extra="forbid") + broker: str | None = None status: str | None = None name: str | None = None - strategy_name: str | None = None - strategy_params: dict[str, Any] | None = None + strategy_id: str | None = Field(default=None, min_length=1) asset_price: float | None = Field(default=None, ge=0.0) managed_by_daemon_id: str | None = None @@ -285,6 +287,7 @@ class PortfolioItem(BaseModel): active_strategy_id: str | None = None strategy_name: str | None = None strategy_params: dict[str, Any] = Field(default_factory=dict) + strategy_revision: int | None = Field(default=None, ge=1) markets: list[str] = Field(default_factory=list) @@ -293,6 +296,45 @@ class PortfolioListResponse(BaseModel): total: int +# --------------------------------------------------------------------------- +# GET /v1/strategies, CRUD /v1/admin/strategies +# --------------------------------------------------------------------------- + +StrategyStatus = Literal["active", "inactive"] + + +class StrategyResponse(BaseModel): + strategy_id: str + name: str + config: dict[str, Any] = Field(default_factory=dict) + status: StrategyStatus + revision: int = Field(ge=1) + created_at: datetime | None = None + updated_at: datetime | None = None + + +class StrategyCreateRequest(BaseModel): + model_config = ConfigDict(extra="forbid") + + name: str = Field(..., min_length=1) + config: dict[str, Any] = Field(default_factory=dict) + status: StrategyStatus = "active" + + +class StrategyUpdateRequest(BaseModel): + model_config = ConfigDict(extra="forbid") + + name: str | None = Field(default=None, min_length=1) + config: dict[str, Any] | None = None + status: StrategyStatus | None = None + + @model_validator(mode="after") + def require_change(self) -> "StrategyUpdateRequest": + if not self.model_fields_set: + raise ValueError("at least one field is required") + return self + + # --------------------------------------------------------------------------- # GET /v1/portfolios/{id}/tags, CRUD /v1/portfolios/{id}/comments (REQ-030, REQ-031) # --------------------------------------------------------------------------- diff --git a/app/services/portfolio_service.py b/app/services/portfolio_service.py index f0dd035..adad524 100644 --- a/app/services/portfolio_service.py +++ b/app/services/portfolio_service.py @@ -36,6 +36,7 @@ from app.schemas.portfolio import ( TransferRequest, UpdatePortfolioRequest, ) +from app.services.strategy_set_service import StrategyInactiveError, StrategyNotFoundError def _infer_symbol(portfolio_id: str) -> str: @@ -59,6 +60,9 @@ def _to_portfolio_summary(portfolio: Portfolio): price=portfolio.price, profit_buffer=portfolio.profit_buffer, active_strategy_id=portfolio.active_strategy_id, + strategy_name=portfolio.strategy_name, + strategy_params=portfolio.strategy_params, + strategy_revision=portfolio.strategy_revision, avabile_strategies=portfolio.avabile_strategies, managed_by_daemon_id=portfolio.managed_by_daemon_id, created_at=portfolio.created_at, @@ -92,7 +96,10 @@ def _to_strategy_set_response(strategy_set: StrategySet | None): name=strategy_set.name, config=strategy_set.config, nick=strategy_set.nick, + status=strategy_set.status, + revision=strategy_set.revision, created_at=strategy_set.created_at, + updated_at=strategy_set.updated_at, ) @@ -213,14 +220,12 @@ class PortfolioService: if asset.status != "active": raise ValueError(f"asset is inactive: {payload.asset_id}") - active_strategy_id = payload.active_strategy_id - if active_strategy_id is None: - strategy_set = self._repository.get_or_create_strategy_set( - payload.strategy_name, - json.dumps(payload.strategy_params, sort_keys=True, ensure_ascii=True), - ) - active_strategy_id = strategy_set.id - avabile_strategies = _merge_strategy_ids(active_strategy_id, payload.avabile_strategies) + strategy_set = self._repository.get_strategy_set(payload.strategy_id) + if strategy_set is None: + raise StrategyNotFoundError(f"strategy not found: {payload.strategy_id}") + if strategy_set.status != "active": + raise StrategyInactiveError(f"strategy is inactive: {payload.strategy_id}") + active_strategy_id = strategy_set.id portfolio = self._repository.create_portfolio( str(uuid.uuid4()), name=payload.name, @@ -233,7 +238,7 @@ class PortfolioService: asset_price=payload.asset_price, profit_buffer=payload.profit_buffer, active_strategy_id=active_strategy_id, - avabile_strategies=avabile_strategies, + avabile_strategies=[active_strategy_id], ) return self.get_portfolio_status(portfolio_id=portfolio.portfolio_id, recent_trades=5) @@ -241,42 +246,19 @@ class PortfolioService: fields = payload.model_dump(exclude_none=True) current = self._repository.get(portfolio_id) - if "strategy_name" in fields or "strategy_params" in fields: - current_strategy_set = None - if current.active_strategy_id: - current_strategy_set = self._repository.get_strategy_set(current.active_strategy_id) - - strategy_name = fields.get("strategy_name") - if strategy_name is None and current_strategy_set is not None: - strategy_name = current_strategy_set.name - if strategy_name is None: - raise ValueError("strategy_name is required when no active strategy is set") - - strategy_params = fields.get("strategy_params") - if strategy_params is None and current_strategy_set is not None: - strategy_params = _parse_strategy_config(current_strategy_set.config) - if strategy_params is None: - strategy_params = {} - - strategy_set = self._repository.get_or_create_strategy_set( - strategy_name, - json.dumps(strategy_params, sort_keys=True, ensure_ascii=True), - ) - fields["active_strategy_id"] = strategy_set.id + if "strategy_id" in fields: + strategy_id = str(fields.pop("strategy_id")) + strategy_set = self._repository.get_strategy_set(strategy_id) + if strategy_set is None: + raise StrategyNotFoundError(f"strategy not found: {strategy_id}") + if strategy_set.status != "active": + raise StrategyInactiveError(f"strategy is inactive: {strategy_id}") + fields["active_strategy_id"] = strategy_id fields["avabile_strategies"] = _merge_strategy_ids( - strategy_set.id, - fields.get("avabile_strategies", current.avabile_strategies), + strategy_id, + current.avabile_strategies, ) - elif "active_strategy_id" in fields: - fields["avabile_strategies"] = _merge_strategy_ids( - fields.get("active_strategy_id"), - fields.get("avabile_strategies", current.avabile_strategies), - ) - - fields.pop("strategy_name", None) - fields.pop("strategy_params", None) - self._repository.update_portfolio(portfolio_id, **fields) return self.get_portfolio_status(portfolio_id=portfolio_id, recent_trades=5) diff --git a/app/services/strategy_set_service.py b/app/services/strategy_set_service.py index 50f0d0f..d654c35 100644 --- a/app/services/strategy_set_service.py +++ b/app/services/strategy_set_service.py @@ -1,16 +1,28 @@ -"""Strategy set service orchestration.""" +"""Strategy catalog service orchestration.""" from __future__ import annotations +from collections.abc import Callable +from typing import cast + from fastapi import Depends from app.core.config import Settings, get_settings +from app.domain.errors import StrategyInactiveError, StrategyNotFoundError 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 +class StrategyInUseError(ValueError): + """The strategy is referenced by at least one portfolio.""" + + +class StrategyConflictError(ValueError): + """The requested catalog mutation conflicts with another strategy.""" + + def get_strategy_set_repository(settings: Settings = Depends(get_settings)) -> PortfolioRepository: return get_portfolio_repository_from_settings(settings) @@ -19,56 +31,58 @@ 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 list(self, *, include_inactive: bool = True) -> list[StrategySet]: + return self._repository.list_strategy_sets(include_inactive=include_inactive) - 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 get(self, strategy_set_id: str, *, active_only: bool = False) -> StrategySet: + strategy = self._repository.get_strategy_set(strategy_set_id) + if strategy is None: + raise StrategyNotFoundError(f"strategy not found: {strategy_set_id}") + if active_only and strategy.status != "active": + raise StrategyInactiveError(f"strategy is inactive: {strategy_set_id}") + return strategy + + def require_active(self, strategy_set_id: str) -> StrategySet: + return self.get(strategy_set_id, active_only=True) 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) + create_strategy = cast( + Callable[..., StrategySet] | None, + getattr(self._repository, "create_strategy_set", None), + ) + if create_strategy is not None: + strategy = create_strategy(payload.name, payload.config, status=payload.status) + else: + strategy = self._repository.get_or_create_strategy_set(payload.name, payload.config) + if payload.status != strategy.status: + strategy = self.update( + strategy.id, + UpdateStrategySetRequest(status=payload.status), + ) + return strategy 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 + try: + return self._repository.update_strategy_set( + strategy_set_id, + name=payload.name, + config=payload.config, + status=payload.status, + ) + except ValueError as exc: + detail = str(exc) + if "찾을 수 없습니다" in detail: + raise StrategyNotFoundError(f"strategy not found: {strategy_set_id}") from exc + raise StrategyConflictError(detail) from exc 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) + self.get(strategy_set_id) + try: + self._repository.delete_strategy_set(strategy_set_id) + except ValueError as exc: + detail = str(exc) + if "in use" in detail: + raise StrategyInUseError(detail) from exc + if "찾을 수 없습니다" in detail: + raise StrategyNotFoundError(f"strategy not found: {strategy_set_id}") from exc + raise diff --git a/docs/data-model.md b/docs/data-model.md index f2f9def..5c47198 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -102,6 +102,28 @@ - Only `status="active"` assets can be referenced when creating a new portfolio. - Deactivated assets do not affect existing portfolios; only new creation is blocked. +## Entity: `StrategySet` + +- Purpose: stable, reusable strategy catalog entry selected by portfolio mutations through its ID +- Source of Truth: `strategy_sets`; migration `20260814_0029` adds lifecycle and revision fields and seeds/reuses the canonical `target_value` / `{}` entry +- Lifecycle: active → updated with monotonically increasing revision → optionally inactive → deleted only when unreferenced + +### Key Fields + +- id :: text :: stable strategy ID; never changes during PATCH +- name/config :: text :: strategy identity/configuration pair; unique together +- nick :: text :: unique derived display key (`name:sha1(config)[:12]`), recomputed from the latest row state under the PATCH row lock +- status :: text :: `active` or `inactive`; inactive entries remain valid historical references but cannot be newly assigned +- revision :: integer :: starts at 1 and increments once per effective mutation; no-op PATCH does not increment +- created_at/updated_at :: timestamptz :: creation and last effective mutation timestamps + +### Relationships and Deletion + +- `portfolios.active_strategy_id` and entries in `portfolios.avabile_strategies` reference strategy IDs. +- New selection uses a strategy `FOR SHARE` lock in the same transaction as portfolio persistence; PATCH/DELETE use `FOR UPDATE` so concurrent mutation, inactivation, and deletion serialize. +- Deletion is rejected while either reference form exists. The reference check and delete share one transaction, preventing check-then-delete orphan races. Inactivation does not rewrite existing portfolio references. +- Trade snapshot persistence does not write either strategy-selection column. Strategy selection is owned only by portfolio create/update paths, so stale BUY/SELL state cannot resurrect a deleted strategy reference. + ## Entity: `Portfolio` - Purpose: represent an owned investment portfolio with balances, strategy references, tags, comments, and trading history @@ -116,7 +138,8 @@ - symbol :: text :: display symbol populated via JOIN on assets (read-only, not stored in portfolios) - status :: text :: `"enabled"` (active, default) | `"disabled"` (soft-deleted); set to `"disabled"` by `DELETE /v1/portfolios/{id}`; only `status` can be mutated on a disabled portfolio (restoration via PATCH); all other writes → HTTP 409 - owner_user_id :: text|null :: owning user reference for attribution and authorization -- active_strategy_id :: text|null :: currently selected strategy-set reference +- active_strategy_id :: text|null :: currently selected stable strategy-catalog ID; create requires an active strategy and update may select another active strategy +- strategy_revision :: integer|null :: read-only revision joined from the currently referenced StrategySet; subsequent API reads expose the latest committed shared config revision - cash_balance :: double precision :: current cash snapshot after the latest ledger event - asset_quantity :: double precision :: current holdings snapshot after the latest ledger event - managed_by_daemon_id :: text|null :: soft reference to daemons.id; controls daemon-scoped API access; NULL = unassigned (fail-closed for ledger writes and asset listing); set to NULL on soft delete; added by migration 20260513_0016 @@ -333,6 +356,7 @@ - Alembic revisions in migrations/versions/ are the ordered schema history for persistent environments. - app/repositories/postgres.py includes ensure_schema() for bootstrap safety, but long-term schema evolution should run through Alembic. +- Current Alembic head is `20260814_0029`; its strategy columns, default seed, and constraints are guarded for ensure_schema-first and repeatable execution. - Comments, tags, auth tokens, and ledger rows are normalized into dedicated tables keyed to users or portfolios. ## Current Gaps / Planned Changes diff --git a/docs/interface-contract.md b/docs/interface-contract.md index 01c50f5..de1b318 100644 --- a/docs/interface-contract.md +++ b/docs/interface-contract.md @@ -56,7 +56,7 @@ ### Compatibility Rules -- Route prefixes /health, /auth, /users, /strategy-sets, and /portfolios define the current internal API surface. +- Route prefixes `/health`, `/auth`, `/users`, and `/portfolios` define the unversioned internal API surface. The legacy `/strategy-sets` routes are removed; strategy catalog consumers use `/v1/strategies` and admins use `/v1/admin/strategies`. - Permission semantics read, trade-write, and admin must stay aligned across auth guards and token metadata. - Breaking route or schema changes require REQ and Batch documentation updates before release. @@ -76,6 +76,8 @@ - Environment configuration :: app/core/config.py resolves PORTFOLIOHUB_* and TIMESCALEDB_* values before command execution - Portfolio identification (bat-006) :: all portfolio mutation commands accept `` (positional) + `--user ` instead of raw portfolio_id; `comment` sub-commands accept portfolio_id (UUID) directly - Tag commands (bat-005) :: `pfhub tag create/list/deactivate/apply/remove` operate on the tags master table; `pfhub portfolio tag add/list/remove` has been removed (breaking change) +- Portfolio strategy selection :: `pfhub portfolio create` requires `--strategy-id`; `portfolio update` accepts optional `--strategy-id`. Strategy names/configs are not accepted on portfolio mutations. +- Strategy catalog commands :: `pfhub strategy list/create/update/delete`; list and mutation output includes `status` and `revision`, update preserves the strategy ID, and referenced strategies cannot be deleted. ### Outputs @@ -299,18 +301,29 @@ ### Endpoint: `POST /v1/portfolios` - Auth: any valid token; service/client → HTTP 403 -- Body: `{"asset_id": "", "broker": "", "name": "", "cash_balance": , "asset_quantity": , "asset_price": , "profit_buffer": , "strategy_name": "", "strategy_params": {}, "managed_by_daemon_id": "", "user_id": " (admin only)"}` -- Response: HTTP 201 `PortfolioItem {"portfolio_id": "", "asset_id": "", "symbol": "", "owner_user_id": "", "broker": "", "status": "", "name": "", ...}` +- 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 422 if asset not found or inactive; HTTP 403 if service/client token +- 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 ### Endpoint: `PATCH /v1/portfolios/{portfolio_id}` - Auth: any valid token; service/client → HTTP 403; ownership required -- Body: `{"broker": "", "status": "", "name": "", "strategy_name": "", "strategy_params": {}, "asset_price": , "managed_by_daemon_id": ""}` -- Response: HTTP 200 `PortfolioItem {"portfolio_id": "", "asset_id": "", "symbol": "", ...}` +- 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 not found; HTTP 409 if portfolio is disabled and non-status fields are provided +- 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 + +## Interface: `v1 Strategy Catalog API` + +- `GET /v1/strategies` and `GET /v1/strategies/{strategy_id}` require read permission and expose active strategies only. +- `GET /v1/admin/strategies[/{strategy_id}]` and admin `POST`, `PATCH`, `DELETE` require admin permission and expose both active and inactive entries. +- Create body: `{"name": "", "config": {}, "status": "active|inactive"}`; response is `StrategyResponse {strategy_id, name, config, status, revision, created_at, updated_at}`. +- Patch accepts at least one of `name`, `config`, or `status`. An effective no-op preserves `revision`; a real mutation increments it while preserving `strategy_id` and keeping the derived `nick` consistent with name/config. +- Strategy PATCH locks the catalog row (`FOR UPDATE`) through the mutation transaction. Concurrent partial PATCH requests therefore serialize and derive `nick` from the latest committed name/config pair. +- Inactive strategies remain readable to admins and remain attached to existing portfolios, but cannot be selected for new or updated portfolio assignments. +- New portfolio selection takes a `FOR SHARE` lock on the active strategy in the same transaction as the portfolio INSERT/UPDATE. PATCH/DELETE take `FOR UPDATE`, preventing concurrent inactivation/deletion from racing the selection; DELETE also checks both reference forms and deletes in that transaction. +- Delete returns HTTP 409 while any portfolio references the strategy. Unknown IDs return HTTP 404; schema validation returns HTTP 422; conflicts return HTTP 409. ### Endpoint: `DELETE /v1/portfolios/{portfolio_id}` (Soft Delete) diff --git a/docs/project-structure.md b/docs/project-structure.md index abb16d5..16d13ff 100644 --- a/docs/project-structure.md +++ b/docs/project-structure.md @@ -138,7 +138,7 @@ |-----------|-----------|------| | ensure_schema() 전용 | `idx_auth_tokens_service_name_unique` (partial index, BAT-006/REQ-017: `AND revoked_at IS NULL` 조건 추가) | Alembic revision 20260520_0022에도 CONCURRENTLY 버전 존재 | | Alembic 전용 | `portfolios.market`, `auth_tokens.runtime_id`, `idx_auth_tokens_runtime_id_unique`, `broker_credentials`, `ledger_trades` | ensure_schema()에 없음 — v1 API 전제 조건 | -| 양쪽 공유 (IF NOT EXISTS 보호) | users, auth_tokens, strategy_sets, portfolios, portfolio_ledger, portfolio_comments, portfolio_tags + 대부분의 인덱스 | ensure_schema() 우선 실행 후 Alembic 실행 시 충돌 없음 | +| 양쪽 공유 (IF NOT EXISTS 보호) | users, auth_tokens, strategy_sets (`status`, `revision`, `updated_at` 포함), portfolios, portfolio_ledger, portfolio_comments, portfolio_tags + 대부분의 인덱스 | ensure_schema() 우선 실행 후 Alembic `20260814_0029` 실행 시 충돌 없음; 기본 전략 seed/constraints도 존재 여부 guard 적용 | ### 허용 실행 순서 diff --git a/docs/runtime-flows.md b/docs/runtime-flows.md index a803de0..cddf609 100644 --- a/docs/runtime-flows.md +++ b/docs/runtime-flows.md @@ -121,13 +121,24 @@ - Any valid token; `_require_write_token` blocks service/client (`token_type ∈ {service, client}` → HTTP 403). - admin with `body.user_id` → creates portfolio for target user; otherwise uses `auth.user_id`. - `repo.get_asset_by_id(asset_id)` → None or inactive → HTTP 422. - - `repo.get_or_create_strategy_set(strategy_name, strategy_params_json)` → resolves strategy. + - Required `strategy_id` is resolved with `repo.get_strategy_set(strategy_id)`; unknown → HTTP 404, inactive → HTTP 409. Portfolio mutation never creates a strategy implicitly. - `repo.create_portfolio(...)` with `status="enabled"` → HTTP 201 `PortfolioItem`. 2. **PATCH /v1/portfolios/{portfolio_id}** (update): - `_require_write_token` blocks service/client → HTTP 403. - `_get_portfolio_or_404(auth, portfolio_id, repo)` → checks ownership; non-owner non-admin → HTTP 403. - If `portfolio.status == "disabled"`: non-status fields present → HTTP 409 (only status restoration allowed). + - Optional `strategy_id` is resolved before `repo.update_portfolio(...)`; unknown → HTTP 404, inactive → HTTP 409. The stable catalog ID becomes `active_strategy_id`. - `repo.update_portfolio(portfolio_id, **fields)` → HTTP 200 `PortfolioItem`. + +## Flow: `v1 Strategy Catalog` + +1. Read callers list active entries at `GET /v1/strategies`; admin reads include inactive entries under `/v1/admin/strategies`. +2. Admin create canonicalizes the JSON config. A new row stores the requested status at revision 1; a matching existing name/config reuses its stable ID and applies an actual status change through the normal update path. +3. Admin PATCH requires at least one field. The row is locked `FOR UPDATE`; effective no-ops keep the current revision, while concurrent real changes serialize, preserve ID, recompute the derived nick from the latest committed name/config, increment revision once, and update `updated_at`. +4. Inactive entries are retained for existing portfolio references but rejected for new assignments. +5. Portfolio selection locks an active strategy `FOR SHARE` in the same transaction as portfolio INSERT/UPDATE. Admin PATCH/DELETE lock it `FOR UPDATE`; DELETE checks both `active_strategy_id` and `avabile_strategies` and deletes in one transaction. Any reference returns HTTP 409, otherwise deletion returns HTTP 204 without an orphan or inactive-selection race. +6. Legacy unversioned `/strategy-sets` routes are not registered. +7. BUY/SELL persistence updates balances, price, buffer, timestamps, and ledger atomically but never writes `active_strategy_id` or `avabile_strategies`; a stale trade snapshot therefore cannot restore an old or deleted catalog assignment after a concurrent strategy change. 3. **DELETE /v1/portfolios/{portfolio_id}** (soft delete): - `_require_write_token` blocks service/client → HTTP 403. - `_get_portfolio_or_404` → ownership check. diff --git a/migrations/versions/20260814_0029_strategy_catalog_v1.py b/migrations/versions/20260814_0029_strategy_catalog_v1.py new file mode 100644 index 0000000..e8ed1c0 --- /dev/null +++ b/migrations/versions/20260814_0029_strategy_catalog_v1.py @@ -0,0 +1,121 @@ +"""strategy_sets: active status, revision, and default strategy + +Revision ID: 20260814_0029 +Revises: 20260715_0028 +Create Date: 2026-08-14 00:00:00 + +The migration is additive and idempotent for databases where ensure_schema() +may already have created the columns. Existing strategy IDs and portfolio +references are preserved. The canonical target_value/{} strategy is reused +when present, otherwise one stable default row is inserted. +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op +from sqlalchemy import inspect, text + +revision = "20260814_0029" +down_revision = "20260715_0028" +branch_labels = None +depends_on = None + +_DEFAULT_STRATEGY_ID = "00000000-0000-0000-0000-000000000029" +_DEFAULT_NAME = "target_value" +_DEFAULT_CONFIG = "{}" +_DEFAULT_NICK = "target_value:bf21a9e8fbc5" + + +def _column_exists(inspector: sa.engine.Inspector, table_name: str, column_name: str) -> bool: + return any(column["name"] == column_name for column in inspector.get_columns(table_name)) + + +def upgrade() -> None: + bind = op.get_bind() + inspector = inspect(bind) + if "strategy_sets" not in inspector.get_table_names(): + return + + if not _column_exists(inspector, "strategy_sets", "status"): + op.add_column( + "strategy_sets", + sa.Column("status", sa.Text(), nullable=False, server_default=sa.text("'active'")), + ) + if not _column_exists(inspector, "strategy_sets", "revision"): + op.add_column( + "strategy_sets", + sa.Column("revision", sa.Integer(), nullable=False, server_default=sa.text("1")), + ) + if not _column_exists(inspector, "strategy_sets", "updated_at"): + op.add_column( + "strategy_sets", + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False, server_default=sa.text("NOW()")), + ) + + bind.execute(text("UPDATE strategy_sets SET status = 'active' WHERE status IS NULL")) + bind.execute(text("UPDATE strategy_sets SET revision = 1 WHERE revision IS NULL OR revision < 1")) + bind.execute(text("UPDATE strategy_sets SET updated_at = created_at WHERE updated_at IS NULL")) + bind.execute( + text( + """ + INSERT INTO strategy_sets (id, name, config, nick, status, revision) + SELECT :id, :name, :config, :nick, 'active', 1 + WHERE NOT EXISTS ( + SELECT 1 FROM strategy_sets WHERE name = :name AND config = :config + ) + """ + ), + { + "id": _DEFAULT_STRATEGY_ID, + "name": _DEFAULT_NAME, + "config": _DEFAULT_CONFIG, + "nick": _DEFAULT_NICK, + }, + ) + + # The same (name, config) row is the canonical default; ensure it is selectable. + bind.execute( + text( + """ + UPDATE strategy_sets + SET status = 'active', updated_at = CASE WHEN status = 'active' THEN updated_at ELSE NOW() END + WHERE name = :name AND config = :config AND status <> 'active' + """ + ), + {"name": _DEFAULT_NAME, "config": _DEFAULT_CONFIG}, + ) + + # PostgreSQL has no ADD CONSTRAINT IF NOT EXISTS, so guard by catalog lookup. + bind.execute( + text( + """ + DO $$ + BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'ck_strategy_sets_status' + AND conrelid = 'strategy_sets'::regclass + ) THEN + ALTER TABLE strategy_sets + ADD CONSTRAINT ck_strategy_sets_status + CHECK (status IN ('active', 'inactive')); + END IF; + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'ck_strategy_sets_revision_positive' + AND conrelid = 'strategy_sets'::regclass + ) THEN + ALTER TABLE strategy_sets + ADD CONSTRAINT ck_strategy_sets_revision_positive + CHECK (revision >= 1); + END IF; + END $$; + """ + ) + ) + + +def downgrade() -> None: + """Forward-only data migration; existing references and revisions are retained.""" + return diff --git a/tests/test_asset_catalog.py b/tests/test_asset_catalog.py index c78b0dd..9162e66 100644 --- a/tests/test_asset_catalog.py +++ b/tests/test_asset_catalog.py @@ -583,8 +583,11 @@ class _PortfolioServiceFakeRepo: """FakeRepo for PortfolioService catalog validation tests.""" def __init__(self, asset: Asset | None = None) -> None: + from app.domain.models import StrategySet + self._asset = asset - self._strategy_sets: dict = {} + strategy = StrategySet(id="set-1", name="target_value", config="{}", nick="target_value:default") + self._strategy_sets: dict = {strategy.id: strategy} self._portfolios: dict = {} def get_asset_by_id(self, asset_id: str) -> Asset | None: @@ -625,7 +628,7 @@ class _PortfolioServiceFakeRepo: return p def get_strategy_set(self, id: str): - return None + return self._strategy_sets.get(id) def list_recent_trades(self, portfolio_id: str, *, limit: int = 5): return [] @@ -633,7 +636,7 @@ class _PortfolioServiceFakeRepo: def list_comments(self, portfolio_id: str): return [] - def list_strategy_sets(self): + def list_strategy_sets(self, *, include_inactive: bool = True): return [] def list_portfolios(self, **kwargs): @@ -669,6 +672,7 @@ def test_portfolio_create_with_active_asset() -> None: name="Test_Portfolio", asset_id=_CATALOG_ASSET_ID, broker="kis", + strategy_id="set-1", ), owner_user_id="user-1", ) @@ -688,6 +692,7 @@ def test_portfolio_create_unknown_asset() -> None: name="Test_Portfolio", asset_id="nonexistent-uuid", broker="kis", + strategy_id="set-1", ), owner_user_id="user-1", ) @@ -706,6 +711,7 @@ def test_portfolio_create_inactive_asset() -> None: name="Test_Portfolio", asset_id=_CATALOG_ASSET_ID, broker="kis", + strategy_id="set-1", ), owner_user_id="user-1", ) @@ -745,6 +751,8 @@ def test_portfolio_create_by_symbol() -> None: "portfolio", "create", "Test_Portfolio", "--user", "alice", "--symbol", "005930", + "--strategy-id", + "set-1", "--force", ]) assert result.exit_code == 0, result.output diff --git a/tests/test_cli.py b/tests/test_cli.py index 7f22b01..69b1cd6 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -469,9 +469,40 @@ class FakeRepo: 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]: + def list_strategy_sets(self, *, include_inactive: bool = True) -> 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) + values = list(self._strategy_sets.values()) + if not include_inactive: + values = [item for item in values if item.status == "active"] + return sorted(values, key=lambda s: s.created_at or datetime.now(timezone.utc), reverse=True) + + def update_strategy_set( + self, + id: str, + *, + name: str | None = None, + config: str | None = None, + status: str | None = None, + ) -> StrategySet: + strategy = self._strategy_sets.get(id) + if strategy is None: + raise ValueError("전략을 찾을 수 없습니다") + final_name = name if name is not None else strategy.name + final_config = config if config is not None else strategy.config + final_status = status if status is not None else strategy.status + if (final_name, final_config, final_status) == ( + strategy.name, + strategy.config, + strategy.status, + ): + return strategy + strategy.name = final_name + strategy.config = final_config + strategy.status = final_status + strategy.nick = f"{strategy.name}:updated" + strategy.revision += 1 + strategy.updated_at = datetime.now(timezone.utc) + return strategy def delete_strategy_set(self, id: str) -> None: if id in self._strategy_sets: @@ -670,8 +701,8 @@ def test_cli_create_buy_and_delete_flow() -> None: _TEST_ASSET_ID, "--cash-balance", "1000000", - "--strategy-params", - '{"target_asset_ratio": 0.5}', + "--strategy-id", + "set-1", "--force", ], ) @@ -717,6 +748,8 @@ def test_cli_write_command_prompts_before_create() -> None: _TEST_ASSET_ID, "--cash-balance", "1000000", + "--strategy-id", + "set-1", ], input="y\n", ) @@ -740,6 +773,8 @@ def test_cli_write_command_can_abort() -> None: _TEST_ASSET_ID, "--cash-balance", "1000000", + "--strategy-id", + "set-1", ], input="n\n", ) @@ -1434,3 +1469,32 @@ def test_cli_service_token_rebind_rejects_cross_user_daemon() -> None: assert result.exit_code == 1 assert "해당 토큰 사용자에 속하지 않습니다" in result.stdout + + +def test_cli_strategy_commands_show_status_revision_and_preserve_id() -> None: + repo = FakeRepo() + runner = build_runner(repo) + + listed = runner.invoke(runner.app, ["strategy", "list"]) + updated = runner.invoke( + runner.app, + ["strategy", "update", "set-1", "--status", "inactive", "--name", "target_value_v2"], + ) + no_change = runner.invoke(runner.app, ["strategy", "update", "set-1", "--status", "inactive"]) + + assert listed.exit_code == 0 + assert "Status" in listed.output and "Revision" in listed.output + assert updated.exit_code == 0 + assert "ID: set-1" in updated.output + assert "Status: inactive" in updated.output + assert "Revision: 2" in updated.output + assert no_change.exit_code == 0 + assert "Revision: 2" in no_change.output + + +def test_cli_strategy_update_requires_a_field() -> None: + repo = FakeRepo() + runner = build_runner(repo) + result = runner.invoke(runner.app, ["strategy", "update", "set-1"]) + assert result.exit_code == 1 + assert "at least one" in result.output diff --git a/tests/test_domain_services.py b/tests/test_domain_services.py index e7c44e0..69195b6 100644 --- a/tests/test_domain_services.py +++ b/tests/test_domain_services.py @@ -90,7 +90,7 @@ class FakeRepo: self._strategy_sets[self.strategy_set.id] = self.strategy_set return self.strategy_set - def list_strategy_sets(self) -> list[StrategySet]: + def list_strategy_sets(self, *, include_inactive: bool = True) -> 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: diff --git a/tests/test_portfolio_comments.py b/tests/test_portfolio_comments.py index 7d665bd..ae46f5f 100644 --- a/tests/test_portfolio_comments.py +++ b/tests/test_portfolio_comments.py @@ -70,7 +70,7 @@ class FakeRepo: def get_strategy_set(self, id: str) -> StrategySet | None: return self.strategy_set if id == self.strategy_set.id else None - def list_strategy_sets(self): + def list_strategy_sets(self, *, include_inactive: bool = True): return [self.strategy_set] def delete_strategy_set(self, id: str) -> None: diff --git a/tests/test_portfolio_ownership.py b/tests/test_portfolio_ownership.py index 1c3f650..5f0464d 100644 --- a/tests/test_portfolio_ownership.py +++ b/tests/test_portfolio_ownership.py @@ -164,7 +164,7 @@ def test_db_enabled_env_token_without_user_context_is_rejected(monkeypatch) -> N response = client.post( "/portfolios", headers={"Authorization": "Bearer trade-env-token"}, - json={"name": "new-1", "asset_id": "00000000-0000-0000-0000-000000000001"}, + json={"name": "new-1", "asset_id": "00000000-0000-0000-0000-000000000001", "strategy_id": "set-1"}, ) app.dependency_overrides.clear() diff --git a/tests/test_portfolio_uuid.py b/tests/test_portfolio_uuid.py index f48c3e9..20dbf1f 100644 --- a/tests/test_portfolio_uuid.py +++ b/tests/test_portfolio_uuid.py @@ -201,7 +201,7 @@ class FakeRepo: def get_strategy_set(self, id: str) -> StrategySet | None: return self._strategy_sets.get(id) - def list_strategy_sets(self) -> list[StrategySet]: + def list_strategy_sets(self, *, include_inactive: bool = True) -> list[StrategySet]: return list(self._strategy_sets.values()) def delete_strategy_set(self, id: str) -> None: @@ -350,7 +350,7 @@ def test_create_portfolio_assigns_uuid_id() -> None: result = runner.invoke( runner.app, - ["portfolio", "create", "SAMSUNG", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--force"], + ["portfolio", "create", "SAMSUNG", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--strategy-id", "set-1", "--force"], ) assert result.exit_code == 0, result.output @@ -371,7 +371,7 @@ def test_create_returns_uuid_in_response() -> None: result = runner.invoke( runner.app, - ["--format", "json", "portfolio", "create", "MY_PORTFOLIO", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--force"], + ["--format", "json", "portfolio", "create", "MY_PORTFOLIO", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--strategy-id", "set-1", "--force"], ) assert result.exit_code == 0, result.output @@ -389,7 +389,7 @@ def test_status_resolves_by_name_and_user() -> None: create_result = runner.invoke( runner.app, - ["--format", "json", "portfolio", "create", "MY_PORT", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--force"], + ["--format", "json", "portfolio", "create", "MY_PORT", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--strategy-id", "set-1", "--force"], ) assert create_result.exit_code == 0, create_result.output @@ -437,7 +437,7 @@ def test_create_without_user_fails() -> None: result = runner.invoke( runner.app, - ["portfolio", "create", "MY_PORT", "--asset-id", _TEST_ASSET_ID, "--force"], + ["portfolio", "create", "MY_PORT", "--asset-id", _TEST_ASSET_ID, "--strategy-id", "set-1", "--force"], ) assert result.exit_code != 0 @@ -450,7 +450,7 @@ def test_create_with_unknown_user_fails() -> None: result = runner.invoke( runner.app, - ["portfolio", "create", "MY_PORT", "--asset-id", _TEST_ASSET_ID, "--user", "ghost", "--force"], + ["portfolio", "create", "MY_PORT", "--asset-id", _TEST_ASSET_ID, "--user", "ghost", "--strategy-id", "set-1", "--force"], ) assert result.exit_code == 1 @@ -464,7 +464,7 @@ def test_name_validation_rejects_invalid_characters() -> None: result = runner.invoke( runner.app, - ["portfolio", "create", "invalid name!", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--force"], + ["portfolio", "create", "invalid name!", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--strategy-id", "set-1", "--force"], ) assert result.exit_code == 1 @@ -478,7 +478,7 @@ def test_name_validation_rejects_empty_name() -> None: result = runner.invoke( runner.app, - ["portfolio", "create", "", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--force"], + ["portfolio", "create", "", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--strategy-id", "set-1", "--force"], ) assert result.exit_code != 0 @@ -492,11 +492,11 @@ def test_alice_and_bob_can_have_same_portfolio_name() -> None: alice_result = runner.invoke( runner.app, - ["portfolio", "create", "SAMSUNG", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--force"], + ["portfolio", "create", "SAMSUNG", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--strategy-id", "set-1", "--force"], ) bob_result = runner.invoke( runner.app, - ["portfolio", "create", "SAMSUNG", "--asset-id", _TEST_ASSET_ID, "--user", "bob", "--force"], + ["portfolio", "create", "SAMSUNG", "--asset-id", _TEST_ASSET_ID, "--user", "bob", "--strategy-id", "set-1", "--force"], ) assert alice_result.exit_code == 0, alice_result.output @@ -513,7 +513,7 @@ def test_buy_resolves_portfolio_by_name_and_user() -> None: runner.invoke( runner.app, - ["portfolio", "create", "MY_PORT", "--asset-id", _TEST_ASSET_ID, "--cash-balance", "1000000", "--user", "alice", "--force"], + ["portfolio", "create", "MY_PORT", "--asset-id", _TEST_ASSET_ID, "--cash-balance", "1000000", "--user", "alice", "--strategy-id", "set-1", "--force"], ) buy_result = runner.invoke( runner.app, @@ -532,7 +532,7 @@ def test_delete_resolves_portfolio_by_name_and_user() -> None: runner.invoke( runner.app, - ["portfolio", "create", "MY_PORT", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--force"], + ["portfolio", "create", "MY_PORT", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--strategy-id", "set-1", "--force"], ) delete_result = runner.invoke( runner.app, @@ -552,7 +552,7 @@ def test_alice_cannot_delete_bobs_portfolio() -> None: runner.invoke( runner.app, - ["portfolio", "create", "SAMSUNG", "--asset-id", _TEST_ASSET_ID, "--user", "bob", "--force"], + ["portfolio", "create", "SAMSUNG", "--asset-id", _TEST_ASSET_ID, "--user", "bob", "--strategy-id", "set-1", "--force"], ) delete_result = runner.invoke( runner.app, @@ -575,7 +575,7 @@ def test_name_validation_accepts_korean() -> None: result = runner.invoke( runner.app, - ["portfolio", "create", "삼성전자 포트폴리오", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--force"], + ["portfolio", "create", "삼성전자 포트폴리오", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--strategy-id", "set-1", "--force"], ) assert result.exit_code == 0, result.output @@ -589,7 +589,7 @@ def test_name_validation_accepts_ascii_unchanged() -> None: result = runner.invoke( runner.app, - ["portfolio", "create", "my-portfolio_v1:test.01", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--force"], + ["portfolio", "create", "my-portfolio_v1:test.01", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--strategy-id", "set-1", "--force"], ) assert result.exit_code == 0, result.output @@ -602,7 +602,7 @@ def test_name_validation_rejects_over_100_chars() -> None: long_name = "가" * 101 with pytest.raises(Exception): - CreatePortfolioRequest(name=long_name, asset_id="some-uuid") + CreatePortfolioRequest(name=long_name, asset_id="some-uuid", strategy_id="set-1") def test_update_new_name_renames_portfolio() -> None: @@ -612,7 +612,7 @@ def test_update_new_name_renames_portfolio() -> None: runner.invoke( runner.app, - ["portfolio", "create", "OLD_NAME", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--force"], + ["portfolio", "create", "OLD_NAME", "--asset-id", _TEST_ASSET_ID, "--user", "alice", "--strategy-id", "set-1", "--force"], ) rename_result = runner.invoke( runner.app, diff --git a/tests/test_strategy_catalog_migration.py b/tests/test_strategy_catalog_migration.py new file mode 100644 index 0000000..542320f --- /dev/null +++ b/tests/test_strategy_catalog_migration.py @@ -0,0 +1,74 @@ +"""Idempotency contract for the Issue #26 strategy-catalog migration.""" + +from __future__ import annotations + +import importlib.util +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch + + +_MIGRATION_PATH = ( + Path(__file__).parents[1] + / "migrations/versions/20260814_0029_strategy_catalog_v1.py" +) + + +def _load_migration(): + spec = importlib.util.spec_from_file_location("strategy_catalog_0029", _MIGRATION_PATH) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class _Bind: + def __init__(self) -> None: + self.executed: list[str] = [] + self.params: list[dict | None] = [] + + def execute(self, statement, params=None): + self.executed.append(str(statement)) + self.params.append(params) + return SimpleNamespace() + + +class _Inspector: + def __init__(self, columns: set[str]) -> None: + self.columns = columns + + def get_table_names(self) -> list[str]: + return ["strategy_sets"] + + def get_columns(self, table_name: str) -> list[dict[str, str]]: + assert table_name == "strategy_sets" + return [{"name": name} for name in sorted(self.columns)] + + +def test_upgrade_is_repeatable_and_uses_data_and_constraint_guards() -> None: + migration = _load_migration() + columns = {"id", "name", "config", "nick", "created_at"} + inspector = _Inspector(columns) + bind = _Bind() + added: list[str] = [] + + def add_column(table_name: str, column) -> None: + assert table_name == "strategy_sets" + columns.add(column.name) + added.append(column.name) + + with ( + patch.object(migration.op, "get_bind", return_value=bind), + patch.object(migration.op, "add_column", side_effect=add_column), + patch.object(migration, "inspect", return_value=inspector), + ): + migration.upgrade() + migration.upgrade() + + assert added == ["status", "revision", "updated_at"] + sql = "\n".join(bind.executed) + assert "WHERE NOT EXISTS" in sql + assert "IF NOT EXISTS" in sql + assert "ck_strategy_sets_status" in sql + assert "ck_strategy_sets_revision_positive" in sql + assert any(params and params.get("nick") == "target_value:bf21a9e8fbc5" for params in bind.params) diff --git a/tests/test_strategy_catalog_postgres_integration.py b/tests/test_strategy_catalog_postgres_integration.py new file mode 100644 index 0000000..14d1e06 --- /dev/null +++ b/tests/test_strategy_catalog_postgres_integration.py @@ -0,0 +1,328 @@ +"""Real-PostgreSQL concurrency and migration verification for Issue #26. + +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 hashlib +import os +import subprocess +import threading +import uuid +from pathlib import Path + +import psycopg +import pytest +from psycopg import sql + +from app.domain.errors import StrategyNotFoundError +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", +) + +_ROOT = Path(__file__).parents[1] + + +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_catalog_fixture(repo: PostgresPortfolioRepository) -> tuple[str, str, str]: + repo.ensure_schema() + strategy = repo.create_strategy_set(f"concurrency-{uuid.uuid4().hex}", "{}") + asset_id = str(uuid.uuid4()) + owner_user_id = f"qc-user-{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"QC{uuid.uuid4().hex[:10]}"), + ) + return strategy.id, asset_id, owner_user_id + + +def test_concurrent_strategy_patch_keeps_derived_nick_consistent() -> None: + owner = _repo() + strategy_id, asset_id, owner_user_id = _prepare_catalog_fixture(owner) + renamed = f"concurrent-renamed-{uuid.uuid4().hex}" + barrier = threading.Barrier(3) + errors: list[BaseException] = [] + + def patch_name() -> None: + repo = _repo() + try: + barrier.wait() + repo.update_strategy_set(strategy_id, name=renamed) + except BaseException as exc: # pragma: no cover - only populated on failure + errors.append(exc) + finally: + repo.close() + + def patch_config() -> None: + repo = _repo() + try: + barrier.wait() + repo.update_strategy_set(strategy_id, config='{"window":21}') + except BaseException as exc: # pragma: no cover - only populated on failure + errors.append(exc) + finally: + repo.close() + + threads = [threading.Thread(target=patch_name), threading.Thread(target=patch_config)] + for thread in threads: + thread.start() + barrier.wait() + for thread in threads: + thread.join(timeout=10) + assert not thread.is_alive() + + final = owner.get_strategy_set(strategy_id) + assert errors == [] + assert final is not None + assert final.name == renamed + assert final.config == '{"window":21}' + assert final.revision == 3 + expected_nick = f"{final.name}:{hashlib.sha1(final.config.encode()).hexdigest()[:12]}" + assert final.nick == expected_nick + portfolio_id = str(uuid.uuid4()) + owner.create_portfolio( + portfolio_id, + owner_user_id=owner_user_id, + broker="kis", + status="enabled", + name="latest-revision-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], + ) + portfolio = owner.get(portfolio_id) + assert portfolio.strategy_params == {"window": 21} + assert portfolio.strategy_revision == 3 + with owner._connect().cursor() as cur: + cur.execute("DELETE FROM portfolios WHERE portfolio_id = %s", (portfolio_id,)) + owner.delete_strategy_set(strategy_id) + with owner._connect().cursor() as cur: + cur.execute("DELETE FROM assets WHERE asset_id = %s", (asset_id,)) + cur.execute("DELETE FROM users WHERE id = %s", (owner_user_id,)) + owner.close() + + +def test_portfolio_selection_and_strategy_delete_never_create_orphan() -> None: + owner = _repo() + strategy_id, asset_id, owner_user_id = _prepare_catalog_fixture(owner) + portfolio_id = str(uuid.uuid4()) + barrier = threading.Barrier(3) + outcomes: list[str] = [] + + def select_strategy() -> None: + repo = _repo() + try: + barrier.wait() + repo.create_portfolio( + portfolio_id, + owner_user_id=owner_user_id, + broker="kis", + status="enabled", + name="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], + ) + outcomes.append("selected") + except StrategyNotFoundError: + outcomes.append("selection-rejected") + finally: + repo.close() + + def delete_strategy() -> None: + repo = _repo() + try: + barrier.wait() + repo.delete_strategy_set(strategy_id) + outcomes.append("deleted") + except ValueError as exc: + assert "in use" in str(exc) + outcomes.append("delete-rejected") + finally: + repo.close() + + threads = [threading.Thread(target=select_strategy), threading.Thread(target=delete_strategy)] + for thread in threads: + thread.start() + barrier.wait() + for thread in threads: + thread.join(timeout=10) + assert not thread.is_alive() + + with owner._connect().cursor() as cur: + cur.execute("SELECT COUNT(*) FROM portfolios WHERE portfolio_id = %s", (portfolio_id,)) + portfolio_exists = cur.fetchone()[0] == 1 + cur.execute("SELECT COUNT(*) FROM strategy_sets WHERE id = %s", (strategy_id,)) + strategy_exists = cur.fetchone()[0] == 1 + assert not (portfolio_exists and not strategy_exists) + assert sorted(outcomes) in (["delete-rejected", "selected"], ["deleted", "selection-rejected"]) + + with owner._connect().cursor() as cur: + cur.execute("DELETE FROM portfolios WHERE portfolio_id = %s", (portfolio_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,)) + owner.close() + + +def test_stale_trade_save_cannot_restore_a_deleted_strategy_reference() -> None: + owner = _repo() + strategy_a, asset_id, owner_user_id = _prepare_catalog_fixture(owner) + strategy_b = owner.create_strategy_set(f"replacement-{uuid.uuid4().hex}", "{}") + portfolio_id = str(uuid.uuid4()) + owner.create_portfolio( + portfolio_id, + owner_user_id=owner_user_id, + broker="kis", + status="enabled", + name="stale-trade-check", + asset_id=asset_id, + cash_balance=1_000, + asset_quantity=0, + asset_price=100, + profit_buffer=0, + active_strategy_id=strategy_a, + avabile_strategies=[strategy_a], + ) + stale = owner.get(portfolio_id) + + concurrent = _repo() + concurrent.update_portfolio( + portfolio_id, + active_strategy_id=strategy_b.id, + avabile_strategies=[strategy_b.id], + ) + concurrent.delete_strategy_set(strategy_a) + concurrent.close() + + stale.cash_balance = 900 + stale.asset_quantity = 1 + owner.save( + stale, + current_price=100, + action="BUY", + trade_price=100, + quantity=1, + fee_amount=0, + tax_amount=0, + total_value_after=1_000, + strategy_set_id=strategy_a, + note="stale snapshot regression", + ) + + current = owner.get(portfolio_id) + assert current.active_strategy_id == strategy_b.id + assert current.avabile_strategies == [strategy_b.id] + assert owner.get_strategy_set(strategy_a) is None + with owner._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 strategy_sets WHERE id = %s", (strategy_b.id,)) + cur.execute("DELETE FROM assets WHERE asset_id = %s", (asset_id,)) + cur.execute("DELETE FROM users WHERE id = %s", (owner_user_id,)) + owner.close() + + +def test_alembic_upgrade_is_repeatable_on_fresh_postgres_database() -> None: + database_name = f"portfolio_hub_0029_{uuid.uuid4().hex[:12]}" + maintenance = _db_values("postgres") + connection = psycopg.connect(**maintenance, autocommit=True) + try: + with connection.cursor() as cur: + cur.execute(sql.SQL("CREATE DATABASE {}").format(sql.Identifier(database_name))) + env = os.environ.copy() + env.update( + { + "TIMESCALEDB_HOST": str(maintenance["host"]), + "TIMESCALEDB_PORT": str(maintenance["port"]), + "TIMESCALEDB_NAME": database_name, + "TIMESCALEDB_USER": str(maintenance["user"]), + "TIMESCALEDB_PASSWORD": str(maintenance["password"]), + "TIMESCALEDB_SSLMODE": str(maintenance["sslmode"]), + } + ) + for _ in range(2): + subprocess.run( + [str(_ROOT / ".venv/bin/python"), "-m", "alembic", "upgrade", "head"], + cwd=_ROOT, + env=env, + check=True, + capture_output=True, + text=True, + ) + test_db = psycopg.connect(**_db_values(database_name)) + try: + with test_db.cursor() as cur: + cur.execute( + """ + SELECT id, status, revision, nick + FROM strategy_sets + WHERE name = 'target_value' AND config = '{}' + """ + ) + rows = cur.fetchall() + assert rows == [ + ( + "00000000-0000-0000-0000-000000000029", + "active", + 1, + "target_value:bf21a9e8fbc5", + ) + ] + cur.execute("SELECT version_num FROM alembic_version") + assert cur.fetchone()[0] == "20260814_0029" + finally: + test_db.close() + finally: + with connection.cursor() as cur: + cur.execute( + "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = %s", + (database_name,), + ) + cur.execute(sql.SQL("DROP DATABASE IF EXISTS {}").format(sql.Identifier(database_name))) + connection.close() diff --git a/tests/test_v1_portfolios_crud.py b/tests/test_v1_portfolios_crud.py index b742697..ed5145c 100644 --- a/tests/test_v1_portfolios_crud.py +++ b/tests/test_v1_portfolios_crud.py @@ -79,6 +79,9 @@ class _FakeRepo: def get_or_create_strategy_set(self, name: str, config: str) -> StrategySet: return self._strategy_set + def get_strategy_set(self, id: str) -> StrategySet | None: + return self._strategy_set if id == self._strategy_set.id else None + def create_portfolio( self, portfolio_id: str, @@ -190,7 +193,7 @@ def test_post_admin_creates_for_target_user() -> None: with _with_repo(repo): resp = client.post( "/v1/portfolios", - json={"asset_id": _ASSET_UUID, "broker": "kiwoom", "user_id": _OTHER_USER_ID}, + json={"asset_id": _ASSET_UUID, "broker": "kiwoom", "strategy_id": "strat-001", "user_id": _OTHER_USER_ID}, headers=_auth("admin-tok"), ) assert resp.status_code == 201 @@ -209,7 +212,7 @@ def test_post_user_creates_for_self() -> None: with _with_repo(repo): resp = client.post( "/v1/portfolios", - json={"asset_id": _ASSET_UUID, "broker": "kiwoom"}, + json={"asset_id": _ASSET_UUID, "broker": "kiwoom", "strategy_id": "strat-001"}, headers=_auth("user-tok"), ) assert resp.status_code == 201 @@ -230,7 +233,7 @@ def test_post_service_token_returns_403() -> None: with _with_repo(repo, db_auth=True): resp = client.post( "/v1/portfolios", - json={"asset_id": _ASSET_UUID, "broker": "kiwoom"}, + json={"asset_id": _ASSET_UUID, "broker": "kiwoom", "strategy_id": "strat-001"}, headers=_auth("svc-raw-token"), ) assert resp.status_code == 403 @@ -244,7 +247,7 @@ def test_post_inactive_asset_returns_422() -> None: with _with_repo(repo): resp = client.post( "/v1/portfolios", - json={"asset_id": _ASSET_UUID, "broker": "kiwoom"}, + json={"asset_id": _ASSET_UUID, "broker": "kiwoom", "strategy_id": "strat-001"}, headers=_auth("admin-tok"), ) assert resp.status_code == 422 @@ -259,7 +262,7 @@ def test_post_missing_asset_returns_422() -> None: with _with_repo(repo): resp = client.post( "/v1/portfolios", - json={"asset_id": "no-such-uuid", "broker": "kiwoom"}, + json={"asset_id": "no-such-uuid", "broker": "kiwoom", "strategy_id": "strat-001"}, headers=_auth("admin-tok"), ) assert resp.status_code == 422 @@ -268,7 +271,7 @@ def test_post_missing_asset_returns_422() -> None: def test_post_no_bearer_returns_401() -> None: """REQ-025 AC-6: no token → HTTP 401.""" client = _make_client() - resp = client.post("/v1/portfolios", json={"asset_id": _ASSET_UUID, "broker": "kiwoom"}) + resp = client.post("/v1/portfolios", json={"asset_id": _ASSET_UUID, "broker": "kiwoom", "strategy_id": "strat-001"}) assert resp.status_code == 401 @@ -280,7 +283,7 @@ def test_post_non_admin_user_id_ignored() -> None: with _with_repo(repo): resp = client.post( "/v1/portfolios", - json={"asset_id": _ASSET_UUID, "broker": "kiwoom", "user_id": _OTHER_USER_ID}, + json={"asset_id": _ASSET_UUID, "broker": "kiwoom", "strategy_id": "strat-001", "user_id": _OTHER_USER_ID}, headers=_auth("user-tok"), ) assert resp.status_code == 201 @@ -499,3 +502,48 @@ def test_delete_already_disabled_is_idempotent() -> None: resp = client.delete(f"/v1/portfolios/{_PORTFOLIO_UUID}", headers=_auth("admin-tok")) assert resp.status_code == 200 assert resp.json()["status"] == "disabled" + + +# Issue #26: strategy catalog references on portfolio mutations. +def test_post_strategy_reference_errors_and_required_contract() -> None: + repo = _FakeRepo(asset=_make_asset(_ASSET_UUID, "active")) + client = _make_client() + with _with_repo(repo): + missing = client.post( + "/v1/portfolios", + json={"asset_id": _ASSET_UUID, "broker": "kiwoom", "strategy_id": "missing"}, + headers=_auth("admin-tok"), + ) + invalid = client.post( + "/v1/portfolios", + json={"asset_id": _ASSET_UUID, "broker": "kiwoom"}, + headers=_auth("admin-tok"), + ) + repo._strategy_set.status = "inactive" + inactive = client.post( + "/v1/portfolios", + json={"asset_id": _ASSET_UUID, "broker": "kiwoom", "strategy_id": "strat-001"}, + headers=_auth("admin-tok"), + ) + assert missing.status_code == 404 + assert invalid.status_code == 422 + assert inactive.status_code == 409 + + +def test_patch_strategy_reference_errors_are_404_and_409() -> None: + repo = _repo_with_portfolio(owner=_OTHER_USER_ID) + client = _make_client() + with _with_repo(repo): + missing = client.patch( + f"/v1/portfolios/{_PORTFOLIO_UUID}", + json={"strategy_id": "missing"}, + headers=_auth("admin-tok"), + ) + repo._strategy_set.status = "inactive" + inactive = client.patch( + f"/v1/portfolios/{_PORTFOLIO_UUID}", + json={"strategy_id": "strat-001"}, + headers=_auth("admin-tok"), + ) + assert missing.status_code == 404 + assert inactive.status_code == 409 diff --git a/tests/test_v1_portfolios_list.py b/tests/test_v1_portfolios_list.py index 0c70277..3b3f575 100644 --- a/tests/test_v1_portfolios_list.py +++ b/tests/test_v1_portfolios_list.py @@ -49,6 +49,7 @@ def _make_portfolio( active_strategy_id: str | None = None, strategy_name: str | None = None, strategy_params: dict | None = None, + strategy_revision: int | None = None, markets: list[str] | None = None, ) -> Portfolio: return Portfolio( @@ -69,6 +70,7 @@ def _make_portfolio( active_strategy_id=active_strategy_id, strategy_name=strategy_name, strategy_params=strategy_params if strategy_params is not None else {}, + strategy_revision=strategy_revision, markets=markets if markets is not None else [], ) @@ -375,6 +377,7 @@ def test_req055_ac1_bound_portfolio_returns_strategy_fields() -> None: active_strategy_id=_STRATEGY_ID, strategy_name="tvr_v1", strategy_params={"target_weight": 0.5, "rebalance_band_pct": 0.05}, + strategy_revision=7, markets=["KRX"], ) repo = _FakeListRepo(portfolios=[p]) @@ -387,6 +390,7 @@ def test_req055_ac1_bound_portfolio_returns_strategy_fields() -> None: assert item["active_strategy_id"] == _STRATEGY_ID assert item["strategy_name"] == "tvr_v1" assert item["strategy_params"] == {"target_weight": 0.5, "rebalance_band_pct": 0.05} + assert item["strategy_revision"] == 7 def test_req055_ac2_unbound_portfolio_returns_null_strategy() -> None: diff --git a/tests/test_v1_strategies.py b/tests/test_v1_strategies.py new file mode 100644 index 0000000..ab3d2f9 --- /dev/null +++ b/tests/test_v1_strategies.py @@ -0,0 +1,205 @@ +"""Contract tests for the versioned strategy catalog API.""" + +from __future__ import annotations + +import hashlib +from contextlib import ExitStack +from datetime import datetime, timezone +from unittest.mock import patch + +from fastapi.testclient import TestClient + +from app.core.config import Settings, get_settings +from app.domain.models import StrategySet +from app.main import create_app + + +class FakeStrategyRepo: + def __init__(self) -> None: + now = datetime.now(timezone.utc) + self.strategies = { + "active-1": StrategySet( + id="active-1", name="target_value", config="{}", nick="target_value:base", + status="active", revision=1, created_at=now, updated_at=now, + ), + "inactive-1": StrategySet( + id="inactive-1", name="archived", config="{}", nick="archived:base", + status="inactive", revision=3, created_at=now, updated_at=now, + ), + } + self.referenced: set[str] = set() + self.closed = False + + def close(self) -> None: + self.closed = True + + def list_strategy_sets(self, *, include_inactive: bool = True) -> list[StrategySet]: + values = list(self.strategies.values()) + return values if include_inactive else [item for item in values if item.status == "active"] + + def get_strategy_set(self, id: str) -> StrategySet | None: + return self.strategies.get(id) + + def get_or_create_strategy_set(self, name: str, config: str) -> StrategySet: + return self.create_strategy_set(name, config) + + def create_strategy_set(self, name: str, config: str, *, status: str = "active") -> StrategySet: + existing = next( + (item for item in self.strategies.values() if item.name == name and item.config == config), + None, + ) + if existing is not None: + return existing + now = datetime.now(timezone.utc) + item = StrategySet( + id=f"created-{len(self.strategies)}", + name=name, + config=config, + nick=f"{name}:{hashlib.sha1(config.encode()).hexdigest()[:12]}", + status=status, + created_at=now, + updated_at=now, + ) + self.strategies[item.id] = item + return item + + def update_strategy_set( + self, + id: str, + *, + name: str | None = None, + config: str | None = None, + status: str | None = None, + ) -> StrategySet: + item = self.strategies.get(id) + if item is None: + raise ValueError("전략을 찾을 수 없습니다") + final_name = name if name is not None else item.name + final_config = config if config is not None else item.config + if any( + other.id != id and other.name == final_name and other.config == final_config + for other in self.strategies.values() + ): + raise ValueError("동일한 name/config 전략이 이미 존재합니다") + final_status = status if status is not None else item.status + if (final_name, final_config, final_status) == (item.name, item.config, item.status): + return item + item.name = final_name + item.config = final_config + item.status = final_status + item.nick = f"{final_name}:{hashlib.sha1(final_config.encode()).hexdigest()[:12]}" + item.revision += 1 + item.updated_at = datetime.now(timezone.utc) + return item + + def delete_strategy_set(self, id: str) -> None: + if id in self.referenced: + raise ValueError("Strategy set is in use by 1 portfolio(s) and cannot be deleted") + if id not in self.strategies: + raise ValueError("전략을 찾을 수 없습니다") + del self.strategies[id] + + +def _client() -> TestClient: + settings = Settings( + allowed_origins_raw="http://localhost:3000", + admin_tokens_raw="admin-token", + read_tokens_raw="read-token", + ) + app = create_app(settings_factory=lambda: settings) + app.dependency_overrides[get_settings] = lambda: settings + return TestClient(app) + + +def _repos(repo: FakeStrategyRepo) -> ExitStack: + stack = ExitStack() + stack.enter_context(patch("app.api.v1_strategies.get_portfolio_repository_from_settings", return_value=repo)) + stack.enter_context(patch("app.core.auth.get_portfolio_repository_from_settings", return_value=repo)) + return stack + + +def _auth(token: str) -> dict[str, str]: + return {"Authorization": f"Bearer {token}"} + + +def test_public_catalog_excludes_inactive_and_inactive_lookup_conflicts() -> None: + repo = FakeStrategyRepo() + with _repos(repo): + response = _client().get("/v1/strategies", headers=_auth("read-token")) + inactive = _client().get("/v1/strategies/inactive-1", headers=_auth("read-token")) + assert response.status_code == 200 + assert [item["strategy_id"] for item in response.json()] == ["active-1"] + assert inactive.status_code == 409 + + +def test_admin_catalog_requires_admin_and_reports_missing_strategy() -> None: + repo = FakeStrategyRepo() + with _repos(repo): + forbidden = _client().get("/v1/admin/strategies", headers=_auth("read-token")) + missing = _client().get("/v1/admin/strategies/missing", headers=_auth("admin-token")) + assert forbidden.status_code == 403 + assert missing.status_code == 404 + + +def test_admin_create_and_patch_preserve_id_and_revision_semantics() -> None: + repo = FakeStrategyRepo() + with _repos(repo): + created = _client().post( + "/v1/admin/strategies", + json={"name": "momentum", "config": {"window": 20}}, + headers=_auth("admin-token"), + ) + strategy_id = created.json()["strategy_id"] + changed = _client().patch( + f"/v1/admin/strategies/{strategy_id}", + json={"name": "momentum-v2"}, + headers=_auth("admin-token"), + ) + no_op = _client().patch( + f"/v1/admin/strategies/{strategy_id}", + json={"name": "momentum-v2"}, + headers=_auth("admin-token"), + ) + assert created.status_code == 201 + assert created.json()["revision"] == 1 + assert changed.status_code == 200 + assert changed.json()["strategy_id"] == strategy_id + assert changed.json()["revision"] == 2 + assert no_op.json()["revision"] == 2 + assert repo.strategies[strategy_id].nick.startswith("momentum-v2:") + + +def test_admin_create_inactive_starts_at_revision_one() -> None: + repo = FakeStrategyRepo() + with _repos(repo): + created = _client().post( + "/v1/admin/strategies", + json={"name": "retired", "config": {}, "status": "inactive"}, + headers=_auth("admin-token"), + ) + assert created.status_code == 201 + assert created.json()["status"] == "inactive" + assert created.json()["revision"] == 1 + + +def test_admin_patch_validation_conflict_and_reference_safe_delete() -> None: + repo = FakeStrategyRepo() + repo.referenced.add("active-1") + with _repos(repo): + empty = _client().patch("/v1/admin/strategies/active-1", json={}, headers=_auth("admin-token")) + conflict = _client().patch( + "/v1/admin/strategies/inactive-1", + json={"name": "target_value"}, + headers=_auth("admin-token"), + ) + referenced = _client().delete("/v1/admin/strategies/active-1", headers=_auth("admin-token")) + deleted = _client().delete("/v1/admin/strategies/inactive-1", headers=_auth("admin-token")) + assert empty.status_code == 422 + assert conflict.status_code == 409 + assert referenced.status_code == 409 + assert deleted.status_code == 204 + + +def test_legacy_strategy_set_routes_are_removed() -> None: + response = _client().get("/strategy-sets", headers=_auth("admin-token")) + assert response.status_code == 404