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

Refactor chat system layer #1798

Closed
julianlam opened this issue Jul 6, 2014 · 30 comments
Closed

Refactor chat system layer #1798

julianlam opened this issue Jul 6, 2014 · 30 comments

Comments

@julianlam
Copy link
Member

Writing this down before I forget...

@barisusakli, it seems the current chat backend works like so, please correct me if I am wrong:

  • When a chat message is sent, a new message object, identified by an mid is created, and the mid itself is appended to the list: messages:{fromUid}:{toUid}
  • If the user on the receiving end is offline (no socket session ids are present in the user's room), a notification is sent
  • ... and a socket message is emitted saying a chat message was received

It seems as though it would be easier if you were to use the rooms abstraction throughout, to manage the chats:

  • julian (uid 382) chats with baris (uid 859), they enter (or re-enter) room chat_382_859, room id is constructed by ordering the uids in numerical order, and then separating them with underscores (see methods introduced in 7b31592)
  • On new message, check socket ids present in room, and if any recipients are offline, send them a notification as normal (7b31592 again)
  • Send chat message event to the room instead of the sockets
  • Extend this further, three person chat? chat_382_859_1044
    • No need to do server.getUserSockets(someUid) to determine receipient sockets to send chat notifications, just send the notification object to the room, because socket.io can do that

Worth it?


Want to back this issue? Place a bounty on it! We accept bounties via Bountysource.

@julianlam
Copy link
Member Author

This is currently blocked by #1788, please do not do any work on it yet.

@psychobunny
Copy link
Contributor

Feature creep: add some action hooks, with toID and from ID plx ;)

@psychobunny
Copy link
Contributor

actually, make it a filter and include the message :D.. could think of a few cool use cases here.

@julianlam
Copy link
Member Author

Feature creep: Be able to add users to a chat room... necessitates renaming of socket.io rooms 😛

@julianlam
Copy link
Member Author

#1788 is no longer blocking this issue.

@barisusakli
Copy link
Member

Extend this further, three person chat? chat_382_859_1044
No need to do server.getUserSockets(someUid) to determine receipient sockets to send chat notifications, just send the notification object to the room, because socket.io can do that

Problems with this is storing chat history becomes a pain in the neck.

Previously it was just a list messages:<uid0>:<uid1> this doesn't play well with multiple users. For example two people chat and then a third one joins in. The new person shouldn't get access to the chat history of the other two. So we cant just rename the list to messages:<uid0>:<uid1>:<uid2>.

I don't think the chat_x_y_z is a scalable solution in this case, so I'm thinking about having unique room ids that users can join. And messages are stored on a per user per room basis.

Some data structures

uid:<uid>:chatrooms sorted set of room ids the user is in, will replace uid:<uid>:chats

chatroom:<chatroomid>:uids sorted set of uids that are in the room with the id chatroomid

uid:<uid>:chatroom:<chatroomid>:messages sorted set of message ids for user uid in room chatroomid. So each user in a room gets his own history of messages, this way a third user that joins in only gets the new messages and not the whole history. This replaces messages:<uid0>:<uid1>

Other stuff that needs to be figured out.

  1. How do you join/leave rooms.
  2. How do you add/remove people to/from rooms in the UI.
  3. Skype seems to create a new room™ as soon as you add someone to a conversation and the new conversation doesnt have any history maybe try to mimic that.
  4. Upgrade script to convert the old structures into rooms >_>

//end of brain derp

@julianlam
Copy link
Member Author

Sound like something we should leave to v0.6.0 😆

@Miaourt
Copy link

Miaourt commented Aug 1, 2014

I "moraly" support ! (No skillz in programing, and Javascript....duh)

@BlackFoxController
Copy link

That's a great idea.. yes .. chat rooms :)

@erayaydin
Copy link

Chat rooms will be good :)

@nomisum
Copy link

nomisum commented Oct 2, 2014

+1 here... would be a welcomed feature 👍

@Miaourt
Copy link

Miaourt commented Oct 2, 2014

That have already be put on 0.6.0,also i think more noize around could be
cool xP
Le 2 oct. 2014 16:14, "nomisum" notifications@github.com a écrit :

+1 here... would be a welcomed feature [image: 👍]


Reply to this email directly or view it on GitHub
#1798 (comment).

@psychobunny
Copy link
Contributor

Haha. NodeBB going to become a nextgen chat app. Time to remove all that code bloat we dont need in topics, categories etc :P

@julianlam
Copy link
Member Author

Theoretically, we could simplify the entire chat layer by creating a hidden forum called "chats", and every time you start a chat, it creates a new topic in that category :shipit:

#baddesigndecisions

@psychobunny
Copy link
Contributor

imo time to rename this issue to "GROUP CHAT" :P

We've already closed a few as dupes because this one isn't as searchable :P

@pitaj
Copy link
Contributor

pitaj commented Jan 24, 2015

I say copy how Google Hangouts does it.

@leader21
Copy link

sorry to bother, but any news on this guys?

@quickerlab
Copy link

yes, group chat room will be great! any plan for the feature?

@deltaflux
Copy link

Is anyone actively working on this? I'm currently evaluating NodeBB and the forums/groups look like exactly what I need but I also need private group chat as well as large chat rooms (IRC style) for with support for large user numbers (100+ per room) and I'm trying to get a gauge on how much NodeBB could cover.

@mamuesp
Copy link

mamuesp commented May 4, 2015

Nearly same question here ... I planned to write a (small) node.js based forum software by myself, as I found discourse and nodeBB (BTW I earn my money as software developer ...). My "brain concept" had a similar stucture in mind I found in nodeBB - that's great! But some functions are really missing - ok, private messaging is covered by User2User-Chat, that's fine, but group chat is a feature really needed. Why is it not possible to have chat between two groups, handled by the messenger.js, and a layer on top which handles the single users? Just a thought ...

@julianlam
Copy link
Member Author

@mamuesp No reason, really. The system was envisioned to be user-to-user, and only after release did we learn that many users wanted group chats :)

We could always tear the existing code down and re-write, but there is lots of code (incl. third-party contributor) code that we'd love to re-use, so it's more a matter of finding a sufficiently large block of time to dedicate to this.

@mamuesp
Copy link

mamuesp commented May 5, 2015

THX for the information. It's your project, so the user-to-user aspect has to be respected. As I'm courious, I'll analyze the "group chat" point a little more - perhaps we may discuss somethng later on.

@henrywright
Copy link
Contributor

Thinking about how this (seemingly huge) task could be simplified. Perhaps re-use much of the code that already exists in NodeBB. Could group chats just be "private" categories, formatted to feel "chat-like" at the front-end?

@pitaj
Copy link
Contributor

pitaj commented Aug 14, 2015

Noooooooooooo

@henrywright
Copy link
Contributor

@pitaj lol

@erayaydin
Copy link

@henrywright oh hell no

They are different thinks.

@julianlam
Copy link
Member Author

Hahah...

Could group chats just be "private" categories, formatted to feel "chat-like" at the front-end?

I'm going to admit that while it seems like a novel idea, in practice, it is likely end up closer to a square-peg-round-hole implementation.

@henrywright
Copy link
Contributor

Haha, sorry for the bad idea :)

@pitaj
Copy link
Contributor

pitaj commented Sep 30, 2015

Any update on this? It's still by far the largest missing feature IMO

@akhoury
Copy link
Member

akhoury commented Dec 8, 2015

+1 for group messages

@barisusakli barisusakli added this to the 1.0.0 milestone Dec 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests