Skip to content

refactor(database): move step logic into separate package #810

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.

Sign up for GitHub

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

Merged
ecrupper merged 11 commits into from
Apr 17, 2023
Merged

refactor(database): move step logic into separate package #810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
74626c9
Merge branch 'main' of github.com:go-vela/server
jbrockopp Feb 14, 2023
974c8e6
Merge branch 'main' of github.com:go-vela/server
jbrockopp Mar 3, 2023
2f2c425
Merge branch 'main' of github.com:go-vela/server
jbrockopp Mar 20, 2023
528291a
Merge branch 'main' of github.com:go-vela/server
jbrockopp Apr 8, 2023
bf36fd5
feat(database): add step engine
jbrockopp Apr 8, 2023
7daf15b
chore(database): remove old step logic
jbrockopp Apr 8, 2023
eddabc3
chore: updates for database step engine
jbrockopp Apr 8, 2023
cc2aa4b
Merge branch 'main' into refactor/database/step
jbrockopp Apr 8, 2023
f0928c3
Merge branch 'main' into refactor/database/step
jbrockopp Apr 10, 2023
b5275ad
Merge branch 'main' into refactor/database/step
ecrupper Apr 14, 2023
fadf283
Merge branch 'main' into refactor/database/step
jbrockopp Apr 17, 2023
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
2 changes: 1 addition & 1 deletion api/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1837,7 +1837,7 @@ func CancelBuild(c *gin.Context) {

for page > 0 {
// retrieve build steps (per page) from the database
stepsPart, err := database.FromContext(c).GetBuildStepList(b, page, perPage)
stepsPart, _, err := database.FromContext(c).ListStepsForBuild(b, map[string]interface{}{}, page, perPage)
if err != nil {
retErr := fmt.Errorf("unable to retrieve steps for build %s: %w", entry, err)
util.HandleError(c, http.StatusNotFound, retErr)
Expand Down
4 changes: 2 additions & 2 deletions api/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ func recordGauges(c *gin.Context) {
// step_image_count
if q.StepImageCount {
// send API call to capture the total number of step images
stepImageMap, err := database.FromContext(c).GetStepImageCount()
stepImageMap, err := database.FromContext(c).ListStepImageCount()
if err != nil {
logrus.Errorf("unable to get count of all step images: %v", err)
}
Expand All @@ -337,7 +337,7 @@ func recordGauges(c *gin.Context) {
// step_status_count
if q.StepStatusCount {
// send API call to capture the total number of step statuses
stepStatusMap, err := database.FromContext(c).GetStepStatusCount()
stepStatusMap, err := database.FromContext(c).ListStepStatusCount()
if err != nil {
logrus.Errorf("unable to get count of all step statuses: %v", err)
}
Expand Down
22 changes: 6 additions & 16 deletions api/step.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func CreateStep(c *gin.Context) {
}

// send API call to capture the created step
s, _ := database.FromContext(c).GetStep(input.GetNumber(), b)
s, _ := database.FromContext(c).GetStepForBuild(b, input.GetNumber())

c.JSON(http.StatusCreated, s)
}
Expand Down Expand Up @@ -235,18 +235,8 @@ func GetSteps(c *gin.Context) {
// ensure per_page isn't above or below allowed values
perPage = util.MaxInt(1, util.MinInt(100, perPage))

// send API call to capture the total number of steps for the build
t, err := database.FromContext(c).GetBuildStepCount(b)
if err != nil {
retErr := fmt.Errorf("unable to get steps count for build %s: %w", entry, err)

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

return
}

// send API call to capture the list of steps for the build
s, err := database.FromContext(c).GetBuildStepList(b, page, perPage)
s, t, err := database.FromContext(c).ListStepsForBuild(b, map[string]interface{}{}, page, perPage)
if err != nil {
retErr := fmt.Errorf("unable to get steps for build %s: %w", entry, err)

Expand Down Expand Up @@ -464,7 +454,7 @@ func UpdateStep(c *gin.Context) {
}

// send API call to capture the updated step
s, _ = database.FromContext(c).GetStep(s.GetNumber(), b)
s, _ = database.FromContext(c).GetStepForBuild(b, s.GetNumber())

c.JSON(http.StatusOK, s)
}
Expand Down Expand Up @@ -535,7 +525,7 @@ func DeleteStep(c *gin.Context) {
}).Infof("deleting step %s", entry)

// send API call to remove the step
err := database.FromContext(c).DeleteStep(s.GetID())
err := database.FromContext(c).DeleteStep(s)
if err != nil {
retErr := fmt.Errorf("unable to delete step %s: %w", entry, err)

Expand Down Expand Up @@ -576,7 +566,7 @@ func planSteps(database database.Service, p *pipeline.Build, b *library.Build) (
}

// send API call to capture the created step
s, err = database.GetStep(s.GetNumber(), b)
s, err = database.GetStepForBuild(b, s.GetNumber())
if err != nil {
return steps, fmt.Errorf("unable to get step %s: %w", s.GetName(), err)
}
Expand Down Expand Up @@ -625,7 +615,7 @@ func planSteps(database database.Service, p *pipeline.Build, b *library.Build) (
}

// send API call to capture the created step
s, err = database.GetStep(s.GetNumber(), b)
s, err = database.GetStepForBuild(b, s.GetNumber())
if err != nil {
return steps, fmt.Errorf("unable to get step %s: %w", s.GetName(), err)
}
Expand Down
33 changes: 0 additions & 33 deletions database/postgres/ddl/step.go
View file
Open in desktop

This file was deleted.

67 changes: 0 additions & 67 deletions database/postgres/dml/step.go
View file
Open in desktop

This file was deleted.

23 changes: 17 additions & 6 deletions database/postgres/postgres.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/go-vela/server/database/postgres/ddl"
"github.com/go-vela/server/database/repo"
"github.com/go-vela/server/database/secret"
"github.com/go-vela/server/database/step"
"github.com/go-vela/server/database/user"
"github.com/go-vela/server/database/worker"
"github.com/go-vela/types/constants"
Expand Down Expand Up @@ -58,6 +59,8 @@ type (
repo.RepoService
// https://pkg.go.dev/github.com/go-vela/server/database/secret#SecretService
secret.SecretService
// https://pkg.go.dev/github.com/go-vela/server/database/step#StepService
step.StepService
// https://pkg.go.dev/github.com/go-vela/server/database/user#UserService
user.UserService
// https://pkg.go.dev/github.com/go-vela/server/database/worker#WorkerService
Expand Down Expand Up @@ -175,6 +178,8 @@ func NewTest() (*client, sqlmock.Sqlmock, error) {
_mock.ExpectExec(secret.CreateTypeOrgRepo).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(secret.CreateTypeOrgTeam).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(secret.CreateTypeOrg).WillReturnResult(sqlmock.NewResult(1, 1))
// ensure the mock expects the step queries
_mock.ExpectExec(step.CreatePostgresTable).WillReturnResult(sqlmock.NewResult(1, 1))
// ensure the mock expects the user queries
_mock.ExpectExec(user.CreatePostgresTable).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(user.CreateUserRefreshIndex).WillReturnResult(sqlmock.NewResult(1, 1))
Expand Down Expand Up @@ -273,12 +278,6 @@ func createTables(c *client) error {
return fmt.Errorf("unable to create %s table: %w", constants.TableService, err)
}

// create the steps table
err = c.Postgres.Exec(ddl.CreateStepTable).Error
if err != nil {
return fmt.Errorf("unable to create %s table: %w", constants.TableStep, err)
}

return nil
}

Expand Down Expand Up @@ -382,6 +381,18 @@ func createServices(c *client) error {
return err
}

// create the database agnostic step service
//
// https://pkg.go.dev/github.com/go-vela/server/database/repo#New
Copy link
Member

@wass3rw3rk wass3rw3rk Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// https://pkg.go.dev/github.com/go-vela/server/database/repo#New
// https://pkg.go.dev/github.com/go-vela/server/database/step#New

Copy link
Contributor Author

@jbrockopp jbrockopp Apr 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch 😄 I've addressed this within 19c554e as a part of #816

My line of thinking is we can merge this one as-is and that PR will resolve the typo

c.StepService, err = step.New(
step.WithClient(c.Postgres),
step.WithLogger(c.Logger),
step.WithSkipCreation(c.config.SkipCreation),
)
if err != nil {
return err
}

// create the database agnostic user service
//
// https://pkg.go.dev/github.com/go-vela/server/database/user#New
Expand Down
7 changes: 5 additions & 2 deletions database/postgres/postgres_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/go-vela/server/database/postgres/ddl"
"github.com/go-vela/server/database/repo"
"github.com/go-vela/server/database/secret"
"github.com/go-vela/server/database/step"
"github.com/go-vela/server/database/user"
"github.com/go-vela/server/database/worker"
"github.com/go-vela/types/library"
Expand Down Expand Up @@ -82,7 +83,6 @@ func TestPostgres_setupDatabase(t *testing.T) {
// ensure the mock expects the table queries
_mock.ExpectExec(ddl.CreateBuildTable).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(ddl.CreateServiceTable).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(ddl.CreateStepTable).WillReturnResult(sqlmock.NewResult(1, 1))

// ensure the mock expects the index queries
_mock.ExpectExec(ddl.CreateBuildRepoIDIndex).WillReturnResult(sqlmock.NewResult(1, 1))
Expand All @@ -107,6 +107,8 @@ func TestPostgres_setupDatabase(t *testing.T) {
_mock.ExpectExec(secret.CreateTypeOrgRepo).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(secret.CreateTypeOrgTeam).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(secret.CreateTypeOrg).WillReturnResult(sqlmock.NewResult(1, 1))
// ensure the mock expects the step queries
_mock.ExpectExec(step.CreatePostgresTable).WillReturnResult(sqlmock.NewResult(1, 1))
// ensure the mock expects the user queries
_mock.ExpectExec(user.CreatePostgresTable).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(user.CreateUserRefreshIndex).WillReturnResult(sqlmock.NewResult(1, 1))
Expand Down Expand Up @@ -175,7 +177,6 @@ func TestPostgres_createTables(t *testing.T) {
// ensure the mock expects the table queries
_mock.ExpectExec(ddl.CreateBuildTable).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(ddl.CreateServiceTable).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(ddl.CreateStepTable).WillReturnResult(sqlmock.NewResult(1, 1))

tests := []struct {
failure bool
Expand Down Expand Up @@ -272,6 +273,8 @@ func TestPostgres_createServices(t *testing.T) {
_mock.ExpectExec(secret.CreateTypeOrgRepo).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(secret.CreateTypeOrgTeam).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(secret.CreateTypeOrg).WillReturnResult(sqlmock.NewResult(1, 1))
// ensure the mock expects the step queries
_mock.ExpectExec(step.CreatePostgresTable).WillReturnResult(sqlmock.NewResult(1, 1))
// ensure the mock expects the user queries
_mock.ExpectExec(user.CreatePostgresTable).WillReturnResult(sqlmock.NewResult(1, 1))
_mock.ExpectExec(user.CreateUserRefreshIndex).WillReturnResult(sqlmock.NewResult(1, 1))
Expand Down
94 changes: 0 additions & 94 deletions database/postgres/step.go
View file
Open in desktop

This file was deleted.

Loading