From 636ce0683674866a1367511e6b138e081b2409ff Mon Sep 17 00:00:00 2001 From: Marco Date: Mon, 20 May 2024 15:36:13 +0200 Subject: [PATCH] Improve reconnection handling 1. Lobbies are only identified by their passphrases 2. Improve logging 3. Do not close an existing websocket connection for a player but ignore the request --- api/handler/handler.go | 25 +++++++++---------------- api/handler/handler_test.go | 12 +++++------- api/handler/websocket.go | 12 ++++++++---- api/lobby_info.go | 7 ------- api/player_info.go | 1 - chess/game.go | 3 ++- chess/player.go | 4 +++- connection/type.go | 28 ++++++++++++++++++++++++++-- lobbies/registry.go | 10 +++------- lobbies/usher.go | 6 ------ 10 files changed, 56 insertions(+), 52 deletions(-) delete mode 100644 api/lobby_info.go diff --git a/api/handler/handler.go b/api/handler/handler.go index 886e099..b2b4a99 100644 --- a/api/handler/handler.go +++ b/api/handler/handler.go @@ -30,7 +30,6 @@ func HostGameHandler(c *gin.Context) { passphrase := lobby.Passphrase.String() info := api.PlayerInfo{ PlayerID: &player.Uuid, - LobbyID: &lobby.Uuid, Passphrase: &passphrase, } c.Header("Access-Control-Allow-Origin", "*") @@ -54,12 +53,12 @@ func GetLobbyForPassphraseHandler(c *gin.Context) { return } - lobbyInfo := api.LobbyInfo{ - ID: &lobby.Uuid, + passphrase := api.Passphrase{ + Value: (*string)(&lobby.Passphrase), } c.Header("Access-Control-Allow-Origin", "*") - c.IndentedJSON(http.StatusOK, lobbyInfo) + c.IndentedJSON(http.StatusOK, passphrase) } // TODO: this will be replaced by the JoinGameHandler() @@ -77,8 +76,7 @@ func JoinPrivateGame(c *gin.Context) { if req.Passphrase != nil && *req.Passphrase != "" && - req.PlayerID != nil && - req.LobbyID != nil { //is reconnect + req.PlayerID != nil { //is reconnect lobby := u.FindExistingPrivateLobby(utils.Passphrase(*req.Passphrase)) var found bool if lobby != nil { @@ -90,7 +88,6 @@ func JoinPrivateGame(c *gin.Context) { http.StatusOK, api.PlayerInfo{ PlayerID: req.PlayerID, - LobbyID: req.LobbyID, Passphrase: req.Passphrase, }) return @@ -114,7 +111,6 @@ func JoinPrivateGame(c *gin.Context) { info := api.PlayerInfo{ PlayerID: &player.Uuid, - LobbyID: &lobby.Uuid, Passphrase: req.Passphrase, } c.Header("Access-Control-Allow-Origin", "*") @@ -124,25 +120,22 @@ func JoinPrivateGame(c *gin.Context) { func JoinGameHandler(c *gin.Context) { limiter.Take() - id := c.Param("id") - idAsUUID, err := uuid.Parse(id) + passphrase := api.Passphrase{} + err := c.ShouldBindJSON(&passphrase) if err != nil { c.IndentedJSON(http.StatusBadRequest, nil) return } - passphrase := api.Passphrase{} - c.ShouldBindJSON(&passphrase) - u := lobbies.GetUsher() - lobby := u.GetLobbyByID(idAsUUID) + lobby := u.FindExistingPrivateLobby(utils.Passphrase(*passphrase.Value)) if lobby == nil { c.IndentedJSON(http.StatusNotFound, nil) return } - lobbyInfo := api.LobbyInfo{ - ID: &lobby.Uuid, + lobbyInfo := api.Passphrase{ + Value: (*string)(&lobby.Passphrase), } c.IndentedJSON(http.StatusOK, lobbyInfo) diff --git a/api/handler/handler_test.go b/api/handler/handler_test.go index ff5fead..ce02ece 100644 --- a/api/handler/handler_test.go +++ b/api/handler/handler_test.go @@ -9,13 +9,12 @@ import ( "testing" "github.com/gin-gonic/gin" - "github.com/google/uuid" "github.com/stretchr/testify/assert" ) func Test_GetLobbyFromPassphraseHandler(t *testing.T) { var passphraseURLParameter string - var hostedLobbyId uuid.UUID + var receivedPhrase string t.Run("host a lobby", func(t *testing.T) { r1 := httptest.NewRecorder() @@ -30,8 +29,7 @@ func Test_GetLobbyFromPassphraseHandler(t *testing.T) { err := json.Unmarshal(r1.Body.Bytes(), &playerInfo) assert.NoError(t, err) - receivedPhrase := *playerInfo.Passphrase - hostedLobbyId = *playerInfo.LobbyID + receivedPhrase = *playerInfo.Passphrase passphrase := utils.NewPassphraseFromString(receivedPhrase) passphraseURLParameter = passphrase.AsURLParam() @@ -48,10 +46,10 @@ func Test_GetLobbyFromPassphraseHandler(t *testing.T) { engine.ServeHTTP(r2, getLobbyRequest) - lobbyInfo := api.LobbyInfo{} - err := json.Unmarshal(r2.Body.Bytes(), &lobbyInfo) + passhrase := api.Passphrase{} + err := json.Unmarshal(r2.Body.Bytes(), &passhrase) assert.NoError(t, err) - assert.Equal(t, hostedLobbyId, *lobbyInfo.ID) + assert.Equal(t, http.StatusOK, r2.Code) }) } diff --git a/api/handler/websocket.go b/api/handler/websocket.go index 5ca8d46..646064a 100644 --- a/api/handler/websocket.go +++ b/api/handler/websocket.go @@ -6,6 +6,7 @@ import ( "fmt" "log" "mchess_server/api" + "mchess_server/utils" "net/http" "mchess_server/lobbies" @@ -23,7 +24,6 @@ var upgrader = gorillaws.Upgrader{ func RegisterWebSocketConnection(c *gin.Context) { limiter.Take() - log.Println(c.Request) conn, err := upgrader.Upgrade(c.Writer, c.Request, nil) if err != nil { log.Println(err) @@ -55,7 +55,7 @@ func waitForAndHandlePlayerID(ctx context.Context, conn *gorillaws.Conn) { return } - lobby := lobbies.GetLobbyRegistry().GetLobbyByUUID(*info.LobbyID) + lobby := lobbies.GetLobbyRegistry().GetLobbyByPassphrase(utils.NewPassphraseFromString(*info.Passphrase)) if lobby == nil { conn.WriteMessage(msgType, []byte("lobby not found")) conn.Close() @@ -69,10 +69,14 @@ func waitForAndHandlePlayerID(ctx context.Context, conn *gorillaws.Conn) { return } if player.Conn.HasWebsocketConnection() { - player.Conn.Close("closing existing connection") + conn.WriteMessage(msgType, []byte("player already connected")) + return } lobby.Game.SetWebsocketConnectionFor(ctx, player, conn) - log.Println("player after setting connection: ", player) + log.Println("player after setting connection: ") + log.Println("id: ", player.Uuid) + log.Println("color: ", player.GetColor()) + log.Println("Connection: ", player.Conn.ID) } func ConnectWsForGame(c *gin.Context) { diff --git a/api/lobby_info.go b/api/lobby_info.go deleted file mode 100644 index 149bf20..0000000 --- a/api/lobby_info.go +++ /dev/null @@ -1,7 +0,0 @@ -package api - -import "github.com/google/uuid" - -type LobbyInfo struct { - ID *uuid.UUID `json:"id,omitempty"` -} diff --git a/api/player_info.go b/api/player_info.go index 3540151..f71a82a 100644 --- a/api/player_info.go +++ b/api/player_info.go @@ -4,6 +4,5 @@ import "github.com/google/uuid" type PlayerInfo struct { PlayerID *uuid.UUID `json:"playerID,omitempty"` - LobbyID *uuid.UUID `json:"lobbyID,omitempty"` Passphrase *string `json:"passphrase,omitempty"` } diff --git a/chess/game.go b/chess/game.go index 6212c8b..c768873 100644 --- a/chess/game.go +++ b/chess/game.go @@ -105,7 +105,7 @@ func (game *Game) Handle() { log.Println("Error while reading message:", err) return } - log.Println("Player ", game.currentTurnPlayer, " moved:\n", receivedMove) + log.Println("Player ", game.currentTurnPlayer.color.String(), " moved:\n", receivedMove) game.gameState = CheckMove case CheckMove: @@ -190,6 +190,7 @@ func (game Game) notifyPlayersAboutGameStart() error { } func (game Game) broadcastMove(move types.Move) error { + log.Println("broadcast move") err := game.GetPlayer1().SendBoardState(move, game.board.PGN(), game.currentTurnPlayer.GetColor()) if err != nil { return err diff --git a/chess/player.go b/chess/player.go index cc663ae..f2d9278 100644 --- a/chess/player.go +++ b/chess/player.go @@ -36,6 +36,7 @@ func (p Player) hasWebsocketConnection() bool { func (p *Player) SetWebsocketConnection(ctx context.Context, ws *gorillaws.Conn) { p.Conn.SetWebsocketConnection(ws) + p.Conn.SetForColor(p.color) } func (p *Player) SetWebsocketConnectionAndSendBoardState( @@ -49,6 +50,7 @@ func (p *Player) SetWebsocketConnectionAndSendBoardState( func (p *Player) SetColor(color types.ChessColor) { p.color = color + p.Conn.SetForColor(p.color) } func (p *Player) GetColor() types.ChessColor { @@ -157,7 +159,7 @@ func (p *Player) ReadMove() (types.Move, error) { func (p *Player) readMessage() ([]byte, error) { msg, err := p.Conn.Read() - log.Printf("Reading message: %s from player %s", string(msg), p.Uuid.String()) + log.Printf("Reading message from %s: %s", p.color.String(), string(msg)) return msg, err } diff --git a/connection/type.go b/connection/type.go index dcd3ca7..7cc3ba9 100644 --- a/connection/type.go +++ b/connection/type.go @@ -3,22 +3,27 @@ package connection import ( "context" "log" + "mchess_server/types" "sync" + "github.com/google/uuid" gorillaws "github.com/gorilla/websocket" ) type Connection struct { + ID uuid.UUID ws *gorillaws.Conn wsConnectionEstablished chan bool wsWriteLock sync.Mutex ctx context.Context buffer MessageBuffer disconnectCallback func() + forColor types.ChessColor } func NewConnection(options ...func(*Connection)) *Connection { connection := Connection{ + ID: uuid.New(), buffer: *newMessageBuffer(100), wsConnectionEstablished: make(chan bool), } @@ -50,6 +55,10 @@ func WithDisconnectCallback(cb func()) func(*Connection) { } } +func (conn *Connection) SetForColor(color types.ChessColor) { + conn.forColor = color +} + func (conn *Connection) SetDisconnectCallback(cb func()) { conn.disconnectCallback = cb } @@ -60,6 +69,7 @@ func (conn *Connection) HasWebsocketConnection() bool { func (conn *Connection) SetWebsocketConnection(ws *gorillaws.Conn) { if ws == nil { + conn.logConnection("ERROR: setting ws = null") return } @@ -67,14 +77,16 @@ func (conn *Connection) SetWebsocketConnection(ws *gorillaws.Conn) { select { case conn.wsConnectionEstablished <- true: + conn.logConnection("case wsConnectionEstablished <- true") default: + conn.logConnection("DEFAULT CASE") } go func() { for { _, msg, err := conn.ws.ReadMessage() if err != nil { - log.Println("while reading from websocket: %w", err) + conn.logConnection("while reading from websocket: %w", err) conn.unsetWebsocketConnection() if conn.disconnectCallback != nil { @@ -85,21 +97,27 @@ func (conn *Connection) SetWebsocketConnection(ws *gorillaws.Conn) { conn.buffer.Insert(string(msg)) } }() + defer conn.logConnection("websocket connection set") } func (conn *Connection) unsetWebsocketConnection() { + conn.logConnection("websocket connection unset") conn.ws = nil } func (conn *Connection) Write(msg []byte) error { + conn.logConnection("about to write") + conn.logConnection("locking") conn.wsWriteLock.Lock() + defer conn.logConnection("unlocking") defer conn.wsWriteLock.Unlock() if conn.ws == nil { //if ws is not yet set, we wait for it + conn.logConnection("waiting for wsConnectionEstablished channel") <-conn.wsConnectionEstablished } - log.Printf("Writing message: %s", string(msg)) + conn.logConnection("Writing message: %s", string(msg)) return conn.ws.WriteMessage(gorillaws.TextMessage, msg) } @@ -114,7 +132,13 @@ func (conn *Connection) Read() ([]byte, error) { } func (conn *Connection) Close(msg string) { + conn.logConnection("closing websocket connection") conn.ws.WriteMessage(gorillaws.TextMessage, []byte(msg)) conn.ws.Close() conn.ws = nil } + +func (con *Connection) logConnection(v ...any) { + log.Println("on connection: ", con.ID) + log.Println("for color: ", con.forColor.String(), v) +} diff --git a/lobbies/registry.go b/lobbies/registry.go index a346a2e..b8e17c4 100644 --- a/lobbies/registry.go +++ b/lobbies/registry.go @@ -7,7 +7,7 @@ import ( ) type LobbyRegistry struct { - lobbies map[uuid.UUID]*Lobby + lobbies map[utils.Passphrase]*Lobby } var instance *LobbyRegistry @@ -21,7 +21,7 @@ func GetLobbyRegistry() *LobbyRegistry { } func newLobbyRegistry() *LobbyRegistry { - return &LobbyRegistry{lobbies: make(map[uuid.UUID]*Lobby)} + return &LobbyRegistry{lobbies: make(map[utils.Passphrase]*Lobby)} } func (r *LobbyRegistry) CreateNewPrivateLobby() *Lobby { @@ -42,10 +42,6 @@ func (r *LobbyRegistry) GetLobbyForPlayer() *Lobby { return newLobby } -func (r *LobbyRegistry) GetLobbyByUUID(uuid uuid.UUID) *Lobby { - return r.lobbies[uuid] -} - func (r *LobbyRegistry) GetLobbyByPassphrase(p utils.Passphrase) *Lobby { for _, lobby := range r.lobbies { if lobby.Passphrase == p { @@ -56,6 +52,6 @@ func (r *LobbyRegistry) GetLobbyByPassphrase(p utils.Passphrase) *Lobby { } func (r *LobbyRegistry) addNewLobby(lobby *Lobby) uuid.UUID { - r.lobbies[lobby.Uuid] = lobby + r.lobbies[lobby.Passphrase] = lobby return lobby.Uuid } diff --git a/lobbies/usher.go b/lobbies/usher.go index 86a95d5..00fcfc1 100644 --- a/lobbies/usher.go +++ b/lobbies/usher.go @@ -3,8 +3,6 @@ package lobbies import ( "mchess_server/chess" "mchess_server/utils" - - "github.com/google/uuid" ) type Usher struct { @@ -41,10 +39,6 @@ func (u *Usher) FindExistingPrivateLobby(p utils.Passphrase) *Lobby { return lobby } -func (*Usher) GetLobbyByID(id uuid.UUID) *Lobby { - return GetLobbyRegistry().GetLobbyByUUID(id) -} - func (u *Usher) AddPlayerToLobbyAndStartGameIfFull(player *chess.Player, lobby *Lobby) { lobby.AddPlayerAndStartGameIfFull(player) }