Refactor architecture: extract reconciler, add interfaces, reduce complexity, improve test coverage

Architecture:
- Extract reconcileNetworking into internal/reconcile package with 7 consumer-side interfaces
- Add locked modifyState helper to fix state.yaml read-modify-write race condition
- Extract CrowdSec and VPN handler groups with interfaces documenting dependency surface
- Replace raw map[string]any YAML manipulation with typed AddDHCPStaticLease/RemoveDHCPStaticLease
- Extract Cloudflare API functions into cfClient struct, eliminating repeated auth boilerplate

Complexity reduction:
- haproxy.GenerateWithOpts: 50 → 6 (extracted 8 focused helpers)
- reconcile.Reconcile: 42 → 11 (extracted buildRoutes, buildDNSEntries, writeHAProxyConfig)
- AutheliaUpdateConfig: 25 → 10 (extracted enableAuthelia/disableAuthelia)

Test coverage improvements:
- reconcile: 4.5% → 56.8% (stub-based tests for route building, DNS entries, orchestration)
- dnsfilter: 10.7% → 45.6% (Manager.Compile, AddList, ToggleList, custom entries)
- config: 67.3% → 89.8% (DHCP static lease mutation tests)
This commit is contained in:
2026-07-14 04:21:30 +00:00
parent 1e7d93256e
commit 428d47f876
35 changed files with 1856 additions and 1194 deletions

View File

@@ -44,11 +44,11 @@ type FilterData struct {
// FilterStats holds current filtering statistics.
type FilterStats struct {
Enabled bool `json:"enabled"`
TotalBlocked int `json:"totalBlocked"`
ListCount int `json:"listCount"`
EnabledLists int `json:"enabledLists"`
LastUpdated time.Time `json:"lastUpdated"`
Enabled bool `json:"enabled"`
TotalBlocked int `json:"totalBlocked"`
ListCount int `json:"listCount"`
EnabledLists int `json:"enabledLists"`
LastUpdated time.Time `json:"lastUpdated"`
QueryStats *QueryStats `json:"queryStats,omitempty"`
}

View File

@@ -0,0 +1,304 @@
package dnsfilter
import (
"os"
"path/filepath"
"strings"
"testing"
)
func setupTestManager(t *testing.T) *Manager {
t.Helper()
tmp := t.TempDir()
m := NewManager(tmp)
m.SetHostsFilePath(filepath.Join(tmp, "blocked.conf"))
if err := m.EnsureDataDir(); err != nil {
t.Fatal(err)
}
return m
}
func TestFilterData_Roundtrip(t *testing.T) {
m := setupTestManager(t)
fd := &FilterData{
Lists: []Blocklist{
{ID: "abc123", Name: "Test List", Enabled: true, EntryCount: 42},
},
CustomEntries: []CustomEntry{
{Domain: "example.com", Type: "allow"},
{Domain: "ads.example.com", Type: "block"},
},
}
if err := m.SaveFilterData(fd); err != nil {
t.Fatalf("save: %v", err)
}
loaded, err := m.LoadFilterData()
if err != nil {
t.Fatalf("load: %v", err)
}
if len(loaded.Lists) != 1 || loaded.Lists[0].ID != "abc123" {
t.Errorf("lists roundtrip failed: %+v", loaded.Lists)
}
if len(loaded.CustomEntries) != 2 {
t.Errorf("custom entries roundtrip failed: %+v", loaded.CustomEntries)
}
}
func TestLoadFilterData_EmptyOnMissing(t *testing.T) {
m := setupTestManager(t)
fd, err := m.LoadFilterData()
if err != nil {
t.Fatal(err)
}
if fd == nil || len(fd.Lists) != 0 {
t.Error("expected empty filter data for missing file")
}
}
func TestAddList_DeduplicatesURL(t *testing.T) {
m := setupTestManager(t)
bl := Blocklist{URL: "https://example.com/list.txt", Name: "Test"}
if err := m.AddList(bl); err != nil {
t.Fatal(err)
}
err := m.AddList(bl)
if err == nil {
t.Error("expected error on duplicate URL")
}
}
func TestAddList_RequiresURLOrID(t *testing.T) {
m := setupTestManager(t)
err := m.AddList(Blocklist{Name: "No URL"})
if err == nil {
t.Error("expected error for list without URL or ID")
}
}
func TestToggleList(t *testing.T) {
m := setupTestManager(t)
bl := Blocklist{URL: "https://example.com/list.txt", Name: "Test"}
if err := m.AddList(bl); err != nil {
t.Fatal(err)
}
fd, _ := m.LoadFilterData()
id := fd.Lists[0].ID
if !fd.Lists[0].Enabled {
t.Error("expected list to be enabled by default")
}
if err := m.ToggleList(id, false); err != nil {
t.Fatal(err)
}
fd, _ = m.LoadFilterData()
if fd.Lists[0].Enabled {
t.Error("expected list to be disabled after toggle")
}
}
func TestToggleList_NotFound(t *testing.T) {
m := setupTestManager(t)
err := m.ToggleList("nonexistent", true)
if err == nil {
t.Error("expected error for nonexistent list")
}
}
func TestRemoveList(t *testing.T) {
m := setupTestManager(t)
bl := Blocklist{URL: "https://example.com/list.txt", Name: "Test"}
if err := m.AddList(bl); err != nil {
t.Fatal(err)
}
fd, _ := m.LoadFilterData()
id := fd.Lists[0].ID
if err := m.RemoveList(id); err != nil {
t.Fatal(err)
}
fd, _ = m.LoadFilterData()
if len(fd.Lists) != 0 {
t.Error("expected list to be removed")
}
}
func TestSetCustomEntry(t *testing.T) {
m := setupTestManager(t)
if err := m.SetCustomEntry(CustomEntry{Domain: "ads.example.com", Type: "block"}); err != nil {
t.Fatal(err)
}
fd, _ := m.LoadFilterData()
if len(fd.CustomEntries) != 1 || fd.CustomEntries[0].Type != "block" {
t.Errorf("custom entry not saved: %+v", fd.CustomEntries)
}
// Update same domain to allow
if err := m.SetCustomEntry(CustomEntry{Domain: "ads.example.com", Type: "allow"}); err != nil {
t.Fatal(err)
}
fd, _ = m.LoadFilterData()
if len(fd.CustomEntries) != 1 || fd.CustomEntries[0].Type != "allow" {
t.Errorf("custom entry not updated: %+v", fd.CustomEntries)
}
}
func TestSetCustomEntry_InvalidType(t *testing.T) {
m := setupTestManager(t)
err := m.SetCustomEntry(CustomEntry{Domain: "example.com", Type: "invalid"})
if err == nil {
t.Error("expected error for invalid type")
}
}
func TestRemoveCustomEntry(t *testing.T) {
m := setupTestManager(t)
_ = m.SetCustomEntry(CustomEntry{Domain: "ads.example.com", Type: "block"})
if err := m.RemoveCustomEntry("ads.example.com"); err != nil {
t.Fatal(err)
}
fd, _ := m.LoadFilterData()
if len(fd.CustomEntries) != 0 {
t.Error("expected custom entry to be removed")
}
}
func TestCompile_MergesBlocklists(t *testing.T) {
m := setupTestManager(t)
// Create two cached list files
_ = os.WriteFile(m.cacheFile("list1"), []byte("ads.example.com\ntracker.example.com\n"), 0644)
_ = os.WriteFile(m.cacheFile("list2"), []byte("malware.example.com\nads.example.com\n"), 0644)
fd := &FilterData{
Lists: []Blocklist{
{ID: "list1", Name: "List 1", Enabled: true},
{ID: "list2", Name: "List 2", Enabled: true},
},
}
_ = m.SaveFilterData(fd)
count, err := m.Compile()
if err != nil {
t.Fatal(err)
}
// 3 unique domains (ads.example.com deduplicated)
if count != 3 {
t.Errorf("expected 3 domains, got %d", count)
}
content, _ := os.ReadFile(m.HostsFilePath())
output := string(content)
if !strings.Contains(output, "address=/ads.example.com/") {
t.Error("missing ads.example.com in output")
}
if !strings.Contains(output, "address=/tracker.example.com/") {
t.Error("missing tracker.example.com in output")
}
if !strings.Contains(output, "address=/malware.example.com/") {
t.Error("missing malware.example.com in output")
}
}
func TestCompile_DisabledListsSkipped(t *testing.T) {
m := setupTestManager(t)
_ = os.WriteFile(m.cacheFile("enabled"), []byte("good.example.com\n"), 0644)
_ = os.WriteFile(m.cacheFile("disabled"), []byte("should-not-appear.example.com\n"), 0644)
fd := &FilterData{
Lists: []Blocklist{
{ID: "enabled", Name: "Enabled", Enabled: true},
{ID: "disabled", Name: "Disabled", Enabled: false},
},
}
_ = m.SaveFilterData(fd)
count, err := m.Compile()
if err != nil {
t.Fatal(err)
}
if count != 1 {
t.Errorf("expected 1 domain (disabled list skipped), got %d", count)
}
content, _ := os.ReadFile(m.HostsFilePath())
if strings.Contains(string(content), "should-not-appear") {
t.Error("disabled list domains should not appear in output")
}
}
func TestCompile_CustomAllowOverridesBlock(t *testing.T) {
m := setupTestManager(t)
_ = os.WriteFile(m.cacheFile("list1"), []byte("blocked.example.com\nallowed.example.com\n"), 0644)
fd := &FilterData{
Lists: []Blocklist{{ID: "list1", Name: "List 1", Enabled: true}},
CustomEntries: []CustomEntry{{Domain: "allowed.example.com", Type: "allow"}},
}
_ = m.SaveFilterData(fd)
count, err := m.Compile()
if err != nil {
t.Fatal(err)
}
if count != 1 {
t.Errorf("expected 1 domain (allow override), got %d", count)
}
content, _ := os.ReadFile(m.HostsFilePath())
if strings.Contains(string(content), "allowed.example.com") {
t.Error("allowed domain should not be in blocklist")
}
}
func TestCompile_CustomBlockAdds(t *testing.T) {
m := setupTestManager(t)
fd := &FilterData{
CustomEntries: []CustomEntry{{Domain: "custom-blocked.example.com", Type: "block"}},
}
_ = m.SaveFilterData(fd)
count, err := m.Compile()
if err != nil {
t.Fatal(err)
}
if count != 1 {
t.Errorf("expected 1 domain from custom block, got %d", count)
}
content, _ := os.ReadFile(m.HostsFilePath())
if !strings.Contains(string(content), "address=/custom-blocked.example.com/") {
t.Error("custom blocked domain should appear in output")
}
}
func TestCompile_EmptyLists(t *testing.T) {
m := setupTestManager(t)
count, err := m.Compile()
if err != nil {
t.Fatal(err)
}
if count != 0 {
t.Errorf("expected 0 domains for empty lists, got %d", count)
}
}