Merge pull request #272 from 0x0a0d/fix_multi_validator_with_object_strict_remove

fix multi validate with object strict remove

Author: icebob

lib/rules/multi.js

@@ -6,32 +6,33 @@ module.exports = function({ schema, messages }, path, context) {
 	const src = [];
 
 	src.push(`
-		var prevErrLen = errors.length;
-		var errBefore;
 		var hasValid = false;
 		var newVal = value;
+		var checkErrors = [];
 	`);
 
 	for (let i = 0; i < schema.rules.length; i++) {
 		src.push(`
 			if (!hasValid) {
-				errBefore = errors.length;
+				var _errors = [];
 		`);
 
 		const rule = this.getRuleFromSchema(schema.rules[i]);
-		src.push(this.compileRule(rule, context, path, `var tmpVal = ${context.async ? "await " : ""}context.fn[%%INDEX%%](value, field, parent, errors, context);`, "tmpVal"));
+		src.push(this.compileRule(rule, context, path, `var tmpVal = ${context.async ? "await " : ""}context.fn[%%INDEX%%](value, field, parent, _errors, context);`, "tmpVal"));
 		src.push(`
-				if (errors.length == errBefore) {
+				if (_errors.length == 0) {
 					hasValid = true;
 					newVal = tmpVal;
+				} else {
+					Array.prototype.push.apply(checkErrors, _errors);
 				}
 			}
 		`);
 	}
 
 	src.push(`
-		if (hasValid) {
-			errors.length = prevErrLen;
+		if (!hasValid) {
+			Array.prototype.push.apply(errors, checkErrors);
 		}
 
 		return newVal;

lib/rules/object.js

@@ -69,31 +69,37 @@ module.exports = function ({ schema, messages }, path, context) {
 
 		// Strict handler
 		if (schema.strict) {
+			sourceCode.push(`
+				if (errors.length === 0) {
+			`);
 			const allowedProps = Object.keys(subSchema);
 
 			sourceCode.push(`
-				field = parentField;
-				var invalidProps = [];
-				var props = Object.keys(parentObj);
-
-				for (let i = 0; i < props.length; i++) {
-					if (${JSON.stringify(allowedProps)}.indexOf(props[i]) === -1) {
-						invalidProps.push(props[i]);
+					field = parentField;
+					var invalidProps = [];
+					var props = Object.keys(parentObj);
+
+					for (let i = 0; i < props.length; i++) {
+						if (${JSON.stringify(allowedProps)}.indexOf(props[i]) === -1) {
+							invalidProps.push(props[i]);
+						}
 					}
-				}
-				if (invalidProps.length) {
+					if (invalidProps.length) {
 			`);
 			if (schema.strict == "remove") {
 				sourceCode.push(`
-					invalidProps.forEach(function(field) {
-						delete parentObj[field];
-					});
+						invalidProps.forEach(function(field) {
+							delete parentObj[field];
+						});
 				`);
 			} else {
 				sourceCode.push(`
 					${this.makeError({ type: "objectStrict", expected: "\"" + allowedProps.join(", ") + "\"", actual: "invalidProps.join(', ')", messages })}
 				`);
 			}
+			sourceCode.push(`
+					}
+			`);
 			sourceCode.push(`
 				}
 			`);

test/rules/multi.spec.js

@@ -1,11 +1,11 @@
 "use strict";
 
 const Validator = require("../../lib/validator");
-const v = new Validator({
-	useNewCustomCheckerFunction: true,
-});
 
 describe("Test rule: multi", () => {
+	const v = new Validator({
+		useNewCustomCheckerFunction: true,
+	});
 	it("should call item's custom checker function", () => {
 		const fn = jest.fn((v) => v);
 
@@ -31,4 +31,61 @@ describe("Test rule: multi", () => {
 	// it("should value equals to other field", () => {
 	// 	// TODO: move from validator.spec.js
 	// });
+
+	describe("object strict test", function () {
+		it("should pass simple test", () => {
+			const v = new Validator({
+				useNewCustomCheckerFunction: true,
+			});
+			const check = v.compile({
+				$$root: true,
+				type: "multi",
+				rules: ["string", "number"]
+			});
+			expect(check(1)).toBe(true);
+			expect(check("1")).toBe(true);
+			expect(check({a: 1})).toEqual([{"actual": {"a": 1}, "field": undefined, "message": "The '' field must be a string.", "type": "string"}, {"actual": {"a": 1}, "field": undefined, "message": "The '' field must be a number.", "type": "number"}]);
+		});
+		it("should pass object strict remove", () => {
+			const v = new Validator({
+				useNewCustomCheckerFunction: true,
+			});
+
+			v.alias("targetA", {
+				type: "object", strict: "remove", props: {
+					a: "number"
+				}
+			});
+
+			v.alias("targetB", {
+				type: "object", strict: "remove", props: {
+					b: "number"
+				}
+			});
+
+			v.alias("targetC", {
+				type: "object", props: {
+					c: "number"
+				}
+			});
+
+			const check = v.compile({
+				$$root: true,
+				type: "multi",
+				rules: ["targetA", "targetB", "targetC"]
+			});
+
+			expect(check({a: 1})).toBe(true);
+
+			const testB = {b: 2, z: 3};
+			expect(check(testB)).toBe(true);
+			expect(testB).toEqual({b: 2});
+
+			const testC = {c: 3, d: 4};
+			expect(check(testC)).toBe(true);
+			expect(testC).toEqual({c: 3, d: 4});
+
+			expect(check({d: 4})).toEqual([{"actual": undefined, "field": "a", "message": "The 'a' field is required.", "type": "required"}, {"actual": undefined, "field": "b", "message": "The 'b' field is required.", "type": "required"}, {"actual": undefined, "field": "c", "message": "The 'c' field is required.", "type": "required"}]);
+		});
+	});
 });