mirror of
https://github.com/jamiepine/voicebox.git
synced 2026-10-04 01:25:18 -07:00
fix(profiles): delete a profile's generations instead of orphaning them
This commit is contained in:
committed by
capy-ai-staging[bot]
parent
4f54e16824
commit
4750b47b04
@@ -51,6 +51,10 @@ export function useDeleteProfile() {
|
|||||||
mutationFn: (profileId: string) => apiClient.deleteProfile(profileId),
|
mutationFn: (profileId: string) => apiClient.deleteProfile(profileId),
|
||||||
onSuccess: () => {
|
onSuccess: () => {
|
||||||
queryClient.invalidateQueries({ queryKey: ['profiles'] });
|
queryClient.invalidateQueries({ queryKey: ['profiles'] });
|
||||||
|
// Deleting a profile also deletes its generations, which story items
|
||||||
|
// reference — both caches still hold rows that no longer exist.
|
||||||
|
queryClient.invalidateQueries({ queryKey: ['history'] });
|
||||||
|
queryClient.invalidateQueries({ queryKey: ['stories'] });
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
+24
-10
@@ -11,10 +11,23 @@ from sqlalchemy.orm import Session
|
|||||||
from sqlalchemy import or_
|
from sqlalchemy import or_
|
||||||
|
|
||||||
from ..models import GenerationRequest, GenerationResponse, HistoryQuery, HistoryResponse, HistoryListResponse, GenerationVersionResponse, EffectConfig
|
from ..models import GenerationRequest, GenerationResponse, HistoryQuery, HistoryResponse, HistoryListResponse, GenerationVersionResponse, EffectConfig
|
||||||
from ..database import Generation as DBGeneration, GenerationVersion as DBGenerationVersion, VoiceProfile as DBVoiceProfile
|
from ..database import Generation as DBGeneration, GenerationVersion as DBGenerationVersion, StoryItem as DBStoryItem, VoiceProfile as DBVoiceProfile
|
||||||
from .. import config
|
from .. import config
|
||||||
|
|
||||||
|
|
||||||
|
def _delete_generation_children(generation_id: str, db: Session) -> None:
|
||||||
|
"""Remove the rows that reference a generation, plus any version audio files.
|
||||||
|
|
||||||
|
Story items and versions both point at the generation by a non-null FK.
|
||||||
|
The story detail query inner-joins generations, so a leftover story item
|
||||||
|
vanishes from the timeline while staying in the table forever.
|
||||||
|
"""
|
||||||
|
from . import versions as versions_mod
|
||||||
|
|
||||||
|
db.query(DBStoryItem).filter_by(generation_id=generation_id).delete()
|
||||||
|
versions_mod.delete_versions_for_generation(generation_id, db)
|
||||||
|
|
||||||
|
|
||||||
def _get_versions_for_generations(generation_ids: list[str], db: Session) -> dict:
|
def _get_versions_for_generations(generation_ids: list[str], db: Session) -> dict:
|
||||||
"""Fetch versions for many generations in a single query.
|
"""Fetch versions for many generations in a single query.
|
||||||
|
|
||||||
@@ -280,8 +293,7 @@ async def delete_generation(
|
|||||||
return False
|
return False
|
||||||
|
|
||||||
# Delete all version files and records
|
# Delete all version files and records
|
||||||
from . import versions as versions_mod
|
_delete_generation_children(generation_id, db)
|
||||||
versions_mod.delete_versions_for_generation(generation_id, db)
|
|
||||||
|
|
||||||
# Delete main audio file (if not already removed by version cleanup)
|
# Delete main audio file (if not already removed by version cleanup)
|
||||||
if generation.audio_path:
|
if generation.audio_path:
|
||||||
@@ -307,13 +319,11 @@ async def delete_failed_generations(db: Session) -> int:
|
|||||||
Returns:
|
Returns:
|
||||||
Number of generations deleted.
|
Number of generations deleted.
|
||||||
"""
|
"""
|
||||||
from . import versions as versions_mod
|
|
||||||
|
|
||||||
failed = db.query(DBGeneration).filter(DBGeneration.status == "failed").all()
|
failed = db.query(DBGeneration).filter(DBGeneration.status == "failed").all()
|
||||||
count = 0
|
count = 0
|
||||||
for generation in failed:
|
for generation in failed:
|
||||||
# Clean up version files/rows first.
|
# Clean up version files/rows first.
|
||||||
versions_mod.delete_versions_for_generation(generation.id, db)
|
_delete_generation_children(generation.id, db)
|
||||||
|
|
||||||
# Remove the main audio file if it somehow made it to disk.
|
# Remove the main audio file if it somehow made it to disk.
|
||||||
if generation.audio_path:
|
if generation.audio_path:
|
||||||
@@ -352,14 +362,18 @@ async def delete_generations_by_profile(
|
|||||||
count = 0
|
count = 0
|
||||||
for generation in generations:
|
for generation in generations:
|
||||||
# Delete associated version files and rows first
|
# Delete associated version files and rows first
|
||||||
from . import versions as versions_mod
|
_delete_generation_children(generation.id, db)
|
||||||
versions_mod.delete_versions_for_generation(generation.id, db)
|
|
||||||
|
|
||||||
# Delete audio file
|
# Delete audio file
|
||||||
audio_path = config.resolve_storage_path(generation.audio_path)
|
audio_path = config.resolve_storage_path(generation.audio_path)
|
||||||
if audio_path is not None and audio_path.exists():
|
if audio_path is not None and audio_path.exists():
|
||||||
audio_path.unlink()
|
try:
|
||||||
|
audio_path.unlink()
|
||||||
|
except OSError:
|
||||||
|
# A file locked by playback shouldn't abort the whole sweep
|
||||||
|
# and leave the profile half-deleted.
|
||||||
|
pass
|
||||||
|
|
||||||
# Delete from database
|
# Delete from database
|
||||||
db.delete(generation)
|
db.delete(generation)
|
||||||
count += 1
|
count += 1
|
||||||
|
|||||||
@@ -11,7 +11,14 @@ from sqlalchemy import func
|
|||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
from .. import config
|
from .. import config
|
||||||
from ..database import Generation as DBGeneration, ProfileSample as DBProfileSample, VoiceProfile as DBVoiceProfile
|
from ..database import (
|
||||||
|
CaptureSettings as DBCaptureSettings,
|
||||||
|
Generation as DBGeneration,
|
||||||
|
MCPClientBinding as DBMCPClientBinding,
|
||||||
|
ProfileChannelMapping as DBProfileChannelMapping,
|
||||||
|
ProfileSample as DBProfileSample,
|
||||||
|
VoiceProfile as DBVoiceProfile,
|
||||||
|
)
|
||||||
from ..models import (
|
from ..models import (
|
||||||
EffectConfig,
|
EffectConfig,
|
||||||
ProfileSampleResponse,
|
ProfileSampleResponse,
|
||||||
@@ -21,6 +28,7 @@ from ..models import (
|
|||||||
from ..utils.audio import save_audio, validate_and_load_reference_audio
|
from ..utils.audio import save_audio, validate_and_load_reference_audio
|
||||||
from ..utils.cache import _get_cache_dir, clear_profile_cache
|
from ..utils.cache import _get_cache_dir, clear_profile_cache
|
||||||
from ..utils.images import process_avatar, validate_image
|
from ..utils.images import process_avatar, validate_image
|
||||||
|
from . import history
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -429,7 +437,23 @@ async def delete_profile(
|
|||||||
if not profile:
|
if not profile:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
# Generations carry a non-null FK to the profile and the history query
|
||||||
|
# inner-joins profiles, so anything left behind here becomes a row the UI
|
||||||
|
# can never show and a .wav in data/generations the user can never reclaim.
|
||||||
|
deleted_generations = await history.delete_generations_by_profile(profile_id, db)
|
||||||
|
if deleted_generations:
|
||||||
|
logger.info("Deleted %d generations belonging to profile %s", deleted_generations, profile_id)
|
||||||
|
|
||||||
db.query(DBProfileSample).filter_by(profile_id=profile_id).delete()
|
db.query(DBProfileSample).filter_by(profile_id=profile_id).delete()
|
||||||
|
db.query(DBProfileChannelMapping).filter_by(profile_id=profile_id).delete()
|
||||||
|
|
||||||
|
# Nullable pointers at the profile — resolve_profile() already tolerates a
|
||||||
|
# dangling id, but leaving one behind makes the UI show an empty selection
|
||||||
|
# that the user can't clear.
|
||||||
|
db.query(DBMCPClientBinding).filter_by(profile_id=profile_id).update({"profile_id": None})
|
||||||
|
db.query(DBCaptureSettings).filter_by(default_playback_voice_id=profile_id).update(
|
||||||
|
{"default_playback_voice_id": None}
|
||||||
|
)
|
||||||
|
|
||||||
db.delete(profile)
|
db.delete(profile)
|
||||||
db.commit()
|
db.commit()
|
||||||
|
|||||||
@@ -0,0 +1,190 @@
|
|||||||
|
"""
|
||||||
|
Regression tests for DELETE /profiles/{id} cleaning up everything it owns.
|
||||||
|
|
||||||
|
``delete_profile`` used to remove only the profile row, its samples, and the
|
||||||
|
profile directory. Generations carry a non-null FK to the profile and
|
||||||
|
``list_generations`` inner-joins profiles, so every generation made with a
|
||||||
|
deleted profile turned into a row the UI could never show plus a ``.wav`` in
|
||||||
|
``data/generations`` the user could never reclaim — unbounded disk growth with
|
||||||
|
no way out. Version rows, story items, and channel mappings leaked the same way.
|
||||||
|
|
||||||
|
Usage:
|
||||||
|
python -m pytest backend/tests/test_profile_delete_cascade.py -v
|
||||||
|
"""
|
||||||
|
|
||||||
|
import sys
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from sqlalchemy import create_engine
|
||||||
|
from sqlalchemy.orm import sessionmaker
|
||||||
|
|
||||||
|
# Repo root on sys.path so ``backend`` imports as a package (the services use
|
||||||
|
# package-relative imports).
|
||||||
|
sys.path.insert(0, str(Path(__file__).parent.parent.parent))
|
||||||
|
|
||||||
|
from backend import config
|
||||||
|
from backend.database import (
|
||||||
|
Base,
|
||||||
|
CaptureSettings,
|
||||||
|
Generation,
|
||||||
|
GenerationVersion,
|
||||||
|
MCPClientBinding,
|
||||||
|
ProfileChannelMapping,
|
||||||
|
ProfileSample,
|
||||||
|
Story,
|
||||||
|
StoryItem,
|
||||||
|
VoiceProfile,
|
||||||
|
)
|
||||||
|
from backend.models import HistoryQuery
|
||||||
|
from backend.services import history, profiles
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def db(tmp_path, monkeypatch):
|
||||||
|
"""A session backed by a temp SQLite file, with the data dir redirected."""
|
||||||
|
monkeypatch.setattr(config, "_data_dir", tmp_path)
|
||||||
|
|
||||||
|
engine = create_engine(f"sqlite:///{tmp_path / 'test.db'}")
|
||||||
|
Base.metadata.create_all(bind=engine)
|
||||||
|
session = sessionmaker(autocommit=False, autoflush=False, bind=engine)()
|
||||||
|
|
||||||
|
yield session
|
||||||
|
|
||||||
|
session.close()
|
||||||
|
|
||||||
|
|
||||||
|
def _write_wav(name: str) -> tuple[Path, str]:
|
||||||
|
"""Create a placeholder audio file and return (abs path, stored path)."""
|
||||||
|
path = config.get_generations_dir() / name
|
||||||
|
path.write_bytes(b"RIFF placeholder")
|
||||||
|
return path, f"generations/{name}"
|
||||||
|
|
||||||
|
|
||||||
|
def _make_profile(db, profile_id: str = "profile-1") -> VoiceProfile:
|
||||||
|
profile = VoiceProfile(id=profile_id, name=f"Voice {profile_id}", language="en")
|
||||||
|
db.add(profile)
|
||||||
|
db.commit()
|
||||||
|
return profile
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_profile_removes_generations_and_audio(db):
|
||||||
|
"""Generations, versions, and their files go with the profile."""
|
||||||
|
_make_profile(db)
|
||||||
|
gen_file, gen_stored = _write_wav("gen.wav")
|
||||||
|
version_file, version_stored = _write_wav("gen-reverb.wav")
|
||||||
|
|
||||||
|
db.add(Generation(id="gen-1", profile_id="profile-1", text="hello", audio_path=gen_stored))
|
||||||
|
db.add(
|
||||||
|
GenerationVersion(
|
||||||
|
id="version-1",
|
||||||
|
generation_id="gen-1",
|
||||||
|
label="Reverb",
|
||||||
|
audio_path=version_stored,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
db.commit()
|
||||||
|
|
||||||
|
assert await profiles.delete_profile("profile-1", db) is True
|
||||||
|
|
||||||
|
assert db.query(Generation).count() == 0
|
||||||
|
assert db.query(GenerationVersion).count() == 0
|
||||||
|
assert not gen_file.exists()
|
||||||
|
assert not version_file.exists()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_orphaned_generations_are_invisible_in_history(db):
|
||||||
|
"""The history join hides orphans, so leaving them behind strands them."""
|
||||||
|
_make_profile(db)
|
||||||
|
_, gen_stored = _write_wav("gen.wav")
|
||||||
|
db.add(Generation(id="gen-1", profile_id="profile-1", text="hello", audio_path=gen_stored))
|
||||||
|
db.commit()
|
||||||
|
|
||||||
|
assert (await history.list_generations(HistoryQuery(limit=50, offset=0), db)).total == 1
|
||||||
|
|
||||||
|
await profiles.delete_profile("profile-1", db)
|
||||||
|
|
||||||
|
listed = await history.list_generations(HistoryQuery(limit=50, offset=0), db)
|
||||||
|
assert listed.total == 0
|
||||||
|
assert db.query(Generation).count() == 0, "generation is unreachable but still on disk"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_profile_removes_story_items(db):
|
||||||
|
"""Story items reference generations; the story query would hide leftovers."""
|
||||||
|
_make_profile(db)
|
||||||
|
_, gen_stored = _write_wav("gen.wav")
|
||||||
|
db.add(Generation(id="gen-1", profile_id="profile-1", text="hello", audio_path=gen_stored))
|
||||||
|
db.add(Story(id="story-1", name="Chapter one"))
|
||||||
|
db.add(StoryItem(id="item-1", story_id="story-1", generation_id="gen-1"))
|
||||||
|
db.commit()
|
||||||
|
|
||||||
|
await profiles.delete_profile("profile-1", db)
|
||||||
|
|
||||||
|
assert db.query(StoryItem).count() == 0
|
||||||
|
assert db.query(Story).count() == 1, "the story itself must survive"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_profile_clears_references_to_it(db):
|
||||||
|
"""Samples, channel mappings, and default-voice pointers are cleaned up."""
|
||||||
|
_make_profile(db)
|
||||||
|
db.add(ProfileSample(id="sample-1", profile_id="profile-1", audio_path="p.wav", reference_text="hi"))
|
||||||
|
db.add(ProfileChannelMapping(profile_id="profile-1", channel_id="channel-1"))
|
||||||
|
db.add(MCPClientBinding(client_id="claude-code", profile_id="profile-1"))
|
||||||
|
db.add(CaptureSettings(id=1, default_playback_voice_id="profile-1"))
|
||||||
|
db.commit()
|
||||||
|
|
||||||
|
await profiles.delete_profile("profile-1", db)
|
||||||
|
|
||||||
|
assert db.query(ProfileSample).count() == 0
|
||||||
|
assert db.query(ProfileChannelMapping).count() == 0
|
||||||
|
assert db.query(MCPClientBinding).filter_by(client_id="claude-code").one().profile_id is None
|
||||||
|
assert db.query(CaptureSettings).filter_by(id=1).one().default_playback_voice_id is None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_profile_survives_unremovable_audio(db, monkeypatch):
|
||||||
|
"""A file locked by playback must not abort the delete half-way through."""
|
||||||
|
_make_profile(db)
|
||||||
|
gen_file, gen_stored = _write_wav("gen.wav")
|
||||||
|
db.add(Generation(id="gen-1", profile_id="profile-1", text="hello", audio_path=gen_stored))
|
||||||
|
db.commit()
|
||||||
|
|
||||||
|
real_unlink = Path.unlink
|
||||||
|
|
||||||
|
def refuse_locked_file(self, *args, **kwargs):
|
||||||
|
if self == gen_file:
|
||||||
|
raise OSError("file is in use by another process")
|
||||||
|
return real_unlink(self, *args, **kwargs)
|
||||||
|
|
||||||
|
monkeypatch.setattr(Path, "unlink", refuse_locked_file)
|
||||||
|
|
||||||
|
assert await profiles.delete_profile("profile-1", db) is True
|
||||||
|
assert db.query(VoiceProfile).count() == 0
|
||||||
|
assert db.query(Generation).count() == 0
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_profile_leaves_other_profiles_alone(db):
|
||||||
|
"""Cleanup is scoped to the deleted profile."""
|
||||||
|
_make_profile(db, "profile-1")
|
||||||
|
_make_profile(db, "profile-2")
|
||||||
|
_, keep_stored = _write_wav("keep.wav")
|
||||||
|
_, drop_stored = _write_wav("drop.wav")
|
||||||
|
db.add(Generation(id="gen-keep", profile_id="profile-2", text="keep", audio_path=keep_stored))
|
||||||
|
db.add(Generation(id="gen-drop", profile_id="profile-1", text="drop", audio_path=drop_stored))
|
||||||
|
db.add(ProfileChannelMapping(profile_id="profile-2", channel_id="channel-1"))
|
||||||
|
db.commit()
|
||||||
|
|
||||||
|
await profiles.delete_profile("profile-1", db)
|
||||||
|
|
||||||
|
assert [g.id for g in db.query(Generation).all()] == ["gen-keep"]
|
||||||
|
assert db.query(ProfileChannelMapping).count() == 1
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_missing_profile_returns_false(db):
|
||||||
|
assert await profiles.delete_profile("nope", db) is False
|
||||||
Reference in New Issue
Block a user