Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(db): return schedule on create and update #915

Merged
merged 5 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions api/schedule/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,30 +171,24 @@ 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)

util.HandleError(c, http.StatusInternalServerError, retErr)

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)

util.HandleError(c, http.StatusInternalServerError, retErr)

return
}

// send API call to capture the created schedule
s, _ = database.FromContext(c).GetScheduleForRepo(ctx, r, input.GetName())
}

c.JSON(http.StatusCreated, s)
Expand Down
5 changes: 1 addition & 4 deletions api/schedule/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
}
2 changes: 1 addition & 1 deletion cmd/vela-server/schedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down
9 changes: 2 additions & 7 deletions database/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/schedule/count_active_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/schedule/count_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/schedule/count_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
12 changes: 6 additions & 6 deletions database/schedule/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ package schedule

import (
"context"

"github.com/go-vela/types/constants"
"github.com/go-vela/types/database"
"github.com/go-vela/types/library"
"github.com/sirupsen/logrus"
)

// 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())
Expand All @@ -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
}
7 changes: 6 additions & 1 deletion database/schedule/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package schedule

import (
"context"
"reflect"
"testing"

"github.com/DATA-DOG/go-sqlmock"
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
})
}
}
2 changes: 1 addition & 1 deletion database/schedule/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion database/schedule/get_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion database/schedule/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
5 changes: 3 additions & 2 deletions database/schedule/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package schedule

import (
"context"

"github.com/go-vela/types/library"
)

Expand All @@ -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.
Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions database/schedule/list_active_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/schedule/list_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/schedule/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
7 changes: 4 additions & 3 deletions database/schedule/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ package schedule

import (
"context"

"github.com/go-vela/types/constants"
"github.com/go-vela/types/database"
"github.com/go-vela/types/library"
"github.com/sirupsen/logrus"
)

// 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())
Expand All @@ -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)
Expand All @@ -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
}
18 changes: 14 additions & 4 deletions database/schedule/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package schedule

import (
"context"
"reflect"
"testing"

"github.com/DATA-DOG/go-sqlmock"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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 {
Expand All @@ -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)
}
})
}
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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 {
Expand All @@ -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)
}
})
}
}