Optimize TransferBalanceAPI and test *cdrlog functionality

apier/v1/accounts.go

@@ -21,7 +21,6 @@ package v1
 import (
 	"encoding/json"
 	"errors"
-	"fmt"
 	"math"
 	"slices"
 	"strings"
@@ -714,28 +713,23 @@ func (apierSv1 *APIerSv1) RemoveBalances(ctx *context.Context, attr *utils.AttrS
 	return nil
 }
 
-// TransferBalance sets a temporary *transfer_balance action, which will be executed immediately.
+// TransferBalance initiates a balance transfer between accounts, immediately executing the configured actions.
 func (apierSv1 *APIerSv1) TransferBalance(ctx *context.Context, attr utils.AttrTransferBalance, reply *string) (err error) {
+
+	// Check for missing mandatory fields in the request attributes.
 	if missing := utils.MissingStructFields(&attr, []string{
 		utils.SourceAccountID, utils.SourceBalanceID,
 		utils.DestinationAccountID, utils.DestinationBalanceID,
 		utils.Units}); len(missing) != 0 {
 		return utils.NewErrMandatoryIeMissing(missing...)
 	}
+
+	// Use default tenant if not specified in the request attributes.
 	if attr.Tenant == "" {
 		attr.Tenant = apierSv1.Config.GeneralCfg().DefaultTenant
 	}
 
-	// Set *transfer_balance action.
-	actionID := fmt.Sprintf("tmp_act_%s", utils.UUIDSha1Prefix())
-	if !attr.Overwrite {
-		var exists bool
-		if exists, err = apierSv1.DataManager.HasData(utils.ActionPrefix, actionID, ""); err != nil {
-			return utils.NewErrServerError(err)
-		} else if exists {
-			return utils.ErrExists
-		}
-	}
+	// Marshal extra parameters including the destination account and balance ID.
 	var extraParams []byte
 	extraParams, err = json.Marshal(map[string]string{
 		utils.DestinationAccountID: attr.Tenant + ":" + attr.DestinationAccountID,
@@ -744,54 +738,26 @@ func (apierSv1 *APIerSv1) TransferBalance(ctx *context.Context, attr utils.AttrT
 	if err != nil {
 		return utils.NewErrServerError(err)
 	}
-	action := &engine.Action{
-		Id:              actionID,
+
+	// Prepare actions for the balance transfer and optional CDR logging.
+	actions := make([]*engine.Action, 0, 2)
+	actions = append(actions, &engine.Action{
 		ActionType:      utils.MetaTransferBalance,
 		ExtraParameters: string(extraParams),
 		Balance: &engine.BalanceFilter{
 			ID:    utils.StringPointer(attr.SourceBalanceID),
 			Value: &utils.ValueFormula{Static: attr.Units},
 		},
-	}
-	if err = apierSv1.DataManager.SetActions(actionID, engine.Actions{action}); err != nil {
-		return utils.NewErrServerError(err)
+	})
+	if attr.Cdrlog {
+		actions = append(actions, &engine.Action{
+			ActionType: utils.CDRLog,
+		})
 	}
 
-	// Remove the action.
-	defer func() {
-		if removeErr := apierSv1.DataManager.RemoveActions(actionID); err != nil {
-			err = errors.Join(err, removeErr)
-			return
-		}
-		if reloadCacheErr := apierSv1.ConnMgr.Call(context.TODO(), apierSv1.Config.ApierCfg().CachesConns,
-			utils.CacheSv1ReloadCache, &utils.AttrReloadCacheWithAPIOpts{
-				ActionIDs: []string{actionID},
-			}, reply); reloadCacheErr != nil {
-			err = errors.Join(err, reloadCacheErr)
-			return
-		}
-		if setLoadIDErr := apierSv1.DataManager.SetLoadIDs(map[string]int64{utils.CacheActions: time.Now().UnixNano()}); setLoadIDErr != nil {
-			err = errors.Join(err, setLoadIDErr)
-			return
-		}
-		*reply = utils.OK
-	}()
-
-	if err = apierSv1.ConnMgr.Call(context.TODO(), apierSv1.Config.ApierCfg().CachesConns,
-		utils.CacheSv1ReloadCache, &utils.AttrReloadCacheWithAPIOpts{
-			ActionIDs: []string{actionID},
-		}, reply); err != nil {
-		return utils.NewErrServerError(err)
-	}
-	//generate a loadID for CacheActions and store it in database
-	if err = apierSv1.DataManager.SetLoadIDs(map[string]int64{utils.CacheActions: time.Now().UnixNano()}); err != nil {
-		return utils.NewErrServerError(err)
-	}
-
-	// Execute the action.
-	at := &engine.ActionTiming{
-		ActionsID: actionID,
-	}
+	// Execute the prepared actions for the account specified.
+	at := &engine.ActionTiming{}
+	at.SetActions(actions)
 	at.SetAccountIDs(utils.StringMap{utils.ConcatenatedKey(attr.Tenant, attr.SourceAccountID): true})
 	if err = at.Execute(apierSv1.FilterS); err != nil {
 		return utils.NewErrServerError(err)

general_tests/balance_it_test.go

@@ -463,7 +463,7 @@ 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,*cdrlog,,,,*monetary,,,,,,,,,,,
+ACT_REMOVE_EXPIRED,*cdrlog,,,,,,,,,,,,,,,
 ACT_REMOVE_EXPIRED,*remove_expired,,,,*monetary,,,,,,,,,,,
 ACT_TOPUP_MONETARY,*cdrlog,"{""BalanceID"":""~*acnt.BalanceID""}",,,,,,,,,,,,,,
 ACT_TOPUP_MONETARY,*topup_reset,,,balance_monetary,*monetary,,*any,,,*unlimited,,150,20,false,false,20

general_tests/transfer_balance_it_test.go

@@ -56,8 +56,13 @@ func TestTransferBalance(t *testing.T) {
 	"db_type": "*internal"
 },
 
+"cdrs": {
+	"enabled": true,
+},
+
 "schedulers": {
-	"enabled": true
+	"enabled": true,
+	"cdrs_conns": ["*internal"]
 },
 
 "apiers": {
@@ -77,7 +82,8 @@ PACKAGE_ACC_DEST,ACT_TOPUP_DEST,*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_TOPUP_SRC,*topup_reset,,,balance_src,*monetary,,*any,,,*unlimited,,10,20,false,false,20
 ACT_TOPUP_DEST,*topup_reset,,,balance_dest,*monetary,,*any,,,*unlimited,,10,10,false,false,10
-ACT_TRANSFER,*transfer_balance,"{""DestinationAccountID"":""cgrates.org:ACC_DEST"",""DestinationBalanceID"":""balance_dest""}",,balance_src,*monetary,,,,,*unlimited,,4,,,,`,
+ACT_TRANSFER,*cdrlog,,,,,,,,,,,,,,,
+ACT_TRANSFER,*transfer_balance,"{""DestinationAccountID"":""cgrates.org:ACC_DEST"",""DestinationBalanceID"":""balance_dest""}",,balance_src,,,,,,*unlimited,,4,,,,`,
 	}
 
 	testEnv := TestEnvironment{
@@ -169,6 +175,7 @@ ACT_TRANSFER,*transfer_balance,"{""DestinationAccountID"":""cgrates.org:ACC_DEST
 			DestinationAccountID: "ACC_DEST",
 			DestinationBalanceID: "balance_dest",
 			Units:                2,
+			Cdrlog:               true,
 		}, &reply); err != nil {
 			t.Error(err)
 		}
@@ -203,4 +210,35 @@ ACT_TRANSFER,*transfer_balance,"{""DestinationAccountID"":""cgrates.org:ACC_DEST
 			t.Errorf("received account with unexpected balance: %v", balance)
 		}
 	})
+
+	t.Run("CheckTransferBalanceCDRs", func(t *testing.T) {
+		var cdrs []*engine.CDR
+		if err := client.Call(context.Background(), utils.CDRsV1GetCDRs, &utils.RPCCDRsFilterWithAPIOpts{
+			RPCCDRsFilter: &utils.RPCCDRsFilter{
+				OrderBy: utils.Cost,
+			}}, &cdrs); err != nil {
+			t.Fatal(err)
+		}
+
+		if len(cdrs) != 2 {
+			t.Errorf("expected to receive 2 cdrs: %v", utils.ToJSON(cdrs))
+		}
+
+		expectedCost := 2. // expected cost of first cdr
+		for _, cdr := range cdrs {
+			if cdr.Account != "ACC_SRC" ||
+				cdr.Destination != "ACC_DEST" ||
+				cdr.RunID != utils.MetaTransferBalance ||
+				cdr.Source != utils.CDRLog ||
+				cdr.ToR != utils.MetaVoice ||
+				cdr.ExtraFields["DestinationBalanceID"] != "balance_dest" ||
+				cdr.ExtraFields["SourceBalanceID"] != "balance_src" {
+				t.Errorf("unexpected cdr received: %v", utils.ToJSON(cdr))
+			}
+			if cdr.Cost != expectedCost {
+				t.Errorf("cost expected to be %v, received %v", expectedCost, cdr.Cost)
+			}
+			expectedCost = 4 // expected cost of second cdr
+		}
+	})
 }

utils/apitpdata.go

@@ -886,7 +886,6 @@ type AttrTransferBalance struct {
 	DestinationAccountID string
 	DestinationBalanceID string
 	Units                float64
-	Overwrite            bool
 	Cdrlog               bool
 	APIOpts              map[string]any
 }