diff --git a/api/config.go b/api/config.go index 29055ae71..0b1671e15 100644 --- a/api/config.go +++ b/api/config.go @@ -71,6 +71,8 @@ const ( type LimitsConfig struct { // Trigger contains limits for triggers. Trigger TriggerLimits + // Trigger contains limits for teams. + Team TeamLimits } // TriggerLimits contains all limits applied for triggers. @@ -85,5 +87,22 @@ func GetTestLimitsConfig() LimitsConfig { Trigger: TriggerLimits{ MaxNameSize: DefaultTriggerNameMaxSize, }, + Team: TeamLimits{ + MaxNameSize: DefaultTeamNameMaxSize, + MaxDescriptionSize: DefaultTeamDescriptionMaxSize, + }, } } + +const ( + DefaultTeamNameMaxSize = 100 + DefaultTeamDescriptionMaxSize = 1000 +) + +// TeamLimits contains all limits applied for triggers. +type TeamLimits struct { + // MaxNameSize is the amount of characters allowed in team name. + MaxNameSize int + // MaxNameSize is the amount of characters allowed in team description. + MaxDescriptionSize int +} diff --git a/api/controller/team.go b/api/controller/team.go index 3c35ac0e2..5de8e9f6b 100644 --- a/api/controller/team.go +++ b/api/controller/team.go @@ -49,8 +49,13 @@ func CreateTeam(dataBase moira.Database, team dto.TeamModel, userID string) (dto return dto.SaveTeamResponse{}, api.ErrorInternalServer(fmt.Errorf("cannot generate unique id for team")) } } + err := dataBase.SaveTeam(teamID, team.ToMoiraTeam()) if err != nil { + if errors.Is(err, database.ErrTeamWithNameAlreadyExists) { + return dto.SaveTeamResponse{}, api.ErrorInvalidRequest(fmt.Errorf("cannot save team: %w", err)) + } + return dto.SaveTeamResponse{}, api.ErrorInternalServer(fmt.Errorf("cannot save team: %w", err)) } @@ -295,6 +300,10 @@ func AddTeamUsers(dataBase moira.Database, teamID string, newUsers []string) (dt func UpdateTeam(dataBase moira.Database, teamID string, team dto.TeamModel) (dto.SaveTeamResponse, *api.ErrorResponse) { err := dataBase.SaveTeam(teamID, team.ToMoiraTeam()) if err != nil { + if errors.Is(err, database.ErrTeamWithNameAlreadyExists) { + return dto.SaveTeamResponse{}, api.ErrorInvalidRequest(fmt.Errorf("cannot save team: %w", err)) + } + return dto.SaveTeamResponse{}, api.ErrorInternalServer(fmt.Errorf("cannot save team: %w", err)) } return dto.SaveTeamResponse{ID: teamID}, nil diff --git a/api/controller/team_test.go b/api/controller/team_test.go index 0eb937e4f..eda2eafeb 100644 --- a/api/controller/team_test.go +++ b/api/controller/team_test.go @@ -82,6 +82,14 @@ func TestCreateTeam(t *testing.T) { So(response, ShouldResemble, dto.SaveTeamResponse{}) So(err, ShouldResemble, api.ErrorInternalServer(fmt.Errorf("cannot save team: %w", returnErr))) }) + + Convey("team with name already exists error, while saving", func() { + dataBase.EXPECT().GetTeam(gomock.Any()).Return(moira.Team{}, database.ErrNil) + dataBase.EXPECT().SaveTeam(gomock.Any(), team.ToMoiraTeam()).Return(database.ErrTeamWithNameAlreadyExists) + response, err := CreateTeam(dataBase, team, user) + So(response, ShouldResemble, dto.SaveTeamResponse{}) + So(err, ShouldResemble, api.ErrorInvalidRequest(fmt.Errorf("cannot save team: %w", database.ErrTeamWithNameAlreadyExists))) + }) }) } diff --git a/api/dto/team.go b/api/dto/team.go index 9b08cb7ee..eafaf4aa8 100644 --- a/api/dto/team.go +++ b/api/dto/team.go @@ -1,17 +1,17 @@ package dto import ( + "errors" "fmt" "net/http" "unicode/utf8" + "github.com/moira-alert/moira/api/middleware" + "github.com/moira-alert/moira" ) -const ( - teamNameLimit = 100 - teamDescriptionLimit = 1000 -) +var errEmptyTeamName = errors.New("team name cannot be empty") // TeamModel is a structure that represents team entity in HTTP transfer. type TeamModel struct { @@ -31,15 +31,20 @@ func NewTeamModel(team moira.Team) TeamModel { // Bind is a method that implements Binder interface from chi and checks that validity of data in request. func (t TeamModel) Bind(request *http.Request) error { + limits := middleware.GetLimits(request) + if t.Name == "" { - return fmt.Errorf("team name cannot be empty") + return errEmptyTeamName } - if utf8.RuneCountInString(t.Name) > teamNameLimit { - return fmt.Errorf("team name cannot be longer than %d characters", teamNameLimit) + + if utf8.RuneCountInString(t.Name) > limits.Team.MaxNameSize { + return fmt.Errorf("team name cannot be longer than %d characters", limits.Team.MaxNameSize) } - if utf8.RuneCountInString(t.Description) > teamDescriptionLimit { - return fmt.Errorf("team description cannot be longer than %d characters", teamNameLimit) + + if utf8.RuneCountInString(t.Description) > limits.Team.MaxDescriptionSize { + return fmt.Errorf("team description cannot be longer than %d characters", limits.Team.MaxDescriptionSize) } + return nil } diff --git a/api/dto/team_test.go b/api/dto/team_test.go new file mode 100644 index 000000000..2886f6945 --- /dev/null +++ b/api/dto/team_test.go @@ -0,0 +1,57 @@ +package dto + +import ( + "fmt" + "net/http" + "strings" + "testing" + + "github.com/moira-alert/moira/api" + "github.com/moira-alert/moira/api/middleware" + + . "github.com/smartystreets/goconvey/convey" +) + +func TestTeamValidation(t *testing.T) { + Convey("Test team validation", t, func() { + teamModel := TeamModel{} + + limits := api.GetTestLimitsConfig() + + request, _ := http.NewRequest("POST", "/api/teams", nil) + request.Header.Set("Content-Type", "application/json") + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", limits)) + + Convey("with empty team.Name", func() { + err := teamModel.Bind(request) + + So(err, ShouldResemble, errEmptyTeamName) + }) + + Convey("with team.Name has characters more than in limit", func() { + teamModel.Name = strings.Repeat("ё", limits.Team.MaxNameSize+1) + + err := teamModel.Bind(request) + + So(err, ShouldResemble, fmt.Errorf("team name cannot be longer than %d characters", limits.Team.MaxNameSize)) + }) + + Convey("with team.Description has characters more than in limit", func() { + teamModel.Name = strings.Repeat("ё", limits.Team.MaxNameSize) + teamModel.Description = strings.Repeat("ё", limits.Team.MaxDescriptionSize+1) + + err := teamModel.Bind(request) + + So(err, ShouldResemble, fmt.Errorf("team description cannot be longer than %d characters", limits.Team.MaxDescriptionSize)) + }) + + Convey("with valid team", func() { + teamModel.Name = strings.Repeat("ё", limits.Team.MaxNameSize) + teamModel.Description = strings.Repeat("ё", limits.Team.MaxDescriptionSize) + + err := teamModel.Bind(request) + + So(err, ShouldBeNil) + }) + }) +} diff --git a/cmd/api/config.go b/cmd/api/config.go index b63770873..41b998b37 100644 --- a/cmd/api/config.go +++ b/cmd/api/config.go @@ -56,6 +56,8 @@ type apiConfig struct { type LimitsConfig struct { // Trigger contains the limits applied to triggers. Trigger TriggerLimitsConfig `yaml:"trigger"` + // Team contains the limits applied to teams. + Team TeamLimitsConfig `yaml:"team"` } // TriggerLimitsConfig represents the limits which will be applied to all triggers. @@ -64,12 +66,24 @@ type TriggerLimitsConfig struct { MaxNameSize int `yaml:"max_name_size"` } +// TeamLimitsConfig represents the limits which will be applied to all teams. +type TeamLimitsConfig struct { + // MaxNameSize is the max amount of characters allowed in team name. + MaxNameSize int `yaml:"max_name_size"` + // MaxDescriptionSize is the max amount of characters allowed in team description. + MaxDescriptionSize int `yaml:"max_description_size"` +} + // ToLimits converts LimitsConfig to api.LimitsConfig. func (conf LimitsConfig) ToLimits() api.LimitsConfig { return api.LimitsConfig{ Trigger: api.TriggerLimits{ MaxNameSize: conf.Trigger.MaxNameSize, }, + Team: api.TeamLimits{ + MaxNameSize: conf.Team.MaxNameSize, + MaxDescriptionSize: conf.Team.MaxDescriptionSize, + }, } } @@ -259,6 +273,10 @@ func getDefault() config { Trigger: TriggerLimitsConfig{ MaxNameSize: api.DefaultTriggerNameMaxSize, }, + Team: TeamLimitsConfig{ + MaxNameSize: api.DefaultTeamNameMaxSize, + MaxDescriptionSize: api.DefaultTeamDescriptionMaxSize, + }, }, }, Web: webConfig{ diff --git a/cmd/api/config_test.go b/cmd/api/config_test.go index 4d7f86a2b..2892ab5db 100644 --- a/cmd/api/config_test.go +++ b/cmd/api/config_test.go @@ -92,6 +92,10 @@ func Test_webConfig_getDefault(t *testing.T) { Trigger: TriggerLimitsConfig{ MaxNameSize: api.DefaultTriggerNameMaxSize, }, + Team: TeamLimitsConfig{ + MaxNameSize: api.DefaultTeamNameMaxSize, + MaxDescriptionSize: api.DefaultTeamDescriptionMaxSize, + }, }, }, Web: webConfig{ diff --git a/cmd/cli/from_2.13_to_2.14.go b/cmd/cli/from_2.13_to_2.14.go new file mode 100644 index 000000000..d4c739d90 --- /dev/null +++ b/cmd/cli/from_2.13_to_2.14.go @@ -0,0 +1,27 @@ +package main + +import "github.com/moira-alert/moira" + +func updateFrom213(logger moira.Logger, database moira.Database) error { + logger.Info().Msg("Update 2.13 -> 2.14 started") + + err := fillTeamNamesHash(logger, database) + if err != nil { + return err + } + + logger.Info().Msg("Update 2.13 -> 2.14 was finished") + return nil +} + +func downgradeTo213(logger moira.Logger, database moira.Database) error { + logger.Info().Msg("Downgrade 2.14 -> 2.13 started") + + err := removeTeamNamesHash(logger, database) + if err != nil { + return err + } + + logger.Info().Msg("Downgrade 2.14 -> 2.13 was finished") + return nil +} diff --git a/cmd/cli/main.go b/cmd/cli/main.go index e1b6bf899..2e1f91ec5 100644 --- a/cmd/cli/main.go +++ b/cmd/cli/main.go @@ -25,7 +25,15 @@ var ( GoVersion = "unknown" ) -var moiraValidVersions = []string{"2.3", "2.6", "2.7", "2.9", "2.11", "2.12"} +var moiraValidVersions = []string{ + "2.3", + "2.6", + "2.7", + "2.9", + "2.11", + "2.12", + "2.13", +} var ( configFileName = flag.String("config", "/etc/moira/cli.yml", "Path to configuration file") @@ -125,6 +133,13 @@ func main() { //nolint Error(err). Msg("Fail to update from version 2.12") } + case "2.13": + err := updateFrom213(logger, database) + if err != nil { + logger.Fatal(). + Error(err). + Msg("Fail to update from version 2.13") + } } } @@ -173,6 +188,13 @@ func main() { //nolint Error(err). Msg("Fail to update to version 2.12") } + case "2.13": + err := downgradeTo213(logger, database) + if err != nil { + logger.Fatal(). + Error(err). + Msg("Fail to update to version 2.13") + } } } diff --git a/cmd/cli/teams_names.go b/cmd/cli/teams_names.go new file mode 100644 index 000000000..a0cb27a4e --- /dev/null +++ b/cmd/cli/teams_names.go @@ -0,0 +1,219 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strconv" + "strings" + + goredis "github.com/go-redis/redis/v8" + "github.com/moira-alert/moira" + "github.com/moira-alert/moira/database/redis" +) + +const ( + teamsKey = "moira-teams" + teamsByNamesKey = "moira-teams-by-names" +) + +var errTeamsCountAndUniqueNamesCountMismatch = errors.New( + "count of teams does not match count of unique names after transformation") + +// fillTeamNamesHash does the following +// 1. Get all teams from DB. +// 2. Group teams with same names. +// 3. For teams with same names change name (example: ["name", "name", "name"] -> ["name", "name1", "name2"]). +// 4. Update teams with name changed. +// 5. Save pairs teamName:team.ID to "moira-teams-by-names" redis hash. +func fillTeamNamesHash(logger moira.Logger, database moira.Database) error { + logger.Info().Msg("Start filling \"moira-teams-by-names\" hash") + + switch db := database.(type) { + case *redis.DbConnector: + logger.Info().Msg("collecting teams from redis node...") + + teamsMap, err := db.Client().HGetAll(db.Context(), teamsKey).Result() + if err != nil { + return fmt.Errorf("failed to fetch teams from redis node: %w", err) + } + + logger.Info(). + Int("total_teams_count", len(teamsMap)). + Msg("fetched teams") + + teamsByNameMap, err := groupTeamsByNames(logger, teamsMap) + if err != nil { + return fmt.Errorf("failed to group teams by names: %w", err) + } + + teamByUniqueName := transformTeamsByNameMap(teamsByNameMap) + + if len(teamByUniqueName) != len(teamsMap) { + return errTeamsCountAndUniqueNamesCountMismatch + } + + client := db.Client() + ctx := db.Context() + + _, pipeErr := client.TxPipelined( + ctx, + func(pipe goredis.Pipeliner) error { + return updateTeamsInPipe(ctx, logger, pipe, teamByUniqueName) + }) + if pipeErr != nil { + return pipeErr + } + + default: + return makeUnknownDBError(database) + } + return nil +} + +func groupTeamsByNames(logger moira.Logger, teamsMap map[string]string) (map[string][]teamWithID, error) { + teamsByNameMap := make(map[string][]teamWithID, len(teamsMap)) + + for teamID, marshaledTeam := range teamsMap { + team, err := unmarshalTeam(teamID, []byte(marshaledTeam)) + if err != nil { + return nil, err + } + + lowercaseTeamName := strings.ToLower(team.Name) + + teamWithNameList, exists := teamsByNameMap[lowercaseTeamName] + if exists { + teamWithNameList = append(teamWithNameList, team) + teamsByNameMap[lowercaseTeamName] = teamWithNameList + } else { + teamsByNameMap[lowercaseTeamName] = []teamWithID{team} + } + } + + logger.Info(). + Int("unique_team_names_count", len(teamsByNameMap)). + Msg("grouped teams with same names") + + return teamsByNameMap, nil +} + +func transformTeamsByNameMap(teamsByNameMap map[string][]teamWithID) map[string]teamWithID { + teamByUniqueName := make(map[string]teamWithID, len(teamsByNameMap)) + + for _, teams := range teamsByNameMap { + for i, team := range teams { + iStr := strconv.FormatInt(int64(i), 10) + + if i > 0 { + team.Name += iStr + } + + for { + // sometimes we have the following situation in db (IDs and team names): + // moira-teams: { + // team1: "team name", + // team2: "team Name", + // team3: "Team name1" + // } + // so we can't just add 1 to one of [team1, team2] + lowercasedTeamName := strings.ToLower(team.Name) + + _, exists := teamByUniqueName[lowercasedTeamName] + if exists { + team.Name += "_" + iStr + } else { + teamByUniqueName[lowercasedTeamName] = team + break + } + } + } + } + + return teamByUniqueName +} + +func updateTeamsInPipe(ctx context.Context, logger moira.Logger, pipe goredis.Pipeliner, teamsByUniqueName map[string]teamWithID) error { + for _, team := range teamsByUniqueName { + teamBytes, err := getTeamBytes(team) + if err != nil { + return err + } + + err = pipe.HSet(ctx, teamsKey, team.ID, teamBytes).Err() + if err != nil { + logger.Error(). + Error(err). + String("team_id", team.ID). + String("new_team_name", team.Name). + Msg("failed to update team name") + + return fmt.Errorf("failed to update team name: %w", err) + } + + err = pipe.HSet(ctx, teamsByNamesKey, strings.ToLower(team.Name), team.ID).Err() + if err != nil { + logger.Error(). + Error(err). + String("team_id", team.ID). + String("new_team_name", team.Name). + Msg("failed to add team name to redis hash") + + return fmt.Errorf("failed to add team name to redis hash: %w", err) + } + } + + return nil +} + +// removeTeamNamesHash remove "moira-teams-by-names" redis hash. +// Note that if fillTeamNamesHash have been called, then team names would not be changed back. +func removeTeamNamesHash(logger moira.Logger, database moira.Database) error { + logger.Info().Msg("Start removing \"moira-teams-by-names\" redis hash") + + switch db := database.(type) { + case *redis.DbConnector: + _, err := db.Client().Del(db.Context(), teamsByNamesKey).Result() + if err != nil { + return fmt.Errorf("failed to delete teamsByNameKey: %w", err) + } + + default: + return makeUnknownDBError(database) + } + + return nil +} + +type teamStorageElement struct { + Name string `json:"name"` + Description string `json:"description"` +} + +type teamWithID struct { + teamStorageElement + ID string +} + +func unmarshalTeam(teamID string, teamBytes []byte) (teamWithID, error) { + var storedTeam teamStorageElement + err := json.Unmarshal(teamBytes, &storedTeam) + if err != nil { + return teamWithID{}, fmt.Errorf("failed to deserialize team: %w", err) + } + + return teamWithID{ + teamStorageElement: storedTeam, + ID: teamID, + }, nil +} + +func getTeamBytes(team teamWithID) ([]byte, error) { + bytes, err := json.Marshal(team.teamStorageElement) + if err != nil { + return nil, fmt.Errorf("failed to marshal team: %w", err) + } + + return bytes, nil +} diff --git a/cmd/cli/teams_names_test.go b/cmd/cli/teams_names_test.go new file mode 100644 index 000000000..e440ba498 --- /dev/null +++ b/cmd/cli/teams_names_test.go @@ -0,0 +1,255 @@ +package main + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/moira-alert/moira/database/redis" + logging "github.com/moira-alert/moira/logging/zerolog_adapter" + + . "github.com/smartystreets/goconvey/convey" +) + +var testTeams = []teamWithID{ + { + teamStorageElement: teamStorageElement{ + Name: "First team", + Description: "first desc", + }, + ID: "team1", + }, + { + teamStorageElement: teamStorageElement{ + Name: "Second team", + Description: "second desc", + }, + ID: "team2", + }, + { + teamStorageElement: teamStorageElement{ + Name: "Third team", + Description: "third desc", + }, + ID: "team3", + }, + { + teamStorageElement: teamStorageElement{ + Name: "Fourth team", + Description: "fourth desc", + }, + ID: "team4", + }, +} + +func Test_fillTeamNamesHash(t *testing.T) { + Convey("Test filling \"moira-teams-by-names\" redis hash", t, func() { + conf := getDefault() + logger, err := logging.ConfigureLog(conf.LogFile, conf.LogLevel, "test", conf.LogPrettyFormat) + if err != nil { + t.Fatal(err) + } + + db := redis.NewTestDatabase(logger) + db.Flush() + defer db.Flush() + + ctx := context.Background() + client := db.Client() + + Convey("with empty database", func() { + err = fillTeamNamesHash(logger, db) + So(err, ShouldBeNil) + + res, existErr := client.Exists(ctx, teamsByNamesKey).Result() + So(existErr, ShouldBeNil) + So(res, ShouldEqual, 0) + }) + + Convey("with teams which have unique names", func() { + defer db.Flush() + + var teamNames, actualTeamNames map[string]string + + teamNames = make(map[string]string, len(testTeams)) + + for _, team := range testTeams { + var teamBytes []byte + + teamBytes, err = getTeamBytes(team) + So(err, ShouldBeNil) + + err = client.HSet(ctx, teamsKey, team.ID, teamBytes).Err() + So(err, ShouldBeNil) + + teamNames[strings.ToLower(team.Name)] = team.ID + } + + err = fillTeamNamesHash(logger, db) + So(err, ShouldBeNil) + + actualTeamNames, err = client.HGetAll(ctx, teamsByNamesKey).Result() + So(err, ShouldBeNil) + So(actualTeamNames, ShouldResemble, teamNames) + }) + + Convey("with teams no unique names", func() { + defer db.Flush() + + testTeams[0].Name = "Team name" + testTeams[1].Name = "teaM name" + testTeams[2].Name = "Team name" + + for _, team := range testTeams { + var teamBytes []byte + + teamBytes, err = getTeamBytes(team) + So(err, ShouldBeNil) + + err = client.HSet(ctx, teamsKey, team.ID, teamBytes).Err() + So(err, ShouldBeNil) + } + + err = fillTeamNamesHash(logger, db) + So(err, ShouldBeNil) + + var actualTeamNames map[string]string + + actualTeamNames, err = client.HGetAll(ctx, teamsByNamesKey).Result() + So(err, ShouldBeNil) + So(actualTeamNames, ShouldHaveLength, len(testTeams)) + + expectedLowercasedTeamNames := []string{"team name", "team name1", "team name2", strings.ToLower(testTeams[3].Name)} + for _, name := range expectedLowercasedTeamNames { + _, ok := actualTeamNames[name] + So(ok, ShouldBeTrue) + } + + for i, team := range testTeams { + Convey(fmt.Sprintf("for team %v fields ok", i), func() { + var marshaledTeam string + + marshaledTeam, err = client.HGet(ctx, teamsKey, team.ID).Result() + So(err, ShouldBeNil) + + var actualTeam teamWithID + + actualTeam, err = unmarshalTeam(team.ID, []byte(marshaledTeam)) + So(err, ShouldBeNil) + So(actualTeam.ID, ShouldEqual, team.ID) + So(actualTeam.Description, ShouldEqual, team.Description) + + if i < 3 { + So(actualTeam.Name, ShouldBeIn, []string{team.Name, team.Name + "1", team.Name + "2"}) + } else { + So(actualTeam.Name, ShouldEqual, team.Name) + } + }) + } + }) + + Convey("with teams has no unique names and adding one number does not help", func() { + testTeams[0].Name = "Team name" + testTeams[1].Name = "teaM name" + testTeams[2].Name = "Team name1" + + for _, team := range testTeams { + var teamBytes []byte + + teamBytes, err = getTeamBytes(team) + So(err, ShouldBeNil) + + err = client.HSet(ctx, teamsKey, team.ID, teamBytes).Err() + So(err, ShouldBeNil) + } + + err = fillTeamNamesHash(logger, db) + So(err, ShouldBeNil) + + var actualTeamNames map[string]string + + actualTeamNames, err = client.HGetAll(ctx, teamsByNamesKey).Result() + So(err, ShouldBeNil) + So(actualTeamNames, ShouldHaveLength, len(testTeams)) + + // depends on order of map iteration + expectedLowercasedTeamNames := []string{"team name", "team name1", "team name1_0", "team name1_1", strings.ToLower(testTeams[3].Name)} + for name := range actualTeamNames { + So(name, ShouldBeIn, expectedLowercasedTeamNames) + } + + for i, team := range testTeams { + Convey(fmt.Sprintf("for team %v fields ok", i), func() { + var marshaledTeam string + + marshaledTeam, err = client.HGet(ctx, teamsKey, team.ID).Result() + So(err, ShouldBeNil) + + var actualTeam teamWithID + + actualTeam, err = unmarshalTeam(team.ID, []byte(marshaledTeam)) + So(err, ShouldBeNil) + So(actualTeam.ID, ShouldEqual, team.ID) + So(actualTeam.Description, ShouldEqual, team.Description) + + if i < 3 { + So(actualTeam.Name, ShouldBeIn, []string{team.Name, team.Name + "1", team.Name + "1_1", team.Name + "_0"}) + } else { + So(actualTeam.Name, ShouldEqual, team.Name) + } + }) + } + }) + }) +} + +func Test_removeTeamNamesHash(t *testing.T) { + Convey("Test removing \"moira-teams-by-names\" hash", t, func() { + conf := getDefault() + logger, err := logging.ConfigureLog(conf.LogFile, conf.LogLevel, "test", conf.LogPrettyFormat) + if err != nil { + t.Fatal(err) + } + + db := redis.NewTestDatabase(logger) + db.Flush() + defer db.Flush() + + ctx := context.Background() + client := db.Client() + + Convey("with empty database", func() { + err = removeTeamNamesHash(logger, db) + So(err, ShouldBeNil) + + res, existErr := client.Exists(ctx, teamsByNamesKey).Result() + So(existErr, ShouldBeNil) + So(res, ShouldEqual, 0) + }) + + Convey("with filled teams and teams by names hashes", func() { + defer db.Flush() + + for _, team := range testTeams { + var teamBytes []byte + + teamBytes, err = getTeamBytes(team) + So(err, ShouldBeNil) + + err = client.HSet(ctx, teamsKey, team.ID, teamBytes).Err() + So(err, ShouldBeNil) + + err = client.HSet(ctx, teamsByNamesKey, strings.ToLower(team.Name), team.ID).Err() + So(err, ShouldBeNil) + } + + err = removeTeamNamesHash(logger, db) + So(err, ShouldBeNil) + + res, existErr := client.Exists(ctx, teamsByNamesKey).Result() + So(existErr, ShouldBeNil) + So(res, ShouldEqual, 0) + }) + }) +} diff --git a/database/database.go b/database/database.go index 825dab56d..459e25b31 100644 --- a/database/database.go +++ b/database/database.go @@ -20,3 +20,6 @@ type ErrLockNotAcquired struct { func (e *ErrLockNotAcquired) Error() string { return fmt.Sprintf("lock was not acquired: %v", e.Err) } + +// ErrTeamWithNameAlreadyExists may be returned from SaveTeam method. +var ErrTeamWithNameAlreadyExists = fmt.Errorf("team with such name alredy exists") diff --git a/database/redis/database.go b/database/redis/database.go index 03cf0cd56..de7df50b3 100644 --- a/database/redis/database.go +++ b/database/redis/database.go @@ -33,6 +33,8 @@ const ( testSource DBSource = "test" ) +var _ moira.Database = &DbConnector{} + // DbConnector contains redis client. type DbConnector struct { client *redis.UniversalClient diff --git a/database/redis/teams.go b/database/redis/teams.go index be94b4c58..ce47a2230 100644 --- a/database/redis/teams.go +++ b/database/redis/teams.go @@ -1,26 +1,116 @@ package redis import ( + "errors" "fmt" + "strings" + "github.com/go-redis/redis/v8" "github.com/moira-alert/moira" + "github.com/moira-alert/moira/database" "github.com/moira-alert/moira/database/redis/reply" ) +const teamSaveAttempts = 3 + // SaveTeam saves team into redis. func (connector *DbConnector) SaveTeam(teamID string, team moira.Team) error { c := *connector.client teamBytes, err := reply.MarshallTeam(team) if err != nil { - return fmt.Errorf("save team error: %w", err) + return fmt.Errorf("failed to marshal team: %w", err) + } + + existedTeam, err := connector.GetTeam(teamID) + if err != nil && !errors.Is(err, database.ErrNil) { + return fmt.Errorf("failed to get team: %w", err) } + for i := 0; i < teamSaveAttempts; i++ { + // need to use watch here because if team name is updated + // we also need to change name in moira-teams-names set + err = c.Watch( + connector.context, + func(tx *redis.Tx) error { + return connector.saveTeamNameInTx(tx, teamID, team.Name, existedTeam.Name) + }, + teamsByNamesKey) + + if err == nil { + break + } + + if !errors.Is(err, redis.TxFailedErr) { + return err + } + } + + // save team err = c.HSet(connector.context, teamsKey, teamID, teamBytes).Err() if err != nil { - return fmt.Errorf("save team redis error: %w", err) + return fmt.Errorf("failed to save team metadata: %w", err) } - return nil + + return err +} + +func (connector *DbConnector) saveTeamNameInTx( + tx *redis.Tx, + teamID string, + newTeamName string, + existedTeamName string, +) error { + teamWithSuchNameID, err := connector.getTeamIDByNameInTx(tx, newTeamName) + if err != nil && !errors.Is(err, database.ErrNil) { + return err + } + + // team with such id does not exist but another team with such name exists + if teamWithSuchNameID != "" && teamWithSuchNameID != teamID { + return database.ErrTeamWithNameAlreadyExists + } + + newTeamLowercaseName := strings.ToLower(newTeamName) + existedTeamLowercaseName := strings.ToLower(existedTeamName) + + _, err = tx.TxPipelined( + connector.context, + func(pipe redis.Pipeliner) error { + updateTeamName := existedTeamName != "" && existedTeamLowercaseName != newTeamLowercaseName + + // if team.Name is changed + if updateTeamName { + // remove old team.Name from team names redis hash + err = pipe.HDel(connector.context, teamsByNamesKey, existedTeamLowercaseName).Err() + if err != nil { + return fmt.Errorf("failed to update team name: %w", err) + } + } + + // save new team.Name to team names redis hash + err = pipe.HSet(connector.context, teamsByNamesKey, newTeamLowercaseName, teamID).Err() + if err != nil { + return fmt.Errorf("failed to save team name: %w", err) + } + + return nil + }) + + return err +} + +func (connector *DbConnector) getTeamIDByNameInTx(tx *redis.Tx, teamName string) (string, error) { + teamID, err := tx.HGet(connector.context, teamsByNamesKey, strings.ToLower(teamName)).Result() + if err != nil { + if errors.Is(err, redis.Nil) { + return "", database.ErrNil + } + + return "", fmt.Errorf("failed to check team name existence: %w", err) + } + + return teamID, nil } // GetTeam retrieves team from redis by it's id. @@ -37,6 +127,22 @@ func (connector *DbConnector) GetTeam(teamID string) (moira.Team, error) { return team, nil } +// GetTeamByName retrieves team from redis by its name. +func (connector *DbConnector) GetTeamByName(name string) (moira.Team, error) { + c := *connector.client + + teamID, err := c.HGet(connector.context, teamsByNamesKey, strings.ToLower(name)).Result() + if err != nil { + if errors.Is(err, redis.Nil) { + return moira.Team{}, database.ErrNil + } + + return moira.Team{}, fmt.Errorf("failed to get team by name: %w", err) + } + + return connector.GetTeam(teamID) +} + // SaveTeamsAndUsers is a function that saves users for one team and teams for bunch of users in one transaction. func (connector *DbConnector) SaveTeamsAndUsers(teamID string, users []string, teams map[string][]string) error { c := *connector.client @@ -110,32 +216,48 @@ func (connector *DbConnector) IsTeamContainUser(teamID, userID string) (bool, er func (connector *DbConnector) DeleteTeam(teamID, userID string) error { c := *connector.client - pipe := c.TxPipeline() - - err := pipe.SRem(connector.context, userTeamsKey(userID), teamID).Err() + team, err := connector.GetTeam(teamID) if err != nil { - return fmt.Errorf("failed to remove team from user's teams: %w", err) - } + if errors.Is(err, database.ErrNil) { + return nil + } - err = pipe.Del(connector.context, teamUsersKey(teamID)).Err() - if err != nil { - return fmt.Errorf("failed to remove team users: %w", err) + return fmt.Errorf("failed to get team to delete: %w", err) } - err = pipe.HDel(connector.context, teamsKey, teamID).Err() + err = c.HDel(connector.context, teamsByNamesKey, strings.ToLower(team.Name)).Err() if err != nil { - return fmt.Errorf("failed to remove team metadata: %w", err) + return fmt.Errorf("failed to remove team name: %w", err) } - _, err = pipe.Exec(connector.context) - if err != nil { - return fmt.Errorf("cannot commit transaction and delete team: %w", err) - } + _, err = c.TxPipelined( + connector.context, + func(pipe redis.Pipeliner) error { + err = pipe.SRem(connector.context, userTeamsKey(userID), teamID).Err() + if err != nil { + return fmt.Errorf("failed to remove team from user's teams: %w", err) + } - return nil + err = pipe.Del(connector.context, teamUsersKey(teamID)).Err() + if err != nil { + return fmt.Errorf("failed to remove team users: %w", err) + } + + err = pipe.HDel(connector.context, teamsKey, teamID).Err() + if err != nil { + return fmt.Errorf("failed to remove team metadata: %w", err) + } + + return nil + }) + + return err } -const teamsKey = "moira-teams" +const ( + teamsKey = "moira-teams" + teamsByNamesKey = "moira-teams-by-names" +) func userTeamsKey(userID string) string { return fmt.Sprintf("moira-userTeams:%s", userID) diff --git a/database/redis/teams_test.go b/database/redis/teams_test.go index d3db66077..cb4093ba9 100644 --- a/database/redis/teams_test.go +++ b/database/redis/teams_test.go @@ -1,6 +1,7 @@ package redis import ( + "strings" "testing" "github.com/moira-alert/moira" @@ -36,6 +37,10 @@ func TestTeamStoring(t *testing.T) { So(err, ShouldBeNil) So(actualTeam, ShouldResemble, team) + actualTeam, err = dataBase.GetTeamByName(team.Name) + So(err, ShouldBeNil) + So(actualTeam, ShouldResemble, team) + actualTeam, err = dataBase.GetTeam("nonExistentTeam") So(err, ShouldResemble, database.ErrNil) So(actualTeam, ShouldResemble, moira.Team{}) @@ -130,20 +135,141 @@ func TestTeamStoring(t *testing.T) { Name: "TeamName", Description: "Team Description", } + err = dataBase.SaveTeam(teamToDeleteID, teamToDelete) So(err, ShouldBeNil) + err = dataBase.SaveTeamsAndUsers(teamToDeleteID, []string{userOfTeamToDeleteID}, map[string][]string{teamToDeleteID: {userOfTeamToDeleteID}}) So(err, ShouldBeNil) + err = dataBase.DeleteTeam(teamToDeleteID, userOfTeamToDeleteID) So(err, ShouldBeNil) + actualTeam, err = dataBase.GetTeam(teamToDeleteID) So(err, ShouldResemble, database.ErrNil) So(actualTeam, ShouldResemble, moira.Team{}) + + actualTeam, err = dataBase.GetTeamByName(teamToDelete.Name) + So(err, ShouldResemble, database.ErrNil) + So(actualTeam, ShouldResemble, moira.Team{}) + actualTeams, err = dataBase.GetUserTeams(userOfTeamToDeleteID) So(err, ShouldBeNil) So(actualTeams, ShouldHaveLength, 0) + actualUsers, err = dataBase.GetTeamUsers(teamToDeleteID) So(err, ShouldBeNil) So(actualUsers, ShouldHaveLength, 0) }) } + +func TestSaveAndGetTeam(t *testing.T) { + Convey("Test saving team", t, func() { + logger, _ := logging.GetLogger("dataBase") + dataBase := NewTestDatabase(logger) + dataBase.Flush() + defer dataBase.Flush() + + team := moira.Team{ + ID: "someTeamID", + Name: "Test team name", + Description: "Test description", + } + + Convey("when no team, get returns database.ErrNil", func() { + gotTeam, err := dataBase.GetTeam(team.ID) + + So(err, ShouldResemble, database.ErrNil) + So(gotTeam, ShouldResemble, moira.Team{}) + }) + + Convey("when no team, get by name returns database.ErrNil", func() { + gotTeam, err := dataBase.GetTeamByName(team.Name) + + So(err, ShouldResemble, database.ErrNil) + So(gotTeam, ShouldResemble, moira.Team{}) + }) + + Convey("when no team with such name, saved ok", func() { + err := dataBase.SaveTeam(team.ID, team) + + So(err, ShouldBeNil) + + Convey("and getting team by id returns saved team", func() { + gotTeam, err := dataBase.GetTeam(team.ID) + + So(err, ShouldBeNil) + So(gotTeam, ShouldResemble, team) + }) + + Convey("and getting team by name returns saved team", func() { + gotTeam, err := dataBase.GetTeamByName(team.Name) + + So(err, ShouldBeNil) + So(gotTeam, ShouldResemble, team) + }) + + Convey("and updating team ok", func() { + team.Name = strings.ToUpper(team.Name) + + err := dataBase.SaveTeam(team.ID, team) + So(err, ShouldBeNil) + + gotTeam, err := dataBase.GetTeam(team.ID) + So(err, ShouldBeNil) + So(gotTeam, ShouldResemble, team) + + gotTeam, err = dataBase.GetTeamByName(team.Name) + So(err, ShouldBeNil) + So(gotTeam, ShouldResemble, team) + }) + }) + + Convey("with changing name of existing team", func() { + err := dataBase.SaveTeam(team.ID, team) + So(err, ShouldBeNil) + + otherTeam := moira.Team{ + ID: "otherTeamID", + Name: "Other team name", + Description: "others description", + } + + err = dataBase.SaveTeam(otherTeam.ID, otherTeam) + So(err, ShouldBeNil) + + prevName := otherTeam.Name + + Convey("to name of existed team (no matter upper/lower-case) returns error", func() { + otherTeam.Name = team.Name + + err = dataBase.SaveTeam(otherTeam.ID, otherTeam) + So(err, ShouldResemble, database.ErrTeamWithNameAlreadyExists) + + otherTeam.Name = strings.ToLower(team.Name) + + err = dataBase.SaveTeam(otherTeam.ID, otherTeam) + So(err, ShouldResemble, database.ErrTeamWithNameAlreadyExists) + }) + + Convey("to new name, no team with prev name exist", func() { + otherTeam.Name = team.Name + "1" + + err = dataBase.SaveTeam(otherTeam.ID, otherTeam) + So(err, ShouldBeNil) + + gotTeam, err := dataBase.GetTeam(otherTeam.ID) + So(err, ShouldBeNil) + So(gotTeam, ShouldResemble, otherTeam) + + gotTeam, err = dataBase.GetTeamByName(prevName) + So(err, ShouldResemble, database.ErrNil) + So(gotTeam, ShouldResemble, moira.Team{}) + + gotTeam, err = dataBase.GetTeamByName(otherTeam.Name) + So(err, ShouldBeNil) + So(gotTeam, ShouldResemble, otherTeam) + }) + }) + }) +} diff --git a/interfaces.go b/interfaces.go index 94fc776bd..137d96afc 100644 --- a/interfaces.go +++ b/interfaces.go @@ -143,6 +143,7 @@ type Database interface { // Teams management SaveTeam(teamID string, team Team) error GetTeam(teamID string) (Team, error) + GetTeamByName(name string) (Team, error) SaveTeamsAndUsers(teamID string, users []string, usersTeams map[string][]string) error GetUserTeams(userID string) ([]string, error) GetTeamUsers(teamID string) ([]string, error) diff --git a/local/api.yml b/local/api.yml index d852a09b5..1f47563d3 100644 --- a/local/api.yml +++ b/local/api.yml @@ -53,6 +53,9 @@ api: limits: trigger: max_name_size: 200 + team: + max_name_size: 100 + max_description_size: 1000 web: contacts_template: - type: mail diff --git a/mock/moira-alert/database.go b/mock/moira-alert/database.go index a9953446c..2eddd945a 100644 --- a/mock/moira-alert/database.go +++ b/mock/moira-alert/database.go @@ -715,6 +715,21 @@ func (mr *MockDatabaseMockRecorder) GetTeam(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTeam", reflect.TypeOf((*MockDatabase)(nil).GetTeam), arg0) } +// GetTeamByName mocks base method. +func (m *MockDatabase) GetTeamByName(arg0 string) (moira.Team, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetTeamByName", arg0) + ret0, _ := ret[0].(moira.Team) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetTeamByName indicates an expected call of GetTeamByName. +func (mr *MockDatabaseMockRecorder) GetTeamByName(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTeamByName", reflect.TypeOf((*MockDatabase)(nil).GetTeamByName), arg0) +} + // GetTeamContactIDs mocks base method. func (m *MockDatabase) GetTeamContactIDs(arg0 string) ([]string, error) { m.ctrl.T.Helper()