diff --git a/internal/manager/api_impl/interfaces.go b/internal/manager/api_impl/interfaces.go index 3404afab..18408e5b 100644 --- a/internal/manager/api_impl/interfaces.go +++ b/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) - FetchWorkerTags(ctx context.Context) ([]*persistence.WorkerTag, error) + FetchWorkerTag(ctx context.Context, uuid string) (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) diff --git a/internal/manager/api_impl/jobs_test.go b/internal/manager/api_impl/jobs_test.go index b5134cab..c1344a28 100644 --- a/internal/manager/api_impl/jobs_test.go +++ b/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) diff --git a/internal/manager/api_impl/mocks/api_impl_mock.gen.go b/internal/manager/api_impl/mocks/api_impl_mock.gen.go index 6cca0d26..cb19a71e 100644 --- a/internal/manager/api_impl/mocks/api_impl_mock.gen.go +++ b/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) diff --git a/internal/manager/api_impl/worker_mgt.go b/internal/manager/api_impl/worker_mgt.go index de185aac..3a525c60 100644 --- a/internal/manager/api_impl/worker_mgt.go +++ b/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 { - if wc == nil { - return nil - } - +func workerTagDBtoAPI(wc persistence.WorkerTag) api.WorkerTag { 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 } diff --git a/internal/manager/api_impl/worker_mgt_test.go b/internal/manager/api_impl/worker_mgt_test.go index 366f7d9f..1384255d 100644 --- a/internal/manager/api_impl/worker_mgt_test.go +++ b/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}, diff --git a/internal/manager/eventbus/events_workertags.go b/internal/manager/eventbus/events_workertags.go index cc581d00..c6bc9b55 100644 --- a/internal/manager/eventbus/events_workertags.go +++ b/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, diff --git a/internal/manager/persistence/jobs.go b/internal/manager/persistence/jobs.go index 262bab7c..d70dc515 100644 --- a/internal/manager/persistence/jobs.go +++ b/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 diff --git a/internal/manager/persistence/jobs_test.go b/internal/manager/persistence/jobs_test.go index 06779b95..9dde4585 100644 --- a/internal/manager/persistence/jobs_test.go +++ b/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) diff --git a/internal/manager/persistence/worker_tag.go b/internal/manager/persistence/worker_tag.go index aacddd0c..26e0efbd 100644 --- a/internal/manager/persistence/worker_tag.go +++ b/internal/manager/persistence/worker_tag.go @@ -9,32 +9,24 @@ import ( "projects.blender.org/studio/flamenco/internal/manager/persistence/sqlc" ) -type WorkerTag struct { - Model +type WorkerTag = sqlc.WorkerTag - UUID string - Name string - Description string - - Workers []*Worker -} - -func (db *DB) CreateWorkerTag(ctx context.Context, wc *WorkerTag) error { +func (db *DB) CreateWorkerTag(ctx context.Context, tag *WorkerTag) error { queries := db.queries() now := db.now() dbID, err := queries.CreateWorkerTag(ctx, sqlc.CreateWorkerTagParams{ CreatedAt: now, - UUID: wc.UUID, - Name: wc.Name, - Description: wc.Description, + UUID: tag.UUID, + Name: tag.Name, + Description: tag.Description, }) if err != nil { return fmt.Errorf("creating new worker tag: %w", err) } - wc.ID = uint(dbID) - wc.CreatedAt = now + tag.ID = dbID + 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") } - - gormTags := make([]*WorkerTag, len(tags)) - for index, tag := range tags { - gormTags[index] = convertSqlcWorkerTag(tag) - } - return gormTags, nil + return tags, 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") } - - gormTags := make([]*WorkerTag, len(tags)) - for index, tag := range tags { - gormTags[index] = convertSqlcWorkerTag(tag) - } - return gormTags, nil + return tags, 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) } - - gormTags := make([]WorkerTag, len(tags)) - for index, tag := range tags { - gormTags[index] = *convertSqlcWorkerTag(tag) - } - return gormTags, nil + return tags, nil } diff --git a/internal/manager/persistence/worker_tag_test.go b/internal/manager/persistence/worker_tag_test.go index 57fa2604..2ca612fb 100644 --- a/internal/manager/persistence/worker_tag_test.go +++ b/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) { diff --git a/internal/manager/persistence/workers.go b/internal/manager/persistence/workers.go index 589c7d16..b7c54f59 100644 --- a/internal/manager/persistence/workers.go +++ b/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, - } -} diff --git a/internal/manager/persistence/workers_test.go b/internal/manager/persistence/workers_test.go index dc8fcb58..4a3cf52d 100644 --- a/internal/manager/persistence/workers_test.go +++ b/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) {