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

Internal references dereferenced in external files #77

Closed
Glutexo opened this issue Aug 19, 2020 · 12 comments · Fixed by #82
Closed

Internal references dereferenced in external files #77

Glutexo opened this issue Aug 19, 2020 · 12 comments · Fixed by #82
Assignees

Comments

@Glutexo
Copy link
Collaborator

Glutexo commented Aug 19, 2020

This is more of an question on whether is this even possible. It applies to a state when resolve_types does not include RESOLVE_INTERNAL. The motivation for this is that it is not possible to have recursive schema definitions in referenced local files. Like:

{
  "$defs": {
    "NestedObject": {
      "propertyNames": {
        "minLength": 1
      },
      "additionalProperties": {
        "$ref": "#/$defs/NestedObject"
      }
    }
  }
}

Expected Behaviour

Local references in external files are kept local. They are injected to the main specification and converted to the new path.

Minimal Example Spec

  • OpenAPI specification
    {
      "openapi": "3.0.0",
      "info": {
        "title": "Test API",
        "version": "1"
      },
      "paths": {
        "/endpoint": {
          "post": {
            "requestBody": {
              "content": {
                "application/json": {
                  "schema": {"$ref": "_schemas.json#/$defs/Parameter"}
                }
              }
            },
            "responses": {}
          }
        }
      }
    }

External file with schema definitions.

   {
     "$id": "_schemas.json",
     "$schemas": "https://json-schema.org/draft/2019-09/schema#",
     "$defs": {
       "Something": {
         "type": "object"
       },
       "Parameter": {
         "properties": {
           "something": {
             "$ref": "#/$defs/Something"
           }
         }
       }
     }
   }

Actual Behaviour

Local references in external files are dereferenced. But how? They’d need to be somewhere into the composed specification. Something like:

{
  ...
  "paths": {
   "/endpoint": {
     "post": {
       "requestBody": {
         "content": {
           "application/json": {
             "schema": {"$ref": "#/x-prance-refs/_schemas.json%23%2F%24defs%2FSomething"}
           }
         }
       },
       ...
     }
   }
  },
  "x-prance-refs": {
      "_schemas.json#/$defs/Something": {
        "type": "object"
      }
   }
}

Steps to Reproduce

Here is a simple script exposing what is dereferenced:

from prance import ResolvingParser
from prance.util.resolver import RESOLVE_FILES


def _main():
    parser = ResolvingParser("_openapi.json", resolve_types=RESOLVE_FILES)
    parser.parse()
    print(parser.specification["paths"]["/endpoint"]["post"]["requestBody"]["content"]["application/json"]["schema"])


if __name__ == "__main__":
    _main()

Environment

  • Swagger/OpenAPI version used: 3.0.0
  • Backend: openapi-spec-validator

@jfinkhaeuser

@jfinkhaeuser
Copy link
Collaborator

Ok! That's a good suggestion. I think that similar to #76, it should be possible to do, but I haven't had time to figure that out yet, either.

I think today I might actually manage. Thank you!

@jfinkhaeuser
Copy link
Collaborator

So there was a bug #78 that prevented your desired behaviour anyway.

With the bug resolved, you'll simply get your request body to contain "$ref": "#/$defs/Something" which is of course semantically wrong, but does the resolution as documented.

I think what I can add is something similar to the recursion_limit_handler that's called when a reference is not resolved automatically. And then you could write yourself a handler that rewrites the reference path and adds the x-prance-refs field. I could even add such a handler to the code base. That's not too hard to do.

I'll mark this as a feature request. Shouldn't take too long.

@jfinkhaeuser
Copy link
Collaborator

Hmm. This is a tad more complex, because one might want to install different handlers for different RESOLVE_* flags - or have a single handler do different things.

I guess the basics are clear, I'm just trying to come up with a clean design now.

@Glutexo
Copy link
Collaborator Author

Glutexo commented Oct 7, 2020

Hey @jfinkhaeuser, thanks a lot for looking into this! I found out that with the new version, I am not able to use local references in external files at all. If I take the exact example from the original issue description, I get this error:

prance.ValidationError: Unresolvable JSON pointer: '$defs/Something'

It works however when I put the Something immediately into the OpenAPI specification instead of the referenced file. That is obviously not semantically correct, because in schema.json I am referencing form schema.json and not from openapi.json. With 0.18.3, this works correctly.

@Glutexo
Copy link
Collaborator Author

Glutexo commented Oct 7, 2020

@tahmidefaz, do you want to go down this rabbit hole and help @jfinkhaeuser with this?

@jfinkhaeuser
Copy link
Collaborator

Thanks. Unfortunately, due to a broken arm, this will have to wait a little. I suspect I can type more than a very slow comment in 1-2 weeks again.

@Glutexo
Copy link
Collaborator Author

Glutexo commented Oct 12, 2020

Oh. 😢 Take care of yourself!

@jfinkhaeuser
Copy link
Collaborator

I'm so sorry guys, I will get into this Real Soon (TM). My arm is better - thank you! - but of course a bunch of paid work piled up, so this project can't take precedence. It shouldn't be too long now, I hope (famous last words...).

@Glutexo
Copy link
Collaborator Author

Glutexo commented Nov 3, 2020

Me and @tahmidefaz came up with an idea of utilizing our upcoming Hackathon week at Red Hat to tackle this. If this goes well, there may a PR appear. We’ll keep you updated!

And glad to hear you are recovering! 🙏🏻

@tahmidefaz
Copy link
Contributor

Just opened a PR in #82 which introduces a new parameter called resolve_method to translate external refs to components/schemas. @jfinkhaeuser please take a look whenever you get some time. 🙂

@jfinkhaeuser
Copy link
Collaborator

I merged this now. It's not exactly what I had in mind, because I wanted #76 solved as well - but after some time looking at the PR, I figure what I really need is to refactor the core of the resolver a bit before I can provide a clean solution for both. In the meantime, here it is. I'll push a release momentarily.

@tahmidefaz
Copy link
Contributor

tahmidefaz commented Dec 8, 2020

@jfinkhaeuser Thank you so much for your help with this! I appreciate it! I will check out the latest release. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants