Fixing race condition on status serialization (#14147)

* More atomic serialization process for status

* Improving the granularity of tests on api4/status_test.go
Этот коммит содержится в:
Jesús Espino
2020-03-27 11:40:52 +01:00
коммит произвёл GitHub
родитель d92cfc851a
Коммит d2848cc167
2 изменённых файлов: 144 добавлений и 101 удалений

Просмотреть файл

@@ -15,44 +15,60 @@ func TestGetUserStatus(t *testing.T) {
defer th.TearDown() defer th.TearDown()
Client := th.Client Client := th.Client
userStatus, resp := Client.GetUserStatus(th.BasicUser.Id, "") t.Run("offline status", func(t *testing.T) {
CheckNoError(t, resp) userStatus, resp := Client.GetUserStatus(th.BasicUser.Id, "")
assert.Equal(t, "offline", userStatus.Status) CheckNoError(t, resp)
assert.Equal(t, "offline", userStatus.Status)
})
th.App.SetStatusOnline(th.BasicUser.Id, true) t.Run("online status", func(t *testing.T) {
userStatus, resp = Client.GetUserStatus(th.BasicUser.Id, "") th.App.SetStatusOnline(th.BasicUser.Id, true)
CheckNoError(t, resp) userStatus, resp := Client.GetUserStatus(th.BasicUser.Id, "")
assert.Equal(t, "online", userStatus.Status) CheckNoError(t, resp)
assert.Equal(t, "online", userStatus.Status)
})
th.App.SetStatusAwayIfNeeded(th.BasicUser.Id, true) t.Run("away status", func(t *testing.T) {
userStatus, resp = Client.GetUserStatus(th.BasicUser.Id, "") th.App.SetStatusAwayIfNeeded(th.BasicUser.Id, true)
CheckNoError(t, resp) userStatus, resp := Client.GetUserStatus(th.BasicUser.Id, "")
assert.Equal(t, "away", userStatus.Status) CheckNoError(t, resp)
assert.Equal(t, "away", userStatus.Status)
})
th.App.SetStatusDoNotDisturb(th.BasicUser.Id) t.Run("dnd status", func(t *testing.T) {
userStatus, resp = Client.GetUserStatus(th.BasicUser.Id, "") th.App.SetStatusDoNotDisturb(th.BasicUser.Id)
CheckNoError(t, resp) userStatus, resp := Client.GetUserStatus(th.BasicUser.Id, "")
assert.Equal(t, "dnd", userStatus.Status) CheckNoError(t, resp)
assert.Equal(t, "dnd", userStatus.Status)
})
th.App.SetStatusOffline(th.BasicUser.Id, true) t.Run("back to offline status", func(t *testing.T) {
userStatus, resp = Client.GetUserStatus(th.BasicUser.Id, "") th.App.SetStatusOffline(th.BasicUser.Id, true)
CheckNoError(t, resp) userStatus, resp := Client.GetUserStatus(th.BasicUser.Id, "")
assert.Equal(t, "offline", userStatus.Status) CheckNoError(t, resp)
assert.Equal(t, "offline", userStatus.Status)
})
//Get user2 status logged as user1 t.Run("get other user status", func(t *testing.T) {
userStatus, resp = Client.GetUserStatus(th.BasicUser2.Id, "") //Get user2 status logged as user1
CheckNoError(t, resp) userStatus, resp := Client.GetUserStatus(th.BasicUser2.Id, "")
assert.Equal(t, "offline", userStatus.Status) CheckNoError(t, resp)
assert.Equal(t, "offline", userStatus.Status)
})
Client.Logout() t.Run("get status from logged out user", func(t *testing.T) {
Client.Logout()
_, resp = Client.GetUserStatus(th.BasicUser2.Id, "") _, resp := Client.GetUserStatus(th.BasicUser2.Id, "")
CheckUnauthorizedStatus(t, resp) CheckUnauthorizedStatus(t, resp)
})
th.LoginBasic2() t.Run("get status from other user", func(t *testing.T) {
userStatus, resp = Client.GetUserStatus(th.BasicUser2.Id, "") th.LoginBasic2()
CheckNoError(t, resp) userStatus, resp := Client.GetUserStatus(th.BasicUser2.Id, "")
assert.Equal(t, "offline", userStatus.Status) CheckNoError(t, resp)
assert.Equal(t, "offline", userStatus.Status)
})
} }
func TestGetUsersStatusesByIds(t *testing.T) { func TestGetUsersStatusesByIds(t *testing.T) {
@@ -62,40 +78,55 @@ func TestGetUsersStatusesByIds(t *testing.T) {
usersIds := []string{th.BasicUser.Id, th.BasicUser2.Id} usersIds := []string{th.BasicUser.Id, th.BasicUser2.Id}
usersStatuses, resp := Client.GetUsersStatusesByIds(usersIds) t.Run("empty userIds list", func(t *testing.T) {
CheckNoError(t, resp) _, resp := Client.GetUsersStatusesByIds([]string{})
for _, userStatus := range usersStatuses { CheckBadRequestStatus(t, resp)
assert.Equal(t, "offline", userStatus.Status) })
}
th.App.SetStatusOnline(th.BasicUser.Id, true) t.Run("offline status", func(t *testing.T) {
th.App.SetStatusOnline(th.BasicUser2.Id, true) usersStatuses, resp := Client.GetUsersStatusesByIds(usersIds)
usersStatuses, resp = Client.GetUsersStatusesByIds(usersIds) CheckNoError(t, resp)
CheckNoError(t, resp) for _, userStatus := range usersStatuses {
for _, userStatus := range usersStatuses { assert.Equal(t, "offline", userStatus.Status)
assert.Equal(t, "online", userStatus.Status) }
} })
th.App.SetStatusAwayIfNeeded(th.BasicUser.Id, true) t.Run("online status", func(t *testing.T) {
th.App.SetStatusAwayIfNeeded(th.BasicUser2.Id, true) th.App.SetStatusOnline(th.BasicUser.Id, true)
usersStatuses, resp = Client.GetUsersStatusesByIds(usersIds) th.App.SetStatusOnline(th.BasicUser2.Id, true)
CheckNoError(t, resp) usersStatuses, resp := Client.GetUsersStatusesByIds(usersIds)
for _, userStatus := range usersStatuses { CheckNoError(t, resp)
assert.Equal(t, "away", userStatus.Status) for _, userStatus := range usersStatuses {
} assert.Equal(t, "online", userStatus.Status)
}
})
th.App.SetStatusDoNotDisturb(th.BasicUser.Id) t.Run("away status", func(t *testing.T) {
th.App.SetStatusDoNotDisturb(th.BasicUser2.Id) th.App.SetStatusAwayIfNeeded(th.BasicUser.Id, true)
usersStatuses, resp = Client.GetUsersStatusesByIds(usersIds) th.App.SetStatusAwayIfNeeded(th.BasicUser2.Id, true)
CheckNoError(t, resp) usersStatuses, resp := Client.GetUsersStatusesByIds(usersIds)
for _, userStatus := range usersStatuses { CheckNoError(t, resp)
assert.Equal(t, "dnd", userStatus.Status) for _, userStatus := range usersStatuses {
} assert.Equal(t, "away", userStatus.Status)
}
})
Client.Logout() t.Run("dnd status", func(t *testing.T) {
th.App.SetStatusDoNotDisturb(th.BasicUser.Id)
th.App.SetStatusDoNotDisturb(th.BasicUser2.Id)
usersStatuses, resp := Client.GetUsersStatusesByIds(usersIds)
CheckNoError(t, resp)
for _, userStatus := range usersStatuses {
assert.Equal(t, "dnd", userStatus.Status)
}
})
_, resp = Client.GetUsersStatusesByIds(usersIds) t.Run("get statuses from logged out user", func(t *testing.T) {
CheckUnauthorizedStatus(t, resp) Client.Logout()
_, resp := Client.GetUsersStatusesByIds(usersIds)
CheckUnauthorizedStatus(t, resp)
})
} }
func TestUpdateUserStatus(t *testing.T) { func TestUpdateUserStatus(t *testing.T) {
@@ -103,40 +134,57 @@ func TestUpdateUserStatus(t *testing.T) {
defer th.TearDown() defer th.TearDown()
Client := th.Client Client := th.Client
toUpdateUserStatus := &model.Status{Status: "online", UserId: th.BasicUser.Id} t.Run("set online status", func(t *testing.T) {
updateUserStatus, resp := Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus) toUpdateUserStatus := &model.Status{Status: "online", UserId: th.BasicUser.Id}
CheckNoError(t, resp) updateUserStatus, resp := Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus)
assert.Equal(t, "online", updateUserStatus.Status) CheckNoError(t, resp)
assert.Equal(t, "online", updateUserStatus.Status)
})
toUpdateUserStatus.Status = "away" t.Run("set away status", func(t *testing.T) {
updateUserStatus, resp = Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus) toUpdateUserStatus := &model.Status{Status: "away", UserId: th.BasicUser.Id}
CheckNoError(t, resp) updateUserStatus, resp := Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus)
assert.Equal(t, "away", updateUserStatus.Status) CheckNoError(t, resp)
assert.Equal(t, "away", updateUserStatus.Status)
})
toUpdateUserStatus.Status = "dnd" t.Run("set dnd status", func(t *testing.T) {
updateUserStatus, resp = Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus) toUpdateUserStatus := &model.Status{Status: "dnd", UserId: th.BasicUser.Id}
CheckNoError(t, resp) updateUserStatus, resp := Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus)
assert.Equal(t, "dnd", updateUserStatus.Status) CheckNoError(t, resp)
assert.Equal(t, "dnd", updateUserStatus.Status)
})
toUpdateUserStatus.Status = "offline" t.Run("set offline status", func(t *testing.T) {
updateUserStatus, resp = Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus) toUpdateUserStatus := &model.Status{Status: "offline", UserId: th.BasicUser.Id}
CheckNoError(t, resp) updateUserStatus, resp := Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus)
assert.Equal(t, "offline", updateUserStatus.Status) CheckNoError(t, resp)
assert.Equal(t, "offline", updateUserStatus.Status)
})
toUpdateUserStatus.Status = "online" t.Run("set status for other user as regular user", func(t *testing.T) {
toUpdateUserStatus.UserId = th.BasicUser2.Id toUpdateUserStatus := &model.Status{Status: "online", UserId: th.BasicUser2.Id}
_, resp = Client.UpdateUserStatus(th.BasicUser2.Id, toUpdateUserStatus) _, resp := Client.UpdateUserStatus(th.BasicUser2.Id, toUpdateUserStatus)
CheckForbiddenStatus(t, resp) CheckForbiddenStatus(t, resp)
})
toUpdateUserStatus.Status = "online" t.Run("set status for other user as admin user", func(t *testing.T) {
updateUserStatus, _ = th.SystemAdminClient.UpdateUserStatus(th.BasicUser2.Id, toUpdateUserStatus) toUpdateUserStatus := &model.Status{Status: "online", UserId: th.BasicUser2.Id}
assert.Equal(t, "online", updateUserStatus.Status) updateUserStatus, _ := th.SystemAdminClient.UpdateUserStatus(th.BasicUser2.Id, toUpdateUserStatus)
assert.Equal(t, "online", updateUserStatus.Status)
})
_, resp = Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus) t.Run("not matching status user id and the user id passed in the function", func(t *testing.T) {
CheckBadRequestStatus(t, resp) toUpdateUserStatus := &model.Status{Status: "online", UserId: th.BasicUser2.Id}
_, resp := Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus)
CheckBadRequestStatus(t, resp)
})
Client.Logout() t.Run("get statuses from logged out user", func(t *testing.T) {
toUpdateUserStatus := &model.Status{Status: "online", UserId: th.BasicUser2.Id}
Client.Logout()
_, resp = Client.UpdateUserStatus(th.BasicUser2.Id, toUpdateUserStatus) _, resp := Client.UpdateUserStatus(th.BasicUser2.Id, toUpdateUserStatus)
CheckUnauthorizedStatus(t, resp) CheckUnauthorizedStatus(t, resp)
})
} }

Просмотреть файл

@@ -28,10 +28,9 @@ type Status struct {
} }
func (o *Status) ToJson() string { func (o *Status) ToJson() string {
tempChannelId := o.ActiveChannel oCopy := *o
o.ActiveChannel = "" oCopy.ActiveChannel = ""
b, _ := json.Marshal(o) b, _ := json.Marshal(oCopy)
o.ActiveChannel = tempChannelId
return string(b) return string(b)
} }
@@ -47,18 +46,14 @@ func StatusFromJson(data io.Reader) *Status {
} }
func StatusListToJson(u []*Status) string { func StatusListToJson(u []*Status) string {
activeChannels := make([]string, len(u)) uCopy := make([]Status, len(u))
for index, s := range u { for i, s := range u {
activeChannels[index] = s.ActiveChannel sCopy := *s
s.ActiveChannel = "" sCopy.ActiveChannel = ""
} uCopy[i] = sCopy
b, _ := json.Marshal(u)
for index, s := range u {
s.ActiveChannel = activeChannels[index]
} }
b, _ := json.Marshal(uCopy)
return string(b) return string(b)
} }