From a9508b25463ff7723bb1c2a1954c23871fdb1a74 Mon Sep 17 00:00:00 2001 From: varsnotwars Date: Thu, 14 Aug 2025 02:06:23 +1000 Subject: [PATCH] chore: simplify convert reaction (#5001) --- server/router/api/v1/memo_service.go | 125 +++++++++++++----- .../router/api/v1/memo_service_converter.go | 28 +--- server/router/api/v1/reaction_service.go | 24 +--- 3 files changed, 106 insertions(+), 71 deletions(-) diff --git a/server/router/api/v1/memo_service.go b/server/router/api/v1/memo_service.go index 85ce0748d..378ea7bcf 100644 --- a/server/router/api/v1/memo_service.go +++ b/server/router/api/v1/memo_service.go @@ -82,7 +82,7 @@ func (s *APIV1Service) CreateMemo(ctx context.Context, request *v1pb.CreateMemoR } } - memoMessage, err := s.convertMemoFromStore(ctx, memo, nil) + memoMessage, err := s.convertMemoFromStore(ctx, memo, []*store.Reaction{}) if err != nil { return nil, errors.Wrap(err, "failed to convert memo") } @@ -179,32 +179,39 @@ func (s *APIV1Service) ListMemos(ctx context.Context, request *v1pb.ListMemosReq } } - reactionMap := make(map[string][]*store.Reaction) + if len(memos) == 0 { + response := &v1pb.ListMemosResponse{ + Memos: memoMessages, + NextPageToken: nextPageToken, + } + return response, nil + } + reactionMap := make(map[string][]*store.Reaction) memoNames := make([]string, 0, len(memos)) + for _, m := range memos { - memoNames = append(memoNames, fmt.Sprintf("'%s/%s'", MemoNamePrefix, m.UID)) + memoNames = append(memoNames, fmt.Sprintf("'%s%s'", MemoNamePrefix, m.UID)) } - if len(memoNames) > 0 { - reactions, err := s.Store.ListReactions(ctx, &store.FindReaction{ - Filters: []string{fmt.Sprintf("content_id in [%s]", strings.Join(memoNames, ", "))}, - }) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to list reactions") - } - - for _, reaction := range reactions { - reactionMap[reaction.ContentID] = append(reactionMap[reaction.ContentID], reaction) - } + // REACTIONS + reactions, err := s.Store.ListReactions(ctx, &store.FindReaction{ + Filters: []string{fmt.Sprintf("content_id in [%s]", strings.Join(memoNames, ", "))}, + }) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to list reactions") + } + for _, reaction := range reactions { + reactionMap[reaction.ContentID] = append(reactionMap[reaction.ContentID], reaction) } for _, memo := range memos { - name := fmt.Sprintf("'%s/%s'", MemoNamePrefix, memo.UID) + name := fmt.Sprintf("%s%s", MemoNamePrefix, memo.UID) memoMessage, err := s.convertMemoFromStore(ctx, memo, reactionMap[name]) if err != nil { return nil, errors.Wrap(err, "failed to convert memo") } + memoMessages = append(memoMessages, memoMessage) } @@ -242,7 +249,14 @@ func (s *APIV1Service) GetMemo(ctx context.Context, request *v1pb.GetMemoRequest } } - memoMessage, err := s.convertMemoFromStore(ctx, memo, nil) + reactions, err := s.Store.ListReactions(ctx, &store.FindReaction{ + ContentID: &request.Name, + }) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to list reactions") + } + + memoMessage, err := s.convertMemoFromStore(ctx, memo, reactions) if err != nil { return nil, errors.Wrap(err, "failed to convert memo") } @@ -361,7 +375,14 @@ func (s *APIV1Service) UpdateMemo(ctx context.Context, request *v1pb.UpdateMemoR if err != nil { return nil, errors.Wrap(err, "failed to get memo") } - memoMessage, err := s.convertMemoFromStore(ctx, memo, nil) + reactions, err := s.Store.ListReactions(ctx, &store.FindReaction{ + ContentID: &request.Memo.Name, + }) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to list reactions") + } + + memoMessage, err := s.convertMemoFromStore(ctx, memo, reactions) if err != nil { return nil, errors.Wrap(err, "failed to convert memo") } @@ -397,7 +418,14 @@ func (s *APIV1Service) DeleteMemo(ctx context.Context, request *v1pb.DeleteMemoR return nil, status.Errorf(codes.PermissionDenied, "permission denied") } - if memoMessage, err := s.convertMemoFromStore(ctx, memo, nil); err == nil { + reactions, err := s.Store.ListReactions(ctx, &store.FindReaction{ + ContentID: &request.Name, + }) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to list reactions") + } + + if memoMessage, err := s.convertMemoFromStore(ctx, memo, reactions); err == nil { // Try to dispatch webhook when memo is deleted. if err := s.DispatchMemoDeletedWebhook(ctx, memoMessage); err != nil { slog.Warn("Failed to dispatch memo deleted webhook", slog.Any("err", err)) @@ -543,25 +571,58 @@ func (s *APIV1Service) ListMemoComments(ctx context.Context, request *v1pb.ListM return nil, status.Errorf(codes.Internal, "failed to list memo relations") } - var memos []*v1pb.Memo - for _, memoRelation := range memoRelations { - memo, err := s.Store.GetMemo(ctx, &store.FindMemo{ - ID: &memoRelation.MemoID, - }) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to get memo") + if len(memoRelations) == 0 { + response := &v1pb.ListMemoCommentsResponse{ + Memos: []*v1pb.Memo{}, } - if memo != nil { - memoMessage, err := s.convertMemoFromStore(ctx, memo, nil) - if err != nil { - return nil, errors.Wrap(err, "failed to convert memo") - } - memos = append(memos, memoMessage) + return response, nil + } + + memoRelationIDs := make([]string, 0, len(memoRelations)) + for _, m := range memoRelations { + memoRelationIDs = append(memoRelationIDs, fmt.Sprintf("%d", m.MemoID)) + } + memos, err := s.Store.ListMemos(ctx, &store.FindMemo{ + Filters: []string{fmt.Sprintf("id in [%s]", strings.Join(memoRelationIDs, ", "))}, + }) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to list memos") + } + + memoIDToNameMap := make(map[int32]string) + memoNamesForQuery := make([]string, 0, len(memos)) + + for _, memo := range memos { + memoName := fmt.Sprintf("%s%s", MemoNamePrefix, memo.UID) + memoIDToNameMap[memo.ID] = memoName + memoNamesForQuery = append(memoNamesForQuery, fmt.Sprintf("'%s'", memoName)) + } + reactions, err := s.Store.ListReactions(ctx, &store.FindReaction{ + Filters: []string{fmt.Sprintf("content_id in [%s]", strings.Join(memoNamesForQuery, ", "))}, + }) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to list reactions") + } + + memoReactionsMap := make(map[string][]*store.Reaction) + for _, reaction := range reactions { + memoReactionsMap[reaction.ContentID] = append(memoReactionsMap[reaction.ContentID], reaction) + } + + var memosResponse []*v1pb.Memo + for _, m := range memos { + memoName := memoIDToNameMap[m.ID] + reactions := memoReactionsMap[memoName] + + memoMessage, err := s.convertMemoFromStore(ctx, m, reactions) + if err != nil { + return nil, errors.Wrap(err, "failed to convert memo") } + memosResponse = append(memosResponse, memoMessage) } response := &v1pb.ListMemoCommentsResponse{ - Memos: memos, + Memos: memosResponse, } return response, nil } diff --git a/server/router/api/v1/memo_service_converter.go b/server/router/api/v1/memo_service_converter.go index bf4b3febc..06500e274 100644 --- a/server/router/api/v1/memo_service_converter.go +++ b/server/router/api/v1/memo_service_converter.go @@ -6,8 +6,6 @@ import ( "time" "github.com/pkg/errors" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/timestamppb" "github.com/usememos/gomark/parser" @@ -51,6 +49,13 @@ func (s *APIV1Service) convertMemoFromStore(ctx context.Context, memo *store.Mem memoMessage.Parent = &parentName } + memoMessage.Reactions = []*v1pb.Reaction{} + + for _, reaction := range reactions { + reactionResponse := convertReactionFromStore(reaction) + memoMessage.Reactions = append(memoMessage.Reactions, reactionResponse) + } + listMemoRelationsResponse, err := s.ListMemoRelations(ctx, &v1pb.ListMemoRelationsRequest{Name: name}) if err != nil { return nil, errors.Wrap(err, "failed to list memo relations") @@ -63,25 +68,6 @@ func (s *APIV1Service) convertMemoFromStore(ctx context.Context, memo *store.Mem } memoMessage.Attachments = listMemoAttachmentsResponse.Attachments - if len(reactions) > 0 { - for _, reaction := range reactions { - reactionMessage, err := s.convertReactionFromStore(ctx, reaction) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to convert reaction") - } - memoMessage.Reactions = append(memoMessage.Reactions, reactionMessage) - } - } else { - // done for backwards compatibility - // can remove once convertMemoFromStore is only responsible for mapping - // and all related DB entities are passed in as arguments purely for converting to request entities - listMemoReactionsResponse, err := s.ListMemoReactions(ctx, &v1pb.ListMemoReactionsRequest{Name: name}) - if err != nil { - return nil, errors.Wrap(err, "failed to list memo reactions") - } - memoMessage.Reactions = listMemoReactionsResponse.Reactions - } - nodes, err := parser.Parse(tokenizer.Tokenize(memo.Content)) if err != nil { return nil, errors.Wrap(err, "failed to parse content") diff --git a/server/router/api/v1/reaction_service.go b/server/router/api/v1/reaction_service.go index cd98634e7..561d776ad 100644 --- a/server/router/api/v1/reaction_service.go +++ b/server/router/api/v1/reaction_service.go @@ -26,10 +26,7 @@ func (s *APIV1Service) ListMemoReactions(ctx context.Context, request *v1pb.List Reactions: []*v1pb.Reaction{}, } for _, reaction := range reactions { - reactionMessage, err := s.convertReactionFromStore(ctx, reaction) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to convert reaction") - } + reactionMessage := convertReactionFromStore(reaction) response.Reactions = append(response.Reactions, reactionMessage) } return response, nil @@ -49,10 +46,8 @@ func (s *APIV1Service) UpsertMemoReaction(ctx context.Context, request *v1pb.Ups return nil, status.Errorf(codes.Internal, "failed to upsert reaction") } - reactionMessage, err := s.convertReactionFromStore(ctx, reaction) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to convert reaction") - } + reactionMessage := convertReactionFromStore(reaction) + return reactionMessage, nil } @@ -71,20 +66,13 @@ func (s *APIV1Service) DeleteMemoReaction(ctx context.Context, request *v1pb.Del return &emptypb.Empty{}, nil } -func (s *APIV1Service) convertReactionFromStore(ctx context.Context, reaction *store.Reaction) (*v1pb.Reaction, error) { - creator, err := s.Store.GetUser(ctx, &store.FindUser{ - ID: &reaction.CreatorID, - }) - if err != nil { - return nil, err - } - +func convertReactionFromStore(reaction *store.Reaction) *v1pb.Reaction { reactionUID := fmt.Sprintf("%d", reaction.ID) return &v1pb.Reaction{ Name: fmt.Sprintf("%s%s", ReactionNamePrefix, reactionUID), - Creator: fmt.Sprintf("%s%d", UserNamePrefix, creator.ID), + Creator: fmt.Sprintf("%s%d", UserNamePrefix, reaction.CreatorID), ContentId: reaction.ContentID, ReactionType: reaction.ReactionType, CreateTime: timestamppb.New(time.Unix(reaction.CreatedTs, 0)), - }, nil + } }