Extract DomainBuckets to eliminate duplicate accessor methods

pkg/cli/access_log.go

@@ -28,31 +28,10 @@ type AccessLogEntry struct {
 
 // DomainAnalysis represents analysis of domains from access logs
 type DomainAnalysis struct {
-	AllowedDomains []string
-	DeniedDomains  []string
-	TotalRequests  int
-	AllowedCount   int
-	DeniedCount    int
-}
-
-// GetAllowedDomains returns the list of allowed domains
-func (d *DomainAnalysis) GetAllowedDomains() []string {
-	return d.AllowedDomains
-}
-
-// GetDeniedDomains returns the list of denied domains
-func (d *DomainAnalysis) GetDeniedDomains() []string {
-	return d.DeniedDomains
-}
-
-// SetAllowedDomains sets the list of allowed domains
-func (d *DomainAnalysis) SetAllowedDomains(domains []string) {
-	d.AllowedDomains = domains
-}
-
-// SetDeniedDomains sets the list of denied domains
-func (d *DomainAnalysis) SetDeniedDomains(domains []string) {
-	d.DeniedDomains = domains
+	DomainBuckets
+	TotalRequests int
+	AllowedCount  int
+	DeniedCount   int
 }
 
 // AddMetrics adds metrics from another analysis
@@ -73,8 +52,10 @@ func parseSquidAccessLog(logPath string, verbose bool) (*DomainAnalysis, error)
 	defer file.Close()
 
 	analysis := &DomainAnalysis{
-		AllowedDomains: []string{},
-		DeniedDomains:  []string{},
+		DomainBuckets: DomainBuckets{
+			AllowedDomains: []string{},
+			DeniedDomains:  []string{},
+		},
 	}
 
 	allowedDomainsSet := make(map[string]bool)
@@ -211,8 +192,10 @@ func analyzeMultipleAccessLogs(accessLogsDir string, verbose bool) (*DomainAnaly
 		parseSquidAccessLog,
 		func() *DomainAnalysis {
 			return &DomainAnalysis{
-				AllowedDomains: []string{},
-				DeniedDomains:  []string{},
+				DomainBuckets: DomainBuckets{
+					AllowedDomains: []string{},
+					DeniedDomains:  []string{},
+				},
 			}
 		},
 	)

pkg/cli/audit_test.go

@@ -1045,11 +1045,13 @@ func TestBuildAuditDataWithFirewall(t *testing.T) {
 	}
 
 	firewallAnalysis := &FirewallAnalysis{
+		DomainBuckets: DomainBuckets{
+			AllowedDomains: []string{"api.github.com:443", "npmjs.org:443"},
+			DeniedDomains:  []string{"blocked.example.com:443"},
+		},
 		TotalRequests:   10,
 		AllowedRequests: 7,
 		DeniedRequests:  3,
-		AllowedDomains:  []string{"api.github.com:443", "npmjs.org:443"},
-		DeniedDomains:   []string{"blocked.example.com:443"},
 		RequestsByDomain: map[string]DomainRequestStats{
 			"api.github.com:443":      {Allowed: 5, Denied: 0},
 			"npmjs.org:443":           {Allowed: 2, Denied: 0},
@@ -1116,11 +1118,13 @@ func TestGenerateAuditReportWithFirewall(t *testing.T) {
 	}
 
 	firewallAnalysis := &FirewallAnalysis{
+		DomainBuckets: DomainBuckets{
+			AllowedDomains: []string{"api.github.com:443", "npmjs.org:443"},
+			DeniedDomains:  []string{"blocked.example.com:443"},
+		},
 		TotalRequests:   10,
 		AllowedRequests: 7,
 		DeniedRequests:  3,
-		AllowedDomains:  []string{"api.github.com:443", "npmjs.org:443"},
-		DeniedDomains:   []string{"blocked.example.com:443"},
 		RequestsByDomain: map[string]DomainRequestStats{
 			"api.github.com:443":      {Allowed: 5, Denied: 0},
 			"npmjs.org:443":           {Allowed: 2, Denied: 0},
@@ -1174,11 +1178,13 @@ func TestGenerateAuditReportWithFirewall(t *testing.T) {
 func TestRenderJSONWithFirewall(t *testing.T) {
 	// Create test audit data with firewall analysis
 	firewallAnalysis := &FirewallAnalysis{
+		DomainBuckets: DomainBuckets{
+			AllowedDomains: []string{"api.github.com:443"},
+			DeniedDomains:  []string{"blocked.example.com:443"},
+		},
 		TotalRequests:   10,
 		AllowedRequests: 7,
 		DeniedRequests:  3,
-		AllowedDomains:  []string{"api.github.com:443"},
-		DeniedDomains:   []string{"blocked.example.com:443"},
 		RequestsByDomain: map[string]DomainRequestStats{
 			"api.github.com:443":      {Allowed: 7, Denied: 0},
 			"blocked.example.com:443": {Allowed: 0, Denied: 3},

pkg/cli/domain_buckets.go

@@ -0,0 +1,29 @@
+package cli
+
+// DomainBuckets holds allowed and denied domain lists with accessor methods.
+// This struct is embedded by DomainAnalysis and FirewallAnalysis to share
+// domain management functionality and eliminate code duplication.
+type DomainBuckets struct {
+	AllowedDomains []string
+	DeniedDomains  []string
+}
+
+// GetAllowedDomains returns the list of allowed domains
+func (d *DomainBuckets) GetAllowedDomains() []string {
+	return d.AllowedDomains
+}
+
+// GetDeniedDomains returns the list of denied domains
+func (d *DomainBuckets) GetDeniedDomains() []string {
+	return d.DeniedDomains
+}
+
+// SetAllowedDomains sets the list of allowed domains
+func (d *DomainBuckets) SetAllowedDomains(domains []string) {
+	d.AllowedDomains = domains
+}
+
+// SetDeniedDomains sets the list of denied domains
+func (d *DomainBuckets) SetDeniedDomains(domains []string) {
+	d.DeniedDomains = domains
+}

pkg/cli/domain_buckets_test.go

@@ -0,0 +1,54 @@
+package cli
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestDomainBucketsAccessors(t *testing.T) {
+	buckets := &DomainBuckets{}
+
+	// Test initial state
+	if buckets.GetAllowedDomains() != nil {
+		t.Errorf("Expected nil allowed domains initially, got %v", buckets.GetAllowedDomains())
+	}
+	if buckets.GetDeniedDomains() != nil {
+		t.Errorf("Expected nil denied domains initially, got %v", buckets.GetDeniedDomains())
+	}
+
+	// Test SetAllowedDomains
+	allowedDomains := []string{"example.com", "github.com"}
+	buckets.SetAllowedDomains(allowedDomains)
+	if !reflect.DeepEqual(buckets.GetAllowedDomains(), allowedDomains) {
+		t.Errorf("Expected allowed domains %v, got %v", allowedDomains, buckets.GetAllowedDomains())
+	}
+
+	// Test SetDeniedDomains
+	deniedDomains := []string{"blocked.com", "malicious.com"}
+	buckets.SetDeniedDomains(deniedDomains)
+	if !reflect.DeepEqual(buckets.GetDeniedDomains(), deniedDomains) {
+		t.Errorf("Expected denied domains %v, got %v", deniedDomains, buckets.GetDeniedDomains())
+	}
+}
+
+func TestDomainBucketsWithEmbedding(t *testing.T) {
+	// This test verifies that types embedding DomainBuckets can access
+	// the domain accessor methods.
+	type TestAnalysis struct {
+		DomainBuckets
+		TotalRequests int
+	}
+
+	analysis := &TestAnalysis{}
+
+	// Test that we can call accessor methods through embedding
+	analysis.SetAllowedDomains([]string{"test.com"})
+	analysis.SetDeniedDomains([]string{"bad.com"})
+
+	if len(analysis.GetAllowedDomains()) != 1 {
+		t.Errorf("Expected 1 allowed domain, got %d", len(analysis.GetAllowedDomains()))
+	}
+	if len(analysis.GetDeniedDomains()) != 1 {
+		t.Errorf("Expected 1 denied domain, got %d", len(analysis.GetDeniedDomains()))
+	}
+}

pkg/cli/firewall_log.go

@@ -117,34 +117,13 @@ type FirewallLogEntry struct {
 // FirewallAnalysis represents analysis of firewall logs
 // This mirrors the structure from the JavaScript parser
 type FirewallAnalysis struct {
+	DomainBuckets
 	TotalRequests    int
 	AllowedRequests  int
 	DeniedRequests   int
-	AllowedDomains   []string
-	DeniedDomains    []string
 	RequestsByDomain map[string]DomainRequestStats
 }
 
-// GetAllowedDomains returns the list of allowed domains
-func (f *FirewallAnalysis) GetAllowedDomains() []string {
-	return f.AllowedDomains
-}
-
-// GetDeniedDomains returns the list of denied domains
-func (f *FirewallAnalysis) GetDeniedDomains() []string {
-	return f.DeniedDomains
-}
-
-// SetAllowedDomains sets the list of allowed domains
-func (f *FirewallAnalysis) SetAllowedDomains(domains []string) {
-	f.AllowedDomains = domains
-}
-
-// SetDeniedDomains sets the list of denied domains
-func (f *FirewallAnalysis) SetDeniedDomains(domains []string) {
-	f.DeniedDomains = domains
-}
-
 // AddMetrics adds metrics from another analysis
 func (f *FirewallAnalysis) AddMetrics(other LogAnalysis) {
 	if otherFirewall, ok := other.(*FirewallAnalysis); ok {
@@ -287,8 +266,10 @@ func parseFirewallLog(logPath string, verbose bool) (*FirewallAnalysis, error) {
 	defer file.Close()
 
 	analysis := &FirewallAnalysis{
-		AllowedDomains:   []string{},
-		DeniedDomains:    []string{},
+		DomainBuckets: DomainBuckets{
+			AllowedDomains: []string{},
+			DeniedDomains:  []string{},
+		},
 		RequestsByDomain: make(map[string]DomainRequestStats),
 	}
 
@@ -433,8 +414,10 @@ func analyzeMultipleFirewallLogs(logsDir string, verbose bool) (*FirewallAnalysi
 		parseFirewallLog,
 		func() *FirewallAnalysis {
 			return &FirewallAnalysis{
-				AllowedDomains:   []string{},
-				DeniedDomains:    []string{},
+				DomainBuckets: DomainBuckets{
+					AllowedDomains: []string{},
+					DeniedDomains:  []string{},
+				},
 				RequestsByDomain: make(map[string]DomainRequestStats),
 			}
 		},

pkg/cli/firewall_log_integration_test.go

@@ -142,11 +142,13 @@ func TestFirewallLogSummaryBuilding(t *testing.T) {
 				WorkflowName: "workflow-1",
 			},
 			FirewallAnalysis: &FirewallAnalysis{
+				DomainBuckets: DomainBuckets{
+					AllowedDomains: []string{"api.github.com:443", "api.npmjs.org:443"},
+					DeniedDomains:  []string{"blocked.example.com:443"},
+				},
 				TotalRequests:   10,
 				AllowedRequests: 8,
 				DeniedRequests:  2,
-				AllowedDomains:  []string{"api.github.com:443", "api.npmjs.org:443"},
-				DeniedDomains:   []string{"blocked.example.com:443"},
 				RequestsByDomain: map[string]DomainRequestStats{
 					"api.github.com:443":      {Allowed: 5, Denied: 0},
 					"api.npmjs.org:443":       {Allowed: 3, Denied: 0},
@@ -159,11 +161,13 @@ func TestFirewallLogSummaryBuilding(t *testing.T) {
 				WorkflowName: "workflow-2",
 			},
 			FirewallAnalysis: &FirewallAnalysis{
+				DomainBuckets: DomainBuckets{
+					AllowedDomains: []string{"api.github.com:443"},
+					DeniedDomains:  []string{"denied.site:443"},
+				},
 				TotalRequests:   5,
 				AllowedRequests: 3,
 				DeniedRequests:  2,
-				AllowedDomains:  []string{"api.github.com:443"},
-				DeniedDomains:   []string{"denied.site:443"},
 				RequestsByDomain: map[string]DomainRequestStats{
 					"api.github.com:443": {Allowed: 3, Denied: 0},
 					"denied.site:443":    {Allowed: 0, Denied: 2},

pkg/cli/log_aggregation_test.go

@@ -18,8 +18,10 @@ func TestFirewallAnalysisImplementsLogAnalysis(t *testing.T) {
 
 func TestDomainAnalysisGettersSetters(t *testing.T) {
 	analysis := &DomainAnalysis{
-		AllowedDomains: []string{"example.com", "test.com"},
-		DeniedDomains:  []string{"blocked.com"},
+		DomainBuckets: DomainBuckets{
+			AllowedDomains: []string{"example.com", "test.com"},
+			DeniedDomains:  []string{"blocked.com"},
+		},
 	}
 
 	// Test getters
@@ -75,8 +77,10 @@ func TestDomainAnalysisAddMetrics(t *testing.T) {
 
 func TestFirewallAnalysisGettersSetters(t *testing.T) {
 	analysis := &FirewallAnalysis{
-		AllowedDomains:   []string{"api.github.com:443", "api.npmjs.org:443"},
-		DeniedDomains:    []string{"blocked.example.com:443"},
+		DomainBuckets: DomainBuckets{
+			AllowedDomains: []string{"api.github.com:443", "api.npmjs.org:443"},
+			DeniedDomains:  []string{"blocked.example.com:443"},
+		},
 		RequestsByDomain: make(map[string]DomainRequestStats),
 	}
 
@@ -192,8 +196,10 @@ func TestAggregateLogFilesWithAccessLogs(t *testing.T) {
 		parseSquidAccessLog,
 		func() *DomainAnalysis {
 			return &DomainAnalysis{
-				AllowedDomains: []string{},
-				DeniedDomains:  []string{},
+				DomainBuckets: DomainBuckets{
+					AllowedDomains: []string{},
+					DeniedDomains:  []string{},
+				},
 			}
 		},
 	)
@@ -265,8 +271,10 @@ func TestAggregateLogFilesWithFirewallLogs(t *testing.T) {
 		parseFirewallLog,
 		func() *FirewallAnalysis {
 			return &FirewallAnalysis{
-				AllowedDomains:   []string{},
-				DeniedDomains:    []string{},
+				DomainBuckets: DomainBuckets{
+					AllowedDomains: []string{},
+					DeniedDomains:  []string{},
+				},
 				RequestsByDomain: make(map[string]DomainRequestStats),
 			}
 		},
@@ -313,8 +321,10 @@ func TestAggregateLogFilesNoFiles(t *testing.T) {
 		parseSquidAccessLog,
 		func() *DomainAnalysis {
 			return &DomainAnalysis{
-				AllowedDomains: []string{},
-				DeniedDomains:  []string{},
+				DomainBuckets: DomainBuckets{
+					AllowedDomains: []string{},
+					DeniedDomains:  []string{},
+				},
 			}
 		},
 	)
@@ -360,8 +370,10 @@ func TestAggregateLogFilesWithParseErrors(t *testing.T) {
 		parseSquidAccessLog,
 		func() *DomainAnalysis {
 			return &DomainAnalysis{
-				AllowedDomains: []string{},
-				DeniedDomains:  []string{},
+				DomainBuckets: DomainBuckets{
+					AllowedDomains: []string{},
+					DeniedDomains:  []string{},
+				},
 			}
 		},
 	)