chore: simplify convert reaction (#5001)

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
 }

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")

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
+	}
 }