Skip to content

Commit

Permalink
fix(base.ReadEntries): Simplify ReadEntries by using PagedReader in r…
Browse files Browse the repository at this point in the history
…ender

Also, move testing only 'mustBeArray' to its callsite.
  • Loading branch information
dustmop committed Feb 12, 2019
1 parent 6c798d7 commit 4f914a6
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 61 deletions.
29 changes: 1 addition & 28 deletions base/entries.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
package base

import (
"fmt"

"github.com/qri-io/dataset/dsio"
)

// ReadEntries reads entries and returns them as a native go array or map
func ReadEntries(reader dsio.EntryReader, all bool, limit int, offset int) (interface{}, error) {
func ReadEntries(reader dsio.EntryReader) (interface{}, error) {
obj := make(map[string]interface{})
array := make([]interface{}, 0)
numRead := 0

tlt, err := dsio.GetTopLevelType(reader.Structure())
if err != nil {
Expand All @@ -25,39 +22,15 @@ func ReadEntries(reader dsio.EntryReader, all bool, limit int, offset int) (inte
}
return nil, err
}
if !all && i < offset {
continue
}

if tlt == "object" {
obj[val.Key] = val.Value
} else {
array = append(array, val.Value)
}

numRead++
if !all && numRead == limit {
break
}
}

if tlt == "object" {
return obj, nil
}
return array, nil
}

// ReadEntriesToArray reads entries and returns them as a native go array
func ReadEntriesToArray(reader dsio.EntryReader, all bool, limit int, offset int) ([]interface{}, error) {
entries, err := ReadEntries(reader, all, limit, offset)
if err != nil {
return nil, err
}

array, ok := entries.([]interface{})
if !ok {
return nil, fmt.Errorf("cannot convert top-level to array")
}

return array, nil
}
33 changes: 4 additions & 29 deletions base/entries_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,13 @@ func TestReadEntriesArray(t *testing.T) {
cases := []struct {
description string
rdrCount, expectCount int
all bool
limit, offset int
}{
{"read all 50", 50, 50, true, 0, 0},
{"read 40 of 50", 50, 40, false, 40, 0},
{"read 40-60 of 100", 100, 20, false, 20, 40},
{"read all 50", 50, 50},
}

for i, c := range cases {
r := newTestJSONArrayReader(c.rdrCount)
got, err := ReadEntries(r, c.all, c.limit, c.offset)
got, err := ReadEntries(r)
if err != nil {
t.Errorf("case %d %s unexpected error. '%s'", i, c.description, err)
continue
Expand All @@ -43,17 +39,13 @@ func TestReadEntriesObject(t *testing.T) {
cases := []struct {
description string
rdrCount, expectCount int
all bool
limit, offset int
}{
{"read all 50", 50, 50, true, 0, 0},
{"read 40 of 50", 50, 40, false, 40, 0},
{"read 40-60 of 100", 100, 20, false, 20, 40},
{"read all 50", 50, 50},
}

for i, c := range cases {
r := newTestJSONObjectReader(c.rdrCount)
got, err := ReadEntries(r, c.all, c.limit, c.offset)
got, err := ReadEntries(r)
if err != nil {
t.Errorf("case %d %s unexpected error. '%s'", i, c.description, err)
continue
Expand All @@ -70,23 +62,6 @@ func TestReadEntriesObject(t *testing.T) {
}
}

func TestReadEntriesToArray(t *testing.T) {
r := newTestJSONArrayReader(1)
got, err := ReadEntriesToArray(r, false, 1000, 0)
if err != nil {
t.Error(err)
}

expLen := 1
if expLen != len(got) {
t.Errorf("entry length mismatch. expected: %d, got: %d", expLen, len(got))
}

if _, err := ReadEntriesToArray(newTestJSONObjectReader(1), false, 1, 0); err == nil {
t.Errorf("expected reading object to error")
}
}

// newTestJSONArrayReader creates a dsio.EntryReader with a number of entries that matches entryCount
// with an array as the top level type
func newTestJSONArrayReader(entryCount int) dsio.EntryReader {
Expand Down
5 changes: 4 additions & 1 deletion base/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ func Render(r repo.Repo, ref repo.DatasetRef, tmplData []byte, limit, offset int
return nil, fmt.Errorf("error allocating data reader: %s", err)
}

bodyEntries, err := ReadEntries(rr, all, limit, offset)
if !all {
rr = &dsio.PagedReader{Reader: rr, Limit: limit, Offset: offset}
}
bodyEntries, err := ReadEntries(rr)
if err != nil {
return nil, err
}
Expand Down
10 changes: 9 additions & 1 deletion lib/datasets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ func TestDatasetRequestsGet(t *testing.T) {
moviesDs.OpenBodyFile(node.Repo.Filesystem())
moviesBodyFile := moviesDs.BodyFile()
reader := dsio.NewCSVReader(moviesDs.Structure, moviesBodyFile)
moviesBody, _ := base.ReadEntriesToArray(reader, true, 0, 0)
moviesBody := mustBeArray(base.ReadEntries(reader))

cases := []struct {
params *GetParams
Expand Down Expand Up @@ -890,3 +890,11 @@ func TestDatasetRequestsDiff(t *testing.T) {
}
}
}

// Convert the interface value into an array, or panic if not possible
func mustBeArray(i interface{}, err error) []interface{} {
if err != nil {
panic(err)
}
return i.([]interface{})
}
4 changes: 2 additions & 2 deletions lib/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (r *ExportRequests) Export(p *ExportParams, fileWritten *string) (err error

// TODO (dlong): Look into combining this functionality (reading body, changing structure),
// and moving it into a method of `actions`.
bodyEntries, err := base.ReadEntries(reader, true, 0, 0)
bodyEntries, err := base.ReadEntries(reader)
if err != nil {
return err
}
Expand All @@ -165,7 +165,7 @@ func (r *ExportRequests) Export(p *ExportParams, fileWritten *string) (err error

case "yaml":

bodyEntries, err := base.ReadEntries(reader, true, 0, 0)
bodyEntries, err := base.ReadEntries(reader)
if err != nil {
return err
}
Expand Down

0 comments on commit 4f914a6

Please sign in to comment.