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

oas3-valid-(content-)schema-example: can't handle circular references #915

Closed
m-mohr opened this issue Jan 14, 2020 · 14 comments
Closed

oas3-valid-(content-)schema-example: can't handle circular references #915

m-mohr opened this issue Jan 14, 2020 · 14 comments
Labels
OpenAPI Issues related to the OpenAPI ruleset t/bug Something isn't working

Comments

@m-mohr
Copy link
Contributor

m-mohr commented Jan 14, 2020

Describe the bug

It seems that in some cases the rules oas3-valid-schema-example and oas3-valid-content-schema-example fail although the examples and JSON paths are valid (afaik).

I get in the command line:

 2040:25  error  oas3-valid-content-schema-example  can't resolve reference #/components/schemas/process_json_schema from id #
 4745:15  error  oas3-valid-content-schema-example  can't resolve reference #/components/schemas/process_arguments from id #
 4745:15  error  oas3-valid-schema-example          can't resolve reference #/components/schemas/process_arguments from id #
 5016:29  error  oas3-valid-content-schema-example  can't resolve reference #/components/schemas/process_arguments from id #

To Reproduce

OpenAPI document: https://raw.githubusercontent.com/Open-EO/openeo-api/spectral/openapi.yaml

.spectral.yml

extends: "spectral:oas"

Run: spectral lint openapi.yaml

Expected behavior

No warnings/errors

Environment (remove any that are not applicable):

  • Library version: 5.0.0 and develop branch
  • OS: Windows 10
@m-mohr m-mohr added the t/bug Something isn't working label Jan 14, 2020
@m-mohr
Copy link
Contributor Author

m-mohr commented Jan 15, 2020

@P0lip Any idea about this? Or any clue where I can start to debug? Or am I'm missing something here?

@m-mohr
Copy link
Contributor Author

m-mohr commented Jan 17, 2020

What I figured from debugging is that it seems to be related to recursive schemas. Once I removed them, the tests pass. Example to test against:

{
      openapi: '3.0.2',
      components: {
        schemas: {
          foo: {
            type: 'object',
            required: ["bar"],
            properties: {
              bar: {
                oneOf: [
                  {
                    nullable: true,
                    enum: [null]
                  },
                  {
                    $ref: '#/components/schemas/foo'
                  }
                ]
              }
            },
            example: {
              bar: {
                bar: {
                  bar: null
                }
              }
            },
          },
        },
      },
    }

@P0lip Do you think there's a reasonable fix to this? I have no clue how that should be handled as long as refs get resolved beforehand...

@m-mohr m-mohr changed the title oas3-valid-(content-)schema-example: can't resolve reference oas3-valid-(content-)schema-example: can't handle circular references Jan 17, 2020
@m-mohr
Copy link
Contributor Author

m-mohr commented Jan 17, 2020

I thought about rejecting to validate examples with circular schemas and returning just an information instead of an error, but not sure how I can do that...

A corresponding test could look like that

  test('will inform when circular references example is used', async () => {
    const data = {
      openapi: '3.0.2',
      components: {
        schemas: {
          foo: {
            type: 'object',
            required: ["bar"],
            properties: {
              bar: {
                oneOf: [
                  {
                    nullable: true,
                    enum: [null]
                  },
                  {
                    $ref: '#/components/schemas/foo'
                  }
                ]
              }
            },
            example: {
              bar: {
                bar: {
                  bar: null
                }
              }
            },
          },
        },
      },
    };

    const results = await s.run(data);

    expect(results).toEqual([
      expect.objectContaining({
        code: 'oas3-valid-schema-example',
        message: 'Example ignored due to circular references',
        severity: DiagnosticSeverity.Information,
      }),
    ]);
  });

@DavidBiesack
Copy link

Any updates on this? We use some recursive schemas, and Spectral 5.4.0 can't handle them.
We have an error schema that has a nested errors property with type: array whose items are the same error type.

@m-mohr
Copy link
Contributor Author

m-mohr commented Jul 15, 2020

This is getting more and more a real pain, we now have to exclude quite a lot of examples from the spec already because we have so many circular references. It's starting to really hurt to a point where Spectral could potentially be dropped from our pipeline. Are there any plans to tackle this in an upcoming version? @nulltoken

@philsturgeon
Copy link
Contributor

@m-mohr I can appreciate that you are frustrated by this but the only advice I’d give you is what you’ve already done: if a rule is causing you trouble then disable it. If we disable it at a higher level then we’ll be causing problems for everyone who doesn’t have a problem with it.

We’ll get to this issue when we can, but there’s lots of work to be done, and as you’ve said, you’ve disabled it. 👍🏼

@philsturgeon
Copy link
Contributor

Btw I’ve mentioned #1054 as the larger solution a few times. What’s new is that this will be a major part in v6.0, which will be coming soon. We’re getting a few more features sorted out for v5.6 then focusing on v6, so resolution errors should be a thing of the past for spectral, and any resolution bugs will be resolved by the resolver library.

@m-mohr
Copy link
Contributor Author

m-mohr commented Jul 15, 2020

@philsturgeon I don't want it to be disabled on a higher level, best would just be to fix it. Recursive Schemas are not so uncommon, I guess.

About disabling: I'm still trying to disable it for recent changes, but it's not a trivial task as paths reported are also not really helpful: #1280 It's sometimes a 5 minute change in the API leads to a 5 hour attempt to fix Spectral in CI. So if #1054 fixes things, then I'm all for it.

@philsturgeon
Copy link
Contributor

Yes in an ideal world all bugs would be immediately fixed, but managing products involves taking in large amounts of feedback from various resources and prioritising what can be done when, with the limited resources you have available.

PRs are always welcome when you feel that something is more urgent than others do, seeing as this is a free open source project, but so far we’ve not had anyone mention this problem other than this thread. We’d have got to it sooner if it was a blocker, or if more people had the issue, but we’ve just not had that feedback.

The reported paths issue you link to is also part of v6.0 so that version is going to solve a lot of what you need. All were aiming for in the next minor is #1021, then we can begin the work of rewriting everything to take care of these underlying tech debt issues that lead to various issues in resolution.

Thank you for your feedback, it has been heard. 🙌🏻

@m-mohr
Copy link
Contributor Author

m-mohr commented Jul 15, 2020

Totally understood. I had a PR up (#915), but didn't had enough knowledge to fix it properly, could just "ignore" it, it's just too deep in the code and - as it seems - needs a new parser. So I'm happy to hear that this might be solved in 6.0 and I'm also happy to test or so.

@P0lip P0lip added the OpenAPI Issues related to the OpenAPI ruleset label Sep 30, 2020
@ajcool2k
Copy link

@philsturgeon you mentioned v6 would be up soon. Is it still on track? Running into infinite loops :/

@philsturgeon
Copy link
Contributor

philsturgeon commented Mar 23, 2021

Hey! The team has been rammed with Elements, Collaborative "Web Projects", and Workspace Groups for the last few months but we're about to rapid fire release a bunch of that stuff and get back to meaningful improvements to Spectral.

Spectral v6 will need a certain amount of refactoring for OpenAPI v3.1 (#1302) and loads of other improvements will be happening, so yeah it's coming! Again, #1054 is the issue to watch so subscribe to that for updates.

@P0lip
Copy link
Contributor

P0lip commented Oct 28, 2021

A similar issue occurring for this project
repro.zip

@m-mohr
Copy link
Contributor Author

m-mohr commented Nov 30, 2021

It seems that in spectral 6.1.0 this is solved? At least I could remove my exceptions after the 5 to 6 migration and don't get errors any longer.

@m-mohr m-mohr closed this as completed Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OpenAPI Issues related to the OpenAPI ruleset t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants