From 770af1fe763fbe4baa56e4dca07d79d0ce735fc7 Mon Sep 17 00:00:00 2001 From: Azamat Date: Thu, 2 Apr 2026 23:09:04 +0300 Subject: [PATCH] [feat] change str id type to UUID --- adapter/docker/runtime.py | 36 ++++------ adapter/http/fastapi/schemas.py | 15 ++-- domain/sandbox.py | 5 +- repository/sandbox_lock.py | 5 +- repository/sandbox_session.py | 7 +- tasks.md | 11 +++ test/test_create_http.py | 20 +++--- test/test_docker_runtime.py | 43 ++++-------- test/test_sandbox_usecase.py | 121 +++++++++++++++++--------------- usecase/interface.py | 11 +-- usecase/sandbox.py | 49 +++++-------- 11 files changed, 150 insertions(+), 173 deletions(-) diff --git a/adapter/docker/runtime.py b/adapter/docker/runtime.py index d24d110..ad6e84a 100644 --- a/adapter/docker/runtime.py +++ b/adapter/docker/runtime.py @@ -24,16 +24,13 @@ class DockerSandboxRuntime(SandboxRuntime): def create( self, *, - session_id: str, - chat_id: str, + session_id: UUID, + chat_id: UUID, created_at: datetime, expires_at: datetime, ) -> SandboxSession: - normalized_chat_id = chat_id - try: - normalized_chat_id = _canonical_chat_id(chat_id) - chat_path = self._chat_path(normalized_chat_id) + chat_path = self._chat_path(chat_id) dependencies_path = self._readonly_host_path( self._config.dependencies_host_path ) @@ -44,19 +41,19 @@ class DockerSandboxRuntime(SandboxRuntime): container = self._client.containers.run( self._config.image, detach=True, - labels=self._labels(session_id, normalized_chat_id, expires_at), + labels=self._labels(session_id, chat_id, expires_at), mounts=self._mounts(chat_path, dependencies_path, lambda_tools_path), ) except (DockerException, OSError, ValueError) as exc: - raise SandboxStartError(normalized_chat_id) from exc + raise SandboxStartError(str(chat_id)) from exc container_id = str(getattr(container, 'id', '')).strip() if not container_id: - raise SandboxStartError(normalized_chat_id) + raise SandboxStartError(str(chat_id)) return SandboxSession( session_id=session_id, - chat_id=normalized_chat_id, + chat_id=chat_id, container_id=container_id, status=SandboxStatus.RUNNING, created_at=created_at, @@ -74,13 +71,13 @@ class DockerSandboxRuntime(SandboxRuntime): def _labels( self, - session_id: str, - chat_id: str, + session_id: UUID, + chat_id: UUID, expires_at: datetime, ) -> dict[str, str]: return { - 'session_id': session_id, - 'chat_id': chat_id, + 'session_id': str(session_id), + 'chat_id': str(chat_id), 'expires_at': expires_at.isoformat(), } @@ -110,12 +107,9 @@ class DockerSandboxRuntime(SandboxRuntime): ), ] - def _chat_path(self, chat_id: str) -> Path: - if not chat_id.strip(): - raise ValueError('invalid chat path') - + def _chat_path(self, chat_id: UUID) -> Path: chats_root = self._host_path(self._config.chats_root) - chat_path = (chats_root / chat_id).resolve(strict=False) + chat_path = (chats_root / str(chat_id)).resolve(strict=False) if not chat_path.is_relative_to(chats_root): raise ValueError('invalid chat path') return chat_path @@ -128,7 +122,3 @@ class DockerSandboxRuntime(SandboxRuntime): def _host_path(self, path_value: str) -> Path: return Path(path_value).expanduser().resolve(strict=False) - - -def _canonical_chat_id(chat_id: str) -> str: - return str(UUID(str(chat_id).strip())) diff --git a/adapter/http/fastapi/schemas.py b/adapter/http/fastapi/schemas.py index a34f702..35992ee 100644 --- a/adapter/http/fastapi/schemas.py +++ b/adapter/http/fastapi/schemas.py @@ -1,7 +1,7 @@ from datetime import datetime from uuid import UUID -from pydantic import BaseModel, ConfigDict, Field, field_validator +from pydantic import BaseModel, ConfigDict class HealthResponse(BaseModel): @@ -11,19 +11,14 @@ class HealthResponse(BaseModel): class CreateSandboxRequest(BaseModel): - model_config = ConfigDict(extra='forbid', str_strip_whitespace=True) + model_config = ConfigDict(extra='forbid') - chat_id: str = Field(min_length=1) - - @field_validator('chat_id') - @classmethod - def validate_chat_id(cls, value: str) -> str: - return str(UUID(value)) + chat_id: UUID class SandboxSessionResponse(BaseModel): - session_id: str - chat_id: str + session_id: UUID + chat_id: UUID container_id: str status: str expires_at: datetime diff --git a/domain/sandbox.py b/domain/sandbox.py index 110b4e4..a9b0f40 100644 --- a/domain/sandbox.py +++ b/domain/sandbox.py @@ -1,6 +1,7 @@ from dataclasses import dataclass from datetime import datetime from enum import Enum +from uuid import UUID class SandboxStatus(str, Enum): @@ -13,8 +14,8 @@ class SandboxStatus(str, Enum): @dataclass(frozen=True, slots=True) class SandboxSession: - session_id: str - chat_id: str + session_id: UUID + chat_id: UUID container_id: str status: SandboxStatus created_at: datetime diff --git a/repository/sandbox_lock.py b/repository/sandbox_lock.py index 704aeae..b13cd65 100644 --- a/repository/sandbox_lock.py +++ b/repository/sandbox_lock.py @@ -1,6 +1,7 @@ import threading from types import TracebackType from typing import Protocol +from uuid import UUID from usecase.interface import LockContext, SandboxLifecycleLocker @@ -31,9 +32,9 @@ class _ChatLock(LockContext): class ProcessLocalSandboxLifecycleLocker(SandboxLifecycleLocker): def __init__(self) -> None: self._registry_lock = threading.Lock() - self._locks_by_chat_id: dict[str, _SyncLock] = {} + self._locks_by_chat_id: dict[UUID, _SyncLock] = {} - def lock(self, chat_id: str) -> LockContext: + def lock(self, chat_id: UUID) -> LockContext: with self._registry_lock: lock = self._locks_by_chat_id.get(chat_id) if lock is None: diff --git a/repository/sandbox_session.py b/repository/sandbox_session.py index 6707d0c..3a8857f 100644 --- a/repository/sandbox_session.py +++ b/repository/sandbox_session.py @@ -1,5 +1,6 @@ import threading from datetime import datetime +from uuid import UUID from domain.sandbox import SandboxSession from usecase.interface import SandboxSessionRepository @@ -7,10 +8,10 @@ from usecase.interface import SandboxSessionRepository class InMemorySandboxSessionRepository(SandboxSessionRepository): def __init__(self) -> None: - self._sessions_by_chat_id: dict[str, SandboxSession] = {} + self._sessions_by_chat_id: dict[UUID, SandboxSession] = {} self._lock = threading.Lock() - def get_active_by_chat_id(self, chat_id: str) -> SandboxSession | None: + def get_active_by_chat_id(self, chat_id: UUID) -> SandboxSession | None: with self._lock: return self._sessions_by_chat_id.get(chat_id) @@ -26,7 +27,7 @@ class InMemorySandboxSessionRepository(SandboxSessionRepository): with self._lock: self._sessions_by_chat_id[session.chat_id] = session - def delete(self, session_id: str) -> None: + def delete(self, session_id: UUID) -> None: with self._lock: for chat_id, session in tuple(self._sessions_by_chat_id.items()): if session.session_id == session_id: diff --git a/tasks.md b/tasks.md index cb1e38f..058983e 100644 --- a/tasks.md +++ b/tasks.md @@ -216,3 +216,14 @@ - Файлы: `repository/sandbox_lock.py`, при необходимости тесты в `test/*` - Решение: добавить eviction/ref-count/weakref policy во внешнем lock registry без нарушения сериализации lifecycle для активного `chat_id` - Критерии приемки: registry locks не растет бесконечно без причины; сериализация для активных чатов сохраняется; поведение покрыто тестами + +### M18. Перевести sandbox ids на UUID types + +- Субагент: `feature-developer` +- Статус: completed +- Зависимости: `M15` +- Commit required: no +- Scope: сделать `chat_id` и `session_id` типом `UUID` внутри sandbox scope, оставив `container_id` строкой как внешний Docker identifier +- Файлы: `domain/sandbox.py`, `usecase/interface.py`, `usecase/sandbox.py`, `repository/sandbox_session.py`, `adapter/http/fastapi/*`, `adapter/docker/runtime.py`, `adapter/di/container.py`, `test/*` +- Решение: HTTP boundary принимает/возвращает UUID, usecase и repository работают с UUID objects, Docker labels продолжают сериализоваться в строки через `str(uuid)` +- Критерии приемки: внутри sandbox flow `chat_id` и `session_id` больше не строки; `container_id` остается `str`; pydantic корректно сериализует UUID в response; `make pre-commit` проходит diff --git a/test/test_create_http.py b/test/test_create_http.py index bbdbc6d..2a474ff 100644 --- a/test/test_create_http.py +++ b/test/test_create_http.py @@ -1,6 +1,7 @@ import asyncio import json from datetime import UTC, datetime, timedelta +from uuid import UUID from docker import DockerClient from fastapi import FastAPI @@ -28,8 +29,9 @@ from repository.sandbox_session import InMemorySandboxSessionRepository from usecase.interface import Attrs from usecase.sandbox import CleanupExpiredSandboxes, CreateSandbox, CreateSandboxCommand -CHAT_ID = '123e4567-e89b-12d3-a456-426614174000' +CHAT_ID = UUID('123e4567-e89b-12d3-a456-426614174000') NON_CANONICAL_CHAT_ID = '123E4567E89B12D3A456426614174000' +SESSION_ID = UUID('00000000-0000-0000-0000-000000000011') class FakeLogger: @@ -253,7 +255,7 @@ def test_post_create_returns_session_with_canonical_chat_id(monkeypatch) -> None config = build_config() expires_at = datetime(2026, 4, 2, 12, 5, tzinfo=UTC) session = SandboxSession( - session_id='session-123', + session_id=SESSION_ID, chat_id=CHAT_ID, container_id='container-123', status=SandboxStatus.RUNNING, @@ -284,8 +286,8 @@ def test_post_create_returns_session_with_canonical_chat_id(monkeypatch) -> None assert status_code == 200 assert response == { - 'session_id': 'session-123', - 'chat_id': CHAT_ID, + 'session_id': str(SESSION_ID), + 'chat_id': str(CHAT_ID), 'container_id': 'container-123', 'status': 'running', 'expires_at': '2026-04-02T12:05:00Z', @@ -306,7 +308,7 @@ def test_post_create_rejects_non_uuid_chat_id(monkeypatch) -> None: config = build_config() expires_at = datetime(2026, 4, 2, 12, 5, tzinfo=UTC) session = SandboxSession( - session_id='session-123', + session_id=SESSION_ID, chat_id=CHAT_ID, container_id='container-123', status=SandboxStatus.RUNNING, @@ -344,7 +346,7 @@ def test_post_create_rejects_non_uuid_chat_id(monkeypatch) -> None: def test_post_create_maps_start_errors_to_service_unavailable(monkeypatch) -> None: config = build_config() logger = FakeLogger() - create_usecase = FakeCreateSandboxUsecase(error=SandboxStartError(CHAT_ID)) + create_usecase = FakeCreateSandboxUsecase(error=SandboxStartError(str(CHAT_ID))) cleanup_usecase = FakeCleanupExpiredSandboxes() docker_client = FakeDockerClient() container = build_container( @@ -362,7 +364,7 @@ def test_post_create_maps_start_errors_to_service_unavailable(monkeypatch) -> No app = app_module.create_app(config=config) status_code, response = asyncio.run( - exercise_create_request(app, {'chat_id': CHAT_ID}) + exercise_create_request(app, {'chat_id': str(CHAT_ID)}) ) assert status_code == 503 @@ -391,7 +393,7 @@ def test_post_create_maps_generic_sandbox_errors_to_internal_error(monkeypatch) app = app_module.create_app(config=config) status_code, response = asyncio.run( - exercise_create_request(app, {'chat_id': CHAT_ID}) + exercise_create_request(app, {'chat_id': str(CHAT_ID)}) ) assert status_code == 500 @@ -403,7 +405,7 @@ def test_removed_user_endpoint_returns_not_found(monkeypatch) -> None: config = build_config() expires_at = datetime(2026, 4, 2, 12, 5, tzinfo=UTC) session = SandboxSession( - session_id='session-123', + session_id=SESSION_ID, chat_id=CHAT_ID, container_id='container-123', status=SandboxStatus.RUNNING, diff --git a/test/test_docker_runtime.py b/test/test_docker_runtime.py index 829024d..d266eff 100644 --- a/test/test_docker_runtime.py +++ b/test/test_docker_runtime.py @@ -1,6 +1,7 @@ from datetime import UTC, datetime, timedelta from pathlib import Path from typing import Any, TypedDict +from uuid import UUID import pytest from docker import DockerClient @@ -12,8 +13,9 @@ from adapter.docker.runtime import DockerSandboxRuntime from domain.error import SandboxError, SandboxStartError from domain.sandbox import SandboxStatus -CHAT_ID = '123e4567-e89b-12d3-a456-426614174000' +CHAT_ID = UUID('123e4567-e89b-12d3-a456-426614174000') NON_CANONICAL_CHAT_ID = '123E4567E89B12D3A456426614174000' +SESSION_ID = UUID('00000000-0000-0000-0000-000000000010') class FakeContainer: @@ -107,26 +109,26 @@ def test_runtime_create_applies_mount_policy_and_labels_with_canonical_chat_id( expires_at = created_at + timedelta(minutes=5) session = runtime.create( - session_id='session-123', - chat_id=NON_CANONICAL_CHAT_ID, + session_id=SESSION_ID, + chat_id=UUID(NON_CANONICAL_CHAT_ID), created_at=created_at, expires_at=expires_at, ) - assert session.session_id == 'session-123' + assert session.session_id == SESSION_ID assert session.chat_id == CHAT_ID assert session.container_id == 'container-123' assert session.status is SandboxStatus.RUNNING assert session.created_at == created_at assert session.expires_at == expires_at - assert (tmp_path / 'chats' / CHAT_ID).is_dir() + assert (tmp_path / 'chats' / str(CHAT_ID)).is_dir() call = containers.run_calls[0] assert call['args'] == ('sandbox:latest',) assert call['kwargs']['detach'] is True assert call['kwargs']['labels'] == { - 'session_id': 'session-123', - 'chat_id': CHAT_ID, + 'session_id': str(SESSION_ID), + 'chat_id': str(CHAT_ID), 'expires_at': expires_at.isoformat(), } @@ -134,7 +136,7 @@ def test_runtime_create_applies_mount_policy_and_labels_with_canonical_chat_id( assert [dict(mount) for mount in mounts] == [ { 'Target': '/workspace/chat', - 'Source': str((tmp_path / 'chats' / CHAT_ID).resolve(strict=False)), + 'Source': str((tmp_path / 'chats' / str(CHAT_ID)).resolve(strict=False)), 'Type': 'bind', 'ReadOnly': False, }, @@ -164,14 +166,14 @@ def test_runtime_create_raises_start_error_when_container_id_is_missing( with pytest.raises(SandboxStartError) as excinfo: runtime.create( - session_id='session-123', + session_id=SESSION_ID, chat_id=CHAT_ID, created_at=datetime(2026, 4, 2, 12, 0, tzinfo=UTC), expires_at=datetime(2026, 4, 2, 12, 5, tzinfo=UTC), ) assert str(excinfo.value) == 'sandbox_start_failed' - assert excinfo.value.chat_id == CHAT_ID + assert excinfo.value.chat_id == str(CHAT_ID) def test_runtime_stop_ignores_missing_container(tmp_path: Path) -> None: @@ -195,24 +197,3 @@ def test_runtime_stop_wraps_docker_errors(tmp_path: Path) -> None: runtime.stop('container-123') assert str(excinfo.value) == 'sandbox_stop_failed' - - -@pytest.mark.parametrize('chat_id', ['.', 'a/..', 'x/../y']) -def test_runtime_create_rejects_non_uuid_chat_id(tmp_path: Path, chat_id: str) -> None: - config = build_config(tmp_path) - (tmp_path / 'dependencies').mkdir() - (tmp_path / 'lambda-tools').mkdir() - containers = FakeContainers() - runtime = DockerSandboxRuntime(config, FakeDockerClient(containers)) - - with pytest.raises(SandboxStartError) as excinfo: - runtime.create( - session_id='session-123', - chat_id=chat_id, - created_at=datetime(2026, 4, 2, 12, 0, tzinfo=UTC), - expires_at=datetime(2026, 4, 2, 12, 5, tzinfo=UTC), - ) - - assert str(excinfo.value) == 'sandbox_start_failed' - assert excinfo.value.chat_id == chat_id - assert containers.run_calls == [] diff --git a/test/test_sandbox_usecase.py b/test/test_sandbox_usecase.py index 26e094f..4fedb21 100644 --- a/test/test_sandbox_usecase.py +++ b/test/test_sandbox_usecase.py @@ -1,18 +1,28 @@ import threading from datetime import UTC, datetime, timedelta +from uuid import UUID from domain.sandbox import SandboxSession, SandboxStatus from repository.sandbox_lock import ProcessLocalSandboxLifecycleLocker from repository.sandbox_session import InMemorySandboxSessionRepository from usecase.sandbox import CleanupExpiredSandboxes, CreateSandbox, CreateSandboxCommand -CHAT_ID = '11111111-1111-1111-1111-111111111111' +CHAT_ID = UUID('11111111-1111-1111-1111-111111111111') NON_CANONICAL_CHAT_ID = '11111111111111111111111111111111' -EXPIRED_CHAT_ID = '22222222-2222-2222-2222-222222222222' -BOUNDARY_CHAT_ID = '33333333-3333-3333-3333-333333333333' -ACTIVE_CHAT_ID = '44444444-4444-4444-4444-444444444444' -FAIL_CHAT_ID = '55555555-5555-5555-5555-555555555555' -CLEAN_CHAT_ID = '66666666-6666-6666-6666-666666666666' +EXPIRED_CHAT_ID = UUID('22222222-2222-2222-2222-222222222222') +BOUNDARY_CHAT_ID = UUID('33333333-3333-3333-3333-333333333333') +ACTIVE_CHAT_ID = UUID('44444444-4444-4444-4444-444444444444') +FAIL_CHAT_ID = UUID('55555555-5555-5555-5555-555555555555') +CLEAN_CHAT_ID = UUID('66666666-6666-6666-6666-666666666666') +SESSION_REUSED_ID = UUID('00000000-0000-0000-0000-000000000001') +SESSION_OLD_ID = UUID('00000000-0000-0000-0000-000000000002') +SESSION_NEW_ID = UUID('00000000-0000-0000-0000-000000000003') +SESSION_EXPIRED_ID = UUID('00000000-0000-0000-0000-000000000004') +SESSION_BOUNDARY_ID = UUID('00000000-0000-0000-0000-000000000005') +SESSION_ACTIVE_ID = UUID('00000000-0000-0000-0000-000000000006') +SESSION_FAIL_ID = UUID('00000000-0000-0000-0000-000000000007') +SESSION_CLEAN_ID = UUID('00000000-0000-0000-0000-000000000008') +SESSION_REPLACEMENT_ID = UUID('00000000-0000-0000-0000-000000000009') class FakeClock: @@ -52,9 +62,9 @@ class FakeLockContext: class FakeLocker: def __init__(self) -> None: - self.chat_ids: list[str] = [] + self.chat_ids: list[UUID] = [] - def lock(self, chat_id: str) -> FakeLockContext: + def lock(self, chat_id: UUID) -> FakeLockContext: self.chat_ids.append(chat_id) return FakeLockContext() @@ -63,7 +73,7 @@ class TrackingLockContext: def __init__( self, locker: 'TrackingLocker', - chat_id: str, + chat_id: UUID, inner_context, ) -> None: self._locker = locker @@ -89,9 +99,9 @@ class TrackingLocker: self._state_lock = threading.Lock() self._attempts = 0 self.second_attempted = threading.Event() - self.chat_ids: list[str] = [] + self.chat_ids: list[UUID] = [] - def lock(self, chat_id: str) -> TrackingLockContext: + def lock(self, chat_id: UUID) -> TrackingLockContext: return TrackingLockContext(self, chat_id, self._locker.lock(chat_id)) @@ -105,8 +115,8 @@ class BlockingCreateRuntime: def create( self, *, - session_id: str, - chat_id: str, + session_id: UUID, + chat_id: UUID, created_at: datetime, expires_at: datetime, ) -> SandboxSession: @@ -150,8 +160,8 @@ class FakeRuntime: def create( self, *, - session_id: str, - chat_id: str, + session_id: UUID, + chat_id: UUID, created_at: datetime, expires_at: datetime, ) -> SandboxSession: @@ -190,7 +200,7 @@ class FailingStopRuntime(FakeRuntime): def test_create_sandbox_reuses_active_session_when_not_expired() -> None: now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) session = SandboxSession( - session_id='session-1', + session_id=SESSION_REUSED_ID, chat_id=CHAT_ID, container_id='container-1', status=SandboxStatus.RUNNING, @@ -223,8 +233,8 @@ def test_create_sandbox_reuses_active_session_when_not_expired() -> None: 'info', 'sandbox_reused', { - 'chat_id': CHAT_ID, - 'session_id': 'session-1', + 'chat_id': str(CHAT_ID), + 'session_id': str(SESSION_REUSED_ID), 'container_id': 'container-1', }, ) @@ -236,7 +246,7 @@ def test_create_sandbox_replaces_expired_session_and_creates_new_one( ) -> None: now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) expired_session = SandboxSession( - session_id='session-old', + session_id=SESSION_OLD_ID, chat_id=CHAT_ID, container_id='container-old', status=SandboxStatus.RUNNING, @@ -256,23 +266,23 @@ def test_create_sandbox_replaces_expired_session_and_creates_new_one( logger=logger, ttl=timedelta(minutes=5), ) - monkeypatch.setattr('usecase.sandbox._new_session_id', lambda: 'session-new') + monkeypatch.setattr('usecase.sandbox._new_session_id', lambda: SESSION_NEW_ID) result = usecase.execute(CreateSandboxCommand(chat_id=CHAT_ID)) assert runtime.stop_calls == ['container-old'] assert runtime.create_calls == [ { - 'session_id': 'session-new', + 'session_id': SESSION_NEW_ID, 'chat_id': CHAT_ID, 'created_at': now, 'expires_at': now + timedelta(minutes=5), } ] assert result == SandboxSession( - session_id='session-new', + session_id=SESSION_NEW_ID, chat_id=CHAT_ID, - container_id='container-session-new', + container_id=f'container-{SESSION_NEW_ID}', status=SandboxStatus.RUNNING, created_at=now, expires_at=now + timedelta(minutes=5), @@ -284,8 +294,8 @@ def test_create_sandbox_replaces_expired_session_and_creates_new_one( 'info', 'sandbox_replaced', { - 'chat_id': CHAT_ID, - 'session_id': 'session-old', + 'chat_id': str(CHAT_ID), + 'session_id': str(SESSION_OLD_ID), 'container_id': 'container-old', }, ), @@ -293,9 +303,9 @@ def test_create_sandbox_replaces_expired_session_and_creates_new_one( 'info', 'sandbox_created', { - 'chat_id': CHAT_ID, - 'session_id': 'session-new', - 'container_id': 'container-session-new', + 'chat_id': str(CHAT_ID), + 'session_id': str(SESSION_NEW_ID), + 'container_id': f'container-{SESSION_NEW_ID}', }, ), ] @@ -316,7 +326,7 @@ def test_create_sandbox_creates_new_session_when_none_exists() -> None: ttl=timedelta(minutes=5), ) - result = usecase.execute(CreateSandboxCommand(chat_id=NON_CANONICAL_CHAT_ID)) + result = usecase.execute(CreateSandboxCommand(chat_id=UUID(NON_CANONICAL_CHAT_ID))) assert result.chat_id == CHAT_ID assert result.container_id == f'container-{result.session_id}' @@ -332,15 +342,14 @@ def test_create_sandbox_creates_new_session_when_none_exists() -> None: } assert runtime.stop_calls == [] assert repository.get_active_by_chat_id(CHAT_ID) == result - assert repository.get_active_by_chat_id(NON_CANONICAL_CHAT_ID) is None assert locker.chat_ids == [CHAT_ID] assert logger.messages == [ ( 'info', 'sandbox_created', { - 'chat_id': CHAT_ID, - 'session_id': result.session_id, + 'chat_id': str(CHAT_ID), + 'session_id': str(result.session_id), 'container_id': result.container_id, }, ) @@ -363,7 +372,7 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id( logger=logger, ttl=timedelta(minutes=5), ) - monkeypatch.setattr('usecase.sandbox._new_session_id', lambda: 'session-new') + monkeypatch.setattr('usecase.sandbox._new_session_id', lambda: SESSION_NEW_ID) results: list[SandboxSession | None] = [None, None] errors: list[Exception] = [] @@ -392,9 +401,9 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id( assert errors == [] assert results[0] == results[1] assert results[0] == SandboxSession( - session_id='session-new', + session_id=SESSION_NEW_ID, chat_id=CHAT_ID, - container_id='container-session-new', + container_id=f'container-{SESSION_NEW_ID}', status=SandboxStatus.RUNNING, created_at=now, expires_at=now + timedelta(minutes=5), @@ -408,18 +417,18 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id( 'info', 'sandbox_created', { - 'chat_id': CHAT_ID, - 'session_id': 'session-new', - 'container_id': 'container-session-new', + 'chat_id': str(CHAT_ID), + 'session_id': str(SESSION_NEW_ID), + 'container_id': f'container-{SESSION_NEW_ID}', }, ), ( 'info', 'sandbox_reused', { - 'chat_id': CHAT_ID, - 'session_id': 'session-new', - 'container_id': 'container-session-new', + 'chat_id': str(CHAT_ID), + 'session_id': str(SESSION_NEW_ID), + 'container_id': f'container-{SESSION_NEW_ID}', }, ), ] @@ -428,7 +437,7 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id( def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() -> None: now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) expired_session = SandboxSession( - session_id='session-expired', + session_id=SESSION_EXPIRED_ID, chat_id=EXPIRED_CHAT_ID, container_id='container-expired', status=SandboxStatus.RUNNING, @@ -436,7 +445,7 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() -> expires_at=now - timedelta(seconds=1), ) boundary_session = SandboxSession( - session_id='session-boundary', + session_id=SESSION_BOUNDARY_ID, chat_id=BOUNDARY_CHAT_ID, container_id='container-boundary', status=SandboxStatus.RUNNING, @@ -444,7 +453,7 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() -> expires_at=now, ) active_session = SandboxSession( - session_id='session-active', + session_id=SESSION_ACTIVE_ID, chat_id=ACTIVE_CHAT_ID, container_id='container-active', status=SandboxStatus.RUNNING, @@ -479,8 +488,8 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() -> 'info', 'sandbox_cleaned', { - 'chat_id': EXPIRED_CHAT_ID, - 'session_id': 'session-expired', + 'chat_id': str(EXPIRED_CHAT_ID), + 'session_id': str(SESSION_EXPIRED_ID), 'container_id': 'container-expired', }, ), @@ -488,8 +497,8 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() -> 'info', 'sandbox_cleaned', { - 'chat_id': BOUNDARY_CHAT_ID, - 'session_id': 'session-boundary', + 'chat_id': str(BOUNDARY_CHAT_ID), + 'session_id': str(SESSION_BOUNDARY_ID), 'container_id': 'container-boundary', }, ), @@ -499,7 +508,7 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() -> def test_cleanup_expired_sandboxes_skips_replaced_session_from_stale_snapshot() -> None: now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) expired_snapshot = SandboxSession( - session_id='session-expired', + session_id=SESSION_EXPIRED_ID, chat_id=CHAT_ID, container_id='container-expired', status=SandboxStatus.RUNNING, @@ -507,7 +516,7 @@ def test_cleanup_expired_sandboxes_skips_replaced_session_from_stale_snapshot() expires_at=now - timedelta(seconds=1), ) replacement_session = SandboxSession( - session_id='session-new', + session_id=SESSION_REPLACEMENT_ID, chat_id=CHAT_ID, container_id='container-new', status=SandboxStatus.RUNNING, @@ -539,7 +548,7 @@ def test_cleanup_expired_sandboxes_skips_replaced_session_from_stale_snapshot() def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None: now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) failing_session = SandboxSession( - session_id='session-fail', + session_id=SESSION_FAIL_ID, chat_id=FAIL_CHAT_ID, container_id='container-fail', status=SandboxStatus.RUNNING, @@ -547,7 +556,7 @@ def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None: expires_at=now - timedelta(minutes=1), ) cleaned_session = SandboxSession( - session_id='session-clean', + session_id=SESSION_CLEAN_ID, chat_id=CLEAN_CHAT_ID, container_id='container-clean', status=SandboxStatus.RUNNING, @@ -580,8 +589,8 @@ def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None: 'error', 'sandbox_clean_failed', { - 'chat_id': FAIL_CHAT_ID, - 'session_id': 'session-fail', + 'chat_id': str(FAIL_CHAT_ID), + 'session_id': str(SESSION_FAIL_ID), 'container_id': 'container-fail', 'error': 'RuntimeError', }, @@ -590,8 +599,8 @@ def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None: 'info', 'sandbox_cleaned', { - 'chat_id': CLEAN_CHAT_ID, - 'session_id': 'session-clean', + 'chat_id': str(CLEAN_CHAT_ID), + 'session_id': str(SESSION_CLEAN_ID), 'container_id': 'container-clean', }, ), diff --git a/usecase/interface.py b/usecase/interface.py index 0c0e321..15c581a 100644 --- a/usecase/interface.py +++ b/usecase/interface.py @@ -2,6 +2,7 @@ from collections.abc import Mapping from datetime import datetime from types import TracebackType from typing import Protocol, TypeAlias +from uuid import UUID from domain.sandbox import SandboxSession from domain.user import User @@ -19,13 +20,13 @@ class UserRepository(Protocol): class SandboxSessionRepository(Protocol): - def get_active_by_chat_id(self, chat_id: str) -> SandboxSession | None: ... + def get_active_by_chat_id(self, chat_id: UUID) -> SandboxSession | None: ... def list_expired(self, now: datetime) -> list[SandboxSession]: ... def save(self, session: SandboxSession) -> None: ... - def delete(self, session_id: str) -> None: ... + def delete(self, session_id: UUID) -> None: ... class LockContext(Protocol): @@ -40,15 +41,15 @@ class LockContext(Protocol): class SandboxLifecycleLocker(Protocol): - def lock(self, chat_id: str) -> LockContext: ... + def lock(self, chat_id: UUID) -> LockContext: ... class SandboxRuntime(Protocol): def create( self, *, - session_id: str, - chat_id: str, + session_id: UUID, + chat_id: UUID, created_at: datetime, expires_at: datetime, ) -> SandboxSession: ... diff --git a/usecase/sandbox.py b/usecase/sandbox.py index d04dd82..83ee39d 100644 --- a/usecase/sandbox.py +++ b/usecase/sandbox.py @@ -14,7 +14,7 @@ from usecase.interface import ( @dataclass(frozen=True, slots=True) class CreateSandboxCommand: - chat_id: str + chat_id: UUID class CreateSandbox: @@ -35,7 +35,7 @@ class CreateSandbox: self._ttl = ttl def execute(self, command: CreateSandboxCommand) -> SandboxSession: - chat_id = _canonical_chat_id(command.chat_id) + chat_id = command.chat_id with self._locker.lock(chat_id): session = self._repository.get_active_by_chat_id(chat_id) @@ -44,22 +44,14 @@ class CreateSandbox: if session is not None and session.expires_at > now: self._logger.info( 'sandbox_reused', - attrs={ - 'chat_id': chat_id, - 'session_id': session.session_id, - 'container_id': session.container_id, - }, + attrs=_sandbox_attrs(session), ) return session if session is not None: self._logger.info( 'sandbox_replaced', - attrs={ - 'chat_id': chat_id, - 'session_id': session.session_id, - 'container_id': session.container_id, - }, + attrs=_sandbox_attrs(session), ) self._runtime.stop(session.container_id) self._repository.delete(session.session_id) @@ -75,11 +67,7 @@ class CreateSandbox: self._repository.save(new_session) self._logger.info( 'sandbox_created', - attrs={ - 'chat_id': chat_id, - 'session_id': new_session.session_id, - 'container_id': new_session.container_id, - }, + attrs=_sandbox_attrs(new_session), ) return new_session @@ -107,14 +95,11 @@ class CleanupExpiredSandboxes: try: cleaned_session = self._cleanup_session(session) except Exception as exc: + attrs = _sandbox_attrs(session) + attrs['error'] = type(exc).__name__ self._logger.error( 'sandbox_clean_failed', - attrs={ - 'chat_id': session.chat_id, - 'session_id': session.session_id, - 'container_id': session.container_id, - 'error': type(exc).__name__, - }, + attrs=attrs, ) continue @@ -124,11 +109,7 @@ class CleanupExpiredSandboxes: cleaned_sessions.append(cleaned_session) self._logger.info( 'sandbox_cleaned', - attrs={ - 'chat_id': cleaned_session.chat_id, - 'session_id': cleaned_session.session_id, - 'container_id': cleaned_session.container_id, - }, + attrs=_sandbox_attrs(cleaned_session), ) return cleaned_sessions @@ -151,9 +132,13 @@ class CleanupExpiredSandboxes: return current_session -def _new_session_id() -> str: - return uuid4().hex +def _new_session_id() -> UUID: + return uuid4() -def _canonical_chat_id(chat_id: str) -> str: - return str(UUID(str(chat_id).strip())) +def _sandbox_attrs(session: SandboxSession) -> dict[str, str]: + return { + 'chat_id': str(session.chat_id), + 'session_id': str(session.session_id), + 'container_id': session.container_id, + }