Skip to content

Commit

Permalink
Check sender of ENTER presence message in space.enter()
Browse files Browse the repository at this point in the history
It shouldn’t return as a result of somebody else entering the space.

I’m not sure whether this is the best way of predicating whether or not
to return — perhaps checking for some unique ID on the received ENTER
presence message would be better. But it seems like an OK low-thought
solution at least for the time being.
  • Loading branch information
lawrence-forooghian committed Oct 19, 2023
1 parent 1431c3c commit a4e0351
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 2 deletions.
62 changes: 61 additions & 1 deletion src/Space.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ describe('Space', () => {
beforeEach<SpaceTestContext>(({ presence }) => {
vi.spyOn(presence, 'subscribe').mockImplementation(
async (_, listener?: (presenceMessage: Types.PresenceMessage) => void) => {
listener!(createPresenceMessage('enter' /* arbitrarily chosen */));
listener!(
createPresenceMessage('enter' /* arbitrarily chosen */, { clientId: 'MOCK_CLIENT_ID', connectionId: '1' }),
);
},
);
});
Expand All @@ -74,6 +76,64 @@ describe('Space', () => {
expect(spy).toHaveBeenNthCalledWith(1, createProfileUpdate({ current: { name: 'Betty' } }));
});

describe.each([
{
scenario: 'when it receives a presence message from a client ID and connection ID that matches ours',
presenceMessageData: { clientId: 'MOCK_CLIENT_ID', connectionId: '1' },
shouldComplete: true,
},
{
scenario: 'when it receives a presence message from a client ID that isn’t ours',
presenceMessageData: { clientId: 'OTHER_MOCK_CLIENT_ID', connectionId: '1' },
shouldComplete: false,
},
{
scenario: 'when it receives a presence message from a connection ID that isn’t ours',
presenceMessageData: { clientId: 'MOCK_CLIENT_ID', connectionId: '2' },
shouldComplete: false,
},
])('$scenario', ({ presenceMessageData, shouldComplete }) => {
it<SpaceTestContext>(shouldComplete ? 'completes' : 'does not complete', async ({ space, presence }) => {
const unsubscribeSpy = vi.spyOn(presence, 'unsubscribe');

vi.spyOn(presence, 'subscribe').mockImplementation(
async (_, listener?: (presenceMessage: Types.PresenceMessage) => void) => {
listener!(createPresenceMessage('enter' /* arbitrarily chosen */, presenceMessageData));
},
);

space.enter();

// Note: There’s no nice way (i.e. without timeouts) to actually check that space.enter() didn’t complete, so we use "did it remove its presence listener?" as a proxy for "did it complete?"
if (shouldComplete) {
expect(unsubscribeSpy).toHaveBeenCalled();
} else {
expect(unsubscribeSpy).not.toHaveBeenCalled();
}
});
});

it<SpaceTestContext>('doesn’t complete as a result of a presence message from a client ID that is not ours', async ({
space,
presence,
}) => {
const unsubscribeSpy = vi.spyOn(presence, 'unsubscribe');

vi.spyOn(presence, 'subscribe').mockImplementation(
async (_, listener?: (presenceMessage: Types.PresenceMessage) => void) => {
listener!(
createPresenceMessage('enter' /* arbitrarily chosen */, {
clientId: 'OTHER_MOCK_CLIENT_ID',
connectionId: '1',
}),
);
},
);

space.enter();
expect(unsubscribeSpy).not.toHaveBeenCalled();
});

it<SpaceTestContext>('returns current space members', async ({ presenceMap, space }) => {
presenceMap.set('1', createPresenceMessage('enter'));
presenceMap.set('2', createPresenceMessage('update', { clientId: '2', connectionId: '2' }));
Expand Down
11 changes: 10 additions & 1 deletion src/Space.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,16 @@ class Space extends EventEmitter<SpaceEventMap> {
return new Promise((resolve) => {
const presence = this.channel.presence;

const presenceListener = async () => {
const presenceListener = async (presenceMessage: Types.PresenceMessage) => {
if (
!(
presenceMessage.clientId == this.client.auth.clientId &&
presenceMessage.connectionId == this.client.connection.id
)
) {
return;
}

presence.unsubscribe(presenceListener);

const presenceMessages = await presence.get();
Expand Down

0 comments on commit a4e0351

Please sign in to comment.