Add support for deleting individual images from zip galleries

pkg/file/delete.go

@@ -70,6 +70,7 @@ type Deleter struct {
 	dirs           []string
 	TrashPath      string            // if set, files will be moved to this directory instead of being permanently deleted
 	trashedPaths   map[string]string // map of original path -> trash path (only used when TrashPath is set)
+	zipEntries     map[string][]string // zipPath -> entries to remove; rewrites happen at Commit
 }
 
 func NewDeleter() *Deleter {
@@ -173,6 +174,16 @@ func (d *Deleter) dirsInternal(paths []string, bypassTrash bool) error {
 	return nil
 }
 
+// ZipEntry marks a single entry within a zip for removal.
+// On Commit, each zip with pending entries is rewritten exactly once without those entries.
+// entryRelPath must be the relative path of the entry within the zip using forward slashes.
+func (d *Deleter) ZipEntry(zipPath, entryRelPath string) {
+	if d.zipEntries == nil {
+		d.zipEntries = make(map[string][]string)
+	}
+	d.zipEntries[zipPath] = append(d.zipEntries[zipPath], entryRelPath)
+}
+
 // Rollback tries to rename all marked files and directories back to their
 // original names and clears the marked list. Any errors encountered are
 // logged. All files will be attempted regardless of any errors occurred.
@@ -186,6 +197,7 @@ func (d *Deleter) Rollback() {
 	d.files = nil
 	d.dirs = nil
 	d.trashedPaths = make(map[string]string)
+	d.zipEntries = nil
 }
 
 // Commit deletes all files marked for deletion and clears the marked list.
@@ -212,9 +224,23 @@ func (d *Deleter) Commit() {
 		}
 	}
 
+	// Rewrite each zip that has pending entry removals.
+	for zipPath, entries := range d.zipEntries {
+		tmpPath, err := RemoveEntriesFromZip(zipPath, entries)
+		if err != nil {
+			logger.Warnf("Error rewriting zip %q: %v", zipPath, err)
+			continue
+		}
+		if err := d.RenamerRemover.Rename(tmpPath, zipPath); err != nil {
+			_ = d.RenamerRemover.Remove(tmpPath)
+			logger.Warnf("Error applying zip rewrite for %q: %v", zipPath, err)
+		}
+	}
+
 	d.files = nil
 	d.dirs = nil
 	d.trashedPaths = make(map[string]string)
+	d.zipEntries = nil
 }
 
 func (d *Deleter) renameForDelete(path string, bypassTrash bool) error {

pkg/file/delete_test.go

@@ -0,0 +1,183 @@
+package file
+
+import (
+	"archive/zip"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// fakeRenamerRemover records Rename/Remove calls and delegates to real OS operations.
+type fakeRenamerRemover struct {
+	renames []renameOp
+	removes []string
+}
+
+type renameOp struct{ from, to string }
+
+func (f *fakeRenamerRemover) Rename(oldpath, newpath string) error {
+	f.renames = append(f.renames, renameOp{oldpath, newpath})
+	return os.Rename(oldpath, newpath)
+}
+
+func (f *fakeRenamerRemover) Remove(name string) error {
+	f.removes = append(f.removes, name)
+	return os.Remove(name)
+}
+
+func (f *fakeRenamerRemover) RemoveAll(path string) error {
+	return os.RemoveAll(path)
+}
+
+func (f *fakeRenamerRemover) Stat(path string) (fs.FileInfo, error) {
+	return os.Stat(path)
+}
+
+func newDeleterWithFake(t *testing.T) (*Deleter, *fakeRenamerRemover) {
+	t.Helper()
+	fake := &fakeRenamerRemover{}
+	d := NewDeleter()
+	d.RenamerRemover = fake
+	return d, fake
+}
+
+func TestDeleter_ZipEntry_Accumulates(t *testing.T) {
+	d, _ := newDeleterWithFake(t)
+
+	d.ZipEntry("/path/to/gallery.zip", "a.jpg")
+	d.ZipEntry("/path/to/gallery.zip", "b.jpg")
+
+	entries := d.zipEntries["/path/to/gallery.zip"]
+	if len(entries) != 2 {
+		t.Errorf("expected 2 entries, got %d: %v", len(entries), entries)
+	}
+}
+
+func TestDeleter_ZipEntry_MultipleZips(t *testing.T) {
+	d, _ := newDeleterWithFake(t)
+
+	d.ZipEntry("/zip/one.zip", "img1.jpg")
+	d.ZipEntry("/zip/two.zip", "img2.jpg")
+	d.ZipEntry("/zip/one.zip", "img3.jpg")
+
+	if len(d.zipEntries["/zip/one.zip"]) != 2 {
+		t.Errorf("one.zip: expected 2 entries, got %d", len(d.zipEntries["/zip/one.zip"]))
+	}
+	if len(d.zipEntries["/zip/two.zip"]) != 1 {
+		t.Errorf("two.zip: expected 1 entry, got %d", len(d.zipEntries["/zip/two.zip"]))
+	}
+}
+
+func TestDeleter_Rollback_ClearsZipEntries(t *testing.T) {
+	d, _ := newDeleterWithFake(t)
+
+	d.ZipEntry("/path/to/gallery.zip", "a.jpg")
+	d.Rollback()
+
+	if d.zipEntries != nil {
+		t.Errorf("expected zipEntries to be nil after Rollback, got %v", d.zipEntries)
+	}
+}
+
+func TestDeleter_Commit_RewritesZip(t *testing.T) {
+	dir := t.TempDir()
+	zipPath := filepath.Join(dir, "gallery.zip")
+	makeTestZip(t, zipPath, "", map[string]string{
+		"keep.jpg":   "keep",
+		"remove.jpg": "remove",
+	})
+
+	d, fake := newDeleterWithFake(t)
+	d.ZipEntry(zipPath, "remove.jpg")
+	d.Commit()
+
+	// Rename should have been called once: tmpPath → zipPath
+	if len(fake.renames) != 1 {
+		t.Fatalf("expected 1 Rename call, got %d", len(fake.renames))
+	}
+	if fake.renames[0].to != zipPath {
+		t.Errorf("Rename target = %q, want %q", fake.renames[0].to, zipPath)
+	}
+
+	// The original zip should now contain only keep.jpg
+	r, err := zip.OpenReader(zipPath)
+	if err != nil {
+		t.Fatalf("open rewritten zip: %v", err)
+	}
+	defer r.Close()
+
+	names := make(map[string]bool)
+	for _, f := range r.File {
+		names[f.Name] = true
+	}
+	if names["remove.jpg"] {
+		t.Error("remove.jpg still present after Commit")
+	}
+	if !names["keep.jpg"] {
+		t.Error("keep.jpg missing after Commit")
+	}
+}
+
+func TestDeleter_Commit_ClearsZipEntries(t *testing.T) {
+	dir := t.TempDir()
+	zipPath := filepath.Join(dir, "gallery.zip")
+	makeTestZip(t, zipPath, "", map[string]string{
+		"a.jpg": "a",
+	})
+
+	d, _ := newDeleterWithFake(t)
+	d.ZipEntry(zipPath, "a.jpg")
+	d.Commit()
+
+	if d.zipEntries != nil {
+		t.Errorf("expected zipEntries to be nil after Commit, got %v", d.zipEntries)
+	}
+}
+
+func TestDeleter_Commit_MultipleZips(t *testing.T) {
+	dir := t.TempDir()
+
+	zip1 := filepath.Join(dir, "one.zip")
+	zip2 := filepath.Join(dir, "two.zip")
+	makeTestZip(t, zip1, "", map[string]string{"a.jpg": "a", "b.jpg": "b"})
+	makeTestZip(t, zip2, "", map[string]string{"c.jpg": "c", "d.jpg": "d"})
+
+	d, fake := newDeleterWithFake(t)
+	d.ZipEntry(zip1, "a.jpg")
+	d.ZipEntry(zip2, "c.jpg")
+	d.Commit()
+
+	if len(fake.renames) != 2 {
+		t.Fatalf("expected 2 Rename calls, got %d", len(fake.renames))
+	}
+
+	for _, zp := range []string{zip1, zip2} {
+		r, err := zip.OpenReader(zp)
+		if err != nil {
+			t.Fatalf("open zip %s: %v", zp, err)
+		}
+		names := make(map[string]bool)
+		for _, f := range r.File {
+			names[f.Name] = true
+		}
+		r.Close()
+
+		switch zp {
+		case zip1:
+			if names["a.jpg"] {
+				t.Error("one.zip: a.jpg should have been removed")
+			}
+			if !names["b.jpg"] {
+				t.Error("one.zip: b.jpg should be present")
+			}
+		case zip2:
+			if names["c.jpg"] {
+				t.Error("two.zip: c.jpg should have been removed")
+			}
+			if !names["d.jpg"] {
+				t.Error("two.zip: d.jpg should be present")
+			}
+		}
+	}
+}

pkg/file/zip.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"os"
 	"path/filepath"
 
 	"github.com/stashapp/stash/pkg/logger"
@@ -47,41 +48,11 @@ func newZipFS(fs models.FS, path string, size int64) (*zipFS, error) {
 		return nil, err
 	}
 
-	// Concat all Name and Comment for better detection result
-	var buffer bytes.Buffer
-	for _, f := range zipReader.File {
-		buffer.WriteString(f.Name)
-		buffer.WriteString(f.Comment)
-	}
-	buffer.WriteString(zipReader.Comment)
-
-	// Detect encoding
-	d, err := chardet.NewTextDetector().DetectBest(buffer.Bytes())
-	if err != nil {
-		// If we can't detect the encoding, just assume it's UTF8
-		logger.Warnf("Unable to detect decoding for %s: %w", path, err)
-	}
-
-	// If the charset is not UTF8, decode'em
-	if d != nil && d.Charset != "UTF-8" {
-		logger.Debugf("Detected non-utf8 zip charset %s (%s): %s", d.Charset, d.Language, path)
-
-		e, _ := charset.Lookup(d.Charset)
-		if e == nil {
-			// if we can't find the encoding, just assume it's UTF8
-			logger.Warnf("Failed to lookup charset %s, language %s", d.Charset, d.Language)
-		} else {
-			decoder := e.NewDecoder()
-			for _, f := range zipReader.File {
-				newName, _, err := transform.String(decoder, f.Name)
-				if err != nil {
-					reader.Close()
-					logger.Warnf("Failed to decode %v: %v", []byte(f.Name), err)
-				} else {
-					f.Name = newName
-				}
-				// Comments are not decoded cuz stash doesn't use that
-			}
+	// Detect and apply non-UTF-8 encoding for filenames.
+	for i, name := range decodeZipEntryNames(zipReader.File, zipReader.Comment) {
+		if name != zipReader.File[i].Name {
+			logger.Debugf("Decoded non-utf8 zip entry in %s: %q -> %q", path, zipReader.File[i].Name, name)
+			zipReader.File[i].Name = name
 		}
 	}
 
@@ -188,3 +159,93 @@ func (f *wrappedReadCloser) Close() error {
 	_ = f.ReadCloser.Close()
 	return f.outer.Close()
 }
+
+// decodeZipEntryNames returns the decoded UTF-8 name for each entry in files.
+// If the zip uses a non-UTF-8 encoding, chardet is used to detect and decode.
+// Names are returned unchanged when detection fails or the charset is already UTF-8.
+func decodeZipEntryNames(files []*zip.File, archiveComment string) []string {
+	names := make([]string, len(files))
+	for i, f := range files {
+		names[i] = f.Name
+	}
+
+	var buf bytes.Buffer
+	for _, f := range files {
+		buf.WriteString(f.Name)
+		buf.WriteString(f.Comment)
+	}
+	buf.WriteString(archiveComment)
+
+	d, err := chardet.NewTextDetector().DetectBest(buf.Bytes())
+	if err != nil || d == nil || d.Charset == "UTF-8" {
+		return names
+	}
+
+	e, _ := charset.Lookup(d.Charset)
+	if e == nil {
+		return names
+	}
+
+	decoder := e.NewDecoder()
+	for i, f := range files {
+		if decoded, _, err := transform.String(decoder, f.Name); err == nil {
+			names[i] = decoded
+		}
+	}
+	return names
+}
+
+// RemoveEntriesFromZip rewrites the zip at zipPath to a new temporary file in the same
+// directory, excluding all entries whose relative paths (using forward slashes) appear in
+// entriesToRemove. Returns the path of the temporary file. The caller is responsible for
+// atomically replacing the original zip with the temp file, or cleaning it up on rollback.
+func RemoveEntriesFromZip(zipPath string, entriesToRemove []string) (string, error) {
+	r, err := zip.OpenReader(zipPath)
+	if err != nil {
+		return "", fmt.Errorf("opening zip %q: %w", zipPath, err)
+	}
+	defer r.Close()
+
+	exclude := make(map[string]bool, len(entriesToRemove))
+	for _, e := range entriesToRemove {
+		exclude[e] = true
+	}
+
+	decodedNames := decodeZipEntryNames(r.File, r.Comment)
+
+	tmpFile, err := os.CreateTemp(filepath.Dir(zipPath), "*.tmp")
+	if err != nil {
+		return "", fmt.Errorf("creating temp file: %w", err)
+	}
+	tmpPath := tmpFile.Name()
+
+	w := zip.NewWriter(tmpFile)
+	for i, f := range r.File {
+		if exclude[filepath.ToSlash(decodedNames[i])] {
+			continue
+		}
+		if err := w.Copy(f); err != nil {
+			_ = w.Close()
+			_ = tmpFile.Close()
+			_ = os.Remove(tmpPath)
+			return "", fmt.Errorf("copying zip entry %q: %w", f.Name, err)
+		}
+	}
+	if err := w.SetComment(r.Comment); err != nil {
+		_ = w.Close()
+		_ = tmpFile.Close()
+		_ = os.Remove(tmpPath)
+		return "", fmt.Errorf("setting zip comment: %w", err)
+	}
+	if err := w.Close(); err != nil {
+		_ = tmpFile.Close()
+		_ = os.Remove(tmpPath)
+		return "", fmt.Errorf("finalizing zip: %w", err)
+	}
+	if err := tmpFile.Close(); err != nil {
+		_ = os.Remove(tmpPath)
+		return "", fmt.Errorf("closing temp file: %w", err)
+	}
+
+	return tmpPath, nil
+}