Coverage priorities 1-5: test pure functions, extract interfaces, mock-based handler tests

- Priority 1: Test NewSendEmailTask + NewSendPushTask (5 tests) - Priority 2: Test customHTTPErrorHandler — all 15+ branches (21 tests) - Priority 3: Extract Enqueuer interface + payload builders in worker pkg (5 tests) - Priority 4: Extract ClassifyFile/ComputeRelPath in migrate-encrypt (6 tests) - Priority 5: Define Handler interfaces, refactor to accept them, mock-based tests (14 tests) - Fix .gitignore: /worker instead of worker to stop ignoring internal/worker/ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-01 20:30:09 -05:00
parent 00fd674b56
commit bec880886b
83 changed files with 19569 additions and 730 deletions
--- a/internal/router/error_handler_test.go
+++ b/internal/router/error_handler_test.go
@@ -0,0 +1,262 @@
+package router
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"testing"
+
+	"github.com/labstack/echo/v4"
+
+	"github.com/treytartt/honeydue-api/internal/apperrors"
+	"github.com/treytartt/honeydue-api/internal/dto/responses"
+	"github.com/treytartt/honeydue-api/internal/i18n"
+	"github.com/treytartt/honeydue-api/internal/services"
+)
+
+func TestMain(m *testing.M) {
+	// Initialize i18n so LocalizedMessage returns real translations
+	_ = i18n.Init()
+	os.Exit(m.Run())
+}
+
+// makeContext creates a fresh echo.Context and response recorder for testing.
+func makeContext(method, path string) (echo.Context, *httptest.ResponseRecorder) {
+	e := echo.New()
+	req := httptest.NewRequest(method, path, nil)
+	req.Header.Set("Accept-Language", "en")
+	rec := httptest.NewRecorder()
+	c := e.NewContext(req, rec)
+	// Set up i18n localizer on context (same key the middleware uses)
+	if i18n.Bundle != nil {
+		loc := i18n.NewLocalizer("en")
+		c.Set(i18n.LocalizerKey, loc)
+	}
+	return c, rec
+}
+
+// decodeError reads the JSON response into an ErrorResponse.
+func decodeError(t *testing.T, rec *httptest.ResponseRecorder) responses.ErrorResponse {
+	t.Helper()
+	var resp responses.ErrorResponse
+	if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("failed to decode error response: %v\nbody: %s", err, rec.Body.String())
+	}
+	return resp
+}
+
+// --- AppError branch ---
+
+func TestErrorHandler_AppError_NotFound(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/tasks/1")
+	err := apperrors.NotFound("error.task_not_found")
+	customHTTPErrorHandler(err, c)
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusNotFound)
+	}
+	resp := decodeError(t, rec)
+	if resp.Error == "" {
+		t.Error("expected non-empty error message")
+	}
+}
+
+func TestErrorHandler_AppError_Forbidden(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/res/1")
+	err := apperrors.Forbidden("error.task_access_denied")
+	customHTTPErrorHandler(err, c)
+	if rec.Code != http.StatusForbidden {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusForbidden)
+	}
+}
+
+func TestErrorHandler_AppError_BadRequest(t *testing.T) {
+	c, rec := makeContext(http.MethodPost, "/tasks")
+	err := apperrors.BadRequest("error.task_already_cancelled")
+	customHTTPErrorHandler(err, c)
+	if rec.Code != http.StatusBadRequest {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusBadRequest)
+	}
+}
+
+func TestErrorHandler_AppError_WithWrappedErr(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/x")
+	err := apperrors.Internal(fmt.Errorf("db conn failed"))
+	customHTTPErrorHandler(err, c)
+	if rec.Code != http.StatusInternalServerError {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusInternalServerError)
+	}
+}
+
+func TestErrorHandler_AppError_I18nFallback(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/x")
+	err := apperrors.NotFound("nonexistent.i18n.key").WithMessage("fallback msg")
+	customHTTPErrorHandler(err, c)
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusNotFound)
+	}
+	resp := decodeError(t, rec)
+	if resp.Error != "fallback msg" {
+		t.Errorf("error = %q, want %q", resp.Error, "fallback msg")
+	}
+}
+
+// --- Echo HTTPError branch ---
+
+func TestErrorHandler_EchoHTTPError_404(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/nope")
+	err := echo.NewHTTPError(http.StatusNotFound, "not found")
+	customHTTPErrorHandler(err, c)
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusNotFound)
+	}
+	resp := decodeError(t, rec)
+	if resp.Error != "not found" {
+		t.Errorf("error = %q, want %q", resp.Error, "not found")
+	}
+}
+
+func TestErrorHandler_EchoHTTPError_405(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/x")
+	err := echo.ErrMethodNotAllowed
+	customHTTPErrorHandler(err, c)
+	if rec.Code != http.StatusMethodNotAllowed {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusMethodNotAllowed)
+	}
+}
+
+// --- Sentinel error branch ---
+
+func TestErrorHandler_ErrTaskNotFound(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/t")
+	customHTTPErrorHandler(services.ErrTaskNotFound, c)
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusNotFound)
+	}
+}
+
+func TestErrorHandler_ErrCompletionNotFound(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/t")
+	customHTTPErrorHandler(services.ErrCompletionNotFound, c)
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusNotFound)
+	}
+}
+
+func TestErrorHandler_ErrTaskAccessDenied(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/t")
+	customHTTPErrorHandler(services.ErrTaskAccessDenied, c)
+	if rec.Code != http.StatusForbidden {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusForbidden)
+	}
+}
+
+func TestErrorHandler_ErrTaskAlreadyCancelled(t *testing.T) {
+	c, rec := makeContext(http.MethodPost, "/t")
+	customHTTPErrorHandler(services.ErrTaskAlreadyCancelled, c)
+	if rec.Code != http.StatusBadRequest {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusBadRequest)
+	}
+}
+
+func TestErrorHandler_ErrTaskAlreadyArchived(t *testing.T) {
+	c, rec := makeContext(http.MethodPost, "/t")
+	customHTTPErrorHandler(services.ErrTaskAlreadyArchived, c)
+	if rec.Code != http.StatusBadRequest {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusBadRequest)
+	}
+}
+
+func TestErrorHandler_ErrResidenceNotFound(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/r")
+	customHTTPErrorHandler(services.ErrResidenceNotFound, c)
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusNotFound)
+	}
+}
+
+func TestErrorHandler_ErrResidenceAccessDenied(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/r")
+	customHTTPErrorHandler(services.ErrResidenceAccessDenied, c)
+	if rec.Code != http.StatusForbidden {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusForbidden)
+	}
+}
+
+func TestErrorHandler_ErrNotResidenceOwner(t *testing.T) {
+	c, rec := makeContext(http.MethodDelete, "/r")
+	customHTTPErrorHandler(services.ErrNotResidenceOwner, c)
+	if rec.Code != http.StatusForbidden {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusForbidden)
+	}
+}
+
+func TestErrorHandler_ErrPropertiesLimitReached(t *testing.T) {
+	c, rec := makeContext(http.MethodPost, "/r")
+	customHTTPErrorHandler(services.ErrPropertiesLimitReached, c)
+	if rec.Code != http.StatusForbidden {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusForbidden)
+	}
+}
+
+func TestErrorHandler_ErrCannotRemoveOwner(t *testing.T) {
+	c, rec := makeContext(http.MethodDelete, "/r")
+	customHTTPErrorHandler(services.ErrCannotRemoveOwner, c)
+	if rec.Code != http.StatusBadRequest {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusBadRequest)
+	}
+}
+
+func TestErrorHandler_ErrShareCodeExpired(t *testing.T) {
+	c, rec := makeContext(http.MethodPost, "/r")
+	customHTTPErrorHandler(services.ErrShareCodeExpired, c)
+	if rec.Code != http.StatusBadRequest {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusBadRequest)
+	}
+}
+
+func TestErrorHandler_ErrShareCodeInvalid(t *testing.T) {
+	c, rec := makeContext(http.MethodPost, "/r")
+	customHTTPErrorHandler(services.ErrShareCodeInvalid, c)
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusNotFound)
+	}
+}
+
+func TestErrorHandler_ErrUserAlreadyMember(t *testing.T) {
+	c, rec := makeContext(http.MethodPost, "/r")
+	customHTTPErrorHandler(services.ErrUserAlreadyMember, c)
+	if rec.Code != http.StatusConflict {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusConflict)
+	}
+}
+
+// --- Default branch ---
+
+func TestErrorHandler_UnknownError(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/x")
+	customHTTPErrorHandler(errors.New("random unexpected error"), c)
+	if rec.Code != http.StatusInternalServerError {
+		t.Errorf("code = %d, want %d", rec.Code, http.StatusInternalServerError)
+	}
+}
+
+// --- Committed response ---
+
+func TestErrorHandler_CommittedResponse_Noop(t *testing.T) {
+	c, rec := makeContext(http.MethodGet, "/x")
+	// Write something to commit the response
+	c.JSON(http.StatusOK, map[string]string{"ok": "true"})
+	// Capture the body after first write
+	bodyBefore := rec.Body.String()
+
+	// Now call error handler — it should be a no-op
+	customHTTPErrorHandler(errors.New("should be ignored"), c)
+	bodyAfter := rec.Body.String()
+
+	if bodyBefore != bodyAfter {
+		t.Errorf("body changed after committed response:\nbefore: %s\nafter: %s", bodyBefore, bodyAfter)
+	}
+}
--- a/internal/router/router_helpers.go
+++ b/internal/router/router_helpers.go
@@ -0,0 +1,115 @@
+package router
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/treytartt/honeydue-api/internal/monitoring"
+)
+
+// CorsOrigins returns the CORS allowed origins based on debug mode.
+func CorsOrigins(debug bool, configuredOrigins []string) []string {
+	if debug {
+		return []string{
+			"http://localhost:3000",
+			"http://localhost:3001",
+			"http://localhost:8080",
+			"http://localhost:8000",
+			"http://127.0.0.1:3000",
+			"http://127.0.0.1:3001",
+			"http://127.0.0.1:8080",
+			"http://127.0.0.1:8000",
+		}
+	}
+	if len(configuredOrigins) > 0 {
+		return configuredOrigins
+	}
+	return []string{
+		"https://api.myhoneydue.com",
+		"https://myhoneydue.com",
+		"https://admin.myhoneydue.com",
+	}
+}
+
+// ShouldSkipTimeout returns true for paths that should bypass timeout middleware.
+func ShouldSkipTimeout(path, host, adminHost string) bool {
+	return (adminHost != "" && host == adminHost) ||
+		strings.HasPrefix(path, "/_next") ||
+		strings.HasSuffix(path, "/ws")
+}
+
+// ShouldSkipBodyLimit returns true for webhook endpoints.
+func ShouldSkipBodyLimit(path string) bool {
+	return strings.HasPrefix(path, "/api/subscription/webhook")
+}
+
+// ShouldSkipGzip returns true for media endpoints.
+func ShouldSkipGzip(path string) bool {
+	return strings.HasPrefix(path, "/api/media/")
+}
+
+// ParseEndpoint splits "GET /api/foo" into method and path.
+func ParseEndpoint(endpoint string) (method, path string) {
+	parts := strings.SplitN(endpoint, " ", 2)
+	if len(parts) == 2 {
+		return parts[0], parts[1]
+	}
+	return endpoint, ""
+}
+
+// AllowedProxyHosts builds the list of hosts allowed for admin proxy.
+func AllowedProxyHosts(adminHost string) []string {
+	var hosts []string
+	if adminHost != "" {
+		hosts = append(hosts, adminHost)
+	}
+	hosts = append(hosts, "localhost:3001", "127.0.0.1:3001", "localhost:8000", "127.0.0.1:8000")
+	return hosts
+}
+
+// DetermineAdminRoute decides how to route admin subdomain requests.
+func DetermineAdminRoute(path string) string {
+	if strings.HasPrefix(path, "/admin") {
+		return "redirect"
+	}
+	if strings.HasPrefix(path, "/api/") {
+		return "passthrough"
+	}
+	return "proxy"
+}
+
+// FormatPrometheusMetrics converts HTTP stats to Prometheus text format.
+func FormatPrometheusMetrics(stats monitoring.HTTPStats) string {
+	var b strings.Builder
+
+	b.WriteString("# HELP http_requests_total Total number of HTTP requests.\n")
+	b.WriteString("# TYPE http_requests_total counter\n")
+	for statusCode, count := range stats.ByStatusCode {
+		fmt.Fprintf(&b, "http_requests_total{status_code=\"%d\"} %d\n", statusCode, count)
+	}
+
+	b.WriteString("# HELP http_endpoint_requests_total Total requests per endpoint.\n")
+	b.WriteString("# TYPE http_endpoint_requests_total counter\n")
+	for endpoint, epStats := range stats.ByEndpoint {
+		method, path := ParseEndpoint(endpoint)
+		fmt.Fprintf(&b, "http_endpoint_requests_total{method=\"%s\",path=\"%s\"} %d\n", method, path, epStats.Count)
+	}
+
+	b.WriteString("# HELP http_request_duration_ms Average request duration in milliseconds per endpoint.\n")
+	b.WriteString("# TYPE http_request_duration_ms gauge\n")
+	for endpoint, epStats := range stats.ByEndpoint {
+		method, path := ParseEndpoint(endpoint)
+		fmt.Fprintf(&b, "http_request_duration_ms{method=\"%s\",path=\"%s\",quantile=\"avg\"} %.2f\n", method, path, epStats.AvgLatencyMs)
+		fmt.Fprintf(&b, "http_request_duration_ms{method=\"%s\",path=\"%s\",quantile=\"p95\"} %.2f\n", method, path, epStats.P95LatencyMs)
+	}
+
+	b.WriteString("# HELP http_error_rate Overall error rate (4xx+5xx / total).\n")
+	b.WriteString("# TYPE http_error_rate gauge\n")
+	fmt.Fprintf(&b, "http_error_rate %.4f\n", stats.ErrorRate)
+
+	b.WriteString("# HELP http_requests_per_minute Current request rate.\n")
+	b.WriteString("# TYPE http_requests_per_minute gauge\n")
+	fmt.Fprintf(&b, "http_requests_per_minute %.2f\n", stats.RequestsPerMinute)
+
+	return b.String()
+}
--- a/internal/router/router_helpers_test.go
+++ b/internal/router/router_helpers_test.go
@@ -0,0 +1,200 @@
+package router
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/treytartt/honeydue-api/internal/monitoring"
+)
+
+// --- CorsOrigins ---
+
+func TestCorsOrigins_Debug(t *testing.T) {
+	origins := CorsOrigins(true, nil)
+	if len(origins) != 8 {
+		t.Errorf("len = %d, want 8", len(origins))
+	}
+	found := false
+	for _, o := range origins {
+		if o == "http://localhost:3000" {
+			found = true
+		}
+	}
+	if !found {
+		t.Error("expected localhost:3000 in debug origins")
+	}
+}
+
+func TestCorsOrigins_ProductionConfigured(t *testing.T) {
+	custom := []string{"https://example.com"}
+	origins := CorsOrigins(false, custom)
+	if len(origins) != 1 || origins[0] != "https://example.com" {
+		t.Errorf("got %v, want [https://example.com]", origins)
+	}
+}
+
+func TestCorsOrigins_ProductionDefault(t *testing.T) {
+	origins := CorsOrigins(false, nil)
+	if len(origins) != 3 {
+		t.Errorf("len = %d, want 3", len(origins))
+	}
+	found := false
+	for _, o := range origins {
+		if o == "https://myhoneydue.com" {
+			found = true
+		}
+	}
+	if !found {
+		t.Error("expected myhoneydue.com in default origins")
+	}
+}
+
+// --- ShouldSkipTimeout ---
+
+func TestShouldSkipTimeout_AdminHost_True(t *testing.T) {
+	if !ShouldSkipTimeout("/some/path", "admin.example.com", "admin.example.com") {
+		t.Error("expected true for admin host")
+	}
+}
+
+func TestShouldSkipTimeout_NextPath_True(t *testing.T) {
+	if !ShouldSkipTimeout("/_next/static/chunk.js", "app.example.com", "admin.example.com") {
+		t.Error("expected true for _next path")
+	}
+}
+
+func TestShouldSkipTimeout_WsPath_True(t *testing.T) {
+	if !ShouldSkipTimeout("/api/events/ws", "app.example.com", "admin.example.com") {
+		t.Error("expected true for /ws path")
+	}
+}
+
+func TestShouldSkipTimeout_NormalPath_False(t *testing.T) {
+	if ShouldSkipTimeout("/api/tasks/", "app.example.com", "admin.example.com") {
+		t.Error("expected false for normal path")
+	}
+}
+
+func TestShouldSkipTimeout_EmptyAdminHost_False(t *testing.T) {
+	if ShouldSkipTimeout("/api/tasks/", "admin.example.com", "") {
+		t.Error("expected false when admin host is empty")
+	}
+}
+
+// --- ShouldSkipBodyLimit ---
+
+func TestShouldSkipBodyLimit_Webhook_True(t *testing.T) {
+	if !ShouldSkipBodyLimit("/api/subscription/webhook/apple/") {
+		t.Error("expected true for webhook path")
+	}
+}
+
+func TestShouldSkipBodyLimit_Normal_False(t *testing.T) {
+	if ShouldSkipBodyLimit("/api/tasks/") {
+		t.Error("expected false for normal path")
+	}
+}
+
+// --- ShouldSkipGzip ---
+
+func TestShouldSkipGzip_Media_True(t *testing.T) {
+	if !ShouldSkipGzip("/api/media/document/123") {
+		t.Error("expected true for media path")
+	}
+}
+
+func TestShouldSkipGzip_Api_False(t *testing.T) {
+	if ShouldSkipGzip("/api/tasks/") {
+		t.Error("expected false for non-media path")
+	}
+}
+
+// --- ParseEndpoint ---
+
+func TestParseEndpoint_MethodAndPath(t *testing.T) {
+	method, path := ParseEndpoint("GET /api/tasks/")
+	if method != "GET" || path != "/api/tasks/" {
+		t.Errorf("got (%q, %q), want (GET, /api/tasks/)", method, path)
+	}
+}
+
+func TestParseEndpoint_NoSpace(t *testing.T) {
+	method, path := ParseEndpoint("GET")
+	if method != "GET" || path != "" {
+		t.Errorf("got (%q, %q), want (GET, \"\")", method, path)
+	}
+}
+
+// --- AllowedProxyHosts ---
+
+func TestAllowedProxyHosts_WithAdmin(t *testing.T) {
+	hosts := AllowedProxyHosts("admin.example.com")
+	if len(hosts) != 5 {
+		t.Errorf("len = %d, want 5", len(hosts))
+	}
+	if hosts[0] != "admin.example.com" {
+		t.Errorf("first host = %q, want admin.example.com", hosts[0])
+	}
+}
+
+func TestAllowedProxyHosts_WithoutAdmin(t *testing.T) {
+	hosts := AllowedProxyHosts("")
+	if len(hosts) != 4 {
+		t.Errorf("len = %d, want 4", len(hosts))
+	}
+}
+
+// --- DetermineAdminRoute ---
+
+func TestDetermineAdminRoute_Admin_Redirect(t *testing.T) {
+	got := DetermineAdminRoute("/admin/dashboard")
+	if got != "redirect" {
+		t.Errorf("got %q, want redirect", got)
+	}
+}
+
+func TestDetermineAdminRoute_Api_Passthrough(t *testing.T) {
+	got := DetermineAdminRoute("/api/tasks/")
+	if got != "passthrough" {
+		t.Errorf("got %q, want passthrough", got)
+	}
+}
+
+func TestDetermineAdminRoute_Other_Proxy(t *testing.T) {
+	got := DetermineAdminRoute("/dashboard")
+	if got != "proxy" {
+		t.Errorf("got %q, want proxy", got)
+	}
+}
+
+// --- FormatPrometheusMetrics ---
+
+func TestFormatPrometheusMetrics_Output(t *testing.T) {
+	stats := monitoring.HTTPStats{
+		RequestsTotal:     100,
+		RequestsPerMinute: 10.5,
+		ErrorRate:         0.05,
+		ByStatusCode:      map[int]int64{200: 90, 500: 10},
+		ByEndpoint: map[string]monitoring.EndpointStats{
+			"GET /api/tasks/": {Count: 50, AvgLatencyMs: 12.5, P95LatencyMs: 45.0},
+		},
+	}
+
+	output := FormatPrometheusMetrics(stats)
+
+	// Check key sections exist
+	checks := []string{
+		"http_requests_total",
+		"http_endpoint_requests_total",
+		"http_request_duration_ms",
+		"http_error_rate",
+		"http_requests_per_minute",
+		`method="GET"`,
+		`path="/api/tasks/"`,
+	}
+	for _, check := range checks {
+		if !strings.Contains(output, check) {
+			t.Errorf("output missing %q", check)
+		}
+	}
+}