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

Support mutual recursive $refs when both refs are pointing to schema fragments #210

Closed
psunbury opened this issue Jun 13, 2016 · 18 comments
Closed

Comments

@psunbury
Copy link

I am currently working on a project where we are splitting up a very large JSON schema. In doing so I am breaking up the definitions into separate definition files/schemas. However when doing this I seem to be getting a stack over flow whenever one schema file refers to another (using IDs) that also refers back to it. What is odd is I have tested some simple examples doing the same thing but that do work. I was trying to make an example case to show here but was not able to make one to cause the same error. I have looked extensively for information about this and haven't been able to find anything. I also upgraded to AJV 4.1.2 and got the same error. I am adding all these schemas programmatically with "addSchema". They work fine when they are not circular but when they are circular it no longer works.
Is this just a limitation? Am I doing something wrong? Any help us appreciated. This is the looped call stack:

RangeError: Maximum call stack size exceeded
    at String.replace (native)
    at Url.format (url.js:413:23)
    at Url.parse (url.js:344:20)
    at Object.urlParse [as parse] (url.js:84:5)
    at Ajv._resolve (node_modules/ajv/lib/compile/resolve.js:62:15)
    at Ajv.resolve (node_modules/ajv/lib/compile/resolve.js:39:22)
    at Object.resolveRef (node_modules/ajv/lib/compile/index.js:128:21)
    at Object.generate_ref [as code] (node_modules/ajv/lib/dotjs/ref.js:22:22)
    at Object.generate_validate [as validate] (node_modules/ajv/lib/dotjs/validate.js:374:37)
    at Object.generate_allOf [as code] (node_modules/ajv/lib/dotjs/allOf.js:22:27)
    at Object.generate_validate [as validate] (node_modules/ajv/lib/dotjs/validate.js:374:37)
    at Object.generate_properties [as code] (node_modules/ajv/lib/dotjs/properties.js:198:26)
    at Object.generate_validate [as validate] (node_modules/ajv/lib/dotjs/validate.js:374:37)
    at Object.generate_allOf [as code] (node_modules/ajv/lib/dotjs/allOf.js:22:27)
    at generate_validate (node_modules/ajv/lib/dotjs/validate.js:374:37)
    at localCompile (node_modules/ajv/lib/compile/index.js:53:22)
    at Ajv.compile (node_modules/ajv/lib/compile/index.js:42:10)
    at Ajv.localCompile (node_modules/ajv/lib/compile/index.js:48:22)
    at Ajv.resolve (node_modules/ajv/lib/compile/resolve.js:52:19)
    at Object.resolveRef (node_modules/ajv/lib/compile/index.js:128:21)
    at Object.generate_ref [as code] (node_modules/ajv/lib/dotjs/ref.js:22:22)
    at Object.generate_validate [as validate] (node_modules/ajv/lib/dotjs/validate.js:374:37)
    at Object.generate_anyOf [as code] (node_modules/ajv/lib/dotjs/anyOf.js:34:27)

@epoberezkin
Copy link
Member

epoberezkin commented Jun 13, 2016

Even if schema is recursive it should have some branch(es) when the recursion stops, same as with functions. Otherwise you'll get stack overflow exception.

Theoretically you can get this exception if your data is extremely deep, but all the cases of endless recursion I came across were with badly defined schemas.

For example, this simple schema will cause stack overflow when you try to validate any data:

{
  "$ref": "#/definitions/recursive",
  "definitions": {
    "recursive": { "$ref": "#" }
  }
}

https://tonicdev.com/esp/575f2c68a580751300ba16f6

Some example of schema and data that fail would help to understand whether the issue is with your schema or with ajv. Maybe you can post a "working" example to https://tonicdev.com/ ?

@epoberezkin
Copy link
Member

epoberezkin commented Jun 13, 2016

Mutual recursion that you describe is supported, but you can get the same problem, if there is no branch that stops recursion (or if its condition is never satisfied).

You can see a working example of mutual recursion in the beginning here: http://code.tutsplus.com/tutorials/validating-data-with-json-schema-part-2--cms-25640 (navigation.json refers to page.json and vice versa).

@psunbury
Copy link
Author

Yes but if I have it in the same file it works. It only doesn't work when I separate it into separate schema. So shouldn't what you are describing not work for either method?

@epoberezkin
Copy link
Member

Not in all cases. References can resolve differently when you split the schema. Could you please post the example?

@epoberezkin
Copy link
Member

Or can be a bug of course, in any case if I can't reproduce, I can't fix.

@psunbury
Copy link
Author

Yep. I totally get what you are saying. The schema is more than 3k lines and I doubt I can share it. I was trying to make you a simple example to demonstrate it but I couldn't reproduce the error, so I must be missing something. I'll read your links and see what I can do, if I can't clear it up, I'll work more on a test case.

@psunbury
Copy link
Author

Can I email you the schema files?

@epoberezkin
Copy link
Member

sure, I'll have a look

@psunbury
Copy link
Author

Did you get the files? I sent them to your evgeny.poberezkin@mailonline.co.uk account. I want to make sure you go it because email with attached files sometimes don't always go through.

@epoberezkin
Copy link
Member

I didn't... Maybe email dropbox/google docs link?

@psunbury
Copy link
Author

Sent.

@epoberezkin
Copy link
Member

please add the code that fails too

@psunbury
Copy link
Author

Ok I added a tester file. It's not the same code we use but the code we are using has lots in it and probably would just take up more time and be confusing. I seem to be getting the same problem with the test code.
It works after removing
"caption": {
"$ref": "def#/definitions/text_assets"
}
From basicAssets.json. Just like the code we have does, so I am pretty sure it is showing the same issue.

@psunbury
Copy link
Author

Tell me if you need anything else from me.

@epoberezkin
Copy link
Member

epoberezkin commented Jun 18, 2016

This is the minimal failing test case:

ajv.addSchema({
  "id" : "def",
  "definitions": {
    "text_assets": {
      "properties": {
        "asset": {
          "$ref": "asset_base#/definitions/asset_image"
        }
      }
    }
  }
});

ajv.addSchema({
  "id" : "asset_base",
  "definitions": {
    "asset_image": {
      "properties": {
        "caption": {
          "$ref": "def#/definitions/text_assets"
        }
      }
    }
  }
});

ajv.compile({ "$ref": "def#/definitions/text_assets" });

It appears that the support of mutual references is limited to the case when at least one of them is the reference to the whole schema and the case when both are fragments is not supported.

The workaround is to extract one of the schema fragments that participate in the mutual reference into a separate schema:

ajv.addSchema({
  "id" : "def",
  "definitions": {
    "text_assets": {
      "properties": {
        "asset": {
          "$ref": "asset_image"
        }
      }
    }
  }
});

ajv.addSchema({
  "id" : "asset_image",
  "properties": {
    "caption": {
      "$ref": "def#/definitions/text_assets"
    }
  }
});

ajv.compile({ "$ref": "def#/definitions/text_assets" });

That's a good finding, something to be improved. Thank you.

@epoberezkin epoberezkin changed the title Getting "Maximum call stack exceeded" when making circular ref with $ref Support mutual recursive $refs when both refs are pointing to schema fragments Jun 18, 2016
epoberezkin added a commit that referenced this issue Jun 18, 2016
@psunbury
Copy link
Author

Thanks a lot for looking into this. This is great.
What is a rough time frame for an update to this bug (week, month, year?)? I just want to know if I should be using the workaround or just wait for the fix.
Thanks.

@epoberezkin
Copy link
Member

Can't promise, definitely not year, can be a month though. There is a third option btw - to submit a PR :)

@epoberezkin
Copy link
Member

In 4.1.7

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

No branches or pull requests

2 participants