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

handling of $dynamicRef on openApi v3.1 spec schema #1573

Open
seriousme opened this issue Apr 28, 2021 · 7 comments
Open

handling of $dynamicRef on openApi v3.1 spec schema #1573

seriousme opened this issue Apr 28, 2021 · 7 comments

Comments

@seriousme
Copy link
Contributor

What version of Ajv are you using? Does the issue happen if you use the latest version?
8.2.0
Ajv options object

{ strict:false }

JSON Schema

 {
  "type": "object",
  "properties": {
    "name": {
      "type": "string"
    },
    "in": {
      "enum": [
        "query",
        "header",
        "path",
        "cookie"
      ]
    },
    "required": {
      "default": false,
      "type": "boolean"
    },
    "schema": {
      "$dynamicRef": "#meta"
    },
    "content": {
      "type": "object"
    }
  },
  "required": [
    "in"
  ],
  "oneOf": [
    {
      "required": [
        "schema"
      ]
    },
    {
      "required": [
        "content"
      ]
    }
  ],
  "$defs": {
    "schema": {
      "$dynamicAnchor": "meta",
      "type": [
        "object",
        "boolean"
      ]
    }
  }
}

Sample data

{
  "name": "limit",
  "in": "query",
  "description": "How many items to return at one time (max 100)",
  "required": false,
  "schema": {
    "type": "integer",
    "format": "int32"
  }
}

Your code

const Ajv = require("ajv/dist/2020.js")

const ajv = new Ajv(options)

const validate = ajv.compile(schema);

console.log(validate(data));
console.log(validate.errors);

https://runkit.com/seriousme/ajv-dynamicref

Validation result, data AFTER validation, error messages

false
[
  {
    instancePath: '/schema',
    schemaPath: '#/oneOf/0/required',
    keyword: 'required',
    params: { missingProperty: 'schema' },
    message: "must have required property 'schema'"
  },
  {
    instancePath: '/schema',
    schemaPath: '#/oneOf/1/required',
    keyword: 'required',
    params: { missingProperty: 'content' },
    message: "must have required property 'content'"
  },
  {
    instancePath: '/schema',
    schemaPath: '#/oneOf',
    keyword: 'oneOf',
    params: { passingSchemas: null },
    message: 'must match exactly one schema in oneOf'
  }
]

What results did you expect?
I expected the validation to succeed.
https://json-schema.hyperjump.io/ says the data matches the schema.

The schema is part of the OpenApi 3.1 schema (which is not under my control)
The data is part of the well known Petstore example
( I created a v3.1 Petstore example by taking the v3.0 Petstore example, updating the openapi version to "3.1.0" and adding a licence URL)

Validation of the full v3.1 Petstore example against the full 3.1 schema works with:

Which leads me to think that the issue is not in the schema but in Ajv ;-)

I also tried to resolve the issue by tweaking the schema:

  • Replacing "$dynamicRef": "#meta" by "type": [ "object", "boolean"] solves the issue.
  • Replacing "$dynamicRef": "#meta" by "$ref": "#/$defs/schema" solves the issue as well.
    (when applied to the full OpenApi 3.1 schema this lets Ajv validate the full Petstore example as well)
  • Removing /$defs/schema from the schema still gives the same error.

Hence my suspicion that the problem is caused by the handling of $dynamicRef by Ajv

Are you going to resolve the issue?
I'm happy to help but I think it requires some quite deep knowledge of Ajv internals.

@seriousme
Copy link
Contributor Author

seriousme commented Apr 28, 2021

I did some more analysis by generating code for both and the static variant with:

 "schema": {
      "type": "[object,boolean]"
    },

and the dynamic variant with:

 "schema": {
      "$dynamicRef": "#meta"
    },

And doing a diff on those.
The static variant does (as expected) a direct check:

module.exports = validate20;
---< snipped code >----
if (
  !(
    data3 &&
    typeof data3 == "object" &&
    !Array.isArray(data3)
  ) &&
  typeof data3 !== "boolean"
) {
  validate20.errors = [
    {
      instancePath: instancePath + "/schema",
      schemaPath: "#/properties/schema/type",
      keyword: "type",
      params: { type: schema31.properties.schema.type },
      message: "must be object,boolean",
    },
  ];
  return false;
}

The dynamic variant does a recursive call:

module.exports = validate20;
---< snipped code >----
if (
  !validate20(data.schema, {
    instancePath: instancePath + "/schema",
    parentData: data,
    parentDataProperty: "schema",
    rootData,
    dynamicAnchors,
  })
) {
  vErrors =
    vErrors === null
      ? validate20.errors
      : vErrors.concat(validate20.errors);
  errors = vErrors.length;
}

This works great if the dynamicAnchor sits at the top of the schema (as in ajv/spec/dynamic-ref.spec.ts). But less convenient if the anchor is positioned in a sub path (in this case #/$defs/schema)

While tracing both calls to validate20 I noticed that dynamicAnchors is empty.
From the code it also seems that dynamicAnchors is just passed around without being modified as no assignment is being done on dynamicAnchors where is one might expect that dynamicAnchors would be kind of dynamic ;-)

Cheers,
Hans
ps. full code af the dynamic variant is available at https://runkit.com/seriousme/oas-dynamic-validation

@epoberezkin
Copy link
Member

Will investigate. $dynamicRef is quite limited indeed, it might be unsupported case…

@OlivierCuyp
Copy link

OlivierCuyp commented May 21, 2022

@epoberezkin any progress on this one ? It seems this blocks the express-openapi-validator from being able to handle OpenApi v3.1 schema.
It would be great to have both issues solved ^^.

@epoberezkin
Copy link
Member

Sorry, didn't come around to improving it yet. Is there still an interest in this case being supported?

@seriousme
Copy link
Contributor Author

Yes, it would be nice if AJV supports this as I currently need to preprocess the openapi 3.1 meta schema to be able to verify openapi schemas.

See: https://github.com/seriousme/openapi-schema-validator/blob/master/test/convert-3.1.js

@Jackman3005
Copy link

Please Send Help 🙏

@epoberezkin could you help elaborate on what some of the challenges are to implement this or share some of the thoughts you've had on how you would approach it? If you can provide us with enough detail possibly I or someone else would feel more comfortable picking it up and attempting a PR. I haven't contributed to this library yet and it feels overwhelming 😵‍💫 to dig in on this and attempt a proper solution without some guidance from someone who knows the library well.

Why?

As mentioned here and by @OlivierCuyp. This seems to be the only major blocker for express-openapi-validator from adopting OpenAPI v3.1 (and I imagine any other libraries that use AJV for OpenAPI v3.1 parsing/validation). As other libraries are moving to support 3.1 we're now stuck unable upgrade to use new features and fixes for our other downstream consumers of our OpenAPI spec 😭.

Moar Bounty?

I saw there has been a $500 bounty for this for over a year now. What else would be an effective way to get this prioritized?

Thanks!

@epoberezkin Thanks for all you've already done on this incredible library and thanks in advance for any help you can provide to move this forward!

@seriousme
Copy link
Contributor Author

Hi,

the easy fix as mentioned above is to preprocess the openAPI spec and replace the DynamicRefs by normal refs before passing the schema to AJV.

Similarly AJV might possibly do a lookup of dynamicAnchors once it encounters a dynamicRef and store the locations so that upon code generation the locations of the dynamicAnchors are already known.

My 2cts,
Hans

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

No branches or pull requests

4 participants