fix(media): report recordings as saved only while their path names the reserved file
- commit
- 8802f1663e2083e6882fe0c72824f499bee0b0b0
- author
- brice · · 2026-09-30 15:01 UTC
- parent
- d4cd3ed
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +77 -1
M internal/adapters/capturefile/capturefile.go +7 -0
| @@ -47,6 +47,13 @@ func create(path string, access int) (*os.File, *Reservation, error) { | |||
| 47 | 47 | return f, &Reservation{path: path, info: info}, nil | |
| 48 | 48 | } | |
| 49 | 49 | ||
| 50 | + | // Owns reports whether path still names the reserved file, so a moved and | |
| 51 | + | // replaced or deleted output is never reported as saved. | |
| 52 | + | func (r *Reservation) Owns() bool { | |
| 53 | + | cur, err := os.Lstat(r.path) | |
| 54 | + | return err == nil && os.SameFile(r.info, cur) | |
| 55 | + | } | |
| 56 | + | ||
| 50 | 57 | // Remove deletes the reserved file if path still names it. A missing path is | |
| 51 | 58 | // success; a path that now names a different file (the original was moved and | |
| 52 | 59 | // replaced) is left untouched and is not an error. There is an unavoidable | |
| @@ -47,6 +47,13 @@ func create(path string, access int) (*os.File, *Reservation, error) { | |||||
| 47 | return f, &Reservation{path: path, info: info}, nil | 47 | return f, &Reservation{path: path, info: info}, nil | ||
| 48 | } | 48 | } | ||
| 49 | 49 | ||||
| 50 | + | // Owns reports whether path still names the reserved file, so a moved and | |||
| 51 | + | // replaced or deleted output is never reported as saved. | |||
| 52 | + | func (r *Reservation) Owns() bool { | |||
| 53 | + | cur, err := os.Lstat(r.path) | |||
| 54 | + | return err == nil && os.SameFile(r.info, cur) | |||
| 55 | + | } | |||
| 56 | + | ||||
| 50 | // Remove deletes the reserved file if path still names it. A missing path is | 57 | // Remove deletes the reserved file if path still names it. A missing path is | ||
| 51 | // success; a path that now names a different file (the original was moved and | 58 | // success; a path that now names a different file (the original was moved and | ||
| 52 | // replaced) is left untouched and is not an error. There is an unavoidable | 59 | // replaced) is left untouched and is not an error. There is an unavoidable | ||
M internal/adapters/capturefile/capturefile_test.go +25 -0
| @@ -93,3 +93,28 @@ func TestRemoveLeavesReplacement(t *testing.T) { | |||
| 93 | 93 | t.Fatalf("moved original touched: %v", err) | |
| 94 | 94 | } | |
| 95 | 95 | } | |
| 96 | + | ||
| 97 | + | func TestOwnsOnlyTheReservedFile(t *testing.T) { | |
| 98 | + | dir := t.TempDir() | |
| 99 | + | path := filepath.Join(dir, "a") | |
| 100 | + | f, res, err := Create(path) | |
| 101 | + | if err != nil { | |
| 102 | + | t.Fatal(err) | |
| 103 | + | } | |
| 104 | + | defer f.Close() | |
| 105 | + | if !res.Owns() { | |
| 106 | + | t.Fatal("reserved file not owned") | |
| 107 | + | } | |
| 108 | + | if err := os.Rename(path, filepath.Join(dir, "moved")); err != nil { | |
| 109 | + | t.Fatal(err) | |
| 110 | + | } | |
| 111 | + | if res.Owns() { | |
| 112 | + | t.Fatal("missing path reported as owned") | |
| 113 | + | } | |
| 114 | + | if err := os.WriteFile(path, []byte("user"), 0o600); err != nil { | |
| 115 | + | t.Fatal(err) | |
| 116 | + | } | |
| 117 | + | if res.Owns() { | |
| 118 | + | t.Fatal("replacement reported as owned") | |
| 119 | + | } | |
| 120 | + | } | |
| @@ -93,3 +93,28 @@ func TestRemoveLeavesReplacement(t *testing.T) { | |||||
| 93 | t.Fatalf("moved original touched: %v", err) | 93 | t.Fatalf("moved original touched: %v", err) | ||
| 94 | } | 94 | } | ||
| 95 | } | 95 | } | ||
| 96 | + | ||||
| 97 | + | func TestOwnsOnlyTheReservedFile(t *testing.T) { | |||
| 98 | + | dir := t.TempDir() | |||
| 99 | + | path := filepath.Join(dir, "a") | |||
| 100 | + | f, res, err := Create(path) | |||
| 101 | + | if err != nil { | |||
| 102 | + | t.Fatal(err) | |||
| 103 | + | } | |||
| 104 | + | defer f.Close() | |||
| 105 | + | if !res.Owns() { | |||
| 106 | + | t.Fatal("reserved file not owned") | |||
| 107 | + | } | |||
| 108 | + | if err := os.Rename(path, filepath.Join(dir, "moved")); err != nil { | |||
| 109 | + | t.Fatal(err) | |||
| 110 | + | } | |||
| 111 | + | if res.Owns() { | |||
| 112 | + | t.Fatal("missing path reported as owned") | |||
| 113 | + | } | |||
| 114 | + | if err := os.WriteFile(path, []byte("user"), 0o600); err != nil { | |||
| 115 | + | t.Fatal(err) | |||
| 116 | + | } | |||
| 117 | + | if res.Owns() { | |||
| 118 | + | t.Fatal("replacement reported as owned") | |||
| 119 | + | } | |||
| 120 | + | } | |||
M internal/adapters/ffmpeg/ffmpeg.go +1 -1
| @@ -475,7 +475,7 @@ func (w *Writer) shutdown(abort bool) error { | |||
| 475 | 475 | if err != nil { | |
| 476 | 476 | err = errors.Join(err, w.removeOutput()) | |
| 477 | 477 | } else { | |
| 478 | - | w.Saved = true | |
| 478 | + | w.Saved = w.res.Owns() | |
| 479 | 479 | } | |
| 480 | 480 | return err | |
| 481 | 481 | } | |
| @@ -475,7 +475,7 @@ func (w *Writer) shutdown(abort bool) error { | |||||
| 475 | if err != nil { | 475 | if err != nil { | ||
| 476 | err = errors.Join(err, w.removeOutput()) | 476 | err = errors.Join(err, w.removeOutput()) | ||
| 477 | } else { | 477 | } else { | ||
| 478 | - | w.Saved = true | 478 | + | w.Saved = w.res.Owns() |
| 479 | } | 479 | } | ||
| 480 | return err | 480 | return err | ||
| 481 | } | 481 | } | ||
M internal/adapters/ffmpeg/ffmpeg_test.go +44 -0
| @@ -663,6 +663,50 @@ func TestTerminalCleanupSparesReplacementFile(t *testing.T) { | |||
| 663 | 663 | } | |
| 664 | 664 | } | |
| 665 | 665 | ||
| 666 | + | func TestSuccessfulCloseReportsOnlyOwnedFile(t *testing.T) { | |
| 667 | + | for _, tc := range []struct { | |
| 668 | + | name string | |
| 669 | + | change func(t *testing.T, path, moved string) | |
| 670 | + | wantSaved bool | |
| 671 | + | }{ | |
| 672 | + | {"kept", func(*testing.T, string, string) {}, true}, | |
| 673 | + | {"moved and replaced", func(t *testing.T, path, moved string) { | |
| 674 | + | if err := os.Rename(path, moved); err != nil { | |
| 675 | + | t.Fatal(err) | |
| 676 | + | } | |
| 677 | + | if err := os.WriteFile(path, []byte("user data"), 0o600); err != nil { | |
| 678 | + | t.Fatal(err) | |
| 679 | + | } | |
| 680 | + | }, false}, | |
| 681 | + | {"deleted", func(t *testing.T, path, _ string) { | |
| 682 | + | if err := os.Remove(path); err != nil { | |
| 683 | + | t.Fatal(err) | |
| 684 | + | } | |
| 685 | + | }, false}, | |
| 686 | + | } { | |
| 687 | + | t.Run(tc.name, func(t *testing.T) { | |
| 688 | + | w := helperWriter(t, "drain") | |
| 689 | + | w.closeTimeout = 10 * time.Second | |
| 690 | + | dir := t.TempDir() | |
| 691 | + | path := filepath.Join(dir, "v.mp4") | |
| 692 | + | f := newFrame(4, 4, 16, 0) | |
| 693 | + | if err := w.Start(context.Background(), f, path, settings()); err != nil { | |
| 694 | + | t.Fatal(err) | |
| 695 | + | } | |
| 696 | + | if err := w.Write(context.Background(), f); err != nil { | |
| 697 | + | t.Fatal(err) | |
| 698 | + | } | |
| 699 | + | tc.change(t, path, filepath.Join(dir, "moved.mp4")) | |
| 700 | + | if err := w.Close(); err != nil { | |
| 701 | + | t.Fatalf("Close = %v", err) | |
| 702 | + | } | |
| 703 | + | if w.Saved != tc.wantSaved { | |
| 704 | + | t.Fatalf("Saved = %v, want %v", w.Saved, tc.wantSaved) | |
| 705 | + | } | |
| 706 | + | }) | |
| 707 | + | } | |
| 708 | + | } | |
| 709 | + | ||
| 666 | 710 | func TestRealFFmpeg(t *testing.T) { | |
| 667 | 711 | bin, err := exec.LookPath("ffmpeg") | |
| 668 | 712 | if err != nil { | |
| @@ -663,6 +663,50 @@ func TestTerminalCleanupSparesReplacementFile(t *testing.T) { | |||||
| 663 | } | 663 | } | ||
| 664 | } | 664 | } | ||
| 665 | 665 | ||||
| 666 | + | func TestSuccessfulCloseReportsOnlyOwnedFile(t *testing.T) { | |||
| 667 | + | for _, tc := range []struct { | |||
| 668 | + | name string | |||
| 669 | + | change func(t *testing.T, path, moved string) | |||
| 670 | + | wantSaved bool | |||
| 671 | + | }{ | |||
| 672 | + | {"kept", func(*testing.T, string, string) {}, true}, | |||
| 673 | + | {"moved and replaced", func(t *testing.T, path, moved string) { | |||
| 674 | + | if err := os.Rename(path, moved); err != nil { | |||
| 675 | + | t.Fatal(err) | |||
| 676 | + | } | |||
| 677 | + | if err := os.WriteFile(path, []byte("user data"), 0o600); err != nil { | |||
| 678 | + | t.Fatal(err) | |||
| 679 | + | } | |||
| 680 | + | }, false}, | |||
| 681 | + | {"deleted", func(t *testing.T, path, _ string) { | |||
| 682 | + | if err := os.Remove(path); err != nil { | |||
| 683 | + | t.Fatal(err) | |||
| 684 | + | } | |||
| 685 | + | }, false}, | |||
| 686 | + | } { | |||
| 687 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 688 | + | w := helperWriter(t, "drain") | |||
| 689 | + | w.closeTimeout = 10 * time.Second | |||
| 690 | + | dir := t.TempDir() | |||
| 691 | + | path := filepath.Join(dir, "v.mp4") | |||
| 692 | + | f := newFrame(4, 4, 16, 0) | |||
| 693 | + | if err := w.Start(context.Background(), f, path, settings()); err != nil { | |||
| 694 | + | t.Fatal(err) | |||
| 695 | + | } | |||
| 696 | + | if err := w.Write(context.Background(), f); err != nil { | |||
| 697 | + | t.Fatal(err) | |||
| 698 | + | } | |||
| 699 | + | tc.change(t, path, filepath.Join(dir, "moved.mp4")) | |||
| 700 | + | if err := w.Close(); err != nil { | |||
| 701 | + | t.Fatalf("Close = %v", err) | |||
| 702 | + | } | |||
| 703 | + | if w.Saved != tc.wantSaved { | |||
| 704 | + | t.Fatalf("Saved = %v, want %v", w.Saved, tc.wantSaved) | |||
| 705 | + | } | |||
| 706 | + | }) | |||
| 707 | + | } | |||
| 708 | + | } | |||
| 709 | + | ||||
| 666 | func TestRealFFmpeg(t *testing.T) { | 710 | func TestRealFFmpeg(t *testing.T) { | ||
| 667 | bin, err := exec.LookPath("ffmpeg") | 711 | bin, err := exec.LookPath("ffmpeg") | ||
| 668 | if err != nil { | 712 | if err != nil { | ||