Refactor response writer & access logger (#33323)

And add comments & tests

services/context/access_log.go

@@ -18,13 +18,14 @@ import (
 	"code.gitea.io/gitea/modules/web/middleware"
 )
 
-type routerLoggerOptions struct {
-	req            *http.Request
+type accessLoggerTmplData struct {
 	Identity       *string
 	Start          *time.Time
-	ResponseWriter http.ResponseWriter
-	Ctx            map[string]any
-	RequestID      *string
+	ResponseWriter struct {
+		Status, Size int
+	}
+	Ctx       map[string]any
+	RequestID *string
 }
 
 const keyOfRequestIDInTemplate = ".RequestID"
@@ -51,51 +52,65 @@ func parseRequestIDFromRequestHeader(req *http.Request) string {
 	return requestID
 }
 
+type accessLogRecorder struct {
+	logger        log.BaseLogger
+	logTemplate   *template.Template
+	needRequestID bool
+}
+
+func (lr *accessLogRecorder) record(start time.Time, respWriter ResponseWriter, req *http.Request) {
+	var requestID string
+	if lr.needRequestID {
+		requestID = parseRequestIDFromRequestHeader(req)
+	}
+
+	reqHost, _, err := net.SplitHostPort(req.RemoteAddr)
+	if err != nil {
+		reqHost = req.RemoteAddr
+	}
+
+	identity := "-"
+	data := middleware.GetContextData(req.Context())
+	if signedUser, ok := data[middleware.ContextDataKeySignedUser].(*user_model.User); ok {
+		identity = signedUser.Name
+	}
+	buf := bytes.NewBuffer([]byte{})
+	tmplData := accessLoggerTmplData{
+		Identity: &identity,
+		Start:    &start,
+		Ctx: map[string]any{
+			"RemoteAddr": req.RemoteAddr,
+			"RemoteHost": reqHost,
+			"Req":        req,
+		},
+		RequestID: &requestID,
+	}
+	tmplData.ResponseWriter.Status = respWriter.Status()
+	tmplData.ResponseWriter.Size = respWriter.WrittenSize()
+	err = lr.logTemplate.Execute(buf, tmplData)
+	if err != nil {
+		log.Error("Could not execute access logger template: %v", err.Error())
+	}
+
+	lr.logger.Log(1, log.INFO, "%s", buf.String())
+}
+
+func newAccessLogRecorder() *accessLogRecorder {
+	return &accessLogRecorder{
+		logger:        log.GetLogger("access"),
+		logTemplate:   template.Must(template.New("log").Parse(setting.Log.AccessLogTemplate)),
+		needRequestID: len(setting.Log.RequestIDHeaders) > 0 && strings.Contains(setting.Log.AccessLogTemplate, keyOfRequestIDInTemplate),
+	}
+}
+
 // AccessLogger returns a middleware to log access logger
 func AccessLogger() func(http.Handler) http.Handler {
-	logger := log.GetLogger("access")
-	needRequestID := len(setting.Log.RequestIDHeaders) > 0 && strings.Contains(setting.Log.AccessLogTemplate, keyOfRequestIDInTemplate)
-	logTemplate, _ := template.New("log").Parse(setting.Log.AccessLogTemplate)
+	recorder := newAccessLogRecorder()
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 			start := time.Now()
-
-			var requestID string
-			if needRequestID {
-				requestID = parseRequestIDFromRequestHeader(req)
-			}
-
-			reqHost, _, err := net.SplitHostPort(req.RemoteAddr)
-			if err != nil {
-				reqHost = req.RemoteAddr
-			}
-
 			next.ServeHTTP(w, req)
-			rw := w.(ResponseWriter)
-
-			identity := "-"
-			data := middleware.GetContextData(req.Context())
-			if signedUser, ok := data[middleware.ContextDataKeySignedUser].(*user_model.User); ok {
-				identity = signedUser.Name
-			}
-			buf := bytes.NewBuffer([]byte{})
-			err = logTemplate.Execute(buf, routerLoggerOptions{
-				req:            req,
-				Identity:       &identity,
-				Start:          &start,
-				ResponseWriter: rw,
-				Ctx: map[string]any{
-					"RemoteAddr": req.RemoteAddr,
-					"RemoteHost": reqHost,
-					"Req":        req,
-				},
-				RequestID: &requestID,
-			})
-			if err != nil {
-				log.Error("Could not execute access logger template: %v", err.Error())
-			}
-
-			logger.Info("%s", buf.String())
+			recorder.record(start, w.(ResponseWriter), req)
 		})
 	}
 }