stoplightio / spectral

A flexible JSON/YAML linter for creating automated style guides, with baked in support for OpenAPI v3.1, v3.0, and v2.0 as well as AsyncAPI v2.x.
https://stoplight.io/spectral
Apache License 2.0
2.43k stars 235 forks source link

Nested aliases lead to performance issues #2489

Closed ductaily closed 1 year ago

ductaily commented 1 year ago

Describe the bug

Define a ruleset with nested aliases and use the alias that contains all nested aliases in the given of the rule. Run the ruleset on a large OpenAPI document.

The execution of the rules on the document does not seem to terminate.

To Reproduce

Example to run: https://github.com/ductaily/spectral-alias-performance-example Place into the src directory a valid.json with the content here.

aliases: {
    odataV4: ['$.[?(@property === "x-sap-api-type" && @ === "ODATAV4")]'],
    odataV2: ['$.[?(@property === "x-sap-api-type" && @ === "ODATA")]'],
    odataV2All: ["#odataV2^.paths.*.*.responses[?(@property.match(/^[12]/))]"],
    odataV4All: ["#odataV4^.paths.*.*.responses[?(@property.match(/^[12]/))]"],
    odataAll: [
      "#odataV4All.content.application/json.schema.properties.value.items",
      "#odataV4All.schema.properties.value.items",
      "#odataV2All.content.application/json.schema.properties.d.properties.results.items",
      "#odataV2All.schema.properties.d.properties.results.items",
      '#odataV4All.content.application/json[?(@.type == "object" && @.properties && !@.properties.value)]',
      "#odataV4All[?(@.properties && !@.properties.value)]",
      '#odataV2All.content.application/json.schema.properties[?(@.type === "object" && @.properties && !@.properties.results)]',
      '#odataV2All.schema.properties[?(@.type === "object" && @.properties && !@.properties.results)]',
    ],
    restPrefix: [
      '$[?(@root["x-sap-api-type"] === undefined && @property === "paths")]',
      '$[?(@property === "x-sap-api-type" && @ === "REST")]',
    ],
    restAll: [
      '#restPrefix^.paths.*.*.responses[?(@property.match(/^[12]/))].content.application/json[?(@.type == "object")]',
      '#restPrefix^.paths.*.*.responses[?(@property.match(/^[12]/))][?(@.type == "object")]',
    ],
  }
  //...
  rules: {
    "odata-place-holder": {
      message: "Field `x-custom-extension` missing.",
      severity: DiagnosticSeverity.Error,
      given: ["#restAll", "#odataAll"],
      then: {
        field: "x-custom-extension",
        function: truthy
      }
    }
  }

Expected behavior The ruleset execution should terminate in a similar time as if the JSON paths were flat.

Screenshots If applicable, add screenshots to help explain your problem.

Environment (remove any that are not applicable):

Additional context It seems that resolving the aliases is problematic. We ran the same paths flattened without nesting and spectral terminates after approx. 2 minutes.

P0lip commented 1 year ago

Hey! Could you provide JSON path expressions without aliases? Overall aliases shouldn't have any meaningful impact on the runtime performance, as we expand them to actual JSON path expressions before we start the linting process.

ductaily commented 1 year ago

Hi, thanks for taking this. Here is how the path expressions without aliases look like:

aliases: {
  odataAll: [
      // OData arrays
      '$[?(@property === "x-sap-api-type" && @ === "ODATAV4")]^.paths.*.*.responses[?(@property.match(/^[12]/))].content.application/json.schema.properties.value.items',
      '$[?(@property === "x-sap-api-type" && @ === "ODATAV4")]^.paths.*.*.responses[?(@property.match(/^[12]/))].schema.properties.value.items',
      '$[?(@property === "x-sap-api-type" && @ === "ODATA")]^.paths.*.*.responses[?(@property.match(/^[12]/))].content.application/json.schema.properties.d.properties.results.items',
      '$[?(@property === "x-sap-api-type" && @ === "ODATA")]^.paths.*.*.responses[?(@property.match(/^[12]/))].schema.properties.d.properties.results.items',
      // OData objects
      '$[?(@property === "x-sap-api-type" && @ === "ODATAV4")]^.paths.*.*.responses[?(@property.match(/^[12]/))].content.application/json[?(@.type == "object" && @.properties && !@.properties.value)]',
      '$[?(@property === "x-sap-api-type" && @ === "ODATAV4")]^.paths.*.*.responses[?(@property.match(/^[12]/))][?(@.properties && !@.properties.value)]',
      '$[?(@property === "x-sap-api-type" && @ === "ODATA")]^.paths.*.*.responses[?(@property.match(/^[12]/))].content.application/json.schema.properties[?(@.type === "object" && @.properties && !@.properties.results)]',
      '$[?(@property === "x-sap-api-type" && @ === "ODATA")]^.paths.*.*.responses[?(@property.match(/^[12]/))].schema.properties[?(@.type === "object" && @.properties && !@.properties.results)]',
  ],
  restAll: [
      '$[?(@root["x-sap-api-type"] === undefined && @property === "paths")]^.paths.*.*.responses[?(@property.match(/^[12]/))].content.application/json[?(@.type == "object")]',
      '$[?(@property === "x-sap-api-type" && @ === "REST")]^.paths.*.*.responses[?(@property.match(/^[12]/))].content.application/json[?(@.type == "object")]',
      '$[?(@root["x-sap-api-type"] === undefined && @property === "paths")]^.paths.*.*.responses[?(@property.match(/^[12]/))][?(@.type == "object")]',
      '$[?(@property === "x-sap-api-type" && @ === "REST")]^.paths.*.*.responses[?(@property.match(/^[12]/))][?(@.type == "object")]',
    ]
}
P0lip commented 1 year ago

Thank you! I'll try to take a look at it this week.

P0lip commented 1 year ago

@ductaily alright, I took and look. The issue is caused by .[ which is interpreted as a deep, meaning we traverse the entire document. Try changing it back to

   odataV4: ['$[?(@property === "x-sap-api-type" && @ === "ODATAV4")]'],
   odataV2: ['$[?(@property === "x-sap-api-type" && @ === "ODATA")]'],

That should resolve the issue

ductaily commented 1 year ago

I can confirm, this solves the issue. Thanks a lot!

If that is an intended behavior and I should be more careful when to use . then I would close the issue as completed.

P0lip commented 1 year ago

If that is an intended behavior

As things stand, that is an intended behavior. It may change in the future once #2395 is completed.