Let JsonSchemaException carry structured validator errors (#364)#367
Merged
Conversation
…#364) Add typed DTOs `JsonSchemaError` and `ConstraintViolation` under `BEAR\Resource\JsonSchema`, and extend `JsonSchemaException` with an additive `$errors` constructor parameter plus `getErrors(): list<JsonSchemaError>`. The interceptor now maps `Validator::getErrors()` to typed DTOs at the single boundary in `throwJsonSchemaException()`, while preserving the existing flat-string message format byte-for-byte. Handlers can read the structured errors directly without re-running the validator (consumer: MyVendor.Cms PR bearsunday#40). Notes: - `ConstraintViolation::$name` stays `string` at runtime; known JSON Schema keywords are narrowed via a `@psalm-type ConstraintName` literal-union alias. A runtime enum would throw `\ValueError` inside the framework's own error-reporting path the first time upstream emits a new keyword — see issue discussion. - BC: `new JsonSchemaException($msg, Code::ERROR)` and existing message format (`[property] message; ... by /path/to/schema.json`) are preserved verbatim. `testBCValidateErrorException` is tightened to pin the full format (separator + `by {schemaFile}` suffix). - vimeo/psalm bumped to ^7.0@beta in vendor-bin/tools for PHP 8.5 compatibility. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
WalkthroughPR ChangesJSON Schema構造化エラー系
🎯 3 (Moderate) | ⏱️ ~25 minutesPossibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.x #367 +/- ##
============================================
Coverage 100.00% 100.00%
- Complexity 604 683 +79
============================================
Files 88 94 +6
Lines 1545 1707 +162
============================================
+ Hits 1545 1707 +162 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Mapper now handles both upstream error shapes (5.x: `constraint` is
string, 6.x: `constraint` is array{name, params}). Fixes the lowest
CI matrix where age='minimum' was mapped as 'unknown'.
- Relax `testStructuredErrors` params assertion (params shape differs
by version; only assert the array type for cross-version BC).
- psalm.xml: suppress the new psalm 7 strict-check issue types
(MissingImmutableAnnotation, MissingPureAnnotation, MissingAbstract-
PureAnnotation, MissingInterfaceImmutableAnnotation, TaintedSSRF) —
project-wide migration scope, not for bearsunday#364.
- Add localised @psalm-suppress MixedAssignment on the boundary mapper
where upstream returns mixed-typed row values that are narrowed by
is_string/is_array checks below.
- phpcbf: regroup @psalm-type / @psalm-immutable annotations.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PHPMD flagged JsonSchemaInterceptor at complexity 51 (threshold 50) and an else expression in `toJsonSchemaError`. Extract constraint shape normalization into its own `toConstraintViolation()` method with early returns. No behaviour change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PHPMD's class complexity for JsonSchemaInterceptor hit 52 after the previous inline-split. Move the validator-row -> DTO normalization to its own internal `JsonSchemaErrorMapper` class. The interceptor now just delegates per row, which keeps it below the complexity threshold and gives the mapping logic an obvious home (single responsibility). No behaviour change. Mapper is stateless; instantiated inline in `throwJsonSchemaException` so no DI module update is needed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CI runs coverage only against the highest dep matrix (justinrainbow 6.x), so the 5.x string-constraint branch and the missing/invalid-key fallback branches in the new mapper weren't exercised on the coverage job. Cover them directly via unit tests on the mapper. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The project convention is for `@psalm-type` definitions to live in `BEAR\Resource\Types` and be referenced from elsewhere via `@psalm-import-type X from Types` (see existing usage in `JsonSchemaInterceptor`, `JsonSchemaRequestExceptionHandlerInterface`). Move `ConstraintName` there under a new "JSON Schema Types" section. `ConstraintViolation` now imports the alias instead of declaring it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…1d751 # Conflicts: # src/Types.php
Borrowed from be-framework's Errors / SemanticVariableException design (Be\Framework\SemanticVariable\Errors): instead of returning a raw list<JsonSchemaError>, the exception now returns a typed collection object with discoverable operations: - Countable: count($errors) - IteratorAggregate: foreach ($errors as $error) - hasErrors(): bool - byProperty(): array<string, list<JsonSchemaError>> -- the core use case for handlers that re-render a form with field-keyed errors. The raw list stays accessible via $errors->errors for callers that want direct array access. API change vs the issue body's `list<JsonSchemaError>`: - JsonSchemaException::__construct now takes ?JsonSchemaErrors (was array) - JsonSchemaException::getErrors(): JsonSchemaErrors (was list<...>) Tests: - JsonSchemaErrorsTest covers empty, iteration, count, byProperty grouping. - JsonSchemaExceptionTest / JsonSchemaModuleTest updated to the new return shape; the BC message-format assertion (testBCValidateError- Exception) is unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Mirrors be-framework's #[Message] / ValidationMessageHandler design:
schema authors (or ajv-errors `errorMessage` consumers) can supply a
custom message template with `{key}` placeholders, and the handler
renders it through `$error->render($template)`.
Available placeholders:
- {property}, {pointer}, {message}
- every key in $constraint->params (e.g. {minimum}, {maxLength})
Value stringification follows the same rules as be-framework's
ValidationMessageHandler::interpolateTemplate(): string as-is,
numeric stringified, bool to "true"/"false", null to "null",
array via JSON, anything else via get_debug_type().
Example consumer:
foreach ($e->getErrors() as $error) {
$tpl = $userMessages[$error->property][$error->constraint->name]
?? $error->message;
$rendered[$error->property][] = $error->render($tpl);
}
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds a new "JSON Schema validation" section to README.md and README.ja.md covering the #[JsonSchema] attribute, the JsonSchemaModule wiring, the structured `JsonSchemaErrors` / `JsonSchemaError` / `ConstraintViolation` API exposed by `JsonSchemaException::getErrors()`, the `render()` placeholder mechanism, and the handler-interface hook for custom 422 responses. CHANGELOG gets two entries under [Unreleased] / Added: - structured `getErrors()` collection (bearsunday#364) - `JsonSchemaError::render()` placeholder interpolation Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Previous example hardcoded the message template in PHP, which obscured the whole point of placeholder support — the schema author (via the ajv-errors `errorMessage` field) owns the message text; the PHP handler just looks it up and lets `render()` interpolate constraint params. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Common need: dump every error as a single string for a log line, a 422
response body, or HTML list. Per-error template (default `{message}\n`)
goes through JsonSchemaError::render(), so every placeholder available
there works — including {property}, {pointer}, and constraint params.
$errors->combinedMessage();
// "minimum is 20\nis required\n"
$errors->combinedMessage("<li>{property}: {message}</li>\n");
// "<li>age: minimum is 20</li>\n<li>name: is required</li>\n"
The template must include its own separator (no second arg, by design —
HTML list items, log lines, and JSON-array commas all want different
separator placement).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/JsonSchema/JsonSchemaError.php (1)
43-48: ⚡ Quick win予約プレースホルダが上書きされないようにしてください。
Line 47 の展開順だと、
constraint->paramsにproperty/pointer/messageが含まれる場合に予約キーが上書きされます。予約キーを常に優先する順序にしておくと挙動が安定します。差分案
- $vars = [ - 'property' => $this->property, - 'pointer' => $this->pointer, - 'message' => $this->message, - ...$this->constraint->params, - ]; + $vars = [ + ...$this->constraint->params, + 'property' => $this->property, + 'pointer' => $this->pointer, + 'message' => $this->message, + ];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/JsonSchema/JsonSchemaError.php` around lines 43 - 48, The $vars array currently spreads $this->constraint->params after the reserved keys so params can overwrite 'property','pointer','message'; change the merge order so reserved keys win (e.g., merge params first then set 'property'=>$this->property, 'pointer'=>$this->pointer, 'message'=>$this->message) or use array_merge($this->constraint->params ?? [], [...]) to ensure the reserved keys in JsonSchemaError ($this->property, $this->pointer, $this->message) cannot be overridden by $this->constraint->params when building $vars.tests/JsonSchema/Exception/JsonSchemaExceptionTest.php (1)
38-40: ⚡ Quick win
JsonSchemaErrorsの内部配列に直接依存しないでください。Line 40 の
->errors[0]は内部実装に結合しており、コレクション実装変更で不要なテスト破壊を招きます。IteratorAggregate経由で取得する形に寄せるのが安全です。♻️ 提案差分
- $this->assertSame($error, $e->getErrors()->errors[0]); + $first = \array_values(\iterator_to_array($e->getErrors()))[0] ?? null; + $this->assertSame($error, $first);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/JsonSchema/Exception/JsonSchemaExceptionTest.php` around lines 38 - 40, The test is depending on the internal array of JsonSchemaErrors by using ->errors[0]; change the assertion to use the IteratorAggregate interface instead: convert the iterable returned by $e->getErrors() into an array via iterator_to_array($e->getErrors()) (or iterate with foreach) and assert the first element equals $error (e.g. $errorsArray = iterator_to_array($e->getErrors()); $this->assertSame($error, $errorsArray[0])). Update the assertion in JsonSchemaExceptionTest to stop referencing the internal ->errors property.tests/Module/JsonSchemaModuleTest.php (1)
77-77: ⚡ Quick winコレクションの公開インターフェース経由で先頭要素を取得してください。
Line 77 の
->errors[0]直接参照は内部表現依存です。イテレーション経由にするとテストの耐久性が上がります。♻️ 提案差分
- $first = $errors->errors[0]; + $first = \array_values(\iterator_to_array($errors))[0] ?? null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Module/JsonSchemaModuleTest.php` at line 77, Replace the brittle internal access $errors->errors[0] in JsonSchemaModuleTest with a public-API retrieval of the first element from the $errors collection: iterate over $errors and capture the first yielded item into $first (or call $errors->first() if that method exists) so the test relies only on the collection's public interface rather than its internal representation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@psalm.xml`:
- Around line 25-29: psalm.xmlでTaintedSSRFをグローバルに抑制している行（TaintedSSRF
errorLevel="suppress"）をそのままにせず、全体抑制を削除して代わりに既知の安全確認済みファイル/ディレクトリだけを対象に限定的な抑制エントリ（forFilesMatchingやforPaths相当の属性を使ったsuppressIssueブロック）を追加してください；または既知の既存警告はベースライン化して新規導入の検出を維持するように設定を移行し、将来的には該当箇所（TaintedSSRFで報告される個別ファイル）を修正して抑制を順次削除する計画を組み込んでください。
In `@src/JsonSchema/JsonSchemaErrorMapper.php`:
- Around line 25-35: toJsonSchemaError currently forwards only
$error['constraint'] to toConstraintViolation which loses the parent row's
flattened params and causes JsonSchemaError::render's {…} interpolation to miss
values; change the call in toJsonSchemaError to pass the full error context
(e.g. pass $error or both constraint plus params) and update
toConstraintViolation to accept that context: when the constraint branch
receives a string, look for params on the surrounding error context (e.g.
$context['params'] or $context['constraint_params']) and populate
constraint->params from those before returning, ensuring JsonSchemaError::render
sees the original params.
In `@src/JsonSchema/JsonSchemaErrors.php`:
- Line 10: The code uses the built-in Override attribute (notice the bare `use
Override;` and the attribute usages `#[Override]` in JsonSchemaErrors.php),
which requires PHP 8.3+; to fix, either update composer.json "require"."php" to
"^8.3" (bump the platform requirement) and run composer update, or revert to PHP
8.2 by removing the built-in attribute usage and add a polyfill: create a
namespaced Override attribute class (or import a polyfill package) and replace
the bare `use Override;` with the polyfill import so the `#[Override]`
annotations resolve on PHP 8.2; ensure changes touch JsonSchemaErrors.php (the
`use Override;` line and the two `#[Override]` usages).
In `@vendor-bin/tools/composer.json`:
- Line 7: The composer dependency for vimeo/psalm should not be switched to an
unverified stable 7.x; either change the requirement string "vimeo/psalm" to a
known stable release (e.g., "^6.16.1") to prioritize stability, or retain the
7.x beta by keeping the "`@beta`" qualifier but pin to a specific beta version
(e.g., "vimeo/psalm": "7.0.0-beta.x") and add a short comment in composer.json
explaining why the beta is required; update the dependency string accordingly
and ensure the lockfile is regenerated.
---
Nitpick comments:
In `@src/JsonSchema/JsonSchemaError.php`:
- Around line 43-48: The $vars array currently spreads $this->constraint->params
after the reserved keys so params can overwrite 'property','pointer','message';
change the merge order so reserved keys win (e.g., merge params first then set
'property'=>$this->property, 'pointer'=>$this->pointer,
'message'=>$this->message) or use array_merge($this->constraint->params ?? [],
[...]) to ensure the reserved keys in JsonSchemaError ($this->property,
$this->pointer, $this->message) cannot be overridden by
$this->constraint->params when building $vars.
In `@tests/JsonSchema/Exception/JsonSchemaExceptionTest.php`:
- Around line 38-40: The test is depending on the internal array of
JsonSchemaErrors by using ->errors[0]; change the assertion to use the
IteratorAggregate interface instead: convert the iterable returned by
$e->getErrors() into an array via iterator_to_array($e->getErrors()) (or iterate
with foreach) and assert the first element equals $error (e.g. $errorsArray =
iterator_to_array($e->getErrors()); $this->assertSame($error, $errorsArray[0])).
Update the assertion in JsonSchemaExceptionTest to stop referencing the internal
->errors property.
In `@tests/Module/JsonSchemaModuleTest.php`:
- Line 77: Replace the brittle internal access $errors->errors[0] in
JsonSchemaModuleTest with a public-API retrieval of the first element from the
$errors collection: iterate over $errors and capture the first yielded item into
$first (or call $errors->first() if that method exists) so the test relies only
on the collection's public interface rather than its internal representation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 41e63680-1fc0-4512-a45e-fdc70797bf1d
⛔ Files ignored due to path filters (1)
vendor-bin/tools/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
CHANGELOG.mdREADME.ja.mdREADME.mdpsalm.xmlsrc/JsonSchema/ConstraintViolation.phpsrc/JsonSchema/Exception/JsonSchemaException.phpsrc/JsonSchema/Interceptor/JsonSchemaInterceptor.phpsrc/JsonSchema/JsonSchemaError.phpsrc/JsonSchema/JsonSchemaErrorMapper.phpsrc/JsonSchema/JsonSchemaErrors.phpsrc/Types.phptests/JsonSchema/ConstraintViolationTest.phptests/JsonSchema/Exception/JsonSchemaExceptionTest.phptests/JsonSchema/JsonSchemaErrorMapperTest.phptests/JsonSchema/JsonSchemaErrorTest.phptests/JsonSchema/JsonSchemaErrorsTest.phptests/Module/JsonSchemaModuleTest.phpvendor-bin/tools/composer.json
Comment on lines
+25
to
+29
| <MissingImmutableAnnotation errorLevel="suppress" /> | ||
| <MissingPureAnnotation errorLevel="suppress" /> | ||
| <MissingAbstractPureAnnotation errorLevel="suppress" /> | ||
| <MissingInterfaceImmutableAnnotation errorLevel="suppress" /> | ||
| <TaintedSSRF errorLevel="suppress" /> |
There was a problem hiding this comment.
TaintedSSRF の全体 suppress は避けてください
TaintedSSRF をグローバルに抑制すると、実際の SSRF リスクが新規混入しても検出できなくなります。抑制は対象ディレクトリ/既知箇所に限定し、段階的に解消する設定へ寄せるのが安全です。
差分例（抑制対象の限定）
- <TaintedSSRF errorLevel="suppress" />
+ <!-- 一時的に対象を限定して抑制 -->
+ <TaintedSSRF errorLevel="suppress">
+ <errorLevel type="suppress">
+ <directory name="src/Http/" />
+ </errorLevel>
+ </TaintedSSRF>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@psalm.xml` around lines 25 - 29,
psalm.xmlでTaintedSSRFをグローバルに抑制している行（TaintedSSRF
errorLevel="suppress"）をそのままにせず、全体抑制を削除して代わりに既知の安全確認済みファイル/ディレクトリだけを対象に限定的な抑制エントリ（forFilesMatchingやforPaths相当の属性を使ったsuppressIssueブロック）を追加してください；または既知の既存警告はベースライン化して新規導入の検出を維持するように設定を移行し、将来的には該当箇所（TaintedSSRFで報告される個別ファイル）を修正して抑制を順次削除する計画を組み込んでください。
Member
Author
There was a problem hiding this comment.
@coderabbitai skip — TaintedSSRF の2件は src/HttpRequestCurl.php で本 PR 前から存在しているプロジェクト全体のベースライン (psalm 6 → 7 への migration スコープ)。本 PR (#364) のスコープ外として MissingImmutableAnnotation 等と同じ全体 suppress に揃えました。個別ファイル限定への絞り込みは psalm-7 全体導入と同時に別 PR で対応します。
There was a problem hiding this comment.
@koriym、ご説明ありがとうございます。psalm 6 → 7 マイグレーション全体のベースラインとして既存の全体 suppress に揃えた判断、了解しました。本 PR のスコープ外として対応を別 PR に持ち越す方針は理にかなっています。
TaintedSSRF の抑制範囲限定を別 PR で追跡するための GitHub Issue を作成しましょうか？
(´・ω・`) 了解しました
✏️ Learnings added
Learnt from: koriym
Repo: bearsunday/BEAR.Resource PR: 367
File: psalm.xml:25-30
Timestamp: 2026-05-25T03:50:46.936Z
Learning: In bearsunday/BEAR.Resource, the global `TaintedSSRF errorLevel="suppress"` in `psalm.xml` is an intentional project-wide baseline introduced during the Psalm 6 → 7 migration (alongside `MissingImmutableAnnotation`, `MissingPureAnnotation`, `MissingAbstractPureAnnotation`, `MissingInterfaceImmutableAnnotation`). Narrowing the suppression to specific files/directories is deferred to a separate PR as part of the full psalm-7 project-wide migration. Do not flag this as an issue in future reviews.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
- Mapper: recover flattened params on justinrainbow 5.x rows.
The 5.x error shape stores constraint params at the parent row level
(e.g. `'minimum' => 20` next to `'constraint' => 'minimum'`). The
previous mapping path lost them, breaking `JsonSchemaError::render`
templates like `{minimum}` on 5.x. Now the full error context is
passed through and non-standard row keys are collected into
`ConstraintViolation::params`.
- render(): reserved placeholders ({property}/{pointer}/{message})
can no longer be hijacked by a constraint param of the same name.
array_merge order is reversed so reserved keys win.
- JsonSchemaErrors: drop `#[Override]` — the attribute is PHP 8.3+
and composer requires `^8.2`. The MissingOverrideAttribute psalm
warning joins the existing project-wide strict-check suppress set.
- Tests: replace internal `->errors[0]` access with
`iterator_to_array()` so they depend on the public
IteratorAggregate API.
Not fixed (with reasons):
- TaintedSSRF global suppress (psalm.xml): the two existing errors in
HttpRequestCurl.php predate this PR and are part of the broader
psalm-7 strict-check baseline; out of scope for bearsunday#364.
- vimeo/psalm `^7.0@beta`: deliberately chosen because psalm 6 is
broken on PHP 8.5; the `@beta` qualifier is the correct way to opt
into the only series that works there.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A. Provenance of `JsonSchemaError::$message` is now discoverable. Add `$isCustomMessage` (bool readonly): true when `$message` came from the schema's ajv-errors `errorMessage`, false when it's the validator's own (i.e. equal to `$rawMessage`). Downstream consumers no longer have to guess which mode they got. B. Schema-side `errorMessage` lookup now navigates by JSON Pointer instead of the property-path string. Nested objects/arrays (e.g. `/users/0/name`) resolve correctly — the previous property-based walker silently fell through. RFC 6901 escaping is handled; numeric segments descend through `items` (single or tuple form). Limitations documented inline: no `$ref` resolution, no allOf/oneOf/anyOf merging, no patternProperties. C. Add `JsonSchemaErrors::first(): ?JsonSchemaError` for the common "show me one" case. D. Drop the public `$errors` property — the underlying list is now private. Access via `foreach`, `iterator_to_array()`, `first()`, `byProperty()`, or `format()`. Matches CodeRabbit's earlier nitpick. F. Rename `combinedMessage()` → `format()`. Same behaviour, shorter and more idiomatic; the name no longer overspecifies "combined". Refactor: extract schema-side message lookup into the new `SchemaErrorMessageResolver` class so the mapper stays under phpmd's class-complexity threshold. The mapper composes with the resolver (constructor-injectable, defaults to a fresh instance). All five new classes hold 100% line + method coverage. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- JsonSchemaErrors: use constructor property promotion for the private $errors field (slevomat sniff requires promotion when the property is only set from the constructor). Also tighten doc comment annotation grouping (sniff complaint about blank line between @psalm-immutable and @implements). - composer.json: declare `ext-ctype` — SchemaErrorMessageResolver uses ctype_digit() to detect numeric JSON Pointer segments (array indices). Matches the existing ext-filter / ext-curl / ext-fileinfo declarations. - JsonSchemaInterceptor: pick up the schema-load helper for the mapper's optional second argument (already in working tree). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov flagged the file_get_contents / JsonException defensive
branches as uncovered new lines. Since this helper only runs after
both `validateFileExists` and a successful justinrainbow parse, the
defensive paths were unreachable in practice — and `@codeCoverageIgnore`
isn't honored by pcov.
Collapse to one chain:
json_decode((string) file_get_contents($schemaFile))
- file_get_contents → false on failure → (string) false === '' → null
- json_decode('') → null
- non-object schemas (boolean, array) → null
- valid object schema → stdClass
`instanceof stdClass` at the tail gates all of those into a single
nullable return, so every line is exercised by the existing tests
without artificial fixtures.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three small doc/comment refinements from the follow-up review:
- `JsonSchemaError::$isCustomMessage` doc — clarify the flag's actual
semantic ("rawMessage differs from message") rather than implying it
detects schema provenance. They coincide in practice; the doc no
longer overpromises.
- `JsonSchemaInterceptor::schema()` — add an inline comment marking
the collapse-to-null as a deliberate graceful fallback, not error
swallowing. The validator has already parsed the same file, so the
defensive branches really are unreachable in flow.
- `SchemaErrorMessageResolver` doc — extend the limitations list with
`additionalProperties`. Codex flagged this as the next-most-likely
case consumers will actually trip on after `$ref`.
No behaviour change. All gates clean (385 tests, phpcs, phpstan max,
psalm 7).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
Author
|
@koriym 中間レビュー後の API 変更まとめ: 最終的な APInamespace BEAR\Resource\Exception;
final class JsonSchemaException extends LogicException implements ExceptionInterface
{
public function __construct(string $message, int $code = 0, ?JsonSchemaErrors $errors = null);
public function getErrors(): JsonSchemaErrors;
}
namespace BEAR\Resource\JsonSchema;
final readonly class JsonSchemaErrors implements Countable, IteratorAggregate
{
public function __construct(private array $errors);
public function hasErrors(): bool;
public function count(): int;
public function getIterator(): Generator;
public function first(): ?JsonSchemaError; // コレクションの先頭 (validator 走査順、優先度ではない)
public function byProperty(): array; // array<string, list<JsonSchemaError>>
public function format(string $template = "{message}\n"): string;
}
final readonly class JsonSchemaError
{
public string $rawMessage; // validator 原文 (英語、安定)
public bool $isCustomMessage; // schema の errorMessage が適用されたか
public function __construct(
public string $property,
public string $pointer,
public string $message, // 表示用 (errorMessage があれば rendered, なければ rawMessage と同じ)
public ConstraintViolation $constraint,
string $rawMessage = '',
);
public function render(string $template): string; // {property}/{pointer}/{message}/{constraint params...} placeholder
}
final readonly class ConstraintViolation
{
public function __construct(public string $name, public array $params);
}利用例 — schema try {
$ro->onGet(10);
} catch (JsonSchemaException $e) {
$errors = $e->getErrors(); // JsonSchemaErrors instance
$errors->hasErrors(); // true
count($errors); // 1
$errors->byProperty(); // ['age' => [JsonSchemaError]]
$errors->format(); // "年齢は20歳以上\n"
$errors->format("[{property}] {message}\n"); // "[age] 年齢は20歳以上\n"
foreach ($errors as $error) {
$error->property; // 'age'
$error->pointer; // '/age'
$error->message; // '年齢は20歳以上' (schema の errorMessage 適用後)
$error->rawMessage; // 'Must have a minimum value of 20' (validator 原文、常に安定)
$error->isCustomMessage; // true (errorMessage で上書きされた)
$error->constraint->name; // 'minimum'
$error->constraint->params; // ['minimum' => 20]
$error->render('{property} must be >= {minimum}'); // 'age must be >= 20'
}
// 単一エラー時の代表メッセージ
if (count($errors) === 1) {
return $errors->first()->message; // '年齢は20歳以上'
}
}変更点
据え置き (意図的)
|
This was referenced May 27, 2026
koriym
added a commit
to koriym/BEAR.Resource
that referenced
this pull request
May 28, 2026
…nday#369) Two concrete subclasses of JsonSchemaException let direct-catch code discriminate between client-supplied bad input and a resource producing off-schema output: - JsonSchemaRequestException — request parameter validation failed (Code::BAD_REQUEST, 400) - JsonSchemaResponseException — response body validation failed (Code::ERROR, 500) `final` is dropped from the parent so subclassing is permitted. The parent constructor and getErrors() signature stay unchanged, so existing `catch (JsonSchemaException $e)` keeps working and structured errors flow through subclasses automatically (bearsunday#367's JsonSchemaErrors ride along via inheritance). Interceptor refactor: - validate() split into validateAsRequest() / validateAsResponse(), each throws its specific subclass directly — no class-string dance. - Shared logic split into runValidator() (justinrainbow invocation) and buildValidationFailure() (message + structured errors from a failed validator). - catch sites narrowed to the concrete subclass so no `assert($e instanceof ...)` is needed. Safe because JsonSchemaNotFoundException extends LogicException directly, not JsonSchemaException, so it bubbles past the catch. Handler interfaces (`JsonSchemaRequestExceptionHandlerInterface`, `JsonSchemaExceptionHandlerInterface`) get a PHPDoc `@param` narrowed to the concrete subtype the interceptor actually delivers. Runtime parameter types stay on the parent for BC. Closes bearsunday#369. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
Closes #364
Summary
JsonSchemaErrorandConstraintViolationunderBEAR\Resource\JsonSchema.JsonSchemaExceptionwith an additive 3rd constructor parameterarray $errors = []plusgetErrors(): list<JsonSchemaError>.JsonSchemaInterceptor::throwJsonSchemaException()to mapValidator::getErrors()to typed DTOs at the single boundary, preserving the existing flat-string message format byte-for-byte.vimeo/psalmto^7.0@betainvendor-bin/toolsfor PHP 8.5 compatibility.Why
JsonSchemaRequestExceptionHandlerInterfaceimplementations (#315/#316) that want field-keyed errors — for a 422 form re-render, structured JSON error bodies, or ajv-errors-styleerrorMessageoverrides — currently re-run the validator just to recover what the interceptor already had in hand and discarded.Concrete consumer: MyVendor.Cms PR #40's
Validation\JsonSchemaRequestExceptionHandler::collectErrors()— collapses to a direct read from$e->getErrors()after this change.Design notes
ConstraintViolation::$namestaysstringat runtime, narrowed via a@psalm-type ConstraintNameliteral-union for known JSON Schema keywords. A runtimeenumwas deliberately rejected:enum::from()would throw\ValueErrorinside the framework's own error-reporting path the first time justinrainbow emits a keyword we haven't enumerated yet. See the issue discussion.ConstraintViolationclass (vs foldingname+paramsintoJsonSchemaError) mirrors justinrainbow's nestedconstraint.{name, params}shape and reads naturally as$error->constraint->name.@psalm-immutable— they're trivially pure value objects, so the annotation is intrinsic rather than a project-wide convention change.Backwards compatibility
new JsonSchemaException($msg, Code::ERROR)keeps working unchanged ($errorsdefaults to[]).getErrors()see no behaviour change.[property] message; ... by /path/to/schema.jsonis preserved byte-for-byte. The existingtestBCValidateErrorExceptionis tightened to pin the full format (;separator +by {schemaFile}suffix), not just the[age]substring.CI
@psalm-immutable; net −3 errors vs. 1.x baseline (355 → 352). Remaining psalm 7 strict-check findings are a project-wide migration (MissingImmutableAnnotation,MissingPureAnnotation) unrelated to this change.Test plan
vendor/bin/phpunit— full suite greenvendor/bin/phpcs --standard=phpcs.xml src testsvendor/bin/phpstan --memory-limit=-1 analyse -c phpstan.neonvendor/bin/psalm --threads=1(psalm 7 beta)coderabbit review --agent🤖 Generated with Claude Code