Transition from ex-GORM structs to sqlc structs (3/5)

Replace old used-to-be-GORM datastructures (#104305) with sqlc-generated
structs. This also makes it possible to use more specific structs that
are more taylored to the specific queries, increasing efficiency.

This commit deals with worker tags (on both workers and jobs).

Functional changes are kept to a minimum, as the API still serves the
same data.

Because this work covers so much of Flamenco's code, it's been split up
into different commits. Each commit brings Flamenco to a state where it
compiles and unit tests pass. Only the result of the final commit has
actually been tested properly.

Ref: #104343

internal/manager/api_impl/interfaces.go

@@ -72,8 +72,8 @@ type PersistenceService interface {
 	// Worker tag management.
 	WorkerSetTags(ctx context.Context, worker *persistence.Worker, tagUUIDs []string) error
 	CreateWorkerTag(ctx context.Context, tag *persistence.WorkerTag) error
-	FetchWorkerTag(ctx context.Context, uuid string) (*persistence.WorkerTag, error)
+	FetchWorkerTag(ctx context.Context, uuid string) (persistence.WorkerTag, error)
-	FetchWorkerTags(ctx context.Context) ([]*persistence.WorkerTag, error)
+	FetchWorkerTags(ctx context.Context) ([]persistence.WorkerTag, error)
 	DeleteWorkerTag(ctx context.Context, uuid string) error
 	SaveWorkerTag(ctx context.Context, tag *persistence.WorkerTag) error
 	FetchTagsOfWorker(ctx context.Context, workerUUID string) ([]persistence.WorkerTag, error)

internal/manager/api_impl/jobs_test.go

@@ -330,7 +330,7 @@ func TestSubmitJobWithWorkerTag(t *testing.T) {
 
 	workerTagUUID := "04435762-9dc8-4f13-80b7-643a6fa5b6fd"
 	tag := persistence.WorkerTag{
-		Model:       persistence.Model{ID: 47},
+		ID:          47,
 		UUID:        workerTagUUID,
 		Name:        "first tag",
 		Description: "my first tag",
@@ -383,7 +383,7 @@ func TestSubmitJobWithWorkerTag(t *testing.T) {
 		Settings: persistence.StringInterfaceMap{},
 		Metadata: persistence.StringStringMap{},
 
-		WorkerTagID: &tag.ID,
+		WorkerTagID: ptr(uint(tag.ID)),
 		WorkerTag:   &tag,
 	}
 	mf.persistence.EXPECT().FetchJob(gomock.Any(), queuedJob.JobID).Return(&dbJob, nil)

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

@@ -144,7 +144,7 @@ func (mr *MockPersistenceServiceMockRecorder) CreateWorker(arg0, arg1 interface{
 }
 
 // CreateWorkerTag mocks base method.
-func (m *MockPersistenceService) CreateWorkerTag(arg0 context.Context, arg1 *persistence.WorkerTag) error {
+func (m *MockPersistenceService) CreateWorkerTag(arg0 context.Context, arg1 *sqlc.WorkerTag) error {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "CreateWorkerTag", arg0, arg1)
 	ret0, _ := ret[0].(error)
@@ -231,10 +231,10 @@ func (mr *MockPersistenceServiceMockRecorder) FetchJobs(arg0 interface{}) *gomoc
 }
 
 // FetchTagsOfWorker mocks base method.
-func (m *MockPersistenceService) FetchTagsOfWorker(arg0 context.Context, arg1 string) ([]persistence.WorkerTag, error) {
+func (m *MockPersistenceService) FetchTagsOfWorker(arg0 context.Context, arg1 string) ([]sqlc.WorkerTag, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "FetchTagsOfWorker", arg0, arg1)
-	ret0, _ := ret[0].([]persistence.WorkerTag)
+	ret0, _ := ret[0].([]sqlc.WorkerTag)
 	ret1, _ := ret[1].(error)
 	return ret0, ret1
 }
@@ -306,10 +306,10 @@ func (mr *MockPersistenceServiceMockRecorder) FetchWorker(arg0, arg1 interface{}
 }
 
 // FetchWorkerTag mocks base method.
-func (m *MockPersistenceService) FetchWorkerTag(arg0 context.Context, arg1 string) (*persistence.WorkerTag, error) {
+func (m *MockPersistenceService) FetchWorkerTag(arg0 context.Context, arg1 string) (sqlc.WorkerTag, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "FetchWorkerTag", arg0, arg1)
-	ret0, _ := ret[0].(*persistence.WorkerTag)
+	ret0, _ := ret[0].(sqlc.WorkerTag)
 	ret1, _ := ret[1].(error)
 	return ret0, ret1
 }
@@ -321,10 +321,10 @@ func (mr *MockPersistenceServiceMockRecorder) FetchWorkerTag(arg0, arg1 interfac
 }
 
 // FetchWorkerTags mocks base method.
-func (m *MockPersistenceService) FetchWorkerTags(arg0 context.Context) ([]*persistence.WorkerTag, error) {
+func (m *MockPersistenceService) FetchWorkerTags(arg0 context.Context) ([]sqlc.WorkerTag, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "FetchWorkerTags", arg0)
-	ret0, _ := ret[0].([]*persistence.WorkerTag)
+	ret0, _ := ret[0].([]sqlc.WorkerTag)
 	ret1, _ := ret[1].(error)
 	return ret0, ret1
 }
@@ -466,7 +466,7 @@ func (mr *MockPersistenceServiceMockRecorder) SaveWorkerStatus(arg0, arg1 interf
 }
 
 // SaveWorkerTag mocks base method.
-func (m *MockPersistenceService) SaveWorkerTag(arg0 context.Context, arg1 *persistence.WorkerTag) error {
+func (m *MockPersistenceService) SaveWorkerTag(arg0 context.Context, arg1 *sqlc.WorkerTag) error {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "SaveWorkerTag", arg0, arg1)
 	ret0, _ := ret[0].(error)

internal/manager/api_impl/worker_mgt.go

@@ -336,9 +336,6 @@ func (f *Flamenco) FetchWorkerTag(e echo.Context, tagUUID string) error {
 		logger.Error().Err(err).Msg("fetching worker tag")
 		return sendAPIError(e, http.StatusInternalServerError, "error fetching worker tag: %v", err)
 	}
-	if tag == nil {
-		panic("Could fetch a worker tag without error, but then the returned tag was still nil")
-	}
 
 	return e.JSON(http.StatusOK, workerTagDBtoAPI(tag))
 }
@@ -388,7 +385,7 @@ func (f *Flamenco) UpdateWorkerTag(e echo.Context, tagUUID string) error {
 	}
 	logger = logCtx.Logger()
 
-	if err := f.persist.SaveWorkerTag(ctx, dbTag); err != nil {
+	if err := f.persist.SaveWorkerTag(ctx, &dbTag); err != nil {
 		logger.Error().Err(err).Msg("saving worker tag")
 		return sendAPIError(e, http.StatusInternalServerError, "error saving worker tag")
 	}
@@ -414,7 +411,7 @@ func (f *Flamenco) FetchWorkerTags(e echo.Context) error {
 	apiTags := []api.WorkerTag{}
 	for _, dbTag := range dbTags {
 		apiTag := workerTagDBtoAPI(dbTag)
-		apiTags = append(apiTags, *apiTag)
+		apiTags = append(apiTags, apiTag)
 	}
 
 	tagList := api.WorkerTagList{
@@ -466,10 +463,10 @@ func (f *Flamenco) CreateWorkerTag(e echo.Context) error {
 	logger.Info().Msg("created new worker tag")
 
 	// SocketIO broadcast of tag creation.
-	sioUpdate := eventbus.NewWorkerTagUpdate(&dbTag)
+	sioUpdate := eventbus.NewWorkerTagUpdate(dbTag)
 	f.broadcaster.BroadcastNewWorkerTag(sioUpdate)
 
-	return e.JSON(http.StatusOK, workerTagDBtoAPI(&dbTag))
+	return e.JSON(http.StatusOK, workerTagDBtoAPI(dbTag))
 }
 
 func workerSummary(w persistence.Worker) api.WorkerSummary {
@@ -504,11 +501,7 @@ func workerDBtoAPI(w persistence.Worker) api.Worker {
 	return apiWorker
 }
 
-func workerTagDBtoAPI(wc *persistence.WorkerTag) *api.WorkerTag {
+func workerTagDBtoAPI(wc persistence.WorkerTag) api.WorkerTag {
-	if wc == nil {
-		return nil
-	}
-
 	uuid := wc.UUID // Take a copy for safety.
 
 	apiTag := api.WorkerTag{
@@ -518,5 +511,5 @@ func workerTagDBtoAPI(wc *persistence.WorkerTag) *api.WorkerTag {
 	if len(wc.Description) > 0 {
 		apiTag.Description = &wc.Description
 	}
-	return &apiTag
+	return apiTag
 }

internal/manager/api_impl/worker_mgt_test.go

@@ -307,7 +307,7 @@ func TestWorkerTagCRUDHappyFlow(t *testing.T) {
 	assertResponseJSON(t, echo, http.StatusOK, &apiTag)
 
 	// Fetch the tag
-	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(&expectDBTag, nil)
+	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(expectDBTag, nil)
 	echo = mf.prepareMockedRequest(nil)
 	require.NoError(t, mf.flamenco.FetchWorkerTag(echo, UUID))
 	assertResponseJSON(t, echo, http.StatusOK, &apiTag)
@@ -330,7 +330,7 @@ func TestWorkerTagCRUDHappyFlow(t *testing.T) {
 			Description: apiTag.Description,
 		},
 	})
-	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(&expectDBTag, nil)
+	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(expectDBTag, nil)
 	mf.persistence.EXPECT().SaveWorkerTag(gomock.Any(), &expectNewDBTag)
 	echo = mf.prepareMockedJSONRequest(newAPITag)
 	require.NoError(t, mf.flamenco.UpdateWorkerTag(echo, UUID))
@@ -353,7 +353,7 @@ func TestWorkerTagCRUDHappyFlow(t *testing.T) {
 			Description: newAPITag.Description,
 		},
 	})
-	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(&expectDBTag, nil)
+	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(expectDBTag, nil)
 	mf.persistence.EXPECT().SaveWorkerTag(gomock.Any(), &expectNewDBTag)
 	echo = mf.prepareMockedJSONRequest(newAPITag)
 	require.NoError(t, mf.flamenco.UpdateWorkerTag(echo, UUID))
@@ -376,14 +376,14 @@ func TestWorkerTagCRUDHappyFlow(t *testing.T) {
 			Description: newAPITag.Description,
 		},
 	})
-	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(&expectDBTag, nil)
+	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(expectDBTag, nil)
 	mf.persistence.EXPECT().SaveWorkerTag(gomock.Any(), &expectNewDBTag)
 	echo = mf.prepareMockedJSONRequest(newAPITag)
 	require.NoError(t, mf.flamenco.UpdateWorkerTag(echo, UUID))
 	assertResponseNoContent(t, echo)
 
 	// Delete.
-	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(&expectDBTag, nil)
+	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(expectDBTag, nil)
 	mf.persistence.EXPECT().DeleteWorkerTag(gomock.Any(), UUID)
 	mf.broadcaster.EXPECT().BroadcastWorkerTagUpdate(api.EventWorkerTagUpdate{
 		Tag:        api.WorkerTag{Id: &UUID},

internal/manager/eventbus/events_workertags.go

@@ -11,7 +11,7 @@ import (
 // NewWorkerTagUpdate returns a partial EventWorkerTagUpdate struct for the
 // given worker tag. It only fills in the fields that represent the current
 // state of the tag.
-func NewWorkerTagUpdate(tag *persistence.WorkerTag) api.EventWorkerTagUpdate {
+func NewWorkerTagUpdate(tag persistence.WorkerTag) api.EventWorkerTagUpdate {
 	tagUpdate := api.EventWorkerTagUpdate{
 		Tag: api.WorkerTag{
 			Id:          &tag.UUID,

internal/manager/persistence/jobs.go

@@ -355,7 +355,7 @@ func (db *DB) FetchJob(ctx context.Context, jobUUID string) (*Job, error) {
 		case err != nil:
 			return nil, workerTagError(err, "fetching worker tag of job")
 		}
-		gormJob.WorkerTag = workerTag
+		gormJob.WorkerTag = &workerTag
 	}
 
 	return &gormJob, nil
@@ -384,7 +384,7 @@ func (db *DB) FetchJobs(ctx context.Context) ([]*Job, error) {
 			case err != nil:
 				return nil, workerTagError(err, "fetching worker tag of job")
 			}
-			gormJob.WorkerTag = workerTag
+			gormJob.WorkerTag = &workerTag
 		}
 
 		gormJobs[index] = &gormJob

internal/manager/persistence/jobs_test.go

@@ -97,7 +97,7 @@ func TestStoreAuthoredJobWithWorkerTag(t *testing.T) {
 	require.NotNil(t, fetchedJob)
 
 	require.NotNil(t, fetchedJob.WorkerTagID)
-	assert.Equal(t, *fetchedJob.WorkerTagID, workerTag.ID)
+	assert.Equal(t, int64(*fetchedJob.WorkerTagID), workerTag.ID)
 
 	require.NotNil(t, fetchedJob.WorkerTag)
 	assert.Equal(t, fetchedJob.WorkerTag.Name, workerTag.Name)

internal/manager/persistence/worker_tag.go

@@ -9,32 +9,24 @@ import (
 	"projects.blender.org/studio/flamenco/internal/manager/persistence/sqlc"
 )
 
-type WorkerTag struct {
+type WorkerTag = sqlc.WorkerTag
-	Model
 
-	UUID        string
+func (db *DB) CreateWorkerTag(ctx context.Context, tag *WorkerTag) error {
-	Name        string
-	Description string
-
-	Workers []*Worker
-}
-
-func (db *DB) CreateWorkerTag(ctx context.Context, wc *WorkerTag) error {
 	queries := db.queries()
 
 	now := db.now()
 	dbID, err := queries.CreateWorkerTag(ctx, sqlc.CreateWorkerTagParams{
 		CreatedAt:   now,
-		UUID:        wc.UUID,
+		UUID:        tag.UUID,
-		Name:        wc.Name,
+		Name:        tag.Name,
-		Description: wc.Description,
+		Description: tag.Description,
 	})
 	if err != nil {
 		return fmt.Errorf("creating new worker tag: %w", err)
 	}
 
-	wc.ID = uint(dbID)
+	tag.ID = dbID
-	wc.CreatedAt = now
+	tag.CreatedAt = now
 
 	return nil
 }
@@ -51,25 +43,25 @@ func (db *DB) HasWorkerTags(ctx context.Context) (bool, error) {
 	return count > 0, nil
 }
 
-func (db *DB) FetchWorkerTag(ctx context.Context, uuid string) (*WorkerTag, error) {
+func (db *DB) FetchWorkerTag(ctx context.Context, uuid string) (WorkerTag, error) {
 	queries := db.queries()
 
 	workerTag, err := queries.FetchWorkerTagByUUID(ctx, uuid)
 	if err != nil {
-		return nil, workerTagError(err, "fetching worker tag")
+		return WorkerTag{}, workerTagError(err, "fetching worker tag")
 	}
 
-	return convertSqlcWorkerTag(workerTag), nil
+	return workerTag, nil
 }
 
 // fetchWorkerTagByID fetches the worker tag using the given database instance.
-func fetchWorkerTagByID(ctx context.Context, queries *sqlc.Queries, id int64) (*WorkerTag, error) {
+func fetchWorkerTagByID(ctx context.Context, queries *sqlc.Queries, id int64) (WorkerTag, error) {
 	workerTag, err := queries.FetchWorkerTagByID(ctx, id)
 	if err != nil {
-		return nil, workerTagError(err, "fetching worker tag")
+		return WorkerTag{}, workerTagError(err, "fetching worker tag")
 	}
 
-	return convertSqlcWorkerTag(workerTag), nil
+	return workerTag, nil
 }
 
 func (db *DB) SaveWorkerTag(ctx context.Context, tag *WorkerTag) error {
@@ -80,7 +72,7 @@ func (db *DB) SaveWorkerTag(ctx context.Context, tag *WorkerTag) error {
 		UUID:        tag.UUID,
 		Name:        tag.Name,
 		Description: tag.Description,
-		WorkerTagID: int64(tag.ID),
+		WorkerTagID: tag.ID,
 	})
 	if err != nil {
 		return workerTagError(err, "saving worker tag")
@@ -112,36 +104,26 @@ func (db *DB) DeleteWorkerTag(ctx context.Context, uuid string) error {
 	return nil
 }
 
-func (db *DB) FetchWorkerTags(ctx context.Context) ([]*WorkerTag, error) {
+func (db *DB) FetchWorkerTags(ctx context.Context) ([]WorkerTag, error) {
 	queries := db.queries()
 
 	tags, err := queries.FetchWorkerTags(ctx)
 	if err != nil {
 		return nil, workerTagError(err, "fetching all worker tags")
 	}
-
+	return tags, nil
-	gormTags := make([]*WorkerTag, len(tags))
-	for index, tag := range tags {
-		gormTags[index] = convertSqlcWorkerTag(tag)
-	}
-	return gormTags, nil
 }
 
 func (db *DB) fetchWorkerTagsWithUUID(
 	ctx context.Context,
 	queries *sqlc.Queries,
 	tagUUIDs []string,
-) ([]*WorkerTag, error) {
+) ([]WorkerTag, error) {
 	tags, err := queries.FetchWorkerTagsByUUIDs(ctx, tagUUIDs)
 	if err != nil {
 		return nil, workerTagError(err, "fetching all worker tags")
 	}
-
+	return tags, nil
-	gormTags := make([]*WorkerTag, len(tags))
-	for index, tag := range tags {
-		gormTags[index] = convertSqlcWorkerTag(tag)
-	}
-	return gormTags, nil
 }
 
 func (db *DB) WorkerSetTags(ctx context.Context, worker *Worker, tagUUIDs []string) error {
@@ -180,10 +162,5 @@ func (db *DB) FetchTagsOfWorker(ctx context.Context, workerUUID string) ([]Worke
 	if err != nil {
 		return nil, workerTagError(err, "fetching tags of worker %s", workerUUID)
 	}
-
+	return tags, nil
-	gormTags := make([]WorkerTag, len(tags))
-	for index, tag := range tags {
-		gormTags[index] = *convertSqlcWorkerTag(tag)
-	}
-	return gormTags, nil
 }

internal/manager/persistence/worker_tag_test.go

@@ -19,7 +19,7 @@ func TestCreateFetchTag(t *testing.T) {
 	// Test fetching non-existent tag
 	fetchedTag, err := f.db.FetchWorkerTag(f.ctx, "7ee21bc8-ff1a-42d2-a6b6-cc4b529b189f")
 	assert.ErrorIs(t, err, ErrWorkerTagNotFound)
-	assert.Nil(t, fetchedTag)
+	assert.Zero(t, fetchedTag)
 
 	// New tag creation is already done in the workerTestFixtures() call.
 	assert.NotNil(t, f.tag)
@@ -32,7 +32,6 @@ func TestCreateFetchTag(t *testing.T) {
 	assert.Equal(t, f.tag.UUID, fetchedTag.UUID)
 	assert.Equal(t, f.tag.Name, fetchedTag.Name)
 	assert.Equal(t, f.tag.Description, fetchedTag.Description)
-	assert.Zero(t, fetchedTag.Workers)
 }
 
 func TestFetchDeleteTags(t *testing.T) {

internal/manager/persistence/workers.go

@@ -248,20 +248,3 @@ func (db *DB) SummarizeWorkerStatuses(ctx context.Context) (WorkerStatusCount, e
 
 	return statusCounts, nil
 }
-
-// convertSqlcWorkerTag converts a worker tag from the SQLC-generated model to
-// the model expected by the rest of the code. This is mostly in place to aid in
-// the GORM to SQLC migration. It is intended that eventually the rest of the
-// code will use the same SQLC-generated model.
-func convertSqlcWorkerTag(tag sqlc.WorkerTag) *WorkerTag {
-	return &WorkerTag{
-		Model: Model{
-			ID:        uint(tag.ID),
-			CreatedAt: tag.CreatedAt,
-			UpdatedAt: tag.UpdatedAt.Time,
-		},
-		UUID:        tag.UUID,
-		Name:        tag.Name,
-		Description: tag.Description,
-	}
-}

internal/manager/persistence/workers_test.go

@@ -362,11 +362,6 @@ func TestDeleteWorkerWithTagAssigned(t *testing.T) {
 
 	// Delete the Worker.
 	require.NoError(t, f.db.DeleteWorker(f.ctx, f.worker.UUID))
-
-	// Check the Worker has been unassigned from the tag.
-	tag, err := f.db.FetchWorkerTag(f.ctx, f.tag.UUID)
-	require.NoError(t, err)
-	assert.Empty(t, tag.Workers)
 }
 
 func TestSummarizeWorkerStatuses(t *testing.T) {