Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 16, 2025

Four domain accessor methods (GetAllowedDomains, GetDeniedDomains, SetAllowedDomains, SetDeniedDomains) were duplicated across DomainAnalysis and FirewallAnalysis structs—36 lines of identical code across two files.

Changes

  • New DomainBuckets struct with shared domain fields and accessor methods
  • Embedded DomainBuckets in both DomainAnalysis and FirewallAnalysis
  • Removed duplicate methods from both structs (18 lines each)

Implementation

// Shared domain management
type DomainBuckets struct {
    AllowedDomains []string
    DeniedDomains  []string
}

func (d *DomainBuckets) GetAllowedDomains() []string { return d.AllowedDomains }
// ... other accessors

// Embedding eliminates duplication
type DomainAnalysis struct {
    DomainBuckets  // Methods promoted automatically
    TotalRequests int
    AllowedCount  int
    DeniedCount   int
}

type FirewallAnalysis struct {
    DomainBuckets  // Same methods, zero duplication
    TotalRequests   int
    AllowedRequests int
    DeniedRequests  int
    RequestsByDomain map[string]DomainRequestStats
}

Net: 20 lines of code eliminated, single source of truth for domain accessor logic.

Original prompt

This section details on the original issue you should resolve

<issue_title>[duplicate-code] 🔍 Duplicate Code Detected: Domain Accessor Methods</issue_title>
<issue_description># 🔍 Duplicate Code Detected: Domain Accessor Methods

*Analysis of commit *

Assignee: @copilot

Summary

The domain accessor helpers (GetAllowedDomains, GetDeniedDomains, SetAllowedDomains, SetDeniedDomains) are implemented twice—once on DomainAnalysis in the squid access log parser and again on FirewallAnalysis in the firewall log parser. The method bodies are identical, duplicating 16+ lines of code and increasing maintenance effort.

Duplication Details

Pattern: Duplicated domain accessor methods on analysis structs

  • Severity: Medium
  • Occurrences: 2 structs, 8 methods total
  • Locations:
    • pkg/cli/access_log.go (lines 38-55)
    • pkg/cli/firewall_log.go (lines 128-145)
  • Code Sample:
    // GetAllowedDomains returns the list of allowed domains
    func (a *DomainAnalysis) GetAllowedDomains() []string {
        return a.AllowedDomains
    }
    
    // SetAllowedDomains sets the list of allowed domains
    func (a *DomainAnalysis) SetAllowedDomains(domains []string) {
        a.AllowedDomains = domains
    }

Impact Analysis

  • Maintainability: Any change to accessor behavior must be updated in both types, risking drift.
  • Bug Risk: Future logic adjustments (e.g., deduping or sorting) could be missed in one copy.
  • Code Bloat: Repeats four identically structured methods per struct without reuse.

Refactoring Recommendations

  1. Extract shared accessor mixin

    • Extract a small struct (e.g., DomainBuckets) that stores allowed/denied slices plus helper methods, and embed it in both analysis types.
    • Estimated effort: 1-2 hours; mostly mechanical refactor.
    • Benefits: Single implementation, easier to evolve behavior.
  2. Leverage interface default impl

    • Introduce helper functions that operate on any type implementing getter/setter contract, reducing per-struct boilerplate.
    • Benefits: Keeps types lean without duplication.

Implementation Checklist

  • Review duplication findings
  • Prioritize refactoring tasks
  • Create refactoring plan
  • Implement changes
  • Update tests
  • Verify no functionality broken

Analysis Metadata

  • Analyzed Files: 2
  • Detection Method: Serena semantic code analysis
  • Commit:
  • Analysis Date: 2025-11-16

AI generated by Duplicate Code Detector</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…r methods

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove duplicate domain accessor methods from analysis structs Extract DomainBuckets to eliminate duplicate accessor methods Nov 16, 2025
Copilot finished work on behalf of pelikhan November 16, 2025 21:32
Copilot AI requested a review from pelikhan November 16, 2025 21:32
@pelikhan pelikhan marked this pull request as ready for review November 16, 2025 21:38
Copilot AI review requested due to automatic review settings November 16, 2025 21:38
Copilot finished reviewing on behalf of pelikhan November 16, 2025 21:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR successfully refactors duplicate domain accessor methods by extracting a shared DomainBuckets struct. The refactoring eliminates 36 lines of duplicate code across DomainAnalysis and FirewallAnalysis structs, creating a single source of truth for domain management.

Key changes:

  • Introduced new DomainBuckets struct with AllowedDomains/DeniedDomains fields and four accessor methods
  • Embedded DomainBuckets in both DomainAnalysis and FirewallAnalysis to automatically promote the accessor methods
  • Updated all initialization sites and tests to use the nested DomainBuckets structure

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/cli/domain_buckets.go New file defining the shared DomainBuckets struct with accessor methods
pkg/cli/domain_buckets_test.go Test coverage for the new DomainBuckets struct and embedding behavior
pkg/cli/access_log.go Refactored DomainAnalysis to embed DomainBuckets, removing duplicate accessor methods
pkg/cli/firewall_log.go Refactored FirewallAnalysis to embed DomainBuckets, removing duplicate accessor methods
pkg/cli/logs_summary_test.go Updated test initialization to use nested DomainBuckets structure
pkg/cli/logs_report_test.go Updated multiple test cases to use nested DomainBuckets structure
pkg/cli/log_aggregation_test.go Updated test initialization for both analysis types
pkg/cli/firewall_log_integration_test.go Updated integration tests to use nested DomainBuckets structure
pkg/cli/audit_test.go Updated audit tests to use nested DomainBuckets structure

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pelikhan pelikhan merged commit 2d7a5a7 into main Nov 16, 2025
127 of 133 checks passed
@pelikhan pelikhan deleted the copilot/remove-duplicate-domain-accessors branch November 16, 2025 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] 🔍 Duplicate Code Detected: Domain Accessor Methods

2 participants