Quellcode durchsuchen

make buddy icon preview show upon upload on aim 6

- Send a user info update after setting icon
- Make sure user info updates all have BART_ID
- Store BART_ID in Session so we don't have to go DB every time
- Stop assuming every BART item is a buddy icon

Update BART handling in feedbag to check the item type and repsond
appropriately. Right now, just broadcast item update if buddy icon.

In next commit, build in support for multiple BART_IDs in user info
update (for AIM expressions, etc).
Mike vor 8 Monaten
Ursprung
Commit
fc49bc6218

+ 4 - 1
cmd/server/factory.go

@@ -233,6 +233,7 @@ func OSCAR(deps Container) *oscar.Server {
 		deps.hmacCookieBaker,
 		deps.chatSessionManager,
 		deps.sqLiteUserStore,
+		deps.sqLiteUserStore,
 		deps.rateLimitClasses,
 	)
 	bartService := foodgroup.NewBARTService(
@@ -335,7 +336,7 @@ func OSCAR(deps Container) *oscar.Server {
 // KerberosAPI creates an HTTP server for the Kerberos server.
 func KerberosAPI(deps Container) *kerberos.Server {
 	logger := deps.logger.With("svc", "Kerberos")
-	authService := foodgroup.NewAuthService(deps.cfg, deps.inMemorySessionManager, deps.inMemorySessionManager, deps.chatSessionManager, deps.sqLiteUserStore, deps.hmacCookieBaker, deps.chatSessionManager, deps.sqLiteUserStore, deps.rateLimitClasses)
+	authService := foodgroup.NewAuthService(deps.cfg, deps.inMemorySessionManager, deps.inMemorySessionManager, deps.chatSessionManager, deps.sqLiteUserStore, deps.hmacCookieBaker, deps.chatSessionManager, deps.sqLiteUserStore, deps.sqLiteUserStore, deps.rateLimitClasses)
 	return kerberos.NewKerberosServer(deps.Listeners, logger, authService)
 }
 
@@ -392,6 +393,7 @@ func TOC(deps Container) *toc.Server {
 				deps.hmacCookieBaker,
 				deps.chatSessionManager,
 				deps.sqLiteUserStore,
+				deps.sqLiteUserStore,
 				deps.rateLimitClasses,
 			),
 			BuddyListRegistry: deps.sqLiteUserStore,
@@ -489,6 +491,7 @@ func WebAPI(deps Container) *webapi.Server {
 			deps.hmacCookieBaker,
 			deps.chatSessionManager,
 			deps.sqLiteUserStore,
+			deps.sqLiteUserStore,
 			deps.rateLimitClasses,
 		),
 		BuddyListRegistry: deps.sqLiteUserStore,

+ 11 - 0
foodgroup/auth.go

@@ -26,6 +26,7 @@ func NewAuthService(
 	cookieBaker CookieBaker,
 	chatMessageRelayer ChatMessageRelayer,
 	accountManager AccountManager,
+	bartItemManager BARTItemManager,
 	classes wire.RateLimitClasses,
 ) *AuthService {
 	return &AuthService{
@@ -37,6 +38,7 @@ func NewAuthService(
 		userManager:         userManager,
 		chatMessageRelayer:  chatMessageRelayer,
 		accountManager:      accountManager,
+		bartItemManager:     bartItemManager,
 		rateLimitClasses:    classes,
 		timeNow:             time.Now,
 	}
@@ -54,6 +56,7 @@ type AuthService struct {
 	sessionRetriever    SessionRetriever
 	userManager         UserManager
 	accountManager      AccountManager
+	bartItemManager     BARTItemManager
 	rateLimitClasses    wire.RateLimitClasses
 	timeNow             func() time.Time
 }
@@ -127,6 +130,14 @@ func (s AuthService) RegisterBOSSession(ctx context.Context, serverCookie state.
 	sess.SetClientID(serverCookie.ClientID)
 	sess.SetMemberSince(time.Now())
 
+	bartID, err := s.bartItemManager.BuddyIconMetadata(ctx, sess.IdentScreenName())
+	if err != nil {
+		return nil, fmt.Errorf("BuddyIconMetadata: %w", err)
+	}
+	if bartID != nil {
+		sess.SetBuddyIcon(*bartID)
+	}
+
 	// indicate whether the client supports/wants multiple concurrent sessions
 	sess.SetMultiConnFlag(wire.MultiConnFlag(serverCookie.MultiConnFlag))
 

+ 51 - 7
foodgroup/auth_test.go

@@ -1568,7 +1568,7 @@ func TestAuthService_RegisterChatSession_HappyPath(t *testing.T) {
 	chatCookieBuf := &bytes.Buffer{}
 	assert.NoError(t, wire.MarshalBE(serverCookie, chatCookieBuf))
 
-	svc := NewAuthService(config.Config{}, nil, nil, chatSessionRegistry, nil, nil, nil, nil, wire.DefaultRateLimitClasses())
+	svc := NewAuthService(config.Config{}, nil, nil, chatSessionRegistry, nil, nil, nil, nil, nil, wire.DefaultRateLimitClasses())
 
 	have, err := svc.RegisterChatSession(context.Background(), serverCookie)
 	assert.NoError(t, err)
@@ -1629,9 +1629,31 @@ func TestAuthService_RegisterBOSSession(t *testing.T) {
 						},
 					},
 				},
+				bartItemManagerParams: bartItemManagerParams{
+					buddyIconMetadataParams: buddyIconMetadataParams{
+						{
+							screenName: screenName.IdentScreenName(),
+							result: &wire.BARTID{
+								Type: wire.BARTTypesBuddyIcon,
+								BARTInfo: wire.BARTInfo{
+									Flags: wire.BARTFlagsKnown,
+									Hash:  []byte{'m', 'y', 'i', 'c', 'o', 'n'},
+								},
+							},
+						},
+					},
+				},
 			},
 			wantSess: func(session *state.Session) bool {
-				return true
+				want := wire.BARTID{
+					Type: wire.BARTTypesBuddyIcon,
+					BARTInfo: wire.BARTInfo{
+						Flags: wire.BARTFlagsKnown,
+						Hash:  []byte{'m', 'y', 'i', 'c', 'o', 'n'},
+					},
+				}
+				has, hasIcon := session.BuddyIcon()
+				return assert.True(t, hasIcon) && assert.Equal(t, want, has)
 			},
 		},
 		{
@@ -1666,6 +1688,14 @@ func TestAuthService_RegisterBOSSession(t *testing.T) {
 						},
 					},
 				},
+				bartItemManagerParams: bartItemManagerParams{
+					buddyIconMetadataParams: buddyIconMetadataParams{
+						{
+							screenName: screenName.IdentScreenName(),
+							result:     nil,
+						},
+					},
+				},
 			},
 			wantSess: func(session *state.Session) bool {
 				return session.UserInfoBitmask()&wire.OServiceUserFlagBot == wire.OServiceUserFlagBot
@@ -1702,6 +1732,14 @@ func TestAuthService_RegisterBOSSession(t *testing.T) {
 						},
 					},
 				},
+				bartItemManagerParams: bartItemManagerParams{
+					buddyIconMetadataParams: buddyIconMetadataParams{
+						{
+							screenName: uin.IdentScreenName(),
+							result:     nil,
+						},
+					},
+				},
 			},
 			wantSess: func(sess *state.Session) bool {
 				uinMatches := fmt.Sprintf("%d", sess.UIN()) == uin.String()
@@ -1731,8 +1769,14 @@ func TestAuthService_RegisterBOSSession(t *testing.T) {
 					ConfirmStatus(matchContext(), params.screenName).
 					Return(params.confirmStatus, nil)
 			}
+			bartItemManager := newMockBARTItemManager(t)
+			for _, params := range tc.mockParams.buddyIconMetadataParams {
+				bartItemManager.EXPECT().
+					BuddyIconMetadata(matchContext(), params.screenName).
+					Return(params.result, params.err)
+			}
 
-			svc := NewAuthService(config.Config{}, sessionRegistry, nil, nil, userManager, nil, nil, accountManager, wire.DefaultRateLimitClasses())
+			svc := NewAuthService(config.Config{}, sessionRegistry, nil, nil, userManager, nil, nil, accountManager, bartItemManager, wire.DefaultRateLimitClasses())
 
 			have, err := svc.RegisterBOSSession(context.Background(), tc.cookie)
 			assert.NoError(t, err)
@@ -1762,7 +1806,7 @@ func TestAuthService_RetrieveBOSSession_HappyPath(t *testing.T) {
 		User(matchContext(), sess.IdentScreenName()).
 		Return(&state.User{IdentScreenName: sess.IdentScreenName()}, nil)
 
-	svc := NewAuthService(config.Config{}, nil, sessionRetriever, nil, userManager, nil, nil, nil, wire.DefaultRateLimitClasses())
+	svc := NewAuthService(config.Config{}, nil, sessionRetriever, nil, userManager, nil, nil, nil, nil, wire.DefaultRateLimitClasses())
 
 	have, err := svc.RetrieveBOSSession(context.Background(), aimAuthCookie)
 	assert.NoError(t, err)
@@ -1786,7 +1830,7 @@ func TestAuthService_RetrieveBOSSession_SessionNotFound(t *testing.T) {
 		User(matchContext(), sess.IdentScreenName()).
 		Return(&state.User{IdentScreenName: sess.IdentScreenName()}, nil)
 
-	svc := NewAuthService(config.Config{}, nil, sessionRetriever, nil, userManager, nil, nil, nil, wire.DefaultRateLimitClasses())
+	svc := NewAuthService(config.Config{}, nil, sessionRetriever, nil, userManager, nil, nil, nil, nil, wire.DefaultRateLimitClasses())
 
 	have, err := svc.RetrieveBOSSession(context.Background(), aimAuthCookie)
 	assert.NoError(t, err)
@@ -1879,7 +1923,7 @@ func TestAuthService_SignoutChat(t *testing.T) {
 					RemoveSession(matchSession(params.screenName))
 			}
 
-			svc := NewAuthService(config.Config{}, nil, nil, sessionManager, nil, nil, chatMessageRelayer, nil, wire.DefaultRateLimitClasses())
+			svc := NewAuthService(config.Config{}, nil, nil, sessionManager, nil, nil, chatMessageRelayer, nil, nil, wire.DefaultRateLimitClasses())
 			svc.SignoutChat(context.Background(), tt.userSession)
 		})
 	}
@@ -1924,7 +1968,7 @@ func TestAuthService_Signout(t *testing.T) {
 			for _, params := range tt.mockParams.removeSessionParams {
 				sessionManager.EXPECT().RemoveSession(matchSession(params.screenName))
 			}
-			svc := NewAuthService(config.Config{}, sessionManager, nil, nil, nil, nil, nil, nil, wire.DefaultRateLimitClasses())
+			svc := NewAuthService(config.Config{}, sessionManager, nil, nil, nil, nil, nil, nil, nil, wire.DefaultRateLimitClasses())
 
 			svc.Signout(context.Background(), tt.userSession)
 		})

+ 30 - 10
foodgroup/bart.go

@@ -1,6 +1,7 @@
 package foodgroup
 
 import (
+	"bytes"
 	"context"
 	"crypto/md5"
 	"errors"
@@ -29,6 +30,7 @@ func NewBARTService(
 	return BARTService{
 		bartItemManager:        bartItemManager,
 		buddyUpdateBroadcaster: newBuddyNotifier(bartItemManager, relationshipFetcher, messageRelayer, sessionRetriever),
+		messageRelayer:         messageRelayer,
 		logger:                 logger,
 	}
 }
@@ -36,6 +38,7 @@ func NewBARTService(
 type BARTService struct {
 	bartItemManager        BARTItemManager
 	buddyUpdateBroadcaster buddyBroadcaster
+	messageRelayer         MessageRelayer
 	logger                 *slog.Logger
 }
 
@@ -48,14 +51,37 @@ func (s BARTService) UpsertItem(ctx context.Context, sess *state.Session, inFram
 
 	if err := s.bartItemManager.InsertBARTItem(ctx, hash, inBody.Data, inBody.Type); err != nil {
 		if !errors.Is(err, state.ErrBARTItemExists) {
-			return wire.SNACMessage{}, err
+			return wire.SNACMessage{}, fmt.Errorf("failed to insert BART item: %w", err)
 		}
 	}
 
 	s.logger.DebugContext(ctx, "successfully uploaded BART item", "hash", fmt.Sprintf("%x", hash))
 
-	if err := s.buddyUpdateBroadcaster.BroadcastBuddyArrived(ctx, sess.IdentScreenName(), sess.TLVUserInfo()); err != nil {
-		return wire.SNACMessage{}, err
+	bartID, hasIcon := sess.BuddyIcon()
+	if hasIcon && bytes.Equal(hash, bartID.Hash) {
+		// unset unknown flag
+		bartID.Flags ^= wire.BARTFlagsUnknown
+		sess.SetBuddyIcon(bartID)
+
+		s.messageRelayer.RelayToScreenName(ctx, sess.IdentScreenName(), wire.SNACMessage{
+			Frame: wire.SNACFrame{
+				FoodGroup: wire.OService,
+				SubGroup:  wire.OServiceUserInfoUpdate,
+			},
+			Body: newOServiceUserInfoUpdate(sess),
+		})
+
+		if err := s.buddyUpdateBroadcaster.BroadcastBuddyArrived(ctx, sess.IdentScreenName(), sess.TLVUserInfo()); err != nil {
+			return wire.SNACMessage{}, err
+		}
+	} else {
+		bartID = wire.BARTID{
+			Type: inBody.Type,
+			BARTInfo: wire.BARTInfo{
+				Flags: wire.BARTFlagsCustom,
+				Hash:  hash,
+			},
+		}
 	}
 
 	return wire.SNACMessage{
@@ -66,13 +92,7 @@ func (s BARTService) UpsertItem(ctx context.Context, sess *state.Session, inFram
 		},
 		Body: wire.SNAC_0x10_0x03_BARTUploadReply{
 			Code: wire.BARTReplyCodesSuccess,
-			ID: wire.BARTID{
-				Type: inBody.Type,
-				BARTInfo: wire.BARTInfo{
-					Flags: wire.BARTFlagsKnown,
-					Hash:  hash,
-				},
-			},
+			ID:   bartID,
 		},
 	}, nil
 }

+ 267 - 15
foodgroup/bart_test.go

@@ -1,7 +1,9 @@
 package foodgroup
 
 import (
+	"bytes"
 	"context"
+	"io"
 	"log/slog"
 	"testing"
 
@@ -13,6 +15,9 @@ import (
 )
 
 func TestBARTService_UpsertItem(t *testing.T) {
+	itemHash := []byte{0x4e, 0xd9, 0xc1, 0x96, 0x45, 0xdb, 0x5a, 0xec, 0xdb, 0xf5, 0xc7, 0xa2, 0x4e, 0x8e, 0xa0, 0xed}
+	itemData := []byte{'i', 't', 'e', 'm', 'd', 'a', 't', 'a'}
+
 	cases := []struct {
 		// name is the unit test name
 		name string
@@ -25,26 +30,36 @@ func TestBARTService_UpsertItem(t *testing.T) {
 		mockParams mockParams
 		// expectOutput is the SNAC sent from the server to client
 		expectOutput wire.SNACMessage
+		// wantErr is the expected error
+		wantErr error
+		// sessionMatch verifies the session state after completion
+		sessionMatch func(session *state.Session)
 	}{
 		{
-			name:        "upsert item",
-			userSession: newTestSession("user_screen_name"),
+			name: "insert new buddy icon for current session",
+			userSession: newTestSession("user_screen_name", sessOptBuddyIcon(wire.BARTID{
+				Type: wire.BARTTypesBuddyIcon,
+				BARTInfo: wire.BARTInfo{
+					Flags: wire.BARTFlagsCustom | wire.BARTFlagsUnknown,
+					Hash:  itemHash,
+				},
+			})),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
 				},
 				Body: wire.SNAC_0x10_0x02_BARTUploadQuery{
-					Type: 1,
-					Data: []byte{'i', 't', 'e', 'm', 'd', 'a', 't', 'a'},
+					Type: wire.BARTTypesBuddyIcon,
+					Data: itemData,
 				},
 			},
 			mockParams: mockParams{
 				bartItemManagerParams: bartItemManagerParams{
 					bartItemManagerUpsertParams: bartItemManagerUpsertParams{
 						{
-							itemHash: []byte{0x4e, 0xd9, 0xc1, 0x96, 0x45, 0xdb, 0x5a, 0xec, 0xdb, 0xf5, 0xc7, 0xa2, 0x4e, 0x8e, 0xa0, 0xed},
-							payload:  []byte{'i', 't', 'e', 'm', 'd', 'a', 't', 'a'},
-							bartType: 1,
+							itemHash: itemHash,
+							payload:  itemData,
+							bartType: wire.BARTTypesBuddyIcon,
 						},
 					},
 				},
@@ -52,6 +67,35 @@ func TestBARTService_UpsertItem(t *testing.T) {
 					broadcastBuddyArrivedParams: broadcastBuddyArrivedParams{
 						{
 							screenName: state.DisplayScreenName("user_screen_name"),
+							bodyMatcher: func(tlvInfo wire.TLVUserInfo) bool {
+								bartID, exists := tlvInfo.Bytes(wire.OServiceUserInfoBARTInfo)
+								return exists &&
+									tlvInfo.ScreenName == "user_screen_name" &&
+									bytes.Contains(bartID, itemHash)
+							},
+						},
+					},
+				},
+				messageRelayerParams: messageRelayerParams{
+					relayToScreenNameParams: relayToScreenNameParams{
+						{
+							screenName: state.NewIdentScreenName("user_screen_name"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.OService,
+									SubGroup:  wire.OServiceUserInfoUpdate,
+								},
+								Body: func(val any) bool {
+									snac, ok := val.(wire.SNAC_0x01_0x0F_OServiceUserInfoUpdate)
+									if !ok {
+										return false
+									}
+									bartID, exists := snac.UserInfo[0].Bytes(wire.OServiceUserInfoBARTInfo)
+									return exists &&
+										snac.UserInfo[0].ScreenName == "user_screen_name" &&
+										bytes.Contains(bartID, itemHash)
+								},
+							},
 						},
 					},
 				},
@@ -67,12 +111,208 @@ func TestBARTService_UpsertItem(t *testing.T) {
 					ID: wire.BARTID{
 						Type: wire.BARTTypesBuddyIcon,
 						BARTInfo: wire.BARTInfo{
-							Flags: wire.BARTFlagsKnown,
-							Hash:  []byte{0x4e, 0xd9, 0xc1, 0x96, 0x45, 0xdb, 0x5a, 0xec, 0xdb, 0xf5, 0xc7, 0xa2, 0x4e, 0x8e, 0xa0, 0xed},
+							Flags: wire.BARTFlagsCustom | wire.BARTFlagsKnown,
+							Hash:  itemHash,
 						},
 					},
 				},
 			},
+			sessionMatch: func(session *state.Session) {
+				have, hasIcon := session.BuddyIcon()
+				assert.True(t, hasIcon)
+				want := wire.BARTID{
+					Type: wire.BARTTypesBuddyIcon,
+					BARTInfo: wire.BARTInfo{
+						Flags: wire.BARTFlagsCustom | wire.BARTFlagsKnown,
+						Hash:  itemHash,
+					},
+				}
+				assert.Equal(t, want, have)
+			},
+		},
+		{
+			name: "insert existing buddy icon for current session",
+			userSession: newTestSession("user_screen_name", sessOptBuddyIcon(wire.BARTID{
+				Type: wire.BARTTypesBuddyIcon,
+				BARTInfo: wire.BARTInfo{
+					Flags: wire.BARTFlagsCustom | wire.BARTFlagsUnknown,
+					Hash:  itemHash,
+				},
+			})),
+			inputSNAC: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					RequestID: 1234,
+				},
+				Body: wire.SNAC_0x10_0x02_BARTUploadQuery{
+					Type: wire.BARTTypesBuddyIcon,
+					Data: itemData,
+				},
+			},
+			mockParams: mockParams{
+				bartItemManagerParams: bartItemManagerParams{
+					bartItemManagerUpsertParams: bartItemManagerUpsertParams{
+						{
+							itemHash: itemHash,
+							payload:  itemData,
+							bartType: wire.BARTTypesBuddyIcon,
+							err:      state.ErrBARTItemExists,
+						},
+					},
+				},
+				buddyBroadcasterParams: buddyBroadcasterParams{
+					broadcastBuddyArrivedParams: broadcastBuddyArrivedParams{
+						{
+							screenName: state.DisplayScreenName("user_screen_name"),
+							bodyMatcher: func(tlvInfo wire.TLVUserInfo) bool {
+								bartID, exists := tlvInfo.Bytes(wire.OServiceUserInfoBARTInfo)
+								return exists &&
+									tlvInfo.ScreenName == "user_screen_name" &&
+									bytes.Contains(bartID, itemHash)
+							},
+						},
+					},
+				},
+				messageRelayerParams: messageRelayerParams{
+					relayToScreenNameParams: relayToScreenNameParams{
+						{
+							screenName: state.NewIdentScreenName("user_screen_name"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.OService,
+									SubGroup:  wire.OServiceUserInfoUpdate,
+								},
+								Body: func(val any) bool {
+									snac, ok := val.(wire.SNAC_0x01_0x0F_OServiceUserInfoUpdate)
+									if !ok {
+										return false
+									}
+									bartID, exists := snac.UserInfo[0].Bytes(wire.OServiceUserInfoBARTInfo)
+									return exists &&
+										snac.UserInfo[0].ScreenName == "user_screen_name" &&
+										bytes.Contains(bartID, itemHash)
+								},
+							},
+						},
+					},
+				},
+			},
+			expectOutput: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					FoodGroup: wire.BART,
+					SubGroup:  wire.BARTUploadReply,
+					RequestID: 1234,
+				},
+				Body: wire.SNAC_0x10_0x03_BARTUploadReply{
+					Code: wire.BARTReplyCodesSuccess,
+					ID: wire.BARTID{
+						Type: wire.BARTTypesBuddyIcon,
+						BARTInfo: wire.BARTInfo{
+							Flags: wire.BARTFlagsCustom | wire.BARTFlagsKnown,
+							Hash:  itemHash,
+						},
+					},
+				},
+			},
+			sessionMatch: func(session *state.Session) {
+				have, hasIcon := session.BuddyIcon()
+				assert.True(t, hasIcon)
+				want := wire.BARTID{
+					Type: wire.BARTTypesBuddyIcon,
+					BARTInfo: wire.BARTInfo{
+						Flags: wire.BARTFlagsCustom | wire.BARTFlagsKnown,
+						Hash:  itemHash,
+					},
+				}
+				assert.Equal(t, want, have)
+			},
+		},
+		{
+			name:        "insert new buddy icon, get insertion error",
+			userSession: newTestSession("user_screen_name"),
+			inputSNAC: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					RequestID: 1234,
+				},
+				Body: wire.SNAC_0x10_0x02_BARTUploadQuery{
+					Type: wire.BARTTypesBuddyIcon,
+					Data: itemData,
+				},
+			},
+			mockParams: mockParams{
+				bartItemManagerParams: bartItemManagerParams{
+					bartItemManagerUpsertParams: bartItemManagerUpsertParams{
+						{
+							itemHash: itemHash,
+							payload:  itemData,
+							bartType: wire.BARTTypesBuddyIcon,
+							err:      io.EOF,
+						},
+					},
+				},
+			},
+			expectOutput: wire.SNACMessage{},
+			wantErr:      io.EOF,
+		},
+		{
+			name: "insert new buddy icon unrelated to current session",
+			userSession: newTestSession("user_screen_name", sessOptBuddyIcon(wire.BARTID{
+				Type: wire.BARTTypesBuddyIcon,
+				BARTInfo: wire.BARTInfo{
+					Flags: wire.BARTFlagsCustom | wire.BARTFlagsKnown,
+					Hash:  []byte("unrelated icon"),
+				},
+			})),
+			inputSNAC: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					RequestID: 1234,
+				},
+				Body: wire.SNAC_0x10_0x02_BARTUploadQuery{
+					Type: wire.BARTTypesBuddyIcon,
+					Data: itemData,
+				},
+			},
+			mockParams: mockParams{
+				bartItemManagerParams: bartItemManagerParams{
+					bartItemManagerUpsertParams: bartItemManagerUpsertParams{
+						{
+							itemHash: itemHash,
+							payload:  itemData,
+							bartType: wire.BARTTypesBuddyIcon,
+						},
+					},
+				},
+				messageRelayerParams: messageRelayerParams{},
+			},
+			expectOutput: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					FoodGroup: wire.BART,
+					SubGroup:  wire.BARTUploadReply,
+					RequestID: 1234,
+				},
+				Body: wire.SNAC_0x10_0x03_BARTUploadReply{
+					Code: wire.BARTReplyCodesSuccess,
+					ID: wire.BARTID{
+						Type: wire.BARTTypesBuddyIcon,
+						BARTInfo: wire.BARTInfo{
+							Flags: wire.BARTFlagsCustom,
+							Hash:  itemHash,
+						},
+					},
+				},
+			},
+			sessionMatch: func(session *state.Session) {
+				// assert session icon didn't change
+				have, hasIcon := session.BuddyIcon()
+				assert.True(t, hasIcon)
+				want := wire.BARTID{
+					Type: wire.BARTTypesBuddyIcon,
+					BARTInfo: wire.BARTInfo{
+						Flags: wire.BARTFlagsCustom | wire.BARTFlagsKnown,
+						Hash:  []byte("unrelated icon"),
+					},
+				}
+				assert.Equal(t, want, have)
+			},
 		},
 	}
 
@@ -82,24 +322,36 @@ func TestBARTService_UpsertItem(t *testing.T) {
 			for _, params := range tc.mockParams.bartItemManagerUpsertParams {
 				bartItemManager.EXPECT().
 					InsertBARTItem(matchContext(), params.itemHash, params.payload, params.bartType).
-					Return(nil)
+					Return(params.err)
 			}
 			buddyUpdateBroadcaster := newMockbuddyBroadcaster(t)
 			for _, params := range tc.mockParams.broadcastBuddyArrivedParams {
 				buddyUpdateBroadcaster.EXPECT().
-					BroadcastBuddyArrived(mock.Anything, state.NewIdentScreenName(params.screenName.String()), mock.MatchedBy(func(userInfo wire.TLVUserInfo) bool {
-						return userInfo.ScreenName == params.screenName.String()
-					})).
+					BroadcastBuddyArrived(mock.Anything,
+						state.NewIdentScreenName(params.screenName.String()),
+						mock.MatchedBy(params.bodyMatcher)).
 					Return(params.err)
 			}
-			svc := NewBARTService(slog.Default(), bartItemManager, nil, nil, nil)
+			messageRelayer := newMockMessageRelayer(t)
+			for _, params := range tc.mockParams.relayToScreenNameParams {
+				messageRelayer.EXPECT().
+					RelayToScreenName(matchContext(), params.screenName, mock.MatchedBy(func(message wire.SNACMessage) bool {
+						return params.message.Frame == message.Frame &&
+							params.message.Body.(func(any) bool)(message.Body)
+					}))
+			}
+			svc := NewBARTService(slog.Default(), bartItemManager, messageRelayer, nil, nil)
 			svc.buddyUpdateBroadcaster = buddyUpdateBroadcaster
 
 			output, err := svc.UpsertItem(context.Background(), tc.userSession, tc.inputSNAC.Frame,
 				tc.inputSNAC.Body.(wire.SNAC_0x10_0x02_BARTUploadQuery))
 
-			assert.NoError(t, err)
+			assert.ErrorIs(t, err, tc.wantErr)
 			assert.Equal(t, output, tc.expectOutput)
+
+			if tc.sessionMatch != nil {
+				tc.sessionMatch(tc.userSession)
+			}
 		})
 	}
 }

+ 0 - 27
foodgroup/buddy.go

@@ -150,10 +150,6 @@ func (s buddyNotifier) BroadcastBuddyArrived(ctx context.Context, screenName sta
 		recipients = append(recipients, user.User)
 	}
 
-	if err := s.setBuddyIcon(ctx, screenName, &userInfo); err != nil {
-		return fmt.Errorf("failed to set buddy icon for %s: %w", screenName.String(), err)
-	}
-
 	s.messageRelayer.RelayToScreenNames(ctx, recipients, wire.SNACMessage{
 		Frame: wire.SNACFrame{
 			FoodGroup: wire.Buddy,
@@ -236,7 +232,6 @@ func (s buddyNotifier) BroadcastVisibility(
 		return fmt.Errorf("retrieving relationships: %w", err)
 	}
 
-	buddyIconSet := false
 	yourTLVInfo := you.TLVUserInfo()
 
 	for _, relationship := range relationships {
@@ -251,21 +246,11 @@ func (s buddyNotifier) BroadcastVisibility(
 
 		if !relationship.YouBlock {
 			if relationship.IsOnTheirList {
-				if !buddyIconSet {
-					// lazy load your buddy icon
-					if err := s.setBuddyIcon(ctx, you.IdentScreenName(), &yourTLVInfo); err != nil {
-						return fmt.Errorf("failed to set buddy icon for %s: %w", you.IdentScreenName().String(), err)
-					}
-					buddyIconSet = true
-				}
 				// tell them you're online
 				s.unicastBuddyArrived(ctx, yourTLVInfo, theirSess.IdentScreenName())
 			}
 			if relationship.IsOnYourList {
 				theirInfo := theirSess.TLVUserInfo()
-				if err := s.setBuddyIcon(ctx, theirSess.IdentScreenName(), &theirInfo); err != nil {
-					return fmt.Errorf("failed to set buddy icon for %s: %w", you.IdentScreenName().String(), err)
-				}
 				// tell you they're online
 				s.unicastBuddyArrived(ctx, theirInfo, you.IdentScreenName())
 			}
@@ -284,18 +269,6 @@ func (s buddyNotifier) BroadcastVisibility(
 	return nil
 }
 
-// setBuddyIcon adds buddy icon metadata to TLV user info
-func (s buddyNotifier) setBuddyIcon(ctx context.Context, you state.IdentScreenName, myInfo *wire.TLVUserInfo) error {
-	icon, err := s.bartItemManager.BuddyIconMetadata(ctx, you)
-	if err != nil {
-		return fmt.Errorf("retrieve buddy icon ref: %v", err)
-	}
-	if icon != nil {
-		myInfo.Append(wire.NewTLVBE(wire.OServiceUserInfoBARTInfo, *icon))
-	}
-	return nil
-}
-
 func (s buddyNotifier) unicastBuddyDeparted(ctx context.Context, from *state.Session, to state.IdentScreenName) {
 	s.messageRelayer.RelayToScreenName(ctx, to, wire.SNACMessage{
 		Frame: wire.SNACFrame{

+ 49 - 195
foodgroup/buddy_test.go

@@ -4,12 +4,11 @@ import (
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 
 	"github.com/mk6i/retro-aim-server/state"
 	"github.com/mk6i/retro-aim-server/wire"
-
-	"github.com/stretchr/testify/assert"
 )
 
 func TestBuddyService_RightsQuery(t *testing.T) {
@@ -234,30 +233,19 @@ func TestBuddyNotifier_BroadcastBuddyArrived(t *testing.T) {
 	cases := []struct {
 		// name is the unit test name
 		name string
-		// sourceSession is the session of the user
-		userSession *state.Session
+		// screenName is the user screen name
+		screenName state.IdentScreenName
+		// userInfo is the user info passed to BroadcastBuddyArrived
+		userInfo wire.TLVUserInfo
 		// mockParams is the list of params sent to mocks that satisfy this
 		// method's dependencies
 		mockParams mockParams
 	}{
 		{
-			name:        "happy path",
-			userSession: newTestSession("me"),
+			name:       "happy path",
+			screenName: state.NewIdentScreenName("me"),
+			userInfo:   wire.TLVUserInfo{ScreenName: "me"},
 			mockParams: mockParams{
-				bartItemManagerParams: bartItemManagerParams{
-					buddyIconMetadataParams: buddyIconMetadataParams{
-						{
-							screenName: state.NewIdentScreenName("me"),
-							result: &wire.BARTID{
-								Type: wire.BARTTypesBuddyIcon,
-								BARTInfo: wire.BARTInfo{
-									Flags: wire.BARTFlagsKnown,
-									Hash:  []byte{'m', 'y', 'i', 'c', 'o', 'n'},
-								},
-							},
-						},
-					},
-				},
 				relationshipFetcherParams: relationshipFetcherParams{
 					allRelationshipsParams: allRelationshipsParams{
 						{
@@ -310,16 +298,16 @@ func TestBuddyNotifier_BroadcastBuddyArrived(t *testing.T) {
 								state.NewIdentScreenName("friend1-visible"),
 								state.NewIdentScreenName("friend2-visible"),
 							},
-							message: newBuddyArrivedNotif(userInfoWithBARTIcon(
-								newTestSession("me"),
-								wire.BARTID{
-									Type: wire.BARTTypesBuddyIcon,
-									BARTInfo: wire.BARTInfo{
-										Flags: wire.BARTFlagsKnown,
-										Hash:  []byte{'m', 'y', 'i', 'c', 'o', 'n'},
-									},
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.Buddy,
+									SubGroup:  wire.BuddyArrived,
+									RequestID: wire.ReqIDFromServer,
 								},
-							)),
+								Body: wire.SNAC_0x03_0x0B_BuddyArrived{
+									TLVUserInfo: wire.TLVUserInfo{ScreenName: "me"},
+								},
+							},
 						},
 					},
 				},
@@ -335,12 +323,6 @@ func TestBuddyNotifier_BroadcastBuddyArrived(t *testing.T) {
 					AllRelationships(matchContext(), params.screenName, params.filter).
 					Return(params.result, params.err)
 			}
-			bartItemManager := newMockBARTItemManager(t)
-			for _, params := range tc.mockParams.buddyIconMetadataParams {
-				bartItemManager.EXPECT().
-					BuddyIconMetadata(matchContext(), params.screenName).
-					Return(params.result, params.err)
-			}
 			messageRelayer := newMockMessageRelayer(t)
 			for _, params := range tc.mockParams.relayToScreenNamesParams {
 				messageRelayer.EXPECT().
@@ -348,12 +330,11 @@ func TestBuddyNotifier_BroadcastBuddyArrived(t *testing.T) {
 			}
 
 			svc := buddyNotifier{
-				bartItemManager:     bartItemManager,
 				relationshipFetcher: relationshipFetcher,
 				messageRelayer:      messageRelayer,
 			}
 
-			err := svc.BroadcastBuddyArrived(context.Background(), tc.userSession.IdentScreenName(), tc.userSession.TLVUserInfo())
+			err := svc.BroadcastBuddyArrived(context.Background(), tc.screenName, tc.userInfo)
 			assert.NoError(t, err)
 		})
 	}
@@ -458,19 +439,11 @@ func TestBuddyService_BroadcastDeparture(t *testing.T) {
 					AllRelationships(matchContext(), params.screenName, params.filter).
 					Return(params.result, params.err)
 			}
-			bartItemManager := newMockBARTItemManager(t)
-			for _, params := range tc.mockParams.buddyIconMetadataParams {
-				bartItemManager.EXPECT().
-					BuddyIconMetadata(matchContext(), params.screenName).
-					Return(params.result, params.err)
-			}
-
 			messageRelayer := newMockMessageRelayer(t)
 			for _, params := range tc.mockParams.relayToScreenNamesParams {
 				messageRelayer.EXPECT().
-					RelayToScreenNames(mock.Anything, params.screenNames, params.message)
+					RelayToScreenNames(matchContext(), params.screenNames, params.message)
 			}
-
 			svc := buddyNotifier{
 				relationshipFetcher: relationshipFetcher,
 				messageRelayer:      messageRelayer,
@@ -500,22 +473,6 @@ func Test_buddyNotifier_BroadcastVisibility(t *testing.T) {
 			name:        "happy path",
 			userSession: newTestSession("me"),
 			mockParams: mockParams{
-				bartItemManagerParams: bartItemManagerParams{
-					buddyIconMetadataParams: buddyIconMetadataParams{
-						{
-							screenName: state.NewIdentScreenName("me"),
-							result:     nil,
-						},
-						{
-							screenName: state.NewIdentScreenName("friend3-visible-on-your-list"),
-							result:     nil,
-						},
-						{
-							screenName: state.NewIdentScreenName("friend4-visible-on-both-lists"),
-							result:     nil,
-						},
-					},
-				},
 				relationshipFetcherParams: relationshipFetcherParams{
 					allRelationshipsParams: allRelationshipsParams{
 						{
@@ -586,35 +543,35 @@ func Test_buddyNotifier_BroadcastVisibility(t *testing.T) {
 					relayToScreenNameParams: relayToScreenNameParams{
 						{
 							screenName: state.NewIdentScreenName("friend2-visible-on-their-list"),
-							message:    newBuddyArrivedNotif(newTestSession("me").TLVUserInfo()),
+							message:    newBuddyArrivedNotif("me"),
 						},
 						{
 							screenName: state.NewIdentScreenName("me"),
-							message:    newBuddyArrivedNotif(newTestSession("friend3-visible-on-your-list").TLVUserInfo()),
+							message:    newBuddyArrivedNotif("friend3-visible-on-your-list"),
 						},
 						{
 							screenName: state.NewIdentScreenName("friend4-visible-on-both-lists"),
-							message:    newBuddyArrivedNotif(newTestSession("me").TLVUserInfo()),
+							message:    newBuddyArrivedNotif("me"),
 						},
 						{
 							screenName: state.NewIdentScreenName("me"),
-							message:    newBuddyArrivedNotif(newTestSession("friend4-visible-on-both-lists").TLVUserInfo()),
+							message:    newBuddyArrivedNotif("friend4-visible-on-both-lists"),
 						},
 						{
 							screenName: state.NewIdentScreenName("friend5-blocked-on-their-list"),
-							message:    newBuddyDepartedNotif(newTestSession("me")),
+							message:    newBuddyDepartedNotif("me"),
 						},
 						{
 							screenName: state.NewIdentScreenName("me"),
-							message:    newBuddyDepartedNotif(newTestSession("friend6-blocked-on-your-list")),
+							message:    newBuddyDepartedNotif("friend6-blocked-on-your-list"),
 						},
 						{
 							screenName: state.NewIdentScreenName("me"),
-							message:    newBuddyDepartedNotif(newTestSession("friend7-blocked-on-both-lists")),
+							message:    newBuddyDepartedNotif("friend7-blocked-on-both-lists"),
 						},
 						{
 							screenName: state.NewIdentScreenName("friend7-blocked-on-both-lists"),
-							message:    newBuddyDepartedNotif(newTestSession("me")),
+							message:    newBuddyDepartedNotif("me"),
 						},
 					},
 				},
@@ -657,22 +614,6 @@ func Test_buddyNotifier_BroadcastVisibility(t *testing.T) {
 			name:        "don't send departure notifications",
 			userSession: newTestSession("me"),
 			mockParams: mockParams{
-				bartItemManagerParams: bartItemManagerParams{
-					buddyIconMetadataParams: buddyIconMetadataParams{
-						{
-							screenName: state.NewIdentScreenName("me"),
-							result:     nil,
-						},
-						{
-							screenName: state.NewIdentScreenName("friend3-visible-on-your-list"),
-							result:     nil,
-						},
-						{
-							screenName: state.NewIdentScreenName("friend4-visible-on-both-lists"),
-							result:     nil,
-						},
-					},
-				},
 				relationshipFetcherParams: relationshipFetcherParams{
 					allRelationshipsParams: allRelationshipsParams{
 						{
@@ -715,19 +656,19 @@ func Test_buddyNotifier_BroadcastVisibility(t *testing.T) {
 					relayToScreenNameParams: relayToScreenNameParams{
 						{
 							screenName: state.NewIdentScreenName("friend2-visible-on-their-list"),
-							message:    newBuddyArrivedNotif(newTestSession("me").TLVUserInfo()),
+							message:    newBuddyArrivedNotif("me"),
 						},
 						{
 							screenName: state.NewIdentScreenName("me"),
-							message:    newBuddyArrivedNotif(newTestSession("friend3-visible-on-your-list").TLVUserInfo()),
+							message:    newBuddyArrivedNotif("friend3-visible-on-your-list"),
 						},
 						{
 							screenName: state.NewIdentScreenName("friend4-visible-on-both-lists"),
-							message:    newBuddyArrivedNotif(newTestSession("me").TLVUserInfo()),
+							message:    newBuddyArrivedNotif("me"),
 						},
 						{
 							screenName: state.NewIdentScreenName("me"),
-							message:    newBuddyArrivedNotif(newTestSession("friend4-visible-on-both-lists").TLVUserInfo()),
+							message:    newBuddyArrivedNotif("friend4-visible-on-both-lists"),
 						},
 					},
 				},
@@ -754,92 +695,6 @@ func Test_buddyNotifier_BroadcastVisibility(t *testing.T) {
 			},
 			doSendDepartures: false,
 		},
-		{
-			name:        "users have buddy icons",
-			userSession: newTestSession("me"),
-			mockParams: mockParams{
-				bartItemManagerParams: bartItemManagerParams{
-					buddyIconMetadataParams: buddyIconMetadataParams{
-						{
-							screenName: state.NewIdentScreenName("me"),
-							result: &wire.BARTID{
-								Type: wire.BARTTypesBuddyIcon,
-								BARTInfo: wire.BARTInfo{
-									Flags: wire.BARTFlagsKnown,
-									Hash:  []byte{'m', 'y', 'i', 'c', 'o', 'n'},
-								},
-							},
-						},
-						{
-							screenName: state.NewIdentScreenName("friend-visible-on-both-lists"),
-							result: &wire.BARTID{
-								Type: wire.BARTTypesBuddyIcon,
-								BARTInfo: wire.BARTInfo{
-									Flags: wire.BARTFlagsKnown,
-									Hash:  []byte{'t', 'h', 'e', 'i', 'r', 'i', 'c', 'o', 'n'},
-								},
-							},
-						},
-					},
-				},
-				relationshipFetcherParams: relationshipFetcherParams{
-					allRelationshipsParams: allRelationshipsParams{
-						{
-							screenName: state.NewIdentScreenName("me"),
-							filter:     nil,
-							result: []state.Relationship{
-								{
-									User:          state.NewIdentScreenName("friend-visible-on-both-lists"),
-									BlocksYou:     false,
-									YouBlock:      false,
-									IsOnYourList:  true,
-									IsOnTheirList: true,
-								},
-							},
-						},
-					},
-				},
-				messageRelayerParams: messageRelayerParams{
-					relayToScreenNameParams: relayToScreenNameParams{
-						{
-							screenName: state.NewIdentScreenName("friend-visible-on-both-lists"),
-							message: newBuddyArrivedNotif(userInfoWithBARTIcon(
-								newTestSession("me"),
-								wire.BARTID{
-									Type: wire.BARTTypesBuddyIcon,
-									BARTInfo: wire.BARTInfo{
-										Flags: wire.BARTFlagsKnown,
-										Hash:  []byte{'m', 'y', 'i', 'c', 'o', 'n'},
-									},
-								},
-							)),
-						},
-						{
-							screenName: state.NewIdentScreenName("me"),
-							message: newBuddyArrivedNotif(userInfoWithBARTIcon(
-								newTestSession("friend-visible-on-both-lists"),
-								wire.BARTID{
-									Type: wire.BARTTypesBuddyIcon,
-									BARTInfo: wire.BARTInfo{
-										Flags: wire.BARTFlagsKnown,
-										Hash:  []byte{'t', 'h', 'e', 'i', 'r', 'i', 'c', 'o', 'n'},
-									},
-								},
-							)),
-						},
-					},
-				},
-				sessionRetrieverParams: sessionRetrieverParams{
-					retrieveSessionParams: retrieveSessionParams{
-						{
-							screenName: state.NewIdentScreenName("friend-visible-on-both-lists"),
-							result:     newTestSession("friend-visible-on-both-lists"),
-						},
-					},
-				},
-			},
-			doSendDepartures: true,
-		},
 	}
 
 	for _, tc := range cases {
@@ -850,16 +705,13 @@ func Test_buddyNotifier_BroadcastVisibility(t *testing.T) {
 					AllRelationships(matchContext(), params.screenName, params.filter).
 					Return(params.result, params.err)
 			}
-			bartItemManager := newMockBARTItemManager(t)
-			for _, params := range tc.mockParams.buddyIconMetadataParams {
-				bartItemManager.EXPECT().
-					BuddyIconMetadata(matchContext(), params.screenName).
-					Return(params.result, params.err)
-			}
 			messageRelayer := newMockMessageRelayer(t)
 			for _, params := range tc.mockParams.relayToScreenNameParams {
 				messageRelayer.EXPECT().
-					RelayToScreenName(mock.Anything, params.screenName, params.message)
+					RelayToScreenName(matchContext(), params.screenName, mock.MatchedBy(func(message wire.SNACMessage) bool {
+						return params.message.Frame == message.Frame &&
+							params.message.Body.(func(any) bool)(message.Body)
+					}))
 			}
 			sessionRetriever := newMockSessionRetriever(t)
 			for _, params := range tc.mockParams.retrieveSessionParams {
@@ -869,7 +721,6 @@ func Test_buddyNotifier_BroadcastVisibility(t *testing.T) {
 			}
 
 			svc := buddyNotifier{
-				bartItemManager:     bartItemManager,
 				relationshipFetcher: relationshipFetcher,
 				messageRelayer:      messageRelayer,
 				sessionRetriever:    sessionRetriever,
@@ -881,33 +732,36 @@ func Test_buddyNotifier_BroadcastVisibility(t *testing.T) {
 	}
 }
 
-func newBuddyDepartedNotif(me *state.Session) wire.SNACMessage {
+func newBuddyDepartedNotif(screenName state.DisplayScreenName) wire.SNACMessage {
 	return wire.SNACMessage{
 		Frame: wire.SNACFrame{
 			FoodGroup: wire.Buddy,
 			SubGroup:  wire.BuddyDeparted,
 			RequestID: wire.ReqIDFromServer,
 		},
-		Body: wire.SNAC_0x03_0x0C_BuddyDeparted{
-			TLVUserInfo: wire.TLVUserInfo{
-				// don't include the TLV block, otherwise the AIM client fails
-				// to process the block event
-				ScreenName:   me.IdentScreenName().String(),
-				WarningLevel: me.Warning(),
-			},
+		Body: func(val any) bool {
+			snac, ok := val.(wire.SNAC_0x03_0x0C_BuddyDeparted)
+			if !ok {
+				return false
+			}
+			return snac.ScreenName == screenName.String()
 		},
 	}
 }
 
-func newBuddyArrivedNotif(userInfo wire.TLVUserInfo) wire.SNACMessage {
+func newBuddyArrivedNotif(screenName state.DisplayScreenName) wire.SNACMessage {
 	return wire.SNACMessage{
 		Frame: wire.SNACFrame{
 			FoodGroup: wire.Buddy,
 			SubGroup:  wire.BuddyArrived,
 			RequestID: wire.ReqIDFromServer,
 		},
-		Body: wire.SNAC_0x03_0x0B_BuddyArrived{
-			TLVUserInfo: userInfo,
+		Body: func(val any) bool {
+			snac, ok := val.(wire.SNAC_0x03_0x0B_BuddyArrived)
+			if !ok {
+				return false
+			}
+			return snac.ScreenName == screenName.String() && len(snac.TLVUserInfo.TLVList) > 0
 		},
 	}
 }

+ 54 - 31
foodgroup/feedbag.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"strconv"
 	"time"
 
 	"github.com/mk6i/retro-aim-server/state"
@@ -205,7 +206,7 @@ func (s FeedbagService) UpsertItem(ctx context.Context, sess *state.Session, inF
 		case wire.FeedbagClassIdBuddy, wire.FeedbagClassIDPermit, wire.FeedbagClassIDDeny:
 			filter = append(filter, state.NewIdentScreenName(item.Name))
 		case wire.FeedbagClassIdBart:
-			if err := s.broadcastIconUpdate(ctx, sess, item); err != nil {
+			if err := s.setBARTItem(ctx, sess, item); err != nil {
 				return wire.SNACMessage{}, err
 			}
 		case wire.FeedbagClassIdPdinfo:
@@ -234,47 +235,59 @@ func (s FeedbagService) UpsertItem(ctx context.Context, sess *state.Session, inF
 	}, nil
 }
 
-// broadcastIconUpdate informs clients about buddy icon update. If the BART
+// setBARTItem informs clients about buddy icon update. If the BART
 // store doesn't have the icon, then tell the client to upload the buddy icon.
 // If the icon already exists, tell the user's buddies about the icon change.
-func (s FeedbagService) broadcastIconUpdate(ctx context.Context, sess *state.Session, item wire.FeedbagItem) error {
-	btlv := wire.BARTInfo{}
-	if b, hasBuf := item.Bytes(wire.FeedbagAttributesBartInfo); hasBuf {
-		if err := wire.UnmarshalBE(&btlv, bytes.NewBuffer(b)); err != nil {
-			return err
-		}
-	} else {
+func (s FeedbagService) setBARTItem(ctx context.Context, sess *state.Session, item wire.FeedbagItem) error {
+	b, hasBuf := item.Bytes(wire.FeedbagAttributesBartInfo)
+	if !hasBuf {
 		return errors.New("unable to extract icon payload")
 	}
 
-	if bytes.Equal(btlv.Hash, wire.GetClearIconHash()) {
-		s.logger.DebugContext(ctx, "user is clearing icon",
-			"hash", fmt.Sprintf("%x", btlv.Hash))
-		// tell buddies about the icon update
-		return s.buddyBroadcaster.BroadcastBuddyArrived(ctx, sess.IdentScreenName(), sess.TLVUserInfo())
+	itemType, err := strconv.ParseUint(item.Name, 0, 16)
+	if err != nil {
+		return fmt.Errorf("invalid BART item type %q: %w", item.Name, err)
 	}
 
-	bid := wire.BARTID{
-		Type: wire.BARTTypesBuddyIcon,
-		BARTInfo: wire.BARTInfo{
-			Flags: wire.BARTFlagsCustom,
-			Hash:  btlv.Hash,
-		},
+	bartID := wire.BARTID{
+		Type: uint16(itemType),
 	}
-	if b, err := s.bartItemManager.BARTItem(ctx, btlv.Hash); err != nil {
+	if err := wire.UnmarshalBE(&bartID.BARTInfo, bytes.NewBuffer(b)); err != nil {
 		return err
-	} else if len(b) == 0 {
-		// icon doesn't exist, tell the client to upload buddy icon
-		s.logger.DebugContext(ctx, "icon doesn't exist in BART store, client must upload the icon file",
-			"hash", fmt.Sprintf("%x", btlv.Hash))
-		bid.Flags |= wire.BARTFlagsUnknown
+	}
+
+	itemExists := false
+
+	if bytes.Equal(bartID.Hash, wire.GetClearIconHash()) {
+		s.logger.DebugContext(ctx, "user is clearing icon",
+			"hash", fmt.Sprintf("%x", bartID.Hash))
+		itemExists = true
 	} else {
-		s.logger.DebugContext(ctx, "icon already exists in BART store, don't upload the icon file",
-			"hash", fmt.Sprintf("%x", btlv.Hash))
-		// tell buddies about the icon update
-		if err := s.buddyBroadcaster.BroadcastBuddyArrived(ctx, sess.IdentScreenName(), sess.TLVUserInfo()); err != nil {
+		existingItem, err := s.bartItemManager.BARTItem(ctx, bartID.Hash)
+		if err != nil {
 			return err
 		}
+		itemExists = len(existingItem) > 0
+	}
+
+	if itemExists {
+		if bartID.Type == wire.BARTTypesBuddyIconSmall || bartID.Type == wire.BARTTypesBuddyIcon {
+			sess.SetBuddyIcon(bartID)
+			// tell buddies about the icon update
+			if err := s.buddyBroadcaster.BroadcastBuddyArrived(ctx, sess.IdentScreenName(), sess.TLVUserInfo()); err != nil {
+				return err
+			}
+		}
+		s.logger.DebugContext(ctx, "icon already exists in BART store, don't upload the icon file",
+			"hash", fmt.Sprintf("%x", bartID.Hash))
+	} else {
+		// icon doesn't exist, tell the client to upload buddy icon
+		bartID.Flags |= wire.BARTFlagsUnknown
+		if bartID.Type == wire.BARTTypesBuddyIconSmall || bartID.Type == wire.BARTTypesBuddyIcon {
+			sess.SetBuddyIcon(bartID)
+		}
+		s.logger.DebugContext(ctx, "icon doesn't exist in BART store, client must upload the icon file",
+			"hash", fmt.Sprintf("%x", bartID.Hash))
 	}
 
 	s.messageRelayer.RelayToScreenName(ctx, sess.IdentScreenName(), wire.SNACMessage{
@@ -283,10 +296,20 @@ func (s FeedbagService) broadcastIconUpdate(ctx context.Context, sess *state.Ses
 			SubGroup:  wire.OServiceBartReply,
 		},
 		Body: wire.SNAC_0x01_0x21_OServiceBARTReply{
-			BARTID: bid,
+			BARTID: bartID,
 		},
 	})
 
+	if bartID.Type == wire.BARTTypesBuddyIconSmall || bartID.Type == wire.BARTTypesBuddyIcon {
+		s.messageRelayer.RelayToScreenName(ctx, sess.IdentScreenName(), wire.SNACMessage{
+			Frame: wire.SNACFrame{
+				FoodGroup: wire.OService,
+				SubGroup:  wire.OServiceUserInfoUpdate,
+			},
+			Body: newOServiceUserInfoUpdate(sess),
+		})
+	}
+
 	return nil
 }
 

+ 237 - 9
foodgroup/feedbag_test.go

@@ -1,7 +1,9 @@
 package foodgroup
 
 import (
+	"bytes"
 	"context"
+	"fmt"
 	"log/slog"
 	"testing"
 	"time"
@@ -384,6 +386,8 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 		expectOutput wire.SNACMessage
 		// wantTypingEventsEnabled indicates that the session should have typing events enabled
 		wantTypingEventsEnabled bool
+		// sessionMatch verifies the session state after completion
+		sessionMatch func(session *state.Session)
 	}{
 		{
 			name:        "add buddies",
@@ -755,11 +759,13 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 				Body: wire.SNAC_0x13_0x08_FeedbagInsertItem{
 					Items: []wire.FeedbagItem{
 						{
+							Name:    fmt.Sprintf("%d", wire.BARTTypesBuddyIcon),
 							ClassID: wire.FeedbagClassIdBart,
 							TLVLBlock: wire.TLVLBlock{
 								TLVList: wire.TLVList{
 									wire.NewTLVBE(wire.FeedbagAttributesBartInfo, wire.BARTInfo{
-										Hash: []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
+										Flags: wire.BARTFlagsCustom,
+										Hash:  []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
 									}),
 								},
 							},
@@ -782,11 +788,13 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 							screenName: state.NewIdentScreenName("me"),
 							items: []wire.FeedbagItem{
 								{
+									Name:    fmt.Sprintf("%d", wire.BARTTypesBuddyIcon),
 									ClassID: wire.FeedbagClassIdBart,
 									TLVLBlock: wire.TLVLBlock{
 										TLVList: wire.TLVList{
 											wire.NewTLVBE(wire.FeedbagAttributesBartInfo, wire.BARTInfo{
-												Hash: []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
+												Flags: wire.BARTFlagsCustom,
+												Hash:  []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
 											}),
 										},
 									},
@@ -815,6 +823,25 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 								},
 							},
 						},
+						{
+							screenName: state.NewIdentScreenName("me"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.OService,
+									SubGroup:  wire.OServiceUserInfoUpdate,
+								},
+								Body: func(val any) bool {
+									snac, ok := val.(wire.SNAC_0x01_0x0F_OServiceUserInfoUpdate)
+									if !ok {
+										return false
+									}
+									bartID, exists := snac.UserInfo[0].Bytes(wire.OServiceUserInfoBARTInfo)
+									return assert.True(t, exists) &&
+										assert.Equal(t, "me", snac.UserInfo[0].ScreenName) &&
+										assert.True(t, bytes.Contains(bartID, []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'}), "user info BART hash doesn't match")
+								},
+							},
+						},
 					},
 				},
 			},
@@ -828,6 +855,18 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 					Results: []uint16{0x0000},
 				},
 			},
+			sessionMatch: func(session *state.Session) {
+				have, hasIcon := session.BuddyIcon()
+				assert.True(t, hasIcon)
+				want := wire.BARTID{
+					Type: wire.BARTTypesBuddyIcon,
+					BARTInfo: wire.BARTInfo{
+						Flags: wire.BARTFlagsCustom | wire.BARTFlagsUnknown,
+						Hash:  []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
+					},
+				}
+				assert.Equal(t, want, have)
+			},
 		},
 		{
 			name:        "add icon hash to feedbag, icon already exists in BART store, notify buddies about icon change",
@@ -839,11 +878,13 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 				Body: wire.SNAC_0x13_0x08_FeedbagInsertItem{
 					Items: []wire.FeedbagItem{
 						{
+							Name:    fmt.Sprintf("%d", wire.BARTTypesBuddyIcon),
 							ClassID: wire.FeedbagClassIdBart,
 							TLVLBlock: wire.TLVLBlock{
 								TLVList: wire.TLVList{
 									wire.NewTLVBE(wire.FeedbagAttributesBartInfo, wire.BARTInfo{
-										Hash: []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
+										Flags: wire.BARTFlagsCustom,
+										Hash:  []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
 									}),
 								},
 							},
@@ -867,11 +908,13 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 							screenName: state.NewIdentScreenName("me"),
 							items: []wire.FeedbagItem{
 								{
+									Name:    fmt.Sprintf("%d", wire.BARTTypesBuddyIcon),
 									ClassID: wire.FeedbagClassIdBart,
 									TLVLBlock: wire.TLVLBlock{
 										TLVList: wire.TLVList{
 											wire.NewTLVBE(wire.FeedbagAttributesBartInfo, wire.BARTInfo{
-												Hash: []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
+												Flags: wire.BARTFlagsCustom,
+												Hash:  []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
 											}),
 										},
 									},
@@ -895,13 +938,32 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 									BARTID: wire.BARTID{
 										Type: wire.BARTTypesBuddyIcon,
 										BARTInfo: wire.BARTInfo{
-											Flags: wire.BARTFlagsCustom,
+											Flags: wire.BARTFlagsCustom | wire.BARTFlagsKnown,
 											Hash:  []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
 										},
 									},
 								},
 							},
 						},
+						{
+							screenName: state.NewIdentScreenName("me"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.OService,
+									SubGroup:  wire.OServiceUserInfoUpdate,
+								},
+								Body: func(val any) bool {
+									snac, ok := val.(wire.SNAC_0x01_0x0F_OServiceUserInfoUpdate)
+									if !ok {
+										return false
+									}
+									bartID, exists := snac.UserInfo[0].Bytes(wire.OServiceUserInfoBARTInfo)
+									return assert.True(t, exists) &&
+										assert.Equal(t, "me", snac.UserInfo[0].ScreenName) &&
+										assert.True(t, bytes.Contains(bartID, []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'}), "user info BART hash doesn't match")
+								},
+							},
+						},
 					},
 				},
 				buddyBroadcasterParams: buddyBroadcasterParams{
@@ -922,6 +984,18 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 					Results: []uint16{0x0000},
 				},
 			},
+			sessionMatch: func(session *state.Session) {
+				have, hasIcon := session.BuddyIcon()
+				assert.True(t, hasIcon)
+				want := wire.BARTID{
+					Type: wire.BARTTypesBuddyIcon,
+					BARTInfo: wire.BARTInfo{
+						Flags: wire.BARTFlagsCustom | wire.BARTFlagsKnown,
+						Hash:  []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
+					},
+				}
+				assert.Equal(t, want, have)
+			},
 		},
 		{
 			name:        "clear icon, notify buddies about icon change",
@@ -933,11 +1007,13 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 				Body: wire.SNAC_0x13_0x08_FeedbagInsertItem{
 					Items: []wire.FeedbagItem{
 						{
+							Name:    fmt.Sprintf("%d", wire.BARTTypesBuddyIcon),
 							ClassID: wire.FeedbagClassIdBart,
 							TLVLBlock: wire.TLVLBlock{
 								TLVList: wire.TLVList{
 									wire.NewTLVBE(wire.FeedbagAttributesBartInfo, wire.BARTInfo{
-										Hash: wire.GetClearIconHash(),
+										Flags: wire.BARTFlagsKnown,
+										Hash:  wire.GetClearIconHash(),
 									}),
 								},
 							},
@@ -952,11 +1028,13 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 							screenName: state.NewIdentScreenName("me"),
 							items: []wire.FeedbagItem{
 								{
+									Name:    fmt.Sprintf("%d", wire.BARTTypesBuddyIcon),
 									ClassID: wire.FeedbagClassIdBart,
 									TLVLBlock: wire.TLVLBlock{
 										TLVList: wire.TLVList{
 											wire.NewTLVBE(wire.FeedbagAttributesBartInfo, wire.BARTInfo{
-												Hash: wire.GetClearIconHash(),
+												Flags: wire.BARTFlagsKnown,
+												Hash:  wire.GetClearIconHash(),
 											}),
 										},
 									},
@@ -972,6 +1050,140 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 						},
 					},
 				},
+				messageRelayerParams: messageRelayerParams{
+					relayToScreenNameParams: relayToScreenNameParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.OService,
+									SubGroup:  wire.OServiceBartReply,
+								},
+								Body: wire.SNAC_0x01_0x21_OServiceBARTReply{
+									BARTID: wire.BARTID{
+										Type: wire.BARTTypesBuddyIcon,
+										BARTInfo: wire.BARTInfo{
+											Flags: wire.BARTFlagsKnown,
+											Hash:  wire.GetClearIconHash(),
+										},
+									},
+								},
+							},
+						},
+						{
+							screenName: state.NewIdentScreenName("me"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.OService,
+									SubGroup:  wire.OServiceUserInfoUpdate,
+								},
+								Body: func(val any) bool {
+									snac, ok := val.(wire.SNAC_0x01_0x0F_OServiceUserInfoUpdate)
+									if !ok {
+										return false
+									}
+									bartID, exists := snac.UserInfo[0].Bytes(wire.OServiceUserInfoBARTInfo)
+									return assert.True(t, exists) &&
+										assert.Equal(t, "me", snac.UserInfo[0].ScreenName) &&
+										assert.True(t, bytes.Contains(bartID, wire.GetClearIconHash()), "user info BART hash doesn't match")
+								},
+							},
+						},
+					},
+				},
+			},
+			expectOutput: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					FoodGroup: wire.Feedbag,
+					SubGroup:  wire.FeedbagStatus,
+					RequestID: 1234,
+				},
+				Body: wire.SNAC_0x13_0x0E_FeedbagStatus{
+					Results: []uint16{0x0000},
+				},
+			},
+			sessionMatch: func(session *state.Session) {
+				bartInfo, hasIcon := session.BuddyIcon()
+				assert.True(t, hasIcon)
+				assert.Equal(t, wire.GetClearIconHash(), bartInfo.Hash)
+			},
+		},
+		{
+			name:        "add non-icon to feedbag, icon doesn't exist in BART store, don't broadcast change",
+			userSession: newTestSession("me"),
+			inputSNAC: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					RequestID: 1234,
+				},
+				Body: wire.SNAC_0x13_0x08_FeedbagInsertItem{
+					Items: []wire.FeedbagItem{
+						{
+							Name:    fmt.Sprintf("%d", wire.BARTTypesArriveSound),
+							ClassID: wire.FeedbagClassIdBart,
+							TLVLBlock: wire.TLVLBlock{
+								TLVList: wire.TLVList{
+									wire.NewTLVBE(wire.FeedbagAttributesBartInfo, wire.BARTInfo{
+										Flags: wire.BARTFlagsCustom,
+										Hash:  []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
+									}),
+								},
+							},
+						},
+					},
+				},
+			},
+			mockParams: mockParams{
+				bartItemManagerParams: bartItemManagerParams{
+					bartItemManagerRetrieveParams: bartItemManagerRetrieveParams{
+						{
+							itemHash: []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
+							result:   []byte{}, // icon doesn't exist
+						},
+					},
+				},
+				feedbagManagerParams: feedbagManagerParams{
+					feedbagUpsertParams: feedbagUpsertParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+							items: []wire.FeedbagItem{
+								{
+									Name:    fmt.Sprintf("%d", wire.BARTTypesArriveSound),
+									ClassID: wire.FeedbagClassIdBart,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesBartInfo, wire.BARTInfo{
+												Flags: wire.BARTFlagsCustom,
+												Hash:  []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
+											}),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+				messageRelayerParams: messageRelayerParams{
+					relayToScreenNameParams: relayToScreenNameParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.OService,
+									SubGroup:  wire.OServiceBartReply,
+								},
+								Body: wire.SNAC_0x01_0x21_OServiceBARTReply{
+									BARTID: wire.BARTID{
+										Type: wire.BARTTypesArriveSound,
+										BARTInfo: wire.BARTInfo{
+											Flags: wire.BARTFlagsCustom | wire.BARTFlagsUnknown,
+											Hash:  []byte{'t', 'h', 'e', 'h', 'a', 's', 'h'},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
 			},
 			expectOutput: wire.SNACMessage{
 				Frame: wire.SNACFrame{
@@ -983,6 +1195,10 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 					Results: []uint16{0x0000},
 				},
 			},
+			sessionMatch: func(session *state.Session) {
+				_, hasIcon := session.BuddyIcon()
+				assert.False(t, hasIcon)
+			},
 		},
 	}
 
@@ -996,8 +1212,16 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 			}
 			messageRelayer := newMockMessageRelayer(t)
 			for _, params := range tc.mockParams.messageRelayerParams.relayToScreenNameParams {
-				messageRelayer.EXPECT().
-					RelayToScreenName(mock.Anything, params.screenName, params.message)
+				if matcherFn, ok := params.message.Body.(func(val any) bool); ok {
+					messageRelayer.EXPECT().
+						RelayToScreenName(matchContext(), params.screenName, mock.MatchedBy(func(message wire.SNACMessage) bool {
+							return params.message.Frame == message.Frame &&
+								matcherFn(message.Body)
+						}))
+				} else {
+					messageRelayer.EXPECT().
+						RelayToScreenName(matchContext(), params.screenName, params.message)
+				}
 			}
 			bartItemManager := newMockBARTItemManager(t)
 			for _, params := range tc.mockParams.bartItemManagerParams.bartItemManagerRetrieveParams {
@@ -1026,6 +1250,10 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 			assert.Equal(t, output, tc.expectOutput)
 
 			assert.Equal(t, tc.wantTypingEventsEnabled, tc.userSession.TypingEventsEnabled())
+
+			if tc.sessionMatch != nil {
+				tc.sessionMatch(tc.userSession)
+			}
 		})
 	}
 }

+ 11 - 2
foodgroup/helpers_test.go

@@ -236,6 +236,7 @@ type bartItemManagerUpsertParams []struct {
 	itemHash []byte
 	payload  []byte
 	bartType uint16
+	err      error
 }
 
 // buddyIconMetadataParams is the list of parameters passed at the mock
@@ -677,8 +678,9 @@ type broadcastVisibilityParams []struct {
 // broadcastBuddyArrivedParams is the list of parameters passed at the mock
 // buddyBroadcaster.BroadcastBuddyArrived call site
 type broadcastBuddyArrivedParams []struct {
-	screenName state.DisplayScreenName
-	err        error
+	screenName  state.DisplayScreenName
+	err         error
+	bodyMatcher func(snac wire.TLVUserInfo) bool
 }
 
 // broadcastBuddyDepartedParams is the list of parameters passed at the mock
@@ -838,6 +840,13 @@ func sessRemoteAddr(remoteAddr netip.AddrPort) func(session *state.Session) {
 	}
 }
 
+// sessBuddyIcon sets session buddy icon
+func sessOptBuddyIcon(icon wire.BARTID) func(session *state.Session) {
+	return func(session *state.Session) {
+		session.SetBuddyIcon(icon)
+	}
+}
+
 // newTestSession creates a session object with 0 or more functional options
 // applied
 func newTestSession(screenName state.DisplayScreenName, options ...func(session *state.Session)) *state.Session {

+ 22 - 0
state/session.go

@@ -50,6 +50,7 @@ const (
 // methods may be safely accessed by multiple goroutines.
 type Session struct {
 	awayMessage             string
+	buddyIcon               wire.BARTID
 	caps                    [][16]byte
 	chatRoomCookie          string
 	clientID                string
@@ -456,6 +457,11 @@ func (s *Session) userInfo() wire.TLVList {
 		tlvs.Append(wire.NewTLVBE(wire.OServiceUserInfoIdleTime, uint16(s.nowFn().Sub(s.idleTime).Minutes())))
 	}
 
+	// set buddy icon metadata, if user has buddy icon
+	if bartID, hasIcon := s.BuddyIcon(); hasIcon {
+		tlvs.Append(wire.NewTLVBE(wire.OServiceUserInfoBARTInfo, bartID))
+	}
+
 	// ICQ direct-connect info. The TLV is required for buddy arrival events to
 	// work in ICQ, even if the values are set to default.
 	if s.userInfoBitmask&wire.OServiceUserFlagICQ == wire.OServiceUserFlagICQ {
@@ -699,6 +705,22 @@ func (s *Session) Profile() UserProfile {
 	return s.profile
 }
 
+// SetBuddyIcon stores the session's buddy icon metadata.
+func (s *Session) SetBuddyIcon(icon wire.BARTID) {
+	s.mutex.Lock()
+	defer s.mutex.Unlock()
+	s.buddyIcon = icon
+}
+
+// BuddyIcon returns the session's buddy icon metadata and reports whether it
+// has been set. The icon is considered set if its type is non-zero.
+func (s *Session) BuddyIcon() (wire.BARTID, bool) {
+	s.mutex.RLock()
+	defer s.mutex.RUnlock()
+	icon := s.buddyIcon
+	return icon, icon.Type != 0
+}
+
 // SetMemberSince sets the member since timestamp.
 func (s *Session) SetMemberSince(t time.Time) {
 	s.mutex.Lock()