refactor(web): reduce cognitive complexity in middleware and monitors

webapp/backend/pkg/web/heartbeat_monitor.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/analogj/scrutiny/webapp/backend/pkg"
 	"github.com/analogj/scrutiny/webapp/backend/pkg/database"
+	"github.com/analogj/scrutiny/webapp/backend/pkg/models"
 	"github.com/analogj/scrutiny/webapp/backend/pkg/notify"
 	"github.com/sirupsen/logrus"
 )
@@ -187,62 +188,38 @@ func (m *HeartbeatMonitor) checkAndSendHeartbeat() {
 	deviceRepo, err := m.getOrCreateRepo()
 	if err != nil {
 		m.logger.Errorf("Failed to get/create repository for heartbeat: %v", err)
-		m.statusMu.Lock()
-		m.lastError = err
-		m.lastErrorTime = time.Now()
-		m.statusMu.Unlock()
+		m.recordError(err)
 		return
 	}
 
 	settings, err := deviceRepo.LoadSettings(m.ctx)
 	if err != nil {
 		m.resetRepo()
 		m.logger.Errorf("Failed to load settings for heartbeat: %v", err)
-		m.statusMu.Lock()
-		m.lastError = err
-		m.lastErrorTime = time.Now()
-		m.statusMu.Unlock()
+		m.recordError(err)
 		return
 	}
 
 	if settings == nil || !settings.Metrics.HeartbeatEnabled {
 		m.logger.Debug("Heartbeat notifications are disabled")
-		m.statusMu.Lock()
-		m.lastError = nil
-		m.statusMu.Unlock()
+		m.clearError()
 		return
 	}
 
 	devices, err := deviceRepo.GetDevices(m.ctx)
 	if err != nil {
 		m.resetRepo()
 		m.logger.Errorf("Failed to load devices for heartbeat: %v", err)
-		m.statusMu.Lock()
-		m.lastError = err
-		m.lastErrorTime = time.Now()
-		m.statusMu.Unlock()
+		m.recordError(err)
 		return
 	}
 
 	// Clear previous error on successful data load
-	m.statusMu.Lock()
-	m.lastError = nil
-	m.statusMu.Unlock()
+	m.clearError()
 
 	// Filter to monitored devices (not archived, not muted)
 	totalCount := len(devices)
-	monitoredCount := 0
-	allHealthy := true
-
-	for _, device := range devices {
-		if device.Archived || device.Muted {
-			continue
-		}
-		monitoredCount++
-		if device.DeviceStatus != pkg.DeviceStatusPassed {
-			allHealthy = false
-		}
-	}
+	monitoredCount, allHealthy := countHealthyMonitored(devices)
 
 	// Don't send heartbeat if no monitored devices
 	if monitoredCount == 0 {
@@ -258,22 +235,59 @@ func (m *HeartbeatMonitor) checkAndSendHeartbeat() {
 
 	// All monitored drives are healthy -- send heartbeat
 	m.logger.Infof("All %d monitored drives healthy, sending heartbeat notification", monitoredCount)
+	m.sendHeartbeatNotification(monitoredCount, totalCount, settings)
+}
+
+// recordError stores the most recent heartbeat error and the time it occurred.
+func (m *HeartbeatMonitor) recordError(err error) {
+	m.statusMu.Lock()
+	m.lastError = err
+	m.lastErrorTime = time.Now()
+	m.statusMu.Unlock()
+}
 
+// clearError clears the most recent heartbeat error after a successful operation.
+func (m *HeartbeatMonitor) clearError() {
+	m.statusMu.Lock()
+	m.lastError = nil
+	m.statusMu.Unlock()
+}
+
+// countHealthyMonitored returns the count of monitored (non-archived, non-muted) devices and
+// whether all of them currently report a passed status.
+func countHealthyMonitored(devices []models.Device) (monitoredCount int, allHealthy bool) {
+	allHealthy = true
+	for i := range devices {
+		if devices[i].Archived || devices[i].Muted {
+			continue
+		}
+		monitoredCount++
+		if devices[i].DeviceStatus != pkg.DeviceStatusPassed {
+			allHealthy = false
+		}
+	}
+	return monitoredCount, allHealthy
+}
+
+// sendHeartbeatNotification builds and dispatches the heartbeat, routing through the notification
+// gate when present (bypassing quiet hours, as heartbeats are informational).
+func (m *HeartbeatMonitor) sendHeartbeatNotification(monitoredCount, totalCount int, settings *models.Settings) {
 	notification := notify.NewHeartbeat(m.logger, m.appEngine.Config, monitoredCount, totalCount)
 	notification.LoadHeartbeatDatabaseUrls(m.ctx, m.deviceRepo)
 
-	// Route through notification gate (bypass quiet hours -- heartbeats are informational)
 	if gate := m.appEngine.NotificationGate; gate != nil {
 		gate.TrySend(&notification, settings, true)
-	} else {
-		if err := notification.Send(); err != nil {
-			if err.Error() == "no notification endpoints configured" {
-				m.logger.Warn("Heartbeat ready but no notification endpoints are configured. Configure notify.urls in scrutiny.yaml to receive heartbeat alerts.")
-				return
-			}
-			m.logger.Errorf("Failed to send heartbeat notification: %v", err)
+		m.logger.Info("Heartbeat notification sent successfully")
+		return
+	}
+
+	if err := notification.Send(); err != nil {
+		if err.Error() == "no notification endpoints configured" {
+			m.logger.Warn("Heartbeat ready but no notification endpoints are configured. Configure notify.urls in scrutiny.yaml to receive heartbeat alerts.")
 			return
 		}
+		m.logger.Errorf("Failed to send heartbeat notification: %v", err)
+		return
 	}
 
 	m.logger.Info("Heartbeat notification sent successfully")

webapp/backend/pkg/web/middleware/auth.go

@@ -182,58 +182,63 @@ func AuthMiddleware(appConfig config.Interface, logger *logrus.Entry) gin.Handle
 
 	return func(c *gin.Context) {
 		c.Set("AUTH_ENABLED", ac.authEnabled)
-		requestPath := c.Request.URL.Path
-
-		// When general auth is disabled, only metrics may require its own token.
-		if !ac.authEnabled {
-			if ac.metricsToken != "" && isMetricsPath(requestPath) && !ac.validateMetricsToken(c) {
-				rejectUnauthorized(c, "Metrics endpoint requires authentication. Provide a Bearer token.")
-				return
-			}
-			c.Next()
-			return
-		}
+		ac.authorize(c)
+	}
+}
 
-		// Served docs can be public or private independently of the broader auth toggle.
-		if isDocsPath(requestPath) {
-			if ac.docsPublic {
-				c.Next()
-				return
-			}
-			if ac.validateGeneralAuth(c) {
-				c.Next()
-				return
-			}
-			rejectMissingOrInvalid(c)
-			return
-		}
+// authorize applies the request-path-based authentication rules for a single request.
+func (ac *authContext) authorize(c *gin.Context) {
+	requestPath := c.Request.URL.Path
 
-		// Non-API routes (frontend static files, SPA routes) bypass auth.
-		// Only /api/* paths are subject to authentication.
-		if !strings.Contains(requestPath, "/api/") {
-			c.Next()
-			return
-		}
+	if !ac.authEnabled {
+		ac.handleAuthDisabled(c, requestPath)
+		return
+	}
 
-		// Public API routes bypass authentication entirely.
-		if isPublicPath(requestPath) {
-			c.Next()
-			return
-		}
+	// Served docs can be public or private independently of the broader auth toggle.
+	if isDocsPath(requestPath) {
+		ac.handleDocsPath(c)
+		return
+	}
 
-		// Dedicated metrics token: accepted as an alternative for /api/metrics.
-		if isMetricsPath(requestPath) && ac.validateMetricsToken(c) {
-			c.Set("AUTH_TYPE", "metrics_token")
-			c.Next()
-			return
-		}
+	// Non-API routes (frontend static files, SPA routes) and public API routes bypass auth.
+	// Only authenticated /api/* paths are subject to authentication.
+	if !strings.Contains(requestPath, "/api/") || isPublicPath(requestPath) {
+		c.Next()
+		return
+	}
 
-		// General auth: API token or JWT.
-		if ac.validateGeneralAuth(c) {
-			c.Next()
-			return
-		}
+	// Dedicated metrics token: accepted as an alternative for /api/metrics.
+	if isMetricsPath(requestPath) && ac.validateMetricsToken(c) {
+		c.Set("AUTH_TYPE", "metrics_token")
+		c.Next()
+		return
+	}
+
+	// General auth: API token or JWT.
+	if ac.validateGeneralAuth(c) {
+		c.Next()
+		return
+	}
+
+	rejectMissingOrInvalid(c)
+}
+
+// handleAuthDisabled handles requests when general auth is disabled: only the metrics endpoint may
+// still require its own token.
+func (ac *authContext) handleAuthDisabled(c *gin.Context, requestPath string) {
+	if ac.metricsToken != "" && isMetricsPath(requestPath) && !ac.validateMetricsToken(c) {
+		rejectUnauthorized(c, "Metrics endpoint requires authentication. Provide a Bearer token.")
+		return
+	}
+	c.Next()
+}
 
-		rejectMissingOrInvalid(c)
+// handleDocsPath authorizes the served docs path, which may be public or require general auth.
+func (ac *authContext) handleDocsPath(c *gin.Context) {
+	if ac.docsPublic || ac.validateGeneralAuth(c) {
+		c.Next()
+		return
 	}
+	rejectMissingOrInvalid(c)
 }

webapp/backend/pkg/web/middleware/logger.go

@@ -36,16 +36,7 @@ func LoggerMiddleware(logger *logrus.Entry) gin.HandlerFunc {
 	}
 
 	return func(c *gin.Context) {
-
-		//clone the request body reader.
-		var reqBody string
-		if c.Request.Body != nil {
-			buf, _ := ioutil.ReadAll(c.Request.Body)
-			reqBodyReader1 := ioutil.NopCloser(bytes.NewBuffer(buf))
-			reqBodyReader2 := ioutil.NopCloser(bytes.NewBuffer(buf)) //We have to create a new Buffer, because reqBodyReader1 will be read.
-			c.Request.Body = reqBodyReader2
-			reqBody = readBody(reqBodyReader1)
-		}
+		reqBody := cloneRequestBody(c)
 
 		// other handler can change c.Path so:
 		path := c.Request.URL.Path
@@ -54,8 +45,7 @@ func LoggerMiddleware(logger *logrus.Entry) gin.HandlerFunc {
 		c.Set("LOGGER", logger)
 		start := time.Now()
 		c.Next()
-		stop := time.Since(start)
-		latency := int(math.Ceil(float64(stop.Nanoseconds()) / 1000000.0))
+		latency := int(math.Ceil(float64(time.Since(start).Nanoseconds()) / 1000000.0))
 		statusCode := c.Writer.Status()
 		clientIP := c.ClientIP()
 		clientUserAgent := c.Request.UserAgent()
@@ -77,26 +67,51 @@ func LoggerMiddleware(logger *logrus.Entry) gin.HandlerFunc {
 			"userAgent":  clientUserAgent,
 		})
 
-		if len(c.Errors) > 0 {
-			entry.Error(c.Errors.ByType(gin.ErrorTypePrivate).String())
-		} else {
-			msg := fmt.Sprintf("%s - %s [%s] \"%s %s\" %d %d \"%s\" \"%s\" (%dms)", clientIP, hostname, time.Now().Format(timeFormat), c.Request.Method, path, statusCode, respLength, referer, clientUserAgent, latency)
-			if statusCode >= http.StatusInternalServerError {
-				entry.Error(msg)
-			} else if statusCode >= http.StatusBadRequest {
-				entry.Warn(msg)
-			} else {
-				entry.Info(msg)
-			}
-		}
-		if strings.Contains(path, "/api/") {
-			//only debug log request/response from api endpoint.
-			if len(reqBody) > 0 {
-				entry.WithField("bodyType", "request").Debugln(reqBody) // Print request body
-			}
-			entry.WithField("bodyType", "response").Debugln(blw.body.String())
-		}
+		//nolint:gocritic // fixed access-log format; %q would change the quoting/escaping
+		msg := fmt.Sprintf("%s - %s [%s] \"%s %s\" %d %d \"%s\" \"%s\" (%dms)", clientIP, hostname, time.Now().Format(timeFormat), c.Request.Method, path, statusCode, respLength, referer, clientUserAgent, latency)
+		logRequestResult(c, entry, msg, statusCode)
+		logAPIBodies(entry, path, reqBody, blw.body.String())
+	}
+}
+
+// cloneRequestBody reads and replaces the request body so it can be both logged and consumed
+// downstream, returning the body content (empty when there is no body).
+func cloneRequestBody(c *gin.Context) string {
+	if c.Request.Body == nil {
+		return ""
+	}
+	buf, _ := ioutil.ReadAll(c.Request.Body)
+	// We have to create a new Buffer because the read reader will be consumed.
+	c.Request.Body = ioutil.NopCloser(bytes.NewBuffer(buf))
+	return readBody(ioutil.NopCloser(bytes.NewBuffer(buf)))
+}
+
+// logRequestResult logs the request at the level appropriate for its status code, or logs the
+// private gin errors when present.
+func logRequestResult(c *gin.Context, entry *logrus.Entry, msg string, statusCode int) {
+	if len(c.Errors) > 0 {
+		entry.Error(c.Errors.ByType(gin.ErrorTypePrivate).String())
+		return
+	}
+	switch {
+	case statusCode >= http.StatusInternalServerError:
+		entry.Error(msg)
+	case statusCode >= http.StatusBadRequest:
+		entry.Warn(msg)
+	default:
+		entry.Info(msg)
+	}
+}
+
+// logAPIBodies debug-logs the request and response bodies for /api/ endpoints only.
+func logAPIBodies(entry *logrus.Entry, path, reqBody, respBody string) {
+	if !strings.Contains(path, "/api/") {
+		return
+	}
+	if len(reqBody) > 0 {
+		entry.WithField("bodyType", "request").Debugln(reqBody) // Print request body
 	}
+	entry.WithField("bodyType", "response").Debugln(respBody)
 }
 
 // Response Logging