Skip to content

Refactor to eliminate code duplication and improve maintainability - #9

Open
powerje wants to merge 8 commits into
trunkfrom
cleanup
Open

Refactor to eliminate code duplication and improve maintainability#9
powerje wants to merge 8 commits into
trunkfrom
cleanup

Conversation

@powerje

@powerje powerje commented Nov 18, 2025

Copy link
Copy Markdown
Contributor

Summary

This PR implements a comprehensive refactoring to eliminate code duplication, improve maintainability, and enhance testability across the codebase. The changes eliminate ~315 lines of duplicated code while adding ~621 net lines (including extensive test coverage).

Key Changes

Code Duplication Elimination (~315 lines removed):

  • Extract server URL retry logic to reusable TryServerURLs helper (~75 lines)
  • Add WithUser middleware to eliminate repeated user DB fetches (~130 lines)
  • Extract main.go initialization logic to app.Initializer (~110 lines)

New Components:

  • internal/helpers/plex.go - Generic helper for trying multiple server URLs with fallback
  • internal/middleware/user.go - Middleware to fetch user once and add to context
  • internal/app/initializer.go - Extracted initialization logic from main.go

Bug Fixes:

  • Fix context.Background() usage in pairing handler with proper 30-second timeout

Infrastructure:

  • Add database transaction support (BeginTx, WithTransaction helper)
  • Comprehensive test coverage for all new functionality

Files Changed (18 files)

  • New files: internal/helpers/plex.go, internal/helpers/plex_test.go, internal/middleware/user.go, internal/middleware/user_test.go, internal/app/initializer.go
  • Modified: main.go, internal/db/db.go, internal/db/db_test.go, handlers (media.go, mappings.go, pairing.go, play.go, media_detail.go), handler tests

Commits (8)

  1. Add TryServerURLs helper to eliminate server retry duplication
  2. Refactor media.go to use TryServerURLs helper
  3. Refactor mappings.go to use TryServerURLs helper
  4. Add WithUser middleware to eliminate user fetch duplication
  5. Integrate WithUser middleware into auth chain
  6. Add WithUser middleware and refactor handlers to use user from context
  7. Fix context.Background() usage in pairing handler with timeout
  8. Add transaction support and extract main initialization logic

Test Plan

  • All existing tests pass (verified with ./checks.sh)
  • New test coverage added:
    • internal/helpers/plex_test.go - TryServerURLs helper tests
    • internal/middleware/user_test.go - WithUser middleware tests
    • internal/db/db_test.go - Transaction tests (BeginTx, WithTransaction)
  • Code formatting passes (go fmt)
  • Linting passes (golangci-lint - 0 issues)
  • Race detector passes (go test -race)
  • Build succeeds

Impact

  • Lines changed: +960 insertions, -339 deletions (net +621, including tests)
  • Code quality: Eliminated significant duplication, improved separation of concerns
  • Maintainability: Extracted reusable components, better testability
  • No breaking changes: All existing functionality preserved

- Created WithUser middleware that fetches user from DB and adds to context
- Eliminated ~130 lines of duplicate user retrieval code across handlers
- Updated 6 handlers: media.go (3 methods), mappings.go (3 methods),
  media_detail.go (1), play.go (1)
- Integrated WithUser into auth middleware chain (WithUserID -> WithUser -> RequireAuth)
- Updated all handler tests to chain both middlewares correctly
- All tests passing, no lint issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant