IntenseWebs/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2025-02-25 18:55:24 -06:00
Code Issues Packages Projects Releases Wiki Activity

#10939 Migrate "Session.Save" to Sync by default (#10944)

Browse Source 
* Migrate "Session.Save" to Sync by default

* fixing unreachable code

* removing shadowing

*  whitespace changes, refactoring flow control logic

* removing unnecessary whitespace
This commit is contained in:
Evan do Carmo
2019-05-30 12:10:24 -04:00
committed by Joram Wilander
parent 6aba17ff9d
commit ae6fed827a
8 changed files with 156 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
app/oauth.go
View File

@@ -342,11 +342,10 @@ func (a *App) newSession(appName string, user *model.User) (*model.Session, *mod
session.AddProp(model.SESSION_PROP_OS, "OAuth2")
session.AddProp(model.SESSION_PROP_BROWSER, "OAuth2")
result := <-a.Srv.Store.Session().Save(session)
if result.Err != nil {
session, err := a.Srv.Store.Session().Save(session)
if err != nil {
return nil, model.NewAppError("newSession", "api.oauth.get_access_token.internal_session.app_error", nil, "", http.StatusInternalServerError)
}
session = result.Data.(*model.Session)
a.AddSessionToCache(session)

14
app/session.go
View File

@@ -14,11 +14,10 @@ import (
func (a *App) CreateSession(session *model.Session) (*model.Session, *model.AppError) {
session.Token = ""
result := <-a.Srv.Store.Session().Save(session)
if result.Err != nil {
return nil, result.Err
session, err := a.Srv.Store.Session().Save(session)
if err != nil {
return nil, err
}
session = result.Data.(*model.Session)
a.AddSessionToCache(session)
@@ -303,11 +302,10 @@ func (a *App) createSessionForUserAccessToken(tokenString string) (*model.Sessio
session.AddProp(model.SESSION_PROP_TYPE, model.SESSION_TYPE_USER_ACCESS_TOKEN)
session.SetExpireInDays(model.SESSION_USER_ACCESS_TOKEN_EXPIRY)
result = <-a.Srv.Store.Session().Save(session)
if result.Err != nil {
return nil, result.Err
session, err = a.Srv.Store.Session().Save(session)
if err != nil {
return nil, err
}
session = result.Data.(*model.Session)
a.AddSessionToCache(session)

55
store/sqlstore/session_store.go
View File

@@ -45,37 +45,34 @@ func (me SqlSessionStore) CreateIndexesIfNotExists() {
me.CreateIndexIfNotExists("idx_sessions_last_activity_at", "Sessions", "LastActivityAt")
}
func (me SqlSessionStore) Save(session *model.Session) store.StoreChannel {
return store.Do(func(result *store.StoreResult) {
if len(session.Id) > 0 {
result.Err = model.NewAppError("SqlSessionStore.Save", "store.sql_session.save.existing.app_error", nil, "id="+session.Id, http.StatusBadRequest)
return
func (me SqlSessionStore) Save(session *model.Session) (*model.Session, *model.AppError) {
if len(session.Id) > 0 {
return nil, model.NewAppError("SqlSessionStore.Save", "store.sql_session.save.existing.app_error", nil, "id="+session.Id, http.StatusBadRequest)
}
session.PreSave()
tcs := me.Team().GetTeamsForUser(session.UserId)
if err := me.GetMaster().Insert(session); err != nil {
return nil, model.NewAppError("SqlSessionStore.Save", "store.sql_session.save.app_error", nil, "id="+session.Id+", "+err.Error(), http.StatusInternalServerError)
}
rtcs := <-tcs
if rtcs.Err != nil {
return nil, model.NewAppError("SqlSessionStore.Save", "store.sql_session.save.app_error", nil, "id="+session.Id+", "+rtcs.Err.Error(), http.StatusInternalServerError)
}
tempMembers := rtcs.Data.([]*model.TeamMember)
session.TeamMembers = make([]*model.TeamMember, 0, len(tempMembers))
for _, tm := range tempMembers {
if tm.DeleteAt == 0 {
session.TeamMembers = append(session.TeamMembers, tm)
}
}
session.PreSave()
tcs := me.Team().GetTeamsForUser(session.UserId)
if err := me.GetMaster().Insert(session); err != nil {
result.Err = model.NewAppError("SqlSessionStore.Save", "store.sql_session.save.app_error", nil, "id="+session.Id+", "+err.Error(), http.StatusInternalServerError)
return
} else {
result.Data = session
}
if rtcs := <-tcs; rtcs.Err != nil {
result.Err = model.NewAppError("SqlSessionStore.Save", "store.sql_session.save.app_error", nil, "id="+session.Id+", "+rtcs.Err.Error(), http.StatusInternalServerError)
return
} else {
tempMembers := rtcs.Data.([]*model.TeamMember)
session.TeamMembers = make([]*model.TeamMember, 0, len(tempMembers))
for _, tm := range tempMembers {
if tm.DeleteAt == 0 {
session.TeamMembers = append(session.TeamMembers, tm)
}
}
}
})
return session, nil
}
func (me SqlSessionStore) Get(sessionIdOrToken string) store.StoreChannel {

2
store/store.go
View File

@@ -312,7 +312,7 @@ type BotStore interface {
}
type SessionStore interface {
Save(session *model.Session) StoreChannel
Save(session *model.Session) (*model.Session, *model.AppError)
Get(sessionIdOrToken string) StoreChannel
GetSessions(userId string) StoreChannel
GetSessionsWithActiveDeviceIds(userId string) ([]*model.Session, *model.AppError)

19
store/storetest/mocks/SessionStore.go
View File

@@ -147,19 +147,28 @@ func (_m *SessionStore) RemoveAllSessions() store.StoreChannel {
}
// Save provides a mock function with given fields: session
func (_m *SessionStore) Save(session *model.Session) store.StoreChannel {
func (_m *SessionStore) Save(session *model.Session) (*model.Session, *model.AppError) {
ret := _m.Called(session)
var r0 store.StoreChannel
if rf, ok := ret.Get(0).(func(*model.Session) store.StoreChannel); ok {
var r0 *model.Session
if rf, ok := ret.Get(0).(func(*model.Session) *model.Session); ok {
r0 = rf(session)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(store.StoreChannel)
r0 = ret.Get(0).(*model.Session)
}
}
return r0
var r1 *model.AppError
if rf, ok := ret.Get(1).(func(*model.Session) *model.AppError); ok {
r1 = rf(session)
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*model.AppError)
}
}
return r0, r1
}
// UpdateDeviceId provides a mock function with given fields: id, deviceId, expiresAt

6
store/storetest/oauth_store.go
View File

@@ -8,7 +8,6 @@ import (
"github.com/mattermost/mattermost-server/model"
"github.com/mattermost/mattermost-server/store"
"github.com/stretchr/testify/require"
)
@@ -409,12 +408,13 @@ func testOAuthStoreDeleteApp(t *testing.T, ss store.Store) {
t.Fatal(err)
}
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
s1.Token = model.NewId()
s1.IsOAuth = true
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
ad1 := model.AccessData{}
ad1.ClientId = a1.Id

127
store/storetest/session_store.go
View File

@@ -10,6 +10,7 @@ import (
"github.com/mattermost/mattermost-server/store"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestSessionStore(t *testing.T, ss store.Store) {
@@ -30,27 +31,33 @@ func TestSessionStore(t *testing.T, ss store.Store) {
}
func testSessionStoreSave(t *testing.T, ss store.Store) {
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
if err := (<-ss.Session().Save(&s1)).Err; err != nil {
if _, err := ss.Session().Save(s1); err != nil {
t.Fatal(err)
}
}
func testSessionGet(t *testing.T, ss store.Store) {
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
store.Must(ss.Session().Save(&s1))
s2 := model.Session{}
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
s2 := &model.Session{}
s2.UserId = s1.UserId
store.Must(ss.Session().Save(&s2))
s3 := model.Session{}
s2, err = ss.Session().Save(s2)
require.Nil(t, err)
s3 := &model.Session{}
s3.UserId = s1.UserId
s3.ExpiresAt = 1
store.Must(ss.Session().Save(&s3))
s3, err = ss.Session().Save(s3)
require.Nil(t, err)
if rs1 := (<-ss.Session().Get(s1.Id)); rs1.Err != nil {
t.Fatal(rs1.Err)
@@ -70,22 +77,28 @@ func testSessionGet(t *testing.T, ss store.Store) {
}
func testSessionGetWithDeviceId(t *testing.T, ss store.Store) {
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
s1.ExpiresAt = model.GetMillis() + 10000
store.Must(ss.Session().Save(&s1))
s2 := model.Session{}
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
s2 := &model.Session{}
s2.UserId = s1.UserId
s2.DeviceId = model.NewId()
s2.ExpiresAt = model.GetMillis() + 10000
store.Must(ss.Session().Save(&s2))
s3 := model.Session{}
s2, err = ss.Session().Save(s2)
require.Nil(t, err)
s3 := &model.Session{}
s3.UserId = s1.UserId
s3.ExpiresAt = 1
s3.DeviceId = model.NewId()
store.Must(ss.Session().Save(&s3))
s3, err = ss.Session().Save(s3)
require.Nil(t, err)
if data, err := ss.Session().GetSessionsWithActiveDeviceIds(s1.UserId); err != nil {
t.Fatal(err)
@@ -97,9 +110,11 @@ func testSessionGetWithDeviceId(t *testing.T, ss store.Store) {
}
func testSessionRemove(t *testing.T, ss store.Store) {
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
if rs1 := (<-ss.Session().Get(s1.Id)); rs1.Err != nil {
t.Fatal(rs1.Err)
@@ -117,9 +132,11 @@ func testSessionRemove(t *testing.T, ss store.Store) {
}
func testSessionRemoveAll(t *testing.T, ss store.Store) {
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
if rs1 := (<-ss.Session().Get(s1.Id)); rs1.Err != nil {
t.Fatal(rs1.Err)
@@ -137,9 +154,11 @@ func testSessionRemoveAll(t *testing.T, ss store.Store) {
}
func testSessionRemoveByUser(t *testing.T, ss store.Store) {
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
if rs1 := (<-ss.Session().Get(s1.Id)); rs1.Err != nil {
t.Fatal(rs1.Err)
@@ -157,9 +176,11 @@ func testSessionRemoveByUser(t *testing.T, ss store.Store) {
}
func testSessionRemoveToken(t *testing.T, ss store.Store) {
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
if rs1 := (<-ss.Session().Get(s1.Id)); rs1.Err != nil {
t.Fatal(rs1.Err)
@@ -185,17 +206,21 @@ func testSessionRemoveToken(t *testing.T, ss store.Store) {
}
func testSessionUpdateDeviceId(t *testing.T, ss store.Store) {
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
if rs1 := (<-ss.Session().UpdateDeviceId(s1.Id, model.PUSH_NOTIFY_APPLE+":1234567890", s1.ExpiresAt)); rs1.Err != nil {
t.Fatal(rs1.Err)
}
s2 := model.Session{}
s2 := &model.Session{}
s2.UserId = model.NewId()
store.Must(ss.Session().Save(&s2))
s2, err = ss.Session().Save(s2)
require.Nil(t, err)
if rs2 := (<-ss.Session().UpdateDeviceId(s2.Id, model.PUSH_NOTIFY_APPLE+":1234567890", s1.ExpiresAt)); rs2.Err != nil {
t.Fatal(rs2.Err)
@@ -203,17 +228,21 @@ func testSessionUpdateDeviceId(t *testing.T, ss store.Store) {
}
func testSessionUpdateDeviceId2(t *testing.T, ss store.Store) {
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
if rs1 := (<-ss.Session().UpdateDeviceId(s1.Id, model.PUSH_NOTIFY_APPLE_REACT_NATIVE+":1234567890", s1.ExpiresAt)); rs1.Err != nil {
t.Fatal(rs1.Err)
}
s2 := model.Session{}
s2 := &model.Session{}
s2.UserId = model.NewId()
store.Must(ss.Session().Save(&s2))
s2, err = ss.Session().Save(s2)
require.Nil(t, err)
if rs2 := (<-ss.Session().UpdateDeviceId(s2.Id, model.PUSH_NOTIFY_APPLE_REACT_NATIVE+":1234567890", s1.ExpiresAt)); rs2.Err != nil {
t.Fatal(rs2.Err)
@@ -221,9 +250,11 @@ func testSessionUpdateDeviceId2(t *testing.T, ss store.Store) {
}
func testSessionStoreUpdateLastActivityAt(t *testing.T, ss store.Store) {
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
if err := (<-ss.Session().UpdateLastActivityAt(s1.Id, 1234567890)).Err; err != nil {
t.Fatal(err)
@@ -240,10 +271,12 @@ func testSessionStoreUpdateLastActivityAt(t *testing.T, ss store.Store) {
}
func testSessionCount(t *testing.T, ss store.Store) {
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
s1.ExpiresAt = model.GetMillis() + 100000
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
if count, err := ss.Session().AnalyticsSessionCount(); err != nil {
t.Fatal(err)
@@ -257,29 +290,37 @@ func testSessionCount(t *testing.T, ss store.Store) {
func testSessionCleanup(t *testing.T, ss store.Store) {
now := model.GetMillis()
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = model.NewId()
s1.ExpiresAt = 0 // never expires
store.Must(ss.Session().Save(&s1))
s2 := model.Session{}
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
s2 := &model.Session{}
s2.UserId = s1.UserId
s2.ExpiresAt = now + 1000000 // expires in the future
store.Must(ss.Session().Save(&s2))
s3 := model.Session{}
s2, err = ss.Session().Save(s2)
require.Nil(t, err)
s3 := &model.Session{}
s3.UserId = model.NewId()
s3.ExpiresAt = 1 // expired
store.Must(ss.Session().Save(&s3))
s4 := model.Session{}
s3, err = ss.Session().Save(s3)
require.Nil(t, err)
s4 := &model.Session{}
s4.UserId = model.NewId()
s4.ExpiresAt = 2 // expired
store.Must(ss.Session().Save(&s4))
s4, err = ss.Session().Save(s4)
require.Nil(t, err)
ss.Session().Cleanup(now, 1)
err := (<-ss.Session().Get(s1.Id)).Err
err = (<-ss.Session().Get(s1.Id)).Err
assert.Nil(t, err)
err = (<-ss.Session().Get(s2.Id)).Err

34
store/storetest/user_access_token_store.go
View File

@@ -8,6 +8,7 @@ import (
"github.com/mattermost/mattermost-server/model"
"github.com/mattermost/mattermost-server/store"
"github.com/stretchr/testify/require"
)
func TestUserAccessTokenStore(t *testing.T, ss store.Store) {
@@ -23,11 +24,12 @@ func testUserAccessTokenSaveGetDelete(t *testing.T, ss store.Store) {
Description: "testtoken",
}
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = uat.UserId
s1.Token = uat.Token
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
if result := <-ss.UserAccessToken().Save(uat); result.Err != nil {
t.Fatal(result.Err)
@@ -65,19 +67,20 @@ func testUserAccessTokenSaveGetDelete(t *testing.T, ss store.Store) {
t.Fatal(result.Err)
}
if err := (<-ss.Session().Get(s1.Token)).Err; err == nil {
if err = (<-ss.Session().Get(s1.Token)).Err; err == nil {
t.Fatal("should error - session should be deleted")
}
if err := (<-ss.UserAccessToken().GetByToken(s1.Token)).Err; err == nil {
if err = (<-ss.UserAccessToken().GetByToken(s1.Token)).Err; err == nil {
t.Fatal("should error - access token should be deleted")
}
s2 := model.Session{}
s2 := &model.Session{}
s2.UserId = uat.UserId
s2.Token = uat.Token
store.Must(ss.Session().Save(&s2))
s2, err = ss.Session().Save(s2)
require.Nil(t, err)
if result := <-ss.UserAccessToken().Save(uat); result.Err != nil {
t.Fatal(result.Err)
@@ -103,29 +106,31 @@ func testUserAccessTokenDisableEnable(t *testing.T, ss store.Store) {
Description: "testtoken",
}
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = uat.UserId
s1.Token = uat.Token
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
if result := <-ss.UserAccessToken().Save(uat); result.Err != nil {
t.Fatal(result.Err)
}
if err := (<-ss.UserAccessToken().UpdateTokenDisable(uat.Id)).Err; err != nil {
if err = (<-ss.UserAccessToken().UpdateTokenDisable(uat.Id)).Err; err != nil {
t.Fatal(err)
}
if err := (<-ss.Session().Get(s1.Token)).Err; err == nil {
if err = (<-ss.Session().Get(s1.Token)).Err; err == nil {
t.Fatal("should error - session should be deleted")
}
s2 := model.Session{}
s2 := &model.Session{}
s2.UserId = uat.UserId
s2.Token = uat.Token
store.Must(ss.Session().Save(&s2))
s2, err = ss.Session().Save(s2)
require.Nil(t, err)
if err := (<-ss.UserAccessToken().UpdateTokenEnable(uat.Id)).Err; err != nil {
t.Fatal(err)
@@ -145,11 +150,12 @@ func testUserAccessTokenSearch(t *testing.T, ss store.Store) {
Description: "testtoken",
}
s1 := model.Session{}
s1 := &model.Session{}
s1.UserId = uat.UserId
s1.Token = uat.Token
store.Must(ss.Session().Save(&s1))
s1, err := ss.Session().Save(s1)
require.Nil(t, err)
if result := <-ss.UserAccessToken().Save(uat); result.Err != nil {
t.Fatal(result.Err)