[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 { KibanaSystem } from './system_types';
test('can get exposed values after starting', () => {
type CoreType = { bar: string };
type DepsType = { quux: string };
type ExposedType = {
core: CoreType;
deps: DepsType;
};
interface ICoreType {
bar: string;
}
interface IDepsType {
quux: string;
}
interface IExposedType {
core: ICoreType;
deps: IDepsType;
}
class FooSystem extends KibanaSystem<CoreType, DepsType, ExposedType> {
start() {
class FooSystem extends KibanaSystem<ICoreType, IDepsType, IExposedType> {
public start() {
return {
core: this.kibana,
deps: this.deps,
@ -39,7 +45,7 @@ test('can get exposed values after starting', () => {
test('throws if start returns a promise', () => {
class FooSystem extends KibanaSystem<any, any, any> {
async start() {
public async start() {
return 'foo';
}
}
@ -55,9 +61,11 @@ test('throws if start returns a promise', () => {
test('throws if stop returns a promise', () => {
class FooSystem extends KibanaSystem<any, any, any> {
start() {}
public start() {
// noop
}
async stop() {
public async stop() {
return 'stop';
}
}

View file

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

View file

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

View file

@ -1,15 +1,15 @@
import { System } from './system';
import { SystemName, SystemMetadata, SystemsType } from './system_types';
import { getSortedSystemNames } from './sorted_systems';
import { System } from './system';
import { ISystemMetadata, ISystemsType, SystemName } from './system_types';
export type KibanaSystemApiFactory<C, M> = (
name: SystemName,
metadata?: M
) => C;
export class SystemLoader<C, M extends SystemMetadata> {
private readonly _systems = new Map<SystemName, System<C, M, any, any>>();
private _startedSystems: SystemName[] = [];
export class SystemLoader<C, M extends ISystemMetadata> {
private readonly systems = new Map<SystemName, System<C, M, any, any>>();
private startedSystems: SystemName[] = [];
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
* 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 => {
this.addSystem(systemSpec);
});
}
addSystem<D extends SystemsType, E = void>(system: System<C, M, D, E>) {
if (this._systems.has(system.name)) {
public addSystem<D extends ISystemsType, E = void>(
system: System<C, M, D, E>
) {
if (this.systems.has(system.name)) {
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();
getSortedSystemNames(this._systems)
.map(systemName => this._systems.get(systemName)!)
getSortedSystemNames(this.systems)
.map(systemName => this.systems.get(systemName)!)
.forEach(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() {
for (const [systemName, system] of this._systems) {
for (const [systemName, system] of this.systems) {
for (const systemDependency of system.dependencies) {
if (!this._systems.has(systemDependency)) {
if (!this.systems.has(systemDependency)) {
throw new Error(
`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>
) {
const dependenciesValues = {} as D;
for (const dependency of system.dependencies) {
dependenciesValues[dependency] = this._systems
dependenciesValues[dependency] = this.systems
.get(dependency)!
.getExposedValues();
}
const kibanaSystemApi = this._kibanaSystemApiFactory(
const kibanaSystemApi = this.kibanaSystemApiFactory(
system.name,
system.metadata
);
system.start(kibanaSystemApi, dependenciesValues);
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 = [];
this.startedSystems.push(system.name);
}
}

View file

@ -1,18 +1,18 @@
export type SystemName = string;
export type SystemMetadata = {
export interface ISystemMetadata {
[key: string]: any;
};
}
export type SystemsType = {
export interface ISystemsType {
[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) {}
abstract start(): E;
public abstract start(): E;
stop() {
public stop() {
// 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
*/
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>;
}

View file

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