From 10592c246baf14e7459fb5a801114e6ef240c073 Mon Sep 17 00:00:00 2001 From: DanB Date: Mon, 23 Jan 2017 14:10:28 +0100 Subject: [PATCH] Fix *remove_account action --- engine/action.go | 34 ++++++++++++++------------- engine/actions_test.go | 37 +++++++++++++++++++---------- engine/storage_map.go | 27 +++++++++++---------- scheduler/scheduler_test.go | 47 +++++++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 42 deletions(-) create mode 100644 scheduler/scheduler_test.go diff --git a/engine/action.go b/engine/action.go index e4d83ba4f..1a1652c8c 100644 --- a/engine/action.go +++ b/engine/action.go @@ -561,28 +561,30 @@ func removeAccountAction(ub *Account, sq *StatsQueueTriggered, a *Action, acs Ac } _, err := guardian.Guardian.Guard(func() (interface{}, error) { - // clean the account id from all action plans - allAPs, err := ratingStorage.GetAllActionPlans() + acntAPids, err := ratingStorage.GetAccountActionPlans(accID, false, utils.NonTransactional) if err != nil && err != utils.ErrNotFound { utils.Logger.Err(fmt.Sprintf("Could not get action plans: %s: %v", accID, err)) return 0, err } - //var dirtyAps []string - aps := make([]string, len(allAPs)) - i := 0 - for key, ap := range allAPs { - - if _, exists := ap.AccountIDs[accID]; exists { - - // save action plan - delete(ap.AccountIDs, accID) - ratingStorage.SetActionPlan(key, ap, true, utils.NonTransactional) - //dirtyAps = append(dirtyAps, utils.ACTION_PLAN_PREFIX+key) + for _, apID := range acntAPids { + ap, err := ratingStorage.GetActionPlan(apID, false, utils.NonTransactional) + if err != nil { + utils.Logger.Err(fmt.Sprintf("Could not retrieve action plan: %s: %v", apID, err)) + return 0, err + } + delete(ap.AccountIDs, accID) + if err := ratingStorage.SetActionPlan(apID, ap, true, utils.NonTransactional); err != nil { + utils.Logger.Err(fmt.Sprintf("Could not save action plan: %s: %v", apID, err)) + return 0, err } - aps[i] = key - i++ } - if err = ratingStorage.CacheDataFromDB(utils.ACTION_PLAN_PREFIX, aps, true); err != nil { + if err = ratingStorage.CacheDataFromDB(utils.ACTION_PLAN_PREFIX, acntAPids, true); err != nil { + return 0, err + } + if err = ratingStorage.RemAccountActionPlans(accID, nil); err != nil { + return 0, err + } + if err = ratingStorage.CacheDataFromDB(utils.AccountActionPlansPrefix, []string{accID}, true); err != nil { return 0, err } return 0, nil diff --git a/engine/actions_test.go b/engine/actions_test.go index 5aa386ad9..543de3b0b 100644 --- a/engine/actions_test.go +++ b/engine/actions_test.go @@ -509,7 +509,7 @@ func TestActionPlansRemoveMember(t *testing.T) { accountingStorage.SetAccount(account2) ap1 := &ActionPlan{ - Id: "test", + Id: "TestActionPlansRemoveMember1", AccountIDs: utils.StringMap{"one": true}, ActionTimings: []*ActionTiming{ &ActionTiming{ @@ -550,16 +550,27 @@ func TestActionPlansRemoveMember(t *testing.T) { }, } - err := ratingStorage.SetActionPlan(ap1.Id, ap1, true, utils.NonTransactional) - - if err != nil { - t.Errorf("Set action plan test: %v", err) + if err := ratingStorage.SetActionPlan(ap1.Id, ap1, true, utils.NonTransactional); err != nil { + t.Error(err) } - - err = ratingStorage.SetActionPlan(ap2.Id, ap2, true, utils.NonTransactional) - - if err != nil { - t.Errorf("Set action plan test 2: %v", err) + if err = ratingStorage.SetActionPlan(ap2.Id, ap2, true, utils.NonTransactional); err != nil { + t.Error(err) + } + if err = ratingStorage.CacheDataFromDB(utils.ACTION_PLAN_PREFIX, []string{ap1.Id, ap2.Id}, true); err != nil { + t.Error(err) + } + if err = ratingStorage.SetAccountActionPlans(account1.ID, []string{ap1.Id}, false); err != nil { + t.Error(err) + } + if err = ratingStorage.CacheDataFromDB(utils.AccountActionPlansPrefix, []string{account1.ID}, true); err != nil { + t.Error(err) + } + ratingStorage.GetAccountActionPlans(account1.ID, true, utils.NonTransactional) // FixMe: remove here after finishing testing of map + if err = ratingStorage.SetAccountActionPlans(account2.ID, []string{ap2.Id}, false); err != nil { + t.Error(err) + } + if err = ratingStorage.CacheDataFromDB(utils.AccountActionPlansPrefix, []string{account2.ID}, false); err != nil { + t.Error(err) } actions := []*Action{ @@ -572,7 +583,7 @@ func TestActionPlansRemoveMember(t *testing.T) { ratingStorage.SetActions(actions[0].Id, actions, utils.NonTransactional) at := &ActionTiming{ - accountIDs: utils.StringMap{"one": true}, + accountIDs: utils.StringMap{account1.ID: true}, Timing: &RateInterval{}, actions: actions, } @@ -581,13 +592,13 @@ func TestActionPlansRemoveMember(t *testing.T) { t.Errorf("Execute Action: %v", err) } - apr, err1 := ratingStorage.GetActionPlan("test", false, utils.NonTransactional) + apr, err1 := ratingStorage.GetActionPlan(ap1.Id, false, utils.NonTransactional) if err1 != nil { t.Errorf("Get action plan test: %v", err1) } - if _, exist := apr.AccountIDs["one"]; exist { + if _, exist := apr.AccountIDs[account1.ID]; exist { t.Errorf("Account one is not deleted ") } diff --git a/engine/storage_map.go b/engine/storage_map.go index 015a3723c..3223f2ad5 100644 --- a/engine/storage_map.go +++ b/engine/storage_map.go @@ -1023,34 +1023,33 @@ func (ms *MapStorage) GetAllActionPlans() (ats map[string]*ActionPlan, err error } func (ms *MapStorage) GetAccountActionPlans(acntID string, skipCache bool, transactionID string) (apIDs []string, err error) { - ms.mu.RLock() - defer ms.mu.RUnlock() key := utils.AccountActionPlansPrefix + acntID - var values []byte if !skipCache { - if ap, ok := cache.Get(key); !ok { - return nil, utils.ErrNotFound - } else { - return ap.([]string), nil + if x, ok := cache.Get(key); ok { + if x == nil { + return nil, utils.ErrNotFound + } + return x.([]string), nil } } - if _, ok := ms.dict[key]; !ok { + ms.mu.RLock() + values, ok := ms.dict[key] + ms.mu.RUnlock() + if !ok { cache.Set(key, nil, cacheCommit(transactionID), transactionID) return nil, utils.ErrNotFound } - if err = ms.ms.Unmarshal(values, &apIDs); err != nil { return nil, err } - cache.Set(key, apIDs, cacheCommit(transactionID), transactionID) - return //apIDs, nil + return } func (ms *MapStorage) SetAccountActionPlans(acntID string, apIDs []string, overwrite bool) (err error) { if !overwrite { oldaPlIDs, err := ms.GetAccountActionPlans(acntID, true, utils.NonTransactional) - if err != nil { + if err != nil && err != utils.ErrNotFound { return err } else { for _, oldAPid := range oldaPlIDs { @@ -1063,9 +1062,11 @@ func (ms *MapStorage) SetAccountActionPlans(acntID string, apIDs []string, overw ms.mu.Lock() defer ms.mu.Unlock() - if _, err = ms.ms.Marshal(&apIDs); err != nil { + result, err := ms.ms.Marshal(apIDs) + if err != nil { return err } + ms.dict[utils.AccountActionPlansPrefix+acntID] = result return } diff --git a/scheduler/scheduler_test.go b/scheduler/scheduler_test.go new file mode 100644 index 000000000..66e1c3b61 --- /dev/null +++ b/scheduler/scheduler_test.go @@ -0,0 +1,47 @@ +/* +Real-time Online/Offline Charging System (OCS) for Telecom & ISP environments +Copyright (C) ITsysCOM GmbH + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see +*/ +package scheduler + +import ( + "testing" + "time" + + "github.com/cgrates/cgrates/engine" +) + +func TestSchedulerUpdateActStats(t *testing.T) { + sched := &Scheduler{actStatsInterval: 1 * time.Millisecond, actSuccessStats: make(map[string]map[time.Time]bool)} + sched.updateActStats(&engine.Action{Id: "REMOVE_1", ActionType: engine.REMOVE_ACCOUNT}, false) + if len(sched.actSuccessStats[engine.REMOVE_ACCOUNT]) != 1 { + t.Errorf("Wrong stats: %+v", sched.actSuccessStats[engine.REMOVE_ACCOUNT]) + } + sched.updateActStats(&engine.Action{Id: "REMOVE_2", ActionType: engine.REMOVE_ACCOUNT}, false) + if len(sched.actSuccessStats[engine.REMOVE_ACCOUNT]) != 2 { + t.Errorf("Wrong stats: %+v", sched.actSuccessStats[engine.REMOVE_ACCOUNT]) + } + sched.updateActStats(&engine.Action{Id: "LOG1", ActionType: engine.LOG}, false) + if len(sched.actSuccessStats[engine.LOG]) != 1 || + len(sched.actSuccessStats[engine.REMOVE_ACCOUNT]) != 2 { + t.Errorf("Wrong stats: %+v", sched.actSuccessStats) + } + time.Sleep(sched.actStatsInterval) + sched.updateActStats(&engine.Action{Id: "REMOVE_3", ActionType: engine.REMOVE_ACCOUNT}, false) + if len(sched.actSuccessStats[engine.REMOVE_ACCOUNT]) != 1 || len(sched.actSuccessStats) != 1 { + t.Errorf("Wrong stats: %+v", sched.actSuccessStats) + } +}