Skip to content

Commit 91d11f9

Browse files
committed
Fix recursive conditional restriction parsing and add tests
1 parent cfc8cd7 commit 91d11f9

File tree

7 files changed

+239
-104
lines changed

7 files changed

+239
-104
lines changed

packages/dictionary/src/metaSchema/dictionarySchemas.ts

+16-6
Original file line numberDiff line numberDiff line change
@@ -66,47 +66,57 @@ export type SchemaFieldValueType = zod.infer<typeof SchemaFieldValueType>;
6666
/* ****************************** *
6767
* Field Type Restriction Objects *
6868
* ****************************** */
69-
export const BooleanFieldRestrictions = zod.object({ required: zod.boolean() }).partial();
69+
export const BooleanFieldRestrictions = zod.object({ empty: zod.boolean(), required: zod.boolean() }).partial();
7070
export type BooleanFieldRestrictions = zod.infer<typeof BooleanFieldRestrictions>;
7171

72-
const BooleanFieldRestrictionsObject = BooleanFieldRestrictions.or(ConditionalRestriction(BooleanFieldRestrictions));
72+
const BooleanFieldRestrictionsObject = BooleanFieldRestrictions.strict().or(
73+
ConditionalRestriction(BooleanFieldRestrictions.strict()),
74+
);
7375
export type BooleanFieldRestrictionsObject = zod.infer<typeof BooleanFieldRestrictionsObject>;
7476

7577
export const IntegerFieldRestrictions = zod
7678
.object({
7779
codeList: RestrictionCodeListInteger.or(ReferenceTag),
80+
empty: zod.boolean(),
7881
required: zod.boolean(),
7982
range: RestrictionIntegerRange,
8083
})
8184
.partial();
8285
export type IntegerFieldRestrictions = zod.infer<typeof IntegerFieldRestrictions>;
8386

84-
const IntegerFieldRestrictionsObject = IntegerFieldRestrictions.or(ConditionalRestriction(IntegerFieldRestrictions));
87+
const IntegerFieldRestrictionsObject = IntegerFieldRestrictions.strict().or(
88+
ConditionalRestriction(IntegerFieldRestrictions.strict()),
89+
);
8590
export type IntegerFieldRestrictionsObject = zod.infer<typeof IntegerFieldRestrictionsObject>;
8691

8792
export const NumberFieldRestrictions = zod
8893
.object({
8994
codeList: RestrictionCodeListNumber.or(ReferenceTag),
95+
empty: zod.boolean(),
9096
required: zod.boolean(),
9197
range: RestrictionNumberRange,
9298
})
9399
.partial();
94100
export type NumberFieldRestrictions = zod.infer<typeof NumberFieldRestrictions>;
95101

96-
const NumberFieldRestrictionsObject = NumberFieldRestrictions.or(ConditionalRestriction(NumberFieldRestrictions));
102+
const NumberFieldRestrictionsObject = NumberFieldRestrictions.strict().or(
103+
ConditionalRestriction(NumberFieldRestrictions.strict()),
104+
);
97105
export type NumberFieldRestrictionsObject = zod.infer<typeof NumberFieldRestrictionsObject>;
98106

99107
export const StringFieldRestrictions = zod
100108
.object({
101109
codeList: RestrictionCodeListString.or(ReferenceTag),
110+
empty: zod.boolean(),
102111
required: zod.boolean(),
103112
regex: RestrictionRegex.or(ReferenceTag),
104-
// TODO: regex can be optionally be an array. this would simplify resolving references and allow multiple regex conditions in a single object
105113
})
106114
.partial();
107115
export type StringFieldRestrictions = zod.infer<typeof StringFieldRestrictions>;
108116

109-
const StringFieldRestrictionsObject = StringFieldRestrictions.or(ConditionalRestriction(StringFieldRestrictions));
117+
const StringFieldRestrictionsObject = StringFieldRestrictions.strict().or(
118+
ConditionalRestriction(StringFieldRestrictions.strict()),
119+
);
110120
export type StringFieldRestrictionsObject = zod.infer<typeof StringFieldRestrictionsObject>;
111121

112122
export const AnyFieldRestrictions = zod.union([

packages/dictionary/src/metaSchema/restrictionsSchemas.ts

+6-8
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export const Integer = zod.number().int();
2525

2626
export const FieldRestrictionTypes = {
2727
codeList: 'codeList',
28+
empty: 'empty',
2829
range: 'range',
2930
required: 'required',
3031
regex: 'regex',
@@ -33,9 +34,6 @@ export const FieldRestrictionTypes = {
3334
} as const;
3435
export type FieldRestrictionType = Values<typeof FieldRestrictionTypes>;
3536

36-
export const RestrictionScript = zod.array(zod.string().or(ReferenceTag)).min(1); //TODO: script formatting validation
37-
export type RestrictionScript = zod.infer<typeof RestrictionScript>;
38-
3937
export const RestrictionCodeListString = zod.union([zod.string(), ReferenceTag]).array().min(1);
4038
export type RestrictionCodeListString = zod.infer<typeof RestrictionCodeListString>;
4139

@@ -213,16 +211,16 @@ export type ConditionalRestriction<TRestrictionObject> = {
213211
| ConditionalRestriction<TRestrictionObject>
214212
| (TRestrictionObject | ConditionalRestriction<TRestrictionObject>)[];
215213
};
216-
export const ConditionalRestriction = <TRestrictionObject>(
217-
restrictionsSchema: ZodSchema<TRestrictionObject>,
218-
): ZodSchema<ConditionalRestriction<TRestrictionObject>> => {
214+
export const ConditionalRestriction = <TRestrictionObjectSchema extends zod.ZodTypeAny>(
215+
restrictionsSchema: TRestrictionObjectSchema,
216+
): ZodSchema<ConditionalRestriction<zod.infer<TRestrictionObjectSchema>>> => {
219217
const restrictionOrConditional = zod.union([
220218
restrictionsSchema,
221219
zod.lazy(() => ConditionalRestriction(restrictionsSchema)),
222220
]);
223221
return zod.object({
224222
if: ConditionalRestrictionTest,
225-
then: restrictionOrConditional.or(restrictionOrConditional).optional(),
226-
else: restrictionOrConditional.or(restrictionOrConditional).optional(),
223+
then: restrictionOrConditional.or(restrictionOrConditional.array()).optional(),
224+
else: restrictionOrConditional.or(restrictionOrConditional.array()).optional(),
227225
});
228226
};

packages/dictionary/test/dictionaryTypes.spec.ts packages/dictionary/test/metaSchema/dictionarySchemas.spec.ts

+2-88
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,9 @@
1818
*/
1919

2020
import { expect } from 'chai';
21-
import {
22-
BooleanFieldRestrictions,
23-
Dictionary,
24-
DictionaryMeta,
25-
Integer,
26-
IntegerFieldRestrictions,
27-
NameValue,
28-
NumberFieldRestrictions,
29-
RestrictionIntegerRange,
30-
RestrictionNumberRange,
31-
Schema,
32-
SchemaField,
33-
StringFieldRestrictions,
34-
VersionString,
35-
} from '../src';
21+
import { Dictionary, DictionaryMeta, NameValue, Schema, SchemaField, VersionString } from '../../src';
3622

37-
describe('Dictionary Types', () => {
23+
describe('Dictionary Schemas', () => {
3824
describe('NameValue', () => {
3925
it('Rejects empty string', () => {
4026
expect(NameValue.safeParse('').success).false;
@@ -54,78 +40,6 @@ describe('Dictionary Types', () => {
5440
});
5541
});
5642

57-
describe('Integer', () => {
58-
it("Can't be float", () => {
59-
expect(Integer.safeParse(1.3).success).false;
60-
expect(Integer.safeParse(2.0000001).success).false;
61-
// Note: float precision issues, if the float resolves to a whole number the value will be accepted.
62-
});
63-
it("Can't be string, boolean, object, array", () => {
64-
expect(Integer.safeParse('1').success).false;
65-
expect(Integer.safeParse(true).success).false;
66-
expect(Integer.safeParse([1]).success).false;
67-
expect(Integer.safeParse({}).success).false;
68-
expect(Integer.safeParse({ thing: 1 }).success).false;
69-
});
70-
it('Can be integer', () => {
71-
expect(Integer.safeParse(1).success).true;
72-
expect(Integer.safeParse(0).success).true;
73-
expect(Integer.safeParse(-1).success).true;
74-
expect(Integer.safeParse(1123).success).true;
75-
});
76-
});
77-
describe('RangeRestriction', () => {
78-
it("Integer Range Can't have exclusiveMin and Min", () => {
79-
expect(RestrictionIntegerRange.safeParse({ exclusiveMin: 0, min: 0 }).success).false;
80-
expect(RestrictionIntegerRange.safeParse({ min: 0 }).success).true;
81-
expect(RestrictionIntegerRange.safeParse({ exclusiveMin: 0 }).success).true;
82-
});
83-
it("Integer Range Can't have exclusiveMax and Max", () => {
84-
expect(RestrictionIntegerRange.safeParse({ exclusiveMax: 0, max: 0 }).success).false;
85-
expect(RestrictionIntegerRange.safeParse({ max: 0 }).success).true;
86-
expect(RestrictionIntegerRange.safeParse({ exclusiveMax: 0 }).success).true;
87-
});
88-
it("Number Range Can't have exclusiveMin and Min", () => {
89-
expect(RestrictionNumberRange.safeParse({ exclusiveMin: 0, min: 0 }).success).false;
90-
expect(RestrictionNumberRange.safeParse({ min: 0 }).success).true;
91-
expect(RestrictionNumberRange.safeParse({ exclusiveMin: 0 }).success).true;
92-
});
93-
it("Number Range Can't have exclusiveMax and Max", () => {
94-
expect(RestrictionNumberRange.safeParse({ exclusiveMax: 0, max: 0 }).success).false;
95-
expect(RestrictionNumberRange.safeParse({ max: 0 }).success).true;
96-
expect(RestrictionNumberRange.safeParse({ exclusiveMax: 0 }).success).true;
97-
});
98-
});
99-
describe('RegexRestriction', () => {
100-
it('Accepts valid regex', () => {
101-
expect(StringFieldRestrictions.safeParse({ regex: '[a-zA-Z]' }).success).true;
102-
expect(
103-
StringFieldRestrictions.safeParse({
104-
regex: '^({((([WUBRG]|([0-9]|[1-9][0-9]*))(/[WUBRG])?)|(X)|([WUBRG](/[WUBRG])?/[P]))})+$',
105-
}).success,
106-
).true;
107-
});
108-
it('Rejects invalid regex', () => {
109-
expect(StringFieldRestrictions.safeParse({ regex: '[' }).success).false;
110-
});
111-
});
112-
describe('UniqueRestriction', () => {
113-
it('All fields accept unique restriction', () => {
114-
expect(StringFieldRestrictions.safeParse({ unique: true }).success).true;
115-
expect(NumberFieldRestrictions.safeParse({ unique: true }).success).true;
116-
expect(IntegerFieldRestrictions.safeParse({ unique: true }).success).true;
117-
expect(BooleanFieldRestrictions.safeParse({ unique: true }).success).true;
118-
});
119-
});
120-
describe('ScriptRestriction', () => {
121-
it('All fields accept script restriction', () => {
122-
expect(StringFieldRestrictions.safeParse({ script: ['()=>true'] }).success).true;
123-
expect(NumberFieldRestrictions.safeParse({ script: ['()=>true'] }).success).true;
124-
expect(IntegerFieldRestrictions.safeParse({ script: ['()=>true'] }).success).true;
125-
expect(BooleanFieldRestrictions.safeParse({ script: ['()=>true'] }).success).true;
126-
});
127-
});
128-
12943
describe('Fields', () => {
13044
it('Can have no restrictions', () => {
13145
const fieldString: SchemaField = {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
/*
2+
* Copyright (c) 2023 The Ontario Institute for Cancer Research. All rights reserved
3+
*
4+
* This program and the accompanying materials are made available under the terms of
5+
* the GNU Affero General Public License v3.0. You should have received a copy of the
6+
* GNU Affero General Public License along with this program.
7+
* If not, see <http://www.gnu.org/licenses/>.
8+
*
9+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY
10+
* EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
11+
* OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT
12+
* SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
13+
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
14+
* TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
15+
* OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER
16+
* IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
17+
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
18+
*/
19+
20+
import { expect } from 'chai';
21+
import {
22+
BooleanFieldRestrictions,
23+
Integer,
24+
IntegerFieldRestrictions,
25+
NumberFieldRestrictions,
26+
RestrictionIntegerRange,
27+
RestrictionNumberRange,
28+
SchemaField,
29+
StringFieldRestrictions,
30+
type ConditionalRestriction,
31+
type SchemaStringField,
32+
type StringFieldRestrictionsObject,
33+
} from '../../src';
34+
import assert from 'assert';
35+
36+
describe('Restriction Schemas', () => {
37+
describe('Integer', () => {
38+
it("Can't be float", () => {
39+
expect(Integer.safeParse(1.3).success).false;
40+
expect(Integer.safeParse(2.0000001).success).false;
41+
// Note: float precision issues, if the float resolves to a whole number the value will be accepted.
42+
});
43+
it("Can't be string, boolean, object, array", () => {
44+
expect(Integer.safeParse('1').success).false;
45+
expect(Integer.safeParse(true).success).false;
46+
expect(Integer.safeParse([1]).success).false;
47+
expect(Integer.safeParse({}).success).false;
48+
expect(Integer.safeParse({ thing: 1 }).success).false;
49+
});
50+
it('Can be integer', () => {
51+
expect(Integer.safeParse(1).success).true;
52+
expect(Integer.safeParse(0).success).true;
53+
expect(Integer.safeParse(-1).success).true;
54+
expect(Integer.safeParse(1123).success).true;
55+
});
56+
});
57+
describe('RangeRestriction', () => {
58+
it("Integer Range Can't have exclusiveMin and Min", () => {
59+
expect(RestrictionIntegerRange.safeParse({ exclusiveMin: 0, min: 0 }).success).false;
60+
expect(RestrictionIntegerRange.safeParse({ min: 0 }).success).true;
61+
expect(RestrictionIntegerRange.safeParse({ exclusiveMin: 0 }).success).true;
62+
});
63+
it("Integer Range Can't have exclusiveMax and Max", () => {
64+
expect(RestrictionIntegerRange.safeParse({ exclusiveMax: 0, max: 0 }).success).false;
65+
expect(RestrictionIntegerRange.safeParse({ max: 0 }).success).true;
66+
expect(RestrictionIntegerRange.safeParse({ exclusiveMax: 0 }).success).true;
67+
});
68+
it("Number Range Can't have exclusiveMin and Min", () => {
69+
expect(RestrictionNumberRange.safeParse({ exclusiveMin: 0, min: 0 }).success).false;
70+
expect(RestrictionNumberRange.safeParse({ min: 0 }).success).true;
71+
expect(RestrictionNumberRange.safeParse({ exclusiveMin: 0 }).success).true;
72+
});
73+
it("Number Range Can't have exclusiveMax and Max", () => {
74+
expect(RestrictionNumberRange.safeParse({ exclusiveMax: 0, max: 0 }).success).false;
75+
expect(RestrictionNumberRange.safeParse({ max: 0 }).success).true;
76+
expect(RestrictionNumberRange.safeParse({ exclusiveMax: 0 }).success).true;
77+
});
78+
});
79+
describe('RegexRestriction', () => {
80+
it('Accepts valid regex', () => {
81+
expect(StringFieldRestrictions.safeParse({ regex: '[a-zA-Z]' }).success).true;
82+
expect(
83+
StringFieldRestrictions.safeParse({
84+
regex: '^({((([WUBRG]|([0-9]|[1-9][0-9]*))(/[WUBRG])?)|(X)|([WUBRG](/[WUBRG])?/[P]))})+$',
85+
}).success,
86+
).true;
87+
});
88+
it('Rejects invalid regex', () => {
89+
expect(StringFieldRestrictions.safeParse({ regex: '[' }).success).false;
90+
});
91+
});
92+
describe('UniqueRestriction', () => {
93+
it('All fields accept unique restriction', () => {
94+
expect(StringFieldRestrictions.safeParse({ unique: true }).success).true;
95+
expect(NumberFieldRestrictions.safeParse({ unique: true }).success).true;
96+
expect(IntegerFieldRestrictions.safeParse({ unique: true }).success).true;
97+
expect(BooleanFieldRestrictions.safeParse({ unique: true }).success).true;
98+
});
99+
});
100+
describe('ScriptRestriction', () => {
101+
it('All fields accept script restriction', () => {
102+
expect(StringFieldRestrictions.safeParse({ script: ['()=>true'] }).success).true;
103+
expect(NumberFieldRestrictions.safeParse({ script: ['()=>true'] }).success).true;
104+
expect(IntegerFieldRestrictions.safeParse({ script: ['()=>true'] }).success).true;
105+
expect(BooleanFieldRestrictions.safeParse({ script: ['()=>true'] }).success).true;
106+
});
107+
});
108+
109+
describe('ConditionalRestrictions', () => {
110+
// These parsing functions seem unnecessary but they are checking for a failure case that was found:
111+
// The restrictions property is a union between a restrictions object and conditional restriction schema,
112+
// and the restriction object has all optional fields, so it will match with a conditional restriction
113+
// object successfully and strip out the if/then/else properties. To avoid this scenario, the RestrictionObject
114+
// schemas make the restriction validation `strict()`. These parsing tests are ensuring that this behaviour
115+
// is not changed.
116+
117+
it('Parses single conditional restriction', () => {
118+
const restrictions: ConditionalRestriction<StringFieldRestrictions> = {
119+
if: {
120+
conditions: [{ fields: ['another-field'], match: { value: 'asdf' } }],
121+
},
122+
then: { required: true },
123+
else: { empty: true },
124+
};
125+
const field: SchemaStringField = {
126+
name: 'example-string',
127+
valueType: 'string',
128+
restrictions,
129+
};
130+
const parseResult = SchemaField.safeParse(field);
131+
expect(parseResult.success).true;
132+
assert(parseResult.success === true);
133+
134+
expect(parseResult.data).deep.equal(field);
135+
});
136+
});
137+
it('Parses conditional restrictions in an array', () => {
138+
const restrictions: Array<StringFieldRestrictionsObject> = [
139+
{ codeList: ['value1', 'value2'] },
140+
{
141+
if: {
142+
conditions: [{ fields: ['another-field'], match: { value: 'asdf' } }],
143+
},
144+
then: { required: true },
145+
else: { empty: true },
146+
},
147+
];
148+
const field: SchemaStringField = {
149+
name: 'example-string',
150+
valueType: 'string',
151+
restrictions,
152+
};
153+
const parseResult = SchemaField.safeParse(field);
154+
expect(parseResult.success).true;
155+
assert(parseResult.success === true);
156+
157+
expect(parseResult.data).deep.equal(field);
158+
});
159+
160+
it('Parses nested conditional restrictions', () => {
161+
const restrictions: Array<StringFieldRestrictionsObject> = [
162+
{ codeList: ['value1', 'value2'] },
163+
{
164+
if: {
165+
conditions: [{ fields: ['first-dependent-field'], match: { value: 'asdf' } }],
166+
},
167+
then: {
168+
if: {
169+
conditions: [{ fields: ['second-dependent-field'], match: { range: { max: 10, min: 0 } } }],
170+
},
171+
then: { required: true },
172+
else: { empty: true },
173+
},
174+
else: { empty: true },
175+
},
176+
];
177+
const field: SchemaStringField = {
178+
name: 'example-string',
179+
valueType: 'string',
180+
restrictions,
181+
};
182+
const parseResult = SchemaField.safeParse(field);
183+
expect(parseResult.success).true;
184+
assert(parseResult.success === true);
185+
186+
expect(parseResult.data).deep.equal(field);
187+
});
188+
});

0 commit comments

Comments
 (0)