-
Notifications
You must be signed in to change notification settings - Fork 0
業務例外発生時にexceptionIdとexceptionValuesが返却されるようにする #2746
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
base: main
Are you sure you want to change the base?
The head ref may contain hidden characters: "feature/\u696D\u52D9\u4F8B\u5916\u767A\u751F\u6642\u306BexceptionId\u3068exceptionValues\u304C\u8FD4\u5374\u3055\u308C\u308B\u3088\u3046\u306B\u3059\u308B"
業務例外発生時にexceptionIdとexceptionValuesが返却されるようにする #2746
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
業務例外発生時にexceptionId
とexceptionValues
をHTTPレスポンスで返却できるようにする改修です。
ErrorMessage
クラスを導入し、エラーメッセージとプレースホルダー値を管理BusinessError
/BusinessException
をexceptionId
ベースにリファクタリング- 例外フィルターでProblemDetails拡張に
exceptionId
/exceptionValues
を設定 - 各種ドメイン例外を
BusinessException
派生に変更し、テストを更新
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
ErrorMessage.cs | 新クラス追加: メッセージとプレースホルダー値を保持 |
BusinessError.cs / BusinessException.cs / BusinessErrorCollection.cs | ErrorCode →ExceptionId ・string →ErrorMessage へ変更 |
BusinessExceptionFilterBase.cs / BusinessExceptionDevelopmentFilter.cs | exceptionId /exceptionValues をProblemDetailsに追加 |
各ドメイン例外 (Ordering, Assets, Baskets, Authorization 等) | BusinessException 派生に変更し、ErrorMessage を使用 |
テストプロジェクト | 上記変更に合わせてアサーションや変数名を修正 |
Comments suppressed due to low confidence (3)
samples/Dressca/dressca-backend/src/Dressca.Web/Runtime/BusinessExceptionFilterBase.cs:49
- [nitpick]
BusinessExceptionFilterBase
とBusinessExceptionDevelopmentFilter
で同一の拡張処理が重複しています。共通ロジックをヘルパーメソッドや基底クラスに切り出してDRYを意識すると保守性が向上します。
// 暫定の実装として、1つ目のBusinessErrorのexceptionIdとexceptionValuesを設定
samples/Dressca/dressca-backend/src/Dressca.Web/Runtime/BusinessExceptionFilterBase.cs:50
- ProblemDetailsの拡張プロパティ(
exceptionId
/exceptionValues
)を返却する挙動について、現状テストが追加されていません。HTTPレスポンスで正しく設定されることを検証する単体テストを追加しましょう。
problemDetails.Extensions.Add("exceptionId", businessEx.GetBusinessErrors.First().ExceptionId);
samples/Dressca/dressca-backend/src/Dressca.ApplicationCore/Ordering/OrderNotFoundException.cs:11
- [nitpick]
exceptionId
文字列のフォーマット(キャメルケース、スネークケースなど)がクラス間で一貫していないように見えます。クライアントAPIが期待するIDフォーマットを定め、統一してください。
private const string ExceptionId = "orderNotFound";
samples/Dressca/dressca-backend/src/Dressca.SystemCommon/ErrorMessage.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.SystemCommon/ErrorMessage.cs
Outdated
Show resolved
Hide resolved
...essca/dressca-backend/src/Dressca.ApplicationCore/Authorization/PermissionDeniedException.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.SystemCommon/ErrorMessage.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.ApplicationCore/Ordering/OrderNotFoundException.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.Web/Runtime/BusinessExceptionFilter.cs
Outdated
Show resolved
Hide resolved
...nd/tests/Dressca.UnitTests.ApplicationCore/ApplicationService/AssetApplicationServiceTest.cs
Show resolved
Hide resolved
samples/Dressca/dressca-backend/tests/Dressca.UnitTests.SystemCommon/BusinessErrorTest.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.SystemCommon/ErrorMessage.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.SystemCommon/ErrorMessage.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.SystemCommon/ErrorMessage.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.SystemCommon/ErrorMessage.cs
Outdated
Show resolved
Hide resolved
...essca/dressca-backend/src/Dressca.ApplicationCore/Authorization/PermissionDeniedException.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.ApplicationCore/Ordering/OrderNotFoundException.cs
Outdated
Show resolved
Hide resolved
...nd/tests/Dressca.UnitTests.ApplicationCore/ApplicationService/AssetApplicationServiceTest.cs
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.Web/Runtime/BusinessExceptionDevelopmentFilter.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/tests/Dressca.UnitTests.SystemCommon/ErrorMessageTest.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/tests/Dressca.UnitTests.SystemCommon/BusinessErrorTest.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.SystemCommon/ErrorMessage.cs
Outdated
Show resolved
Hide resolved
samples/Dressca/dressca-backend/src/Dressca.SystemCommon/ErrorMessage.cs
Fixed
Show resolved
Hide resolved
public string Message { get; private set; } | ||
|
||
/// <summary> | ||
/// エラーメッセージのプレースホルダーの値を取得します。 | ||
/// </summary> | ||
public object[] ErrorMessageValues { get; private set; } = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
コンストラクターで初期化した後、どこからも変更しないのであれば private set;
は不要ではないですか?
セッターを残しておく必要があるんでしたっけ?
そして ErrorMessageValues
をコンストラクターで初期化することが確定しているなら、このプロパティを空の配列で初期化する意義はないように思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
コンストラクターで初期化した後は変更しない想定のため、不要なsetterと空の配列での初期化処理を削除しました。
Test Result 📝Test was a success. Coverage 📐Summary
Dressca.ApplicationCore - 96%
Dressca.EfInfrastructure - 16.5%
Dressca.Store.Assets.StaticFiles - 17.2%
Dressca.SystemCommon - 83.3%
Dressca.Web - 26.6%
Dressca.Web.Consumer - 43.9%
Dressca.Web.Consumer.Dto - 58.4%
|
この Pull request で実施したこと
BusinessException
のexceptionId
の値をMaiaに合わせて修正BusinessError
が保持するエラーメッセージとそのプレースホルダーの値をErrorMessage
クラスで管理するよう修正exceptionId
とexceptionValues
を返すよう修正この Pull request では実施していないこと
BusinessException
が複数のBusinessError
を保持する場合BusinessError
が複数のエラーメッセージを保持する場合Issues や Discussions 、関連する Web サイトなどへのリンク
なし