SystemStore migration to return plain errors (#14835)

* SystemStore migration to return plain errors

* Fix nilness

* Fix translations

* Fix merge

* Fix layers

* Fix merge errors

* Lint: remove unnecessary use of sprint

* Fix merge errors

* Fix i18n

Co-authored-by: Agniva De Sarker <agnivade@yahoo.co.in>
Этот коммит содержится в:
Rodrigo Villablanca
2020-08-13 11:02:57 -04:00
коммит произвёл GitHub
родитель e8d431ee7c
Коммит 20e44399c7
18 изменённых файлов: 252 добавлений и 173 удалений

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

@@ -6332,7 +6332,7 @@ func (s *OpenTracingLayerStatusStore) UpdateLastActivityAt(userId string, lastAc
return err
}
func (s *OpenTracingLayerSystemStore) Get() (model.StringMap, *model.AppError) {
func (s *OpenTracingLayerSystemStore) Get() (model.StringMap, error) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.Get")
s.Root.Store.SetContext(newCtx)
@@ -6350,7 +6350,7 @@ func (s *OpenTracingLayerSystemStore) Get() (model.StringMap, *model.AppError) {
return result, err
}
func (s *OpenTracingLayerSystemStore) GetByName(name string) (*model.System, *model.AppError) {
func (s *OpenTracingLayerSystemStore) GetByName(name string) (*model.System, error) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.GetByName")
s.Root.Store.SetContext(newCtx)
@@ -6368,7 +6368,7 @@ func (s *OpenTracingLayerSystemStore) GetByName(name string) (*model.System, *mo
return result, err
}
func (s *OpenTracingLayerSystemStore) InsertIfExists(system *model.System) (*model.System, *model.AppError) {
func (s *OpenTracingLayerSystemStore) InsertIfExists(system *model.System) (*model.System, error) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.InsertIfExists")
s.Root.Store.SetContext(newCtx)
@@ -6386,7 +6386,7 @@ func (s *OpenTracingLayerSystemStore) InsertIfExists(system *model.System) (*mod
return result, err
}
func (s *OpenTracingLayerSystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) {
func (s *OpenTracingLayerSystemStore) PermanentDeleteByName(name string) (*model.System, error) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.PermanentDeleteByName")
s.Root.Store.SetContext(newCtx)
@@ -6404,7 +6404,7 @@ func (s *OpenTracingLayerSystemStore) PermanentDeleteByName(name string) (*model
return result, err
}
func (s *OpenTracingLayerSystemStore) Save(system *model.System) *model.AppError {
func (s *OpenTracingLayerSystemStore) Save(system *model.System) error {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.Save")
s.Root.Store.SetContext(newCtx)
@@ -6422,7 +6422,7 @@ func (s *OpenTracingLayerSystemStore) Save(system *model.System) *model.AppError
return err
}
func (s *OpenTracingLayerSystemStore) SaveOrUpdate(system *model.System) *model.AppError {
func (s *OpenTracingLayerSystemStore) SaveOrUpdate(system *model.System) error {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.SaveOrUpdate")
s.Root.Store.SetContext(newCtx)
@@ -6440,7 +6440,7 @@ func (s *OpenTracingLayerSystemStore) SaveOrUpdate(system *model.System) *model.
return err
}
func (s *OpenTracingLayerSystemStore) Update(system *model.System) *model.AppError {
func (s *OpenTracingLayerSystemStore) Update(system *model.System) error {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.Update")
s.Root.Store.SetContext(newCtx)

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

@@ -4630,45 +4630,143 @@ func (s *RetryLayerStatusStore) UpdateLastActivityAt(userId string, lastActivity
}
func (s *RetryLayerSystemStore) Get() (model.StringMap, *model.AppError) {
func (s *RetryLayerSystemStore) Get() (model.StringMap, error) {
return s.SystemStore.Get()
tries := 0
for {
result, err := s.SystemStore.Get()
if err == nil {
return result, err
}
if !isRepeatableError(err) {
return result, err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return result, err
}
}
}
func (s *RetryLayerSystemStore) GetByName(name string) (*model.System, *model.AppError) {
func (s *RetryLayerSystemStore) GetByName(name string) (*model.System, error) {
return s.SystemStore.GetByName(name)
tries := 0
for {
result, err := s.SystemStore.GetByName(name)
if err == nil {
return result, err
}
if !isRepeatableError(err) {
return result, err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return result, err
}
}
}
func (s *RetryLayerSystemStore) InsertIfExists(system *model.System) (*model.System, *model.AppError) {
func (s *RetryLayerSystemStore) InsertIfExists(system *model.System) (*model.System, error) {
return s.SystemStore.InsertIfExists(system)
tries := 0
for {
result, err := s.SystemStore.InsertIfExists(system)
if err == nil {
return result, err
}
if !isRepeatableError(err) {
return result, err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return result, err
}
}
}
func (s *RetryLayerSystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) {
func (s *RetryLayerSystemStore) PermanentDeleteByName(name string) (*model.System, error) {
return s.SystemStore.PermanentDeleteByName(name)
tries := 0
for {
result, err := s.SystemStore.PermanentDeleteByName(name)
if err == nil {
return result, err
}
if !isRepeatableError(err) {
return result, err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return result, err
}
}
}
func (s *RetryLayerSystemStore) Save(system *model.System) *model.AppError {
func (s *RetryLayerSystemStore) Save(system *model.System) error {
return s.SystemStore.Save(system)
tries := 0
for {
err := s.SystemStore.Save(system)
if err == nil {
return err
}
if !isRepeatableError(err) {
return err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return err
}
}
}
func (s *RetryLayerSystemStore) SaveOrUpdate(system *model.System) *model.AppError {
func (s *RetryLayerSystemStore) SaveOrUpdate(system *model.System) error {
return s.SystemStore.SaveOrUpdate(system)
tries := 0
for {
err := s.SystemStore.SaveOrUpdate(system)
if err == nil {
return err
}
if !isRepeatableError(err) {
return err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return err
}
}
}
func (s *RetryLayerSystemStore) Update(system *model.System) *model.AppError {
func (s *RetryLayerSystemStore) Update(system *model.System) error {
return s.SystemStore.Update(system)
tries := 0
for {
err := s.SystemStore.Update(system)
if err == nil {
return err
}
if !isRepeatableError(err) {
return err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return err
}
}
}

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

@@ -6,10 +6,11 @@ package sqlstore
import (
"context"
"database/sql"
"net/http"
"github.com/mattermost/mattermost-server/v5/model"
"github.com/mattermost/mattermost-server/v5/store"
"github.com/pkg/errors"
)
type SqlSystemStore struct {
@@ -31,38 +32,38 @@ func newSqlSystemStore(sqlStore SqlStore) store.SystemStore {
func (s SqlSystemStore) createIndexesIfNotExists() {
}
func (s SqlSystemStore) Save(system *model.System) *model.AppError {
func (s SqlSystemStore) Save(system *model.System) error {
if err := s.GetMaster().Insert(system); err != nil {
return model.NewAppError("SqlSystemStore.Save", "store.sql_system.save.app_error", nil, err.Error(), http.StatusInternalServerError)
return errors.Wrapf(err, "failed to save system property with name=%s", system.Name)
}
return nil
}
func (s SqlSystemStore) SaveOrUpdate(system *model.System) *model.AppError {
func (s SqlSystemStore) SaveOrUpdate(system *model.System) error {
if err := s.GetMaster().SelectOne(&model.System{}, "SELECT * FROM Systems WHERE Name = :Name", map[string]interface{}{"Name": system.Name}); err == nil {
if _, err := s.GetMaster().Update(system); err != nil {
return model.NewAppError("SqlSystemStore.SaveOrUpdate", "store.sql_system.update.app_error", nil, err.Error(), http.StatusInternalServerError)
return errors.Wrapf(err, "failed to update system property with name=%s", system.Name)
}
} else {
if err := s.GetMaster().Insert(system); err != nil {
return model.NewAppError("SqlSystemStore.SaveOrUpdate", "store.sql_system.save.app_error", nil, err.Error(), http.StatusInternalServerError)
return errors.Wrapf(err, "failed to save system property with name=%s", system.Name)
}
}
return nil
}
func (s SqlSystemStore) Update(system *model.System) *model.AppError {
func (s SqlSystemStore) Update(system *model.System) error {
if _, err := s.GetMaster().Update(system); err != nil {
return model.NewAppError("SqlSystemStore.Update", "store.sql_system.update.app_error", nil, err.Error(), http.StatusInternalServerError)
return errors.Wrapf(err, "failed to update system property with name=%s", system.Name)
}
return nil
}
func (s SqlSystemStore) Get() (model.StringMap, *model.AppError) {
func (s SqlSystemStore) Get() (model.StringMap, error) {
var systems []model.System
props := make(model.StringMap)
if _, err := s.GetReplica().Select(&systems, "SELECT * FROM Systems"); err != nil {
return nil, model.NewAppError("SqlSystemStore.Get", "store.sql_system.get.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "failed to system properties")
}
for _, prop := range systems {
props[prop.Name] = prop.Value
@@ -71,19 +72,19 @@ func (s SqlSystemStore) Get() (model.StringMap, *model.AppError) {
return props, nil
}
func (s SqlSystemStore) GetByName(name string) (*model.System, *model.AppError) {
func (s SqlSystemStore) GetByName(name string) (*model.System, error) {
var system model.System
if err := s.GetMaster().SelectOne(&system, "SELECT * FROM Systems WHERE Name = :Name", map[string]interface{}{"Name": name}); err != nil {
return nil, model.NewAppError("SqlSystemStore.GetByName", "store.sql_system.get_by_name.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrapf(err, "failed to get system property with name=%s", system.Name)
}
return &system, nil
}
func (s SqlSystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) {
func (s SqlSystemStore) PermanentDeleteByName(name string) (*model.System, error) {
var system model.System
if _, err := s.GetMaster().Exec("DELETE FROM Systems WHERE Name = :Name", map[string]interface{}{"Name": name}); err != nil {
return nil, model.NewAppError("SqlSystemStore.PermanentDeleteByName", "store.sql_system.permanent_delete_by_name.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrapf(err, "failed to permanent delete system property with name=%s", system.Name)
}
return &system, nil
@@ -91,12 +92,12 @@ func (s SqlSystemStore) PermanentDeleteByName(name string) (*model.System, *mode
// InsertIfExists inserts a given system value if it does not already exist. If a value
// already exists, it returns the old one, else returns the new one.
func (s SqlSystemStore) InsertIfExists(system *model.System) (*model.System, *model.AppError) {
func (s SqlSystemStore) InsertIfExists(system *model.System) (*model.System, error) {
tx, err := s.GetMaster().BeginTx(context.Background(), &sql.TxOptions{
Isolation: sql.LevelSerializable,
})
if err != nil {
return nil, model.NewAppError("SqlSystemStore.InsertIfExists", "store.sql_system.save.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "begin_transaction")
}
defer finalizeTransaction(tx)
@@ -104,7 +105,7 @@ func (s SqlSystemStore) InsertIfExists(system *model.System) (*model.System, *mo
if err := tx.SelectOne(&origSystem, `SELECT * FROM Systems
WHERE Name = :Name`,
map[string]interface{}{"Name": system.Name}); err != nil && err != sql.ErrNoRows {
return nil, model.NewAppError("SqlSystemStore.InsertIfExists", "store.sql_system.get_by_name.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrapf(err, "failed to get system property with name=%s", system.Name)
}
if origSystem.Value != "" {
@@ -114,11 +115,11 @@ func (s SqlSystemStore) InsertIfExists(system *model.System) (*model.System, *mo
// Key does not exist, need to insert.
if err := tx.Insert(system); err != nil {
return nil, model.NewAppError("SqlSystemStore.InsertIfExists", "store.sql_system.save.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrapf(err, "failed to save system property with name=%s", system.Name)
}
if err := tx.Commit(); err != nil {
return nil, model.NewAppError("SqlSystemStore.InsertIfExists", "store.sql_system.save.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "commit_transaction")
}
return system, nil
}

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

@@ -429,13 +429,13 @@ type OAuthStore interface {
}
type SystemStore interface {
Save(system *model.System) *model.AppError
SaveOrUpdate(system *model.System) *model.AppError
Update(system *model.System) *model.AppError
Get() (model.StringMap, *model.AppError)
GetByName(name string) (*model.System, *model.AppError)
PermanentDeleteByName(name string) (*model.System, *model.AppError)
InsertIfExists(system *model.System) (*model.System, *model.AppError)
Save(system *model.System) error
SaveOrUpdate(system *model.System) error
Update(system *model.System) error
Get() (model.StringMap, error)
GetByName(name string) (*model.System, error)
PermanentDeleteByName(name string) (*model.System, error)
InsertIfExists(system *model.System) (*model.System, error)
}
type WebhookStore interface {

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

@@ -15,7 +15,7 @@ type SystemStore struct {
}
// Get provides a mock function with given fields:
func (_m *SystemStore) Get() (model.StringMap, *model.AppError) {
func (_m *SystemStore) Get() (model.StringMap, error) {
ret := _m.Called()
var r0 model.StringMap
@@ -27,20 +27,18 @@ func (_m *SystemStore) Get() (model.StringMap, *model.AppError) {
}
}
var r1 *model.AppError
if rf, ok := ret.Get(1).(func() *model.AppError); ok {
var r1 error
if rf, ok := ret.Get(1).(func() error); ok {
r1 = rf()
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*model.AppError)
}
r1 = ret.Error(1)
}
return r0, r1
}
// GetByName provides a mock function with given fields: name
func (_m *SystemStore) GetByName(name string) (*model.System, *model.AppError) {
func (_m *SystemStore) GetByName(name string) (*model.System, error) {
ret := _m.Called(name)
var r0 *model.System
@@ -52,20 +50,18 @@ func (_m *SystemStore) GetByName(name string) (*model.System, *model.AppError) {
}
}
var r1 *model.AppError
if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
var r1 error
if rf, ok := ret.Get(1).(func(string) error); ok {
r1 = rf(name)
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*model.AppError)
}
r1 = ret.Error(1)
}
return r0, r1
}
// InsertIfExists provides a mock function with given fields: system
func (_m *SystemStore) InsertIfExists(system *model.System) (*model.System, *model.AppError) {
func (_m *SystemStore) InsertIfExists(system *model.System) (*model.System, error) {
ret := _m.Called(system)
var r0 *model.System
@@ -77,20 +73,18 @@ func (_m *SystemStore) InsertIfExists(system *model.System) (*model.System, *mod
}
}
var r1 *model.AppError
if rf, ok := ret.Get(1).(func(*model.System) *model.AppError); ok {
var r1 error
if rf, ok := ret.Get(1).(func(*model.System) error); ok {
r1 = rf(system)
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*model.AppError)
}
r1 = ret.Error(1)
}
return r0, r1
}
// PermanentDeleteByName provides a mock function with given fields: name
func (_m *SystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) {
func (_m *SystemStore) PermanentDeleteByName(name string) (*model.System, error) {
ret := _m.Called(name)
var r0 *model.System
@@ -102,61 +96,53 @@ func (_m *SystemStore) PermanentDeleteByName(name string) (*model.System, *model
}
}
var r1 *model.AppError
if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
var r1 error
if rf, ok := ret.Get(1).(func(string) error); ok {
r1 = rf(name)
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*model.AppError)
}
r1 = ret.Error(1)
}
return r0, r1
}
// Save provides a mock function with given fields: system
func (_m *SystemStore) Save(system *model.System) *model.AppError {
func (_m *SystemStore) Save(system *model.System) error {
ret := _m.Called(system)
var r0 *model.AppError
if rf, ok := ret.Get(0).(func(*model.System) *model.AppError); ok {
var r0 error
if rf, ok := ret.Get(0).(func(*model.System) error); ok {
r0 = rf(system)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*model.AppError)
}
r0 = ret.Error(0)
}
return r0
}
// SaveOrUpdate provides a mock function with given fields: system
func (_m *SystemStore) SaveOrUpdate(system *model.System) *model.AppError {
func (_m *SystemStore) SaveOrUpdate(system *model.System) error {
ret := _m.Called(system)
var r0 *model.AppError
if rf, ok := ret.Get(0).(func(*model.System) *model.AppError); ok {
var r0 error
if rf, ok := ret.Get(0).(func(*model.System) error); ok {
r0 = rf(system)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*model.AppError)
}
r0 = ret.Error(0)
}
return r0
}
// Update provides a mock function with given fields: system
func (_m *SystemStore) Update(system *model.System) *model.AppError {
func (_m *SystemStore) Update(system *model.System) error {
ret := _m.Called(system)
var r0 *model.AppError
if rf, ok := ret.Get(0).(func(*model.System) *model.AppError); ok {
var r0 error
if rf, ok := ret.Get(0).(func(*model.System) error); ok {
r0 = rf(system)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*model.AppError)
}
r0 = ret.Error(0)
}
return r0

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

@@ -111,7 +111,7 @@ func testInsertIfExists(t *testing.T, ss store.Store) {
go func() {
defer wg.Done()
s1 := &model.System{Name: model.SYSTEM_CLUSTER_ENCRYPTION_KEY, Value: "firstKey"}
var err *model.AppError
var err error
s2, err = ss.System().InsertIfExists(s1)
require.Nil(t, err)
}()
@@ -119,7 +119,7 @@ func testInsertIfExists(t *testing.T, ss store.Store) {
go func() {
defer wg.Done()
s1 := &model.System{Name: model.SYSTEM_CLUSTER_ENCRYPTION_KEY, Value: "secondKey"}
var err *model.AppError
var err error
s3, err = ss.System().InsertIfExists(s1)
require.Nil(t, err)
}()

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

@@ -5724,7 +5724,7 @@ func (s *TimerLayerStatusStore) UpdateLastActivityAt(userId string, lastActivity
return err
}
func (s *TimerLayerSystemStore) Get() (model.StringMap, *model.AppError) {
func (s *TimerLayerSystemStore) Get() (model.StringMap, error) {
start := timemodule.Now()
result, err := s.SystemStore.Get()
@@ -5740,7 +5740,7 @@ func (s *TimerLayerSystemStore) Get() (model.StringMap, *model.AppError) {
return result, err
}
func (s *TimerLayerSystemStore) GetByName(name string) (*model.System, *model.AppError) {
func (s *TimerLayerSystemStore) GetByName(name string) (*model.System, error) {
start := timemodule.Now()
result, err := s.SystemStore.GetByName(name)
@@ -5756,7 +5756,7 @@ func (s *TimerLayerSystemStore) GetByName(name string) (*model.System, *model.Ap
return result, err
}
func (s *TimerLayerSystemStore) InsertIfExists(system *model.System) (*model.System, *model.AppError) {
func (s *TimerLayerSystemStore) InsertIfExists(system *model.System) (*model.System, error) {
start := timemodule.Now()
result, err := s.SystemStore.InsertIfExists(system)
@@ -5772,7 +5772,7 @@ func (s *TimerLayerSystemStore) InsertIfExists(system *model.System) (*model.Sys
return result, err
}
func (s *TimerLayerSystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) {
func (s *TimerLayerSystemStore) PermanentDeleteByName(name string) (*model.System, error) {
start := timemodule.Now()
result, err := s.SystemStore.PermanentDeleteByName(name)
@@ -5788,7 +5788,7 @@ func (s *TimerLayerSystemStore) PermanentDeleteByName(name string) (*model.Syste
return result, err
}
func (s *TimerLayerSystemStore) Save(system *model.System) *model.AppError {
func (s *TimerLayerSystemStore) Save(system *model.System) error {
start := timemodule.Now()
err := s.SystemStore.Save(system)
@@ -5804,7 +5804,7 @@ func (s *TimerLayerSystemStore) Save(system *model.System) *model.AppError {
return err
}
func (s *TimerLayerSystemStore) SaveOrUpdate(system *model.System) *model.AppError {
func (s *TimerLayerSystemStore) SaveOrUpdate(system *model.System) error {
start := timemodule.Now()
err := s.SystemStore.SaveOrUpdate(system)
@@ -5820,7 +5820,7 @@ func (s *TimerLayerSystemStore) SaveOrUpdate(system *model.System) *model.AppErr
return err
}
func (s *TimerLayerSystemStore) Update(system *model.System) *model.AppError {
func (s *TimerLayerSystemStore) Update(system *model.System) error {
start := timemodule.Now()
err := s.SystemStore.Update(system)