Refactor App.New() to accept client dependency for better testability

[sgaunet]

Problem

The App.New() constructor in internal/app/app.go (lines 36-48) always creates a NewPostgreSQLClient() internally, making it difficult to inject mocks or alternative implementations for testing.

Current implementation:

func New() *App {
    client, err := NewPostgreSQLClient(connectionString)
    if err != nil {
        logger.Log().Error().Err(err).Msg("Failed to create PostgreSQL client")
        return nil
    }
    
    return &App{
        client: client,
    }
}

Issues

1. Hard to test: Can't inject mock clients without modifying production code
2. Tight coupling: App is tightly coupled to PostgreSQLClientImpl
3. No flexibility: Can't swap implementations (e.g., read-only client, different database)
4. Hidden dependencies: Connection string comes from environment, not visible in API
5. Error handling: Returns nil on error instead of error value

Current Workarounds

Tests must use the PostgreSQLClient interface with mocks, but can't easily test App.New() itself.

Proposed Solution

Option 1: Constructor Dependency Injection (Recommended)

Accept the client as a parameter:

func New(client PostgreSQLClient) *App {
    return &App{
        client: client,
    }
}

// Add convenience constructor for production use
func NewWithConnectionString(connectionString string) (*App, error) {
    client, err := NewPostgreSQLClient(connectionString)
    if err != nil {
        return nil, fmt.Errorf("failed to create PostgreSQL client: %w", err)
    }
    
    return &App{
        client: client,
    }, nil
}

Usage in tests:

func TestApp(t *testing.T) {
    mockClient := new(MockPostgreSQLClient)
    mockClient.On("ListDatabases").Return([]string{"db1", "db2"}, nil)
    
    app := New(mockClient)
    databases, err := app.ListDatabases()
    // ... assertions ...
}

Usage in production:

// In main.go
app, err := app.NewWithConnectionString(connectionString)
if err != nil {
    logger.Log().Fatal().Err(err).Msg("Failed to create app")
}

Option 2: Factory Pattern

type AppFactory struct {
    ClientFactory func(string) (PostgreSQLClient, error)
}

func (f *AppFactory) NewApp(connectionString string) (*App, error) {
    client, err := f.ClientFactory(connectionString)
    if err != nil {
        return nil, err
    }
    return &App{client: client}, nil
}

// Default factory
var DefaultFactory = &AppFactory{
    ClientFactory: func(cs string) (PostgreSQLClient, error) {
        return NewPostgreSQLClient(cs)
    },
}

Recommended Approach

Use Option 1 (Constructor Dependency Injection) as it's:

• Simple and clear
• Follows Go best practices
• Easy to test
• Explicit about dependencies

Impact

• Severity: MEDIUM
• Type: Refactoring, Testing Enhancement
• Location: internal/app/app.go:36-48
• Benefits: Improved testability, looser coupling, better error handling

Migration Guide

Before:

app := app.New()

After:

// Production code
app, err := app.NewWithConnectionString(connectionString)
if err != nil {
    // handle error
}

// Test code
mockClient := new(MockPostgreSQLClient)
app := app.New(mockClient)

Checklist

• Update App.New() to accept PostgreSQLClient interface
• Add NewWithConnectionString() convenience constructor
• Return error instead of nil on failure
• Update main.go to use new constructor
• Update all tests to use new constructor pattern
• Add unit tests for App without database dependencies
• Update CLAUDE.md documentation
• Consider adding NewFromEnv() constructor for environment-based setup