소스 검색

Phase 4: person storage + management

- storage.py: SQLite persons table with async CRUD
  - add_person, get_person (by id or nickname), list_persons
  - update_person (dynamic field updates), delete_person
  - asyncio.to_thread() for non-blocking DB access
  - Thread-safe init with async lock
- tools.py: person_manage now uses real storage layer
  - add with validation, get by id/nickname, list, update, delete
  - Proper error responses for missing fields/records
- tests/test_storage.py: 14 tests (CRUD round-trips, edge cases)
- tests/test_tools.py: person_manage tests updated (add/get/list/update/delete)

Full suite: 95 tests passing.
Lukas Goldschmidt 1 개월 전
부모
커밋
72de37b75f
4개의 변경된 파일616개의 추가작업 그리고 11개의 파일을 삭제
  1. 249 0
      src/astro_mcp/storage.py
  2. 55 4
      src/astro_mcp/tools.py
  3. 197 0
      tests/test_storage.py
  4. 115 7
      tests/test_tools.py

+ 249 - 0
src/astro_mcp/storage.py

@@ -0,0 +1,249 @@
+"""
+SQLite storage layer for astro-mcp.
+
+Persons table for storing birth data.
+All DB access uses asyncio.to_thread() for non-blocking operation.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import logging
+import sqlite3
+import uuid
+from datetime import datetime, timezone
+from pathlib import Path
+from typing import Any
+
+from .config import DB_PATH
+
+logger = logging.getLogger("astro-mcp.storage")
+
+_init_lock = asyncio.Lock()
+_initialized = False
+
+
+def _get_conn() -> sqlite3.Connection:
+    """Get a thread-local SQLite connection."""
+    conn = sqlite3.connect(str(DB_PATH))
+    conn.execute("PRAGMA journal_mode=WAL")
+    conn.execute("PRAGMA busy_timeout=5000")
+    conn.row_factory = sqlite3.Row
+    return conn
+
+
+def _init_db_sync() -> None:
+    """Synchronous database initialization."""
+    DB_PATH.parent.mkdir(parents=True, exist_ok=True)
+    conn = _get_conn()
+    conn.executescript("""
+        CREATE TABLE IF NOT EXISTS persons (
+            id          TEXT PRIMARY KEY,
+            name        TEXT NOT NULL,
+            nickname    TEXT UNIQUE,
+            birth_datetime  TEXT NOT NULL,
+            latitude    REAL NOT NULL,
+            longitude   REAL NOT NULL,
+            elevation   REAL DEFAULT 0.0,
+            created_at  TEXT NOT NULL,
+            updated_at  TEXT
+        );
+        CREATE INDEX IF NOT EXISTS idx_persons_nickname ON persons(nickname);
+        CREATE INDEX IF NOT EXISTS idx_persons_name ON persons(name);
+    """)
+    conn.commit()
+    conn.close()
+
+
+async def init_db() -> None:
+    """Initialize the database (idempotent, async-safe)."""
+    global _initialized
+    if _initialized:
+        return
+    async with _init_lock:
+        if _initialized:
+            return
+        await asyncio.to_thread(_init_db_sync)
+        _initialized = True
+        logger.info(f"database initialized at {DB_PATH}")
+
+
+def _row_to_dict(row: sqlite3.Row) -> dict[str, Any]:
+    """Convert a sqlite3.Row to a plain dict."""
+    return dict(row)
+
+
+# ── CRUD Operations ─────────────────────────────────────────────────
+
+async def add_person(
+    name: str,
+    birth_datetime: str,
+    latitude: float,
+    longitude: float,
+    elevation: float = 0.0,
+    nickname: str | None = None,
+) -> dict[str, Any]:
+    """Add a new person to the database.
+
+    Returns:
+        The created person dict.
+    """
+    await init_db()
+    person_id = str(uuid.uuid4())[:8]
+    now = datetime.now(timezone.utc).isoformat()
+
+    def _insert():
+        conn = _get_conn()
+        conn.execute(
+            """INSERT INTO persons (id, name, nickname, birth_datetime, latitude, longitude, elevation, created_at)
+               VALUES (?, ?, ?, ?, ?, ?, ?, ?)""",
+            (person_id, name, nickname, birth_datetime, latitude, longitude, elevation, now),
+        )
+        conn.commit()
+        row = conn.execute("SELECT * FROM persons WHERE id = ?", (person_id,)).fetchone()
+        conn.close()
+        return _row_to_dict(row)
+
+    result = await asyncio.to_thread(_insert)
+    logger.info(f"added person {result['id']} ({name})")
+    return result
+
+
+async def get_person(
+    person_id: str | None = None,
+    nickname: str | None = None,
+) -> dict[str, Any] | None:
+    """Get a person by ID or nickname.
+
+    Returns:
+        Person dict or None if not found.
+    """
+    await init_db()
+
+    def _fetch():
+        conn = _get_conn()
+        if person_id:
+            row = conn.execute("SELECT * FROM persons WHERE id = ?", (person_id,)).fetchone()
+        elif nickname:
+            row = conn.execute("SELECT * FROM persons WHERE nickname = ?", (nickname,)).fetchone()
+        else:
+            row = None
+        conn.close()
+        return _row_to_dict(row) if row else None
+
+    return await asyncio.to_thread(_fetch)
+
+
+async def list_persons(
+    limit: int = 100,
+    offset: int = 0,
+) -> list[dict[str, Any]]:
+    """List all persons, ordered by creation date (newest first).
+
+    Returns:
+        List of person dicts.
+    """
+    await init_db()
+
+    def _fetch():
+        conn = _get_conn()
+        rows = conn.execute(
+            "SELECT * FROM persons ORDER BY created_at DESC LIMIT ? OFFSET ?",
+            (limit, offset),
+        ).fetchall()
+        conn.close()
+        return [_row_to_dict(r) for r in rows]
+
+    return await asyncio.to_thread(_fetch)
+
+
+async def update_person(
+    person_id: str,
+    name: str | None = None,
+    nickname: str | None = None,
+    birth_datetime: str | None = None,
+    latitude: float | None = None,
+    longitude: float | None = None,
+    elevation: float | None = None,
+) -> dict[str, Any] | None:
+    """Update a person's data. Only provided fields are updated.
+
+    Returns:
+        Updated person dict, or None if not found.
+    """
+    await init_db()
+    now = datetime.now(timezone.utc).isoformat()
+
+    def _update():
+        conn = _get_conn()
+        # Check existence
+        existing = conn.execute("SELECT * FROM persons WHERE id = ?", (person_id,)).fetchone()
+        if not existing:
+            conn.close()
+            return None
+
+        # Build dynamic update
+        fields = []
+        values = []
+        if name is not None:
+            fields.append("name = ?")
+            values.append(name)
+        if nickname is not None:
+            fields.append("nickname = ?")
+            values.append(nickname)
+        if birth_datetime is not None:
+            fields.append("birth_datetime = ?")
+            values.append(birth_datetime)
+        if latitude is not None:
+            fields.append("latitude = ?")
+            values.append(latitude)
+        if longitude is not None:
+            fields.append("longitude = ?")
+            values.append(longitude)
+        if elevation is not None:
+            fields.append("elevation = ?")
+            values.append(elevation)
+
+        if not fields:
+            conn.close()
+            return _row_to_dict(existing)
+
+        fields.append("updated_at = ?")
+        values.append(now)
+        values.append(person_id)
+
+        conn.execute(
+            f"UPDATE persons SET {', '.join(fields)} WHERE id = ?",
+            values,
+        )
+        conn.commit()
+        row = conn.execute("SELECT * FROM persons WHERE id = ?", (person_id,)).fetchone()
+        conn.close()
+        return _row_to_dict(row)
+
+    result = await asyncio.to_thread(_update)
+    if result:
+        logger.info(f"updated person {person_id}")
+    return result
+
+
+async def delete_person(person_id: str) -> bool:
+    """Delete a person by ID.
+
+    Returns:
+        True if deleted, False if not found.
+    """
+    await init_db()
+
+    def _delete():
+        conn = _get_conn()
+        cursor = conn.execute("DELETE FROM persons WHERE id = ?", (person_id,))
+        conn.commit()
+        deleted = cursor.rowcount > 0
+        conn.close()
+        return deleted
+
+    result = await asyncio.to_thread(_delete)
+    if result:
+        logger.info(f"deleted person {person_id}")
+    return result

+ 55 - 4
src/astro_mcp/tools.py

@@ -529,10 +529,61 @@ async def person_manage(
     Returns:
         Operation result with person data or error.
     """
-    return {
-        "action": action,
-        "_note": "Person management not yet implemented -- requires storage layer (Phase 4)",
-    }
+    from . import storage
+
+    action = action.lower().strip()
+
+    if action == "add":
+        if not name or not birth_datetime or latitude is None or longitude is None:
+            return {"error": "add requires: name, birth_datetime, latitude, longitude"}
+        person = await storage.add_person(
+            name=name,
+            birth_datetime=birth_datetime,
+            latitude=latitude,
+            longitude=longitude,
+            elevation=elevation if elevation is not None else 0.0,
+            nickname=nickname,
+        )
+        return {"action": "add", "person": person}
+
+    elif action == "get":
+        if not person_id and not nickname:
+            return {"error": "get requires: person_id or nickname"}
+        person = await storage.get_person(person_id=person_id, nickname=nickname)
+        if not person:
+            return {"action": "get", "error": "not_found"}
+        return {"action": "get", "person": person}
+
+    elif action == "list":
+        persons = await storage.list_persons()
+        return {"action": "list", "persons": persons, "count": len(persons)}
+
+    elif action == "update":
+        if not person_id:
+            return {"error": "update requires: person_id"}
+        person = await storage.update_person(
+            person_id=person_id,
+            name=name,
+            nickname=nickname,
+            birth_datetime=birth_datetime,
+            latitude=latitude,
+            longitude=longitude,
+            elevation=elevation,
+        )
+        if not person:
+            return {"action": "update", "error": "not_found"}
+        return {"action": "update", "person": person}
+
+    elif action == "delete":
+        if not person_id:
+            return {"error": "delete requires: person_id"}
+        deleted = await storage.delete_person(person_id)
+        if not deleted:
+            return {"action": "delete", "error": "not_found"}
+        return {"action": "delete", "deleted": True, "person_id": person_id}
+
+    else:
+        return {"error": f"unknown action: {action}. Use: add, get, list, update, delete"}
 
 
 # ── Tool: list_house_systems ─────────────────────────────────────────

+ 197 - 0
tests/test_storage.py

@@ -0,0 +1,197 @@
+"""
+Tests for the person storage layer.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import os
+import tempfile
+from pathlib import Path
+
+import pytest
+
+from src.astro_mcp import storage
+
+
+@pytest.fixture
+def tmp_db(tmp_path, monkeypatch):
+    """Use a temporary database for each test."""
+    db_path = tmp_path / "test_astro.sqlite3"
+    monkeypatch.setattr(storage, "DB_PATH", db_path)
+    storage._initialized = False  # Reset init state
+    yield db_path
+    storage._initialized = False  # Cleanup
+
+
+class TestAddPerson:
+    def test_add_person(self, tmp_db):
+        result = asyncio.run(storage.add_person(
+            name="Test Person",
+            birth_datetime="2000-01-01T12:00:00Z",
+            latitude=47.0,
+            longitude=8.0,
+        ))
+        assert result["name"] == "Test Person"
+        assert result["birth_datetime"] == "2000-01-01T12:00:00Z"
+        assert result["latitude"] == 47.0
+        assert result["longitude"] == 8.0
+        assert result["elevation"] == 0.0
+        assert "id" in result
+        assert "created_at" in result
+
+    def test_add_person_with_nickname(self, tmp_db):
+        result = asyncio.run(storage.add_person(
+            name="Test Person",
+            birth_datetime="2000-01-01T12:00:00Z",
+            latitude=47.0,
+            longitude=8.0,
+            nickname="testy",
+        ))
+        assert result["nickname"] == "testy"
+
+    def test_add_person_with_elevation(self, tmp_db):
+        result = asyncio.run(storage.add_person(
+            name="Test Person",
+            birth_datetime="2000-01-01T12:00:00Z",
+            latitude=47.0,
+            longitude=8.0,
+            elevation=500.0,
+        ))
+        assert result["elevation"] == 500.0
+
+
+class TestGetPerson:
+    def test_get_by_id(self, tmp_db):
+        added = asyncio.run(storage.add_person(
+            name="Test Person",
+            birth_datetime="2000-01-01T12:00:00Z",
+            latitude=47.0,
+            longitude=8.0,
+        ))
+        fetched = asyncio.run(storage.get_person(person_id=added["id"]))
+        assert fetched is not None
+        assert fetched["id"] == added["id"]
+        assert fetched["name"] == "Test Person"
+
+    def test_get_by_nickname(self, tmp_db):
+        asyncio.run(storage.add_person(
+            name="Test Person",
+            birth_datetime="2000-01-01T12:00:00Z",
+            latitude=47.0,
+            longitude=8.0,
+            nickname="testy",
+        ))
+        fetched = asyncio.run(storage.get_person(nickname="testy"))
+        assert fetched is not None
+        assert fetched["nickname"] == "testy"
+
+    def test_get_not_found(self, tmp_db):
+        result = asyncio.run(storage.get_person(person_id="nonexistent"))
+        assert result is None
+
+    def test_get_no_params(self, tmp_db):
+        result = asyncio.run(storage.get_person())
+        assert result is None
+
+
+class TestListPersons:
+    def test_empty_list(self, tmp_db):
+        result = asyncio.run(storage.list_persons())
+        assert result == []
+
+    def test_list_persons(self, tmp_db):
+        for i in range(3):
+            asyncio.run(storage.add_person(
+                name=f"Person {i}",
+                birth_datetime="2000-01-01T12:00:00Z",
+                latitude=47.0,
+                longitude=8.0,
+            ))
+        result = asyncio.run(storage.list_persons())
+        assert len(result) == 3
+
+    def test_list_ordered_newest_first(self, tmp_db):
+        for i in range(3):
+            asyncio.run(storage.add_person(
+                name=f"Person {i}",
+                birth_datetime="2000-01-01T12:00:00Z",
+                latitude=47.0,
+                longitude=8.0,
+            ))
+        result = asyncio.run(storage.list_persons())
+        # Newest first
+        assert result[0]["name"] == "Person 2"
+        assert result[-1]["name"] == "Person 0"
+
+
+class TestUpdatePerson:
+    def test_update_name(self, tmp_db):
+        added = asyncio.run(storage.add_person(
+            name="Original",
+            birth_datetime="2000-01-01T12:00:00Z",
+            latitude=47.0,
+            longitude=8.0,
+        ))
+        updated = asyncio.run(storage.update_person(
+            person_id=added["id"],
+            name="Updated",
+        ))
+        assert updated is not None
+        assert updated["name"] == "Updated"
+        assert updated["updated_at"] is not None
+
+    def test_update_birth_data(self, tmp_db):
+        added = asyncio.run(storage.add_person(
+            name="Test",
+            birth_datetime="2000-01-01T12:00:00Z",
+            latitude=47.0,
+            longitude=8.0,
+        ))
+        updated = asyncio.run(storage.update_person(
+            person_id=added["id"],
+            birth_datetime="1995-06-15T08:30:00Z",
+            latitude=52.0,
+            longitude=13.0,
+        ))
+        assert updated["birth_datetime"] == "1995-06-15T08:30:00Z"
+        assert updated["latitude"] == 52.0
+        assert updated["longitude"] == 13.0
+
+    def test_update_not_found(self, tmp_db):
+        result = asyncio.run(storage.update_person(
+            person_id="nonexistent",
+            name="Updated",
+        ))
+        assert result is None
+
+    def test_update_no_fields_returns_unchanged(self, tmp_db):
+        added = asyncio.run(storage.add_person(
+            name="Test",
+            birth_datetime="2000-01-01T12:00:00Z",
+            latitude=47.0,
+            longitude=8.0,
+        ))
+        updated = asyncio.run(storage.update_person(person_id=added["id"]))
+        assert updated is not None
+        assert updated["name"] == "Test"  # Unchanged
+
+
+class TestDeletePerson:
+    def test_delete_person(self, tmp_db):
+        added = asyncio.run(storage.add_person(
+            name="Test",
+            birth_datetime="2000-01-01T12:00:00Z",
+            latitude=47.0,
+            longitude=8.0,
+        ))
+        deleted = asyncio.run(storage.delete_person(added["id"]))
+        assert deleted is True
+
+        # Verify it's gone
+        fetched = asyncio.run(storage.get_person(person_id=added["id"]))
+        assert fetched is None
+
+    def test_delete_not_found(self, tmp_db):
+        result = asyncio.run(storage.delete_person("nonexistent"))
+        assert result is False

+ 115 - 7
tests/test_tools.py

@@ -304,14 +304,122 @@ class TestListHouseSystems:
 
 
 # ── Tests: person_manage (stub) ─────────────────────────────────────
-
 class TestPersonManage:
-    def test_stub_returns_note(self):
-        from src.astro_mcp import tools
-        result = asyncio.run(
-            tools.person_manage(action="list")
-        )
-        assert "_note" in result
+    def test_add_and_get(self, tmp_path, monkeypatch):
+        """Test add + get round-trip with real storage."""
+        from src.astro_mcp import tools, storage
+        db_path = tmp_path / "test.sqlite3"
+        monkeypatch.setattr(storage, "DB_PATH", db_path)
+        storage._initialized = False
+
+        import asyncio
+
+        # Add
+        add_result = asyncio.run(tools.person_manage(
+            action="add",
+            name="Lucky",
+            birth_datetime="1990-05-15T10:30:00Z",
+            latitude=47.3,
+            longitude=8.5,
+            nickname="lucky",
+        ))
+        assert "person" in add_result
+        assert add_result["person"]["name"] == "Lucky"
+        person_id = add_result["person"]["id"]
+
+        # Get by ID
+        get_result = asyncio.run(tools.person_manage(
+            action="get", person_id=person_id,
+        ))
+        assert get_result["person"]["name"] == "Lucky"
+
+        # Get by nickname
+        get_result2 = asyncio.run(tools.person_manage(
+            action="get", nickname="lucky",
+        ))
+        assert get_result2["person"]["id"] == person_id
+
+    def test_list_persons(self, tmp_path, monkeypatch):
+        from src.astro_mcp import tools, storage
+        db_path = tmp_path / "test.sqlite3"
+        monkeypatch.setattr(storage, "DB_PATH", db_path)
+        storage._initialized = False
+
+        import asyncio
+        for i in range(3):
+            asyncio.run(tools.person_manage(
+                action="add", name=f"P{i}",
+                birth_datetime="2000-01-01T12:00:00Z",
+                latitude=0.0, longitude=0.0,
+            ))
+
+        result = asyncio.run(tools.person_manage(action="list"))
+        assert result["count"] == 3
+
+    def test_update_person(self, tmp_path, monkeypatch):
+        from src.astro_mcp import tools, storage
+        db_path = tmp_path / "test.sqlite3"
+        monkeypatch.setattr(storage, "DB_PATH", db_path)
+        storage._initialized = False
+
+        import asyncio
+        add_result = asyncio.run(tools.person_manage(
+            action="add", name="Original",
+            birth_datetime="2000-01-01T12:00:00Z",
+            latitude=0.0, longitude=0.0,
+        ))
+        pid = add_result["person"]["id"]
+
+        update_result = asyncio.run(tools.person_manage(
+            action="update", person_id=pid, name="Updated",
+        ))
+        assert update_result["person"]["name"] == "Updated"
+
+    def test_delete_person(self, tmp_path, monkeypatch):
+        from src.astro_mcp import tools, storage
+        db_path = tmp_path / "test.sqlite3"
+        monkeypatch.setattr(storage, "DB_PATH", db_path)
+        storage._initialized = False
+
+        import asyncio
+        add_result = asyncio.run(tools.person_manage(
+            action="add", name="ToDelete",
+            birth_datetime="2000-01-01T12:00:00Z",
+            latitude=0.0, longitude=0.0,
+        ))
+        pid = add_result["person"]["id"]
+
+        del_result = asyncio.run(tools.person_manage(
+            action="delete", person_id=pid,
+        ))
+        assert del_result["deleted"] is True
+
+        get_result = asyncio.run(tools.person_manage(
+            action="get", person_id=pid,
+        ))
+        assert "error" in get_result
+
+    def test_missing_required_fields(self, tmp_path, monkeypatch):
+        from src.astro_mcp import tools, storage
+        db_path = tmp_path / "test.sqlite3"
+        monkeypatch.setattr(storage, "DB_PATH", db_path)
+        storage._initialized = False
+
+        import asyncio
+        result = asyncio.run(tools.person_manage(
+            action="add", name="Test",
+        ))
+        assert "error" in result
+
+    def test_unknown_action(self, tmp_path, monkeypatch):
+        from src.astro_mcp import tools, storage
+        db_path = tmp_path / "test.sqlite3"
+        monkeypatch.setattr(storage, "DB_PATH", db_path)
+        storage._initialized = False
+
+        import asyncio
+        result = asyncio.run(tools.person_manage(action="bogus"))
+        assert "error" in result
 
 
 # ── Tests: get_transit_preview (stub) ───────────────────────────────