refactor(filtering): simplify version check to use HEAD request with ETag
- commit
- 562ba6b3c3b2b1823964e1645f2ce9065a3edb26
- author
- bnema · · 2025-11-29 13:55 UTC
- parent
- 0db378a
- signature
- GPG Unverified
2B82207D370127A0The key is unknown
2 files · +46 -181
M internal/filtering/updater.go +15 -59
| @@ -1,12 +1,9 @@ | |||
| 1 | 1 | package filtering | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | - | "bufio" | |
| 5 | - | "bytes" | |
| 6 | 4 | "context" | |
| 7 | 5 | "crypto/sha256" | |
| 8 | 6 | "fmt" | |
| 9 | - | "io" | |
| 10 | 7 | "net/http" | |
| 11 | 8 | "strings" | |
| 12 | 9 | "sync" | |
| @@ -144,21 +141,17 @@ func (fu *FilterUpdater) checkSingleUpdate(ctx context.Context, url string) (*Fi | |||
| 144 | 141 | return fu.downloadUpdate(ctx, url, "", "") | |
| 145 | 142 | } | |
| 146 | 143 | ||
| 147 | - | // fetchRemoteVersion fetches only the header of a filter list to extract version | |
| 148 | - | // Uses HTTP Range request to minimize bandwidth (fetches ~1KB instead of ~100KB+) | |
| 144 | + | // fetchRemoteVersion fetches version info using HEAD request (ETag or Last-Modified) | |
| 149 | 145 | func (fu *FilterUpdater) fetchRemoteVersion(ctx context.Context, url string) (string, error) { | |
| 150 | - | req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) | |
| 146 | + | req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil) | |
| 151 | 147 | if err != nil { | |
| 152 | 148 | return "", fmt.Errorf("failed to create request: %w", err) | |
| 153 | 149 | } | |
| 154 | - | ||
| 155 | - | // Request only first 1024 bytes (enough to get version header) | |
| 156 | - | req.Header.Set("Range", "bytes=0-1023") | |
| 157 | 150 | req.Header.Set("User-Agent", "Dumber Browser/1.0 Filter Updater") | |
| 158 | 151 | ||
| 159 | 152 | resp, err := fu.httpClient.Do(req) | |
| 160 | 153 | if err != nil { | |
| 161 | - | return "", fmt.Errorf("range request failed: %w", err) | |
| 154 | + | return "", fmt.Errorf("HEAD request failed: %w", err) | |
| 162 | 155 | } | |
| 163 | 156 | defer func() { | |
| 164 | 157 | if closeErr := resp.Body.Close(); closeErr != nil { | |
| @@ -166,38 +159,19 @@ func (fu *FilterUpdater) fetchRemoteVersion(ctx context.Context, url string) (st | |||
| 166 | 159 | } | |
| 167 | 160 | }() | |
| 168 | 161 | ||
| 169 | - | // Accept both 200 (server ignores Range) and 206 (Partial Content) | |
| 170 | - | if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusPartialContent { | |
| 171 | - | return "", fmt.Errorf("request returned %d", resp.StatusCode) | |
| 172 | - | } | |
| 173 | - | ||
| 174 | - | // Read the response (limited to 1KB even if server ignores Range) | |
| 175 | - | data, err := io.ReadAll(io.LimitReader(resp.Body, 1024)) | |
| 176 | - | if err != nil { | |
| 177 | - | return "", fmt.Errorf("failed to read response: %w", err) | |
| 178 | - | } | |
| 179 | - | ||
| 180 | - | // Parse version from content header (format: "! Version: 202511291248") | |
| 181 | - | scanner := bufio.NewScanner(bytes.NewReader(data)) | |
| 182 | - | for scanner.Scan() { | |
| 183 | - | line := scanner.Text() | |
| 184 | - | if strings.HasPrefix(line, "! Version:") { | |
| 185 | - | version := strings.TrimSpace(strings.TrimPrefix(line, "! Version:")) | |
| 186 | - | if version != "" && !strings.Contains(version, "%") { | |
| 187 | - | return version, nil | |
| 188 | - | } | |
| 189 | - | } | |
| 162 | + | if resp.StatusCode != http.StatusOK { | |
| 163 | + | return "", fmt.Errorf("HEAD request returned %d", resp.StatusCode) | |
| 190 | 164 | } | |
| 191 | 165 | ||
| 192 | - | // Fallback: use HTTP headers (ETag or Last-Modified) | |
| 166 | + | // Use ETag or Last-Modified as version identifier | |
| 193 | 167 | if etag := resp.Header.Get("ETag"); etag != "" { | |
| 194 | - | return "etag:" + etag, nil | |
| 168 | + | return etag, nil | |
| 195 | 169 | } | |
| 196 | 170 | if lastMod := resp.Header.Get("Last-Modified"); lastMod != "" { | |
| 197 | - | return "lastmod:" + lastMod, nil | |
| 171 | + | return lastMod, nil | |
| 198 | 172 | } | |
| 199 | 173 | ||
| 200 | - | return "", fmt.Errorf("no version info found for filter list") | |
| 174 | + | return "", fmt.Errorf("no ETag or Last-Modified header") | |
| 201 | 175 | } | |
| 202 | 176 | ||
| 203 | 177 | // needsUpdate determines if a filter list needs updating based on version comparison | |
| @@ -278,14 +252,12 @@ func (fu *FilterUpdater) downloadUpdate(ctx context.Context, url, lastModified, | |||
| 278 | 252 | // Calculate content hash | |
| 279 | 253 | hash := fmt.Sprintf("%x", sha256.Sum256(currentContent)) | |
| 280 | 254 | ||
| 281 | - | // Extract version from content or use HTTP headers as fallback | |
| 282 | - | version := fu.extractVersion(currentContent) | |
| 283 | - | if version == "" { | |
| 284 | - | if etagHeader := resp.Header.Get("ETag"); etagHeader != "" { | |
| 285 | - | version = "etag:" + etagHeader | |
| 286 | - | } else if lastModHeader := resp.Header.Get("Last-Modified"); lastModHeader != "" { | |
| 287 | - | version = "lastmod:" + lastModHeader | |
| 288 | - | } | |
| 255 | + | // Use HTTP headers as version identifier (same as fetchRemoteVersion) | |
| 256 | + | version := "" | |
| 257 | + | if etag := resp.Header.Get("ETag"); etag != "" { | |
| 258 | + | version = etag | |
| 259 | + | } else if lastMod := resp.Header.Get("Last-Modified"); lastMod != "" { | |
| 260 | + | version = lastMod | |
| 289 | 261 | } | |
| 290 | 262 | ||
| 291 | 263 | // Create update object | |
| @@ -487,22 +459,6 @@ func (fu *FilterUpdater) applyUpdate(update *FilterUpdate) error { | |||
| 487 | 459 | return nil | |
| 488 | 460 | } | |
| 489 | 461 | ||
| 490 | - | // extractVersion extracts the version from filter list content | |
| 491 | - | func (fu *FilterUpdater) extractVersion(content []byte) string { | |
| 492 | - | scanner := bufio.NewScanner(bytes.NewReader(content)) | |
| 493 | - | // Only scan first 20 lines for version header | |
| 494 | - | for i := 0; i < 20 && scanner.Scan(); i++ { | |
| 495 | - | line := scanner.Text() | |
| 496 | - | if strings.HasPrefix(line, "! Version:") { | |
| 497 | - | version := strings.TrimSpace(strings.TrimPrefix(line, "! Version:")) | |
| 498 | - | // Skip placeholder values like %timestamp% | |
| 499 | - | if version != "" && !strings.Contains(version, "%") { | |
| 500 | - | return version | |
| 501 | - | } | |
| 502 | - | } | |
| 503 | - | } | |
| 504 | - | return "" | |
| 505 | - | } | |
| 506 | 462 | ||
| 507 | 463 | // getListID generates a unique ID for a filter list URL | |
| 508 | 464 | func (fu *FilterUpdater) getListID(url string) string { | |
| @@ -1,12 +1,9 @@ | |||||
| 1 | package filtering | 1 | package filtering | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | - | "bufio" | |||
| 5 | - | "bytes" | |||
| 6 | "context" | 4 | "context" | ||
| 7 | "crypto/sha256" | 5 | "crypto/sha256" | ||
| 8 | "fmt" | 6 | "fmt" | ||
| 9 | - | "io" | |||
| 10 | "net/http" | 7 | "net/http" | ||
| 11 | "strings" | 8 | "strings" | ||
| 12 | "sync" | 9 | "sync" | ||
| @@ -144,21 +141,17 @@ func (fu *FilterUpdater) checkSingleUpdate(ctx context.Context, url string) (*Fi | |||||
| 144 | return fu.downloadUpdate(ctx, url, "", "") | 141 | return fu.downloadUpdate(ctx, url, "", "") | ||
| 145 | } | 142 | } | ||
| 146 | 143 | ||||
| 147 | - | // fetchRemoteVersion fetches only the header of a filter list to extract version | 144 | + | // fetchRemoteVersion fetches version info using HEAD request (ETag or Last-Modified) |
| 148 | - | // Uses HTTP Range request to minimize bandwidth (fetches ~1KB instead of ~100KB+) | |||
| 149 | func (fu *FilterUpdater) fetchRemoteVersion(ctx context.Context, url string) (string, error) { | 145 | func (fu *FilterUpdater) fetchRemoteVersion(ctx context.Context, url string) (string, error) { | ||
| 150 | - | req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) | 146 | + | req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil) |
| 151 | if err != nil { | 147 | if err != nil { | ||
| 152 | return "", fmt.Errorf("failed to create request: %w", err) | 148 | return "", fmt.Errorf("failed to create request: %w", err) | ||
| 153 | } | 149 | } | ||
| 154 | - | ||||
| 155 | - | // Request only first 1024 bytes (enough to get version header) | |||
| 156 | - | req.Header.Set("Range", "bytes=0-1023") | |||
| 157 | req.Header.Set("User-Agent", "Dumber Browser/1.0 Filter Updater") | 150 | req.Header.Set("User-Agent", "Dumber Browser/1.0 Filter Updater") | ||
| 158 | 151 | ||||
| 159 | resp, err := fu.httpClient.Do(req) | 152 | resp, err := fu.httpClient.Do(req) | ||
| 160 | if err != nil { | 153 | if err != nil { | ||
| 161 | - | return "", fmt.Errorf("range request failed: %w", err) | 154 | + | return "", fmt.Errorf("HEAD request failed: %w", err) |
| 162 | } | 155 | } | ||
| 163 | defer func() { | 156 | defer func() { | ||
| 164 | if closeErr := resp.Body.Close(); closeErr != nil { | 157 | if closeErr := resp.Body.Close(); closeErr != nil { | ||
| @@ -166,38 +159,19 @@ func (fu *FilterUpdater) fetchRemoteVersion(ctx context.Context, url string) (st | |||||
| 166 | } | 159 | } | ||
| 167 | }() | 160 | }() | ||
| 168 | 161 | ||||
| 169 | - | // Accept both 200 (server ignores Range) and 206 (Partial Content) | 162 | + | if resp.StatusCode != http.StatusOK { |
| 170 | - | if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusPartialContent { | 163 | + | return "", fmt.Errorf("HEAD request returned %d", resp.StatusCode) |
| 171 | - | return "", fmt.Errorf("request returned %d", resp.StatusCode) | |||
| 172 | - | } | |||
| 173 | - | ||||
| 174 | - | // Read the response (limited to 1KB even if server ignores Range) | |||
| 175 | - | data, err := io.ReadAll(io.LimitReader(resp.Body, 1024)) | |||
| 176 | - | if err != nil { | |||
| 177 | - | return "", fmt.Errorf("failed to read response: %w", err) | |||
| 178 | - | } | |||
| 179 | - | ||||
| 180 | - | // Parse version from content header (format: "! Version: 202511291248") | |||
| 181 | - | scanner := bufio.NewScanner(bytes.NewReader(data)) | |||
| 182 | - | for scanner.Scan() { | |||
| 183 | - | line := scanner.Text() | |||
| 184 | - | if strings.HasPrefix(line, "! Version:") { | |||
| 185 | - | version := strings.TrimSpace(strings.TrimPrefix(line, "! Version:")) | |||
| 186 | - | if version != "" && !strings.Contains(version, "%") { | |||
| 187 | - | return version, nil | |||
| 188 | - | } | |||
| 189 | - | } | |||
| 190 | } | 164 | } | ||
| 191 | 165 | ||||
| 192 | - | // Fallback: use HTTP headers (ETag or Last-Modified) | 166 | + | // Use ETag or Last-Modified as version identifier |
| 193 | if etag := resp.Header.Get("ETag"); etag != "" { | 167 | if etag := resp.Header.Get("ETag"); etag != "" { | ||
| 194 | - | return "etag:" + etag, nil | 168 | + | return etag, nil |
| 195 | } | 169 | } | ||
| 196 | if lastMod := resp.Header.Get("Last-Modified"); lastMod != "" { | 170 | if lastMod := resp.Header.Get("Last-Modified"); lastMod != "" { | ||
| 197 | - | return "lastmod:" + lastMod, nil | 171 | + | return lastMod, nil |
| 198 | } | 172 | } | ||
| 199 | 173 | ||||
| 200 | - | return "", fmt.Errorf("no version info found for filter list") | 174 | + | return "", fmt.Errorf("no ETag or Last-Modified header") |
| 201 | } | 175 | } | ||
| 202 | 176 | ||||
| 203 | // needsUpdate determines if a filter list needs updating based on version comparison | 177 | // needsUpdate determines if a filter list needs updating based on version comparison | ||
| @@ -278,14 +252,12 @@ func (fu *FilterUpdater) downloadUpdate(ctx context.Context, url, lastModified, | |||||
| 278 | // Calculate content hash | 252 | // Calculate content hash | ||
| 279 | hash := fmt.Sprintf("%x", sha256.Sum256(currentContent)) | 253 | hash := fmt.Sprintf("%x", sha256.Sum256(currentContent)) | ||
| 280 | 254 | ||||
| 281 | - | // Extract version from content or use HTTP headers as fallback | 255 | + | // Use HTTP headers as version identifier (same as fetchRemoteVersion) |
| 282 | - | version := fu.extractVersion(currentContent) | 256 | + | version := "" |
| 283 | - | if version == "" { | 257 | + | if etag := resp.Header.Get("ETag"); etag != "" { |
| 284 | - | if etagHeader := resp.Header.Get("ETag"); etagHeader != "" { | 258 | + | version = etag |
| 285 | - | version = "etag:" + etagHeader | 259 | + | } else if lastMod := resp.Header.Get("Last-Modified"); lastMod != "" { |
| 286 | - | } else if lastModHeader := resp.Header.Get("Last-Modified"); lastModHeader != "" { | 260 | + | version = lastMod |
| 287 | - | version = "lastmod:" + lastModHeader | |||
| 288 | - | } | |||
| 289 | } | 261 | } | ||
| 290 | 262 | ||||
| 291 | // Create update object | 263 | // Create update object | ||
| @@ -487,22 +459,6 @@ func (fu *FilterUpdater) applyUpdate(update *FilterUpdate) error { | |||||
| 487 | return nil | 459 | return nil | ||
| 488 | } | 460 | } | ||
| 489 | 461 | ||||
| 490 | - | // extractVersion extracts the version from filter list content | |||
| 491 | - | func (fu *FilterUpdater) extractVersion(content []byte) string { | |||
| 492 | - | scanner := bufio.NewScanner(bytes.NewReader(content)) | |||
| 493 | - | // Only scan first 20 lines for version header | |||
| 494 | - | for i := 0; i < 20 && scanner.Scan(); i++ { | |||
| 495 | - | line := scanner.Text() | |||
| 496 | - | if strings.HasPrefix(line, "! Version:") { | |||
| 497 | - | version := strings.TrimSpace(strings.TrimPrefix(line, "! Version:")) | |||
| 498 | - | // Skip placeholder values like %timestamp% | |||
| 499 | - | if version != "" && !strings.Contains(version, "%") { | |||
| 500 | - | return version | |||
| 501 | - | } | |||
| 502 | - | } | |||
| 503 | - | } | |||
| 504 | - | return "" | |||
| 505 | - | } | |||
| 506 | 462 | ||||
| 507 | // getListID generates a unique ID for a filter list URL | 463 | // getListID generates a unique ID for a filter list URL | ||
| 508 | func (fu *FilterUpdater) getListID(url string) string { | 464 | func (fu *FilterUpdater) getListID(url string) string { | ||
M internal/filtering/updater_test.go +31 -122
| @@ -14,53 +14,42 @@ import ( | |||
| 14 | 14 | func TestFetchRemoteVersion(t *testing.T) { | |
| 15 | 15 | tests := []struct { | |
| 16 | 16 | name string | |
| 17 | - | responseBody string | |
| 18 | 17 | responseStatus int | |
| 18 | + | etag string | |
| 19 | + | lastModified string | |
| 19 | 20 | wantVersion string | |
| 20 | 21 | wantErr bool | |
| 21 | - | useETag bool | |
| 22 | 22 | }{ | |
| 23 | 23 | { | |
| 24 | - | name: "valid version header", | |
| 25 | - | responseBody: `[Adblock Plus 2.0] | |
| 26 | - | ! Version: 202511291248 | |
| 27 | - | ! Title: EasyList | |
| 28 | - | ! Last modified: 29 Nov 2025 12:48 UTC | |
| 29 | - | `, | |
| 24 | + | name: "returns ETag when present", | |
| 30 | 25 | responseStatus: http.StatusOK, | |
| 31 | - | wantVersion: "202511291248", | |
| 26 | + | etag: `"abc123"`, | |
| 27 | + | wantVersion: `"abc123"`, | |
| 32 | 28 | wantErr: false, | |
| 33 | 29 | }, | |
| 34 | 30 | { | |
| 35 | - | name: "partial content response (206)", | |
| 36 | - | responseBody: `[Adblock Plus 2.0] | |
| 37 | - | ! Version: 202501011200 | |
| 38 | - | ! Title: Test | |
| 39 | - | `, | |
| 40 | - | responseStatus: http.StatusPartialContent, | |
| 41 | - | wantVersion: "202501011200", | |
| 31 | + | name: "returns Last-Modified when no ETag", | |
| 32 | + | responseStatus: http.StatusOK, | |
| 33 | + | lastModified: "Sat, 29 Nov 2025 12:48:00 GMT", | |
| 34 | + | wantVersion: "Sat, 29 Nov 2025 12:48:00 GMT", | |
| 42 | 35 | wantErr: false, | |
| 43 | 36 | }, | |
| 44 | 37 | { | |
| 45 | - | name: "placeholder version falls back to ETag", | |
| 46 | - | responseBody: `! Title: uBlock filters | |
| 47 | - | ! Last modified: %timestamp% | |
| 48 | - | `, | |
| 38 | + | name: "prefers ETag over Last-Modified", | |
| 49 | 39 | responseStatus: http.StatusOK, | |
| 50 | - | wantVersion: `etag:"abc123"`, | |
| 40 | + | etag: `"xyz789"`, | |
| 41 | + | lastModified: "Sat, 29 Nov 2025 12:48:00 GMT", | |
| 42 | + | wantVersion: `"xyz789"`, | |
| 51 | 43 | wantErr: false, | |
| 52 | - | useETag: true, | |
| 53 | 44 | }, | |
| 54 | 45 | { | |
| 55 | - | name: "no version info returns error", | |
| 56 | - | responseBody: `[Adblock Plus 2.0]\n! Title: No Version\n`, | |
| 46 | + | name: "error when no version headers", | |
| 57 | 47 | responseStatus: http.StatusOK, | |
| 58 | 48 | wantVersion: "", | |
| 59 | 49 | wantErr: true, | |
| 60 | 50 | }, | |
| 61 | 51 | { | |
| 62 | 52 | name: "server error", | |
| 63 | - | responseBody: "", | |
| 64 | 53 | responseStatus: http.StatusInternalServerError, | |
| 65 | 54 | wantVersion: "", | |
| 66 | 55 | wantErr: true, | |
| @@ -69,26 +58,24 @@ func TestFetchRemoteVersion(t *testing.T) { | |||
| 69 | 58 | ||
| 70 | 59 | for _, tt := range tests { | |
| 71 | 60 | t.Run(tt.name, func(t *testing.T) { | |
| 72 | - | // Create test server | |
| 73 | 61 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 74 | - | // Verify Range header is set | |
| 75 | - | rangeHeader := r.Header.Get("Range") | |
| 76 | - | assert.Contains(t, rangeHeader, "bytes=0-") | |
| 62 | + | // Verify HEAD request | |
| 63 | + | assert.Equal(t, http.MethodHead, r.Method) | |
| 77 | 64 | ||
| 78 | - | if tt.useETag { | |
| 79 | - | w.Header().Set("ETag", `"abc123"`) | |
| 65 | + | if tt.etag != "" { | |
| 66 | + | w.Header().Set("ETag", tt.etag) | |
| 67 | + | } | |
| 68 | + | if tt.lastModified != "" { | |
| 69 | + | w.Header().Set("Last-Modified", tt.lastModified) | |
| 80 | 70 | } | |
| 81 | 71 | w.WriteHeader(tt.responseStatus) | |
| 82 | - | _, _ = w.Write([]byte(tt.responseBody)) | |
| 83 | 72 | })) | |
| 84 | 73 | defer server.Close() | |
| 85 | 74 | ||
| 86 | - | // Create updater with test server | |
| 87 | 75 | fu := &FilterUpdater{ | |
| 88 | 76 | httpClient: &http.Client{Timeout: 5 * time.Second}, | |
| 89 | 77 | } | |
| 90 | 78 | ||
| 91 | - | // Test fetchRemoteVersion | |
| 92 | 79 | version, err := fu.fetchRemoteVersion(context.Background(), server.URL) | |
| 93 | 80 | ||
| 94 | 81 | if tt.wantErr { | |
| @@ -101,109 +88,35 @@ func TestFetchRemoteVersion(t *testing.T) { | |||
| 101 | 88 | } | |
| 102 | 89 | } | |
| 103 | 90 | ||
| 104 | - | func TestExtractVersion(t *testing.T) { | |
| 105 | - | tests := []struct { | |
| 106 | - | name string | |
| 107 | - | content []byte | |
| 108 | - | want string | |
| 109 | - | }{ | |
| 110 | - | { | |
| 111 | - | name: "standard format", | |
| 112 | - | content: []byte(`[Adblock Plus 2.0] | |
| 113 | - | ! Version: 202511291248 | |
| 114 | - | ! Title: EasyList`), | |
| 115 | - | want: "202511291248", | |
| 116 | - | }, | |
| 117 | - | { | |
| 118 | - | name: "version with spaces", | |
| 119 | - | content: []byte(`! Version: 20251129 | |
| 120 | - | ! Title: Test`), | |
| 121 | - | want: "20251129", | |
| 122 | - | }, | |
| 123 | - | { | |
| 124 | - | name: "no version", | |
| 125 | - | content: []byte(`! Title: No Version\n! Author: Test`), | |
| 126 | - | want: "", | |
| 127 | - | }, | |
| 128 | - | { | |
| 129 | - | name: "placeholder version ignored", | |
| 130 | - | content: []byte(`! Title: uBlock filters | |
| 131 | - | ! Version: %timestamp% | |
| 132 | - | ! Expires: 5 days`), | |
| 133 | - | want: "", | |
| 134 | - | }, | |
| 135 | - | { | |
| 136 | - | name: "empty content", | |
| 137 | - | content: []byte{}, | |
| 138 | - | want: "", | |
| 139 | - | }, | |
| 140 | - | { | |
| 141 | - | name: "version after 20 lines (not found)", | |
| 142 | - | content: []byte(`line1 | |
| 143 | - | line2 | |
| 144 | - | line3 | |
| 145 | - | line4 | |
| 146 | - | line5 | |
| 147 | - | line6 | |
| 148 | - | line7 | |
| 149 | - | line8 | |
| 150 | - | line9 | |
| 151 | - | line10 | |
| 152 | - | line11 | |
| 153 | - | line12 | |
| 154 | - | line13 | |
| 155 | - | line14 | |
| 156 | - | line15 | |
| 157 | - | line16 | |
| 158 | - | line17 | |
| 159 | - | line18 | |
| 160 | - | line19 | |
| 161 | - | line20 | |
| 162 | - | line21 | |
| 163 | - | ! Version: 202511291248`), | |
| 164 | - | want: "", | |
| 165 | - | }, | |
| 166 | - | } | |
| 167 | - | ||
| 168 | - | fu := &FilterUpdater{} | |
| 169 | - | ||
| 170 | - | for _, tt := range tests { | |
| 171 | - | t.Run(tt.name, func(t *testing.T) { | |
| 172 | - | got := fu.extractVersion(tt.content) | |
| 173 | - | assert.Equal(t, tt.want, got) | |
| 174 | - | }) | |
| 175 | - | } | |
| 176 | - | } | |
| 177 | - | ||
| 178 | 91 | func TestNeedsUpdate(t *testing.T) { | |
| 179 | 92 | tests := []struct { | |
| 180 | 93 | name string | |
| 181 | 94 | storedVersion string | |
| 182 | - | remoteVersion string | |
| 95 | + | remoteETag string | |
| 183 | 96 | serverError bool | |
| 184 | 97 | expectedResult bool | |
| 185 | 98 | }{ | |
| 186 | 99 | { | |
| 187 | - | name: "versions differ - needs update", | |
| 188 | - | storedVersion: "202511280000", | |
| 189 | - | remoteVersion: "202511291248", | |
| 100 | + | name: "etags differ - needs update", | |
| 101 | + | storedVersion: `"old-etag"`, | |
| 102 | + | remoteETag: `"new-etag"`, | |
| 190 | 103 | expectedResult: true, | |
| 191 | 104 | }, | |
| 192 | 105 | { | |
| 193 | - | name: "versions same - no update", | |
| 194 | - | storedVersion: "202511291248", | |
| 195 | - | remoteVersion: "202511291248", | |
| 106 | + | name: "etags same - no update", | |
| 107 | + | storedVersion: `"same-etag"`, | |
| 108 | + | remoteETag: `"same-etag"`, | |
| 196 | 109 | expectedResult: false, | |
| 197 | 110 | }, | |
| 198 | 111 | { | |
| 199 | 112 | name: "no stored version - needs update", | |
| 200 | 113 | storedVersion: "", | |
| 201 | - | remoteVersion: "202511291248", | |
| 114 | + | remoteETag: `"any-etag"`, | |
| 202 | 115 | expectedResult: true, | |
| 203 | 116 | }, | |
| 204 | 117 | { | |
| 205 | 118 | name: "server error - assume update needed", | |
| 206 | - | storedVersion: "202511280000", | |
| 119 | + | storedVersion: `"old-etag"`, | |
| 207 | 120 | serverError: true, | |
| 208 | 121 | expectedResult: true, | |
| 209 | 122 | }, | |
| @@ -211,32 +124,28 @@ func TestNeedsUpdate(t *testing.T) { | |||
| 211 | 124 | ||
| 212 | 125 | for _, tt := range tests { | |
| 213 | 126 | t.Run(tt.name, func(t *testing.T) { | |
| 214 | - | // Create test server | |
| 215 | 127 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 216 | 128 | if tt.serverError { | |
| 217 | 129 | w.WriteHeader(http.StatusInternalServerError) | |
| 218 | 130 | return | |
| 219 | 131 | } | |
| 132 | + | w.Header().Set("ETag", tt.remoteETag) | |
| 220 | 133 | w.WriteHeader(http.StatusOK) | |
| 221 | - | _, _ = w.Write([]byte("! Version: " + tt.remoteVersion + "\n")) | |
| 222 | 134 | })) | |
| 223 | 135 | defer server.Close() | |
| 224 | 136 | ||
| 225 | - | // Create mock store | |
| 226 | 137 | mockStore := &mockFilterStore{ | |
| 227 | 138 | sourceVersions: map[string]string{ | |
| 228 | 139 | server.URL: tt.storedVersion, | |
| 229 | 140 | }, | |
| 230 | 141 | } | |
| 231 | 142 | ||
| 232 | - | // Create manager and updater | |
| 233 | 143 | fm := &FilterManager{store: mockStore} | |
| 234 | 144 | fu := &FilterUpdater{ | |
| 235 | 145 | manager: fm, | |
| 236 | 146 | httpClient: &http.Client{Timeout: 5 * time.Second}, | |
| 237 | 147 | } | |
| 238 | 148 | ||
| 239 | - | // Test needsUpdate | |
| 240 | 149 | result := fu.needsUpdate(context.Background(), server.URL) | |
| 241 | 150 | assert.Equal(t, tt.expectedResult, result) | |
| 242 | 151 | }) | |
| @@ -14,53 +14,42 @@ import ( | |||||
| 14 | func TestFetchRemoteVersion(t *testing.T) { | 14 | func TestFetchRemoteVersion(t *testing.T) { | ||
| 15 | tests := []struct { | 15 | tests := []struct { | ||
| 16 | name string | 16 | name string | ||
| 17 | - | responseBody string | |||
| 18 | responseStatus int | 17 | responseStatus int | ||
| 18 | + | etag string | |||
| 19 | + | lastModified string | |||
| 19 | wantVersion string | 20 | wantVersion string | ||
| 20 | wantErr bool | 21 | wantErr bool | ||
| 21 | - | useETag bool | |||
| 22 | }{ | 22 | }{ | ||
| 23 | { | 23 | { | ||
| 24 | - | name: "valid version header", | 24 | + | name: "returns ETag when present", |
| 25 | - | responseBody: `[Adblock Plus 2.0] | |||
| 26 | - | ! Version: 202511291248 | |||
| 27 | - | ! Title: EasyList | |||
| 28 | - | ! Last modified: 29 Nov 2025 12:48 UTC | |||
| 29 | - | `, | |||
| 30 | responseStatus: http.StatusOK, | 25 | responseStatus: http.StatusOK, | ||
| 31 | - | wantVersion: "202511291248", | 26 | + | etag: `"abc123"`, |
| 27 | + | wantVersion: `"abc123"`, | |||
| 32 | wantErr: false, | 28 | wantErr: false, | ||
| 33 | }, | 29 | }, | ||
| 34 | { | 30 | { | ||
| 35 | - | name: "partial content response (206)", | 31 | + | name: "returns Last-Modified when no ETag", |
| 36 | - | responseBody: `[Adblock Plus 2.0] | 32 | + | responseStatus: http.StatusOK, |
| 37 | - | ! Version: 202501011200 | 33 | + | lastModified: "Sat, 29 Nov 2025 12:48:00 GMT", |
| 38 | - | ! Title: Test | 34 | + | wantVersion: "Sat, 29 Nov 2025 12:48:00 GMT", |
| 39 | - | `, | |||
| 40 | - | responseStatus: http.StatusPartialContent, | |||
| 41 | - | wantVersion: "202501011200", | |||
| 42 | wantErr: false, | 35 | wantErr: false, | ||
| 43 | }, | 36 | }, | ||
| 44 | { | 37 | { | ||
| 45 | - | name: "placeholder version falls back to ETag", | 38 | + | name: "prefers ETag over Last-Modified", |
| 46 | - | responseBody: `! Title: uBlock filters | |||
| 47 | - | ! Last modified: %timestamp% | |||
| 48 | - | `, | |||
| 49 | responseStatus: http.StatusOK, | 39 | responseStatus: http.StatusOK, | ||
| 50 | - | wantVersion: `etag:"abc123"`, | 40 | + | etag: `"xyz789"`, |
| 41 | + | lastModified: "Sat, 29 Nov 2025 12:48:00 GMT", | |||
| 42 | + | wantVersion: `"xyz789"`, | |||
| 51 | wantErr: false, | 43 | wantErr: false, | ||
| 52 | - | useETag: true, | |||
| 53 | }, | 44 | }, | ||
| 54 | { | 45 | { | ||
| 55 | - | name: "no version info returns error", | 46 | + | name: "error when no version headers", |
| 56 | - | responseBody: `[Adblock Plus 2.0]\n! Title: No Version\n`, | |||
| 57 | responseStatus: http.StatusOK, | 47 | responseStatus: http.StatusOK, | ||
| 58 | wantVersion: "", | 48 | wantVersion: "", | ||
| 59 | wantErr: true, | 49 | wantErr: true, | ||
| 60 | }, | 50 | }, | ||
| 61 | { | 51 | { | ||
| 62 | name: "server error", | 52 | name: "server error", | ||
| 63 | - | responseBody: "", | |||
| 64 | responseStatus: http.StatusInternalServerError, | 53 | responseStatus: http.StatusInternalServerError, | ||
| 65 | wantVersion: "", | 54 | wantVersion: "", | ||
| 66 | wantErr: true, | 55 | wantErr: true, | ||
| @@ -69,26 +58,24 @@ func TestFetchRemoteVersion(t *testing.T) { | |||||
| 69 | 58 | ||||
| 70 | for _, tt := range tests { | 59 | for _, tt := range tests { | ||
| 71 | t.Run(tt.name, func(t *testing.T) { | 60 | t.Run(tt.name, func(t *testing.T) { | ||
| 72 | - | // Create test server | |||
| 73 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 61 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| 74 | - | // Verify Range header is set | 62 | + | // Verify HEAD request |
| 75 | - | rangeHeader := r.Header.Get("Range") | 63 | + | assert.Equal(t, http.MethodHead, r.Method) |
| 76 | - | assert.Contains(t, rangeHeader, "bytes=0-") | |||
| 77 | 64 | ||||
| 78 | - | if tt.useETag { | 65 | + | if tt.etag != "" { |
| 79 | - | w.Header().Set("ETag", `"abc123"`) | 66 | + | w.Header().Set("ETag", tt.etag) |
| 67 | + | } | |||
| 68 | + | if tt.lastModified != "" { | |||
| 69 | + | w.Header().Set("Last-Modified", tt.lastModified) | |||
| 80 | } | 70 | } | ||
| 81 | w.WriteHeader(tt.responseStatus) | 71 | w.WriteHeader(tt.responseStatus) | ||
| 82 | - | _, _ = w.Write([]byte(tt.responseBody)) | |||
| 83 | })) | 72 | })) | ||
| 84 | defer server.Close() | 73 | defer server.Close() | ||
| 85 | 74 | ||||
| 86 | - | // Create updater with test server | |||
| 87 | fu := &FilterUpdater{ | 75 | fu := &FilterUpdater{ | ||
| 88 | httpClient: &http.Client{Timeout: 5 * time.Second}, | 76 | httpClient: &http.Client{Timeout: 5 * time.Second}, | ||
| 89 | } | 77 | } | ||
| 90 | 78 | ||||
| 91 | - | // Test fetchRemoteVersion | |||
| 92 | version, err := fu.fetchRemoteVersion(context.Background(), server.URL) | 79 | version, err := fu.fetchRemoteVersion(context.Background(), server.URL) | ||
| 93 | 80 | ||||
| 94 | if tt.wantErr { | 81 | if tt.wantErr { | ||
| @@ -101,109 +88,35 @@ func TestFetchRemoteVersion(t *testing.T) { | |||||
| 101 | } | 88 | } | ||
| 102 | } | 89 | } | ||
| 103 | 90 | ||||
| 104 | - | func TestExtractVersion(t *testing.T) { | |||
| 105 | - | tests := []struct { | |||
| 106 | - | name string | |||
| 107 | - | content []byte | |||
| 108 | - | want string | |||
| 109 | - | }{ | |||
| 110 | - | { | |||
| 111 | - | name: "standard format", | |||
| 112 | - | content: []byte(`[Adblock Plus 2.0] | |||
| 113 | - | ! Version: 202511291248 | |||
| 114 | - | ! Title: EasyList`), | |||
| 115 | - | want: "202511291248", | |||
| 116 | - | }, | |||
| 117 | - | { | |||
| 118 | - | name: "version with spaces", | |||
| 119 | - | content: []byte(`! Version: 20251129 | |||
| 120 | - | ! Title: Test`), | |||
| 121 | - | want: "20251129", | |||
| 122 | - | }, | |||
| 123 | - | { | |||
| 124 | - | name: "no version", | |||
| 125 | - | content: []byte(`! Title: No Version\n! Author: Test`), | |||
| 126 | - | want: "", | |||
| 127 | - | }, | |||
| 128 | - | { | |||
| 129 | - | name: "placeholder version ignored", | |||
| 130 | - | content: []byte(`! Title: uBlock filters | |||
| 131 | - | ! Version: %timestamp% | |||
| 132 | - | ! Expires: 5 days`), | |||
| 133 | - | want: "", | |||
| 134 | - | }, | |||
| 135 | - | { | |||
| 136 | - | name: "empty content", | |||
| 137 | - | content: []byte{}, | |||
| 138 | - | want: "", | |||
| 139 | - | }, | |||
| 140 | - | { | |||
| 141 | - | name: "version after 20 lines (not found)", | |||
| 142 | - | content: []byte(`line1 | |||
| 143 | - | line2 | |||
| 144 | - | line3 | |||
| 145 | - | line4 | |||
| 146 | - | line5 | |||
| 147 | - | line6 | |||
| 148 | - | line7 | |||
| 149 | - | line8 | |||
| 150 | - | line9 | |||
| 151 | - | line10 | |||
| 152 | - | line11 | |||
| 153 | - | line12 | |||
| 154 | - | line13 | |||
| 155 | - | line14 | |||
| 156 | - | line15 | |||
| 157 | - | line16 | |||
| 158 | - | line17 | |||
| 159 | - | line18 | |||
| 160 | - | line19 | |||
| 161 | - | line20 | |||
| 162 | - | line21 | |||
| 163 | - | ! Version: 202511291248`), | |||
| 164 | - | want: "", | |||
| 165 | - | }, | |||
| 166 | - | } | |||
| 167 | - | ||||
| 168 | - | fu := &FilterUpdater{} | |||
| 169 | - | ||||
| 170 | - | for _, tt := range tests { | |||
| 171 | - | t.Run(tt.name, func(t *testing.T) { | |||
| 172 | - | got := fu.extractVersion(tt.content) | |||
| 173 | - | assert.Equal(t, tt.want, got) | |||
| 174 | - | }) | |||
| 175 | - | } | |||
| 176 | - | } | |||
| 177 | - | ||||
| 178 | func TestNeedsUpdate(t *testing.T) { | 91 | func TestNeedsUpdate(t *testing.T) { | ||
| 179 | tests := []struct { | 92 | tests := []struct { | ||
| 180 | name string | 93 | name string | ||
| 181 | storedVersion string | 94 | storedVersion string | ||
| 182 | - | remoteVersion string | 95 | + | remoteETag string |
| 183 | serverError bool | 96 | serverError bool | ||
| 184 | expectedResult bool | 97 | expectedResult bool | ||
| 185 | }{ | 98 | }{ | ||
| 186 | { | 99 | { | ||
| 187 | - | name: "versions differ - needs update", | 100 | + | name: "etags differ - needs update", |
| 188 | - | storedVersion: "202511280000", | 101 | + | storedVersion: `"old-etag"`, |
| 189 | - | remoteVersion: "202511291248", | 102 | + | remoteETag: `"new-etag"`, |
| 190 | expectedResult: true, | 103 | expectedResult: true, | ||
| 191 | }, | 104 | }, | ||
| 192 | { | 105 | { | ||
| 193 | - | name: "versions same - no update", | 106 | + | name: "etags same - no update", |
| 194 | - | storedVersion: "202511291248", | 107 | + | storedVersion: `"same-etag"`, |
| 195 | - | remoteVersion: "202511291248", | 108 | + | remoteETag: `"same-etag"`, |
| 196 | expectedResult: false, | 109 | expectedResult: false, | ||
| 197 | }, | 110 | }, | ||
| 198 | { | 111 | { | ||
| 199 | name: "no stored version - needs update", | 112 | name: "no stored version - needs update", | ||
| 200 | storedVersion: "", | 113 | storedVersion: "", | ||
| 201 | - | remoteVersion: "202511291248", | 114 | + | remoteETag: `"any-etag"`, |
| 202 | expectedResult: true, | 115 | expectedResult: true, | ||
| 203 | }, | 116 | }, | ||
| 204 | { | 117 | { | ||
| 205 | name: "server error - assume update needed", | 118 | name: "server error - assume update needed", | ||
| 206 | - | storedVersion: "202511280000", | 119 | + | storedVersion: `"old-etag"`, |
| 207 | serverError: true, | 120 | serverError: true, | ||
| 208 | expectedResult: true, | 121 | expectedResult: true, | ||
| 209 | }, | 122 | }, | ||
| @@ -211,32 +124,28 @@ func TestNeedsUpdate(t *testing.T) { | |||||
| 211 | 124 | ||||
| 212 | for _, tt := range tests { | 125 | for _, tt := range tests { | ||
| 213 | t.Run(tt.name, func(t *testing.T) { | 126 | t.Run(tt.name, func(t *testing.T) { | ||
| 214 | - | // Create test server | |||
| 215 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 127 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| 216 | if tt.serverError { | 128 | if tt.serverError { | ||
| 217 | w.WriteHeader(http.StatusInternalServerError) | 129 | w.WriteHeader(http.StatusInternalServerError) | ||
| 218 | return | 130 | return | ||
| 219 | } | 131 | } | ||
| 132 | + | w.Header().Set("ETag", tt.remoteETag) | |||
| 220 | w.WriteHeader(http.StatusOK) | 133 | w.WriteHeader(http.StatusOK) | ||
| 221 | - | _, _ = w.Write([]byte("! Version: " + tt.remoteVersion + "\n")) | |||
| 222 | })) | 134 | })) | ||
| 223 | defer server.Close() | 135 | defer server.Close() | ||
| 224 | 136 | ||||
| 225 | - | // Create mock store | |||
| 226 | mockStore := &mockFilterStore{ | 137 | mockStore := &mockFilterStore{ | ||
| 227 | sourceVersions: map[string]string{ | 138 | sourceVersions: map[string]string{ | ||
| 228 | server.URL: tt.storedVersion, | 139 | server.URL: tt.storedVersion, | ||
| 229 | }, | 140 | }, | ||
| 230 | } | 141 | } | ||
| 231 | 142 | ||||
| 232 | - | // Create manager and updater | |||
| 233 | fm := &FilterManager{store: mockStore} | 143 | fm := &FilterManager{store: mockStore} | ||
| 234 | fu := &FilterUpdater{ | 144 | fu := &FilterUpdater{ | ||
| 235 | manager: fm, | 145 | manager: fm, | ||
| 236 | httpClient: &http.Client{Timeout: 5 * time.Second}, | 146 | httpClient: &http.Client{Timeout: 5 * time.Second}, | ||
| 237 | } | 147 | } | ||
| 238 | 148 | ||||
| 239 | - | // Test needsUpdate | |||
| 240 | result := fu.needsUpdate(context.Background(), server.URL) | 149 | result := fu.needsUpdate(context.Background(), server.URL) | ||
| 241 | assert.Equal(t, tt.expectedResult, result) | 150 | assert.Equal(t, tt.expectedResult, result) | ||
| 242 | }) | 151 | }) | ||