feat!: ユーザー辞書データに改行やnull文字が入っていた場合にエラーとする #1522
Conversation
|
現在テストが落ちていますが、 #1523 がマージされれば通るはずです。 |
takana-v
force-pushed
the
fix/validate-surface
branch
from
f4fbba7 to
7615029
Compare
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
voicevox_engine/user_dict/model.py:66
- [nitpick] The method name 'remove_newlines_and_null' could be more descriptive. Consider renaming it to 'sanitize_surface'.
def remove_newlines_and_null(cls, surface: str) -> str:
通常のAPI経由の使用では、surface以外に改行やnull文字が混入しないため、今回はsurfaceのみ実装しました。
Annotated patternを使うことで実装できそうです。 ちなみにですが、CSVのエントリをダブルクォートで囲んでエスケープする方法は使えなさそうでした。 |
|
おーーーありがとうございます！！ ちょっとメンテナンス性考えて整理してみました！
ということで、少なくとも今回はデータ変換ではなく検証のみにするのはどうでしょう･･･！！ @takana-v さんが書いてくださったブランチのコードでほとんどできてると認識してます！（注文が多くてすみません。。。） 個人用メモ：
|
takana-v
changed the title
|
変換ではなく検証のみを行うように変更しました。
Pydanticは、「入力をチェックする」ものではなく、「最終的な出力の型を保証する」という考え方のようです。
入力チェックをModelの外に切り出すのであれば、Pydanticではなくdataclassを使うべきなんですかね...？
一方Pydanticに乗っかるのであれば、こんな感じにデータ変換をすることになるかなと思います。
ちょっとユーザー辞書周りのデータ変換フローがややこしい感じがしたので一旦整理してみました。 辞書登録時
辞書読み込み時
|
|
いったんコメントお答えまで！
自分もドキュメント読んでみた感じ、そういう雰囲気を感じました！ だとしたら 検証をdataclassで行うかPydanticで行うか
すでに値として
仰ってる感じになると思います！ CsvSafeStr型のようなものを作ってUserDictWord (dataclass)の下に持たせれば、変換し忘れを防止できるかもです。
おーーーー！！！ ありがとうございます！！
って感じですかね！ 一旦、やるやらは置いといて一番良さそうなのはこれ！というのを考えてみました！！
（ただの感想です：なるほどこれがリファクタリングなんだなぁと感じました。） |
voicevox_engine/user_dict/model.py
Outdated
| surface: Annotated[ | ||
| str, | ||
| AfterValidator(convert_to_zenkaku), | ||
| AfterValidator(check_newlines_and_null), | ||
| ] = Field(description="表層形") |
There was a problem hiding this comment.
（ただのコメントです）
CsvSafeStr使ったほうが良さそう？
と思ったけど、これは,とか使えるのが正しいのか〜〜〜。
Co-authored-by: Copilot <[email protected]>
|
指摘があった点の変更を行いました。
これに関しては、 コード例from typing import Any
from pydantic import BaseModel, Field, ValidationInfo, field_validator, model_validator
class UserDictWord(BaseModel):
pronunciation: str # 本来はきちんと検証する
mora_count: int = Field(default=None, validate_default=True)
priority: int = Field(default=None, validate_default=True)
cost: int = Field(default=None, validate_default=True)
@field_validator("mora_count", mode="before")
@classmethod
def check_mora_count(cls, value: Any, info: ValidationInfo) -> Any:
if value is None:
# info.dataから検証済のフィールドを参照できる
# （先に定義されたフィールドから順に検証される）
value = len(info.data["pronunciation"]) # 仮の処理
return value
@model_validator(mode="before")
@classmethod
def check_priority_and_cost(cls, data: Any) -> Any:
if isinstance(data, dict):
if "priority" in data and "cost" not in data:
if isinstance(data["priority"], int):
data["cost"] = data["priority"] * 1000 # 仮の処理
if "cost" in data and "priority" not in data:
if isinstance(data["cost"], int):
data["priority"] = data["cost"] // 1000 # 仮の処理
return data
if __name__ == "__main__":
print(UserDictWord(pronunciation="テスト", cost=5000))
# > pronunciation='テスト' mora_count=3 priority=5 cost=5000
print(UserDictWord(pronunciation="テスト", priority=5))
# > pronunciation='テスト' mora_count=3 priority=5 cost=5000
print(UserDictWord(pronunciation="テスト", mora_count=3, priority=6, cost=6000))
# > pronunciation='テスト' mora_count=3 priority=6 cost=6000
try:
print(UserDictWord(pronunciation="テスト"))
except Exception as e:
print(e)
# > 2 validation error for UserDictWord (priority, costがintじゃないエラー) |
なるほどです･･･！ Pydanticの欠点を一言で表すと、コンストラクタに渡す型とフィールドの型を別々にできない点だなーと感じました！ class UserDictWord:
def __init__(self, pronunciation: str, mora_count: int | "auto")
self.pronunciation = pronunciation
self.mora_count = mora_count if mora_count != "auto" else count(pronunciation)でも提案の通り |
Hiroshiba
changed the title
内容
UserDictWordモデルの作成時に、Surfaceに含まれる改行コードならびにnull文字を削除するようにします。これにより、不正な形式の辞書CSVファイルが生成されることを防ぎます。
関連 Issue
スクリーンショット・動画など
その他
コンマやダブルクオーテーション等の文字列に関しては、
UserDictWordモデルの作成時に全角に変換されるため大丈夫そうです。また、
pronunciationに関しては、カタカナ以外の文字を受け付けないので、こちらも大丈夫そうです。