Skip to content

Commit

Permalink
internal/frontend: move base types to their own packages
Browse files Browse the repository at this point in the history
This change moves serverError, basePage and errorPage out to different
packages so that the fetch page logic can use them but be moved out of
internal/frontend.

For golang/go#61399

Change-Id: I72ccee40d1847d3211ca851a320530c4c1dcf2e2
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/517976
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
kokoro-CI: kokoro <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
  • Loading branch information
matloob committed Aug 15, 2023
1 parent 82f79ed commit 712da68
Show file tree
Hide file tree
Showing 16 changed files with 265 additions and 214 deletions.
50 changes: 26 additions & 24 deletions internal/frontend/404.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"golang.org/x/pkgsite/internal/cookie"
"golang.org/x/pkgsite/internal/derrors"
"golang.org/x/pkgsite/internal/experiment"
"golang.org/x/pkgsite/internal/frontend/page"
"golang.org/x/pkgsite/internal/frontend/serrors"
"golang.org/x/pkgsite/internal/log"
"golang.org/x/pkgsite/internal/stdlib"
"golang.org/x/pkgsite/internal/version"
Expand All @@ -29,10 +31,10 @@ import (
// errUnitNotFoundWithoutFetch returns a 404 with instructions to the user on
// how to manually fetch the package. No fetch button is provided. This is used
// for very large modules or modules that previously 500ed.
var errUnitNotFoundWithoutFetch = &serverError{
status: http.StatusNotFound,
epage: &errorPage{
messageTemplate: template.MakeTrustedTemplate(`
var errUnitNotFoundWithoutFetch = &serrors.ServerError{
Status: http.StatusNotFound,
Epage: &page.ErrorPage{
MessageTemplate: template.MakeTrustedTemplate(`
<h3 class="Error-message">{{.StatusText}}</h3>
<p class="Error-message">Check that you entered the URL correctly or try fetching it following the
<a href="/about#adding-a-package">instructions here</a>.</p>`),
Expand Down Expand Up @@ -66,15 +68,15 @@ func (s *Server) servePathNotFoundPage(w http.ResponseWriter, r *http.Request,
}

if experiment.IsActive(ctx, internal.ExperimentEnableStdFrontendFetch) {
return &serverError{
status: http.StatusNotFound,
epage: &errorPage{
templateName: "fetch",
return &serrors.ServerError{
Status: http.StatusNotFound,
Epage: &page.ErrorPage{
TemplateName: "fetch",
MessageData: stdlib.ModulePath,
},
}
}
return &serverError{status: http.StatusNotFound}
return &serrors.ServerError{Status: http.StatusNotFound}
}

fr, err := previousFetchStatusAndResponse(ctx, db, fullPath, modulePath, requestedVersion)
Expand Down Expand Up @@ -150,10 +152,10 @@ func (s *Server) servePathNotFoundPage(w http.ResponseWriter, r *http.Request,
http.Redirect(w, r, "/search?q="+url.QueryEscape(fullPath), http.StatusFound)
return nil
}
return &serverError{
status: fr.status,
epage: &errorPage{
messageTemplate: uncheckedconversions.TrustedTemplateFromStringKnownToSatisfyTypeContract(`
return &serrors.ServerError{
Status: fr.status,
Epage: &page.ErrorPage{
MessageTemplate: uncheckedconversions.TrustedTemplateFromStringKnownToSatisfyTypeContract(`
<h3 class="Error-message">{{.StatusText}}</h3>
<p class="Error-message">` + html.UnescapeString(fr.responseText) + `</p>`),
MessageData: struct{ StatusText string }{http.StatusText(fr.status)},
Expand Down Expand Up @@ -190,24 +192,24 @@ func pathNotFoundError(ctx context.Context, fullPath, requestedVersion string) e
}
if stdlib.Contains(fullPath) {
if experiment.IsActive(ctx, internal.ExperimentEnableStdFrontendFetch) {
return &serverError{
status: http.StatusNotFound,
epage: &errorPage{
templateName: "fetch",
return &serrors.ServerError{
Status: http.StatusNotFound,
Epage: &page.ErrorPage{
TemplateName: "fetch",
MessageData: stdlib.ModulePath,
},
}
}
return &serverError{status: http.StatusNotFound}
return &serrors.ServerError{Status: http.StatusNotFound}
}
path := fullPath
if requestedVersion != version.Latest {
path = fmt.Sprintf("%s@%s", fullPath, requestedVersion)
}
return &serverError{
status: http.StatusNotFound,
epage: &errorPage{
templateName: "fetch",
return &serrors.ServerError{
Status: http.StatusNotFound,
Epage: &page.ErrorPage{
TemplateName: "fetch",
MessageData: path,
},
}
Expand All @@ -222,8 +224,8 @@ func previousFetchStatusAndResponse(ctx context.Context, db internal.PostgresDB,
// Get all candidate module paths for this path.
paths, err := modulePathsToFetch(ctx, db, fullPath, modulePath)
if err != nil {
var serr *serverError
if errors.As(err, &serr) && serr.status == http.StatusBadRequest {
var serr *serrors.ServerError
if errors.As(err, &serr) && serr.Status == http.StatusBadRequest {
// Return this as an invalid argument so that we don't log it in
// servePathNotFoundPage above.
return nil, derrors.InvalidArgument
Expand Down
6 changes: 4 additions & 2 deletions internal/frontend/badge.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ package frontend
import (
"net/http"
"strings"

"golang.org/x/pkgsite/internal/frontend/page"
)

type badgePage struct {
basePage
page.BasePage
// LinkPath is the URL path of the badge will link to.
LinkPath string
// BadgePath is the URL path of the badge SVG.
Expand All @@ -35,7 +37,7 @@ func (s *Server) badgeHandler(w http.ResponseWriter, r *http.Request) {
}

page := badgePage{
basePage: s.newBasePage(r, "Badge"),
BasePage: s.newBasePage(r, "Badge"),
LinkPath: path,
BadgePath: "badge/" + path + ".svg",
}
Expand Down
35 changes: 19 additions & 16 deletions internal/frontend/details.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ package frontend
import (
"context"
"errors"
mstats "golang.org/x/pkgsite/internal/middleware/stats"
"net/http"
"strings"

"golang.org/x/pkgsite/internal/frontend/page"
"golang.org/x/pkgsite/internal/frontend/serrors"
mstats "golang.org/x/pkgsite/internal/middleware/stats"

"github.com/google/safehtml/template"
"go.opencensus.io/stats"
"go.opencensus.io/stats/view"
Expand All @@ -29,7 +32,7 @@ func (s *Server) serveDetails(w http.ResponseWriter, r *http.Request, ds interna

ctx := r.Context()
if r.Method != http.MethodGet && r.Method != http.MethodHead {
return &serverError{status: http.StatusMethodNotAllowed}
return &serrors.ServerError{Status: http.StatusMethodNotAllowed}
}
if r.URL.Path == "/" {
s.serveHomepage(ctx, w, r)
Expand All @@ -50,14 +53,14 @@ func (s *Server) serveDetails(w http.ResponseWriter, r *http.Request, ds interna

urlInfo, err := extractURLPathInfo(r.URL.Path)
if err != nil {
var epage *errorPage
var epage *page.ErrorPage
if uerr := new(userError); errors.As(err, &uerr) {
epage = &errorPage{MessageData: uerr.userMessage}
epage = &page.ErrorPage{MessageData: uerr.userMessage}
}
return &serverError{
status: http.StatusBadRequest,
err: err,
epage: epage,
return &serrors.ServerError{
Status: http.StatusBadRequest,
Err: err,
Epage: epage,
}
}
if !isSupportedVersion(urlInfo.fullPath, urlInfo.requestedVersion) {
Expand Down Expand Up @@ -88,10 +91,10 @@ func stdlibRedirectURL(fullPath string) string {
}

func invalidVersionError(fullPath, requestedVersion string) error {
return &serverError{
status: http.StatusBadRequest,
epage: &errorPage{
messageTemplate: template.MakeTrustedTemplate(`
return &serrors.ServerError{
Status: http.StatusBadRequest,
Epage: &page.ErrorPage{
MessageTemplate: template.MakeTrustedTemplate(`
<h3 class="Error-message">{{.Version}} is not a valid semantic version.</h3>
<p class="Error-message">
To search for packages like {{.Path}}, <a href="/search?q={{.Path}}">click here</a>.
Expand All @@ -102,10 +105,10 @@ func invalidVersionError(fullPath, requestedVersion string) error {
}

func datasourceNotSupportedErr() error {
return &serverError{
status: http.StatusFailedDependency,
epage: &errorPage{
messageTemplate: template.MakeTrustedTemplate(
return &serrors.ServerError{
Status: http.StatusFailedDependency,
Epage: &page.ErrorPage{
MessageTemplate: template.MakeTrustedTemplate(
`<h3 class="Error-message">This page is not supported by this datasource.</h3>`),
},
}
Expand Down
29 changes: 15 additions & 14 deletions internal/frontend/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"golang.org/x/pkgsite/internal/derrors"
"golang.org/x/pkgsite/internal/experiment"
"golang.org/x/pkgsite/internal/fetch"
"golang.org/x/pkgsite/internal/frontend/serrors"
"golang.org/x/pkgsite/internal/log"
"golang.org/x/pkgsite/internal/proxy"
"golang.org/x/pkgsite/internal/queue"
Expand Down Expand Up @@ -99,16 +100,16 @@ func (s *Server) serveFetch(w http.ResponseWriter, r *http.Request, ds internal.
if r.Method != http.MethodPost {
// If a user makes a GET request, treat this as a request for the
// "fetch" package, which does not exist.
return &serverError{status: http.StatusNotFound}
return &serrors.ServerError{Status: http.StatusNotFound}
}

urlInfo, err := extractURLPathInfo(strings.TrimPrefix(r.URL.Path, "/fetch"))
if err != nil {
return &serverError{status: http.StatusBadRequest}
return &serrors.ServerError{Status: http.StatusBadRequest}
}
status, responseText := s.fetchAndPoll(r.Context(), ds, urlInfo.modulePath, urlInfo.fullPath, urlInfo.requestedVersion)
if status != http.StatusOK {
return &serverError{status: status, responseText: responseText}
return &serrors.ServerError{Status: status, ResponseText: responseText}
}
return nil
}
Expand Down Expand Up @@ -144,9 +145,9 @@ func (s *Server) fetchAndPoll(ctx context.Context, ds internal.DataSource, modul
db := ds.(internal.PostgresDB)
modulePaths, err := modulePathsToFetch(ctx, db, fullPath, modulePath)
if err != nil {
var serr *serverError
var serr *serrors.ServerError
if errors.As(err, &serr) {
return serr.status, http.StatusText(serr.status)
return serr.Status, http.StatusText(serr.Status)
}
log.Errorf(ctx, "fetchAndPoll(ctx, ds, q, %q, %q, %q): %v", modulePath, fullPath, requestedVersion, err)
return http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)
Expand Down Expand Up @@ -497,9 +498,9 @@ func modulePathsToFetch(ctx context.Context, ds internal.DataSource, fullPath, m
}
um, err := ds.GetUnitMeta(ctx, fullPath, modulePath, version.Latest)
if err != nil && !errors.Is(err, derrors.NotFound) {
return nil, &serverError{
status: http.StatusInternalServerError,
err: err,
return nil, &serrors.ServerError{
Status: http.StatusInternalServerError,
Err: err,
}
}
if err == nil {
Expand All @@ -520,16 +521,16 @@ func candidateModulePaths(fullPath string) (_ []string, err error) {
return []string{stdlib.ModulePath}, nil
}
if !isValidPath(fullPath) {
return nil, &serverError{
status: http.StatusBadRequest,
err: fmt.Errorf("isValidPath(%q): false", fullPath),
return nil, &serrors.ServerError{
Status: http.StatusBadRequest,
Err: fmt.Errorf("isValidPath(%q): false", fullPath),
}
}
paths := internal.CandidateModulePaths(fullPath)
if paths == nil {
return nil, &serverError{
status: http.StatusBadRequest,
err: fmt.Errorf("invalid path: %q", fullPath),
return nil, &serrors.ServerError{
Status: http.StatusBadRequest,
Err: fmt.Errorf("invalid path: %q", fullPath),
}
}
if len(paths) > maxPathsToFetch {
Expand Down
6 changes: 4 additions & 2 deletions internal/frontend/homepage.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"context"
"math/rand"
"net/http"

"golang.org/x/pkgsite/internal/frontend/page"
)

// searchTip represents a snippet of text on the homepage demonstrating
Expand Down Expand Up @@ -38,7 +40,7 @@ var searchTips = []searchTip{

// Homepage contains fields used in rendering the homepage template.
type homepage struct {
basePage
page.BasePage

// TipIndex is the index of the initial search tip to render.
TipIndex int
Expand All @@ -61,7 +63,7 @@ type LocalModule struct {

func (s *Server) serveHomepage(ctx context.Context, w http.ResponseWriter, r *http.Request) {
s.servePage(ctx, w, "homepage", homepage{
basePage: s.newBasePage(r, "Go Packages"),
BasePage: s.newBasePage(r, "Go Packages"),
SearchTips: searchTips,
TipIndex: rand.Intn(len(searchTips)),
LocalModules: s.localModules,
Expand Down
73 changes: 73 additions & 0 deletions internal/frontend/page/page.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Copyright 2023 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 page defines common fields shared by pages when rendering templages.
package page

import (
"github.com/google/safehtml"
"github.com/google/safehtml/template"
"golang.org/x/pkgsite/internal/experiment"
)

// BasePage contains fields shared by all pages when rendering templates.
type BasePage struct {
// HTMLTitle is the value to use in the page’s <title> tag.
HTMLTitle string

// MetaDescription is the html used for rendering the <meta name="Description"> tag.
MetaDescription safehtml.HTML

// Query is the current search query (if applicable).
Query string

// Experiments contains the experiments currently active.
Experiments *experiment.Set

// DevMode indicates whether the server is running in development mode.
DevMode bool

// LocalMode indicates whether the server is running in local mode (i.e. ./cmd/pkgsite).
LocalMode bool

// AppVersionLabel contains the current version of the app.
AppVersionLabel string

// GoogleTagManagerID is the ID used to load Google Tag Manager.
GoogleTagManagerID string

// AllowWideContent indicates whether the content should be displayed in a
// way that’s amenable to wider viewports.
AllowWideContent bool

// Enables the two and three column layouts on the unit page.
UseResponsiveLayout bool

// SearchPrompt is the prompt/placeholder for search input.
SearchPrompt string

// SearchMode is the search mode for the current search request.
SearchMode string

// SearchModePackage is the value of const searchModePackage. It is used in
// the search bar dropdown.
SearchModePackage string

// SearchModeSymbol is the value of const searchModeSymbol. It is used in
// the search bar dropdown.
SearchModeSymbol string
}

func (p *BasePage) SetBasePage(bp BasePage) {
bp.SearchMode = p.SearchMode
*p = bp
}

// ErrorPage contains fields for rendering a HTTP error page.
type ErrorPage struct {
BasePage
TemplateName string
MessageTemplate template.TrustedTemplate
MessageData any
}
Loading

0 comments on commit 712da68

Please sign in to comment.