diff --git a/internal/handler/download_test.go b/internal/handler/download_test.go index 980e234..c9d4918 100644 --- a/internal/handler/download_test.go +++ b/internal/handler/download_test.go @@ -943,64 +943,66 @@ func TestMavenHandler_GradlePluginMarkerMetadataFallback(t *testing.T) { } } -func TestMavenHandler_GradlePluginImplementationMetadataFallback(t *testing.T) { - paths := map[string]string{ - "/com/diffplug/spotless/spotless-plugin-gradle/8.4.0/spotless-plugin-gradle-8.4.0.jar.sha1": "impl-sha1", - "/com/diffplug/spotless/spotless-plugin-gradle/8.4.0/spotless-plugin-gradle-8.4.0.jar.sha256": "impl-sha256", - } +func TestMavenHandler_GradlePluginMarkerMetadataFallback_ForwardsConditionalHeadersWithoutCache(t *testing.T) { + const ( + requestPath = "/com/diffplug/spotless/com.diffplug.spotless.gradle.plugin/maven-metadata.xml" + etagValue = `"marker-etag"` + ) - primaryHits := map[string]int{} - pluginHits := map[string]int{} + primaryHits := 0 + pluginHits := 0 primary := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - primaryHits[r.URL.Path]++ - if _, ok := paths[r.URL.Path]; ok { - http.NotFound(w, r) - return + primaryHits++ + if r.URL.Path != requestPath { + t.Fatalf("unexpected path to primary upstream: %s", r.URL.Path) } - t.Fatalf("unexpected path to primary upstream: %s", r.URL.Path) + http.NotFound(w, r) })) defer primary.Close() pluginPortal := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - pluginHits[r.URL.Path]++ - body, ok := paths[r.URL.Path] - if !ok { - http.NotFound(w, r) + pluginHits++ + if r.URL.Path != requestPath { + t.Fatalf("unexpected path to plugin portal: %s", r.URL.Path) + } + if got := r.Header.Get("If-None-Match"); got == etagValue { + w.WriteHeader(http.StatusNotModified) return } - _, _ = io.WriteString(w, body) + + w.Header().Set("ETag", etagValue) + _, _ = io.WriteString(w, "") })) defer pluginPortal.Close() proxy, _, _, _ := setupTestProxy(t) - proxy.HTTPClient = primary.Client() + proxy.CacheMetadata = false h := NewMavenHandler(proxy, "http://localhost", primary.URL, pluginPortal.URL) srv := httptest.NewServer(h.Routes()) defer srv.Close() - for reqPath, wantBody := range paths { - resp, err := http.Get(srv.URL + reqPath) - if err != nil { - t.Fatalf("GET %s failed: %v", reqPath, err) - } - body, _ := io.ReadAll(resp.Body) - _ = resp.Body.Close() + req, err := http.NewRequest(http.MethodGet, srv.URL+requestPath, nil) + if err != nil { + t.Fatalf("failed to build request: %v", err) + } + req.Header.Set("If-None-Match", etagValue) - if resp.StatusCode != http.StatusOK { - t.Fatalf("GET %s: status = %d, want %d", reqPath, resp.StatusCode, http.StatusOK) - } - if string(body) != wantBody { - t.Fatalf("GET %s: body = %q, want %q", reqPath, body, wantBody) - } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("request failed: %v", err) + } + _ = resp.Body.Close() - if primaryHits[reqPath] == 0 { - t.Fatalf("GET %s did not hit primary upstream", reqPath) - } - if pluginHits[reqPath] == 0 { - t.Fatalf("GET %s did not hit plugin portal fallback", reqPath) - } + if resp.StatusCode != http.StatusNotModified { + t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusNotModified) + } + if primaryHits != 1 { + t.Fatalf("primary hits = %d, want 1", primaryHits) + } + if pluginHits != 1 { + t.Fatalf("plugin portal hits = %d, want 1", pluginHits) } } diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 3df2777..badd8ab 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -721,11 +721,47 @@ func (p *Proxy) writeMetadataCachedResponse(w http.ResponseWriter, r *http.Reque // proxyMetadataStream forwards an upstream metadata response by streaming it to the client // without buffering the full body in memory. func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upstreamURL string, acceptHeaders ...string) { - req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, upstreamURL, nil) + resp, err := p.fetchMetadataStreamResponse(r, upstreamURL, acceptHeaders...) if err != nil { - http.Error(w, "failed to create request", http.StatusInternalServerError) + http.Error(w, "failed to fetch from upstream", http.StatusBadGateway) return } + defer func() { _ = resp.Body.Close() }() + + p.writeMetadataStreamResponse(w, resp) +} + +// proxyMetadataStreamWithFallback streams metadata from the primary upstream and +// retries against fallbackURL only when the primary returns 404 Not Found. +func (p *Proxy) proxyMetadataStreamWithFallback(w http.ResponseWriter, r *http.Request, primaryURL, fallbackURL string, acceptHeaders ...string) { + resp, err := p.fetchMetadataStreamResponse(r, primaryURL, acceptHeaders...) + if err != nil { + http.Error(w, "failed to fetch from upstream", http.StatusBadGateway) + return + } + + if resp.StatusCode != http.StatusNotFound || fallbackURL == "" { + defer func() { _ = resp.Body.Close() }() + p.writeMetadataStreamResponse(w, resp) + return + } + _ = resp.Body.Close() + + fallbackResp, fallbackErr := p.fetchMetadataStreamResponse(r, fallbackURL, acceptHeaders...) + if fallbackErr != nil { + http.Error(w, "failed to fetch from upstream", http.StatusBadGateway) + return + } + defer func() { _ = fallbackResp.Body.Close() }() + + p.writeMetadataStreamResponse(w, fallbackResp) +} + +func (p *Proxy) fetchMetadataStreamResponse(r *http.Request, upstreamURL string, acceptHeaders ...string) (*http.Response, error) { + req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, upstreamURL, nil) + if err != nil { + return nil, err + } accept := contentTypeJSON if len(acceptHeaders) > 0 && acceptHeaders[0] != "" { @@ -739,12 +775,10 @@ func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upst } } - resp, err := p.HTTPClient.Do(req) - if err != nil { - http.Error(w, "failed to fetch from upstream", http.StatusBadGateway) - return - } - defer func() { _ = resp.Body.Close() }() + return p.HTTPClient.Do(req) +} + +func (p *Proxy) writeMetadataStreamResponse(w http.ResponseWriter, resp *http.Response) { for _, header := range []string{"Content-Type", "Content-Length", "Last-Modified", "ETag"} { if v := resp.Header.Get(header); v != "" { diff --git a/internal/handler/handler_test.go b/internal/handler/handler_test.go index bbcab72..cbea214 100644 --- a/internal/handler/handler_test.go +++ b/internal/handler/handler_test.go @@ -127,15 +127,15 @@ func (f *mockFetcher) Head(_ context.Context, _ string) (int64, string, error) { } // setupTestProxy creates a Proxy with a real DB (SQLite in temp dir) and mock storage/fetcher. -func setupTestProxy(t *testing.T) (*Proxy, *database.DB, *mockStorage, *mockFetcher) { - t.Helper() +func setupTestProxy(tb testing.TB) (*Proxy, *database.DB, *mockStorage, *mockFetcher) { + tb.Helper() - dir := t.TempDir() + dir := tb.TempDir() db, err := database.Create(dir + "/test.db") if err != nil { - t.Fatalf("failed to create test database: %v", err) + tb.Fatalf("failed to create test database: %v", err) } - t.Cleanup(func() { _ = db.Close() }) + tb.Cleanup(func() { _ = db.Close() }) store := newMockStorage() fetcher := &mockFetcher{} diff --git a/internal/handler/maven.go b/internal/handler/maven.go index c423645..d79a904 100644 --- a/internal/handler/maven.go +++ b/internal/handler/maven.go @@ -12,6 +12,7 @@ const ( mavenCentralUpstream = "https://repo1.maven.org/maven2" gradlePluginPortalUpstream = "https://plugins.gradle.org/m2" minMavenParts = 4 // group path segments + artifact + version + filename + gradlePluginMarkerSuffix = ".gradle.plugin" ) // MavenHandler handles Maven repository protocol requests. @@ -62,7 +63,7 @@ func (h *MavenHandler) handleRequest(w http.ResponseWriter, r *http.Request) { filename := path.Base(urlPath) if h.isMetadataFile(filename) { - h.handleMetadata(w, r, urlPath) + h.handleMetadata(w, r, urlPath, filename) return } @@ -76,16 +77,26 @@ func (h *MavenHandler) handleRequest(w http.ResponseWriter, r *http.Request) { h.proxyUpstream(w, r) } -func (h *MavenHandler) handleMetadata(w http.ResponseWriter, r *http.Request, urlPath string) { +func (h *MavenHandler) handleMetadata(w http.ResponseWriter, r *http.Request, urlPath, filename string) { cacheKey := strings.ReplaceAll(urlPath, "/", "_") + if !h.shouldFallbackToPluginPortal(urlPath, filename) { + h.proxy.ProxyCached(w, r, h.upstreamURL+r.URL.Path, "maven", cacheKey, "*/*") + return + } + upstreamURL := fmt.Sprintf("%s/%s", h.upstreamURL, urlPath) + pluginPortalURL := fmt.Sprintf("%s/%s", h.pluginPortalUpstreamURL, urlPath) + if !h.proxy.CacheMetadata { + h.proxy.proxyMetadataStreamWithFallback(w, r, upstreamURL, pluginPortalURL, "*/*") + return + } body, contentType, err := h.proxy.FetchOrCacheMetadata(r.Context(), "maven", cacheKey, upstreamURL, "*/*") if err != nil { if errors.Is(err, ErrUpstreamNotFound) { pluginPortalURL := fmt.Sprintf("%s/%s", h.pluginPortalUpstreamURL, urlPath) h.proxy.Logger.Info("maven metadata unavailable in primary upstream, trying Gradle Plugin Portal", - "path", urlPath) + "path", urlPath, "filename", filename) body, contentType, err = h.proxy.FetchOrCacheMetadata(r.Context(), "maven", cacheKey, pluginPortalURL, "*/*") } } @@ -172,6 +183,67 @@ func (h *MavenHandler) isArtifactFile(filename string) bool { return false } +func (h *MavenHandler) shouldFallbackToPluginPortal(urlPath, filename string) bool { + if !h.isPluginPortalFallbackFile(filename) { + return false + } + + group, artifact, ok := h.extractGroupAndArtifactFromPath(urlPath, filename) + if !ok { + return false + } + + if !strings.HasSuffix(artifact, gradlePluginMarkerSuffix) { + return false + } + + markerPrefix := strings.TrimSuffix(artifact, gradlePluginMarkerSuffix) + return markerPrefix != "" && markerPrefix == group +} + +func (h *MavenHandler) isPluginPortalFallbackFile(filename string) bool { + if filename == "maven-metadata.xml" || strings.HasPrefix(filename, "maven-metadata.xml.") { + return true + } + if strings.HasSuffix(filename, ".pom") || strings.HasSuffix(filename, ".module") { + return true + } + + for _, suffix := range []string{".sha1", ".sha256", ".sha512", ".md5", ".asc"} { + if !strings.HasSuffix(filename, suffix) { + continue + } + + base := strings.TrimSuffix(filename, suffix) + return strings.HasSuffix(base, ".pom") || strings.HasSuffix(base, ".module") + } + + return false +} + +func (h *MavenHandler) extractGroupAndArtifactFromPath(urlPath, filename string) (group, artifact string, ok bool) { + const ( + artifactVersionPathOffset = 3 // .../{artifact}/{version}/{filename} + metadataPathOffset = 2 // .../{artifact}/maven-metadata.xml + ) + + parts := strings.Split(urlPath, "/") + + pathOffset := artifactVersionPathOffset + if filename == "maven-metadata.xml" || strings.HasPrefix(filename, "maven-metadata.xml.") { + pathOffset = metadataPathOffset + } + segmentIdx := len(parts) - pathOffset + + if segmentIdx <= 0 || segmentIdx >= len(parts) { + return "", "", false + } + + group = strings.Join(parts[:segmentIdx], ".") + artifact = parts[segmentIdx] + return group, artifact, group != "" && artifact != "" +} + // isMetadataFile returns true if the filename is Maven metadata. func (h *MavenHandler) isMetadataFile(filename string) bool { return filename == "maven-metadata.xml" || diff --git a/internal/handler/maven_test.go b/internal/handler/maven_test.go index 9ca5eb6..38b7bfd 100644 --- a/internal/handler/maven_test.go +++ b/internal/handler/maven_test.go @@ -67,61 +67,64 @@ func TestMavenIsArtifactFile(t *testing.T) { } } -func TestMavenIsMetadataFile(t *testing.T) { +func TestMavenShouldFallbackToPluginPortal(t *testing.T) { h := &MavenHandler{} tests := []struct { name string + urlPath string filename string want bool }{ { - name: "pom is artifact, not metadata", + name: "marker pom", + urlPath: "com/diffplug/spotless/com.diffplug.spotless.gradle.plugin/8.4.0/com.diffplug.spotless.gradle.plugin-8.4.0.pom", filename: "com.diffplug.spotless.gradle.plugin-8.4.0.pom", - want: false, + want: true, }, { - name: "pom checksum is metadata", + name: "marker pom checksum", + urlPath: "com/diffplug/spotless/com.diffplug.spotless.gradle.plugin/8.4.0/com.diffplug.spotless.gradle.plugin-8.4.0.pom.sha1", filename: "com.diffplug.spotless.gradle.plugin-8.4.0.pom.sha1", want: true, }, { - name: "metadata file", + name: "marker metadata", + urlPath: "com/diffplug/spotless/com.diffplug.spotless.gradle.plugin/maven-metadata.xml", filename: "maven-metadata.xml", want: true, }, { - name: "metadata checksum", + name: "marker metadata checksum", + urlPath: "com/diffplug/spotless/com.diffplug.spotless.gradle.plugin/maven-metadata.xml.sha256", filename: "maven-metadata.xml.sha256", want: true, }, { - name: "jar checksum is metadata", - filename: "guava-32.1.3-jre.jar.sha1", - want: true, - }, - { - name: "asc signature is metadata", - filename: "guava-32.1.3-jre.jar.asc", - want: true, + name: "non marker pom checksum", + urlPath: "com/google/guava/guava/32.1.3-jre/guava-32.1.3-jre.pom.sha1", + filename: "guava-32.1.3-jre.pom.sha1", + want: false, }, { - name: "regular jar is not metadata", - filename: "guava-32.1.3-jre.jar", + name: "jar checksum", + urlPath: "com/diffplug/spotless/com.diffplug.spotless.gradle.plugin/8.4.0/com.diffplug.spotless.gradle.plugin-8.4.0.jar.sha1", + filename: "com.diffplug.spotless.gradle.plugin-8.4.0.jar.sha1", want: false, }, { - name: "pom checksum is metadata", - filename: "guava-32.1.3-jre.pom.sha1", - want: true, + name: "path too short", + urlPath: "com.diffplug.spotless.gradle.plugin/maven-metadata.xml", + filename: "maven-metadata.xml", + want: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := h.isMetadataFile(tt.filename) + got := h.shouldFallbackToPluginPortal(tt.urlPath, tt.filename) if got != tt.want { - t.Errorf("isMetadataFile(%q) = %v, want %v", tt.filename, got, tt.want) + t.Errorf("shouldFallbackToPluginPortal(%q, %q) = %v, want %v", tt.urlPath, tt.filename, got, tt.want) } }) }