From 6e2da2a29714673eefc737bc6ee9589815f47695 Mon Sep 17 00:00:00 2001 From: Ricardo Arturo Cabral Mejia Date: Thu, 20 Oct 2022 19:25:54 -0400 Subject: [PATCH] test: refactor settings --- src/adapters/web-server-adapter.ts | 5 +- src/adapters/web-socket-server-adapter.ts | 6 +- src/factories/message-handler-factory.ts | 12 +- src/factories/settings-factory.ts | 4 + src/handlers/event-message-handler.ts | 4 +- src/handlers/subscribe-message-handler.ts | 7 +- src/index.ts | 4 +- src/utils/settings.ts | 149 ++++++++------- test/unit/factories/settings-factory.spec.ts | 24 +++ .../handlers/event-message-handler.spec.ts | 4 +- test/unit/utils/settings.spec.ts | 180 ++++++++++++++++-- 11 files changed, 294 insertions(+), 105 deletions(-) create mode 100644 src/factories/settings-factory.ts create mode 100644 test/unit/factories/settings-factory.spec.ts diff --git a/src/adapters/web-server-adapter.ts b/src/adapters/web-server-adapter.ts index af1e89fe..c8e680ee 100644 --- a/src/adapters/web-server-adapter.ts +++ b/src/adapters/web-server-adapter.ts @@ -2,13 +2,14 @@ import { Duplex, EventEmitter } from 'stream' import { IncomingMessage, Server, ServerResponse } from 'http' import packageJson from '../../package.json' +import { ISettings } from '../@types/settings' import { IWebServerAdapter } from '../@types/adapters' -import { Settings } from '../utils/settings' export class WebServerAdapter extends EventEmitter implements IWebServerAdapter { public constructor( private readonly webServer: Server, + private readonly settings: () => ISettings, ) { super() this.webServer.on('request', this.onWebServerRequest.bind(this)) @@ -25,7 +26,7 @@ export class WebServerAdapter extends EventEmitter implements IWebServerAdapter if (request.method === 'GET' && request.headers['accept'] === 'application/nostr+json') { const { info: { name, description, pubkey, contact }, - } = Settings + } = this.settings() const relayInformationDocument = { name, diff --git a/src/adapters/web-socket-server-adapter.ts b/src/adapters/web-socket-server-adapter.ts index 0aab924a..57fd8d85 100644 --- a/src/adapters/web-socket-server-adapter.ts +++ b/src/adapters/web-socket-server-adapter.ts @@ -5,6 +5,7 @@ import { IWebSocketAdapter, IWebSocketServerAdapter } from '../@types/adapters' import { WebSocketAdapterEvent, WebSocketServerAdapterEvent } from '../constants/adapter' import { Event } from '../@types/event' import { Factory } from '../@types/base' +import { ISettings } from '../@types/settings' import { propEq } from 'ramda' import { WebServerAdapter } from './web-server-adapter' @@ -22,9 +23,10 @@ export class WebSocketServerAdapter extends WebServerAdapter implements IWebSock private readonly createWebSocketAdapter: Factory< IWebSocketAdapter, [WebSocket, IncomingMessage, IWebSocketServerAdapter] - > + >, + settings: () => ISettings, ) { - super(webServer) + super(webServer, settings) this.webSocketsAdapters = new WeakMap() diff --git a/src/factories/message-handler-factory.ts b/src/factories/message-handler-factory.ts index 8b061c8f..4311886a 100644 --- a/src/factories/message-handler-factory.ts +++ b/src/factories/message-handler-factory.ts @@ -1,4 +1,5 @@ import { IncomingMessage, MessageType } from '../@types/messages' +import { createSettings } from './settings-factory' import { DelegatedEventMessageHandler } from '../handlers/delegated-event-message-handler' import { delegatedEventStrategyFactory } from './delegated-event-strategy-factory' import { EventMessageHandler } from '../handlers/event-message-handler' @@ -6,7 +7,6 @@ import { eventStrategyFactory } from './event-strategy-factory' import { IEventRepository } from '../@types/repositories' import { isDelegatedEvent } from '../utils/event' import { IWebSocketAdapter } from '../@types/adapters' -import { Settings } from '../utils/settings' import { SubscribeMessageHandler } from '../handlers/subscribe-message-handler' import { UnsubscribeMessageHandler } from '../handlers/unsubscribe-message-handler' @@ -17,13 +17,17 @@ export const messageHandlerFactory = ( case MessageType.EVENT: { if (isDelegatedEvent(message[1])) { - return new DelegatedEventMessageHandler(adapter, delegatedEventStrategyFactory(eventRepository), Settings) + return new DelegatedEventMessageHandler( + adapter, + delegatedEventStrategyFactory(eventRepository), + createSettings + ) } - return new EventMessageHandler(adapter, eventStrategyFactory(eventRepository), Settings) + return new EventMessageHandler(adapter, eventStrategyFactory(eventRepository), createSettings) } case MessageType.REQ: - return new SubscribeMessageHandler(adapter, eventRepository) + return new SubscribeMessageHandler(adapter, eventRepository, createSettings) case MessageType.CLOSE: return new UnsubscribeMessageHandler(adapter,) default: diff --git a/src/factories/settings-factory.ts b/src/factories/settings-factory.ts new file mode 100644 index 00000000..1265d010 --- /dev/null +++ b/src/factories/settings-factory.ts @@ -0,0 +1,4 @@ +import { ISettings } from '../@types/settings' +import { SettingsStatic } from '../utils/settings' + +export const createSettings = (): ISettings => SettingsStatic.createSettings() diff --git a/src/handlers/event-message-handler.ts b/src/handlers/event-message-handler.ts index 3b4a6149..4c7bae95 100644 --- a/src/handlers/event-message-handler.ts +++ b/src/handlers/event-message-handler.ts @@ -13,7 +13,7 @@ export class EventMessageHandler implements IMessageHandler { public constructor( protected readonly webSocket: IWebSocketAdapter, protected readonly strategyFactory: Factory>, [Event, IWebSocketAdapter]>, - private readonly settings: ISettings + private readonly settings: () => ISettings ) { } public async handleMessage(message: IncomingEventMessage): Promise { @@ -49,7 +49,7 @@ export class EventMessageHandler implements IMessageHandler { protected canAcceptEvent(event: Event): string | undefined { const now = Math.floor(Date.now()/1000) - const limits = this.settings.limits.event + const limits = this.settings().limits.event if (limits.createdAt.maxPositiveDelta > 0) { if (event.created_at > now + limits.createdAt.maxPositiveDelta) { return `created_at is more than ${limits.createdAt.maxPositiveDelta} seconds in the future` diff --git a/src/handlers/subscribe-message-handler.ts b/src/handlers/subscribe-message-handler.ts index 9ec59656..c4407095 100644 --- a/src/handlers/subscribe-message-handler.ts +++ b/src/handlers/subscribe-message-handler.ts @@ -8,8 +8,8 @@ import { streamEach, streamEnd, streamFilter, streamMap } from '../utils/stream' import { SubscriptionFilter, SubscriptionId } from '../@types/subscription' import { Event } from '../@types/event' import { IEventRepository } from '../@types/repositories' +import { ISettings } from '../@types/settings' import { IWebSocketAdapter } from '../@types/adapters' -import { Settings } from '../utils/settings' import { SubscribeMessage } from '../@types/messages' import { WebSocketAdapterEvent } from '../constants/adapter' @@ -19,6 +19,7 @@ export class SubscribeMessageHandler implements IMessageHandler, IAbortable { public constructor( private readonly webSocket: IWebSocketAdapter, private readonly eventRepository: IEventRepository, + private readonly settings: () => ISettings, ) { this.abortController = new AbortController() } @@ -66,7 +67,7 @@ export class SubscribeMessageHandler implements IMessageHandler, IAbortable { } private canSubscribe(subscriptionId: string, filters: SubscriptionFilter[]): string | undefined { - const maxSubscriptions = Settings.limits.client.subscription.maxSubscriptions + const maxSubscriptions = this.settings().limits.client.subscription.maxSubscriptions if (maxSubscriptions > 0) { const subscriptions = this.webSocket.getSubscriptions() if (!subscriptions.has(subscriptionId) && subscriptions.size + 1 > maxSubscriptions) { @@ -74,7 +75,7 @@ export class SubscribeMessageHandler implements IMessageHandler, IAbortable { } } - const maxFilters = Settings.limits.client.subscription.maxFilters + const maxFilters = this.settings().limits.client.subscription.maxFilters if (maxFilters > 0) { if (filters.length > maxFilters) { return `Too many filters: Number of filters per susbscription must be less or equal to ${maxFilters}` diff --git a/src/index.ts b/src/index.ts index 55aa50bf..bec74b84 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,6 +4,7 @@ import http from 'http' import process from 'process' import { WebSocketServer } from 'ws' +import { createSettings } from './factories/settings-factory' import { EventRepository } from './repositories/event-repository' import { getDbClient } from './database/client' import packageJson from '../package.json' @@ -64,7 +65,8 @@ if (cluster.isPrimary) { const adapter = new WebSocketServerAdapter( server, wss, - webSocketAdapterFactory(eventRepository) + webSocketAdapterFactory(eventRepository), + createSettings, ) adapter.listen(port) diff --git a/src/utils/settings.ts b/src/utils/settings.ts index 22cd9564..80861f69 100644 --- a/src/utils/settings.ts +++ b/src/utils/settings.ts @@ -1,4 +1,4 @@ -import { existsSync, readFileSync, writeFileSync } from 'fs' +import fs from 'fs' import { homedir } from 'os' import { join } from 'path' import { mergeDeepRight } from 'ramda' @@ -6,85 +6,94 @@ import { mergeDeepRight } from 'ramda' import { ISettings } from '../@types/settings' import packageJson from '../../package.json' -export const getSettingsFilePath = (filename = 'settings.json'): string => join( - process.env.NOSTR_CONFIG_DIR ?? join(homedir(), '.nostr'), - filename, -) +export class SettingsStatic { + static _settings: ISettings -let _settings: ISettings + public static getSettingsFilePath(filename = 'settings.json') { + return join( + process.env.NOSTR_CONFIG_DIR ?? join(homedir(), '.nostr'), + filename + ) + } -export const getDefaultSettings = (): ISettings => ({ - info: { - relay_url: `wss://${packageJson.name}.your-domain.com`, - name: `${packageJson.name}.your-domain.com`, - description: packageJson.description, - pubkey: '', - contact: 'operator@your-domain.com', - }, - limits: { - event: { - eventId: { - minLeadingZeroBits: 0, - }, - kind: { - whitelist: [], - blacklist: [], - }, - pubkey: { - minLeadingZeroBits: 0, - whitelist: [], - blacklist: [], - }, - createdAt: { - maxPositiveDelta: 900, // +15 min - maxNegativeDelta: 0, // disabled + public static getDefaultSettings(): ISettings { + return { + info: { + relay_url: 'wss://nostr-ts-relay.your-domain.com', + name: `${packageJson.name}.your-domain.com`, + description: packageJson.description, + pubkey: 'replace-with-your-pubkey', + contact: 'operator@your-domain.com', }, - }, - client: { - subscription: { - maxSubscriptions: 10, - maxFilters: 10, + limits: { + event: { + eventId: { + minLeadingZeroBits: 0, + }, + kind: { + whitelist: [], + blacklist: [], + }, + pubkey: { + minLeadingZeroBits: 0, + whitelist: [], + blacklist: [], + }, + createdAt: { + maxPositiveDelta: 900, + maxNegativeDelta: 0, // disabled + }, + }, + client: { + subscription: { + maxSubscriptions: 10, + maxFilters: 10, + }, + }, }, - }, - }, -}) - -const loadSettings = (path: string) => { - return JSON.parse( - readFileSync( - path, - { encoding: 'utf-8' }, - ), - ) -} - -const createSettings = (): ISettings => { - const path = getSettingsFilePath() - const defaults = getDefaultSettings() - try { - if (_settings) { - return _settings } + } + + public static loadSettings(path: string) { + return JSON.parse( + fs.readFileSync( + path, + { encoding: 'utf-8' } + ) + ) + } - if (!existsSync(path)) { - saveSettings(path, defaults) + public static createSettings(): ISettings { + if (SettingsStatic._settings) { + return SettingsStatic._settings } + const path = SettingsStatic.getSettingsFilePath() + const defaults = SettingsStatic.getDefaultSettings() + try { - _settings = mergeDeepRight(defaults, loadSettings(path)) + if (fs.existsSync(path)) { + SettingsStatic._settings = mergeDeepRight( + defaults, + SettingsStatic.loadSettings(path) + ) + } else { + SettingsStatic.saveSettings(path, defaults) + SettingsStatic._settings = mergeDeepRight({}, defaults) + } - return _settings - } catch (error) { - console.error('Unable to read config file. Reason: %s', error.message) + return SettingsStatic._settings + } catch (error) { + console.error('Unable to read config file. Reason: %s', error.message) - return defaults + return defaults + } } -} -export const saveSettings = (path: string, settings: ISettings) => { - return writeFileSync( - path, - JSON.stringify(settings, null, 2), - { encoding: 'utf-8' } - ) + public static saveSettings(path: string, settings: ISettings) { + return fs.writeFileSync( + path, + JSON.stringify(settings, null, 2), + { encoding: 'utf-8' } + ) + } } -export const Settings = createSettings() diff --git a/test/unit/factories/settings-factory.spec.ts b/test/unit/factories/settings-factory.spec.ts new file mode 100644 index 00000000..ac7cb93a --- /dev/null +++ b/test/unit/factories/settings-factory.spec.ts @@ -0,0 +1,24 @@ +import { expect } from 'chai' +import Sinon from 'sinon' + +import { createSettings } from '../../../src/factories/settings-factory' +import { SettingsStatic } from '../../../src/utils/settings' + +describe('getSettings', () => { + let createSettingsStub: Sinon.SinonStub + + beforeEach(() => { + createSettingsStub = Sinon.stub(SettingsStatic, 'createSettings') + }) + + afterEach(() => { + createSettingsStub.restore() + }) + + it('calls createSettings and returns', () => { + const settings = Symbol() + createSettingsStub.returns(settings) + + expect(createSettings()).to.equal(settings) + }) +}) \ No newline at end of file diff --git a/test/unit/handlers/event-message-handler.spec.ts b/test/unit/handlers/event-message-handler.spec.ts index 51add026..8a0e7c3b 100644 --- a/test/unit/handlers/event-message-handler.spec.ts +++ b/test/unit/handlers/event-message-handler.spec.ts @@ -64,7 +64,7 @@ describe('EventMessageHandler', () => { handler = new EventMessageHandler( webSocket as any, strategyFactoryStub, - {} as any, + () => ({}) as any, ) }) @@ -168,7 +168,7 @@ describe('EventMessageHandler', () => { handler = new EventMessageHandler( {} as any, () => null, - settings, + () => settings, ) }) diff --git a/test/unit/utils/settings.spec.ts b/test/unit/utils/settings.spec.ts index 9facf57b..bd7b8aa2 100644 --- a/test/unit/utils/settings.spec.ts +++ b/test/unit/utils/settings.spec.ts @@ -1,11 +1,13 @@ import { expect } from 'chai' +import fs from 'fs' import { homedir } from 'os' import { join } from 'path' +import Sinon from 'sinon' -import { getDefaultSettings, getSettingsFilePath } from '../../../src/utils/settings' +import { SettingsStatic } from '../../../src/utils/settings' -describe('Settings', () => { - describe('getSettingsFilePath', () => { +describe('SettingsStatic', () => { + describe('.getSettingsFilePath', () => { let originalEnv: NodeJS.ProcessEnv beforeEach(() => { @@ -18,40 +20,39 @@ describe('Settings', () => { }) it('returns string ending with settings.json by default', () => { - expect(getSettingsFilePath()).to.be.a('string').and.to.match(/settings\.json$/) + expect(SettingsStatic.getSettingsFilePath()).to.be.a('string').and.to.match(/settings\.json$/) }) it('returns string ending with given string', () => { - expect(getSettingsFilePath('ending')).to.be.a('string').and.to.match(/ending$/) + expect(SettingsStatic.getSettingsFilePath('ending')).to.be.a('string').and.to.match(/ending$/) }) it('returns path begins with user\'s home dir by default', () => { - expect(getSettingsFilePath()).to.be.a('string').and.equal(`${join(homedir(), '.nostr')}/settings.json`) + expect(SettingsStatic.getSettingsFilePath()).to.be.a('string').and.equal(`${join(homedir(), '.nostr')}/settings.json`) }) it('returns path with NOSTR_CONFIG_DIR if set', () => { process.env.NOSTR_CONFIG_DIR = '/some/path' - expect(getSettingsFilePath()).to.be.a('string').and.equal('/some/path/settings.json') + expect(SettingsStatic.getSettingsFilePath()).to.be.a('string').and.equal('/some/path/settings.json') }) }) - describe('getDefaultSettings', () => { + describe('.getDefaultSettings', () => { it('returns object with info', () => { - expect(getDefaultSettings()) + expect(SettingsStatic.getDefaultSettings()) .to.have.property('info') .and.to.deep.equal({ relay_url: 'wss://nostr-ts-relay.your-domain.com', name: 'nostr-ts-relay.your-domain.com', description: 'A nostr relay written in Typescript.', - pubkey: '', + pubkey: 'replace-with-your-pubkey', contact: 'operator@your-domain.com', }) }) - it('returns object with default limits', () => { - expect(getDefaultSettings()) + expect(SettingsStatic.getDefaultSettings()) .to.have.property('limits') .and.to.deep.equal({ event: { @@ -82,15 +83,156 @@ describe('Settings', () => { }) }) - // describe('loadSettings', () => { + describe('.loadSettings', () => { + let readFileSyncStub: Sinon.SinonStub + + beforeEach(() => { + readFileSyncStub = Sinon.stub(fs, 'readFileSync') + }) + + afterEach(() => { + readFileSyncStub.restore() + }) + + it('loads settings from given path', () => { + readFileSyncStub.returns('"content"') + + expect(SettingsStatic.loadSettings('/some/path')).to.equal('content') + + expect(readFileSyncStub).to.have.been.calledOnceWithExactly( + '/some/path', + { encoding: 'utf-8' } + ) + }) + }) + + describe('.createSettings', () => { + let existsSyncStub: Sinon.SinonStub + let getSettingsFilePathStub: Sinon.SinonStub + let getDefaultSettingsStub: Sinon.SinonStub + let saveSettingsStub: Sinon.SinonStub + let loadSettingsStub: Sinon.SinonStub + + let sandbox: Sinon.SinonSandbox - // }) + beforeEach(() => { + SettingsStatic._settings = undefined - // describe('createSettings', () => { + sandbox = Sinon.createSandbox() - // }) + existsSyncStub = sandbox.stub(fs, 'existsSync') + getSettingsFilePathStub = sandbox.stub(SettingsStatic, 'getSettingsFilePath') + getDefaultSettingsStub = sandbox.stub(SettingsStatic, 'getDefaultSettings') + saveSettingsStub = sandbox.stub(SettingsStatic, 'saveSettings') + loadSettingsStub = sandbox.stub(SettingsStatic, 'loadSettings') + }) - // describe('saveSettings', () => { + afterEach(() => { + sandbox.restore() + }) + + it('creates settings from default if settings file is missing', () => { + getDefaultSettingsStub.returns({}) + getSettingsFilePathStub.returns('/some/path/settings.json') + existsSyncStub.returns(false) + + expect(SettingsStatic.createSettings()).to.deep.equal({}) + + expect(existsSyncStub).to.have.been.calledOnceWithExactly('/some/path/settings.json') + expect(getSettingsFilePathStub).to.have.been.calledOnce + expect(getDefaultSettingsStub).to.have.been.calledOnce + expect(saveSettingsStub).to.have.been.calledOnceWithExactly( + '/some/path/settings.json', + {}, + ) + expect(loadSettingsStub).not.to.have.been.called + }) + + it('returns default settings if saving settings file throws', () => { + const error = new Error('mistakes were made') + const defaults = Symbol() + getSettingsFilePathStub.returns('/some/path/settings.json') + getDefaultSettingsStub.returns(defaults) + saveSettingsStub.throws(error) + existsSyncStub.returns(false) + + expect(SettingsStatic.createSettings()).to.equal(defaults) + + expect(existsSyncStub).to.have.been.calledOnceWithExactly('/some/path/settings.json') + expect(getSettingsFilePathStub).to.have.been.calledOnce + expect(getDefaultSettingsStub).to.have.been.calledOnce + expect(saveSettingsStub).to.have.been.calledOnceWithExactly( + '/some/path/settings.json', + defaults, + ) + expect(loadSettingsStub).not.to.have.been.called + }) - // }) -}) \ No newline at end of file + it('loads settings from file if settings file is exists', () => { + getDefaultSettingsStub.returns({}) + loadSettingsStub.returns({}) + getSettingsFilePathStub.returns('/some/path/settings.json') + existsSyncStub.returns(true) + + expect(SettingsStatic.createSettings()).to.deep.equal({}) + + expect(existsSyncStub).to.have.been.calledOnceWithExactly('/some/path/settings.json') + expect(getSettingsFilePathStub).to.have.been.calledOnce + expect(getDefaultSettingsStub).to.have.been.calledOnce + expect(saveSettingsStub).not.to.have.been.called + expect(loadSettingsStub).to.have.been.calledOnceWithExactly('/some/path/settings.json') + }) + + it('returns defaults if loading settings file throws', () => { + const defaults = Symbol() + const error = new Error('mistakes were made') + getDefaultSettingsStub.returns(defaults) + loadSettingsStub.throws(error) + getSettingsFilePathStub.returns('/some/path/settings.json') + existsSyncStub.returns(true) + + expect(SettingsStatic.createSettings()).to.equal(defaults) + + expect(existsSyncStub).to.have.been.calledOnceWithExactly('/some/path/settings.json') + expect(getSettingsFilePathStub).to.have.been.calledOnce + expect(getDefaultSettingsStub).to.have.been.calledOnce + expect(saveSettingsStub).not.to.have.been.called + expect(loadSettingsStub).to.have.been.calledOnceWithExactly('/some/path/settings.json') + }) + + it('returns cached settings if set', () => { + const cachedSettings = Symbol() + SettingsStatic._settings = cachedSettings as any + + expect(SettingsStatic.createSettings()).to.equal(cachedSettings) + + expect(getSettingsFilePathStub).not.to.have.been.calledOnce + expect(getDefaultSettingsStub).not.to.have.been.calledOnce + expect(existsSyncStub).not.to.have.been.called + expect(saveSettingsStub).not.to.have.been.called + expect(loadSettingsStub).not.to.have.been.called + }) + }) + + describe('.saveSettings', () => { + let writeFileSyncStub: Sinon.SinonStub + + beforeEach(() => { + writeFileSyncStub = Sinon.stub(fs, 'writeFileSync') + }) + + afterEach(() => { + writeFileSyncStub.restore() + }) + + it('saves settings to given path', () => { + SettingsStatic.saveSettings('/some/path/settings.json', {key: 'value'} as any) + + expect(writeFileSyncStub).to.have.been.calledOnceWithExactly( + '/some/path/settings.json', + '{\n "key": "value"\n}', + { encoding: 'utf-8' } + ) + }) + }) +})