Remove the channel Iterator method from ActivePipelines (#175)

* Remove the iter channel.

* Renamed and fixed comments about iter usage.

* Failed to save the file...

Author: Skarlso

handlers/hook.go

@@ -122,9 +122,10 @@ func GitWebHook(c echo.Context) error {
 	}
 
 	var foundPipe *gaia.Pipeline
-	for pipe := range pipeline.GlobalActivePipelines.Iter() {
+	for _, pipe := range pipeline.GlobalActivePipelines.GetAll() {
 		if pipe.Repo.URL == p.Repo.GitURL || pipe.Repo.URL == p.Repo.HTMLURL || pipe.Repo.URL == p.Repo.SSHURL {
 			foundPipe = &pipe
+			break
 		}
 	}
 	err = pipeline.UpdateRepository(foundPipe)

handlers/pipeline.go

@@ -94,12 +94,8 @@ func PipelineNameAvailable(c echo.Context) error {
 
 // PipelineGetAll returns all registered pipelines.
 func PipelineGetAll(c echo.Context) error {
-	var pipelines []gaia.Pipeline
-
 	// Get all active pipelines
-	for pipeline := range pipeline.GlobalActivePipelines.Iter() {
-		pipelines = append(pipelines, pipeline)
-	}
+	pipelines := pipeline.GlobalActivePipelines.GetAll()
 
 	// Return as json
 	return c.JSON(http.StatusOK, pipelines)
@@ -116,18 +112,12 @@ func PipelineGet(c echo.Context) error {
 	}
 
 	// Look up pipeline for the given id
-	var foundPipeline gaia.Pipeline
-	for pipeline := range pipeline.GlobalActivePipelines.Iter() {
+	for _, pipeline := range pipeline.GlobalActivePipelines.GetAll() {
 		if pipeline.ID == pipelineID {
-			foundPipeline = pipeline
+			return c.JSON(http.StatusOK, pipeline)
 		}
 	}
 
-	if foundPipeline.Name != "" {
-		return c.JSON(http.StatusOK, foundPipeline)
-	}
-
-	// Pipeline not found
 	return c.String(http.StatusNotFound, errPipelineNotFound.Error())
 }
 
@@ -142,9 +132,10 @@ func PipelineUpdate(c echo.Context) error {
 
 	// Look up pipeline for the given id
 	var foundPipeline gaia.Pipeline
-	for pipeline := range pipeline.GlobalActivePipelines.Iter() {
+	for _, pipeline := range pipeline.GlobalActivePipelines.GetAll() {
 		if pipeline.ID == p.ID {
 			foundPipeline = pipeline
+			break
 		}
 	}
 
@@ -248,14 +239,13 @@ func PipelineDelete(c echo.Context) error {
 
 	// Look up pipeline for the given id
 	var foundPipeline gaia.Pipeline
-	var index int
 	var deletedPipelineIndex int
-	for pipeline := range pipeline.GlobalActivePipelines.Iter() {
+	for index, pipeline := range pipeline.GlobalActivePipelines.GetAll() {
 		if pipeline.ID == pipelineID {
 			foundPipeline = pipeline
 			deletedPipelineIndex = index
+			break
 		}
-		index++
 	}
 
 	if foundPipeline.Name == "" {
@@ -302,9 +292,10 @@ func PipelineTrigger(c echo.Context) error {
 
 	// Look up pipeline for the given id
 	var foundPipeline gaia.Pipeline
-	for pipeline := range pipeline.GlobalActivePipelines.Iter() {
+	for _, pipeline := range pipeline.GlobalActivePipelines.GetAll() {
 		if pipeline.ID == pipelineID {
 			foundPipeline = pipeline
+			break
 		}
 	}
 
@@ -343,9 +334,10 @@ func PipelineResetToken(c echo.Context) error {
 
 	// Look up pipeline for the given id
 	var foundPipeline gaia.Pipeline
-	for pipeline := range pipeline.GlobalActivePipelines.Iter() {
+	for _, pipeline := range pipeline.GlobalActivePipelines.GetAll() {
 		if pipeline.ID == pipelineID {
 			foundPipeline = pipeline
+			break
 		}
 	}
 
@@ -408,9 +400,10 @@ func PipelineStart(c echo.Context) error {
 
 	// Look up pipeline for the given id
 	var foundPipeline gaia.Pipeline
-	for pipeline := range pipeline.GlobalActivePipelines.Iter() {
+	for _, pipeline := range pipeline.GlobalActivePipelines.GetAll() {
 		if pipeline.ID == pipelineID {
 			foundPipeline = pipeline
+			break
 		}
 	}
 
@@ -437,10 +430,7 @@ type getAllWithLatestRun struct {
 func PipelineGetAllWithLatestRun(c echo.Context) error {
 	// Get all active pipelines
 	storeService, _ := services.StorageService()
-	var pipelines []gaia.Pipeline
-	for pipeline := range pipeline.GlobalActivePipelines.Iter() {
-		pipelines = append(pipelines, pipeline)
-	}
+	pipelines := pipeline.GlobalActivePipelines.GetAll()
 
 	// Iterate all pipelines
 	var pipelinesWithLatestRun []getAllWithLatestRun

handlers/pipeline_run.go

@@ -73,9 +73,10 @@ func PipelineStop(c echo.Context) error {
 
 	// Look up pipeline for the given id
 	var foundPipeline gaia.Pipeline
-	for pipeline := range pipeline.GlobalActivePipelines.Iter() {
+	for _, pipeline := range pipeline.GlobalActivePipelines.GetAll() {
 		if pipeline.ID == p {
 			foundPipeline = pipeline
+			break
 		}
 	}
 

workers/pipeline/create_pipeline.go

@@ -177,17 +177,11 @@ func ValidatePipelineName(pName string) error {
 		}
 
 		// Check if pipeline name is already in use.
-		alreadyInUse := false
-		for activePipeline := range GlobalActivePipelines.Iter() {
+		for _, activePipeline := range GlobalActivePipelines.GetAll() {
 			if strings.ToLower(s) == strings.ToLower(activePipeline.Name) {
-				alreadyInUse = true
+				return errPipelineNameInUse
 			}
 		}
-
-		// Throw error because it's already in use.
-		if alreadyInUse {
-			return errPipelineNameInUse
-		}
 	}
 	return nil
 }

workers/pipeline/git.go

@@ -187,12 +187,9 @@ func gitCloneRepo(repo *gaia.GitRepo) error {
 
 func updateAllCurrentPipelines() {
 	gaia.Cfg.Logger.Debug("starting updating of pipelines...")
-	var allPipelines []gaia.Pipeline
+	allPipelines := GlobalActivePipelines.GetAll()
 	var wg sync.WaitGroup
 	sem := make(chan int, 4)
-	for pipeline := range GlobalActivePipelines.Iter() {
-		allPipelines = append(allPipelines, pipeline)
-	}
 	for _, p := range allPipelines {
 		wg.Add(1)
 		go func(pipe gaia.Pipeline) {

workers/pipeline/pipeline.go

@@ -133,18 +133,13 @@ func (ap *ActivePipelines) Remove(index int) {
 
 // GetByName looks up the pipeline by the given name.
 func (ap *ActivePipelines) GetByName(n string) *gaia.Pipeline {
-	var foundPipeline gaia.Pipeline
-	for pipeline := range ap.Iter() {
+	for _, pipeline := range ap.GetAll() {
 		if pipeline.Name == n {
-			foundPipeline = pipeline
+			return &pipeline
 		}
 	}
 
-	if foundPipeline.Name == "" {
-		return nil
-	}
-
-	return &foundPipeline
+	return nil
 }
 
 // Replace takes the given pipeline and replaces it in the ActivePipelines
@@ -174,62 +169,43 @@ func (ap *ActivePipelines) Replace(p gaia.Pipeline) bool {
 
 // ReplaceByName replaces the pipeline that has the given name with the given pipeline.
 func (ap *ActivePipelines) ReplaceByName(n string, p gaia.Pipeline) bool {
-
-	var index int
-	var pipelineIndex int
-	var found bool
-
-	for pipeline := range ap.Iter() {
+	for index, pipeline := range ap.GetAll() {
 		if pipeline.Name == n {
-			found = true
-			pipelineIndex = index
+			ap.Update(index, p)
+			return true
 		}
-		index++
 	}
-
-	if found {
-		ap.Update(pipelineIndex, p)
-	}
-
-	return found
-
+	return false
 }
 
-// Iter iterates over the pipelines in the concurrent slice.
-func (ap *ActivePipelines) Iter() <-chan gaia.Pipeline {
-	c := make(chan gaia.Pipeline)
-
-	go func() {
-		ap.RLock()
-		defer ap.RUnlock()
-		for _, pipeline := range ap.Pipelines {
-			c <- pipeline
-		}
-		close(c)
-	}()
-
+// GetAll iterates over the pipelines in the concurrent slice.
+func (ap *ActivePipelines) GetAll() []gaia.Pipeline {
+	c := make([]gaia.Pipeline, 0)
+	ap.RLock()
+	defer ap.RUnlock()
+	for _, pipeline := range ap.Pipelines {
+		c = append(c, pipeline)
+	}
 	return c
 }
 
 // Contains checks if the given pipeline name has been already appended
 // to the given ActivePipelines instance.
 func (ap *ActivePipelines) Contains(n string) bool {
-	var foundPipeline bool
-	for pipeline := range ap.Iter() {
+	for _, pipeline := range ap.GetAll() {
 		if pipeline.Name == n {
-			foundPipeline = true
+			return true
 		}
 	}
 
-	return foundPipeline
+	return false
 }
 
 // RemoveDeletedPipelines removes the pipelines whose names are NOT
 // present in `existingPipelineNames` from the given ActivePipelines instance.
 func (ap *ActivePipelines) RemoveDeletedPipelines(existingPipelineNames []string) {
 	var deletedPipelineIndices []int
-	var index int
-	for pipeline := range ap.Iter() {
+	for index, pipeline := range ap.GetAll() {
 		found := false
 		for _, name := range existingPipelineNames {
 			if pipeline.Name == name {
@@ -240,7 +216,6 @@ func (ap *ActivePipelines) RemoveDeletedPipelines(existingPipelineNames []string
 		if !found {
 			deletedPipelineIndices = append(deletedPipelineIndices, index)
 		}
-		index++
 	}
 	for _, idx := range deletedPipelineIndices {
 		ap.Remove(idx)

workers/pipeline/pipeline_test.go

@@ -74,7 +74,7 @@ func TestRemove(t *testing.T) {
 	ap.Remove(1)
 
 	count := 0
-	for pipeline := range ap.Iter() {
+	for _, pipeline := range ap.GetAll() {
 		count++
 		if pipeline.Name == "Pipeline B" {
 			t.Fatalf("Pipeline B still exists. It should have been removed.")
@@ -185,7 +185,7 @@ func TestIter(t *testing.T) {
 	}
 
 	count := 0
-	for pipeline := range ap.Iter() {
+	for _, pipeline := range ap.GetAll() {
 		count++
 		retrievedNames = append(retrievedNames, pipeline.Name)
 	}
@@ -246,7 +246,7 @@ func TestRemoveDeletedPipelines(t *testing.T) {
 
 	ap.RemoveDeletedPipelines(existingPipelineNames)
 
-	for pipeline := range ap.Iter() {
+	for _, pipeline := range ap.GetAll() {
 		if pipeline.Name == "Pipeline B" {
 			t.Fatalf("Pipeline B still exists. It should have been removed.")
 		}