BOT-296/Server Modules

[Mecharyry]

Problem

A majority, if not all, of the Server logic is stored in a single file. This made testing the logic difficult because certain scenarios had to be built from a sequence of user actions.

Solution

Split the Server into testable modules that use mocks for all dependencies.

• Router determines whether a given client, human or bot, is able to connect to the server. A human client is passed off to the BotLocator. Conversely, a bot is granted immediate access.
• BotLocator finds and assigns a human to a bot if it is not already attached to another bot for a given room.
• Mover sends a direction to all rooms that the given client belongs. In the case of a human, this will be the bot room directly.
• ServerCreator handles the creation of the server and mainly passes off to collaborators given certain events.
• Server entry point for node server, just used to create a default server using the ServerCreator.
• ClientType js version of an enum that is used by the router to properly encapsulate the ClientType that is passed through the socket.query.
• Observer to notify tests in scenarios where the ordinary socket lifecycle reporting cannot be used i.e. disconnection.

Additional changes

• Each test that requires the use of a server creates and destroys as part of testing. Before you would manually run the server via the command line and then run the tests on top.
• A mixture of single quotes and double quotes in js part, this has been aligned.
• Dropped the use of prototype in favour of scoping correctly in the constructor of each "object", removing the confusing use of this everywhere.
• Use mocha.expect BDD style assertions.

[blundell]

what about @simonbrowndotje's Structurizer, i believe its free for open source projects?

or as step 1 just draw it (but dont' let S Brown see you 👁 )

[ouchadam]

since you aren't retaining any state you don't reallly need to new these up.

If you want to keep and treat them like classes, you should probably use classes 👻

module.exports = class Disconnector {

    disconnectRoom(roomName) {
        this._anInternalMethod()
    }

    _anInternalMethod() {
    }

}

and then the usage convention is

const Disconnector = require('./disconnector')
const disconnector = new Disconnector()

[Mecharyry]

Ah cool, I figured I'd learn something in the PR 😄

[ouchadam]

this looks a little confusing, the room ids are keys in clientsAndRooms[clientId]?

Also because we're in JS land we can start dropping fori loops in favour of .map/.forEach

[Mecharyry]

roomsClientIsIn are the keys for the rooms that a given client belongs too. Yes this is confusing, I'll look into making this easier to read 👍

[Mecharyry]

I'm going to make some changes as recommended by @ouchadam and will reopen after 😄

[blundell]

reopened = looks for diagrams...

[Mecharyry]

Sorry, @blundell I don't have time to dedicate to looking into structurizer atm, I have a number of PRs that I would like to open following this one to improve the stability of the app. I've created an issue and this will be created before we do any sort of release of TpBot.

#319

[Mecharyry]

Adam has spoken, down with vars!

[ouchadam]

happy to ship after that!

[Mecharyry]

💥 @ouchadam