From c4ddd00540d838add7bdea9faf741ad4f288cca6 Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Fri, 17 Apr 2020 17:39:06 +0200 Subject: [PATCH] Add isConfigSchema typeguard and stop using `instanceof Type` checks in core (#63821) * add isConfigSchema type guard * replace instanceof checks with isConfigSchema * add dummy test plugin using a route with validation schema * remove `?.` prop access * remove test plugin * fix test description --- packages/kbn-config-schema/src/index.ts | 1 + .../kbn-config-schema/src/typeguards/index.ts | 20 +++++++ .../src/typeguards/is_config_schema.test.ts | 56 +++++++++++++++++++ .../src/typeguards/is_config_schema.ts | 24 ++++++++ packages/kbn-config-schema/src/types/type.ts | 3 + src/core/server/http/router/router.ts | 4 +- .../server/http/router/validator/validator.ts | 4 +- src/core/server/plugins/plugin.ts | 4 +- 8 files changed, 110 insertions(+), 6 deletions(-) create mode 100644 packages/kbn-config-schema/src/typeguards/index.ts create mode 100644 packages/kbn-config-schema/src/typeguards/is_config_schema.test.ts create mode 100644 packages/kbn-config-schema/src/typeguards/is_config_schema.ts diff --git a/packages/kbn-config-schema/src/index.ts b/packages/kbn-config-schema/src/index.ts index fc3e3c541846a..5d387f327e58f 100644 --- a/packages/kbn-config-schema/src/index.ts +++ b/packages/kbn-config-schema/src/index.ts @@ -60,6 +60,7 @@ import { export { ObjectType, TypeOf, Type }; export { ByteSizeValue } from './byte_size_value'; export { SchemaTypeError, ValidationError } from './errors'; +export { isConfigSchema } from './typeguards'; function any(options?: TypeOptions) { return new AnyType(options); diff --git a/packages/kbn-config-schema/src/typeguards/index.ts b/packages/kbn-config-schema/src/typeguards/index.ts new file mode 100644 index 0000000000000..e724878eb33e9 --- /dev/null +++ b/packages/kbn-config-schema/src/typeguards/index.ts @@ -0,0 +1,20 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export { isConfigSchema } from './is_config_schema'; diff --git a/packages/kbn-config-schema/src/typeguards/is_config_schema.test.ts b/packages/kbn-config-schema/src/typeguards/is_config_schema.test.ts new file mode 100644 index 0000000000000..e0ef3835ca0a3 --- /dev/null +++ b/packages/kbn-config-schema/src/typeguards/is_config_schema.test.ts @@ -0,0 +1,56 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { schema } from '..'; +import { isConfigSchema } from './is_config_schema'; + +describe('isConfigSchema', () => { + it('returns true for every sub classes of `Type`', () => { + expect(isConfigSchema(schema.any())).toBe(true); + expect(isConfigSchema(schema.arrayOf(schema.string()))).toBe(true); + expect(isConfigSchema(schema.boolean())).toBe(true); + expect(isConfigSchema(schema.buffer())).toBe(true); + expect(isConfigSchema(schema.byteSize())).toBe(true); + expect(isConfigSchema(schema.duration())).toBe(true); + expect(isConfigSchema(schema.literal(''))).toBe(true); + expect(isConfigSchema(schema.mapOf(schema.string(), schema.number()))).toBe(true); + expect(isConfigSchema(schema.nullable(schema.string()))).toBe(true); + expect(isConfigSchema(schema.number())).toBe(true); + expect(isConfigSchema(schema.object({}))).toBe(true); + expect(isConfigSchema(schema.oneOf([schema.string()]))).toBe(true); + expect(isConfigSchema(schema.recordOf(schema.string(), schema.object({})))).toBe(true); + expect(isConfigSchema(schema.string())).toBe(true); + expect(isConfigSchema(schema.stream())).toBe(true); + }); + + it('returns false for every javascript data type', () => { + expect(isConfigSchema('foo')).toBe(false); + expect(isConfigSchema(42)).toBe(false); + expect(isConfigSchema(new Date())).toBe(false); + expect(isConfigSchema(null)).toBe(false); + expect(isConfigSchema(undefined)).toBe(false); + expect(isConfigSchema([1, 2, 3])).toBe(false); + expect(isConfigSchema({ foo: 'bar' })).toBe(false); + expect(isConfigSchema(function() {})).toBe(false); + }); + + it('returns true as long as `__isKbnConfigSchemaType` is true', () => { + expect(isConfigSchema({ __isKbnConfigSchemaType: true })).toBe(true); + }); +}); diff --git a/packages/kbn-config-schema/src/typeguards/is_config_schema.ts b/packages/kbn-config-schema/src/typeguards/is_config_schema.ts new file mode 100644 index 0000000000000..20e68ab2ead25 --- /dev/null +++ b/packages/kbn-config-schema/src/typeguards/is_config_schema.ts @@ -0,0 +1,24 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Type } from '../types'; + +export function isConfigSchema(obj: any): obj is Type { + return obj ? obj.__isKbnConfigSchemaType === true : false; +} diff --git a/packages/kbn-config-schema/src/types/type.ts b/packages/kbn-config-schema/src/types/type.ts index 6d5ddf6b24afb..5ca16c61399e7 100644 --- a/packages/kbn-config-schema/src/types/type.ts +++ b/packages/kbn-config-schema/src/types/type.ts @@ -32,6 +32,9 @@ export abstract class Type { // sets the value to `null` while still keeping the type. public readonly type: V = null! as V; + // used for the `isConfigSchema` typeguard + public readonly __isKbnConfigSchemaType = true; + /** * Internal "schema" backed by Joi. * @type {Schema} diff --git a/src/core/server/http/router/router.ts b/src/core/server/http/router/router.ts index b4e7fc2a989b6..69402a74eda5f 100644 --- a/src/core/server/http/router/router.ts +++ b/src/core/server/http/router/router.ts @@ -20,7 +20,7 @@ import { Request, ResponseObject, ResponseToolkit } from 'hapi'; import Boom from 'boom'; -import { Type } from '@kbn/config-schema'; +import { isConfigSchema } from '@kbn/config-schema'; import { Logger } from '../../logging'; import { KibanaRequest } from './request'; import { KibanaResponseFactory, kibanaResponseFactory, IKibanaResponse } from './response'; @@ -139,7 +139,7 @@ function routeSchemasFromRouteConfig( if (route.validate !== false) { Object.entries(route.validate).forEach(([key, schema]) => { - if (!(schema instanceof Type || typeof schema === 'function')) { + if (!(isConfigSchema(schema) || typeof schema === 'function')) { throw new Error( `Expected a valid validation logic declared with '@kbn/config-schema' package or a RouteValidationFunction at key: [${key}].` ); diff --git a/src/core/server/http/router/validator/validator.ts b/src/core/server/http/router/validator/validator.ts index 97dd2bc894f81..6c766e69f0f37 100644 --- a/src/core/server/http/router/validator/validator.ts +++ b/src/core/server/http/router/validator/validator.ts @@ -17,7 +17,7 @@ * under the License. */ -import { ValidationError, Type, schema, ObjectType } from '@kbn/config-schema'; +import { ValidationError, Type, schema, ObjectType, isConfigSchema } from '@kbn/config-schema'; import { Stream } from 'stream'; import { RouteValidationError } from './validator_error'; @@ -236,7 +236,7 @@ export class RouteValidator

{ data?: unknown, namespace?: string ): RouteValidationResultType { - if (validationRule instanceof Type) { + if (isConfigSchema(validationRule)) { return validationRule.validate(data, {}, namespace); } else if (typeof validationRule === 'function') { return this.validateFunction(validationRule, data, namespace); diff --git a/src/core/server/plugins/plugin.ts b/src/core/server/plugins/plugin.ts index 7c67ab7a48df1..d7cfaa14d2343 100644 --- a/src/core/server/plugins/plugin.ts +++ b/src/core/server/plugins/plugin.ts @@ -21,7 +21,7 @@ import { join } from 'path'; import typeDetect from 'type-detect'; import { Subject } from 'rxjs'; import { first } from 'rxjs/operators'; -import { Type } from '@kbn/config-schema'; +import { isConfigSchema } from '@kbn/config-schema'; import { Logger } from '../logging'; import { @@ -150,7 +150,7 @@ export class PluginWrapper< } const configDescriptor = pluginDefinition.config; - if (!(configDescriptor.schema instanceof Type)) { + if (!isConfigSchema(configDescriptor.schema)) { throw new Error('Configuration schema expected to be an instance of Type'); } return configDescriptor;