Skip to content

Commit

Permalink
docstore/memdocstore: #3508 first review
Browse files Browse the repository at this point in the history
renamed option to AllowNestedSliceQueries
  • Loading branch information
eqinox76 committed Dec 12, 2024
1 parent 02ce845 commit e16b6c9
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 12 deletions.
17 changes: 8 additions & 9 deletions docstore/memdocstore/mem.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ type Options struct {
// When the collection is closed, its contents are saved to the file.
Filename string

// AllowNestedSlicesQuery allows querying with nested slices.
// AllowNestedSliceQueries allows querying with nested slices.
// This makes the memdocstore more compatible with MongoDB,
// but other providers may not support this feature.
// See https://github.com/google/go-cloud/pull/3511 for more details.
AllowNestedSlicesQuery bool
AllowNestedSliceQueries bool

// Call this function when the collection is closed.
// For internal use only.
Expand Down Expand Up @@ -405,18 +405,17 @@ func (c *collection) checkRevision(arg driver.Document, current storedDoc) error

// getAtFieldPath gets the value of m at fp. It returns an error if fp is invalid
// (see getParentMap).
func getAtFieldPath(m map[string]interface{}, fp []string, nested bool) (result interface{}, err error) {

var get func(m interface{}, name string) interface{}
get = func(m interface{}, name string) interface{} {
func getAtFieldPath(m map[string]any, fp []string, nested bool) (result any, err error) {
var get func(m any, name string) any
get = func(m any, name string) any {
switch concrete := m.(type) {
case map[string]interface{}:
case map[string]any:
return concrete[name]
case []interface{}:
case []any:
if !nested {
return nil

Check warning on line 416 in docstore/memdocstore/mem.go

View check run for this annotation

Codecov / codecov/patch

docstore/memdocstore/mem.go#L416

Added line #L416 was not covered by tests
}
result := []interface{}{}
var result []any
for _, e := range concrete {
result = append(result, get(e, name))
}
Expand Down
25 changes: 24 additions & 1 deletion docstore/memdocstore/mem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func TestQueryNested(t *testing.T) {
return c
}

dc, err := newCollection(drivertest.KeyField, nil, &Options{AllowNestedSlicesQuery: true})
dc, err := newCollection(drivertest.KeyField, nil, &Options{AllowNestedSliceQueries: true})
if err != nil {
t.Fatal(err)
}
Expand All @@ -157,6 +157,9 @@ func TestQueryNested(t *testing.T) {
doc := docmap{drivertest.KeyField: "TestQueryNested",
"list": []any{docmap{"a": "A"}},
"map": docmap{"b": "B"},
"listOfMaps": []any{docmap{"id": "1"}, docmap{"id": "2"}, docmap{"id": "3"}},
"mapOfLists": docmap{"ids": []any{"1", "2", "3"}},
"deep": []any{docmap{"nesting": []any{docmap{"of": docmap{"elements": "yes"}}}}},
dc.RevisionField(): nil,
}
if err := coll.Put(ctx, doc); err != nil {
Expand All @@ -167,10 +170,30 @@ func TestQueryNested(t *testing.T) {
if got != 1 {
t.Errorf("got %v docs when filtering by list.a, want 1", got)
}
got = count(coll.Query().Where("list.a", "=", "missing").Get(ctx))
if got != 0 {
t.Errorf("got %v docs when filtering by list.a, want 0", got)
}
got = count(coll.Query().Where("map.b", "=", "B").Get(ctx))
if got != 1 {
t.Errorf("got %v docs when filtering by map.b, want 1", got)
}
got = count(coll.Query().Where("listOfMaps.id", "=", "1").Get(ctx))
if got != 1 {
t.Errorf("got %v docs when filtering by listOfMaps.id, want 1", got)
}
got = count(coll.Query().Where("listOfMaps.id", "=", "123").Get(ctx))
if got != 0 {
t.Errorf("got %v docs when filtering by listOfMaps.id, want 0", got)
}
got = count(coll.Query().Where("mapOfLists.ids", "=", "1").Get(ctx))
if got != 1 {
t.Errorf("got %v docs when filtering by listOfMaps.1, want 1", got)
}
got = count(coll.Query().Where("deep.nesting.of.elements", "=", "yes").Get(ctx))
if got != 1 {
t.Errorf("got %v docs when filtering by deep.nesting.of.elements, want 1", got)
}
}

func TestSortDocs(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions docstore/memdocstore/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (c *collection) RunGetQuery(_ context.Context, q *driver.Query) (driver.Doc

var resultDocs []storedDoc
for _, doc := range c.docs {
if filtersMatch(q.Filters, doc, c.opts.AllowNestedSlicesQuery) {
if filtersMatch(q.Filters, doc, c.opts.AllowNestedSliceQueries) {
resultDocs = append(resultDocs, doc)
}
}
Expand Down Expand Up @@ -138,6 +138,8 @@ func compare(x1, x2 interface{}) (int, bool) {
}
return -1, true
}
// for AllowNestedSliceQueries
// when querying for x2 in the document and x1 is a list of values we only need one value to match
if v1.Kind() == reflect.Slice {
for i := 0; i < v1.Len(); i++ {
if c, ok := compare(x2, v1.Index(i).Interface()); ok {
Expand All @@ -149,7 +151,6 @@ func compare(x1, x2 interface{}) (int, bool) {
}
}
}
return -1, true
}
if v1.Kind() == reflect.String && v2.Kind() == reflect.String {
return strings.Compare(v1.String(), v2.String()), true
Expand Down

0 comments on commit e16b6c9

Please sign in to comment.