Skip to content

Commit eccd75d

Browse files
wadakatuwadakatu
committed
refactor(validator): address review feedback for JSON content type matching
- Replace str_contains substring match with RFC 6838 structured syntax suffix matching (=== 'application/json' || str_ends_with('+json')) - Rename findJsonSchema to findJsonContentType for clearer intent - Handle JSON content type without schema key (skip validation, matching prior behavior) instead of producing contradictory error message - Fix error message terminology: "JSON-compatible response schema" - Add PHPDoc describing the matching strategy and RFC reference - Add tests: empty body with problem+json, case-insensitive content type, schema-less JSON content type, OAS 3.1 non-JSON content type failure
1 parent f03277f commit eccd75d

4 files changed

Lines changed: 125 additions & 11 deletions

File tree

‎src/OpenApiResponseValidator.php‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
use function implode;
1414
use function json_decode;
1515
use function json_encode;
16-
use function str_contains;
16+
use function str_ends_with;
1717
use function strtolower;
1818

1919
final class OpenApiResponseValidator
@@ -67,22 +67,28 @@ public function validate(
6767

6868
/** @var array<string, array<string, mixed>> $content */
6969
$content = $responseSpec['content'];
70-
$schema = $this->findJsonSchema($content);
70+
$jsonContentType = $this->findJsonContentType($content);
7171

72-
if ($schema === null) {
73-
/** @var string[] $definedTypes */
72+
if ($jsonContentType === null) {
7473
$definedTypes = array_keys($content);
7574

7675
return OpenApiValidationResult::failure([
7776
"No JSON-compatible content type found for {$method} {$matchedPath} (status {$statusCode}) in '{$specName}' spec. Defined content types: " . implode(', ', $definedTypes),
7877
]);
7978
}
8079

80+
if (!isset($content[$jsonContentType]['schema'])) {
81+
return OpenApiValidationResult::success($matchedPath);
82+
}
83+
8184
if ($responseBody === null) {
8285
return OpenApiValidationResult::failure([
83-
"Response body is empty but {$method} {$matchedPath} (status {$statusCode}) defines a JSON schema in '{$specName}' spec.",
86+
"Response body is empty but {$method} {$matchedPath} (status {$statusCode}) defines a JSON-compatible response schema in '{$specName}' spec.",
8487
]);
8588
}
89+
90+
/** @var array<string, mixed> $schema */
91+
$schema = $content[$jsonContentType]['schema'];
8692
$jsonSchema = OpenApiSchemaConverter::convert($schema, $version);
8793

8894
// opis/json-schema requires an object, so encode then decode
@@ -121,16 +127,21 @@ public function validate(
121127
}
122128

123129
/**
124-
* @param array<string, array<string, mixed>> $content
130+
* Find the first JSON-compatible content type from the response spec.
131+
*
132+
* Matches "application/json" exactly and any type with a "+json" structured
133+
* syntax suffix (RFC 6838), such as "application/problem+json" and
134+
* "application/vnd.api+json". Matching is case-insensitive.
125135
*
126-
* @return null|array<string, mixed>
136+
* @param array<string, array<string, mixed>> $content
127137
*/
128-
private function findJsonSchema(array $content): ?array
138+
private function findJsonContentType(array $content): ?string
129139
{
130140
foreach ($content as $contentType => $mediaType) {
131-
if (str_contains(strtolower($contentType), 'json') && isset($mediaType['schema'])) {
132-
/** @var array<string, mixed> */
133-
return $mediaType['schema'];
141+
$lower = strtolower($contentType);
142+
143+
if ($lower === 'application/json' || str_ends_with($lower, '+json')) {
144+
return $contentType;
134145
}
135146
}
136147

‎tests/Unit/OpenApiResponseValidatorTest.php‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,21 @@ public function v30_problem_json_invalid_response_fails(): void
195195
$this->assertNotEmpty($result->errors());
196196
}
197197

198+
#[Test]
199+
public function v30_problem_json_empty_body_fails(): void
200+
{
201+
$result = $this->validator->validate(
202+
'petstore-3.0',
203+
'GET',
204+
'/v1/pets',
205+
400,
206+
null,
207+
);
208+
209+
$this->assertFalse($result->isValid());
210+
$this->assertStringContainsString('Response body is empty', $result->errors()[0]);
211+
}
212+
198213
#[Test]
199214
public function v30_content_without_json_compatible_type_fails(): void
200215
{
@@ -211,6 +226,40 @@ public function v30_content_without_json_compatible_type_fails(): void
211226
$this->assertStringContainsString('application/xml', $result->errors()[0]);
212227
}
213228

229+
#[Test]
230+
public function v30_case_insensitive_content_type_matches(): void
231+
{
232+
$result = $this->validator->validate(
233+
'petstore-3.0',
234+
'GET',
235+
'/v1/pets',
236+
422,
237+
[
238+
'type' => 'https://example.com/validation-error',
239+
'title' => 'Validation Error',
240+
'status' => 422,
241+
],
242+
);
243+
244+
$this->assertTrue($result->isValid());
245+
$this->assertSame('/v1/pets', $result->matchedPath());
246+
}
247+
248+
#[Test]
249+
public function v30_json_content_type_without_schema_skips_validation(): void
250+
{
251+
$result = $this->validator->validate(
252+
'petstore-3.0',
253+
'GET',
254+
'/v1/pets',
255+
500,
256+
['error' => 'something went wrong'],
257+
);
258+
259+
$this->assertTrue($result->isValid());
260+
$this->assertSame('/v1/pets', $result->matchedPath());
261+
}
262+
214263
// ========================================
215264
// OAS 3.1 tests
216265
// ========================================
@@ -274,6 +323,22 @@ public function v31_problem_json_valid_response_passes(): void
274323
$this->assertSame('/v1/pets', $result->matchedPath());
275324
}
276325

326+
#[Test]
327+
public function v31_content_without_json_compatible_type_fails(): void
328+
{
329+
$result = $this->validator->validate(
330+
'petstore-3.1',
331+
'POST',
332+
'/v1/pets',
333+
415,
334+
'<error>Unsupported</error>',
335+
);
336+
337+
$this->assertFalse($result->isValid());
338+
$this->assertStringContainsString('No JSON-compatible content type found', $result->errors()[0]);
339+
$this->assertStringContainsString('application/xml', $result->errors()[0]);
340+
}
341+
277342
#[Test]
278343
public function v31_no_content_response_passes(): void
279344
{

‎tests/fixtures/specs/petstore-3.0.json‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,34 @@
4242
}
4343
}
4444
},
45+
"422": {
46+
"description": "Validation error",
47+
"content": {
48+
"Application/Problem+JSON": {
49+
"schema": {
50+
"type": "object",
51+
"required": ["type", "title", "status"],
52+
"properties": {
53+
"type": {
54+
"type": "string"
55+
},
56+
"title": {
57+
"type": "string"
58+
},
59+
"status": {
60+
"type": "integer"
61+
}
62+
}
63+
}
64+
}
65+
}
66+
},
67+
"500": {
68+
"description": "Internal server error",
69+
"content": {
70+
"application/json": {}
71+
}
72+
},
4573
"400": {
4674
"description": "Bad request",
4775
"content": {

‎tests/fixtures/specs/petstore-3.1.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,16 @@
9999
}
100100
}
101101
}
102+
},
103+
"415": {
104+
"description": "Unsupported media type",
105+
"content": {
106+
"application/xml": {
107+
"schema": {
108+
"type": "string"
109+
}
110+
}
111+
}
102112
}
103113
}
104114
}

0 commit comments

Comments
 (0)