# Code Duplication Refactoring - Quick Reference Guide ## Top 3 Quick Wins (Do These First!) ### 1. Remove Duplicate Type Conversion Function (15 minutes) **File:** `/Users/igodwin/Workspace/notifier/api/grpc/handler.go` **Problem:** Function `convertDomainToProtoType` (lines 331-344) is an exact duplicate of `convertDomainTypeToProto` (lines 294-307). **Action:** 1. Delete lines 331-344 2. Replace call at line 368: ```diff - Type: convertDomainToProtoType(notif.Type), + Type: convertDomainTypeToProto(notif.Type), ``` **Result:** Removes 14 lines of duplicate code --- ### 2. Extract Notifier Validation Pattern (30 minutes) **Files:** - `/Users/igodwin/Workspace/notifier/internal/notifier/smtp.go` (lines 63-70) - `/Users/igodwin/Workspace/notifier/internal/notifier/slack.go` (lines 78-85) - `/Users/igodwin/Workspace/notifier/internal/notifier/ntfy.go` (lines 185-192) - `/Users/igodwin/Workspace/notifier/internal/notifier/stdout.go` (lines 26-33) **Problem:** All 4 notifiers have identical validation at start of Send(): ```go if err := ValidateContext(ctx); err != nil { return nil, err } if err := s.Validate(notification); err != nil { return nil, err } ``` **Action:** 1. Add to `/Users/igodwin/Workspace/notifier/internal/notifier/notifier.go`: ```go // ValidateNotification validates context and notification for sending func ValidateNotification(ctx context.Context, n *domain.Notification, validator domain.Notifier) error { if err := ValidateContext(ctx); err != nil { return err } return validator.Validate(n) } ``` 2. Replace validation blocks in all 4 files with: ```go if err := ValidateNotification(ctx, notification, s); err != nil { return nil, err } ``` **Result:** Reduces duplication from 4 places to 1, easier maintenance --- ### 3. Add NotificationResult Error Helper (30 minutes) **Files:** - `/Users/igodwin/Workspace/notifier/internal/notifier/smtp.go` (multiple locations) - `/Users/igodwin/Workspace/notifier/internal/notifier/slack.go` (multiple locations) - `/Users/igodwin/Workspace/notifier/internal/notifier/ntfy.go` (multiple locations) **Problem:** Repeated error result creation pattern in all notifiers **Action:** 1. Add to `BaseNotifier` struct: ```go func (b *BaseNotifier) ErrorResult(notificationID, msg string) *domain.NotificationResult { return &domain.NotificationResult{ NotificationID: notificationID, Success: false, Error: msg, SentAt: time.Now(), } } ``` 2. Replace error creation calls with: ```go return b.ErrorResult(notification.ID, err.Error()), err ``` **Result:** Consistent error handling across all notifiers --- ## Medium-Effort Improvements ### 4. Refactor Filter Matching (40 minutes) **File:** `/Users/igodwin/Workspace/notifier/internal/service/service.go` (lines 481-540) **Problem:** 60+ lines of repetitive filter matching logic **Solution:** Extract helper functions: ```go func contains[T comparable](items []T, target T) bool { for _, item := range items { if item == target { return true } } return false } func notificationHasRecipient(n *domain.Notification, recipients []string) bool { for _, fr := range recipients { if contains(n.Recipients, fr) { return true } } return false } ``` Then simplify `matchesFilter` to use these helpers. **Result:** Reduces 60+ lines to ~30, easier to extend --- ### 5. Extract Auth Validation (50 minutes) **Files:** - `/Users/igodwin/Workspace/notifier/internal/auth/rest_middleware.go` (lines 35-50) - `/Users/igodwin/Workspace/notifier/internal/auth/grpc_middleware.go` (lines 38-50, 82-94) **Problem:** Same validation logic in 3 places **Solution:** Add to `APIKeyStore`: ```go type ValidatedKey struct { APIKey *APIKey ClientID string Roles []string } func (s *APIKeyStore) ValidateAndAuthorize(keyStr string) (*ValidatedKey, error) { key, err := s.ValidateKey(keyStr) if err != nil { return nil, err } allowed, err := s.CheckRateLimit(keyStr) if err != nil || !allowed { return nil, fmt.Errorf("rate limit exceeded") } if err := s.UpdateLastUsed(keyStr); err != nil { // Log warning but continue } return &ValidatedKey{ APIKey: key, ClientID: key.ClientID, Roles: key.Roles, }, nil } ``` Use in both middleware implementations. **Result:** Consistent auth logic, easier to update validation rules --- ### 6. Create HTTP Request Helper (45 minutes) **Files:** - `/Users/igodwin/Workspace/notifier/internal/notifier/slack.go` (sendToSlack) - `/Users/igodwin/Workspace/notifier/internal/notifier/ntfy.go` (sendToTopic) **Problem:** 25+ lines of identical HTTP handling **Solution:** Add to notifier package: ```go func sendJSONPostRequest(ctx context.Context, client *http.Client, url string, payload interface{}, headers map[string]string) error { jsonData, err := json.Marshal(payload) if err != nil { return fmt.Errorf("failed to marshal: %w", err) } req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewBuffer(jsonData)) if err != nil { return fmt.Errorf("failed to create request: %w", err) } req.Header.Set("Content-Type", "application/json") for k, v := range headers { req.Header.Set(k, v) } resp, err := client.Do(req) if err != nil { return fmt.Errorf("failed to send request: %w", err) } defer resp.Body.Close() if resp.StatusCode < 200 || resp.StatusCode >= 300 { return fmt.Errorf("API returned status: %d", resp.StatusCode) } return nil } ``` **Result:** Single source of truth for HTTP logic --- ## More Involved Refactorings ### 7. Generic Notifier Registration (60 minutes) **File:** `/Users/igodwin/Workspace/notifier/cmd/server/main.go` (lines 193-241) Creates generic registration helper to reduce 50+ lines of repetitive code. ### 8. Generic Default Account Resolution (45 minutes) **File:** `/Users/igodwin/Workspace/notifier/internal/config/config.go` (lines 346-380) Extract generic helper to eliminate 35+ lines of repetitive default account lookup. --- ## Implementation Checklist ### Phase 1 (Recommended: Next Hour) - [ ] Remove duplicate `convertDomainToProtoType` (15 min) - [ ] Extract validation helper (30 min) - [ ] Add error result helper (30 min) - **Total: 75 minutes** ### Phase 2 (Recommended: Next Sprint) - [ ] Refactor filter matching (40 min) - [ ] Extract auth validation (50 min) - [ ] Create HTTP helper (45 min) - **Total: 135 minutes** ### Phase 3 (Nice to Have: Later) - [ ] Generic registration function (60 min) - [ ] Generic default resolution (45 min) - **Total: 105 minutes** --- ## Testing Strategy 1. **Phase 1 refactorings:** Run existing unit tests - no behavior changes 2. **Phase 2 refactorings:** Add unit tests for new helper functions 3. **Phase 3 refactorings:** Comprehensive integration tests for registration flow All changes maintain backward compatibility. --- ## Expected Benefits | Metric | Phase 1 | Phase 1+2 | All Phases | |--------|---------|-----------|-----------| | Lines of duplication removed | ~60 | ~250 | ~380 | | Number of duplicate patterns | 3 | 6 | 8 | | Estimated maintenance effort reduction | 15% | 40% | 60% | --- ## Files to Review After Refactoring 1. All notifier implementations (`internal/notifier/`) 2. Service filtering logic (`internal/service/service.go`) 3. Auth middleware files (`internal/auth/`) 4. Main server initialization (`cmd/server/main.go`) 5. Config file (`internal/config/config.go`)