Changes infoemed by review by epoberezkin

* remove extra parameter from add_keyword; instead change the invoke targets to pass function * make the tests all require 'q' property; confirm it in test_validate.
brainz80 · Mar 12, 2018 · 5708ba9 · 5708ba9
1 parent 49cf145
commit 5708ba9
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 13 deletions.
diff --git a/keywords/add_keyword.js b/keywords/add_keyword.js
@@ -2,14 +2,14 @@
 
 var url = require('url');
 
-module.exports = function (ajv, keyword, jsonPatch, func, patchSchema) {
+module.exports = function (ajv, keyword, jsonPatch, patchSchema) {
   ajv.addKeyword(keyword, {
     macro: function (schema, parentSchema, it) {
       var source = schema.source;
       var patch = schema.with;
       if (source.$ref) source = JSON.parse(JSON.stringify(getSchema(source.$ref)));
       if (patch.$ref) patch = getSchema(patch.$ref);
-      jsonPatch[func].call(null, source, patch, true);
+      jsonPatch.call(null, source, patch, true);
       return source;
 
       function getSchema($ref) {

diff --git a/keywords/merge.js b/keywords/merge.js
@@ -4,5 +4,5 @@ var addKeyword = require('./add_keyword');
 var jsonMergePatch = require('json-merge-patch');
 
 module.exports = function(ajv) {
-  addKeyword(ajv, '$merge', jsonMergePatch, 'apply', { "type": "object" });
+  addKeyword(ajv, '$merge', jsonMergePatch.apply, { "type": "object" });
 };
diff --git a/keywords/patch.js b/keywords/patch.js
@@ -4,7 +4,7 @@ var addKeyword = require('./add_keyword');
 var jsonPatch = require('fast-json-patch');
 
 module.exports = function(ajv) {
-  addKeyword(ajv, '$patch', jsonPatch, 'applyPatch', {
+  addKeyword(ajv, '$patch', jsonPatch.applyPatch, {
     "type": "array",
     "items": {
       "type": "object",

diff --git a/spec/async.spec.js b/spec/async.spec.js
@@ -20,7 +20,8 @@ describe('async schema loading', function() {
         "$merge": {
           "source": { "$ref": "obj.json#" },
           "with": {
-            "properties": { "q": { "type": "number" } }
+            "properties": { "q": { "type": "number" } },
+            "required": [ "q" ]
           }
         }
       };
@@ -35,7 +36,8 @@ describe('async schema loading', function() {
         "$patch": {
           "source": { "$ref": "obj.json#" },
           "with": [
-            { "op": "add", "path": "/properties/q", "value": { "type": "number" } }
+            { "op": "add", "path": "/properties/q", "value": { "type": "number" } },
+            { "op": "add", "path": "/required/-", "value": "q" }
           ]
         }
       };
@@ -59,7 +61,8 @@ describe('async schema loading', function() {
         "id": "obj.json#",
         "type": "object",
         "properties": { "p": { "type": "string" } },
-        "additionalProperties": false
+        "additionalProperties": false,
+        "required": [ "p" ]
       };
       return Promise.resolve(schema);
     }

diff --git a/spec/merge.spec.js b/spec/merge.spec.js
@@ -26,7 +26,8 @@ describe('keyword $merge', function() {
             "additionalProperties": false
           },
           "with": {
-            "properties": { "q": { "type": "number" } }
+            "properties": { "q": { "type": "number" } },
+            "required": [ "q" ]
           }
         }
       };
@@ -53,7 +54,8 @@ describe('keyword $merge', function() {
         "$merge": {
           "source": { "$ref": "obj.json#" },
           "with": {
-            "properties": { "q": { "type": "number" } }
+            "properties": { "q": { "type": "number" } },
+            "required": [ "q" ]
           }
         }
       };
@@ -79,7 +81,8 @@ describe('keyword $merge', function() {
         "$merge": {
           "source": { "$ref": "#/definitions/source" },
           "with": {
-            "properties": { "q": { "type": "number" } }
+            "properties": { "q": { "type": "number" } },
+            "required": [ "q" ]
           }
         }
       };
@@ -96,13 +99,15 @@ describe('keyword $merge', function() {
       var sourceSchema = {
         "type": "object",
         "properties": { "p": { "type": "string" } },
-        "additionalProperties": false
+        "additionalProperties": false,
+        "required": [ "p" ]
       };
 
       var patchSchema = {
         "type": "object",
         "properties": { "q": { "type": "number" } },
-        "additionalProperties": false
+        "additionalProperties": false,
+        "required": [ "q" ]
       };
 
       ajv.addSchema(sourceSchema, "obj1.json#");
@@ -138,7 +143,8 @@ describe('keyword $merge', function() {
           "patch":{
             "type": "object",
             "properties": { "q": { "type": "number" } },
-            "additionalProperties": false
+            "additionalProperties": false,
+            "required" : [ "q" ]
           }
         },
         "$merge": {

diff --git a/spec/test_validate.js b/spec/test_validate.js
@@ -4,6 +4,8 @@ var assert = require('assert');
 
 module.exports = function (validate, keyword) {
   assert.strictEqual(validate({ p: 'abc', q: 1 }), true);
+
+  // property q should be a number
   assert.strictEqual(validate({ p: 'foo', q: 'bar' }), false);
   var errs = validate.errors;
   assert.equal(errs.length, 2);
@@ -13,4 +15,16 @@ module.exports = function (validate, keyword) {
   assert.equal(errs[1].keyword, keyword);
   assert.equal(errs[1].dataPath, '');
   assert.equal(errs[1].schemaPath, '#/' + keyword);
+
+  // an object without q should fail
+  assert.strictEqual(validate({ p: 'foo' }), false);
+  errs = validate.errors;
+  assert.equal(errs.length, 2);
+  assert.equal(errs[0].keyword, 'required');
+  assert.equal(errs[0].dataPath, '');
+  assert.equal(errs[0].schemaPath, '#/required');
+  assert.deepEqual(errs[0].params, { missingProperty: 'q' });
+  assert.equal(errs[1].keyword, keyword);
+  assert.equal(errs[1].dataPath, '');
+  assert.equal(errs[1].schemaPath, '#/' + keyword);
 };