fix double chat signon

.mockery.yaml

@@ -177,6 +177,9 @@ packages:
       ChatNavService:
         config:
           filename: "mock_chat_nav_service_test.go"
+      ChatSessionManager:
+        config:
+          filename: "mock_chat_session_manager_test.go"
       ICBMService:
         config:
           filename: "mock_icbm_service_test.go"

cmd/server/factory.go

@@ -440,9 +440,10 @@ func TOC(deps Container) *toc.Server {
 				deps.inMemorySessionManager,
 				deps.sqLiteUserStore,
 			),
-			CookieBaker:      deps.hmacCookieBaker,
-			DirSearchService: foodgroup.NewODirService(logger, deps.sqLiteUserStore),
-			ICBMService:      deps.icbmSvc,
+			ChatSessionManager: deps.chatSessionManager,
+			CookieBaker:        deps.hmacCookieBaker,
+			DirSearchService:   foodgroup.NewODirService(logger, deps.sqLiteUserStore),
+			ICBMService:        deps.icbmSvc,
 			LocateService: foodgroup.NewLocateService(
 				deps.sqLiteUserStore,
 				deps.inMemorySessionManager,

server/toc/cmd_client.go

@@ -116,25 +116,26 @@ func (c *ChatRegistry) Sessions() []*state.SessionInstance {
 //   - Receives incoming messages from the OSCAR server and translates them into
 //     TOC responses for the client.
 type OSCARProxy struct {
-	AdminService      AdminService
-	AuthService       AuthService
-	BuddyListRegistry BuddyListRegistry
-	BuddyService      BuddyService
-	ChatNavService    ChatNavService
-	ChatService       ChatService
-	CookieBaker       CookieBaker
-	DirSearchService  DirSearchService
-	ICBMService       ICBMService
-	LocateService     LocateService
-	Logger            *slog.Logger
-	OServiceService   OServiceService
-	PermitDenyService PermitDenyService
-	TOCConfigStore    TOCConfigStore
-	SessionRetriever  SessionRetriever
-	FeedbagService    FeedbagService
-	FeedbagManager    FeedbagManager
-	SNACRateLimits    wire.SNACRateLimits
-	HTTPIPRateLimiter *IPRateLimiter
+	AdminService       AdminService
+	AuthService        AuthService
+	BuddyListRegistry  BuddyListRegistry
+	BuddyService       BuddyService
+	ChatNavService     ChatNavService
+	ChatService        ChatService
+	ChatSessionManager ChatSessionManager
+	CookieBaker        CookieBaker
+	DirSearchService   DirSearchService
+	ICBMService        ICBMService
+	LocateService      LocateService
+	Logger             *slog.Logger
+	OServiceService    OServiceService
+	PermitDenyService  PermitDenyService
+	TOCConfigStore     TOCConfigStore
+	SessionRetriever   SessionRetriever
+	FeedbagService     FeedbagService
+	FeedbagManager     FeedbagManager
+	SNACRateLimits     wire.SNACRateLimits
+	HTTPIPRateLimiter  *IPRateLimiter
 	// RandIntn is the source for feedbag item ID generation.
 	// Inject a deterministic func in tests to assert exact feedbag item slices.
 	RandIntn func(n int) int
@@ -531,6 +532,12 @@ func (s OSCARProxy) ChatAccept(
 		return 0, s.runtimeErr(ctx, fmt.Errorf("AuthService.RegisterChatSession: %w", err))
 	}
 
+	chatSess.Session().OnSessionClose(func() {
+		ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+		defer cancel()
+		s.AuthService.SignoutChat(ctx, chatSess)
+	})
+
 	if msg, isLimited := s.checkRateLimit(ctx, me, wire.OService, wire.OServiceClientOnline); isLimited {
 		return 0, msg
 	}
@@ -720,6 +727,12 @@ func (s OSCARProxy) ChatJoin(
 		return 0, s.runtimeErr(ctx, fmt.Errorf("AuthService.RegisterChatSession: %w", err))
 	}
 
+	chatSess.Session().OnSessionClose(func() {
+		ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+		defer cancel()
+		s.AuthService.SignoutChat(ctx, chatSess)
+	})
+
 	if msg, isLimited := s.checkRateLimit(ctx, me, wire.OService, wire.OServiceClientOnline); isLimited {
 		return 0, msg
 	}
@@ -766,8 +779,6 @@ func (s OSCARProxy) ChatLeave(ctx context.Context, chatRegistry *ChatRegistry, a
 		return s.runtimeErr(ctx, fmt.Errorf("chatRegistry.RetrieveSess: chat session `%d` not found", chatID))
 	}
 
-	s.AuthService.SignoutChat(ctx, me)
-
 	me.CloseInstance() // stop async server SNAC reply handler for this chat room
 
 	chatRegistry.RemoveSess(chatID)
@@ -2393,10 +2404,7 @@ func (s OSCARProxy) Signon(ctx context.Context, args []byte, recalcWarning func(
 		if err := s.BuddyListRegistry.UnregisterBuddyList(ctx, instance.IdentScreenName()); err != nil {
 			s.Logger.ErrorContext(ctx, "error removing buddy list entry", "err", err.Error())
 		}
-		for _, sess := range chatRegistry.Sessions() {
-			s.AuthService.SignoutChat(ctx, sess)
-			sess.CloseInstance() // stop async server SNAC reply handler for this chat room
-		}
+		s.ChatSessionManager.RemoveUserFromAllChats(instance.IdentScreenName())
 		s.AuthService.Signout(ctx, instance)
 	})
 

server/toc/cmd_client_test.go

@@ -1538,15 +1538,6 @@ func TestOSCARProxy_RecvClientCmd_ChatLeave(t *testing.T) {
 				reg.RegisterSess(0, newTestSession("me"))
 				return reg
 			}(),
-			mockParams: mockParams{
-				authParams: authParams{
-					signoutChatParams: signoutChatParams{
-						{
-							me: state.NewIdentScreenName("me"),
-						},
-					},
-				},
-			},
 			wantMsg: []string{"CHAT_LEFT:0"},
 		},
 		{
@@ -1572,14 +1563,8 @@ func TestOSCARProxy_RecvClientCmd_ChatLeave(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			ctx := context.Background()
 
-			authSvc := newMockAuthService(t)
-			for _, params := range tc.mockParams.signoutChatParams {
-				authSvc.EXPECT().SignoutChat(ctx, matchSession(params.me))
-			}
-
 			svc := OSCARProxy{
-				Logger:      slog.Default(),
-				AuthService: authSvc,
+				Logger: slog.Default(),
 			}
 			msg := svc.RecvClientCmd(ctx, nil, tc.givenChatRegistry, tc.givenCmd, nil, nil)
 
@@ -6960,14 +6945,21 @@ func TestOSCARProxy_Signon(t *testing.T) {
 
 			buddySvc := newMockBuddyService(t)
 
+			chatSessionMgr := newMockChatSessionManager(t)
+			for _, params := range tc.mockParams.removeUserFromAllChatsParams {
+				chatSessionMgr.EXPECT().
+					RemoveUserFromAllChats(params.user)
+			}
+
 			svc := OSCARProxy{
-				AuthService:       authSvc,
-				BuddyListRegistry: buddyRegistry,
-				BuddyService:      buddySvc,
-				Logger:            slog.Default(),
-				TOCConfigStore:    tocCfg,
-				FeedbagService:    fbSvc,
-				FeedbagManager:    fbMgr,
+				AuthService:        authSvc,
+				BuddyListRegistry:  buddyRegistry,
+				BuddyService:       buddySvc,
+				ChatSessionManager: chatSessionMgr,
+				Logger:             slog.Default(),
+				TOCConfigStore:     tocCfg,
+				FeedbagService:     fbSvc,
+				FeedbagManager:     fbMgr,
 			}
 			sess, msg := svc.Signon(ctx, tc.givenCmd,
 				func(ctx context.Context, instance *state.SessionInstance) error { return nil },

server/toc/helpers_test.go

@@ -314,6 +314,17 @@ type feedbagServiceDeleteItemParams []struct {
 	err    error
 }
 
+// removeUserFromAllChatsParams is the list of parameters for each expected
+// ChatSessionManager.RemoveUserFromAllChats call.
+type removeUserFromAllChatsParams []struct {
+	user state.IdentScreenName
+}
+
+// chatSessionManagerParams groups mock expectations for ChatSessionManager.
+type chatSessionManagerParams struct {
+	removeUserFromAllChatsParams
+}
+
 type mockParams struct {
 	adminParams
 	authParams
@@ -321,6 +332,7 @@ type mockParams struct {
 	buddyParams
 	chatNavParams
 	chatParams
+	chatSessionManagerParams
 	cookieBakerParams
 	dirSearchParams
 	icbmParams

server/toc/mock_chat_session_manager_test.go

@@ -0,0 +1,77 @@
+// Code generated by mockery; DO NOT EDIT.
+// github.com/vektra/mockery
+// template: testify
+
+package toc
+
+import (
+	"github.com/mk6i/open-oscar-server/state"
+	mock "github.com/stretchr/testify/mock"
+)
+
+// newMockChatSessionManager creates a new instance of mockChatSessionManager. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
+// The first argument is typically a *testing.T value.
+func newMockChatSessionManager(t interface {
+	mock.TestingT
+	Cleanup(func())
+}) *mockChatSessionManager {
+	mock := &mockChatSessionManager{}
+	mock.Mock.Test(t)
+
+	t.Cleanup(func() { mock.AssertExpectations(t) })
+
+	return mock
+}
+
+// mockChatSessionManager is an autogenerated mock type for the ChatSessionManager type
+type mockChatSessionManager struct {
+	mock.Mock
+}
+
+type mockChatSessionManager_Expecter struct {
+	mock *mock.Mock
+}
+
+func (_m *mockChatSessionManager) EXPECT() *mockChatSessionManager_Expecter {
+	return &mockChatSessionManager_Expecter{mock: &_m.Mock}
+}
+
+// RemoveUserFromAllChats provides a mock function for the type mockChatSessionManager
+func (_mock *mockChatSessionManager) RemoveUserFromAllChats(user state.IdentScreenName) {
+	_mock.Called(user)
+	return
+}
+
+// mockChatSessionManager_RemoveUserFromAllChats_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemoveUserFromAllChats'
+type mockChatSessionManager_RemoveUserFromAllChats_Call struct {
+	*mock.Call
+}
+
+// RemoveUserFromAllChats is a helper method to define mock.On call
+//   - user state.IdentScreenName
+func (_e *mockChatSessionManager_Expecter) RemoveUserFromAllChats(user interface{}) *mockChatSessionManager_RemoveUserFromAllChats_Call {
+	return &mockChatSessionManager_RemoveUserFromAllChats_Call{Call: _e.mock.On("RemoveUserFromAllChats", user)}
+}
+
+func (_c *mockChatSessionManager_RemoveUserFromAllChats_Call) Run(run func(user state.IdentScreenName)) *mockChatSessionManager_RemoveUserFromAllChats_Call {
+	_c.Call.Run(func(args mock.Arguments) {
+		var arg0 state.IdentScreenName
+		if args[0] != nil {
+			arg0 = args[0].(state.IdentScreenName)
+		}
+		run(
+			arg0,
+		)
+	})
+	return _c
+}
+
+func (_c *mockChatSessionManager_RemoveUserFromAllChats_Call) Return() *mockChatSessionManager_RemoveUserFromAllChats_Call {
+	_c.Call.Return()
+	return _c
+}
+
+func (_c *mockChatSessionManager_RemoveUserFromAllChats_Call) RunAndReturn(run func(user state.IdentScreenName)) *mockChatSessionManager_RemoveUserFromAllChats_Call {
+	_c.Run(run)
+	return _c
+}

server/toc/types.go

@@ -142,3 +142,9 @@ type OSCARProxyer interface {
 	RecvClientCmd(ctx context.Context, sessBOS *state.Session, chatRegistry *ChatRegistry, payload []byte, toCh chan<- []string, doAsync func(f func() error)) []string
 	NewServeMux() http.Handler
 }
+
+// ChatSessionManager is the interface for closing chat sessions
+// when a client disconnects.
+type ChatSessionManager interface {
+	RemoveUserFromAllChats(user state.IdentScreenName)
+}