Skip to content

Commit 88ffb02

Browse files
committed
fix(laravel): harden auto-assert config parsing and idempotency keying
Addresses PR #54 review feedback (silent-failure-hunter): - Parse openapi-contract-testing.auto_assert with FILTER_VALIDATE_BOOLEAN so "true"/"1" strings from env() are accepted; unrecognized values now fail the test loudly instead of silently skipping validation. - Key idempotency on (spec, method, path) tuples stored in a WeakMap so explicit calls against a different spec path re-run instead of being silently no-oped. - Extract method/path from the Request Laravel passes to createTestResponse rather than calling app('request') later, removing container coupling. - Replace the broad catch(Throwable) in LaravelConfigMock with a bound() precondition so genuine framework errors propagate. - Correct the createTestResponse docblock to reference MakesHttpRequests (where the method lives) and add @param for $request.
1 parent 67351a1 commit 88ffb02

2 files changed

Lines changed: 81 additions & 32 deletions

File tree

src/Laravel/ValidatesOpenApiSchema.php

Lines changed: 67 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,36 @@
44

55
namespace Studio\OpenApiContractTesting\Laravel;
66

7+
use const FILTER_NULL_ON_FAILURE;
8+
use const FILTER_VALIDATE_BOOLEAN;
9+
710
use Illuminate\Testing\TestResponse;
811
use JsonException;
912
use Studio\OpenApiContractTesting\HttpMethod;
1013
use Studio\OpenApiContractTesting\OpenApiCoverageTracker;
1114
use Studio\OpenApiContractTesting\OpenApiResponseValidator;
1215
use Studio\OpenApiContractTesting\OpenApiSpecResolver;
16+
use Symfony\Component\HttpFoundation\Request;
1317
use Symfony\Component\HttpFoundation\Response;
1418
use WeakMap;
1519

20+
use function filter_var;
21+
use function get_debug_type;
1622
use function is_numeric;
1723
use function is_string;
24+
use function sprintf;
1825
use function str_contains;
1926
use function strtolower;
27+
use function strtoupper;
28+
use function var_export;
2029

2130
trait ValidatesOpenApiSchema
2231
{
2332
use OpenApiSpecResolver;
2433
private static ?OpenApiResponseValidator $cachedValidator = null;
2534
private static ?int $cachedMaxErrors = null;
2635

27-
/** @var null|WeakMap<TestResponse, true> */
36+
/** @var null|WeakMap<TestResponse, array<string, true>> */
2837
private static ?WeakMap $validatedResponses = null;
2938

3039
public static function resetValidatorCache(): void
@@ -35,16 +44,25 @@ public static function resetValidatorCache(): void
3544
}
3645

3746
/**
38-
* Overrides Illuminate\Foundation\Testing\TestCase::createTestResponse so
39-
* every HTTP test call runs schema validation when auto_assert is enabled.
47+
* Overrides Laravel's MakesHttpRequests::createTestResponse hook so every
48+
* HTTP test call runs schema validation when auto_assert is enabled.
4049
* When the library is used outside Laravel, this method is never called.
4150
*
51+
* Method and path are resolved from the Request passed in by Laravel
52+
* rather than from app('request'), so auto-assert stays independent of
53+
* container state and sees the exact values the framework dispatched.
54+
*
4255
* @param Response $response
56+
* @param null|Request $request
4357
*/
4458
protected function createTestResponse($response, $request = null): TestResponse
4559
{
4660
$testResponse = parent::createTestResponse($response, $request);
47-
$this->maybeAutoAssertOpenApiSchema($testResponse);
61+
62+
$method = $request !== null ? HttpMethod::tryFrom(strtoupper($request->getMethod())) : null;
63+
$path = $request?->getPathInfo();
64+
65+
$this->maybeAutoAssertOpenApiSchema($testResponse, $method, $path);
4866

4967
return $testResponse;
5068
}
@@ -54,11 +72,7 @@ protected function maybeAutoAssertOpenApiSchema(
5472
?HttpMethod $method = null,
5573
?string $path = null,
5674
): void {
57-
if (config('openapi-contract-testing.auto_assert') !== true) {
58-
return;
59-
}
60-
61-
if (self::isAlreadyValidated($response)) {
75+
if (!$this->isAutoAssertEnabled()) {
6276
return;
6377
}
6478

@@ -86,10 +100,8 @@ protected function assertResponseMatchesOpenApiSchema(
86100
?HttpMethod $method = null,
87101
?string $path = null,
88102
): void {
89-
if (self::isAlreadyValidated($response)) {
90-
return;
91-
}
92-
self::markValidated($response);
103+
$resolvedMethod = $method !== null ? $method->value : app('request')->getMethod();
104+
$resolvedPath = $path ?? app('request')->getPathInfo();
93105

94106
$specName = $this->resolveOpenApiSpec();
95107
if ($specName === '') {
@@ -101,8 +113,16 @@ protected function assertResponseMatchesOpenApiSchema(
101113
);
102114
}
103115

104-
$resolvedMethod = $method !== null ? $method->value : app('request')->getMethod();
105-
$resolvedPath = $path ?? app('request')->getPathInfo();
116+
// Idempotency key includes the spec so that validating the same
117+
// response against a different spec (or a different method/path on
118+
// the same spec) still runs — auto-assert's no-op only applies to
119+
// exact repeats.
120+
$signature = $specName . ':' . $resolvedMethod . ' ' . $resolvedPath;
121+
122+
if (self::isAlreadyValidated($response, $signature)) {
123+
return;
124+
}
125+
self::markValidated($response, $signature);
106126

107127
$content = $response->getContent();
108128
if ($content === false) {
@@ -124,6 +144,8 @@ protected function assertResponseMatchesOpenApiSchema(
124144
// Record coverage for any matched endpoint, including those where body
125145
// validation was skipped (e.g. non-JSON content types). "Covered" means
126146
// the endpoint was exercised in a test, not that its body was validated.
147+
// Note: under auto_assert, this records coverage for every Laravel HTTP
148+
// call — including responses with no explicit contract-test intent.
127149
if ($result->matchedPath() !== null) {
128150
OpenApiCoverageTracker::record(
129151
$specName,
@@ -139,16 +161,18 @@ protected function assertResponseMatchesOpenApiSchema(
139161
);
140162
}
141163

142-
private static function isAlreadyValidated(TestResponse $response): bool
164+
private static function isAlreadyValidated(TestResponse $response, string $signature): bool
143165
{
144166
return self::$validatedResponses !== null &&
145-
isset(self::$validatedResponses[$response]);
167+
isset(self::$validatedResponses[$response][$signature]);
146168
}
147169

148-
private static function markValidated(TestResponse $response): void
170+
private static function markValidated(TestResponse $response, string $signature): void
149171
{
150172
self::$validatedResponses ??= new WeakMap();
151-
self::$validatedResponses[$response] = true;
173+
$signatures = self::$validatedResponses[$response] ?? [];
174+
$signatures[$signature] = true;
175+
self::$validatedResponses[$response] = $signatures;
152176
}
153177

154178
private static function getOrCreateValidator(): OpenApiResponseValidator
@@ -164,6 +188,30 @@ private static function getOrCreateValidator(): OpenApiResponseValidator
164188
return self::$cachedValidator;
165189
}
166190

191+
private function isAutoAssertEnabled(): bool
192+
{
193+
$raw = config('openapi-contract-testing.auto_assert', false);
194+
195+
if ($raw === true) {
196+
return true;
197+
}
198+
if ($raw === false || $raw === null) {
199+
return false;
200+
}
201+
202+
$parsed = filter_var($raw, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);
203+
if ($parsed === null) {
204+
$this->fail(sprintf(
205+
'openapi-contract-testing.auto_assert must be a boolean (or a boolean-compatible value '
206+
. 'like "true"/"false"/"1"/"0"), got %s: %s.',
207+
get_debug_type($raw),
208+
var_export($raw, true),
209+
));
210+
}
211+
212+
return $parsed;
213+
}
214+
167215
/** @return null|array<string, mixed> */
168216
private function extractJsonBody(TestResponse $response, string $content, string $contentType): ?array
169217
{

tests/Helpers/LaravelConfigMock.php

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44

55
namespace Studio\OpenApiContractTesting\Laravel;
66

7-
use Throwable;
7+
use Illuminate\Container\Container;
88

99
use function array_key_exists;
10-
use function function_exists;
10+
use function class_exists;
1111

1212
/**
1313
* Namespace-level config() mock for unit testing.
@@ -29,11 +29,16 @@
2929
* Resolution order:
3030
* 1. A unit-test override in $GLOBALS['__openapi_testing_config'] wins — unit
3131
* tests set this explicitly to control what the trait sees.
32-
* 2. Otherwise, defer to the real framework helper when an app is running
33-
* (integration tests using orchestra/testbench). The global helper is
34-
* invoked via a variable function to bypass both PHP namespace resolution
35-
* (which would recurse into this mock) and cs-fixer's global_namespace_import
36-
* rule (which would strip a leading backslash and break the call).
32+
* 2. When a Laravel container is bootstrapped and has a "config" binding (i.e.
33+
* integration tests using orchestra/testbench), delegate to the framework's
34+
* config() helper via a variable function. The variable call avoids both
35+
* PHP namespace resolution (which would recurse into this mock) and any
36+
* future auto-import of the global \config() by cs-fixer's
37+
* global_namespace_import rule (which would add a "use function config;"
38+
* and re-break this mock for the same reason as a manual import).
39+
* 3. Otherwise (pure unit tests with no booted app), return the provided
40+
* default. The container-bound check avoids swallowing real binding errors:
41+
* only an unbootstrapped container falls through here.
3742
*/
3843
function config(string $key, mixed $default = null): mixed
3944
{
@@ -44,14 +49,10 @@ function config(string $key, mixed $default = null): mixed
4449
return $GLOBALS['__openapi_testing_config'][$key];
4550
}
4651

47-
if (function_exists('config')) {
52+
if (class_exists(Container::class) && Container::getInstance()->bound('config')) {
4853
$globalConfig = 'config';
4954

50-
try {
51-
return $globalConfig($key, $default);
52-
} catch (Throwable) {
53-
return $default;
54-
}
55+
return $globalConfig($key, $default);
5556
}
5657

5758
return $default;

0 commit comments

Comments
 (0)