Skip to content

Commit 0de784b

Browse files
committed
refactor(validator): address review feedback for content negotiation
Allow failure() to preserve matchedPath so coverage tracking works for matched endpoints even on validation failures. Add docblocks to isJsonContentType() and isContentTypeInSpec() with preconditions. Clarify content negotiation comment to document the intentional asymmetry between JSON and non-JSON type matching. Narrow catch(Throwable) to catch(JsonException) in extractJsonBody() to avoid masking unrelated errors.
1 parent 016d309 commit 0de784b

3 files changed

Lines changed: 23 additions & 11 deletions

File tree

src/Laravel/ValidatesOpenApiSchema.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
namespace Studio\OpenApiContractTesting\Laravel;
66

77
use Illuminate\Testing\TestResponse;
8+
use JsonException;
89
use Studio\OpenApiContractTesting\HttpMethod;
910
use Studio\OpenApiContractTesting\OpenApiCoverageTracker;
1011
use Studio\OpenApiContractTesting\OpenApiResponseValidator;
11-
use Throwable;
1212

1313
use function is_string;
1414
use function str_contains;
@@ -94,7 +94,7 @@ private function extractJsonBody(TestResponse $response, string $content, string
9494

9595
try {
9696
return $response->json();
97-
} catch (Throwable $e) {
97+
} catch (JsonException $e) {
9898
$this->fail(
9999
'Response body could not be parsed as JSON: ' . $e->getMessage()
100100
. ($contentType === '' ? ' (no Content-Type header was present on the response)' : ''),

src/OpenApiResponseValidator.php

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function validate(
4949
if (!isset($pathSpec[$lowerMethod])) {
5050
return OpenApiValidationResult::failure([
5151
"Method {$method} not defined for path {$matchedPath} in '{$specName}' spec.",
52-
]);
52+
], $matchedPath);
5353
}
5454

5555
$statusCodeStr = (string) $statusCode;
@@ -58,7 +58,7 @@ public function validate(
5858
if (!isset($responses[$statusCodeStr])) {
5959
return OpenApiValidationResult::failure([
6060
"Status code {$statusCode} not defined for {$method} {$matchedPath} in '{$specName}' spec.",
61-
]);
61+
], $matchedPath);
6262
}
6363

6464
$responseSpec = $responses[$statusCodeStr];
@@ -71,8 +71,9 @@ public function validate(
7171
/** @var array<string, array<string, mixed>> $content */
7272
$content = $responseSpec['content'];
7373

74-
// When the actual response Content-Type is provided, use it to select
75-
// the correct media type entry from the spec (content negotiation).
74+
// When the actual response Content-Type is provided, handle content negotiation:
75+
// non-JSON types are checked for spec presence only, while JSON-compatible types
76+
// fall through to schema validation against the first JSON media type in the spec.
7677
if ($responseContentType !== null) {
7778
$normalizedType = $this->normalizeMediaType($responseContentType);
7879

@@ -86,10 +87,13 @@ public function validate(
8687

8788
return OpenApiValidationResult::failure([
8889
"Response Content-Type '{$normalizedType}' is not defined for {$method} {$matchedPath} (status {$statusCode}) in '{$specName}' spec. Defined content types: {$defined}",
89-
]);
90+
], $matchedPath);
9091
}
9192

9293
// JSON-compatible response: fall through to existing JSON schema validation.
94+
// JSON types are treated as interchangeable (e.g. application/vnd.api+json
95+
// validates against an application/json spec entry) because the schema is
96+
// the same regardless of the specific JSON media type.
9397
}
9498

9599
$jsonContentType = $this->findJsonContentType($content);
@@ -108,7 +112,7 @@ public function validate(
108112
if ($responseBody === null) {
109113
return OpenApiValidationResult::failure([
110114
"Response body is empty but {$method} {$matchedPath} (status {$statusCode}) defines a JSON-compatible response schema in '{$specName}' spec.",
111-
]);
115+
], $matchedPath);
112116
}
113117

114118
/** @var array<string, mixed> $schema */
@@ -147,7 +151,7 @@ public function validate(
147151
}
148152
}
149153

150-
return OpenApiValidationResult::failure($errors);
154+
return OpenApiValidationResult::failure($errors, $matchedPath);
151155
}
152156

153157
/**
@@ -186,6 +190,10 @@ private function normalizeMediaType(string $contentType): string
186190
}
187191

188192
/**
193+
* Check whether the given (already normalised, lower-cased) response content
194+
* type matches any content type key defined in the spec. Spec keys are
195+
* lower-cased before comparison.
196+
*
189197
* @param array<string, array<string, mixed>> $content
190198
*/
191199
private function isContentTypeInSpec(string $responseContentType, array $content): bool
@@ -199,6 +207,10 @@ private function isContentTypeInSpec(string $responseContentType, array $content
199207
return false;
200208
}
201209

210+
/**
211+
* True for "application/json" or any "+json" structured syntax suffix (RFC 6838).
212+
* Expects a lower-cased media type without parameters.
213+
*/
202214
private function isJsonContentType(string $lowerContentType): bool
203215
{
204216
return $lowerContentType === 'application/json' || str_ends_with($lowerContentType, '+json');

src/OpenApiValidationResult.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ public static function success(?string $matchedPath = null): self
2323
}
2424

2525
/** @param string[] $errors */
26-
public static function failure(array $errors): self
26+
public static function failure(array $errors, ?string $matchedPath = null): self
2727
{
28-
return new self(false, $errors);
28+
return new self(false, $errors, $matchedPath);
2929
}
3030

3131
public function isValid(): bool

0 commit comments

Comments
 (0)