From 8ef6f9b930136d319c6665a8be648feed941ef83 Mon Sep 17 00:00:00 2001 From: Nebu Pookins Date: Sun, 5 Jul 2026 12:51:26 -0700 Subject: [PATCH] Add support for deleting individual images from zip galleries When deleting an image whose file lives inside a zip/cbz archive, remove just that entry from the zip. (Previously, these deletion requests would be silently ignored). Uses a deferred rewrite strategy: entries are accumulated during the transaction and the zip is rewritten at Commit() time, keeping bulk deletions efficient. Also extracts a shared decodeZipEntryNames helper to avoid duplicating charset-detection logic between the zip FS reader and the rewriter. --- pkg/file/delete.go | 26 ++++ pkg/file/delete_test.go | 183 +++++++++++++++++++++++++ pkg/file/zip.go | 131 +++++++++++++----- pkg/file/zip_test.go | 292 ++++++++++++++++++++++++++++++++++++++++ pkg/image/delete.go | 45 +++++-- 5 files changed, 631 insertions(+), 46 deletions(-) create mode 100644 pkg/file/delete_test.go create mode 100644 pkg/file/zip_test.go diff --git a/pkg/file/delete.go b/pkg/file/delete.go index c36068faac..3eec15e5a1 100644 --- a/pkg/file/delete.go +++ b/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 { diff --git a/pkg/file/delete_test.go b/pkg/file/delete_test.go new file mode 100644 index 0000000000..b327781dcb --- /dev/null +++ b/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") + } + } + } +} diff --git a/pkg/file/zip.go b/pkg/file/zip.go index 6d00c7e350..793d81509d 100644 --- a/pkg/file/zip.go +++ b/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 +} diff --git a/pkg/file/zip_test.go b/pkg/file/zip_test.go new file mode 100644 index 0000000000..21ffe9e75f --- /dev/null +++ b/pkg/file/zip_test.go @@ -0,0 +1,292 @@ +package file + +import ( + "archive/zip" + "io" + "os" + "path/filepath" + "testing" +) + +// makeTestZip creates a zip file at path containing the given entries (name → content). +// If comment is non-empty it is set as the archive-level comment. +func makeTestZip(t *testing.T, path, comment string, entries map[string]string) { + t.Helper() + f, err := os.Create(path) + if err != nil { + t.Fatalf("create zip: %v", err) + } + defer f.Close() + + w := zip.NewWriter(f) + for name, content := range entries { + fw, err := w.Create(name) + if err != nil { + t.Fatalf("zip.Create %q: %v", name, err) + } + if _, err := fw.Write([]byte(content)); err != nil { + t.Fatalf("zip write %q: %v", name, err) + } + } + if comment != "" { + if err := w.SetComment(comment); err != nil { + t.Fatalf("zip.SetComment: %v", err) + } + } + if err := w.Close(); err != nil { + t.Fatalf("zip.Close: %v", err) + } +} + +// readZipEntries returns a map of name → content from the zip at path. +func readZipEntries(t *testing.T, path string) map[string]string { + t.Helper() + r, err := zip.OpenReader(path) + if err != nil { + t.Fatalf("open zip %q: %v", path, err) + } + defer r.Close() + + out := make(map[string]string, len(r.File)) + for _, f := range r.File { + rc, err := f.Open() + if err != nil { + t.Fatalf("open entry %q: %v", f.Name, err) + } + data, err := io.ReadAll(rc) + rc.Close() + if err != nil { + t.Fatalf("read entry %q: %v", f.Name, err) + } + out[f.Name] = string(data) + } + return out +} + +func readZipComment(t *testing.T, path string) string { + t.Helper() + r, err := zip.OpenReader(path) + if err != nil { + t.Fatalf("open zip %q: %v", path, err) + } + defer r.Close() + return r.Comment +} + +func TestRemoveEntriesFromZip_Single(t *testing.T) { + dir := t.TempDir() + zipPath := filepath.Join(dir, "gallery.zip") + makeTestZip(t, zipPath, "", map[string]string{ + "keep.jpg": "keep-data", + "remove.jpg": "remove-data", + }) + + tmpPath, err := RemoveEntriesFromZip(zipPath, []string{"remove.jpg"}) + if err != nil { + t.Fatalf("RemoveEntriesFromZip: %v", err) + } + defer os.Remove(tmpPath) + + got := readZipEntries(t, tmpPath) + if _, present := got["remove.jpg"]; present { + t.Error("remove.jpg should not be in output zip") + } + if got["keep.jpg"] != "keep-data" { + t.Errorf("keep.jpg content = %q, want %q", got["keep.jpg"], "keep-data") + } +} + +func TestRemoveEntriesFromZip_Multiple(t *testing.T) { + dir := t.TempDir() + zipPath := filepath.Join(dir, "gallery.zip") + makeTestZip(t, zipPath, "", map[string]string{ + "a.jpg": "a", + "b.jpg": "b", + "c.jpg": "c", + }) + + tmpPath, err := RemoveEntriesFromZip(zipPath, []string{"a.jpg", "b.jpg"}) + if err != nil { + t.Fatalf("RemoveEntriesFromZip: %v", err) + } + defer os.Remove(tmpPath) + + got := readZipEntries(t, tmpPath) + for _, name := range []string{"a.jpg", "b.jpg"} { + if _, present := got[name]; present { + t.Errorf("%s should not be in output zip", name) + } + } + if got["c.jpg"] != "c" { + t.Errorf("c.jpg content = %q, want %q", got["c.jpg"], "c") + } +} + +func TestRemoveEntriesFromZip_EntryNotPresent(t *testing.T) { + dir := t.TempDir() + zipPath := filepath.Join(dir, "gallery.zip") + makeTestZip(t, zipPath, "", map[string]string{ + "a.jpg": "a", + "b.jpg": "b", + }) + + // "ghost.jpg" does not exist in the zip — should not cause an error + tmpPath, err := RemoveEntriesFromZip(zipPath, []string{"ghost.jpg"}) + if err != nil { + t.Fatalf("RemoveEntriesFromZip: %v", err) + } + defer os.Remove(tmpPath) + + got := readZipEntries(t, tmpPath) + if len(got) != 2 { + t.Errorf("expected 2 entries, got %d: %v", len(got), got) + } +} + +func TestRemoveEntriesFromZip_AllEntries(t *testing.T) { + dir := t.TempDir() + zipPath := filepath.Join(dir, "gallery.zip") + makeTestZip(t, zipPath, "", map[string]string{ + "a.jpg": "a", + "b.jpg": "b", + }) + + tmpPath, err := RemoveEntriesFromZip(zipPath, []string{"a.jpg", "b.jpg"}) + if err != nil { + t.Fatalf("RemoveEntriesFromZip: %v", err) + } + defer os.Remove(tmpPath) + + got := readZipEntries(t, tmpPath) + if len(got) != 0 { + t.Errorf("expected empty zip, got entries: %v", got) + } +} + +func TestRemoveEntriesFromZip_PreservesContent(t *testing.T) { + dir := t.TempDir() + zipPath := filepath.Join(dir, "gallery.zip") + want := "the quick brown fox" + makeTestZip(t, zipPath, "", map[string]string{ + "keep.jpg": want, + "remove.jpg": "trash", + }) + + tmpPath, err := RemoveEntriesFromZip(zipPath, []string{"remove.jpg"}) + if err != nil { + t.Fatalf("RemoveEntriesFromZip: %v", err) + } + defer os.Remove(tmpPath) + + got := readZipEntries(t, tmpPath) + if got["keep.jpg"] != want { + t.Errorf("keep.jpg content = %q, want %q", got["keep.jpg"], want) + } +} + +func TestRemoveEntriesFromZip_PreservesComment(t *testing.T) { + dir := t.TempDir() + zipPath := filepath.Join(dir, "gallery.zip") + wantComment := "Created by Stash" + makeTestZip(t, zipPath, wantComment, map[string]string{ + "keep.jpg": "data", + "remove.jpg": "trash", + }) + + tmpPath, err := RemoveEntriesFromZip(zipPath, []string{"remove.jpg"}) + if err != nil { + t.Fatalf("RemoveEntriesFromZip: %v", err) + } + defer os.Remove(tmpPath) + + if got := readZipComment(t, tmpPath); got != wantComment { + t.Errorf("archive comment = %q, want %q", got, wantComment) + } +} + +func TestRemoveEntriesFromZip_SubdirectoryEntry(t *testing.T) { + dir := t.TempDir() + zipPath := filepath.Join(dir, "gallery.zip") + makeTestZip(t, zipPath, "", map[string]string{ + "subdir/keep.jpg": "keep", + "subdir/remove.jpg": "remove", + }) + + tmpPath, err := RemoveEntriesFromZip(zipPath, []string{"subdir/remove.jpg"}) + if err != nil { + t.Fatalf("RemoveEntriesFromZip: %v", err) + } + defer os.Remove(tmpPath) + + got := readZipEntries(t, tmpPath) + if _, present := got["subdir/remove.jpg"]; present { + t.Error("subdir/remove.jpg should not be in output zip") + } + if got["subdir/keep.jpg"] != "keep" { + t.Errorf("subdir/keep.jpg content = %q, want %q", got["subdir/keep.jpg"], "keep") + } +} + +func TestRemoveEntriesFromZip_InvalidPath(t *testing.T) { + _, err := RemoveEntriesFromZip("/nonexistent/path/gallery.zip", []string{"a.jpg"}) + if err == nil { + t.Error("expected error for non-existent zip, got nil") + } +} + +func TestDecodeZipEntryNames_UTF8(t *testing.T) { + files := []*zip.File{ + {FileHeader: zip.FileHeader{Name: "hello.jpg"}}, + {FileHeader: zip.FileHeader{Name: "world.jpg"}}, + } + + got := decodeZipEntryNames(files, "") + want := []string{"hello.jpg", "world.jpg"} + + if len(got) != len(want) { + t.Fatalf("len = %d, want %d", len(got), len(want)) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("[%d] = %q, want %q", i, got[i], want[i]) + } + } +} + +func TestDecodeZipEntryNames_Empty(t *testing.T) { + got := decodeZipEntryNames(nil, "") + if len(got) != 0 { + t.Errorf("expected empty slice, got %v", got) + } +} + +func TestDecodeZipEntryNames_NonUTF8ShiftJIS(t *testing.T) { + // Shift-JIS encoding of "テスト.jpg" (te-su-to = test in Japanese) + shiftJISName := "\x83\x65\x83\x58\x83\x67.jpg" + files := []*zip.File{ + {FileHeader: zip.FileHeader{Name: shiftJISName}}, + } + + got := decodeZipEntryNames(files, "") + + if len(got) != 1 { + t.Fatalf("len = %d, want 1", len(got)) + } + // The name should have been decoded to valid UTF-8 (different from the raw bytes) + if got[0] == shiftJISName { + t.Error("expected name to be decoded from Shift-JIS, but it was returned unchanged") + } +} + +func TestDecodeZipEntryNames_LengthMatchesInput(t *testing.T) { + files := []*zip.File{ + {FileHeader: zip.FileHeader{Name: "a.jpg"}}, + {FileHeader: zip.FileHeader{Name: "b.jpg"}}, + {FileHeader: zip.FileHeader{Name: "c.jpg"}}, + } + + got := decodeZipEntryNames(files, "") + if len(got) != len(files) { + t.Errorf("len = %d, want %d", len(got), len(files)) + } +} diff --git a/pkg/image/delete.go b/pkg/image/delete.go index 28bb54a593..fd6a4553eb 100644 --- a/pkg/image/delete.go +++ b/pkg/image/delete.go @@ -3,6 +3,7 @@ package image import ( "context" "fmt" + "path/filepath" "github.com/stashapp/stash/pkg/file" "github.com/stashapp/stash/pkg/fsutil" @@ -168,7 +169,8 @@ func (s *Service) destroyImage(ctx context.Context, i *models.Image, fileDeleter return s.Repository.Destroy(ctx, i.ID) } -// deleteFiles deletes files for the image from the database and file system, if they are not in use by other images +// deleteFiles deletes files for the image from the database and file system, if they are not in use by other images. +// For files inside a zip archive, the entry is removed from the zip rather than deleting the zip itself. func (s *Service) deleteFiles(ctx context.Context, i *models.Image, fileDeleter *FileDeleter) error { if err := i.LoadFiles(ctx, s.Repository); err != nil { return err @@ -186,10 +188,17 @@ func (s *Service) deleteFiles(ctx context.Context, i *models.Image, fileDeleter continue } - // don't delete files in zip archives - const deleteFile = true - if f.Base().ZipFileID == nil { + if f.Base().ZipFileID != nil { + logger.Infof("Removing image from zip: %s", f.Base().Path) + if err := file.Destroy(ctx, s.File, f, fileDeleter.Deleter, false); err != nil { + return err + } + if err := s.removeEntryFromZip(f, fileDeleter); err != nil { + return err + } + } else { logger.Info("Deleting image file: ", f.Base().Path) + const deleteFile = true if err := file.Destroy(ctx, s.File, f, fileDeleter.Deleter, deleteFile); err != nil { return err } @@ -199,6 +208,23 @@ func (s *Service) deleteFiles(ctx context.Context, i *models.Image, fileDeleter return nil } +// removeEntryFromZip queues removal of f's entry from its containing zip archive. +func (s *Service) removeEntryFromZip(f models.File, fileDeleter *FileDeleter) error { + zipFile := f.Base().ZipFile + if zipFile == nil { + return fmt.Errorf("zip file not loaded for %s", f.Base().Path) + } + + zipPath := zipFile.Base().Path + entryRelPath, err := filepath.Rel(zipPath, f.Base().Path) + if err != nil { + return fmt.Errorf("computing zip entry path: %w", err) + } + + fileDeleter.Deleter.ZipEntry(zipPath, filepath.ToSlash(entryRelPath)) + return nil +} + // destroyFileEntries destroys file entries from the database without deleting // the files from the filesystem func (s *Service) destroyFileEntries(ctx context.Context, i *models.Image) error { @@ -218,13 +244,10 @@ func (s *Service) destroyFileEntries(ctx context.Context, i *models.Image) error continue } - // don't destroy files in zip archives - if f.Base().ZipFileID == nil { - const deleteFile = false - logger.Info("Destroying image file entry: ", f.Base().Path) - if err := file.Destroy(ctx, s.File, f, nil, deleteFile); err != nil { - return err - } + const deleteFile = false + logger.Info("Destroying image file entry: ", f.Base().Path) + if err := file.Destroy(ctx, s.File, f, nil, deleteFile); err != nil { + return err } }