Browse Source

chore: gocyclo fixes

jamesread 5 months ago
parent
commit
84bd405ca9
3 changed files with 134 additions and 76 deletions
  1. 29 27
      service/internal/api/api.go
  2. 10 10
      service/internal/config/config.go
  3. 95 39
      service/internal/executor/executor.go

+ 29 - 27
service/internal/api/api.go

@@ -276,39 +276,41 @@ func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Re
 	}
 }
 
-func (api *oliveTinAPI) internalLogEntryToPb(logEntry *executor.InternalLogEntry, authenticatedUser *authpublic.AuthenticatedUser) *apiv1.LogEntry {
-	pble := &apiv1.LogEntry{
-		ActionTitle:         logEntry.ActionTitle,
-		ActionIcon:          logEntry.ActionIcon,
-		DatetimeStarted:     logEntry.DatetimeStarted.Format("2006-01-02 15:04:05"),
-		DatetimeFinished:    logEntry.DatetimeFinished.Format("2006-01-02 15:04:05"),
-		DatetimeIndex:       logEntry.Index,
-		Output:              logEntry.Output,
-		TimedOut:            logEntry.TimedOut,
-		Blocked:             logEntry.Blocked,
-		ExitCode:            logEntry.ExitCode,
-		Tags:                logEntry.Tags,
-		ExecutionTrackingId: logEntry.ExecutionTrackingID,
-		ExecutionStarted:    logEntry.ExecutionStarted,
-		ExecutionFinished:   logEntry.ExecutionFinished,
-		User:                logEntry.Username,
+func calculateRateLimitExpires(api *oliveTinAPI, logEntry *executor.InternalLogEntry) string {
+	if logEntry.Binding == nil || logEntry.Binding.Action == nil {
+		return ""
 	}
 
-	if !pble.ExecutionFinished {
-		pble.CanKill = acl.IsAllowedKill(api.cfg, authenticatedUser, logEntry.Binding.Action)
+	expiryUnix := api.executor.GetTimeUntilAvailable(logEntry.Binding)
+	if expiryUnix <= 0 {
+		return ""
 	}
 
-	// Calculate rate limit expiry for the action
-	if logEntry.Binding != nil && logEntry.Binding.Action != nil {
-		pble.BindingId = logEntry.Binding.ID
+	return time.Unix(expiryUnix, 0).Format("2006-01-02 15:04:05")
+}
 
-		expiryUnix := api.executor.GetTimeUntilAvailable(logEntry.Binding)
+func (api *oliveTinAPI) internalLogEntryToPb(logEntry *executor.InternalLogEntry, authenticatedUser *authpublic.AuthenticatedUser) *apiv1.LogEntry {
+	pble := &apiv1.LogEntry{
+		ActionTitle:              logEntry.ActionTitle,
+		ActionIcon:               logEntry.ActionIcon,
+		DatetimeStarted:          logEntry.DatetimeStarted.Format("2006-01-02 15:04:05"),
+		DatetimeFinished:         logEntry.DatetimeFinished.Format("2006-01-02 15:04:05"),
+		DatetimeIndex:            logEntry.Index,
+		Output:                   logEntry.Output,
+		TimedOut:                 logEntry.TimedOut,
+		Blocked:                  logEntry.Blocked,
+		ExitCode:                 logEntry.ExitCode,
+		Tags:                     logEntry.Tags,
+		ExecutionTrackingId:      logEntry.ExecutionTrackingID,
+		ExecutionStarted:         logEntry.ExecutionStarted,
+		ExecutionFinished:        logEntry.ExecutionFinished,
+		User:                     logEntry.Username,
+		BindingId:                logEntry.Binding.ID,
+		DatetimeRateLimitExpires: calculateRateLimitExpires(api, logEntry),
+	}
 
-		if expiryUnix > 0 {
-			pble.DatetimeRateLimitExpires = time.Unix(expiryUnix, 0).Format("2006-01-02 15:04:05")
-		} else {
-			pble.DatetimeRateLimitExpires = ""
-		}
+	if !pble.ExecutionFinished {
+		pble.CanKill = acl.IsAllowedKill(api.cfg, authenticatedUser, logEntry.Binding.Action)
 	}
 
 	return pble

+ 10 - 10
service/internal/config/config.go

@@ -34,16 +34,16 @@ type Action struct {
 
 // ActionArgument objects appear on Actions.
 type ActionArgument struct {
-	Name                 string                 `koanf:"name"`
-	Title                string                 `koanf:"title"`
-	Description          string                 `koanf:"description"`
-	Type                 string                 `koanf:"type"`
-	Default              string                 `koanf:"default"`
-	Choices              []ActionArgumentChoice `koanf:"choices"`
-	Entity               string                 `koanf:"entity"`
-	RejectNull           bool                   `koanf:"rejectNull"`
-	Suggestions          map[string]string      `koanf:"suggestions"`
-	SuggestionsBrowserKey string                `koanf:"suggestionsBrowserKey"`
+	Name                  string                 `koanf:"name"`
+	Title                 string                 `koanf:"title"`
+	Description           string                 `koanf:"description"`
+	Type                  string                 `koanf:"type"`
+	Default               string                 `koanf:"default"`
+	Choices               []ActionArgumentChoice `koanf:"choices"`
+	Entity                string                 `koanf:"entity"`
+	RejectNull            bool                   `koanf:"rejectNull"`
+	Suggestions           map[string]string      `koanf:"suggestions"`
+	SuggestionsBrowserKey string                 `koanf:"suggestionsBrowserKey"`
 }
 
 // ActionArgumentChoice represents a predefined choice for an argument.

+ 95 - 39
service/internal/executor/executor.go

@@ -309,59 +309,115 @@ func (e *Executor) GetLogsByBindingId(bindingId string) []*InternalLogEntry {
 	return logs
 }
 
-// GetTimeUntilAvailable calculates when an action will be available again based on rate limits.
-// Returns the Unix timestamp in seconds when the rate limit expires, or 0 if the action is available now.
-func (e *Executor) GetTimeUntilAvailable(binding *ActionBinding) int64 {
-	if len(binding.Action.MaxRate) == 0 {
-		return 0
+// shouldCountExecution checks if a log entry should be counted for rate limiting.
+func shouldCountExecution(logEntry *InternalLogEntry, windowStart time.Time) bool {
+	return !logEntry.Blocked && logEntry.DatetimeStarted.After(windowStart)
+}
+
+// updateOldestExecution updates the oldest execution time if this entry is older.
+func updateOldestExecution(oldestExecutionTime **time.Time, logEntry *InternalLogEntry) {
+	if *oldestExecutionTime == nil {
+		*oldestExecutionTime = &logEntry.DatetimeStarted
+	} else if logEntry.DatetimeStarted.Before(**oldestExecutionTime) {
+		*oldestExecutionTime = &logEntry.DatetimeStarted
+	}
+}
+
+// findOldestExecutionInWindow finds the oldest execution within the time window and counts executions.
+// Returns the count of executions and the oldest execution time, or nil if none found.
+func findOldestExecutionInWindow(logs []*InternalLogEntry, windowStart time.Time) (int, *time.Time) {
+	executions := 0
+	var oldestExecutionTime *time.Time
+
+	for _, logEntry := range logs {
+		if !shouldCountExecution(logEntry, windowStart) {
+			continue
+		}
+
+		executions++
+		updateOldestExecution(&oldestExecutionTime, logEntry)
+	}
+
+	return executions, oldestExecutionTime
+}
+
+// calculateExpiryTime calculates when the oldest execution will fall outside the rate limit window.
+func calculateExpiryTime(oldestExecutionTime time.Time, duration time.Duration, now time.Time) time.Time {
+	expiryTime := oldestExecutionTime.Add(duration)
+	if !expiryTime.After(now) {
+		return time.Time{}
+	}
+	return expiryTime
+}
+
+// updateMaxExpiryTime updates maxExpiryTime if expiryTime is later.
+func updateMaxExpiryTime(maxExpiryTime *time.Time, expiryTime time.Time) {
+	if expiryTime.IsZero() {
+		return
+	}
+
+	if maxExpiryTime.IsZero() || expiryTime.After(*maxExpiryTime) {
+		*maxExpiryTime = expiryTime
+	}
+}
+
+// calculateExpiryForRate calculates the expiry time for a single rate limit rule.
+// Returns the expiry time if the rate limit is exceeded, or zero time if not.
+func calculateExpiryForRate(rate config.RateSpec, logs []*InternalLogEntry, now time.Time) time.Time {
+	duration := parseDuration(rate)
+	if duration <= 0 {
+		return time.Time{}
+	}
+
+	windowStart := now.Add(-duration)
+	executions, oldestExecutionTime := findOldestExecutionInWindow(logs, windowStart)
+
+	if executions < rate.Limit || oldestExecutionTime == nil {
+		return time.Time{}
 	}
 
+	return calculateExpiryTime(*oldestExecutionTime, duration, now)
+}
+
+// getLogsForBinding retrieves logs for a binding ID.
+func (e *Executor) getLogsForBinding(bindingId string) []*InternalLogEntry {
 	e.logmutex.RLock()
-	defer e.logmutex.RUnlock()
+	logs, found := e.LogsByBindingId[bindingId]
+	e.logmutex.RUnlock()
 
-	logs, found := e.LogsByBindingId[binding.ID]
 	if !found || len(logs) == 0 {
-		return 0
+		return nil
 	}
 
-	now := time.Now()
-	var maxExpiryTime time.Time
+	return logs
+}
 
-	for _, rate := range binding.Action.MaxRate {
-		duration := parseDuration(rate)
-		if duration <= 0 {
-			continue
-		}
+// calculateMaxExpiryTimeFromRates calculates the maximum expiry time across all rate limit rules.
+func calculateMaxExpiryTimeFromRates(rates []config.RateSpec, logs []*InternalLogEntry, now time.Time) time.Time {
+	var maxExpiryTime time.Time
 
-		then := now.Add(-duration)
-		executions := 0
-		var oldestExecutionTime *time.Time
+	for _, rate := range rates {
+		expiryTime := calculateExpiryForRate(rate, logs, now)
+		updateMaxExpiryTime(&maxExpiryTime, expiryTime)
+	}
 
-		for _, logEntry := range logs {
-			if logEntry.Blocked {
-				continue
-			}
+	return maxExpiryTime
+}
 
-			if logEntry.DatetimeStarted.After(then) {
-				executions++
-				if oldestExecutionTime == nil || logEntry.DatetimeStarted.Before(*oldestExecutionTime) {
-					oldestExecutionTime = &logEntry.DatetimeStarted
-				}
-			}
-		}
+// GetTimeUntilAvailable calculates when an action will be available again based on rate limits.
+// Returns the Unix timestamp in seconds when the rate limit expires, or 0 if the action is available now.
+func (e *Executor) GetTimeUntilAvailable(binding *ActionBinding) int64 {
+	if len(binding.Action.MaxRate) == 0 {
+		return 0
+	}
 
-		// If we're at or over the limit, calculate when the oldest execution will fall outside the window
-		// Note: getExecutionsCount uses -1 because it counts the current execution, but we're checking
-		// availability before execution, so we compare directly to rate.Limit
-		if executions >= rate.Limit && oldestExecutionTime != nil {
-			// The oldest execution will fall outside the window at: oldestExecutionTime + duration
-			expiryTime := oldestExecutionTime.Add(duration)
-			if expiryTime.After(now) && (maxExpiryTime.IsZero() || expiryTime.After(maxExpiryTime)) {
-				maxExpiryTime = expiryTime
-			}
-		}
+	logs := e.getLogsForBinding(binding.ID)
+	if logs == nil {
+		return 0
 	}
 
+	maxExpiryTime := calculateMaxExpiryTimeFromRates(binding.Action.MaxRate, logs, time.Now())
+
 	if maxExpiryTime.IsZero() {
 		return 0
 	}