refactor(database): return pipeline on created and updated (#882)

* refactor(database): return pipeline on created and updated * fix weird error handling
go-vela · Jun 13, 2023 · ca3e1e3 · ca3e1e3
1 parent d51b1c3
commit ca3e1e3
Show file tree

Hide file tree

Showing 19 changed files with 64 additions and 98 deletions.
diff --git a/api/build/create.go b/api/build/create.go
@@ -308,25 +308,14 @@ func CreateBuild(c *gin.Context) {
 		pipeline.SetRef(input.GetRef())
 
 		// send API call to create the pipeline
-		err = database.FromContext(c).CreatePipeline(pipeline)
+		pipeline, err = database.FromContext(c).CreatePipeline(pipeline)
 		if err != nil {
 			retErr := fmt.Errorf("unable to create new build: failed to create pipeline for %s: %w", r.GetFullName(), err)
 
 			util.HandleError(c, http.StatusBadRequest, retErr)
 
 			return
 		}
-
-		// send API call to capture the created pipeline
-		pipeline, err = database.FromContext(c).GetPipelineForRepo(pipeline.GetCommit(), r)
-		if err != nil {
-			//nolint:lll // ignore long line length due to error message
-			retErr := fmt.Errorf("unable to create new build: failed to get new pipeline %s/%s: %w", r.GetFullName(), pipeline.GetCommit(), err)
-
-			util.HandleError(c, http.StatusInternalServerError, retErr)
-
-			return
-		}
 	}
 
 	input.SetPipelineID(pipeline.GetID())

diff --git a/api/build/restart.go b/api/build/restart.go
@@ -299,25 +299,14 @@ func RestartBuild(c *gin.Context) {
 		pipeline.SetRef(b.GetRef())
 
 		// send API call to create the pipeline
-		err = database.FromContext(c).CreatePipeline(pipeline)
+		pipeline, err = database.FromContext(c).CreatePipeline(pipeline)
 		if err != nil {
 			retErr := fmt.Errorf("unable to create pipeline for %s: %w", r.GetFullName(), err)
 
 			util.HandleError(c, http.StatusBadRequest, retErr)
 
 			return
 		}
-
-		// send API call to capture the created pipeline
-		pipeline, err = database.FromContext(c).GetPipelineForRepo(pipeline.GetCommit(), r)
-		if err != nil {
-			//nolint:lll // ignore long line length due to error message
-			retErr := fmt.Errorf("unable to get new pipeline %s/%s: %w", r.GetFullName(), pipeline.GetCommit(), err)
-
-			util.HandleError(c, http.StatusInternalServerError, retErr)
-
-			return
-		}
 	}
 
 	b.SetPipelineID(pipeline.GetID())

diff --git a/api/pipeline/create.go b/api/pipeline/create.go
@@ -98,7 +98,7 @@ func CreatePipeline(c *gin.Context) {
 	input.SetRepoID(r.GetID())
 
 	// send API call to create the pipeline
-	err = database.FromContext(c).CreatePipeline(input)
+	p, err := database.FromContext(c).CreatePipeline(input)
 	if err != nil {
 		retErr := fmt.Errorf("unable to create pipeline %s/%s: %w", r.GetFullName(), input.GetCommit(), err)
 
@@ -107,15 +107,5 @@ func CreatePipeline(c *gin.Context) {
 		return
 	}
 
-	// send API call to capture the created pipeline
-	p, err := database.FromContext(c).GetPipelineForRepo(input.GetCommit(), r)
-	if err != nil {
-		retErr := fmt.Errorf("unable to capture pipeline %s/%s: %w", r.GetFullName(), input.GetCommit(), err)
-
-		util.HandleError(c, http.StatusInternalServerError, retErr)
-
-		return
-	}
-
 	c.JSON(http.StatusCreated, p)
 }
diff --git a/api/pipeline/update.go b/api/pipeline/update.go
@@ -170,7 +170,7 @@ func UpdatePipeline(c *gin.Context) {
 	}
 
 	// send API call to update the pipeline
-	err = database.FromContext(c).UpdatePipeline(p)
+	p, err = database.FromContext(c).UpdatePipeline(p)
 	if err != nil {
 		retErr := fmt.Errorf("unable to update pipeline %s: %w", entry, err)
 
@@ -179,15 +179,5 @@ func UpdatePipeline(c *gin.Context) {
 		return
 	}
 
-	// send API call to capture the updated pipeline
-	p, err = database.FromContext(c).GetPipelineForRepo(p.GetCommit(), r)
-	if err != nil {
-		retErr := fmt.Errorf("unable to capture pipeline %s: %w", entry, err)
-
-		util.HandleError(c, http.StatusInternalServerError, retErr)
-
-		return
-	}
-
 	c.JSON(http.StatusOK, p)
 }
diff --git a/api/webhook/post.go b/api/webhook/post.go
@@ -561,7 +561,7 @@ func PostWebhook(c *gin.Context) {
 			pipeline.SetRef(b.GetRef())
 
 			// send API call to create the pipeline
-			err = database.FromContext(c).CreatePipeline(pipeline)
+			pipeline, err = database.FromContext(c).CreatePipeline(pipeline)
 			if err != nil {
 				retErr := fmt.Errorf("%s: failed to create pipeline for %s: %w", baseErr, repo.GetFullName(), err)
 
@@ -580,19 +580,6 @@ func PostWebhook(c *gin.Context) {
 
 				return
 			}
-
-			// send API call to capture the created pipeline
-			pipeline, err = database.FromContext(c).GetPipelineForRepo(pipeline.GetCommit(), repo)
-			if err != nil {
-				//nolint:lll // ignore long line length due to error message
-				retErr := fmt.Errorf("%s: failed to get new pipeline %s/%s: %w", baseErr, repo.GetFullName(), pipeline.GetCommit(), err)
-				util.HandleError(c, http.StatusInternalServerError, retErr)
-
-				h.SetStatus(constants.StatusFailure)
-				h.SetError(retErr.Error())
-
-				return
-			}
 		}
 
 		b.SetPipelineID(pipeline.GetID())

diff --git a/cmd/vela-server/schedule.go b/cmd/vela-server/schedule.go
@@ -294,7 +294,7 @@ func processSchedule(s *library.Schedule, compiler compiler.Engine, database dat
 			pipeline.SetRef(b.GetRef())
 
 			// send API call to create the pipeline
-			err = database.CreatePipeline(pipeline)
+			pipeline, err = database.CreatePipeline(pipeline)
 			if err != nil {
 				err = fmt.Errorf("failed to create pipeline for %s: %w", r.GetFullName(), err)
 
@@ -308,12 +308,6 @@ func processSchedule(s *library.Schedule, compiler compiler.Engine, database dat
 
 				return err
 			}
-
-			// send API call to capture the created pipeline
-			pipeline, err = database.GetPipelineForRepo(pipeline.GetCommit(), r)
-			if err != nil {
-				return fmt.Errorf("unable to get new pipeline %s/%s: %w", r.GetFullName(), pipeline.GetCommit(), err)
-			}
 		}
 
 		b.SetPipelineID(pipeline.GetID())

diff --git a/database/pipeline/count_repo_test.go b/database/pipeline/count_repo_test.go
@@ -42,12 +42,12 @@ func TestPipeline_Engine_CountPipelinesForRepo(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreatePipeline(_pipelineOne)
+	_, err := _sqlite.CreatePipeline(_pipelineOne)
 	if err != nil {
 		t.Errorf("unable to create test pipeline for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreatePipeline(_pipelineTwo)
+	_, err = _sqlite.CreatePipeline(_pipelineTwo)
 	if err != nil {
 		t.Errorf("unable to create test pipeline for sqlite: %v", err)
 	}

diff --git a/database/pipeline/count_test.go b/database/pipeline/count_test.go
@@ -41,12 +41,12 @@ func TestPipeline_Engine_CountPipelines(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreatePipeline(_pipelineOne)
+	_, err := _sqlite.CreatePipeline(_pipelineOne)
 	if err != nil {
 		t.Errorf("unable to create test pipeline for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreatePipeline(_pipelineTwo)
+	_, err = _sqlite.CreatePipeline(_pipelineTwo)
 	if err != nil {
 		t.Errorf("unable to create test pipeline for sqlite: %v", err)
 	}

diff --git a/database/pipeline/create.go b/database/pipeline/create.go
@@ -12,7 +12,7 @@ import (
 )
 
 // CreatePipeline creates a new pipeline in the database.
-func (e *engine) CreatePipeline(p *library.Pipeline) error {
+func (e *engine) CreatePipeline(p *library.Pipeline) (*library.Pipeline, error) {
 	e.logger.WithFields(logrus.Fields{
 		"pipeline": p.GetCommit(),
 	}).Tracef("creating pipeline %s in the database", p.GetCommit())
@@ -27,20 +27,27 @@ func (e *engine) CreatePipeline(p *library.Pipeline) error {
 	// https://pkg.go.dev/github.com/go-vela/types/database#Pipeline.Validate
 	err := pipeline.Validate()
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// compress data for the pipeline
 	//
 	// https://pkg.go.dev/github.com/go-vela/types/database#Pipeline.Compress
 	err = pipeline.Compress(e.config.CompressionLevel)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// send query to the database
-	return e.client.
-		Table(constants.TablePipeline).
-		Create(pipeline).
-		Error
+	err = e.client.Table(constants.TablePipeline).Create(pipeline).Error
+	if err != nil {
+		return nil, err
+	}
+
+	err = pipeline.Decompress()
+	if err != nil {
+		return nil, err
+	}
+
+	return pipeline.ToLibrary(), nil
 }
diff --git a/database/pipeline/create_test.go b/database/pipeline/create_test.go
@@ -5,6 +5,7 @@
 package pipeline
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/DATA-DOG/go-sqlmock"
@@ -19,6 +20,7 @@ func TestPipeline_Engine_CreatePipeline(t *testing.T) {
 	_pipeline.SetRef("refs/heads/master")
 	_pipeline.SetType("yaml")
 	_pipeline.SetVersion("1")
+	_pipeline.SetData([]byte{})
 
 	_postgres, _mock := testPostgres(t)
 	defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }()
@@ -57,7 +59,7 @@ VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15) RETURNING "id"`).
 	// run tests
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			err := test.database.CreatePipeline(_pipeline)
+			got, err := test.database.CreatePipeline(_pipeline)
 
 			if test.failure {
 				if err == nil {
@@ -70,6 +72,10 @@ VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15) RETURNING "id"`).
 			if err != nil {
 				t.Errorf("CreatePipeline for %s returned err: %v", test.name, err)
 			}
+
+			if !reflect.DeepEqual(got, _pipeline) {
+				t.Errorf("CreatePipeline for %s returned %s, want %s", test.name, got, _pipeline)
+			}
 		})
 	}
 }
diff --git a/database/pipeline/delete_test.go b/database/pipeline/delete_test.go
@@ -31,7 +31,7 @@ func TestPipeline_Engine_DeletePipeline(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreatePipeline(_pipeline)
+	_, err := _sqlite.CreatePipeline(_pipeline)
 	if err != nil {
 		t.Errorf("unable to create test pipeline for sqlite: %v", err)
 	}

diff --git a/database/pipeline/get_repo_test.go b/database/pipeline/get_repo_test.go
@@ -37,7 +37,7 @@ func TestPipeline_Engine_GetPipelineForRepo(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreatePipeline(_pipeline)
+	_, err := _sqlite.CreatePipeline(_pipeline)
 	if err != nil {
 		t.Errorf("unable to create test pipeline for sqlite: %v", err)
 	}

diff --git a/database/pipeline/get_test.go b/database/pipeline/get_test.go
@@ -37,7 +37,7 @@ func TestPipeline_Engine_GetPipeline(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreatePipeline(_pipeline)
+	_, err := _sqlite.CreatePipeline(_pipeline)
 	if err != nil {
 		t.Errorf("unable to create test pipeline for sqlite: %v", err)
 	}

diff --git a/database/pipeline/interface.go b/database/pipeline/interface.go
@@ -31,7 +31,7 @@ type PipelineInterface interface {
 	// CountPipelinesForRepo defines a function that gets the count of pipelines by repo ID.
 	CountPipelinesForRepo(*library.Repo) (int64, error)
 	// CreatePipeline defines a function that creates a new pipeline.
-	CreatePipeline(*library.Pipeline) error
+	CreatePipeline(*library.Pipeline) (*library.Pipeline, error)
 	// DeletePipeline defines a function that deletes an existing pipeline.
 	DeletePipeline(*library.Pipeline) error
 	// GetPipeline defines a function that gets a pipeline by ID.
@@ -43,5 +43,5 @@ type PipelineInterface interface {
 	// ListPipelinesForRepo defines a function that gets a list of pipelines by repo ID.
 	ListPipelinesForRepo(*library.Repo, int, int) ([]*library.Pipeline, int64, error)
 	// UpdatePipeline defines a function that updates an existing pipeline.
-	UpdatePipeline(*library.Pipeline) error
+	UpdatePipeline(*library.Pipeline) (*library.Pipeline, error)
 }
diff --git a/database/pipeline/list_repo_test.go b/database/pipeline/list_repo_test.go
@@ -53,12 +53,12 @@ func TestPipeline_Engine_ListPipelinesForRepo(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreatePipeline(_pipelineOne)
+	_, err := _sqlite.CreatePipeline(_pipelineOne)
 	if err != nil {
 		t.Errorf("unable to create test pipeline for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreatePipeline(_pipelineTwo)
+	_, err = _sqlite.CreatePipeline(_pipelineTwo)
 	if err != nil {
 		t.Errorf("unable to create test pipeline for sqlite: %v", err)
 	}

diff --git a/database/pipeline/list_test.go b/database/pipeline/list_test.go
@@ -53,12 +53,12 @@ func TestPipeline_Engine_ListPipelines(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreatePipeline(_pipelineOne)
+	_, err := _sqlite.CreatePipeline(_pipelineOne)
 	if err != nil {
 		t.Errorf("unable to create test pipeline for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreatePipeline(_pipelineTwo)
+	_, err = _sqlite.CreatePipeline(_pipelineTwo)
 	if err != nil {
 		t.Errorf("unable to create test pipeline for sqlite: %v", err)
 	}

diff --git a/database/pipeline/update.go b/database/pipeline/update.go
@@ -12,7 +12,7 @@ import (
 )
 
 // UpdatePipeline updates an existing pipeline in the database.
-func (e *engine) UpdatePipeline(p *library.Pipeline) error {
+func (e *engine) UpdatePipeline(p *library.Pipeline) (*library.Pipeline, error) {
 	e.logger.WithFields(logrus.Fields{
 		"pipeline": p.GetCommit(),
 	}).Tracef("updating pipeline %s in the database", p.GetCommit())
@@ -27,20 +27,28 @@ func (e *engine) UpdatePipeline(p *library.Pipeline) error {
 	// https://pkg.go.dev/github.com/go-vela/types/database#Pipeline.Validate
 	err := pipeline.Validate()
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// compress data for the pipeline
 	//
 	// https://pkg.go.dev/github.com/go-vela/types/database#Pipeline.Compress
 	err = pipeline.Compress(e.config.CompressionLevel)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// send query to the database
-	return e.client.
-		Table(constants.TablePipeline).
-		Save(pipeline).
-		Error
+	err = e.client.Table(constants.TablePipeline).Save(pipeline).Error
+	if err != nil {
+		return nil, err
+	}
+
+	// decompress pipeline to return
+	err = pipeline.Decompress()
+	if err != nil {
+		return nil, err
+	}
+
+	return pipeline.ToLibrary(), nil
 }