From 7c25c91e9bc7718e60600342d58247d22e7f7d54 Mon Sep 17 00:00:00 2001 From: Xinwei Xiong <3293172751NSS@gmail.com> Date: Mon, 18 Mar 2024 10:34:48 +0800 Subject: [PATCH] feat: golang fix apt test design (#2084) * feat: make lint format * feat: make lint format * feat: make lint format * fix: fix make lint * feat: add scripts verify shell check * feat: add scripts verify shell check --- .golangci.yml | 2 +- docker-compose.yml | 17 ---- internal/api/msg.go | 7 +- internal/msggateway/context.go | 3 +- internal/msggateway/user_map.go | 86 +++++++++++-------- .../msgtransfer/online_history_msg_handler.go | 9 +- internal/push/offlinepush/getui/push.go | 3 +- internal/push/push_rpc_server.go | 3 +- internal/push/tools.go | 3 +- internal/rpc/friend/black.go | 7 +- internal/rpc/friend/friend.go | 16 +++- internal/rpc/group/group.go | 1 + pkg/rpcclient/notification/friend.go | 5 +- scripts/install/test.sh | 3 +- 14 files changed, 86 insertions(+), 79 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9c7960642..c262cfa2f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -745,7 +745,7 @@ linters: - misspell # Spelling mistakes - staticcheck # Static analysis - unused # Checks for unused code - - goimports # Checks if imports are correctly sorted and formatted + # - goimports # Checks if imports are correctly sorted and formatted - godot # Checks for comment punctuation - bodyclose # Ensures HTTP response body is closed - stylecheck # Style checker for Go code diff --git a/docker-compose.yml b/docker-compose.yml index 8538eec83..ef0714329 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -186,23 +186,6 @@ services: # server: # ipv4_address: ${OPENIM_SERVER_NETWORK_ADDRESS:-172.28.0.8} -### TODO: mysql is required to deploy the openim-chat component - # mysql: - # image: mysql:${MYSQL_IMAGE_VERSION:-5.7} - # platform: linux/amd64 - # ports: - # - "${MYSQL_PORT:-13306}:3306" - # container_name: mysql - # volumes: - # - "${DATA_DIR:-./}/components/mysql/data:/var/lib/mysql" - # - "/etc/localtime:/etc/localtime" - # environment: - # MYSQL_ROOT_PASSWORD: "${MYSQL_PASSWORD:-openIM123}" - # restart: always - # networks: - # server: - # ipv4_address: ${MYSQL_NETWORK_ADDRESS:-172.28.0.15} - # openim-chat: # image: ${IMAGE_REGISTRY:-ghcr.io/openimsdk}/openim-chat:${CHAT_IMAGE_VERSION:-main} # container_name: openim-chat diff --git a/internal/api/msg.go b/internal/api/msg.go index d38c14d4e..ad5001459 100644 --- a/internal/api/msg.go +++ b/internal/api/msg.go @@ -215,25 +215,22 @@ func (m *MessageApi) SendMessage(c *gin.Context) { // Set the receiver ID in the message data. sendMsgReq.MsgData.RecvID = req.RecvID - // Declare a variable to store the message sending status. - var status int - // Attempt to send the message using the client. respPb, err := m.Client.SendMsg(c, sendMsgReq) if err != nil { // Set the status to failed and respond with an error if sending fails. - status = constant.MsgSendFailed apiresp.GinError(c, err) return } // Set the status to successful if the message is sent. - status = constant.MsgSendSuccessed + var status int = constant.MsgSendSuccessed // Attempt to update the message sending status in the system. _, err = m.Client.SetSendMsgStatus(c, &msg.SetSendMsgStatusReq{ Status: int32(status), }) + if err != nil { // Log the error if updating the status fails. apiresp.GinError(c, err) diff --git a/internal/msggateway/context.go b/internal/msggateway/context.go index 85fe5c734..ad679c1a1 100644 --- a/internal/msggateway/context.go +++ b/internal/msggateway/context.go @@ -141,7 +141,6 @@ func (c *UserConnContext) GetBackground() bool { b, err := strconv.ParseBool(c.Req.URL.Query().Get(BackgroundStatus)) if err != nil { return false - } else { - return b } + return b } diff --git a/internal/msggateway/user_map.go b/internal/msggateway/user_map.go index b4cec59fa..89eb20de6 100644 --- a/internal/msggateway/user_map.go +++ b/internal/msggateway/user_map.go @@ -19,7 +19,6 @@ import ( "sync" "github.com/OpenIMSDK/tools/log" - "github.com/OpenIMSDK/tools/utils" ) type UserMap struct { @@ -71,48 +70,65 @@ func (u *UserMap) Set(key string, v *Client) { } func (u *UserMap) delete(key string, connRemoteAddr string) (isDeleteUser bool) { + // Attempt to load the clients associated with the key. allClients, existed := u.m.Load(key) - if existed { - oldClients := allClients.([]*Client) - var a []*Client - for _, client := range oldClients { - if client.ctx.GetRemoteAddr() != connRemoteAddr { - a = append(a, client) - } - } - if len(a) == 0 { - u.m.Delete(key) - return true - } else { - u.m.Store(key, a) - return false + if !existed { + // Return false immediately if the key does not exist. + return false + } + + // Convert allClients to a slice of *Client. + oldClients := allClients.([]*Client) + var remainingClients []*Client + for _, client := range oldClients { + // Keep clients that do not match the connRemoteAddr. + if client.ctx.GetRemoteAddr() != connRemoteAddr { + remainingClients = append(remainingClients, client) } } - return existed + + // If no clients remain after filtering, delete the key from the map. + if len(remainingClients) == 0 { + u.m.Delete(key) + return true + } + + // Otherwise, update the key with the remaining clients. + u.m.Store(key, remainingClients) + return false } -func (u *UserMap) deleteClients(key string, clients []*Client) (isDeleteUser bool) { - m := utils.SliceToMapAny(clients, func(c *Client) (string, struct{}) { - return c.ctx.GetRemoteAddr(), struct{}{} - }) +func (u *UserMap) deleteClients(key string, clientsToDelete []*Client) (isDeleteUser bool) { + // Convert the slice of clients to delete into a map for efficient lookup. + deleteMap := make(map[string]struct{}) + for _, client := range clientsToDelete { + deleteMap[client.ctx.GetRemoteAddr()] = struct{}{} + } + + // Load the current clients associated with the key. allClients, existed := u.m.Load(key) - if existed { - oldClients := allClients.([]*Client) - var a []*Client - for _, client := range oldClients { - if _, ok := m[client.ctx.GetRemoteAddr()]; !ok { - a = append(a, client) - } - } - if len(a) == 0 { - u.m.Delete(key) - return true - } else { - u.m.Store(key, a) - return false + if !existed { + // If the key doesn't exist, return false. + return false + } + + // Filter out clients that are in the deleteMap. + oldClients := allClients.([]*Client) + var remainingClients []*Client + for _, client := range oldClients { + if _, shouldBeDeleted := deleteMap[client.ctx.GetRemoteAddr()]; !shouldBeDeleted { + remainingClients = append(remainingClients, client) } } - return existed + + // Update or delete the key based on the remaining clients. + if len(remainingClients) == 0 { + u.m.Delete(key) + return true + } + + u.m.Store(key, remainingClients) + return false } func (u *UserMap) DeleteAll(key string) { diff --git a/internal/msgtransfer/online_history_msg_handler.go b/internal/msgtransfer/online_history_msg_handler.go index b81bd12b8..50fc93369 100644 --- a/internal/msgtransfer/online_history_msg_handler.go +++ b/internal/msgtransfer/online_history_msg_handler.go @@ -184,12 +184,11 @@ func (och *OnlineHistoryRedisConsumerHandler) getPushStorageMsgList( options2 := msgprocessor.Options(msg.Options) if options2.IsHistory() { return true - } else { - // if !(!options2.IsSenderSync() && conversationID == msg.MsgData.SendID) { - // return false - // } - return false } + // if !(!options2.IsSenderSync() && conversationID == msg.MsgData.SendID) { + // return false + // } + return false } for _, v := range totalMsgs { options := msgprocessor.Options(v.message.Options) diff --git a/internal/push/offlinepush/getui/push.go b/internal/push/offlinepush/getui/push.go index 67f6292db..1a95727e5 100644 --- a/internal/push/offlinepush/getui/push.go +++ b/internal/push/offlinepush/getui/push.go @@ -90,9 +90,8 @@ func (g *Client) Push(ctx context.Context, userIDs []string, title, content stri for i, v := range s.GetSplitResult() { go func(index int, userIDs []string) { defer wg.Done() - if err := g.batchPush(ctx, token, userIDs, pushReq); err != nil { + if err = g.batchPush(ctx, token, userIDs, pushReq); err != nil { log.ZError(ctx, "batchPush failed", err, "index", index, "token", token, "req", pushReq) - err = err } }(i, v.Item) } diff --git a/internal/push/push_rpc_server.go b/internal/push/push_rpc_server.go index a8bb18974..b68b06666 100644 --- a/internal/push/push_rpc_server.go +++ b/internal/push/push_rpc_server.go @@ -90,9 +90,8 @@ func (r *pushServer) PushMsg(ctx context.Context, pbData *pbpush.PushMsgReq) (re if err != nil { if err != errNoOfflinePusher { return nil, err - } else { - log.ZWarn(ctx, "offline push failed", err, "msg", pbData.String()) } + log.ZWarn(ctx, "offline push failed", err, "msg", pbData.String()) } return &pbpush.PushMsgResp{}, nil } diff --git a/internal/push/tools.go b/internal/push/tools.go index 3242767b1..760c8c95b 100644 --- a/internal/push/tools.go +++ b/internal/push/tools.go @@ -26,7 +26,6 @@ func GetContent(msg *sdkws.MsgData) string { _ = proto.Unmarshal(msg.Content, &tips) content := tips.JsonDetail return content - } else { - return string(msg.Content) } + return string(msg.Content) } diff --git a/internal/rpc/friend/black.go b/internal/rpc/friend/black.go index 64c63eb73..4bb0d9f58 100644 --- a/internal/rpc/friend/black.go +++ b/internal/rpc/friend/black.go @@ -79,9 +79,14 @@ func (s *friendServer) AddBlack(ctx context.Context, req *pbfriend.AddBlackReq) CreateTime: time.Now(), Ex: req.Ex, } + if err := s.blackDatabase.Create(ctx, []*relation.BlackModel{&black}); err != nil { return nil, err } - s.notificationSender.BlackAddedNotification(ctx, req) + + if err := s.notificationSender.BlackAddedNotification(ctx, req); err != nil { + return nil, err + } + return &pbfriend.AddBlackResp{}, nil } diff --git a/internal/rpc/friend/friend.go b/internal/rpc/friend/friend.go index 6403a4159..4df4085a9 100644 --- a/internal/rpc/friend/friend.go +++ b/internal/rpc/friend/friend.go @@ -114,26 +114,36 @@ func (s *friendServer) ApplyToAddFriend(ctx context.Context, req *pbfriend.Apply if err := authverify.CheckAccessV3(ctx, req.FromUserID, s.config); err != nil { return nil, err } + if req.ToUserID == req.FromUserID { return nil, errs.ErrCanNotAddYourself.Wrap("req.ToUserID", req.ToUserID) } + if err = CallbackBeforeAddFriend(ctx, s.config, req); err != nil && err != errs.ErrCallbackContinue { return nil, err } + if _, err := s.userRpcClient.GetUsersInfoMap(ctx, []string{req.ToUserID, req.FromUserID}); err != nil { return nil, err } + in1, in2, err := s.friendDatabase.CheckIn(ctx, req.FromUserID, req.ToUserID) if err != nil { return nil, err } + if in1 && in2 { return nil, errs.ErrRelationshipAlready.Wrap() } + if err = s.friendDatabase.AddFriendRequest(ctx, req.FromUserID, req.ToUserID, req.ReqMsg, req.Ex); err != nil { return nil, err } - s.notificationSender.FriendApplicationAddNotification(ctx, req) + + if err = s.notificationSender.FriendApplicationAddNotification(ctx, req); err != nil { + return nil, err + } + if err = CallbackAfterAddFriend(ctx, s.config, req); err != nil && err != errs.ErrCallbackContinue { return nil, err } @@ -197,7 +207,9 @@ func (s *friendServer) RespondFriendApply(ctx context.Context, req *pbfriend.Res if err != nil { return nil, err } - s.notificationSender.FriendApplicationAgreedNotification(ctx, req) + if err := s.notificationSender.FriendApplicationAgreedNotification(ctx, req); err != nil { + return nil, err + } return resp, nil } if req.HandleResult == constant.FriendResponseRefuse { diff --git a/internal/rpc/group/group.go b/internal/rpc/group/group.go index d966bfad8..7ebd237b5 100644 --- a/internal/rpc/group/group.go +++ b/internal/rpc/group/group.go @@ -637,6 +637,7 @@ func (s *groupServer) GetGroupMembersInfo(ctx context.Context, req *pbgroup.GetG return resp, nil } +// GetGroupApplicationList handles functions that get a list of group requests func (s *groupServer) GetGroupApplicationList(ctx context.Context, req *pbgroup.GetGroupApplicationListReq) (*pbgroup.GetGroupApplicationListResp, error) { groupIDs, err := s.db.FindUserManagedGroupID(ctx, req.FromUserID) if err != nil { diff --git a/pkg/rpcclient/notification/friend.go b/pkg/rpcclient/notification/friend.go index dafca055a..751bdf475 100644 --- a/pkg/rpcclient/notification/friend.go +++ b/pkg/rpcclient/notification/friend.go @@ -126,10 +126,7 @@ func (f *FriendNotificationSender) UserInfoUpdatedNotification(ctx context.Conte return f.Notification(ctx, mcontext.GetOpUserID(ctx), changedUserID, constant.UserInfoUpdatedNotification, &tips) } -func (f *FriendNotificationSender) FriendApplicationAddNotification( - ctx context.Context, - req *pbfriend.ApplyToAddFriendReq, -) error { +func (f *FriendNotificationSender) FriendApplicationAddNotification(ctx context.Context, req *pbfriend.ApplyToAddFriendReq) error { tips := sdkws.FriendApplicationTips{FromToUserID: &sdkws.FromToUserID{ FromUserID: req.FromUserID, ToUserID: req.ToUserID, diff --git a/scripts/install/test.sh b/scripts/install/test.sh index 326307570..51d541ece 100755 --- a/scripts/install/test.sh +++ b/scripts/install/test.sh @@ -932,7 +932,7 @@ openim::test::set_group_info() { { "groupInfoForSet": { "groupID": "${1}", - "groupName": "new-name", + "groupName": "new group name", "notification": "new notification", "introduction": "new introduction", "faceURL": "www.newfaceURL.com", @@ -1076,6 +1076,7 @@ function openim::test::group() { local GROUP_ID=$RANDOM local GROUP_ID2=$RANDOM + # Assumes that TEST_GROUP_ID, USER_ID, and other necessary IDs are set as environment variables before running this suite. # 0. Register a friend user. openim::test::user_register "${USER_ID}" "group00" "new_face_url"