Allow nested $ref to resolve for OpenAPI?

[cjsewell]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the issue has not already been raised

Issue

Currently, nested $refs are not resolved, $ref remains as the schema id.

If the id exists in components.schemas its should be rewritten to #/components/schemas/{id of the schema}

This can achieve this by adding modifying the loop at /lib/spec/openapi/index.js#L31

E.g search through each prop, and if a $ref is found with the same value as an existing schema, prefix it with #/components/schemas/

Object.values(openapiObject.components.schemas)
.forEach((_) => {
    delete _.$id;
    if (_.properties) {
        Object.values(_.properties).forEach((prop) => {
            if (prop['$ref'] && typeof openapiObject.components.schemas[prop['$ref']] !== 'undefined') {
                prop['$ref'] = `#/components/schemas/${prop['$ref']}`;
            }
        });
    }
});

I would like to submit this as a pull request, but I'm not too familiar with swagger, JSON schema etc so would like some feedback before I do... Or should I just submit it a go from there?

[mcollina]

Go submit! Remember to add a test!

[Eomm]

Could you share some code with a failing example as first step?

[cjsewell]

Hmm, this may be a larger #ref issue...
While prepping a simple example, my fix only works if all your schemas have correct ids and using

refResolver: {
    buildLocalReference(json, baseUri, fragment, i) {
        return json.$id;
    },
},

Because I assume the id in the ref will be the same as the property name on components.schemas

see cjsewell/fastify-swagger-nested-refs for an example of it failing.

and the branch with-fix with an example using a forked version of fastify-swagger

[Eomm]

// Swagger doesn't accept $id on /definitions schemas.

I agree on that: we should find when remove the $id because we can't mutate the user's input object.
At some point we clone it, we should find out.

We do a lot of schema navigation, so I think we could find an existing loop instead of creating a new one

Because I assume the id in the ref will be the same as the property name on components.schemas

This assumption is not true by the spec. You are not using $ref as the specification would like to (read here for more details)

The $ref resolution should be already done here:

fastify-swagger/lib/spec/swagger/utils.js

Line 167 in 8d5a3e0

const resolved = ref.resolve(schema)

so I think the openapi config is skipping some process

[cjsewell]

Yes, I see. It seems that if the schema is directly in the route ( via querystring, body, params etc ) then it is parsed correctly through resolveCommonParams in lib/spec/openapi/utils.js#290.

But schemas added via fastify.addSchema() or the config openapi.componets.schemas don't parsed in the same way?

[cjsewell]

Perhaps the could be a cleaner solution?
cjsewell@1a816ee

[briceruzand]

Hi, I have exactly the same trouble.
I push a test to reproduce the behavior.

briceruzand/fastify-swagger#462_nested_ref

[peter-mouland]

hey, I'm having the same trouble but for the swagger-api. I've found a workaround, but it's not great.

I name all the $ref $Id's to include the definition path (e.g. #/definitions/my-id) and then in the swagger options strip out the path prefix:

    refResolver: {
        buildLocalReference(json, baseUri, fragment, i) {
            if (json.$id) {
                // remove #/definitions/ from local refs
                return json.$id.split('/').splice(-1);
            }
            return `def-${i}`;
        },
    },

This feels a bit manual, and problematic to be a good solution, but it might help people out of a bind.

The solution above cjsewell@1a816ee produced duplicated definitions which were nested oddly for me.

The original solution worked, once modified to take into account oneOf, allOf and anyOf and item e.g.

function transformDefs (jsonSchema) {
    if (typeof jsonSchema === 'object' && jsonSchema !== null) {
        Object.keys(jsonSchema).forEach(function (key) {
            if (key === '$ref') {
                jsonSchema[key] = `#/definitions/${jsonSchema[key]}`
            } else if (key === 'examples' && Array.isArray(jsonSchema[key]) && (jsonSchema[key].length > 1)) {
                jsonSchema.examples = convertExamplesArrayToObject(jsonSchema.examples)
            } else if (key === 'examples' && Array.isArray(jsonSchema[key]) && (jsonSchema[key].length === 1)) {
                jsonSchema.example = jsonSchema[key][0]
                delete jsonSchema[key]
            } else {
                jsonSchema[key] = transformDefs(jsonSchema[key])
            }
        })
    }
    return jsonSchema
}
...

    // Swagger doesn't accept $id on /definitions schemas.
    // The $ids are needed by Ref() to check the URI so we need
    // to remove them at the end of the process
    Object.values(swaggerObject.definitions)
      .forEach(_ => {
          delete _.$id
          if (_.properties) transformDefs(_.properties)
      })

[peter-mouland]

I've added a PR with tests, some of which are failing at the moment. i.e. if we do not transofrm the ID's using buildLocalReference.

#478