Resolve socket server overload

[szczz]

#525

Changes

• Updates the socket server and client side code to make use of broadcasts. Should resolve the overloading issue.

These changes have been deployed to dev so please test there.

Testing

Steps:

1. Ensure you can perform all create, save, rename etc actions on a Storyline
2. A Storyline should not be accessible from more than one tab
3. Please ensure timeout logic works as expected. Currently set to 5 minutes with a 2 min warning.

---

This change is 

[github-actions]

Your demo site is ready! 🚀 Visit it here: https://ramp4-pcar4.github.io/storylines-editor/SocketServerFix

[mohsin-r]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @szczz)

---

server/index.js line 586 at r1 (raw file):

// Used to broadcast messages to all connected clients
function broadcastToClients(message){

Curious why we need to broadcast a storyline's lock/unlock to all the clients, instead of only the client that requested it (which we already do below with the ws.send).

---

server/index.js line 626 at r1 (raw file):

                logger('INFO', `A client successfully locked the storyline ${uuid}.`);
                const secret = generateKey();
                lockedUuids[uuid] = { secret };

Since we just have one property now, we could probably go back storing the secret itself instead of an object with the secret inside it. But totally upto you since its a pedantic grouse.

---

src/stores/lockStore.ts line 82 at r1 (raw file):

            clearInterval(this.timeInterval);
            if (this.connected) {
                this.socket!.send(JSON.stringify({ uuid: this.uuid, user: this.userName, lock: false }));

I believe the username reference should be removed here? Doesn't seem to be anywhere else.

[szczz]

Reviewable status: 1 of 3 files reviewed, 3 unresolved discussions (waiting on @mohsin-r)

---

server/index.js line 586 at r1 (raw file):

Previously, mohsin-r (Mohsin Reza) wrote…

Curious why we need to broadcast a storyline's lock/unlock to all the clients, instead of only the client that requested it (which we already do below with the ws.send).

This is to prevent polling and repeated requests, in a way its a touch of future proofing as well.

The idea is if all clients are aware that a storyline has been locked, the client won't repeatedly try to access the storyline like we were seeing in the DB (refer to the screenshot I sent on teams). Essentially each client was doing this whenever it requested a locked storyline

Similarly, if we broadcast an unlock message, all clients are aware that a storyline can be accessed.

---

server/index.js line 626 at r1 (raw file):

Previously, mohsin-r (Mohsin Reza) wrote…

Since we just have one property now, we could probably go back storing the secret itself instead of an object with the secret inside it. But totally upto you since its a pedantic grouse.

Donethanks

---

src/stores/lockStore.ts line 82 at r1 (raw file):

Previously, mohsin-r (Mohsin Reza) wrote…

I believe the username reference should be removed here? Doesn't seem to be anywhere else.

Donethanks

[yileifeng]

Functionality and code looks fine to me, just logged 2 issues found with renaming that can be addressed in future: #538, #539

Reviewed 1 of 3 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @mohsin-r)

[mohsin-r]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @szczz)

---

server/index.js line 586 at r1 (raw file):

Previously, szczz (Matt Szczerba) wrote…

This is to prevent polling and repeated requests, in a way its a touch of future proofing as well.

The idea is if all clients are aware that a storyline has been locked, the client won't repeatedly try to access the storyline like we were seeing in the DB (refer to the screenshot I sent on teams). Essentially each client was doing this whenever it requested a locked storyline

Similarly, if we broadcast an unlock message, all clients are aware that a storyline can be accessed.

Gotcha