diff --git a/agents/radius_coa_it_test.go b/agents/radius_coa_it_test.go index 485a3525d..2ac0c8a3d 100644 --- a/agents/radius_coa_it_test.go +++ b/agents/radius_coa_it_test.go @@ -102,7 +102,7 @@ func TestRadiusCoADisconnect(t *testing.T) { ActionsId: "ACT_RAD_COA_ACNT_1001", Actions: []*utils.TPAction{{ Identifier: utils.MetaAlterSessions, - ExtraParameters: "cgrates.org;*string:~*req.Account:1001;1;*radCoATemplate:mycoa;CustomFilter:custom_filter", + ExtraParameters: "localhost:2012;*json;cgrates.org;*string:~*req.Account:1001;1;*radCoATemplate:mycoa;CustomFilter:custom_filter", }}} if err := raDiscRPC.Call(context.Background(), utils.APIerSv2SetActions, actRadCoaAcnt1001, &reply); err != nil { @@ -279,6 +279,6 @@ func TestRadiusCoADisconnect(t *testing.T) { select { case <-done: case <-time.After(time.Second): - t.Error("client did not receive a the expected requests in time") + t.Error("client did not receive the expected requests in time") } } diff --git a/analyzers/analyzers_it_test.go b/analyzers/analyzers_it_test.go index 973cf203b..bde2a1714 100644 --- a/analyzers/analyzers_it_test.go +++ b/analyzers/analyzers_it_test.go @@ -123,7 +123,7 @@ func testAnalyzerSStartEngine(t *testing.T) { // Connect rpc client to rater func testAnalyzerSRPCConn(t *testing.T) { - srv, err := birpc.NewService(new(smock), utils.SessionSv1, true) + srv, err := birpc.NewService(new(smock), utils.AgentV1, true) if err != nil { t.Fatal(err) } diff --git a/apier/v1/sessions_thresholds_it_test.go b/apier/v1/sessions_thresholds_it_test.go index 64506bda6..6d8e093b8 100644 --- a/apier/v1/sessions_thresholds_it_test.go +++ b/apier/v1/sessions_thresholds_it_test.go @@ -126,7 +126,7 @@ func testSessionSv1ItRpcConn(t *testing.T) { if err != nil { t.Fatal(err) } - srv, err := birpc.NewService(new(smock2), utils.SessionSv1, true) + srv, err := birpc.NewService(new(smock2), utils.AgentV1, true) if err != nil { t.Fatal(err) } diff --git a/apier/v1/sessionsv1_it_test.go b/apier/v1/sessionsv1_it_test.go index e959702c3..e18f1af22 100644 --- a/apier/v1/sessionsv1_it_test.go +++ b/apier/v1/sessionsv1_it_test.go @@ -167,7 +167,7 @@ func testSSv1ItRpcConn(t *testing.T) { if err != nil { t.Fatal(err) } - srv, err := birpc.NewService(new(smock), utils.SessionSv1, true) + srv, err := birpc.NewService(new(smock), utils.AgentV1, true) if err != nil { t.Fatal(err) } diff --git a/cmd/cgr-tester/sessions.go b/cmd/cgr-tester/sessions.go index f00e6c2fe..2a319d541 100644 --- a/cmd/cgr-tester/sessions.go +++ b/cmd/cgr-tester/sessions.go @@ -74,7 +74,7 @@ func callSessions(ctx *context.Context, authDur, initDur, updateDur, terminateDu APIOpts: map[string]any{}, } - srv, err := birpc.NewService(new(smock), utils.SessionSv1, true) + srv, err := birpc.NewService(new(smock), utils.AgentV1, true) if err != nil { return err } diff --git a/data/tariffplans/oldtutorial/Actions.csv b/data/tariffplans/oldtutorial/Actions.csv index 71f2be666..dbab1ea6b 100644 --- a/data/tariffplans/oldtutorial/Actions.csv +++ b/data/tariffplans/oldtutorial/Actions.csv @@ -11,4 +11,4 @@ DISABLE_AND_LOG,*log,,,,,,,,,,,,,false,false,10 DISABLE_AND_LOG,*disable_account,,,,,,,,,,,,,false,false,10 TOPUP_100SMS_DE_MOBILE,*topup,,,,*sms,,DST_DE_MOBILE,,,,,100,10,false,false,10 #ACT_RAD_COA_ACNT_1001,*cgr_rpc,"{""Address"":""localhost:2012"",""Transport"":""*json"",""Method"":""SessionSv1.AlterSessions"",""Attempts"":1,""Async"":false,""Params"":{""Filters"":[""*string:~*req.Account:1001""],""Tenant"":""cgrates.org"",""APIOpts"":{""*radCoATemplate"":""mycoa""},""Event"":{""CustomFilter"":""custom_filter""}}}",,,,,,,,,,,,,,20 -ACT_RAD_COA_ACNT_1001,*alter_sessions,cgrates.org;*string:~*req.Account:1001;1;*radCoATemplate:mycoa;CustomFilter:mycustomvalue,,,,,,,,,,,,,, +ACT_RAD_COA_ACNT_1001,*alter_sessions,localhost:2012;*json;cgrates.org;*string:~*req.Account:1001;1;*radCoATemplate:mycoa;CustomFilter:mycustomvalue,,,,,,,,,,,,,, \ No newline at end of file diff --git a/engine/action.go b/engine/action.go index 83e958b85..dcb5b6957 100644 --- a/engine/action.go +++ b/engine/action.go @@ -850,54 +850,73 @@ func cgrRPCAction(ub *Account, a *Action, acs Actions, _ *FilterS, extraData any return } -func alterSessionsAction(_ *Account, a *Action, _ Actions, _ *FilterS, _ any) error { - client, err := rpcclient.NewRPCClient(context.TODO(), utils.TCP, - config.CgrConfig().ListenCfg().RPCJSONListen, - false, "", "", "", 1, 0, - config.CgrConfig().GeneralCfg().MaxReconnectInterval, - utils.FibDuration, - config.CgrConfig().GeneralCfg().ConnectTimeout, - config.CgrConfig().GeneralCfg().ReplyTimeout, - utils.MetaJSON, nil, false, nil) - if err != nil { - return err +// alterSessionsAction processes the `ExtraParameters` field from the action to construct a request +// for the `SessionSv1.AlterSessions` API call. +// +// The ExtraParameters field format is expected as follows: +// - address: string, specifies the server address in the format host:port or *internal. +// - codec: string, specifies the encoding used for communication <*json|*gob|*http_jsonrpc>. +// - tenant: string +// - limit: integer, specifying the maximum number of sessions to alter. +// - filters: strings separated by "&". +// - APIOpts: set of key-value pairs (separated by "&"). +// - Event: set of key-value pairs (separated by "&"). +// +// Parameters are separated by ";" and must be provided in the specified order. +func alterSessionsAction(_ *Account, act *Action, _ Actions, _ *FilterS, _ any) (err error) { + + // Parse action parameters based on the predefined format. + params := strings.Split(act.ExtraParameters, ";") + if len(params) != 7 { + return errors.New("invalid number of parameters; expected 7") } - // Parse action parameters, expecting 5 parameters separated by ";". - params := strings.Split(a.ExtraParameters, ";") - if len(params) != 5 { - return errors.New("invalid number of parameters; expected 5") - } - - // Default limit to 1 if not specified, else parse the limit from parameters. - var limit int - if params[2] == "" { - limit = 1 - } else { - if limit, err = strconv.Atoi(params[2]); err != nil { - return fmt.Errorf("invalid limit parameter: %s", params[2]) + // Establish a client connection. + address := params[0] + codec := params[1] + var client birpc.ClientConnector + switch address { + case utils.MetaInternal: + // For internal connections, use the already registered SessionSv1 birpc.Service object. + var rpcParams *utils.RpcParams + rpcParams, err = utils.GetRpcParams(utils.SessionSv1AlterSessions) + if err != nil { + return fmt.Errorf("retrieving service for *internal calls failed: %w", err) + } + client = rpcParams.Object.(birpc.ClientConnector) + default: + // For external connections, create a new RPCClient. + client, err = rpcclient.NewRPCClient(context.TODO(), utils.TCP, address, + false, "", "", "", 1, 0, + config.CgrConfig().GeneralCfg().MaxReconnectInterval, + utils.FibDuration, + config.CgrConfig().GeneralCfg().ConnectTimeout, + config.CgrConfig().GeneralCfg().ReplyTimeout, + codec, nil, false, nil) + if err != nil { + return fmt.Errorf("failed to init RPCClient: %w", err) } } - // Prepare request argument with provided parameters. + // If conversion fails, limit will default to 0. + limit, _ := strconv.Atoi(params[4]) + + // Prepare request arguments based on provided parameters. attr := utils.SessionFilterWithEvent{ SessionFilter: &utils.SessionFilter{ Limit: &limit, - Tenant: params[0], - Filters: strings.Split(params[1], "&"), + Tenant: params[2], + Filters: strings.Split(params[3], "&"), APIOpts: make(map[string]any), }, Event: make(map[string]any), } - // Use default tenant if not specified. - if attr.Tenant == "" { - attr.Tenant = config.CgrConfig().GeneralCfg().DefaultTenant - } - - // Parse API options and event parameters from provided strings. + // Helper function to parse key-value pairs for API options and event data. parseKVParams := func(paramStr string, targetMap map[string]any) error { for _, tuple := range strings.Split(paramStr, "&") { + // Use strings.Cut to split 'tuple' into key-value pairs at the first occurrence of ':'. + // This ensures that additional ':' characters within the value do not affect parsing. key, value, found := strings.Cut(tuple, ":") if !found { return fmt.Errorf("invalid key-value pair: %s", tuple) @@ -906,10 +925,10 @@ func alterSessionsAction(_ *Account, a *Action, _ Actions, _ *FilterS, _ any) er } return nil } - if err := parseKVParams(params[3], attr.APIOpts); err != nil { + if err := parseKVParams(params[5], attr.APIOpts); err != nil { return err } - if err := parseKVParams(params[4], attr.Event); err != nil { + if err := parseKVParams(params[6], attr.Event); err != nil { return err } diff --git a/engine/actions_test.go b/engine/actions_test.go index db327a2fa..5d18b4d1f 100644 --- a/engine/actions_test.go +++ b/engine/actions_test.go @@ -4575,43 +4575,78 @@ func TestActionsTransferBalance(t *testing.T) { } } -// func TestActionsAlterSessions(t *testing.T) { +type mockSessionSv1Obj struct { + request string +} -// testcases := []struct { -// name string -// extraParams string -// expectedErr string -// }{ -// { -// name: "SuccessfulParse", -// extraParams: "tenant.com;*string:~*req.Account:1001&*prefix:~*req.Destination:+40;1;*radCoATemplate:mytemplate&secondopt:secondval;Account:1002&Destination:+40123456", -// expectedErr: utils.ErrNotFound.Error(), -// }, -// { -// name: "WrongNumberOfParams", -// extraParams: "tenant;;1;", -// }, -// { -// name: "InvalidMap", -// extraParams: "tenant;;1;opt:value;key", -// }, -// } +func (m *mockSessionSv1Obj) AlterSessions(_ *context.Context, params utils.SessionFilterWithEvent, reply *string) error { + m.request = utils.ToJSON(params) + return nil +} -// for _, tc := range testcases { -// t.Run(tc.name, func(t *testing.T) { -// action := &Action{ -// ExtraParameters: tc.extraParams, -// } -// err := alterSessionsAction(nil, action, nil, nil, nil) -// if tc.expectedErr != "" { -// if err == nil || err.Error() != tc.expectedErr { -// t.Errorf("expected error %v, received %v", tc.expectedErr, err) -// } -// return -// } -// if err != nil { -// t.Error(err) -// } -// }) -// } -// } +func TestActionsAlterSessions(t *testing.T) { + testcases := []struct { + name string + registerRpc bool + extraParams string + expectedRequest string + expectedErr string + }{ + { + name: "SuccessfulRequest", + registerRpc: true, + expectedRequest: `{"Limit":1,"Filters":["*string:~*req.Account:1001","*prefix:~*req.Destination:+40"],"Tenant":"tenant.com","APIOpts":{"*radCoATemplate":"mytemplate","secondopt":"secondval"},"Event":{"Account":"1002","Destination":"+40123456"}}`, + extraParams: "*internal;;tenant.com;*string:~*req.Account:1001&*prefix:~*req.Destination:+40;1;*radCoATemplate:mytemplate&secondopt:secondval;Account:1002&Destination:+40123456", + }, + { + name: "FailedServiceRetrieval", + extraParams: "*internal;;tenant.com;*string:~*req.Account:1001&*prefix:~*req.Destination:+40;1;*radCoATemplate:mytemplate&secondopt:secondval;Account:1002&Destination:+40123456", + expectedErr: "retrieving service for *internal calls failed: NOT_FOUND", + }, + { + name: "FailedExternalConnSetup", + extraParams: "localhost:1234;*json;tenant.com;*string:~*req.Account:1001&*prefix:~*req.Destination:+40;1;*radCoATemplate:mytemplate&secondopt:secondval;Account:1002&Destination:+40123456", + expectedErr: "failed to init RPCClient: dial tcp [::1]:1234: connect: connection refused", + }, + { + name: "WrongNumberOfParams", + extraParams: "*internal;;tenant;;1;", + expectedErr: "invalid number of parameters; expected 7", + }, + { + name: "InvalidEventMap", + registerRpc: true, + extraParams: "*internal;;tenant;;1;opt:value;key", + expectedErr: "invalid key-value pair: key", + }, + { + name: "InvalidOptsMap", + registerRpc: true, + extraParams: "*internal;;tenant;;1;opt;key:value", + expectedErr: "invalid key-value pair: opt", + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + action := &Action{ExtraParameters: tc.extraParams} + var mockObj mockSessionSv1Obj + if tc.registerRpc { + utils.RegisterRpcParams(utils.SessionSv1, &mockObj) + t.Cleanup(func() { + utils.UnregisterRpcParams(utils.SessionSv1) + }) + } + err := alterSessionsAction(nil, action, nil, nil, nil) + if tc.expectedErr != "" { + if err == nil || err.Error() != tc.expectedErr { + t.Errorf("expected error %v, received %v", tc.expectedErr, err) + } + } else if err != nil { + t.Error(err) + } else if tc.registerRpc && mockObj.request != tc.expectedRequest { + t.Errorf("expected: %v\nreceived: %v", tc.expectedRequest, mockObj.request) + } + }) + } +} diff --git a/sessions/sessions_birpc_it_test.go b/sessions/sessions_birpc_it_test.go index f86e5be65..f8aec291e 100644 --- a/sessions/sessions_birpc_it_test.go +++ b/sessions/sessions_birpc_it_test.go @@ -113,7 +113,7 @@ func testSessionsBiRPCStartEngine(t *testing.T) { // Connect rpc client to rater func testSessionsBiRPCApierRpcConn(t *testing.T) { - srv, err := birpc.NewService(new(smock), utils.SessionSv1, true) + srv, err := birpc.NewService(new(smock), utils.AgentV1, true) if err != nil { t.Fatal(err) } diff --git a/utils/rpc_params.go b/utils/rpc_params.go index 832e83211..a11078ad0 100644 --- a/utils/rpc_params.go +++ b/utils/rpc_params.go @@ -21,6 +21,7 @@ package utils import ( "fmt" "reflect" + "strings" "sync" "github.com/cgrates/birpc" @@ -86,3 +87,14 @@ func GetRpcParams(method string) (params *RpcParams, err error) { } return } + +func UnregisterRpcParams(name string) { + rpcParamsLock.Lock() + defer rpcParamsLock.Unlock() + for method := range rpcParamsMap { + if strings.HasPrefix(method, name) { + delete(rpcParamsMap, method) + } + } + delete(rpcParamsMap, name) +}