-
-
Notifications
You must be signed in to change notification settings - Fork 240
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
Comments
@P0lip Any idea about this? Or any clue where I can start to debug? Or am I'm missing something here? |
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... |
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,
}),
]);
}); |
Any updates on this? We use some recursive schemas, and Spectral 5.4.0 can't handle them. |
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 |
@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. 👍🏼 |
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. |
@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. |
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. 🙌🏻 |
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. |
@philsturgeon you mentioned v6 would be up soon. Is it still on track? Running into infinite loops :/ |
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. |
A similar issue occurring for this project |
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. |
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:
To Reproduce
OpenAPI document: https://raw.githubusercontent.com/Open-EO/openeo-api/spectral/openapi.yaml
.spectral.yml
Run:
spectral lint openapi.yaml
Expected behavior
No warnings/errors
Environment (remove any that are not applicable):
The text was updated successfully, but these errors were encountered: