From c73db87612d498f7f9a059d35b47e257f9e1ab06 Mon Sep 17 00:00:00 2001 From: Andrey Markelov Date: Sun, 28 Jun 2026 21:10:14 -0700 Subject: [PATCH] Fix ls --only-deleted --limit to count filtered entries Previously --limit counted all entries including live files, so --only-deleted --limit N could return fewer than N deleted entries even when more existed. Now pagination continues until enough deleted entries are collected, and resolved DeletedMetadata entries are retained in the filtered output. --- cmd/ls.go | 67 +++++++++++++++++++------- cmd/ls_test.go | 124 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+), 16 deletions(-) diff --git a/cmd/ls.go b/cmd/ls.go index f20f6aa..395e794 100644 --- a/cmd/ls.go +++ b/cmd/ls.go @@ -159,23 +159,10 @@ func ls(cmd *cobra.Command, args []string) (err error) { var metaRes files.IsMetadata metaRes, _ = getFileMetadata(dbx, path) entries = []files.IsMetadata{metaRes} + entries, err = finalizeLsEntries(dbx, entries, onlyDeleted, opts) } else { - entries = appendMetadataWithLimit(entries, res.Entries, opts.limit) - - for res.HasMore && !metadataLimitReached(entries, opts.limit) { - arg := files.NewListFolderContinueArg(res.Cursor) - - res, err = dbx.ListFolderContinue(arg) - if err != nil { - return err - } - - entries = appendMetadataWithLimit(entries, res.Entries, opts.limit) - } + entries, err = collectAndPrepareLsEntries(dbx, res, opts.limit, onlyDeleted, opts) } - - sortEntries(entries, opts) - entries, err = prepareLsEntries(dbx, entries, onlyDeleted) if err != nil { return err } @@ -197,6 +184,54 @@ func metadataLimitReached(entries []files.IsMetadata, limit uint64) bool { return limit > 0 && uint64(len(entries)) >= limit } +func finalizeLsEntries(dbx files.Client, entries []files.IsMetadata, onlyDeleted bool, opts listOptions) ([]files.IsMetadata, error) { + sortEntries(entries, opts) + return prepareLsEntries(dbx, entries, onlyDeleted) +} + +func collectAndPrepareLsEntries(dbx files.Client, res *files.ListFolderResult, limit uint64, onlyDeleted bool, opts listOptions) ([]files.IsMetadata, error) { + if onlyDeleted && limit > 0 { + entries, err := collectOnlyDeletedEntriesWithLimit(dbx, res, limit) + if err != nil { + return nil, err + } + sortEntries(entries, opts) + return entries, nil + } + + var entries []files.IsMetadata + entries = appendMetadataWithLimit(entries, res.Entries, limit) + for res.HasMore && !metadataLimitReached(entries, limit) { + var err error + res, err = dbx.ListFolderContinue(files.NewListFolderContinueArg(res.Cursor)) + if err != nil { + return nil, err + } + + entries = appendMetadataWithLimit(entries, res.Entries, limit) + } + return finalizeLsEntries(dbx, entries, onlyDeleted, opts) +} + +func collectOnlyDeletedEntriesWithLimit(dbx files.Client, res *files.ListFolderResult, limit uint64) ([]files.IsMetadata, error) { + var entries []files.IsMetadata + for { + filtered, err := prepareLsEntries(dbx, res.Entries, true) + if err != nil { + return nil, err + } + entries = appendMetadataWithLimit(entries, filtered, limit) + if !res.HasMore || metadataLimitReached(entries, limit) { + return entries, nil + } + + res, err = dbx.ListFolderContinue(files.NewListFolderContinueArg(res.Cursor)) + if err != nil { + return nil, err + } + } +} + func lsRecursive(cmd *cobra.Command) bool { recursive, _ := cmd.Flags().GetBool("recursive") recurse, _ := cmd.Flags().GetBool("recurse") @@ -230,7 +265,7 @@ func prepareLsEntries(dbx files.Client, entries []files.IsMetadata, onlyDeleted } switch entry.(type) { case *files.FileMetadata, *files.FolderMetadata: - if !onlyDeleted { + if !onlyDeleted || isDeleted { filtered = append(filtered, entry) } case *files.DeletedMetadata: diff --git a/cmd/ls_test.go b/cmd/ls_test.go index 2ab5374..ddd94df 100644 --- a/cmd/ls_test.go +++ b/cmd/ls_test.go @@ -315,6 +315,130 @@ func TestLsLimitStopsPaginationAndTruncatesOutput(t *testing.T) { } } +func TestLsOnlyDeletedLimitCountsFilteredEntries(t *testing.T) { + cmd, stdout := testLsCmd(t) + setLsOutputJSON(t, cmd) + setLsFlag(t, cmd, "only-deleted", "true") + setLsFlag(t, cmd, "limit", "2") + + continueCalled := false + mock := &mockFilesClient{ + listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { + if !arg.IncludeDeleted { + t.Fatal("ListFolder include_deleted = false, want true") + } + if arg.Limit != 2 { + t.Fatalf("ListFolder limit = %d, want 2", arg.Limit) + } + return &files.ListFolderResult{ + Entries: []files.IsMetadata{ + &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/active-a.txt"}}, + &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/active-b.txt"}}, + &files.DeletedMetadata{Metadata: files.Metadata{PathDisplay: "/deleted-a.txt", PathLower: "/deleted-a.txt"}}, + }, + HasMore: true, + Cursor: "cursor-1", + }, nil + }, + listFolderContinueFn: func(arg *files.ListFolderContinueArg) (*files.ListFolderResult, error) { + continueCalled = true + if arg.Cursor != "cursor-1" { + t.Fatalf("ListFolderContinue cursor = %q, want cursor-1", arg.Cursor) + } + return &files.ListFolderResult{ + Entries: []files.IsMetadata{ + &files.DeletedMetadata{Metadata: files.Metadata{PathDisplay: "/deleted-b.txt", PathLower: "/deleted-b.txt"}}, + &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/active-c.txt"}}, + }, + HasMore: false, + }, nil + }, + listRevisionsFn: func(arg *files.ListRevisionsArg) (*files.ListRevisionsResult, error) { + return files.NewListRevisionsResult(false, []*files.FileMetadata{ + { + Metadata: files.Metadata{ + PathDisplay: arg.Path, + PathLower: arg.Path, + }, + Id: "id:" + arg.Path, + Rev: "rev:" + arg.Path, + }, + }), nil + }, + } + stubFilesClient(t, mock) + + if err := ls(cmd, nil); err != nil { + t.Fatalf("ls error: %v", err) + } + if !continueCalled { + t.Fatal("ListFolderContinue was not called; want pagination until deleted limit is reached") + } + + got := decodeLsOutput(t, stdout) + if !got.Input.OnlyDeleted || got.Input.Limit != 2 { + t.Fatalf("input = %#v, want only_deleted true and limit 2", got.Input) + } + if len(got.Results) != 2 { + t.Fatalf("results = %d, want 2 deleted entries: %#v", len(got.Results), got.Results) + } + for i, want := range []string{"/deleted-a.txt", "/deleted-b.txt"} { + result := got.Results[i] + if result.Result.PathDisplay != want || !result.Result.Deleted { + t.Fatalf("result[%d] = %#v, want deleted path %s", i, result, want) + } + } +} + +func TestLsOnlyDeletedLimitTruncatesFilteredPage(t *testing.T) { + cmd, stdout := testLsCmd(t) + setLsOutputJSON(t, cmd) + setLsFlag(t, cmd, "only-deleted", "true") + setLsFlag(t, cmd, "limit", "1") + + mock := &mockFilesClient{ + listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { + return &files.ListFolderResult{ + Entries: []files.IsMetadata{ + &files.DeletedMetadata{Metadata: files.Metadata{PathDisplay: "/deleted-a.txt", PathLower: "/deleted-a.txt"}}, + &files.DeletedMetadata{Metadata: files.Metadata{PathDisplay: "/deleted-b.txt", PathLower: "/deleted-b.txt"}}, + }, + HasMore: true, + Cursor: "cursor-1", + }, nil + }, + listFolderContinueFn: func(arg *files.ListFolderContinueArg) (*files.ListFolderResult, error) { + t.Fatalf("ListFolderContinue called after reaching filtered limit: %v", arg) + return nil, nil + }, + listRevisionsFn: func(arg *files.ListRevisionsArg) (*files.ListRevisionsResult, error) { + return files.NewListRevisionsResult(false, []*files.FileMetadata{ + { + Metadata: files.Metadata{ + PathDisplay: arg.Path, + PathLower: arg.Path, + }, + Id: "id:" + arg.Path, + Rev: "rev:" + arg.Path, + }, + }), nil + }, + } + stubFilesClient(t, mock) + + if err := ls(cmd, nil); err != nil { + t.Fatalf("ls error: %v", err) + } + + got := decodeLsOutput(t, stdout) + if len(got.Results) != 1 { + t.Fatalf("results = %d, want 1 deleted entry", len(got.Results)) + } + if got.Results[0].Result.PathDisplay != "/deleted-a.txt" || !got.Results[0].Result.Deleted { + t.Fatalf("result = %#v, want first deleted entry", got.Results[0]) + } +} + func TestLsRecurseAliasSetsRecursiveListFolderArg(t *testing.T) { cmd, stdout := testLsCmd(t) setLsOutputJSON(t, cmd)