Manager: replace queryJobs with fetchJobs operation

See the previous two commits for the motivation.

cmd/shaman-checkout-id-setter/main.go

@@ -21,7 +21,6 @@ import (
 	"projects.blender.org/studio/flamenco/internal/appinfo"
 	"projects.blender.org/studio/flamenco/internal/manager/config"
 	"projects.blender.org/studio/flamenco/internal/manager/persistence"
-	"projects.blender.org/studio/flamenco/pkg/api"
 )
 
 func main() {
@@ -72,7 +71,7 @@ func main() {
 	defer persist.Close()
 
 	// Get all jobs from the database.
-	jobs, err := persist.QueryJobs(ctx, api.JobsQuery{})
+	jobs, err := persist.FetchJobs(ctx)
 	if err != nil {
 		log.Fatal().Err(err).Msg("unable to fetch jobs")
 	}

internal/manager/api_impl/interfaces.go

@@ -33,6 +33,8 @@ type PersistenceService interface {
 	StoreAuthoredJob(ctx context.Context, authoredJob job_compilers.AuthoredJob) error
 	// FetchJob fetches a single job, without fetching its tasks.
 	FetchJob(ctx context.Context, jobID string) (*persistence.Job, error)
+	FetchJobs(ctx context.Context) ([]*persistence.Job, error)
+
 	SaveJobPriority(ctx context.Context, job *persistence.Job) error
 	// FetchTask fetches the given task and the accompanying job.
 	FetchTask(ctx context.Context, taskID string) (*persistence.Task, error)
@@ -81,7 +83,6 @@ type PersistenceService interface {
 	CountTaskFailuresOfWorker(ctx context.Context, job *persistence.Job, worker *persistence.Worker, taskType string) (int, error)
 
 	// Database queries.
-	QueryJobs(ctx context.Context, query api.JobsQuery) ([]*persistence.Job, error)
 	QueryJobTaskSummaries(ctx context.Context, jobUUID string) ([]*persistence.Task, error)
 
 	// SetLastRendered sets this job as the one with the most recent rendered image.

internal/manager/api_impl/jobs_query.go

@@ -59,17 +59,11 @@ func (f *Flamenco) FetchJob(e echo.Context, jobID string) error {
 	return e.JSON(http.StatusOK, apiJob)
 }
 
-func (f *Flamenco) QueryJobs(e echo.Context) error {
+func (f *Flamenco) FetchJobs(e echo.Context) error {
 	logger := requestLogger(e)
 
-	var jobsQuery api.QueryJobsJSONRequestBody
-	if err := e.Bind(&jobsQuery); err != nil {
-		logger.Warn().Err(err).Msg("bad request received")
-		return sendAPIError(e, http.StatusBadRequest, "invalid format")
-	}
-
 	ctx := e.Request().Context()
-	dbJobs, err := f.persist.QueryJobs(ctx, api.JobsQuery(jobsQuery))
+	dbJobs, err := f.persist.FetchJobs(ctx)
 	switch {
 	case errors.Is(err, context.Canceled):
 		logger.Debug().AnErr("cause", err).Msg("could not query for jobs, remote end probably closed the connection")

internal/manager/api_impl/jobs_query_test.go

@@ -13,7 +13,7 @@ import (
 	"projects.blender.org/studio/flamenco/pkg/api"
 )
 
-func TestQueryJobs(t *testing.T) {
+func TestFetchJobs(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 
@@ -48,10 +48,10 @@ func TestQueryJobs(t *testing.T) {
 
 	echoCtx := mf.prepareMockedRequest(nil)
 	ctx := echoCtx.Request().Context()
-	mf.persistence.EXPECT().QueryJobs(ctx, api.JobsQuery{}).
+	mf.persistence.EXPECT().FetchJobs(ctx).
 		Return([]*persistence.Job{&activeJob, &deletionQueuedJob}, nil)
 
-	err := mf.flamenco.QueryJobs(echoCtx)
+	err := mf.flamenco.FetchJobs(echoCtx)
 	require.NoError(t, err)
 
 	expectedJobs := api.JobsQueryResult{

internal/manager/api_impl/mocks/api_impl_mock.gen.go

@@ -214,6 +214,21 @@ func (mr *MockPersistenceServiceMockRecorder) FetchJobBlocklist(arg0, arg1 inter
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FetchJobBlocklist", reflect.TypeOf((*MockPersistenceService)(nil).FetchJobBlocklist), arg0, arg1)
 }
 
+// FetchJobs mocks base method.
+func (m *MockPersistenceService) FetchJobs(arg0 context.Context) ([]*persistence.Job, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "FetchJobs", arg0)
+	ret0, _ := ret[0].([]*persistence.Job)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// FetchJobs indicates an expected call of FetchJobs.
+func (mr *MockPersistenceServiceMockRecorder) FetchJobs(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FetchJobs", reflect.TypeOf((*MockPersistenceService)(nil).FetchJobs), arg0)
+}
+
 // FetchTask mocks base method.
 func (m *MockPersistenceService) FetchTask(arg0 context.Context, arg1 string) (*persistence.Task, error) {
 	m.ctrl.T.Helper()
@@ -364,21 +379,6 @@ func (mr *MockPersistenceServiceMockRecorder) QueryJobTaskSummaries(arg0, arg1 i
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "QueryJobTaskSummaries", reflect.TypeOf((*MockPersistenceService)(nil).QueryJobTaskSummaries), arg0, arg1)
 }
 
-// QueryJobs mocks base method.
-func (m *MockPersistenceService) QueryJobs(arg0 context.Context, arg1 api.JobsQuery) ([]*persistence.Job, error) {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "QueryJobs", arg0, arg1)
-	ret0, _ := ret[0].([]*persistence.Job)
-	ret1, _ := ret[1].(error)
-	return ret0, ret1
-}
-
-// QueryJobs indicates an expected call of QueryJobs.
-func (mr *MockPersistenceServiceMockRecorder) QueryJobs(arg0, arg1 interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "QueryJobs", reflect.TypeOf((*MockPersistenceService)(nil).QueryJobs), arg0, arg1)
-}
-
 // RemoveFromJobBlocklist mocks base method.
 func (m *MockPersistenceService) RemoveFromJobBlocklist(arg0 context.Context, arg1, arg2, arg3 string) error {
 	m.ctrl.T.Helper()

internal/manager/persistence/jobs.go

@@ -371,6 +371,38 @@ func (db *DB) FetchJob(ctx context.Context, jobUUID string) (*Job, error) {
 	return &gormJob, nil
 }
 
+func (db *DB) FetchJobs(ctx context.Context) ([]*Job, error) {
+	queries := db.queries()
+
+	sqlcJobs, err := queries.FetchJobs(ctx)
+	if err != nil {
+		return nil, jobError(err, "fetching all jobs")
+	}
+
+	gormJobs := make([]*Job, len(sqlcJobs))
+	for index, sqlcJob := range sqlcJobs {
+		gormJob, err := convertSqlcJob(sqlcJob)
+		if err != nil {
+			return nil, err
+		}
+
+		if sqlcJob.WorkerTagID.Valid {
+			workerTag, err := fetchWorkerTagByID(db.gormDB, uint(sqlcJob.WorkerTagID.Int64))
+			switch {
+			case errors.Is(err, sql.ErrNoRows):
+				return nil, ErrWorkerTagNotFound
+			case err != nil:
+				return nil, workerTagError(err, "fetching worker tag of job")
+			}
+			gormJob.WorkerTag = workerTag
+		}
+
+		gormJobs[index] = &gormJob
+	}
+
+	return gormJobs, nil
+}
+
 // FetchJobShamanCheckoutID fetches the job's Shaman Checkout ID.
 func (db *DB) FetchJobShamanCheckoutID(ctx context.Context, jobUUID string) (string, error) {
 	queries := db.queries()

internal/manager/persistence/jobs_query.go

@@ -3,74 +3,11 @@ package persistence
 
 import (
 	"context"
-	"strings"
 
 	"github.com/rs/zerolog/log"
 	"projects.blender.org/studio/flamenco/pkg/api"
 )
 
-func (db *DB) QueryJobs(ctx context.Context, apiQ api.JobsQuery) ([]*Job, error) {
-	logger := log.Ctx(ctx)
-
-	logger.Debug().Interface("q", apiQ).Msg("querying jobs")
-
-	q := db.gormDB.WithContext(ctx).Model(&Job{})
-
-	// WHERE
-	if apiQ.StatusIn != nil {
-		q = q.Where("status in ?", *apiQ.StatusIn)
-	}
-	if apiQ.Settings != nil {
-		for setting, value := range apiQ.Settings.AdditionalProperties {
-			q = q.Where("json_extract(metadata, ?) = ?", "$."+setting, value)
-		}
-	}
-	if apiQ.Metadata != nil {
-		for setting, value := range apiQ.Metadata.AdditionalProperties {
-			if strings.ContainsRune(value, '%') {
-				q = q.Where("json_extract(metadata, ?) like ?", "$."+setting, value)
-			} else {
-				q = q.Where("json_extract(metadata, ?) = ?", "$."+setting, value)
-			}
-		}
-	}
-
-	// OFFSET
-	if apiQ.Offset != nil {
-		q = q.Offset(*apiQ.Offset)
-	}
-
-	// LIMIT
-	if apiQ.Limit != nil {
-		q = q.Limit(*apiQ.Limit)
-	}
-
-	// ORDER BY
-	if apiQ.OrderBy != nil {
-		sqlOrder := ""
-		for _, order := range *apiQ.OrderBy {
-			if order == "" {
-				continue
-			}
-			switch order[0] {
-			case '-':
-				sqlOrder = order[1:] + " desc"
-			case '+':
-				sqlOrder = order[1:] + " asc"
-			default:
-				sqlOrder = order
-			}
-			q = q.Order(sqlOrder)
-		}
-	}
-
-	q.Preload("Tag")
-
-	result := []*Job{}
-	tx := q.Scan(&result)
-	return result, tx.Error
-}
-
 // QueryJobTaskSummaries retrieves all tasks of the job, but not all fields of those tasks.
 // Fields are synchronised with api.TaskSummary.
 func (db *DB) QueryJobTaskSummaries(ctx context.Context, jobUUID string) ([]*Task, error) {

internal/manager/persistence/jobs_query_test.go

@@ -16,99 +16,6 @@ import (
 	"projects.blender.org/studio/flamenco/pkg/api"
 )
 
-func TestSimpleQuery(t *testing.T) {
-	ctx, close, db, job, _ := jobTasksTestFixtures(t)
-	defer close()
-
-	// Sanity check.
-	if !assert.Equal(t, api.JobStatusUnderConstruction, job.Status, "check job status is as expected") {
-		t.FailNow()
-	}
-
-	// Check empty result when querying for other status.
-	result, err := db.QueryJobs(ctx, api.JobsQuery{
-		StatusIn: &[]api.JobStatus{api.JobStatusActive, api.JobStatusCanceled},
-	})
-	require.NoError(t, err)
-	assert.Len(t, result, 0)
-
-	// Check job was returned properly on correct status.
-	result, err = db.QueryJobs(ctx, api.JobsQuery{
-		StatusIn: &[]api.JobStatus{api.JobStatusUnderConstruction, api.JobStatusCanceled},
-	})
-	require.NoError(t, err)
-	if !assert.Len(t, result, 1) {
-		t.FailNow()
-	}
-	assert.Equal(t, job.ID, result[0].ID)
-
-}
-
-func TestQueryMetadata(t *testing.T) {
-	ctx, close, db := persistenceTestFixtures(0)
-	defer close()
-
-	testJob := persistAuthoredJob(t, ctx, db, createTestAuthoredJobWithTasks())
-
-	otherAuthoredJob := createTestAuthoredJobWithTasks()
-	otherAuthoredJob.Status = api.JobStatusActive
-	otherAuthoredJob.Tasks = []job_compilers.AuthoredTask{}
-	otherAuthoredJob.JobID = "138678c8-efd0-452b-ac05-397ff4c02b26"
-	otherAuthoredJob.Metadata["project"] = "Other Project"
-	otherJob := persistAuthoredJob(t, ctx, db, otherAuthoredJob)
-
-	var (
-		result []*Job
-		err    error
-	)
-
-	// Check empty result when querying for specific metadata:
-	result, err = db.QueryJobs(ctx, api.JobsQuery{
-		Metadata: &api.JobsQuery_Metadata{
-			AdditionalProperties: map[string]string{
-				"project": "Secret Future Project",
-			}}})
-	require.NoError(t, err)
-	assert.Len(t, result, 0)
-
-	// Check job was returned properly when querying for the right project.
-	result, err = db.QueryJobs(ctx, api.JobsQuery{
-		Metadata: &api.JobsQuery_Metadata{
-			AdditionalProperties: map[string]string{
-				"project": testJob.Metadata["project"],
-			}}})
-	require.NoError(t, err)
-	if !assert.Len(t, result, 1) {
-		t.FailNow()
-	}
-	assert.Equal(t, testJob.ID, result[0].ID)
-
-	// Check for the other job
-	result, err = db.QueryJobs(ctx, api.JobsQuery{
-		Metadata: &api.JobsQuery_Metadata{
-			AdditionalProperties: map[string]string{
-				"project": otherJob.Metadata["project"],
-			}}})
-	require.NoError(t, err)
-	if !assert.Len(t, result, 1) {
-		t.FailNow()
-	}
-	assert.Equal(t, otherJob.ID, result[0].ID)
-
-	// Check job was returned properly when querying for empty metadata.
-	result, err = db.QueryJobs(ctx, api.JobsQuery{
-		OrderBy:  &[]string{"status"},
-		Metadata: &api.JobsQuery_Metadata{AdditionalProperties: map[string]string{}},
-	})
-	require.NoError(t, err)
-	if !assert.Len(t, result, 2) {
-		t.FailNow()
-	}
-	// 'active' should come before 'under-construction':
-	assert.Equal(t, otherJob.ID, result[0].ID, "status is %s", result[0].Status)
-	assert.Equal(t, testJob.ID, result[1].ID, "status is %s", result[1].Status)
-}
-
 func TestQueryJobTaskSummaries(t *testing.T) {
 	ctx, close, db, job, authoredJob := jobTasksTestFixtures(t)
 	defer close()

internal/manager/persistence/sqlc/query_jobs.sql

@@ -65,6 +65,10 @@ WHERE uuid = ? LIMIT 1;
 SELECT * FROM jobs
 WHERE id = ? LIMIT 1;
 
+-- name: FetchJobs :many
+-- Fetch all jobs in the database.
+SELECT * fRoM jobs;
+
 -- name: FetchJobShamanCheckoutID :one
 SELECT storage_shaman_checkout_id FROM jobs WHERE uuid=@uuid;
 

internal/manager/persistence/sqlc/query_jobs.sql.go

@@ -398,6 +398,49 @@ func (q *Queries) FetchJobUUIDsUpdatedBefore(ctx context.Context, updatedAtMax s
 	return items, nil
 }
 
+const fetchJobs = `-- name: FetchJobs :many
+SELECT id, created_at, updated_at, uuid, name, job_type, priority, status, activity, settings, metadata, delete_requested_at, storage_shaman_checkout_id, worker_tag_id fRoM jobs
+`
+
+// Fetch all jobs in the database.
+func (q *Queries) FetchJobs(ctx context.Context) ([]Job, error) {
+	rows, err := q.db.QueryContext(ctx, fetchJobs)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	var items []Job
+	for rows.Next() {
+		var i Job
+		if err := rows.Scan(
+			&i.ID,
+			&i.CreatedAt,
+			&i.UpdatedAt,
+			&i.UUID,
+			&i.Name,
+			&i.JobType,
+			&i.Priority,
+			&i.Status,
+			&i.Activity,
+			&i.Settings,
+			&i.Metadata,
+			&i.DeleteRequestedAt,
+			&i.StorageShamanCheckoutID,
+			&i.WorkerTagID,
+		); err != nil {
+			return nil, err
+		}
+		items = append(items, i)
+	}
+	if err := rows.Close(); err != nil {
+		return nil, err
+	}
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+	return items, nil
+}
+
 const fetchJobsDeletionRequested = `-- name: FetchJobsDeletionRequested :many
 SELECT uuid FROM jobs
   WHERE delete_requested_at is not NULL

web/app/src/components/jobs/JobsTable.vue

@@ -136,9 +136,8 @@ export default {
     },
     fetchAllJobs() {
       const jobsApi = new API.JobsApi(getAPIClient());
-      const jobsQuery = {};
       this.jobs.isJobless = false;
-      jobsApi.queryJobs(jobsQuery).then(this.onJobsFetched, function (error) {
+      jobsApi.fetchJobs().then(this.onJobsFetched, function (error) {
         // TODO: error handling.
         console.error(error);
       });

web/app/src/manager-api/manager/JobsApi.js

@@ -24,7 +24,6 @@ import JobMassDeletionSelection from '../model/JobMassDeletionSelection';
 import JobPriorityChange from '../model/JobPriorityChange';
 import JobStatusChange from '../model/JobStatusChange';
 import JobTasksSummary from '../model/JobTasksSummary';
-import JobsQuery from '../model/JobsQuery';
 import JobsQueryResult from '../model/JobsQueryResult';
 import SubmittedJob from '../model/SubmittedJob';
 import Task from '../model/Task';
@@ -411,6 +410,45 @@ export default class JobsApi {
     }
 
 
+    /**
+     * List all jobs in the database.
+     * @return {Promise} a {@link https://www.promisejs.org/|Promise}, with an object containing data of type {@link module:model/JobsQueryResult} and HTTP response
+     */
+    fetchJobsWithHttpInfo() {
+      let postBody = null;
+
+      let pathParams = {
+      };
+      let queryParams = {
+      };
+      let headerParams = {
+      };
+      let formParams = {
+      };
+
+      let authNames = [];
+      let contentTypes = [];
+      let accepts = ['application/json'];
+      let returnType = JobsQueryResult;
+      return this.apiClient.callApi(
+        '/api/v3/jobs', 'GET',
+        pathParams, queryParams, headerParams, formParams, postBody,
+        authNames, contentTypes, accepts, returnType, null
+      );
+    }
+
+    /**
+     * List all jobs in the database.
+     * @return {Promise} a {@link https://www.promisejs.org/|Promise}, with data of type {@link module:model/JobsQueryResult}
+     */
+    fetchJobs() {
+      return this.fetchJobsWithHttpInfo()
+        .then(function(response_and_data) {
+          return response_and_data.data;
+        });
+    }
+
+
     /**
      * Fetch a single task.
      * @param {String} taskId 
@@ -634,51 +672,6 @@ export default class JobsApi {
     }
 
 
-    /**
-     * Fetch list of jobs.
-     * @param {module:model/JobsQuery} jobsQuery Specification of which jobs to get.
-     * @return {Promise} a {@link https://www.promisejs.org/|Promise}, with an object containing data of type {@link module:model/JobsQueryResult} and HTTP response
-     */
-    queryJobsWithHttpInfo(jobsQuery) {
-      let postBody = jobsQuery;
-      // verify the required parameter 'jobsQuery' is set
-      if (jobsQuery === undefined || jobsQuery === null) {
-        throw new Error("Missing the required parameter 'jobsQuery' when calling queryJobs");
-      }
-
-      let pathParams = {
-      };
-      let queryParams = {
-      };
-      let headerParams = {
-      };
-      let formParams = {
-      };
-
-      let authNames = [];
-      let contentTypes = ['application/json'];
-      let accepts = ['application/json'];
-      let returnType = JobsQueryResult;
-      return this.apiClient.callApi(
-        '/api/v3/jobs/query', 'POST',
-        pathParams, queryParams, headerParams, formParams, postBody,
-        authNames, contentTypes, accepts, returnType, null
-      );
-    }
-
-    /**
-     * Fetch list of jobs.
-     * @param {module:model/JobsQuery} jobsQuery Specification of which jobs to get.
-     * @return {Promise} a {@link https://www.promisejs.org/|Promise}, with data of type {@link module:model/JobsQueryResult}
-     */
-    queryJobs(jobsQuery) {
-      return this.queryJobsWithHttpInfo(jobsQuery)
-        .then(function(response_and_data) {
-          return response_and_data.data;
-        });
-    }
-
-
     /**
      * Remove entries from a job blocklist.
      * @param {String} jobId