Skip to content

Commit

Permalink
fix: fixes #3692 - simple discriminator now bypasses isValid() calls …
Browse files Browse the repository at this point in the history
…in getMatchingOption and getClosestMatchingOption for improved performance (#3845)

* fix: fixes #3692 - simple discriminator now bypasses isValid() calls in getMatchingOption and getClosestMatchingOption.ts for improved performance

* chore: update changelog

* docs: add jsdoc to getOptionMatchingSimpleDiscriminator

* test: add missing test for getFirstMatchingOption non-trivial discriminator
  • Loading branch information
michal-kurz authored Oct 5, 2023
1 parent 1576c27 commit 208b148
Show file tree
Hide file tree
Showing 8 changed files with 298 additions and 93 deletions.
192 changes: 100 additions & 92 deletions CHANGELOG.md

Large diffs are not rendered by default.

16 changes: 16 additions & 0 deletions packages/docs/docs/api-reference/utility-functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,22 @@ Using the `schema`, `defaultType` and `options`, extract out the props for the `

- InputPropsType: The extracted `InputPropsType` object

### getOptionMatchingSimpleDiscriminator()

Compares the value of `discriminatorField` within `formData` against the value of `discriminatorField` within schema for each `option`. Returns index of first `option` whose discriminator matches formData. Returns `undefined` if there is no match.

This function does not work with discriminators of `"type": "object"` and `"type": "array"`

#### Parameters

- [formData]: T | undefined - The current formData, if any, used to figure out a match
- options: S[] - The list of options to find a matching options from
- [discriminatorField]: string | undefined - The optional name of the field within the options object whose value is used to determine which option is selected

#### Returns

- number | undefined: index of the matched option

### getSchemaType()

Gets the type of a given `schema`.
Expand Down
46 changes: 46 additions & 0 deletions packages/utils/src/getOptionMatchingSimpleDiscriminator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import get from 'lodash/get';
import { PROPERTIES_KEY } from './constants';
import { RJSFSchema, StrictRJSFSchema } from './types';

/** Compares the value of `discriminatorField` within `formData` against the value of `discriminatorField` within schema for each `option`.
* Returns index of first `option` whose discriminator matches formData. Returns `undefined` if there is no match.
* This function does not work with discriminators of `"type": "object"` and `"type": "array"`
*
* @param formData - The current formData, if any, used to figure out a match
* @param options - The list of options to find a matching options from
* @param [discriminatorField] - The optional name of the field within the options object whose value is used to
* determine which option is selected
* @returns - The index of the matched option or undefined if there is no match
*/
export default function getOptionMatchingSimpleDiscriminator<T = any, S extends StrictRJSFSchema = RJSFSchema>(
formData: T | undefined,
options: S[],
discriminatorField?: string
): number | undefined {
if (formData && discriminatorField) {
const value = get(formData, discriminatorField);

if (value === undefined) {
return;
}

for (let i = 0; i < options.length; i++) {
const option = options[i];
const discriminator = get(option, [PROPERTIES_KEY, discriminatorField], {});

if (discriminator.type === 'object' || discriminator.type === 'array') {
continue;
}

if (discriminator.const === value) {
return i;
}

if (discriminator.enum?.includes(value)) {
return i;
}
}
}

return;
}
2 changes: 2 additions & 0 deletions packages/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import unwrapErrorHandler from './unwrapErrorHandler';
import utcToLocal from './utcToLocal';
import validationDataMerge from './validationDataMerge';
import withIdRefPrefix from './withIdRefPrefix';
import getOptionMatchingSimpleDiscriminator from './getOptionMatchingSimpleDiscriminator';

export * from './types';
export * from './enums';
Expand Down Expand Up @@ -79,6 +80,7 @@ export {
findSchemaDefinition,
getDiscriminatorFieldFromSchema,
getInputProps,
getOptionMatchingSimpleDiscriminator,
getSchemaType,
getSubmitButtonOptions,
getTemplate,
Expand Down
8 changes: 8 additions & 0 deletions packages/utils/src/schema/getClosestMatchingOption.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import get from 'lodash/get';
import has from 'lodash/has';
import isNumber from 'lodash/isNumber';
import isObject from 'lodash/isObject';
import isString from 'lodash/isString';
import reduce from 'lodash/reduce';
Expand All @@ -11,6 +12,7 @@ import { ONE_OF_KEY, REF_KEY, JUNK_OPTION_ID, ANY_OF_KEY } from '../constants';
import guessType from '../guessType';
import { FormContextType, RJSFSchema, StrictRJSFSchema, ValidatorType } from '../types';
import getDiscriminatorFieldFromSchema from '../getDiscriminatorFieldFromSchema';
import getOptionMatchingSimpleDiscriminator from '../getOptionMatchingSimpleDiscriminator';

/** A junk option used to determine when the getFirstMatchingOption call really matches an option rather than returning
* the first item
Expand Down Expand Up @@ -147,6 +149,12 @@ export default function getClosestMatchingOption<
const resolvedOptions = options.map((option) => {
return resolveAllReferences(option, rootSchema);
});

const simpleDiscriminatorMatch = getOptionMatchingSimpleDiscriminator(formData, options, discriminatorField);
if (isNumber(simpleDiscriminatorMatch)) {
return simpleDiscriminatorMatch;
}

// Reduce the array of options down to a list of the indexes that are considered matching options
const allValidIndexes = resolvedOptions.reduce((validList: number[], option, index: number) => {
const testOptions: S[] = [JUNK_OPTION as S, option];
Expand Down
8 changes: 8 additions & 0 deletions packages/utils/src/schema/getMatchingOption.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import get from 'lodash/get';
import has from 'lodash/has';
import isNumber from 'lodash/isNumber';

import { PROPERTIES_KEY } from '../constants';
import { FormContextType, RJSFSchema, StrictRJSFSchema, ValidatorType } from '../types';
import getOptionMatchingSimpleDiscriminator from '../getOptionMatchingSimpleDiscriminator';

/** Given the `formData` and list of `options`, attempts to find the index of the option that best matches the data.
* Deprecated, use `getFirstMatchingOption()` instead.
Expand Down Expand Up @@ -32,6 +34,12 @@ export default function getMatchingOption<
if (formData === undefined) {
return 0;
}

const simpleDiscriminatorMatch = getOptionMatchingSimpleDiscriminator(formData, options, discriminatorField);
if (isNumber(simpleDiscriminatorMatch)) {
return simpleDiscriminatorMatch;
}

for (let i = 0; i < options.length; i++) {
const option = options[i];

Expand Down
81 changes: 81 additions & 0 deletions packages/utils/test/getOptionMatchingSimpleDiscriminator.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import getOptionMatchingSimpleDiscriminator from '../src/getOptionMatchingSimpleDiscriminator';

describe('getOptionMatchingSimpleDiscriminator()', () => {
describe('returns undefined if no option matches discriminator', () => {
test('no options with no data', () => {
expect(getOptionMatchingSimpleDiscriminator({}, [], 'id')).toEqual(undefined);
});

test('no options with data', () => {
expect(getOptionMatchingSimpleDiscriminator({ foo: 'foo' }, [], 'id')).toEqual(undefined);
});

test('options with no data', () => {
expect(
getOptionMatchingSimpleDiscriminator({}, [{ type: 'object', properties: { foo: { const: 'foo' } } }], 'id')
).toEqual(undefined);
});

test('matching property, but no discriminatorField', () => {
expect(
getOptionMatchingSimpleDiscriminator({ foo: 'foo' }, [
{ type: 'object', properties: { foo: { const: 'foo' } } },
])
).toEqual(undefined);
});

test('matching property different from discriminatorField', () => {
expect(
getOptionMatchingSimpleDiscriminator(
{ foo: 'foo' },
[{ type: 'object', properties: { foo: { const: 'foo' } } }],
'bar'
)
).toEqual(undefined);
});
});

describe('returns option index if option matches discriminator', () => {
test('const discriminator', () => {
expect(
getOptionMatchingSimpleDiscriminator(
{ foo: 'foo' },
[{}, { type: 'object', properties: { foo: { const: 'foo' } } }],
'foo'
)
).toEqual(1);
});

test('enum discriminator', () => {
expect(
getOptionMatchingSimpleDiscriminator(
{ foo: 'foo' },
[{}, { type: 'object', properties: { foo: { enum: ['bar', 'foo'] } } }],
'foo'
)
).toEqual(1);
});
});

describe('unsupported (non-simple) discriminator returns undefined', () => {
test('object discriminator', () => {
expect(
getOptionMatchingSimpleDiscriminator(
{ foo: 'foo' },
[{}, { type: 'object', properties: { foo: { type: 'object' } } }],
'foo'
)
).toEqual(undefined);
});

test('array discriminator', () => {
expect(
getOptionMatchingSimpleDiscriminator(
{ foo: 'foo' },
[{}, { type: 'object', properties: { foo: { type: 'array' } } }],
'foo'
)
).toEqual(undefined);
});
});
});
38 changes: 37 additions & 1 deletion packages/utils/test/schema/getFirstMatchingOptionTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ export default function getFirstMatchingOptionTest(testValidator: TestValidatorT
const options = [schema.definitions!.Foo, schema.definitions!.Bar] as RJSFSchema[];
expect(getFirstMatchingOption(testValidator, null, options, schema, 'code')).toEqual(0);
});
it('should return Bar when schema has discriminator for bar', () => {

// simple in the sense of getOptionMatchingSimpleDiscriminator
it('should return Bar when schema has simple discriminator for bar', () => {
// Mock isValid to pass the second value
testValidator.setReturnValues({ isValid: [false, true] });
const schema: RJSFSchema = {
Expand Down Expand Up @@ -167,5 +169,39 @@ export default function getFirstMatchingOptionTest(testValidator: TestValidatorT
const schemaUtils = createSchemaUtils(testValidator, schema);
expect(schemaUtils.getFirstMatchingOption(formData, options, 'code')).toEqual(1);
});

// simple in the sense of getOptionMatchingSimpleDiscriminator
it('should return Bar when schema has non-simple discriminator for bar', () => {
// Mock isValid to pass the second value
testValidator.setReturnValues({ isValid: [false, true] });
const schema: RJSFSchema = {
type: 'object',
definitions: {
Foo: {
title: 'Foo',
type: 'object',
properties: {
code: { title: 'Code', type: 'array', items: { type: 'string', enum: ['foo_coding'] } },
},
},
Bar: {
title: 'Bar',
type: 'object',
properties: {
code: { title: 'Code', type: 'array', items: { type: 'string', enum: ['bar_coding'] } },
},
},
},
discriminator: {
propertyName: 'code',
},
oneOf: [{ $ref: '#/definitions/Foo' }, { $ref: '#/definitions/Bar' }],
};
const formData = { code: ['bar_coding'] };
const options = [schema.definitions!.Foo, schema.definitions!.Bar] as RJSFSchema[];
// Use the schemaUtils to verify the discriminator prop gets passed
const schemaUtils = createSchemaUtils(testValidator, schema);
expect(schemaUtils.getFirstMatchingOption(formData, options, 'code')).toEqual(1);
});
});
}

0 comments on commit 208b148

Please sign in to comment.