crescat-io / saloon-sdk-generator

Generate Saloon SDKs from Postman Collections and OpenAPI Specifications.
MIT License
109 stars 17 forks source link

OpenAPI $ref not resolved #21

Open tuarrep opened 6 months ago

tuarrep commented 6 months ago

Issue

When the OpenAPI schema contains referenced parameters, they are not resolved and not included in the generated SDK.

Here is a test that fails to reproduce this (based on https://github.com/crescat-io/saloon-sdk-generator/blob/master/tests/Samples/spotify.yml) :

test('Resolved references in parameters', function () {
    $specFile = sample_path('spotify.yml');
    $parser = OpenApiParser::build($specFile);
    $spec = $parser->parse();

    expect($spec->endpoints)->not()->toBeNull()
        ->and($spec->endpoints[0]->pathParameters)->toHaveCount(1)
        ->and($spec->endpoints[0]->queryParameters)->toHaveCount(1);
});

For reference, here is the expected SDK (based on the source schema): CleanShot 2024-03-17 at 00 58 35

Resolution & discussion

At the first attempt, I tried: https://github.com/crescat-io/saloon-sdk-generator/blob/master/src/Parsers/OpenApiParser.php

    public static function build($content): self
    {
        return new self(
            Str::endsWith($content, '.json')
-              ? Reader::readFromJsonFile(fileName: realpath($content), resolveReferences: ReferenceContext::RESOLVE_MODE_INLINE)
-              : Reader::readFromYamlFile(fileName: realpath($content), resolveReferences: ReferenceContext::RESOLVE_MODE_INLINE)
+              ? Reader::readFromJsonFile(fileName: realpath($content), resolveReferences: ReferenceContext::RESOLVE_MODE_ALL)
+              : Reader::readFromYamlFile(fileName: realpath($content), resolveReferences: ReferenceContext::RESOLVE_MODE_ALL)
        );
    }

This amply slows down generation. On my machine, the SmokeTest test takes about 12 seconds before the change and times out after (60 seconds)

I also found this commit https://github.com/crescat-io/saloon-sdk-generator/commit/bd9d3252962dd4564b558915fd6b1217bf5d5d25 which changes the resolution mode from RESOLVE_MODE_ALL to RESOLVE_MODE_INLINE without explaining why but surely on purpose.

My second thought was to resolve references in the mapPrams method by overriding context at that time to change its mode:

if(is_a($parameter->schema, Reference::class)) {
    $context = $parameter->schema->getContext();
    $context->mode = 'ReferenceContext::RESOLVE_MODE_ALL';

    $parameter->schema = $parameter->schema->resolve($context);
}

But according to the OpenAPI specification, references can be used at various locations, for example responses. I know responses are not yet parsed, but I think it should be taken in account for the future.


As I think there are choices to make, I preferred to submit an issue first. I'll submit a PR when this discussion converges to a solution.