Skip to content

Commit

Permalink
Disallow to unregister if another function has been registered
Browse files Browse the repository at this point in the history
  • Loading branch information
jrmi committed Feb 5, 2023
1 parent aef559d commit 53f06cc
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 16 deletions.
2 changes: 1 addition & 1 deletion docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ RUN chown -R node /usr/src/app

RUN apk add --no-cache tini

RUN npm install -g npm && npm install -g wire.io@latest
RUN npm install -g npm && npm install -g wire.io@3.3.1

USER node

Expand Down
12 changes: 7 additions & 5 deletions src/__snapshots__/client.test.js.snap
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Client should call remote first function 1`] = `undefined`;
exports[`Client should call remote first function 1`] = `"Function testrpc is not registered"`;

exports[`Client should call remote last function 1`] = `undefined`;
exports[`Client should call remote last function 1`] = `"Function testrpc is not registered"`;

exports[`Client should call remote random function 1`] = `undefined`;
exports[`Client should call remote random function 1`] = `"Function testrpc is not registered"`;

exports[`Client should not be able to register different function type 1`] = `undefined`;
exports[`Client should not be able to register different function type 1`] = `"Can't register a new function under the testrpc with this invoke value."`;

exports[`Client should not be able to register different function type 2`] = `undefined`;
exports[`Client should not be able to register different function type 2`] = `"Function testrpc already exists"`;

exports[`Client should not be able to register different function type 3`] = `"Function testrpc is not registered"`;
7 changes: 5 additions & 2 deletions src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class Wire {
*/
async register(name, callback, { invoke = 'single' } = {}) {
// Add to locally registered callback

this.registeredRPC[name] = callback;

await this._callServerRPC('register', {
Expand All @@ -114,8 +115,10 @@ class Wire {

// Return unregister callback
const unregisterCallback = () => {
delete this.registeredRPC[name];
return this._callServerRPC('unregister', { name });
if (this.registeredRPC[name] === callback) {
delete this.registeredRPC[name];
return this._callServerRPC('unregister', { name });
}
};

this.toUnregister.push(unregisterCallback);
Expand Down
35 changes: 29 additions & 6 deletions src/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ describe('Client', () => {
retry(() => room3.call('testrpc'))
.withTimeout(1000)
.until((result) => expect(result).toEqual({}))
).rejects.toThrowErrorMatchingSnapshot();
).rejects.toMatchSnapshot();
});

it('should call remote last function', async () => {
Expand Down Expand Up @@ -292,7 +292,7 @@ describe('Client', () => {
retry(() => room3.call('testrpc'))
.withTimeout(1000)
.until((result) => expect(result).toEqual({}))
).rejects.toThrowErrorMatchingSnapshot();
).rejects.toMatchSnapshot();
});

it('should call remote random function', async () => {
Expand Down Expand Up @@ -341,7 +341,7 @@ describe('Client', () => {
retry(() => room3.call('testrpc'))
.withTimeout(1000)
.until((result) => expect(result).toEqual({}))
).rejects.toThrowErrorMatchingSnapshot();
).rejects.toMatchSnapshot();
});

it('should not be able to register different function type', async () => {
Expand All @@ -365,7 +365,7 @@ describe('Client', () => {
},
{ invoke: 'first' }
)
).rejects.toThrowErrorMatchingSnapshot();
).rejects.toMatchSnapshot();

await unregister1();

Expand All @@ -377,10 +377,33 @@ describe('Client', () => {
expect(result).toEqual({ answer: 46 });

await expect(
room1.register('testrpc', (params) => {
room2.register('testrpc', (params) => {
return { answer: 47 };
})
).rejects.toThrowErrorMatchingSnapshot();
).rejects.toMatchSnapshot();

// We should be able to replace a single RPC from the registerer
const unregister2 = await room1.register('testrpc', (params) => {
return { answer: 47 };
});

result = await room3.call('testrpc');
expect(result).toEqual({ answer: 47 });

const unregister3 = await room1.register('testrpc', (params) => {
return { answer: 48 };
});

// If we unregister after the function has been replaced it shouldn't unregister
// the existing function
await unregister2();

result = await room3.call('testrpc');
expect(result).toEqual({ answer: 48 });

await unregister3();

await expect(room3.call('testrpc')).rejects.toMatchSnapshot();
});

it('should call onMaster callback on leave and disconnect', async () => {
Expand Down
6 changes: 4 additions & 2 deletions src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,17 @@ export const handleWire = (
*/
const register = ({ name, invoke = 'single' }) => {
const existingInvoke = rooms[roomName].rpc[name]?.invoke;
const existingCallbacks = rooms[roomName].rpc[name]?.callbacks || [];

if (existingInvoke && invoke !== existingInvoke) {
throw new Error(
`Can't register a new function under the ${name} with this invoke value.`
);
}
if (
existingInvoke == 'single' &&
rooms[roomName].rpc[name].callbacks.length >= 1
existingInvoke === 'single' &&
existingCallbacks.length >= 1 &&
existingCallbacks[0] !== registeredRPCs[name]
) {
throw new Error(`Function ${name} already exists`);
}
Expand Down

0 comments on commit 53f06cc

Please sign in to comment.