diff --git a/engine/libindex.go b/engine/libindex.go index a13bafc31..5b2d82f3f 100644 --- a/engine/libindex.go +++ b/engine/libindex.go @@ -783,32 +783,32 @@ func UpdateFilterIndex(dm *DataManager, oldFlt, newFlt *Filter) (err error) { return } -// removeFilterIndexesForFilter removes the itemID for the index keys -// used to remove the old indexes when a filter is updated +// removeFilterIndexesForFilter removes itemIDs from the specified filter index keys. +// Used to update the index map when a filter is modified. func removeFilterIndexesForFilter(dm *DataManager, idxItmType, tnt string, - removeIndexKeys []string, itemIDs utils.StringSet) (err error) { + removeIndexKeys []string, itemIDs utils.StringSet) error { refID := guardian.Guardian.GuardIDs(utils.EmptyString, config.CgrConfig().GeneralCfg().LockingTimeout, idxItmType+tnt) defer guardian.Guardian.UnguardIDs(refID) - for _, idxKey := range removeIndexKeys { // delete old filters indexes for this item - var remIndx map[string]utils.StringSet - if remIndx, err = dm.GetIndexes(idxItmType, tnt, - idxKey, true, false); err != nil { - if err != utils.ErrNotFound { - return - } - err = nil - continue - } - for idx := range itemIDs { - remIndx[idxKey].Remove(idx) - } - if err = dm.SetIndexes(idxItmType, tnt, remIndx, true, utils.NonTransactional); err != nil { - return + // Retrieve current filter indexes. + fltrIdx, err := dm.GetIndexes(idxItmType, tnt, utils.EmptyString, true, false) + if err != nil { + if err != utils.ErrNotFound { + return err + } + return nil + } + + // Remove itemIDs from the specified index keys. + for _, idxKey := range removeIndexKeys { + for itemID := range itemIDs { + fltrIdx[idxKey].Remove(itemID) } } - return + + // Store the updated indexes. + return dm.SetIndexes(idxItmType, tnt, fltrIdx, true, utils.NonTransactional) } // IsDynamicDPPath check dynamic path with ~*stats, ~*resources, ~*accounts, ~*libphonenumber, ~*asm to not be indexed diff --git a/engine/libindex_test.go b/engine/libindex_test.go index a6cc12b97..41f63e401 100644 --- a/engine/libindex_test.go +++ b/engine/libindex_test.go @@ -24,6 +24,7 @@ import ( "github.com/cgrates/cgrates/config" "github.com/cgrates/cgrates/utils" + "github.com/google/go-cmp/cmp" ) func TestLibIndexIsDynamicDPPath(t *testing.T) { @@ -45,39 +46,130 @@ func TestLibIndexIsDynamicDPPath(t *testing.T) { } } -func TestLibIndexRemoveFilterIndexesForFilterErrnotFound(t *testing.T) { +func TestLibIndexRemoveFilterIndexesForFilter(t *testing.T) { cfg := config.NewDefaultCGRConfig() dataDB := NewInternalDB(nil, nil, true, cfg.DataDbCfg().Items) dm := NewDataManager(dataDB, cfg.CacheCfg(), nil) tntCtx := "cgrates.org:*sessions" - test := struct { + + tests := []struct { name string - idx map[string]utils.StringSet - keys []string + idx map[string]utils.StringSet // initial indexes map + keys []string // that will be removed from the index itemIDs utils.StringSet + want map[string]utils.StringSet // indexes map after remove }{ - name: "testing for ErrNotFound", - idx: map[string]utils.StringSet{ - "*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}), - "*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}), + { + name: "remove one filter index from all profiles", + idx: map[string]utils.StringSet{ + "*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}), + "*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}), + }, + keys: []string{"*string:~*req.Account:1001"}, + itemIDs: utils.NewStringSet([]string{"AP1", "AP2"}), + want: map[string]utils.StringSet{ + "*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}), + }, + }, + { + name: "remove one filter index from one profile", + idx: map[string]utils.StringSet{ + "*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}), + "*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}), + }, + keys: []string{"*string:~*req.Account:1001"}, + itemIDs: utils.NewStringSet([]string{"AP1"}), + want: map[string]utils.StringSet{ + "*string:~*req.Account:1001": utils.NewStringSet([]string{"AP2"}), + "*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}), + }, + }, + { + name: "attempt remove non-existent filter index", + idx: map[string]utils.StringSet{ + "*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}), + "*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}), + }, + keys: []string{"*string:~*req.Account:notfound"}, + itemIDs: utils.NewStringSet([]string{"AP1", "AP2"}), + want: map[string]utils.StringSet{ + "*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}), + "*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}), + }, + }, + { + name: "remove all filter indexes from one profile", + idx: map[string]utils.StringSet{ + "*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}), + "*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}), + }, + keys: []string{"*string:~*req.Account:1001", "*string:~*req.Account:1002"}, + itemIDs: utils.NewStringSet([]string{"AP1"}), + want: map[string]utils.StringSet{ + "*string:~*req.Account:1001": utils.NewStringSet([]string{"AP2"}), + "*string:~*req.Account:1002": utils.NewStringSet([]string{"AP2"}), + }, + }, + { + name: "remove all filter indexes from all profiles", + idx: map[string]utils.StringSet{ + "*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}), + "*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}), + }, + keys: []string{"*string:~*req.Account:1001", "*string:~*req.Account:1002"}, + itemIDs: utils.NewStringSet([]string{"AP1", "AP2"}), + want: nil, + }, + { + name: "remove multiple filter indexes from mixed profiles", + idx: map[string]utils.StringSet{ + "*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2", "AP3"}), + "*string:~*req.Destination:1010": utils.NewStringSet([]string{"AP2", "AP3"}), + "*string:~*req.Destination:1011": utils.NewStringSet([]string{"AP1", "AP3", "AP4"}), + "*string:~*req.Destination:1012": utils.NewStringSet([]string{"AP2"}), + }, + keys: []string{"*string:~*req.Destination:1010", "*string:~*req.Destination:1012"}, + itemIDs: utils.NewStringSet([]string{"AP2", "AP3"}), + want: map[string]utils.StringSet{ + "*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2", "AP3"}), + "*string:~*req.Destination:1011": utils.NewStringSet([]string{"AP1", "AP3", "AP4"}), + }, }, - keys: []string{"*string:~*req.Account:1001"}, - itemIDs: utils.NewStringSet([]string{"AP1", "AP2"}), } - t.Run(test.name, func(t *testing.T) { - t.Cleanup(func() { - if err := dataDB.Flush(""); err != nil { - t.Logf("failed to flush dataDB: %v", err) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Cleanup(func() { + if err := dataDB.Flush(""); err != nil { + t.Logf("failed to flush dataDB: %v", err) + } + }) + if err := dm.SetIndexes(utils.CacheAttributeFilterIndexes, tntCtx, test.idx, true, ""); err != nil { + t.Fatalf("dm.SetFilterIndexes() returned unexpected error: %v", err) + } + if err := removeFilterIndexesForFilter(dm, utils.CacheAttributeFilterIndexes, + tntCtx, test.keys, test.itemIDs); err != nil { + t.Fatalf("removeFilterIndexesForFilter() returned unexpected error: %v", err) + } + got, err := dm.GetIndexes(utils.CacheAttributeFilterIndexes, tntCtx, "", true, false) + switch len(test.want) { + case 0: + if !errors.Is(err, utils.ErrNotFound) { + t.Fatalf("dm.GetFilterIndexes(%q,%q) err = %v, want %v", + utils.CacheAttributeFilterIndexes, tntCtx, err, utils.ErrNotFound) + } + default: + if err != nil { + t.Fatalf("dm.GetFilterIndexes(%q,%q) returned unexpected error: %v", + utils.CacheAttributeFilterIndexes, tntCtx, err) + } + } + if diff := cmp.Diff(test.want, got); diff != "" { + t.Errorf("dm.GetFilterIndexes(%q,%q) returned unexpected indexes (-want +got): \n%s", + utils.CacheAttributeFilterIndexes, tntCtx, diff) } }) - if err := dm.SetIndexes(utils.CacheAttributeFilterIndexes, tntCtx, test.idx, true, ""); err != nil { - t.Fatalf("dm.SetIndexes() returned unexpected error: %v", err) - } - err := removeFilterIndexesForFilter(dm, utils.CacheAttributeFilterIndexes, tntCtx, test.keys, test.itemIDs) - if err != nil && !errors.Is(err, utils.ErrNotFound) { - t.Fatalf("Expected error %v, got %v", utils.ErrNotFound, err) - } - }) + } } func TestLibIndexSplitFilterIndexErrWrongIdxKeyFormat(t *testing.T) {