Skip to content

Commit

Permalink
refactor: Final version of the plugin interface
Browse files Browse the repository at this point in the history
After alot of back and forth I think we landed a good plugin design.

The premise was to remove event emission (since it was attached hackily throught the codebase)
and instead use explicit APIs. However the one thing error events worked well with was namespacing
issues from plugins. Which lead me to think, why on earth is that just an error thing?

So new API (again, like millionth refactor/iteration to cure itch)

```
export abstract class DeepstreamPlugin {
  public abstract description: string
  public async whenReady (): Promise<void> {}
  public init? (): void
  public async close (): Promise<void> {}
  public setRecordHandler? (recordHandler: any): void
}

```

Doesn't look very different does it? The main aspect is the fact we no longer care if its sync or not.
Plugin developers conceal that by just using whenReady (which is always async but could be a millisecond
and could be up to initialization timeout).

So what changed?

```
interface DeepstreamExamplePluginOptions {
}

class DeepstreamExample extends DeepstreamPlugin {
  public description = 'An Example Plugin'
  public logger = this.sevices.logger.getNameSpace('DEEPSTREAM_EXAMPLE_PLUGIN')

  constructor (pluginSettings: DeepstreamExamplePluginOptions, private services: DeepstreamServices) {}

  public async whenReady (): Promise<void> {
    return setTimeout(resolve, 10) // to wait for async dependencies, like opening a server, loading a file, etc
  }
  public init (): void {
    // This just allows you to move logic out of the constructor and guarantees its called after
    // the whenReady is already being waited upon.

    // If something failed, you can either
    this.logger.fatal(ERROR_EVENT, 'descriptive message')

    // or
    this.logger.error(ERROR_EVENT, 'descriptive message')
    this.services.onFatalException()
  }
  public async close (): Promise<void> {
    return setTimeout(resolve, 10) // closing a server for example
  }
}
  • Loading branch information
yasserf committed Jul 10, 2019
1 parent 1d89b00 commit 8ff386a
Show file tree
Hide file tree
Showing 21 changed files with 242 additions and 258 deletions.
2 changes: 1 addition & 1 deletion conf/config.yml
Expand Up @@ -220,7 +220,7 @@ permission:
type: config
options:
# Path to the permissionFile. Can be json, js or yml
path: ./permissions.yml
path: ./permissions2.yml
# Amount of times nested cross-references will be loaded. Avoids endless loops
maxRuleIterations: 3
# PermissionResults are cached to increase performance. Lower number means more loading
Expand Down
2 changes: 1 addition & 1 deletion connectors/cache/redis
Submodule redis updated 2 files
+1 −1 package-lock.json
+2 −2 package.json
2 changes: 1 addition & 1 deletion connectors/storage/rethinkdb
41 changes: 21 additions & 20 deletions src/config/config-initialiser.spec.ts
Expand Up @@ -3,6 +3,7 @@ import { expect } from 'chai'
import * as path from 'path'
import * as defaultConfig from '../default-options'
import * as configInitialiser from './config-initialiser'
import { EventEmitter } from 'events'

describe('config-initialiser', () => {
before(() => {
Expand All @@ -20,7 +21,7 @@ describe('config-initialiser', () => {
options: { some: 'options' }
}
} as any
const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
expect(result.services.plugins.custom.description).to.equal('mock-plugin')
})

Expand All @@ -32,7 +33,7 @@ describe('config-initialiser', () => {
options: {}
}
} as any
const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
expect(result.services.toString()).to.equal('[object Object]')
})

Expand All @@ -46,7 +47,7 @@ describe('config-initialiser', () => {
options: { some: 'options' }
}
} as any
const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
expect(result.services.plugins.mock.description).to.equal('mock-plugin')
})
})
Expand All @@ -57,7 +58,7 @@ describe('config-initialiser', () => {
const config = defaultConfig.get()
config[key] = './does-not-exist'
expect(() => {
configInitialiser.initialise(config)
configInitialiser.initialise(new EventEmitter(), config)
}).to.throw(new Error())
})
})
Expand All @@ -67,7 +68,7 @@ describe('config-initialiser', () => {

const config = defaultConfig.get()
config.sslKey = './sslKey.pem'
const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
expect(result.config.sslKey).to.equal('I\'m a key')
})
})
Expand All @@ -83,7 +84,7 @@ describe('config-initialiser', () => {
}
} as any
try {
configInitialiser.initialise(config)
configInitialiser.initialise(new EventEmitter(), config)
} catch (e) {
errored = true
expect(e.toString()).to.contain(path.join('/foobar', '@deepstream/cache-blablub'))
Expand All @@ -104,7 +105,7 @@ describe('config-initialiser', () => {
config.auth = {
type: 'none'
} as any
const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
expect(result.services.authentication.description).to.equal('Open Authentication')
})

Expand All @@ -118,7 +119,7 @@ describe('config-initialiser', () => {
path: './users.json'
}
}
const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
expect(result.services.authentication.description).to.contain('file using')
expect(result.services.authentication.description).to.contain(path.resolve('src/test/config/users.json'))
})
Expand All @@ -138,7 +139,7 @@ describe('config-initialiser', () => {
}
}

const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
expect(result.services.authentication.description).to.equal('http webhook to http://some-url.com')
})

Expand All @@ -148,7 +149,7 @@ describe('config-initialiser', () => {
delete config.auth

expect(() => {
configInitialiser.initialise(config)
configInitialiser.initialise(new EventEmitter(), config)
}).to.throw('No authentication type specified')
})

Expand All @@ -162,7 +163,7 @@ describe('config-initialiser', () => {
}
}

const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
await result.services.authentication.whenReady()
})

Expand All @@ -175,7 +176,7 @@ describe('config-initialiser', () => {
}

expect(() => {
configInitialiser.initialise(config)
configInitialiser.initialise(new EventEmitter(), config)
}).to.throw(/Cannot find module/)
})

Expand All @@ -188,7 +189,7 @@ describe('config-initialiser', () => {
}

expect(() => {
configInitialiser.initialise(config)
configInitialiser.initialise(new EventEmitter(), config)
}).to.throw('Unknown authentication type bla')
})

Expand All @@ -201,7 +202,7 @@ describe('config-initialiser', () => {
options: {}
}

const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
expect(result.services.authentication.description).to.equal('Open Authentication')
delete global.deepstreamCLI
})
Expand All @@ -218,7 +219,7 @@ describe('config-initialiser', () => {
path: './basic-permission-config.json'
}
}
const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
expect(result.services.permission.description).to.contain('valve permissions loaded from')
expect(result.services.permission.description).to.contain(path.resolve('./src/test/config/basic-permission-config.json'))
})
Expand All @@ -233,7 +234,7 @@ describe('config-initialiser', () => {
}
}
expect(() => {
configInitialiser.initialise(config)
configInitialiser.initialise(new EventEmitter(), config)
}).to.throw('Unknown permission type does-not-exist')
})

Expand All @@ -247,7 +248,7 @@ describe('config-initialiser', () => {
}
}

const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
await result.services.permission.whenReady()
})

Expand All @@ -260,7 +261,7 @@ describe('config-initialiser', () => {
}

expect(() => {
configInitialiser.initialise(config)
configInitialiser.initialise(new EventEmitter(), config)
}).to.throw(/Cannot find module/)
})

Expand All @@ -269,7 +270,7 @@ describe('config-initialiser', () => {
delete config.permission

expect(() => {
configInitialiser.initialise(config)
configInitialiser.initialise(new EventEmitter(), config)
}).to.throw('No permission type specified')
})

Expand All @@ -282,7 +283,7 @@ describe('config-initialiser', () => {
options: {}
}

const result = configInitialiser.initialise(config)
const result = configInitialiser.initialise(new EventEmitter(), config)
expect(result.services.permission.description).to.equal('none')
delete global.deepstreamCLI
})
Expand Down
12 changes: 10 additions & 2 deletions src/config/config-initialiser.ts
Expand Up @@ -2,7 +2,7 @@ import * as fs from 'fs'
import * as utils from '../utils/utils'
import * as fileUtils from './file-utils'
import { DeepstreamConfig, DeepstreamServices, ConnectionEndpoint, PluginConfig, Logger, Storage, StorageReadCallback, StorageWriteCallback, Authentication, Permission, LOG_LEVEL } from '../types'
import { JSONObject } from '../../binary-protocol/src/message-constants'
import { JSONObject, EVENT } from '../../binary-protocol/src/message-constants'
import { DistributedClusterRegistry } from '../services/cluster-registry/distributed-cluster-registry'
import { SingleClusterNode } from '../services/cluster-node/single-cluster-node'
import { DefaultSubscriptionRegistryFactory } from '../services/subscription-registry/default-subscription-registry-factory'
Expand All @@ -21,6 +21,7 @@ import { LocalMonitoring } from '../services/monitoring/noop-monitoring'
import { DistributedLockRegistry } from '../services/lock/distributed-lock-registry'
import { DistributedStateRegistryFactory } from '../services/cluster-state/distributed-state-registry-factory'
import { get as getDefaultOptions } from '../default-options'
import Deepstream from '../deepstream.io';

let commandLineArguments: any

Expand Down Expand Up @@ -62,13 +63,20 @@ export const registerPlugin = function (name: string, construct: Function) {
* Takes a configuration object and instantiates functional properties.
* CLI arguments will be considered.
*/
export const initialise = function (config: DeepstreamConfig): { config: DeepstreamConfig, services: DeepstreamServices } {
export const initialise = function (deepstream: Deepstream, config: DeepstreamConfig): { config: DeepstreamConfig, services: DeepstreamServices } {
commandLineArguments = global.deepstreamCLI || {}
handleUUIDProperty(config)
handleSSLProperties(config)
mergeConnectionOptions(config)

const services = {} as DeepstreamServices
services.notifyFatalException = () => {
if (config.exitOnPluginError) {
process.exit(1)
} else {
deepstream.emit(EVENT.FATAL_EXCEPTION)
}
}
services.logger = handleLogger(config, services)

services.subscriptions = new (resolvePluginClass(config.subscriptions, 'subscriptions'))(config.subscriptions.options, services, config)
Expand Down
5 changes: 3 additions & 2 deletions src/config/js-yaml-loader.ts
Expand Up @@ -5,6 +5,7 @@ import * as path from 'path'
import { get as getDefaultOptions } from '../default-options'
import { merge } from '../utils/utils'
import { DeepstreamConfig } from '../types'
import Deepstream from '../deepstream.io'

const configInitialiser = require('./config-initialiser')
const fileUtils = require('./file-utils')
Expand Down Expand Up @@ -65,9 +66,9 @@ export const loadConfigWithoutInitialisation = function (filePath: string | null
* Configuraiton file will be transformed to a deepstream object by evaluating
* some properties like the plugins (logger and connectors).
*/
export const loadConfig = function (filePath: string | null, args?: object) {
export const loadConfig = function (deepstream: Deepstream, filePath: string | null, args?: object) {
const config = loadConfigWithoutInitialisation(filePath, args)
const result = configInitialiser.initialise(config.config)
const result = configInitialiser.initialise(deepstream, config.config)
return {
config: result.config,
services: result.services,
Expand Down
6 changes: 3 additions & 3 deletions src/deepstream.io.ts
Expand Up @@ -226,7 +226,7 @@ export class Deepstream extends EventEmitter {
*/
private async serviceInit () {
const readyPromises = Object.keys(this.services).reduce((promises, serviceName) => {
if (['connectionEndpoints', 'plugins'].includes(serviceName)) {
if (['connectionEndpoints', 'plugins', 'notifyFatalException'].includes(serviceName)) {
return promises
}
const service = (this.services as any)[serviceName] as DeepstreamPlugin
Expand Down Expand Up @@ -399,12 +399,12 @@ private async pluginsShutdown () {
private loadConfig (config: PartialDeepstreamConfig | string | null): void {
let result
if (config === null || typeof config === 'string') {
result = jsYamlLoader.loadConfig(config)
result = jsYamlLoader.loadConfig(this, config)
this.configFile = result.file
} else {
configInitialiser.mergeConnectionOptions(config)
const rawConfig = merge(getDefaultOptions(), config) as DeepstreamConfig
result = configInitialiser.initialise(rawConfig)
result = configInitialiser.initialise(this, rawConfig)
}
configValidator.validate(result.config)
this.config = result.config
Expand Down

0 comments on commit 8ff386a

Please sign in to comment.