fix(base.ListDatasets): support -1 limit to list all datasets

and switch to our chosen `offset,limit` arg ordering
qri-io · Feb 18, 2021 · bd2f831 · bd2f831
1 parent 0fa4207
commit bd2f831
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 14 deletions.
diff --git a/base/dataset.go b/base/dataset.go
@@ -157,12 +157,15 @@ func CloseDataset(ds *dataset.Dataset) (err error) {
 }
 
 // ListDatasets lists datasets from a repo
-func ListDatasets(ctx context.Context, r repo.Repo, term string, limit, offset int, RPC, publishedOnly, showVersions bool) (res []reporef.DatasetRef, err error) {
+func ListDatasets(ctx context.Context, r repo.Repo, term string, offset, limit int, RPC, publishedOnly, showVersions bool) (res []reporef.DatasetRef, err error) {
 	fs := r.Filesystem()
 	num, err := r.RefCount()
 	if err != nil {
 		return nil, err
 	}
+	if limit < 0 {
+		limit = num
+	}
 	res, err = r.References(0, num)
 	if err != nil {
 		log.Debug(err.Error())

diff --git a/base/dataset_test.go b/base/dataset_test.go
@@ -16,7 +16,7 @@ func TestListDatasets(t *testing.T) {
 	ref := addCitiesDataset(t, r)
 
 	// Limit to one
-	res, err := ListDatasets(ctx, r, "", 1, 0, false, false, false)
+	res, err := ListDatasets(ctx, r, "", 0, 1, false, false, false)
 	if err != nil {
 		t.Error(err.Error())
 	}
@@ -25,7 +25,7 @@ func TestListDatasets(t *testing.T) {
 	}
 
 	// Limit to published datasets
-	res, err = ListDatasets(ctx, r, "", 1, 0, false, true, false)
+	res, err = ListDatasets(ctx, r, "", 0, 1, false, true, false)
 	if err != nil {
 		t.Error(err.Error())
 	}
@@ -39,7 +39,7 @@ func TestListDatasets(t *testing.T) {
 	}
 
 	// Limit to published datasets, after publishing cities
-	res, err = ListDatasets(ctx, r, "", 1, 0, false, true, false)
+	res, err = ListDatasets(ctx, r, "", 0, 1, false, true, false)
 	if err != nil {
 		t.Error(err.Error())
 	}
@@ -49,21 +49,29 @@ func TestListDatasets(t *testing.T) {
 	}
 
 	// Limit to datasets with "city" in their name
-	res, err = ListDatasets(ctx, r, "city", 1, 0, false, false, false)
+	res, err = ListDatasets(ctx, r, "city", 0, 1, false, false, false)
 	if err != nil {
 		t.Error(err.Error())
 	}
 	if len(res) != 0 {
-		t.Error("expected no datasets with \"city\" in their name")
+		t.Error(`expected no datasets with "city" in their name`)
 	}
 
 	// Limit to datasets with "cit" in their name
-	res, err = ListDatasets(ctx, r, "cit", 1, 0, false, false, false)
+	res, err = ListDatasets(ctx, r, "cit", 0, 1, false, false, false)
 	if err != nil {
 		t.Error(err.Error())
 	}
 	if len(res) != 1 {
-		t.Error("expected one dataset with \"cit\" in their name")
+		t.Error(`expected one dataset with \"cit\" in their name`)
+	}
+
+	res, err = ListDatasets(ctx, r, "", 0, -1, false, false, false)
+	if err != nil {
+		t.Error(err)
+	}
+	if len(res) != 1 {
+		t.Errorf("expected -1 limit to return all datasets. got %d", len(res))
 	}
 }
 

diff --git a/lib/datasets.go b/lib/datasets.go
@@ -139,7 +139,7 @@ func (m *DatasetMethods) List(ctx context.Context, p *ListParams) ([]dsref.Versi
 		}
 		// TODO(dlong): Filtered by p.Published flag
 	} else if ref.Peername == "" || pro.Peername == ref.Peername {
-		refs, err = base.ListDatasets(ctx, m.inst.repo, p.Term, p.Limit, p.Offset, p.RPC, p.Public, p.ShowNumVersions)
+		refs, err = base.ListDatasets(ctx, m.inst.repo, p.Term, p.Offset, p.Limit, p.RPC, p.Public, p.ShowNumVersions)
 		if errors.Is(err, ErrListWarning) {
 			listWarning = err
 			err = nil

diff --git a/p2p/datasets.go b/p2p/datasets.go
@@ -83,7 +83,7 @@ func (n *QriNode) handleDatasetsList(ws *WrappedStream, msg Message) (hangup boo
 			dlp.Limit = listMax
 		}
 
-		refs, err := base.ListDatasets(context.TODO(), n.Repo, dlp.Term, dlp.Limit, dlp.Offset, false, true, false)
+		refs, err := base.ListDatasets(context.TODO(), n.Repo, dlp.Term, dlp.Offset, dlp.Limit, false, true, false)
 		if err != nil {
 			log.Error(err)
 			return

diff --git a/registry/regserver/mock.go b/registry/regserver/mock.go
@@ -107,7 +107,7 @@ type MockRepoSearch struct {
 // Search implements the registry.Searchable interface
 func (ss MockRepoSearch) Search(p registry.SearchParams) ([]*dataset.Dataset, error) {
 	ctx := context.Background()
-	refs, err := base.ListDatasets(ctx, ss.Repo, p.Q, 1000, 0, false, true, false)
+	refs, err := base.ListDatasets(ctx, ss.Repo, p.Q, 0, 1000, false, true, false)
 	if err != nil {
 		return nil, err
 	}

diff --git a/remote/browse.go b/remote/browse.go
@@ -60,7 +60,7 @@ func (rf RepoFeeds) Feed(ctx context.Context, userID, name string, offset, limit
 		return nil, fmt.Errorf("unknown feed name '%s'", name)
 	}
 
-	refs, err := base.ListDatasets(ctx, rf.Repo, "", limit, offset, false, true, false)
+	refs, err := base.ListDatasets(ctx, rf.Repo, "", offset, limit, false, true, false)
 	if err != nil {
 		return nil, err
 	}

diff --git a/transform/startf/qri/qri.go b/transform/startf/qri/qri.go
@@ -54,8 +54,7 @@ func (m *Module) ListDatasets(thread *starlark.Thread, _ *starlark.Builtin, args
 		return starlark.None, fmt.Errorf("no qri repo available to list datasets")
 	}
 
-	// TODO (b5) - remove hardcoded limit
-	refs, err := m.repo.References(0, 100000)
+	refs, err := m.repo.References(0, -1)
 	if err != nil {
 		return starlark.None, fmt.Errorf("error getting dataset list: %s", err.Error())
 	}