move chat OService logic into separate handlers

cmd/main.go

@@ -60,13 +60,14 @@ func listenBOS(cfg server.Config, sm *server.InMemorySessionManager, fm *server.
 
 	fmt.Printf("BOS server listening on %s\n", server.Address(cfg.OSCARHost, cfg.BOSPort))
 
+	router := server.NewRouter()
 	for {
 		conn, err := listener.Accept()
 		if err != nil {
 			log.Println(err)
 			continue
 		}
-		go handleBOSConnection(cfg, sm, fm, cr, conn)
+		go handleBOSConnection(cfg, sm, fm, cr, conn, router)
 	}
 }
 
@@ -79,13 +80,14 @@ func listenChat(cfg server.Config, fm *server.FeedbagStore, cr *server.ChatRegis
 
 	fmt.Printf("Chat server listening on %s\n", server.Address(cfg.OSCARHost, cfg.ChatPort))
 
+	router := server.NewRouterForChat()
 	for {
 		conn, err := listener.Accept()
 		if err != nil {
 			log.Println(err)
 			continue
 		}
-		go handleChatConnection(cfg, fm, cr, conn)
+		go handleChatConnection(cfg, fm, cr, conn, router)
 	}
 }
 
@@ -111,7 +113,7 @@ func handleAuthConnection(cfg server.Config, sm *server.InMemorySessionManager,
 	}
 }
 
-func handleBOSConnection(cfg server.Config, sm *server.InMemorySessionManager, fm *server.FeedbagStore, cr *server.ChatRegistry, conn net.Conn) {
+func handleBOSConnection(cfg server.Config, sm *server.InMemorySessionManager, fm *server.FeedbagStore, cr *server.ChatRegistry, conn net.Conn, router server.Router) {
 	sess, seq, err := server.VerifyLogin(sm, conn)
 	if err != nil {
 		fmt.Printf("user disconnected with error: %s\n", err.Error())
@@ -125,28 +127,7 @@ func handleBOSConnection(cfg server.Config, sm *server.InMemorySessionManager, f
 		conn.Close()
 	}()
 
-	onClientReady := func(sess *server.Session, sm server.SessionManager) error {
-		if err := server.NotifyArrival(sess, sm, fm); err != nil {
-			return err
-		}
-		buddies, err := fm.Buddies(sess.ScreenName)
-		if err != nil {
-			return err
-		}
-		for _, buddy := range buddies {
-			err := server.NotifyBuddyArrived(buddy, sess.ScreenName, sm)
-			switch {
-			case errors.Is(err, server.ErrSessNotFound):
-				continue
-			case err != nil:
-				return err
-			}
-		}
-		return nil
-	}
-
-	foodGroups := []uint16{0x0001, 0x0002, 0x0003, 0x0004, 0x0009, 0x0013, 0x000D}
-	if err := server.ReadBos(cfg, onClientReady, sess, seq, sm, fm, cr, conn, foodGroups); err != nil {
+	if err := server.ReadBos(cfg, sess, seq, sm, fm, cr, conn, server.ChatRoom{}, router); err != nil {
 		switch {
 		case errors.Is(io.EOF, err):
 			fallthrough
@@ -158,7 +139,7 @@ func handleBOSConnection(cfg server.Config, sm *server.InMemorySessionManager, f
 	}
 }
 
-func handleChatConnection(cfg server.Config, fm *server.FeedbagStore, cr *server.ChatRegistry, conn net.Conn) {
+func handleChatConnection(cfg server.Config, fm *server.FeedbagStore, cr *server.ChatRegistry, conn net.Conn, router server.Router) {
 	cookie, seq, err := server.VerifyChatLogin(conn)
 	if err != nil {
 		fmt.Printf("user disconnected with error: %s\n", err.Error())
@@ -186,16 +167,7 @@ func handleChatConnection(cfg server.Config, fm *server.FeedbagStore, cr *server
 		conn.Close()
 	}()
 
-	foodGroups := []uint16{0x0001, 0x0002, 0x0003, 0x0004, 0x0009, 0x0013, 0x000D, 0x000E}
-
-	onClientReady := func(sess *server.Session, sm server.SessionManager) error {
-		server.SendChatRoomInfoUpdate(sess, sm, room)
-		server.AlertUserJoined(sess, sm)
-		server.SetOnlineChatUsers(sess, sm)
-		return nil
-	}
-
-	if err := server.ReadBos(cfg, onClientReady, chatSess, seq, room.SessionManager, fm, cr, conn, foodGroups); err != nil {
+	if err := server.ReadBos(cfg, chatSess, seq, room.SessionManager, fm, cr, conn, room, router); err != nil {
 		if err != io.EOF {
 			fmt.Printf("user disconnected with error: %s\n", err.Error())
 		} else {

server/MockOServiceHandler.go

@@ -3,6 +3,8 @@
 package server
 
 import (
+	io "io"
+
 	oscar "github.com/mkaminski/goaim/oscar"
 	mock "github.com/stretchr/testify/mock"
 )
@@ -20,13 +22,13 @@ func (_m *MockOServiceHandler) EXPECT() *MockOServiceHandler_Expecter {
 	return &MockOServiceHandler_Expecter{mock: &_m.Mock}
 }
 
-// ClientOnlineHandler provides a mock function with given fields: snacPayloadIn, onReadyCB, sess, sm
-func (_m *MockOServiceHandler) ClientOnlineHandler(snacPayloadIn oscar.SNAC_0x01_0x02_OServiceClientOnline, onReadyCB OnReadyCB, sess *Session, sm SessionManager) error {
-	ret := _m.Called(snacPayloadIn, onReadyCB, sess, sm)
+// ClientOnlineHandler provides a mock function with given fields: snacPayloadIn, sess, sm, fm, room
+func (_m *MockOServiceHandler) ClientOnlineHandler(snacPayloadIn oscar.SNAC_0x01_0x02_OServiceClientOnline, sess *Session, sm SessionManager, fm FeedbagManager, room ChatRoom) error {
+	ret := _m.Called(snacPayloadIn, sess, sm, fm, room)
 
 	var r0 error
-	if rf, ok := ret.Get(0).(func(oscar.SNAC_0x01_0x02_OServiceClientOnline, OnReadyCB, *Session, SessionManager) error); ok {
-		r0 = rf(snacPayloadIn, onReadyCB, sess, sm)
+	if rf, ok := ret.Get(0).(func(oscar.SNAC_0x01_0x02_OServiceClientOnline, *Session, SessionManager, FeedbagManager, ChatRoom) error); ok {
+		r0 = rf(snacPayloadIn, sess, sm, fm, room)
 	} else {
 		r0 = ret.Error(0)
 	}
@@ -41,16 +43,17 @@ type MockOServiceHandler_ClientOnlineHandler_Call struct {
 
 // ClientOnlineHandler is a helper method to define mock.On call
 //   - snacPayloadIn oscar.SNAC_0x01_0x02_OServiceClientOnline
-//   - onReadyCB OnReadyCB
 //   - sess *Session
 //   - sm SessionManager
-func (_e *MockOServiceHandler_Expecter) ClientOnlineHandler(snacPayloadIn interface{}, onReadyCB interface{}, sess interface{}, sm interface{}) *MockOServiceHandler_ClientOnlineHandler_Call {
-	return &MockOServiceHandler_ClientOnlineHandler_Call{Call: _e.mock.On("ClientOnlineHandler", snacPayloadIn, onReadyCB, sess, sm)}
+//   - fm FeedbagManager
+//   - room ChatRoom
+func (_e *MockOServiceHandler_Expecter) ClientOnlineHandler(snacPayloadIn interface{}, sess interface{}, sm interface{}, fm interface{}, room interface{}) *MockOServiceHandler_ClientOnlineHandler_Call {
+	return &MockOServiceHandler_ClientOnlineHandler_Call{Call: _e.mock.On("ClientOnlineHandler", snacPayloadIn, sess, sm, fm, room)}
 }
 
-func (_c *MockOServiceHandler_ClientOnlineHandler_Call) Run(run func(snacPayloadIn oscar.SNAC_0x01_0x02_OServiceClientOnline, onReadyCB OnReadyCB, sess *Session, sm SessionManager)) *MockOServiceHandler_ClientOnlineHandler_Call {
+func (_c *MockOServiceHandler_ClientOnlineHandler_Call) Run(run func(snacPayloadIn oscar.SNAC_0x01_0x02_OServiceClientOnline, sess *Session, sm SessionManager, fm FeedbagManager, room ChatRoom)) *MockOServiceHandler_ClientOnlineHandler_Call {
 	_c.Call.Run(func(args mock.Arguments) {
-		run(args[0].(oscar.SNAC_0x01_0x02_OServiceClientOnline), args[1].(OnReadyCB), args[2].(*Session), args[3].(SessionManager))
+		run(args[0].(oscar.SNAC_0x01_0x02_OServiceClientOnline), args[1].(*Session), args[2].(SessionManager), args[3].(FeedbagManager), args[4].(ChatRoom))
 	})
 	return _c
 }
@@ -60,7 +63,7 @@ func (_c *MockOServiceHandler_ClientOnlineHandler_Call) Return(_a0 error) *MockO
 	return _c
 }
 
-func (_c *MockOServiceHandler_ClientOnlineHandler_Call) RunAndReturn(run func(oscar.SNAC_0x01_0x02_OServiceClientOnline, OnReadyCB, *Session, SessionManager) error) *MockOServiceHandler_ClientOnlineHandler_Call {
+func (_c *MockOServiceHandler_ClientOnlineHandler_Call) RunAndReturn(run func(oscar.SNAC_0x01_0x02_OServiceClientOnline, *Session, SessionManager, FeedbagManager, ChatRoom) error) *MockOServiceHandler_ClientOnlineHandler_Call {
 	_c.Call.Return(run)
 	return _c
 }
@@ -378,6 +381,49 @@ func (_c *MockOServiceHandler_UserInfoQueryHandler_Call) RunAndReturn(run func(*
 	return _c
 }
 
+// WriteOServiceHostOnline provides a mock function with given fields: w, sequence
+func (_m *MockOServiceHandler) WriteOServiceHostOnline(w io.Writer, sequence *uint32) error {
+	ret := _m.Called(w, sequence)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(io.Writer, *uint32) error); ok {
+		r0 = rf(w, sequence)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// MockOServiceHandler_WriteOServiceHostOnline_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'WriteOServiceHostOnline'
+type MockOServiceHandler_WriteOServiceHostOnline_Call struct {
+	*mock.Call
+}
+
+// WriteOServiceHostOnline is a helper method to define mock.On call
+//   - w io.Writer
+//   - sequence *uint32
+func (_e *MockOServiceHandler_Expecter) WriteOServiceHostOnline(w interface{}, sequence interface{}) *MockOServiceHandler_WriteOServiceHostOnline_Call {
+	return &MockOServiceHandler_WriteOServiceHostOnline_Call{Call: _e.mock.On("WriteOServiceHostOnline", w, sequence)}
+}
+
+func (_c *MockOServiceHandler_WriteOServiceHostOnline_Call) Run(run func(w io.Writer, sequence *uint32)) *MockOServiceHandler_WriteOServiceHostOnline_Call {
+	_c.Call.Run(func(args mock.Arguments) {
+		run(args[0].(io.Writer), args[1].(*uint32))
+	})
+	return _c
+}
+
+func (_c *MockOServiceHandler_WriteOServiceHostOnline_Call) Return(_a0 error) *MockOServiceHandler_WriteOServiceHostOnline_Call {
+	_c.Call.Return(_a0)
+	return _c
+}
+
+func (_c *MockOServiceHandler_WriteOServiceHostOnline_Call) RunAndReturn(run func(io.Writer, *uint32) error) *MockOServiceHandler_WriteOServiceHostOnline_Call {
+	_c.Call.Return(run)
+	return _c
+}
+
 // NewMockOServiceHandler creates a new instance of MockOServiceHandler. 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 NewMockOServiceHandler(t interface {

server/oservice.go

@@ -10,7 +10,8 @@ import (
 )
 
 type OServiceHandler interface {
-	ClientOnlineHandler(snacPayloadIn oscar.SNAC_0x01_0x02_OServiceClientOnline, onReadyCB OnReadyCB, sess *Session, sm SessionManager) error
+	WriteOServiceHostOnline(w io.Writer, sequence *uint32) error
+	ClientOnlineHandler(snacPayloadIn oscar.SNAC_0x01_0x02_OServiceClientOnline, sess *Session, sm SessionManager, fm FeedbagManager, room ChatRoom) error
 	ClientVersionsHandler(snacPayloadIn oscar.SNAC_0x01_0x17_OServiceClientVersions) XMessage
 	IdleNotificationHandler(sess *Session, sm SessionManager, fm *FeedbagStore, snacPayloadIn oscar.SNAC_0x01_0x11_OServiceIdleNotification) error
 	RateParamsQueryHandler() XMessage
@@ -30,14 +31,14 @@ type OServiceRouter struct {
 	OServiceHandler
 }
 
-func (rt OServiceRouter) RouteOService(cfg Config, ready OnReadyCB, cr *ChatRegistry, sm SessionManager, fm *FeedbagStore, sess *Session, SNACFrame oscar.SnacFrame, r io.Reader, w io.Writer, sequence *uint32) error {
+func (rt OServiceRouter) RouteOService(cfg Config, cr *ChatRegistry, sm SessionManager, fm *FeedbagStore, sess *Session, room ChatRoom, SNACFrame oscar.SnacFrame, r io.Reader, w io.Writer, sequence *uint32) error {
 	switch SNACFrame.SubGroup {
 	case oscar.OServiceClientOnline:
 		inSNAC := oscar.SNAC_0x01_0x02_OServiceClientOnline{}
 		if err := oscar.Unmarshal(&inSNAC, r); err != nil {
 			return err
 		}
-		return rt.ClientOnlineHandler(inSNAC, ready, sess, sm)
+		return rt.ClientOnlineHandler(inSNAC, sess, sm, fm, room)
 	case oscar.OServiceServiceRequest:
 		inSNAC := oscar.SNAC_0x01_0x04_OServiceServiceRequest{}
 		if err := oscar.Unmarshal(&inSNAC, r); err != nil {
@@ -95,14 +96,14 @@ func (rt OServiceRouter) RouteOService(cfg Config, ready OnReadyCB, cr *ChatRegi
 type OServiceService struct {
 }
 
-func WriteOServiceHostOnline(foodGroups []uint16, w io.Writer, sequence *uint32) error {
+func (s OServiceService) WriteOServiceHostOnline(w io.Writer, sequence *uint32) error {
 	fmt.Println("writeOServiceHostOnline...")
 	snacFrameOut := oscar.SnacFrame{
 		FoodGroup: OSERVICE,
 		SubGroup:  oscar.OServiceHostOnline,
 	}
 	snacPayloadOut := oscar.SNAC_0x01_0x03_OServiceHostOnline{
-		FoodGroups: foodGroups,
+		FoodGroups: []uint16{0x0001, 0x0002, 0x0003, 0x0004, 0x0009, 0x0013, 0x000D},
 	}
 	return writeOutSNAC(oscar.SnacFrame{}, snacFrameOut, snacPayloadOut, sequence, w)
 }
@@ -198,13 +199,27 @@ func (s OServiceService) UserInfoQueryHandler(sess *Session) XMessage {
 	}
 }
 
-type OnReadyCB func(sess *Session, sm SessionManager) error
-
-func (s OServiceService) ClientOnlineHandler(snacPayloadIn oscar.SNAC_0x01_0x02_OServiceClientOnline, onReadyCB OnReadyCB, sess *Session, sm SessionManager) error {
+func (s OServiceService) ClientOnlineHandler(snacPayloadIn oscar.SNAC_0x01_0x02_OServiceClientOnline, sess *Session, sm SessionManager, fm FeedbagManager, room ChatRoom) error {
 	for _, version := range snacPayloadIn.GroupVersions {
-		fmt.Printf("ClientOnlineHandler read SNAC client messageType: %+v\n", version)
+		fmt.Printf("hahaha ClientOnlineHandler read SNAC client messageType: %+v\n", version)
+	}
+	if err := NotifyArrival(sess, sm, fm); err != nil {
+		return err
+	}
+	buddies, err := fm.Buddies(sess.ScreenName)
+	if err != nil {
+		return err
+	}
+	for _, buddy := range buddies {
+		err := NotifyBuddyArrived(buddy, sess.ScreenName, sm)
+		switch {
+		case errors.Is(err, ErrSessNotFound):
+			continue
+		case err != nil:
+			return err
+		}
 	}
-	return onReadyCB(sess, sm)
+	return nil
 }
 
 func (s OServiceService) SetUserInfoFieldsHandler(sess *Session, sm SessionManager, fm *FeedbagStore, snacPayloadIn oscar.SNAC_0x01_0x1E_OServiceSetUserInfoFields) (XMessage, error) {
@@ -306,3 +321,35 @@ func (s OServiceService) ServiceRequestHandler(cfg Config, cr *ChatRegistry, ses
 // verify it's correctly unmarshalled.
 func (s OServiceService) RateParamsSubAddHandler(oscar.SNAC_0x01_0x08_OServiceRateParamsSubAdd) {
 }
+
+func NewOServiceRouterForChat() OServiceRouter {
+	return OServiceRouter{
+		OServiceHandler: OServiceServiceForChat{},
+	}
+}
+
+type OServiceServiceForChat struct {
+	OServiceService
+}
+
+func (s OServiceServiceForChat) WriteOServiceHostOnline(w io.Writer, sequence *uint32) error {
+	fmt.Println("writeOServiceHostOnline...")
+	snacFrameOut := oscar.SnacFrame{
+		FoodGroup: OSERVICE,
+		SubGroup:  oscar.OServiceHostOnline,
+	}
+	snacPayloadOut := oscar.SNAC_0x01_0x03_OServiceHostOnline{
+		FoodGroups: []uint16{0x0001, 0x0002, 0x0003, 0x0004, 0x0009, 0x0013, 0x000D, 0x000E},
+	}
+	return writeOutSNAC(oscar.SnacFrame{}, snacFrameOut, snacPayloadOut, sequence, w)
+}
+
+func (s OServiceServiceForChat) ClientOnlineHandler(snacPayloadIn oscar.SNAC_0x01_0x02_OServiceClientOnline, sess *Session, sm SessionManager, fm FeedbagManager, room ChatRoom) error {
+	for _, version := range snacPayloadIn.GroupVersions {
+		fmt.Printf("ClientOnlineHandler read SNAC client messageType: %+v\n", version)
+	}
+	SendChatRoomInfoUpdate(sess, sm, room)
+	AlertUserJoined(sess, sm)
+	SetOnlineChatUsers(sess, sm)
+	return nil
+}

server/oservice_test.go

@@ -410,7 +410,7 @@ func TestOServiceRouter_RouteOService(t *testing.T) {
 				Return(tc.output, tc.handlerErr).
 				Maybe()
 			svc.EXPECT().
-				ClientOnlineHandler(tc.input.snacOut, mock.Anything, mock.Anything, mock.Anything).
+				ClientOnlineHandler(tc.input.snacOut, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
 				Return(tc.handlerErr).
 				Maybe()
 			svc.EXPECT().
@@ -431,7 +431,7 @@ func TestOServiceRouter_RouteOService(t *testing.T) {
 			bufOut := &bytes.Buffer{}
 			seq := uint32(1)
 
-			err := router.RouteOService(Config{}, nil, nil, nil, nil, nil, tc.input.snacFrame, bufIn, bufOut, &seq)
+			err := router.RouteOService(Config{}, nil, nil, nil, nil, ChatRoom{}, tc.input.snacFrame, bufIn, bufOut, &seq)
 			assert.ErrorIs(t, err, tc.expectErr)
 			if tc.expectErr != nil {
 				return

server/protocol.go

@@ -272,8 +272,8 @@ func Signout(sess *Session, sm SessionManager, fm *FeedbagStore) {
 	sm.Remove(sess)
 }
 
-func ReadBos(cfg Config, ready OnReadyCB, sess *Session, seq uint32, sm SessionManager, fm *FeedbagStore, cr *ChatRegistry, rwc io.ReadWriter, foodGroups []uint16) error {
-	if err := WriteOServiceHostOnline(foodGroups, rwc, &seq); err != nil {
+func ReadBos(cfg Config, sess *Session, seq uint32, sm SessionManager, fm *FeedbagStore, cr *ChatRegistry, rwc io.ReadWriter, room ChatRoom, router Router) error {
+	if err := router.WriteOServiceHostOnline(rwc, &seq); err != nil {
 		return err
 	}
 
@@ -282,12 +282,10 @@ func ReadBos(cfg Config, ready OnReadyCB, sess *Session, seq uint32, sm SessionM
 	errCh := make(chan error, 1)
 	go readIncomingRequests(rwc, msgCh, errCh)
 
-	router := NewRouter()
-
 	for {
 		select {
 		case m := <-msgCh:
-			if err := router.routeIncomingRequests(cfg, ready, sm, sess, fm, cr, rwc, &seq, m.snac, m.buf); err != nil {
+			if err := router.routeIncomingRequests(cfg, sm, sess, fm, cr, rwc, &seq, m.snac, m.buf, room); err != nil {
 				switch {
 				case errors.Is(err, ErrUnsupportedSubGroup) || errors.Is(err, ErrUnsupportedFoodGroup):
 					if err := sendInvalidSNACErr(m.snac, rwc, &seq); err != nil {
@@ -323,6 +321,12 @@ func NewRouter() Router {
 	}
 }
 
+func NewRouterForChat() Router {
+	r := NewRouter()
+	r.OServiceRouter = NewOServiceRouterForChat()
+	return r
+}
+
 type Router struct {
 	ICBMRouter
 	LocateRouter
@@ -332,10 +336,10 @@ type Router struct {
 	ChatRouter
 }
 
-func (rt *Router) routeIncomingRequests(cfg Config, ready OnReadyCB, sm SessionManager, sess *Session, fm *FeedbagStore, cr *ChatRegistry, rw io.ReadWriter, sequence *uint32, snac oscar.SnacFrame, buf io.Reader) error {
+func (rt *Router) routeIncomingRequests(cfg Config, sm SessionManager, sess *Session, fm *FeedbagStore, cr *ChatRegistry, rw io.ReadWriter, sequence *uint32, snac oscar.SnacFrame, buf io.Reader, room ChatRoom) error {
 	switch snac.FoodGroup {
 	case OSERVICE:
-		return rt.RouteOService(cfg, ready, cr, sm, fm, sess, snac, buf, rw, sequence)
+		return rt.RouteOService(cfg, cr, sm, fm, sess, room, snac, buf, rw, sequence)
 	case LOCATE:
 		return rt.RouteLocate(sess, sm, fm, snac, buf, rw, sequence)
 	case BUDDY: