diff --git a/api/schedule/create.go b/api/schedule/create.go index 3c5c4790a..c50f11744 100644 --- a/api/schedule/create.go +++ b/api/schedule/create.go @@ -171,7 +171,7 @@ func CreateSchedule(c *gin.Context) { dbSchedule.SetActive(true) // send API call to update the schedule - err = database.FromContext(c).UpdateSchedule(ctx, dbSchedule, true) + s, err = database.FromContext(c).UpdateSchedule(ctx, dbSchedule, true) if err != nil { retErr := fmt.Errorf("unable to set schedule %s to active: %w", dbSchedule.GetName(), err) @@ -179,12 +179,9 @@ func CreateSchedule(c *gin.Context) { return } - - // send API call to capture the updated schedule - s, _ = database.FromContext(c).GetScheduleForRepo(ctx, r, dbSchedule.GetName()) } else { // send API call to create the schedule - err = database.FromContext(c).CreateSchedule(ctx, s) + s, err = database.FromContext(c).CreateSchedule(ctx, s) if err != nil { retErr := fmt.Errorf("unable to create new schedule %s: %w", r.GetName(), err) @@ -192,9 +189,6 @@ func CreateSchedule(c *gin.Context) { return } - - // send API call to capture the created schedule - s, _ = database.FromContext(c).GetScheduleForRepo(ctx, r, input.GetName()) } c.JSON(http.StatusCreated, s) diff --git a/api/schedule/update.go b/api/schedule/update.go index a58990f33..578639c8b 100644 --- a/api/schedule/update.go +++ b/api/schedule/update.go @@ -129,7 +129,7 @@ func UpdateSchedule(c *gin.Context) { s.SetUpdatedBy(u.GetName()) // update the schedule within the database - err = database.FromContext(c).UpdateSchedule(ctx, s, true) + s, err = database.FromContext(c).UpdateSchedule(ctx, s, true) if err != nil { retErr := fmt.Errorf("unable to update scheduled %s: %w", scheduleName, err) @@ -138,8 +138,5 @@ func UpdateSchedule(c *gin.Context) { return } - // capture the updated scheduled - s, _ = database.FromContext(c).GetScheduleForRepo(ctx, r, scheduleName) - c.JSON(http.StatusOK, s) } diff --git a/cmd/vela-server/schedule.go b/cmd/vela-server/schedule.go index cd1a19664..2144af965 100644 --- a/cmd/vela-server/schedule.go +++ b/cmd/vela-server/schedule.go @@ -116,7 +116,7 @@ func processSchedules(ctx context.Context, start time.Time, compiler compiler.En schedule.SetScheduledAt(time.Now().UTC().Unix()) // send API call to update schedule for ensuring scheduled_at field is set - err = database.UpdateSchedule(ctx, schedule, false) + _, err = database.UpdateSchedule(ctx, schedule, false) if err != nil { logrus.WithError(err).Warnf("%s %s", scheduleErr, schedule.GetName()) diff --git a/database/integration_test.go b/database/integration_test.go index c99c3587a..5d7fbe0cb 100644 --- a/database/integration_test.go +++ b/database/integration_test.go @@ -928,7 +928,7 @@ func testSchedules(t *testing.T, db Interface, resources *Resources) { // create the schedules for _, schedule := range resources.Schedules { - err := db.CreateSchedule(ctx, schedule) + _, err := db.CreateSchedule(ctx, schedule) if err != nil { t.Errorf("unable to create schedule %d: %v", schedule.GetID(), err) } @@ -1004,16 +1004,11 @@ func testSchedules(t *testing.T, db Interface, resources *Resources) { // update the schedules for _, schedule := range resources.Schedules { schedule.SetUpdatedAt(time.Now().UTC().Unix()) - err = db.UpdateSchedule(ctx, schedule, true) + got, err := db.UpdateSchedule(ctx, schedule, true) if err != nil { t.Errorf("unable to update schedule %d: %v", schedule.GetID(), err) } - // lookup the schedule by ID - got, err := db.GetSchedule(ctx, schedule.GetID()) - if err != nil { - t.Errorf("unable to get schedule %d by ID: %v", schedule.GetID(), err) - } if !reflect.DeepEqual(got, schedule) { t.Errorf("GetSchedule() is %v, want %v", got, schedule) } diff --git a/database/schedule/count_active_test.go b/database/schedule/count_active_test.go index 866e11425..5d555ea8c 100644 --- a/database/schedule/count_active_test.go +++ b/database/schedule/count_active_test.go @@ -47,12 +47,12 @@ func TestSchedule_Engine_CountActiveSchedules(t *testing.T) { _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() - err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) + _, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } - err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) + _, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } diff --git a/database/schedule/count_repo_test.go b/database/schedule/count_repo_test.go index 7bfbbe9bd..d2429c595 100644 --- a/database/schedule/count_repo_test.go +++ b/database/schedule/count_repo_test.go @@ -51,12 +51,12 @@ func TestSchedule_Engine_CountSchedulesForRepo(t *testing.T) { _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() - err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) + _, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } - err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) + _, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } diff --git a/database/schedule/count_test.go b/database/schedule/count_test.go index d8523b784..91537d919 100644 --- a/database/schedule/count_test.go +++ b/database/schedule/count_test.go @@ -45,12 +45,12 @@ func TestSchedule_Engine_CountSchedules(t *testing.T) { _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() - err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) + _, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } - err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) + _, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } diff --git a/database/schedule/create.go b/database/schedule/create.go index f04b95508..8a5907263 100644 --- a/database/schedule/create.go +++ b/database/schedule/create.go @@ -7,6 +7,7 @@ package schedule import ( "context" + "github.com/go-vela/types/constants" "github.com/go-vela/types/database" "github.com/go-vela/types/library" @@ -14,7 +15,7 @@ import ( ) // CreateSchedule creates a new schedule in the database. -func (e *engine) CreateSchedule(ctx context.Context, s *library.Schedule) error { +func (e *engine) CreateSchedule(ctx context.Context, s *library.Schedule) (*library.Schedule, error) { e.logger.WithFields(logrus.Fields{ "schedule": s.GetName(), }).Tracef("creating schedule %s in the database", s.GetName()) @@ -25,12 +26,11 @@ func (e *engine) CreateSchedule(ctx context.Context, s *library.Schedule) error // validate the necessary fields are populated err := schedule.Validate() if err != nil { - return err + return nil, err } // send query to the database - return e.client. - Table(constants.TableSchedule). - Create(schedule). - Error + result := e.client.Table(constants.TableSchedule).Create(schedule) + + return schedule.ToLibrary(), result.Error } diff --git a/database/schedule/create_test.go b/database/schedule/create_test.go index 426115915..4a987b577 100644 --- a/database/schedule/create_test.go +++ b/database/schedule/create_test.go @@ -6,6 +6,7 @@ package schedule import ( "context" + "reflect" "testing" "github.com/DATA-DOG/go-sqlmock" @@ -59,7 +60,7 @@ VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10) RETURNING "id"`). // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - err := test.database.CreateSchedule(context.TODO(), _schedule) + got, err := test.database.CreateSchedule(context.TODO(), _schedule) if test.failure { if err == nil { @@ -72,6 +73,10 @@ VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10) RETURNING "id"`). if err != nil { t.Errorf("CreateSchedule for %s returned err: %v", test.name, err) } + + if !reflect.DeepEqual(got, _schedule) { + t.Errorf("CreateSchedule for %s returned %s, want %s", test.name, got, _schedule) + } }) } } diff --git a/database/schedule/delete_test.go b/database/schedule/delete_test.go index 0e958de52..e302da6b9 100644 --- a/database/schedule/delete_test.go +++ b/database/schedule/delete_test.go @@ -33,7 +33,7 @@ func TestSchedule_Engine_DeleteSchedule(t *testing.T) { _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() - err := _sqlite.CreateSchedule(context.TODO(), _schedule) + _, err := _sqlite.CreateSchedule(context.TODO(), _schedule) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } diff --git a/database/schedule/get_repo_test.go b/database/schedule/get_repo_test.go index 56b010e97..b29072ec7 100644 --- a/database/schedule/get_repo_test.go +++ b/database/schedule/get_repo_test.go @@ -44,7 +44,7 @@ func TestSchedule_Engine_GetScheduleForRepo(t *testing.T) { _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() - err := _sqlite.CreateSchedule(context.TODO(), _schedule) + _, err := _sqlite.CreateSchedule(context.TODO(), _schedule) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } diff --git a/database/schedule/get_test.go b/database/schedule/get_test.go index 6fc0ed11e..b5175d8b0 100644 --- a/database/schedule/get_test.go +++ b/database/schedule/get_test.go @@ -38,7 +38,7 @@ func TestSchedule_Engine_GetSchedule(t *testing.T) { _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() - err := _sqlite.CreateSchedule(context.TODO(), _schedule) + _, err := _sqlite.CreateSchedule(context.TODO(), _schedule) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } diff --git a/database/schedule/interface.go b/database/schedule/interface.go index 6f7309d16..402499ab1 100644 --- a/database/schedule/interface.go +++ b/database/schedule/interface.go @@ -6,6 +6,7 @@ package schedule import ( "context" + "github.com/go-vela/types/library" ) @@ -32,7 +33,7 @@ type ScheduleInterface interface { // CountSchedulesForRepo defines a function that gets the count of schedules by repo ID. CountSchedulesForRepo(context.Context, *library.Repo) (int64, error) // CreateSchedule defines a function that creates a new schedule. - CreateSchedule(context.Context, *library.Schedule) error + CreateSchedule(context.Context, *library.Schedule) (*library.Schedule, error) // DeleteSchedule defines a function that deletes an existing schedule. DeleteSchedule(context.Context, *library.Schedule) error // GetSchedule defines a function that gets a schedule by ID. @@ -46,5 +47,5 @@ type ScheduleInterface interface { // ListSchedulesForRepo defines a function that gets a list of schedules by repo ID. ListSchedulesForRepo(context.Context, *library.Repo, int, int) ([]*library.Schedule, int64, error) // UpdateSchedule defines a function that updates an existing schedule. - UpdateSchedule(context.Context, *library.Schedule, bool) error + UpdateSchedule(context.Context, *library.Schedule, bool) (*library.Schedule, error) } diff --git a/database/schedule/list_active_test.go b/database/schedule/list_active_test.go index 35591a4ad..90ae96cfe 100644 --- a/database/schedule/list_active_test.go +++ b/database/schedule/list_active_test.go @@ -56,12 +56,12 @@ func TestSchedule_Engine_ListActiveSchedules(t *testing.T) { _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() - err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) + _, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } - err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) + _, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } diff --git a/database/schedule/list_repo_test.go b/database/schedule/list_repo_test.go index 425e5dddb..2536f73be 100644 --- a/database/schedule/list_repo_test.go +++ b/database/schedule/list_repo_test.go @@ -60,12 +60,12 @@ func TestSchedule_Engine_ListSchedulesForRepo(t *testing.T) { _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() - err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) + _, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } - err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) + _, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } diff --git a/database/schedule/list_test.go b/database/schedule/list_test.go index cdb13f756..6dd64af6c 100644 --- a/database/schedule/list_test.go +++ b/database/schedule/list_test.go @@ -55,12 +55,12 @@ func TestSchedule_Engine_ListSchedules(t *testing.T) { _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() - err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) + _, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } - err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) + _, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } diff --git a/database/schedule/update.go b/database/schedule/update.go index 934b50c59..23ab855ba 100644 --- a/database/schedule/update.go +++ b/database/schedule/update.go @@ -6,6 +6,7 @@ package schedule import ( "context" + "github.com/go-vela/types/constants" "github.com/go-vela/types/database" "github.com/go-vela/types/library" @@ -13,7 +14,7 @@ import ( ) // UpdateSchedule updates an existing schedule in the database. -func (e *engine) UpdateSchedule(ctx context.Context, s *library.Schedule, fields bool) error { +func (e *engine) UpdateSchedule(ctx context.Context, s *library.Schedule, fields bool) (*library.Schedule, error) { e.logger.WithFields(logrus.Fields{ "schedule": s.GetName(), }).Tracef("updating schedule %s in the database", s.GetName()) @@ -24,7 +25,7 @@ func (e *engine) UpdateSchedule(ctx context.Context, s *library.Schedule, fields // validate the necessary fields are populated err := schedule.Validate() if err != nil { - return err + return nil, err } // If "fields" is true, update entire record; otherwise, just update scheduled_at (part of processSchedule) @@ -37,5 +38,5 @@ func (e *engine) UpdateSchedule(ctx context.Context, s *library.Schedule, fields err = e.client.Table(constants.TableSchedule).Model(schedule).UpdateColumn("scheduled_at", s.GetScheduledAt()).Error } - return err + return schedule.ToLibrary(), err } diff --git a/database/schedule/update_test.go b/database/schedule/update_test.go index 345c7b2db..c13e44ae8 100644 --- a/database/schedule/update_test.go +++ b/database/schedule/update_test.go @@ -6,6 +6,7 @@ package schedule import ( "context" + "reflect" "testing" "github.com/DATA-DOG/go-sqlmock" @@ -41,7 +42,7 @@ WHERE "id" = $10`). _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() - err := _sqlite.CreateSchedule(context.TODO(), _schedule) + _, err := _sqlite.CreateSchedule(context.TODO(), _schedule) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } @@ -67,7 +68,8 @@ WHERE "id" = $10`). // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - err = test.database.UpdateSchedule(context.TODO(), _schedule, true) + got, err := test.database.UpdateSchedule(context.TODO(), _schedule, true) + _schedule.SetUpdatedAt(got.GetUpdatedAt()) if test.failure { if err == nil { @@ -80,6 +82,10 @@ WHERE "id" = $10`). if err != nil { t.Errorf("UpdateSchedule for %s returned err: %v", test.name, err) } + + if !reflect.DeepEqual(got, _schedule) { + t.Errorf("UpdateSchedule for %s returned %s, want %s", test.name, got, _schedule) + } }) } } @@ -113,7 +119,7 @@ func TestSchedule_Engine_UpdateSchedule_NotConfig(t *testing.T) { _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() - err := _sqlite.CreateSchedule(context.TODO(), _schedule) + _, err := _sqlite.CreateSchedule(context.TODO(), _schedule) if err != nil { t.Errorf("unable to create test schedule for sqlite: %v", err) } @@ -139,7 +145,7 @@ func TestSchedule_Engine_UpdateSchedule_NotConfig(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - err = test.database.UpdateSchedule(context.TODO(), _schedule, false) + got, err := test.database.UpdateSchedule(context.TODO(), _schedule, false) if test.failure { if err == nil { @@ -152,6 +158,10 @@ func TestSchedule_Engine_UpdateSchedule_NotConfig(t *testing.T) { if err != nil { t.Errorf("UpdateSchedule for %s returned err: %v", test.name, err) } + + if !reflect.DeepEqual(got, _schedule) { + t.Errorf("CreateSchedule for %s returned %s, want %s", test.name, got, _schedule) + } }) } }