diff --git a/internal/api/handler.go b/internal/api/handler.go index bed0abf..4cb3905 100644 --- a/internal/api/handler.go +++ b/internal/api/handler.go @@ -137,7 +137,7 @@ func (h *IngestionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Device classification if h.cfg.Site.Collect.Device { - device = h.classifyDevice(event.Width) + device = h.classifyDevice(event.Width, userAgent) } // Referrer classification @@ -271,19 +271,43 @@ func (h *IngestionHandler) ipInCIDR(ip, cidr string) bool { return network.Contains(testIP) } -// Classifies screen width into device categories using configured breakpoints -// FIXME: we need a more robust mechanism for classifying devices. Breakpoints -// are the only ones I can think of *right now* but I'm positive there are better -// mechanisns. We'll get to this later. -func (h *IngestionHandler) classifyDevice(width int) string { - if width == 0 { - return "unknown" - } - if width < h.cfg.Limits.DeviceBreakpoints.Mobile { - return "mobile" - } - if width < h.cfg.Limits.DeviceBreakpoints.Tablet { +// Classifies device using both screen width and User-Agent parsing +// Uses UA hints for better detection, falls back to width breakpoints +func (h *IngestionHandler) classifyDevice(width int, userAgent string) string { + // First try User-Agent based detection for better accuracy + ua := strings.ToLower(userAgent) + + // Tablet detection via UA (must come before mobile: Android tablets lack "mobile" keyword) + if strings.Contains(ua, "tablet") || + strings.Contains(ua, "ipad") || + (strings.Contains(ua, "android") && !strings.Contains(ua, "mobile")) { return "tablet" } - return "desktop" + + // Mobile detection via UA + if strings.Contains(ua, "mobile") || + strings.Contains(ua, "iphone") || + strings.Contains(ua, "ipod") || + strings.Contains(ua, "windows phone") || + strings.Contains(ua, "blackberry") { + return "mobile" + } + + // If UA doesn't provide clear signal, use width breakpoints + if width > 0 { + if width < h.cfg.Limits.DeviceBreakpoints.Mobile { + return "mobile" + } + if width < h.cfg.Limits.DeviceBreakpoints.Tablet { + return "tablet" + } + return "desktop" + } + + // Default to desktop if UA suggests desktop browser + if userAgent != "" { + return "desktop" + } + + return "unknown" } diff --git a/internal/api/handler_test.go b/internal/api/handler_test.go index cb788d9..cee1dbb 100644 --- a/internal/api/handler_test.go +++ b/internal/api/handler_test.go @@ -2,7 +2,6 @@ package api import ( "bytes" - "fmt" "net/http" "net/http/httptest" "testing" @@ -207,51 +206,113 @@ func TestIngestionHandler_InvalidJSON(t *testing.T) { } } -func TestIngestionHandler_DeviceClassification(t *testing.T) { - cfg := config.Config{ - Site: config.SiteConfig{ - Domains: []string{"example.com"}, - Collect: config.CollectConfig{ - Pageviews: true, - Device: true, - }, - Path: config.PathConfig{}, - }, - Limits: config.LimitsConfig{ - MaxPaths: 100, - MaxSources: 50, - }, - } - +func newTestHandler(cfg *config.Config) *IngestionHandler { pathNorm := normalize.NewPathNormalizer(cfg.Site.Path) pathRegistry := aggregate.NewPathRegistry(cfg.Limits.MaxPaths) refRegistry := normalize.NewReferrerRegistry(cfg.Limits.MaxSources) metricsAgg := aggregate.NewMetricsAggregator( pathRegistry, aggregate.NewCustomEventRegistry(100), - &cfg, + cfg, ) + return NewIngestionHandler(cfg, pathNorm, pathRegistry, refRegistry, metricsAgg) +} - handler := NewIngestionHandler(&cfg, pathNorm, pathRegistry, refRegistry, metricsAgg) +func TestClassifyDevice_UA(t *testing.T) { + cfg := &config.Config{ + Limits: config.LimitsConfig{ + DeviceBreakpoints: config.DeviceBreaks{ + Mobile: 768, + Tablet: 1024, + }, + }, + } + h := newTestHandler(cfg) tests := []struct { - name string - width int + name string + width int + userAgent string + want string }{ - {"mobile", 375}, - {"tablet", 768}, - {"desktop", 1920}, + // UA takes priority + { + name: "iphone via UA", + width: 390, + userAgent: "Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15", + want: "mobile", + }, + { + name: "android phone via UA", + width: 0, + userAgent: "Mozilla/5.0 (Linux; Android 13; Pixel 7) Mobile Safari/537.36", + want: "mobile", + }, + { + name: "windows phone via UA", + width: 0, + userAgent: "Mozilla/5.0 (compatible; MSIE 10.0; Windows Phone 8.0)", + want: "mobile", + }, + { + name: "ipad via UA", + width: 1024, + userAgent: "Mozilla/5.0 (iPad; CPU OS 17_0 like Mac OS X) AppleWebKit/605.1.15", + want: "tablet", + }, + { + name: "android tablet via UA (no mobile keyword)", + width: 0, + userAgent: "Mozilla/5.0 (Linux; Android 13; SM-T870) AppleWebKit/537.36", + want: "tablet", + }, + // Falls back to width when UA is desktop + { + name: "desktop UA wide screen", + width: 1920, + userAgent: "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 Chrome/120.0", + want: "desktop", + }, + { + name: "desktop UA narrow width", + width: 500, + userAgent: "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 Chrome/120.0", + want: "mobile", + }, + // Width-only fallback + { + name: "no UA mobile width", + width: 375, + userAgent: "", + want: "mobile", + }, + { + name: "no UA tablet width", + width: 800, + userAgent: "", + want: "tablet", + }, + { + name: "no UA desktop width", + width: 1440, + userAgent: "", + want: "desktop", + }, + // Unknown + { + name: "no UA no width", + width: 0, + userAgent: "", + want: "unknown", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - body := fmt.Sprintf(`{"d":"example.com","p":"/test","w":%d}`, tt.width) - req := httptest.NewRequest("POST", "/api/event", bytes.NewBufferString(body)) - w := httptest.NewRecorder() - handler.ServeHTTP(w, req) - - if w.Code != http.StatusNoContent { - t.Errorf("expected status %d, got %d", http.StatusNoContent, w.Code) + got := h.classifyDevice(tt.width, tt.userAgent) + if got != tt.want { + t.Errorf("classifyDevice(%d, %q) = %q, want %q", + tt.width, tt.userAgent, got, tt.want) } }) }