Skip to content

Commit

Permalink
Merge pull request #819 from go-kivik/unmakeReady
Browse files Browse the repository at this point in the history 
Remove the single-doc operating mode of the resultset
  • Loading branch information
@flimzy
flimzy authored Oct 12, 2023
2 parents 055b99d + 77cada0 commit bad77c7
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 122 deletions.
2 changes: 1 addition & 1 deletion db.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func (r *Document) ScanDoc(i interface{}) error {
}

// Attachments returns an attachments iterator if the document includes
// attachments.
// attachments and they are not inline.
func (r *Document) Attachments() (*AttachmentsIterator, error) {
if r.err != nil {
return nil, r.err
Expand Down
47 changes: 18 additions & 29 deletions resultset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ func (r *ResultSet) Metadata() (*ResultMetadata, error) {
//
// Refer to the documentation for [encoding/json.Unmarshal] for unmarshaling
// details.
func (r *ResultSet) ScanValue(dest interface{}) (err error) {
runlock, err := r.makeReady(&err)
func (r *ResultSet) ScanValue(dest interface{}) error {
runlock, err := r.makeReady()
if err != nil {
return err
}
Expand All @@ -174,8 +174,8 @@ func (r *ResultSet) ScanValue(dest interface{}) (err error) {
//
// If the row returned an error, it will be returned rather than
// unmarshaling the doc, as error rows do not include docs.
func (r *ResultSet) ScanDoc(dest interface{}) (err error) {
runlock, err := r.makeReady(&err)
func (r *ResultSet) ScanDoc(dest interface{}) error {
runlock, err := r.makeReady()
if err != nil {
return err
}
Expand All @@ -196,8 +196,8 @@ func (r *ResultSet) ScanDoc(dest interface{}) (err error) {
//
// Unlike [ResultSet.ScanValue] and [ResultSet.ScanDoc], this may successfully
// scan the key, and also return an error, if the row itself represents an error.
func (r *ResultSet) ScanKey(dest interface{}) (err error) {
runlock, err := r.makeReady(&err)
func (r *ResultSet) ScanKey(dest interface{}) error {
runlock, err := r.makeReady()
if err != nil {
return err
}
Expand All @@ -211,7 +211,7 @@ func (r *ResultSet) ScanKey(dest interface{}) (err error) {

// ID returns the ID of the most recent result.
func (r *ResultSet) ID() (string, error) {
runlock, err := r.makeReady(nil)
runlock, err := r.makeReady()
if err != nil {
return "", err
}
Expand All @@ -224,7 +224,7 @@ func (r *ResultSet) ID() (string, error) {
// as those from views) include revision IDs, so this will return an empty
// string in such cases.
func (r *ResultSet) Rev() (string, error) {
runlock, err := r.makeReady(nil)
runlock, err := r.makeReady()
if err != nil {
return "", err
}
Expand All @@ -236,7 +236,7 @@ func (r *ResultSet) Rev() (string, error) {
// Key returns the Key of the most recent result as a raw JSON string. For
// compound keys, [ResultSet.ScanKey] may be more convenient.
func (r *ResultSet) Key() (string, error) {
runlock, err := r.makeReady(nil)
runlock, err := r.makeReady()
if err != nil {
return "", err
}
Expand All @@ -248,7 +248,7 @@ func (r *ResultSet) Key() (string, error) {
// Attachments returns an attachments iterator if the document includes
// attachments.
func (r *ResultSet) Attachments() (*AttachmentsIterator, error) {
runlock, err := r.makeReady(nil)
runlock, err := r.makeReady()
if err != nil {
return nil, err
}
Expand All @@ -268,31 +268,20 @@ func (r *ResultSet) Attachments() (*AttachmentsIterator, error) {
}

// makeReady ensures that the iterator is ready to be read from. If i.err is
// set, it is returned. In the case that [iter.Next] has not been called, the
// returned unlock function will also close the iterator, and set e if
// [iter.Close] errors and e != nil.
func (r *ResultSet) makeReady(e *error) (unlock func(), err error) {
// set, it is returned.
func (r *ResultSet) makeReady() (unlock func(), err error) {
r.mu.Lock()
if r.err != nil {
r.mu.Unlock()
return nil, r.err
}
if r.state == stateClosed {
r.mu.Unlock()
return nil, &internal.Error{Status: http.StatusBadRequest, Message: "kivik: Iterator is closed"}
}
if !stateIsReady(r.state) {
if !r.iter.next() {
r.mu.Unlock()
return nil, &internal.Error{Status: http.StatusNotFound, Message: "no results"}
}
var once sync.Once
r.wg.Add(1)
return func() {
once.Do(func() {
r.wg.Done()
r.mu.Unlock()
if err := r.Close(); err != nil && e != nil {
*e = err
}
})
}, nil
r.mu.Unlock()
return nil, &internal.Error{Status: http.StatusBadRequest, Message: "kivik: Iterator access before calling Next"}
}
var once sync.Once
r.wg.Add(1)
Expand Down
104 changes: 12 additions & 92 deletions resultset_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,9 @@ func TestResultSet_Getters(t *testing.T) {
}
r := newResultSet(context.Background(), nil, rowsi)

result, _ := r.ID()
if result != id {
t.Errorf("Unexpected result: %v", result)
_, err := r.ID()
if !testy.ErrorMatches("kivik: Iterator access before calling Next", err) {
t.Errorf("Unexpected error: %v", err)
}
})

Expand All @@ -360,57 +360,9 @@ func TestResultSet_Getters(t *testing.T) {
}
r := newResultSet(context.Background(), nil, rowsi)

result, _ := r.Key()
if result != string(key) {
t.Errorf("Unexpected result: %v", result)
}
})
})

t.Run("after close", func(t *testing.T) {
rowsi := &mock.Rows{
OffsetFunc: func() int64 { return offset },
TotalRowsFunc: func() int64 { return totalrows },
UpdateSeqFunc: func() string { return updateseq },
}
r := &ResultSet{
iter: &iter{
state: stateRowReady,
curVal: &driver.Row{
ID: id,
Key: key,
},
feed: &rowsIterator{Rows: rowsi},
},
rowsi: rowsi,
}

if err := r.Close(); err != nil {
t.Fatal(err)
}

t.Run("ID", func(t *testing.T) {
result, _ := r.ID()
if id != result {
t.Errorf("Unexpected result: %v", result)
}
})

t.Run("Key", func(t *testing.T) {
result, _ := r.Key()
if string(key) != result {
t.Errorf("Unexpected result: %v", result)
}
})

t.Run("ScanKey", func(t *testing.T) {
var result json.RawMessage
err := r.ScanKey(&result)
if err != nil {
t.Fatal(err)
}
if string(result) != string(key) {
t.Errorf("Unexpected result: %v", result)
_, err := r.Key()
if !testy.ErrorMatches("kivik: Iterator access before calling Next", err) {
t.Errorf("Unexpected error: %v", err)
}
})
})
Expand Down Expand Up @@ -568,8 +520,8 @@ func Test_bug576(t *testing.T) {

var result interface{}
err := rows.ScanDoc(&result)
const wantErr = "no results"
wantStatus := http.StatusNotFound
const wantErr = "kivik: Iterator access before calling Next"
wantStatus := http.StatusBadRequest
if !testy.ErrorMatches(wantErr, err) {
t.Errorf("unexpected error: %s", err)
}
Expand All @@ -578,41 +530,6 @@ func Test_bug576(t *testing.T) {
}
}

func TestResultSet_single(t *testing.T) {
const wantRev = "1-abc"
const docContent = `{"_id":"foo"}`
wantDoc := map[string]interface{}{"_id": "foo"}
rows := newResultSet(context.Background(), nil, &mock.Rows{
NextFunc: func(row *driver.Row) error {
row.Rev = wantRev
row.Doc = strings.NewReader(docContent)
return nil
},
})

rev, err := rows.Rev()
if err != nil {
t.Fatal(err)
}
if rev != wantRev {
t.Errorf("Unexpected rev: %s", rev)
}
var doc map[string]interface{}
if err := rows.ScanDoc(&doc); err != nil {
t.Fatal(err)
}
if d := cmp.Diff(wantDoc, doc); d != "" {
t.Error(d)
}
rev2, err := rows.Rev()
if err != nil {
t.Fatal(err)
}
if rev2 != wantRev {
t.Errorf("Unexpected rev on second read: %s", rev2)
}
}

func TestResultSet_Close_blocks(t *testing.T) {
t.Parallel()

Expand All @@ -636,6 +553,7 @@ func TestResultSet_Close_blocks(t *testing.T) {
},
work: func(rs *ResultSet) {
var i interface{}
rs.Next()
_ = rs.ScanDoc(&i)
},
})
Expand All @@ -651,6 +569,7 @@ func TestResultSet_Close_blocks(t *testing.T) {
},
work: func(rs *ResultSet) {
var i interface{}
rs.Next()
_ = rs.ScanValue(&i)
},
})
Expand All @@ -667,6 +586,7 @@ func TestResultSet_Close_blocks(t *testing.T) {
},
},
work: func(rs *ResultSet) {
rs.Next()
atts, err := rs.Attachments()
if err != nil {
t.Fatal(err)
Expand All @@ -687,7 +607,7 @@ func TestResultSet_Close_blocks(t *testing.T) {
time.Sleep(delay / 2)
_ = rs.Close()
if elapsed := time.Since(start); elapsed < delay {
t.Errorf("rs.Close() didn't block long enouggh (%v < %v)", elapsed, delay)
t.Errorf("rs.Close() didn't block long enough (%v < %v)", elapsed, delay)
}
})
}

0 comments on commit bad77c7

Please sign in to comment.