internal/frontend: show module readme links in sub-units

Show a module's README links on all units within the module.

For golang/go#42968

Change-Id: I8701a04a16e1dd766b1b23ce12d52d01190840fd
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/275276
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>

8 files changed, 177 insertions, 58 deletions

diff --git a/content/static/html/helpers/_unit_meta.tmpl b/content/static/html/helpers/_unit_meta.tmpl
index 21ed4287..3ddc1b92 100644
--- a/content/static/html/helpers/_unit_meta.tmpl
+++ b/content/static/html/helpers/_unit_meta.tmpl
@@ -15,13 +15,15 @@
     {{else}}
       Repository URL not available.
     {{end}}
-    {{range .Details.ReadmeLinks}}
-      <div class="UnitMeta-repo">{{.Body}}</div>
-      <a href="{{.Href}}" title="{{.Href}}" target="_blank" rel="noopener">{{.Href}}</a>
-    {{end}}
-    {{range .Details.DocLinks}}
-      <div class="UnitMeta-repo">{{.Body}}</div>
-      <a href="{{.Href}}" title="{{.Href}}" target="_blank" rel="noopener">{{.Href}}</a>
-    {{end}}
+    {{template "unit_meta_links" .Details.ReadmeLinks}}
+    {{template "unit_meta_links" .Details.DocLinks}}
+    {{template "unit_meta_links" .Details.ModuleReadmeLinks}}
   </div>
 {{end}}
+
+{{define "unit_meta_links"}}
+  {{range .}}
+    <div class="UnitMeta-repo">{{.Body}}</div>
+    <a href="{{.Href}}" title="{{.Href}}" target="_blank" rel="noopener">{{.Href}}</a>
+  {{end}}
+{{end}}
\ No newline at end of file
diff --git a/internal/datasource.go b/internal/datasource.go
index 0747af4c..6f886c17 100644
--- a/internal/datasource.go
+++ b/internal/datasource.go
@@ -21,4 +21,6 @@ type DataSource interface {
 	GetUnit(ctx context.Context, pathInfo *UnitMeta, fields FieldSet) (_ *Unit, err error)
 	// GetUnitMeta returns information about a path.
 	GetUnitMeta(ctx context.Context, path, requestedModulePath, requestedVersion string) (_ *UnitMeta, err error)
+	// GetModuleReadme gets the readme for the module.
+	GetModuleReadme(ctx context.Context, modulePath, resolvedVersion string) (*Readme, error)
 }
diff --git a/internal/frontend/readme.go b/internal/frontend/readme.go
index 09b1de8c..b2871e06 100644
--- a/internal/frontend/readme.go
+++ b/internal/frontend/readme.go
@@ -24,6 +24,7 @@ import (
 	"github.com/yuin/goldmark/util"
 	"golang.org/x/pkgsite/internal"
 	"golang.org/x/pkgsite/internal/derrors"
+	"golang.org/x/pkgsite/internal/source"
 )
 
 // Heading holds data about a heading within a readme used in the
@@ -61,12 +62,16 @@ type Readme struct {
 // This function is exported for use by external tools.
 func ProcessReadme(ctx context.Context, u *internal.Unit) (_ *Readme, err error) {
 	defer derrors.Wrap(&err, "ProcessReadme(%q, %q, %q)", u.Path, u.ModulePath, u.Version)
-	if u.Readme == nil || u.Readme.Contents == "" {
+	return processReadme(u.Readme, u.SourceInfo)
+}
+
+func processReadme(readme *internal.Readme, sourceInfo *source.Info) (_ *Readme, err error) {
+	if readme == nil || readme.Contents == "" {
 		return &Readme{}, nil
 	}
-	if !isMarkdown(u.Readme.Filepath) {
+	if !isMarkdown(readme.Filepath) {
 		t := template.Must(template.New("").Parse(`<pre class="readme">{{.}}</pre>`))
-		h, err := t.ExecuteToHTML(u.Readme.Contents)
+		h, err := t.ExecuteToHTML(readme.Contents)
 		if err != nil {
 			return nil, err
 		}
@@ -93,8 +98,8 @@ func ProcessReadme(ctx context.Context, u *internal.Unit) (_ *Readme, err error)
 			// before it is rendered.
 			parser.WithASTTransformers(
 				util.Prioritized(&astTransformer{
-					info:   u.SourceInfo,
-					readme: u.Readme,
+					info:   sourceInfo,
+					readme: readme,
 				}, astTransformerPriority),
 				// Extract links after we have transformed the URLs.
 				util.Prioritized(el, astTransformerPriority+1),
@@ -110,10 +115,10 @@ func ProcessReadme(ctx context.Context, u *internal.Unit) (_ *Readme, err error)
 	)
 	gdMarkdown.Renderer().AddOptions(
 		renderer.WithNodeRenderers(
-			util.Prioritized(newHTMLRenderer(u.SourceInfo, u.Readme), 100),
+			util.Prioritized(newHTMLRenderer(sourceInfo, readme), 100),
 		),
 	)
-	contents := []byte(u.Readme.Contents)
+	contents := []byte(readme.Contents)
 	gdParser := gdMarkdown.Parser()
 	reader := gmtext.NewReader(contents)
 	pctx := parser.NewContext(parser.WithIDs(newIDs()))
diff --git a/internal/frontend/server_test.go b/internal/frontend/server_test.go
index 6af263ce..707d1e90 100644
--- a/internal/frontend/server_test.go
+++ b/internal/frontend/server_test.go
@@ -81,7 +81,7 @@ type serverTestCase struct {
 // Units with this prefix will be marked as excluded.
 const excludedModulePath = "github.com/module/excluded"
 
-const linksReadme = `
+const moduleLinksReadme = `
 # Heading
 Stuff
 
@@ -90,6 +90,11 @@ Stuff
 - [title2](javascript://pwned)
 `
 
+const packageLinksReadme = `
+# Links
+- [pkg title](http://url2)
+`
+
 var testModules = []testModule{
 	{
 		// An ordinary module, with three versions.
@@ -160,15 +165,29 @@ var testModules = []testModule{
 	},
 	{
 		// A module with links.
-		path:            "github.com/links",
+		path:            "github.com/links/mod",
 		redistributable: true,
 		versions:        []string{"v1.0.0"},
 		packages: []testPackage{
 			{
-				suffix:         "pkg",
+				suffix:         "",
+				readmeFilePath: sample.ReadmeFilePath, // required
+				readmeContents: moduleLinksReadme,
+			},
+			{
+				// This package has no readme, just links in its godoc. So the
+				// UnitMeta (right sidebar) links will be from the godoc and
+				// module readme.
+				suffix: "no_readme",
+				doc:    sample.DocumentationHTML.String(),
+			},
+			{
+				// This package has a readme as well as godoc links, so the
+				// UnitMeta links will be from all three places.
+				suffix:         "has_readme",
+				readmeFilePath: "has_readme/README.md", // required
+				readmeContents: packageLinksReadme,
 				doc:            sample.DocumentationHTML.String(),
-				readmeContents: linksReadme,
-				readmeFilePath: sample.ReadmeFilePath,
 			},
 		},
 	},
@@ -680,6 +699,54 @@ func serverTestCases() []serverTestCase {
 	}
 }
 
+func checkLink(n int, title, url, body string) htmlcheck.Checker {
+	// The first div under .UnitMeta is "Links" and the second is the
+	// repository, so div numbers below start with 3. There is no "a" for
+	// "Links", so the "a" numbers are off by one.
+	return in("",
+		in(fmt.Sprintf("div:nth-of-type(%d)", n+2), hasText(title)),
+		in(fmt.Sprintf("a:nth-of-type(%d)", n+1), href(url), hasText(body)))
+}
+
+var linksTestCases = []serverTestCase{
+	{
+		name:           "module links",
+		urlPath:        "/github.com/links/mod",
+		wantStatusCode: http.StatusOK,
+		want: in(".UnitMeta",
+			// Module readme links.
+			checkLink(1, "title1", "http://url1", "http://url1"),
+			checkLink(2, "title2", "about:invalid#zGoSafez", "javascript://pwned"),
+		),
+	},
+	{
+		name:           "no_readme package links",
+		urlPath:        "/github.com/links/mod/no_readme",
+		wantStatusCode: http.StatusOK,
+		want: in(".UnitMeta",
+			// Package doc links are first.
+			checkLink(1, "pkg.go.dev", "https://pkg.go.dev", "https://pkg.go.dev"),
+			// Then module readmes.
+			checkLink(2, "title1", "http://url1", "http://url1"),
+			checkLink(3, "title2", "about:invalid#zGoSafez", "javascript://pwned"),
+		),
+	},
+	{
+		name:           "has_readme package links",
+		urlPath:        "/github.com/links/mod/has_readme",
+		wantStatusCode: http.StatusOK,
+		want: in(".UnitMeta",
+			// Package readme links are first.
+			checkLink(1, "pkg title", "http://url2", "http://url2"),
+			// Package doc links are second.
+			checkLink(2, "pkg.go.dev", "https://pkg.go.dev", "https://pkg.go.dev"),
+			// Module readme links are third.
+			checkLink(3, "title1", "http://url1", "http://url1"),
+			checkLink(4, "title2", "about:invalid#zGoSafez", "javascript://pwned"),
+		),
+	},
+}
+
 // TestServer checks the contents of served pages by looking for
 // strings and elements in the parsed HTML response body.
 //
@@ -705,21 +772,7 @@ func TestServer(t *testing.T) {
 		{
 			name: "goldmark and readme outline experiments",
 			testCasesFunc: func() []serverTestCase {
-				return append(serverTestCases(), serverTestCase{
-					name:           "links",
-					urlPath:        "/github.com/links/pkg",
-					wantStatusCode: http.StatusOK,
-					// There is one div before the actual links appear, hence
-					// the numbers are off by one.
-					want: in(".UnitMeta",
-						in("div:nth-of-type(3)", hasText("title1")),
-						in("a:nth-of-type(2)", href("http://url1"), hasText("http://url1")),
-						in("div:nth-of-type(4)", hasText("title2")),
-						in("a:nth-of-type(3)", href("about:invalid#zGoSafez"), hasText("javascript://pwned")),
-						in("div:nth-of-type(5)", hasText("pkg.go.dev")),
-						in("a:nth-of-type(4)", href("https://pkg.go.dev"), hasText("https://pkg.go.dev")),
-					),
-				})
+				return append(serverTestCases(), linksTestCases...)
 			},
 			experiments: []string{internal.ExperimentReadmeOutline, internal.ExperimentGoldmark},
 		},
diff --git a/internal/frontend/unit_main.go b/internal/frontend/unit_main.go
index 4ad91dac..42468b47 100644
--- a/internal/frontend/unit_main.go
+++ b/internal/frontend/unit_main.go
@@ -51,14 +51,19 @@ type MainDetails struct {
 	// used to render the readme outline in the sidebar.
 	ReadmeOutline []*Heading
 
-	// ReadmeLinks are from the "Links" section of the readme file,
-	// and are displayed on the right sidebar.
+	// ReadmeLinks are from the "Links" section of this unit's readme file, and
+	// are displayed on the right sidebar.
 	ReadmeLinks []link
 
 	// DocLinks are from the "Links" section of the Go package documentation,
 	// and are displayed on the right sidebar.
 	DocLinks []link
 
+	// ModuleReadmeLinks are from the "Links" section of this unit's module, if
+	// the unit is not itself a module. They are displayed on the right sidebar.
+	// See https://golang.org/issue/42968.
+	ModuleReadmeLinks []link
+
 	// ImportedByCount is the number of packages that import this path.
 	// When the count is > limit it will read as 'limit+'. This field
 	// is not supported when using a datasource proxy.
@@ -127,9 +132,9 @@ func fetchMainDetails(ctx context.Context, ds internal.DataSource, um *internal.
 		return nil, err
 	}
 	var (
-		docParts = &dochtml.Parts{}
-		docLinks []link
-		files    []*File
+		docParts           = &dochtml.Parts{}
+		docLinks, modLinks []link
+		files              []*File
 	)
 	if unit.Documentation != nil {
 		end := middleware.ElapsedStat(ctx, "DecodePackage")
@@ -156,25 +161,42 @@ func fetchMainDetails(ctx context.Context, ds internal.DataSource, um *internal.
 		files = sourceFiles(unit, docPkg)
 		end()
 	}
+	// If the unit is not a module, fetch the module readme to extract its
+	// links.
+	// In the unlikely event that the module is redistributable but the unit is
+	// not, we will not show the module links on the unit page.
+	if unit.Path != unit.ModulePath && unit.IsRedistributable && experiment.IsActive(ctx, internal.ExperimentGoldmark) {
+		modReadme, err := ds.GetModuleReadme(ctx, unit.ModulePath, unit.Version)
+		if err != nil {
+			return nil, err
+		}
+		rm, err := processReadme(modReadme, um.SourceInfo)
+		if err != nil {
+			return nil, err
+		}
+		modLinks = rm.Links
+	}
+
 	return &MainDetails{
-		ExpandReadme:    expandReadme,
-		NestedModules:   nestedModules,
-		Subdirectories:  subdirectories,
-		Licenses:        transformLicenseMetadata(um.Licenses),
-		CommitTime:      absoluteTime(um.CommitTime),
-		Readme:          readme.HTML,
-		ReadmeOutline:   readme.Outline,
-		ReadmeLinks:     readme.Links,
-		DocLinks:        docLinks,
-		DocOutline:      docParts.Outline,
-		DocBody:         docParts.Body,
-		SourceFiles:     files,
-		RepositoryURL:   um.SourceInfo.RepoURL(),
-		SourceURL:       um.SourceInfo.DirectoryURL(internal.Suffix(um.Path, um.ModulePath)),
-		MobileOutline:   docParts.MobileOutline,
-		NumImports:      unit.NumImports,
-		ImportedByCount: importedByCount,
-		IsPackage:       unit.IsPackage(),
+		ExpandReadme:      expandReadme,
+		NestedModules:     nestedModules,
+		Subdirectories:    subdirectories,
+		Licenses:          transformLicenseMetadata(um.Licenses),
+		CommitTime:        absoluteTime(um.CommitTime),
+		Readme:            readme.HTML,
+		ReadmeOutline:     readme.Outline,
+		ReadmeLinks:       readme.Links,
+		DocLinks:          docLinks,
+		ModuleReadmeLinks: modLinks,
+		DocOutline:        docParts.Outline,
+		DocBody:           docParts.Body,
+		SourceFiles:       files,
+		RepositoryURL:     um.SourceInfo.RepoURL(),
+		SourceURL:         um.SourceInfo.DirectoryURL(internal.Suffix(um.Path, um.ModulePath)),
+		MobileOutline:     docParts.MobileOutline,
+		NumImports:        unit.NumImports,
+		ImportedByCount:   importedByCount,
+		IsPackage:         unit.IsPackage(),
 	}, nil
 }
 
diff --git a/internal/localdatasource/datasource.go b/internal/localdatasource/datasource.go
index 116b74ba..94ee63e9 100644
--- a/internal/localdatasource/datasource.go
+++ b/internal/localdatasource/datasource.go
@@ -181,3 +181,8 @@ func (ds *DataSource) GetLatestMajorVersion(ctx context.Context, seriesPath stri
 func (ds *DataSource) GetNestedModules(ctx context.Context, modulePath string) ([]*internal.ModuleInfo, error) {
 	return nil, nil
 }
+
+// GetModuleReadme is not implemented.
+func (*DataSource) GetModuleReadme(ctx context.Context, modulePath, resolvedVersion string) (*internal.Readme, error) {
+	return nil, nil
+}
diff --git a/internal/postgres/unit.go b/internal/postgres/unit.go
index 49e9a151..8a2fec9b 100644
--- a/internal/postgres/unit.go
+++ b/internal/postgres/unit.go
@@ -404,3 +404,28 @@ func (db *DB) getPathsInModule(ctx context.Context, modulePath, resolvedVersion
 	}
 	return paths, nil
 }
+
+// GetModuleReadme returns the README corresponding to the modulePath and version.
+func (db *DB) GetModuleReadme(ctx context.Context, modulePath, resolvedVersion string) (_ *internal.Readme, err error) {
+	defer derrors.Wrap(&err, "GetModuleReadme(ctx, %q, %q)", modulePath, resolvedVersion)
+	var readme internal.Readme
+	err = db.db.QueryRow(ctx, `
+		SELECT file_path, contents
+		FROM modules m
+		INNER JOIN units u
+		ON u.module_id = m.id
+		INNER JOIN readmes r
+		ON u.id = r.unit_id
+		WHERE
+		    m.module_path=$1
+			AND m.version=$2
+			AND m.module_path=u.path`, modulePath, resolvedVersion).Scan(&readme.Filepath, &readme.Contents)
+	switch err {
+	case sql.ErrNoRows:
+		return nil, derrors.NotFound
+	case nil:
+		return &readme, nil
+	default:
+		return nil, err
+	}
+}
diff --git a/internal/proxydatasource/details.go b/internal/proxydatasource/details.go
index 37da084f..6171df17 100644
--- a/internal/proxydatasource/details.go
+++ b/internal/proxydatasource/details.go
@@ -71,3 +71,8 @@ func (*DataSource) GetExperiments(ctx context.Context) ([]*internal.Experiment,
 func (ds *DataSource) GetNestedModules(ctx context.Context, modulePath string) (_ []*internal.ModuleInfo, err error) {
 	return nil, nil
 }
+
+// GetModuleReadme is unimplemented.
+func (ds *DataSource) GetModuleReadme(ctx context.Context, modulePath, resolvedVersion string) (*internal.Readme, error) {
+	return nil, nil
+}