move buddy list population from OService.ClientOnline to Feedbag.Use

This is the first step towards supporting both client-side and
server-side buddy list management. Prior to this commit, the buddy list
is populated using the feedbag mechanism in OService.ClientOnline. This
logic is redundant and can cause confusion for older AIM clients that
use client-side buddy list management. Instead, we now populate the
buddy list using feedbag only when a feedbag-enabled client tells to via
the Feedbag.Use SNAC.

foodgroup/feedbag.go

@@ -337,3 +337,23 @@ func (s FeedbagService) DeleteItem(ctx context.Context, sess *state.Session, inF
 // correctly unmarshalled.
 func (s FeedbagService) StartCluster(context.Context, wire.SNACFrame, wire.SNAC_0x13_0x11_FeedbagStartCluster) {
 }
+
+// Use sends a user the contents of their buddy list. It's invoked at sign-on
+// by AIM clients that use the feedbag food group for buddy list management (as
+// opposed to client-side management).
+func (s FeedbagService) Use(ctx context.Context, sess *state.Session) error {
+	buddies, err := s.feedbagManager.Buddies(sess.ScreenName())
+	if err != nil {
+		return err
+	}
+	for _, screenName := range buddies {
+		buddy := s.messageRelayer.RetrieveByScreenName(screenName)
+		if buddy == nil || buddy.Invisible() {
+			continue
+		}
+		if err := unicastArrival(ctx, buddy, sess, s.messageRelayer, s.feedbagManager); err != nil {
+			return err
+		}
+	}
+	return nil
+}

foodgroup/feedbag_test.go

@@ -1495,3 +1495,120 @@ func TestFeedbagService_DeleteItem(t *testing.T) {
 		})
 	}
 }
+
+func TestFeedbagService_Use(t *testing.T) {
+	tests := []struct {
+		// name is the name of the test
+		name string
+		// joiningChatter is the session of the arriving user
+		sess *state.Session
+		// bodyIn is the SNAC body sent from the arriving user's client to the
+		// server
+		bodyIn wire.SNAC_0x01_0x02_OServiceClientOnline
+		// buddiesParams contains params for looking up arriving user's
+		// buddies
+		buddiesParams buddiesParams
+		// retrieveByScreenNameParams contains params for looking up the
+		// session for each of the arriving user's buddies
+		retrieveByScreenNameParams retrieveByScreenNameParams
+		// relayToScreenNameParams contains params for sending arrival
+		// notifications for each of the arriving user's buddies to the
+		// arriving user's client
+		relayToScreenNameParams relayToScreenNameParams
+		// feedbagParams contains params for retrieving a user's feedbag
+		feedbagParams feedbagParams
+		wantErr       error
+	}{
+		{
+			name:   "notify arriving user's buddies of its arrival and populate the arriving user's buddy list",
+			sess:   newTestSession("test-user"),
+			bodyIn: wire.SNAC_0x01_0x02_OServiceClientOnline{},
+			buddiesParams: buddiesParams{
+				{
+					screenName: "test-user",
+					results:    []string{"buddy1", "buddy3"},
+				},
+			},
+			retrieveByScreenNameParams: retrieveByScreenNameParams{
+				{
+					screenName: "buddy1",
+					sess:       newTestSession("buddy1"),
+				},
+				{
+					screenName: "buddy3",
+					sess:       newTestSession("buddy3"),
+				},
+			},
+			relayToScreenNameParams: relayToScreenNameParams{
+				{
+					screenName: "test-user",
+					message: wire.SNACMessage{
+						Frame: wire.SNACFrame{
+							FoodGroup: wire.Buddy,
+							SubGroup:  wire.BuddyArrived,
+						},
+						Body: wire.SNAC_0x03_0x0B_BuddyArrived{
+							TLVUserInfo: newTestSession("buddy1").TLVUserInfo(),
+						},
+					},
+				},
+				{
+					screenName: "test-user",
+					message: wire.SNACMessage{
+						Frame: wire.SNACFrame{
+							FoodGroup: wire.Buddy,
+							SubGroup:  wire.BuddyArrived,
+						},
+						Body: wire.SNAC_0x03_0x0B_BuddyArrived{
+							TLVUserInfo: newTestSession("buddy3").TLVUserInfo(),
+						},
+					},
+				},
+			},
+			feedbagParams: feedbagParams{
+				//{
+				//	screenName: "test-user",
+				//	results:    []wire.FeedbagItem{},
+				//},
+				{
+					screenName: "buddy1",
+					results:    []wire.FeedbagItem{},
+				},
+				{
+					screenName: "buddy3",
+					results:    []wire.FeedbagItem{},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			feedbagManager := newMockFeedbagManager(t)
+			messageRelayer := newMockMessageRelayer(t)
+			for _, params := range tt.buddiesParams {
+				feedbagManager.EXPECT().
+					Buddies(params.screenName).
+					Return(params.results, nil)
+			}
+			for _, params := range tt.retrieveByScreenNameParams {
+				messageRelayer.EXPECT().
+					RetrieveByScreenName(params.screenName).
+					Return(params.sess)
+			}
+			for _, params := range tt.relayToScreenNameParams {
+				messageRelayer.EXPECT().
+					RelayToScreenName(mock.Anything, params.screenName, params.message)
+			}
+			for _, params := range tt.feedbagParams {
+				feedbagManager.EXPECT().
+					Feedbag(params.screenName).
+					Return(params.results, nil)
+			}
+
+			svc := NewFeedbagService(slog.Default(), messageRelayer, feedbagManager, nil)
+
+			haveErr := svc.Use(nil, tt.sess)
+			assert.ErrorIs(t, tt.wantErr, haveErr)
+		})
+	}
+}

foodgroup/mock_feedbag_manager_test.go

@@ -3,6 +3,8 @@
 package foodgroup
 
 import (
+	context "context"
+
 	state "github.com/mk6i/retro-aim-server/state"
 	mock "github.com/stretchr/testify/mock"
 
@@ -405,6 +407,53 @@ func (_c *mockFeedbagManager_FeedbagUpsert_Call) RunAndReturn(run func(string, [
 	return _c
 }
 
+// Use provides a mock function with given fields: ctx, sess
+func (_m *mockFeedbagManager) Use(ctx context.Context, sess *state.Session) error {
+	ret := _m.Called(ctx, sess)
+
+	if len(ret) == 0 {
+		panic("no return value specified for Use")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, *state.Session) error); ok {
+		r0 = rf(ctx, sess)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// mockFeedbagManager_Use_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Use'
+type mockFeedbagManager_Use_Call struct {
+	*mock.Call
+}
+
+// Use is a helper method to define mock.On call
+//   - ctx context.Context
+//   - sess *state.Session
+func (_e *mockFeedbagManager_Expecter) Use(ctx interface{}, sess interface{}) *mockFeedbagManager_Use_Call {
+	return &mockFeedbagManager_Use_Call{Call: _e.mock.On("Use", ctx, sess)}
+}
+
+func (_c *mockFeedbagManager_Use_Call) Run(run func(ctx context.Context, sess *state.Session)) *mockFeedbagManager_Use_Call {
+	_c.Call.Run(func(args mock.Arguments) {
+		run(args[0].(context.Context), args[1].(*state.Session))
+	})
+	return _c
+}
+
+func (_c *mockFeedbagManager_Use_Call) Return(_a0 error) *mockFeedbagManager_Use_Call {
+	_c.Call.Return(_a0)
+	return _c
+}
+
+func (_c *mockFeedbagManager_Use_Call) RunAndReturn(run func(context.Context, *state.Session) error) *mockFeedbagManager_Use_Call {
+	_c.Call.Return(run)
+	return _c
+}
+
 // newMockFeedbagManager creates a new instance of mockFeedbagManager. 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 newMockFeedbagManager(t interface {

foodgroup/oservice.go

@@ -448,34 +448,10 @@ func (s OServiceServiceForBOS) HostOnline() wire.SNACMessage {
 }
 
 // ClientOnline runs when the current user is ready to join.
-// It performs the following sequence of actions:
-//   - Announce current user's arrival to users who have the current user on
-//     their buddy list.
-//   - Send current user its buddy list.
+// It announces current user's arrival to users who have the current user on
+// their buddy list.
 func (s OServiceServiceForBOS) ClientOnline(ctx context.Context, _ wire.SNAC_0x01_0x02_OServiceClientOnline, sess *state.Session) error {
-	if err := broadcastArrival(ctx, sess, s.messageRelayer, s.feedbagManager); err != nil {
-		return err
-	}
-
-	return s.retrieveOnlineBuddies(ctx, sess)
-}
-
-func (s OServiceServiceForBOS) retrieveOnlineBuddies(ctx context.Context, sess *state.Session) error {
-	buddies, err := s.feedbagManager.Buddies(sess.ScreenName())
-	if err != nil {
-		return err
-	}
-	for _, screenName := range buddies {
-		buddy := s.messageRelayer.RetrieveByScreenName(screenName)
-		if buddy == nil || buddy.Invisible() {
-			continue
-		}
-		if err := unicastArrival(ctx, buddy, sess, s.messageRelayer, s.feedbagManager); err != nil {
-			return err
-		}
-	}
-
-	return nil
+	return broadcastArrival(ctx, sess, s.messageRelayer, s.feedbagManager)
 }
 
 // NewOServiceServiceForChat creates a new instance of OServiceServiceForChat.

foodgroup/oservice_test.go

@@ -739,7 +739,7 @@ func TestOServiceServiceForBOS_ClientOnline(t *testing.T) {
 		// bodyIn is the SNAC body sent from the arriving user's client to the
 		// server
 		bodyIn wire.SNAC_0x01_0x02_OServiceClientOnline
-		// buddyLookupParams contains params for looking up arriving user's
+		// buddiesParams contains params for looking up arriving user's
 		// buddies
 		buddyLookupParams buddiesLookupParams
 		// adjacentUsersParams contains params for looking up users who have
@@ -752,16 +752,16 @@ func TestOServiceServiceForBOS_ClientOnline(t *testing.T) {
 		// retrieveByScreenNameParams contains params for looking up the
 		// session for each of the arriving user's buddies
 		retrieveByScreenNameParams retrieveByScreenNameParams
-		// sendToScreenNameParams contains params for sending arrival
+		// relayToScreenNameParams contains params for sending arrival
 		// notifications for each of the arriving user's buddies to the
 		// arriving user's client
-		sendToScreenNameParams relayToScreenNameParams
+		relayToScreenNameParams relayToScreenNameParams
 		// feedbagParams contains params for retrieving a user's feedbag
 		feedbagParams feedbagParams
 		wantErr       error
 	}{
 		{
-			name:   "notify arriving user's buddies of its arrival and populate the arriving user's buddy list",
+			name:   "notify arriving user's buddies of their arrival",
 			sess:   newTestSession("test-user"),
 			bodyIn: wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			interestedUsersParams: adjacentUsersParams{
@@ -784,61 +784,11 @@ func TestOServiceServiceForBOS_ClientOnline(t *testing.T) {
 					},
 				},
 			},
-			buddyLookupParams: buddiesLookupParams{
-				{
-					screenName: "test-user",
-					buddies:    []string{"buddy1", "buddy3"},
-				},
-			},
-			retrieveByScreenNameParams: retrieveByScreenNameParams{
-				{
-					screenName: "buddy1",
-					sess:       newTestSession("buddy1"),
-				},
-				{
-					screenName: "buddy3",
-					sess:       newTestSession("buddy3"),
-				},
-			},
-			sendToScreenNameParams: relayToScreenNameParams{
-				{
-					screenName: "test-user",
-					message: wire.SNACMessage{
-						Frame: wire.SNACFrame{
-							FoodGroup: wire.Buddy,
-							SubGroup:  wire.BuddyArrived,
-						},
-						Body: wire.SNAC_0x03_0x0B_BuddyArrived{
-							TLVUserInfo: newTestSession("buddy1").TLVUserInfo(),
-						},
-					},
-				},
-				{
-					screenName: "test-user",
-					message: wire.SNACMessage{
-						Frame: wire.SNACFrame{
-							FoodGroup: wire.Buddy,
-							SubGroup:  wire.BuddyArrived,
-						},
-						Body: wire.SNAC_0x03_0x0B_BuddyArrived{
-							TLVUserInfo: newTestSession("buddy3").TLVUserInfo(),
-						},
-					},
-				},
-			},
 			feedbagParams: feedbagParams{
 				{
 					screenName: "test-user",
 					results:    []wire.FeedbagItem{},
 				},
-				{
-					screenName: "buddy1",
-					results:    []wire.FeedbagItem{},
-				},
-				{
-					screenName: "buddy3",
-					results:    []wire.FeedbagItem{},
-				},
 			},
 		},
 	}
@@ -865,7 +815,7 @@ func TestOServiceServiceForBOS_ClientOnline(t *testing.T) {
 					RetrieveByScreenName(params.screenName).
 					Return(params.sess)
 			}
-			for _, params := range tt.sendToScreenNameParams {
+			for _, params := range tt.relayToScreenNameParams {
 				messageRelayer.EXPECT().
 					RelayToScreenName(mock.Anything, params.screenName, params.message)
 			}
@@ -920,7 +870,7 @@ func TestOServiceServiceForChat_ClientOnline(t *testing.T) {
 		// broadcastExcept contains params for broadcasting chat arrival to all
 		// chat participants except the user joining
 		broadcastExcept broadcastExcept
-		// sendToScreenNameParams contains params for sending chat room
+		// relayToScreenNameParams contains params for sending chat room
 		// metadata and chat participant list to joining user
 		sendToScreenNameParams sendToScreenNameParams
 		wantErr                error

server/oscar/handler/feedbag.go

@@ -19,6 +19,7 @@ type FeedbagService interface {
 	RightsQuery(ctx context.Context, inFrame wire.SNACFrame) wire.SNACMessage
 	StartCluster(ctx context.Context, inFrame wire.SNACFrame, inBody wire.SNAC_0x13_0x11_FeedbagStartCluster)
 	UpsertItem(ctx context.Context, sess *state.Session, inFrame wire.SNACFrame, items []wire.FeedbagItem) (wire.SNACMessage, error)
+	Use(ctx context.Context, sess *state.Session) error
 }
 
 func NewFeedbagHandler(logger *slog.Logger, feedbagService FeedbagService) FeedbagHandler {
@@ -67,9 +68,9 @@ func (h FeedbagHandler) QueryIfModified(ctx context.Context, sess *state.Session
 	return rw.SendSNAC(outSNAC.Frame, outSNAC.Body)
 }
 
-func (h FeedbagHandler) Use(ctx context.Context, _ *state.Session, inFrame wire.SNACFrame, _ io.Reader, _ oscar.ResponseWriter) error {
+func (h FeedbagHandler) Use(ctx context.Context, sess *state.Session, inFrame wire.SNACFrame, _ io.Reader, _ oscar.ResponseWriter) error {
 	h.LogRequest(ctx, inFrame, nil)
-	return nil
+	return h.FeedbagService.Use(ctx, sess)
 }
 
 func (h FeedbagHandler) InsertItem(ctx context.Context, sess *state.Session, inFrame wire.SNACFrame, r io.Reader, rw oscar.ResponseWriter) error {

server/oscar/handler/feedbag_test.go

@@ -333,6 +333,10 @@ func TestFeedbagHandler_Use(t *testing.T) {
 	}
 
 	svc := newMockFeedbagService(t)
+	svc.EXPECT().
+		Use(mock.Anything, mock.Anything).
+		Return(nil)
+
 	h := NewFeedbagHandler(slog.Default(), svc)
 	responseWriter := newMockResponseWriter(t)
 

server/oscar/handler/mock_feedbag_test.go

@@ -341,6 +341,53 @@ func (_c *mockFeedbagService_UpsertItem_Call) RunAndReturn(run func(context.Cont
 	return _c
 }
 
+// Use provides a mock function with given fields: ctx, sess
+func (_m *mockFeedbagService) Use(ctx context.Context, sess *state.Session) error {
+	ret := _m.Called(ctx, sess)
+
+	if len(ret) == 0 {
+		panic("no return value specified for Use")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, *state.Session) error); ok {
+		r0 = rf(ctx, sess)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// mockFeedbagService_Use_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Use'
+type mockFeedbagService_Use_Call struct {
+	*mock.Call
+}
+
+// Use is a helper method to define mock.On call
+//   - ctx context.Context
+//   - sess *state.Session
+func (_e *mockFeedbagService_Expecter) Use(ctx interface{}, sess interface{}) *mockFeedbagService_Use_Call {
+	return &mockFeedbagService_Use_Call{Call: _e.mock.On("Use", ctx, sess)}
+}
+
+func (_c *mockFeedbagService_Use_Call) Run(run func(ctx context.Context, sess *state.Session)) *mockFeedbagService_Use_Call {
+	_c.Call.Run(func(args mock.Arguments) {
+		run(args[0].(context.Context), args[1].(*state.Session))
+	})
+	return _c
+}
+
+func (_c *mockFeedbagService_Use_Call) Return(_a0 error) *mockFeedbagService_Use_Call {
+	_c.Call.Return(_a0)
+	return _c
+}
+
+func (_c *mockFeedbagService_Use_Call) RunAndReturn(run func(context.Context, *state.Session) error) *mockFeedbagService_Use_Call {
+	_c.Call.Return(run)
+	return _c
+}
+
 // newMockFeedbagService creates a new instance of mockFeedbagService. 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 newMockFeedbagService(t interface {