From ed51df29b3a11c11b7fc5f10e73ba15fbd47cf46 Mon Sep 17 00:00:00 2001
From: Laszlo Fogas <laszlo@laszlo.cloud>
Date: Mon, 22 Jul 2019 13:43:18 +0200
Subject: [PATCH] Factor out sideeffects

---
 server/build.go            |  2 ++
 server/hook.go             |  8 ++++----
 server/procBuilder.go      | 13 ++++++++-----
 server/procBuilder_test.go |  3 ++-
 4 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/server/build.go b/server/build.go
index b94a7e119..65f7e39da 100644
--- a/server/build.go
+++ b/server/build.go
@@ -344,6 +344,7 @@ func PostApproval(c *gin.Context) {
 		store.UpdateBuild(c, build)
 		return
 	}
+	build = setBuildStepsOnBuild(b.Curr, buildItems)
 
 	err = store.FromContext(c).ProcCreate(build.Procs)
 	if err != nil {
@@ -558,6 +559,7 @@ func PostBuild(c *gin.Context) {
 		c.JSON(500, build)
 		return
 	}
+	build = setBuildStepsOnBuild(b.Curr, buildItems)
 
 	err = store.FromContext(c).ProcCreate(build.Procs)
 	if err != nil {
diff --git a/server/hook.go b/server/hook.go
index 6fe847d24..89818441d 100644
--- a/server/hook.go
+++ b/server/hook.go
@@ -177,7 +177,7 @@ func PostHook(c *gin.Context) {
 		return
 	}
 
-	if zeroSteps(*build, remoteYamlConfigs) {
+	if zeroSteps(build, remoteYamlConfigs) {
 		c.String(200, "Step conditions yield zero runnable steps")
 		return
 	}
@@ -261,6 +261,7 @@ func PostHook(c *gin.Context) {
 		store.UpdateBuild(c, build)
 		return
 	}
+	build = setBuildStepsOnBuild(b.Curr, buildItems)
 
 	err = store.FromContext(c).ProcCreate(build.Procs)
 	if err != nil {
@@ -298,11 +299,10 @@ func branchFiltered(build *model.Build, remoteYamlConfigs []*remote.FileMeta) bo
 	return true
 }
 
-// uses pass by value as procBuilder has side effects on build. Something to be fixed
-func zeroSteps(build model.Build, remoteYamlConfigs []*remote.FileMeta) bool {
+func zeroSteps(build *model.Build, remoteYamlConfigs []*remote.FileMeta) bool {
 	b := procBuilder{
 		Repo:  &model.Repo{},
-		Curr:  &build,
+		Curr:  build,
 		Last:  &model.Build{},
 		Netrc: &model.Netrc{},
 		Secs:  []*model.Secret{},
diff --git a/server/procBuilder.go b/server/procBuilder.go
index a78318d90..7ef21ef54 100644
--- a/server/procBuilder.go
+++ b/server/procBuilder.go
@@ -128,14 +128,11 @@ func (b *procBuilder) Build() ([]*buildItem, error) {
 				item.Labels = map[string]string{}
 			}
 
-			b.Curr.Procs = append(b.Curr.Procs, proc)
 			items = append(items, item)
 			pidSequence++
 		}
 	}
 
-	setBuildSteps(b.Curr, items)
-
 	return items, nil
 }
 
@@ -214,8 +211,12 @@ func (b *procBuilder) toInternalRepresentation(parsed *yaml.Config, environ map[
 	).Compile(parsed)
 }
 
-func setBuildSteps(build *model.Build, buildItems []*buildItem) {
-	pcounter := len(buildItems)
+func setBuildStepsOnBuild(build *model.Build, buildItems []*buildItem) *model.Build {
+	for _, item := range buildItems {
+		build.Procs = append(build.Procs, item.Proc)
+	}
+	pcounter := len(build.Procs)
+
 	for _, item := range buildItems {
 		for _, stage := range item.Config.Stages {
 			var gid int
@@ -239,6 +240,8 @@ func setBuildSteps(build *model.Build, buildItems []*buildItem) {
 			}
 		}
 	}
+
+	return build
 }
 
 // return the metadata from the cli context.
diff --git a/server/procBuilder_test.go b/server/procBuilder_test.go
index 1c456cc29..ea3925ee9 100644
--- a/server/procBuilder_test.go
+++ b/server/procBuilder_test.go
@@ -262,7 +262,8 @@ pipeline:
 		},
 	}
 
-	_, err := b.Build()
+	buildItems, err := b.Build()
+	build = setBuildStepsOnBuild(build, buildItems)
 	if err != nil {
 		t.Fatal(err)
 	}