Просмотр исходного кода

webapi: resolve presence via LocateService.UserInfoQuery

Mike 2 недель назад
Родитель
Сommit
a4dfb0c3f3

+ 0 - 1
cmd/server/factory.go

@@ -611,7 +611,6 @@ func WebAPI(deps Container) *webapi.Server {
 		OfflineMessageManager: deps.sqLiteUserStore,
 		BuddyBroadcaster:      oscarBuddyBroadcaster,
 		ProfileManager:        deps.sqLiteUserStore,
-		RelationshipFetcher:   deps.sqLiteUserStore,
 		// Phase 3 additions
 		PreferenceManager: deps.sqLiteUserStore.NewWebPreferenceManager(),
 		// Phase 4 additions for OSCAR Bridge

+ 0 - 3
server/webapi/handler.go

@@ -34,9 +34,6 @@ type Handler struct {
 	OfflineMessageManager OfflineMessageManager
 	BuddyBroadcaster      BuddyBroadcaster
 	ProfileManager        ProfileManager
-	RelationshipFetcher   interface {
-		Relationship(ctx context.Context, me state.IdentScreenName, them state.IdentScreenName) (state.Relationship, error)
-	}
 	// Phase 3 additions
 	PreferenceManager PreferenceManager
 	// Phase 4 additions for OSCAR Bridge

+ 0 - 5
server/webapi/handlers/messaging.go

@@ -14,11 +14,6 @@ import (
 	"github.com/mk6i/open-oscar-server/wire"
 )
 
-// RelationshipFetcher defines methods for fetching user relationships
-type RelationshipFetcher interface {
-	Relationship(ctx context.Context, me state.IdentScreenName, them state.IdentScreenName) (state.Relationship, error)
-}
-
 // ICBMService defines methods for ICBM operations
 type ICBMService interface {
 	ChannelMsgToHost(ctx context.Context, instance *state.SessionInstance, inFrame wire.SNACFrame, inBody wire.SNAC_0x04_0x06_ICBMChannelMsgToHost) (*wire.SNACMessage, error)

+ 6 - 5
server/webapi/handlers/mocks_test.go

@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/mock"
 
 	"github.com/mk6i/open-oscar-server/state"
+	"github.com/mk6i/open-oscar-server/wire"
 )
 
 // MockSessionRetriever is a mock implementation of SessionRetriever
@@ -29,12 +30,12 @@ func (m *MockSessionRetriever) RetrieveSession(screenName state.IdentScreenName)
 	return nil
 }
 
-// MockRelationshipFetcher is a mock implementation of RelationshipFetcher
-type MockRelationshipFetcher struct {
+// MockLocateService is a mock implementation of LocateService
+type MockLocateService struct {
 	mock.Mock
 }
 
-func (m *MockRelationshipFetcher) Relationship(ctx context.Context, me state.IdentScreenName, them state.IdentScreenName) (state.Relationship, error) {
-	args := m.Called(ctx, me, them)
-	return args.Get(0).(state.Relationship), args.Error(1)
+func (m *MockLocateService) UserInfoQuery(ctx context.Context, instance *state.SessionInstance, inFrame wire.SNACFrame, inBody wire.SNAC_0x02_0x05_LocateUserInfoQuery) (wire.SNACMessage, error) {
+	args := m.Called(ctx, instance, inFrame, inBody)
+	return args.Get(0).(wire.SNACMessage), args.Error(1)
 }

+ 84 - 102
server/webapi/handlers/presence.go

@@ -15,13 +15,20 @@ import (
 
 // PresenceHandler handles Web AIM API presence-related endpoints.
 type PresenceHandler struct {
-	SessionManager      *state.WebAPISessionManager
-	SessionRetriever    SessionRetriever
-	FeedbagService      FeedbagService
-	BuddyBroadcaster    BuddyBroadcaster
-	ProfileManager      ProfileManager
-	RelationshipFetcher RelationshipFetcher
-	Logger              *slog.Logger
+	SessionManager   *state.WebAPISessionManager
+	SessionRetriever SessionRetriever
+	FeedbagService   FeedbagService
+	BuddyBroadcaster BuddyBroadcaster
+	ProfileManager   ProfileManager
+	LocateService    LocateService
+	Logger           *slog.Logger
+}
+
+// LocateService issues OSCAR locate user-info queries. A single query performs
+// the blocking relationship check, the online/offline session lookup, and
+// returns the user's presence info plus optional profile and away-message data.
+type LocateService interface {
+	UserInfoQuery(ctx context.Context, instance *state.SessionInstance, inFrame wire.SNACFrame, inBody wire.SNAC_0x02_0x05_LocateUserInfoQuery) (wire.SNACMessage, error)
 }
 
 // BuddyBroadcaster broadcasts buddy presence updates
@@ -30,6 +37,10 @@ type BuddyBroadcaster interface {
 	BroadcastBuddyDeparted(ctx context.Context, screenName state.IdentScreenName) error
 }
 
+// maxPresenceTargets caps how many screen names a single presence/get request
+// may query in target-list ("t=") mode.
+const maxPresenceTargets = 10
+
 // ProfileManager manages user profiles (uses types.ProfileManager)
 type ProfileManager interface {
 	SetProfile(ctx context.Context, screenName state.IdentScreenName, profile state.UserProfile) error
@@ -107,7 +118,7 @@ func (h *PresenceHandler) GetPresence(w http.ResponseWriter, r *http.Request) {
 
 	if getBuddyList {
 		// Retrieve buddy list from feedbag
-		groups, err := h.getBuddyListGroups(ctx, session)
+		groups, err := h.getBuddyListGroups(ctx, session, wantProfileMsg)
 		if err != nil {
 			h.Logger.ErrorContext(ctx, "failed to get buddy list", "err", err.Error())
 			// Return empty buddy list on error instead of failing
@@ -117,6 +128,10 @@ func (h *PresenceHandler) GetPresence(w http.ResponseWriter, r *http.Request) {
 	} else if targetUsers != "" {
 		// Get presence for specific users
 		users := strings.Split(targetUsers, ",")
+		if len(users) > maxPresenceTargets {
+			h.sendError(w, http.StatusBadRequest, fmt.Sprintf("too many screen names requested (max %d)", maxPresenceTargets))
+			return
+		}
 		presenceList := make([]BuddyPresenceInfo, 0, len(users))
 
 		for _, user := range users {
@@ -124,40 +139,7 @@ func (h *PresenceHandler) GetPresence(w http.ResponseWriter, r *http.Request) {
 			if user == "" {
 				continue
 			}
-
-			userScreenName := state.NewIdentScreenName(user)
-
-			// Check blocking relationship (OSCAR compliant)
-			rel, err := h.RelationshipFetcher.Relationship(ctx, session.ScreenName.IdentScreenName(), userScreenName)
-			if err != nil {
-				h.Logger.WarnContext(ctx, "failed to get relationship", "error", err)
-				// On error, show as offline
-				presence := BuddyPresenceInfo{
-					AimID:    user,
-					State:    "offline",
-					UserType: "aim",
-				}
-				presenceList = append(presenceList, presence)
-				continue
-			}
-
-			// OSCAR compliance: mutual invisibility when blocking
-			if rel.YouBlock || rel.BlocksYou {
-				presence := BuddyPresenceInfo{
-					AimID:    user,
-					State:    "offline",
-					UserType: "aim",
-				}
-				presenceList = append(presenceList, presence)
-			} else {
-				presence := h.getUserPresence(userScreenName)
-				if wantProfileMsg && presence.ProfileMsg == "" && h.SessionRetriever != nil {
-					if oscarSess := h.SessionRetriever.RetrieveSession(userScreenName); oscarSess != nil {
-						presence.ProfileMsg = oscarSess.Profile().ProfileText
-					}
-				}
-				presenceList = append(presenceList, presence)
-			}
+			presenceList = append(presenceList, h.getUserPresence(ctx, session.OSCARSession, state.NewIdentScreenName(user), wantProfileMsg))
 		}
 
 		presenceData.Users = presenceList
@@ -180,9 +162,7 @@ func (h *PresenceHandler) GetPresence(w http.ResponseWriter, r *http.Request) {
 }
 
 // getBuddyListGroups retrieves the buddy list organized by groups.
-func (h *PresenceHandler) getBuddyListGroups(ctx context.Context, session *state.WebAPISession) ([]BuddyGroupInfo, error) {
-	screenName := session.ScreenName.IdentScreenName()
-
+func (h *PresenceHandler) getBuddyListGroups(ctx context.Context, session *state.WebAPISession, wantProfileMsg bool) ([]BuddyGroupInfo, error) {
 	// Get feedbag items via the feedbag service
 	frame := wire.SNACFrame{FoodGroup: wire.Feedbag, SubGroup: wire.FeedbagQuery}
 	reply, err := h.FeedbagService.Query(ctx, session.OSCARSession, frame)
@@ -249,36 +229,10 @@ func (h *PresenceHandler) getBuddyListGroups(ctx context.Context, session *state
 			}
 		}
 
-		buddyScreenName := state.NewIdentScreenName(buddyName)
-
-		// Check blocking relationship (OSCAR compliant)
-		rel, err := h.RelationshipFetcher.Relationship(ctx, screenName, buddyScreenName)
-		if err != nil {
-			h.Logger.WarnContext(ctx, "failed to get relationship", "error", err)
-			// On error, include the buddy but they'll appear offline
-			presence := BuddyPresenceInfo{
-				AimID:    buddyName,
-				State:    "offline",
-				UserType: "aim",
-			}
-			group.Buddies = append(group.Buddies, presence)
-			continue
-		}
-
-		// OSCAR compliance: mutual invisibility when blocking
-		if rel.YouBlock || rel.BlocksYou {
-			// Add them as offline to maintain buddy list structure
-			presence := BuddyPresenceInfo{
-				AimID:    buddyName,
-				State:    "offline",
-				UserType: "aim",
-			}
-			group.Buddies = append(group.Buddies, presence)
-		} else {
-			// Normal presence lookup
-			presence := h.getUserPresence(buddyScreenName)
-			group.Buddies = append(group.Buddies, presence)
-		}
+		// UserInfoQuery performs the blocking check and online lookup; blocked or
+		// offline buddies come back as "offline", preserving the list structure.
+		presence := h.getUserPresence(ctx, session.OSCARSession, state.NewIdentScreenName(buddyName), wantProfileMsg)
+		group.Buddies = append(group.Buddies, presence)
 	}
 
 	// Convert map to slice
@@ -290,45 +244,73 @@ func (h *PresenceHandler) getBuddyListGroups(ctx context.Context, session *state
 	return groups, nil
 }
 
-// getUserPresence gets the current presence state for a user.
-func (h *PresenceHandler) getUserPresence(screenName state.IdentScreenName) BuddyPresenceInfo {
+// getUserPresence resolves a user's presence by issuing a locate UserInfoQuery
+// on behalf of the requesting OSCAR session (instance). UserInfoQuery performs
+// the OSCAR blocking check and online lookup internally: blocked and offline
+// users both come back as a locate error, which we surface as "offline".
+func (h *PresenceHandler) getUserPresence(ctx context.Context, instance *state.SessionInstance, target state.IdentScreenName, wantProfileMsg bool) BuddyPresenceInfo {
 	// Default offline presence
 	presence := BuddyPresenceInfo{
-		AimID:    screenName.String(),
+		AimID:    target.String(),
 		State:    "offline",
 		UserType: "aim",
 	}
 
-	// Check if user is online by looking for their OSCAR session
-	if session := h.SessionRetriever.RetrieveSession(screenName); session != nil {
-		presence.State = "online"
+	// Determine user type
+	if strings.HasPrefix(target.String(), "admin") {
+		presence.UserType = "admin"
+	} else if isICQScreenName(target.String()) {
+		presence.UserType = "icq"
+	}
 
-		// Check user status
-		if session.Away() {
-			presence.State = "away"
-			// TODO: Get away message from session
-		} else if session.AllUserStatusBitmask(wire.OServiceUserStatusDND) {
-			presence.State = "dnd"
-		}
+	// Web-only sessions have no OSCAR instance to query on behalf of.
+	if instance == nil {
+		return presence
+	}
 
-		// Check idle time
-		if session.Idle() {
-			presence.State = "idle"
-			idleTime := time.Since(session.IdleTime())
-			presence.IdleTime = int(idleTime.Minutes())
-		}
+	reqType := wire.LocateTypeUnavailable // away message
+	if wantProfileMsg {
+		reqType |= wire.LocateTypeSig // profile text
+	}
 
-		// Get online time
-		presence.OnlineTime = session.SignonTime().Unix()
+	reply, err := h.LocateService.UserInfoQuery(ctx, instance, wire.SNACFrame{},
+		wire.SNAC_0x02_0x05_LocateUserInfoQuery{Type: uint16(reqType), ScreenName: target.String()})
+	if err != nil {
+		h.Logger.WarnContext(ctx, "failed to query user info", "screenName", target.String(), "error", err)
+		return presence
+	}
 
-		// TODO: Get status message from profile
+	info, ok := reply.Body.(wire.SNAC_0x02_0x06_LocateUserInfoReply)
+	if !ok {
+		// Locate error => user is blocked or offline.
+		return presence
 	}
 
-	// Determine user type
-	if strings.HasPrefix(screenName.String(), "admin") {
-		presence.UserType = "admin"
-	} else if isICQScreenName(screenName.String()) {
-		presence.UserType = "icq"
+	presence.State = "online"
+
+	if tod, ok := info.Uint32BE(wire.OServiceUserInfoSignonTOD); ok {
+		presence.OnlineTime = int64(tod)
+	}
+
+	if info.IsAway() {
+		presence.State = "away"
+	} else if status, ok := info.Uint32BE(wire.OServiceUserInfoStatus); ok && status&wire.OServiceUserStatusDND != 0 {
+		presence.State = "dnd"
+	}
+
+	if idle, ok := info.Uint16BE(wire.OServiceUserInfoIdleTime); ok && idle > 0 {
+		presence.State = "idle"
+		presence.IdleTime = int(idle)
+	}
+
+	if msg, ok := info.LocateInfo.String(wire.LocateTLVTagsInfoUnavailableData); ok {
+		presence.AwayMsg = msg
+	}
+
+	if wantProfileMsg {
+		if prof, ok := info.LocateInfo.String(wire.LocateTLVTagsInfoSigData); ok {
+			presence.ProfileMsg = prof
+		}
 	}
 
 	return presence

+ 56 - 33
server/webapi/handlers/presence_test.go

@@ -100,18 +100,37 @@ func (m *MockProfileManager) Profile(ctx context.Context, screenName state.Ident
 	return args.Get(0).(state.UserProfile), args.Error(1)
 }
 
+// onlineUserInfoReply builds a locate UserInfoReply for an online user,
+// optionally marking them idle by the given number of minutes (0 = not idle).
+func onlineUserInfoReply(screenName string, idleMinutes uint16) wire.SNACMessage {
+	info := wire.TLVUserInfo{ScreenName: screenName}
+	if idleMinutes > 0 {
+		info.Append(wire.NewTLVBE(wire.OServiceUserInfoIdleTime, idleMinutes))
+	}
+	return wire.SNACMessage{
+		Body: wire.SNAC_0x02_0x06_LocateUserInfoReply{TLVUserInfo: info},
+	}
+}
+
+// screenNameMatcher matches a UserInfoQuery request body by its target screen name.
+func screenNameMatcher(screenName string) any {
+	return mock.MatchedBy(func(b wire.SNAC_0x02_0x05_LocateUserInfoQuery) bool {
+		return b.ScreenName == screenName
+	})
+}
+
 func TestPresenceHandler_GetPresence(t *testing.T) {
 	tests := []struct {
 		name               string
 		queryParams        string
-		setupMocks         func(*MockSessionRetriever, *MockFeedbagService, *MockRelationshipFetcher)
+		setupMocks         func(*MockFeedbagService, *MockLocateService)
 		expectedStatusCode int
 		checkResponse      func(*testing.T, string)
 	}{
 		{
 			name:        "Success_BuddyList",
 			queryParams: "bl=1",
-			setupMocks: func(sr *MockSessionRetriever, fr *MockFeedbagService, rf *MockRelationshipFetcher) {
+			setupMocks: func(fr *MockFeedbagService, ls *MockLocateService) {
 				// Return feedbag with a group and buddy
 				fr.On("Query", mock.Anything, mock.Anything, mock.Anything).
 					Return(wire.SNACMessage{
@@ -122,10 +141,8 @@ func TestPresenceHandler_GetPresence(t *testing.T) {
 							},
 						},
 					}, nil)
-				rf.On("Relationship", mock.Anything, state.NewIdentScreenName("testuser"), state.NewIdentScreenName("buddy1")).
-					Return(state.Relationship{}, nil)
-				sr.On("RetrieveSession", state.NewIdentScreenName("buddy1")).
-					Return(nil)
+				ls.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, screenNameMatcher("buddy1")).
+					Return(onlineUserInfoReply("buddy1", 0), nil)
 			},
 			expectedStatusCode: http.StatusOK,
 			checkResponse: func(t *testing.T, body string) {
@@ -133,21 +150,18 @@ func TestPresenceHandler_GetPresence(t *testing.T) {
 				assert.Contains(t, body, `"groups"`)
 				assert.Contains(t, body, `"Friends"`)
 				assert.Contains(t, body, `"buddy1"`)
-				assert.Contains(t, body, `"offline"`)
+				assert.Contains(t, body, `"online"`)
 			},
 		},
 		{
 			name:        "Success_TargetUsers",
 			queryParams: "t=user1,user2",
-			setupMocks: func(sr *MockSessionRetriever, fr *MockFeedbagService, rf *MockRelationshipFetcher) {
-				rf.On("Relationship", mock.Anything, state.NewIdentScreenName("testuser"), state.NewIdentScreenName("user1")).
-					Return(state.Relationship{}, nil)
-				rf.On("Relationship", mock.Anything, state.NewIdentScreenName("testuser"), state.NewIdentScreenName("user2")).
-					Return(state.Relationship{}, nil)
-				sr.On("RetrieveSession", state.NewIdentScreenName("user1")).
-					Return(nil)
-				sr.On("RetrieveSession", state.NewIdentScreenName("user2")).
-					Return(nil)
+			setupMocks: func(fr *MockFeedbagService, ls *MockLocateService) {
+				ls.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, screenNameMatcher("user1")).
+					Return(onlineUserInfoReply("user1", 0), nil)
+				// user2 is idle for 7 minutes.
+				ls.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, screenNameMatcher("user2")).
+					Return(onlineUserInfoReply("user2", 7), nil)
 			},
 			expectedStatusCode: http.StatusOK,
 			checkResponse: func(t *testing.T, body string) {
@@ -155,15 +169,16 @@ func TestPresenceHandler_GetPresence(t *testing.T) {
 				assert.Contains(t, body, `"users"`)
 				assert.Contains(t, body, `"user1"`)
 				assert.Contains(t, body, `"user2"`)
+				assert.Contains(t, body, `"idle"`)
 			},
 		},
 		{
-			name:        "Success_BlockedUserOffline",
+			name:        "Success_BlockedOrOfflineUser",
 			queryParams: "t=blockeduser",
-			setupMocks: func(sr *MockSessionRetriever, fr *MockFeedbagService, rf *MockRelationshipFetcher) {
-				rf.On("Relationship", mock.Anything, state.NewIdentScreenName("testuser"), state.NewIdentScreenName("blockeduser")).
-					Return(state.Relationship{YouBlock: true}, nil)
-				// RetrieveSession should NOT be called for blocked users
+			setupMocks: func(fr *MockFeedbagService, ls *MockLocateService) {
+				// A blocked or offline user comes back as a locate error.
+				ls.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, screenNameMatcher("blockeduser")).
+					Return(wire.SNACMessage{Body: wire.SNACError{Code: wire.ErrorCodeNotLoggedOn}}, nil)
 			},
 			expectedStatusCode: http.StatusOK,
 			checkResponse: func(t *testing.T, body string) {
@@ -175,31 +190,40 @@ func TestPresenceHandler_GetPresence(t *testing.T) {
 		{
 			name:               "Success_EmptyRequest",
 			queryParams:        "",
-			setupMocks:         func(sr *MockSessionRetriever, fr *MockFeedbagService, rf *MockRelationshipFetcher) {},
+			setupMocks:         func(fr *MockFeedbagService, ls *MockLocateService) {},
 			expectedStatusCode: http.StatusOK,
 			checkResponse: func(t *testing.T, body string) {
 				assert.Contains(t, body, `"statusCode":200`)
 			},
 		},
+		{
+			name:        "Error_TooManyTargets",
+			queryParams: "t=u1,u2,u3,u4,u5,u6,u7,u8,u9,u10,u11",
+			// No UserInfoQuery should be issued; the request is rejected up front.
+			setupMocks:         func(fr *MockFeedbagService, ls *MockLocateService) {},
+			expectedStatusCode: http.StatusBadRequest,
+			checkResponse: func(t *testing.T, body string) {
+				assert.Contains(t, body, "too many screen names requested")
+			},
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			sessionRetriever := &MockSessionRetriever{}
 			feedbagService := &MockFeedbagService{}
-			relFetcher := &MockRelationshipFetcher{}
+			locateService := &MockLocateService{}
 
-			sessionMgr, aimsid := createTestSessionManager("testuser")
+			oscarInstance := state.NewSession().AddInstance()
+			sessionMgr, aimsid := createTestSessionManagerWithOSCAR("testuser", oscarInstance)
 
 			handler := &PresenceHandler{
-				SessionManager:      sessionMgr,
-				SessionRetriever:    sessionRetriever,
-				FeedbagService:      feedbagService,
-				RelationshipFetcher: relFetcher,
-				Logger:              slog.Default(),
+				SessionManager: sessionMgr,
+				FeedbagService: feedbagService,
+				LocateService:  locateService,
+				Logger:         slog.Default(),
 			}
 
-			tt.setupMocks(sessionRetriever, feedbagService, relFetcher)
+			tt.setupMocks(feedbagService, locateService)
 
 			reqURL := "/presence/get?aimsid=" + aimsid
 			if tt.queryParams != "" {
@@ -219,9 +243,8 @@ func TestPresenceHandler_GetPresence(t *testing.T) {
 				tt.checkResponse(t, responseBody)
 			}
 
-			sessionRetriever.AssertExpectations(t)
 			feedbagService.AssertExpectations(t)
-			relFetcher.AssertExpectations(t)
+			locateService.AssertExpectations(t)
 		})
 	}
 }

+ 7 - 7
server/webapi/server.go

@@ -49,13 +49,13 @@ func NewServer(listeners []string, logger *slog.Logger, handler Handler, apiKeyV
 	}
 
 	presenceHandler := &handlers.PresenceHandler{
-		SessionManager:      sessionManager,
-		SessionRetriever:    handler.SessionRetriever,
-		FeedbagService:      handler.FeedbagService,
-		BuddyBroadcaster:    handler.BuddyBroadcaster,
-		ProfileManager:      handler.ProfileManager,
-		RelationshipFetcher: handler.RelationshipFetcher,
-		Logger:              logger,
+		SessionManager:   sessionManager,
+		SessionRetriever: handler.SessionRetriever,
+		FeedbagService:   handler.FeedbagService,
+		BuddyBroadcaster: handler.BuddyBroadcaster,
+		ProfileManager:   handler.ProfileManager,
+		LocateService:    handler.LocateService,
+		Logger:           logger,
 	}
 
 	buddyListHandler := handlers.NewBuddyListHandler(