[MM-24945] Refactor job store to use squirrel (#14559)

* Refactor job store to use squirrel * Using multiple Set calls instead of SetMap * Commiting suggestions. Co-authored-by: Miguel de la Cruz <mgdelacroix@gmail.com> * Commiting suggestions. Co-authored-by: Miguel de la Cruz <mgdelacroix@gmail.com> Co-authored-by: Dante Pippi <dantepippi@gmai.com> Co-authored-by: mattermod <mattermod@users.noreply.github.com> Co-authored-by: Miguel de la Cruz <mgdelacroix@gmail.com>
2020-06-02 06:42:31 -03:00
--- a/store/sqlstore/job_store.go
+++ b/store/sqlstore/job_store.go
@@ -7,6 +7,8 @@ import (
 	"database/sql"
 	"net/http"

+	sq "github.com/Masterminds/squirrel"
+
 	"github.com/mattermost/gorp"
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/store"
@@ -42,16 +44,17 @@ func (jss SqlJobStore) Save(job *model.Job) (*model.Job, *model.AppError) {
 }

 func (jss SqlJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) {
-	query := "UPDATE Jobs SET LastActivityAt = :LastActivityAt, Status = :Status, Progress = :Progress, Data = :Data WHERE Id = :Id AND Status = :OldStatus"
-	params := map[string]interface{}{
-		"Id":             job.Id,
-		"OldStatus":      currentStatus,
-		"LastActivityAt": model.GetMillis(),
-		"Status":         job.Status,
-		"Data":           job.DataToJson(),
-		"Progress":       job.Progress,
+	sql, args, err := jss.getQueryBuilder().
+		Update("Jobs").
+		Set("LastActivityAt", model.GetMillis()).
+		Set("Status", job.Status).
+		Set("Data", job.DataToJson()).
+		Set("Progress", job.Progress).
+		Where(sq.Eq{"Id": job.Id, "Status": currentStatus}).ToSql()
+	if err != nil {
+		return false, model.NewAppError("SqlJobStore.UpdateOptimistically", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
-	sqlResult, err := jss.GetMaster().Exec(query, params)
+	sqlResult, err := jss.GetMaster().Exec(sql, args...)
 	if err != nil {
 		return false, model.NewAppError("SqlJobStore.UpdateOptimistically", "store.sql_job.update.app_error", nil, "id="+job.Id+", "+err.Error(), http.StatusInternalServerError)
 	}
@@ -86,22 +89,23 @@ func (jss SqlJobStore) UpdateStatus(id string, status string) (*model.Job, *mode
 }

 func (jss SqlJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) {
-	var startAtClause string
+	sql := jss.getQueryBuilder().
+		Update("Jobs").
+		Set("LastActivityAt", model.GetMillis()).
+		Set("Status", newStatus).
+		Where(sq.Eq{"Id": id, "Status": currentStatus})
+
 	if newStatus == model.JOB_STATUS_IN_PROGRESS {
-		startAtClause = "StartAt = :StartAt,"
+		sql = sql.Set("StartAt", model.GetMillis())
 	}
-	query := "UPDATE Jobs SET " + startAtClause + " Status = :NewStatus, LastActivityAt = :LastActivityAt WHERE Id = :Id AND Status = :OldStatus"
-	params := map[string]interface{}{
-		"Id":             id,
-		"OldStatus":      currentStatus,
-		"NewStatus":      newStatus,
-		"StartAt":        model.GetMillis(),
-		"LastActivityAt": model.GetMillis(),
+	query, args, err := sql.ToSql()
+	if err != nil {
+		return false, model.NewAppError("SqlJobStore.UpdateStatusOptimistically", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}

-	sqlResult, err := jss.GetMaster().Exec(query, params)
+	sqlResult, err := jss.GetMaster().Exec(query, args...)
 	if err != nil {
-		return false, model.NewAppError("SqlJobStore.UpdateStatus", "store.sql_job.update.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError)
+		return false, model.NewAppError("SqlJobStore.UpdateStatusOptimistically", "store.sql_job.update.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError)
 	}
 	rows, err := sqlResult.RowsAffected()
 	if err != nil {
@@ -115,10 +119,15 @@ func (jss SqlJobStore) UpdateStatusOptimistically(id string, currentStatus strin
 }

 func (jss SqlJobStore) Get(id string) (*model.Job, *model.AppError) {
-	query := "SELECT * FROM Jobs WHERE Id = :Id"
-
+	query, args, err := jss.getQueryBuilder().
+		Select("*").
+		From("Jobs").
+		Where(sq.Eq{"Id": id}).ToSql()
+	if err != nil {
+		return nil, model.NewAppError("SqlJobStore.Get", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
 	var status *model.Job
-	if err := jss.GetReplica().SelectOne(&status, query, map[string]interface{}{"Id": id}); err != nil {
+	if err = jss.GetReplica().SelectOne(&status, query, args...); err != nil {
 		if err == sql.ErrNoRows {
 			return nil, model.NewAppError("SqlJobStore.Get", "store.sql_job.get.app_error", nil, "Id="+id+", "+err.Error(), http.StatusNotFound)
 		}
@@ -128,29 +137,53 @@ func (jss SqlJobStore) Get(id string) (*model.Job, *model.AppError) {
 }

 func (jss SqlJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) {
-	query := "SELECT * FROM Jobs ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset"
+	query, args, err := jss.getQueryBuilder().
+		Select("*").
+		From("Jobs").
+		OrderBy("CreateAt DESC").
+		Limit(uint64(limit)).
+		Offset(uint64(offset)).ToSql()
+	if err != nil {
+		return nil, model.NewAppError("SqlJobStore.GetAllPage", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}

 	var statuses []*model.Job
-	if _, err := jss.GetReplica().Select(&statuses, query, map[string]interface{}{"Limit": limit, "Offset": offset}); err != nil {
+	if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil {
 		return nil, model.NewAppError("SqlJobStore.GetAllPage", "store.sql_job.get_all.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 	return statuses, nil
 }

 func (jss SqlJobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) {
-	query := "SELECT * FROM Jobs WHERE Type = :Type ORDER BY CreateAt DESC"
+	query, args, err := jss.getQueryBuilder().
+		Select("*").
+		From("Jobs").
+		Where(sq.Eq{"Type": jobType}).
+		OrderBy("CreateAt DESC").ToSql()
+	if err != nil {
+		return nil, model.NewAppError("SqlJobStore.GetAllByType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
 	var statuses []*model.Job
-	if _, err := jss.GetReplica().Select(&statuses, query, map[string]interface{}{"Type": jobType}); err != nil {
+	if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil {
 		return nil, model.NewAppError("SqlJobStore.GetAllByType", "store.sql_job.get_all.app_error", nil, "Type="+jobType+", "+err.Error(), http.StatusInternalServerError)
 	}
 	return statuses, nil
 }

 func (jss SqlJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) {
-	query := "SELECT * FROM Jobs WHERE Type = :Type ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset"
+	query, args, err := jss.getQueryBuilder().
+		Select("*").
+		From("Jobs").
+		Where(sq.Eq{"Type": jobType}).
+		OrderBy("CreateAt DESC").
+		Limit(uint64(limit)).
+		Offset(uint64(offset)).ToSql()
+	if err != nil {
+		return nil, model.NewAppError("SqlJobStore.GetAllByTypePage", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}

 	var statuses []*model.Job
-	if _, err := jss.GetReplica().Select(&statuses, query, map[string]interface{}{"Type": jobType, "Limit": limit, "Offset": offset}); err != nil {
+	if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil {
 		return nil, model.NewAppError("SqlJobStore.GetAllByTypePage", "store.sql_job.get_all.app_error", nil, "Type="+jobType+", "+err.Error(), http.StatusInternalServerError)
 	}
 	return statuses, nil
@@ -158,27 +191,48 @@ func (jss SqlJobStore) GetAllByTypePage(jobType string, offset int, limit int) (

 func (jss SqlJobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError) {
 	var statuses []*model.Job
-	query := "SELECT * FROM Jobs WHERE Status = :Status ORDER BY CreateAt ASC"
+	query, args, err := jss.getQueryBuilder().
+		Select("*").
+		From("Jobs").
+		Where(sq.Eq{"Status": status}).
+		OrderBy("CreateAt ASC").ToSql()
+	if err != nil {
+		return nil, model.NewAppError("SqlJobStore.GetAllByStatus", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}

-	if _, err := jss.GetReplica().Select(&statuses, query, map[string]interface{}{"Status": status}); err != nil {
+	if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil {
 		return nil, model.NewAppError("SqlJobStore.GetAllByStatus", "store.sql_job.get_all.app_error", nil, "Status="+status+", "+err.Error(), http.StatusInternalServerError)
 	}
 	return statuses, nil
 }

 func (jss SqlJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) {
-	query := "SELECT * FROM Jobs WHERE Status = :Status AND Type = :Type ORDER BY CreateAt DESC LIMIT 1"
+	query, args, err := jss.getQueryBuilder().
+		Select("*").
+		From("Jobs").
+		Where(sq.Eq{"Status": status, "Type": jobType}).
+		OrderBy("CreateAt DESC").
+		Limit(1).ToSql()
+	if err != nil {
+		return nil, model.NewAppError("SqlJobStore.GetNewestJobByStatusAndType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}

 	var job *model.Job
-	if err := jss.GetReplica().SelectOne(&job, query, map[string]interface{}{"Status": status, "Type": jobType}); err != nil && err != sql.ErrNoRows {
+	if err = jss.GetReplica().SelectOne(&job, query, args...); err != nil && err != sql.ErrNoRows {
 		return nil, model.NewAppError("SqlJobStore.GetNewestJobByStatusAndType", "store.sql_job.get_newest_job_by_status_and_type.app_error", nil, "Status="+status+", "+err.Error(), http.StatusInternalServerError)
 	}
 	return job, nil
 }

 func (jss SqlJobStore) GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) {
-	query := "SELECT COUNT(*) FROM Jobs WHERE Status = :Status AND Type = :Type"
-	count, err := jss.GetReplica().SelectInt(query, map[string]interface{}{"Status": status, "Type": jobType})
+	query, args, err := jss.getQueryBuilder().
+		Select("COUNT(*)").
+		From("Jobs").
+		Where(sq.Eq{"Status": status, "Type": jobType}).ToSql()
+	if err != nil {
+		return 0, model.NewAppError("SqlJobStore.GetCountByStatusAndType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+	count, err := jss.GetReplica().SelectInt(query, args...)
 	if err != nil {
 		return int64(0), model.NewAppError("SqlJobStore.GetCountByStatusAndType", "store.sql_job.get_count_by_status_and_type.app_error", nil, "Status="+status+", "+err.Error(), http.StatusInternalServerError)
 	}
@@ -186,8 +240,14 @@ func (jss SqlJobStore) GetCountByStatusAndType(status string, jobType string) (i
 }

 func (jss SqlJobStore) Delete(id string) (string, *model.AppError) {
-	query := "DELETE FROM Jobs WHERE Id = :Id"
-	if _, err := jss.GetMaster().Exec(query, map[string]interface{}{"Id": id}); err != nil {
+	sql, args, err := jss.getQueryBuilder().
+		Delete("Jobs").
+		Where(sq.Eq{"Id": id}).ToSql()
+	if err != nil {
+		return "", model.NewAppError("SqlJobStore.DeleteByType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	if _, err = jss.GetMaster().Exec(sql, args...); err != nil {
 		return "", model.NewAppError("SqlJobStore.DeleteByType", "store.sql_job.delete.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError)
 	}
 	return id, nil