From 9320ffd2b5f02c6c94a231dbf51fc6883afff429 Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
Date: Wed, 3 Apr 2024 01:17:25 +0200
Subject: [PATCH] [BUG] Make delay writer actually work

- Reading the code of this delay writer implemenation, it looks like
that it should only actually write content to the `io.Writer` if x
amount of time has passed by. However in practice it was always printing
the buffer even if the X amount of time didn't pass yet. This is in line
with what was being said in the issue that this was to help with
https://github.com/go-gitea/gitea/issues/9610.
- This was caused by the extra `Close()` calls which in turn caused that
when the second `Close` is called (which is done in a defer already) it
would've printed the buffer anyway. So remove the extra calls to `Close()`.
- Add unit test.
---
 cmd/hook.go      |  9 ++----
 cmd/hook_test.go | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/cmd/hook.go b/cmd/hook.go
index 48a64fd197..15ed1eb7f4 100644
--- a/cmd/hook.go
+++ b/cmd/hook.go
@@ -347,11 +347,10 @@ Forgejo or set your environment appropriately.`, "")
 	}
 
 	var out io.Writer
-	var dWriter *delayWriter
 	out = &nilWriter{}
 	if setting.Git.VerbosePush {
 		if setting.Git.VerbosePushDelay > 0 {
-			dWriter = newDelayWriter(os.Stdout, setting.Git.VerbosePushDelay)
+			dWriter := newDelayWriter(os.Stdout, setting.Git.VerbosePushDelay)
 			defer dWriter.Close()
 			out = dWriter
 		} else {
@@ -414,7 +413,6 @@ Forgejo or set your environment appropriately.`, "")
 			hookOptions.RefFullNames = refFullNames
 			resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
 			if extra.HasError() {
-				_ = dWriter.Close()
 				hookPrintResults(results)
 				return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error)
 			}
@@ -434,7 +432,6 @@ Forgejo or set your environment appropriately.`, "")
 		}
 		fmt.Fprintf(out, "Processed %d references in total\n", total)
 
-		_ = dWriter.Close()
 		hookPrintResults(results)
 		return nil
 	}
@@ -447,7 +444,6 @@ Forgejo or set your environment appropriately.`, "")
 
 	resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
 	if resp == nil {
-		_ = dWriter.Close()
 		hookPrintResults(results)
 		return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error)
 	}
@@ -463,9 +459,8 @@ Forgejo or set your environment appropriately.`, "")
 			return fail(ctx, extra.UserMsg, "SetDefaultBranch failed: %v", extra.Error)
 		}
 	}
-	_ = dWriter.Close()
-	hookPrintResults(results)
 
+	hookPrintResults(results)
 	return nil
 }
 
diff --git a/cmd/hook_test.go b/cmd/hook_test.go
index d4e16dc411..0c1bee29f4 100644
--- a/cmd/hook_test.go
+++ b/cmd/hook_test.go
@@ -7,10 +7,20 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"os"
 	"strings"
 	"testing"
+	"time"
+
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v2"
 )
 
 func TestPktLine(t *testing.T) {
@@ -83,3 +93,72 @@ func TestPktLine(t *testing.T) {
 		assert.Empty(t, w.Bytes())
 	})
 }
+
+func TestDelayWriter(t *testing.T) {
+	// Setup the environment.
+	defer test.MockVariableValue(&setting.InternalToken, "Random")()
+	defer test.MockVariableValue(&setting.InstallLock, true)()
+	defer test.MockVariableValue(&setting.Git.VerbosePush, true)()
+	require.NoError(t, os.Setenv("SSH_ORIGINAL_COMMAND", "true"))
+
+	// Setup the Stdin.
+	f, err := os.OpenFile(t.TempDir()+"/stdin", os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o666)
+	require.NoError(t, err)
+	_, err = f.Write([]byte("00000000000000000000 00000000000000000001 refs/head/main\n"))
+	require.NoError(t, err)
+	_, err = f.Seek(0, 0)
+	require.NoError(t, err)
+	defer test.MockVariableValue(os.Stdin, *f)()
+
+	// Setup the server that processes the hooks.
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(time.Millisecond * 600)
+	}))
+	defer ts.Close()
+	defer test.MockVariableValue(&setting.LocalURL, ts.URL+"/")()
+
+	app := cli.NewApp()
+	app.Commands = []*cli.Command{subcmdHookPreReceive}
+
+	// Capture what's being written into stdout
+	captureStdout := func(t *testing.T) (finish func() (output string)) {
+		t.Helper()
+
+		r, w, err := os.Pipe()
+		require.NoError(t, err)
+		resetStdout := test.MockVariableValue(os.Stdout, *w)
+
+		return func() (output string) {
+			w.Close()
+			resetStdout()
+
+			out, err := io.ReadAll(r)
+			require.NoError(t, err)
+			return string(out)
+		}
+	}
+
+	t.Run("Should delay", func(t *testing.T) {
+		defer test.MockVariableValue(&setting.Git.VerbosePushDelay, time.Millisecond*500)()
+		finish := captureStdout(t)
+
+		err = app.Run([]string{"./forgejo", "pre-receive"})
+		require.NoError(t, err)
+		out := finish()
+
+		require.Contains(t, out, "* Checking 1 references")
+		require.Contains(t, out, "Checked 1 references in total")
+	})
+
+	t.Run("Shouldn't delay", func(t *testing.T) {
+		defer test.MockVariableValue(&setting.Git.VerbosePushDelay, time.Second*5)()
+		finish := captureStdout(t)
+
+		err = app.Run([]string{"./forgejo", "pre-receive"})
+		require.NoError(t, err)
+		out := finish()
+
+		require.NoError(t, err)
+		require.Empty(t, out)
+	})
+}