refactor: dry - extract modifyLabels helper in Gmail mock #6
Description
Summary
The Gmail mock's label modification logic (add/remove labels from messages) is copy-pasted verbatim across ModifyMessage, BatchModifyMessages, and ModifyThread. This creates ~80 lines of duplicated, deeply nested code (4 levels deep) that is error-prone to maintain.
Location
- File:
internal/gmail/gmail_service_mock.go - Lines: L142-169 (
ModifyMessage), L235-261 (BatchModifyMessages), L296-323 (ModifyThread)
Category
Type: DRY Violation
Severity: High
Evidence
Each of the three functions contains identical label add/remove logic:
// Remove labels - repeated 3x
newLabels := []string{}
for _, label := range msg.LabelIds {
shouldRemove := false
for _, remove := range req.RemoveLabelIds {
if label == remove { shouldRemove = true; break }
}
if !shouldRemove { newLabels = append(newLabels, label) }
}
// Add labels - repeated 3x
for _, add := range req.AddLabelIds {
found := false
for _, label := range newLabels {
if label == add { found = true; break }
}
if !found { newLabels = append(newLabels, add) }
}Suggested Refactoring
- Extract a helper function:
func modifyLabels(current []string, add, remove []string) []string - Replace all 3 inline implementations with calls to the helper
- The helper eliminates ~80 lines and reduces nesting from 4 levels to 1
Effort Estimate
- Size: Small (< 1 hour)
- Risk: Low (mock-only code, tests validate correctness)
- Tests Required: Yes (existing tests should continue to pass)
Acceptance Criteria
-
modifyLabelshelper function extracted in gmail_service_mock.go -
ModifyMessage,BatchModifyMessages,ModifyThreadall use the helper - All existing tests pass
- No regression in mock behavior