From acbc914dea8d4e533028b03929193a4af1ac32b4 Mon Sep 17 00:00:00 2001 From: memoclaw Date: Mon, 30 Mar 2026 20:01:16 +0800 Subject: [PATCH] fix(webhooks): trigger memo updates for attachment and relation changes (#5795) Co-authored-by: memoclaw <265580040+memoclaw@users.noreply.github.com> --- .../router/api/v1/memo_attachment_service.go | 38 ++++++--- server/router/api/v1/memo_relation_service.go | 30 +++++-- server/router/api/v1/memo_service.go | 51 ++---------- server/router/api/v1/memo_update_helpers.go | 78 +++++++++++++++++ server/router/api/v1/sse_service_test.go | 83 +++++++++++++++++++ 5 files changed, 216 insertions(+), 64 deletions(-) create mode 100644 server/router/api/v1/memo_update_helpers.go diff --git a/server/router/api/v1/memo_attachment_service.go b/server/router/api/v1/memo_attachment_service.go index d9aa0a3ee..d687c59e9 100644 --- a/server/router/api/v1/memo_attachment_service.go +++ b/server/router/api/v1/memo_attachment_service.go @@ -35,20 +35,36 @@ func (s *APIV1Service) SetMemoAttachments(ctx context.Context, request *v1pb.Set if memo.CreatorID != user.ID && !isSuperUser(user) { return nil, status.Errorf(codes.PermissionDenied, "permission denied") } + if err := s.setMemoAttachmentsInternal(ctx, memo, request.Attachments); err != nil { + return nil, err + } + if err := s.touchMemoUpdatedTimestamp(ctx, memo.ID); err != nil { + return nil, err + } + updatedMemo, parentMemo, memoMessage, err := s.buildUpdatedMemoState(ctx, memo.ID) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to build updated memo state") + } + s.dispatchMemoUpdatedSideEffects(ctx, updatedMemo, parentMemo, memoMessage) + + return &emptypb.Empty{}, nil +} + +func (s *APIV1Service) setMemoAttachmentsInternal(ctx context.Context, memo *store.Memo, requestAttachments []*v1pb.Attachment) error { attachments, err := s.Store.ListAttachments(ctx, &store.FindAttachment{ MemoID: &memo.ID, }) if err != nil { - return nil, status.Errorf(codes.Internal, "failed to list attachments") + return status.Errorf(codes.Internal, "failed to list attachments") } // Delete attachments that are not in the request. for _, attachment := range attachments { found := false - for _, requestAttachment := range request.Attachments { + for _, requestAttachment := range requestAttachments { requestAttachmentUID, err := ExtractAttachmentUIDFromName(requestAttachment.Name) if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "invalid attachment name: %v", err) + return status.Errorf(codes.InvalidArgument, "invalid attachment name: %v", err) } if attachment.UID == requestAttachmentUID { found = true @@ -60,24 +76,24 @@ func (s *APIV1Service) SetMemoAttachments(ctx context.Context, request *v1pb.Set ID: int32(attachment.ID), MemoID: &memo.ID, }); err != nil { - return nil, status.Errorf(codes.Internal, "failed to delete attachment") + return status.Errorf(codes.Internal, "failed to delete attachment") } } } - slices.Reverse(request.Attachments) + slices.Reverse(requestAttachments) // Update attachments' memo_id in the request. - for index, attachment := range request.Attachments { + for index, attachment := range requestAttachments { attachmentUID, err := ExtractAttachmentUIDFromName(attachment.Name) if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "invalid attachment name: %v", err) + return status.Errorf(codes.InvalidArgument, "invalid attachment name: %v", err) } tempAttachment, err := s.Store.GetAttachment(ctx, &store.FindAttachment{UID: &attachmentUID}) if err != nil { - return nil, status.Errorf(codes.Internal, "failed to get attachment: %v", err) + return status.Errorf(codes.Internal, "failed to get attachment: %v", err) } if tempAttachment == nil { - return nil, status.Errorf(codes.NotFound, "attachment not found: %s", attachmentUID) + return status.Errorf(codes.NotFound, "attachment not found: %s", attachmentUID) } updatedTs := time.Now().Unix() + int64(index) if err := s.Store.UpdateAttachment(ctx, &store.UpdateAttachment{ @@ -85,11 +101,11 @@ func (s *APIV1Service) SetMemoAttachments(ctx context.Context, request *v1pb.Set MemoID: &memo.ID, UpdatedTs: &updatedTs, }); err != nil { - return nil, status.Errorf(codes.Internal, "failed to update attachment: %v", err) + return status.Errorf(codes.Internal, "failed to update attachment: %v", err) } } - return &emptypb.Empty{}, nil + return nil } func (s *APIV1Service) ListMemoAttachments(ctx context.Context, request *v1pb.ListMemoAttachmentsRequest) (*v1pb.ListMemoAttachmentsResponse, error) { diff --git a/server/router/api/v1/memo_relation_service.go b/server/router/api/v1/memo_relation_service.go index 25d97f24a..382d72143 100644 --- a/server/router/api/v1/memo_relation_service.go +++ b/server/router/api/v1/memo_relation_service.go @@ -35,18 +35,34 @@ func (s *APIV1Service) SetMemoRelations(ctx context.Context, request *v1pb.SetMe if memo.CreatorID != user.ID && !isSuperUser(user) { return nil, status.Errorf(codes.PermissionDenied, "permission denied") } + if err := s.setMemoRelationsInternal(ctx, memo, request.Relations); err != nil { + return nil, err + } + if err := s.touchMemoUpdatedTimestamp(ctx, memo.ID); err != nil { + return nil, err + } + updatedMemo, parentMemo, memoMessage, err := s.buildUpdatedMemoState(ctx, memo.ID) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to build updated memo state") + } + s.dispatchMemoUpdatedSideEffects(ctx, updatedMemo, parentMemo, memoMessage) + + return &emptypb.Empty{}, nil +} + +func (s *APIV1Service) setMemoRelationsInternal(ctx context.Context, memo *store.Memo, relations []*v1pb.MemoRelation) error { referenceType := store.MemoRelationReference // Delete all reference relations first. if err := s.Store.DeleteMemoRelation(ctx, &store.DeleteMemoRelation{ MemoID: &memo.ID, Type: &referenceType, }); err != nil { - return nil, status.Errorf(codes.Internal, "failed to delete memo relation") + return status.Errorf(codes.Internal, "failed to delete memo relation") } - for _, relation := range request.Relations { + for _, relation := range relations { // Ignore reflexive relations. - if request.Name == relation.RelatedMemo.Name { + if buildMemoName(memo.UID) == relation.RelatedMemo.Name { continue } // Ignore comment relations as there's no need to update a comment's relation. @@ -56,22 +72,22 @@ func (s *APIV1Service) SetMemoRelations(ctx context.Context, request *v1pb.SetMe } relatedMemoUID, err := ExtractMemoUIDFromName(relation.RelatedMemo.Name) if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "invalid related memo name: %v", err) + return status.Errorf(codes.InvalidArgument, "invalid related memo name: %v", err) } relatedMemo, err := s.Store.GetMemo(ctx, &store.FindMemo{UID: &relatedMemoUID}) if err != nil { - return nil, status.Errorf(codes.Internal, "failed to get related memo") + return status.Errorf(codes.Internal, "failed to get related memo") } if _, err := s.Store.UpsertMemoRelation(ctx, &store.MemoRelation{ MemoID: memo.ID, RelatedMemoID: relatedMemo.ID, Type: convertMemoRelationTypeToStore(relation.Type), }); err != nil { - return nil, status.Errorf(codes.Internal, "failed to upsert memo relation") + return status.Errorf(codes.Internal, "failed to upsert memo relation") } } - return &emptypb.Empty{}, nil + return nil } func (s *APIV1Service) ListMemoRelations(ctx context.Context, request *v1pb.ListMemoRelationsRequest) (*v1pb.ListMemoRelationsResponse, error) { diff --git a/server/router/api/v1/memo_service.go b/server/router/api/v1/memo_service.go index 31016ae1e..830ee1eb6 100644 --- a/server/router/api/v1/memo_service.go +++ b/server/router/api/v1/memo_service.go @@ -469,19 +469,11 @@ func (s *APIV1Service) UpdateMemo(ctx context.Context, request *v1pb.UpdateMemoR payload.Location = convertLocationToStore(request.Memo.Location) update.Payload = payload } else if path == "attachments" { - _, err := s.SetMemoAttachments(ctx, &v1pb.SetMemoAttachmentsRequest{ - Name: request.Memo.Name, - Attachments: request.Memo.Attachments, - }) - if err != nil { + if err := s.setMemoAttachmentsInternal(ctx, memo, request.Memo.Attachments); err != nil { return nil, errors.Wrap(err, "failed to set memo attachments") } } else if path == "relations" { - _, err := s.SetMemoRelations(ctx, &v1pb.SetMemoRelationsRequest{ - Name: request.Memo.Name, - Relations: request.Memo.Relations, - }) - if err != nil { + if err := s.setMemoRelationsInternal(ctx, memo, request.Memo.Relations); err != nil { return nil, errors.Wrap(err, "failed to set memo relations") } } @@ -497,44 +489,11 @@ func (s *APIV1Service) UpdateMemo(ctx context.Context, request *v1pb.UpdateMemoR if err != nil { return nil, errors.Wrap(err, "failed to get memo") } - reactions, err := s.Store.ListReactions(ctx, &store.FindReaction{ - ContentID: &request.Memo.Name, - }) + memo, parentMemo, memoMessage, err := s.buildUpdatedMemoState(ctx, memo.ID) if err != nil { - return nil, status.Errorf(codes.Internal, "failed to list reactions") + return nil, errors.Wrap(err, "failed to build updated memo state") } - attachments, err := s.Store.ListAttachments(ctx, &store.FindAttachment{ - MemoID: &memo.ID, - }) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to list attachments") - } - - relations, err := s.loadMemoRelations(ctx, memo) - if err != nil { - return nil, errors.Wrap(err, "failed to load memo relations") - } - memoMessage, err := s.convertMemoFromStore(ctx, memo, reactions, attachments, relations) - if err != nil { - return nil, errors.Wrap(err, "failed to convert memo") - } - var parentMemo *store.Memo - if memo.ParentUID != nil { - parentMemo, _ = s.Store.GetMemo(ctx, &store.FindMemo{UID: memo.ParentUID}) - } - // Try to dispatch webhook when memo is updated. - if err := s.DispatchMemoUpdatedWebhook(ctx, memoMessage); err != nil { - slog.Warn("Failed to dispatch memo updated webhook", slog.Any("err", err)) - } - - // Broadcast live refresh event. - s.SSEHub.Broadcast(&SSEEvent{ - Type: SSEEventMemoUpdated, - Name: memoMessage.Name, - Parent: memoMessage.GetParent(), - Visibility: memo.Visibility, - CreatorID: resolveSSECreatorID(memo, parentMemo), - }) + s.dispatchMemoUpdatedSideEffects(ctx, memo, parentMemo, memoMessage) return memoMessage, nil } diff --git a/server/router/api/v1/memo_update_helpers.go b/server/router/api/v1/memo_update_helpers.go new file mode 100644 index 000000000..2c5f7e044 --- /dev/null +++ b/server/router/api/v1/memo_update_helpers.go @@ -0,0 +1,78 @@ +package v1 + +import ( + "context" + "log/slog" + "time" + + "github.com/pkg/errors" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + v1pb "github.com/usememos/memos/proto/gen/api/v1" + "github.com/usememos/memos/store" +) + +func (s *APIV1Service) touchMemoUpdatedTimestamp(ctx context.Context, memoID int32) error { + updatedTs := time.Now().Unix() + if err := s.Store.UpdateMemo(ctx, &store.UpdateMemo{ + ID: memoID, + UpdatedTs: &updatedTs, + }); err != nil { + return status.Errorf(codes.Internal, "failed to update memo timestamp") + } + return nil +} + +func (s *APIV1Service) buildUpdatedMemoState(ctx context.Context, memoID int32) (*store.Memo, *store.Memo, *v1pb.Memo, error) { + memo, err := s.Store.GetMemo(ctx, &store.FindMemo{ID: &memoID}) + if err != nil { + return nil, nil, nil, errors.Wrap(err, "failed to get memo") + } + if memo == nil { + return nil, nil, nil, errors.New("memo not found") + } + + memoName := buildMemoName(memo.UID) + reactions, err := s.Store.ListReactions(ctx, &store.FindReaction{ + ContentID: &memoName, + }) + if err != nil { + return nil, nil, nil, errors.Wrap(err, "failed to list reactions") + } + attachments, err := s.Store.ListAttachments(ctx, &store.FindAttachment{ + MemoID: &memo.ID, + }) + if err != nil { + return nil, nil, nil, errors.Wrap(err, "failed to list attachments") + } + relations, err := s.loadMemoRelations(ctx, memo) + if err != nil { + return nil, nil, nil, errors.Wrap(err, "failed to load memo relations") + } + memoMessage, err := s.convertMemoFromStore(ctx, memo, reactions, attachments, relations) + if err != nil { + return nil, nil, nil, errors.Wrap(err, "failed to convert memo") + } + + var parentMemo *store.Memo + if memo.ParentUID != nil { + parentMemo, _ = s.Store.GetMemo(ctx, &store.FindMemo{UID: memo.ParentUID}) + } + + return memo, parentMemo, memoMessage, nil +} + +func (s *APIV1Service) dispatchMemoUpdatedSideEffects(ctx context.Context, memo *store.Memo, parentMemo *store.Memo, memoMessage *v1pb.Memo) { + if err := s.DispatchMemoUpdatedWebhook(ctx, memoMessage); err != nil { + slog.Warn("Failed to dispatch memo updated webhook", slog.Any("err", err)) + } + + s.SSEHub.Broadcast(&SSEEvent{ + Type: SSEEventMemoUpdated, + Name: memoMessage.Name, + Parent: memoMessage.GetParent(), + Visibility: memo.Visibility, + CreatorID: resolveSSECreatorID(memo, parentMemo), + }) +} diff --git a/server/router/api/v1/sse_service_test.go b/server/router/api/v1/sse_service_test.go index 180b2aec6..689a35b31 100644 --- a/server/router/api/v1/sse_service_test.go +++ b/server/router/api/v1/sse_service_test.go @@ -187,3 +187,86 @@ func TestDeleteMemoReaction_SSEEvent(t *testing.T) { assert.Contains(t, payload, memo.Name) mustNotReceive(t, client.events, 100*time.Millisecond) } + +func TestSetMemoAttachments_EmitsMemoUpdatedSSEEvent(t *testing.T) { + ctx := context.Background() + svc := newIntegrationService(t) + + user, err := svc.Store.CreateUser(ctx, &store.User{ + Username: "user", Role: store.RoleAdmin, Email: "user@example.com", + }) + require.NoError(t, err) + uctx := userCtx(ctx, user.ID) + + memo, err := svc.CreateMemo(uctx, &v1pb.CreateMemoRequest{ + Memo: &v1pb.Memo{Content: "memo with attachments", Visibility: v1pb.Visibility_PUBLIC}, + }) + require.NoError(t, err) + + attachment, err := svc.CreateAttachment(uctx, &v1pb.CreateAttachmentRequest{ + Attachment: &v1pb.Attachment{ + Filename: "test.txt", + Size: 5, + Type: "text/plain", + Content: []byte("hello"), + }, + }) + require.NoError(t, err) + + client := svc.SSEHub.Subscribe(user.ID, store.RoleAdmin) + defer svc.SSEHub.Unsubscribe(client) + + _, err = svc.SetMemoAttachments(uctx, &v1pb.SetMemoAttachmentsRequest{ + Name: memo.Name, + Attachments: []*v1pb.Attachment{ + {Name: attachment.Name}, + }, + }) + require.NoError(t, err) + + data := mustReceive(t, client.events, time.Second) + payload := string(data) + assert.Contains(t, payload, `"memo.updated"`) + assert.Contains(t, payload, memo.Name) + mustNotReceive(t, client.events, 100*time.Millisecond) +} + +func TestSetMemoRelations_EmitsMemoUpdatedSSEEvent(t *testing.T) { + ctx := context.Background() + svc := newIntegrationService(t) + + user, err := svc.Store.CreateUser(ctx, &store.User{ + Username: "user", Role: store.RoleAdmin, Email: "user@example.com", + }) + require.NoError(t, err) + uctx := userCtx(ctx, user.ID) + + memo1, err := svc.CreateMemo(uctx, &v1pb.CreateMemoRequest{ + Memo: &v1pb.Memo{Content: "memo one", Visibility: v1pb.Visibility_PUBLIC}, + }) + require.NoError(t, err) + memo2, err := svc.CreateMemo(uctx, &v1pb.CreateMemoRequest{ + Memo: &v1pb.Memo{Content: "memo two", Visibility: v1pb.Visibility_PUBLIC}, + }) + require.NoError(t, err) + + client := svc.SSEHub.Subscribe(user.ID, store.RoleAdmin) + defer svc.SSEHub.Unsubscribe(client) + + _, err = svc.SetMemoRelations(uctx, &v1pb.SetMemoRelationsRequest{ + Name: memo1.Name, + Relations: []*v1pb.MemoRelation{ + { + RelatedMemo: &v1pb.MemoRelation_Memo{Name: memo2.Name}, + Type: v1pb.MemoRelation_REFERENCE, + }, + }, + }) + require.NoError(t, err) + + data := mustReceive(t, client.events, time.Second) + payload := string(data) + assert.Contains(t, payload, `"memo.updated"`) + assert.Contains(t, payload, memo1.Name) + mustNotReceive(t, client.events, 100*time.Millisecond) +}