From a4569181b7c057661b325ca780666d54bc516bd2 Mon Sep 17 00:00:00 2001 From: tarepan Date: Wed, 12 Jun 2024 04:09:52 +0000 Subject: [PATCH 1/5] =?UTF-8?q?refactor:=20`Setting`=20=E3=82=92=E3=83=A2?= =?UTF-8?q?=E3=82=B8=E3=83=A5=E3=83=BC=E3=83=AB=E5=86=85=E3=81=AB=E9=99=90?= =?UTF-8?q?=E5=B1=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- run.py | 8 ++-- test/unit/setting/test_setting.py | 54 ++++++++++++---------- voicevox_engine/app/routers/setting.py | 19 ++------ voicevox_engine/setting/setting_manager.py | 11 +++-- 4 files changed, 45 insertions(+), 47 deletions(-) diff --git a/run.py b/run.py index 7ee12ea7c..5ac842ddb 100644 --- a/run.py +++ b/run.py @@ -297,19 +297,19 @@ def main() -> None: ) setting_loader = SettingHandler(args.setting_file) - settings = setting_loader.load() + setting_cors_policy_mode, setting_allow_origin_raw = setting_loader.load() # 複数方式で指定可能な場合、優先度は上から「引数」「環境変数」「設定ファイル」「デフォルト値」 root_dir = select_first_not_none([voicevox_dir, engine_root()]) cors_policy_mode = select_first_not_none( - [arg_cors_policy_mode, settings.cors_policy_mode] + [arg_cors_policy_mode, setting_cors_policy_mode] ) setting_allow_origin = None - if settings.allow_origin is not None: - setting_allow_origin = settings.allow_origin.split(" ") + if setting_allow_origin_raw is not None: + setting_allow_origin = setting_allow_origin_raw.split(" ") allow_origin = select_first_not_none_or_none( [arg_allow_origin, setting_allow_origin] ) diff --git a/test/unit/setting/test_setting.py b/test/unit/setting/test_setting.py index 421a000b7..276366344 100644 --- a/test/unit/setting/test_setting.py +++ b/test/unit/setting/test_setting.py @@ -11,13 +11,14 @@ def test_setting_handler_load_not_exist_file() -> None: """`SettingHandler` に存在しない設定ファイルのパスを渡すとデフォルト値になる。""" # Inputs setting_loader = SettingHandler(Path("not_exist.yaml")) - settings = setting_loader.load() # Expects - true_setting = {"allow_origin": None, "cors_policy_mode": CorsPolicyMode.localapps} + true_cors_policy_mode = CorsPolicyMode.localapps + true_allow_origin = None # Outputs - setting = settings.model_dump() + cors_policy_mode, allow_origin = setting_loader.load() # Test - assert true_setting == setting + assert true_cors_policy_mode == cors_policy_mode + assert true_allow_origin == allow_origin def test_setting_handler_load_exist_file_1() -> None: @@ -25,13 +26,14 @@ def test_setting_handler_load_exist_file_1() -> None: # Inputs setting_path = Path("test/unit/setting/setting-test-load-1.yaml") setting_loader = SettingHandler(setting_path) - settings = setting_loader.load() # Expects - true_setting = {"allow_origin": None, "cors_policy_mode": CorsPolicyMode.localapps} + true_cors_policy_mode = CorsPolicyMode.localapps + true_allow_origin = None # Outputs - setting = settings.model_dump() + cors_policy_mode, allow_origin = setting_loader.load() # Test - assert true_setting == setting + assert true_cors_policy_mode == cors_policy_mode + assert true_allow_origin == allow_origin def test_setting_handler_load_exist_file_2() -> None: @@ -39,13 +41,15 @@ def test_setting_handler_load_exist_file_2() -> None: # Inputs setting_path = Path("test/unit/setting/setting-test-load-2.yaml") setting_loader = SettingHandler(setting_path) - settings = setting_loader.load() + cors_policy_mode, allow_origin = setting_loader.load() # Expects - true_setting = {"allow_origin": None, "cors_policy_mode": "all"} + true_cors_policy_mode = CorsPolicyMode.all + true_allow_origin = None # Outputs - setting = settings.model_dump() + cors_policy_mode, allow_origin = setting_loader.load() # Test - assert true_setting == setting + assert true_cors_policy_mode == cors_policy_mode + assert true_allow_origin == allow_origin def test_setting_handler_load_exist_file_3() -> None: @@ -53,16 +57,15 @@ def test_setting_handler_load_exist_file_3() -> None: # Inputs setting_path = Path("test/unit/setting/setting-test-load-3.yaml") setting_loader = SettingHandler(setting_path) - settings = setting_loader.load() + cors_policy_mode, allow_origin = setting_loader.load() # Expects - true_setting = { - "allow_origin": "192.168.254.255 192.168.255.255", - "cors_policy_mode": CorsPolicyMode.localapps, - } + true_cors_policy_mode = CorsPolicyMode.localapps + true_allow_origin = "192.168.254.255 192.168.255.255" # Outputs - setting = settings.model_dump() + cors_policy_mode, allow_origin = setting_loader.load() # Test - assert true_setting == setting + assert true_cors_policy_mode == cors_policy_mode + assert true_allow_origin == allow_origin def test_setting_handler_save(tmp_path: Path) -> None: @@ -70,15 +73,18 @@ def test_setting_handler_save(tmp_path: Path) -> None: # Inputs setting_path = tmp_path / "setting-test-dump.yaml" setting_loader = SettingHandler(setting_path) - new_setting = Setting(cors_policy_mode=CorsPolicyMode.localapps) + new_setting_cors = CorsPolicyMode.localapps + new_setting_origin = None # Expects - true_setting = {"allow_origin": None, "cors_policy_mode": CorsPolicyMode.localapps} + true_cors_policy_mode = CorsPolicyMode.localapps + true_allow_origin = None # Outputs - setting_loader.save(new_setting) + setting_loader.save(new_setting_cors, new_setting_origin) # NOTE: `.load()` の正常動作を前提とする - setting = setting_loader.load().model_dump() + cors_policy_mode, allow_origin = setting_loader.load() # Test - assert true_setting == setting + assert true_cors_policy_mode == cors_policy_mode + assert true_allow_origin == allow_origin def test_setting_invalid_input() -> None: diff --git a/voicevox_engine/app/routers/setting.py b/voicevox_engine/app/routers/setting.py index eddf66830..cd5fe229e 100644 --- a/voicevox_engine/app/routers/setting.py +++ b/voicevox_engine/app/routers/setting.py @@ -8,7 +8,7 @@ from voicevox_engine.engine_manifest import BrandName from voicevox_engine.setting.model import CorsPolicyMode -from voicevox_engine.setting.setting_manager import Setting, SettingHandler +from voicevox_engine.setting.setting_manager import SettingHandler from voicevox_engine.utility.path_utility import resource_root from ..dependencies import check_disabled_mutable_api @@ -31,10 +31,7 @@ def setting_get(request: Request) -> Response: """ 設定ページを返します。 """ - settings = setting_loader.load() - - cors_policy_mode = settings.cors_policy_mode - allow_origin = settings.allow_origin + cors_policy_mode, allow_origin = setting_loader.load() if allow_origin is None: allow_origin = "" @@ -56,15 +53,7 @@ def setting_post( cors_policy_mode: Annotated[CorsPolicyMode, Form()], allow_origin: Annotated[str | SkipJsonSchema[None], Form()] = None, ) -> None: - """ - 設定を更新します。 - """ - settings = Setting( - cors_policy_mode=cors_policy_mode, - allow_origin=allow_origin, - ) - - # 更新した設定へ上書き - setting_loader.save(settings) + """設定を更新します。""" + setting_loader.save(cors_policy_mode, allow_origin) return router diff --git a/voicevox_engine/setting/setting_manager.py b/voicevox_engine/setting/setting_manager.py index ab8967956..d349aa985 100644 --- a/voicevox_engine/setting/setting_manager.py +++ b/voicevox_engine/setting/setting_manager.py @@ -33,7 +33,7 @@ def __init__(self, setting_file_path: Path) -> None: """ self.setting_file_path = setting_file_path - def load(self) -> Setting: + def load(self) -> tuple[CorsPolicyMode, str | None]: """設定値をファイルから読み込む。""" if not self.setting_file_path.is_file(): # 設定ファイルが存在しないためデフォルト値を取得 @@ -43,14 +43,17 @@ def load(self) -> Setting: # FIXME: 型チェックと例外処理を追加する setting = yaml.safe_load(self.setting_file_path.read_text(encoding="utf-8")) - return Setting( + setting_obj = Setting( cors_policy_mode=setting["cors_policy_mode"], allow_origin=setting["allow_origin"], ) + return setting_obj.cors_policy_mode, setting_obj.allow_origin - def save(self, settings: Setting) -> None: + def save(self, cors_policy_mode: CorsPolicyMode, allow_origin: str | None) -> None: """設定値をファイルへ書き込む。""" - settings_dict = settings.model_dump() + settings_dict = Setting( + cors_policy_mode=cors_policy_mode, allow_origin=allow_origin + ).model_dump() if isinstance(settings_dict["cors_policy_mode"], Enum): settings_dict["cors_policy_mode"] = settings_dict["cors_policy_mode"].value From 265d8f08b03114df2a2d1e6cf60f455389266dd9 Mon Sep 17 00:00:00 2001 From: tarepan Date: Wed, 12 Jun 2024 04:24:07 +0000 Subject: [PATCH 2/5] =?UTF-8?q?refactor:=20`Setting`=20=E3=82=92=E3=83=97?= =?UTF-8?q?=E3=83=A9=E3=82=A4=E3=83=99=E3=83=BC=E3=83=88=E5=8C=96=E3=83=BB?= =?UTF-8?q?=E7=B0=A1=E7=95=A5=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/unit/setting/test_setting.py | 4 ++-- voicevox_engine/setting/setting_manager.py | 16 +++++++--------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/test/unit/setting/test_setting.py b/test/unit/setting/test_setting.py index 276366344..38b0c2236 100644 --- a/test/unit/setting/test_setting.py +++ b/test/unit/setting/test_setting.py @@ -4,7 +4,7 @@ from pydantic import ValidationError from voicevox_engine.setting.model import CorsPolicyMode -from voicevox_engine.setting.setting_manager import Setting, SettingHandler +from voicevox_engine.setting.setting_manager import _Setting, SettingHandler def test_setting_handler_load_not_exist_file() -> None: @@ -91,4 +91,4 @@ def test_setting_invalid_input() -> None: """`Setting` は不正な入力に対してエラーを送出する。""" # Test with pytest.raises(ValidationError) as _: - Setting(cors_policy_mode="invalid_value", allow_origin="*") + _Setting(cors_policy_mode="invalid_value", allow_origin="*") diff --git a/voicevox_engine/setting/setting_manager.py b/voicevox_engine/setting/setting_manager.py index d349aa985..6344a7b5e 100644 --- a/voicevox_engine/setting/setting_manager.py +++ b/voicevox_engine/setting/setting_manager.py @@ -4,19 +4,17 @@ from pathlib import Path import yaml -from pydantic import BaseModel, Field +from pydantic import BaseModel from ..utility.path_utility import get_save_dir from .model import CorsPolicyMode -class Setting(BaseModel): - """ - エンジンの設定情報 - """ +class _Setting(BaseModel): + """エンジンの設定情報""" - cors_policy_mode: CorsPolicyMode = Field(title="リソース共有ポリシー") - allow_origin: str | None = Field(default=None, title="許可するオリジン") + cors_policy_mode: CorsPolicyMode # リソース共有ポリシー + allow_origin: str | None # 許可するオリジン USER_SETTING_PATH: Path = get_save_dir() / "setting.yml" @@ -43,7 +41,7 @@ def load(self) -> tuple[CorsPolicyMode, str | None]: # FIXME: 型チェックと例外処理を追加する setting = yaml.safe_load(self.setting_file_path.read_text(encoding="utf-8")) - setting_obj = Setting( + setting_obj = _Setting( cors_policy_mode=setting["cors_policy_mode"], allow_origin=setting["allow_origin"], ) @@ -51,7 +49,7 @@ def load(self) -> tuple[CorsPolicyMode, str | None]: def save(self, cors_policy_mode: CorsPolicyMode, allow_origin: str | None) -> None: """設定値をファイルへ書き込む。""" - settings_dict = Setting( + settings_dict = _Setting( cors_policy_mode=cors_policy_mode, allow_origin=allow_origin ).model_dump() From 3390d1e6371c3c63e3465ef1ef35c063213cffa0 Mon Sep 17 00:00:00 2001 From: tarepan Date: Wed, 12 Jun 2024 04:53:04 +0000 Subject: [PATCH 3/5] =?UTF-8?q?refactor:=20`allow=5Forigin`=20=E3=81=AE?= =?UTF-8?q?=E5=9E=8B=E3=82=A8=E3=82=A4=E3=83=AA=E3=82=A2=E3=82=B9=E3=82=92?= =?UTF-8?q?=E8=BF=BD=E5=8A=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- voicevox_engine/setting/setting_manager.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/voicevox_engine/setting/setting_manager.py b/voicevox_engine/setting/setting_manager.py index 6344a7b5e..5f2985e35 100644 --- a/voicevox_engine/setting/setting_manager.py +++ b/voicevox_engine/setting/setting_manager.py @@ -2,6 +2,7 @@ from enum import Enum from pathlib import Path +from typing import TypeAlias import yaml from pydantic import BaseModel @@ -10,11 +11,14 @@ from .model import CorsPolicyMode +_AllowOrigin: TypeAlias = str | None + + class _Setting(BaseModel): """エンジンの設定情報""" cors_policy_mode: CorsPolicyMode # リソース共有ポリシー - allow_origin: str | None # 許可するオリジン + allow_origin: _AllowOrigin # 許可するオリジン USER_SETTING_PATH: Path = get_save_dir() / "setting.yml" @@ -31,7 +35,7 @@ def __init__(self, setting_file_path: Path) -> None: """ self.setting_file_path = setting_file_path - def load(self) -> tuple[CorsPolicyMode, str | None]: + def load(self) -> tuple[CorsPolicyMode, _AllowOrigin]: """設定値をファイルから読み込む。""" if not self.setting_file_path.is_file(): # 設定ファイルが存在しないためデフォルト値を取得 @@ -47,7 +51,7 @@ def load(self) -> tuple[CorsPolicyMode, str | None]: ) return setting_obj.cors_policy_mode, setting_obj.allow_origin - def save(self, cors_policy_mode: CorsPolicyMode, allow_origin: str | None) -> None: + def save(self, cors_policy_mode: CorsPolicyMode, allow_origin: _AllowOrigin) -> None: """設定値をファイルへ書き込む。""" settings_dict = _Setting( cors_policy_mode=cors_policy_mode, allow_origin=allow_origin From 8f936be99d05c772b76adb78a1e9e57cb5172abe Mon Sep 17 00:00:00 2001 From: tarepan Date: Wed, 12 Jun 2024 04:56:01 +0000 Subject: [PATCH 4/5] =?UTF-8?q?refactor:=20`=5FSetting`=20=E3=82=A4?= =?UTF-8?q?=E3=83=B3=E3=82=B9=E3=82=BF=E3=83=B3=E3=82=B9=E5=8C=96=20util?= =?UTF-8?q?=20=E3=82=92=E5=88=A9=E7=94=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/unit/setting/test_setting.py | 2 +- voicevox_engine/setting/setting_manager.py | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/test/unit/setting/test_setting.py b/test/unit/setting/test_setting.py index 38b0c2236..c7363ef0e 100644 --- a/test/unit/setting/test_setting.py +++ b/test/unit/setting/test_setting.py @@ -4,7 +4,7 @@ from pydantic import ValidationError from voicevox_engine.setting.model import CorsPolicyMode -from voicevox_engine.setting.setting_manager import _Setting, SettingHandler +from voicevox_engine.setting.setting_manager import SettingHandler, _Setting def test_setting_handler_load_not_exist_file() -> None: diff --git a/voicevox_engine/setting/setting_manager.py b/voicevox_engine/setting/setting_manager.py index 5f2985e35..559b0a18f 100644 --- a/voicevox_engine/setting/setting_manager.py +++ b/voicevox_engine/setting/setting_manager.py @@ -42,13 +42,10 @@ def load(self) -> tuple[CorsPolicyMode, _AllowOrigin]: setting = {"allow_origin": None, "cors_policy_mode": "localapps"} else: # 指定された設定ファイルから値を取得 - # FIXME: 型チェックと例外処理を追加する + # FIXME: 例外処理を追加する setting = yaml.safe_load(self.setting_file_path.read_text(encoding="utf-8")) - setting_obj = _Setting( - cors_policy_mode=setting["cors_policy_mode"], - allow_origin=setting["allow_origin"], - ) + setting_obj = _Setting.model_validate(setting) return setting_obj.cors_policy_mode, setting_obj.allow_origin def save(self, cors_policy_mode: CorsPolicyMode, allow_origin: _AllowOrigin) -> None: From 65cd39e3edfabf8b66c4006d74d5657844bc5ee5 Mon Sep 17 00:00:00 2001 From: tarepan Date: Wed, 12 Jun 2024 04:56:06 +0000 Subject: [PATCH 5/5] fix: lint --- voicevox_engine/setting/setting_manager.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/voicevox_engine/setting/setting_manager.py b/voicevox_engine/setting/setting_manager.py index 559b0a18f..df80a4b88 100644 --- a/voicevox_engine/setting/setting_manager.py +++ b/voicevox_engine/setting/setting_manager.py @@ -10,7 +10,6 @@ from ..utility.path_utility import get_save_dir from .model import CorsPolicyMode - _AllowOrigin: TypeAlias = str | None @@ -48,7 +47,9 @@ def load(self) -> tuple[CorsPolicyMode, _AllowOrigin]: setting_obj = _Setting.model_validate(setting) return setting_obj.cors_policy_mode, setting_obj.allow_origin - def save(self, cors_policy_mode: CorsPolicyMode, allow_origin: _AllowOrigin) -> None: + def save( + self, cors_policy_mode: CorsPolicyMode, allow_origin: _AllowOrigin + ) -> None: """設定値をファイルへ書き込む。""" settings_dict = _Setting( cors_policy_mode=cors_policy_mode, allow_origin=allow_origin