From 3f46cf6f60863a356c8994f1df306464feb8ae25 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Sat, 18 Jul 2020 09:57:58 +0530 Subject: [PATCH] MM-18006: Fix flaky test CreateOrRestoreGroupMember (#14955) Updating a Group or a GroupMember only changed the UpdateAt or CreateAt times respectively. And it threw an error if number of rows changed was not 1. However, it can happen that 2 calls happen so fast that 1 milisecond does not pass, or even 2 concurrent calls at the same time might happen so that model.GetMillis return the same timestamp. In those cases, the number of rows updated can be 0. The error should just check if the number is greater than 1, instead of not equal to 1. This makes it more robust and correct. Co-authored-by: Mattermod --- i18n/en.json | 8 ++++---- store/sqlstore/group_store.go | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/i18n/en.json b/i18n/en.json index 478dad9c64..073364f092 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -6727,12 +6727,12 @@ "translation": "group syncable was already deleted" }, { - "id": "store.sql_group.no_rows", - "translation": "no matching group found" + "id": "store.sql_group.more_than_one_row_changed", + "translation": "More than one row changed." }, { - "id": "store.sql_group.no_rows_changed", - "translation": "no rows changed" + "id": "store.sql_group.no_rows", + "translation": "no matching group found" }, { "id": "store.sql_group.permanent_delete_members_by_user.app_error", diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index 6271b35587..9d97b9049d 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -232,8 +232,8 @@ func (s *SqlGroupStore) Update(group *model.Group) (*model.Group, *model.AppErro } return nil, model.NewAppError("SqlGroupStore.GroupUpdate", "store.update_error", nil, err.Error(), http.StatusInternalServerError) } - if rowsChanged != 1 { - return nil, model.NewAppError("SqlGroupStore.GroupUpdate", "store.sql_group.no_rows_changed", nil, "", http.StatusInternalServerError) + if rowsChanged > 1 { + return nil, model.NewAppError("SqlGroupStore.GroupUpdate", "store.sql_group.more_than_one_row_changed", nil, "", http.StatusInternalServerError) } return group, nil @@ -427,8 +427,8 @@ func (s *SqlGroupStore) UpsertMember(groupID string, userID string) (*model.Grou if rowsChanged, err = s.GetMaster().Update(member); err != nil { return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.update_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId+", "+err.Error(), http.StatusInternalServerError) } - if rowsChanged != 1 { - return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.sql_group.no_rows_changed", nil, "", http.StatusInternalServerError) + if rowsChanged > 1 { + return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.sql_group.more_than_one_row_changed", nil, "", http.StatusInternalServerError) } }