From 16c28348e7cb51286ea27be575e7c702cb3f6978 Mon Sep 17 00:00:00 2001 From: Bastien Granger Date: Sat, 16 Jul 2022 10:41:50 -0400 Subject: [PATCH 1/8] feat(events): create an omitEvent method --- src/Events.ts | 11 +++++++++++ src/index.ts | 7 ++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/Events.ts b/src/Events.ts index 09b1c09..44bdc3e 100644 --- a/src/Events.ts +++ b/src/Events.ts @@ -36,6 +36,17 @@ export function combineEvents< export function createEventBus(args: { events: C, channels?: Channel[] }): C { const transports = (args.channels || []).map(c => new Transport(c)) +export function omitEvents< + Events extends EventDeclaration, + OmittedEvents extends keyof Events +>(events: Events, omittedEvents: OmittedEvents[]): Omit { + return Object.keys(events).reduce((acc, event) => { + if (!omittedEvents.includes(event as OmittedEvents)) { + acc[event as keyof Events] = events[event] + } + return acc + }, {} as any) +} const eventBus: Partial = {} for (const event in args.events) { diff --git a/src/index.ts b/src/index.ts index c3a7167..b6e779f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,10 @@ export { slot, Slot } from './Slot' -export { EventDeclaration, combineEvents, createEventBus } from './Events' +export { + EventDeclaration, + combineEvents, + createEventBus, + omitEvents +} from './Events' export { Channel } from './Channel' export { GenericChannel } from './Channels/GenericChannel' export { ChunkedChannel } from './Channels/ChunkedChannel' From 0f782b58ece3713114c2266fb7034466715b4c01 Mon Sep 17 00:00:00 2001 From: Bastien Granger Date: Sat, 16 Jul 2022 10:43:43 -0400 Subject: [PATCH 2/8] feat(events): add a new blacklist param to createEventBus This blacklist param will allow clients to ignore certain events so the far end does not need to wait them to be register to trigger --- src/Events.ts | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/Events.ts b/src/Events.ts index 44bdc3e..d7d1308 100644 --- a/src/Events.ts +++ b/src/Events.ts @@ -34,8 +34,6 @@ export function combineEvents< return Object.assign({}, ...args) } -export function createEventBus(args: { events: C, channels?: Channel[] }): C { - const transports = (args.channels || []).map(c => new Transport(c)) export function omitEvents< Events extends EventDeclaration, OmittedEvents extends keyof Events @@ -48,10 +46,27 @@ export function omitEvents< }, {} as any) } +export function createEventBus(args: { + events: C + channels?: Channel[] + blackList?: Array +}): C { + const filteredEventList = args.blackList + ? Object.keys(omitEvents(args.events, args.blackList)) + : undefined + + const transports = (args.channels || []).map( + (c) => new Transport(c, filteredEventList) + ) + const eventBus: Partial = {} for (const event in args.events) { if (args.events.hasOwnProperty(event)) { - eventBus[event] = (connectSlot(event, transports, args.events[event].config) as C[Extract]) + eventBus[event] = connectSlot( + event, + transports, + args.events[event].config + ) as C[Extract] } } From 853a089d8677921aea8a7796fe1c90e73c7b9da9 Mon Sep 17 00:00:00 2001 From: Bastien Granger Date: Sat, 16 Jul 2022 11:09:59 -0400 Subject: [PATCH 3/8] feat(transport): support list of event update The far end can blacklist some events. It will trigger a fake registration of these event handlers and remove them from the list of handlers. This end will then not wait of the far end to be ready to trigger the events --- src/Message.ts | 9 ++++++ src/Slot.ts | 79 ++++++++++++++++++++++++++++++++++++++++-------- src/Transport.ts | 54 +++++++++++++++++++++++++++++++-- 3 files changed, 127 insertions(+), 15 deletions(-) diff --git a/src/Message.ts b/src/Message.ts index c2115b2..2750064 100644 --- a/src/Message.ts +++ b/src/Message.ts @@ -28,17 +28,25 @@ export type TransportRegistrationMessage = { slotName: string, param: string } + export type TransportUnregistrationMessage = { type: 'handler_unregistered', slotName: string, param: string } + +export type TransportEventListMessage = { + type: 'event_list', + eventList: string[] +} + export type TransportMessage = TransportError | TransportRegistrationMessage | TransportRequest | TransportResponse | TransportUnregistrationMessage + | TransportEventListMessage export function isTransportMessage(m: { type: string }): m is TransportMessage { switch (m.type) { @@ -47,6 +55,7 @@ export function isTransportMessage(m: { type: string }): m is TransportMessage { case 'error': case 'handler_unregistered': case 'handler_registered': + case 'event_list': return true default: return false diff --git a/src/Slot.ts b/src/Slot.ts index 597ee9b..5d33ffc 100644 --- a/src/Slot.ts +++ b/src/Slot.ts @@ -165,23 +165,34 @@ export function connectSlot( // Signal to all transports that we will accept handlers for this slotName transports.forEach((transport, transportKey) => { - const remoteHandlerRegistered = ( param = DEFAULT_PARAM, handler: Handler ) => { + // If the remote end of the communication channel had blacklisted an + // event but is now trying to register a handler. We ignore it and + // consider the blacklist to be the source of truth + if (!remoteHandlersConnected[transportKey]) { + return + } + // Store handler const paramHandlers = handlers[transportKey][param] || [] handlers[transportKey][param] = paramHandlers.concat(handler) // Call lazy callbacks if needed - if (getParamHandlers(param, handlers).length === 1) callLazyConnectCallbacks(param) + if (getParamHandlers(param, handlers).length === 1) { + callLazyConnectCallbacks(param) + } // Release potential buffered events if (!remoteHandlersConnected[transportKey][param]) { awaitHandlerRegistration(String(transportKey), param) } + // call onRegister callback on slot for each transport. It will + // release the event once triggered. If one is not registered then + // event will not be sent. remoteHandlersConnected[transportKey][param].onRegister() } @@ -191,13 +202,50 @@ export function connectSlot( ) => { const paramHandlers = handlers[transportKey][param] || [] const handlerIndex = paramHandlers.indexOf(handler) - if (handlerIndex > -1) handlers[transportKey][param].splice(handlerIndex, 1) - if (getParamHandlers(param, handlers).length === 0) callLazyDisonnectCallbacks(param) - awaitHandlerRegistration(String(transportKey), param) + + if (handlerIndex > -1) + handlers[transportKey][param].splice(handlerIndex, 1) + + if (getParamHandlers(param, handlers).length === 0) + callLazyDisonnectCallbacks(param) + + if (remoteHandlersConnected[transportKey]) + awaitHandlerRegistration(String(transportKey), param) + } - transport.addRemoteHandlerRegistrationCallback(slotName, remoteHandlerRegistered) - transport.addRemoteHandlerUnregistrationCallback(slotName, remoteHandlerUnregistered) + const remoteEventListChangedListener = () => { + // Because the remote end communicated a blacklist of event it does + // not want to listen, and because the local end has registered a + // handler for the remote end for this blacklisted events. The local + // ends need to: + // 1 - resolve the onRegister promise + // 2 - remove the useless handler from the remote handlers list + if (remoteHandlersConnected[transportKey]) { + Object.keys(remoteHandlersConnected[transportKey]).forEach( + (param) => { + remoteHandlersConnected[transportKey][ + param + ].onRegister() + } + ) + } + delete remoteHandlersConnected[transportKey] + } + + transport.addRemoteHandlerRegistrationCallback( + slotName, + remoteHandlerRegistered + ) + transport.addRemoteHandlerUnregistrationCallback( + slotName, + remoteHandlerUnregistered + ) + + transport.addRemoteEventListChangedListener( + slotName, + remoteEventListChangedListener + ) }) /* @@ -255,11 +303,12 @@ export function connectSlot( return Promise.all(transportConnectionPromises).then(() => { return callHandlersWithParameters() }) - } - - else { + } else { transports.forEach((_t, transportKey) => { - if (!remoteHandlersConnected[transportKey][param]) { + if ( + remoteHandlersConnected[transportKey] && + !remoteHandlersConnected[transportKey][param] + ) { awaitHandlerRegistration(String(transportKey), param) } }) @@ -267,8 +316,12 @@ export function connectSlot( const transportPromises: Promise[] = transports.reduce( (acc, _t, transportKey) => [ ...acc, - remoteHandlersConnected[transportKey][param].registered - ], [] + ...((remoteHandlersConnected[transportKey] && [ + remoteHandlersConnected[transportKey][param].registered + ]) ?? + []) + ], + [] ) return Promise.all(transportPromises).then(() => { diff --git a/src/Transport.ts b/src/Transport.ts index 1cb3c4b..65671b0 100644 --- a/src/Transport.ts +++ b/src/Transport.ts @@ -6,7 +6,8 @@ import { TransportRegistrationMessage, TransportUnregistrationMessage, TransportResponse, - TransportRequest + TransportRequest, + TransportEventListMessage } from './Message' let _ID = 0 @@ -74,6 +75,13 @@ export class Transport { private _remoteHandlerDeletionCallbacks: { [slotName: string]: RemoteHandlerCallback } = {} + /** + * Callbacks provided by each slot allowing to remove blacklisted events + * declaration from the remote handlers. + */ + private _remoteEventListChangedCallbacks: + { [slotName: string]: () => void } = {} + /** * Requests that have been sent to the far end, but have yet to be fulfilled */ @@ -81,7 +89,7 @@ export class Transport { private _channelReady = false - constructor(private _channel: Channel) { + constructor(private _channel: Channel, event_list?: string[]) { this._channel.onData((message: TransportMessage) => { switch (message.type) { case 'request': @@ -94,6 +102,8 @@ export class Transport { return this._unregisterRemoteHandler(message) case 'error': return this._errorReceived(message) + case 'event_list': + return this._remoteEventListReceived(message) default: assertNever(message) } @@ -107,6 +117,18 @@ export class Transport { this._channel.send(msg) }) }) + + // Also send the list of events this end is interested in so the far + // end can know when to wait for this end to be ready when triggering + // a specific slot, and when NOT to wait. + // This is necessary only when some events have been blacklisted + // when calling createEventBus + if (event_list) { + this._channel.send({ + type: "event_list", + eventList: event_list + }) + } }) this._channel.onDisconnect(() => { this._channelReady = false @@ -122,6 +144,24 @@ export class Transport { this._channel.onError(e => this._rejectAllPendingRequests(e)) } + /** + * This event is triggered when events have been blacklisted from the far + * end. It will call onRegister on blacklisted handlers to fake their + * registration so this end doesn't wait on the far end have registered them + * to be able to trigger them. + */ + private _remoteEventListReceived({ + eventList + }: TransportEventListMessage): void { + Object.keys(this._remoteEventListChangedCallbacks).forEach( + (slotName) => { + if (!eventList.includes(slotName)) { + this._remoteEventListChangedCallbacks[slotName]() + } + } + ) + } + /** * When a request is received from the far end, call all the local subscribers, * and send either a response or an error mirroring the request id, @@ -298,6 +338,16 @@ export class Transport { } } + public addRemoteEventListChangedListener( + slotName: string, + eventListChangedListener: () => void + ): void { + if (!this._remoteEventListChangedCallbacks[slotName]) { + this._remoteEventListChangedCallbacks[slotName] = + eventListChangedListener + } + } + /** * Called when a local handler is registered, to send a `handler_registered` * message to the far end. From 595d405b291e2e9affd9c0de8c54c95066745de4 Mon Sep 17 00:00:00 2001 From: Bastien Granger Date: Mon, 18 Jul 2022 09:16:16 -0400 Subject: [PATCH 4/8] test(event): add coverage for omittedEvents --- test/Event.spec.ts | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/test/Event.spec.ts b/test/Event.spec.ts index a1b2576..8e56381 100644 --- a/test/Event.spec.ts +++ b/test/Event.spec.ts @@ -1,5 +1,5 @@ import { slot } from './../src/Slot' -import { combineEvents, createEventBus } from './../src/Events' +import { combineEvents, createEventBus, omitEvents } from './../src/Events' import { TestChannel } from './TestChannel' import { DEFAULT_PARAM } from './../src/Constants' @@ -41,6 +41,32 @@ describe('combineEvents()', () => { }) +describe('omitEvent()', () => { + it('should omit events from the the list', () => { + const events = { + hello: slot<{ name: string }>(), + how: slot<{ mode: 'simple' | 'advanced' }>(), + are: slot<{ tense: number }>(), + you: slot<{ reflective: boolean }>(), + } + + const filteredList = omitEvents(events, ['hello']) + expect(Object.keys(filteredList)).toEqual(['how', 'are', 'you']) + }) + + it('should return the list of all event when omitted list is empty', () => { + const events = { + hello: slot<{ name: string }>(), + how: slot<{ mode: 'simple' | 'advanced' }>(), + are: slot<{ tense: number }>(), + you: slot<{ reflective: boolean }>(), + } + + const filteredList = omitEvents(events, []) + expect(Object.keys(filteredList)).toEqual(Object.keys(events)) + }) +}) + describe('createEventBus()', () => { const events = { From 369bb5cc58ccaddfc7e4c2058b721bf6cd05dcb2 Mon Sep 17 00:00:00 2001 From: Bastien Granger Date: Mon, 18 Jul 2022 10:24:58 -0400 Subject: [PATCH 5/8] test(slot): add coverage for the new event_list event concept --- test/Slot.spec.ts | 172 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 168 insertions(+), 4 deletions(-) diff --git a/test/Slot.spec.ts b/test/Slot.spec.ts index 3296038..5b77d0c 100644 --- a/test/Slot.spec.ts +++ b/test/Slot.spec.ts @@ -30,9 +30,7 @@ describe('slot', () => { }) describe('connectSlot', () => { - describe('without parameter', () => { - describe('trigger', () => { it('should use default parameter', async () => { const numberToString = connectSlot('numberToString', []) @@ -55,7 +53,6 @@ describe('connectSlot', () => { }) describe('with no transports', () => { - it('should call a single local handler registered for a parameter', async () => { const numberToString = connectSlot('numberToString', []) numberToString.on('a', num => `a${num.toString()}`) @@ -130,7 +127,6 @@ describe('connectSlot', () => { }) describe('with local and remote handlers', () => { - it('should call both local handlers and remote handlers', async () => { const { channel, transport } = makeTestTransport() const broadcastBool = connectSlot('broadcastBool', [transport]) @@ -140,6 +136,7 @@ describe('connectSlot', () => { // Handlers should not be called until a remote handler is registered await Promise.resolve() + expect(localCalled).toEqual(false) channel.fakeReceive({ @@ -321,4 +318,171 @@ describe('connectSlot', () => { }) }) }) + + describe('with two remote endpoints: A and B', () => { + describe('no event list sent by any endpoints', () => { + it('should wait for all remote endpoints to have signaled registration before sending the event', async () => { + const { channel: channelA, transport: transportA } = + makeTestTransport() + const { channel: channelB, transport: transportB } = + makeTestTransport() + const broadcastBool = connectSlot('broadcastBool', [ + transportA, + transportB, + ]) + + broadcastBool(true) + + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(channelA.sendSpy.mock.calls.length).toBe(0) + expect(channelB.sendSpy.mock.calls.length).toBe(0) + + // Endpoint A signals registration + channelA.fakeReceive({ + param: DEFAULT_PARAM, + slotName: 'broadcastBool', + type: 'handler_registered', + }) + + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(channelA.sendSpy.mock.calls.length).toBe(0) + expect(channelB.sendSpy.mock.calls.length).toBe(0) + + // Endpoint B signals registration + channelB.fakeReceive({ + param: DEFAULT_PARAM, + slotName: 'broadcastBool', + type: 'handler_registered', + }) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(channelA.sendSpy.mock.calls.length).toBe(1) + expect(channelB.sendSpy.mock.calls.length).toBe(1) + + const messageToA = + channelA.sendSpy.mock.calls[ + channelA.sendSpy.mock.calls.length - 1 + ][0] + expect(messageToA).toMatchObject({ + data: true, + param: DEFAULT_PARAM, + slotName: 'broadcastBool', + type: 'request', + }) + + const messageToB = + channelB.sendSpy.mock.calls[ + channelB.sendSpy.mock.calls.length - 1 + ][0] + expect(messageToB).toMatchObject({ + data: true, + param: DEFAULT_PARAM, + slotName: 'broadcastBool', + type: 'request', + }) + }) + }) + + describe('an empty event is list sent to one endpoint (A)', () => { + it('should NOT wait for remote endpoint A but SHOULD wait on remote endpoint B to have signaled registration before sending the event', async () => { + const { channel: channelA, transport: transportA } = + makeTestTransport() + const { channel: channelB, transport: transportB } = + makeTestTransport() + + const broadcastBool = connectSlot('broadcastBool', [ + transportA, + transportB, + ]) + + // Receiving an empty list is the same as blacklisting all events + channelA.fakeReceive({ + type: 'event_list', + eventList: [], + }) + + // This will be called only when B is ready we don't care about + // A as his event white list is empty + let called = false + broadcastBool.on((_b) => { + called = true + }) + + broadcastBool(true) + + await new Promise((resolve) => setTimeout(resolve, 0)) + + // Should not fire as there B is not registered + expect(called).toBe(false) + + // Endpoint B signals registration + channelB.fakeReceive({ + param: DEFAULT_PARAM, + slotName: 'broadcastBool', + type: 'handler_registered', + }) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(called).toBe(true) + }) + }) + + describe('an event list is sent to one endpoint (A)', () => { + // This is the same test as before. But this time the because the + // event IS in the white list, we need to wait for A AND B to be + // registered to trigger the event + it('should wait for remote endpoint A and remote endpoint B to have signaled registration before sending the event', async () => { + const { channel: channelA, transport: transportA } = + makeTestTransport() + const { channel: channelB, transport: transportB } = + makeTestTransport() + + const broadcastBool = connectSlot('broadcastBool', [ + transportA, + transportB, + ]) + + // This will be called only when A and B are ready + let called = false + broadcastBool.on((_b) => { + called = true + }) + + channelA.fakeReceive({ + type: 'event_list', + eventList: ['broadcastBool'], + }) + + broadcastBool(true) + + await new Promise((resolve) => setTimeout(resolve, 0)) + + // Should not fire as none of A and B are registered + expect(called).toBe(false) + + // Endpoint A signals registration + channelA.fakeReceive({ + param: DEFAULT_PARAM, + slotName: 'broadcastBool', + type: 'handler_registered', + }) + + // Should not fire as there B is not registered + expect(called).toBe(false) + + // Endpoint B signals registration + channelB.fakeReceive({ + param: DEFAULT_PARAM, + slotName: 'broadcastBool', + type: 'handler_registered', + }) + await new Promise((resolve) => setTimeout(resolve, 0)) + + // Should fire as A and B are registered + expect(called).toBe(true) + }) + }) + }) }) From 3f3843b4410ce09d5bce0ab8d90af27e5f3e7ce9 Mon Sep 17 00:00:00 2001 From: Bastien Granger Date: Wed, 20 Jul 2022 11:35:00 -0400 Subject: [PATCH 6/8] fix(transport): invert the event_list logic to blacklist --- src/Events.ts | 5 +---- src/Message.ts | 2 +- src/Transport.ts | 10 +++++----- test/Slot.spec.ts | 14 +++++++------- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/Events.ts b/src/Events.ts index d7d1308..231d099 100644 --- a/src/Events.ts +++ b/src/Events.ts @@ -51,12 +51,9 @@ export function createEventBus(args: { channels?: Channel[] blackList?: Array }): C { - const filteredEventList = args.blackList - ? Object.keys(omitEvents(args.events, args.blackList)) - : undefined const transports = (args.channels || []).map( - (c) => new Transport(c, filteredEventList) + (c) => new Transport(c, (args.blackList as string[])) ) const eventBus: Partial = {} diff --git a/src/Message.ts b/src/Message.ts index 2750064..ffb086e 100644 --- a/src/Message.ts +++ b/src/Message.ts @@ -37,7 +37,7 @@ export type TransportUnregistrationMessage = { export type TransportEventListMessage = { type: 'event_list', - eventList: string[] + blackList: string[] } export type TransportMessage = diff --git a/src/Transport.ts b/src/Transport.ts index 65671b0..f75d24b 100644 --- a/src/Transport.ts +++ b/src/Transport.ts @@ -89,7 +89,7 @@ export class Transport { private _channelReady = false - constructor(private _channel: Channel, event_list?: string[]) { + constructor(private _channel: Channel, blackList?: string[]) { this._channel.onData((message: TransportMessage) => { switch (message.type) { case 'request': @@ -123,10 +123,10 @@ export class Transport { // a specific slot, and when NOT to wait. // This is necessary only when some events have been blacklisted // when calling createEventBus - if (event_list) { + if (blackList) { this._channel.send({ type: "event_list", - eventList: event_list + blackList }) } }) @@ -151,11 +151,11 @@ export class Transport { * to be able to trigger them. */ private _remoteEventListReceived({ - eventList + blackList }: TransportEventListMessage): void { Object.keys(this._remoteEventListChangedCallbacks).forEach( (slotName) => { - if (!eventList.includes(slotName)) { + if (blackList.includes(slotName)) { this._remoteEventListChangedCallbacks[slotName]() } } diff --git a/test/Slot.spec.ts b/test/Slot.spec.ts index 5b77d0c..583fbcb 100644 --- a/test/Slot.spec.ts +++ b/test/Slot.spec.ts @@ -385,7 +385,7 @@ describe('connectSlot', () => { }) }) - describe('an empty event is list sent to one endpoint (A)', () => { + describe('a blacklist is sent to one endpoint (A)', () => { it('should NOT wait for remote endpoint A but SHOULD wait on remote endpoint B to have signaled registration before sending the event', async () => { const { channel: channelA, transport: transportA } = makeTestTransport() @@ -397,10 +397,10 @@ describe('connectSlot', () => { transportB, ]) - // Receiving an empty list is the same as blacklisting all events + // Receiving a black list of events channelA.fakeReceive({ type: 'event_list', - eventList: [], + blackList: ['broadcastBool'], }) // This will be called only when B is ready we don't care about @@ -429,10 +429,10 @@ describe('connectSlot', () => { }) }) - describe('an event list is sent to one endpoint (A)', () => { + describe('an empty blacklist is sent to one endpoint (A)', () => { // This is the same test as before. But this time the because the - // event IS in the white list, we need to wait for A AND B to be - // registered to trigger the event + // blackList is empty, we need to wait for A AND B to be registered + // to trigger the event it('should wait for remote endpoint A and remote endpoint B to have signaled registration before sending the event', async () => { const { channel: channelA, transport: transportA } = makeTestTransport() @@ -452,7 +452,7 @@ describe('connectSlot', () => { channelA.fakeReceive({ type: 'event_list', - eventList: ['broadcastBool'], + blackList: [], }) broadcastBool(true) From 2b86a22b479fe5adf1541b88dc576e836aa6aa49 Mon Sep 17 00:00:00 2001 From: Bastien Granger Date: Wed, 20 Jul 2022 12:21:59 -0400 Subject: [PATCH 7/8] chore: renamed the blackList ignoredEvents --- src/Events.ts | 4 ++-- src/Message.ts | 2 +- src/Transport.ts | 41 ++++++++++++++++++++--------------------- test/Slot.spec.ts | 16 ++++++++-------- 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/Events.ts b/src/Events.ts index 231d099..dd1eded 100644 --- a/src/Events.ts +++ b/src/Events.ts @@ -49,11 +49,11 @@ export function omitEvents< export function createEventBus(args: { events: C channels?: Channel[] - blackList?: Array + ignoredEvents?: Array }): C { const transports = (args.channels || []).map( - (c) => new Transport(c, (args.blackList as string[])) + (c) => new Transport(c, (args.ignoredEvents as string[])) ) const eventBus: Partial = {} diff --git a/src/Message.ts b/src/Message.ts index ffb086e..a3a512d 100644 --- a/src/Message.ts +++ b/src/Message.ts @@ -37,7 +37,7 @@ export type TransportUnregistrationMessage = { export type TransportEventListMessage = { type: 'event_list', - blackList: string[] + ignoredEvents: string[] } export type TransportMessage = diff --git a/src/Transport.ts b/src/Transport.ts index f75d24b..4702f80 100644 --- a/src/Transport.ts +++ b/src/Transport.ts @@ -79,7 +79,7 @@ export class Transport { * Callbacks provided by each slot allowing to remove blacklisted events * declaration from the remote handlers. */ - private _remoteEventListChangedCallbacks: + private _remoteIgnoredEventsCallbacks: { [slotName: string]: () => void } = {} /** @@ -89,7 +89,7 @@ export class Transport { private _channelReady = false - constructor(private _channel: Channel, blackList?: string[]) { + constructor(private _channel: Channel, ignoredEvents?: string[]) { this._channel.onData((message: TransportMessage) => { switch (message.type) { case 'request': @@ -103,7 +103,7 @@ export class Transport { case 'error': return this._errorReceived(message) case 'event_list': - return this._remoteEventListReceived(message) + return this._remoteIgnoredEventsReceived(message) default: assertNever(message) } @@ -118,15 +118,14 @@ export class Transport { }) }) - // Also send the list of events this end is interested in so the far - // end can know when to wait for this end to be ready when triggering - // a specific slot, and when NOT to wait. - // This is necessary only when some events have been blacklisted - // when calling createEventBus - if (blackList) { + // Also send the list of events this end is not interested in so the + // far end can know when to wait or not for this end to be ready + // when triggering a specific slot. This is necessary only when some + // events have been listed as ignored when calling createEventBus + if (ignoredEvents) { this._channel.send({ type: "event_list", - blackList + ignoredEvents }) } }) @@ -145,18 +144,18 @@ export class Transport { } /** - * This event is triggered when events have been blacklisted from the far - * end. It will call onRegister on blacklisted handlers to fake their - * registration so this end doesn't wait on the far end have registered them - * to be able to trigger them. + * This event is triggered when events have been listed as ignored by the far + * end. It will call onRegister on ignored events' handlers to fake their + * registration so this end doesn't wait on the far end to have registered + * them to be able to trigger them. */ - private _remoteEventListReceived({ - blackList + private _remoteIgnoredEventsReceived({ + ignoredEvents }: TransportEventListMessage): void { - Object.keys(this._remoteEventListChangedCallbacks).forEach( + Object.keys(this._remoteIgnoredEventsCallbacks).forEach( (slotName) => { - if (blackList.includes(slotName)) { - this._remoteEventListChangedCallbacks[slotName]() + if (ignoredEvents.includes(slotName)) { + this._remoteIgnoredEventsCallbacks[slotName]() } } ) @@ -342,8 +341,8 @@ export class Transport { slotName: string, eventListChangedListener: () => void ): void { - if (!this._remoteEventListChangedCallbacks[slotName]) { - this._remoteEventListChangedCallbacks[slotName] = + if (!this._remoteIgnoredEventsCallbacks[slotName]) { + this._remoteIgnoredEventsCallbacks[slotName] = eventListChangedListener } } diff --git a/test/Slot.spec.ts b/test/Slot.spec.ts index 583fbcb..85c8fed 100644 --- a/test/Slot.spec.ts +++ b/test/Slot.spec.ts @@ -320,7 +320,7 @@ describe('connectSlot', () => { }) describe('with two remote endpoints: A and B', () => { - describe('no event list sent by any endpoints', () => { + describe('no ignoredEvents list sent by any endpoints', () => { it('should wait for all remote endpoints to have signaled registration before sending the event', async () => { const { channel: channelA, transport: transportA } = makeTestTransport() @@ -385,7 +385,7 @@ describe('connectSlot', () => { }) }) - describe('a blacklist is sent to one endpoint (A)', () => { + describe('an ignoredEvents list is sent to one endpoint (A)', () => { it('should NOT wait for remote endpoint A but SHOULD wait on remote endpoint B to have signaled registration before sending the event', async () => { const { channel: channelA, transport: transportA } = makeTestTransport() @@ -397,10 +397,10 @@ describe('connectSlot', () => { transportB, ]) - // Receiving a black list of events + // Receiving a list of events to ignore channelA.fakeReceive({ type: 'event_list', - blackList: ['broadcastBool'], + ignoredEvents: ['broadcastBool'], }) // This will be called only when B is ready we don't care about @@ -429,10 +429,10 @@ describe('connectSlot', () => { }) }) - describe('an empty blacklist is sent to one endpoint (A)', () => { + describe('an empty ignoredEvents list is sent to one endpoint (A)', () => { // This is the same test as before. But this time the because the - // blackList is empty, we need to wait for A AND B to be registered - // to trigger the event + // ignoredEvents list is empty, we need to wait for A AND B to been + // registered to trigger the event. it('should wait for remote endpoint A and remote endpoint B to have signaled registration before sending the event', async () => { const { channel: channelA, transport: transportA } = makeTestTransport() @@ -452,7 +452,7 @@ describe('connectSlot', () => { channelA.fakeReceive({ type: 'event_list', - blackList: [], + ignoredEvents: [], }) broadcastBool(true) From b3c821672399957bd3a776912811762817b31f82 Mon Sep 17 00:00:00 2001 From: Bastien Granger Date: Wed, 20 Jul 2022 15:05:03 -0400 Subject: [PATCH 8/8] fix(events): omit ignored events in event bus creation --- src/Events.ts | 33 +++++++++++++------------------ src/index.ts | 3 +-- test/Event.spec.ts | 49 +++++++++++++++++++++------------------------- 3 files changed, 37 insertions(+), 48 deletions(-) diff --git a/src/Events.ts b/src/Events.ts index dd1eded..bf05cdf 100644 --- a/src/Events.ts +++ b/src/Events.ts @@ -34,23 +34,14 @@ export function combineEvents< return Object.assign({}, ...args) } -export function omitEvents< - Events extends EventDeclaration, - OmittedEvents extends keyof Events ->(events: Events, omittedEvents: OmittedEvents[]): Omit { - return Object.keys(events).reduce((acc, event) => { - if (!omittedEvents.includes(event as OmittedEvents)) { - acc[event as keyof Events] = events[event] - } - return acc - }, {} as any) -} - -export function createEventBus(args: { - events: C - channels?: Channel[] - ignoredEvents?: Array -}): C { +export function createEventBus< + C extends EventDeclaration, + T extends Array +>(args: { + events: C; + channels?: Channel[]; + ignoredEvents?: T; +}): Omit { const transports = (args.channels || []).map( (c) => new Transport(c, (args.ignoredEvents as string[])) @@ -58,7 +49,11 @@ export function createEventBus(args: { const eventBus: Partial = {} for (const event in args.events) { - if (args.events.hasOwnProperty(event)) { + if ( + args.events.hasOwnProperty(event) && + (!args.ignoredEvents || + (args.ignoredEvents && !args.ignoredEvents.includes(event))) + ) { eventBus[event] = connectSlot( event, transports, @@ -67,5 +62,5 @@ export function createEventBus(args: { } } - return eventBus as C + return eventBus as Omit } diff --git a/src/index.ts b/src/index.ts index b6e779f..9777a82 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2,8 +2,7 @@ export { slot, Slot } from './Slot' export { EventDeclaration, combineEvents, - createEventBus, - omitEvents + createEventBus } from './Events' export { Channel } from './Channel' export { GenericChannel } from './Channels/GenericChannel' diff --git a/test/Event.spec.ts b/test/Event.spec.ts index 8e56381..912212f 100644 --- a/test/Event.spec.ts +++ b/test/Event.spec.ts @@ -41,36 +41,11 @@ describe('combineEvents()', () => { }) -describe('omitEvent()', () => { - it('should omit events from the the list', () => { - const events = { - hello: slot<{ name: string }>(), - how: slot<{ mode: 'simple' | 'advanced' }>(), - are: slot<{ tense: number }>(), - you: slot<{ reflective: boolean }>(), - } - - const filteredList = omitEvents(events, ['hello']) - expect(Object.keys(filteredList)).toEqual(['how', 'are', 'you']) - }) - - it('should return the list of all event when omitted list is empty', () => { - const events = { - hello: slot<{ name: string }>(), - how: slot<{ mode: 'simple' | 'advanced' }>(), - are: slot<{ tense: number }>(), - you: slot<{ reflective: boolean }>(), - } - - const filteredList = omitEvents(events, []) - expect(Object.keys(filteredList)).toEqual(Object.keys(events)) - }) -}) - describe('createEventBus()', () => { const events = { - numberToString: slot() + numberToString: slot(), + eventToIgnore: slot() } const param = DEFAULT_PARAM @@ -127,4 +102,24 @@ describe('createEventBus()', () => { data: '5' }) }) + + describe('when a ignored list is passed to createEventBus()', () => { + it('should not connect the ignored slot and should return a filtered eventBus', () => { + const channel = new TestChannel() + const eventBus = createEventBus({ + events, + channels: [channel], + ignoredEvents: ['eventToIgnore'] + }) + channel.callConnected() + const isIncluded = Object.keys(eventBus).includes('eventToIgnore') + expect(isIncluded).toBe(false) + // An event_list message should have been received with the list of + // ignoredEvents + expect(channel.sendSpy).toHaveBeenCalledWith({ + type: 'event_list', + ignoredEvents: ['eventToIgnore'] + }) + }) + }) })