From 812c40a30703efd159675a1ff1b26a64f18b14d0 Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Mon, 4 Nov 2019 13:47:59 +0100 Subject: [PATCH] Adjust govet settings and fix issues found by it (#12947) --- .golangci.yml | 5 +++-- api4/user.go | 5 ----- app/config.go | 12 ++++++++---- app/session.go | 3 +-- store/sqlstore/audit_store.go | 6 +++--- store/sqlstore/compliance_store.go | 2 +- store/sqlstore/emoji_store.go | 2 +- store/sqlstore/file_info_store.go | 11 +++++++---- store/sqlstore/group_store.go | 4 ++-- store/sqlstore/supplier_reactions.go | 4 ++-- store/sqlstore/user_store.go | 21 +++++++++------------ 11 files changed, 37 insertions(+), 38 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 6788712517..7fe08ea5ec 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,10 +3,11 @@ run: modules-download-mode: vendor linters-settings: - govet: - check-shadowing: true gofmt: simplify: true + govet: + check-shadowing: true + enable-all: true linters: disable-all: true diff --git a/api4/user.go b/api4/user.go index 06c44975fc..25984727f5 100644 --- a/api4/user.go +++ b/api4/user.go @@ -925,11 +925,6 @@ func patchUser(c *Context, w http.ResponseWriter, r *http.Request) { } if c.App.Session.IsOAuth && patch.Email != nil { - if err != nil { - c.Err = err - return - } - if ouser.Email != *patch.Email { c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS) c.Err.DetailedError += ", attempted email update by oauth app" diff --git a/app/config.go b/app/config.go index 3537ff7a44..b86dcf5abc 100644 --- a/app/config.go +++ b/app/config.go @@ -138,8 +138,10 @@ func (a *App) ensurePostActionCookieSecret() error { return err } system.Value = string(v) - if err = a.Srv.Store.System().Save(system); err == nil { - // If we were able to save the key, use it, otherwise ignore the error. + // If we were able to save the key, use it, otherwise log the error. + if appErr := a.Srv.Store.System().Save(system); appErr != nil { + mlog.Error("Failed to save PostActionCookieSecret", mlog.Err(appErr)) + } else { secret = newSecret } } @@ -199,8 +201,10 @@ func (a *App) ensureAsymmetricSigningKey() error { return err } system.Value = string(v) - if err = a.Srv.Store.System().Save(system); err == nil { - // If we were able to save the key, use it, otherwise ignore the error. + // If we were able to save the key, use it, otherwise log the error. + if appErr := a.Srv.Store.System().Save(system); appErr != nil { + mlog.Error("Failed to save AsymmetricSigningKey", mlog.Err(appErr)) + } else { key = newKey } } diff --git a/app/session.go b/app/session.go index dcfde84102..fa51e255d9 100644 --- a/app/session.go +++ b/app/session.go @@ -72,8 +72,7 @@ func (a *App) GetSession(token string) (*model.Session, *model.AppError) { return nil, model.NewAppError("GetSession", "api.context.invalid_token.error", map[string]interface{}{"Token": token}, "", http.StatusUnauthorized) } - if session != nil && - *a.Config().ServiceSettings.SessionIdleTimeoutInMinutes > 0 && + if *a.Config().ServiceSettings.SessionIdleTimeoutInMinutes > 0 && !session.IsOAuth && session.Props[model.SESSION_PROP_TYPE] != model.SESSION_TYPE_USER_ACCESS_TOKEN { diff --git a/store/sqlstore/audit_store.go b/store/sqlstore/audit_store.go index 98758b8907..65d8bee87a 100644 --- a/store/sqlstore/audit_store.go +++ b/store/sqlstore/audit_store.go @@ -85,9 +85,9 @@ func (s SqlAuditStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, return 0, model.NewAppError("SqlAuditStore.PermanentDeleteBatch", "store.sql_audit.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) } - rowsAffected, err1 := sqlResult.RowsAffected() - if err1 != nil { - return 0, model.NewAppError("SqlAuditStore.PermanentDeleteBatch", "store.sql_audit.permanent_delete_batch.app_error", nil, ""+err1.Error(), http.StatusInternalServerError) + rowsAffected, err := sqlResult.RowsAffected() + if err != nil { + return 0, model.NewAppError("SqlAuditStore.PermanentDeleteBatch", "store.sql_audit.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) } return rowsAffected, nil } diff --git a/store/sqlstore/compliance_store.go b/store/sqlstore/compliance_store.go index 35ff5f4039..b41884d3a1 100644 --- a/store/sqlstore/compliance_store.go +++ b/store/sqlstore/compliance_store.go @@ -75,7 +75,7 @@ func (us SqlComplianceStore) Get(id string) (*model.Compliance, *model.AppError) return nil, model.NewAppError("SqlComplianceStore.Get", "store.sql_compliance.get.finding.app_error", nil, err.Error(), http.StatusInternalServerError) } if obj == nil { - return nil, model.NewAppError("SqlComplianceStore.Get", "store.sql_compliance.get.finding.app_error", nil, err.Error(), http.StatusNotFound) + return nil, model.NewAppError("SqlComplianceStore.Get", "store.sql_compliance.get.finding.app_error", nil, "", http.StatusNotFound) } return obj.(*model.Compliance), nil } diff --git a/store/sqlstore/emoji_store.go b/store/sqlstore/emoji_store.go index 533f88e1f5..60868b326c 100644 --- a/store/sqlstore/emoji_store.go +++ b/store/sqlstore/emoji_store.go @@ -136,7 +136,7 @@ func (es SqlEmojiStore) Delete(emoji *model.Emoji, time int64) *model.AppError { AND DeleteAt = 0`, map[string]interface{}{"DeleteAt": time, "UpdateAt": time, "Id": emoji.Id}); err != nil { return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.app_error", nil, "id="+emoji.Id+", err="+err.Error(), http.StatusInternalServerError) } else if rows, _ := sqlResult.RowsAffected(); rows == 0 { - return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.no_results", nil, "id="+emoji.Id+", err="+err.Error(), http.StatusBadRequest) + return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.no_results", nil, "id="+emoji.Id, http.StatusBadRequest) } es.removeFromCache(emoji) diff --git a/store/sqlstore/file_info_store.go b/store/sqlstore/file_info_store.go index 156840968d..99b67438d7 100644 --- a/store/sqlstore/file_info_store.go +++ b/store/sqlstore/file_info_store.go @@ -266,10 +266,12 @@ func (s SqlFileInfoStore) PermanentDeleteBatch(endTime int64, limit int64) (int6 if err != nil { return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteBatch", "store.sql_file_info.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) } - rowsAffected, err1 := sqlResult.RowsAffected() - if err1 != nil { + + rowsAffected, err := sqlResult.RowsAffected() + if err != nil { return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteBatch", "store.sql_file_info.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) } + return rowsAffected, nil } @@ -281,9 +283,10 @@ func (s SqlFileInfoStore) PermanentDeleteByUser(userId string) (int64, *model.Ap return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteByUser", "store.sql_file_info.PermanentDeleteByUser.app_error", nil, ""+err.Error(), http.StatusInternalServerError) } - rowsAffected, err1 := sqlResult.RowsAffected() - if err1 != nil { + rowsAffected, err := sqlResult.RowsAffected() + if err != nil { return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteByUser", "store.sql_file_info.PermanentDeleteByUser.app_error", nil, ""+err.Error(), http.StatusInternalServerError) } + return rowsAffected, nil } diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index f0e4555ff3..796b38e414 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -586,7 +586,7 @@ func (s *SqlGroupStore) UpdateGroupSyncable(groupSyncable *model.GroupSyncable) case model.GroupSyncableTypeChannel: _, err = s.GetMaster().Update(groupSyncableToGroupChannel(groupSyncable)) default: - return nil, model.NewAppError("SqlGroupStore.GroupUpdateGroupSyncable", "model.group_syncable.type.app_error", nil, "group_id="+groupSyncable.GroupId+", syncable_id="+groupSyncable.SyncableId+", "+err.Error(), http.StatusInternalServerError) + return nil, model.NewAppError("SqlGroupStore.GroupUpdateGroupSyncable", "model.group_syncable.type.app_error", nil, "group_id="+groupSyncable.GroupId+", syncable_id="+groupSyncable.SyncableId, http.StatusInternalServerError) } if err != nil { @@ -619,7 +619,7 @@ func (s *SqlGroupStore) DeleteGroupSyncable(groupID string, syncableID string, s case model.GroupSyncableTypeChannel: _, err = s.GetMaster().Update(groupSyncableToGroupChannel(groupSyncable)) default: - return nil, model.NewAppError("SqlGroupStore.GroupDeleteGroupSyncable", "model.group_syncable.type.app_error", nil, "group_id="+groupSyncable.GroupId+", syncable_id="+groupSyncable.SyncableId+", "+err.Error(), http.StatusInternalServerError) + return nil, model.NewAppError("SqlGroupStore.GroupDeleteGroupSyncable", "model.group_syncable.type.app_error", nil, "group_id="+groupSyncable.GroupId+", syncable_id="+groupSyncable.SyncableId, http.StatusInternalServerError) } if err != nil { diff --git a/store/sqlstore/supplier_reactions.go b/store/sqlstore/supplier_reactions.go index e3822af708..7128c76a7e 100644 --- a/store/sqlstore/supplier_reactions.go +++ b/store/sqlstore/supplier_reactions.go @@ -160,8 +160,8 @@ func (s *SqlReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int return 0, model.NewAppError("SqlReactionStore.PermanentDeleteBatch", "store.sql_reaction.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) } - rowsAffected, err1 := sqlResult.RowsAffected() - if err1 != nil { + rowsAffected, err := sqlResult.RowsAffected() + if err != nil { return 0, model.NewAppError("SqlReactionStore.PermanentDeleteBatch", "store.sql_reaction.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) } return rowsAffected, nil diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 5f15cf23b1..84f5911a07 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -1448,10 +1448,9 @@ func (us SqlUserStore) GetUsersBatchForIndexing(startTime, endTime int64, limit OrderBy("u.CreateAt"). Limit(uint64(limit)). ToSql() - _, err1 := us.GetSearchReplica().Select(&users, usersQuery, args...) - - if err1 != nil { - return nil, model.NewAppError("SqlUserStore.GetUsersBatchForIndexing", "store.sql_user.get_users_batch_for_indexing.get_users.app_error", nil, err1.Error(), http.StatusInternalServerError) + _, err := us.GetSearchReplica().Select(&users, usersQuery, args...) + if err != nil { + return nil, model.NewAppError("SqlUserStore.GetUsersBatchForIndexing", "store.sql_user.get_users_batch_for_indexing.get_users.app_error", nil, err.Error(), http.StatusInternalServerError) } userIds := []string{} @@ -1478,10 +1477,9 @@ func (us SqlUserStore) GetUsersBatchForIndexing(startTime, endTime int64, limit Join("Channels c ON cm.ChannelId = c.Id"). Where(sq.Eq{"c.Type": "O", "cm.UserId": userIds}). ToSql() - _, err2 := us.GetSearchReplica().Select(&channelMembers, channelMembersQuery, args...) - - if err2 != nil { - return nil, model.NewAppError("SqlUserStore.GetUsersBatchForIndexing", "store.sql_user.get_users_batch_for_indexing.get_channel_members.app_error", nil, err2.Error(), http.StatusInternalServerError) + _, err = us.GetSearchReplica().Select(&channelMembers, channelMembersQuery, args...) + if err != nil { + return nil, model.NewAppError("SqlUserStore.GetUsersBatchForIndexing", "store.sql_user.get_users_batch_for_indexing.get_channel_members.app_error", nil, err.Error(), http.StatusInternalServerError) } var teamMembers []*model.TeamMember @@ -1490,10 +1488,9 @@ func (us SqlUserStore) GetUsersBatchForIndexing(startTime, endTime int64, limit From("TeamMembers"). Where(sq.Eq{"UserId": userIds, "DeleteAt": 0}). ToSql() - _, err3 := us.GetSearchReplica().Select(&teamMembers, teamMembersQuery, args...) - - if err3 != nil { - return nil, model.NewAppError("SqlUserStore.GetUsersBatchForIndexing", "store.sql_user.get_users_batch_for_indexing.get_team_members.app_error", nil, err3.Error(), http.StatusInternalServerError) + _, err = us.GetSearchReplica().Select(&teamMembers, teamMembersQuery, args...) + if err != nil { + return nil, model.NewAppError("SqlUserStore.GetUsersBatchForIndexing", "store.sql_user.get_users_batch_for_indexing.get_team_members.app_error", nil, err.Error(), http.StatusInternalServerError) } userMap := map[string]*model.UserForIndexing{}