$ref errors with ajv 6
Closed this issue · 10 comments
Updating ajv-merge-patch devDependency to ajv@6.5.1 produces the following test failures:
1) async schema loading "before each" hook for "should load missing schemas":
can't resolve reference http://json-schema.org/draft-06/schema# from id #
2) errors missing $ref "before each" hook for "should throw exception if cannot resolve $ref":
can't resolve reference http://json-schema.org/draft-06/schema# from id #
3) keyword $merge "before each" hook for "should extend schema defined in $merge":
can't resolve reference http://json-schema.org/draft-06/schema# from id #
4) keyword $patch "before each" hook for "should extend schema defined in $patch":
can't resolve reference http://json-schema.org/draft-06/schema# from id #
I ran into these errors trying to use relative $ref as the source in my schema.
I stupidly forgot to update the tests to add the draft-06 meta schema before capturing the error.
With this patch:
diff --git a/spec/merge.spec.js b/spec/merge.spec.js
index c9a573b..6b3aea8 100644
--- a/spec/merge.spec.js
+++ b/spec/merge.spec.js
@@ -10,6 +10,11 @@ describe('keyword $merge', function() {
beforeEach(function() {
ajvInstances = [ new Ajv, new Ajv ];
+ ajvInstances.forEach(
+ function (ajv) {
+ ajv.addMetaSchema(require('ajv/lib/refs/json-schema-draft-06.json'));
+ }
+ );
addKeywords(ajvInstances[0]);
addMerge(ajvInstances[1]);
});
diff --git a/spec/patch.spec.js b/spec/patch.spec.js
index 11ea424..42c938b 100644
--- a/spec/patch.spec.js
+++ b/spec/patch.spec.js
@@ -10,6 +10,11 @@ describe('keyword $patch', function() {
beforeEach(function() {
ajvInstances = [ new Ajv, new Ajv ];
+ ajvInstances.forEach(
+ function (ajv) {
+ ajv.addMetaSchema(require('ajv/lib/refs/json-schema-draft-06.json'));
+ }
+ );
addKeywords(ajvInstances[0]);
addPatch(ajvInstances[1]);
});The tests failures related to my issue are:
3) keyword $merge should extend schema defined in $ref:
Error: schema with key or id "" already exists
at checkUnique (node_modules/ajv/lib/ajv.js:478:11)
at Ajv.addSchema (node_modules/ajv/lib/ajv.js:135:3)
at testMerge (spec/merge.spec.js:55:11)
at Array.forEach (<anonymous>)
at Context.<anonymous> (spec/merge.spec.js:45:18)
4) keyword $merge should extend schema defined with relative $ref:
can't resolve reference #/definitions/source from id #
5) keyword $merge should extend schema with patch defined with relative $ref:
can't resolve reference #/definitions/patch from id #
6) keyword $patch should extend schema defined in $ref:
Error: schema with key or id "" already exists
at checkUnique (node_modules/ajv/lib/ajv.js:478:11)
at Ajv.addSchema (node_modules/ajv/lib/ajv.js:135:3)
at testPatch (spec/patch.spec.js:55:11)
at Array.forEach (<anonymous>)
at Context.<anonymous> (spec/patch.spec.js:45:18)
7) keyword $patch should extend schema defined with relative $ref:
can't resolve reference #/definitions/source from id #
I'm wading out of my depth here, but I found that when ajv.getSchema is called by the custom keyword macro, the root schema is undefined. If I force resolveSchema to fall into the if (refPath != baseId) block, the unresolved reference errors no longer occur, leaving only two failures:
3) keyword $merge should extend schema defined in $ref:
Error: schema with key or id "" already exists
at checkUnique (node_modules/ajv/lib/ajv.js:478:11)
at Ajv.addSchema (node_modules/ajv/lib/ajv.js:135:3)
at testMerge (spec/merge.spec.js:55:11)
at Array.forEach (<anonymous>)
at Context.<anonymous> (spec/merge.spec.js:45:18)
4) keyword $patch should extend schema defined in $ref:
Error: schema with key or id "" already exists
at checkUnique (node_modules/ajv/lib/ajv.js:478:11)
at Ajv.addSchema (node_modules/ajv/lib/ajv.js:135:3)
at testPatch (spec/patch.spec.js:55:11)
at Array.forEach (<anonymous>)
at Context.<anonymous> (spec/patch.spec.js:45:18)
@epoberezkin any ideas? I could work on a PR to fix this, but I'm not sure how to go about it yet. I'll keep digging while I await a response...
I found that changing id to $id fixes the test cases in this library when used with ajv >= 6. Is there some reason why an id is required for internal refs?
I think I've figured this out - trying to resolve a $ref in a macro custom keyword did not work, presumably b/c the root schema is undergoing compilation when the macro is evaluated.
The following patch to ajv fixes the issue:
diff --git a/lib/compile/resolve.js b/lib/compile/resolve.js
index cdd1a59..1f2a786 100644
--- a/lib/compile/resolve.js
+++ b/lib/compile/resolve.js
@@ -70,7 +70,7 @@ function resolveSchema(root, ref) {
var p = URI.parse(ref)
, refPath = _getFullPath(p)
, baseId = getFullPath(this._getId(root.schema));
- if (refPath !== baseId) {
+ if (!(root instanceof SchemaObject) || refPath !== baseId) {
var id = normalizeId(refPath);
var refVal = this._refs[id];
if (typeof refVal == 'string') {I think I can write a test case for this in ajv and open a PR there to resolve the issue.
I’ll need to catch up with it :)
I will review it over weekend.
I’d say we need to split these 2 issues. Firstly, it looks like this library wasn’t updated for draft-07/ajv v6. Could you please make a PR to address this here? That will be a major release of this package.
Secondly, as I understood, the issue in this library (after this update) is a generic one related to macro keywords (that’s what I need to catch up with). It would definitely help to have the simplest possible failing test case in ajv, ideally without this library, that would demonstrate this issue.
Thank you @jstewmon
@epoberezkin thanks for taking a look.
I submitted a PR to ajv with my fix and a minimal test case.
I submitted a PR here to update the peer dep to >= v6.0.0.
Thank you
It's merged