diff --git a/debian/changelog b/debian/changelog index 5479098..6a2faca 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +govpp-snmp-agentx (1.1.7-1) bookworm; urgency=medium + + * Refactor VPPClient constructor to use idiomatic Go patterns + * Remove redundant NewVPPClient() constructor function + * Update all code to use direct struct initialization (&VPPClient{}) + * Improve code maintainability and follow Go best practices + + -- Pim van Pelt Fri, 15 Nov 2024 00:00:00 +0000 + govpp-snmp-agentx (1.1.6-1) bookworm; urgency=medium * Replace forked go-agentx dependency with upstream github.com/posteo/go-agentx diff --git a/src/agentx/agentx.go b/src/agentx/agentx.go index 4908dc2..99de50e 100644 --- a/src/agentx/agentx.go +++ b/src/agentx/agentx.go @@ -45,4 +45,4 @@ func StartAgentXRoutine(interfaceMIB *ifmib.InterfaceMIB) error { logger.Printf("Successfully registered with AgentX at %s://%s", network, address) return nil -} \ No newline at end of file +} diff --git a/src/agentx/agentx_test.go b/src/agentx/agentx_test.go index 34eb4dc..587edc1 100644 --- a/src/agentx/agentx_test.go +++ b/src/agentx/agentx_test.go @@ -22,7 +22,7 @@ func TestAgentXAddrFlagParsing(t *testing.T) { // Test Unix socket path testAddr := "/var/run/test.sock" *AgentXAddr = testAddr - + if *AgentXAddr != testAddr { t.Errorf("Expected AgentX address to be '%s', got '%s'", testAddr, *AgentXAddr) } @@ -30,7 +30,7 @@ func TestAgentXAddrFlagParsing(t *testing.T) { // Test TCP address testAddr = "192.168.1.1:705" *AgentXAddr = testAddr - + if *AgentXAddr != testAddr { t.Errorf("Expected AgentX address to be '%s', got '%s'", testAddr, *AgentXAddr) } @@ -43,12 +43,12 @@ func TestFlagRegistration(t *testing.T) { t.Error("Expected agentx.addr flag to be registered") return } - + if f.DefValue != "localhost:705" { t.Errorf("Expected flag default value to be 'localhost:705', got '%s'", f.DefValue) } - + if f.Usage != "Address to connect to (hostname:port or Unix socket path)" { t.Errorf("Unexpected flag usage string: %s", f.Usage) } -} \ No newline at end of file +} diff --git a/src/config/config.go b/src/config/config.go index f389d12..0d23a25 100644 --- a/src/config/config.go +++ b/src/config/config.go @@ -5,4 +5,4 @@ package config // Global configuration variables var ( Debug bool -) \ No newline at end of file +) diff --git a/src/config/config_test.go b/src/config/config_test.go index 2d62ada..b2095a9 100644 --- a/src/config/config_test.go +++ b/src/config/config_test.go @@ -27,4 +27,4 @@ func TestDebugFlagSet(t *testing.T) { if Debug != false { t.Errorf("Expected Debug to be false after setting, got %v", Debug) } -} \ No newline at end of file +} diff --git a/src/logger/logger.go b/src/logger/logger.go index f348ab9..78f0e6d 100644 --- a/src/logger/logger.go +++ b/src/logger/logger.go @@ -6,7 +6,7 @@ import ( "fmt" "path/filepath" "runtime" - + "govpp-snmp-agentx/config" ) @@ -16,15 +16,15 @@ func getCallerInfo() string { if !ok { return "unknown:unknown" } - + fn := runtime.FuncForPC(pc) if fn == nil { return "unknown:unknown" } - + funcName := filepath.Base(fn.Name()) fileName := filepath.Base(file) - + return fmt.Sprintf("%s:%s", fileName, funcName) } @@ -49,4 +49,4 @@ func Debugf(format string, args ...interface{}) { // Sync flushes any buffered log entries (no-op for fmt.Println) func Sync() { // No buffering with fmt.Println, so this is a no-op -} \ No newline at end of file +} diff --git a/src/logger/logger_test.go b/src/logger/logger_test.go index 90dad94..9df361c 100644 --- a/src/logger/logger_test.go +++ b/src/logger/logger_test.go @@ -33,11 +33,11 @@ func TestPrintf(t *testing.T) { if !strings.HasPrefix(output, "INFO ") { t.Errorf("Expected output to start with 'INFO ', got: %s", output) } - + if !strings.Contains(output, "logger_test.go:logger.TestPrintf") { t.Errorf("Expected output to contain caller info, got: %s", output) } - + if !strings.Contains(output, "test message: hello") { t.Errorf("Expected output to contain message, got: %s", output) } @@ -71,7 +71,7 @@ func TestDebugfWithDebugEnabled(t *testing.T) { if !strings.HasPrefix(output, "DEBUG ") { t.Errorf("Expected output to start with 'DEBUG ', got: %s", output) } - + if !strings.Contains(output, "debug message: test") { t.Errorf("Expected output to contain message, got: %s", output) } @@ -110,4 +110,4 @@ func TestDebugfWithDebugDisabled(t *testing.T) { func TestSync(t *testing.T) { // Test that Sync doesn't panic (it's a no-op now) Sync() -} \ No newline at end of file +} diff --git a/src/main.go b/src/main.go index 8212961..224b224 100644 --- a/src/main.go +++ b/src/main.go @@ -16,7 +16,7 @@ import ( "govpp-snmp-agentx/vpp" ) -const Version = "1.1.6-1" +const Version = "1.1.7-1" func main() { debug := flag.Bool("debug", false, "Enable debug logging") @@ -46,7 +46,7 @@ func main() { } // Create VPP client and managers - vppClient := vpp.NewVPPClient() + vppClient := &vpp.VPPClient{} interfaceManager := vpp.NewInterfaceManager(vppClient) statsManager := vpp.NewStatsManager(vppClient) diff --git a/src/vpp/vpp.go b/src/vpp/vpp.go index cc9b1af..c2c24ac 100644 --- a/src/vpp/vpp.go +++ b/src/vpp/vpp.go @@ -29,11 +29,6 @@ type VPPClient struct { connected bool } -// NewVPPClient creates a new VPP client instance -func NewVPPClient() *VPPClient { - return &VPPClient{} -} - // Connect establishes connections to both VPP API and Stats sockets func (c *VPPClient) Connect() error { logger.Debugf("Connecting to VPP (API: %s, Stats: %s)", *ApiAddr, *StatsAddr) diff --git a/src/vpp/vpp_iface_test.go b/src/vpp/vpp_iface_test.go index 391972c..18a0e09 100644 --- a/src/vpp/vpp_iface_test.go +++ b/src/vpp/vpp_iface_test.go @@ -10,7 +10,7 @@ import ( ) func TestNewInterfaceManager(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewInterfaceManager(client) if manager == nil { @@ -36,7 +36,7 @@ func TestNewInterfaceManager(t *testing.T) { } func TestInterfaceManagerSetEventCallback(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewInterfaceManager(client) var callbackCalled bool @@ -82,7 +82,7 @@ func TestInterfaceManagerSetEventCallback(t *testing.T) { } func TestInterfaceManagerGetAllInterfaceDetailsWithoutConnection(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewInterfaceManager(client) _, err := manager.GetAllInterfaceDetails() @@ -101,7 +101,7 @@ func TestInterfaceManagerGetAllInterfaceDetailsWithoutConnection(t *testing.T) { } func TestInterfaceManagerStartEventWatcherWithoutConnection(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewInterfaceManager(client) err := manager.StartEventWatcher() @@ -120,7 +120,7 @@ func TestInterfaceManagerStartEventWatcherWithoutConnection(t *testing.T) { } func TestInterfaceManagerHandleInterfaceEventWithoutCallback(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewInterfaceManager(client) // Should not panic when callback is nil @@ -128,7 +128,7 @@ func TestInterfaceManagerHandleInterfaceEventWithoutCallback(t *testing.T) { } func TestInterfaceManagerInitializeEventWatchingWithoutConnection(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewInterfaceManager(client) err := manager.InitializeEventWatching() @@ -220,7 +220,7 @@ func TestInterfaceEventCallback(t *testing.T) { } func TestInterfaceManagerStartStopEventMonitoring(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewInterfaceManager(client) if manager.running { @@ -248,7 +248,7 @@ func TestInterfaceManagerStartStopEventMonitoring(t *testing.T) { } func TestInterfaceManagerEventMonitoringWithConnectionChanges(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewInterfaceManager(client) // Set a callback to track calls diff --git a/src/vpp/vpp_stats_test.go b/src/vpp/vpp_stats_test.go index 0cea8e3..ea6e90a 100644 --- a/src/vpp/vpp_stats_test.go +++ b/src/vpp/vpp_stats_test.go @@ -10,7 +10,7 @@ import ( ) func TestNewStatsManager(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewStatsManager(client) if manager == nil { @@ -36,7 +36,7 @@ func TestNewStatsManager(t *testing.T) { } func TestStatsManagerSetStatsCallback(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewStatsManager(client) var callbackCalled bool @@ -85,7 +85,7 @@ func TestStatsManagerSetStatsCallback(t *testing.T) { } func TestStatsManagerSetPeriod(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewStatsManager(client) newPeriod := 5 * time.Second @@ -97,7 +97,7 @@ func TestStatsManagerSetPeriod(t *testing.T) { } func TestStatsManagerStartStopStatsRoutine(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewStatsManager(client) if manager.running { @@ -125,7 +125,7 @@ func TestStatsManagerStartStopStatsRoutine(t *testing.T) { } func TestStatsManagerGetInterfaceStatsWithoutConnection(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewStatsManager(client) _, err := manager.GetInterfaceStats() @@ -206,7 +206,7 @@ func TestStatsCallback(t *testing.T) { } func TestStatsManagerQueryAndReportStatsWithoutConnection(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewStatsManager(client) // Should return false when not connected @@ -216,7 +216,7 @@ func TestStatsManagerQueryAndReportStatsWithoutConnection(t *testing.T) { } func TestStatsManagerWithShortPeriod(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} manager := NewStatsManager(client) // Set a very short period for testing diff --git a/src/vpp/vpp_test.go b/src/vpp/vpp_test.go index 96ff40e..30b26d0 100644 --- a/src/vpp/vpp_test.go +++ b/src/vpp/vpp_test.go @@ -7,7 +7,7 @@ import ( ) func TestNewVPPClient(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} if client == nil { t.Fatal("NewVPPClient() returned nil") @@ -27,7 +27,7 @@ func TestNewVPPClient(t *testing.T) { } func TestVPPClientDisconnect(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} // Should be safe to call disconnect on unconnected client client.Disconnect() @@ -38,7 +38,7 @@ func TestVPPClientDisconnect(t *testing.T) { } func TestVPPClientNewAPIChannelWithoutConnection(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} _, err := client.NewAPIChannel() if err == nil { @@ -56,7 +56,7 @@ func TestVPPClientNewAPIChannelWithoutConnection(t *testing.T) { } func TestVPPClientCheckLivenessWithoutConnection(t *testing.T) { - client := NewVPPClient() + client := &VPPClient{} if client.CheckLiveness() { t.Error("CheckLiveness() should return false when not connected") @@ -86,7 +86,7 @@ func TestVPPClientConnectWithInvalidPaths(t *testing.T) { *StatsAddr = origStatsAddr }() - client := NewVPPClient() + client := &VPPClient{} err := client.Connect() if err == nil {