Merge branch 'master' of https://github.com/cgrates/cgrates

2026-02-21 07:08:45 +05:00 · 2017-01-17 12:44:05 +02:00
parent 52a3f510de a643c31567
commit 92004da949
8 changed files with 331 additions and 163 deletions
--- a/apier/v1/accounts.go
+++ b/apier/v1/accounts.go
@@ -24,7 +24,6 @@ import (
 	"strings"
 	"time"

-	"github.com/cgrates/cgrates/cache"
 	"github.com/cgrates/cgrates/engine"
 	"github.com/cgrates/cgrates/guardian"
 	"github.com/cgrates/cgrates/utils"
@@ -46,27 +45,29 @@ func (self *ApierV1) GetAccountActionPlan(attrs AttrAcntAction, reply *[]*Accoun
 	if missing := utils.MissingStructFields(&attrs, []string{"Tenant", "Account"}); len(missing) != 0 {
 		return utils.NewErrMandatoryIeMissing(strings.Join(missing, ","), "")
 	}
-	accountATs := make([]*AccountActionTiming, 0) // needs to be initialized if remains empty
-	allAPs, err := self.RatingDb.GetAllActionPlans()
-	if err != nil {
+	acntID := utils.AccountKey(attrs.Tenant, attrs.Account)
+	acntAPids, err := self.RatingDb.GetAccountActionPlans(acntID, false, utils.NonTransactional)
+	if err != nil && err != utils.ErrNotFound {
 		return utils.NewErrServerError(err)
 	}
-	accID := utils.AccountKey(attrs.Tenant, attrs.Account)
-	for _, ap := range allAPs {
-		if ap == nil {
-			continue
+	var acntAPs []*engine.ActionPlan
+	for _, apID := range acntAPids {
+		if ap, err := self.RatingDb.GetActionPlan(apID, false, utils.NonTransactional); err != nil {
+			return err
+		} else if ap != nil {
+			acntAPs = append(acntAPs, ap)
 		}
-		if _, exists := ap.AccountIDs[accID]; exists {
-			for _, at := range ap.ActionTimings {
-				accountATs = append(accountATs, &AccountActionTiming{
-					ActionPlanId: ap.Id,
-					Uuid:         at.Uuid,
-					ActionsId:    at.ActionsID,
-					NextExecTime: at.GetNextStartTime(time.Now()),
-				})
-			}
+	}
+	accountATs := make([]*AccountActionTiming, 0) // needs to be initialized if remains empty
+	for _, ap := range acntAPs {
+		for _, at := range ap.ActionTimings {
+			accountATs = append(accountATs, &AccountActionTiming{
+				ActionPlanId: ap.Id,
+				Uuid:         at.Uuid,
+				ActionsId:    at.ActionsID,
+				NextExecTime: at.GetNextStartTime(time.Now()),
+			})
 		}
-
 	}
 	*reply = accountATs
 	return nil
@@ -133,7 +134,7 @@ func (self *ApierV1) RemActionTiming(attrs AttrRemActionTiming, reply *string) (
 		return 0, nil
 	}, 0, utils.ACTION_PLAN_PREFIX)
 	if accID != "" && attrs.ActionTimingId != "" { // Rebuild index for accounts pointing towards ActionPlans
-		if err = self.RatingDb.RemAccountActionPlans(accID, []string{attrs.ActionTimingId}); err != nil {
+		if err = self.RatingDb.RemAccountActionPlans(accID, []string{attrs.ActionPlanId}); err != nil {
 			return
 		}
 		if err = self.RatingDb.CacheDataFromDB(utils.AccountActionPlansPrefix, []string{accID}, true); err != nil {
@@ -172,7 +173,6 @@ func (self *ApierV1) SetAccount(attr utils.AttrSetAccount, reply *string) (err e
 			}
 		}
 		if len(attr.ActionPlanId) != 0 {
-
 			_, err := guardian.Guardian.Guard(func() (interface{}, error) {
 				var ap *engine.ActionPlan
 				ap, err := self.RatingDb.GetActionPlan(attr.ActionPlanId, false, utils.NonTransactional)
@@ -203,25 +203,31 @@ func (self *ApierV1) SetAccount(attr utils.AttrSetAccount, reply *string) (err e
 					}
 				}
 				// clean previous action plans
-				actionPlansMap, err := self.RatingDb.GetAllActionPlans()
-				if err != nil {
-					if err == utils.ErrNotFound { // if no action plans just continue
-						return 0, nil
-					}
+				acntAPids, err := self.RatingDb.GetAccountActionPlans(accID, false, utils.NonTransactional)
+				if err != nil && err != utils.ErrNotFound {
 					return 0, err
 				}
-				for actionPlanID, ap := range actionPlansMap {
-					if actionPlanID == attr.ActionPlanId {
-						// don't remove it if it's the current one
-						continue
-					}
-					if _, exists := ap.AccountIDs[accID]; exists {
+				for _, apID := range acntAPids {
+					if apID != attr.ActionPlanId {
+						ap, err := self.RatingDb.GetActionPlan(apID, false, utils.NonTransactional)
+						if err != nil {
+							return 0, err
+						}
 						delete(ap.AccountIDs, accID)
-						// clean from cache
-						cache.RemKey(utils.ACTION_PLAN_PREFIX+actionPlanID, true, utils.NonTransactional)
+						if err = self.RatingDb.SetActionPlan(apID, ap, true, utils.NonTransactional); err != nil {
+							return 0, err
+						}
+						if err = self.RatingDb.CacheDataFromDB(utils.ACTION_PLAN_PREFIX, []string{ap.Id}, true); err != nil {
+							return 0, err
+						}
 					}
 				}
-
+				if err = self.RatingDb.SetAccountActionPlans(accID, []string{attr.ActionPlanId}, false); err != nil {
+					return 0, err
+				}
+				if err = self.RatingDb.CacheDataFromDB(utils.AccountActionPlansPrefix, []string{accID}, true); err != nil {
+					return 0, err
+				}
 				return 0, nil
 			}, 0, utils.ACTION_PLAN_PREFIX)
 			if err != nil {
--- a/apier/v1/apier.go
+++ b/apier/v1/apier.go
@@ -116,6 +116,15 @@ func (v1 *ApierV1) ComputeReverseAliases(ignr string, reply *string) (err error)
 	return
 }

+// ComputeReverseAliases will rebuild complete reverse aliases data
+func (v1 *ApierV1) ComputeAccountActionPlans(ignr string, reply *string) (err error) {
+	if err = v1.RatingDb.RebuildReverseForPrefix(utils.AccountActionPlansPrefix); err != nil {
+		return
+	}
+	*reply = utils.OK
+	return
+}
+
 func (apier *ApierV1) GetSharedGroup(sgId string, reply *engine.SharedGroup) error {
 	if sg, err := apier.RatingDb.GetSharedGroup(sgId, false, utils.NonTransactional); err != nil && err != utils.ErrNotFound { // Not found is not an error here
 		return err
@@ -667,12 +676,12 @@ func (self *ApierV1) SetActionPlan(attrs AttrSetActionPlan, reply *string) (err
 }

 type AttrGetActionPlan struct {
-	Id string
+	ID string
 }

 func (self *ApierV1) GetActionPlan(attr AttrGetActionPlan, reply *[]*engine.ActionPlan) error {
 	var result []*engine.ActionPlan
-	if attr.Id == "" || attr.Id == "*" {
+	if attr.ID == "" || attr.ID == "*" {
 		aplsMap, err := self.RatingDb.GetAllActionPlans()
 		if err != nil {
 			return err
@@ -681,7 +690,7 @@ func (self *ApierV1) GetActionPlan(attr AttrGetActionPlan, reply *[]*engine.Acti
 			result = append(result, apls)
 		}
 	} else {
-		apls, err := self.RatingDb.GetActionPlan(attr.Id, false, utils.NonTransactional)
+		apls, err := self.RatingDb.GetActionPlan(attr.ID, false, utils.NonTransactional)
 		if err != nil {
 			return err
 		}
--- a/apier/v1/apier_it_test.go
+++ b/apier/v1/apier_it_test.go
@@ -1222,6 +1222,11 @@ func TestApierComputeReverse(t *testing.T) {
 	} else if reply != utils.OK {
 		t.Error("Received: ", reply)
 	}
+	if err := rater.Call("ApierV1.ComputeAccountActionPlans", "", &reply); err != nil {
+		t.Error(err)
+	} else if reply != utils.OK {
+		t.Error("Received: ", reply)
+	}
 }

 func TestApierResetDataAfterLoadFromFolder(t *testing.T) {
--- a/apier/v2/accounts.go
+++ b/apier/v2/accounts.go
@@ -114,45 +114,51 @@ func (self *ApierV2) SetAccount(attr AttrSetAccount, reply *string) error {
 		}
 		if attr.ActionPlanIDs != nil {
 			_, err := guardian.Guardian.Guard(func() (interface{}, error) {
-				actionPlansMap, err := self.RatingDb.GetAllActionPlans()
-				if err != nil {
-					if err == utils.ErrNotFound { // if no action plans just continue
-						return 0, nil
-					}
+				acntAPids, err := self.RatingDb.GetAccountActionPlans(accID, false, utils.NonTransactional)
+				if err != nil && err != utils.ErrNotFound {
 					return 0, err
 				}
 				if attr.ActionPlansOverwrite {
 					// clean previous action plans
-					for actionPlanID, ap := range actionPlansMap {
-						if _, exists := ap.AccountIDs[accID]; exists {
-							delete(ap.AccountIDs, accID)
-							dirtyActionPlans[actionPlanID] = ap
+					for i := 0; i < len(acntAPids); {
+						apID := acntAPids[i]
+						if utils.IsSliceMember(*attr.ActionPlanIDs, apID) {
+							i++      // increase index since we don't remove from slice
+							continue // not removing the ones where
 						}
+						ap, err := self.RatingDb.GetActionPlan(apID, false, utils.NonTransactional)
+						if err != nil {
+							return 0, err
+						}
+						delete(ap.AccountIDs, accID)
+						dirtyActionPlans[apID] = ap
+						acntAPids = append(acntAPids[:i], acntAPids[i+1:]...) // remove the item from the list so we can overwrite the real list
 					}
 				}
-				for _, actionPlanID := range *attr.ActionPlanIDs {
-					ap, ok := actionPlansMap[actionPlanID]
-					if !ok {
-						return 0, utils.ErrNotFound
+				for _, apID := range *attr.ActionPlanIDs {
+					if utils.IsSliceMember(acntAPids, apID) {
+						continue // Already there
 					}
-
-					if _, exists := ap.AccountIDs[accID]; !exists {
-						if ap.AccountIDs == nil {
-							ap.AccountIDs = make(utils.StringMap)
-						}
-						ap.AccountIDs[accID] = true
-						dirtyActionPlans[actionPlanID] = ap
-						// create tasks
-						for _, at := range ap.ActionTimings {
-							if at.IsASAP() {
-								t := &engine.Task{
-									Uuid:      utils.GenUUID(),
-									AccountID: accID,
-									ActionsID: at.ActionsID,
-								}
-								if err = self.RatingDb.PushTask(t); err != nil {
-									return 0, err
-								}
+					ap, err := self.RatingDb.GetActionPlan(apID, false, utils.NonTransactional)
+					if err != nil {
+						return 0, err
+					}
+					if ap.AccountIDs == nil {
+						ap.AccountIDs = make(utils.StringMap)
+					}
+					ap.AccountIDs[accID] = true
+					dirtyActionPlans[apID] = ap
+					acntAPids = append(acntAPids, apID)
+					// create tasks
+					for _, at := range ap.ActionTimings {
+						if at.IsASAP() {
+							t := &engine.Task{
+								Uuid:      utils.GenUUID(),
+								AccountID: accID,
+								ActionsID: at.ActionsID,
+							}
+							if err = self.RatingDb.PushTask(t); err != nil {
+								return 0, err
 							}
 						}
 					}
@@ -169,17 +175,17 @@ func (self *ApierV2) SetAccount(attr AttrSetAccount, reply *string) error {
 				if err := self.RatingDb.CacheDataFromDB(utils.ACTION_PLAN_PREFIX, apIDs, true); err != nil {
 					return 0, err
 				}
+				if err := self.RatingDb.SetAccountActionPlans(accID, acntAPids, true); err != nil {
+					return 0, err
+				}
+				if err = self.RatingDb.CacheDataFromDB(utils.AccountActionPlansPrefix, []string{accID}, true); err != nil {
+					return 0, err
+				}
 				return 0, nil
 			}, 0, utils.ACTION_PLAN_PREFIX)
 			if err != nil {
 				return 0, err
 			}
-			if err = self.RatingDb.SetAccountActionPlans(accID, *attr.ActionPlanIDs, attr.ActionPlansOverwrite); err != nil {
-				return 0, err
-			}
-			if err = self.RatingDb.CacheDataFromDB(utils.AccountActionPlansPrefix, []string{accID}, true); err != nil {
-				return 0, err
-			}
 		}

 		if attr.ActionTriggerIDs != nil {
--- a/apier/v2/apierv2_it_test.go
+++ b/apier/v2/apierv2_it_test.go
@@ -21,9 +21,12 @@ package v2

 import (
 	"flag"
+	"fmt"
 	"net/rpc"
 	"net/rpc/jsonrpc"
 	"path"
+	"reflect"
+	"strconv"
 	"testing"

 	"github.com/cgrates/cgrates/apier/v1"
@@ -40,6 +43,7 @@ var (
 var apierCfgPath string
 var apierCfg *config.CGRConfig
 var apierRPC *rpc.Client
+var dataDB engine.DataDB // share db connection here so we can check data we set through APIs

 func TestApierV2itLoadConfig(t *testing.T) {
 	apierCfgPath = path.Join(*dataDir, "conf", "samples", "tutmysql")
@@ -62,6 +66,15 @@ func TestApierV2itResetStorDb(t *testing.T) {
 	}
 }

+func TestApierV2itConnectDataDB(t *testing.T) {
+	rdsDb, _ := strconv.Atoi(apierCfg.TpDbName)
+	if rdsITdb, err := engine.NewRedisStorage(fmt.Sprintf("%s:%s", apierCfg.TpDbHost, apierCfg.TpDbPort), rdsDb, apierCfg.TpDbPass, apierCfg.DBDataEncoding, utils.REDIS_MAX_CONNS, nil, 1); err != nil {
+		t.Fatal("Could not connect to Redis", err.Error())
+	} else {
+		dataDB = rdsITdb
+	}
+}
+
 // Start CGR Engine
 func TestApierV2itStartEngine(t *testing.T) {
 	if _, err := engine.StopStartEngine(apierCfgPath, 200); err != nil { // Mongo requires more time to start
@@ -183,6 +196,114 @@ func TestApierV2itFraudMitigation(t *testing.T) {
 	}
 }

+func TestApierV2itSetAccountWithAP(t *testing.T) {
+	argActs1 := utils.AttrSetActions{ActionsId: "TestApierV2itSetAccountWithAP_ACT_1",
+		Actions: []*utils.TPAction{
+			&utils.TPAction{Identifier: engine.TOPUP_RESET, BalanceType: utils.MONETARY, Directions: utils.OUT, Units: "5.0", Weight: 20.0},
+		}}
+	var reply string
+	if err := apierRPC.Call("ApierV2.SetActions", argActs1, &reply); err != nil {
+		t.Error(err)
+	}
+	argAP1 := &v1.AttrSetActionPlan{Id: "TestApierV2itSetAccountWithAP_AP_1",
+		ActionPlan: []*v1.AttrActionPlan{
+			&v1.AttrActionPlan{ActionsId: argActs1.ActionsId, Time: utils.ASAP, Weight: 20.0}}}
+	if _, err := dataDB.GetActionPlan(argAP1.Id, true, utils.NonTransactional); err == nil || err != utils.ErrNotFound {
+		t.Error(err)
+	}
+	if err := apierRPC.Call("ApierV1.SetActionPlan", argAP1, &reply); err != nil {
+		t.Error("Got error on ApierV1.SetActionPlan: ", err.Error())
+	} else if reply != utils.OK {
+		t.Errorf("Calling ApierV1.SetActionPlan received: %s", reply)
+	}
+	argSetAcnt1 := AttrSetAccount{
+		Tenant:        "cgrates.org",
+		Account:       "TestApierV2itSetAccountWithAP1",
+		ActionPlanIDs: &[]string{argAP1.Id},
+	}
+	acntID := utils.AccountKey(argSetAcnt1.Tenant, argSetAcnt1.Account)
+	if _, err := dataDB.GetAccountActionPlans(acntID, true, utils.NonTransactional); err == nil || err != utils.ErrNotFound {
+		t.Error(err)
+	}
+	if err := apierRPC.Call("ApierV2.SetAccount", argSetAcnt1, &reply); err != nil {
+		t.Fatal(err)
+	}
+	if ap, err := dataDB.GetActionPlan(argAP1.Id, true, utils.NonTransactional); err != nil {
+		t.Error(err)
+	} else if _, hasIt := ap.AccountIDs[acntID]; !hasIt {
+		t.Errorf("ActionPlan does not contain the accountID: %+v", ap)
+	}
+	eAAPids := []string{argAP1.Id}
+	if aapIDs, err := dataDB.GetAccountActionPlans(acntID, true, utils.NonTransactional); err != nil {
+		t.Error(err)
+	} else if !reflect.DeepEqual(eAAPids, aapIDs) {
+		t.Errorf("Expecting: %+v, received: %+v", eAAPids, aapIDs)
+	}
+	// Set second AP so we can see the proper indexing done
+	argAP2 := &v1.AttrSetActionPlan{Id: "TestApierV2itSetAccountWithAP_AP_2",
+		ActionPlan: []*v1.AttrActionPlan{
+			&v1.AttrActionPlan{ActionsId: argActs1.ActionsId, MonthDays: "1", Time: "00:00:00", Weight: 20.0}}}
+	if _, err := dataDB.GetActionPlan(argAP2.Id, true, utils.NonTransactional); err == nil || err != utils.ErrNotFound {
+		t.Error(err)
+	}
+	if err := apierRPC.Call("ApierV2.SetActionPlan", argAP2, &reply); err != nil {
+		t.Error("Got error on ApierV2.SetActionPlan: ", err.Error())
+	} else if reply != utils.OK {
+		t.Errorf("Calling ApierV2.SetActionPlan received: %s", reply)
+	}
+	// Test adding new AP
+	argSetAcnt2 := AttrSetAccount{
+		Tenant:        "cgrates.org",
+		Account:       "TestApierV2itSetAccountWithAP1",
+		ActionPlanIDs: &[]string{argAP2.Id},
+	}
+	if err := apierRPC.Call("ApierV2.SetAccount", argSetAcnt2, &reply); err != nil {
+		t.Fatal(err)
+	}
+	if ap, err := dataDB.GetActionPlan(argAP2.Id, true, utils.NonTransactional); err != nil {
+		t.Error(err)
+	} else if _, hasIt := ap.AccountIDs[acntID]; !hasIt {
+		t.Errorf("ActionPlan does not contain the accountID: %+v", ap)
+	}
+	if ap, err := dataDB.GetActionPlan(argAP1.Id, true, utils.NonTransactional); err != nil {
+		t.Error(err)
+	} else if _, hasIt := ap.AccountIDs[acntID]; !hasIt {
+		t.Errorf("ActionPlan does not contain the accountID: %+v", ap)
+	}
+	eAAPids = []string{argAP1.Id, argAP2.Id}
+	if aapIDs, err := dataDB.GetAccountActionPlans(acntID, true, utils.NonTransactional); err != nil {
+		t.Error(err)
+	} else if !reflect.DeepEqual(eAAPids, aapIDs) {
+		t.Errorf("Expecting: %+v, received: %+v", eAAPids, aapIDs)
+	}
+	// test remove and overwrite
+	argSetAcnt2 = AttrSetAccount{
+		Tenant:               "cgrates.org",
+		Account:              "TestApierV2itSetAccountWithAP1",
+		ActionPlanIDs:        &[]string{argAP2.Id},
+		ActionPlansOverwrite: true,
+	}
+	if err := apierRPC.Call("ApierV2.SetAccount", argSetAcnt2, &reply); err != nil {
+		t.Fatal(err)
+	}
+	if ap, err := dataDB.GetActionPlan(argAP1.Id, true, utils.NonTransactional); err != nil {
+		t.Error(err)
+	} else if _, hasIt := ap.AccountIDs[acntID]; hasIt {
+		t.Errorf("ActionPlan does contain the accountID: %+v", ap)
+	}
+	if ap, err := dataDB.GetActionPlan(argAP2.Id, true, utils.NonTransactional); err != nil {
+		t.Error(err)
+	} else if _, hasIt := ap.AccountIDs[acntID]; !hasIt {
+		t.Errorf("ActionPlan does not contain the accountID: %+v", ap)
+	}
+	eAAPids = []string{argAP2.Id}
+	if aapIDs, err := dataDB.GetAccountActionPlans(acntID, true, utils.NonTransactional); err != nil {
+		t.Error(err)
+	} else if !reflect.DeepEqual(eAAPids, aapIDs) {
+		t.Errorf("Expecting: %+v, received: %+v", eAAPids, aapIDs)
+	}
+}
+
 func TestApierV2itKillEngine(t *testing.T) {
 	if err := engine.KillEngine(delay); err != nil {
 		t.Error(err)
--- a/engine/onstor_it_test.go
+++ b/engine/onstor_it_test.go
@@ -77,7 +77,7 @@ var sTestsOnStorIT = []func(t *testing.T){
 	testOnStorITCRUDAccountActionPlans,
 	testOnStorITCRUDAccount,
 	testOnStorITCRUDCdrStatsQueue,
-	//FixMe	testOnStorITCRUDSubscribers,
+	testOnStorITCRUDSubscribers,
 	testOnStorITCRUDUser,
 	testOnStorITCRUDAlias,
 	testOnStorITCRUDReverseAlias,
@@ -926,36 +926,40 @@ func testOnStorITCRUDDestination(t *testing.T) {
 	} else if !reflect.DeepEqual(dst, rcv) {
 		t.Errorf("Expecting: %v, received: %v", dst, rcv)
 	}
-	//FixMe if err = onStor.RemoveDestination(dst.Id, utils.NonTransactional); err != nil {
-	// 	t.Error(err)
-	// }
-	// if _, rcvErr := onStor.GetDestination(dst.Id, true, utils.NonTransactional); rcvErr != utils.ErrNotFound {
-	// 	t.Error(rcvErr)
-	// }
+	if err = onStor.RemoveDestination(dst.Id, utils.NonTransactional); err != nil {
+		t.Error(err)
+	}
+	if _, rcvErr := onStor.GetDestination(dst.Id, true, utils.NonTransactional); rcvErr != utils.ErrNotFound {
+		t.Error(rcvErr)
+	}
 }

 func testOnStorITCRUDReverseDestination(t *testing.T) {
 	dst := &Destination{Id: "CRUDReverseDestination", Prefixes: []string{"+491", "+492", "+493"}}
 	dst2 := &Destination{Id: "CRUDReverseDestination2", Prefixes: []string{"+491", "+492", "+493"}}
-	//FixMe if _, rcvErr := onStor.GetReverseDestination(dst.Id, true, utils.NonTransactional); rcvErr != utils.ErrNotFound {
-	// 	t.Error(rcvErr) //<nil>
-	// }
+	if _, rcvErr := onStor.GetReverseDestination(dst.Id, true, utils.NonTransactional); rcvErr != utils.ErrNotFound {
+		t.Error(rcvErr)
+	}
 	if err := onStor.SetReverseDestination(dst, utils.NonTransactional); err != nil {
 		t.Error(err)
 	}
-	//FixMe if rcv, err := onStor.GetReverseDestination(dst.Id, true, utils.NonTransactional); err != nil {
-	// 	t.Error(err)
-	// } else if !reflect.DeepEqual([]string{dst.Id}, rcv) {
-	// 	t.Errorf("Expecting: %v, received: %v", dst, rcv) //Expecting: CRUDReverseDestination: +491, +492, +493, received: []
-	// }
+	/* FixMe @Edwardo22
+	if rcv, err := onStor.GetReverseDestination(dst.Id, true, utils.NonTransactional); err != nil {
+	 	t.Error(err)
+	 } else if !reflect.DeepEqual([]string{dst.Id}, rcv) {
+	 	t.Errorf("Expecting: %v, received: %v", dst, rcv) //Expecting: CRUDReverseDestination: +491, +492, +493, received: []
+	 }
+	*/
 	if err := onStor.UpdateReverseDestination(dst, dst2, utils.NonTransactional); err != nil {
 		t.Error(err)
 	}
-	//FixMe if rcv, err := onStor.GetReverseDestination(dst2.Id, true, utils.NonTransactional); err != nil {
-	// 	t.Error(err)
-	// } else if !reflect.DeepEqual([]string{dst2.Id}, rcv) {
-	// 	t.Errorf("Expecting: %v, received: %v", dst2, rcv) //Expecting: CRUDReverseDestination2: +491, +492, +493, received: []
-	// }
+	/* FixMe @Edwardo22
+	if rcv, err := onStor.GetReverseDestination(dst2.Id, true, utils.NonTransactional); err != nil {
+	 	t.Error(err)
+	 } else if !reflect.DeepEqual([]string{dst2.Id}, rcv) {
+	 	t.Errorf("Expecting: %v, received: %v", dst2, rcv) //Expecting: CRUDReverseDestination2: +491, +492, +493, received: []
+	}
+	*/
 }

 func testOnStorITCRUDLCR(t *testing.T) {
@@ -963,8 +967,8 @@ func testOnStorITCRUDLCR(t *testing.T) {
 		Tenant:    "cgrates.org",
 		Category:  "call",
 		Direction: "*out",
-		Account:   "*any",
-		Subject:   "*any",
+		Account:   "testOnStorITCRUDLCR",
+		Subject:   "testOnStorITCRUDLCR",
 		Activations: []*LCRActivation{
 			&LCRActivation{
 				ActivationTime: time.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC).Local(),
@@ -987,17 +991,18 @@ func testOnStorITCRUDLCR(t *testing.T) {
 			},
 		},
 	}
-	if _, rcvErr := onStor.GetLCR(utils.LCR_PREFIX+lcr.GetId(), true, utils.NonTransactional); rcvErr != utils.ErrNotFound {
+
+	if _, rcvErr := onStor.GetLCR(lcr.GetId(), true, utils.NonTransactional); rcvErr != utils.ErrNotFound {
 		t.Error(rcvErr)
 	}
 	if err := onStor.SetLCR(lcr, utils.NonTransactional); err != nil {
 		t.Error(err)
 	}
-	//FixMe  if rcv, err := onStor.GetLCR(lcr.GetId(), true, utils.NonTransactional); err != nil {
-	// 	t.Error(err)
-	// } else if !reflect.DeepEqual(lcr, rcv) {
-	// 	t.Errorf("Expecting: %v, received: %v", lcr, rcv)//rcv nil
-	// }
+	if rcv, err := onStor.GetLCR(lcr.GetId(), true, utils.NonTransactional); err != nil {
+		t.Error(err)
+	} else if !reflect.DeepEqual(lcr, rcv) {
+		t.Errorf("Expecting: %v, received: %v", lcr, rcv)
+	}
 }

 func testOnStorITCRUDCdrStats(t *testing.T) {
@@ -1212,20 +1217,20 @@ func testOnStorITCRUDAccountActionPlans(t *testing.T) {
 	} else if !reflect.DeepEqual(expect, rcv) {
 		t.Errorf("Expecting: %v, received: %v", expect, rcv)
 	}
-	// if err := onStor.RemAccountActionPlans(acntID, aAPs2); err != nil {
-	// 	t.Error(err)
-	// }
-	// if rcv, err := onStor.GetAccountActionPlans(acntID, true, utils.NonTransactional); err != nil {
-	// 	t.Error(err)
-	// } else if !reflect.DeepEqual(aAPs, rcv) {
-	// 	t.Errorf("Expecting: %v, received: %v", aAPs, rcv)
-	// }onstor_it_test.go:1238: Expecting: [PACKAGE_10_SHARED_A_5 apl_PACKAGE_1001], received: [PACKAGE_10_SHARED_A_5 USE_SHARED_A apl_PACKAGE_1001]
+	if err := onStor.RemAccountActionPlans(acntID, aAPs2); err != nil {
+		t.Error(err)
+	}
+	if rcv, err := onStor.GetAccountActionPlans(acntID, true, utils.NonTransactional); err != nil {
+		t.Error(err)
+	} else if !reflect.DeepEqual(aAPs, rcv) {
+		t.Errorf("Expecting: %v, received: %v", aAPs, rcv)
+	}
 	if err := onStor.RemAccountActionPlans(acntID, aAPs); err != nil {
 		t.Error(err)
 	}
-	// if _, rcvErr := onStor.GetAccountActionPlans(acntID, true, utils.NonTransactional); rcvErr != utils.ErrNotFound {
-	// 	t.Error(rcvErr)
-	// }
+	if _, rcvErr := onStor.GetAccountActionPlans(acntID, true, utils.NonTransactional); rcvErr != utils.ErrNotFound {
+		t.Error(rcvErr)
+	}
 }

 func testOnStorITCRUDAccount(t *testing.T) {
@@ -1259,7 +1264,12 @@ func testOnStorITCRUDAccount(t *testing.T) {
 func testOnStorITCRUDCdrStatsQueue(t *testing.T) {
 	sq := &StatsQueue{
 		conf: &CdrStats{Id: "TTT"},
-		Cdrs: []*QCdr{&QCdr{Cost: 9.0}},
+		Cdrs: []*QCdr{
+			&QCdr{Cost: 9.0,
+				SetupTime:  time.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC).Local(),
+				AnswerTime: time.Date(2012, 1, 1, 0, 0, 10, 0, time.UTC).Local(),
+				EventTime:  time.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC).Local(),
+			}},
 	}
 	if _, rcvErr := onStor.GetCdrStatsQueue(sq.GetId()); rcvErr != utils.ErrNotFound {
 		t.Error(rcvErr)
@@ -1269,41 +1279,38 @@ func testOnStorITCRUDCdrStatsQueue(t *testing.T) {
 	}
 	if rcv, err := onStor.GetCdrStatsQueue(sq.GetId()); err != nil {
 		t.Error(err)
-	} else if !reflect.DeepEqual(sq.Cdrs[0].Cost, rcv.Cdrs[0].Cost) {
-		t.Errorf("Expecting: %v, received: %v", sq.Cdrs[0].Cost, rcv.Cdrs[0].Cost)
+	} else if !reflect.DeepEqual(sq.Cdrs, rcv.Cdrs) {
+		t.Errorf("Expecting: %v, received: %v", sq.Cdrs, rcv.Cdrs)
 	}
-	// FixMe else if !reflect.DeepEqual(sq.conf.Id, rcv.conf.Id) {
-	// 	t.Errorf("Expecting: %v, received: %v", sq.conf.Id, rcv.conf.Id)
-	// }panic: runtime error: invalid memory address or nil pointer dereference [recovered]
-	// panic: runtime error: invalid memory address or nil pointer dereference
 }

-/*FixMe
 func testOnStorITCRUDSubscribers(t *testing.T) {
-	time, _ := time.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC).Local(),
-	rsr := utils.ParseRSRFieldsMustCompile("^*default", utils.INFIELD_SEP)
-	sub := &SubscriberData{time, rsr}
-	 if _, rcvErr := onStor.GetSubscribers(); rcvErr != utils.ErrNotFound {
-		t.Error(rcvErr) //<nil>
+	if sbs, err := onStor.GetSubscribers(); err != nil {
+		t.Error(err)
+	} else if len(sbs) != 0 {
+		t.Errorf("Received subscribers: %+v", sbs)
 	}
-	if err := onStor.SetSubscriber(utils.NonTransactional, sub); err != nil {
+	sbsc := &SubscriberData{
+		ExpTime: time.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC).Local(),
+		Filters: utils.ParseRSRFieldsMustCompile("^*default", utils.INFIELD_SEP)}
+	sbscID := "testOnStorITCRUDSubscribers"
+	if err := onStor.SetSubscriber(sbscID, sbsc); err != nil {
 		t.Error(err)
 	}
 	if rcv, err := onStor.GetSubscribers(); err != nil {
 		t.Error(err)
-	} else if !reflect.DeepEqual(sub.ExpTime, rcv[""].ExpTime) {
-		t.Errorf("Expecting: %v, received: %v", sub.ExpTime, rcv[""].ExpTime)
-	}  else if !reflect.DeepEqual(*sub.Filters[0], *rcv[""].Filters[0]) {
-		t.Errorf("Expecting: %v, received: %v", *sub.Filters[0], *rcv[""].Filters[0])//1321: Expecting: {*default [] *default []}, received: {*default []  []}
+	} else if !reflect.DeepEqual(sbsc.ExpTime, rcv[sbscID].ExpTime) { // Test just ExpTime since RSRField is more complex behind
+		t.Errorf("Expecting: %v, received: %v", sbsc, rcv[sbscID])
 	}
-	if err := onStor.RemoveSubscriber(utils.NonTransactional); err != nil {
+	if err := onStor.RemoveSubscriber(sbscID); err != nil {
 		t.Error(err)
 	}
-	if _, rcvErr := onStor.GetSubscribers(); rcvErr != utils.ErrNotFound {
-		t.Error(rcvErr)//<nil>
+	if sbs, err := onStor.GetSubscribers(); err != nil {
+		t.Error(err)
+	} else if len(sbs) != 0 {
+		t.Errorf("Received subscribers: %+v", sbs)
 	}
 }
-*/
 func testOnStorITCRUDUser(t *testing.T) {
 	usr := &UserProfile{
 		Tenant:   "test",
@@ -1378,12 +1385,12 @@ func testOnStorITCRUDAlias(t *testing.T) {
 	} else if !reflect.DeepEqual(als, rcv) {
 		t.Errorf("Expecting: %v, received: %v", als, rcv)
 	}
-	//FixMe if err := onStor.RemoveAlias(als.GetId(), utils.NonTransactional); err != nil {
-	// 	t.Error(err)
-	// }
-	// if _, rcvErr := onStor.GetAlias(als.GetId(), true, utils.NonTransactional); rcvErr != utils.ErrNotFound {
-	// 	t.Error(rcvErr)
-	// }
+	if err := onStor.RemoveAlias(als.GetId(), utils.NonTransactional); err != nil {
+		t.Error(err)
+	}
+	if _, rcvErr := onStor.GetAlias(als.GetId(), true, utils.NonTransactional); rcvErr != utils.ErrNotFound {
+		t.Error(rcvErr)
+	}
 }

 func testOnStorITCRUDReverseAlias(t *testing.T) {
--- a/engine/storage_mongo_datadb.go
+++ b/engine/storage_mongo_datadb.go
@@ -791,12 +791,13 @@ func (ms *MongoStorage) GetLCR(key string, skipCache bool, transactionID string)
 	cCommit := cacheCommit(transactionID)
 	if err = col.Find(bson.M{"key": key}).One(&result); err != nil {
 		if err == mgo.ErrNotFound {
-			cache.Set(cacheKey, nil, cacheCommit(transactionID), transactionID)
+			cache.Set(cacheKey, nil, cCommit, transactionID)
 			err = utils.ErrNotFound
 		}
 		return nil, err
 	}
-	cache.Set(cacheKey, result.Value, cCommit, transactionID)
+	lcr = result.Value
+	cache.Set(cacheKey, lcr, cCommit, transactionID)
 	return
 }

@@ -922,9 +923,12 @@ func (ms *MongoStorage) RemoveDestination(destID string, transactionID string) (
 	// get destination for prefix list
 	d, err := ms.GetDestination(destID, false, transactionID)
 	if err != nil {
+		if err == mgo.ErrNotFound {
+			err = nil
+		}
 		return
 	}
-	err = col.Remove(bson.M{"key": key})
+	err = col.Remove(bson.M{"key": destID})
 	if err != nil {
 		return err
 	}
@@ -1326,13 +1330,16 @@ func (ms *MongoStorage) RemoveAlias(key, transactionID string) (err error) {
 		Value AliasValues
 	}
 	session, col := ms.conn(colAls)
-	if err := col.Find(bson.M{"key": origKey}).One(&kv); err == nil {
-		al.Values = kv.Value
-	}
-	err = col.Remove(bson.M{"key": origKey})
-	if err != nil {
+	if err := col.Find(bson.M{"key": origKey}).One(&kv); err != nil {
+		if err == mgo.ErrNotFound {
+			err = nil
+		}
 		return err
 	}
+	al.Values = kv.Value
+	if err = col.Remove(bson.M{"key": origKey}); err != nil {
+		return
+	}
 	cCommit := cacheCommit(transactionID)
 	cache.RemKey(key, cCommit, transactionID)
 	session.Close()
@@ -1343,9 +1350,12 @@ func (ms *MongoStorage) RemoveAlias(key, transactionID string) (err error) {
 		for target, pairs := range value.Pairs {
 			for _, alias := range pairs {
 				rKey := alias + target + al.Context
-				err = col.Update(bson.M{"key": rKey}, bson.M{"$pull": bson.M{"value": tmpKey}})
-				if err != nil {
-					return err
+				if err = col.Update(bson.M{"key": rKey}, bson.M{"$pull": bson.M{"value": tmpKey}}); err != nil {
+					if err == mgo.ErrNotFound {
+						err = nil // cancel the error not to be propagated with return bellow
+					} else {
+						return err
+					}
 				}
 				cache.RemKey(utils.REVERSE_ALIASES_PREFIX+rKey, cCommit, transactionID)
 			}
@@ -1474,11 +1484,11 @@ func (ms *MongoStorage) SetActionTriggers(key string, atrs ActionTriggers, trans
 	session, col := ms.conn(colAtr)
 	defer session.Close()
 	if len(atrs) == 0 {
-		err = col.Remove(bson.M{"key": key}) // delete the key
-		if err != mgo.ErrNotFound {
-			return err
+		err = col.Remove(bson.M{"key": key})
+		if err == mgo.ErrNotFound { // Overwrite not found since it is not really mandatory here to be returned
+			err = nil
 		}
-		return nil
+		return
 	}
 	_, err = col.Upsert(bson.M{"key": key}, &struct {
 		Key   string
@@ -1654,7 +1664,7 @@ func (ms *MongoStorage) RemAccountActionPlans(acntID string, aPlIDs []string) (e
 	if len(aPlIDs) == 0 {
 		return col.Remove(bson.M{"key": acntID})
 	}
-	oldAPlIDs, err := ms.GetAccountActionPlans(acntID, false, utils.NonTransactional)
+	oldAPlIDs, err := ms.GetAccountActionPlans(acntID, true, utils.NonTransactional)
 	if err != nil {
 		return err
 	}
@@ -1671,7 +1681,7 @@ func (ms *MongoStorage) RemAccountActionPlans(acntID string, aPlIDs []string) (e
 	_, err = col.Upsert(bson.M{"key": acntID}, &struct {
 		Key   string
 		Value []string
-	}{Key: acntID, Value: aPlIDs})
+	}{Key: acntID, Value: oldAPlIDs})
 	return
 }

--- a/engine/storage_redis.go
+++ b/engine/storage_redis.go
@@ -557,10 +557,10 @@ func (rs *RedisStorage) GetReverseDestination(key string, skipCache bool, transa
 		}
 	}
 	if ids, err = rs.Cmd("SMEMBERS", key).List(); err != nil {
-		if err.Error() == "wrong type" { // did not find the destination
-			cache.Set(key, nil, cacheCommit(transactionID), transactionID)
-			err = utils.ErrNotFound
-		}
+		return
+	} else if len(ids) == 0 {
+		cache.Set(key, nil, cacheCommit(transactionID), transactionID)
+		err = utils.ErrNotFound
 		return
 	}
 	cache.Set(key, ids, cacheCommit(transactionID), transactionID)
@@ -1267,7 +1267,11 @@ func (rs *RedisStorage) RemAccountActionPlans(acntID string, aPlIDs []string) (e
 	if len(oldaPlIDs) == 0 { // no more elements, remove the reference
 		return rs.Cmd("DEL", key).Err
 	}
-	return rs.Cmd("SET", key, oldaPlIDs).Err
+	var result []byte
+	if result, err = rs.ms.Marshal(oldaPlIDs); err != nil {
+		return err
+	}
+	return rs.Cmd("SET", key, result).Err
 }

 func (rs *RedisStorage) PushTask(t *Task) error {