From 5eac3e86accd1fd309bc4d5af39594e12c1e76b2 Mon Sep 17 00:00:00 2001
From: Felix Kunde <felix-kunde@gmx.de>
Date: Wed, 21 Apr 2021 17:16:43 +0200
Subject: [PATCH] reflect code review

---
 pkg/cluster/cluster.go  |  5 ++---
 pkg/cluster/sync.go     |  7 +++----
 pkg/util/users/users.go | 20 ++++++--------------
 3 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go
index 75a906924..388e637a7 100644
--- a/pkg/cluster/cluster.go
+++ b/pkg/cluster/cluster.go
@@ -646,11 +646,10 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 		c.logger.Debugf("initialize users")
 		// save current state of pgUsers to check for deleted roles later
 		if len(c.pgUsers) > 0 {
-			usersCache := map[string]spec.PgUser{}
+			c.pgUsersCache = map[string]spec.PgUser{}
 			for k, v := range c.pgUsers {
-				usersCache[k] = v
+				c.pgUsersCache[k] = v
 			}
-			c.pgUsersCache = usersCache
 		}
 		if err := c.initUsers(); err != nil {
 			c.logger.Errorf("could not init users: %v", err)
diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go
index 3747600c4..257c662ea 100644
--- a/pkg/cluster/sync.go
+++ b/pkg/cluster/sync.go
@@ -39,11 +39,10 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
 
 	// save current state of pgUsers to check for deleted roles later
 	if len(c.pgUsers) > 0 {
-		usersCache := map[string]spec.PgUser{}
+		c.pgUsersCache = map[string]spec.PgUser{}
 		for k, v := range c.pgUsers {
-			usersCache[k] = v
+			c.pgUsersCache[k] = v
 		}
-		c.pgUsersCache = usersCache
 	}
 	if err = c.initUsers(); err != nil {
 		err = fmt.Errorf("could not init users: %v", err)
@@ -586,7 +585,7 @@ func (c *Cluster) syncRoles() (err error) {
 
 	for _, u := range c.pgUsers {
 		userNames = append(userNames, u.Name)
-		// add team user name with rename suffix in case we need to rename it back
+		// add team member role name with rename suffix in case we need to rename it back
 		if u.Origin == spec.RoleOriginTeamsAPI {
 			userNames = append(userNames, u.Name+constants.RoleRenameSuffix)
 		}
diff --git a/pkg/util/users/users.go b/pkg/util/users/users.go
index df3f5820b..461a45567 100644
--- a/pkg/util/users/users.go
+++ b/pkg/util/users/users.go
@@ -72,8 +72,8 @@ func (strategy DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserM
 		}
 	}
 
-	// No existing roles are deleted or stripped of role memebership/flags
-	// but they will be renamed acting as a simple blocker
+	// No existing roles are deleted or stripped of role membership/flags
+	// but they will be renamed acting as a simple login blocker
 	for name, dbUser := range dbUsers {
 		if _, exists := newUsers[name]; !exists {
 			reqs = append(reqs, spec.PgSyncUserRequest{Kind: spec.PGSyncUserRename, User: dbUser})
@@ -141,18 +141,10 @@ func (strategy DefaultUserSyncStrategy) alterPgUserSet(user spec.PgUser, db *sql
 func (strategy DefaultUserSyncStrategy) alterPgUserRename(user spec.PgUser, db *sql.DB) error {
 	var query string
 
-	renamedBack := false
-
-	nameSuffixDiff := len(user.Name) - len(constants.RoleRenameSuffix)
-
-	if nameSuffixDiff > 0 {
-		if user.Name[nameSuffixDiff:] == constants.RoleRenameSuffix {
-			query = fmt.Sprintf(alterUserRenameSQL, user.Name, user.Name[:nameSuffixDiff], "")
-			renamedBack = true
-		}
-	}
-
-	if !renamedBack {
+	if strings.HasSuffix(user.Name, constants.RoleRenameSuffix) {
+		newName := strings.TrimSuffix(user.Name, constants.RoleRenameSuffix)
+		query = fmt.Sprintf(alterUserRenameSQL, user.Name, newName, "")
+	} else {
 		query = fmt.Sprintf(alterUserRenameSQL, user.Name, user.Name, constants.RoleRenameSuffix)
 	}