diff --git a/app/api/v1_strategies.py b/app/api/v1_strategies.py index 1b224f2..b6e9eb8 100644 --- a/app/api/v1_strategies.py +++ b/app/api/v1_strategies.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from typing import NoReturn from fastapi import APIRouter, Depends, HTTPException, Response, status @@ -11,7 +12,11 @@ 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.schemas.v1 import ( + StrategyCreateRequest, + StrategyResponse, + StrategyUpdateRequest, +) from app.services.strategy_set_service import ( StrategyConflictError, StrategyInactiveError, @@ -44,6 +49,7 @@ def _response(strategy: StrategySet) -> StrategyResponse: parsed = {} return StrategyResponse( strategy_id=strategy.id, + nick=strategy.nick, name=strategy.name, config=parsed if isinstance(parsed, dict) else {}, status=strategy.status, @@ -53,7 +59,7 @@ def _response(strategy: StrategySet) -> StrategyResponse: ) -def _raise_strategy_error(exc: ValueError) -> None: +def _raise_strategy_error(exc: ValueError) -> NoReturn: if isinstance(exc, StrategyNotFoundError): raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc if isinstance(exc, (StrategyInactiveError, StrategyInUseError, StrategyConflictError)): @@ -129,14 +135,18 @@ def create_admin_strategy( ) -> StrategyResponse: service, repository = _get_service(settings) try: - strategy = service.create( - CreateStrategySetRequest( - name=body.name, - config=_canonical_config(body.config), - status=body.status, + try: + strategy = service.create( + CreateStrategySetRequest( + name=body.name, + nick=body.nick, + config=_canonical_config(body.config), + status=body.status, + ) ) - ) - return _response(strategy) + return _response(strategy) + except ValueError as exc: + _raise_strategy_error(exc) finally: _close_repository(repository) @@ -155,6 +165,7 @@ def update_admin_strategy( strategy_id, UpdateStrategySetRequest( name=body.name, + nick=body.nick, config=_canonical_config(body.config) if body.config is not None else None, status=body.status, ), diff --git a/app/repositories/portfolio_repository.py b/app/repositories/portfolio_repository.py index bbba237..3f8bdfd 100644 --- a/app/repositories/portfolio_repository.py +++ b/app/repositories/portfolio_repository.py @@ -243,7 +243,14 @@ 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 create_strategy_set( + self, + name: str, + config: str, + *, + nick: str | None = None, + status: str = "active", + ) -> StrategySet: ... def get_strategy_set(self, id: str) -> StrategySet | None: ... @@ -254,6 +261,7 @@ class PortfolioRepository(Protocol): id: str, *, name: str | None = None, + nick: str | None = None, config: str | None = None, status: str | None = None, ) -> StrategySet: ... diff --git a/app/repositories/postgres.py b/app/repositories/postgres.py index 9441986..b347659 100644 --- a/app/repositories/postgres.py +++ b/app/repositories/postgres.py @@ -1360,28 +1360,38 @@ class PostgresPortfolioRepository: 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: + def create_strategy_set( + self, + name: str, + config: str, + *, + nick: str | None = None, + 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, status) - VALUES (%s, %s, %s, %s, %s) - ON CONFLICT (name, config) DO NOTHING - """, - (new_id, name, config, nick, status), - ) - cur.execute( - """ - 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() + final_nick = nick or f"{name}:{hashlib.sha1(config.encode('utf-8')).hexdigest()[:12]}" + try: + with self._connect().cursor() as cur: + new_id = str(uuid.uuid4()) + cur.execute( + """ + 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, final_nick, status), + ) + cur.execute( + """ + 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() + except psycopg.errors.UniqueViolation as exc: + raise ValueError("동일한 nick 전략이 이미 존재합니다") from exc return self._row_to_strategy_set(row) def get_strategy_set(self, id: str) -> StrategySet | None: @@ -1414,6 +1424,7 @@ class PostgresPortfolioRepository: id: str, *, name: str | None = None, + nick: str | None = None, config: str | None = None, status: str | None = None, ) -> StrategySet: @@ -1435,15 +1446,16 @@ class PostgresPortfolioRepository: raise ValueError("전략을 찾을 수 없습니다") current = self._row_to_strategy_set(row) final_name = name if name is not None else current.name + final_nick = nick if nick is not None else current.nick 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_nick == current.nick 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 @@ -1456,11 +1468,11 @@ class PostgresPortfolioRepository: WHERE id = %s RETURNING id, name, config, nick, status, revision, created_at, updated_at """, - (final_name, final_config, nick, final_status, id), + (final_name, final_config, final_nick, final_status, id), ) updated = cur.fetchone() except psycopg.errors.UniqueViolation as exc: - raise ValueError("동일한 name/config 전략이 이미 존재합니다") from exc + raise ValueError("동일한 name/config 또는 nick 전략이 이미 존재합니다") from exc if updated is None: raise ValueError("전략을 찾을 수 없습니다") return self._row_to_strategy_set(updated) diff --git a/app/schemas/portfolio.py b/app/schemas/portfolio.py index c238f10..42b3174 100644 --- a/app/schemas/portfolio.py +++ b/app/schemas/portfolio.py @@ -186,6 +186,7 @@ class CreateStrategySetRequest(BaseModel): model_config = ConfigDict(extra="forbid") name: str + nick: str | None = Field(default=None, min_length=1) config: str status: Literal["active", "inactive"] = "active" @@ -194,6 +195,7 @@ class UpdateStrategySetRequest(BaseModel): model_config = ConfigDict(extra="forbid") name: str | None = None + nick: str | None = Field(default=None, min_length=1) config: str | None = None status: Literal["active", "inactive"] | None = None diff --git a/app/schemas/v1.py b/app/schemas/v1.py index caf1c99..b465fa1 100644 --- a/app/schemas/v1.py +++ b/app/schemas/v1.py @@ -305,6 +305,7 @@ StrategyStatus = Literal["active", "inactive"] class StrategyResponse(BaseModel): strategy_id: str + nick: str name: str config: dict[str, Any] = Field(default_factory=dict) status: StrategyStatus @@ -317,6 +318,7 @@ class StrategyCreateRequest(BaseModel): model_config = ConfigDict(extra="forbid") name: str = Field(..., min_length=1) + nick: str | None = Field(default=None, min_length=1) config: dict[str, Any] = Field(default_factory=dict) status: StrategyStatus = "active" @@ -325,6 +327,7 @@ class StrategyUpdateRequest(BaseModel): model_config = ConfigDict(extra="forbid") name: str | None = Field(default=None, min_length=1) + nick: str | None = Field(default=None, min_length=1) config: dict[str, Any] | None = None status: StrategyStatus | None = None diff --git a/app/services/strategy_set_service.py b/app/services/strategy_set_service.py index d654c35..1c941b8 100644 --- a/app/services/strategy_set_service.py +++ b/app/services/strategy_set_service.py @@ -46,26 +46,49 @@ class StrategySetService: return self.get(strategy_set_id, active_only=True) def create(self, payload: CreateStrategySetRequest) -> StrategySet: - 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), + try: + create_strategy = cast( + Callable[..., StrategySet] | None, + getattr(self._repository, "create_strategy_set", None), ) - return strategy + if create_strategy is not None: + strategy = create_strategy( + payload.name, + payload.config, + nick=payload.nick, + status=payload.status, + ) + else: + strategy = self._repository.get_or_create_strategy_set(payload.name, payload.config) + if payload.nick is not None and payload.nick != strategy.nick: + strategy = self.update( + strategy.id, + UpdateStrategySetRequest(nick=payload.nick), + ) + if payload.status != strategy.status: + strategy = self.update( + strategy.id, + UpdateStrategySetRequest(status=payload.status), + ) + return strategy + except StrategyConflictError: + raise + except ValueError as exc: + raise StrategyConflictError(str(exc)) from exc def update(self, strategy_set_id: str, payload: UpdateStrategySetRequest) -> StrategySet: try: + if payload.nick is None: + return self._repository.update_strategy_set( + strategy_set_id, + name=payload.name, + config=payload.config, + status=payload.status, + ) return self._repository.update_strategy_set( strategy_set_id, name=payload.name, + nick=payload.nick, config=payload.config, status=payload.status, ) diff --git a/tests/test_strategy_catalog_postgres_integration.py b/tests/test_strategy_catalog_postgres_integration.py index 14d1e06..6dadedc 100644 --- a/tests/test_strategy_catalog_postgres_integration.py +++ b/tests/test_strategy_catalog_postgres_integration.py @@ -6,7 +6,6 @@ PostgreSQL endpoint. Normal unit-test environments skip this module. from __future__ import annotations -import hashlib import os import subprocess import threading @@ -73,9 +72,12 @@ def _prepare_catalog_fixture(repo: PostgresPortfolioRepository) -> tuple[str, st return strategy.id, asset_id, owner_user_id -def test_concurrent_strategy_patch_keeps_derived_nick_consistent() -> None: +def test_concurrent_strategy_patch_preserves_existing_nick() -> None: owner = _repo() strategy_id, asset_id, owner_user_id = _prepare_catalog_fixture(owner) + initial = owner.get_strategy_set(strategy_id) + assert initial is not None + initial_nick = initial.nick renamed = f"concurrent-renamed-{uuid.uuid4().hex}" barrier = threading.Barrier(3) errors: list[BaseException] = [] @@ -114,8 +116,7 @@ def test_concurrent_strategy_patch_keeps_derived_nick_consistent() -> 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 + assert final.nick == initial_nick portfolio_id = str(uuid.uuid4()) owner.create_portfolio( portfolio_id, diff --git a/tests/test_v1_strategies.py b/tests/test_v1_strategies.py index ab3d2f9..bc58c28 100644 --- a/tests/test_v1_strategies.py +++ b/tests/test_v1_strategies.py @@ -43,19 +43,29 @@ class FakeStrategyRepo: 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: + def create_strategy_set( + self, + name: str, + config: str, + *, + nick: str | None = None, + 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 + final_nick = nick or f"{name}:{hashlib.sha1(config.encode()).hexdigest()[:12]}" + if any(item.nick == final_nick for item in self.strategies.values()): + raise ValueError("동일한 nick 전략이 이미 존재합니다") 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]}", + nick=final_nick, status=status, created_at=now, updated_at=now, @@ -68,6 +78,7 @@ class FakeStrategyRepo: id: str, *, name: str | None = None, + nick: str | None = None, config: str | None = None, status: str | None = None, ) -> StrategySet: @@ -75,19 +86,27 @@ class FakeStrategyRepo: if item is None: raise ValueError("전략을 찾을 수 없습니다") final_name = name if name is not None else item.name + final_nick = nick if nick is not None else item.nick 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 전략이 이미 존재합니다") + if any(other.id != id and other.nick == final_nick for other in self.strategies.values()): + raise ValueError("동일한 nick 전략이 이미 존재합니다") final_status = status if status is not None else item.status - if (final_name, final_config, final_status) == (item.name, item.config, item.status): + if (final_name, final_nick, final_config, final_status) == ( + item.name, + item.nick, + item.config, + item.status, + ): return item item.name = final_name + item.nick = final_nick 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 @@ -126,9 +145,13 @@ def test_public_catalog_excludes_inactive_and_inactive_lookup_conflicts() -> Non repo = FakeStrategyRepo() with _repos(repo): response = _client().get("/v1/strategies", headers=_auth("read-token")) + active = _client().get("/v1/strategies/active-1", 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 response.json()[0]["nick"] == "target_value:base" + assert active.status_code == 200 + assert active.json()["nick"] == "target_value:base" assert inactive.status_code == 409 @@ -141,32 +164,78 @@ def test_admin_catalog_requires_admin_and_reports_missing_strategy() -> None: assert missing.status_code == 404 -def test_admin_create_and_patch_preserve_id_and_revision_semantics() -> None: +def test_admin_create_custom_nick_and_patch_nick_only_preserve_id_and_increment_revision() -> None: repo = FakeStrategyRepo() with _repos(repo): created = _client().post( "/v1/admin/strategies", - json={"name": "momentum", "config": {"window": 20}}, + json={"name": "momentum", "nick": "mom-20", "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"}, + json={"nick": "mom-primary"}, headers=_auth("admin-token"), ) no_op = _client().patch( f"/v1/admin/strategies/{strategy_id}", - json={"name": "momentum-v2"}, + json={"nick": "mom-primary"}, headers=_auth("admin-token"), ) assert created.status_code == 201 + assert created.json()["nick"] == "mom-20" assert created.json()["revision"] == 1 assert changed.status_code == 200 assert changed.json()["strategy_id"] == strategy_id + assert changed.json()["nick"] == "mom-primary" assert changed.json()["revision"] == 2 assert no_op.json()["revision"] == 2 - assert repo.strategies[strategy_id].nick.startswith("momentum-v2:") + assert repo.strategies[strategy_id].nick == "mom-primary" + + +def test_admin_patch_omitting_nick_preserves_custom_nick_when_name_and_config_change() -> None: + repo = FakeStrategyRepo() + with _repos(repo): + created = _client().post( + "/v1/admin/strategies", + json={"name": "momentum", "nick": "mom-custom", "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", "config": {"window": 30}}, + headers=_auth("admin-token"), + ) + assert changed.status_code == 200 + assert changed.json()["nick"] == "mom-custom" + assert changed.json()["revision"] == 2 + + +def test_admin_create_omitting_nick_retains_legacy_generated_value() -> None: + repo = FakeStrategyRepo() + with _repos(repo): + created = _client().post( + "/v1/admin/strategies", + json={"name": "legacy", "config": {"window": 20}}, + headers=_auth("admin-token"), + ) + canonical_config = '{"window":20}' + expected = f"legacy:{hashlib.sha1(canonical_config.encode()).hexdigest()[:12]}" + assert created.status_code == 201 + assert created.json()["nick"] == expected + + +def test_admin_duplicate_nick_returns_conflict() -> None: + repo = FakeStrategyRepo() + with _repos(repo): + duplicate = _client().post( + "/v1/admin/strategies", + json={"name": "duplicate", "nick": "target_value:base", "config": {}}, + headers=_auth("admin-token"), + ) + assert duplicate.status_code == 409 def test_admin_create_inactive_starts_at_revision_one() -> None: