Skip to content

Commit

Permalink
Add isConfigSchema typeguard and stop using instanceof Type checks …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
pgayvallet authored Apr 17, 2020
1 parent 3f98f0f commit c4ddd00
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 6 deletions.
1 change: 1 addition & 0 deletions packages/kbn-config-schema/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>) {
return new AnyType(options);
Expand Down
20 changes: 20 additions & 0 deletions packages/kbn-config-schema/src/typeguards/index.ts
Original file line number Diff line number Diff line change
@@ -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';
56 changes: 56 additions & 0 deletions packages/kbn-config-schema/src/typeguards/is_config_schema.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
24 changes: 24 additions & 0 deletions packages/kbn-config-schema/src/typeguards/is_config_schema.ts
Original file line number Diff line number Diff line change
@@ -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<any> {
return obj ? obj.__isKbnConfigSchemaType === true : false;
}
3 changes: 3 additions & 0 deletions packages/kbn-config-schema/src/types/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ export abstract class Type<V> {
// 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}
Expand Down
4 changes: 2 additions & 2 deletions src/core/server/http/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -139,7 +139,7 @@ function routeSchemasFromRouteConfig<P, Q, B>(

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}].`
);
Expand Down
4 changes: 2 additions & 2 deletions src/core/server/http/router/validator/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -236,7 +236,7 @@ export class RouteValidator<P = {}, Q = {}, B = {}> {
data?: unknown,
namespace?: string
): RouteValidationResultType<typeof validationRule> {
if (validationRule instanceof Type) {
if (isConfigSchema(validationRule)) {
return validationRule.validate(data, {}, namespace);
} else if (typeof validationRule === 'function') {
return this.validateFunction(validationRule, data, namespace);
Expand Down
4 changes: 2 additions & 2 deletions src/core/server/plugins/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit c4ddd00

Please sign in to comment.