Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Self referencing schemas results in SystemStackError #291

Open
moberegger opened this issue Sep 26, 2024 · 4 comments
Open

Self referencing schemas results in SystemStackError #291

moberegger opened this issue Sep 26, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@moberegger
Copy link

When providing openapi_first a spec containing a schema like

components:
  schemas:
    MySelfRef:
      type: object
      properties:
        foo: 
          type: string     
        bar: 
          $ref: '#/component/schemas/MySelfRef'

it hits a stack level too deep (SystemStackError) error.

I have a general idea of what is happening. openapi_first is successfully loading the schema, but in doing so it creates self referencing hashes as it de-$references everything. So it basically ends up doing something like this:

my_schema = {
  type: 'object',
  required: ['foo'],
  properties: {
    foo: {
      type: 'string',
    },
    bar: {
      '$ref': '#/component/schemas/BoomRef',
    },
  },
}
my_schema[:properties][:bar][:$ref] = my_schema

This schema gets passed off to json_schemer to build a new JSONSchemer::Schema which works until something is executed on it. For example:

boom =  JSONSchemer.schema(my_schema) # No error, creates a JSONSchemer::Schema
# Later on something runs...
boom.valid_schema? # Results in SystemStackError

This happens because openapi_first de-references everything to keep the entire schema local. It doesn't know that '#/component/schemas/BoomRef' is self referencing, so it de-references that and basically picks itself up from the file_cache and creates a self referencing hash. This causes an error when trying to any recursive work on it; in the example above, json_schemer attempts a deep_stringify_keys that runs forever until the stack error hits.

There are a few ways to do self referencing schemas like this in Openapi. One way is like

my_schema = {
  type: 'object',
  required: ['foo'],
  properties: {
    foo: {
      type: 'string',
    },
    bar: {
      '$ref': '#',
    },
  },
}

but during dereferencing openapi_first sees that '#' and tries to load it from a file, which results in an error.

Another way is

my_schema = {
  type: 'object',
  required: ['foo'],
  '$dynamicAnchor': 'node'.
  properties: {
    foo: {
      type: 'string',
    },
    bar: {
      '$dynamicRef': 'node'.
    },
  },
}

which seems to work, but requires changes to the schema.

It would be nice if openapi_first could handle that first schema. I think what we could do is compare object_ids during the de-referencing process, and if we encounter a schema whose object_id matches the object_id of the thing it is attempting to make a $ref to, we simply swap that value out with a '#'.

Additionally, it would be nice if openapi_first could handle the second schema example. I think what we could do here is modify Dereferencer slightly so that when it encounters a bare '#' it skips trying to resolve it and instead just uses that as the $ref value.

Do you have any thoughts on this? I think I have a pretty good idea of how it works, so I could try submitting a PR to accommodate those if you find either solution acceptable.

@ahx ahx added the bug Something isn't working label Sep 26, 2024
@ahx
Copy link
Owner

ahx commented Sep 26, 2024

Hi. Thanks for the report. In general, and also related to #285, I would like to change from "dereference everything" to "dereference everything outside JSON Schema schemas" and let json_schemer handle $refs inside JSON Schemas (local and across files) correctly via it's .openapi interface. Do you think that makes sense and could work?
I have not fully wrapped my head around that, so I think your option sounds good as well. A PR would be very much appreciated. If you don't get a fix to work, just a failing test would also be much appreciated.

@ahx
Copy link
Owner

ahx commented Dec 6, 2024

This should be fixed on main. I was able to reproduce this with this test: 9c2aaaa

@moberegger
Copy link
Author

moberegger commented Dec 20, 2024

Oh this is cool! Really like the dynamic ref resolver approach! This looks like it will solve a lot of problems.

I did give it a whirl and ran into some issues. The Openapi spec we've inherited takes a modular approach with specifying routes by using $refs like:

paths:
  /companies:
    $ref: paths/companies.yml

And that YAML file will have something like

get:
  responses:
    '200':
      content:
        application/json:
          schema:
            $ref: ../components/schemas/CompanyPaged.yml
    '400':
      $ref: ../components/responses/400.yml
    '403':
      $ref: ../components/responses/403.yml

What I think is happening is that those $refs don't load because when the YAML file is loading, it sees the $ref and instead of de-referencing it, defers it to the dynamic ref resolver. But only json_schemer knows what to do with those, so when openapi_first's router runs it doesn't "see" the schemas. At least I think so.

My understanding is that the above setup as valid for an Openapi spec - or at least that's what I've been told; I may be wrong on that. Not sure if it's possible to dereference things in the Openapi part, and only use the ref resolver when dealing with the JSON schemas via json_schemer.

Still poking around to see if I can get it into a format that openapi_first expects.

@ahx
Copy link
Owner

ahx commented Dec 20, 2024

Hi Michael, I am afraid that the 2.2 release with the RefResolver is far from done now. I am also looking into a (missing?) feature in json_schemer to how to keep the refs intact for subschemas. That said, thanks for taking a look. Can you prepare a PR with a failing test? That would be great. Happy Holidays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants