Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(firestore): implement limitToLast #2420

Merged
merged 14 commits into from Jul 27, 2020
21 changes: 0 additions & 21 deletions firestore/examples_test.go
Expand Up @@ -432,27 +432,6 @@ func ExampleQuery_Documents() {
_ = iter2 // TODO: Use iter2.
}

func ExampleQuery_GetAll() {
ctx := context.Background()
client, err := firestore.NewClient(ctx, "project-id")
if err != nil {
// TODO: Handle error.
}
defer client.Close()

q := client.Collection("States").Select("pop").
Where("pop", ">", 10).
OrderBy("pop", firestore.Desc).
Limit(10)

docs, err := q.GetAll(ctx)
if err != nil {
// TODO: Handle error.
}
// docs is a slice with DocumentSnapshots.
fmt.Println(docs)
}

// This example is just like the one above, but illustrates
// how to use the XXXPath methods of Query for field paths
// that can't be expressed as a dot-separated string.
Expand Down
79 changes: 3 additions & 76 deletions firestore/integration_test.go
Expand Up @@ -657,6 +657,7 @@ func TestIntegration_QueryDocuments(t *testing.T) {
{q.StartAfter(1), wants[2:]},
{q.EndAt(1), wants[:2]},
{q.EndBefore(1), wants[:1]},
{q.LimitToLast(2), wants[1:]},
} {
gotDocs, err := test.q.Documents(ctx).GetAll()
if err != nil {
Expand Down Expand Up @@ -709,83 +710,9 @@ func TestIntegration_QueryDocuments_LimitToLast_Fail(t *testing.T) {
ctx := context.Background()
coll := integrationColl(t)
q := coll.Select("q").OrderBy("q", Asc).LimitToLast(1)
gotDocs, err := q.Documents(ctx).GetAll()
got, err := q.Documents(ctx).Next()
if err == nil {
t.Errorf("got %v docs, want error", len(gotDocs))
}
}

func TestIntegration_QueryGetAll(t *testing.T) {
tritone marked this conversation as resolved.
Show resolved Hide resolved
ctx := context.Background()
coll := integrationColl(t)
h := testHelper{t}
var wants []map[string]interface{}
for i := 0; i < 3; i++ {
doc := coll.NewDoc()
// To support running this test in parallel with the others, use a field name
// that we don't use anywhere else.
h.mustCreate(doc, map[string]interface{}{"q": i, "x": 1})
wants = append(wants, map[string]interface{}{"q": int64(i)})
}
q := coll.Select("q").OrderBy("q", Asc)
for i, test := range []struct {
q Query
want []map[string]interface{}
}{
{q, wants},
{q.Where("q", ">", 1), wants[2:]},
{q.WherePath([]string{"q"}, ">", 1), wants[2:]},
{q.Offset(1).Limit(1), wants[1:2]},
{q.StartAt(1), wants[1:]},
{q.StartAfter(1), wants[2:]},
{q.EndAt(1), wants[:2]},
{q.EndBefore(1), wants[:1]},
{q.LimitToLast(2), wants[1:]},
} {
gotDocs, err := test.q.GetAll(ctx)
if err != nil {
t.Errorf("#%d: %+v: %v", i, test.q, err)
continue
}
if len(gotDocs) != len(test.want) {
t.Errorf("#%d: %+v: got %d docs, want %d", i, test.q, len(gotDocs), len(test.want))
continue
}
for j, g := range gotDocs {
if got, want := g.Data(), test.want[j]; !testEqual(got, want) {
t.Errorf("#%d: %+v, #%d: got\n%+v\nwant\n%+v", i, test.q, j, got, want)
}
}
}
_, err := coll.Select("q").Where("x", "==", 1).OrderBy("q", Asc).GetAll(ctx)
codeEq(t, "Where and OrderBy on different fields without an index", codes.FailedPrecondition, err)

// Using the collection itself as the query should return the full documents.
allDocs, err := coll.GetAll(ctx)
if err != nil {
t.Fatal(err)
}
seen := map[int64]bool{} // "q" values we see.
for _, d := range allDocs {
data := d.Data()
q, ok := data["q"]
if !ok {
// A document from another test.
continue
}
if seen[q.(int64)] {
t.Errorf("%v: duplicate doc", data)
}
seen[q.(int64)] = true
if data["x"] != int64(1) {
t.Errorf("%v: wrong or missing 'x'", data)
}
if len(data) != 2 {
t.Errorf("%v: want two keys", data)
}
}
if got, want := len(seen), len(wants); got != want {
t.Errorf("got %d docs with 'q', want %d", len(seen), len(wants))
t.Errorf("got %v doc, want error", got)
}
}

Expand Down
73 changes: 32 additions & 41 deletions firestore/query.go
Expand Up @@ -579,57 +579,18 @@ func trunc32(i int) int32 {
return int32(i)
}

// GetAll returns an array of query's resulting documents.
func (q Query) GetAll(ctx context.Context) ([]*DocumentSnapshot, error) {
limitedToLast := q.limitToLast

if q.limitToLast {
// Flip order statements before posting a request.
for i := range q.orders {
if q.orders[i].dir == Asc {
q.orders[i].dir = Desc
} else {
q.orders[i].dir = Asc
}
}
// Swap cursors.
q.startVals, q.endVals = q.endVals, q.startVals
q.startDoc, q.endDoc = q.endDoc, q.startDoc
q.startBefore, q.endBefore = q.endBefore, q.startBefore

q.limitToLast = false
}
docs, err := q.Documents(ctx).GetAll()
if err != nil {
return nil, err
}
if limitedToLast {
// Flip docs order before return.
for i, j := 0, len(docs)-1; i < j; {
docs[i], docs[j] = docs[j], docs[i]
i++
j--
}
}
return docs, nil
}

// Documents returns an iterator over the query's resulting documents.
func (q Query) Documents(ctx context.Context) *DocumentIterator {
if q.limitToLast {
return &DocumentIterator{
err: errors.New("firestore: queries that include limitToLast constraints cannot be streamed. Use Query.GetAll() instead"),
}
}
return &DocumentIterator{
iter: newQueryDocumentIterator(withResourceHeader(ctx, q.c.path()), &q, nil),
iter: newQueryDocumentIterator(withResourceHeader(ctx, q.c.path()), &q, nil), q: q,
}
}

// DocumentIterator is an iterator over documents returned by a query.
type DocumentIterator struct {
iter docIterator
err error
q Query
IlyaFaer marked this conversation as resolved.
Show resolved Hide resolved
}

// Unexported interface so we can have two different kinds of DocumentIterator: one
Expand All @@ -649,6 +610,9 @@ func (it *DocumentIterator) Next() (*DocumentSnapshot, error) {
if it.err != nil {
return nil, it.err
}
if it.q.limitToLast {
return nil, errors.New("firestore: queries that include limitToLast constraints cannot be streamed. Use DocumentIterator.GetAll() instead")
}
ds, err := it.iter.next()
if err != nil {
it.err = err
Expand All @@ -672,6 +636,25 @@ func (it *DocumentIterator) Stop() {
// It is not necessary to call Stop on the iterator after calling GetAll.
func (it *DocumentIterator) GetAll() ([]*DocumentSnapshot, error) {
defer it.Stop()

q := &it.q
limitedToLast := q.limitToLast
if q.limitToLast {
// Flip order statements before posting a request.
for i := range q.orders {
if q.orders[i].dir == Asc {
q.orders[i].dir = Desc
} else {
q.orders[i].dir = Asc
}
}
// Swap cursors.
q.startVals, q.endVals = q.endVals, q.startVals
q.startDoc, q.endDoc = q.endDoc, q.startDoc
q.startBefore, q.endBefore = q.endBefore, q.startBefore

q.limitToLast = false
}
var docs []*DocumentSnapshot
for {
doc, err := it.Next()
Expand All @@ -683,6 +666,14 @@ func (it *DocumentIterator) GetAll() ([]*DocumentSnapshot, error) {
}
docs = append(docs, doc)
}
if limitedToLast {
// Flip docs order before return.
for i, j := 0, len(docs)-1; i < j; {
docs[i], docs[j] = docs[j], docs[i]
i++
j--
}
}
return docs, nil
}

tritone marked this conversation as resolved.
Show resolved Hide resolved
Expand Down