Skip to content

Commit

Permalink
Fix: Refactor Redis error handling and improve key deletion logic
Browse files Browse the repository at this point in the history
Removed redundant error checks and imports. Enhanced the key deletion logic by checking the result of deletion operations and only appending keys that were actually deleted. This refactoring simplifies the code and improves readability.

Signed-off-by: Christian Roessner <[email protected]>
  • Loading branch information
Christian Roessner committed Nov 25, 2024
1 parent 8747eca commit ade288c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 33 deletions.
7 changes: 0 additions & 7 deletions server/core/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package core

import (
stderrors "errors"
"fmt"
"net/http"

Expand All @@ -34,7 +33,6 @@ import (
"github.com/go-kit/log/level"
"github.com/pquerna/otp"
"github.com/pquerna/otp/totp"
"github.com/redis/go-redis/v9"
"github.com/spf13/viper"
"golang.org/x/text/cases"
"golang.org/x/text/language"
Expand Down Expand Up @@ -793,11 +791,6 @@ func registerTotpPOSTHandler(ctx *gin.Context) {
if _, err = rediscli.WriteHandle.Del(ctx, userKey).Result(); err != nil {
stats.RedisWriteCounter.Inc()

if stderrors.Is(err, redis.Nil) {

continue
}

level.Error(log.Logger).Log(global.LogKeyGUID, guid, global.LogKeyMsg, err)

break
Expand Down
46 changes: 20 additions & 26 deletions server/core/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ func processFlushCache(ctx *gin.Context, userCmd *FlushUserCmd, guid string) (re
// 5. Returns false.
func processUserCmd(ctx *gin.Context, userCmd *FlushUserCmd, guid string) (removedKeys []string, noUserAccountFound bool) {
var (
result int64
removeHash bool
accountName string
ipAddresses []string
Expand Down Expand Up @@ -499,28 +500,24 @@ func processUserCmd(ctx *gin.Context, userCmd *FlushUserCmd, guid string) (remov

// Remove PW_HIST_SET from Redis
key := getPWHistIPsRedisKey(accountName)
if err = rediscli.WriteHandle.Del(ctx, key).Err(); err != nil {
if !stderrors.Is(err, redis.Nil) {
level.Error(log.Logger).Log(global.LogKeyGUID, guid, global.LogKeyMsg, err)
} else {
err = nil
}
if result, err = rediscli.WriteHandle.Del(ctx, key).Result(); err != nil {
level.Error(log.Logger).Log(global.LogKeyGUID, guid, global.LogKeyMsg, err)
} else {
removedKeys = append(removedKeys, key)
if result > 0 {
removedKeys = append(removedKeys, key)
}
}

defer stats.RedisWriteCounter.Inc()

// Remove account from BLOCKED_ACCOUNTS
// Remove an account from AFFECTED_ACCOUNTS
key = config.LoadableConfig.Server.Redis.Prefix + global.RedisAffectedAccountsKey
if err = rediscli.WriteHandle.SRem(ctx, key, accountName).Err(); err != nil {
if !stderrors.Is(err, redis.Nil) {
level.Error(log.Logger).Log(global.LogKeyGUID, guid, global.LogKeyMsg, err)
} else {
err = nil
}
if result, err = rediscli.WriteHandle.SRem(ctx, key, accountName).Result(); err != nil {
level.Error(log.Logger).Log(global.LogKeyGUID, guid, global.LogKeyMsg, err)
} else {
removedKeys = append(removedKeys, key)
if result > 0 {
removedKeys = append(removedKeys, key)
}
}

removedKeys = append(removedKeys, removeUserFromCache(ctx, userCmd, userKeys, guid, removeHash)...)
Expand Down Expand Up @@ -591,7 +588,10 @@ func prepareRedisUserKeys(ctx context.Context, guid string, accountName string)
// If any error occurs during the removal process, it logs the error and immediately returns.
// After successful removal, it logs the keys that have been flushed.
func removeUserFromCache(ctx context.Context, userCmd *FlushUserCmd, userKeys config.StringSet, guid string, removeHash bool) []string {
var err error
var (
result int64
err error
)

removedKeys := make([]string, 0)

Expand All @@ -612,21 +612,15 @@ func removeUserFromCache(ctx context.Context, userCmd *FlushUserCmd, userKeys co
}

for _, userKey := range userKeys.GetStringSlice() {
if err = rediscli.WriteHandle.Del(ctx, userKey).Err(); err != nil {
if !stderrors.Is(err, redis.Nil) {
stats.RedisWriteCounter.Inc()

level.Error(log.Logger).Log(global.LogKeyGUID, guid, global.LogKeyMsg, err)
if result, err = rediscli.WriteHandle.Del(ctx, userKey).Result(); err != nil {
level.Error(log.Logger).Log(global.LogKeyGUID, guid, global.LogKeyMsg, err)

return removedKeys
}
return removedKeys
}

stats.RedisWriteCounter.Inc()

if err != nil {
level.Warn(log.Logger).Log(global.LogKeyGUID, guid, "keys", userKey, "status", "not found")
} else {
if result > 0 {
removedKeys = append(removedKeys, userKey)

level.Info(log.Logger).Log(global.LogKeyGUID, guid, "keys", userKey, "status", "flushed")
Expand Down

0 comments on commit ade288c

Please sign in to comment.