From c3b2098c98b4e15293a4f644f14c0b9ad7ca0e54 Mon Sep 17 00:00:00 2001 From: kdedon Date: Thu, 20 Jul 2023 00:45:37 -0500 Subject: [PATCH 1/5] Add client list, fix channel race, hjoin, tests --- cherrysrv/channel.go | 19 +++++++++---------- cherrysrv/client.go | 26 +++++++++++++++----------- cherrysrv/commands.go | 8 +++++++- cherrysrv/utils.go | 5 +++-- cherrysrv/utils_test.go | 10 +++++----- 5 files changed, 39 insertions(+), 29 deletions(-) diff --git a/cherrysrv/channel.go b/cherrysrv/channel.go index 76eab99..71fb094 100644 --- a/cherrysrv/channel.go +++ b/cherrysrv/channel.go @@ -81,15 +81,16 @@ func (c *Channel) findClient(client *Client) bool { return false } -// TODO: Review subtle bug: -// if addClient is blocked because removeClient is working -// and removeClient leaves channel with 0 elements removing it -// from CHANNELS directory completely, addClient will add a client to a -// removed channel. func (channel *Channel) addClient(newClient *Client) { channel.Lock() defer channel.Unlock() + // check if a previous action emptied a channel + if len(channel.clients) == 0 { + DEBUG.Printf("%s had 0 clients on addClient, adding it to the directory", channel) + CHANNELS.Store(channel.Key(), channel) + } + channel.clients = append(channel.clients, newClient) } @@ -125,11 +126,9 @@ func (channel *Channel) removeClient(client *Client) bool { // len(c.clients) >= 2 // we loop through all the slice, NOT starting in pos=2 - - for i := 0; i < len; i++ { - if channel.clients[i] == client { - channel.clients[i] = channel.clients[len-1] // TODO: confirm is copying pointer, not content - channel.clients = channel.clients[:len-1] + for i, c := range channel.clients { + if c == client { + channel.clients = append(channel.clients[:i], channel.clients[i+1:]...) return true } diff --git a/cherrysrv/client.go b/cherrysrv/client.go index 9f08048..2a2a793 100644 --- a/cherrysrv/client.go +++ b/cherrysrv/client.go @@ -16,9 +16,10 @@ const ( // Client connection storing basic PC data type Client struct { - conn *net.TCPConn // tcpsocket connection. - Name string // Name of the user. - Status atomic.Int32 + conn *net.TCPConn // tcpsocket connection. + Name string // Name of the user. + Status atomic.Int32 + Channels []*Channel } func (c *Client) String() string { @@ -193,16 +194,19 @@ func (clt *Client) UpdateInMain(format string, args ...interface{}) { CLIENTS.Range(broadcast) } -// delete me from all the channels. This is extremely CPU consuming. -// TODO: add a slice of channels that the user has joined. func (clt *Client) RemoveMeFromAllChannels() { - - removeClient := func(key string, channel *Channel) bool { - + for _, channel := range clt.Channels { channel.removeClient(clt) - - return true } - CHANNELS.Range(removeClient) + clt.Channels = nil +} + +func (clt *Client) RemoveChannel(channel *Channel) { + for i, c := range clt.Channels { + if c == channel { + clt.Channels = append(clt.Channels[:i], clt.Channels[i+1:]...) + return + } + } } diff --git a/cherrysrv/commands.go b/cherrysrv/commands.go index 0407b05..f4d7962 100644 --- a/cherrysrv/commands.go +++ b/cherrysrv/commands.go @@ -281,6 +281,7 @@ func do_login(clt *Client, args string) { mainChannel, _ := CHANNELS.Load("#main") mainChannel.addClient(clt) + clt.Channels = append(clt.Channels, mainChannel) /* Update player */ @@ -331,6 +332,7 @@ func do_join(clt *Client, args string) { if ok { channel.addClient(clt) + clt.Channels = append(clt.Channels, channel) channel.Say(clt, "joined the channel") return @@ -347,6 +349,7 @@ func do_join(clt *Client, args string) { NewChannel := newChannel(channelName, false) NewChannel.addClient(clt) + clt.Channels = append(clt.Channels, NewChannel) CHANNELS.Store(NewChannel.Key(), NewChannel) DEBUG.Printf("adding %s to CHANNELS", NewChannel) @@ -374,6 +377,7 @@ func do_hjoin(clt *Client, args string) { if ok { channel.addClient(clt) + clt.Channels = append(clt.Channels, channel) channel.Say(clt, "hjoined the channel") return @@ -389,7 +393,8 @@ func do_hjoin(clt *Client, args string) { } NewChannel := newChannel(channelName, true) - channel.addClient(clt) + NewChannel.addClient(clt) + clt.Channels = append(clt.Channels, NewChannel) CHANNELS.Store(NewChannel.Key(), NewChannel) DEBUG.Printf("adding %s to CHANNELS", NewChannel) @@ -425,6 +430,7 @@ func do_leave(clt *Client, args string) { channel.Say(clt, "left the channel") channel.removeClient(clt) + clt.RemoveChannel(channel) return } diff --git a/cherrysrv/utils.go b/cherrysrv/utils.go index a84b907..e617a10 100644 --- a/cherrysrv/utils.go +++ b/cherrysrv/utils.go @@ -48,9 +48,10 @@ func ValidUsername(username string) (validusername string, err error) { return notvalid, fmt.Errorf("username cannot be longer than 16 chars") } - if isDigit(username[1]) { + // this test is no longer necessary due to @ requirement + /*if isDigit(username[1]) { return notvalid, fmt.Errorf("username cannot start with a number") - } + }*/ if !isASCIIPrintable(username[1:]) { return notvalid, fmt.Errorf("username can only contain ASCII chars and numbers") diff --git a/cherrysrv/utils_test.go b/cherrysrv/utils_test.go index c5b485f..32d84a1 100644 --- a/cherrysrv/utils_test.go +++ b/cherrysrv/utils_test.go @@ -43,12 +43,12 @@ func TestValidUsername(t *testing.T) { wantErr bool }{ // {"empty string", "", NOSTRING, true}, - {"valid name", "JohnnyCash", "JohnnyCash", false}, - {"valid name w/numbers", "JohnnyCash12", "JohnnyCash12", false}, - {"name with space", "Johnny Cash", NOSTRING, true}, + {"valid name", "@JohnnyCash", "@JohnnyCash", false}, + {"valid name w/numbers", "@JohnnyCash12", "@JohnnyCash12", false}, + {"name with space", "@Johnny Cash", NOSTRING, true}, {"srv", "srv", NOSTRING, true}, - {"name too long", "a1234567890123456", NOSTRING, true}, - {"name at limit", "a123456789012345", "a123456789012345", false}, + {"name too long", "@a1234567890123456", NOSTRING, true}, + {"name at limit", "@a12345678901234", "@a12345678901234", false}, {"name starts w/number", "1John", NOSTRING, true}, } for _, tt := range tests { From 844e856373aab8905fc0b8136c8883909b25ec5c Mon Sep 17 00:00:00 2001 From: kdedon Date: Thu, 20 Jul 2023 08:47:49 -0500 Subject: [PATCH 2/5] Use interface for client.conn for easier testing --- cherrysrv/client.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cherrysrv/client.go b/cherrysrv/client.go index 2a2a793..7566a26 100644 --- a/cherrysrv/client.go +++ b/cherrysrv/client.go @@ -16,8 +16,8 @@ const ( // Client connection storing basic PC data type Client struct { - conn *net.TCPConn // tcpsocket connection. - Name string // Name of the user. + conn net.Conn // tcpsocket connection. + Name string // Name of the user. Status atomic.Int32 Channels []*Channel } @@ -26,7 +26,7 @@ func (c *Client) String() string { return c.Name } -func newClient(conn *net.TCPConn) *Client { +func newClient(conn net.Conn) *Client { client := &Client{ conn: conn, From 64bf3dd7069be7680f9996aa2a412cf8bbf64061 Mon Sep 17 00:00:00 2001 From: kdedon Date: Thu, 20 Jul 2023 17:35:34 -0500 Subject: [PATCH 3/5] guard join/leave, add history, happy path tests --- cherrysrv/channel.go | 15 ++++---- cherrysrv/client_test.go | 73 ++++++++++++++++++++++++++++++++++++ cherrysrv/commands.go | 40 +++++++++++++++++++- cherrysrv/main.go | 2 + cherrysrv/ringbuffer.go | 42 +++++++++++++++++++++ cherrysrv/ringbuffer_test.go | 52 +++++++++++++++++++++++++ 6 files changed, 215 insertions(+), 9 deletions(-) create mode 100644 cherrysrv/client_test.go create mode 100644 cherrysrv/ringbuffer.go create mode 100644 cherrysrv/ringbuffer_test.go diff --git a/cherrysrv/channel.go b/cherrysrv/channel.go index 71fb094..53394ff 100644 --- a/cherrysrv/channel.go +++ b/cherrysrv/channel.go @@ -3,6 +3,7 @@ package main import ( "fmt" "sort" + "strings" "sync" ) @@ -10,8 +11,9 @@ type Channel struct { clients []*Client // clients in the channel. Name string // Name of the channel (incl #) hidden bool - closeOnEmpty bool // only #main should have this as false - sync.RWMutex // for adding/removing client connections + closeOnEmpty bool // only #main should have this as false + sync.RWMutex // for adding/removing client connections + history *ringbuffer // keep the last n messages for repeating } func newChannel(name string, hiddenChannel bool) *Channel { @@ -21,6 +23,7 @@ func newChannel(name string, hiddenChannel bool) *Channel { hidden: hiddenChannel, closeOnEmpty: true, RWMutex: sync.RWMutex{}, + history: newRingBuffer(10), } } @@ -85,12 +88,6 @@ func (channel *Channel) addClient(newClient *Client) { channel.Lock() defer channel.Unlock() - // check if a previous action emptied a channel - if len(channel.clients) == 0 { - DEBUG.Printf("%s had 0 clients on addClient, adding it to the directory", channel) - CHANNELS.Store(channel.Key(), channel) - } - channel.clients = append(channel.clients, newClient) } @@ -149,8 +146,10 @@ func (channel *Channel) Say(from *Client, format string, args ...interface{}) { } func (c *Channel) write(from *Client, message string) { + trimmed := strings.TrimSpace(message) c.RLock() defer c.RUnlock() + c.history.add(&trimmed) len := len(c.clients) diff --git a/cherrysrv/client_test.go b/cherrysrv/client_test.go new file mode 100644 index 0000000..6e7d127 --- /dev/null +++ b/cherrysrv/client_test.go @@ -0,0 +1,73 @@ +package main + +import ( + "bufio" + "fmt" + "net" + "testing" +) + +// TestClient is a set of ordered happy path tests +func TestClient(t *testing.T) { + init_logger() + init_commands() + main_channel := NewChannelMain("#main") + CHANNELS.Store(main_channel.Key(), main_channel) + + server, client := net.Pipe() + connWrapper := bufio.NewReadWriter(bufio.NewReader(client), bufio.NewWriter(client)) + + nc := newClient(server) + go nc.clientLoop() + + if res, _, err := connWrapper.ReadLine(); err == nil { + fmt.Println(string(res)) + } + + username := "@tester" + chan1 := "#test" + chan2 := "#test2" + + clientTests := []struct { + input []byte + expected []string + }{ + {[]byte("/who\n"), []string{fmt.Sprintf(">/who>0>%s", nc.Name)}}, + {[]byte("/join #test\n"), []string{">/join>0>/join requires you to be logged"}}, + {[]byte("/nusers\n"), []string{">/nusers>0>/nusers requires you to be logged"}}, + {[]byte("/users\n"), []string{">/users>0>/users requires you to be logged"}}, + {[]byte("/login\n"), []string{">/login>0>/login "}}, + {[]byte(fmt.Sprintf("/login %s\n", username)), []string{fmt.Sprintf(">/login>0>you're now %s", username)}}, + {[]byte("/login @tester2\n"), []string{">/login>0>you're already logged in"}}, + {[]byte("/nusers\n"), []string{">/nusers>0>1"}}, + {[]byte("/users\n"), []string{">/users>0>@tester"}}, + {[]byte("/join\n"), []string{">/join>0>/join <#channel>"}}, + {[]byte(fmt.Sprintf("/join %s\n", chan1)), []string{fmt.Sprintf(">/join>0>%s joined %s", username, chan1)}}, + {[]byte(fmt.Sprintf("/say %s hello\n", chan1)), []string{fmt.Sprintf(">%s>%s>hello", chan1, username)}}, + {[]byte(fmt.Sprintf("/say %s goodbye\n", chan1)), []string{fmt.Sprintf(">%s>%s>goodbye", chan1, username)}}, + {[]byte("/nusers #bigapple\n"), []string{">/users #bigapple>0>#bigapple is not a valid channel"}}, + {[]byte(fmt.Sprintf("/nusers %s\n", chan1)), []string{fmt.Sprintf(">/users %s>0>1", chan1)}}, + {[]byte(fmt.Sprintf("/users %s\n", chan1)), []string{fmt.Sprintf(">/users %s>0>%s", chan1, username)}}, + {[]byte(fmt.Sprintf("/history %s\n", chan1)), []string{fmt.Sprintf(">/history>1>%s>%s>hello", chan1, username), fmt.Sprintf(">/history>0>%s>%s>goodbye", chan1, username)}}, + {[]byte("/list\n"), []string{fmt.Sprintf(">/list>1>%s", main_channel.Name), fmt.Sprintf(">/list>0>%s", chan1)}}, + {[]byte(fmt.Sprintf("/leave %s\n", chan1)), []string{fmt.Sprintf(">%s>%s>left the channel", chan1, username)}}, + {[]byte("/list\n"), []string{fmt.Sprintf(">/list>0>%s", main_channel.Name)}}, + {[]byte(fmt.Sprintf("/hjoin %s\n", chan2)), []string{fmt.Sprintf(">/hjoin>0>%s hjoined %s", username, chan2)}}, + {[]byte("/list\n"), []string{fmt.Sprintf(">/list>0>%s", main_channel.Name)}}, + {[]byte(fmt.Sprintf("/leave %s\n", chan2)), []string{fmt.Sprintf(">%s>%s>left the channel", chan2, username)}}, + {[]byte("/logoff\n"), []string{fmt.Sprintf(">/logoff>0>Goodbye %s", username)}}, + } + + for _, test := range clientTests { + client.Write(test.input) + + for i, ex := range test.expected { + if res, _, err := connWrapper.ReadLine(); err != nil { + t.Errorf("failed read, expected %s", test.expected[i]) + } else if string(res) != ex { + t.Errorf("got %s, expected %s", string(res), ex) + } + } + + } +} diff --git a/cherrysrv/commands.go b/cherrysrv/commands.go index f4d7962..b2f5a43 100644 --- a/cherrysrv/commands.go +++ b/cherrysrv/commands.go @@ -3,6 +3,7 @@ package main import ( "runtime" "sort" + "strings" ) func init_commands() { @@ -21,6 +22,7 @@ func init_commands() { COMMANDS["leave"] = do_leave COMMANDS["list"] = do_list COMMANDS["license"] = do_license + COMMANDS["history"] = do_history } func do_help(clt *Client, args string) { @@ -37,6 +39,7 @@ func do_help(clt *Client, args string) { "/hlist - show available hidden channels", "/join <#channel> - join/create a channel", "/hjoin <#channel> - join/create hidden channel", + "/history <#channel> - return previous messages", "/license - view license agreement", "/logoff - logoff"}) @@ -156,6 +159,26 @@ func do_say(clt *Client, args string) { } +// get previous n messages written to channel +func do_history(clt *Client, channelName string) { + + channel, ok := CHANNELS.Load(channelName) + + if !ok { + clt.Say("%s is not a valid channel", channelName) + return + } + + var history []string + for _, p := range channel.history.readAll() { + if p != nil { + history = append(history, strings.TrimPrefix(*p, ">")) + } + } + + clt.SayN(">/history>", history) +} + // update login levels. Unused for now func sys_log(clt *Client, args string) { @@ -285,7 +308,7 @@ func do_login(clt *Client, args string) { /* Update player */ - clt.Say("/login>0>you're now %s", clt) + clt.Say(">/login>0>you're now %s", clt) clt.UpdateInMain(">!login>%s has joined the server", clt) INFO.Printf("%s has logged in as %s", oldName, clt) @@ -328,6 +351,11 @@ func do_join(clt *Client, args string) { channelName, _ := split2(args, " ") + // Multiple users can add at the same time, or a channel can be deleted from Map + // after the initial lookup. + CHNMTX.Lock() + defer CHNMTX.Unlock() + channel, ok := CHANNELS.Load(channelName) if ok { @@ -373,6 +401,11 @@ func do_hjoin(clt *Client, args string) { channelName, _ := split2(args, " ") + // Multiple users can add at the same time, or a channel can be deleted from Map + // after the initial lookup. + CHNMTX.Lock() + defer CHNMTX.Unlock() + channel, ok := CHANNELS.Load(channelName) if ok { @@ -419,6 +452,11 @@ func do_leave(clt *Client, args string) { channelName, _ := split2(args, " ") + // Multiple users can add at the same time, or a channel can be deleted from Map + // after the initial lookup. + CHNMTX.Lock() + defer CHNMTX.Unlock() + channel, ok := CHANNELS.Load(channelName) if ok { diff --git a/cherrysrv/main.go b/cherrysrv/main.go index 0c359df..2c42ec5 100644 --- a/cherrysrv/main.go +++ b/cherrysrv/main.go @@ -28,6 +28,7 @@ import ( "os/signal" "runtime" "strings" + "sync" "syscall" "time" @@ -57,6 +58,7 @@ var ( COMMANDS = make(map[string]do_command) CLIENTS cmap.Map[string, *Client] // CLIENTS cmap.Cmap CHANNELS cmap.Map[string, *Channel] + CHNMTX = sync.Mutex{} SCHEDULER *tasks.Scheduler TIME uint64 STARTEDON time.Time diff --git a/cherrysrv/ringbuffer.go b/cherrysrv/ringbuffer.go new file mode 100644 index 0000000..86e5e11 --- /dev/null +++ b/cherrysrv/ringbuffer.go @@ -0,0 +1,42 @@ +package main + +import ( + "sync" +) + +// ringbuffer is a very niave ringbuffer that holds pointers to strings and returns +// them in the order they were added. It uses locking to prevent threading issues. +type ringbuffer struct { + buffer []*string + pos int + mtx sync.RWMutex +} + +func newRingBuffer(size int) *ringbuffer { + return &ringbuffer{ + buffer: make([]*string, size), + mtx: sync.RWMutex{}, + } +} + +func (rb *ringbuffer) add(s *string) { + rb.mtx.Lock() + defer rb.mtx.Unlock() + + rb.buffer[rb.pos] = s + rb.pos++ + if rb.pos > len(rb.buffer)-1 { + rb.pos = 0 + } +} + +func (rb *ringbuffer) readAll() []*string { + res := make([]*string, len(rb.buffer)) + + rb.mtx.RLock() + defer rb.mtx.RUnlock() + copy(res[:len(rb.buffer)-rb.pos], rb.buffer[rb.pos:]) + copy(res[len(rb.buffer)-rb.pos:], rb.buffer[:rb.pos]) + + return res +} diff --git a/cherrysrv/ringbuffer_test.go b/cherrysrv/ringbuffer_test.go new file mode 100644 index 0000000..798bb6b --- /dev/null +++ b/cherrysrv/ringbuffer_test.go @@ -0,0 +1,52 @@ +package main + +import ( + "testing" +) + +func testSlice(a, b []*string) bool { + if len(a) != len(b) { + return false + } + + for i, p := range a { + if p != b[i] { + return false + } + } + return true +} + +func TestBuffer(t *testing.T) { + buff := newRingBuffer(5) + var a, b, c, d, e, f, g string + a = "1" + b = "2" + c = "3" + d = "4" + e = "5" + f = "6" + g = "7" + + var expected = []struct { + p *string + b []*string + }{ + {&a, []*string{nil, nil, nil, nil, &a}}, + {&b, []*string{nil, nil, nil, &a, &b}}, + {&c, []*string{nil, nil, &a, &b, &c}}, + {&d, []*string{nil, &a, &b, &c, &d}}, + {&e, []*string{&a, &b, &c, &d, &e}}, + {&f, []*string{&b, &c, &d, &e, &f}}, + {&g, []*string{&c, &d, &e, &f, &g}}, + } + + for _, test := range expected { + buff.add(test.p) + res := buff.readAll() + if !testSlice(test.b, res) { + t.Errorf("expected: %v, got %v", test.b, res) + } + + } +} From f4d5be30b2f924cf21501172b222ddb4c0c7d264 Mon Sep 17 00:00:00 2001 From: kdedon Date: Fri, 21 Jul 2023 00:03:21 -0500 Subject: [PATCH 4/5] Small fixes and broken multi-client tests --- cherrysrv/channel.go | 1 + cherrysrv/client.go | 2 +- cherrysrv/client_test.go | 101 ++++++++++++++++++++++++++++++++++----- cherrysrv/commands.go | 5 -- cherrysrv/ringbuffer.go | 2 +- 5 files changed, 92 insertions(+), 19 deletions(-) diff --git a/cherrysrv/channel.go b/cherrysrv/channel.go index 53394ff..78c548e 100644 --- a/cherrysrv/channel.go +++ b/cherrysrv/channel.go @@ -89,6 +89,7 @@ func (channel *Channel) addClient(newClient *Client) { defer channel.Unlock() channel.clients = append(channel.clients, newClient) + newClient.Channels = append(newClient.Channels, channel) } func (channel *Channel) removeClient(client *Client) bool { diff --git a/cherrysrv/client.go b/cherrysrv/client.go index 7566a26..fe4320a 100644 --- a/cherrysrv/client.go +++ b/cherrysrv/client.go @@ -183,7 +183,7 @@ func (clt *Client) UpdateInMain(format string, args ...interface{}) { broadcast := func(key string, client *Client) bool { - if clt == client { // we don't want to send the message to us + if clt == client || !client.isLogged() { // we don't want to send the message to us or non-logged users return true } diff --git a/cherrysrv/client_test.go b/cherrysrv/client_test.go index 6e7d127..78e3033 100644 --- a/cherrysrv/client_test.go +++ b/cherrysrv/client_test.go @@ -7,6 +7,26 @@ import ( "testing" ) +func genClient() (c *Client, out net.Conn, in *bufio.Reader) { + server, out := net.Pipe() + + in = bufio.NewReader(out) + + c = newClient(server) + go c.clientLoop() + + if res, _, err := in.ReadLine(); err == nil { + fmt.Println(string(res)) + } + + return +} + +type multiCase struct { + i *bufio.Reader + s string +} + // TestClient is a set of ordered happy path tests func TestClient(t *testing.T) { init_logger() @@ -14,15 +34,7 @@ func TestClient(t *testing.T) { main_channel := NewChannelMain("#main") CHANNELS.Store(main_channel.Key(), main_channel) - server, client := net.Pipe() - connWrapper := bufio.NewReadWriter(bufio.NewReader(client), bufio.NewWriter(client)) - - nc := newClient(server) - go nc.clientLoop() - - if res, _, err := connWrapper.ReadLine(); err == nil { - fmt.Println(string(res)) - } + c1, out, in := genClient() username := "@tester" chan1 := "#test" @@ -32,7 +44,7 @@ func TestClient(t *testing.T) { input []byte expected []string }{ - {[]byte("/who\n"), []string{fmt.Sprintf(">/who>0>%s", nc.Name)}}, + {[]byte("/who\n"), []string{fmt.Sprintf(">/who>0>%s", c1.Name)}}, {[]byte("/join #test\n"), []string{">/join>0>/join requires you to be logged"}}, {[]byte("/nusers\n"), []string{">/nusers>0>/nusers requires you to be logged"}}, {[]byte("/users\n"), []string{">/users>0>/users requires you to be logged"}}, @@ -59,10 +71,10 @@ func TestClient(t *testing.T) { } for _, test := range clientTests { - client.Write(test.input) + out.Write(test.input) for i, ex := range test.expected { - if res, _, err := connWrapper.ReadLine(); err != nil { + if res, _, err := in.ReadLine(); err != nil { t.Errorf("failed read, expected %s", test.expected[i]) } else if string(res) != ex { t.Errorf("got %s, expected %s", string(res), ex) @@ -71,3 +83,68 @@ func TestClient(t *testing.T) { } } + +// TestMultipleClients +func TestMultipleClients(t *testing.T) { + init_logger() + init_commands() + main_channel := NewChannelMain("#main") + CHANNELS.Store(main_channel.Key(), main_channel) + + _, out1, in1 := genClient() + _, out2, in2 := genClient() + _, out3, in3 := genClient() + + username1 := "@tester1" + username2 := "@tester2" + username3 := "@tester3" + + chan1 := "#test" + + clientTests := []struct { + o net.Conn + input []byte + expected []multiCase + }{ + {out1, []byte(fmt.Sprintf("/login %s\n", username1)), + []multiCase{{in1, fmt.Sprintf(">/login>0>you're now %s", username1)}}}, + + {out2, []byte(fmt.Sprintf("/login %s\n", username1)), + []multiCase{{in2, fmt.Sprintf(">/login>0>%s is already taken, please select another @name", username1)}}}, + + {out2, []byte(fmt.Sprintf("/login %s\n", username2)), + []multiCase{{in2, fmt.Sprintf(">/login>0>you're now %s", username2)}, + {in1, fmt.Sprintf(">#main>!login>%s has joined the server", username2)}}}, + + {out3, []byte(fmt.Sprintf("/login %s\n", username3)), + []multiCase{{in3, fmt.Sprintf(">/login>0>you're now %s", username3)}, + {in2, fmt.Sprintf(">#main>!login>%s has joined the server", username3)}, + {in1, fmt.Sprintf(">#main>!login>%s has joined the server", username3)}}}, + + {out1, []byte(fmt.Sprintf("/join %s\n", chan1)), + []multiCase{{in1, fmt.Sprintf(">/join>0>%s joined %s", username1, chan1)}}}, + + /*{out2, []byte(fmt.Sprintf("/join %s\n", chan1)), + []multiCase{{in2, fmt.Sprintf(">/join>0>%s joined %s", username2, chan1)}}}, + + /* {out3, []byte(fmt.Sprintf("/join %s\n", chan1)), + []multiCase{{in3, fmt.Sprintf(">/join>0>%s joined %s", username3, chan1)}}},*/ + + /*{out1, in1, []byte("/logoff\n"), []string{fmt.Sprintf(">/logoff>0>Goodbye %s", username1)}}, + {out2, in2, []byte("/logoff\n"), []string{fmt.Sprintf(">/logoff>0>Goodbye %s", username2)}}, + {out3, in3, []byte("/logoff\n"), []string{fmt.Sprintf(">/logoff>0>Goodbye %s", username3)}},*/ + } + + for _, test := range clientTests { + test.o.Write(test.input) + + for _, ex := range test.expected { + if res, _, err := ex.i.ReadLine(); err != nil { + t.Errorf("failed read on %p, expected %s", ex.i, ex.s) + } else if string(res) != ex.s { + t.Errorf("got %s, expected %s", string(res), ex.s) + } + } + + } +} diff --git a/cherrysrv/commands.go b/cherrysrv/commands.go index b2f5a43..e0c352b 100644 --- a/cherrysrv/commands.go +++ b/cherrysrv/commands.go @@ -304,7 +304,6 @@ func do_login(clt *Client, args string) { mainChannel, _ := CHANNELS.Load("#main") mainChannel.addClient(clt) - clt.Channels = append(clt.Channels, mainChannel) /* Update player */ @@ -360,7 +359,6 @@ func do_join(clt *Client, args string) { if ok { channel.addClient(clt) - clt.Channels = append(clt.Channels, channel) channel.Say(clt, "joined the channel") return @@ -377,7 +375,6 @@ func do_join(clt *Client, args string) { NewChannel := newChannel(channelName, false) NewChannel.addClient(clt) - clt.Channels = append(clt.Channels, NewChannel) CHANNELS.Store(NewChannel.Key(), NewChannel) DEBUG.Printf("adding %s to CHANNELS", NewChannel) @@ -410,7 +407,6 @@ func do_hjoin(clt *Client, args string) { if ok { channel.addClient(clt) - clt.Channels = append(clt.Channels, channel) channel.Say(clt, "hjoined the channel") return @@ -427,7 +423,6 @@ func do_hjoin(clt *Client, args string) { NewChannel := newChannel(channelName, true) NewChannel.addClient(clt) - clt.Channels = append(clt.Channels, NewChannel) CHANNELS.Store(NewChannel.Key(), NewChannel) DEBUG.Printf("adding %s to CHANNELS", NewChannel) diff --git a/cherrysrv/ringbuffer.go b/cherrysrv/ringbuffer.go index 86e5e11..94ae88d 100644 --- a/cherrysrv/ringbuffer.go +++ b/cherrysrv/ringbuffer.go @@ -4,7 +4,7 @@ import ( "sync" ) -// ringbuffer is a very niave ringbuffer that holds pointers to strings and returns +// ringbuffer is a very naive ringbuffer that holds pointers to strings and returns // them in the order they were added. It uses locking to prevent threading issues. type ringbuffer struct { buffer []*string From 7318975743b83ab5d415282a0f6b0787fec7315a Mon Sep 17 00:00:00 2001 From: kdedon Date: Sat, 22 Jul 2023 00:03:26 -0500 Subject: [PATCH 5/5] Fix multi-tests --- cherrysrv/client_test.go | 87 +++++++++++++++++++++++++++++----------- 1 file changed, 63 insertions(+), 24 deletions(-) diff --git a/cherrysrv/client_test.go b/cherrysrv/client_test.go index 78e3033..ae071da 100644 --- a/cherrysrv/client_test.go +++ b/cherrysrv/client_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net" "testing" + "time" ) func genClient() (c *Client, out net.Conn, in *bufio.Reader) { @@ -24,7 +25,8 @@ func genClient() (c *Client, out net.Conn, in *bufio.Reader) { type multiCase struct { i *bufio.Reader - s string + c net.Conn + s []string } // TestClient is a set of ordered happy path tests @@ -84,6 +86,19 @@ func TestClient(t *testing.T) { } } +func fullRead(buff *bufio.Reader, conn net.Conn, c chan []string) { + conn.SetReadDeadline(time.Now().Add(250 * time.Millisecond)) + var s []string + for { + if res, _, err := buff.ReadLine(); err != nil { + break + } else { + s = append(s, string(res)) + } + } + c <- s +} + // TestMultipleClients func TestMultipleClients(t *testing.T) { init_logger() @@ -107,42 +122,66 @@ func TestMultipleClients(t *testing.T) { expected []multiCase }{ {out1, []byte(fmt.Sprintf("/login %s\n", username1)), - []multiCase{{in1, fmt.Sprintf(">/login>0>you're now %s", username1)}}}, + []multiCase{{in1, out1, []string{fmt.Sprintf(">/login>0>you're now %s", username1)}}}}, {out2, []byte(fmt.Sprintf("/login %s\n", username1)), - []multiCase{{in2, fmt.Sprintf(">/login>0>%s is already taken, please select another @name", username1)}}}, + []multiCase{{in2, out2, []string{fmt.Sprintf(">/login>0>%s is already taken, please select another @name", username1)}}}}, {out2, []byte(fmt.Sprintf("/login %s\n", username2)), - []multiCase{{in2, fmt.Sprintf(">/login>0>you're now %s", username2)}, - {in1, fmt.Sprintf(">#main>!login>%s has joined the server", username2)}}}, + []multiCase{{in2, out2, []string{fmt.Sprintf(">/login>0>you're now %s", username2)}}, + {in1, out1, []string{fmt.Sprintf(">#main>!login>%s has joined the server", username2)}}}}, {out3, []byte(fmt.Sprintf("/login %s\n", username3)), - []multiCase{{in3, fmt.Sprintf(">/login>0>you're now %s", username3)}, - {in2, fmt.Sprintf(">#main>!login>%s has joined the server", username3)}, - {in1, fmt.Sprintf(">#main>!login>%s has joined the server", username3)}}}, + []multiCase{{in3, out3, []string{fmt.Sprintf(">/login>0>you're now %s", username3)}}, + {in2, out2, []string{fmt.Sprintf(">#main>!login>%s has joined the server", username3)}}, + {in1, out1, []string{fmt.Sprintf(">#main>!login>%s has joined the server", username3)}}}}, {out1, []byte(fmt.Sprintf("/join %s\n", chan1)), - []multiCase{{in1, fmt.Sprintf(">/join>0>%s joined %s", username1, chan1)}}}, - - /*{out2, []byte(fmt.Sprintf("/join %s\n", chan1)), - []multiCase{{in2, fmt.Sprintf(">/join>0>%s joined %s", username2, chan1)}}}, - - /* {out3, []byte(fmt.Sprintf("/join %s\n", chan1)), - []multiCase{{in3, fmt.Sprintf(">/join>0>%s joined %s", username3, chan1)}}},*/ - - /*{out1, in1, []byte("/logoff\n"), []string{fmt.Sprintf(">/logoff>0>Goodbye %s", username1)}}, - {out2, in2, []byte("/logoff\n"), []string{fmt.Sprintf(">/logoff>0>Goodbye %s", username2)}}, - {out3, in3, []byte("/logoff\n"), []string{fmt.Sprintf(">/logoff>0>Goodbye %s", username3)}},*/ + []multiCase{{in1, out1, []string{fmt.Sprintf(">/join>0>%s joined %s", username1, chan1)}}}}, + + {out2, []byte(fmt.Sprintf("/join %s\n", chan1)), + []multiCase{{in2, out2, []string{fmt.Sprintf(">%s>%s>joined the channel", chan1, username2)}}, + {in1, out1, []string{fmt.Sprintf(">%s>%s>joined the channel", chan1, username2)}}}}, + + {out3, []byte(fmt.Sprintf("/join %s\n", chan1)), + []multiCase{{in3, out3, []string{fmt.Sprintf(">%s>%s>joined the channel", chan1, username3)}}, + {in2, out2, []string{fmt.Sprintf(">%s>%s>joined the channel", chan1, username3)}}, + {in1, out1, []string{fmt.Sprintf(">%s>%s>joined the channel", chan1, username3)}}}}, + + {out1, []byte(fmt.Sprintf("/say %s hello\n", chan1)), + []multiCase{{in3, out3, []string{fmt.Sprintf(">%s>%s>hello", chan1, username1)}}, + {in2, out2, []string{fmt.Sprintf(">%s>%s>hello", chan1, username1)}}, + {in1, out1, []string{fmt.Sprintf(">%s>%s>hello", chan1, username1)}}}}, + + {out1, []byte("/logoff\n"), + []multiCase{{in1, out1, []string{fmt.Sprintf(">/logoff>0>Goodbye %s", username1)}}, + {in2, out2, []string{fmt.Sprintf(">#main>!logoff>%s is leaving", username1)}}, + {in3, out3, []string{fmt.Sprintf(">#main>!logoff>%s is leaving", username1)}}}}, + {out2, []byte("/logoff\n"), + []multiCase{{in2, out2, []string{fmt.Sprintf(">/logoff>0>Goodbye %s", username2)}}, + {in3, out3, []string{fmt.Sprintf(">#main>!logoff>%s is leaving", username2)}}}}, + {out3, []byte("/logoff\n"), + []multiCase{{in3, out3, []string{fmt.Sprintf(">/logoff>0>Goodbye %s", username3)}}}}, } for _, test := range clientTests { test.o.Write(test.input) - for _, ex := range test.expected { - if res, _, err := ex.i.ReadLine(); err != nil { - t.Errorf("failed read on %p, expected %s", ex.i, ex.s) - } else if string(res) != ex.s { - t.Errorf("got %s, expected %s", string(res), ex.s) + var rets []chan []string + for i, ex := range test.expected { + rets = append(rets, make(chan []string)) + go fullRead(ex.i, ex.c, rets[i]) + } + for i, ex := range test.expected { + res := <-rets[i] + if len(ex.s) != len(res) { + t.Errorf("got %v, expected %v", res, ex.s) + } else { + for i, s := range ex.s { + if s != res[i] { + t.Errorf("got %s, expected %s", res[i], s) + } + } } }