refactor(validation): replace mixed decoded body with a DecodedBody envelope (#248)#250
Merged
Merged
Conversation
…nvelope (#248) The decoded request / response body flowed through the validators as a bare `mixed`, which could not distinguish an absent body from a literal JSON `null` body. PR #247 patched that gap with an internal marker enum (`PresentJsonNull`) that every consumer had to hand-unwrap, with two divergent unwrap idioms and no static guarantee the marker was unwrapped. Introduce a `DecodedBody` envelope (`final readonly`, named constructors `absent()` / `present()` / `fromLegacy()`) that carries the absent / present distinction as a single typed value end to end. The framework adapters build the envelope; the `@internal` body validators now receive it by type, so a consumer can no longer forget the envelope exists. The public `OpenApiResponseValidator::validate()` / `OpenApiRequestValidator::validate()` body parameter stays `mixed` for backward compatibility — `DecodedBody::fromLegacy()` normalizes it at the entry point (a plain `null` becomes an absent body, as before). This keeps the change a v1.x minor; removing `mixed` from the public signature remains a separate v2 concern. Removes the `PresentJsonNull` marker and the `$bodyWasPresent` flag.
…lic validators (#248) Address review feedback on PR #250: the `DecodedBody` passthrough at the public `validate()` boundary was introduced but never exercised through the real entry point. - Add regression tests passing a `DecodedBody` envelope directly into `OpenApiResponseValidator::validate()` / `OpenApiRequestValidator::validate()` (present value, present literal null, and absent), so a fromLegacy() double-wrap regression is caught at the integration boundary. - Document the body parameter on both public `validate()` methods: it accepts a `DecodedBody` or a bare legacy value, a bare `null` reads as an absent body, and a literal JSON null body needs `DecodedBody::present(null)`. - Document the decoded-JSON shape of `DecodedBody::$value` on the constructor.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
decoded body を validator 内部で
mixedのまま流していた設計上の弱点を解消し、absent / present の区別を単一の型DecodedBodyenvelope で end-to-end に保持するようにしました。変更内容
PR #247 で導入した内部マーカー enum
PresentJsonNullは、全 consumer が手動で unwrap する必要があり(unwrap idiom も2種類)、mixedのため「マーカーは必ず unwrap される」契約を PHPStan が静的に強制できませんでした。本 PR は envelope 型DecodedBodyを導入してこれを置き換えます。src/DecodedBody.php(新規)—final readonlyの公開クラス。named constructorabsent()/present(mixed)/fromLegacy(mixed)を提供。presentフラグとvalueで absent / present / literal-null を表現PresentJsonNullマーカー enum と$bodyWasPresentフラグを撤去 — body validator は!$body->presentで判定extractJsonBody/extractRequestBody、SymfonyextractSymfonyJsonBody)がDecodedBodyを生成@internalのResponseBodyValidator/RequestBodyValidatorは body 引数をDecodedBody型で受け取る — ライブラリ内 consumer は envelope を受け取らざるを得ず、unwrap 漏れが構造的に起きないOpenApiResponseValidator::validate()/OpenApiRequestValidator::validate()の body 引数はmixedのまま据え置き。入口でDecodedBody::fromLegacy()が正規化(素のnullは従来どおり absent 扱い)。これにより v1.x minor リリースとして出せます。公開シグネチャからmixedを消すのは v2 専用案件(tech-debt(validator): make OpenApiResponseValidator strictRequiredTracker ctor param required (v2) #234 と同類)として残置検証
tests/Unit/DecodedBodyTest.phpを先に作成(RED → GREEN)。RequestBodyValidatorTest/ResponseBodyValidatorTestの直接呼び出しをDecodedBody渡しに更新し、tech-debt(adapters) — JSON body decoding to literalnull/ scalar is silently treated as "no body" #246 の literal-null 挙動テストをDecodedBody::present(null)として回帰維持OpenApiResponseValidatorTest/OpenApiRequestValidatorTestは無変更 — 公開validate()のmixed受けを叩くため。後方互換の証跡composer ciグリーン(cs-check / PHPStan level 6 / 1775 tests, 4170 assertions)src/Pest/および Pest 統合テストに変更 API への参照は無く、公開validate()のmixed経由で後方互換。CI の別ジョブで実行されますレビュー時の注意点
mixed排除は SemVer 上 major bump 必須のため、本 PR では BC を維持しmixedを残しています(issue が許容した段階的移行)。@internalvalidator 層ではDecodedBodyを型で強制しており、issue ゴール「unwrap 漏れを PHPStan が検出」はライブラリ内 consumer について成立しますPresentJsonNull廃止により、follow-up issue tech-debt(test) — OpenApiResponseValidator の PresentJsonNull unwrap 経路に回帰テストが無い #249(present-literal-null 経路の回帰テスト追加)の対象はDecodedBody::present(null)経路に読み替わります関連情報
フォローアップ Issue
/pr-review-toolkit:review-prのマルチエージェントレビューで挙がった、本 PR スコープ外として切り出した項目: