feat(strategy): expose configurable nick contract
This commit is contained in:
@@ -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,
|
||||
),
|
||||
|
||||
@@ -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: ...
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user