fix data race in bos_service and chat_service tests

Caused by issue described in
stretchr/testify#1229.

The workaround here is to close out the session in the same goroutine as
handleNewConnection. This actually simplifies the code by reducing the #
of goroutines per connection by 1 and allowing the unit tests to
synchronize without a waitgroup.

.github/workflows/go.yml

@@ -0,0 +1,31 @@
+# This workflow will build a golang project
+# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-go
+
+name: Go
+
+on:
+  push:
+    branches: [ "main" ]
+  pull_request:
+    branches: [ "main" ]
+
+jobs:
+
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Set up Go
+        uses: actions/setup-go@v4
+        with:
+          go-version: '1.21'
+
+      - name: Build
+        run: go build -v ./...
+
+      - name: Test
+        run: go test -v -race -coverprofile=coverage.out -covermode=atomic ./...
+
+      - name: Upload coverage reports to Codecov
+        uses: codecov/codecov-action@v3

.mockery.yaml

@@ -3,7 +3,7 @@ dir: "{{.InterfaceDir}}"
 mockname: "mock{{.InterfaceName}}"
 inpackage: True
 packages:
-  github.com/mkaminski/goaim/server:
+  github.com/mk6i/retro-aim-server/server:
     interfaces:
       AuthHandler:
         config:
@@ -44,7 +44,7 @@ packages:
       ChatServiceRouter:
         config:
           filename: "chat_service_router_mock_test.go"
-  github.com/mkaminski/goaim/handler:
+  github.com/mk6i/retro-aim-server/handler:
     interfaces:
       FeedbagManager:
         config:

LICENSE

@@ -1,6 +1,6 @@
 MIT License
 
-Copyright (c) 2023 Mike Kaminski
+Copyright (c) 2024 mk6i
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

README.md

@@ -1,4 +1,5 @@
 # Retro AIM Server
+[![codecov](https://codecov.io/github/mk6i/retro-aim-server/graph/badge.svg?token=MATKPP77JT)](https://codecov.io/github/mk6i/retro-aim-server)
 
 Retro AIM Server is a server implementation of the OSCAR protocol that supports AIM versions 5.0-5.9.
 

cmd/main.go

@@ -6,11 +6,11 @@ import (
 	"os"
 	"sync"
 
-	"github.com/mkaminski/goaim/handler"
-	"github.com/mkaminski/goaim/state"
+	"github.com/mk6i/retro-aim-server/handler"
+	"github.com/mk6i/retro-aim-server/state"
 
 	"github.com/kelseyhightower/envconfig"
-	"github.com/mkaminski/goaim/server"
+	"github.com/mk6i/retro-aim-server/server"
 )
 
 func main() {

codecov.yml

@@ -0,0 +1,3 @@
+coverage:
+  round: down
+  range: 70..79

go.mod

@@ -1,11 +1,11 @@
-module github.com/mkaminski/goaim
+module github.com/mk6i/retro-aim-server
 
-go 1.20
+go 1.21.3
 
 require (
-	github.com/google/uuid v1.3.0
+	github.com/google/uuid v1.5.0
 	github.com/kelseyhightower/envconfig v1.4.0
-	github.com/mattn/go-sqlite3 v1.14.17
+	github.com/mattn/go-sqlite3 v1.14.19
 	github.com/stretchr/testify v1.8.4
 )
 

go.sum

@@ -1,12 +1,12 @@
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
-github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
+github.com/google/uuid v1.5.0 h1:1p67kYwdtXjb0gL0BPiP1Av9wiZPo5A8z2cWkTZ+eyU=
+github.com/google/uuid v1.5.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/kelseyhightower/envconfig v1.4.0 h1:Im6hONhd3pLkfDFsbRgu68RDNkGF1r3dvMUtDTo2cv8=
 github.com/kelseyhightower/envconfig v1.4.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg=
-github.com/mattn/go-sqlite3 v1.14.17 h1:mCRHCLDUBXgpKAqIKsaAaAsrAlbkeomtRFKXh2L6YIM=
-github.com/mattn/go-sqlite3 v1.14.17/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S2DGjv9HUNg=
+github.com/mattn/go-sqlite3 v1.14.19 h1:fhGleo2h1p8tVChob4I9HpmVFIAkKGpiukdrgQbWfGI=
+github.com/mattn/go-sqlite3 v1.14.19/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S2DGjv9HUNg=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=

handler/auth.go

@@ -6,9 +6,9 @@ import (
 	"errors"
 
 	"github.com/google/uuid"
-	"github.com/mkaminski/goaim/oscar"
-	"github.com/mkaminski/goaim/server"
-	"github.com/mkaminski/goaim/state"
+	"github.com/mk6i/retro-aim-server/oscar"
+	"github.com/mk6i/retro-aim-server/server"
+	"github.com/mk6i/retro-aim-server/state"
 )
 
 // NewAuthService creates a new instance of AuthService.

handler/auth_test.go

@@ -5,9 +5,9 @@ import (
 	"testing"
 
 	"github.com/google/uuid"
-	"github.com/mkaminski/goaim/oscar"
-	"github.com/mkaminski/goaim/server"
-	"github.com/mkaminski/goaim/state"
+	"github.com/mk6i/retro-aim-server/oscar"
+	"github.com/mk6i/retro-aim-server/server"
+	"github.com/mk6i/retro-aim-server/state"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 )

handler/buddy.go

@@ -3,8 +3,8 @@ package handler
 import (
 	"context"
 
-	"github.com/mkaminski/goaim/oscar"
-	"github.com/mkaminski/goaim/state"
+	"github.com/mk6i/retro-aim-server/oscar"
+	"github.com/mk6i/retro-aim-server/state"
 )
 
 func NewBuddyService() *BuddyService {

handler/buddy_test.go

@@ -3,7 +3,7 @@ package handler
 import (
 	"testing"
 
-	"github.com/mkaminski/goaim/oscar"
+	"github.com/mk6i/retro-aim-server/oscar"
 	"github.com/stretchr/testify/assert"
 )
 

handler/chat.go

@@ -3,18 +3,18 @@ package handler
 import (
 	"context"
 
-	"github.com/mkaminski/goaim/oscar"
-	"github.com/mkaminski/goaim/state"
+	"github.com/mk6i/retro-aim-server/oscar"
+	"github.com/mk6i/retro-aim-server/state"
 )
 
-func NewChatService(chatRegistry *state.ChatRegistry) *ChatService {
+func NewChatService(chatRegistry ChatRegistry) *ChatService {
 	return &ChatService{
 		chatRegistry: chatRegistry,
 	}
 }
 
 type ChatService struct {
-	chatRegistry *state.ChatRegistry
+	chatRegistry ChatRegistry
 }
 
 func (s ChatService) ChannelMsgToHostHandler(ctx context.Context, sess *state.Session, chatID string, inFrame oscar.SNACFrame, inBody oscar.SNAC_0x0E_0x05_ChatChannelMsgToHost) (*oscar.SNACMessage, error) {

handler/chat_nav.go

@@ -6,8 +6,8 @@ import (
 
 	"log/slog"
 
-	"github.com/mkaminski/goaim/oscar"
-	"github.com/mkaminski/goaim/state"
+	"github.com/mk6i/retro-aim-server/oscar"
+	"github.com/mk6i/retro-aim-server/state"
 )
 
 // NewChatNavService creates a new instance of NewChatNavService.

handler/chat_nav_test.go

@@ -5,8 +5,8 @@ import (
 	"testing"
 	"time"
 
-	"github.com/mkaminski/goaim/oscar"
-	"github.com/mkaminski/goaim/state"
+	"github.com/mk6i/retro-aim-server/oscar"
+	"github.com/mk6i/retro-aim-server/state"
 	"github.com/stretchr/testify/assert"
 )
 

handler/chat_test.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"testing"
 
-	"github.com/mkaminski/goaim/oscar"
-	"github.com/mkaminski/goaim/state"
+	"github.com/mk6i/retro-aim-server/oscar"
+	"github.com/mk6i/retro-aim-server/state"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 )
@@ -18,10 +18,14 @@ func TestChatService_ChannelMsgToHostHandler(t *testing.T) {
 		userSession *state.Session
 		// inputSNAC is the SNAC sent by the sender client
 		inputSNAC oscar.SNACMessage
+		// mockParams is the list of params sent to mocks that satisfy this
+		// method's dependencies
+		mockParams mockParams
 		// expectSNACToParticipants is the message the server broadcast to chat
 		// room participants (except the sender)
 		expectSNACToParticipants oscar.SNACMessage
 		expectOutput             *oscar.SNACMessage
+		wantErr                  error
 	}{
 		{
 			name:        "send chat room message, expect acknowledgement to sender client",
@@ -47,6 +51,13 @@ func TestChatService_ChannelMsgToHostHandler(t *testing.T) {
 					},
 				},
 			},
+			mockParams: mockParams{
+				chatRegistryParams: chatRegistryParams{
+					chatRegistryRetrieveParams: chatRegistryRetrieveParams{
+						chatID: "the-chat-id",
+					},
+				},
+			},
 			expectSNACToParticipants: oscar.SNACMessage{
 				Frame: oscar.SNACFrame{
 					FoodGroup: oscar.Chat,
@@ -104,6 +115,13 @@ func TestChatService_ChannelMsgToHostHandler(t *testing.T) {
 					},
 				},
 			},
+			mockParams: mockParams{
+				chatRegistryParams: chatRegistryParams{
+					chatRegistryRetrieveParams: chatRegistryRetrieveParams{
+						chatID: "the-chat-id",
+					},
+				},
+			},
 			expectSNACToParticipants: oscar.SNACMessage{
 				Frame: oscar.SNACFrame{
 					FoodGroup: oscar.Chat,
@@ -121,7 +139,36 @@ func TestChatService_ChannelMsgToHostHandler(t *testing.T) {
 					},
 				},
 			},
-			expectOutput: &oscar.SNACMessage{},
+		},
+		{
+			name:        "send chat room message, fail due to missing chat room",
+			userSession: newTestSession("user_sending_chat_msg", sessOptCannedSignonTime),
+			inputSNAC: oscar.SNACMessage{
+				Frame: oscar.SNACFrame{
+					RequestID: 1234,
+				},
+				Body: oscar.SNAC_0x0E_0x05_ChatChannelMsgToHost{
+					Cookie:  1234,
+					Channel: 14,
+					TLVRestBlock: oscar.TLVRestBlock{
+						TLVList: oscar.TLVList{
+							{
+								Tag:   oscar.ChatTLVPublicWhisperFlag,
+								Value: []byte{},
+							},
+						},
+					},
+				},
+			},
+			mockParams: mockParams{
+				chatRegistryParams: chatRegistryParams{
+					chatRegistryRetrieveParams: chatRegistryRetrieveParams{
+						chatID: "the-chat-id",
+						err:    state.ErrChatRoomNotFound,
+					},
+				},
+			},
+			wantErr: state.ErrChatRoomNotFound,
 		},
 	}
 
@@ -130,20 +177,20 @@ func TestChatService_ChannelMsgToHostHandler(t *testing.T) {
 			chatID := "the-chat-id"
 
 			chatSessMgr := newMockChatMessageRelayer(t)
-			chatSessMgr.EXPECT().
-				RelayToAllExcept(mock.Anything, tc.userSession, tc.expectSNACToParticipants)
+			if tc.mockParams.chatRegistryRetrieveParams.err == nil {
+				chatSessMgr.EXPECT().
+					RelayToAllExcept(mock.Anything, tc.userSession, tc.expectSNACToParticipants)
+			}
 
-			svc := NewChatService(state.NewChatRegistry())
-			svc.chatRegistry.Register(state.ChatRoom{Cookie: chatID}, chatSessMgr)
+			chatRegistry := newMockChatRegistry(t)
+			chatRegistry.EXPECT().
+				Retrieve(tc.mockParams.chatRegistryRetrieveParams.chatID).
+				Return(state.ChatRoom{}, chatSessMgr, tc.mockParams.chatRegistryRetrieveParams.err)
 
+			svc := NewChatService(chatRegistry)
 			outputSNAC, err := svc.ChannelMsgToHostHandler(context.Background(), tc.userSession, chatID,
 				tc.inputSNAC.Frame, tc.inputSNAC.Body.(oscar.SNAC_0x0E_0x05_ChatChannelMsgToHost))
-			assert.NoError(t, err)
-
-			if tc.expectOutput.Frame == (oscar.SNACFrame{}) {
-				return // handler doesn't return response
-			}
-
+			assert.ErrorIs(t, err, tc.wantErr)
 			assert.Equal(t, tc.expectOutput, outputSNAC)
 		})
 	}

handler/feedbag.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"time"
 
-	"github.com/mkaminski/goaim/oscar"
-	"github.com/mkaminski/goaim/state"
+	"github.com/mk6i/retro-aim-server/oscar"
+	"github.com/mk6i/retro-aim-server/state"
 )
 
 // NewFeedbagService creates a new instance of FeedbagService.

handler/feedbag_test.go

@@ -4,8 +4,8 @@ import (
 	"testing"
 	"time"
 
-	"github.com/mkaminski/goaim/oscar"
-	"github.com/mkaminski/goaim/state"
+	"github.com/mk6i/retro-aim-server/oscar"
+	"github.com/mk6i/retro-aim-server/state"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 )

handler/icbm.go

@@ -3,8 +3,8 @@ package handler
 import (
 	"context"
 
-	"github.com/mkaminski/goaim/oscar"
-	"github.com/mkaminski/goaim/state"
+	"github.com/mk6i/retro-aim-server/oscar"
+	"github.com/mk6i/retro-aim-server/state"
 )
 
 const (