From b8128f81a0b59e23c5b626f72a1c5241841c7e0d Mon Sep 17 00:00:00 2001
From: Juliusz Chroboczek <jch@irif.fr>
Date: Sun, 31 May 2020 20:41:17 +0200
Subject: [PATCH] Move client notifications into addClient and delClient.

---
 disk.go      |  4 ++++
 group.go     | 48 +++++++++++++++++++++++++++++++++---------------
 webclient.go | 50 +++++++++++---------------------------------------
 3 files changed, 48 insertions(+), 54 deletions(-)

diff --git a/disk.go b/disk.go
index 097dd72..4f025c7 100644
--- a/disk.go
+++ b/disk.go
@@ -36,6 +36,10 @@ func (client *diskClient) getUsername() string {
 	return "RECORDING"
 }
 
+func (client *diskClient) pushClient(id, username string, add bool) error {
+	return nil
+}
+
 func (client *diskClient) Close() error {
 	client.mu.Lock()
 	defer client.mu.Unlock()
diff --git a/group.go b/group.go
index 53d56ae..5a3cbe4 100644
--- a/group.go
+++ b/group.go
@@ -26,6 +26,7 @@ type client interface {
 	getId() string
 	getUsername() string
 	pushConn(conn *upConnection, tracks []*upTrack, label string) error
+	pushClient(id, username string, add bool) error
 }
 
 type chatHistoryEntry struct {
@@ -202,23 +203,23 @@ type userid struct {
 	username string
 }
 
-func addClient(name string, client client, user, pass string) (*group, []userid, error) {
+func addClient(name string, c client, user, pass string) (*group, error) {
 	g, err := addGroup(name, nil)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	perms, err := getPermission(g.description, user, pass)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
-	c, ok := client.(*webClient)
+	w, ok := c.(*webClient)
 	if ok {
-		c.permissions = perms
+		w.permissions = perms
 	}
 
 	if !perms.Op && atomic.LoadUint32(&g.locked) != 0 {
-		return nil, nil, userError("group is locked")
+		return nil, userError("group is locked")
 	}
 
 	g.mu.Lock()
@@ -226,19 +227,26 @@ func addClient(name string, client client, user, pass string) (*group, []userid,
 
 	if !perms.Op && g.description.MaxClients > 0 {
 		if len(g.clients) >= g.description.MaxClients {
-			return nil, nil, userError("too many users")
+			return nil, userError("too many users")
 		}
 	}
-	if g.clients[client.getId()] != nil {
-		return nil, nil, protocolError("duplicate client id")
+	if g.clients[c.getId()] != nil {
+		return nil, protocolError("duplicate client id")
 	}
 
-	var users []userid
-	for _, c := range g.clients {
-		users = append(users, userid{c.getId(), c.getUsername()})
-	}
-	g.clients[client.getId()] = client
-	return g, users, nil
+	g.clients[c.getId()] = c
+
+	go func(clients []client) {
+		for _, cc := range clients {
+			err := c.pushClient(cc.getId(), cc.getUsername(), true)
+			if err == ErrClientDead {
+				return
+			}
+			cc.pushClient(c.getId(), c.getUsername(), true)
+		}
+	}(g.getClientsUnlocked(c))
+
+	return g, nil
 }
 
 func delClient(c client) {
@@ -251,11 +259,21 @@ func delClient(c client) {
 		return
 	}
 	delete(g.clients, c.getId())
+
+	go func(clients []client) {
+		for _, cc := range clients {
+			cc.pushClient(c.getId(), c.getUsername(), false)
+		}
+	}(g.getClientsUnlocked(c))
 }
 
 func (g *group) getClients(except client) []client {
 	g.mu.Lock()
 	defer g.mu.Unlock()
+	return g.getClientsUnlocked(except)
+}
+
+func (g *group) getClientsUnlocked(except client) []client {
 	clients := make([]client, 0, len(g.clients))
 	for _, c := range g.clients {
 		if c != except {
diff --git a/webclient.go b/webclient.go
index 4c3d0fe..cbe2579 100644
--- a/webclient.go
+++ b/webclient.go
@@ -119,6 +119,15 @@ func (c *webClient) getUsername() string {
 	return c.username
 }
 
+func (c *webClient) pushClient(id, username string, add bool) error {
+	return c.write(clientMessage{
+		Type:     "user",
+		Id:       id,
+		Username: username,
+		Del:      !add,
+	})
+}
+
 type rateMap map[string]uint32
 
 func (v *rateMap) UnmarshalJSON(b []byte) error {
@@ -249,50 +258,13 @@ func startClient(conn *websocket.Conn) (err error) {
 	c.writerDone = make(chan struct{})
 	go clientWriter(conn, c.writeCh, c.writerDone)
 
-	g, users, err := addClient(m.Group, c, m.Username, m.Password)
+	g, err := addClient(m.Group, c, m.Username, m.Password)
 	if err != nil {
 		return
 	}
 	c.group = g
 	defer delClient(c)
 
-	for _, u := range users {
-		c.write(clientMessage{
-			Type:     "user",
-			Id:       u.id,
-			Username: u.username,
-		})
-	}
-
-	clients := g.getClients(nil)
-	u := clientMessage{
-		Type:     "user",
-		Id:       c.id,
-		Username: c.username,
-	}
-	for _, c := range clients {
-		c, ok := c.(*webClient)
-		if ok {
-			c.write(u)
-		}
-	}
-
-	defer func() {
-		clients := g.getClients(c)
-		u := clientMessage{
-			Type:     "user",
-			Id:       c.id,
-			Username: c.username,
-			Del:      true,
-		}
-		for _, c := range clients {
-			c, ok := c.(*webClient)
-			if ok {
-				c.write(u)
-			}
-		}
-	}()
-
 	return clientLoop(c, conn)
 }
 
@@ -1597,7 +1569,7 @@ func handleClientMessage(c *webClient, m clientMessage) error {
 			group: c.group,
 			id:    "recording",
 		}
-		_, _, err := addClient(c.group.name, disk, "", "")
+		_, err := addClient(c.group.name, disk, "", "")
 		if err != nil {
 			disk.Close()
 			return c.error(err)
