Fix boundary/child count invariant on shelf and book deletion
When deleting a shelf or book, remove the corresponding boundary from the parent's boundary list so len(boundaries) == len(children) - 1 is maintained. Add API-level tests covering first, middle, and last child deletion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
20
src/api.py
20
src/api.py
@@ -205,11 +205,19 @@ async def update_shelf(shelf_id: str, request: Request) -> dict[str, Any]:
|
|||||||
@router.delete("/api/shelves/{shelf_id}")
|
@router.delete("/api/shelves/{shelf_id}")
|
||||||
async def delete_shelf(shelf_id: str) -> dict[str, Any]:
|
async def delete_shelf(shelf_id: str) -> dict[str, Any]:
|
||||||
with db.connection() as c:
|
with db.connection() as c:
|
||||||
if not db.get_shelf(c, shelf_id):
|
shelf = db.get_shelf(c, shelf_id)
|
||||||
|
if not shelf:
|
||||||
raise HTTPException(404, "Shelf not found")
|
raise HTTPException(404, "Shelf not found")
|
||||||
photos = db.collect_shelf_photos(c, shelf_id)
|
photos = db.collect_shelf_photos(c, shelf_id)
|
||||||
|
rank = db.get_shelf_rank(c, shelf_id)
|
||||||
|
cab = db.get_cabinet(c, shelf.cabinet_id)
|
||||||
with db.transaction() as c:
|
with db.transaction() as c:
|
||||||
db.delete_shelf(c, shelf_id)
|
db.delete_shelf(c, shelf_id)
|
||||||
|
if cab:
|
||||||
|
bounds: list[float] = json.loads(cab.shelf_boundaries) if cab.shelf_boundaries else []
|
||||||
|
if bounds:
|
||||||
|
del bounds[min(rank, len(bounds) - 1)]
|
||||||
|
db.set_cabinet_boundaries(c, cab.id, json.dumps(bounds))
|
||||||
for fn in photos:
|
for fn in photos:
|
||||||
del_photo(fn)
|
del_photo(fn)
|
||||||
return {"ok": True}
|
return {"ok": True}
|
||||||
@@ -293,11 +301,19 @@ async def update_book(book_id: str, request: Request) -> dict[str, Any]:
|
|||||||
@router.delete("/api/books/{book_id}")
|
@router.delete("/api/books/{book_id}")
|
||||||
async def delete_book(book_id: str) -> dict[str, Any]:
|
async def delete_book(book_id: str) -> dict[str, Any]:
|
||||||
with db.connection() as c:
|
with db.connection() as c:
|
||||||
if not db.get_book(c, book_id):
|
book = db.get_book(c, book_id)
|
||||||
|
if not book:
|
||||||
raise HTTPException(404, "Book not found")
|
raise HTTPException(404, "Book not found")
|
||||||
fn = db.get_book_photo(c, book_id)
|
fn = db.get_book_photo(c, book_id)
|
||||||
|
rank = db.get_book_rank(c, book_id)
|
||||||
|
shelf = db.get_shelf(c, book.shelf_id)
|
||||||
with db.transaction() as c:
|
with db.transaction() as c:
|
||||||
db.delete_book(c, book_id)
|
db.delete_book(c, book_id)
|
||||||
|
if shelf:
|
||||||
|
bounds: list[float] = json.loads(shelf.book_boundaries) if shelf.book_boundaries else []
|
||||||
|
if bounds:
|
||||||
|
del bounds[min(rank, len(bounds) - 1)]
|
||||||
|
db.set_shelf_boundaries(c, shelf.id, json.dumps(bounds))
|
||||||
del_photo(fn)
|
del_photo(fn)
|
||||||
return {"ok": True}
|
return {"ok": True}
|
||||||
|
|
||||||
|
|||||||
157
tests/test_api.py
Normal file
157
tests/test_api.py
Normal file
@@ -0,0 +1,157 @@
|
|||||||
|
"""API-level integration tests.
|
||||||
|
|
||||||
|
Uses a minimal FastAPI app (router only, no lifespan) so tests run against
|
||||||
|
a temporary SQLite database without needing config or plugins.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import json
|
||||||
|
from collections.abc import Iterator
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from fastapi import FastAPI
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
import db
|
||||||
|
import files
|
||||||
|
from api import router
|
||||||
|
|
||||||
|
# ── Fixtures ──────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
_app = FastAPI()
|
||||||
|
_app.include_router(router)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def client(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Iterator[TestClient]:
|
||||||
|
"""TestClient backed by a fresh temporary database."""
|
||||||
|
monkeypatch.setattr(db, "DB_PATH", tmp_path / "test.db")
|
||||||
|
monkeypatch.setattr(files, "DATA_DIR", tmp_path)
|
||||||
|
monkeypatch.setattr(files, "IMAGES_DIR", tmp_path / "images")
|
||||||
|
files.init_dirs()
|
||||||
|
db.init_db()
|
||||||
|
db.COUNTERS.clear()
|
||||||
|
with TestClient(_app) as c:
|
||||||
|
yield c
|
||||||
|
db.COUNTERS.clear()
|
||||||
|
|
||||||
|
|
||||||
|
def _seed(tmp_path: Path) -> dict[str, str]:
|
||||||
|
"""Create room/cabinet/shelves/books with boundaries via db helpers; return their IDs."""
|
||||||
|
with db.transaction() as c:
|
||||||
|
room = db.create_room(c)
|
||||||
|
cab = db.create_cabinet(c, room.id)
|
||||||
|
s_a = db.create_shelf(c, cab.id)
|
||||||
|
s_b = db.create_shelf(c, cab.id)
|
||||||
|
s_c = db.create_shelf(c, cab.id)
|
||||||
|
b_a = db.create_book(c, s_a.id)
|
||||||
|
b_b = db.create_book(c, s_a.id)
|
||||||
|
b_c = db.create_book(c, s_a.id)
|
||||||
|
# 3 shelves → 2 interior boundaries
|
||||||
|
db.set_cabinet_boundaries(c, cab.id, json.dumps([0.33, 0.66]))
|
||||||
|
# 3 books in shelf_a → 2 interior boundaries
|
||||||
|
db.set_shelf_boundaries(c, s_a.id, json.dumps([0.33, 0.66]))
|
||||||
|
return {
|
||||||
|
"room": room.id,
|
||||||
|
"cabinet": cab.id,
|
||||||
|
"shelf_a": s_a.id,
|
||||||
|
"shelf_b": s_b.id,
|
||||||
|
"shelf_c": s_c.id,
|
||||||
|
"book_a": b_a.id,
|
||||||
|
"book_b": b_b.id,
|
||||||
|
"book_c": b_c.id,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
# ── Shelf deletion / boundary cleanup ─────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_first_shelf_removes_first_boundary(
|
||||||
|
client: TestClient, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||||
|
) -> None:
|
||||||
|
monkeypatch.setattr(db, "DB_PATH", tmp_path / "test.db")
|
||||||
|
ids = _seed(tmp_path)
|
||||||
|
resp = client.delete(f"/api/shelves/{ids['shelf_a']}")
|
||||||
|
assert resp.status_code == 200
|
||||||
|
with db.connection() as c:
|
||||||
|
cab = db.get_cabinet(c, ids["cabinet"])
|
||||||
|
assert cab is not None
|
||||||
|
bounds = json.loads(cab.shelf_boundaries or "[]")
|
||||||
|
assert bounds == [0.66] # first boundary (0.33) removed
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_middle_shelf_removes_middle_boundary(
|
||||||
|
client: TestClient, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||||
|
) -> None:
|
||||||
|
monkeypatch.setattr(db, "DB_PATH", tmp_path / "test.db")
|
||||||
|
ids = _seed(tmp_path)
|
||||||
|
resp = client.delete(f"/api/shelves/{ids['shelf_b']}")
|
||||||
|
assert resp.status_code == 200
|
||||||
|
with db.connection() as c:
|
||||||
|
cab = db.get_cabinet(c, ids["cabinet"])
|
||||||
|
assert cab is not None
|
||||||
|
bounds = json.loads(cab.shelf_boundaries or "[]")
|
||||||
|
assert bounds == [0.33] # middle boundary (0.66) removed
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_last_shelf_removes_last_boundary(
|
||||||
|
client: TestClient, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||||
|
) -> None:
|
||||||
|
monkeypatch.setattr(db, "DB_PATH", tmp_path / "test.db")
|
||||||
|
ids = _seed(tmp_path)
|
||||||
|
resp = client.delete(f"/api/shelves/{ids['shelf_c']}")
|
||||||
|
assert resp.status_code == 200
|
||||||
|
with db.connection() as c:
|
||||||
|
cab = db.get_cabinet(c, ids["cabinet"])
|
||||||
|
assert cab is not None
|
||||||
|
bounds = json.loads(cab.shelf_boundaries or "[]")
|
||||||
|
assert bounds == [0.33] # last boundary (0.66) removed
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_only_shelf_leaves_no_boundaries(
|
||||||
|
client: TestClient, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||||
|
) -> None:
|
||||||
|
"""Deleting the sole shelf (no boundaries) leaves boundaries unchanged (empty)."""
|
||||||
|
monkeypatch.setattr(db, "DB_PATH", tmp_path / "test.db")
|
||||||
|
ids = _seed(tmp_path)
|
||||||
|
# Delete two shelves first to leave only one
|
||||||
|
client.delete(f"/api/shelves/{ids['shelf_b']}")
|
||||||
|
client.delete(f"/api/shelves/{ids['shelf_c']}")
|
||||||
|
resp = client.delete(f"/api/shelves/{ids['shelf_a']}")
|
||||||
|
assert resp.status_code == 200
|
||||||
|
with db.connection() as c:
|
||||||
|
cab = db.get_cabinet(c, ids["cabinet"])
|
||||||
|
assert cab is not None
|
||||||
|
bounds = json.loads(cab.shelf_boundaries or "[]")
|
||||||
|
assert bounds == []
|
||||||
|
|
||||||
|
|
||||||
|
# ── Book deletion / boundary cleanup ──────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_first_book_removes_first_boundary(
|
||||||
|
client: TestClient, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||||
|
) -> None:
|
||||||
|
monkeypatch.setattr(db, "DB_PATH", tmp_path / "test.db")
|
||||||
|
ids = _seed(tmp_path)
|
||||||
|
resp = client.delete(f"/api/books/{ids['book_a']}")
|
||||||
|
assert resp.status_code == 200
|
||||||
|
with db.connection() as c:
|
||||||
|
shelf = db.get_shelf(c, ids["shelf_a"])
|
||||||
|
assert shelf is not None
|
||||||
|
bounds = json.loads(shelf.book_boundaries or "[]")
|
||||||
|
assert bounds == [0.66]
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_last_book_removes_last_boundary(
|
||||||
|
client: TestClient, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||||
|
) -> None:
|
||||||
|
monkeypatch.setattr(db, "DB_PATH", tmp_path / "test.db")
|
||||||
|
ids = _seed(tmp_path)
|
||||||
|
resp = client.delete(f"/api/books/{ids['book_c']}")
|
||||||
|
assert resp.status_code == 200
|
||||||
|
with db.connection() as c:
|
||||||
|
shelf = db.get_shelf(c, ids["shelf_a"])
|
||||||
|
assert shelf is not None
|
||||||
|
bounds = json.loads(shelf.book_boundaries or "[]")
|
||||||
|
assert bounds == [0.33]
|
||||||
Reference in New Issue
Block a user