internal/frontend: do latest-major-version logic in main serving path

Instead of using middleware to replace information about latest major versions in the HTML, insert them in the usual way, via templates. This is possible because the worker invalidates the cache when a new latest version comes in. Delete the latest-version middleware. Fixes golang/go#44210 Change-Id: I348a25d6b8777bad555e034d38ddb4137eb6ff18 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/293009 Trust: Jonathan Amsterdam <jba@google.com> Run-TryBot: Jonathan Amsterdam <jba@google.com> Reviewed-by: Julie Qiu <julie@golang.org> Reviewed-by: Jamal Carvalho <jamal@golang.org> TryBot-Result: kokoro <noreply+kokoro@google.com>
2021-02-17 10:19:53 -05:00 · 2021-02-17 10:19:53 -05:00 · 4306f9590a
--- a/cmd/frontend/main.go
+++ b/cmd/frontend/main.go
@ -180,7 +180,6 @@ func main() {
 		middleware.Quota(cfg.Quota, cacheClient),
 		middleware.SecureHeaders(!*disableCSP), // must come before any caching for nonces to work
 		middleware.Experiment(experimenter),
-		middleware.LatestVersions(server.GetLatestInfo), // must come before caching for version badge to work
 		middleware.Panic(panicHandler),
 		ermw,
 		middleware.Timeout(54*time.Second),
--- a/cmd/pkgsite/main.go
+++ b/cmd/pkgsite/main.go
@ -71,10 +71,7 @@ func main() {
 	router := dcensus.NewRouter(frontend.TagRoute)
 	server.Install(router.Handle, nil, nil)

-	mw := middleware.Chain(
-		middleware.LatestVersions(server.GetLatestInfo), // must come before caching for version badge to work
-		middleware.Timeout(54*time.Second),
-	)
+	mw := middleware.Timeout(54 * time.Second)
 	log.Infof(ctx, "Listening on addr %s", *httpAddr)
 	log.Fatal(ctx, http.ListenAndServe(*httpAddr, mw(router)))
 }
--- a/content/static/css/stylesheet.css
+++ b/content/static/css/stylesheet.css
@ -447,16 +447,6 @@ pre {
 .DetailsHeader {
  margin-top: 0.75rem;
 }
-.DetailsHeader-banner {
-  align-items: center;
-  background-color: var(--gray-10);
-  display: flex;
-  justify-content: flex-start;
-  padding-top: 0.1rem;
-}
-.DetailsHeader-banner--latest {
-  display: none;
-}
 .DetailsHeader-infoIcon {
  color: var(--gray-3);
  flex-shrink: 0;
--- a/content/static/css/unit_header.css
+++ b/content/static/css/unit_header.css
@ -116,15 +116,6 @@
  padding: 0.75rem 0;
 }

-/*
- * TODO: Replace DetailsHeader-banner with UnitHeader-majorVersionBanner in
- * middleware/latestversion.go after unit page is launched.
- */
-.UnitHeader-majorVersionBanner--latest,
-.DetailsHeader-banner--latest {
-  display: none;
-}
-
 .UnitHeader-detailIcon {
  color: var(--gray-3);
  flex-shrink: 0;
--- a/content/static/html/helpers/_unit_header.tmpl
+++ b/content/static/html/helpers/_unit_header.tmpl
@ -4,7 +4,7 @@
  license that can be found in the LICENSE file.
 -->

-{{/* . = internal/frontend.UnitPage */}}
+{{/* . is internal/frontend.UnitPage */}}

 {{define "unit_header"}}
  <header class="UnitHeader" role="complementary"
@ -46,13 +46,14 @@
          </span>
        </div>
      {{end}}
-      <div class="UnitHeader-majorVersionBanner $$GODISCOVERY_LATESTMAJORCLASS$$" data-test-id="UnitHeader-majorVersionBanner">
-        <img height="19px" width="16px" class="UnitHeader-detailIcon" src="/static/img/pkg-icon-info_19x16.svg" alt="">
-        <span>
-          The highest tagged major version is <a href="/$$GODISCOVERY_LATESTMAJORVERSIONURL$$">$$GODISCOVERY_LATESTMAJORVERSION$$</a>.
-        </span>
-      </div>
-
+      {{if .LatestMajorVersion}}
+        <div class="UnitHeader-majorVersionBanner" data-test-id="UnitHeader-majorVersionBanner">
+          <img height="19px" width="16px" class="UnitHeader-detailIcon" src="/static/img/pkg-icon-info_19x16.svg" alt="">
+          <span>
+            The highest tagged major version is <a href="/{{.LatestMajorVersionURL}}">{{.LatestMajorVersion}}</a>.
+          </span>
+        </div>
+      {{end}}
      <div class="js-fixedHeaderSentinel"></div>
      {{if (eq .SelectedTab.Name "")}}
        <div class="UnitHeader-detail">
--- a/internal/frontend/server_test.go
+++ b/internal/frontend/server_test.go
@ -462,7 +462,6 @@ func serverTestCases() []serverTestCase {
 	pp.Version = pseudoVersion
 	pp.FormattedVersion = "v0.0.0-...-1234567"
 	pp.IsLatestMinor = false
-	p9.IsLatestMajor = false
 	pkgPseudo := &pp

 	pkgInc := &pagecheck.Page{
@ -1186,7 +1185,7 @@ func testServer(t *testing.T, testCases []serverTestCase, experimentNames ...str
 			handler.ServeHTTP(w, httptest.NewRequest("GET", test.urlPath, nil))
 			res := w.Result()
 			if res.StatusCode != test.wantStatusCode {
-				t.Errorf("GET %q = %d, want %d", test.urlPath, res.StatusCode, test.wantStatusCode)
+				t.Fatalf("GET %q = %d, want %d", test.urlPath, res.StatusCode, test.wantStatusCode)
 			}
 			if test.wantLocation != "" {
 				if got := res.Header.Get("Location"); got != test.wantLocation {
@ -1463,9 +1462,7 @@ func newTestServer(t *testing.T, proxyModules []*proxy.Module, redisClient *redi
 	if err != nil {
 		t.Fatal(err)
 	}
-	mw := middleware.Chain(
-		middleware.Experiment(exp),
-		middleware.LatestVersions(s.GetLatestInfo))
+	mw := middleware.Experiment(exp)
 	return s, mw(mux), func() {
 		teardown()
 		postgres.ResetTestDB(testDB, t)
--- a/internal/frontend/unit.go
+++ b/internal/frontend/unit.go
@ -15,6 +15,7 @@ import (

 	"github.com/google/safehtml"
 	"github.com/google/safehtml/uncheckedconversions"
+	"golang.org/x/mod/module"
 	"golang.org/x/pkgsite/internal"
 	"golang.org/x/pkgsite/internal/cookie"
 	"golang.org/x/pkgsite/internal/derrors"
@ -60,6 +61,10 @@ type UnitPage struct {
 	// version in relationship to the latest version of the unit.
 	LatestMinorClass string

+	// Information about the latest major version of the module.
+	LatestMajorVersion    string
+	LatestMajorVersionURL string
+
 	// PageType is the type of page (pkg, cmd, dir, std, or mod).
 	PageType string

@ -146,21 +151,31 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht
 	basePage := s.newBasePage(r, title)
 	basePage.AllowWideContent = true
 	lv := linkVersion(um.Version, um.ModulePath)
+	_, majorVersion, _ := module.SplitPathVersion(um.ModulePath)
+	_, latestMajorVersion, ok := module.SplitPathVersion(latestInfo.MajorModulePath)
+	// Show the banner if there was no error getting the latest major version,
+	// and it is different from the major version of the current module path.
+	var latestMajorVersionNum string
+	if ok && majorVersion != latestMajorVersion && latestMajorVersion != "" {
+		latestMajorVersionNum = strings.TrimPrefix(latestMajorVersion, "/")
+	}
 	page := UnitPage{
-		basePage:           basePage,
-		Unit:               um,
-		Breadcrumb:         displayBreadcrumb(um, info.requestedVersion),
-		Title:              title,
-		SelectedTab:        tabSettings,
-		URLPath:            constructUnitURL(um.Path, um.ModulePath, info.requestedVersion),
-		CanonicalURLPath:   canonicalURLPath(um),
-		DisplayVersion:     displayVersion(um.Version, um.ModulePath),
-		LinkVersion:        lv,
-		LatestURL:          constructUnitURL(um.Path, um.ModulePath, internal.LatestVersion),
-		LatestMinorClass:   latestMinorClass(r.Context(), lv, latestInfo),
-		PageLabels:         pageLabels(um),
-		PageType:           pageType(um),
-		RedirectedFromPath: redirectPath,
+		basePage:              basePage,
+		Unit:                  um,
+		Breadcrumb:            displayBreadcrumb(um, info.requestedVersion),
+		Title:                 title,
+		SelectedTab:           tabSettings,
+		URLPath:               constructUnitURL(um.Path, um.ModulePath, info.requestedVersion),
+		CanonicalURLPath:      canonicalURLPath(um),
+		DisplayVersion:        displayVersion(um.Version, um.ModulePath),
+		LinkVersion:           lv,
+		LatestURL:             constructUnitURL(um.Path, um.ModulePath, internal.LatestVersion),
+		LatestMinorClass:      latestMinorClass(r.Context(), lv, latestInfo),
+		LatestMajorVersion:    latestMajorVersionNum,
+		LatestMajorVersionURL: latestInfo.MajorUnitPath,
+		PageLabels:            pageLabels(um),
+		PageType:              pageType(um),
+		RedirectedFromPath:    redirectPath,
 	}

 	// Use GOOS and GOARCH query parameters to create a build context, which
--- a/internal/middleware/caching.go
+++ b/internal/middleware/caching.go
@ -67,9 +67,9 @@ var (
 		TagKeys:     []tag.Key{keyCacheName, keyCacheOperation},
 	}

-	// To avoid test flakiness, when testMode is true, cache writes are
+	// To avoid test flakiness, when TestMode is true, cache writes are
 	// synchronous.
-	testMode = false
+	TestMode = false
 )

 func recordCacheResult(ctx context.Context, name string, hit bool, latency time.Duration) {
@ -153,7 +153,7 @@ func (c *cache) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	c.delegate.ServeHTTP(rec, r)
 	if rec.bufErr == nil && (rec.statusCode == 0 || rec.statusCode == http.StatusOK) {
 		ttl := c.expirer(r)
-		if testMode {
+		if TestMode {
 			c.put(ctx, key, rec, ttl)
 		} else {
 			go c.put(ctx, key, rec, ttl)
--- a/internal/middleware/caching_test.go
+++ b/internal/middleware/caching_test.go
@ -22,7 +22,7 @@ import (

 func TestCache(t *testing.T) {
 	// force cache writes to be synchronous
-	testMode = true
+	TestMode = true
 	// These variables are mutated before each test case to control the handler
 	// response.
 	var (
--- a/internal/middleware/latestversion.go
+++ b/internal/middleware/latestversion.go
@ -1,81 +0,0 @@
-// Copyright 2019 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package middleware
-
-import (
-	"bytes"
-	"context"
-	"net/http"
-	"regexp"
-
-	"golang.org/x/mod/module"
-	"golang.org/x/pkgsite/internal"
-	"golang.org/x/pkgsite/internal/log"
-)
-
-const (
-	latestMajorClassPlaceholder   = "$$GODISCOVERY_LATESTMAJORCLASS$$"
-	LatestMajorVersionPlaceholder = "$$GODISCOVERY_LATESTMAJORVERSION$$"
-	LatestMajorVersionURL         = "$$GODISCOVERY_LATESTMAJORVERSIONURL$$"
-)
-
-// latestInfoRegexp extracts values needed to determine the latest-version badge from a page's HTML.
-var latestInfoRegexp = regexp.MustCompile(`data-version="([^"]*)" data-mpath="([^"]*)" data-ppath="([^"]*)" data-pagetype="([^"]*)"`)
-
-type latestFunc func(ctx context.Context, unitPath, modulePath string) internal.LatestInfo
-
-// LatestVersions replaces the HTML placeholder values for the badge and banner
-// that displays whether the version of the package or module being served is
-// the latest minor version (badge) and the latest major version (banner).
-func LatestVersions(getLatest latestFunc) Middleware {
-	return func(h http.Handler) http.Handler {
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			crw := &capturingResponseWriter{ResponseWriter: w}
-			h.ServeHTTP(crw, r)
-			body := crw.bytes()
-			matches := latestInfoRegexp.FindSubmatch(body)
-			if matches != nil {
-				modulePath := string(matches[2])
-				_, majorVersion, _ := module.SplitPathVersion(modulePath)
-				unitPath := string(matches[3])
-				latest := getLatest(r.Context(), unitPath, modulePath)
-				_, latestMajorVersion, ok := module.SplitPathVersion(latest.MajorModulePath)
-				var latestMajorVersionText string
-				if ok && len(latestMajorVersion) > 0 {
-					latestMajorVersionText = latestMajorVersion[1:]
-				}
-				latestMajorClass := ""
-				// If the latest major version is the same as the major version of the current
-				// module path, it is currently the latest version so we don't show the banner.
-				// If an error occurs finding a major version (i.e: not found) an empty string
-				// is returned in which case we also don't show the banner.
-				if majorVersion == latestMajorVersion || latestMajorVersion == "" {
-					latestMajorClass += " DetailsHeader-banner--latest"
-				}
-				body = bytes.ReplaceAll(body, []byte(latestMajorClassPlaceholder), []byte(latestMajorClass))
-				body = bytes.ReplaceAll(body, []byte(LatestMajorVersionPlaceholder), []byte(latestMajorVersionText))
-				body = bytes.ReplaceAll(body, []byte(LatestMajorVersionURL), []byte(latest.MajorUnitPath))
-			}
-			if _, err := w.Write(body); err != nil {
-				log.Errorf(r.Context(), "LatestVersions, writing: %v", err)
-			}
-		})
-	}
-}
-
-// capturingResponseWriter is an http.ResponseWriter that captures
-// the body for later processing.
-type capturingResponseWriter struct {
-	http.ResponseWriter
-	buf bytes.Buffer
-}
-
-func (c *capturingResponseWriter) Write(b []byte) (int, error) {
-	return c.buf.Write(b)
-}
-
-func (c *capturingResponseWriter) bytes() []byte {
-	return c.buf.Bytes()
-}
--- a/internal/middleware/latestversion_test.go
+++ b/internal/middleware/latestversion_test.go
@ -1,117 +0,0 @@
-// Copyright 2019 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package middleware
-
-import (
-	"context"
-	"fmt"
-	"io/ioutil"
-	"net/http"
-	"net/http/httptest"
-	"testing"
-
-	"golang.org/x/pkgsite/internal"
-)
-
-func TestLatestMajorVersion(t *testing.T) {
-	for _, test := range []struct {
-		name        string
-		latest      internal.LatestInfo
-		modulePaths []string
-		in          string
-		want        string
-	}{
-		{
-			name:   "module path is not at latest",
-			latest: internal.LatestInfo{MajorModulePath: "foo.com/bar/v3", MajorUnitPath: "foo.com/bar/v3"},
-			modulePaths: []string{
-				"foo.com/bar",
-				"foo.com/bar/v2",
-				"foo.com/bar/v3",
-			},
-			in: `
-				<div class="DetailsHeader-banner$$GODISCOVERY_LATESTMAJORCLASS$$">
-					 data-version="v1.0.0" data-mpath="foo.com/bar" data-ppath="foo.com/bar/far" data-pagetype="pkg">
-					<p>
-						The highest tagged major version is <a href="/$$GODISCOVERY_LATESTMAJORVERSIONURL$$">$$GODISCOVERY_LATESTMAJORVERSION$$</a>.
-					</p>
-				</div>`,
-			want: `
-				<div class="DetailsHeader-banner">
-					 data-version="v1.0.0" data-mpath="foo.com/bar" data-ppath="foo.com/bar/far" data-pagetype="pkg">
-					<p>
-						The highest tagged major version is <a href="/foo.com/bar/v3">v3</a>.
-					</p>
-				</div>`,
-		},
-		{
-			name:   "module path is at latest",
-			latest: internal.LatestInfo{MajorModulePath: "foo.com/bar/v3", MajorUnitPath: "foo.com/bar/v3"},
-			modulePaths: []string{
-				"foo.com/bar",
-				"foo.com/bar/v2",
-				"foo.com/bar/v3",
-			},
-			in: `
-				<div class="DetailsHeader-banner$$GODISCOVERY_LATESTMAJORCLASS$$">
-					 data-version="v3.0.0" data-mpath="foo.com/bar/v3" data-ppath="foo.com/bar/far" data-pagetype="pkg">
-					<p>
-						The highest tagged major version is <a href="/$$GODISCOVERY_LATESTMAJORVERSIONURL$$">$$GODISCOVERY_LATESTMAJORVERSION$$</a>.
-					</p>
-				</div>`,
-			want: `
-				<div class="DetailsHeader-banner DetailsHeader-banner--latest">
-					 data-version="v3.0.0" data-mpath="foo.com/bar/v3" data-ppath="foo.com/bar/far" data-pagetype="pkg">
-					<p>
-						The highest tagged major version is <a href="/foo.com/bar/v3">v3</a>.
-					</p>
-				</div>`,
-		},
-		{
-			name:   "full path is not at the latest",
-			latest: internal.LatestInfo{MajorModulePath: "foo.com/bar/v3", MajorUnitPath: "foo.com/bar/v3/far"},
-			modulePaths: []string{
-				"foo.com/bar",
-				"foo.com/bar/v2",
-				"foo.com/bar/v3",
-			},
-			in: `
-				<div class="DetailsHeader-banner$$GODISCOVERY_LATESTMAJORCLASS$$">
-					data-version="v1.0.0" data-mpath="foo.com/bar" data-ppath="foo.com/bar/far" data-pagetype="pkg">
-					<p>
-						The highest tagged major version is <a href="/$$GODISCOVERY_LATESTMAJORVERSIONURL$$">$$GODISCOVERY_LATESTMAJORVERSION$$</a>.
-					</p>
-				</div>`,
-			want: `
-				<div class="DetailsHeader-banner">
-					data-version="v1.0.0" data-mpath="foo.com/bar" data-ppath="foo.com/bar/far" data-pagetype="pkg">
-					<p>
-						The highest tagged major version is <a href="/foo.com/bar/v3/far">v3</a>.
-					</p>
-				</div>`,
-		},
-	} {
-		t.Run(test.name, func(t *testing.T) {
-			handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				fmt.Fprint(w, test.in)
-			})
-			lfunc := func(context.Context, string, string) internal.LatestInfo { return test.latest }
-			ts := httptest.NewServer(LatestVersions(lfunc)(handler))
-			defer ts.Close()
-			resp, err := ts.Client().Get(ts.URL)
-			if err != nil {
-				t.Fatal(err)
-			}
-			got, err := ioutil.ReadAll(resp.Body)
-			if err != nil {
-				t.Fatal(err)
-			}
-			_ = resp.Body.Close()
-			if string(got) != test.want {
-				t.Errorf("\ngot  %s\nwant %s", got, test.want)
-			}
-		})
-	}
-}
--- a/internal/testing/integration/frontend_test.go
+++ b/internal/testing/integration/frontend_test.go
@ -64,7 +64,6 @@ func setupFrontend(ctx context.Context, t *testing.T, q queue.Queue, rc *redis.C
 	mw := middleware.Chain(
 		middleware.AcceptRequests(http.MethodGet, http.MethodPost),
 		middleware.SecureHeaders(enableCSP),
-		middleware.LatestVersions(s.GetLatestInfo),
 		middleware.Experiment(experimenter),
 	)
 	return httptest.NewServer(mw(mux))
--- a/internal/testing/integration/integration_test.go
+++ b/internal/testing/integration/integration_test.go
@ -20,6 +20,7 @@ import (
 	"golang.org/x/pkgsite/internal"
 	"golang.org/x/pkgsite/internal/godoc/dochtml"
 	"golang.org/x/pkgsite/internal/index"
+	"golang.org/x/pkgsite/internal/middleware"
 	"golang.org/x/pkgsite/internal/postgres"
 	"golang.org/x/pkgsite/internal/proxy"
 )
@ -41,6 +42,8 @@ func TestEndToEndProcessing(t *testing.T) {

 	defer postgres.ResetTestDB(testDB, t)

+	middleware.TestMode = true
+
 	proxyClient, proxyServer, indexClient, teardownClients := setupProxyAndIndex(t)
 	defer teardownClients()

--- a/internal/testing/pagecheck/pagecheck.go
+++ b/internal/testing/pagecheck/pagecheck.go
@ -106,17 +106,11 @@ func UnitHeader(p *Page, versionedURL bool, isPackage bool) htmlcheck.Checker {
 		importsDetails = nil
 	}

-	majorVersionBannerClass := "UnitHeader-majorVersionBanner"
+	var majorVersionBanner htmlcheck.Checker
 	if p.IsLatestMajor {
-		majorVersionBannerClass += "  DetailsHeader-banner--latest"
-	}
-
-	return in("header.UnitHeader",
-		versionBadge(p),
-		in(`[data-test-id="UnitHeader-breadcrumbCurrent"]`, text(curBreadcrumb)),
-		in(`[data-test-id="UnitHeader-title"]`, text(p.Title)),
-		in(`[data-test-id="UnitHeader-majorVersionBanner"]`,
-			attr("class", majorVersionBannerClass),
+		majorVersionBanner = htmlcheck.NotIn(`[data-test-id="UnitHeader-majorVersionBanner"]`)
+	} else {
+		majorVersionBanner = in(`[data-test-id="UnitHeader-majorVersionBanner"]`,
 			in("span",
 				text("The highest tagged major version is "),
 				in("a",
@ -124,7 +118,13 @@ func UnitHeader(p *Page, versionedURL bool, isPackage bool) htmlcheck.Checker {
 					exactText(p.LatestMajorVersion),
 				),
 			),
-		),
+		)
+	}
+	return in("header.UnitHeader",
+		versionBadge(p),
+		in(`[data-test-id="UnitHeader-breadcrumbCurrent"]`, text(curBreadcrumb)),
+		in(`[data-test-id="UnitHeader-title"]`, text(p.Title)),
+		majorVersionBanner,
 		in(`[data-test-id="UnitHeader-version"]`,
 			in("a",
 				href("?tab=versions"),