From ce7a34be46b28d292d8258a7a6d3d7decf9461fc Mon Sep 17 00:00:00 2001 From: Piotr Kuczynski Date: Thu, 5 Aug 2021 14:13:56 +0200 Subject: [PATCH 1/2] Extend discriminator to support number and boolean values --- lib/vocabularies/discriminator/index.ts | 27 ++-- spec/discriminator.spec.ts | 183 +++++++++++++++++++++--- 2 files changed, 181 insertions(+), 29 deletions(-) diff --git a/lib/vocabularies/discriminator/index.ts b/lib/vocabularies/discriminator/index.ts index f8656d8d2..33679f543 100644 --- a/lib/vocabularies/discriminator/index.ts +++ b/lib/vocabularies/discriminator/index.ts @@ -32,7 +32,7 @@ const def: CodeKeywordDefinition = { const valid = gen.let("valid", false) const tag = gen.const("tag", _`${data}${getProperty(tagName)}`) gen.if( - _`typeof ${tag} == "string"`, + _`["string", "number", "boolean"].includes(typeof ${tag})`, () => validateMapping(), () => cxt.error(false, {discrError: DiscrError.Tag, tag, tagName}) ) @@ -41,9 +41,9 @@ const def: CodeKeywordDefinition = { function validateMapping(): void { const mapping = getMapping() gen.if(false) - for (const tagValue in mapping) { + for (const tagValue of mapping.keys()) { gen.elseIf(_`${tag} === ${tagValue}`) - gen.assign(valid, applyTagSchema(mapping[tagValue])) + gen.assign(valid, applyTagSchema(mapping.get(tagValue))) } gen.else() cxt.error(false, {discrError: DiscrError.Mapping, tag, tagName}) @@ -57,8 +57,10 @@ const def: CodeKeywordDefinition = { return _valid } - function getMapping(): {[T in string]?: number} { - const oneOfMapping: {[T in string]?: number} = {} + type OneOfMapping = Map + + function getMapping(): OneOfMapping { + const oneOfMapping: OneOfMapping = new Map() const topRequired = hasRequired(parentSchema) let tagRequired = true for (let i = 0; i < oneOf.length; i++) { @@ -78,7 +80,7 @@ const def: CodeKeywordDefinition = { } function addMappings(sch: AnySchemaObject, i: number): void { - if (sch.const) { + if (sch.const !== undefined) { addMapping(sch.const, i) } else if (sch.enum) { for (const tagValue of sch.enum) { @@ -89,11 +91,16 @@ const def: CodeKeywordDefinition = { } } - function addMapping(tagValue: unknown, i: number): void { - if (typeof tagValue != "string" || tagValue in oneOfMapping) { - throw new Error(`discriminator: "${tagName}" values must be unique strings`) + function addMapping(tagValue: any, i: number): void { + if ( + !["string", "number", "boolean"].includes(typeof tagValue) || + oneOfMapping.has(tagValue) + ) { + throw new Error( + `discriminator: "${tagName}" values must be unique strings, numbers or booleans` + ) } - oneOfMapping[tagValue] = i + oneOfMapping.set(tagValue, i) } } }, diff --git a/spec/discriminator.spec.ts b/spec/discriminator.spec.ts index cf7843880..8585833ef 100644 --- a/spec/discriminator.spec.ts +++ b/spec/discriminator.spec.ts @@ -23,7 +23,7 @@ describe("discriminator keyword", function () { } describe("validation", () => { - const schema1 = { + const stringSchema1 = { type: "object", discriminator: {propertyName: "foo"}, oneOf: [ @@ -44,7 +44,7 @@ describe("discriminator keyword", function () { ], } - const schema2 = { + const stringSchema2 = { type: "object", discriminator: {propertyName: "foo"}, required: ["foo"], @@ -66,18 +66,139 @@ describe("discriminator keyword", function () { ], } - const schemas = [schema1, schema2] - - it("should validate data", () => { - assertValid(schemas, {foo: "x", a: "a"}) - assertValid(schemas, {foo: "y", b: "b"}) - assertValid(schemas, {foo: "z", b: "b"}) - assertInvalid(schemas, {}) - assertInvalid(schemas, {foo: 1}) - assertInvalid(schemas, {foo: "bar"}) - assertInvalid(schemas, {foo: "x", b: "b"}) - assertInvalid(schemas, {foo: "y", a: "a"}) - assertInvalid(schemas, {foo: "z", a: "a"}) + const numberSchema = { + type: "object", + discriminator: {propertyName: "foo"}, + required: ["foo"], + oneOf: [ + { + properties: { + foo: {const: 1}, + a: {type: "string"}, + }, + required: ["a"], + }, + { + properties: { + foo: {enum: [2, 3]}, + b: {type: "string"}, + }, + required: ["b"], + }, + ], + } + + const boolSchema = { + type: "object", + discriminator: {propertyName: "foo"}, + required: ["foo"], + oneOf: [ + { + properties: { + foo: {const: true}, + a: {type: "string"}, + }, + required: ["a"], + }, + { + properties: { + foo: {const: false}, + b: {type: "string"}, + }, + required: ["b"], + }, + ], + } + + const mixedSchema = { + type: "object", + discriminator: {propertyName: "foo"}, + required: ["foo"], + oneOf: [ + { + properties: { + foo: {const: "x"}, + a: {type: "string"}, + }, + required: ["a"], + }, + { + properties: { + foo: {enum: [1, 2]}, + b: {type: "string"}, + }, + required: ["b"], + }, + { + properties: { + foo: {const: true}, + c: {type: "string"}, + }, + required: ["c"], + }, + ], + } + + const stringSchemas = [stringSchema1, stringSchema2] + const numberSchemas = [numberSchema] + const boolSchemas = [boolSchema] + const mixedSchemas = [mixedSchema] + + it("should validate data for string discriminator", () => { + assertValid(stringSchemas, {foo: "x", a: "a"}) + assertValid(stringSchemas, {foo: "y", b: "b"}) + assertValid(stringSchemas, {foo: "z", b: "b"}) + + assertInvalid(stringSchemas, {}) + assertInvalid(stringSchemas, {foo: 1}) + assertInvalid(stringSchemas, {foo: "bar"}) + assertInvalid(stringSchemas, {foo: "x", b: "b"}) + assertInvalid(stringSchemas, {foo: "y", a: "a"}) + assertInvalid(stringSchemas, {foo: "z", a: "a"}) + }) + + it("should validate data for number discriminator", () => { + assertValid(numberSchemas, {foo: 1, a: "a"}) + assertValid(numberSchemas, {foo: 2, b: "b"}) + assertValid(numberSchemas, {foo: 3, b: "b"}) + + assertInvalid(stringSchemas, {}) + assertInvalid(stringSchemas, {foo: "1"}) + assertInvalid(stringSchemas, {foo: "bar"}) + assertInvalid(numberSchemas, {foo: 1, b: "b"}) + assertInvalid(numberSchemas, {foo: 2, a: "a"}) + assertInvalid(numberSchemas, {foo: 3, a: "a"}) + }) + + it("should validate data for boolean discriminator", () => { + assertValid(boolSchemas, {foo: true, a: "a"}) + assertValid(boolSchemas, {foo: false, b: "b"}) + + assertInvalid(boolSchemas, {}) + assertInvalid(boolSchemas, {foo: "1"}) + assertInvalid(boolSchemas, {foo: true, b: "b"}) + assertInvalid(boolSchemas, {foo: false, a: "a"}) + }) + + it("should validate data for mixed type discriminator", () => { + assertValid(mixedSchemas, {foo: "x", a: "a"}) + assertValid(mixedSchemas, {foo: 1, b: "b"}) + assertValid(mixedSchemas, {foo: 2, b: "b"}) + assertValid(mixedSchemas, {foo: true, c: "c"}) + + assertInvalid(mixedSchemas, {}) + assertInvalid(mixedSchemas, {foo: "x"}) + assertInvalid(mixedSchemas, {foo: "x", b: "b"}) + assertInvalid(mixedSchemas, {foo: "x", c: "c"}) + assertInvalid(mixedSchemas, {foo: 1}) + assertInvalid(mixedSchemas, {foo: 1, a: "a"}) + assertInvalid(mixedSchemas, {foo: 1, c: "c"}) + assertInvalid(mixedSchemas, {foo: 2}) + assertInvalid(mixedSchemas, {foo: 2, a: "a"}) + assertInvalid(mixedSchemas, {foo: 2, c: "c"}) + assertInvalid(mixedSchemas, {foo: true}) + assertInvalid(mixedSchemas, {foo: true, a: "a"}) + assertInvalid(mixedSchemas, {foo: true, b: "b"}) }) }) @@ -113,15 +234,15 @@ describe("discriminator keyword", function () { ) }) - it("tag value should be string", () => { + it("tag value should be string, number or boolean", () => { invalidSchema( { type: "object", discriminator: {propertyName: "foo"}, required: ["foo"], - oneOf: [{properties: {foo: {const: 1}}}], + oneOf: [{properties: {foo: {const: {baz: "bar"}}}}], }, - /discriminator: "foo" values must be unique strings/ + /discriminator: "foo" values must be unique strings, numbers or booleans/ ) }) @@ -133,7 +254,27 @@ describe("discriminator keyword", function () { required: ["foo"], oneOf: [{properties: {foo: {const: "a"}}}, {properties: {foo: {const: "a"}}}], }, - /discriminator: "foo" values must be unique strings/ + /discriminator: "foo" values must be unique strings, numbers or booleans/ + ) + + invalidSchema( + { + type: "object", + discriminator: {propertyName: "foo"}, + required: ["foo"], + oneOf: [{properties: {foo: {const: 1}}}, {properties: {foo: {const: 1}}}], + }, + /discriminator: "foo" values must be unique/ + ) + + invalidSchema( + { + type: "object", + discriminator: {propertyName: "foo"}, + required: ["foo"], + oneOf: [{properties: {foo: {const: true}}}, {properties: {foo: {const: true}}}], + }, + /discriminator: "foo" values must be unique/ ) }) @@ -154,7 +295,11 @@ describe("discriminator keyword", function () { function assertValid(schemas: SchemaObject[], data: unknown): void { schemas.forEach((schema) => - ajvs.forEach((ajv) => assert.strictEqual(ajv.validate(schema, data), true)) + ajvs.forEach((ajv) => { + const validate = ajv.compile(schema) + const valid = validate(data) + assert.strictEqual(valid, true) + }) ) } From 85d65f917eb5fcd12285ec99c189f160c4742d8e Mon Sep 17 00:00:00 2001 From: Piotr Kuczynski Date: Wed, 26 Oct 2022 22:14:24 +0200 Subject: [PATCH 2/2] apply code feedback --- lib/vocabularies/discriminator/index.ts | 11 ++++++----- spec/discriminator.spec.ts | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/vocabularies/discriminator/index.ts b/lib/vocabularies/discriminator/index.ts index c3bc87326..aa7b0ec44 100644 --- a/lib/vocabularies/discriminator/index.ts +++ b/lib/vocabularies/discriminator/index.ts @@ -1,6 +1,6 @@ import type {CodeKeywordDefinition, AnySchemaObject, KeywordErrorDefinition} from "../../types" import type {KeywordCxt} from "../../compile/validate" -import {_, getProperty, Name} from "../../compile/codegen" +import {_, or, getProperty, Name} from "../../compile/codegen" import {DiscrError, DiscrErrorObj} from "../discriminator/types" import {resolveRef, SchemaEnv} from "../../compile" import {schemaHasRulesButRef} from "../../compile/util" @@ -33,8 +33,9 @@ const def: CodeKeywordDefinition = { if (!oneOf) throw new Error("discriminator: requires oneOf keyword") const valid = gen.let("valid", false) const tag = gen.const("tag", _`${data}${getProperty(tagName)}`) + const tagTypes = ["string", "number", "boolean"] gen.if( - _`["string", "number", "boolean"].includes(typeof ${tag})`, + or(...tagTypes.map((t) => _`typeof ${tag} == ${t}`), _`${tag} == null`), () => validateMapping(), () => cxt.error(false, {discrError: DiscrError.Tag, tag, tagName}) ) @@ -59,7 +60,7 @@ const def: CodeKeywordDefinition = { return _valid } - type OneOfMapping = Map + type OneOfMapping = Map function getMapping(): OneOfMapping { const oneOfMapping: OneOfMapping = new Map() @@ -101,11 +102,11 @@ const def: CodeKeywordDefinition = { function addMapping(tagValue: any, i: number): void { if ( - !["string", "number", "boolean"].includes(typeof tagValue) || + !(["string", "number", "boolean"].includes(typeof tagValue) || tagValue === null) || oneOfMapping.has(tagValue) ) { throw new Error( - `discriminator: "${tagName}" values must be unique strings, numbers or booleans` + `discriminator: "${tagName}" values must be unique strings, numbers, booleans or nulls` ) } oneOfMapping.set(tagValue, i) diff --git a/spec/discriminator.spec.ts b/spec/discriminator.spec.ts index 35e78d188..ed50be95b 100644 --- a/spec/discriminator.spec.ts +++ b/spec/discriminator.spec.ts @@ -136,6 +136,13 @@ describe("discriminator keyword", function () { }, required: ["c"], }, + { + properties: { + foo: {const: null}, + d: {type: "number"}, + }, + required: ["d"], + }, ], } @@ -185,6 +192,7 @@ describe("discriminator keyword", function () { assertValid(mixedSchemas, {foo: 1, b: "b"}) assertValid(mixedSchemas, {foo: 2, b: "b"}) assertValid(mixedSchemas, {foo: true, c: "c"}) + assertValid(mixedSchemas, {foo: null, d: 123}) assertInvalid(mixedSchemas, {}) assertInvalid(mixedSchemas, {foo: "x"}) @@ -199,6 +207,10 @@ describe("discriminator keyword", function () { assertInvalid(mixedSchemas, {foo: true}) assertInvalid(mixedSchemas, {foo: true, a: "a"}) assertInvalid(mixedSchemas, {foo: true, b: "b"}) + assertInvalid(mixedSchemas, {foo: null}) + assertInvalid(mixedSchemas, {foo: null, a: "a"}) + assertInvalid(mixedSchemas, {foo: null, b: "b"}) + assertInvalid(mixedSchemas, {foo: null, c: "c"}) }) }) @@ -394,7 +406,7 @@ describe("discriminator keyword", function () { ) }) - it("tag value should be string, number or boolean", () => { + it("tag value should be string, number, boolean or null", () => { invalidSchema( { type: "object", @@ -402,7 +414,7 @@ describe("discriminator keyword", function () { required: ["foo"], oneOf: [{properties: {foo: {const: {baz: "bar"}}}}], }, - /discriminator: "foo" values must be unique strings, numbers or booleans/ + /discriminator: "foo" values must be unique strings, numbers, booleans or nulls/ ) }) @@ -414,7 +426,7 @@ describe("discriminator keyword", function () { required: ["foo"], oneOf: [{properties: {foo: {const: "a"}}}, {properties: {foo: {const: "a"}}}], }, - /discriminator: "foo" values must be unique strings, numbers or booleans/ + /discriminator: "foo" values must be unique strings, numbers, booleans or nulls/ ) invalidSchema(