Files
notifier/docs/REFACTORING_QUICKREF.md
2025-10-26 02:25:24 -07:00

13 KiB

Code Duplication Refactoring - Quick Reference Guide

Status: Ready to Implement Total Time: 2.5 hours Complexity: LOW Risk: MINIMAL


Quick Overview

Priority Issue Files Lines Time Status
🔴 HIGH Auth validation dedup 3 32 30m Ready
🟡 MEDIUM API key extraction 2 15 20m Ready
🟡 MEDIUM Notifier registration 1 30 45m Ready
🟢 LOW Error result helpers 3 15 25m Ready

Phase 1: Quick Wins (1 hour)

Issue #2: Extract Bearer Token Parsing

Time: 20 minutes Files Changed: 2 Lines Removed: 15

Step 1: Add helper to internal/auth/auth.go

// At package level, add after APIKeyStore definition:

// extractBearerToken extracts bearer token from Authorization header
func extractBearerToken(authHeader string) string {
    if authHeader == "" {
        return ""
    }
    parts := strings.SplitN(authHeader, " ", 2)
    if len(parts) == 2 && strings.ToLower(parts[0]) == "bearer" {
        return parts[1]
    }
    return ""
}

Step 2: Update internal/auth/rest_middleware.go

Replace lines 73-89:

// OLD (17 lines):
func (m *RESTAuthMiddleware) extractAPIKey(r *http.Request) string {
    authHeader := r.Header.Get("Authorization")
    if authHeader != "" {
        parts := strings.SplitN(authHeader, " ", 2)
        if len(parts) == 2 && strings.ToLower(parts[0]) == "bearer" {
            return parts[1]
        }
    }

    if apiKey := r.Header.Get("X-API-Key"); apiKey != "" {
        return apiKey
    }

    return ""
}

// NEW (6 lines):
func (m *RESTAuthMiddleware) extractAPIKey(r *http.Request) string {
    if token := extractBearerToken(r.Header.Get("Authorization")); token != "" {
        return token
    }
    return r.Header.Get("X-API-Key")
}

Step 3: Update internal/auth/grpc_middleware.go

Replace lines 129-150:

// OLD (22 lines):
func (m *GRPCAuthMiddleware) extractAPIKey(ctx context.Context) string {
    md, ok := metadata.FromIncomingContext(ctx)
    if !ok {
        return ""
    }

    if authHeaders := md.Get("authorization"); len(authHeaders) > 0 {
        authHeader := authHeaders[0]
        parts := strings.SplitN(authHeader, " ", 2)
        if len(parts) == 2 && strings.ToLower(parts[0]) == "bearer" {
            return parts[1]
        }
    }

    if keyHeaders := md.Get("x-api-key"); len(keyHeaders) > 0 {
        return keyHeaders[0]
    }

    return ""
}

// NEW (11 lines):
func (m *GRPCAuthMiddleware) extractAPIKey(ctx context.Context) string {
    md, ok := metadata.FromIncomingContext(ctx)
    if !ok {
        return ""
    }

    if authHeaders := md.Get("authorization"); len(authHeaders) > 0 {
        if token := extractBearerToken(authHeaders[0]); token != "" {
            return token
        }
    }

    return getFirstIfPresent(md.Get("x-api-key"))
}

// Add helper for getting first element
func getFirstIfPresent(vals []string) string {
    if len(vals) > 0 {
        return vals[0]
    }
    return ""
}

Verification:

go test ./internal/auth -v
# Should pass all auth tests

Issue #4: Add Result Helpers to BaseNotifier

Time: 25 minutes Files Changed: 4 (notifier.go + 3 notifiers) Lines Removed: 15

Step 1: Update internal/notifier/notifier.go

Add to BaseNotifier struct (after Type() method):

// ErrorResult returns a failed notification result
func (b *BaseNotifier) ErrorResult(notification *domain.Notification, err error) *domain.NotificationResult {
    return &domain.NotificationResult{
        NotificationID: notification.ID,
        Success:        false,
        Error:          err.Error(),
        SentAt:         time.Now(),
    }
}

// SuccessResult returns a successful notification result
func (b *BaseNotifier) SuccessResult(
    notification *domain.Notification,
    message string,
    recipientCount int,
    providerResponse map[string]interface{},
) *domain.NotificationResult {
    if message == "" {
        message = fmt.Sprintf("Notification sent to %d recipient(s)", recipientCount)
    }
    return &domain.NotificationResult{
        NotificationID: notification.ID,
        Success:        true,
        Message:        message,
        SentAt:         time.Now(),
        ProviderResponse: providerResponse,
    }
}

Step 2: Update each notifier's Send() method

In internal/notifier/slack.go (line ~93):

// OLD:
return &domain.NotificationResult{
    NotificationID: notification.ID,
    Success:        false,
    Error:          err.Error(),
    SentAt:         time.Now(),
}, err

// NEW:
return s.ErrorResult(notification, err), err

In internal/notifier/slack.go (line ~102):

// OLD:
return &domain.NotificationResult{
    NotificationID: notification.ID,
    Success:        true,
    Message:        fmt.Sprintf("Slack notification sent to %d channels", len(notification.Recipients)),
    SentAt:         time.Now(),
    ProviderResponse: map[string]interface{}{
        "channels": notification.Recipients,
    },
}, nil

// NEW:
return s.SuccessResult(
    notification,
    "",  // Will use default message
    len(notification.Recipients),
    map[string]interface{}{"channels": notification.Recipients},
), nil

Do the same for: ntfy.go, smtp.go, stdout.go

Verification:

go test ./internal/notifier -v
# Should pass all notifier tests

Phase 2: Core Refactoring (1.5 hours) 🔧

Issue #1: Extract Auth Validation

Time: 30 minutes Files Changed: 3 Lines Removed: 32

Step 1: Add validation method to auth middleware base

In internal/auth/auth.go, add:

// validateAndAuthorize performs validation and authorization checks
// Returns error if validation fails, or logs but continues on UpdateLastUsed failure
func (m *baseAuthMiddleware) validateAndAuthorize(apiKey string) (*APIKey, error) {
    // Validate API key
    key, err := m.store.ValidateKey(apiKey)
    if err != nil {
        return nil, err
    }

    // Check rate limit
    allowed, err := m.store.CheckRateLimit(apiKey)
    if err != nil || !allowed {
        return nil, ErrRateLimited
    }

    // Update last used (log but don't fail on error)
    if err := m.store.UpdateLastUsed(apiKey); err != nil {
        m.logger.Errorf("Failed to update last used time: %v", err)
    }

    return key, nil
}

// Define base struct for shared auth logic
type baseAuthMiddleware struct {
    store  *APIKeyStore
    logger *logging.Logger
}

Step 2: Update RESTAuthMiddleware in rest_middleware.go

// Change struct to embed base:
type RESTAuthMiddleware struct {
    baseAuthMiddleware
}

// Update Middleware method:
func (m *RESTAuthMiddleware) Middleware(next http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        apiKey := m.extractAPIKey(r)
        if apiKey == "" {
            m.logger.Warnf("REST: Missing API key in request from %s", r.RemoteAddr)
            http.Error(w, "Missing or invalid Authorization header", http.StatusUnauthorized)
            return
        }

        // Use extracted validation method
        key, err := m.validateAndAuthorize(apiKey)
        if err != nil {
            m.logger.Warnf("REST: Validation failed from %s: %v", r.RemoteAddr, err)
            http.Error(w, "Invalid API key or rate limited", http.StatusUnauthorized)
            return
        }

        // Create auth context
        authCtx := &AuthContext{
            APIKey:   key,
            ClientID: key.ClientID,
            Roles:    key.Roles,
        }

        ctx := ContextWithAuth(r.Context(), authCtx)
        m.logger.Debugf("REST: Authenticated client=%s with roles=%v", key.ClientID, key.Roles)

        next.ServeHTTP(w, r.WithContext(ctx))
    })
}

Step 3: Update GRPCAuthMiddleware in grpc_middleware.go

// Change struct:
type GRPCAuthMiddleware struct {
    baseAuthMiddleware
}

// Update UnaryInterceptor:
func (m *GRPCAuthMiddleware) UnaryInterceptor() grpc.UnaryServerInterceptor {
    return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
        apiKey := m.extractAPIKey(ctx)
        if apiKey == "" {
            m.logger.Warnf("gRPC: Missing API key for method=%s", info.FullMethod)
            return nil, status.Error(codes.Unauthenticated, "Missing API key")
        }

        key, err := m.validateAndAuthorize(apiKey)
        if err != nil {
            m.logger.Warnf("gRPC: Validation failed for method=%s: %v", info.FullMethod, err)
            return nil, status.Error(codes.Unauthenticated, "Invalid API key or rate limited")
        }

        authCtx := &AuthContext{
            APIKey:   key,
            ClientID: key.ClientID,
            Roles:    key.Roles,
        }

        newCtx := ContextWithAuth(ctx, authCtx)
        m.logger.Debugf("gRPC: Authenticated client=%s method=%s", key.ClientID, info.FullMethod)

        return handler(newCtx, req)
    }
}

// Update StreamInterceptor similarly

Verification:

go test ./internal/auth -v
# Should pass all auth tests

Issue #3: Extract Notifier Registration Pattern

Time: 45 minutes Files Changed: 1 Lines Removed: 30

Step 1: Add generic registration helper to cmd/server/main.go

// notifierRegistration handles the common pattern for registering notifiers
type notifierRegistration struct {
    notifyType domain.NotificationType
    accountMap map[string]interface{}  // Generic map of configs
    creator    func(string, interface{}) (domain.Notifier, error)
    isDefault  func(interface{}) bool
}

func registerNotifierType(
    reg notifierRegistration,
    factory *notifier.Factory,
    logger *logging.Logger,
) {
    for accountName, config := range reg.accountMap {
        notif, err := reg.creator(accountName, config)
        if err != nil {
            logger.Warnf("Failed to create %s notifier for account '%s': %v",
                reg.notifyType, accountName, err)
            continue
        }

        if err := factory.RegisterNotifier(reg.notifyType, accountName, notif); err != nil {
            logger.Fatalf("Failed to register %s notifier for account '%s': %v",
                reg.notifyType, accountName, err)
        }

        defaultStr := ""
        if reg.isDefault(config) {
            defaultStr = " (default)"
        }
        logger.Infof("Registered %s notifier for account '%s'%s",
            reg.notifyType, accountName, defaultStr)
    }
}

Step 2: Refactor registerNotifiers() function

// OLD: 58 lines with repeated pattern

func registerNotifiers(cfg *config.Config, factory *notifier.Factory, logger *logging.Logger) {
    if cfg.Notifiers.Stdout {
        stdoutNotifier := notifier.NewStdoutNotifier()
        if err := factory.RegisterNotifier(domain.TypeStdout, "", stdoutNotifier); err != nil {
            logger.Fatalf("Failed to register stdout notifier: %v", err)
        }
        logger.Info("Registered stdout notifier")
    }

    // Convert config maps to interface{} for generic handler
    smtpMap := make(map[string]interface{})
    for k, v := range cfg.Notifiers.SMTP {
        smtpMap[k] = v
    }

    registerNotifierType(notifierRegistration{
        notifyType: domain.TypeEmail,
        accountMap: smtpMap,
        creator: func(accountName string, config interface{}) (domain.Notifier, error) {
            return notifier.NewSMTPNotifier(config.(*config.SMTPConfig))
        },
        isDefault: func(config interface{}) bool {
            return config.(*config.SMTPConfig).Default
        },
    }, factory, logger)

    slackMap := make(map[string]interface{})
    for k, v := range cfg.Notifiers.Slack {
        slackMap[k] = v
    }

    registerNotifierType(notifierRegistration{
        notifyType: domain.TypeSlack,
        accountMap: slackMap,
        creator: func(accountName string, config interface{}) (domain.Notifier, error) {
            return notifier.NewSlackNotifier(config.(*config.SlackConfig))
        },
        isDefault: func(config interface{}) bool {
            return config.(*config.SlackConfig).Default
        },
    }, factory, logger)

    // Same for Ntfy...
}

// NEW: 28 lines (30 lines removed)

Verification:

go build ./cmd/server
# Should compile and run successfully

Testing Checklist

Before Starting

  • All tests passing: go test ./...
  • Code compiles: go build ./cmd/server

After Phase 1

  • go test ./internal/auth -v - All auth tests pass
  • go test ./internal/notifier -v - All notifier tests pass
  • go build ./cmd/server - Server builds successfully
  • Verify no behavior changes with go test ./...

After Phase 2

  • All tests pass: go test ./...
  • go fmt ./... - Code is formatted
  • go vet ./... - No vet issues
  • go build ./cmd/server && ./server - Runs successfully

Rollback Instructions

Each refactoring is a separate commit, so if issues arise:

# Rollback Phase 1
git revert <phase-1-commit>

# Or rollback specific issue
git revert <issue-2-commit>
git revert <issue-4-commit>

Summary

Total Time Investment: 2.5 hours Lines Removed: ~92 lines Complexity Reduction: ~40-50% Risk Level: MINIMAL Benefits:

  • Easier maintenance
  • Reduced duplication
  • Better consistency
  • Easier to test
  • No behavior changes

Recommendation: Implement Phase 1 first (1 hour, high value), then Phase 2 in next sprint.