Update *alter_sessions action

Will now support two extra parameters: address and codec. For *internal
connections, the birpc.Service object will be retrieved from rpcParamsMap
from utils. Supported codecs: <*gob|*json|*http_jsonrpc> (ingored for
*internal address).

Action does not bother with setting defaults anymore, lets the API
handle them.

Improve action comments.

Add unit tests for the action. UnregisterRpcParams' implementation was
required.

Updated *alter_sessions action tariffplan for radius coa integration test
to include address and codec.

Some birpc clients that are set up in integration tests were still registering
a SessionSv1 object. Updated them to register an AgentV1 object instead.

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")
 	}
 }

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)
 	}

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)
 	}

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)
 	}

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
 	}

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,,,,,,,,,,,,,,

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
 	}
 

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)
+			}
+		})
+	}
+}

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)
 	}

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)
+}