From 5ac08799e1750ea9f9eaf13896fbb69797929025 Mon Sep 17 00:00:00 2001 From: ionutboangiu Date: Tue, 12 Mar 2024 12:25:00 -0400 Subject: [PATCH] Update *remove actions to support multiple balance types at once --- engine/account.go | 2 +- engine/action.go | 107 +++++++++++++++---------------- engine/action_trigger.go | 2 +- engine/actions_test.go | 14 ++-- engine/balances.go | 7 +- engine/balances_test.go | 10 +-- engine/units_counter.go | 2 +- general_tests/balance_it_test.go | 50 ++++++++++++--- 8 files changed, 111 insertions(+), 83 deletions(-) diff --git a/engine/account.go b/engine/account.go index ef5d8942f..3d30aa374 100644 --- a/engine/account.go +++ b/engine/account.go @@ -210,7 +210,7 @@ func (acc *Account) debitBalanceAction(a *Action, reset, resetIfNegative bool, f continue // just to be safe (cleaned expired balances above) } b.account = acc - if b.MatchFilter(a.Balance, false, false) { + if b.MatchFilter(a.Balance, "", false, false) { if reset || (resetIfNegative && b.Value < 0) { b.SetValue(0) } diff --git a/engine/action.go b/engine/action.go index 5e14ddb6e..14b63a0eb 100644 --- a/engine/action.go +++ b/engine/action.go @@ -348,33 +348,32 @@ func cdrLogAction(acc *Account, a *Action, acs Actions, _ *FilterS, extraData an } // Function to process balances and append CDR if conditions are met. - processBalances := func(checkFunc func(*Balance) bool) error { + processBalances := func(checkFunc func(*Balance, string) bool) error { if acc == nil { return fmt.Errorf("nil account for action %s", utils.ToJSON(action)) } - balanceChain, exists := acc.BalanceMap[action.Balance.GetType()] - if !exists { - return utils.ErrNotFound - } found := false - for _, balance := range balanceChain { - if checkFunc(balance) { - // Create a new CDR instance for each balance that meets the condition. - newCDR := *cdr // Copy CDR's values to a new CDR instance. - newCDR.Cost = balance.Value - newCDR.OriginID = utils.GenUUID() // OriginID must be unique for every CDR. - newCDR.CGRID = utils.Sha1(newCDR.OriginID, newCDR.OriginHost) + for bType, bChain := range acc.BalanceMap { + for _, balance := range bChain { + if checkFunc(balance, bType) { + // Create a new CDR instance for each balance that meets the condition. + newCDR := *cdr // Copy CDR's values to a new CDR instance. + newCDR.Cost = balance.Value + newCDR.OriginID = utils.GenUUID() // OriginID must be unique for every CDR. + newCDR.CGRID = utils.Sha1(newCDR.OriginID, newCDR.OriginHost) + newCDR.ToR = bType - // Clone the ExtraFields map to avoid changing its value in - // CDRs appended previously. - newCDR.ExtraFields = make(map[string]string, len(cdr.ExtraFields)+1) - for key, val := range cdr.ExtraFields { - newCDR.ExtraFields[key] = val + // Clone the ExtraFields map to avoid changing its value in + // CDRs appended previously. + newCDR.ExtraFields = make(map[string]string, len(cdr.ExtraFields)+1) + for key, val := range cdr.ExtraFields { + newCDR.ExtraFields[key] = val + } + newCDR.ExtraFields[utils.BalanceID] = balance.ID + + cdrs = append(cdrs, &newCDR) // Append the address of the new instance. + found = true } - newCDR.ExtraFields[utils.BalanceID] = balance.ID - - cdrs = append(cdrs, &newCDR) // Append the address of the new instance. - found = true } } if !found { @@ -387,16 +386,16 @@ func cdrLogAction(acc *Account, a *Action, acs Actions, _ *FilterS, extraData an // assign the balance values to the CDR cost and append to the list of CDRs. switch action.ActionType { case utils.MetaRemoveBalance: - if err = processBalances(func(b *Balance) bool { - return b.MatchFilter(action.Balance, false, false) + if err = processBalances(func(b *Balance, typ string) bool { + return b.MatchFilter(action.Balance, typ, false, false) }); err != nil { return err } continue case utils.MetaRemoveExpired: - if err = processBalances(func(b *Balance) bool { + if err = processBalances(func(b *Balance, typ string) bool { return b.IsExpiredAt(currentTime) && - b.MatchFilter(action.Balance, false, true) + b.MatchFilter(action.Balance, typ, false, true) }); err != nil { return err } @@ -758,25 +757,24 @@ func removeAccountAction(ub *Account, a *Action, acs Actions, _ *FilterS, extraD }, config.CgrConfig().GeneralCfg().LockingTimeout, utils.ActionPlanPrefix) } -func removeBalanceAction(ub *Account, a *Action, acs Actions, _ *FilterS, extraData any, _ ActionConnCfg) error { - if ub == nil { +func removeBalanceAction(acc *Account, a *Action, _ Actions, _ *FilterS, _ any, + _ ActionConnCfg) error { + if acc == nil { return fmt.Errorf("nil account for %s action", utils.ToJSON(a)) } - if _, exists := ub.BalanceMap[a.Balance.GetType()]; !exists { - return utils.ErrNotFound - } - bChain := ub.BalanceMap[a.Balance.GetType()] found := false - for i := 0; i < len(bChain); i++ { - if bChain[i].MatchFilter(a.Balance, false, false) { - // delete without preserving order - bChain[i] = bChain[len(bChain)-1] - bChain = bChain[:len(bChain)-1] - i-- - found = true + for bType, bChain := range acc.BalanceMap { + for i := 0; i < len(bChain); i++ { + if bChain[i].MatchFilter(a.Balance, bType, false, false) { + // Remove balance without preserving order. + bChain[i] = bChain[len(bChain)-1] + bChain = bChain[:len(bChain)-1] + i-- + found = true + } } + acc.BalanceMap[bType] = bChain } - ub.BalanceMap[a.Balance.GetType()] = bChain if !found { return utils.ErrNotFound } @@ -803,7 +801,7 @@ func transferMonetaryDefaultAction(ub *Account, a *Action, acs Actions, _ *Filte for _, balance := range bChain { if balance.Uuid != defaultBalance.Uuid && balance.ID != defaultBalance.ID && // extra caution - balance.MatchFilter(a.Balance, false, false) { + balance.MatchFilter(a.Balance, "", false, false) { if balance.Value > 0 { defaultBalance.Value += balance.Value balance.Value = 0 @@ -1024,7 +1022,7 @@ func setExpiryAction(ub *Account, a *Action, acs Actions, _ *FilterS, extraData } balanceType := a.Balance.GetType() for _, b := range ub.BalanceMap[balanceType] { - if b.MatchFilter(a.Balance, false, true) { + if b.MatchFilter(a.Balance, "", false, true) { b.ExpirationDate = a.Balance.GetExpirationDate() } } @@ -1196,28 +1194,27 @@ func removeSessionCosts(_ *Account, action *Action, _ Actions, _ *FilterS, _ any return cdrStorage.RemoveSMCosts(smcFilter) } -func removeExpired(acc *Account, action *Action, _ Actions, _ *FilterS, extraData any, _ ActionConnCfg) error { +func removeExpired(acc *Account, action *Action, _ Actions, _ *FilterS, _ any, _ ActionConnCfg) error { if acc == nil { return fmt.Errorf("nil account for %s action", utils.ToJSON(action)) } - bChain, exists := acc.BalanceMap[action.Balance.GetType()] - if !exists { - return utils.ErrNotFound - } found := false - for i := 0; i < len(bChain); i++ { - if bChain[i].IsExpiredAt(time.Now()) && - bChain[i].MatchFilter(action.Balance, false, true) { + for bType, bChain := range acc.BalanceMap { + for i := 0; i < len(bChain); i++ { + if bChain[i].IsExpiredAt(time.Now()) && + bChain[i].MatchFilter(action.Balance, bType, false, false) { - // Delete balance without preserving order. - bChain[i] = bChain[len(bChain)-1] // assign last balance to current balance - bChain = bChain[:len(bChain)-1] // remove last balance - i-- // subtract 1 from index to avoid skipping next balance - found = true + // Remove balance without maintaining order. + bChain[i] = bChain[len(bChain)-1] + bChain = bChain[:len(bChain)-1] + i-- + found = true + } } + acc.BalanceMap[bType] = bChain } - acc.BalanceMap[action.Balance.GetType()] = bChain + if !found { return utils.ErrNotFound } diff --git a/engine/action_trigger.go b/engine/action_trigger.go index 5225f385c..ace1b64d6 100644 --- a/engine/action_trigger.go +++ b/engine/action_trigger.go @@ -139,7 +139,7 @@ func (at *ActionTrigger) Match(a *Action) bool { } thresholdType = t.ThresholdType == "" || at.ThresholdType == t.ThresholdType } - return thresholdType && at.Balance.CreateBalance().MatchFilter(a.Balance, false, false) + return thresholdType && at.Balance.CreateBalance().MatchFilter(a.Balance, "", false, false) } func (at *ActionTrigger) CreateBalance() *Balance { diff --git a/engine/actions_test.go b/engine/actions_test.go index 5f4a50985..06d2ad71e 100644 --- a/engine/actions_test.go +++ b/engine/actions_test.go @@ -3938,19 +3938,15 @@ func TestRemoveBalanceActionErr(t *testing.T) { }, } acs := &Action{ - Balance: &BalanceFilter{}, + Balance: &BalanceFilter{ + ExpirationDate: utils.TimePointer(time.Date(2022, 11, 12, 2, 0, 0, 0, time.UTC)), + Type: utils.StringPointer(utils.MetaMonetary), + Value: &utils.ValueFormula{Static: 10}, + }, } if err := removeBalanceAction(nil, acs, nil, nil, nil, ActionConnCfg{}); err == nil { t.Error(err) } - if err := removeBalanceAction(acc, acs, nil, nil, nil, ActionConnCfg{}); err == nil { - t.Error(err) - } - acs.Balance = &BalanceFilter{ - ExpirationDate: utils.TimePointer(time.Date(2022, 11, 12, 2, 0, 0, 0, time.UTC)), - Type: utils.StringPointer(utils.MetaMonetary), - Value: &utils.ValueFormula{Static: 10}, - } if err := removeBalanceAction(acc, acs, nil, nil, nil, ActionConnCfg{}); err == nil || err != utils.ErrNotFound { t.Error(err) } diff --git a/engine/balances.go b/engine/balances.go index 741268014..6e9a8f605 100644 --- a/engine/balances.go +++ b/engine/balances.go @@ -70,10 +70,15 @@ func (b *Balance) Equal(o *Balance) bool { b.Blocker == o.Blocker } -func (b *Balance) MatchFilter(o *BalanceFilter, skipIds, skipExpiry bool) bool { +func (b *Balance) MatchFilter(o *BalanceFilter, bType string, skipIds, skipExpiry bool) bool { if o == nil { return true } + if bType != "" && o.Type != nil && *o.Type != "" { + if bType != *o.Type { + return false + } + } if !skipIds && o.Uuid != nil && *o.Uuid != "" { return b.Uuid == *o.Uuid } diff --git a/engine/balances_test.go b/engine/balances_test.go index 635330530..73b76773d 100644 --- a/engine/balances_test.go +++ b/engine/balances_test.go @@ -119,12 +119,12 @@ func TestBalanceEqual(t *testing.T) { func TestBalanceMatchFilter(t *testing.T) { mb1 := &Balance{Weight: 1, precision: 1, RatingSubject: "1", DestinationIDs: utils.StringMap{}} mb2 := &BalanceFilter{Weight: utils.Float64Pointer(1), RatingSubject: nil, DestinationIDs: nil} - if !mb1.MatchFilter(mb2, false, false) { + if !mb1.MatchFilter(mb2, "", false, false) { t.Errorf("Match filter failure: %+v == %+v", mb1, mb2) } mb1.Uuid, mb2.Uuid = "id", utils.StringPointer("id") - if !mb1.MatchFilter(mb2, false, false) { + if !mb1.MatchFilter(mb2, "", false, false) { t.Errorf("Match filter failure: %+v == %+v", mb1, mb2) } @@ -133,7 +133,7 @@ func TestBalanceMatchFilter(t *testing.T) { func TestBalanceMatchFilterEmpty(t *testing.T) { mb1 := &Balance{Weight: 1, precision: 1, RatingSubject: "1", DestinationIDs: utils.StringMap{}} mb2 := &BalanceFilter{} - if !mb1.MatchFilter(mb2, false, false) { + if !mb1.MatchFilter(mb2, "", false, false) { t.Errorf("Match filter failure: %+v == %+v", mb1, mb2) } } @@ -141,7 +141,7 @@ func TestBalanceMatchFilterEmpty(t *testing.T) { func TestBalanceMatchFilterId(t *testing.T) { mb1 := &Balance{ID: "T1", Weight: 2, precision: 2, RatingSubject: "2", DestinationIDs: utils.NewStringMap("NAT")} mb2 := &BalanceFilter{ID: utils.StringPointer("T1"), Weight: utils.Float64Pointer(1), RatingSubject: utils.StringPointer("1"), DestinationIDs: nil} - if !mb1.MatchFilter(mb2, false, false) { + if !mb1.MatchFilter(mb2, "", false, false) { t.Errorf("Match filter failure: %+v == %+v", mb1, mb2) } } @@ -149,7 +149,7 @@ func TestBalanceMatchFilterId(t *testing.T) { func TestBalanceMatchFilterDiffId(t *testing.T) { mb1 := &Balance{ID: "T1", Weight: 1, precision: 1, RatingSubject: "1", DestinationIDs: utils.StringMap{}} mb2 := &BalanceFilter{ID: utils.StringPointer("T2"), Weight: utils.Float64Pointer(1), RatingSubject: utils.StringPointer("1"), DestinationIDs: nil} - if mb1.MatchFilter(mb2, false, false) { + if mb1.MatchFilter(mb2, "", false, false) { t.Errorf("Match filter failure: %+v != %+v", mb1, mb2) } } diff --git a/engine/units_counter.go b/engine/units_counter.go index e0cbf7c1a..83ad284d2 100644 --- a/engine/units_counter.go +++ b/engine/units_counter.go @@ -128,7 +128,7 @@ func (ucs UnitCounters) addUnits(amount float64, kind string, cc *CallCost, b *B continue } - if uc.CounterType == utils.MetaBalance && b != nil && b.MatchFilter(c.Filter, true, false) { + if uc.CounterType == utils.MetaBalance && b != nil && b.MatchFilter(c.Filter, "", true, false) { c.Value += amount continue } diff --git a/general_tests/balance_it_test.go b/general_tests/balance_it_test.go index 5b396986d..df016ac8d 100644 --- a/general_tests/balance_it_test.go +++ b/general_tests/balance_it_test.go @@ -700,9 +700,9 @@ PACKAGE_ACC_TEST,ACT_TOPUP_SMS,*asap,10`, ACT_REMOVE_BALANCE_MONETARY,*cdrlog,,,,,,,,,,,,,,, ACT_REMOVE_BALANCE_MONETARY,*remove_balance,,,balance_monetary,*monetary,,,,,,,,,,, ACT_REMOVE_EXPIRED_WITH_CATEGORY,*cdrlog,,,,,,,,,,,,,,, -ACT_REMOVE_EXPIRED_WITH_CATEGORY,*remove_expired,,,,*monetary,category2,,,,,,,,,, +ACT_REMOVE_EXPIRED_WITH_CATEGORY,*remove_expired,,,,,category2,,,,,,,,,, ACT_REMOVE_EXPIRED,*cdrlog,,,,,,,,,,,,,,, -ACT_REMOVE_EXPIRED,*remove_expired,,,,*monetary,,,,,,,,,,, +ACT_REMOVE_EXPIRED,*remove_expired,,,,,,,,,,,,,,, ACT_TOPUP_MONETARY,*cdrlog,"{""BalanceID"":""~*acnt.BalanceID""}",,,,,,,,,,,,,, ACT_TOPUP_MONETARY,*topup_reset,,,balance_monetary,*monetary,,*any,,,*unlimited,,150,20,false,false,20 ACT_TOPUP_SMS,*topup_reset,,,balance_sms,*sms,,*any,,,*unlimited,,1000,10,false,false,10`, @@ -855,6 +855,15 @@ ACT_TOPUP_SMS,*topup_reset,,,balance_sms,*sms,,*any,,,*unlimited,,1000,10,false, utils.Categories: "category2", }, }, + { + // will be removed + BalanceType: utils.MetaSMS, + Value: 16, + Balance: map[string]any{ + utils.ID: "ExpiredSMSBalance", + utils.ExpiryTime: expiryTime, + }, + }, }, } var reply string @@ -878,7 +887,7 @@ ACT_TOPUP_SMS,*topup_reset,,,balance_sms,*sms,,*any,,,*unlimited,,1000,10,false, t.Fatal(err) } - if len(cdrs) != 4 || + if len(cdrs) != 5 || cdrs[0].Cost != 11 || cdrs[0].ExtraFields[utils.BalanceID] != "ExpiredBalanceNotMatching1" || cdrs[1].Cost != 12 || @@ -886,19 +895,25 @@ ACT_TOPUP_SMS,*topup_reset,,,balance_sms,*sms,,*any,,,*unlimited,,1000,10,false, cdrs[2].Cost != 13 || cdrs[2].ExtraFields[utils.BalanceID] != "ExpiredBalanceNotMatching3" || cdrs[3].Cost != 14 || - cdrs[3].ExtraFields[utils.BalanceID] != "MatchingExpiredBalance" { + cdrs[3].ExtraFields[utils.BalanceID] != "MatchingExpiredBalance" || + cdrs[4].Cost != 16 || + cdrs[4].ExtraFields[utils.BalanceID] != "ExpiredSMSBalance" { t.Errorf("unexpected cdrs received: %v", utils.ToJSON(cdrs)) } - assertCommonCDRFields := func(t *testing.T, cdr *engine.CDR) { + assertCommonCDRFields := func(t *testing.T, cdr *engine.CDR, expectedType string) { if cdr.RunID != utils.MetaRemoveExpired || cdr.Source != utils.CDRLog || - cdr.ToR != utils.MetaMonetary { + cdr.ToR != expectedType { t.Fatalf("unexpected cdrs received: %v", utils.ToJSON(cdrs)) } } - for _, cdr := range cdrs { - assertCommonCDRFields(t, cdr) + expType := utils.MetaMonetary + for i, cdr := range cdrs { + if i == len(cdrs)-1 { + expType = utils.MetaSMS + } + assertCommonCDRFields(t, cdr, expType) } }) @@ -972,6 +987,16 @@ ACT_TOPUP_SMS,*topup_reset,,,balance_sms,*sms,,*any,,,*unlimited,,1000,10,false, utils.Categories: "category2", }, }, + { + // will be removed + BalanceType: utils.MetaSMS, + Value: 16, + Balance: map[string]any{ + utils.ID: "ExpiredSMSBalance", + utils.ExpiryTime: expiryTime, + utils.Categories: "category1;category2", + }, + }, }, } if err := client.Call(context.Background(), utils.APIerSv1SetBalances, &attrSetBalance, &reply); err != nil { @@ -994,12 +1019,17 @@ ACT_TOPUP_SMS,*topup_reset,,,balance_sms,*sms,,*any,,,*unlimited,,1000,10,false, t.Fatal(err) } - if len(cdrs) != 1 || + if len(cdrs) != 2 || cdrs[0].Cost != 14 || cdrs[0].ExtraFields[utils.BalanceID] != "MatchingExpiredBalance" || cdrs[0].RunID != utils.MetaRemoveExpired || cdrs[0].Source != utils.CDRLog || - cdrs[0].ToR != utils.MetaMonetary { + cdrs[0].ToR != utils.MetaMonetary || + cdrs[1].Cost != 16 || + cdrs[1].ExtraFields[utils.BalanceID] != "ExpiredSMSBalance" || + cdrs[1].RunID != utils.MetaRemoveExpired || + cdrs[1].Source != utils.CDRLog || + cdrs[1].ToR != utils.MetaSMS { t.Errorf("unexpected cdrs received: %v", utils.ToJSON(cdrs)) } })