From 9e6348c3b56ca159069edc212f6ab57f7ac64acf Mon Sep 17 00:00:00 2001
From: ChrisERo <cer95@cornell.edu>
Date: Fri, 7 Jan 2022 20:02:39 -0500
Subject: [PATCH] fix: resolve issue number 463

Resolved original issue by introducing a boolean chanel by which exechook runner can communicate with main thread.
Then introduced and used webhook executed-at-least-once chanel and added documentation explaining sections of of code
only executed when git-sync pulls for first time.
---
 cmd/git-sync/main.go | 25 +++++++++++++++++++++++++
 pkg/hook/hook.go     | 19 +++++++++++++++++--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go
index 2327081..7a67427 100644
--- a/cmd/git-sync/main.go
+++ b/cmd/git-sync/main.go
@@ -174,6 +174,9 @@ var (
 	}, []string{"status"})
 )
 
+// Channels for ensuring hooks execute at least once before terminating due to GIT_SYNC_ONCE
+var execHookChannel, webHookChannel chan bool  // initialising as null to promptly catch logical errors and to avoid unneeded object creation
+
 const (
 	metricKeySuccess = "success"
 	metricKeyError   = "error"
@@ -359,6 +362,8 @@ func main() {
 		if *flExechookBackoff < time.Second {
 			handleError(log, true, "ERROR: --exechook-backoff must be at least 1s")
 		}
+
+		execHookChannel = make(chan bool)
 	}
 
 	if *flWebhookURL != "" {
@@ -371,6 +376,8 @@ func main() {
 		if *flWebhookBackoff < time.Second {
 			handleError(log, true, "ERROR: --webhook-backoff must be at least 1s")
 		}
+
+		webHookChannel = make(chan bool)
 	}
 
 	if *flPassword != "" && *flPasswordFile != "" {
@@ -556,6 +563,7 @@ func main() {
 			*flWebhookBackoff,
 			hook.NewHookData(),
 			log,
+			webHookChannel,
 		)
 		go webhookRunner.Run(context.Background())
 	}
@@ -576,6 +584,7 @@ func main() {
 			*flExechookBackoff,
 			hook.NewHookData(),
 			log,
+			execHookChannel,
 		)
 		go exechookRunner.Run(context.Background())
 	}
@@ -621,6 +630,22 @@ func main() {
 		}
 
 		if initialSync {
+			// Wait for hooks to complete at least once before checking whether to stop
+			// Assumes that if hook channels are not nil, they will have at least one value before getting closed
+			if execHookChannel != nil {
+				execHookChannelFinishedSuccessfully:= <-execHookChannel
+				if !execHookChannelFinishedSuccessfully {
+					log.Error(nil, "exec hook completed with error")
+				}
+			}
+			if webHookChannel != nil {
+				webHookChannelFinishedSuccessfully:= <-webHookChannel
+				if !webHookChannelFinishedSuccessfully {
+					log.Error(nil, "web hook completed with error")
+				}
+			}
+
+			// Determine if git-sync should terminate
 			if *flOneTime {
 				log.DeleteErrorFile()
 				os.Exit(0)
diff --git a/pkg/hook/hook.go b/pkg/hook/hook.go
index d5b9857..ce027f2 100644
--- a/pkg/hook/hook.go
+++ b/pkg/hook/hook.go
@@ -82,8 +82,8 @@ func (d *hookData) send(newHash string) {
 }
 
 // NewHookRunner returns a new HookRunner
-func NewHookRunner(hook Hook, backoff time.Duration, data *hookData, log *logging.Logger) *HookRunner {
-	return &HookRunner{hook: hook, backoff: backoff, data: data, logger: log}
+func NewHookRunner(hook Hook, backoff time.Duration, data *hookData, log *logging.Logger, hasSucceededOnce chan bool) *HookRunner {
+	return &HookRunner{hook: hook, backoff: backoff, data: data, logger: log, hasCompletedOnce: hasSucceededOnce}
 }
 
 // HookRunner struct
@@ -96,6 +96,8 @@ type HookRunner struct {
 	data *hookData
 	// Logger
 	logger *logging.Logger
+	// Has succeeded once Chanel, sends true if first run executed successfully and false if it failed
+	hasCompletedOnce chan bool
 }
 
 // Send sends hash to hookdata
@@ -123,16 +125,29 @@ func (r *HookRunner) Run(ctx context.Context) {
 			if err := r.hook.Do(ctx, hash); err != nil {
 				r.logger.Error(err, "hook failed")
 				updateHookRunCountMetric(r.hook.Name(), "error")
+				// don't want to sleep unnecessarily if we are going to terminate anyways
+				r.sendCompletedOnceMessage(false)
 				time.Sleep(r.backoff)
 			} else {
 				updateHookRunCountMetric(r.hook.Name(), "success")
 				lastHash = hash
+				r.sendCompletedOnceMessage(true)
 				break
 			}
 		}
 	}
 }
 
+// sendCompletedOnceMessage forwards the success status (as a boolean) of the first execution of HookRunner, the first time
+// to the r.hasCompletedOnce channel
+func (r *HookRunner) sendCompletedOnceMessage(completedSuccessfully bool) {
+	if r.hasCompletedOnce != nil {
+		r.hasCompletedOnce <- completedSuccessfully
+		close(r.hasCompletedOnce)
+		r.hasCompletedOnce = nil
+	}
+}
+
 func updateHookRunCountMetric(name, status string) {
 	hookRunCount.WithLabelValues(name, status).Inc()
 }