fix for deleted file metadata

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

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
 }

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

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
+// message that still embeds it, then reaps the documents/photos metadata row
-// have no message to edit and are skipped. Best-effort throughout: a lookup
+// itself if that leaves nothing referencing it at all. profile_photo/
-// or edit failure is logged and never propagated -- the underlying blob purge
+// sticker_set/gift references have no message to edit and are skipped.
-// has already committed and must not be undone or retried because of this.
+// 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

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

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
 }

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