diff --git a/apier/v1/accounts.go b/apier/v1/accounts.go index e16b50f46..687ac8480 100644 --- 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 { diff --git a/apier/v1/apier.go b/apier/v1/apier.go index 8d5bca27d..f48b54eb9 100644 --- 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 } diff --git a/apier/v1/apier_it_test.go b/apier/v1/apier_it_test.go index 4a679cf4f..e37c8674d 100644 --- 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) { diff --git a/apier/v2/accounts.go b/apier/v2/accounts.go index b3d356636..b5dc5b645 100644 --- 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 { diff --git a/apier/v2/apierv2_it_test.go b/apier/v2/apierv2_it_test.go index 6edc70a12..1b4a84c15 100644 --- 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) diff --git a/engine/onstor_it_test.go b/engine/onstor_it_test.go index df1f12d87..ddf38933d 100644 --- 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) // - // } + 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) // + 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)// + 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) { diff --git a/engine/storage_mongo_datadb.go b/engine/storage_mongo_datadb.go index 67e0c6271..2c271a3e8 100644 --- 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 } diff --git a/engine/storage_redis.go b/engine/storage_redis.go index fb56c6fc1..6ef04ada1 100644 --- 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 {