MM-62160: Avoid SELECT * in user_terms_of_service.go and terms_of_service_store.go (#30423)

* MM-62160: Avoid SELECT * in user_terms_of_service.go

- Added userTermsOfServiceSelectQuery in the constructor
- Replaced raw SQL query with query builder pattern
- Used explicit column selection instead of SELECT *
- Made the implementation more resilient to schema changes

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>

* MM-62160: Also avoid SELECT * in terms_of_service_store.go

- Added termsOfServiceSelectQuery field to store struct
- Replaced SELECT * with explicit column selection
- Updated GetLatest and Get methods to use the query builder pattern
- Made the implementation more resilient to schema changes

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Jesse Hallam 2025-03-28 15:44:52 -03:00 committed by GitHub
parent 0079289224
commit b1a8810bd8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 35 additions and 27 deletions

View file

@ -6,6 +6,7 @@ package sqlstore
import (
"database/sql"
sq "github.com/mattermost/squirrel"
"github.com/pkg/errors"
"github.com/mattermost/mattermost/server/public/model"
@ -16,10 +17,21 @@ import (
type SqlTermsOfServiceStore struct {
*SqlStore
metrics einterfaces.MetricsInterface
termsOfServiceSelectQuery sq.SelectBuilder
}
func newSqlTermsOfServiceStore(sqlStore *SqlStore, metrics einterfaces.MetricsInterface) store.TermsOfServiceStore {
return SqlTermsOfServiceStore{sqlStore, metrics}
s := SqlTermsOfServiceStore{
SqlStore: sqlStore,
metrics: metrics,
}
s.termsOfServiceSelectQuery = s.getQueryBuilder().
Select("Id", "CreateAt", "UserId", "Text").
From("TermsOfService")
return s
}
func (s SqlTermsOfServiceStore) Save(termsOfService *model.TermsOfService) (*model.TermsOfService, error) {
@ -48,18 +60,11 @@ func (s SqlTermsOfServiceStore) Save(termsOfService *model.TermsOfService) (*mod
func (s SqlTermsOfServiceStore) GetLatest(allowFromCache bool) (*model.TermsOfService, error) {
var termsOfService model.TermsOfService
query := s.getQueryBuilder().
Select("*").
From("TermsOfService").
query := s.termsOfServiceSelectQuery.
OrderBy("CreateAt DESC").
Limit(uint64(1))
queryString, args, err := query.ToSql()
if err != nil {
return nil, errors.Wrap(err, "could not build sql query to get latest TOS")
}
if err := s.GetReplica().Get(&termsOfService, queryString, args...); err != nil {
if err := s.GetReplica().GetBuilder(&termsOfService, query); err != nil {
if err == sql.ErrNoRows {
return nil, store.NewErrNotFound("TermsOfService", "CreateAt=latest")
}
@ -71,18 +76,11 @@ func (s SqlTermsOfServiceStore) GetLatest(allowFromCache bool) (*model.TermsOfSe
func (s SqlTermsOfServiceStore) Get(id string, allowFromCache bool) (*model.TermsOfService, error) {
var termsOfService model.TermsOfService
queryString, _, err := s.getQueryBuilder().
Select("*").
From("TermsOfService").
Where("id = ?").
ToSql()
if err != nil {
return nil, errors.Wrap(err, "terms_of_service_to_sql")
}
query := s.termsOfServiceSelectQuery.
Where(sq.Eq{"Id": id})
err = s.GetReplica().Get(&termsOfService, queryString, id)
if err != nil {
if err := s.GetReplica().GetBuilder(&termsOfService, query); err != nil {
if err == sql.ErrNoRows {
return nil, store.NewErrNotFound("TermsOfService", "id")
}

View file

@ -6,6 +6,7 @@ package sqlstore
import (
"database/sql"
sq "github.com/mattermost/squirrel"
"github.com/pkg/errors"
"github.com/mattermost/mattermost/server/public/model"
@ -14,20 +15,29 @@ import (
type SqlUserTermsOfServiceStore struct {
*SqlStore
userTermsOfServiceSelectQuery sq.SelectBuilder
}
func newSqlUserTermsOfServiceStore(sqlStore *SqlStore) store.UserTermsOfServiceStore {
return SqlUserTermsOfServiceStore{sqlStore}
s := SqlUserTermsOfServiceStore{
SqlStore: sqlStore,
}
s.userTermsOfServiceSelectQuery = s.getQueryBuilder().
Select("UserId", "TermsOfServiceId", "CreateAt").
From("UserTermsOfService")
return s
}
func (s SqlUserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfService, error) {
var userTermsOfService model.UserTermsOfService
query := `
SELECT *
FROM UserTermsOfService
WHERE UserId = ?
`
if err := s.GetReplica().Get(&userTermsOfService, query, userId); err != nil {
query := s.userTermsOfServiceSelectQuery.
Where(sq.Eq{"UserId": userId})
if err := s.GetReplica().GetBuilder(&userTermsOfService, query); err != nil {
if err == sql.ErrNoRows {
return nil, store.NewErrNotFound("UserTermsOfService", "userId="+userId)
}