Fix *remove_account action

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

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

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
 }
 

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 <http://www.gnu.org/licenses/>
+*/
+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)
+	}
+}