From 4569cab1dc5306cd1b7adf92fa37bdd33a52ea20 Mon Sep 17 00:00:00 2001 From: "Andreas R." Date: Thu, 1 Oct 2026 11:46:11 +0200 Subject: [PATCH] fix: don't delete objects of sibling prefixes when delete is enabled The target was used as a raw S3 list prefix without a trailing slash, so syncing to "app/feature/x" also listed "app/feature/x-other/...". With delete: true those keys were not found locally and got deleted. List with a trailing-slash prefix and only consider keys below it as delete candidates. Adds unit tests. --- plugin.go | 59 +++++++++++++++++++++++++++++++++---------------- plugin_test.go | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 19 deletions(-) create mode 100644 plugin_test.go diff --git a/plugin.go b/plugin.go index a26fb21..fa0a9b6 100644 --- a/plugin.go +++ b/plugin.go @@ -76,8 +76,41 @@ func (p *Plugin) sanitizeInputs() error { return nil } +// listPrefix returns the S3 key prefix used to list the objects that belong to +// target. It always ends with a "/" (unless target is the bucket root), so +// syncing to "a/b" does not also match sibling prefixes such as "a/b-other/". +func listPrefix(target string) string { + target = strings.Trim(target, "/") + if target == "" { + return "" + } + return target + "/" +} + +// deleteCandidates returns the remote keys below target that have no +// counterpart in local (paths relative to target). +func deleteCandidates(remote []string, local []string, target string) []string { + prefix := listPrefix(target) + known := make(map[string]struct{}, len(local)) + for _, l := range local { + known[l] = struct{}{} + } + + candidates := make([]string, 0) + for _, r := range remote { + // never touch keys outside the target "directory" + if !strings.HasPrefix(r, prefix) { + continue + } + if _, found := known[strings.TrimPrefix(r, prefix)]; !found { + candidates = append(candidates, r) + } + } + return candidates +} + func (p *Plugin) createSyncJobs() { - remote, err := p.client.List(p.Target) + remote, err := p.client.List(listPrefix(p.Target)) if err != nil { fmt.Println(err) os.Exit(1) @@ -119,24 +152,12 @@ func (p *Plugin) createSyncJobs() { }) } if p.Delete { - for _, r := range remote { - found := false - matcher := strings.TrimPrefix(r, p.Target) - matcher = strings.TrimPrefix(matcher, string(os.PathSeparator)) - for _, l := range local { - if l == matcher { - found = true - break - } - } - - if !found { - p.jobs = append(p.jobs, job{ - local: "", - remote: r, - action: "delete", - }) - } + for _, r := range deleteCandidates(remote, local, p.Target) { + p.jobs = append(p.jobs, job{ + local: "", + remote: r, + action: "delete", + }) } } } diff --git a/plugin_test.go b/plugin_test.go new file mode 100644 index 0000000..7d69b22 --- /dev/null +++ b/plugin_test.go @@ -0,0 +1,60 @@ +package main + +import ( + "reflect" + "testing" +) + +func TestListPrefix(t *testing.T) { + cases := map[string]string{ + "": "", + "/": "", + "app/feature/x": "app/feature/x/", + "app/feature/x/": "app/feature/x/", + "/app/feature/x/": "app/feature/x/", + } + for in, want := range cases { + if got := listPrefix(in); got != want { + t.Errorf("listPrefix(%q) = %q, want %q", in, got, want) + } + } +} + +func TestDeleteCandidatesIgnoresSiblingPrefixes(t *testing.T) { + remote := []string{ + "app/feature/x/index.html", + "app/feature/x/old.js", + "app/feature/x-other/index.html", + "app/feature/x-other/main.js", + "app/feature/xy/index.html", + } + local := []string{"index.html"} + + got := deleteCandidates(remote, local, "app/feature/x") + want := []string{"app/feature/x/old.js"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("deleteCandidates() = %v, want %v", got, want) + } +} + +func TestDeleteCandidatesTrailingSlashTarget(t *testing.T) { + remote := []string{"app/x/a.js", "app/x/sub/b.js", "app/x2/a.js"} + local := []string{"sub/b.js"} + + got := deleteCandidates(remote, local, "/app/x/") + want := []string{"app/x/a.js"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("deleteCandidates() = %v, want %v", got, want) + } +} + +func TestDeleteCandidatesBucketRoot(t *testing.T) { + remote := []string{"index.html", "old.html", "assets/a.js"} + local := []string{"index.html", "assets/a.js"} + + got := deleteCandidates(remote, local, "") + want := []string{"old.html"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("deleteCandidates() = %v, want %v", got, want) + } +}