Revise *transfer_balance action and its API

Ensure parameter fields are not abbreviated.

The action will not depend on balance type anymore. It will go through all
balances.

*default balance from source balance can go negative during transfer.
This commit is contained in:
ionutboangiu
2024-02-19 04:32:27 -05:00
committed by Dan Christian Bogos
parent 0c32f1761c
commit 87da08f7fd
7 changed files with 69 additions and 85 deletions

View File

@@ -717,9 +717,9 @@ func (apierSv1 *APIerSv1) RemoveBalances(ctx *context.Context, attr *utils.AttrS
// TransferBalance sets a temporary *transfer_balance action, which will be executed immediately.
func (apierSv1 *APIerSv1) TransferBalance(ctx *context.Context, attr utils.AttrTransferBalance, reply *string) (err error) {
if missing := utils.MissingStructFields(&attr, []string{
utils.SrcAccountID, utils.SrcBalanceID,
utils.DestAccountID, utils.DestBalanceID,
utils.Units, utils.BalanceType}); len(missing) != 0 {
utils.SourceAccountID, utils.SourceBalanceID,
utils.DestinationAccountID, utils.DestinationBalanceID,
utils.Units}); len(missing) != 0 {
return utils.NewErrMandatoryIeMissing(missing...)
}
if attr.Tenant == "" {
@@ -738,8 +738,8 @@ func (apierSv1 *APIerSv1) TransferBalance(ctx *context.Context, attr utils.AttrT
}
var extraParams []byte
extraParams, err = json.Marshal(map[string]string{
utils.DestAccountID: attr.Tenant + ":" + attr.DestAccountID,
utils.DestBalanceID: attr.DestBalanceID,
utils.DestinationAccountID: attr.Tenant + ":" + attr.DestinationAccountID,
utils.DestinationBalanceID: attr.DestinationBalanceID,
})
if err != nil {
return utils.NewErrServerError(err)
@@ -749,8 +749,7 @@ func (apierSv1 *APIerSv1) TransferBalance(ctx *context.Context, attr utils.AttrT
ActionType: utils.MetaTransferBalance,
ExtraParameters: string(extraParams),
Balance: &engine.BalanceFilter{
ID: utils.StringPointer(attr.SrcBalanceID),
Type: utils.StringPointer(attr.BalanceType),
ID: utils.StringPointer(attr.SourceBalanceID),
Value: &utils.ValueFormula{Static: attr.Units},
},
}
@@ -793,7 +792,7 @@ func (apierSv1 *APIerSv1) TransferBalance(ctx *context.Context, attr utils.AttrT
at := &engine.ActionTiming{
ActionsID: actionID,
}
at.SetAccountIDs(utils.StringMap{utils.ConcatenatedKey(attr.Tenant, attr.SrcAccountID): true})
at.SetAccountIDs(utils.StringMap{utils.ConcatenatedKey(attr.Tenant, attr.SourceAccountID): true})
if err = at.Execute(apierSv1.FilterS); err != nil {
return utils.NewErrServerError(err)
}

View File

@@ -1205,6 +1205,18 @@ func (acc *Account) GetBalanceWithID(blcType, blcID string) (blc *Balance) {
return nil
}
// FindBalanceByID searches through all balance types for a balance with the specified ID and returns it.
func (acc *Account) FindBalanceByID(balanceID string) *Balance {
for _, balances := range acc.BalanceMap {
for _, balance := range balances {
if balance.ID == balanceID {
return balance
}
}
}
return nil
}
// FieldAsInterface func to help EventCost FieldAsInterface
func (as *AccountSummary) FieldAsInterface(fldPath []string) (val any, err error) {
if as == nil || len(fldPath) == 0 {

View File

@@ -132,9 +132,6 @@ func transferBalanceAction(srcAcc *Account, act *Action, _ Actions, fltrS *Filte
if srcAcc == nil {
return errors.New("source account is nil")
}
if act.Balance.Type == nil {
return errors.New("balance type is missing")
}
if act.Balance.ID == nil {
return errors.New("source balance ID is missing")
}
@@ -145,7 +142,7 @@ func transferBalanceAction(srcAcc *Account, act *Action, _ Actions, fltrS *Filte
return fmt.Errorf("account %s has no balances to transfer from", srcAcc.ID)
}
srcBalance := srcAcc.GetBalanceWithID(*act.Balance.Type, *act.Balance.ID)
srcBalance := srcAcc.FindBalanceByID(*act.Balance.ID)
if srcBalance == nil || srcBalance.IsExpiredAt(time.Now()) {
return errors.New("source balance not found or expired")
}
@@ -154,13 +151,13 @@ func transferBalanceAction(srcAcc *Account, act *Action, _ Actions, fltrS *Filte
if transferUnits == 0 {
return errors.New("balance value is missing or 0")
}
if transferUnits > srcBalance.Value {
if srcBalance.ID != utils.MetaDefault && transferUnits > srcBalance.Value {
return utils.ErrInsufficientCredit
}
accDestInfo := struct {
DestAccountID string
DestBalanceID string
DestinationAccountID string
DestinationBalanceID string
}{}
if err := json.Unmarshal([]byte(act.ExtraParameters), &accDestInfo); err != nil {
return err
@@ -169,11 +166,11 @@ func transferBalanceAction(srcAcc *Account, act *Action, _ Actions, fltrS *Filte
// This guard is meant to lock the destination account as we are making changes to it. It was not needed
// for the source account due to it being locked from outside this function.
guardErr := guardian.Guardian.Guard(func() error {
destAcc, err := dm.GetAccount(accDestInfo.DestAccountID)
destAcc, err := dm.GetAccount(accDestInfo.DestinationAccountID)
if err != nil {
return fmt.Errorf("retrieving destination account failed: %w", err)
}
destBalance := destAcc.GetBalanceWithID(*act.Balance.Type, accDestInfo.DestBalanceID)
destBalance := destAcc.FindBalanceByID(accDestInfo.DestinationBalanceID)
if destBalance == nil || destBalance.IsExpiredAt(time.Now()) {
return errors.New("destination balance not found or expired")
}
@@ -190,7 +187,7 @@ func transferBalanceAction(srcAcc *Account, act *Action, _ Actions, fltrS *Filte
return fmt.Errorf("updating destination account failed: %w", err)
}
return nil
}, config.CgrConfig().GeneralCfg().LockingTimeout, utils.AccountPrefix+accDestInfo.DestAccountID)
}, config.CgrConfig().GeneralCfg().LockingTimeout, utils.AccountPrefix+accDestInfo.DestinationAccountID)
if guardErr != nil {
return guardErr
}

View File

@@ -4265,12 +4265,11 @@ func TestActionsTransferBalance(t *testing.T) {
Id: "ACT_TRANSFER_BALANCE",
ActionType: utils.MetaTransferBalance,
ExtraParameters: `{
"DestAccountID": "cgrates.org:ACC_DEST",
"DestBalanceID": "BALANCE_DEST"
"DestinationAccountID": "cgrates.org:ACC_DEST",
"DestinationBalanceID": "BALANCE_DEST"
}`,
Balance: &BalanceFilter{
ID: utils.StringPointer("BALANCE_SRC"),
Type: utils.StringPointer(utils.MetaMonetary),
ID: utils.StringPointer("BALANCE_SRC"),
Value: &utils.ValueFormula{
Static: 3,
},
@@ -4284,7 +4283,7 @@ func TestActionsTransferBalance(t *testing.T) {
expectedErr: "source account is nil",
},
{
name: "UnspecifiedBalanceType",
name: "UnspecifiedBalanceID",
srcAcc: &Account{
ID: "cgrates.org:ACC_SRC",
},
@@ -4292,19 +4291,6 @@ func TestActionsTransferBalance(t *testing.T) {
Id: "ACT_TRANSFER_BALANCE",
Balance: &BalanceFilter{},
},
expectedErr: "balance type is missing",
},
{
name: "UnspecifiedBalanceID",
srcAcc: &Account{
ID: "cgrates.org:ACC_SRC",
},
act: &Action{
Id: "ACT_TRANSFER_BALANCE",
Balance: &BalanceFilter{
Type: utils.StringPointer(utils.MetaMonetary),
},
},
expectedErr: "source balance ID is missing",
},
{
@@ -4315,8 +4301,7 @@ func TestActionsTransferBalance(t *testing.T) {
act: &Action{
Id: "ACT_TRANSFER_BALANCE",
Balance: &BalanceFilter{
Type: utils.StringPointer(utils.MetaMonetary),
ID: utils.StringPointer("BALANCE_SRC"),
ID: utils.StringPointer("BALANCE_SRC"),
},
},
expectedErr: "ExtraParameters used to identify the destination balance are missing",
@@ -4330,8 +4315,7 @@ func TestActionsTransferBalance(t *testing.T) {
Id: "ACT_TRANSFER_BALANCE",
ExtraParameters: "invalid_params",
Balance: &BalanceFilter{
Type: utils.StringPointer(utils.MetaMonetary),
ID: utils.StringPointer("BALANCE_SRC"),
ID: utils.StringPointer("BALANCE_SRC"),
},
},
expectedErr: "account cgrates.org:ACC_SRC has no balances to transfer from",
@@ -4353,8 +4337,7 @@ func TestActionsTransferBalance(t *testing.T) {
Id: "ACT_TRANSFER_BALANCE",
ExtraParameters: "invalid_params",
Balance: &BalanceFilter{
Type: utils.StringPointer(utils.MetaMonetary),
ID: utils.StringPointer("BALANCE_SRC"),
ID: utils.StringPointer("BALANCE_SRC"),
},
},
expectedErr: "source balance not found or expired",
@@ -4376,8 +4359,7 @@ func TestActionsTransferBalance(t *testing.T) {
Id: "ACT_TRANSFER_BALANCE",
ExtraParameters: "invalid_params",
Balance: &BalanceFilter{
Type: utils.StringPointer(utils.MetaMonetary),
ID: utils.StringPointer("BALANCE_SRC"),
ID: utils.StringPointer("BALANCE_SRC"),
Value: &utils.ValueFormula{
Static: 3,
},
@@ -4401,12 +4383,11 @@ func TestActionsTransferBalance(t *testing.T) {
act: &Action{
Id: "ACT_TRANSFER_BALANCE",
ExtraParameters: `{
"DestAccountID": "cgrates.org:ACC_DEST",
"DestBalanceID": "BALANCE_DEST"
"DestinationAccountID": "cgrates.org:ACC_DEST",
"DestinationBalanceID": "BALANCE_DEST"
}`,
Balance: &BalanceFilter{
Type: utils.StringPointer(utils.MetaMonetary),
ID: utils.StringPointer("BALANCE_SRC"),
ID: utils.StringPointer("BALANCE_SRC"),
Value: &utils.ValueFormula{
Static: 3,
},
@@ -4433,12 +4414,11 @@ func TestActionsTransferBalance(t *testing.T) {
act: &Action{
Id: "ACT_TRANSFER_BALANCE",
ExtraParameters: `{
"DestAccountID": "cgrates.org:ACC_DEST",
"DestBalanceID": "BALANCE_DEST"
"DestinationAccountID": "cgrates.org:ACC_DEST",
"DestinationBalanceID": "BALANCE_DEST"
}`,
Balance: &BalanceFilter{
Type: utils.StringPointer(utils.MetaMonetary),
ID: utils.StringPointer("BALANCE_SRC"),
ID: utils.StringPointer("BALANCE_SRC"),
Value: &utils.ValueFormula{
Static: 3,
},
@@ -4473,12 +4453,11 @@ func TestActionsTransferBalance(t *testing.T) {
act: &Action{
Id: "ACT_TRANSFER_BALANCE",
ExtraParameters: `{
"DestAccountID": "cgrates.org:ACC_DEST",
"DestBalanceID": "BALANCE_DEST"
"DestinationAccountID": "cgrates.org:ACC_DEST",
"DestinationBalanceID": "BALANCE_DEST"
}`,
Balance: &BalanceFilter{
Type: utils.StringPointer(utils.MetaMonetary),
ID: utils.StringPointer("BALANCE_SRC"),
ID: utils.StringPointer("BALANCE_SRC"),
Value: &utils.ValueFormula{
Static: 0,
},
@@ -4513,12 +4492,11 @@ func TestActionsTransferBalance(t *testing.T) {
act: &Action{
Id: "ACT_TRANSFER_BALANCE",
ExtraParameters: `{
"DestAccountID": "cgrates.org:ACC_DEST",
"DestBalanceID": "BALANCE_DEST"
"DestinationAccountID": "cgrates.org:ACC_DEST",
"DestinationBalanceID": "BALANCE_DEST"
}`,
Balance: &BalanceFilter{
Type: utils.StringPointer(utils.MetaMonetary),
ID: utils.StringPointer("BALANCE_SRC"),
ID: utils.StringPointer("BALANCE_SRC"),
Value: &utils.ValueFormula{
Static: 3,
},
@@ -4553,12 +4531,11 @@ func TestActionsTransferBalance(t *testing.T) {
act: &Action{
Id: "ACT_TRANSFER_BALANCE",
ExtraParameters: `{
"DestAccountID": "cgrates.org:ACC_DEST_FAIL",
"DestBalanceID": "BALANCE_DEST"
"DestinationAccountID": "cgrates.org:ACC_DEST_FAIL",
"DestinationBalanceID": "BALANCE_DEST"
}`,
Balance: &BalanceFilter{
Type: utils.StringPointer(utils.MetaMonetary),
ID: utils.StringPointer("BALANCE_SRC"),
ID: utils.StringPointer("BALANCE_SRC"),
Value: &utils.ValueFormula{
Static: 3,
},

View File

@@ -77,7 +77,7 @@ 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,"{""DestAccountID"":""cgrates.org:ACC_DEST"",""DestBalanceID"":""balance_dest""}",,balance_src,*monetary,,,,,*unlimited,,4,,,,`,
ACT_TRANSFER,*transfer_balance,"{""DestinationAccountID"":""cgrates.org:ACC_DEST"",""DestinationBalanceID"":""balance_dest""}",,balance_src,*monetary,,,,,*unlimited,,4,,,,`,
}
testEnv := TestEnvironment{
@@ -163,13 +163,12 @@ ACT_TRANSFER,*transfer_balance,"{""DestAccountID"":""cgrates.org:ACC_DEST"",""De
t.Run("TransferBalanceByAPI", func(t *testing.T) {
var reply string
if err := client.Call(context.Background(), utils.APIerSv1TransferBalance, utils.AttrTransferBalance{
Tenant: "cgrates.org",
SrcAccountID: "ACC_SRC",
SrcBalanceID: "balance_src",
DestAccountID: "ACC_DEST",
DestBalanceID: "balance_dest",
Units: 2,
BalanceType: utils.MetaMonetary,
Tenant: "cgrates.org",
SourceAccountID: "ACC_SRC",
SourceBalanceID: "balance_src",
DestinationAccountID: "ACC_DEST",
DestinationBalanceID: "balance_dest",
Units: 2,
}, &reply); err != nil {
t.Error(err)
}

View File

@@ -880,15 +880,15 @@ type AttrBalance struct {
}
type AttrTransferBalance struct {
Tenant string
SrcAccountID string
SrcBalanceID string
DestAccountID string
DestBalanceID string
Units float64
BalanceType string
Overwrite bool
Cdrlog bool
Tenant string
SourceAccountID string
SourceBalanceID string
DestinationAccountID string
DestinationBalanceID string
Units float64
Overwrite bool
Cdrlog bool
APIOpts map[string]any
}
// TPResourceProfile is used in APIs to manage remotely offline ResourceProfile

View File

@@ -491,10 +491,10 @@ const (
EventSource = "EventSource"
AccountID = "AccountID"
AccountIDs = "AccountIDs"
SrcAccountID = "SrcAccountID"
DestAccountID = "DestAccountID"
SrcBalanceID = "SrcBalanceID"
DestBalanceID = "DestBalanceID"
SourceAccountID = "SourceAccountID"
DestinationAccountID = "DestinationAccountID"
SourceBalanceID = "SourceBalanceID"
DestinationBalanceID = "DestinationBalanceID"
ResourceID = "ResourceID"
TotalUsage = "TotalUsage"
StatID = "StatID"