Refactoring, cleanup, test backfilling

[rbdr/mobius] / hotline / client_conn.go

diff --git a/hotline/client_conn.go b/hotline/client_conn.go
index c886dfcc712e88eed55b99dc3a80c2d22aced448..e527eba99db5ef1ad101abda698991572a622243 100644 (file)
--- a/hotline/client_conn.go
+++ b/hotline/client_conn.go
@@ -1,38 +1,25 @@
 package hotline
 
 import (
 package hotline
 
 import (

+       "cmp"

        "encoding/binary"
        "fmt"
        "golang.org/x/crypto/bcrypt"
        "io"
        "log/slog"
        "encoding/binary"
        "fmt"
        "golang.org/x/crypto/bcrypt"
        "io"
        "log/slog"

-       "math/big"
-       "sort"
+       "slices"

        "strings"
        "sync"
 )
 
        "strings"
        "sync"
 )
 

-type byClientID []*ClientConn
-
-func (s byClientID) Len() int {
-       return len(s)
-}
-
-func (s byClientID) Swap(i, j int) {
-       s[i], s[j] = s[j], s[i]
-}
-
-func (s byClientID) Less(i, j int) bool {
-       return s[i].uint16ID() < s[j].uint16ID()
-}
-

 // ClientConn represents a client connected to a Server
 type ClientConn struct {
        Connection io.ReadWriteCloser
        RemoteAddr string
 // ClientConn represents a client connected to a Server
 type ClientConn struct {
        Connection io.ReadWriteCloser
        RemoteAddr string

-       ID         *[]byte
+       ID         [2]byte

        Icon       []byte
        Icon       []byte

-       Flags      []byte
+       flagsMU    sync.Mutex
+       Flags      UserFlags

        UserName   []byte
        Account    *Account
        IdleTime   int
        UserName   []byte
        Account    *Account
        IdleTime   int


@@ -45,78 +32,47 @@ type ClientConn struct {
        transfers   map[int]map[[4]byte]*FileTransfer
 
        logger *slog.Logger
        transfers   map[int]map[[4]byte]*FileTransfer
 
        logger *slog.Logger

+
+       sync.Mutex

 }
 
 }
 

-func (cc *ClientConn) sendAll(t int, fields ...Field) {
-       for _, c := range sortedClients(cc.Server.Clients) {
-               cc.Server.outbox <- *NewTransaction(t, c.ID, fields...)
+func (cc *ClientConn) sendAll(t [2]byte, fields ...Field) {
+       for _, c := range cc.Server.Clients {
+               cc.Server.outbox <- NewTransaction(t, c.ID, fields...)

        }
 }
 
        }
 }
 

-func (cc *ClientConn) handleTransaction(transaction Transaction) error {
-       requestNum := binary.BigEndian.Uint16(transaction.Type)
-       if handler, ok := TransactionHandlers[requestNum]; ok {
-               for _, reqField := range handler.RequiredFields {
-                       field := transaction.GetField(reqField.ID)
-
-                       // Validate that required field is present
-                       if field.ID == [2]byte{0, 0} {
-                               cc.logger.Error(
-                                       "Missing required field",
-                                       "RequestType", handler.Name, "FieldID", reqField.ID,
-                               )
-                               return nil
-                       }
-
-                       if len(field.Data) < reqField.minLen {
-                               cc.logger.Info(
-                                       "Field does not meet minLen",
-                                       "RequestType", handler.Name, "FieldID", reqField.ID,
-                               )
-                               return nil
-                       }
-               }
-
-               cc.logger.Debug("Received Transaction", "RequestType", handler.Name)
+func (cc *ClientConn) handleTransaction(transaction Transaction) {
+       if handler, ok := TransactionHandlers[transaction.Type]; ok {
+               cc.logger.Debug("Received Transaction", "RequestType", transaction.Type)

 
 

-               transactions, err := handler.Handler(cc, &transaction)
-               if err != nil {
-                       return fmt.Errorf("error handling transaction: %w", err)
-               }
-               for _, t := range transactions {
+               for _, t := range handler(cc, &transaction) {

                        cc.Server.outbox <- t
                }
                        cc.Server.outbox <- t
                }

-       } else {
-               cc.logger.Error(
-                       "Unimplemented transaction type received", "RequestID", requestNum)

        }
 
        cc.Server.mux.Lock()
        defer cc.Server.mux.Unlock()
 
        }
 
        cc.Server.mux.Lock()
        defer cc.Server.mux.Unlock()
 

-       if requestNum != TranKeepAlive {
+       if transaction.Type != TranKeepAlive {

                // reset the user idle timer
                cc.IdleTime = 0
 
                // if user was previously idle, mark as not idle and notify other connected clients that
                // the user is no longer away
                if cc.Idle {
                // reset the user idle timer
                cc.IdleTime = 0
 
                // if user was previously idle, mark as not idle and notify other connected clients that
                // the user is no longer away
                if cc.Idle {

-                       flagBitmap := big.NewInt(int64(binary.BigEndian.Uint16(cc.Flags)))
-                       flagBitmap.SetBit(flagBitmap, UserFlagAway, 0)
-                       binary.BigEndian.PutUint16(cc.Flags, uint16(flagBitmap.Int64()))
+                       cc.Flags.Set(UserFlagAway, 0)

                        cc.Idle = false
 
                        cc.sendAll(
                                TranNotifyChangeUser,
                        cc.Idle = false
 
                        cc.sendAll(
                                TranNotifyChangeUser,

-                               NewField(FieldUserID, *cc.ID),
-                               NewField(FieldUserFlags, cc.Flags),
+                               NewField(FieldUserID, cc.ID[:]),
+                               NewField(FieldUserFlags, cc.Flags[:]),

                                NewField(FieldUserName, cc.UserName),
                                NewField(FieldUserIconID, cc.Icon),
                        )
                }
        }
                                NewField(FieldUserName, cc.UserName),
                                NewField(FieldUserIconID, cc.Icon),
                        )
                }
        }

-
-       return nil

 }
 
 func (cc *ClientConn) Authenticate(login string, password []byte) bool {
 }
 
 func (cc *ClientConn) Authenticate(login string, password []byte) bool {


@@ -127,24 +83,23 @@ func (cc *ClientConn) Authenticate(login string, password []byte) bool {
        return false
 }
 
        return false
 }
 

-func (cc *ClientConn) uint16ID() uint16 {
-       id, _ := byteToInt(*cc.ID)
-       return uint16(id)
-}
-

 // Authorize checks if the user account has the specified permission
 func (cc *ClientConn) Authorize(access int) bool {
 // Authorize checks if the user account has the specified permission
 func (cc *ClientConn) Authorize(access int) bool {

+       cc.Lock()
+       defer cc.Unlock()
+       if cc.Account == nil {
+               return false
+       }

        return cc.Account.Access.IsSet(access)
 }
 
 // Disconnect notifies other clients that a client has disconnected
 func (cc *ClientConn) Disconnect() {
        cc.Server.mux.Lock()
        return cc.Account.Access.IsSet(access)
 }
 
 // Disconnect notifies other clients that a client has disconnected
 func (cc *ClientConn) Disconnect() {
        cc.Server.mux.Lock()

-       defer cc.Server.mux.Unlock()
+       delete(cc.Server.Clients, cc.ID)
+       cc.Server.mux.Unlock()

 
 

-       delete(cc.Server.Clients, binary.BigEndian.Uint16(*cc.ID))
-
-       for _, t := range cc.notifyOthers(*NewTransaction(TranNotifyDeleteUser, nil, NewField(FieldUserID, *cc.ID))) {
+       for _, t := range cc.notifyOthers(NewTransaction(TranNotifyDeleteUser, [2]byte{}, NewField(FieldUserID, cc.ID[:]))) {

                cc.Server.outbox <- t
        }
 
                cc.Server.outbox <- t
        }
 


@@ -155,7 +110,9 @@ func (cc *ClientConn) Disconnect() {
 
 // notifyOthers sends transaction t to other clients connected to the server
 func (cc *ClientConn) notifyOthers(t Transaction) (trans []Transaction) {
 
 // notifyOthers sends transaction t to other clients connected to the server
 func (cc *ClientConn) notifyOthers(t Transaction) (trans []Transaction) {

-       for _, c := range sortedClients(cc.Server.Clients) {
+       cc.Server.mux.Lock()
+       defer cc.Server.mux.Unlock()
+       for _, c := range cc.Server.Clients {

                if c.ID != cc.ID {
                        t.clientID = c.ID
                        trans = append(trans, t)
                if c.ID != cc.ID {
                        t.clientID = c.ID
                        trans = append(trans, t)


@@ -167,36 +124,44 @@ func (cc *ClientConn) notifyOthers(t Transaction) (trans []Transaction) {
 // NewReply returns a reply Transaction with fields for the ClientConn
 func (cc *ClientConn) NewReply(t *Transaction, fields ...Field) Transaction {
        return Transaction{
 // NewReply returns a reply Transaction with fields for the ClientConn
 func (cc *ClientConn) NewReply(t *Transaction, fields ...Field) Transaction {
        return Transaction{

-               IsReply:   0x01,
-               Type:      []byte{0x00, 0x00},
-               ID:        t.ID,
-               clientID:  cc.ID,
-               ErrorCode: []byte{0, 0, 0, 0},
-               Fields:    fields,
+               IsReply:  1,
+               ID:       t.ID,
+               clientID: cc.ID,
+               Fields:   fields,

        }
 }
 
 // NewErrReply returns an error reply Transaction with errMsg
        }
 }
 
 // NewErrReply returns an error reply Transaction with errMsg

-func (cc *ClientConn) NewErrReply(t *Transaction, errMsg string) Transaction {
-       return Transaction{
-               clientID:  cc.ID,
-               IsReply:   0x01,
-               Type:      []byte{0, 0},
-               ID:        t.ID,
-               ErrorCode: []byte{0, 0, 0, 1},
-               Fields: []Field{
-                       NewField(FieldError, []byte(errMsg)),
+func (cc *ClientConn) NewErrReply(t *Transaction, errMsg string) []Transaction {
+       return []Transaction{
+               {
+                       clientID:  cc.ID,
+                       IsReply:   1,
+                       ID:        t.ID,
+                       ErrorCode: [4]byte{0, 0, 0, 1},
+                       Fields: []Field{
+                               NewField(FieldError, []byte(errMsg)),
+                       },

                },
        }
 }
 
                },
        }
 }
 

+var clientSortFunc = func(a, b *ClientConn) int {
+       return cmp.Compare(
+               binary.BigEndian.Uint16(a.ID[:]),
+               binary.BigEndian.Uint16(b.ID[:]),
+       )
+}
+

 // sortedClients is a utility function that takes a map of *ClientConn and returns a sorted slice of the values.
 // The purpose of this is to ensure that the ordering of client connections is deterministic so that test assertions work.
 // sortedClients is a utility function that takes a map of *ClientConn and returns a sorted slice of the values.
 // The purpose of this is to ensure that the ordering of client connections is deterministic so that test assertions work.

-func sortedClients(unsortedClients map[uint16]*ClientConn) (clients []*ClientConn) {
+func sortedClients(unsortedClients map[[2]byte]*ClientConn) (clients []*ClientConn) {

        for _, c := range unsortedClients {
                clients = append(clients, c)
        }
        for _, c := range unsortedClients {
                clients = append(clients, c)
        }

-       sort.Sort(byClientID(clients))
+
+       slices.SortFunc(clients, clientSortFunc)
+

        return clients
 }
 
        return clients
 }