From b65ad60fe8d2e08e31c9742a01cb21f83e79e3af Mon Sep 17 00:00:00 2001 From: onysd Date: Thu, 3 Sep 2026 11:00:50 +0300 Subject: [PATCH] fix for deleted file metadata --- internal/app/files/retention.go | 25 +++ .../app/files/retention_category_zero_test.go | 3 + .../app/files/retention_manual_purge_test.go | 3 + internal/app/files/retention_purge.go | 35 +++- .../app/files/retention_purge_reap_test.go | 150 ++++++++++++++++++ internal/app/files/secret_chat_files_test.go | 3 + internal/store/postgres/media_refs.go | 15 ++ 7 files changed, 230 insertions(+), 4 deletions(-) create mode 100644 internal/app/files/retention_purge_reap_test.go diff --git a/internal/app/files/retention.go b/internal/app/files/retention.go index 4763372c..30f300fe 100644 --- a/internal/app/files/retention.go +++ b/internal/app/files/retention.go @@ -63,6 +63,9 @@ type mediaRetentionStore interface { // OrphanDocumentIfUnreferenced is the immediate (no grace period) // counterpart to the age-based sweep above -- see its doc comment. OrphanDocumentIfUnreferenced(ctx context.Context, id int64) (bool, error) + // OrphanPhotoIfUnreferenced is OrphanDocumentIfUnreferenced's photo + // counterpart, used the same way by deletePhotoNowIfUnreferenced. + OrphanPhotoIfUnreferenced(ctx context.Context, id int64) (bool, error) // -- "hard" retention mode (TELESRV_STORAGE_RETENTION_MODE=hard) -- // Candidate selection ignores media_references entirely: a document/ @@ -513,6 +516,28 @@ func (s *Service) deleteDocumentNowIfUnreferenced(ctx context.Context, id int64) return true, nil } +// deletePhotoNowIfUnreferenced is deleteDocumentNowIfUnreferenced's photo +// counterpart -- see its doc comment. +func (s *Service) deletePhotoNowIfUnreferenced(ctx context.Context, id int64) (bool, error) { + store, ok := s.media.(mediaRetentionStore) + if !ok { + return false, nil + } + orphaned, err := store.OrphanPhotoIfUnreferenced(ctx, id) + if err != nil { + return false, fmt.Errorf("orphan photo: %w", err) + } + if !orphaned { + return false, nil + } + blobs, err := store.DeletePhotoAndBlobs(ctx, id) + if err != nil { + return false, fmt.Errorf("delete photo: %w", err) + } + s.deleteOrphanedBlobs(ctx, store, blobs) + return true, nil +} + // deleteOrphanedBlobs removes each blob from its backend once confirming // (via CountFileBlobRefs) no other file_blobs row still references // (backend, object_key). Resolves the correct backend per blob via diff --git a/internal/app/files/retention_category_zero_test.go b/internal/app/files/retention_category_zero_test.go index 3afbcb94..5642eb54 100644 --- a/internal/app/files/retention_category_zero_test.go +++ b/internal/app/files/retention_category_zero_test.go @@ -81,6 +81,9 @@ func (f *fakeCategorySweepStore) DeletePhotoAndBlobs(context.Context, int64) ([] func (f *fakeCategorySweepStore) OrphanDocumentIfUnreferenced(context.Context, int64) (bool, error) { return false, nil } +func (f *fakeCategorySweepStore) OrphanPhotoIfUnreferenced(context.Context, int64) (bool, error) { + return false, nil +} func (f *fakeCategorySweepStore) DeleteFileBlobsForDocument(context.Context, int64) ([]domain.FileBlob, error) { return nil, nil } diff --git a/internal/app/files/retention_manual_purge_test.go b/internal/app/files/retention_manual_purge_test.go index e607b69c..b26910da 100644 --- a/internal/app/files/retention_manual_purge_test.go +++ b/internal/app/files/retention_manual_purge_test.go @@ -144,6 +144,9 @@ func (f *fakeManualPurgeStore) DeletePhotoAndBlobs(context.Context, int64) ([]do func (f *fakeManualPurgeStore) OrphanDocumentIfUnreferenced(context.Context, int64) (bool, error) { return false, nil } +func (f *fakeManualPurgeStore) OrphanPhotoIfUnreferenced(context.Context, int64) (bool, error) { + return false, nil +} func (f *fakeManualPurgeStore) SumFileBlobBytes(context.Context) (int64, error) { return 0, nil } func (f *fakeManualPurgeStore) ListOldestMediaForEviction(context.Context, int) ([]domain.EvictionCandidate, error) { return nil, nil diff --git a/internal/app/files/retention_purge.go b/internal/app/files/retention_purge.go index 800d6f48..d8822344 100644 --- a/internal/app/files/retention_purge.go +++ b/internal/app/files/retention_purge.go @@ -63,10 +63,12 @@ func (s *Service) retentionNotifier() (RetentionPurgeMessageEditor, RetentionPur // notifyRetentionPurge turns a just-completed hard-retention/eviction blob // purge of a document/photo into a visible service-message notice on every -// message that still embeds it. profile_photo/sticker_set/gift references -// have no message to edit and are skipped. Best-effort throughout: a lookup -// or edit failure is logged and never propagated -- the underlying blob purge -// has already committed and must not be undone or retried because of this. +// message that still embeds it, then reaps the documents/photos metadata row +// itself if that leaves nothing referencing it at all. profile_photo/ +// sticker_set/gift references have no message to edit and are skipped. +// Best-effort throughout: a lookup or edit failure is logged and never +// propagated -- the underlying blob purge has already committed and must not +// be undone or retried because of this. func (s *Service) notifyRetentionPurge(ctx context.Context, kind domain.MediaKind, mediaID int64) { messages, channels := s.retentionNotifier() if messages == nil && channels == nil { @@ -92,6 +94,31 @@ func (s *Service) notifyRetentionPurge(ctx context.Context, kind domain.MediaKin // No message to edit for these ref kinds. } } + // Every message edited above just had its media replaced with the purge + // notice, which -- like any other media-changing edit -- drops its + // media_references row (see replaceMessageBoxMediaIndexTx / + // replaceChannelMediaIndexTx). If that leaves zero references of ANY + // kind (no live profile_photo/sticker_set/gift reference either, and no + // edit above failed partway through), the documents/photos row is now + // pure dead weight -- nothing anywhere still displays its filename, + // dimensions, or mime type -- so delete it for real instead of keeping + // it forever. deleteDocumentNowIfUnreferenced/deletePhotoNowIfUnreferenced + // re-check media_references themselves before touching anything, so a + // stale/failed edit above simply leaves a live reference behind and this + // becomes a safe no-op -- the exact same check ordinary orphan-mode + // deletion already relies on. + switch kind { + case domain.MediaKindDocument: + if _, err := s.deleteDocumentNowIfUnreferenced(ctx, mediaID); err != nil { + s.log.Warn("delete now-unreferenced document after retention purge failed", + zap.Int64("document_id", mediaID), zap.Error(err)) + } + case domain.MediaKindPhoto: + if _, err := s.deletePhotoNowIfUnreferenced(ctx, mediaID); err != nil { + s.log.Warn("delete now-unreferenced photo after retention purge failed", + zap.Int64("photo_id", mediaID), zap.Error(err)) + } + } } // notifyRetentionPurgeMessageBox parses a message_box ref_key (format diff --git a/internal/app/files/retention_purge_reap_test.go b/internal/app/files/retention_purge_reap_test.go new file mode 100644 index 00000000..cfaaf46f --- /dev/null +++ b/internal/app/files/retention_purge_reap_test.go @@ -0,0 +1,150 @@ +package files + +import ( + "context" + "testing" + "time" + + "go.uber.org/zap" + + "telesrv/internal/domain" + "telesrv/internal/store" +) + +// fakeReapStore is a minimal mediaRetentionStore fake exercising only +// notifyRetentionPurge's new reap step: ListMediaReferences reports one +// message_box reference, and orphanedDocument/orphanedPhoto control what +// OrphanDocumentIfUnreferenced/OrphanPhotoIfUnreferenced report -- simulating +// "the edit above dropped the last reference" (true) vs "something else +// still references it" (false, e.g. a live avatar or a failed edit). Every +// other mediaRetentionStore method is a stub never expected to be called by +// these tests -- but must still exist, or the type assertion +// s.media.(mediaRetentionStore) inside notifyRetentionPurge silently fails +// and the whole test becomes a no-op instead of exercising anything. +type fakeReapStore struct { + store.MediaStore + orphanedDocument bool + orphanedPhoto bool + deletedDocumentIDs []int64 + deletedPhotoIDs []int64 +} + +func (f *fakeReapStore) ListMediaReferences(context.Context, domain.MediaKind, int64) ([]domain.MediaReference, error) { + return []domain.MediaReference{{RefKind: domain.MediaRefKindMessageBox, RefKey: "user:1:box:2"}}, nil +} +func (f *fakeReapStore) OrphanDocumentIfUnreferenced(context.Context, int64) (bool, error) { + return f.orphanedDocument, nil +} +func (f *fakeReapStore) OrphanPhotoIfUnreferenced(context.Context, int64) (bool, error) { + return f.orphanedPhoto, nil +} +func (f *fakeReapStore) DeleteDocumentAndBlobs(_ context.Context, id int64) ([]domain.FileBlob, error) { + f.deletedDocumentIDs = append(f.deletedDocumentIDs, id) + return nil, nil +} +func (f *fakeReapStore) DeletePhotoAndBlobs(_ context.Context, id int64) ([]domain.FileBlob, error) { + f.deletedPhotoIDs = append(f.deletedPhotoIDs, id) + return nil, nil +} +func (f *fakeReapStore) ListOrphanedDocumentIDsOlderThan(context.Context, domain.MediaCategory, time.Time, int) ([]int64, error) { + return nil, nil +} +func (f *fakeReapStore) ListOrphanedPhotoIDsOlderThan(context.Context, time.Time, int) ([]int64, error) { + return nil, nil +} +func (f *fakeReapStore) ListAvatarOrphanedPhotoIDsOlderThan(context.Context, time.Time, int) ([]int64, error) { + return nil, nil +} +func (f *fakeReapStore) CountFileBlobRefs(context.Context, string, string) (int, error) { + return 0, nil +} +func (f *fakeReapStore) ListDocumentIDsForHardRetentionOlderThan(context.Context, domain.MediaCategory, *time.Time, int) ([]int64, error) { + return nil, nil +} +func (f *fakeReapStore) ListPhotoIDsForHardRetentionOlderThan(context.Context, *time.Time, int) ([]int64, error) { + return nil, nil +} +func (f *fakeReapStore) ListAvatarPhotoIDsForHardRetentionOlderThan(context.Context, *time.Time, int) ([]int64, error) { + return nil, nil +} +func (f *fakeReapStore) CountDocumentsForHardRetention(context.Context, domain.MediaCategory, *time.Time) (int, error) { + return 0, nil +} +func (f *fakeReapStore) CountPhotosForHardRetention(context.Context, *time.Time) (int, error) { + return 0, nil +} +func (f *fakeReapStore) CountAvatarPhotosForHardRetention(context.Context, *time.Time) (int, error) { + return 0, nil +} +func (f *fakeReapStore) DeleteFileBlobsForDocument(context.Context, int64) ([]domain.FileBlob, error) { + return nil, nil +} +func (f *fakeReapStore) DeleteFileBlobsForPhoto(context.Context, int64) ([]domain.FileBlob, error) { + return nil, nil +} +func (f *fakeReapStore) SumFileBlobBytes(context.Context) (int64, error) { return 0, nil } +func (f *fakeReapStore) ListOldestMediaForEviction(context.Context, int) ([]domain.EvictionCandidate, error) { + return nil, nil +} + +// fakeReapMessages is the minimal RetentionPurgeMessageEditor: GetMessages +// resolves a peer so notifyRetentionPurgeMessageBox can build the edit +// request, and EditMessage always succeeds. +type fakeReapMessages struct{} + +func (fakeReapMessages) GetMessages(context.Context, int64, []int) (domain.MessageList, error) { + return domain.MessageList{Messages: []domain.Message{{ID: 2, Peer: domain.Peer{Type: domain.PeerTypeUser, ID: 3}}}}, nil +} +func (fakeReapMessages) EditMessage(context.Context, int64, domain.EditMessageRequest) (domain.EditMessageResult, error) { + return domain.EditMessageResult{}, nil +} + +func newTestServiceForReap(media store.MediaStore) *Service { + s := &Service{media: media, log: zap.NewNop(), blobCache: newBlobMetaCache(8)} + s.SetRetentionPurgeNotifier(fakeReapMessages{}, nil) + return s +} + +// TestNotifyRetentionPurgeReapsDocumentWhenFullyUnreferenced guards the fix +// for "documents/photos rows pile up forever even after the message showing +// them was already converted to the purge notice": once nothing references a +// purged document any more, notifyRetentionPurge must delete the row for +// real instead of always keeping it. +func TestNotifyRetentionPurgeReapsDocumentWhenFullyUnreferenced(t *testing.T) { + fake := &fakeReapStore{orphanedDocument: true} + s := newTestServiceForReap(fake) + + s.notifyRetentionPurge(context.Background(), domain.MediaKindDocument, 42) + + if len(fake.deletedDocumentIDs) != 1 || fake.deletedDocumentIDs[0] != 42 { + t.Fatalf("deleted document ids = %v, want [42]", fake.deletedDocumentIDs) + } +} + +// TestNotifyRetentionPurgeKeepsDocumentWhenStillReferenced is the negative +// case: OrphanDocumentIfUnreferenced reporting false (something else -- a +// live avatar, sticker set, or a failed edit -- still references it) must +// leave the row alone, exactly like the safe orphan-mode deletion this reuses. +func TestNotifyRetentionPurgeKeepsDocumentWhenStillReferenced(t *testing.T) { + fake := &fakeReapStore{orphanedDocument: false} + s := newTestServiceForReap(fake) + + s.notifyRetentionPurge(context.Background(), domain.MediaKindDocument, 42) + + if len(fake.deletedDocumentIDs) != 0 { + t.Fatalf("deleted document ids = %v, want none", fake.deletedDocumentIDs) + } +} + +// TestNotifyRetentionPurgeReapsPhotoWhenFullyUnreferenced is the photo +// counterpart of the document test above. +func TestNotifyRetentionPurgeReapsPhotoWhenFullyUnreferenced(t *testing.T) { + fake := &fakeReapStore{orphanedPhoto: true} + s := newTestServiceForReap(fake) + + s.notifyRetentionPurge(context.Background(), domain.MediaKindPhoto, 99) + + if len(fake.deletedPhotoIDs) != 1 || fake.deletedPhotoIDs[0] != 99 { + t.Fatalf("deleted photo ids = %v, want [99]", fake.deletedPhotoIDs) + } +} diff --git a/internal/app/files/secret_chat_files_test.go b/internal/app/files/secret_chat_files_test.go index bbffe18d..79cbd819 100644 --- a/internal/app/files/secret_chat_files_test.go +++ b/internal/app/files/secret_chat_files_test.go @@ -78,6 +78,9 @@ func (f *fakeEncryptedBlobStore) DeletePhotoAndBlobs(context.Context, int64) ([] func (f *fakeEncryptedBlobStore) OrphanDocumentIfUnreferenced(context.Context, int64) (bool, error) { return false, nil } +func (f *fakeEncryptedBlobStore) OrphanPhotoIfUnreferenced(context.Context, int64) (bool, error) { + return false, nil +} func (f *fakeEncryptedBlobStore) ListDocumentIDsForHardRetentionOlderThan(context.Context, domain.MediaCategory, *time.Time, int) ([]int64, error) { return nil, nil } diff --git a/internal/store/postgres/media_refs.go b/internal/store/postgres/media_refs.go index 5c8c17e6..f20051cf 100644 --- a/internal/store/postgres/media_refs.go +++ b/internal/store/postgres/media_refs.go @@ -152,6 +152,21 @@ WHERE id = $1 return tag.RowsAffected() > 0, nil } +// OrphanPhotoIfUnreferenced is OrphanDocumentIfUnreferenced's photo +// counterpart -- see its doc comment. +func (s *MediaStore) OrphanPhotoIfUnreferenced(ctx context.Context, id int64) (bool, error) { + tag, err := s.db.Exec(ctx, ` +UPDATE photos SET orphaned_at = now() +WHERE id = $1 + AND orphaned_at IS NULL + AND NOT EXISTS (SELECT 1 FROM media_references WHERE media_kind = 'photo' AND media_id = $1)`, + id) + if err != nil { + return false, fmt.Errorf("orphan photo if unreferenced: %w", err) + } + return tag.RowsAffected() > 0, nil +} + // ListOrphanedDocumentIDsOlderThan returns document ids in the given category // whose orphaned_at is set and older than cutoff, oldest first, up to limit. func (s *MediaStore) ListOrphanedDocumentIDsOlderThan(ctx context.Context, category domain.MediaCategory, cutoff time.Time, limit int) ([]int64, error) {