Remove s3 config option, reduce size when removing files

attachment/backend.go

@@ -17,6 +17,7 @@ type object struct {
 type backend interface {
 	Put(id string, in io.Reader) error
 	Get(id string) (io.ReadCloser, int64, error)
-	Delete(ids ...string) error
 	List() ([]object, error)
+	Delete(ids ...string) error
+	DeleteIncomplete(cutoff time.Time) error
 }

attachment/backend_file.go

@@ -4,6 +4,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"time"
 
 	"heckel.io/ntfy/v2/log"
 )
@@ -41,6 +42,26 @@ func (b *fileBackend) Put(id string, in io.Reader) error {
 	return nil
 }
 
+func (b *fileBackend) List() ([]object, error) {
+	entries, err := os.ReadDir(b.dir)
+	if err != nil {
+		return nil, err
+	}
+	objects := make([]object, 0, len(entries))
+	for _, e := range entries {
+		info, err := e.Info()
+		if err != nil {
+			return nil, err
+		}
+		objects = append(objects, object{
+			ID:           e.Name(),
+			Size:         info.Size(),
+			LastModified: info.ModTime(),
+		})
+	}
+	return objects, nil
+}
+
 func (b *fileBackend) Get(id string) (io.ReadCloser, int64, error) {
 	file := filepath.Join(b.dir, id)
 	stat, err := os.Stat(file)
@@ -64,22 +85,6 @@ func (b *fileBackend) Delete(ids ...string) error {
 	return nil
 }
 
-func (b *fileBackend) List() ([]object, error) {
-	entries, err := os.ReadDir(b.dir)
-	if err != nil {
-		return nil, err
-	}
-	objects := make([]object, 0, len(entries))
-	for _, e := range entries {
-		info, err := e.Info()
-		if err != nil {
-			return nil, err
-		}
-		objects = append(objects, object{
-			ID:           e.Name(),
-			Size:         info.Size(),
-			LastModified: info.ModTime(),
-		})
-	}
-	return objects, nil
+func (b *fileBackend) DeleteIncomplete(_ time.Time) error {
+	return nil
 }

attachment/backend_s3.go

@@ -4,13 +4,16 @@ import (
 	"context"
 	"io"
 	"strings"
-
-	"heckel.io/ntfy/v2/s3"
+	"time"
 
 	"heckel.io/ntfy/v2/log"
+	"heckel.io/ntfy/v2/s3"
 )
 
-const tagS3Backend = "s3_backend"
+const (
+	tagS3Backend    = "s3_backend"
+	deleteBatchSize = 1000
+)
 
 type s3Backend struct {
 	client *s3.Client
@@ -30,24 +33,6 @@ func (b *s3Backend) Get(id string) (io.ReadCloser, int64, error) {
 	return b.client.GetObject(context.Background(), id)
 }
 
-func (b *s3Backend) Delete(ids ...string) error {
-	// S3 DeleteObjects supports up to 1000 keys per call
-	for i := 0; i < len(ids); i += 1000 {
-		end := i + 1000
-		if end > len(ids) {
-			end = len(ids)
-		}
-		batch := ids[i:end]
-		for _, id := range batch {
-			log.Tag(tagS3Backend).Field("message_id", id).Debug("Deleting attachment from S3")
-		}
-		if err := b.client.DeleteObjects(context.Background(), batch); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 func (b *s3Backend) List() ([]object, error) {
 	objects, err := b.client.ListAllObjects(context.Background())
 	if err != nil {
@@ -68,3 +53,25 @@ func (b *s3Backend) List() ([]object, error) {
 	}
 	return result, nil
 }
+
+func (b *s3Backend) Delete(ids ...string) error {
+	// S3 DeleteObjects supports up to 1000 keys per call
+	for i := 0; i < len(ids); i += deleteBatchSize {
+		end := i + deleteBatchSize
+		if end > len(ids) {
+			end = len(ids)
+		}
+		batch := ids[i:end]
+		for _, id := range batch {
+			log.Tag(tagS3Backend).Field("message_id", id).Debug("Deleting attachment from S3")
+		}
+		if err := b.client.DeleteObjects(context.Background(), batch); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (b *s3Backend) DeleteIncomplete(cutoff time.Time) error {
+	return b.client.AbortIncompleteUploads(context.Background(), cutoff)
+}

attachment/store.go

@@ -28,21 +28,22 @@ var (
 // Store manages attachment storage with shared logic for size tracking, limiting,
 // ID validation, and background sync to reconcile storage with the database.
 type Store struct {
-	backend          backend
-	totalSizeCurrent int64
-	totalSizeLimit   int64
-	localIDs         func() ([]string, error) // returns IDs that should exist
-	closeChan        chan struct{}
-	mu               sync.Mutex // Protects totalSizeCurrent
+	backend   backend
+	limit     int64                    // Defined limit of the store in bytes
+	size      int64                    // Current size of the store in bytes
+	sizes     map[string]int64         // File ID -> size, for subtracting on Remove
+	localIDs  func() ([]string, error) // Returns file IDs that should exist locally, used for sync()
+	closeChan chan struct{}
+	mu        sync.Mutex // Protects size and sizes
 }
 
 // NewFileStore creates a new file-system backed attachment cache
 func NewFileStore(dir string, totalSizeLimit int64, localIDsFn func() ([]string, error)) (*Store, error) {
-	backend, err := newFileBackend(dir)
+	b, err := newFileBackend(dir)
 	if err != nil {
 		return nil, err
 	}
-	return newStore(backend, totalSizeLimit, localIDsFn)
+	return newStore(b, totalSizeLimit, localIDsFn)
 }
 
 // NewS3Store creates a new S3-backed attachment cache. The s3URL must be in the format:
@@ -58,10 +59,11 @@ func NewS3Store(s3URL string, totalSizeLimit int64, localIDs func() ([]string, e
 
 func newStore(backend backend, totalSizeLimit int64, localIDs func() ([]string, error)) (*Store, error) {
 	c := &Store{
-		backend:        backend,
-		totalSizeLimit: totalSizeLimit,
-		localIDs:       localIDs,
-		closeChan:      make(chan struct{}),
+		backend:   backend,
+		limit:     totalSizeLimit,
+		sizes:     make(map[string]int64),
+		localIDs:  localIDs,
+		closeChan: make(chan struct{}),
 	}
 	if localIDs != nil {
 		go c.syncLoop()
@@ -85,7 +87,8 @@ func (c *Store) Write(id string, in io.Reader, limiters ...util.Limiter) (int64,
 	}
 	size := cr.Total()
 	c.mu.Lock()
-	c.totalSizeCurrent += size
+	c.size += size
+	c.sizes[id] = size
 	c.mu.Unlock()
 	return size, nil
 }
@@ -98,15 +101,30 @@ func (c *Store) Read(id string) (io.ReadCloser, int64, error) {
 	return c.backend.Get(id)
 }
 
-// Remove deletes attachment files by ID. It does NOT recompute the total size;
-// the next sync() call will correct it.
+// Remove deletes attachment files by ID and subtracts their known sizes from
+// the total. Sizes for objects not tracked (e.g. written before this process
+// started and before the first sync) are corrected by the next sync() call.
 func (c *Store) Remove(ids ...string) error {
 	for _, id := range ids {
 		if !fileIDRegex.MatchString(id) {
 			return errInvalidFileID
 		}
 	}
-	return c.backend.Delete(ids...)
+	if err := c.backend.Delete(ids...); err != nil {
+		return err
+	}
+	c.mu.Lock()
+	for _, id := range ids {
+		if size, ok := c.sizes[id]; ok {
+			c.size -= size
+			delete(c.sizes, id)
+		}
+	}
+	if c.size < 0 {
+		c.size = 0
+	}
+	c.mu.Unlock()
+	return nil
 }
 
 // sync reconciles the backend storage with the database. It lists all objects,
@@ -130,7 +148,8 @@ func (c *Store) sync() error {
 	// than the grace period to account for races, and skipping objects with invalid IDs.
 	cutoff := time.Now().Add(-orphanGracePeriod)
 	var orphanIDs []string
-	var totalSize int64
+	var size int64
+	sizes := make(map[string]int64, len(remoteObjects))
 	for _, obj := range remoteObjects {
 		if !fileIDRegex.MatchString(obj.ID) {
 			continue
@@ -138,12 +157,14 @@ func (c *Store) sync() error {
 		if _, ok := localIDMap[obj.ID]; !ok && obj.LastModified.Before(cutoff) {
 			orphanIDs = append(orphanIDs, obj.ID)
 		} else {
-			totalSize += obj.Size
+			size += obj.Size
+			sizes[obj.ID] = obj.Size
 		}
 	}
-	log.Tag(tagStore).Debug("Sync: cache size updated to %s", util.FormatSizeHuman(totalSize))
+	log.Tag(tagStore).Debug("Sync: cache size updated to %s", util.FormatSizeHuman(size))
 	c.mu.Lock()
-	c.totalSizeCurrent = totalSize
+	c.size = size
+	c.sizes = sizes
 	c.mu.Unlock()
 	// Delete orphaned attachments
 	if len(orphanIDs) > 0 {
@@ -152,6 +173,10 @@ func (c *Store) sync() error {
 			return fmt.Errorf("attachment sync: failed to delete orphaned objects: %w", err)
 		}
 	}
+	// Clean up incomplete uploads (S3 only)
+	if err := c.backend.DeleteIncomplete(cutoff); err != nil {
+		log.Tag(tagStore).Err(err).Warn("Sync: failed to abort incomplete uploads")
+	}
 	return nil
 }
 
@@ -159,14 +184,14 @@ func (c *Store) sync() error {
 func (c *Store) Size() int64 {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	return c.totalSizeCurrent
+	return c.size
 }
 
 // Remaining returns the remaining capacity for attachments
 func (c *Store) Remaining() int64 {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	remaining := c.totalSizeLimit - c.totalSizeCurrent
+	remaining := c.limit - c.size
 	if remaining < 0 {
 		return 0
 	}

attachment/store_file_test.go

@@ -57,8 +57,8 @@ func TestFileStore_Write_Remove_Success(t *testing.T) {
 	require.Nil(t, c.Remove("abcdefghijk1", "abcdefghijk5"))
 	require.NoFileExists(t, dir+"/abcdefghijk1")
 	require.NoFileExists(t, dir+"/abcdefghijk5")
-	// Size is not recomputed by Remove; it stays stale until next sync
-	require.Equal(t, int64(9990), c.Size())
+	require.Equal(t, int64(8*999), c.Size())
+	require.Equal(t, int64(10240-8*999), c.Remaining())
 }
 
 func TestFileStore_Write_FailedTotalSizeLimit(t *testing.T) {

attachment/store_s3_test.go

@@ -42,7 +42,7 @@ func TestS3Store_WriteReadRemove(t *testing.T) {
 
 	// Remove
 	require.Nil(t, cache.Remove("abcdefghijkl"))
-	// Size is not recomputed by Remove; stays stale until next sync
+	require.Equal(t, int64(0), cache.Size())
 
 	// Read after remove should fail
 	_, _, err = cache.Read("abcdefghijkl")
@@ -107,8 +107,7 @@ func TestS3Store_WriteRemoveMultiple(t *testing.T) {
 	require.Equal(t, int64(500), cache.Size())
 
 	require.Nil(t, cache.Remove("abcdefghijk1", "abcdefghijk3"))
-	// Size not recomputed by Remove
-	require.Equal(t, int64(500), cache.Size())
+	require.Equal(t, int64(300), cache.Size())
 }
 
 func TestS3Store_ReadNotFound(t *testing.T) {