diff --git a/pkg/appview/handlers/readme_resolve_test.go b/pkg/appview/handlers/readme_resolve_test.go new file mode 100644 index 0000000..f017106 --- /dev/null +++ b/pkg/appview/handlers/readme_resolve_test.go @@ -0,0 +1,103 @@ +package handlers + +import ( + "context" + "errors" + "testing" +) + +// stubReadmeSource is a readmeSource that returns canned results. +type stubReadmeSource struct { + fetchBody []byte + fetchErr error + renderErr error +} + +func (s *stubReadmeSource) FetchRaw(_ context.Context, _ string) ([]byte, error) { + if s.fetchErr != nil { + return nil, s.fetchErr + } + return s.fetchBody, nil +} + +func (s *stubReadmeSource) RenderMarkdown(content []byte) (string, error) { + if s.renderErr != nil { + return "", s.renderErr + } + return "
" + string(content) + "
", nil +} + +func TestResolveReadme(t *testing.T) { + notFound := errors.New("404 Not Found") + + tests := []struct { + name string + src *stubReadmeSource + explicit bool + wantHTML string + wantRaw string + wantFailFlag bool + }{ + { + // The image inherited org.opencontainers.image.source from its base + // image, so the derived raw URL points at an unrelated repo and + // 404s. That is a guess of ours failing, not the owner's problem: + // render as if there were simply no README. + name: "derived URL fetch failure is silent", + src: &stubReadmeSource{fetchErr: notFound}, + explicit: false, + wantFailFlag: false, + }, + { + // The owner set io.atcr.readme themselves, so a broken source is + // actionable and worth surfacing. + name: "explicit URL fetch failure surfaces", + src: &stubReadmeSource{fetchErr: notFound}, + explicit: true, + wantFailFlag: true, + }, + { + name: "derived URL render failure is silent", + src: &stubReadmeSource{fetchBody: []byte("# hi"), renderErr: errors.New("boom")}, + explicit: false, + wantRaw: "# hi", + wantFailFlag: false, + }, + { + name: "explicit URL render failure surfaces", + src: &stubReadmeSource{fetchBody: []byte("# hi"), renderErr: errors.New("boom")}, + explicit: true, + wantRaw: "# hi", + wantFailFlag: true, + }, + { + name: "derived URL success renders", + src: &stubReadmeSource{fetchBody: []byte("hello")}, + explicit: false, + wantHTML: "hello
", + wantRaw: "hello", + }, + { + name: "explicit URL success renders", + src: &stubReadmeSource{fetchBody: []byte("hello")}, + explicit: true, + wantHTML: "hello
", + wantRaw: "hello", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + html, raw, failed := resolveReadme(context.Background(), tt.src, "https://example.com/README.md", tt.explicit) + if string(html) != tt.wantHTML { + t.Errorf("html = %q, want %q", html, tt.wantHTML) + } + if raw != tt.wantRaw { + t.Errorf("raw = %q, want %q", raw, tt.wantRaw) + } + if failed != tt.wantFailFlag { + t.Errorf("fetchFailed = %v, want %v", failed, tt.wantFailFlag) + } + }) + } +} diff --git a/pkg/appview/handlers/repository.go b/pkg/appview/handlers/repository.go index 69c4941..69fd7bc 100644 --- a/pkg/appview/handlers/repository.go +++ b/pkg/appview/handlers/repository.go @@ -20,6 +20,43 @@ import ( "github.com/go-chi/chi/v5" ) +// readmeSource is the subset of readme.Fetcher that resolveReadme needs, so +// the resolution logic can be exercised without a live HTTP fetcher. +type readmeSource interface { + FetchRaw(ctx context.Context, readmeURL string) ([]byte, error) + RenderMarkdown(content []byte) (string, error) +} + +// resolveReadme fetches and renders the README at readmeURL. +// +// explicit reports whether the owner actually pointed us at that URL (the +// io.atcr.readme annotation) or whether we derived it from +// org.opencontainers.image.source. Derived URLs are guesses: images routinely +// inherit the source label from their base image, so the raw URL often names +// an unrelated project and simply 404s. That is an expected outcome the owner +// cannot act on, so a derived URL never reports failure to the UI, it renders +// as if no README existed. Failures are still logged either way. +// +// Render failures are treated the same as fetch failures: content that came +// back from a derived URL is not necessarily the owner's README, so an error +// panel telling them their configured source is broken would be wrong. The +// log line stays for diagnosis. +func resolveReadme(ctx context.Context, fetcher readmeSource, readmeURL string, explicit bool) (rendered template.HTML, raw string, fetchFailed bool) { + rawBytes, fetchErr := fetcher.FetchRaw(ctx, readmeURL) + if fetchErr != nil { + slog.Debug("Failed to fetch README from URL", "url", readmeURL, "explicit", explicit, "error", fetchErr) + return "", "", explicit + } + + html, renderErr := fetcher.RenderMarkdown(rawBytes) + if renderErr != nil { + slog.Debug("Failed to render fetched README", "url", readmeURL, "explicit", explicit, "error", renderErr) + return "", string(rawBytes), explicit + } + + return template.HTML(html), string(rawBytes), false +} + // SelectedTagData holds all data for the currently selected tag on the repo page. type SelectedTagData struct { Info *db.TagWithPlatforms @@ -226,7 +263,8 @@ func (h *RepositoryPageHandler) ServeHTTP(w http.ResponseWriter, r *http.Request // Fetch README content from repo page record or annotations. // ReadmeFetchFailed distinguishes "owner never provided a README" (show // CTA to add one) from "we tried to fetch the configured README and it - // failed" (show retry CTA instead). + // failed" (show retry CTA instead). Only an explicitly configured README + // URL can set it; see resolveReadme. var readmeHTML template.HTML var rawDescription string var readmeFetchFailed bool @@ -248,6 +286,10 @@ func (h *RepositoryPageHandler) ServeHTTP(w http.ResponseWriter, r *http.Request } if readmeHTML == "" && h.ReadmeFetcher != nil { readmeURL := repo.ReadmeURL + // A URL from the io.atcr.readme annotation was set deliberately by the + // image author; anything derived from org.opencontainers.image.source + // is a guess on our part. + explicit := readmeURL != "" if readmeURL == "" && repo.SourceURL != "" { readmeURL = readme.DeriveReadmeURL(repo.SourceURL, "main") if readmeURL == "" { @@ -255,20 +297,12 @@ func (h *RepositoryPageHandler) ServeHTTP(w http.ResponseWriter, r *http.Request } } if readmeURL != "" { - rawBytes, fetchErr := h.ReadmeFetcher.FetchRaw(r.Context(), readmeURL) - if fetchErr != nil { - slog.Debug("Failed to fetch README from URL", "url", readmeURL, "error", fetchErr) - readmeFetchFailed = true - } else { - rawDescription = string(rawBytes) - html, renderErr := h.ReadmeFetcher.RenderMarkdown(rawBytes) - if renderErr != nil { - slog.Debug("Failed to render fetched README", "url", readmeURL, "error", renderErr) - readmeFetchFailed = true - } else { - readmeHTML = template.HTML(html) - } + html, raw, failed := resolveReadme(r.Context(), h.ReadmeFetcher, readmeURL, explicit) + readmeHTML = html + if raw != "" { + rawDescription = raw } + readmeFetchFailed = failed } }