[duplicate-code] Duplicate Code Pattern: Overlapping Minimum-Value Validators (TimeoutPositive / PositiveInteger)

[github-actions]

Part of duplicate code analysis: #aw_parent_dup001

Summary

TimeoutPositive (line 39) and PositiveInteger (line 52) in internal/config/validation_rules.go both validate that an integer is >= 1 by delegating to TimeoutMinimum. They share identical structure: call TimeoutMinimum with min=1, then post-process the error. TimeoutPositive only overrides Suggestion; PositiveInteger overrides both Message and Suggestion. Both functions serve the same domain — "value must be at least 1" — but use different semantic framing ("timeout" vs "positive integer").

Duplication Details

Pattern: Redundant minimum-value wrappers over TimeoutMinimum

• Severity: Low
• Occurrences: 2 instances
• Locations:
  • internal/config/validation_rules.go lines 39–46 (TimeoutPositive)
  • internal/config/validation_rules.go lines 52–63 (PositiveInteger)

Code Sample:

// Lines 39-46
func TimeoutPositive(timeout int, fieldName, jsonPath string) *ValidationError {
    if err := TimeoutMinimum(timeout, 1, fieldName, jsonPath); err != nil {
        err.Suggestion = "Use a positive number of seconds (e.g., 30)"
        return err
    }
    return nil
}

// Lines 52-63
func PositiveInteger(value int, fieldName, jsonPath string) *ValidationError {
    logValidation.Printf("Validating positive integer: field=%s, value=%d, jsonPath=%s", fieldName, value, jsonPath)
    if err := TimeoutMinimum(value, 1, fieldName, jsonPath); err != nil {
        logValidation.Printf("Positive integer validation failed: %s=%d is not positive", fieldName, value)
        err.Message = fmt.Sprintf("%s must be a positive integer (>= 1), got %d", fieldName, value)
        err.Suggestion = fmt.Sprintf("Use a positive integer (>= 1) for %s", fieldName)
        return err
    }
    return nil
}

Both functions are thin wrappers over TimeoutMinimum(v, 1, ...). The differences are: (a) PositiveInteger has debug log calls; (b) PositiveInteger also overrides Message while TimeoutPositive only overrides Suggestion; (c) the suggestion wording is domain-specific ("seconds" vs "integer").

Impact Analysis

• Maintainability: Low risk today since TimeoutMinimum centralises the core logic. Risk increases if either wrapper grows (e.g., adding more overrides or log calls).
• Bug Risk: Very low — both wrappers are simple and tested.
• Code Bloat: Minor (~12 lines), but the domain separation ("timeout" vs "non-timeout integer") may justify keeping separate named functions with minimal bodies.

Refactoring Recommendations

1. Have TimeoutPositive delegate to PositiveInteger with a fixed suggestion override (lowest risk, adds consistency):

   func TimeoutPositive(timeout int, fieldName, jsonPath string) *ValidationError {
       if err := PositiveInteger(timeout, fieldName, jsonPath); err != nil {
           err.Suggestion = "Use a positive number of seconds (e.g., 30)"
           return err
       }
       return nil
   }

   This ensures TimeoutPositive also gets the improved Message text from PositiveInteger while keeping the timeout-specific suggestion.

2. Add missing log calls to TimeoutPositive (if delegation is not chosen): add logValidation.Printf calls to TimeoutPositive to match PositiveInteger for observability consistency.

Implementation Checklist

• Review duplication findings
• Decide: delegate TimeoutPositive to PositiveInteger, or add missing log calls
• Implement change in internal/config/validation_rules.go
• Run make test to verify no tests break

Parent Issue

See parent analysis report: #aw_parent_dup001

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Duplicate Code Detector · 126.5 AIC · ⊞ 7.7K · ◷

• expires on Jul 12, 2026, 3:44 AM UTC