From 843eee3b8db97ced89a8b6de3a543049984bf93e Mon Sep 17 00:00:00 2001 From: ionutboangiu Date: Wed, 13 Mar 2024 12:07:55 -0400 Subject: [PATCH] Pass clone of original acc for *cdrlog actions --- engine/action_plan.go | 73 ++++++++++++++++++-------------- engine/action_trigger.go | 61 +++++++++++++++----------- general_tests/balance_it_test.go | 8 ++-- 3 files changed, 81 insertions(+), 61 deletions(-) diff --git a/engine/action_plan.go b/engine/action_plan.go index fb2501ffc..2a3505e83 100644 --- a/engine/action_plan.go +++ b/engine/action_plan.go @@ -195,14 +195,14 @@ func (at *ActionTiming) getActions() (as []*Action, err error) { // Reports on success/fail via channel if != nil func (at *ActionTiming) Execute(fltrS *FilterS, originService string) (err error) { at.ResetStartTimeCache() - aac, err := at.getActions() + acts, err := at.getActions() if err != nil { utils.Logger.Err(fmt.Sprintf("Failed to get actions for %s: %s", at.ActionsID, err)) return } var partialyExecuted bool for accID := range at.accountIDs { - err = guardian.Guardian.Guard(func() error { + _ = guardian.Guardian.Guard(func() error { acc, err := dm.GetAccount(accID) if err != nil { // create account if err != utils.ErrNotFound { @@ -216,45 +216,54 @@ func (at *ActionTiming) Execute(fltrS *FilterS, originService string) (err error } transactionFailed := false removeAccountActionFound := false - currentTime := time.Now() - for _, a := range aac { + accClone := acc.Clone() // *cdrlog action requires the original account + referenceTime := time.Now() + for _, act := range acts { // check action filter - if len(a.Filters) > 0 { - if pass, err := fltrS.Pass(utils.NewTenantID(accID).Tenant, a.Filters, + if len(act.Filters) > 0 { + if pass, err := fltrS.Pass(utils.NewTenantID(accID).Tenant, act.Filters, utils.MapStorage{utils.MetaReq: acc}); err != nil { return err } else if !pass { continue } } - if a.Balance == nil { - a.Balance = &BalanceFilter{} + if act.Balance == nil { + act.Balance = &BalanceFilter{} } - if a.ExpirationString != "" { // if it's *unlimited then it has to be zero time - if expDate, parseErr := utils.ParseTimeDetectLayout(a.ExpirationString, + if act.ExpirationString != "" { // if it's *unlimited then it has to be zero time + if expDate, parseErr := utils.ParseTimeDetectLayout(act.ExpirationString, config.CgrConfig().GeneralCfg().DefaultTimezone); parseErr == nil { - a.Balance.ExpirationDate = &time.Time{} - *a.Balance.ExpirationDate = expDate + act.Balance.ExpirationDate = &time.Time{} + *act.Balance.ExpirationDate = expDate } } - actionFunction, exists := getActionFunc(a.ActionType) + actionFunction, exists := getActionFunc(act.ActionType) if !exists { // do not allow the action plan to be rescheduled at.Timing = nil - utils.Logger.Err(fmt.Sprintf("Function type %v not available, aborting execution!", a.ActionType)) + utils.Logger.Err( + fmt.Sprintf("Function type %v not available, aborting execution!", + act.ActionType)) partialyExecuted = true transactionFailed = true break } - if err := actionFunction(acc, a, aac, fltrS, at.ExtraData, currentTime, - newActionConnCfg(originService, a.ActionType, config.CgrConfig())); err != nil { - utils.Logger.Err(fmt.Sprintf("Error executing action %s: %v!", a.ActionType, err)) + tmpAcc := acc + if act.ActionType == utils.CDRLog { + tmpAcc = accClone + } + if err := actionFunction(tmpAcc, act, acts, fltrS, at.ExtraData, referenceTime, + newActionConnCfg(originService, act.ActionType, config.CgrConfig())); err != nil { + utils.Logger.Err( + fmt.Sprintf("Error executing action %s: %v!", + act.ActionType, err)) partialyExecuted = true transactionFailed = true break } - if a.ActionType == utils.MetaRemoveAccount { + if act.ActionType == utils.MetaRemoveAccount { removeAccountActionFound = true } } @@ -268,34 +277,34 @@ func (at *ActionTiming) Execute(fltrS *FilterS, originService string) (err error err = nil if len(at.accountIDs) == 0 { // action timing executing without accounts referenceTime := time.Now() - for _, a := range aac { - if expDate, parseErr := utils.ParseTimeDetectLayout(a.ExpirationString, - config.CgrConfig().GeneralCfg().DefaultTimezone); (a.Balance == nil || a.Balance.EmptyExpirationDate()) && + for _, act := range acts { + if expDate, parseErr := utils.ParseTimeDetectLayout(act.ExpirationString, + config.CgrConfig().GeneralCfg().DefaultTimezone); (act.Balance == nil || act.Balance.EmptyExpirationDate()) && parseErr == nil && !expDate.IsZero() { - a.Balance.ExpirationDate = &time.Time{} - *a.Balance.ExpirationDate = expDate + act.Balance.ExpirationDate = &time.Time{} + *act.Balance.ExpirationDate = expDate } - actionFunction, exists := getActionFunc(a.ActionType) + actionFunction, exists := getActionFunc(act.ActionType) if !exists { // do not allow the action plan to be rescheduled at.Timing = nil - utils.Logger.Err(fmt.Sprintf("Function type %v not available, aborting execution!", a.ActionType)) + utils.Logger.Err( + fmt.Sprintf("Function type %v not available, aborting execution!", + act.ActionType)) partialyExecuted = true break } - if err := actionFunction(nil, a, aac, fltrS, at.ExtraData, referenceTime, - newActionConnCfg(originService, a.ActionType, config.CgrConfig())); err != nil { - utils.Logger.Err(fmt.Sprintf("Error executing accountless action %s: %v!", a.ActionType, err)) + if err := actionFunction(nil, act, acts, fltrS, at.ExtraData, referenceTime, + newActionConnCfg(originService, act.ActionType, config.CgrConfig())); err != nil { + utils.Logger.Err( + fmt.Sprintf("Error executing accountless action %s: %v!", + act.ActionType, err)) partialyExecuted = true break } } } - if err != nil { - utils.Logger.Warning(fmt.Sprintf("Error executing action plan: %v", err)) - return err - } if partialyExecuted { return utils.ErrPartiallyExecuted } diff --git a/engine/action_trigger.go b/engine/action_trigger.go index daeb3ac61..23e0b2213 100644 --- a/engine/action_trigger.go +++ b/engine/action_trigger.go @@ -46,70 +46,81 @@ type ActionTrigger struct { LastExecutionTime time.Time } -func (at *ActionTrigger) Execute(ub *Account, fltrS *FilterS) (err error) { +func (at *ActionTrigger) Execute(acc *Account, fltrS *FilterS) (err error) { // check for min sleep time if at.Recurrent && !at.LastExecutionTime.IsZero() && time.Since(at.LastExecutionTime) < at.MinSleep { return } at.LastExecutionTime = time.Now() - if ub != nil && ub.Disabled { - return fmt.Errorf("User %s is disabled and there are triggers in action!", ub.ID) + if acc != nil && acc.Disabled { + return fmt.Errorf("User %s is disabled and there are triggers in action!", acc.ID) } // does NOT need to Lock() because it is triggered from a method that took the Lock - var aac Actions - aac, err = dm.GetActions(at.ActionsID, false, utils.NonTransactional) + var acts Actions + acts, err = dm.GetActions(at.ActionsID, false, utils.NonTransactional) if err != nil { - utils.Logger.Err(fmt.Sprintf("Failed to get actions: %v", err)) + utils.Logger.Err( + fmt.Sprintf("Failed to get actions: %v", + err)) return } - aac.Sort() + acts.Sort() at.Executed = true transactionFailed := false removeAccountActionFound := false + accClone := acc.Clone() // *cdrlog action requires the original account referenceTime := time.Now() - for _, a := range aac { + for _, act := range acts { // check action filter - if len(a.Filters) > 0 { - if pass, err := fltrS.Pass(utils.NewTenantID(a.Id).Tenant, a.Filters, - utils.MapStorage{utils.MetaReq: ub}); err != nil { + if len(act.Filters) > 0 { + if pass, err := fltrS.Pass(utils.NewTenantID(act.Id).Tenant, act.Filters, + utils.MapStorage{utils.MetaReq: acc}); err != nil { return err } else if !pass { continue } } - if a.Balance == nil { - a.Balance = &BalanceFilter{} + if act.Balance == nil { + act.Balance = &BalanceFilter{} } - if a.ExpirationString != "" { // if it's *unlimited then it has to be zero time' - if expDate, parseErr := utils.ParseTimeDetectLayout(a.ExpirationString, + if act.ExpirationString != "" { // if it's *unlimited then it has to be zero time' + if expDate, parseErr := utils.ParseTimeDetectLayout(act.ExpirationString, config.CgrConfig().GeneralCfg().DefaultTimezone); parseErr == nil { - a.Balance.ExpirationDate = &time.Time{} - *a.Balance.ExpirationDate = expDate + act.Balance.ExpirationDate = &time.Time{} + *act.Balance.ExpirationDate = expDate } } - actionFunction, exists := getActionFunc(a.ActionType) + actionFunction, exists := getActionFunc(act.ActionType) if !exists { - utils.Logger.Err(fmt.Sprintf("Function type %v not available, aborting execution!", a.ActionType)) + utils.Logger.Err( + fmt.Sprintf("Function type %v not available, aborting execution!", + act.ActionType)) transactionFailed = false break } //go utils.Logger.Info(fmt.Sprintf("Executing %v, %v: %v", ub, sq, a)) - if err := actionFunction(ub, a, aac, fltrS, nil, referenceTime, - newActionConnCfg(utils.RALs, a.ActionType, config.CgrConfig())); err != nil { - utils.Logger.Err(fmt.Sprintf("Error executing action %s: %v!", a.ActionType, err)) + tmpAcc := acc + if act.ActionType == utils.CDRLog { + tmpAcc = accClone + } + if err := actionFunction(tmpAcc, act, acts, fltrS, nil, referenceTime, + newActionConnCfg(utils.RALs, act.ActionType, config.CgrConfig())); err != nil { + utils.Logger.Err( + fmt.Sprintf("Error executing action %s: %v!", + act.ActionType, err)) transactionFailed = false break } - if a.ActionType == utils.MetaRemoveAccount { + if act.ActionType == utils.MetaRemoveAccount { removeAccountActionFound = true } } if transactionFailed || at.Recurrent { at.Executed = false } - if !transactionFailed && ub != nil && !removeAccountActionFound { - dm.SetAccount(ub) + if !transactionFailed && acc != nil && !removeAccountActionFound { + dm.SetAccount(acc) } return } diff --git a/general_tests/balance_it_test.go b/general_tests/balance_it_test.go index df016ac8d..d52dc37b5 100644 --- a/general_tests/balance_it_test.go +++ b/general_tests/balance_it_test.go @@ -697,14 +697,14 @@ cgrates.org,ACC_TEST,PACKAGE_ACC_TEST,,,`, PACKAGE_ACC_TEST,ACT_TOPUP_MONETARY,*asap,10 PACKAGE_ACC_TEST,ACT_TOPUP_SMS,*asap,10`, utils.ActionsCsv: `#ActionsId[0],Action[1],ExtraParameters[2],Filter[3],BalanceId[4],BalanceType[5],Categories[6],DestinationIds[7],RatingSubject[8],SharedGroup[9],ExpiryTime[10],TimingIds[11],Units[12],BalanceWeight[13],BalanceBlocker[14],BalanceDisabled[15],Weight[16] -ACT_REMOVE_BALANCE_MONETARY,*cdrlog,,,,,,,,,,,,,,, ACT_REMOVE_BALANCE_MONETARY,*remove_balance,,,balance_monetary,*monetary,,,,,,,,,,, -ACT_REMOVE_EXPIRED_WITH_CATEGORY,*cdrlog,,,,,,,,,,,,,,, +ACT_REMOVE_BALANCE_MONETARY,*cdrlog,,,,,,,,,,,,,,, ACT_REMOVE_EXPIRED_WITH_CATEGORY,*remove_expired,,,,,category2,,,,,,,,,, -ACT_REMOVE_EXPIRED,*cdrlog,,,,,,,,,,,,,,, +ACT_REMOVE_EXPIRED_WITH_CATEGORY,*cdrlog,,,,,,,,,,,,,,, ACT_REMOVE_EXPIRED,*remove_expired,,,,,,,,,,,,,,, -ACT_TOPUP_MONETARY,*cdrlog,"{""BalanceID"":""~*acnt.BalanceID""}",,,,,,,,,,,,,, +ACT_REMOVE_EXPIRED,*cdrlog,,,,,,,,,,,,,,, ACT_TOPUP_MONETARY,*topup_reset,,,balance_monetary,*monetary,,*any,,,*unlimited,,150,20,false,false,20 +ACT_TOPUP_MONETARY,*cdrlog,"{""BalanceID"":""~*acnt.BalanceID""}",,,,,,,,,,,,,, ACT_TOPUP_SMS,*topup_reset,,,balance_sms,*sms,,*any,,,*unlimited,,1000,10,false,false,10`, }