Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SYSTEST-9338 - Send events to the WS object that subscribed to it #122

Merged
merged 5 commits into from
Apr 21, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions server/src/events.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ function logFatalErr() {
const eventListenerMap = {};

// Associate this message ID with this method so if/when events are sent, we know which message ID to use
function registerEventListener(userId, oMsg) {
function registerEventListener(userId, oMsg, ws) {
if ( ! eventListenerMap[userId] ) {
eventListenerMap[userId] = {};
}
eventListenerMap[userId][oMsg.method] = oMsg.id;
eventListenerMap[userId][oMsg.method] = { id: oMsg.id, ws: ws };
logger.debug(`Registered event listener mapping: ${userId}:${oMsg.method}:${oMsg.id}`);
}

Expand All @@ -80,12 +80,14 @@ function getRegisteredEventListener(userId, method) {

// Remove mapping from event listener request method name from our map
// Attempts to send events to this listener going forward will fail
function deregisterEventListener(userId, oMsg) {
if ( ! eventListenerMap[userId] ) { return; }
delete eventListenerMap[userId][oMsg.method];
logger.debug(`Deregistered event listener for method: ${userId}:${oMsg.method}`);
function deregisterEventListener(userId, oMsg, ws) {
if (!eventListenerMap[userId]) { return; }

if ( eventListenerMap[userId] && eventListenerMap[userId][oMsg.method] && eventListenerMap[userId][oMsg.method].ws === ws ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we log some kind of error if the ws values don't match? Would this be an unexpected error to somehow track down? Or... should we not compare ws values and nuke the listener record no matter what?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. That brings up a scenario I don't know if we thought of. What if a user is listening to the same method across multiple connections. Why they would do this I don't know but I guess it's possible.

Considering the fact that we're still limiting to one listen per user per method I'd say we just nuke it no matter what. But maybe we want to put in the feature for multiple listens per user per connection?

Keaton, what'd be the difficulty on that? I'm assuming we'd just be setting [ws] as an array and some logic on top to add/remove the proper one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per your request, I was able to set ws key to an array. I have renamed it to wsArr and we can now store multiple ws connections inside of the array.

delete eventListenerMap[userId][oMsg.method];
logger.debug(`Deregistered event listener mapping: ${userId}:${oMsg.method}`);
}
}

// Is the given (incoming) message one that enables or disables an event listener?
// Example: {"jsonrpc":"2.0","method":"lifecycle.onInactive","params":{"listen":true|false},"id":1}
// Key: The 'on' in the unqualified method name, and (2) the params.listen parameter (regardless of true|false)
Expand Down Expand Up @@ -142,8 +144,13 @@ function sendBroadcastEvent(ws, userId, method, result, msg, fSuccess, fErr, fFa
}

// sending response to web-socket
function emitResponse(ws, finalResult, msg, userId, method){
let id = getRegisteredEventListener(userId, method);
function emitResponse(finalResult, msg, userId, method) {
const listener = getRegisteredEventListener(userId, method);
if (!listener) {
logger.debug('Event message could not be sent because a listener was not found')
return;
}
const { id, ws } = listener;
const oEventMessage = {
jsonrpc: '2.0',
id: id,
Expand All @@ -155,6 +162,7 @@ function emitResponse(ws, finalResult, msg, userId, method){
logger.info(`${msg}: Sent event message to user ${userId}: ${eventMessage}`);
}


// sendEvent to handle post API event calls, including pre- and post- event trigger processing
function coreSendEvent(isBroadcast, ws, userId, method, result, msg, fSuccess, fErr, fFatalErr) {
try {
Expand Down Expand Up @@ -247,7 +255,7 @@ function coreSendEvent(isBroadcast, ws, userId, method, result, msg, fSuccess, f
// looping over each web-sockets of same group
if ( wsUserMap && wsUserMap.size >=1 ) {
wsUserMap.forEach ((userWithSameGroup, ww) => {
emitResponse(ww, finalResult, msg, userWithSameGroup, method);
emitResponse(finalResult, msg, userWithSameGroup, method);
});
fSuccess.call(null);
} else {
Expand All @@ -256,7 +264,7 @@ function coreSendEvent(isBroadcast, ws, userId, method, result, msg, fSuccess, f
throw new Error(msg);
}
} else {
emitResponse(ws, finalResult, msg, userId, method);
emitResponse(finalResult, msg, userId, method);
fSuccess.call(null);
}
}
Expand Down
4 changes: 2 additions & 2 deletions server/src/messageHandler.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ async function handleMessage(message, userId, ws) {

if ( events.isEventListenerOnMessage(oMsg) ) {
events.sendEventListenerAck(userId, ws, oMsg);
events.registerEventListener(userId, oMsg);
events.registerEventListener(userId, oMsg, ws);
return;
}

// Handle JSON-RPC messages that are event listener disable requests

if ( events.isEventListenerOffMessage(oMsg) ) {
events.deregisterEventListener(userId, oMsg);
events.deregisterEventListener(userId, oMsg, ws);
return;
}

Expand Down
18 changes: 12 additions & 6 deletions server/test/suite/events.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ test(`events.registerEventListener works properly`, () => {
method: "",
id: 12,
};
events.registerEventListener("12345", dummyObject);
const dummyWebSocket = { send: () => {} };
events.registerEventListener("12345", dummyObject, dummyWebSocket);
expect(spy).toHaveBeenCalled();
});

Expand Down Expand Up @@ -92,7 +93,8 @@ test(`events.getRegisteredEventListener works properly if path`, () => {
test(`events.deregisterEventListener works properly`, () => {
const spy = jest.spyOn(logger, "debug");
const oMsgdummy = { method: "lifecycle.onInactive", id: 12 };
events.deregisterEventListener(oMsgdummy);
const dummyWebSocket = { send: () => {} };
events.deregisterEventListener("12345", oMsgdummy.method, dummyWebSocket);
expect(spy).toHaveBeenCalled();
});

Expand Down Expand Up @@ -177,7 +179,8 @@ test(`events.isEventListenerOffMessage works properly`, () => {
test(`events.sendEventListenerAck works properly`, () => {
const spy = jest.spyOn(logger, "debug");
const oMsgdummy = { method: "lifecycle.onInactive", id: 12 };
events.sendEventListenerAck('12345', { send: () => {} }, oMsgdummy);
const dummyWebSocket = { send: () => {} };
events.sendEventListenerAck("12345", dummyWebSocket, oMsgdummy);
expect(spy).toHaveBeenCalled();
});

Expand Down Expand Up @@ -205,9 +208,11 @@ test(`events.sendEvent works properly`, () => {
call: () => {},
},
};
events.registerEventListener("12345", dummyObject);
const dummyWebSocket = { send: () => {} };

events.registerEventListener("12345", dummyObject, dummyWebSocket);
events.sendEvent(
{ send: () => {} },
dummyWebSocket,
"12345",
methodName,
result,
Expand Down Expand Up @@ -286,8 +291,9 @@ test(`events.isAnyRegisteredInGroup works properly with if path`, () => {
});

test(`events.sendBroadcastEvent works properly`, () => {
const dummyWebSocket = { send: () => {} };
const result = events.testExports.sendBroadcastEvent(
{ send: () => {} },
dummyWebSocket,
"12345",
"core",
{},
Expand Down