From 28f7ed077f47f250fba27fc7ada1b54737190472 Mon Sep 17 00:00:00 2001 From: dsx137 <70027572+dsx137@users.noreply.github.com> Date: Tue, 17 Mar 2026 16:02:47 +0800 Subject: [PATCH 1/2] refactor(group): optimize group notification and cache handling - Improve notification sender with bulk operations and fallback logic - Enhance group cache with new methods for member counts and version handling - Refactor group controller with better cache integration - Add more robust error handling and logging fix(group): optimize group cache implementation This commit refactors the GroupCacheRedis implementation to improve code organization and maintainability. The changes include: - Added proper import organization and removed unused imports - Implemented batchGetCache2 function calls for efficient data retrieval - Added comprehensive cache key management functions - Improved version tracking for group members and requests - Enhanced error handling and logging - Added batch operations for better performance - Refactored complex functions into smaller, more manageable pieces The changes focus on improving the overall structure and performance of the group cache system while maintaining backward compatibility. refactor(group): improve notification error handling and add admin context - Convert notification methods to return errors for better error propagation - Add admin context checking for bulk notifications - Enhance group info retrieval with proper error handling - Improve notification sender with fallback user information fix(group): handle nil group member map in notification Add nil check for groupMemberMap[groupInfo.GroupID] before accessing to prevent nil pointer panic. Skip iteration if map is nil. refactor(group): update group notification methods to use UserInfo struct Renamed function parameter from groupMemberUserID string to changedUserInfo *sdkws.UserInfo Updated function logic to use changedUserInfo.UserID instead of separate parameter Refactored notification methods to use consistent UserInfo struct pattern Added proper error handling for opUser retrieval Updated function signatures and internal logic for GroupMemberInfoSetNotificationBulk fix: batchGetGroupRoleLevelMemberIDs with BatchCache fix: GetGroupMemberNums BatchCache perf(group): optimize notification logic for group member operations # Conflicts: # internal/rpc/group/group.go # pkg/common/storage/cache/redis/group.go --- internal/rpc/group/group.go | 7 +- internal/rpc/group/notification.go | 191 ++++++++++++++++++++++-- pkg/common/storage/cache/group.go | 1 + pkg/common/storage/cache/redis/group.go | 166 ++++++++++++++++++-- pkg/common/storage/controller/group.go | 14 ++ 5 files changed, 358 insertions(+), 21 deletions(-) diff --git a/internal/rpc/group/group.go b/internal/rpc/group/group.go index 359ecfa6f..49063a13a 100644 --- a/internal/rpc/group/group.go +++ b/internal/rpc/group/group.go @@ -23,6 +23,9 @@ import ( "strings" "time" + "github.com/openimsdk/tools/utils/stringutil" + "google.golang.org/grpc" + "github.com/openimsdk/open-im-server/v3/pkg/dbbuild" "github.com/openimsdk/open-im-server/v3/pkg/rpcli" "google.golang.org/grpc" @@ -143,8 +146,8 @@ func (g *groupServer) NotificationUserInfoUpdate(ctx context.Context, req *pbgro return nil, err } } - for _, groupID := range groupIDs { - g.notification.GroupMemberInfoSetNotification(ctx, groupID, req.UserID) + if err = g.notification.GroupMemberInfoSetNotificationBulk(ctx, groupIDs, req.NewUserInfo); err != nil { + log.ZError(ctx, stringutil.GetFuncName(1)+" failed", err) } if err = g.db.DeleteGroupMemberHash(ctx, groupIDs); err != nil { return nil, err diff --git a/internal/rpc/group/notification.go b/internal/rpc/group/notification.go index 5a3cbbe26..8cf214459 100644 --- a/internal/rpc/group/notification.go +++ b/internal/rpc/group/notification.go @@ -802,6 +802,182 @@ func (g *NotificationSender) GroupCancelMutedNotification(ctx context.Context, g g.Notification(ctx, mcontext.GetOpUserID(ctx), group.GroupID, constant.GroupCancelMutedNotification, tips) } +func (g *NotificationSender) getPublicUserInfos(ctx context.Context, userIDs []string) ([]*sdkws.PublicUserInfo, error) { + users, err := g.getUsersInfo(ctx, userIDs) + if err != nil { + return nil, err + } + userMap := make(map[string]common_user.CommonUser) + for _, user := range users { + userMap[user.GetUserID()] = user + } + for _, userID := range userIDs { + if _, ok := userMap[userID]; !ok { + return nil, servererrs.ErrUserIDNotFound.WrapMsg(fmt.Sprintf("user %s not found", userID)) + } + } + + return datautil.Slice(users, func(e common_user.CommonUser) *sdkws.PublicUserInfo { + return &sdkws.PublicUserInfo{ + UserID: e.GetUserID(), + Nickname: e.GetNickname(), + FaceURL: e.GetFaceURL(), + Ex: e.GetEx(), + } + }), nil +} + +func (g *NotificationSender) getGroupMembersForUser(ctx context.Context, groupIDs []string, userID string) ([]*sdkws.GroupMemberFullInfo, error) { + members, err := g.db.FindGroupMemberUser(ctx, groupIDs, userID) + if err != nil { + return nil, err + } + if err := g.PopulateGroupMember(ctx, members...); err != nil { + return nil, err + } + log.ZDebug(ctx, "getGroupMembers", "members", members) + return datautil.Slice(members, func(e *model.GroupMember) *sdkws.GroupMemberFullInfo { return g.groupMemberDB2PB(e, 0) }), nil +} + +func (g *NotificationSender) getGroupMemberMapForUser(ctx context.Context, groupIDs []string, userID string) (map[string]*sdkws.GroupMemberFullInfo, error) { + members, err := g.getGroupMembersForUser(ctx, groupIDs, userID) + if err != nil { + return nil, err + } + m := make(map[string]*sdkws.GroupMemberFullInfo) + for i, member := range members { + m[member.GroupID] = members[i] + } + return m, nil +} + +func (g *NotificationSender) migrateFallbackToGroupMemberFullInfo(groupMemberFullInfo *sdkws.GroupMemberFullInfo, publicUserInfo *sdkws.PublicUserInfo) *sdkws.GroupMemberFullInfo { + if groupMemberFullInfo == nil { + groupMemberFullInfo = &sdkws.GroupMemberFullInfo{} + } + if publicUserInfo == nil { + publicUserInfo = &sdkws.PublicUserInfo{} + } + + firstNotEmpty := func(s ...string) string { + for _, s := range s { + if s != "" { + return s + } + } + return "" + } + + return &sdkws.GroupMemberFullInfo{ + GroupID: groupMemberFullInfo.GroupID, + UserID: firstNotEmpty(groupMemberFullInfo.UserID, publicUserInfo.UserID), + RoleLevel: groupMemberFullInfo.RoleLevel, + JoinTime: groupMemberFullInfo.JoinTime, + Nickname: firstNotEmpty(groupMemberFullInfo.Nickname, publicUserInfo.Nickname), + FaceURL: firstNotEmpty(groupMemberFullInfo.FaceURL, publicUserInfo.FaceURL), + AppMangerLevel: groupMemberFullInfo.AppMangerLevel, + JoinSource: groupMemberFullInfo.JoinSource, + OperatorUserID: firstNotEmpty(groupMemberFullInfo.OperatorUserID, publicUserInfo.UserID), + InviterUserID: groupMemberFullInfo.InviterUserID, + Ex: groupMemberFullInfo.Ex, + MuteEndTime: groupMemberFullInfo.MuteEndTime, + } +} + +func (g *NotificationSender) getGroupInfos(ctx context.Context, groupIDs []string) ([]*sdkws.GroupInfo, error) { + var err error + defer func() { + if err != nil { + log.ZError(ctx, stringutil.GetFuncName(1)+" failed", err) + } + }() + + groups, err := g.db.FindGroup(ctx, groupIDs) + if err != nil { + return nil, err + } + numMap, err := g.db.FindGroupMemberNums(ctx, groupIDs) + if err != nil { + return nil, err + } + owners, err := g.db.FindGroupsOwner(ctx, groupIDs) + if err != nil { + return nil, err + } + ownerMap := make(map[string]string) + for _, owner := range owners { + ownerMap[owner.GroupID] = owner.UserID + } + + return datautil.Slice(groups, func(group *model.Group) *sdkws.GroupInfo { + return convert.Db2PbGroupInfo(group, ownerMap[group.GroupID], numMap[group.GroupID]) + }), nil +} + +func (g *NotificationSender) GroupMemberInfoSetNotificationBulk(ctx context.Context, groupIDs []string, changedUserInfo *sdkws.UserInfo) error { + opUserID := mcontext.GetOpUserID(ctx) + opIsAdmin := authverify.CheckUserIsAdmin(ctx, opUserID) + + groupInfos, err := g.getGroupInfos(ctx, groupIDs) + if err != nil { + return err + } + + groupMemberMap, err := g.getGroupMemberMapForUser(ctx, groupIDs, changedUserInfo.UserID) + if err != nil { + return err + } + + opMemberMap := groupMemberMap + if opUserID != changedUserInfo.UserID { + opMemberMap, err = g.getGroupMemberMapForUser(ctx, groupIDs, opUserID) + if err != nil { + return err + } + } + + opPublicUser := &sdkws.PublicUserInfo{ + UserID: changedUserInfo.GetUserID(), + Nickname: changedUserInfo.GetNickname(), + FaceURL: changedUserInfo.GetFaceURL(), + Ex: changedUserInfo.GetEx(), + } + if opUserID != changedUserInfo.UserID { + opPublicUser, err = g.getUser(ctx, opUserID) + if err != nil { + return err + } + } + + for _, groupInfo := range groupInfos { + if groupMemberMap[groupInfo.GroupID] == nil { + continue + } + + opUserGroupMemberFullInfo := datautil.If(opIsAdmin, &sdkws.GroupMemberFullInfo{ + GroupID: groupInfo.GroupID, + UserID: opUserID, + RoleLevel: constant.GroupAdmin, + AppMangerLevel: constant.AppAdmin, + }, opMemberMap[groupInfo.GroupID]) + if opUserGroupMemberFullInfo == nil { + opUserGroupMemberFullInfo = &sdkws.GroupMemberFullInfo{} + } + opUserGroupMemberFullInfo.GroupID = groupInfo.GroupID + + tips := &sdkws.GroupMemberInfoSetTips{ + Group: groupInfo, + OpUser: g.migrateFallbackToGroupMemberFullInfo(opUserGroupMemberFullInfo, opPublicUser), + ChangedUser: groupMemberMap[groupInfo.GroupID], + } + + g.setSortVersion(ctx, &tips.GroupMemberVersion, &tips.GroupMemberVersionID, database.GroupMemberVersionName, tips.Group.GroupID, &tips.GroupSortVersion) + g.Notification(ctx, opUserID, groupInfo.GroupID, constant.GroupMemberInfoSetNotification, tips) + } + + return nil +} + func (g *NotificationSender) GroupMemberInfoSetNotification(ctx context.Context, groupID, groupMemberUserID string) { var err error defer func() { @@ -809,13 +985,11 @@ func (g *NotificationSender) GroupMemberInfoSetNotification(ctx context.Context, log.ZError(ctx, stringutil.GetFuncName(1)+" failed", err) } }() - var group *sdkws.GroupInfo - group, err = g.getGroupInfo(ctx, groupID) + group, err := g.getGroupInfo(ctx, groupID) if err != nil { return } - var user map[string]*sdkws.GroupMemberFullInfo - user, err = g.getGroupMemberMap(ctx, groupID, []string{groupMemberUserID}) + user, err := g.getGroupMemberMap(ctx, groupID, []string{groupMemberUserID}) if err != nil { return } @@ -834,8 +1008,7 @@ func (g *NotificationSender) GroupMemberSetToAdminNotification(ctx context.Conte log.ZError(ctx, stringutil.GetFuncName(1)+" failed", err) } }() - var group *sdkws.GroupInfo - group, err = g.getGroupInfo(ctx, groupID) + group, err := g.getGroupInfo(ctx, groupID) if err != nil { return } @@ -858,13 +1031,11 @@ func (g *NotificationSender) GroupMemberSetToOrdinaryUserNotification(ctx contex log.ZError(ctx, stringutil.GetFuncName(1)+" failed", err) } }() - var group *sdkws.GroupInfo - group, err = g.getGroupInfo(ctx, groupID) + group, err := g.getGroupInfo(ctx, groupID) if err != nil { return } - var user map[string]*sdkws.GroupMemberFullInfo - user, err = g.getGroupMemberMap(ctx, groupID, []string{mcontext.GetOpUserID(ctx), groupMemberUserID}) + user, err := g.getGroupMemberMap(ctx, groupID, []string{mcontext.GetOpUserID(ctx), groupMemberUserID}) if err != nil { return } diff --git a/pkg/common/storage/cache/group.go b/pkg/common/storage/cache/group.go index 05b75745a..4be494bf1 100644 --- a/pkg/common/storage/cache/group.go +++ b/pkg/common/storage/cache/group.go @@ -57,6 +57,7 @@ type GroupCache interface { GetGroupRoleLevelMemberInfo(ctx context.Context, groupID string, roleLevel int32) ([]*model.GroupMember, error) GetGroupRolesLevelMemberInfo(ctx context.Context, groupID string, roleLevels []int32) ([]*model.GroupMember, error) GetGroupMemberNum(ctx context.Context, groupID string) (memberNum int64, err error) + GetGroupMemberNums(ctx context.Context, groupIDs []string) (memberNumMap map[string]int64, err error) DelGroupsMemberNum(groupID ...string) GroupCache //FindSortGroupMemberUserIDs(ctx context.Context, groupID string) ([]string, error) diff --git a/pkg/common/storage/cache/redis/group.go b/pkg/common/storage/cache/redis/group.go index d66716404..a6e7693ff 100644 --- a/pkg/common/storage/cache/redis/group.go +++ b/pkg/common/storage/cache/redis/group.go @@ -2,9 +2,13 @@ package redis import ( "context" + "encoding/json" "fmt" "time" + "github.com/openimsdk/tools/utils/datautil" + "github.com/redis/go-redis/v9" + "github.com/openimsdk/open-im-server/v3/pkg/common/config" "github.com/openimsdk/open-im-server/v3/pkg/common/storage/cache" "github.com/openimsdk/open-im-server/v3/pkg/common/storage/cache/cachekey" @@ -258,6 +262,57 @@ func (g *GroupCacheRedis) GetGroupMemberNum(ctx context.Context, groupID string) }) } +type groupMemberNumCache struct { + GroupID string `json:"group_id"` + MemberNum int64 `json:"member_num"` +} + +type groupMemberNumBatchCache struct { + GroupID string + MemberNum int64 +} + +func (r *groupMemberNumBatchCache) BatchCache(groupID string) { + r.GroupID = groupID +} +func (r *groupMemberNumBatchCache) UnmarshalJSON(bytes []byte) error { + return json.Unmarshal(bytes, &r.MemberNum) +} +func (r *groupMemberNumBatchCache) MarshalJSON() ([]byte, error) { + return json.Marshal(r.MemberNum) +} +func (g *GroupCacheRedis) GetGroupMemberNums(ctx context.Context, groupIDs []string) (map[string]int64, error) { + items, err := batchGetCache2( + ctx, + g.rcClient, + g.expireTime, + groupIDs, + func(groupID string) string { return g.getGroupMemberNumKey(groupID) }, + func(v *groupMemberNumBatchCache) string { return v.GroupID }, + func(ctx context.Context, ids []string) ([]*groupMemberNumBatchCache, error) { + res := make([]*groupMemberNumBatchCache, 0, len(ids)) + for _, groupID := range ids { + num, err := g.groupMemberDB.TakeGroupMemberNum(ctx, groupID) + if err != nil { + return nil, err + } + res = append(res, &groupMemberNumBatchCache{ + GroupID: groupID, + MemberNum: num, + }) + } + return res, nil + }, + ) + if err != nil { + return nil, err + } + + return datautil.SliceToMapAny(items, func(item *groupMemberNumBatchCache) (string, int64) { + return item.GroupID, item.MemberNum + }), nil +} + func (g *GroupCacheRedis) DelGroupsMemberNum(groupID ...string) cache.GroupCache { keys := make([]string, 0, len(groupID)) for _, groupID := range groupID { @@ -280,18 +335,111 @@ func (g *GroupCacheRedis) GetGroupOwner(ctx context.Context, groupID string) (*m return members[0], nil } -func (g *GroupCacheRedis) GetGroupsOwner(ctx context.Context, groupIDs []string) ([]*model.GroupMember, error) { - members := make([]*model.GroupMember, 0, len(groupIDs)) - for _, groupID := range groupIDs { - items, err := g.GetGroupRoleLevelMemberInfo(ctx, groupID, constant.GroupOwner) - if err != nil { - return nil, err +type groupRoleLevelMemberIDsBatchCache struct { + GroupID string + UserIDs []string +} + +func (r *groupRoleLevelMemberIDsBatchCache) BatchCache(groupID string) { + r.GroupID = groupID +} +func (r *groupRoleLevelMemberIDsBatchCache) UnmarshalJSON(bytes []byte) (err error) { + return json.Unmarshal(bytes, &r.UserIDs) +} +func (r *groupRoleLevelMemberIDsBatchCache) MarshalJSON() ([]byte, error) { + return json.Marshal(r.UserIDs) +} + +func (g *GroupCacheRedis) batchGetGroupRoleLevelMemberIDs(ctx context.Context, groupIDs []string, roleLevel int32) (map[string][]string, error) { + items, err := batchGetCache2( + ctx, + g.rcClient, + g.expireTime, + groupIDs, + func(groupID string) string { + return g.getGroupRoleLevelMemberIDsKey(groupID, roleLevel) + }, + func(v *groupRoleLevelMemberIDsBatchCache) string { + return v.GroupID + }, + func(ctx context.Context, ids []string) ([]*groupRoleLevelMemberIDsBatchCache, error) { + res := make([]*groupRoleLevelMemberIDsBatchCache, 0, len(ids)) + for _, groupID := range ids { + userIDs, err := g.groupMemberDB.FindRoleLevelUserIDs(ctx, groupID, roleLevel) + if err != nil { + return nil, err + } + res = append(res, &groupRoleLevelMemberIDsBatchCache{ + GroupID: groupID, + UserIDs: userIDs, + }) + } + return res, nil + }, + ) + if err != nil { + return nil, err + } + + return datautil.SliceToMapAny(items, func(item *groupRoleLevelMemberIDsBatchCache) (string, []string) { + return item.GroupID, item.UserIDs + }), nil +} + +type groupUserIDPair struct { + GroupID string + UserID string +} + +func (g *GroupCacheRedis) batchGetGroupMembersByPairs(ctx context.Context, ids []groupUserIDPair) ([]*model.GroupMember, error) { + return batchGetCache2(ctx, g.rcClient, g.expireTime, ids, func(id groupUserIDPair) string { + return g.getGroupMemberInfoKey(id.GroupID, id.UserID) + }, func(member *model.GroupMember) groupUserIDPair { + return groupUserIDPair{GroupID: member.GroupID, UserID: member.UserID} + }, func(ctx context.Context, ids []groupUserIDPair) ([]*model.GroupMember, error) { + groupIDsByUser := make(map[string][]string) + for _, id := range ids { + groupIDsByUser[id.UserID] = append(groupIDsByUser[id.UserID], id.GroupID) } - if len(items) > 0 { - members = append(members, items[0]) + members := make([]*model.GroupMember, 0, len(ids)) + for userID, groupIDs := range groupIDsByUser { + items, err := g.groupMemberDB.FindInGroup(ctx, userID, groupIDs) + if err != nil { + return nil, err + } + members = append(members, items...) + } + return members, nil + }) +} + +func (g *GroupCacheRedis) GetGroupsOwner(ctx context.Context, groupIDs []string) ([]*model.GroupMember, error) { + ownerIDs, err := g.batchGetGroupRoleLevelMemberIDs(ctx, groupIDs, constant.GroupOwner) + if err != nil { + return nil, err + } + pairs := make([]groupUserIDPair, 0, len(groupIDs)) + for _, groupID := range groupIDs { + ids := ownerIDs[groupID] + if len(ids) == 0 { + continue + } + pairs = append(pairs, groupUserIDPair{GroupID: groupID, UserID: ids[0]}) + } + members, err := g.batchGetGroupMembersByPairs(ctx, pairs) + if err != nil { + return nil, err + } + memberMap := datautil.SliceToMapAny(members, func(member *model.GroupMember) (groupUserIDPair, *model.GroupMember) { + return groupUserIDPair{GroupID: member.GroupID, UserID: member.UserID}, member + }) + result := make([]*model.GroupMember, 0, len(pairs)) + for _, pair := range pairs { + if member, ok := memberMap[pair]; ok { + result = append(result, member) } } - return members, nil + return result, nil } func (g *GroupCacheRedis) GetGroupRoleLevelMemberIDs(ctx context.Context, groupID string, roleLevel int32) ([]string, error) { diff --git a/pkg/common/storage/controller/group.go b/pkg/common/storage/controller/group.go index 24209cd6c..17de62444 100644 --- a/pkg/common/storage/controller/group.go +++ b/pkg/common/storage/controller/group.go @@ -65,6 +65,8 @@ type GroupDatabase interface { FindGroupMemberUserID(ctx context.Context, groupID string) ([]string, error) // FindGroupMemberNum retrieves the number of members in a group. FindGroupMemberNum(ctx context.Context, groupID string) (uint32, error) + // FindGroupMemberNums retrieves the number of members for multiple groups. + FindGroupMemberNums(ctx context.Context, groupIDs []string) (map[string]uint32, error) // FindUserManagedGroupID retrieves group IDs managed by a user. FindUserManagedGroupID(ctx context.Context, userID string) (groupIDs []string, err error) // PageGroupRequest paginates through group requests for specified groups. @@ -232,6 +234,18 @@ func (g *groupDatabase) FindGroupMemberNum(ctx context.Context, groupID string) return uint32(num), nil } +func (g *groupDatabase) FindGroupMemberNums(ctx context.Context, groupIDs []string) (map[string]uint32, error) { + nums, err := g.cache.GetGroupMemberNums(ctx, groupIDs) + if err != nil { + return nil, err + } + result := make(map[string]uint32, len(nums)) + for groupID, num := range nums { + result[groupID] = uint32(num) + } + return result, nil +} + func (g *groupDatabase) TakeGroup(ctx context.Context, groupID string) (*model.Group, error) { return g.cache.GetGroupInfo(ctx, groupID) } From 18e0ca78734ca0612e53f72bb7502b2d67a2008b Mon Sep 17 00:00:00 2001 From: dsx137 <70027572+dsx137@users.noreply.github.com> Date: Fri, 20 Mar 2026 11:24:20 +0800 Subject: [PATCH 2/2] fix: import --- internal/rpc/group/group.go | 1 - pkg/common/storage/cache/redis/group.go | 1 - 2 files changed, 2 deletions(-) diff --git a/internal/rpc/group/group.go b/internal/rpc/group/group.go index 49063a13a..cc51fdf18 100644 --- a/internal/rpc/group/group.go +++ b/internal/rpc/group/group.go @@ -28,7 +28,6 @@ import ( "github.com/openimsdk/open-im-server/v3/pkg/dbbuild" "github.com/openimsdk/open-im-server/v3/pkg/rpcli" - "google.golang.org/grpc" "github.com/openimsdk/open-im-server/v3/pkg/authverify" "github.com/openimsdk/open-im-server/v3/pkg/callbackstruct" diff --git a/pkg/common/storage/cache/redis/group.go b/pkg/common/storage/cache/redis/group.go index a6e7693ff..2c727666b 100644 --- a/pkg/common/storage/cache/redis/group.go +++ b/pkg/common/storage/cache/redis/group.go @@ -18,7 +18,6 @@ import ( "github.com/openimsdk/protocol/constant" "github.com/openimsdk/tools/errs" "github.com/openimsdk/tools/log" - "github.com/redis/go-redis/v9" ) const (