withCache() is defeated for string-loaded specs: assertConfinement() re-parses the spec on every build()

[shadowhand]

Summary

withCache() stops the spec being parsed on every build() — but only for the copy that becomes the OpenApiDocument. For specs loaded with fromYamlString() / fromJsonString(), assertConfinement() parses the same text a second time on every build(), and that parse never consults the cache. With a YAML spec the cache therefore recovers only about half of what it looks like it should.

Mechanism

build() runs the loader, then the confinement guard:

// src/Builder/OpenApiValidatorBuilder.php:464
public function build(): OpenApiValidatorInterface
{
    $document = $this->specLoader()->load();      // cache hit: no parse

    $this->externalRefDetector()->assertConfinement();   // parses the raw text again

    return $this->assembleValidator($document);
}

SpecLoader::loadFromString() returns the cached OpenApiDocument without parsing, as intended. ExternalRefDetector::assertConfinement() then goes back to the original string:

// src/Builder/Internal/ExternalRefDetector.php:58
$parsedSpec = $this->parseSpecContentAsArray($this->config->specContent);

which for a YAML spec is a full Yaml::parse() of the whole document (parseYamlContentAsArray(), line 181), followed by a recursive walk in detect().

Specs loaded from a file are unaffected: fromYamlFile() / fromJsonFile() set externalRefAllowedRoot: dirname($realPath), so assertConfinement() returns at line 50 before parsing anything. The double parse is specific to the string loaders — which are also the only ones whose cache key is portable across machines, so it hits exactly the setup that caches the document to be shipped with a deploy.

Impact

Median ms per build(), 10 iterations alternating between configurations in one process, same 207 KB / 7000-line spec, cache pre-populated and verified hitting:

spec handed to the builder	withCache()	ms per build()
fromYamlString()	no	132.2
fromYamlString()	yes	63.6
fromJsonString()	no	15.2
fromJsonString()	yes	8.7

The cached YAML build spends ~55 of its ~64 ms in the confinement re-parse. Cached JSON is cheap only because json_decode() is fast — it pays the second parse too.

Two things confirm the parse is the cause rather than a cache miss: instrumenting SpecLoader shows hit=yes on every build in both modes, with load() costing ~7 ms; and the cached entries for the two modes are byte-identical (1,925,369 bytes), so the documents downstream of the cache are the same. Calling withExternalRefAllowedRoot() on a string-loaded spec — which short-circuits assertConfinement() — also removes the 55 ms, at the cost of the guard.

Reproduction

$yaml = file_get_contents('openapi.yml');
$cache = new SchemaCache(new FilesystemAdapter(directory: '/tmp/spec-cache'), 3153600000);

$build = fn () => OpenApiValidatorBuilder::create()
    ->fromYamlString($yaml)
    ->withCache($cache)
    ->build();

$build();                                   // populate

$t = microtime(true);
for ($i = 0; $i < 10; $i++) { $build(); }   // every one re-parses $yaml
printf("%.1f ms/build\n", (microtime(true) - $t) / 10 * 1000);

Possible directions

Detecting external $refs from the OpenApiDocument the loader already returned would remove the second parse outright, and would also let the guard cover file-loaded specs. Failing that, SpecLoader could hand the array it parsed to the detector on a miss, and the confinement verdict could be cached under the same key as the document — it is derived from the same bytes, so it is valid for exactly as long.

Happy to open a PR if you have a preference between those.

Environment

• duyler/openapi at dd09fb3 (main); OpenApiValidatorBuilder.php, ExternalRefDetector.php and SpecLoader.php are byte-identical to that commit in the build I measured
• PHP 8.5.9, symfony/cache FilesystemAdapter

[milinsky]

@shadowhand Thanks for your issue. I will soon release a large number of fixes and improvements from the dev branch and return to your pull requests.

[shadowhand]

@milinsky any update on your release?

[milinsky]

@shadowhand Hello! I plan to do this tomorrow. I don’t know how much I’ll manage. There are a lot of changes. Some of them are from your pull requests.