happyz
/
memos
mirror of https://github.com/usememos/memos.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(api): tolerate missing related users in memo conversions (#5809)

This commit is contained in:
boojack 2026-04-06 08:23:18 +08:00 committed by GitHub
parent 87d411bc70
commit 25feef3aad
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 392 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
server/router/api/v1/memo_service.go
View File

@ -2,6 +2,7 @@ package v1
import (
"context"
stderrors "errors"
"fmt"
"log/slog"
"strings"
@ -312,6 +313,14 @@ func (s *APIV1Service) ListMemos(ctx context.Context, request *v1pb.ListMemosReq
memoMessage, err := s.convertMemoFromStoreWithCreators(ctx, memo, reactions, attachments, relations, creatorMap)
if err != nil {
if stderrors.Is(err, errMemoCreatorNotFound) {
slog.Warn("Skipping memo with missing creator",
slog.Int64("memo_id", int64(memo.ID)),
slog.String("memo_uid", memo.UID),
slog.Int64("creator_id", int64(memo.CreatorID)),
)
continue
}
return nil, errors.Wrap(err, "failed to convert memo")
}
@ -384,6 +393,9 @@ func (s *APIV1Service) GetMemo(ctx context.Context, request *v1pb.GetMemoRequest
}
memoMessage, err := s.convertMemoFromStore(ctx, memo, reactions, attachments, relations)
if err != nil {
if stderrors.Is(err, errMemoCreatorNotFound) {
return nil, status.Errorf(codes.NotFound, "memo creator not found")
}
return nil, errors.Wrap(err, "failed to convert memo")
}
return memoMessage, nil
@ -767,6 +779,15 @@ func (s *APIV1Service) ListMemoComments(ctx context.Context, request *v1pb.ListM
memoMessage, err := s.convertMemoFromStoreWithCreators(ctx, m, reactions, attachments, relations, creatorMap)
if err != nil {
if stderrors.Is(err, errMemoCreatorNotFound) {
slog.Warn("Skipping memo comment with missing creator",
slog.Int64("memo_id", int64(m.ID)),
slog.String("memo_uid", m.UID),
slog.Int64("creator_id", int64(m.CreatorID)),
slog.String("parent_name", request.Name),
)
continue
}
return nil, errors.Wrap(err, "failed to convert memo")
}
memosResponse = append(memosResponse, memoMessage)

105
server/router/api/v1/memo_service_converter.go
View File

@ -2,7 +2,9 @@ package v1
import (
"context"
stderrors "errors"
"fmt"
"log/slog"
"time"
"github.com/pkg/errors"
@ -13,6 +15,11 @@ import (
"github.com/usememos/memos/store"
)
var (
errMemoCreatorNotFound = stderrors.New("memo creator not found")
errReactionCreatorNotFound = stderrors.New("reaction creator not found")
)
func (s *APIV1Service) convertMemoFromStore(ctx context.Context, memo *store.Memo, reactions []*store.Reaction, attachments []*store.Attachment, relations []*v1pb.MemoRelation) (*v1pb.Memo, error) {
creatorMap, err := s.listUsersByID(ctx, []int32{memo.CreatorID})
if err != nil {
@ -34,7 +41,7 @@ func (s *APIV1Service) convertMemoFromStoreWithCreators(ctx context.Context, mem
name := fmt.Sprintf("%s%s", MemoNamePrefix, memo.UID)
creator := creatorMap[memo.CreatorID]
if creator == nil {
return nil, errors.New("memo creator not found")
return nil, errMemoCreatorNotFound
}
memoMessage := &v1pb.Memo{
Name: name,
@ -58,13 +65,9 @@ func (s *APIV1Service) convertMemoFromStoreWithCreators(ctx context.Context, mem
memoMessage.Parent = &parentName
}
memoMessage.Reactions = []*v1pb.Reaction{}
for _, reaction := range reactions {
reactionResponse, err := s.convertReactionFromStore(ctx, reaction)
if err != nil {
return nil, errors.Wrap(err, "failed to convert reaction")
}
memoMessage.Reactions = append(memoMessage.Reactions, reactionResponse)
memoMessage.Reactions, err = s.convertReactionsFromStoreWithCreators(ctx, reactions, creatorMap)
if err != nil {
return nil, errors.Wrap(err, "failed to convert reactions")
}
if relations != nil {
@ -88,6 +91,92 @@ func (s *APIV1Service) convertMemoFromStoreWithCreators(ctx context.Context, mem
return memoMessage, nil
}
func (s *APIV1Service) listUsersByIDWithExisting(ctx context.Context, userIDs []int32, existing map[int32]*store.User) (map[int32]*store.User, error) {
usersByID := make(map[int32]*store.User, len(existing)+len(userIDs))
for userID, user := range existing {
if user != nil {
usersByID[userID] = user
}
}
missingUserIDs := make([]int32, 0, len(userIDs))
seenMissingUserIDs := make(map[int32]struct{}, len(userIDs))
for _, userID := range userIDs {
if _, ok := usersByID[userID]; ok {
continue
}
if _, ok := seenMissingUserIDs[userID]; ok {
continue
}
seenMissingUserIDs[userID] = struct{}{}
missingUserIDs = append(missingUserIDs, userID)
}
if len(missingUserIDs) == 0 {
return usersByID, nil
}
missingUsersByID, err := s.listUsersByID(ctx, missingUserIDs)
if err != nil {
return nil, err
}
for userID, user := range missingUsersByID {
if user != nil {
usersByID[userID] = user
}
}
return usersByID, nil
}
func (s *APIV1Service) convertReactionsFromStoreWithCreators(ctx context.Context, reactions []*store.Reaction, creatorMap map[int32]*store.User) ([]*v1pb.Reaction, error) {
if len(reactions) == 0 {
return []*v1pb.Reaction{}, nil
}
creatorIDs := make([]int32, 0, len(reactions))
for _, reaction := range reactions {
creatorIDs = append(creatorIDs, reaction.CreatorID)
}
creatorsByID, err := s.listUsersByIDWithExisting(ctx, creatorIDs, creatorMap)
if err != nil {
return nil, err
}
reactionMessages := make([]*v1pb.Reaction, 0, len(reactions))
for _, reaction := range reactions {
reactionMessage, err := convertReactionFromStoreWithCreators(reaction, creatorsByID)
if err != nil {
if stderrors.Is(err, errReactionCreatorNotFound) {
slog.Warn("Skipping reaction with missing creator",
slog.Int64("reaction_id", int64(reaction.ID)),
slog.Int64("creator_id", int64(reaction.CreatorID)),
slog.String("content_id", reaction.ContentID),
)
continue
}
return nil, err
}
reactionMessages = append(reactionMessages, reactionMessage)
}
return reactionMessages, nil
}
func convertReactionFromStoreWithCreators(reaction *store.Reaction, creatorsByID map[int32]*store.User) (*v1pb.Reaction, error) {
creator := creatorsByID[reaction.CreatorID]
if creator == nil {
return nil, errReactionCreatorNotFound
}
reactionUID := fmt.Sprintf("%d", reaction.ID)
return &v1pb.Reaction{
Name: fmt.Sprintf("%s/%s%s", reaction.ContentID, ReactionNamePrefix, reactionUID),
Creator: BuildUserName(creator.Username),
ContentId: reaction.ContentID,
ReactionType: reaction.ReactionType,
CreateTime: timestamppb.New(time.Unix(reaction.CreatedTs, 0)),
}, nil
}
// batchConvertMemoRelations batch-loads relations for a list of memos and returns
// a map from memo ID to its converted relations. This avoids N+1 queries when listing memos.
func (s *APIV1Service) batchConvertMemoRelations(ctx context.Context, memos []*store.Memo) (map[int32][]*v1pb.MemoRelation, error) {

4
server/router/api/v1/memo_share_service.go
View File

@ -2,6 +2,7 @@ package v1
import (
"context"
stderrors "errors"
"fmt"
"time"
@ -182,6 +183,9 @@ func (s *APIV1Service) GetMemoByShare(ctx context.Context, request *v1pb.GetMemo
memoMessage, err := s.convertMemoFromStore(ctx, memo, reactions, attachments, relations[memo.ID])
if err != nil {
if stderrors.Is(err, errMemoCreatorNotFound) {
return nil, status.Errorf(codes.NotFound, "not found")
}
return nil, errors.Wrap(err, "failed to convert memo")
}
return memoMessage, nil

34
server/router/api/v1/reaction_service.go
View File

@ -2,13 +2,11 @@ package v1
import (
"context"
"fmt"
"time"
"log/slog"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/emptypb"
"google.golang.org/protobuf/types/known/timestamppb"
v1pb "github.com/usememos/memos/proto/gen/api/v1"
"github.com/usememos/memos/store"
@ -52,12 +50,9 @@ func (s *APIV1Service) ListMemoReactions(ctx context.Context, request *v1pb.List
response := &v1pb.ListMemoReactionsResponse{
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")
}
response.Reactions = append(response.Reactions, reactionMessage)
response.Reactions, err = s.convertReactionsFromStoreWithCreators(ctx, reactions, nil)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to convert reactions")
}
return response, nil
}
@ -169,21 +164,18 @@ func (s *APIV1Service) DeleteMemoReaction(ctx context.Context, request *v1pb.Del
}
func (s *APIV1Service) convertReactionFromStore(ctx context.Context, reaction *store.Reaction) (*v1pb.Reaction, error) {
reactionUID := fmt.Sprintf("%d", reaction.ID)
creator, err := s.Store.GetUser(ctx, &store.FindUser{ID: &reaction.CreatorID})
creatorsByID, err := s.listUsersByIDWithExisting(ctx, []int32{reaction.CreatorID}, nil)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to get reaction creator")
}
if creator == nil {
reactionMessage, err := convertReactionFromStoreWithCreators(reaction, creatorsByID)
if err != nil {
slog.Warn("Failed to convert reaction with missing creator",
slog.Int64("reaction_id", int64(reaction.ID)),
slog.Int64("creator_id", int64(reaction.CreatorID)),
slog.String("content_id", reaction.ContentID),
)
return nil, status.Errorf(codes.NotFound, "reaction creator not found")
}
// Generate nested resource name: memos/{memo}/reactions/{reaction}
// reaction.ContentID already contains "memos/{memo}"
return &v1pb.Reaction{
Name: fmt.Sprintf("%s/%s%s", reaction.ContentID, ReactionNamePrefix, reactionUID),
Creator: BuildUserName(creator.Username),
ContentId: reaction.ContentID,
ReactionType: reaction.ReactionType,
CreateTime: timestamppb.New(time.Unix(reaction.CreatedTs, 0)),
}, nil
return reactionMessage, nil
}

120
server/router/api/v1/test/memo_service_test.go
View File

@ -11,6 +11,7 @@ import (
"google.golang.org/protobuf/types/known/timestamppb"
apiv1 "github.com/usememos/memos/proto/gen/api/v1"
"github.com/usememos/memos/store"
)
func TestListMemos(t *testing.T) {
@ -253,6 +254,125 @@ func TestListMemos(t *testing.T) {
require.Equal(t, "👍", userTwoReaction.ReactionType)
}
func TestListMemosSkipsReactionsWithMissingCreators(t *testing.T) {
ctx := context.Background()
ts := NewTestService(t)
defer ts.Cleanup()
owner, err := ts.CreateRegularUser(ctx, "memo-owner")
require.NoError(t, err)
ownerCtx := ts.CreateUserContext(ctx, owner.ID)
reactor, err := ts.CreateRegularUser(ctx, "memo-reactor")
require.NoError(t, err)
reactorCtx := ts.CreateUserContext(ctx, reactor.ID)
memo, err := ts.Service.CreateMemo(ownerCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "memo with orphan reaction",
Visibility: apiv1.Visibility_PUBLIC,
},
})
require.NoError(t, err)
_, err = ts.Service.UpsertMemoReaction(reactorCtx, &apiv1.UpsertMemoReactionRequest{
Name: memo.Name,
Reaction: &apiv1.Reaction{
ContentId: memo.Name,
ReactionType: "👍",
},
})
require.NoError(t, err)
err = ts.Store.DeleteUser(ctx, &store.DeleteUser{ID: reactor.ID})
require.NoError(t, err)
resp, err := ts.Service.ListMemos(ownerCtx, &apiv1.ListMemosRequest{PageSize: 10})
require.NoError(t, err)
require.Len(t, resp.Memos, 1)
require.Equal(t, memo.Name, resp.Memos[0].Name)
require.Empty(t, resp.Memos[0].Reactions)
}
func TestListMemosSkipsMemosWithMissingCreators(t *testing.T) {
ctx := context.Background()
ts := NewTestService(t)
defer ts.Cleanup()
owner, err := ts.CreateRegularUser(ctx, "memo-visible-owner")
require.NoError(t, err)
ownerCtx := ts.CreateUserContext(ctx, owner.ID)
orphanCreator, err := ts.CreateRegularUser(ctx, "memo-orphan-creator")
require.NoError(t, err)
orphanCtx := ts.CreateUserContext(ctx, orphanCreator.ID)
ownerMemo, err := ts.Service.CreateMemo(ownerCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "owner memo",
Visibility: apiv1.Visibility_PRIVATE,
},
})
require.NoError(t, err)
_, err = ts.Service.CreateMemo(orphanCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "orphan memo",
Visibility: apiv1.Visibility_PUBLIC,
},
})
require.NoError(t, err)
err = ts.Store.DeleteUser(ctx, &store.DeleteUser{ID: orphanCreator.ID})
require.NoError(t, err)
resp, err := ts.Service.ListMemos(ownerCtx, &apiv1.ListMemosRequest{PageSize: 10})
require.NoError(t, err)
require.Len(t, resp.Memos, 1)
require.Equal(t, ownerMemo.Name, resp.Memos[0].Name)
}
func TestListMemoCommentsSkipsCommentsWithMissingCreators(t *testing.T) {
ctx := context.Background()
ts := NewTestService(t)
defer ts.Cleanup()
owner, err := ts.CreateRegularUser(ctx, "comment-owner")
require.NoError(t, err)
ownerCtx := ts.CreateUserContext(ctx, owner.ID)
commenter, err := ts.CreateRegularUser(ctx, "comment-orphan")
require.NoError(t, err)
commenterCtx := ts.CreateUserContext(ctx, commenter.ID)
memo, err := ts.Service.CreateMemo(ownerCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "memo with comment",
Visibility: apiv1.Visibility_PUBLIC,
},
})
require.NoError(t, err)
_, err = ts.Service.CreateMemoComment(commenterCtx, &apiv1.CreateMemoCommentRequest{
Name: memo.Name,
Comment: &apiv1.Memo{
Content: "comment to orphan",
Visibility: apiv1.Visibility_PUBLIC,
},
})
require.NoError(t, err)
err = ts.Store.DeleteUser(ctx, &store.DeleteUser{ID: commenter.ID})
require.NoError(t, err)
resp, err := ts.Service.ListMemoComments(ownerCtx, &apiv1.ListMemoCommentsRequest{Name: memo.Name})
require.NoError(t, err)
require.Empty(t, resp.Memos)
}
// TestCreateMemoWithCustomTimestamps tests that custom timestamps can be set when creating memos and comments.
// This addresses issue #5483: https://github.com/usememos/memos/issues/5483
func TestCreateMemoWithCustomTimestamps(t *testing.T) {

48
server/router/api/v1/test/memo_share_service_test.go
View File

@ -110,6 +110,54 @@ func TestGetMemoByShare_IncludesReactions(t *testing.T) {
require.Equal(t, memo.Name, sharedMemo.Reactions[0].ContentId)
}
func TestGetMemoByShare_SkipsReactionsWithMissingCreators(t *testing.T) {
ctx := context.Background()
ts := NewTestService(t)
defer ts.Cleanup()
owner, err := ts.CreateRegularUser(ctx, "share-owner")
require.NoError(t, err)
ownerCtx := ts.CreateUserContext(ctx, owner.ID)
reactor, err := ts.CreateRegularUser(ctx, "share-reaction-orphan")
require.NoError(t, err)
reactorCtx := ts.CreateUserContext(ctx, reactor.ID)
memo, err := ts.Service.CreateMemo(ownerCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "memo with orphan share reaction",
Visibility: apiv1.Visibility_PUBLIC,
},
})
require.NoError(t, err)
_, err = ts.Service.UpsertMemoReaction(reactorCtx, &apiv1.UpsertMemoReactionRequest{
Name: memo.Name,
Reaction: &apiv1.Reaction{
ContentId: memo.Name,
ReactionType: "👍",
},
})
require.NoError(t, err)
share, err := ts.Service.CreateMemoShare(ownerCtx, &apiv1.CreateMemoShareRequest{
Parent: memo.Name,
MemoShare: &apiv1.MemoShare{},
})
require.NoError(t, err)
err = ts.Store.DeleteUser(ctx, &store.DeleteUser{ID: reactor.ID})
require.NoError(t, err)
shareToken := share.Name[strings.LastIndex(share.Name, "/")+1:]
sharedMemo, err := ts.Service.GetMemoByShare(ctx, &apiv1.GetMemoByShareRequest{
ShareId: shareToken,
})
require.NoError(t, err)
require.Empty(t, sharedMemo.Reactions)
}
func TestGetMemoByShare_ReturnsNotFoundForUnknownShare(t *testing.T) {
ctx := context.Background()

40
server/router/api/v1/test/reaction_service_test.go
View File

@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/require"
apiv1 "github.com/usememos/memos/proto/gen/api/v1"
"github.com/usememos/memos/store"
)
func TestDeleteMemoReaction(t *testing.T) {
@ -193,3 +194,42 @@ func TestDeleteMemoReaction(t *testing.T) {
require.NotContains(t, err.Error(), "not found")
})
}
func TestListMemoReactionsSkipsMissingCreators(t *testing.T) {
ctx := context.Background()
ts := NewTestService(t)
defer ts.Cleanup()
owner, err := ts.CreateRegularUser(ctx, "reaction-owner")
require.NoError(t, err)
ownerCtx := ts.CreateUserContext(ctx, owner.ID)
reactor, err := ts.CreateRegularUser(ctx, "reaction-orphan")
require.NoError(t, err)
reactorCtx := ts.CreateUserContext(ctx, reactor.ID)
memo, err := ts.Service.CreateMemo(ownerCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "reaction list memo",
Visibility: apiv1.Visibility_PUBLIC,
},
})
require.NoError(t, err)
_, err = ts.Service.UpsertMemoReaction(reactorCtx, &apiv1.UpsertMemoReactionRequest{
Name: memo.Name,
Reaction: &apiv1.Reaction{
ContentId: memo.Name,
ReactionType: "🔥",
},
})
require.NoError(t, err)
err = ts.Store.DeleteUser(ctx, &store.DeleteUser{ID: reactor.ID})
require.NoError(t, err)
resp, err := ts.Service.ListMemoReactions(ctx, &apiv1.ListMemoReactionsRequest{Name: memo.Name})
require.NoError(t, err)
require.Empty(t, resp.Reactions)
}

40
server/router/api/v1/test/user_notification_test.go
View File

@ -144,6 +144,46 @@ func TestListUserNotificationsOmitsPayloadWhenMemosDeleted(t *testing.T) {
require.Nil(t, resp.Notifications[0].GetMemoComment())
}
func TestListUserNotificationsSkipsNotificationsWithMissingUsers(t *testing.T) {
ctx := context.Background()
ts := NewTestService(t)
defer ts.Cleanup()
owner, err := ts.CreateRegularUser(ctx, "notification-owner")
require.NoError(t, err)
ownerCtx := ts.CreateUserContext(ctx, owner.ID)
commenter, err := ts.CreateRegularUser(ctx, "notification-orphan")
require.NoError(t, err)
commenterCtx := ts.CreateUserContext(ctx, commenter.ID)
memo, err := ts.Service.CreateMemo(ownerCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "Base memo",
Visibility: apiv1.Visibility_PUBLIC,
},
})
require.NoError(t, err)
_, err = ts.Service.CreateMemoComment(commenterCtx, &apiv1.CreateMemoCommentRequest{
Name: memo.Name,
Comment: &apiv1.Memo{
Content: "Comment content",
Visibility: apiv1.Visibility_PUBLIC,
},
})
require.NoError(t, err)
err = ts.Store.DeleteUser(ctx, &store.DeleteUser{ID: commenter.ID})
require.NoError(t, err)
resp, err := ts.Service.ListUserNotifications(ownerCtx, &apiv1.ListUserNotificationsRequest{
Parent: fmt.Sprintf("users/%s", owner.Username),
})
require.NoError(t, err)
require.Empty(t, resp.Notifications)
}
func TestListUserNotificationsRejectsNumericParent(t *testing.T) {
ctx := context.Background()
ts := NewTestService(t)

9
server/router/api/v1/user_service.go
View File

@ -5,6 +5,7 @@ import (
"crypto/rand"
"encoding/hex"
"fmt"
"log/slog"
"regexp"
"strconv"
"strings"
@ -1293,6 +1294,14 @@ func (s *APIV1Service) ListUserNotifications(ctx context.Context, request *v1pb.
for _, inbox := range inboxes {
notification, err := s.convertInboxToUserNotificationWithUsers(ctx, inbox, usersByID)
if err != nil {
if status.Code(err) == codes.NotFound {
slog.Warn("Skipping notification with missing user",
slog.Int64("notification_id", int64(inbox.ID)),
slog.Int64("receiver_id", int64(inbox.ReceiverID)),
slog.Int64("sender_id", int64(inbox.SenderID)),
)
continue
}
return nil, status.Errorf(codes.Internal, "failed to convert inbox: %v", err)
}
notifications = append(notifications, notification)