test(validation): pin the DecodedBody envelope unwrap on the strict-required path#252
Merged
wadakatu merged 3 commits intoMay 18, 2026
Merged
Conversation
…equired path (#249) PR #247 added an unwrap step so the strict-required walker observes the decoded body value rather than the present-literal-null marker. #248 replaced that marker with the DecodedBody envelope, so the unwrap is now `$body->value` in OpenApiResponseValidator::maybeRecordStrictRequired(). That path had no direct regression test — the pr-test review flagged it. Add two regression tests to StrictRequiredValidatorIntegrationTest: - present_literal_null_body_records_no_strict_required_pointer: a literal JSON null body (DecodedBody::present(null)) against a nullable object schema reaches the Success path and records no pointer — documenting that the walker observes a real null, not the envelope. - decoded_body_envelope_is_unwrapped_before_strict_required_walk: a present object body passed as a DecodedBody envelope must be unwrapped before walking. Fails the moment `$body->value` becomes `$body`, since collectPointers() would receive a DecodedBody and return an empty map. Add a `/nullable-object` endpoint to the under-described fixture so the literal-null body validates successfully.
Address pr-review-toolkit feedback on the #249 regression tests: - Drop the inaccurate #246 citation: the DecodedBody::present(null) representation is #248's work, not #246 (which only added the superseded marker enum). - Reframe the literal-null test docstring as a characterization test for the Success path — it does not pin the unwrap (a leaked envelope also yields an empty pointer map), which the sibling test guards. - Clarify that a dropped unwrap skips the observation entirely rather than recording an empty one, and note the teeth test's expected keys come from the input body, not the spec schema. - Remove the inert `required: ["id"]` from the /nullable-object fixture; the literal-null body never exercises a required-key comparison.
…path Close the two coverage gaps the pr-test review flagged on the #249 regression tests: - absent_decoded_body_against_json_schema_fails_and_records_nothing: the framework adapters build the envelope directly, so the production path for a missing body is an explicit DecodedBody::absent() — not a bare null through fromLegacy(). Passing absent() against a JSON-schema endpoint must fail conformance and record nothing, mirroring the present-literal-null case (present(null) validates, absent() does not). - present_literal_null_body_on_oas31_nullable_schema_records_no_pointer: the literal-null Success path is version-specific. The existing test pins OAS 3.0 `nullable: true`; this one pins OAS 3.1 `type: ["object", "null"]` via a new under-described-3.1 fixture, so a converter regression on 3.1 type arrays surfaces here.
wadakatu
deleted the
test/strict-required-decoded-body-regression-issue-249
branch
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
1 participant
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.
Summary
OpenApiResponseValidatorの strict-required 記録経路に、DecodedBodyenvelope を unwrap してから walker へ渡す挙動を固定する回帰テストを追加します。Why
PR #247 は、strict-required walker が present-literal-null マーカーではなく decoded body の実値を観測するよう unwrap を1行追加しました。#248 でそのマーカーが
DecodedBodyenvelope に置き換わったため、現在の unwrap はOpenApiResponseValidator::maybeRecordStrictRequired()に渡す$body->valueです。この経路を直接固定するテストが無いと #247 の pr-test レビューで指摘されていました（severity 6）。Fixes #249
Verification
tests/Unit/Validation/Strict/StrictRequiredValidatorIntegrationTest.phpに回帰テストを4本追加:present_literal_null_body_records_no_strict_required_pointer— literal JSON null body（DecodedBody::present(null)）を OAS 3.0nullable: trueschema で検証し、validation success かつ strict-required observation 無記録（walker が実nullを観測）であることを assert。literal-null Success 経路の characterization テスト。present_literal_null_body_on_oas31_nullable_schema_records_no_pointer— 同じ挙動を OAS 3.1type: ["object","null"]schema で固定（OpenApiSchemaConverterは 3.0 / 3.1 を別経路で処理するため、3.1 type 配列の converter 回帰を捕捉）。absent_decoded_body_against_json_schema_fails_and_records_nothing— アダプタが直接生成するDecodedBody::absent()を JSON schema エンドポイントへ渡すと conformance 失敗し observation 無記録であることを assert（present(null)は success /absent()は failure という envelope の本質的区別を直接経路で固定）。decoded_body_envelope_is_unwrapped_before_strict_required_walk— present な object body をDecodedBodyenvelope として渡し、walker が envelope ではなく内側の配列を観測すること（/ポインタにキーが記録される）を assert。$body->valueを$bodyに変えるとcollectPointers()がDecodedBodyを受け取り空マップを返すため、このテストが落ちる ＝ Issue の「ゴール」を満たす teeth テスト。fixture追加:
under-described.jsonに OAS 3.0nullable: trueの/nullable-objectエンドポイントunder-described-3.1.json（新規）に OAS 3.1type: ["object","null"]の/nullable-objectエンドポイントゴール検証として
src/OpenApiResponseValidator.phpの$body->valueを一時的に$bodyへ書き換え、teeth テストが fail することを確認済み（変更は revert 済み）。composer testpasses（1785 tests, 4192 assertions）composer stanpasses（PHPStan level 6, No errors）composer cs-checkpassesNotes for reviewers
instanceof PresentJsonNullternary を unwrap 対象として記述していますが、tech-debt(validation) — decoded body の mixed を absent/present を表現する型に置き換える #248 でそのマーカーが廃止されたため、回帰対象をDecodedBodyenvelope の$body->value経路に読み替えています（refactor(validation): replace mixed decoded body with a DecodedBody envelope (#248) #250 のレビューでも明記済み）。nullもDecodedBodyobject もどちらも非 array として空マップを返すため）。そのためdecoded_body_envelope_is_unwrapped_before_strict_required_walkが unwrap を実際に固定する teeth テストで、literal-null 系2本は Success 経路の characterization テストです。pr-review-toolkit:review-prのマルチエージェントレビューを実施し、指摘（tech-debt(adapters) — JSON body decoding to literalnull/ scalar is silently treated as "no body" #246 引用誤り・comment 表現・inert fixture フィールド）を反映済み。さらに pr-test レビューが挙げたDecodedBody::absent()直接経路 / OAS 3.1 variant のカバレッジ gap も本 PR で対応しています。