Skip to content

Commit

Permalink
Fix: don't remove remote handler when removing an additional handler
Browse files Browse the repository at this point in the history
We only send `handler_registered` when a first handler is registered,
and `handler_unregsitered` when the last handler is unregistered.

I also put some testing logic in beforeEach statements so that the tests are
independant from each others.
  • Loading branch information
Alexandre Hervé authored and Alexandre Hervé committed Sep 27, 2018
1 parent 3aa9f8e commit 5edd311
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 44 deletions.
41 changes: 26 additions & 15 deletions src/Transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,14 +271,19 @@ export class Transport {
this._localHandlers[slotName] = []
}
this._localHandlers[slotName].push(handler)

const registrationMessage: TransportRegistrationMessage = {
type: 'handler_registered',
slotName
}
this._localHandlerRegistrations.push(registrationMessage)
if (this._channelReady) {
this._channel.send(registrationMessage)
/**
* We notify the far end when adding the first handler only, as they
* only need to know if at least one handler is connected.
*/
if (this._localHandlers[slotName].length === 1) {
const registrationMessage: TransportRegistrationMessage = {
type: 'handler_registered',
slotName
}
this._localHandlerRegistrations.push(registrationMessage)
if (this._channelReady) {
this._channel.send(registrationMessage)
}
}
}

Expand All @@ -290,13 +295,19 @@ export class Transport {
if (this._localHandlers[slotName]) {
const ix = this._localHandlers[slotName].indexOf(handler)
if (ix > -1) {
this._localHandlers[slotName].splice(ix)
const unregistrationMessage: TransportUnregistrationMessage = {
type: 'handler_unregistered',
slotName
}
if (this._channelReady) {
this._channel.send(unregistrationMessage)
this._localHandlers[slotName].splice(ix, 1)
/**
* We notify the far end when removing the last handler only, as they
* only need to know if at least one handler is connected.
*/
if (this._localHandlers[slotName].length === 0) {
const unregistrationMessage: TransportUnregistrationMessage = {
type: 'handler_unregistered',
slotName
}
if (this._channelReady) {
this._channel.send(unregistrationMessage)
}
}
}
}
Expand Down
114 changes: 85 additions & 29 deletions test/Transport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Transport } from './../src/Transport'
import { TransportMessage } from './../src/Message'
import { TestChannel } from './TestChannel'
import * as sinon from 'sinon'
import { SinonSpy } from 'sinon'
import { createEventBus } from '../src/Events'
import { slot } from '../src/Slot'

Expand All @@ -26,28 +27,49 @@ describe('Transport', () => {

context('handler registration, requests and unregistration', () => {

const channel = new TestChannel()
const transport = new Transport(channel)
const handlers = {
buildCelery: sinon.spy(() => ({ color: 'blue' })),
getCarrotStock: sinon.spy()
}
let channel: TestChannel
let transport: Transport
let slots: { [slotName: string]: SinonSpy[] }

it('should send a handler_registered message when a local handler is registered', () => {
beforeEach(() => {
slots = {
buildCelery: [sinon.spy(() => ({ color: 'blue' }))],
getCarrotStock: [sinon.spy(), sinon.spy()]
}
channel = new TestChannel()
transport = new Transport(channel)
channel.callConnected()
Object.keys(handlers).forEach(slotName => {
transport.registerHandler(slotName, handlers[slotName])
})

it('should send a handler_registered message for each slot when a local handler is registered', () => {
Object.keys(slots).forEach(slotName => {
transport.registerHandler(slotName, slots[slotName][0])
channel.sendSpy.calledWith({
type: 'handler_registered',
slotName
}).should.be.True()
})
})

it('should not send a handler_registered message when an additional local handler is registered', () => {
const slotName = 'getCarrotStock'
transport.registerHandler(slotName, slots[slotName][0])
transport.registerHandler(slotName, slots[slotName][1])
channel.sendSpy
.withArgs({ type: 'handler_registered', slotName })
.calledOnce // should have been called exactly once
.should.be.True()
})


it('should call the appropriate handler when a request is received', async () => {

const slotName = 'buildCelery'
const handler = handlers[slotName]
handler.called.should.be.False()
const handler = slots[slotName][0]

// Register handler on slot
transport.registerHandler(slotName, handler)

const request: TransportMessage = {
type: 'request',
slotName,
Expand All @@ -57,6 +79,7 @@ describe('Transport', () => {
constitution: 'strong'
}
}

channel.fakeReceive(request)

await Promise.resolve() // yield to ts-event-bus internals
Expand All @@ -71,19 +94,33 @@ describe('Transport', () => {
})
})

it('should send a handler_unregistered message when a local handler is unregistered', () => {
Object.keys(handlers).forEach(slotName => {
transport.unregisterHandler(slotName, handlers[slotName])
channel.sendSpy.calledWith({
type: 'handler_unregistered',
slotName
}).should.be.True()
})
it('should send a handler_unregistered message when the last local handler is unregistered', () => {

const slotName = 'buildCelery'

// Register one handler on slot
transport.registerHandler(slotName, slots[slotName][0])

// Unregister it
transport.unregisterHandler(slotName, slots[slotName][0])

channel.sendSpy.calledWith({
type: 'handler_unregistered',
slotName
}).should.be.True()
})

it('should not call the unregistered handler when a request is received', async () => {

const slotName = 'buildCelery'
const handler = handlers[slotName]
const handler = slots[slotName][0]

// Register one handler on slot
transport.registerHandler(slotName, handler)

// Unregister it
transport.unregisterHandler(slotName, slots[slotName][0])

const request: TransportMessage = {
type: 'request',
slotName,
Expand All @@ -93,29 +130,48 @@ describe('Transport', () => {
constitution: 'strong'
}
}
handler.resetHistory()
channel.fakeReceive(request)
await Promise.resolve() // yield to ts-event-bus internals
handler.called.should.be.False()
})

it('should not send a handler_unregistered message when an additional local handler is unregistered', () => {

const slotName = 'getCarrotStock'

// Register two handlers on slot
transport.registerHandler(slotName, slots[slotName][0])
transport.registerHandler(slotName, slots[slotName][1])

// Unregister one handler only
transport.unregisterHandler(slotName, slots[slotName][0])

channel.sendSpy.calledWith({
type: 'handler_unregistered',
slotName
}).should.be.False()
})

context('adding, using and removing a remote handler', () => {

const slotName = 'getCarrotStock'
const addLocalHandler = sinon.spy()
const removeLocalHandler = sinon.spy()

let addLocalHandler: SinonSpy
let removeLocalHandler: SinonSpy
let localHandler: (...args: any[]) => Promise<any>

it('should add a local handler when a remote handler registration is received', () => {
beforeEach(() => {
addLocalHandler = sinon.spy()
removeLocalHandler = sinon.spy()
transport.onRemoteHandlerRegistered(slotName, addLocalHandler)
channel.fakeReceive({
type: 'handler_registered',
slotName
})
addLocalHandler.called.should.be.True()
channel.fakeReceive({ type: 'handler_registered', slotName })
localHandler = addLocalHandler.lastCall.args[0]
})

it('should add a local handler when a remote handler registration is received', () => {
addLocalHandler.called.should.be.True()
})

it('should resolve a local pending request when a response is received', () => {
const requestData = { carrotType: 'red' }
const pendingPromise = localHandler(requestData)
Expand Down

0 comments on commit 5edd311

Please sign in to comment.