Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

修正: プリセットの一律 500 エラーを詳細化し 422 化して修正 #1162

Merged
merged 8 commits into from
May 5, 2024
32 changes: 19 additions & 13 deletions test/preset/test_preset.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from unittest import TestCase

from voicevox_engine.preset.Preset import Preset
from voicevox_engine.preset.PresetError import PresetError
from voicevox_engine.preset.PresetError import PresetInputError, PresetInternalError
from voicevox_engine.preset.PresetManager import PresetManager

presets_test_1_yaml_path = Path("test/preset/presets-test-1.yaml")
Expand Down Expand Up @@ -37,26 +37,28 @@ def test_validation_same(self) -> None:
def test_validation_2(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_2_yaml_path)
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルにミスがあります"
PresetInternalError, msg="プリセットの設定ファイルにミスがあります"
):
preset_manager.load_presets()

def test_preset_id(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_3_yaml_path)
with self.assertRaises(PresetError, msg="プリセットのidに重複があります"):
with self.assertRaises(
PresetInternalError, msg="プリセットのidに重複があります"
):
preset_manager.load_presets()

def test_empty_file(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_4_yaml_path)
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルが空の内容です"
PresetInternalError, msg="プリセットの設定ファイルが空の内容です"
):
preset_manager.load_presets()

def test_not_exist_file(self) -> None:
preset_manager = PresetManager(preset_path=Path("test/presets-dummy.yaml"))
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルが見つかりません"
PresetInternalError, msg="プリセットの設定ファイルが見つかりません"
):
preset_manager.load_presets()

Expand Down Expand Up @@ -89,7 +91,7 @@ def test_add_preset(self) -> None:
def test_add_preset_load_failure(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_2_yaml_path)
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルにミスがあります"
PresetInternalError, msg="プリセットの設定ファイルにミスがあります"
):
preset_manager.add_preset(
Preset(
Expand Down Expand Up @@ -182,7 +184,7 @@ def test_add_preset_write_failure(self) -> None:
preset_manager._refresh_cache = lambda: None # type:ignore[method-assign]
preset_manager.preset_path = "" # type: ignore[assignment]
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルに書き込み失敗しました"
PresetInternalError, msg="プリセットの設定ファイルが見つかりません"
):
preset_manager.add_preset(preset)
self.assertEqual(len(preset_manager.presets), 2)
Expand Down Expand Up @@ -217,7 +219,7 @@ def test_update_preset(self) -> None:
def test_update_preset_load_failure(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_2_yaml_path)
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルにミスがあります"
PresetInternalError, msg="プリセットの設定ファイルにミスがあります"
):
preset_manager.update_preset(
Preset(
Expand Down Expand Up @@ -254,7 +256,9 @@ def test_update_preset_not_found(self) -> None:
"postPhonemeLength": 0.1,
}
)
with self.assertRaises(PresetError, msg="更新先のプリセットが存在しません"):
with self.assertRaises(
PresetInputError, msg="更新先のプリセットが存在しません"
):
preset_manager.update_preset(preset)
self.assertEqual(len(preset_manager.presets), 2)
remove(temp_path)
Expand All @@ -281,7 +285,7 @@ def test_update_preset_write_failure(self) -> None:
preset_manager._refresh_cache = lambda: None # type:ignore[method-assign]
preset_manager.preset_path = "" # type: ignore[assignment]
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルに書き込み失敗しました"
PresetInternalError, msg="プリセットの設定ファイルが見つかりません"
):
preset_manager.update_preset(preset)
self.assertEqual(len(preset_manager.presets), 2)
Expand All @@ -300,15 +304,17 @@ def test_delete_preset(self) -> None:
def test_delete_preset_load_failure(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_2_yaml_path)
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルにミスがあります"
PresetInternalError, msg="プリセットの設定ファイルにミスがあります"
):
preset_manager.delete_preset(10)

def test_delete_preset_not_found(self) -> None:
temp_path = self.tmp_dir_path / "presets-test-temp.yaml"
copyfile(presets_test_1_yaml_path, temp_path)
preset_manager = PresetManager(preset_path=temp_path)
with self.assertRaises(PresetError, msg="削除対象のプリセットが存在しません"):
with self.assertRaises(
PresetInputError, msg="削除対象のプリセットが存在しません"
):
preset_manager.delete_preset(10)
self.assertEqual(len(preset_manager.presets), 2)
remove(temp_path)
Expand All @@ -321,7 +327,7 @@ def test_delete_preset_write_failure(self) -> None:
preset_manager._refresh_cache = lambda: None # type:ignore[method-assign]
preset_manager.preset_path = "" # type: ignore[assignment]
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルに書き込み失敗しました"
PresetInternalError, msg="プリセットの設定ファイルが見つかりません"
):
preset_manager.delete_preset(1)
self.assertEqual(len(preset_manager.presets), 2)
Expand Down
18 changes: 13 additions & 5 deletions voicevox_engine/app/routers/preset.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from fastapi import APIRouter, Body, Depends, HTTPException, Query, Response

from voicevox_engine.preset.Preset import Preset
from voicevox_engine.preset.PresetError import PresetError
from voicevox_engine.preset.PresetError import PresetInputError, PresetInternalError
from voicevox_engine.preset.PresetManager import PresetManager

from ..dependencies import check_disabled_mutable_api
Expand All @@ -27,8 +27,10 @@ def get_presets() -> list[Preset]:
"""
try:
presets = preset_manager.load_presets()
except PresetError as err:
except PresetInputError as err:
raise HTTPException(status_code=422, detail=str(err))
except PresetInternalError as err:
raise HTTPException(status_code=500, detail=str(err))
return presets
tarepan marked this conversation as resolved.
Show resolved Hide resolved

@router.post(
Expand All @@ -51,8 +53,10 @@ def add_preset(
"""
try:
id = preset_manager.add_preset(preset)
except PresetError as err:
except PresetInputError as err:
raise HTTPException(status_code=422, detail=str(err))
except PresetInternalError as err:
raise HTTPException(status_code=500, detail=str(err))
return id

@router.post(
Expand All @@ -75,8 +79,10 @@ def update_preset(
"""
try:
id = preset_manager.update_preset(preset)
except PresetError as err:
except PresetInputError as err:
raise HTTPException(status_code=422, detail=str(err))
except PresetInternalError as err:
raise HTTPException(status_code=500, detail=str(err))
return id

@router.post(
Expand All @@ -93,8 +99,10 @@ def delete_preset(
"""
try:
preset_manager.delete_preset(id)
except PresetError as err:
except PresetInputError as err:
raise HTTPException(status_code=422, detail=str(err))
except PresetInternalError as err:
raise HTTPException(status_code=500, detail=str(err))
return Response(status_code=204)

return router
6 changes: 4 additions & 2 deletions voicevox_engine/app/routers/tts_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
ParseKanaError,
Score,
)
from voicevox_engine.preset.PresetError import PresetError
from voicevox_engine.preset.PresetError import PresetInputError, PresetInternalError
from voicevox_engine.preset.PresetManager import PresetManager
from voicevox_engine.tts_pipeline.kana_converter import create_kana, parse_kana
from voicevox_engine.tts_pipeline.tts_engine import TTSEngine
Expand Down Expand Up @@ -88,8 +88,10 @@ def audio_query_from_preset(
core = get_core(core_version)
try:
presets = preset_manager.load_presets()
except PresetError as err:
except PresetInputError as err:
raise HTTPException(status_code=422, detail=str(err))
except PresetInternalError as err:
raise HTTPException(status_code=500, detail=str(err))
for preset in presets:
if preset.id == preset_id:
selected_preset = preset
Expand Down
13 changes: 12 additions & 1 deletion voicevox_engine/preset/PresetError.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,13 @@
class PresetError(Exception):
"""プリセットに関するエラー"""


class PresetInputError(Exception):
"""受け入れ不可能な入力値に起因するエラー"""

pass


class PresetInternalError(Exception):
"""プリセットマネージャーに起因するエラー"""

pass
20 changes: 10 additions & 10 deletions voicevox_engine/preset/PresetManager.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from pydantic import ValidationError, parse_obj_as

from .Preset import Preset
from .PresetError import PresetError
from .PresetError import PresetInputError, PresetInternalError


class PresetManager:
Expand All @@ -32,23 +32,23 @@ def _refresh_cache(self) -> None:
# 更新無し
return
except OSError:
raise PresetError("プリセットの設定ファイルが見つかりません")
raise PresetInternalError("プリセットの設定ファイルが見つかりません")

# データベースの読み込み
with open(self.preset_path, mode="r", encoding="utf-8") as f:
obj = yaml.safe_load(f)
if obj is None:
raise PresetError("プリセットの設定ファイルが空の内容です")
raise PresetInternalError("プリセットの設定ファイルが空の内容です")
try:
_presets = parse_obj_as(List[Preset], obj)
except ValidationError:
raise PresetError("プリセットの設定ファイルにミスがあります")
raise PresetInternalError("プリセットの設定ファイルにミスがあります")

# 全idの一意性をバリデーション
if len([preset.id for preset in _presets]) != len(
{preset.id for preset in _presets}
):
raise PresetError("プリセットのidに重複があります")
raise PresetInternalError("プリセットのidに重複があります")

# キャッシュを更新する
self.presets = _presets
Expand All @@ -72,7 +72,7 @@ def add_preset(self, preset: Preset) -> int:
except Exception as err:
self.presets.pop()
if isinstance(err, FileNotFoundError):
raise PresetError("プリセットの設定ファイルに書き込み失敗しました")
raise PresetInternalError("プリセットの設定ファイルが見つかりません")
else:
raise err

Expand Down Expand Up @@ -100,15 +100,15 @@ def update_preset(self, preset: Preset) -> int:
self.presets[i] = preset
break
else:
raise PresetError("更新先のプリセットが存在しません")
raise PresetInputError("更新先のプリセットが存在しません")

# 変更の反映。失敗時はリバート。
try:
self._write_on_file()
except Exception as err:
self.presets[prev_preset[0]] = prev_preset[1]
if isinstance(err, FileNotFoundError):
raise PresetError("プリセットの設定ファイルに書き込み失敗しました")
raise PresetInternalError("プリセットの設定ファイルが見つかりません")
else:
raise err

Expand All @@ -129,14 +129,14 @@ def delete_preset(self, id: int) -> int:
buf_index = i
break
else:
raise PresetError("削除対象のプリセットが存在しません")
raise PresetInputError("削除対象のプリセットが存在しません")

# 変更の反映。失敗時はリバート。
try:
self._write_on_file()
except FileNotFoundError:
self.presets.insert(buf_index, buf)
raise PresetError("プリセットの設定ファイルに書き込み失敗しました")
raise PresetInternalError("プリセットの設定ファイルが見つかりません")

return id

Expand Down