Merge pull request #234 from Kashoo/issue-98-custom-validation

Issue #98: Pass correct values to final param of custom validation constraints
OldSneerJaw · Feb 19, 2018 · 45945d3 · 45945d3
2 parents 0d869cc + 8b1a7ac
commit 45945d3
Show file tree

Hide file tree

Showing 9 changed files with 116 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,8 @@
 This project adheres to [Semantic Versioning](http://semver.org/). All notable changes will be documented in this file.
 
 ## [Unreleased]
-Nothing yet.
+### Fixed
+- [#98](https://github.com/Kashoo/synctos/issues/98): Final argument of custom validation constraint receives incorrect value
 
 ## [2.0.0] - 2018-02-16
 ### Added

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -53,9 +53,9 @@ In either case, specification files must be denoted by the `.spec.js` filename s
 
 To execute the full test suite and lint the project's JS files with JSHint, run `npm test` from the project's root directory. A detailed, human-readable code coverage report is generated at `build/coverage/lcov-report/index.html`.
 
-### Document definitions validator
+### Document definitions schema validator
 
-Whenever configuration elements are added or updated, the document definitions schema (see the `src/validation/document-definition-schema` and `src/validation/property-validator-schema` modules) must also be updated for use by the document-definitions-validator module. The schema is defined according to the [Joi](https://github.com/hapijs/joi) library's API. See the project's official API [documentation](https://github.com/hapijs/joi/blob/v13.1.1/API.md) for more info.
+Whenever configuration elements are added or updated, the document definitions schema (see the `src/validation/document-definition-schema` and `src/validation/property-validator-schema` modules) must also be updated for use by the document-definitions-validator module. The schema is defined according to the [Joi](https://github.com/hapijs/joi) library's API. See the project's official API [documentation](https://github.com/hapijs/joi/blob/v13.1.2/API.md) for more info.
 
 ### Documentation
 

diff --git a/README.md b/README.md
@@ -515,15 +515,15 @@ propertyValidators: {
     type: 'integer',
     minimumValue: 1,
     maximumValue: 100,
-    customValidation: function(doc, oldDoc, currentItemElement, validationItemStack) {
+    customValidation: function(doc, oldDoc, currentItemEntry, validationItemStack) {
       var parentObjectElement = validationItemStack[validationItemStack.length - 1];
       var parentObjectName = parentObjectElement.itemName;
       var parentObjectValue = parentObjectElement.itemValue;
       var parentObjectOldValue = parentObjectElement.oldItemValue;
 
-      var currentPropName = currentItemElement.itemName;
-      var currentPropValue = currentItemElement.itemValue;
-      var currentPropOldValue = currentItemElement.oldItemValue;
+      var currentPropName = currentItemEntry.itemName;
+      var currentPropValue = currentItemEntry.itemValue;
+      var currentPropOldValue = currentItemEntry.oldItemValue;
 
       var currentPropPath = parentObjectName + '.' + currentPropName;
       var myStringPropPath = parentObjectName + '.myStringProp';

diff --git a/samples/sample-sync-doc-definitions.js b/samples/sample-sync-doc-definitions.js
@@ -15,11 +15,11 @@ function() {
   }
 
   // Checks that a business ID is valid (an integer greater than 0) and is not changed from the old version of the document
-  function validateBusinessIdProperty(doc, oldDoc, currentItemElement, validationItemStack) {
+  function validateBusinessIdProperty(doc, oldDoc, currentItemEntry, validationItemStack) {
     var parentObjectElement = validationItemStack[validationItemStack.length - 1];
 
-    var businessId = currentItemElement.itemValue;
-    var oldBusinessId = currentItemElement.oldItemValue;
+    var businessId = currentItemEntry.itemValue;
+    var oldBusinessId = currentItemEntry.oldItemValue;
 
     var validationErrors = [ ];
 

diff --git a/src/validation/property-validator-schema.js b/src/validation/property-validator-schema.js
@@ -235,7 +235,7 @@ function universalConstraintSchemas(typeEqualitySchema) {
     immutableWhenSetStrict: dynamicConstraintSchema(joi.boolean()),
     mustEqual: dynamicConstraintSchema(mustEqualConstraintSchema(typeEqualitySchema)),
     mustEqualStrict: dynamicConstraintSchema(mustEqualConstraintSchema(typeEqualitySchema)),
-    customValidation: joi.func().maxArity(4) // Function parameters: doc, oldDoc, currentItemElement, validationItemStack
+    customValidation: joi.func().maxArity(4) // Function parameters: doc, oldDoc, currentItemEntry, validationItemStack
   };
 }
 

diff --git a/templates/sync-function-validation-module.js b/templates/sync-function-validation-module.js
@@ -1026,7 +1026,7 @@ function validationModule() {
       var currentItemEntry = itemStack[itemStack.length - 1];
 
       // Copy all but the last/top element so that the item's parent is at the top of the stack for the custom validation function
-      var customValidationItemStack = itemStack.slice(-1);
+      var customValidationItemStack = itemStack.slice(0, -1);
 
       var customValidationErrors = validator.customValidation(doc, oldDoc, currentItemEntry, customValidationItemStack);
 

diff --git a/test/custom-validation.spec.js b/test/custom-validation.spec.js
@@ -0,0 +1,66 @@
+const testHelper = require('../src/testing/test-helper.js');
+
+describe('Custom validation constraint:', () => {
+  beforeEach(() => {
+    testHelper.initSyncFunction('build/sync-functions/test-custom-validation-sync-function.js');
+  });
+
+  it('allows a document if custom validation succeeds', () => {
+    const doc = {
+      _id: 'my-doc',
+      type: 'customValidationDoc',
+      baseProp: {
+        failValidation: false,
+        customValidationProp: 'foo'
+      }
+    };
+
+    testHelper.verifyDocumentCreated(doc);
+  });
+
+  it('blocks a document if custom validation fails', () => {
+    const oldDoc = {
+      _id: 'my-doc',
+      type: 'customValidationDoc',
+      baseProp: { }
+    };
+
+    const doc = {
+      _id: 'my-doc',
+      type: 'customValidationDoc',
+      baseProp: {
+        failValidation: true,
+        customValidationProp: 'foo'
+      }
+    };
+
+    const expectedCurrentItemEntry = {
+      itemValue: doc.baseProp.customValidationProp,
+      oldItemValue: null,
+      itemName: 'customValidationProp'
+    };
+    const expectedValidationItemStack = [
+      { // The document (root)
+        itemValue: doc,
+        oldItemValue: oldDoc,
+        itemName: null
+      },
+      { // The parent of the property with the customValidation constraint
+        itemValue: doc.baseProp,
+        oldItemValue: oldDoc.baseProp,
+        itemName: 'baseProp'
+      }
+    ];
+
+    testHelper.verifyDocumentNotReplaced(
+      doc,
+      oldDoc,
+      'customValidationDoc',
+      [
+        'doc: ' + JSON.stringify(doc),
+        'oldDoc: ' + JSON.stringify(oldDoc),
+        'currentItemEntry: ' + JSON.stringify(expectedCurrentItemEntry),
+        'validationItemStack: ' + JSON.stringify(expectedValidationItemStack)
+      ]);
+  });
+});
diff --git a/test/resources/custom-validation-doc-definitions.js b/test/resources/custom-validation-doc-definitions.js
@@ -0,0 +1,34 @@
+function() {
+  return {
+    customValidationDoc: {
+      typeFilter: simpleTypeFilter,
+      channels: { write: 'write' },
+      propertyValidators: {
+        baseProp: {
+          type: 'object',
+          propertyValidators: {
+            failValidation: {
+              type: 'boolean'
+            },
+            customValidationProp: {
+              type: 'string',
+              customValidation: function(doc, oldDoc, currentItemEntry, validationItemStack) {
+                var parentItemValue = validationItemStack[validationItemStack.length - 1].itemValue;
+                if (parentItemValue && parentItemValue.failValidation) {
+                  return [
+                    'doc: ' + jsonStringify(doc),
+                    'oldDoc: ' + jsonStringify(oldDoc),
+                    'currentItemEntry: ' + jsonStringify(currentItemEntry),
+                    'validationItemStack: ' + jsonStringify(validationItemStack)
+                  ];
+                } else {
+                  return [ ];
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  };
+}
diff --git a/test/resources/underscore-js-doc-definitions.js b/test/resources/underscore-js-doc-definitions.js
@@ -6,10 +6,9 @@
       myProp: {
         type: 'string',
         required: true,
-        customValidation: function(doc, oldDoc, currentItemElement, validationItemStack) {
-          var escapedItemValue = _.chain(currentItemElement.itemValue).escape().value();
-
-          if (escapedItemValue === currentItemElement.itemValue) {
+        customValidation: function(doc, oldDoc, currentItemEntry, validationItemStack) {
+          var escapedItemValue = _.chain(currentItemEntry.itemValue).escape().value();
+          if (escapedItemValue === currentItemEntry.itemValue) {
             return null;
           } else {
             return [ 'escaped value of "myProp" does not match raw value' ];