From b0ad3c10e996228806d39cf2e600778584d62ac8 Mon Sep 17 00:00:00 2001 From: Marc Argent Date: Tue, 11 Jun 2019 18:40:05 +0100 Subject: [PATCH] GH-10932 Migrate 'Session.UpdateLastActivityAt' to Sync by default (#11078) * Migrate Session.UpdateLastActivityAt to Sync by default * GH-10932 fix tests * GH-10932 update add session code --- app/session.go | 4 ++-- app/session_test.go | 12 ++++++++---- store/sqlstore/session_store.go | 14 ++++++-------- store/store.go | 2 +- store/storetest/mocks/SessionStore.go | 8 ++++---- store/storetest/session_store.go | 5 ++--- 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/app/session.go b/app/session.go index 1e0a6a373a..41fe308973 100644 --- a/app/session.go +++ b/app/session.go @@ -223,8 +223,8 @@ func (a *App) UpdateLastActivityAtIfNeeded(session model.Session) { return } - if result := <-a.Srv.Store.Session().UpdateLastActivityAt(session.Id, now); result.Err != nil { - mlog.Error(fmt.Sprintf("Failed to update LastActivityAt for user_id=%v and session_id=%v, err=%v", session.UserId, session.Id, result.Err), mlog.String("user_id", session.UserId)) + if err := a.Srv.Store.Session().UpdateLastActivityAt(session.Id, now); err != nil { + mlog.Error(fmt.Sprintf("Failed to update LastActivityAt for user_id=%v and session_id=%v, err=%v", session.UserId, session.Id, err), mlog.String("user_id", session.UserId)) } session.LastActivityAt = now diff --git a/app/session_test.go b/app/session_test.go index d275921fc1..0516fe289c 100644 --- a/app/session_test.go +++ b/app/session_test.go @@ -56,7 +56,8 @@ func TestGetSessionIdleTimeoutInMinutes(t *testing.T) { // Test regular session, should timeout time := session.LastActivityAt - (1000 * 60 * 6) - <-th.App.Srv.Store.Session().UpdateLastActivityAt(session.Id, time) + err = th.App.Srv.Store.Session().UpdateLastActivityAt(session.Id, time) + require.Nil(t, err) th.App.ClearSessionCacheForUserSkipClusterSend(session.UserId) rsession, err = th.App.GetSession(session.Token) @@ -73,7 +74,8 @@ func TestGetSessionIdleTimeoutInMinutes(t *testing.T) { session, _ = th.App.CreateSession(session) time = session.LastActivityAt - (1000 * 60 * 6) - <-th.App.Srv.Store.Session().UpdateLastActivityAt(session.Id, time) + err = th.App.Srv.Store.Session().UpdateLastActivityAt(session.Id, time) + require.Nil(t, err) th.App.ClearSessionCacheForUserSkipClusterSend(session.UserId) _, err = th.App.GetSession(session.Token) @@ -87,7 +89,8 @@ func TestGetSessionIdleTimeoutInMinutes(t *testing.T) { session, _ = th.App.CreateSession(session) time = session.LastActivityAt - (1000 * 60 * 6) - <-th.App.Srv.Store.Session().UpdateLastActivityAt(session.Id, time) + err = th.App.Srv.Store.Session().UpdateLastActivityAt(session.Id, time) + require.Nil(t, err) th.App.ClearSessionCacheForUserSkipClusterSend(session.UserId) _, err = th.App.GetSession(session.Token) @@ -104,7 +107,8 @@ func TestGetSessionIdleTimeoutInMinutes(t *testing.T) { session, _ = th.App.CreateSession(session) time = session.LastActivityAt - (1000 * 60 * 6) - <-th.App.Srv.Store.Session().UpdateLastActivityAt(session.Id, time) + err = th.App.Srv.Store.Session().UpdateLastActivityAt(session.Id, time) + require.Nil(t, err) th.App.ClearSessionCacheForUserSkipClusterSend(session.UserId) _, err = th.App.GetSession(session.Token) diff --git a/store/sqlstore/session_store.go b/store/sqlstore/session_store.go index 567701ebcd..399f13f4cb 100644 --- a/store/sqlstore/session_store.go +++ b/store/sqlstore/session_store.go @@ -170,14 +170,12 @@ func (me SqlSessionStore) PermanentDeleteSessionsByUser(userId string) *model.Ap return nil } -func (me SqlSessionStore) UpdateLastActivityAt(sessionId string, time int64) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if _, err := me.GetMaster().Exec("UPDATE Sessions SET LastActivityAt = :LastActivityAt WHERE Id = :Id", map[string]interface{}{"LastActivityAt": time, "Id": sessionId}); err != nil { - result.Err = model.NewAppError("SqlSessionStore.UpdateLastActivityAt", "store.sql_session.update_last_activity.app_error", nil, "sessionId="+sessionId, http.StatusInternalServerError) - } else { - result.Data = sessionId - } - }) +func (me SqlSessionStore) UpdateLastActivityAt(sessionId string, time int64) *model.AppError { + _, err := me.GetMaster().Exec("UPDATE Sessions SET LastActivityAt = :LastActivityAt WHERE Id = :Id", map[string]interface{}{"LastActivityAt": time, "Id": sessionId}) + if err != nil { + return model.NewAppError("SqlSessionStore.UpdateLastActivityAt", "store.sql_session.update_last_activity.app_error", nil, "sessionId="+sessionId, http.StatusInternalServerError) + } + return nil } func (me SqlSessionStore) UpdateRoles(userId, roles string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index f709e82c3c..66087f5057 100644 --- a/store/store.go +++ b/store/store.go @@ -319,7 +319,7 @@ type SessionStore interface { Remove(sessionIdOrToken string) StoreChannel RemoveAllSessions() *model.AppError PermanentDeleteSessionsByUser(teamId string) *model.AppError - UpdateLastActivityAt(sessionId string, time int64) StoreChannel + UpdateLastActivityAt(sessionId string, time int64) *model.AppError UpdateRoles(userId string, roles string) StoreChannel UpdateDeviceId(id string, deviceId string, expiresAt int64) (string, *model.AppError) AnalyticsSessionCount() (int64, *model.AppError) diff --git a/store/storetest/mocks/SessionStore.go b/store/storetest/mocks/SessionStore.go index b6d2c56e67..dc39c4924a 100644 --- a/store/storetest/mocks/SessionStore.go +++ b/store/storetest/mocks/SessionStore.go @@ -213,15 +213,15 @@ func (_m *SessionStore) UpdateDeviceId(id string, deviceId string, expiresAt int } // UpdateLastActivityAt provides a mock function with given fields: sessionId, time -func (_m *SessionStore) UpdateLastActivityAt(sessionId string, time int64) store.StoreChannel { +func (_m *SessionStore) UpdateLastActivityAt(sessionId string, time int64) *model.AppError { ret := _m.Called(sessionId, time) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, int64) store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(string, int64) *model.AppError); ok { r0 = rf(sessionId, time) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } diff --git a/store/storetest/session_store.go b/store/storetest/session_store.go index 3fd8265ea3..6a4ca7b9c4 100644 --- a/store/storetest/session_store.go +++ b/store/storetest/session_store.go @@ -258,9 +258,8 @@ func testSessionStoreUpdateLastActivityAt(t *testing.T, ss store.Store) { s1, err := ss.Session().Save(s1) require.Nil(t, err) - if err := (<-ss.Session().UpdateLastActivityAt(s1.Id, 1234567890)).Err; err != nil { - t.Fatal(err) - } + err = ss.Session().UpdateLastActivityAt(s1.Id, 1234567890) + require.Nil(t, err) if session, err := ss.Session().Get(s1.Id); err != nil { t.Fatal(err)