[tslint] fix violations in kbn-system-loader (#19336)

* [tslint/kbn-system-loader] remove tslint overrides

* [tslint/kbn-system-loader] apply autofixes

* [tslint/kbn-system-loader] override max-classes-per-file for tests

* [tslint/kbn-system-loader] use I-prefix for interface names

* [tslint/kbn-system-loader] use comments to "fill" empty blocks

* [tslint/kbn-system-loader] use lowerCamelCase for variable/property names

* [tslint/kbn-system-loader] sort object keys alphabetically

* [tslint/kbn-system-loader] ensure that public methods come before private ones
This commit is contained in:
Spencer 2018-05-23 13:27:04 -07:00 committed by GitHub
parent 2d899275ef
commit 3d08c5cb31
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 175 additions and 162 deletions

View file

@ -1,16 +1,22 @@
/* tslint:disable max-classes-per-file */
import { System } from './system'; import { System } from './system';
import { KibanaSystem } from './system_types'; import { KibanaSystem } from './system_types';
test('can get exposed values after starting', () => { test('can get exposed values after starting', () => {
type CoreType = { bar: string }; interface ICoreType {
type DepsType = { quux: string }; bar: string;
type ExposedType = { }
core: CoreType; interface IDepsType {
deps: DepsType; quux: string;
}; }
interface IExposedType {
core: ICoreType;
deps: IDepsType;
}
class FooSystem extends KibanaSystem<CoreType, DepsType, ExposedType> { class FooSystem extends KibanaSystem<ICoreType, IDepsType, IExposedType> {
start() { public start() {
return { return {
core: this.kibana, core: this.kibana,
deps: this.deps, deps: this.deps,
@ -39,7 +45,7 @@ test('can get exposed values after starting', () => {
test('throws if start returns a promise', () => { test('throws if start returns a promise', () => {
class FooSystem extends KibanaSystem<any, any, any> { class FooSystem extends KibanaSystem<any, any, any> {
async start() { public async start() {
return 'foo'; return 'foo';
} }
} }
@ -55,9 +61,11 @@ test('throws if start returns a promise', () => {
test('throws if stop returns a promise', () => { test('throws if stop returns a promise', () => {
class FooSystem extends KibanaSystem<any, any, any> { class FooSystem extends KibanaSystem<any, any, any> {
start() {} public start() {
// noop
}
async stop() { public async stop() {
return 'stop'; return 'stop';
} }
} }

View file

@ -1,9 +1,9 @@
import { import {
SystemsType, IKibanaSystemClassStatic,
SystemName, ISystemMetadata,
SystemMetadata, ISystemsType,
KibanaSystemClassStatic,
KibanaSystem, KibanaSystem,
SystemName,
} from './system_types'; } from './system_types';
function isPromise(obj: any) { function isPromise(obj: any) {
@ -12,45 +12,45 @@ function isPromise(obj: any) {
); );
} }
export class System<C, M extends SystemMetadata, D extends SystemsType, E> { export class System<C, M extends ISystemMetadata, D extends ISystemsType, E> {
readonly name: SystemName; public readonly name: SystemName;
readonly dependencies: SystemName[]; public readonly dependencies: SystemName[];
readonly metadata?: M; public readonly metadata?: M;
private readonly _systemClass: KibanaSystemClassStatic<C, D, E>; private readonly systemClass: IKibanaSystemClassStatic<C, D, E>;
private _systemInstance?: KibanaSystem<C, D, E>; private systemInstance?: KibanaSystem<C, D, E>;
private _exposedValues?: E; private exposedValues?: E;
constructor( constructor(
name: SystemName, name: SystemName,
config: { config: {
metadata?: M; metadata?: M;
dependencies?: SystemName[]; dependencies?: SystemName[];
implementation: KibanaSystemClassStatic<C, D, E>; implementation: IKibanaSystemClassStatic<C, D, E>;
} }
) { ) {
this.name = name; this.name = name;
this.dependencies = config.dependencies || []; this.dependencies = config.dependencies || [];
this.metadata = config.metadata; this.metadata = config.metadata;
this._systemClass = config.implementation; this.systemClass = config.implementation;
} }
getExposedValues(): E { public getExposedValues(): E {
if (this._systemInstance === undefined) { if (this.systemInstance === undefined) {
throw new Error( throw new Error(
'trying to get the exposed value of a system that is NOT running' 'trying to get the exposed value of a system that is NOT running'
); );
} }
return this._exposedValues!; return this.exposedValues!;
} }
start(kibanaValues: C, dependenciesValues: D) { public start(kibanaValues: C, dependenciesValues: D) {
this._systemInstance = new this._systemClass( this.systemInstance = new this.systemClass(
kibanaValues, kibanaValues,
dependenciesValues dependenciesValues
); );
const exposedValues = this._systemInstance.start(); const exposedValues = this.systemInstance.start();
if (isPromise(exposedValues)) { if (isPromise(exposedValues)) {
throw new Error( throw new Error(
@ -60,15 +60,15 @@ export class System<C, M extends SystemMetadata, D extends SystemsType, E> {
); );
} }
this._exposedValues = this.exposedValues =
exposedValues === undefined ? ({} as E) : exposedValues; exposedValues === undefined ? ({} as E) : exposedValues;
} }
stop() { public stop() {
const stoppedResponse = this._systemInstance && this._systemInstance.stop(); const stoppedResponse = this.systemInstance && this.systemInstance.stop();
this._exposedValues = undefined; this.exposedValues = undefined;
this._systemInstance = undefined; this.systemInstance = undefined;
if (isPromise(stoppedResponse)) { if (isPromise(stoppedResponse)) {
throw new Error( throw new Error(

View file

@ -1,44 +1,53 @@
/* tslint:disable max-classes-per-file */
import { System } from './system'; import { System } from './system';
import { KibanaSystemApiFactory, SystemLoader } from './system_loader';
import { KibanaSystem } from './system_types'; import { KibanaSystem } from './system_types';
import { SystemLoader, KibanaSystemApiFactory } from './system_loader';
// To make types simpler in the tests // To make types simpler in the tests
type CoreType = void; type CoreType = void;
const createCoreValues = () => {}; const createCoreValues = () => {
// noop
};
test('starts system with core api', () => { test('starts system with core api', () => {
expect.assertions(1); expect.assertions(1);
type KibanaCoreApi = { fromCore: boolean; name: string }; interface IKibanaCoreApi {
type Metadata = { configPath?: string }; fromCore: boolean;
name: string;
}
interface IMetadata {
configPath?: string;
}
class FooSystem extends KibanaSystem<KibanaCoreApi, {}> { class FooSystem extends KibanaSystem<IKibanaCoreApi, {}> {
start() { public start() {
expect(this.kibana).toEqual({ expect(this.kibana).toEqual({
name: 'foo',
fromCore: true, fromCore: true,
metadata: { metadata: {
configPath: 'config.path.foo', configPath: 'config.path.foo',
}, },
name: 'foo',
}); });
} }
} }
const foo = new System('foo', { const foo = new System('foo', {
implementation: FooSystem,
metadata: { metadata: {
configPath: 'config.path.foo', configPath: 'config.path.foo',
}, },
implementation: FooSystem,
}); });
const createSystemApi: KibanaSystemApiFactory<KibanaCoreApi, Metadata> = ( const createSystemApi: KibanaSystemApiFactory<IKibanaCoreApi, IMetadata> = (
name, name,
metadata metadata
) => { ) => {
return { return {
name,
metadata,
fromCore: true, fromCore: true,
metadata,
name,
}; };
}; };
@ -51,22 +60,22 @@ test('starts system with core api', () => {
test('system can expose a value', () => { test('system can expose a value', () => {
expect.assertions(1); expect.assertions(1);
type Foo = { interface IFoo {
foo: { foo: {
value: string; value: string;
}; };
}; }
class FooSystem extends KibanaSystem<CoreType, {}, Foo['foo']> { class FooSystem extends KibanaSystem<CoreType, {}, IFoo['foo']> {
start() { public start() {
return { return {
value: 'my-value', value: 'my-value',
}; };
} }
} }
class BarSystem extends KibanaSystem<CoreType, Foo> { class BarSystem extends KibanaSystem<CoreType, IFoo> {
start() { public start() {
expect(this.deps.foo).toEqual({ value: 'my-value' }); expect(this.deps.foo).toEqual({ value: 'my-value' });
} }
} }
@ -89,22 +98,22 @@ test('system can expose a value', () => {
test('system can expose a function', () => { test('system can expose a function', () => {
expect.assertions(2); expect.assertions(2);
type Foo = { interface IFoo {
foo: { foo: {
fn: (val: string) => string; fn: (val: string) => string;
}; };
}; }
class FooSystem extends KibanaSystem<CoreType, {}, Foo['foo']> { class FooSystem extends KibanaSystem<CoreType, {}, IFoo['foo']> {
start(): Foo['foo'] { public start(): IFoo['foo'] {
return { return {
fn: val => `test-${val}`, fn: val => `test-${val}`,
}; };
} }
} }
class BarSystem extends KibanaSystem<CoreType, Foo> { class BarSystem extends KibanaSystem<CoreType, IFoo> {
start() { public start() {
expect(this.deps.foo).toBeDefined(); expect(this.deps.foo).toBeDefined();
expect(this.deps.foo.fn('some-value')).toBe('test-some-value'); expect(this.deps.foo.fn('some-value')).toBe('test-some-value');
} }
@ -128,36 +137,36 @@ test('system can expose a function', () => {
test('can expose value with same name across multiple systems', () => { test('can expose value with same name across multiple systems', () => {
expect.assertions(2); expect.assertions(2);
type Foo = { interface IFoo {
foo: { foo: {
value: string; value: string;
}; };
}; }
type Bar = { interface IBar {
bar: { bar: {
value: string; value: string;
}; };
}; }
class FooSystem extends KibanaSystem<CoreType, {}, Foo['foo']> { class FooSystem extends KibanaSystem<CoreType, {}, IFoo['foo']> {
start(): Foo['foo'] { public start(): IFoo['foo'] {
return { return {
value: 'value-foo', value: 'value-foo',
}; };
} }
} }
class BarSystem extends KibanaSystem<CoreType, {}, Bar['bar']> { class BarSystem extends KibanaSystem<CoreType, {}, IBar['bar']> {
start(): Bar['bar'] { public start(): IBar['bar'] {
return { return {
value: 'value-bar', value: 'value-bar',
}; };
} }
} }
class QuuxSystem extends KibanaSystem<CoreType, Foo & Bar> { class QuuxSystem extends KibanaSystem<CoreType, IFoo & IBar> {
start() { public start() {
expect(this.deps.foo).toEqual({ value: 'value-foo' }); expect(this.deps.foo).toEqual({ value: 'value-foo' });
expect(this.deps.bar).toEqual({ value: 'value-bar' }); expect(this.deps.bar).toEqual({ value: 'value-bar' });
} }
@ -186,32 +195,36 @@ test('can expose value with same name across multiple systems', () => {
test('receives values from dependencies but not transitive dependencies', () => { test('receives values from dependencies but not transitive dependencies', () => {
expect.assertions(3); expect.assertions(3);
type Grandchild = { interface IGrandchild {
grandchild: { grandchild: {
value: string; value: string;
}; };
}; }
type Child = { interface IChild {
child: { child: {
value: string; value: string;
}; };
}; }
class GrandchildSystem extends KibanaSystem< class GrandchildSystem extends KibanaSystem<
CoreType, CoreType,
{}, {},
Grandchild['grandchild'] IGrandchild['grandchild']
> { > {
start() { public start() {
return { return {
value: 'grandchild', value: 'grandchild',
}; };
} }
} }
class ChildSystem extends KibanaSystem<CoreType, Grandchild, Child['child']> { class ChildSystem extends KibanaSystem<
start() { CoreType,
IGrandchild,
IChild['child']
> {
public start() {
expect(this.deps.grandchild).toEqual({ value: 'grandchild' }); expect(this.deps.grandchild).toEqual({ value: 'grandchild' });
return { return {
@ -220,8 +233,8 @@ test('receives values from dependencies but not transitive dependencies', () =>
} }
} }
class ParentSystem extends KibanaSystem<CoreType, Grandchild & Child> { class ParentSystem extends KibanaSystem<CoreType, IGrandchild & IChild> {
start() { public start() {
expect(this.deps.child).toEqual({ value: 'child' }); expect(this.deps.child).toEqual({ value: 'child' });
expect(this.deps.grandchild).toBeUndefined(); expect(this.deps.grandchild).toBeUndefined();
} }
@ -251,24 +264,24 @@ test('receives values from dependencies but not transitive dependencies', () =>
test('keeps reference on registered value', () => { test('keeps reference on registered value', () => {
expect.assertions(1); expect.assertions(1);
type Child = { interface IChild {
child: { child: {
value: {}; value: {};
}; };
}; }
const myRef = {}; const myRef = {};
class ChildSystem extends KibanaSystem<CoreType, {}, Child['child']> { class ChildSystem extends KibanaSystem<CoreType, {}, IChild['child']> {
start() { public start() {
return { return {
value: myRef, value: myRef,
}; };
} }
} }
class ParentSystem extends KibanaSystem<CoreType, Child> { class ParentSystem extends KibanaSystem<CoreType, IChild> {
start() { public start() {
expect(this.deps.child.value).toBe(myRef); expect(this.deps.child.value).toBe(myRef);
} }
} }
@ -291,15 +304,15 @@ test('keeps reference on registered value', () => {
test('can register multiple values in single system', () => { test('can register multiple values in single system', () => {
expect.assertions(1); expect.assertions(1);
type Child = { interface IChild {
child: { child: {
value1: number; value1: number;
value2: number; value2: number;
}; };
}; }
class ChildSystem extends KibanaSystem<CoreType, {}, Child['child']> { class ChildSystem extends KibanaSystem<CoreType, {}, IChild['child']> {
start() { public start() {
return { return {
value1: 1, value1: 1,
value2: 2, value2: 2,
@ -307,8 +320,8 @@ test('can register multiple values in single system', () => {
} }
} }
class ParentSystem extends KibanaSystem<CoreType, Child> { class ParentSystem extends KibanaSystem<CoreType, IChild> {
start() { public start() {
expect(this.deps.child).toEqual({ expect(this.deps.child).toEqual({
value1: 1, value1: 1,
value2: 2, value2: 2,
@ -333,7 +346,9 @@ test('can register multiple values in single system', () => {
test("throws if starting a system that depends on a system that's not present", () => { test("throws if starting a system that depends on a system that's not present", () => {
class FooSystem extends KibanaSystem<CoreType, {}> { class FooSystem extends KibanaSystem<CoreType, {}> {
start() {} public start() {
// noop
}
} }
const foo = new System('foo', { const foo = new System('foo', {
@ -352,7 +367,9 @@ test("throws if starting a system that depends on a system that's not present",
test("throws if adding that has the same name as a system that's already added", () => { test("throws if adding that has the same name as a system that's already added", () => {
class FooSystem extends KibanaSystem<CoreType, {}> { class FooSystem extends KibanaSystem<CoreType, {}> {
start() {} public start() {
// noop
}
} }
const foo = new System('foo', { const foo = new System('foo', {
@ -371,19 +388,19 @@ test('stops systems in reverse order of their starting order', () => {
const events: string[] = []; const events: string[] = [];
class FooSystem extends KibanaSystem<CoreType, {}> { class FooSystem extends KibanaSystem<CoreType, {}> {
start() { public start() {
events.push('start foo'); events.push('start foo');
} }
stop() { public stop() {
events.push('stop foo'); events.push('stop foo');
} }
} }
class BarSystem extends KibanaSystem<CoreType, {}> { class BarSystem extends KibanaSystem<CoreType, {}> {
start() { public start() {
events.push('start bar'); events.push('start bar');
} }
stop() { public stop() {
events.push('stop bar'); events.push('stop bar');
} }
} }
@ -409,18 +426,18 @@ test('stops systems in reverse order of their starting order', () => {
test('can add systems before adding its dependencies', () => { test('can add systems before adding its dependencies', () => {
expect.assertions(1); expect.assertions(1);
type Foo = { interface IFoo {
foo: string; foo: string;
}; }
class FooSystem extends KibanaSystem<CoreType, {}, Foo['foo']> { class FooSystem extends KibanaSystem<CoreType, {}, IFoo['foo']> {
start() { public start() {
return 'value'; return 'value';
} }
} }
class BarSystem extends KibanaSystem<CoreType, Foo> { class BarSystem extends KibanaSystem<CoreType, IFoo> {
start() { public start() {
expect(this.deps.foo).toBe('value'); expect(this.deps.foo).toBe('value');
} }
} }
@ -447,13 +464,13 @@ test('can add multiple system specs at the same time', () => {
const spy = jest.fn(); const spy = jest.fn();
class FooSystem extends KibanaSystem<CoreType, {}> { class FooSystem extends KibanaSystem<CoreType, {}> {
start() { public start() {
spy(); spy();
} }
} }
class BarSystem extends KibanaSystem<CoreType, {}> { class BarSystem extends KibanaSystem<CoreType, {}> {
start() { public start() {
spy(); spy();
} }
} }

View file

@ -1,15 +1,15 @@
import { System } from './system';
import { SystemName, SystemMetadata, SystemsType } from './system_types';
import { getSortedSystemNames } from './sorted_systems'; import { getSortedSystemNames } from './sorted_systems';
import { System } from './system';
import { ISystemMetadata, ISystemsType, SystemName } from './system_types';
export type KibanaSystemApiFactory<C, M> = ( export type KibanaSystemApiFactory<C, M> = (
name: SystemName, name: SystemName,
metadata?: M metadata?: M
) => C; ) => C;
export class SystemLoader<C, M extends SystemMetadata> { export class SystemLoader<C, M extends ISystemMetadata> {
private readonly _systems = new Map<SystemName, System<C, M, any, any>>(); private readonly systems = new Map<SystemName, System<C, M, any, any>>();
private _startedSystems: SystemName[] = []; private startedSystems: SystemName[] = [];
constructor( constructor(
/** /**
@ -17,37 +17,54 @@ export class SystemLoader<C, M extends SystemMetadata> {
* information about a system before it's started, and the return value will * information about a system before it's started, and the return value will
* be injected into the system at startup. * be injected into the system at startup.
*/ */
private readonly _kibanaSystemApiFactory: KibanaSystemApiFactory<C, M> private readonly kibanaSystemApiFactory: KibanaSystemApiFactory<C, M>
) {} ) {}
addSystems(systemSpecs: System<C, M, any, any>[]) { public addSystems(systemSpecs: Array<System<C, M, any, any>>) {
systemSpecs.forEach(systemSpec => { systemSpecs.forEach(systemSpec => {
this.addSystem(systemSpec); this.addSystem(systemSpec);
}); });
} }
addSystem<D extends SystemsType, E = void>(system: System<C, M, D, E>) { public addSystem<D extends ISystemsType, E = void>(
if (this._systems.has(system.name)) { system: System<C, M, D, E>
) {
if (this.systems.has(system.name)) {
throw new Error(`a system named [${system.name}] has already been added`); throw new Error(`a system named [${system.name}] has already been added`);
} }
this._systems.set(system.name, system); this.systems.set(system.name, system);
} }
startSystems() { public startSystems() {
this._ensureAllSystemDependenciesCanBeResolved(); this._ensureAllSystemDependenciesCanBeResolved();
getSortedSystemNames(this._systems) getSortedSystemNames(this.systems)
.map(systemName => this._systems.get(systemName)!) .map(systemName => this.systems.get(systemName)!)
.forEach(systemSpec => { .forEach(systemSpec => {
this.startSystem(systemSpec); this.startSystem(systemSpec);
}); });
} }
/**
* Stop all systems in the reverse order of when they were started
*/
public stopSystems() {
this.startedSystems
.map(systemName => this.systems.get(systemName)!)
.reverse()
.forEach(system => {
system.stop();
this.systems.delete(system.name);
});
this.startedSystems = [];
}
private _ensureAllSystemDependenciesCanBeResolved() { private _ensureAllSystemDependenciesCanBeResolved() {
for (const [systemName, system] of this._systems) { for (const [systemName, system] of this.systems) {
for (const systemDependency of system.dependencies) { for (const systemDependency of system.dependencies) {
if (!this._systems.has(systemDependency)) { if (!this.systems.has(systemDependency)) {
throw new Error( throw new Error(
`System [${systemName}] depends on [${systemDependency}], which is not present` `System [${systemName}] depends on [${systemDependency}], which is not present`
); );
@ -56,38 +73,23 @@ export class SystemLoader<C, M extends SystemMetadata> {
} }
} }
private startSystem<D extends SystemsType, E = void>( private startSystem<D extends ISystemsType, E = void>(
system: System<C, M, D, E> system: System<C, M, D, E>
) { ) {
const dependenciesValues = {} as D; const dependenciesValues = {} as D;
for (const dependency of system.dependencies) { for (const dependency of system.dependencies) {
dependenciesValues[dependency] = this._systems dependenciesValues[dependency] = this.systems
.get(dependency)! .get(dependency)!
.getExposedValues(); .getExposedValues();
} }
const kibanaSystemApi = this._kibanaSystemApiFactory( const kibanaSystemApi = this.kibanaSystemApiFactory(
system.name, system.name,
system.metadata system.metadata
); );
system.start(kibanaSystemApi, dependenciesValues); system.start(kibanaSystemApi, dependenciesValues);
this._startedSystems.push(system.name); this.startedSystems.push(system.name);
}
/**
* Stop all systems in the reverse order of when they were started
*/
stopSystems() {
this._startedSystems
.map(systemName => this._systems.get(systemName)!)
.reverse()
.forEach(system => {
system.stop();
this._systems.delete(system.name);
});
this._startedSystems = [];
} }
} }

View file

@ -1,18 +1,18 @@
export type SystemName = string; export type SystemName = string;
export type SystemMetadata = { export interface ISystemMetadata {
[key: string]: any; [key: string]: any;
}; }
export type SystemsType = { export interface ISystemsType {
[systemName: string]: any; [systemName: string]: any;
}; }
export abstract class KibanaSystem<C, D extends SystemsType, E = void> { export abstract class KibanaSystem<C, D extends ISystemsType, E = void> {
constructor(readonly kibana: C, readonly deps: D) {} constructor(readonly kibana: C, readonly deps: D) {}
abstract start(): E; public abstract start(): E;
stop() { public stop() {
// default implementation of stop does nothing // default implementation of stop does nothing
} }
} }
@ -27,6 +27,6 @@ export abstract class KibanaSystem<C, D extends SystemsType, E = void> {
* *
* See https://www.typescriptlang.org/docs/handbook/interfaces.html#difference-between-the-static-and-instance-sides-of-classes * See https://www.typescriptlang.org/docs/handbook/interfaces.html#difference-between-the-static-and-instance-sides-of-classes
*/ */
export interface KibanaSystemClassStatic<C, D extends SystemsType, E = void> { export interface IKibanaSystemClassStatic<C, D extends ISystemsType, E = void> {
new (kibana: C, deps: D): KibanaSystem<C, D, E>; new (kibana: C, deps: D): KibanaSystem<C, D, E>;
} }

View file

@ -8,7 +8,7 @@ test('returns a topologically ordered sequence', () => {
['d', ['a']], ['d', ['a']],
]); ]);
let sorted = topologicalSort(nodes); const sorted = topologicalSort(nodes);
expect(sorted).toBeDefined(); expect(sorted).toBeDefined();
@ -23,7 +23,7 @@ test('handles multiple "roots" with no deps', () => {
['d', ['a']], ['d', ['a']],
]); ]);
let sorted = topologicalSort(nodes); const sorted = topologicalSort(nodes);
expect(sorted).toBeDefined(); expect(sorted).toBeDefined();

View file

@ -1,14 +0,0 @@
extends: ../../tslint.yaml
rules:
max-classes-per-file: false
interface-name: false
variable-name: false
no-empty: false
object-literal-sort-keys: false
member-ordering: false
member-access: false
ordered-imports: false
interface-over-type-literal: false
array-type: false
prefer-const: false