mirror of
https://github.com/runbear-io/beardrive.git
synced 2026-08-25 08:08:08 +02:00
fix(shares): a folder is not a sync problem (#169)
bdrive share <folder> answered "not synced to this project yet" on a fully synced folder, so users went looking for a sync fault that wasn't there. snap.files maps FILES, so a folder always missed the existence check and landed in the same 404 as a path that doesn't exist — and the CLI bolted "wait a few seconds for the daemon" onto it. Tell the two apart in the handler (any key under p+"/" — never bare p, or notes-archive/x.md makes "notes" a folder): 400 with "share links are per-file" naming the lexicographically smallest file inside, 404 with today's wording otherwise. The CLI's daemon hint was already 404-only, so it stops appearing on its own; the only CLI change is printing a 400 body without httpBodyError's status prefix. The web UI mints through the same handler, so it gets the same distinction. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
31db19b9c6
commit
a3408bb198
@@ -996,6 +996,57 @@ func TestCLIShareSecretGate(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// bdrive share <folder> used to say the folder wasn't synced yet, so users went
|
||||
// looking for a sync fault that wasn't there — and the CLI bolted a "run bdrive
|
||||
// sync" hint onto it. Folders now get the real answer, hint-free.
|
||||
func TestCLIShareFolder(t *testing.T) {
|
||||
e := newCLIEnv(t)
|
||||
run := e.run
|
||||
|
||||
work := t.TempDir()
|
||||
if err := os.MkdirAll(filepath.Join(work, "notes"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, f := range []string{"notes/readme.md", "notes/zeta.md"} {
|
||||
if err := os.WriteFile(filepath.Join(work, filepath.FromSlash(f)), []byte("# "+f+"\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if out, err := run(work, "init", "--name", "share-folder", "--yes"); err != nil {
|
||||
t.Fatalf("init: %v\n%s", err, out)
|
||||
}
|
||||
defer run(work, "stop", work)
|
||||
if out, err := run(work, "sync"); err != nil {
|
||||
t.Fatalf("sync: %v\n%s", err, out)
|
||||
}
|
||||
|
||||
out, err := run(work, "share", "notes")
|
||||
if err == nil {
|
||||
t.Fatalf("share of a folder succeeded:\n%s", out)
|
||||
}
|
||||
for _, want := range []string{"per-file", "notes/readme.md"} {
|
||||
if !strings.Contains(out, want) {
|
||||
t.Fatalf("folder error missing %q:\n%s", want, out)
|
||||
}
|
||||
}
|
||||
for _, unwanted := range []string{"wait a few seconds", "not synced", "400 Bad Request"} {
|
||||
if strings.Contains(out, unwanted) {
|
||||
t.Fatalf("folder error should not contain %q:\n%s", unwanted, out)
|
||||
}
|
||||
}
|
||||
|
||||
// A path that really is missing keeps the sync-timing diagnosis and its hint.
|
||||
out, err = run(work, "share", "missing.md")
|
||||
if err == nil {
|
||||
t.Fatalf("share of a missing file succeeded:\n%s", out)
|
||||
}
|
||||
for _, want := range []string{"not synced to this project yet", "wait a few seconds"} {
|
||||
if !strings.Contains(out, want) {
|
||||
t.Fatalf("missing-file error missing %q:\n%s", want, out)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestCLIStatusReportsUnscannedWork is BEA-106: with the daemon stopped,
|
||||
// `status` answered from the state cache and the journal — neither of which
|
||||
// has seen an edit nobody scanned — and reported the folder clean. A wrong
|
||||
|
||||
@@ -239,6 +239,22 @@ func (db *ShareDB) List(project string) []Share {
|
||||
return out
|
||||
}
|
||||
|
||||
// firstFileUnder returns the lexicographically smallest file under the folder
|
||||
// p, or "" if p is not a folder. The prefix is p+"/" and never bare p: "notes"
|
||||
// is a prefix of "notes-archive/x.md", and that mistake turns a genuine
|
||||
// "not synced" into a wrong "that's a folder". Smallest, not whatever map
|
||||
// iteration hands back, so identical calls suggest the same file.
|
||||
func firstFileUnder(files map[string]FileInfo, p string) string {
|
||||
prefix := p + "/"
|
||||
best := ""
|
||||
for k := range files {
|
||||
if strings.HasPrefix(k, prefix) && (best == "" || k < best) {
|
||||
best = k
|
||||
}
|
||||
}
|
||||
return best
|
||||
}
|
||||
|
||||
// ---- HTTP ----
|
||||
|
||||
// handleShareCreate mints (or returns) the share link for a file. Any
|
||||
@@ -268,6 +284,13 @@ func (s *Server) handleShareCreate(v *volume, w http.ResponseWriter, r *http.Req
|
||||
return
|
||||
}
|
||||
if _, ok := snap.files[p]; !ok {
|
||||
// snap.files maps FILES, so a fully synced folder misses here just like
|
||||
// a path that does not exist. Tell those apart before answering, or the
|
||||
// user goes off to fix a sync fault that isn't there.
|
||||
if inside := firstFileUnder(snap.files, p); inside != "" {
|
||||
http.Error(w, fmt.Sprintf("share links are per-file; %s is a folder - try a file inside it, e.g. %s", p, inside), http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
http.Error(w, fmt.Sprintf("%s is not synced to this project yet", p), http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -902,3 +902,48 @@ func TestShareUnaffectedByAnUnmovedFile(t *testing.T) {
|
||||
t.Fatalf("unmoved share: %d %s", rec.Code, rec.Body)
|
||||
}
|
||||
}
|
||||
|
||||
// A folder is never a key in the files map, so a fully synced folder used to
|
||||
// 404 as "not synced yet" and send the user off to fix a sync fault that
|
||||
// wasn't there.
|
||||
func TestShareFolderIsNotASyncProblem(t *testing.T) {
|
||||
srv, p, _, _, h := shareHub(t)
|
||||
|
||||
// wiki/ is fully synced; sharing it is a per-file problem, not a timing one
|
||||
req := jsonReq(t, "POST", "/api/p/"+p.ID+"/shares", map[string]string{"path": "wiki"})
|
||||
authAs(t, srv, req)
|
||||
rec := doHTTP(h, req)
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("share of a folder: %d, want 400", rec.Code)
|
||||
}
|
||||
body := rec.Body.String()
|
||||
if !strings.Contains(body, "per-file") {
|
||||
t.Fatalf("folder error must say share links are per-file, got %q", body)
|
||||
}
|
||||
// names a file inside, and the SMALLEST one so repeat calls agree
|
||||
if !strings.Contains(body, "wiki/notes.md") {
|
||||
t.Fatalf("folder error must name a file inside it, got %q", body)
|
||||
}
|
||||
if strings.Contains(body, "not synced") {
|
||||
t.Fatalf("folder error must not blame sync, got %q", body)
|
||||
}
|
||||
if n := len(srv.Shares.List(p.ID)); n != 0 {
|
||||
t.Fatalf("folder share minted %d links, want 0", n)
|
||||
}
|
||||
|
||||
// a path that really is unknown keeps today's answer
|
||||
req = jsonReq(t, "POST", "/api/p/"+p.ID+"/shares", map[string]string{"path": "never-synced.md"})
|
||||
authAs(t, srv, req)
|
||||
if rec := doHTTP(h, req); rec.Code != http.StatusNotFound || !strings.Contains(rec.Body.String(), "not synced to this project yet") {
|
||||
t.Fatalf("unknown path: %d %s, want 404 not-synced", rec.Code, rec.Body)
|
||||
}
|
||||
|
||||
// "wik" is a PREFIX of wiki/report.html but is not a folder: still unknown.
|
||||
// A bare HasPrefix(k, p) would call it a folder — the same wrong-diagnosis
|
||||
// class of bug this fix is about.
|
||||
req = jsonReq(t, "POST", "/api/p/"+p.ID+"/shares", map[string]string{"path": "wik"})
|
||||
authAs(t, srv, req)
|
||||
if rec := doHTTP(h, req); rec.Code != http.StatusNotFound {
|
||||
t.Fatalf("prefix-of-a-file path: %d, want 404", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user