From 3b1970d95e3b23b771e69865a17e1d609b4d0a5e Mon Sep 17 00:00:00 2001 From: Sergey Date: Wed, 28 Aug 2024 10:04:07 +0200 Subject: [PATCH] GO-3979: Fix nested not equal query --- pkg/lib/database/filter.go | 100 +++++++++++++++--- pkg/lib/database/filter_test.go | 40 ++++++- .../localstore/objectstore/queries_test.go | 96 +++++++++++++---- 3 files changed, 199 insertions(+), 37 deletions(-) diff --git a/pkg/lib/database/filter.go b/pkg/lib/database/filter.go index 9ebf8f550f..eeefb54b9f 100644 --- a/pkg/lib/database/filter.go +++ b/pkg/lib/database/filter.go @@ -88,7 +88,14 @@ func makeFilter(spaceID string, rawFilter *model.BlockContentDataviewFilter, sto func makeFilterByCondition(spaceID string, rawFilter *model.BlockContentDataviewFilter, store ObjectStore) (Filter, error) { parts := strings.SplitN(rawFilter.RelationKey, ".", 2) if len(parts) == 2 { - return makeFilterNestedIn(spaceID, rawFilter, store, parts[0], parts[1]) + relationKey := parts[0] + nestedRelationKey := parts[1] + + if rawFilter.Condition == model.BlockContentDataviewFilter_NotEqual { + return makeFilterNestedNotIn(spaceID, rawFilter, store, relationKey, nestedRelationKey) + } else { + return makeFilterNestedIn(spaceID, rawFilter, store, relationKey, nestedRelationKey) + } } // replaces "value == false" to "value != true" for expected work with checkboxes @@ -673,18 +680,6 @@ func optionsToMap(spaceID string, key string, store ObjectStore) map[string]stri return result } -// FilterNestedIn returns true for object that has a relation pointing to any object that matches FilterForNestedObjects. -// This filter uses special machinery in able to work: it only functions when IDs field is populated by IDs of objects -// that match FilterForNestedObjects. You can't just use FilterNestedIn without populating IDs field -type FilterNestedIn struct { - Key string - FilterForNestedObjects Filter - - IDs []string -} - -var _ WithNestedFilter = &FilterNestedIn{} - func makeFilterNestedIn(spaceID string, rawFilter *model.BlockContentDataviewFilter, store ObjectStore, relationKey string, nestedRelationKey string) (Filter, error) { rawNestedFilter := proto.Clone(rawFilter).(*model.BlockContentDataviewFilter) rawNestedFilter.RelationKey = nestedRelationKey @@ -708,6 +703,18 @@ func makeFilterNestedIn(spaceID string, rawFilter *model.BlockContentDataviewFil }, nil } +// FilterNestedIn returns true for object that has a relation pointing to any object that matches FilterForNestedObjects. +// This filter uses special machinery in able to work: it only functions when IDs field is populated by IDs of objects +// that match FilterForNestedObjects. You can't just use FilterNestedIn without populating IDs field +type FilterNestedIn struct { + Key string + FilterForNestedObjects Filter + + IDs []string +} + +var _ WithNestedFilter = &FilterNestedIn{} + func (i *FilterNestedIn) FilterObject(g *types.Struct) bool { val := pbtypes.Get(g, i.Key) for _, id := range i.IDs { @@ -734,3 +741,70 @@ func (i *FilterNestedIn) AnystoreFilter() query.Filter { func (i *FilterNestedIn) IterateNestedFilters(fn func(nestedFilter Filter) error) error { return fn(i) } + +// See FilterNestedIn for details +type FilterNestedNotIn struct { + Key string + FilterForNestedObjects Filter + + IDs []string +} + +func makeFilterNestedNotIn(spaceID string, rawFilter *model.BlockContentDataviewFilter, store ObjectStore, relationKey string, nestedRelationKey string) (Filter, error) { + rawNestedFilter := proto.Clone(rawFilter).(*model.BlockContentDataviewFilter) + rawNestedFilter.RelationKey = nestedRelationKey + + subQueryRawFilter := proto.Clone(rawFilter).(*model.BlockContentDataviewFilter) + subQueryRawFilter.RelationKey = nestedRelationKey + subQueryRawFilter.Condition = model.BlockContentDataviewFilter_Equal + + subQueryFilter, err := MakeFilter(spaceID, subQueryRawFilter, store) + if err != nil { + return nil, fmt.Errorf("make nested filter %s -> %s: %w", relationKey, nestedRelationKey, err) + } + records, err := store.QueryRaw(&Filters{FilterObj: subQueryFilter}, 0, 0) + if err != nil { + return nil, fmt.Errorf("enrich nested filter: %w", err) + } + + ids := make([]string, 0, len(records)) + for _, rec := range records { + ids = append(ids, pbtypes.GetString(rec.Details, bundle.RelationKeyId.String())) + } + nestedFilter, err := MakeFilter(spaceID, rawNestedFilter, store) + if err != nil { + return nil, fmt.Errorf("make nested filter %s -> %s: %w", relationKey, nestedRelationKey, err) + } + return &FilterNestedNotIn{ + Key: relationKey, + FilterForNestedObjects: nestedFilter, + IDs: ids, + }, nil +} + +func (i *FilterNestedNotIn) FilterObject(g *types.Struct) bool { + val := pbtypes.Get(g, i.Key) + for _, id := range i.IDs { + eq := FilterEq{Value: pbtypes.String(id), Cond: model.BlockContentDataviewFilter_Equal} + if eq.filterObject(val) { + return false + } + } + return true +} + +func (i *FilterNestedNotIn) AnystoreFilter() query.Filter { + path := []string{i.Key} + conds := make([]query.Filter, 0, len(i.IDs)) + for _, id := range i.IDs { + conds = append(conds, query.Key{ + Path: path, + Filter: query.NewComp(query.CompOpNe, id), + }) + } + return query.And(conds) +} + +func (i *FilterNestedNotIn) IterateNestedFilters(fn func(nestedFilter Filter) error) error { + return fn(i) +} diff --git a/pkg/lib/database/filter_test.go b/pkg/lib/database/filter_test.go index 79a80582e4..02bb204693 100644 --- a/pkg/lib/database/filter_test.go +++ b/pkg/lib/database/filter_test.go @@ -449,7 +449,7 @@ func TestMakeAndFilter(t *testing.T) { } func TestNestedFilters(t *testing.T) { - t.Run("simple", func(t *testing.T) { + t.Run("equal", func(t *testing.T) { store := NewMockObjectStore(t) // Query will occur while nested filter resolving store.EXPECT().QueryRaw(mock.Anything, 0, 0).Return([]Record{ @@ -484,6 +484,44 @@ func TestNestedFilters(t *testing.T) { assertFilter(t, f, obj2, true) }) + t.Run("not equal", func(t *testing.T) { + store := NewMockObjectStore(t) + // Query will occur while nested filter resolving + store.EXPECT().QueryRaw(mock.Anything, 0, 0).Return([]Record{ + { + Details: &types.Struct{ + Fields: map[string]*types.Value{ + bundle.RelationKeyId.String(): pbtypes.String("id1"), + bundle.RelationKeyUniqueKey.String(): pbtypes.String("ot-note"), + }, + }, + }, + { + Details: &types.Struct{ + Fields: map[string]*types.Value{ + bundle.RelationKeyId.String(): pbtypes.String("id2"), + bundle.RelationKeyUniqueKey.String(): pbtypes.String("ot-note"), + }, + }, + }, + }, nil) + + f, err := MakeFilter("spaceId", &model.BlockContentDataviewFilter{ + RelationKey: "type.uniqueKey", + Condition: model.BlockContentDataviewFilter_NotEqual, + Value: pbtypes.String("ot-note"), + }, store) + require.NoError(t, err) + + obj1 := &types.Struct{Fields: map[string]*types.Value{bundle.RelationKeyType.String(): pbtypes.StringList([]string{"id1"})}} + obj2 := &types.Struct{Fields: map[string]*types.Value{bundle.RelationKeyType.String(): pbtypes.StringList([]string{"id2", "id1"})}} + obj3 := &types.Struct{Fields: map[string]*types.Value{bundle.RelationKeyType.String(): pbtypes.StringList([]string{"id3"})}} + obj4 := &types.Struct{Fields: map[string]*types.Value{bundle.RelationKeyType.String(): pbtypes.StringList([]string{"id4", "id5"})}} + assertFilter(t, f, obj1, false) + assertFilter(t, f, obj2, false) + assertFilter(t, f, obj3, true) + assertFilter(t, f, obj4, true) + }) } func TestFilterExists(t *testing.T) { diff --git a/pkg/lib/localstore/objectstore/queries_test.go b/pkg/lib/localstore/objectstore/queries_test.go index 173cb9d115..ece21cef12 100644 --- a/pkg/lib/localstore/objectstore/queries_test.go +++ b/pkg/lib/localstore/objectstore/queries_test.go @@ -953,33 +953,83 @@ func TestQueryRaw(t *testing.T) { }) t.Run("with nested filter", func(t *testing.T) { - s := NewStoreFixture(t) - obj1 := TestObject{ - bundle.RelationKeyId: pbtypes.String("id1"), - bundle.RelationKeyType: pbtypes.String("type1"), - } - type1 := TestObject{ - bundle.RelationKeyId: pbtypes.String("type1"), - bundle.RelationKeyType: pbtypes.String("objectType"), - domain.RelationKey("typeKey"): pbtypes.String("note"), - } + t.Run("equal", func(t *testing.T) { + s := NewStoreFixture(t) + obj1 := TestObject{ + bundle.RelationKeyId: pbtypes.String("id1"), + bundle.RelationKeyType: pbtypes.String("type1"), + } + type1 := TestObject{ + bundle.RelationKeyId: pbtypes.String("type1"), + bundle.RelationKeyType: pbtypes.String("objectType"), + bundle.RelationKeyUniqueKey: pbtypes.String("ot-note"), + } - s.AddObjects(t, []TestObject{obj1, type1}) + s.AddObjects(t, []TestObject{obj1, type1}) - flt, err := database.NewFilters(database.Query{ - Filters: []*model.BlockContentDataviewFilter{ - { - RelationKey: "type.typeKey", - Condition: model.BlockContentDataviewFilter_Equal, - Value: pbtypes.String("note"), + flt, err := database.NewFilters(database.Query{ + Filters: []*model.BlockContentDataviewFilter{ + { + RelationKey: "type.uniqueKey", + Condition: model.BlockContentDataviewFilter_Equal, + Value: pbtypes.String("ot-note"), + }, }, - }, - }, s, arena) - require.NoError(t, err) + }, s, arena) + require.NoError(t, err) + + recs, err := s.QueryRaw(flt, 0, 0) + require.NoError(t, err) + assertRecordsEqual(t, []TestObject{obj1}, recs) + }) + t.Run("not equal", func(t *testing.T) { + s := NewStoreFixture(t) + obj1 := TestObject{ + bundle.RelationKeyId: pbtypes.String("id1"), + bundle.RelationKeyType: pbtypes.String("type1"), + bundle.RelationKeyLayout: pbtypes.Int64(int64(model.ObjectType_basic)), + } + obj2 := TestObject{ + bundle.RelationKeyId: pbtypes.String("id2"), + bundle.RelationKeyType: pbtypes.String("type2"), + bundle.RelationKeyLayout: pbtypes.Int64(int64(model.ObjectType_basic)), + } + type1 := TestObject{ + bundle.RelationKeyId: pbtypes.String("type1"), + bundle.RelationKeyType: pbtypes.String("objectType"), + bundle.RelationKeyUniqueKey: pbtypes.String("ot-template"), + bundle.RelationKeyLayout: pbtypes.Int64(int64(model.ObjectType_objectType)), + } + type2 := TestObject{ + bundle.RelationKeyId: pbtypes.String("type2"), + bundle.RelationKeyType: pbtypes.String("objectType"), + bundle.RelationKeyUniqueKey: pbtypes.String("ot-page"), + bundle.RelationKeyLayout: pbtypes.Int64(int64(model.ObjectType_objectType)), + } + + s.AddObjects(t, []TestObject{obj1, obj2, type1, type2}) + + flt, err := database.NewFilters(database.Query{ + Filters: []*model.BlockContentDataviewFilter{ + { + RelationKey: "type.uniqueKey", + Condition: model.BlockContentDataviewFilter_NotEqual, + Value: pbtypes.String("ot-template"), + }, + { + RelationKey: bundle.RelationKeyLayout.String(), + Condition: model.BlockContentDataviewFilter_Equal, + Value: pbtypes.Int64(int64(model.ObjectType_basic)), + }, + }, + }, s, arena) + require.NoError(t, err) + + recs, err := s.QueryRaw(flt, 0, 0) + require.NoError(t, err) + assertRecordsEqual(t, []TestObject{obj2}, recs) + }) - recs, err := s.QueryRaw(flt, 0, 0) - require.NoError(t, err) - assertRecordsEqual(t, []TestObject{obj1}, recs) }) }