From d3f7cf085350c6cfc1f02498391256b811d3193a Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Wed, 9 Aug 2023 16:03:35 -0400 Subject: [PATCH] internal/frontend: move urlinfo to its own package These are also being moved so that the code in fetch.go can be moved to a new package without depending on internal/frontend. A couple of functions that were only used by details.go are moved to that file. For #61399 Change-Id: Ic299069ea0b3aaeb80dbbf7f1ed4fedf7d1787df Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/518816 Reviewed-by: Robert Findley kokoro-CI: kokoro Run-TryBot: Michael Matloob TryBot-Result: Gopher Robot --- internal/frontend/404.go | 3 +- internal/frontend/details.go | 31 ++- internal/frontend/experiments.go | 54 ++++++ internal/frontend/experments_test.go | 42 +++++ internal/frontend/fetch.go | 19 +- internal/frontend/server.go | 5 +- internal/frontend/unit.go | 33 ++-- .../frontend/{ => urlinfo}/stdlib_test.go | 12 +- internal/frontend/{ => urlinfo}/urlinfo.go | 178 ++++++------------ .../frontend/{ => urlinfo}/urlinfo_test.go | 125 +++++------- 10 files changed, 265 insertions(+), 237 deletions(-) create mode 100644 internal/frontend/experiments.go create mode 100644 internal/frontend/experments_test.go rename internal/frontend/{ => urlinfo}/stdlib_test.go (75%) rename internal/frontend/{ => urlinfo}/urlinfo.go (50%) rename internal/frontend/{ => urlinfo}/urlinfo_test.go (55%) diff --git a/internal/frontend/404.go b/internal/frontend/404.go index ba773d05d..9824dd25b 100644 --- a/internal/frontend/404.go +++ b/internal/frontend/404.go @@ -23,6 +23,7 @@ import ( "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/frontend/urlinfo" "golang.org/x/pkgsite/internal/log" "golang.org/x/pkgsite/internal/stdlib" "golang.org/x/pkgsite/internal/version" @@ -187,7 +188,7 @@ func githubPathRedirect(fullPath string) string { // pathNotFoundError returns a page with an option on how to // add a package or module to the site. func pathNotFoundError(ctx context.Context, fullPath, requestedVersion string) error { - if !isSupportedVersion(fullPath, requestedVersion) { + if !urlinfo.IsSupportedVersion(fullPath, requestedVersion) { return invalidVersionError(fullPath, requestedVersion) } if stdlib.Contains(fullPath) { diff --git a/internal/frontend/details.go b/internal/frontend/details.go index cc4d77621..1aac1ee53 100644 --- a/internal/frontend/details.go +++ b/internal/frontend/details.go @@ -12,6 +12,7 @@ import ( "golang.org/x/pkgsite/internal/frontend/page" "golang.org/x/pkgsite/internal/frontend/serrors" + "golang.org/x/pkgsite/internal/frontend/urlinfo" mstats "golang.org/x/pkgsite/internal/middleware/stats" "github.com/google/safehtml/template" @@ -51,11 +52,11 @@ func (s *Server) serveDetails(w http.ResponseWriter, r *http.Request, ds interna ctx = setExperimentsFromQueryParam(ctx, r) } - urlInfo, err := extractURLPathInfo(r.URL.Path) + urlInfo, err := urlinfo.ExtractURLPathInfo(r.URL.Path) if err != nil { var epage *page.ErrorPage - if uerr := new(userError); errors.As(err, &uerr) { - epage = &page.ErrorPage{MessageData: uerr.userMessage} + if uerr := new(urlinfo.UserError); errors.As(err, &uerr) { + epage = &page.ErrorPage{MessageData: uerr.UserMessage} } return &serrors.ServerError{ Status: http.StatusBadRequest, @@ -63,14 +64,14 @@ func (s *Server) serveDetails(w http.ResponseWriter, r *http.Request, ds interna Epage: epage, } } - if !isSupportedVersion(urlInfo.fullPath, urlInfo.requestedVersion) { - return invalidVersionError(urlInfo.fullPath, urlInfo.requestedVersion) + if !urlinfo.IsSupportedVersion(urlInfo.FullPath, urlInfo.RequestedVersion) { + return invalidVersionError(urlInfo.FullPath, urlInfo.RequestedVersion) } - if urlPath := stdlibRedirectURL(urlInfo.fullPath); urlPath != "" { + if urlPath := stdlibRedirectURL(urlInfo.FullPath); urlPath != "" { http.Redirect(w, r, urlPath, http.StatusMovedPermanently) return } - if err := checkExcluded(ctx, ds, urlInfo.fullPath); err != nil { + if err := checkExcluded(ctx, ds, urlInfo.FullPath); err != nil { return err } return s.serveUnitPage(ctx, w, r, ds, urlInfo) @@ -140,3 +141,19 @@ func recordVersionTypeMetric(ctx context.Context, requestedVersion string) { tag.Upsert(keyVersionType, v), }, versionTypeResults.M(1)) } + +func checkExcluded(ctx context.Context, ds internal.DataSource, fullPath string) error { + db, ok := ds.(internal.PostgresDB) + if !ok { + return nil + } + excluded, err := db.IsExcluded(ctx, fullPath) + if err != nil { + return err + } + if excluded { + // Return NotFound; don't let the user know that the package was excluded. + return &serrors.ServerError{Status: http.StatusNotFound} + } + return nil +} diff --git a/internal/frontend/experiments.go b/internal/frontend/experiments.go new file mode 100644 index 000000000..097479270 --- /dev/null +++ b/internal/frontend/experiments.go @@ -0,0 +1,54 @@ +// 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 frontend + +import ( + "context" + "net/http" + "strings" + + "golang.org/x/pkgsite/internal/experiment" + "golang.org/x/pkgsite/internal/log" +) + +func setExperimentsFromQueryParam(ctx context.Context, r *http.Request) context.Context { + if err := r.ParseForm(); err != nil { + log.Errorf(ctx, "ParseForm: %v", err) + return ctx + } + return newContextFromExps(ctx, r.Form["exp"]) +} + +// newContextFromExps adds and removes experiments from the context's experiment +// set, creates a new set with the changes, and returns a context with the new +// set. Each string in expMods can be either an experiment name, which means +// that the experiment should be added, or "!" followed by an experiment name, +// meaning that it should be removed. +func newContextFromExps(ctx context.Context, expMods []string) context.Context { + var ( + exps []string + remove = map[string]bool{} + ) + set := experiment.FromContext(ctx) + for _, exp := range expMods { + if strings.HasPrefix(exp, "!") { + exp = exp[1:] + if set.IsActive(exp) { + remove[exp] = true + } + } else if !set.IsActive(exp) { + exps = append(exps, exp) + } + } + if len(exps) == 0 && len(remove) == 0 { + return ctx + } + for _, a := range set.Active() { + if !remove[a] { + exps = append(exps, a) + } + } + return experiment.NewContext(ctx, exps...) +} diff --git a/internal/frontend/experments_test.go b/internal/frontend/experments_test.go new file mode 100644 index 000000000..cb28013ec --- /dev/null +++ b/internal/frontend/experments_test.go @@ -0,0 +1,42 @@ +// 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 frontend + +import ( + "context" + "sort" + "testing" + + "github.com/google/go-cmp/cmp" + "golang.org/x/pkgsite/internal/experiment" +) + +func TestNewContextFromExps(t *testing.T) { + for _, test := range []struct { + mods []string + want []string + }{ + { + mods: []string{"c", "a", "b"}, + want: []string{"a", "b", "c"}, + }, + { + mods: []string{"d", "a"}, + want: []string{"a", "b", "c", "d"}, + }, + { + mods: []string{"d", "!b", "!a", "c"}, + want: []string{"c", "d"}, + }, + } { + ctx := experiment.NewContext(context.Background(), "a", "b", "c") + ctx = newContextFromExps(ctx, test.mods) + got := experiment.FromContext(ctx).Active() + sort.Strings(got) + if !cmp.Equal(got, test.want) { + t.Errorf("mods=%v:\ngot %v\nwant %v", test.mods, got, test.want) + } + } +} diff --git a/internal/frontend/fetch.go b/internal/frontend/fetch.go index 9f21b61db..29fdff04f 100644 --- a/internal/frontend/fetch.go +++ b/internal/frontend/fetch.go @@ -25,6 +25,7 @@ import ( "golang.org/x/pkgsite/internal/experiment" "golang.org/x/pkgsite/internal/fetch" "golang.org/x/pkgsite/internal/frontend/serrors" + "golang.org/x/pkgsite/internal/frontend/urlinfo" "golang.org/x/pkgsite/internal/log" "golang.org/x/pkgsite/internal/proxy" "golang.org/x/pkgsite/internal/queue" @@ -39,7 +40,7 @@ var ( // this module version in version_map. errModuleDoesNotExist = errors.New("module does not exist") // errPathDoesNotExistInModule indicates that a module for the path prefix - // exists, but within that module version, this fullPath could not be found. + // exists, but within that module version, this FullPath could not be found. errPathDoesNotExistInModule = errors.New("path does not exist in module") fetchTimeout = 30 * time.Second pollEvery = 1 * time.Second @@ -103,11 +104,11 @@ func (s *Server) serveFetch(w http.ResponseWriter, r *http.Request, ds internal. return &serrors.ServerError{Status: http.StatusNotFound} } - urlInfo, err := extractURLPathInfo(strings.TrimPrefix(r.URL.Path, "/fetch")) + urlInfo, err := urlinfo.ExtractURLPathInfo(strings.TrimPrefix(r.URL.Path, "/fetch")) if err != nil { return &serrors.ServerError{Status: http.StatusBadRequest} } - status, responseText := s.fetchAndPoll(r.Context(), ds, urlInfo.modulePath, urlInfo.fullPath, urlInfo.requestedVersion) + status, responseText := s.fetchAndPoll(r.Context(), ds, urlInfo.ModulePath, urlInfo.FullPath, urlInfo.RequestedVersion) if status != http.StatusOK { return &serrors.ServerError{Status: status, ResponseText: responseText} } @@ -134,14 +135,14 @@ func (s *Server) fetchAndPoll(ctx context.Context, ds internal.DataSource, modul recordFrontendFetchMetric(ctx, status, time.Since(start)) }() - if !isSupportedVersion(fullPath, requestedVersion) { + if !urlinfo.IsSupportedVersion(fullPath, requestedVersion) { return http.StatusBadRequest, http.StatusText(http.StatusBadRequest) } if !experiment.IsActive(ctx, internal.ExperimentEnableStdFrontendFetch) && stdlib.Contains(fullPath) { return http.StatusBadRequest, http.StatusText(http.StatusBadRequest) } - // Generate all possible module paths for the fullPath. + // Generate all possible module paths for the FullPath. db := ds.(internal.PostgresDB) modulePaths, err := modulePathsToFetch(ctx, db, fullPath, modulePath) if err != nil { @@ -165,7 +166,7 @@ func (s *Server) fetchAndPoll(ctx context.Context, ds internal.DataSource, modul } // checkPossibleModulePaths checks all modulePaths at the requestedVersion, to see -// if the fullPath exists. For each module path, it first checks version_map to +// if the FullPath exists. For each module path, it first checks version_map to // see if we already attempted to fetch the module. If not, and shouldQueue is // true, it will enqueue the module to the frontend task queue to be fetched. // checkPossibleModulePaths will then poll the database for each module path, @@ -391,7 +392,7 @@ func checkForPath(ctx context.Context, db internal.PostgresDB, vm, err := db.GetVersionMap(ctx, modulePath, requestedVersion) if err != nil { // If an error is returned, there are two possibilities: - // (1) A row for this modulePath and version does not exist. + // (1) A row for this ModulePath and version does not exist. // This means that the fetch request is not done yet, so return // statusNotFoundInVersionMap so the fetchHandler will call checkForPath // again in a few seconds. @@ -520,10 +521,10 @@ func candidateModulePaths(fullPath string) (_ []string, err error) { if fullPath == stdlib.ModulePath { return []string{stdlib.ModulePath}, nil } - if !isValidPath(fullPath) { + if !urlinfo.IsValidPath(fullPath) { return nil, &serrors.ServerError{ Status: http.StatusBadRequest, - Err: fmt.Errorf("isValidPath(%q): false", fullPath), + Err: fmt.Errorf("urlinfo.IsValidPath(%q): false", fullPath), } } paths := internal.CandidateModulePaths(fullPath) diff --git a/internal/frontend/server.go b/internal/frontend/server.go index 44f4d9be8..5556cf229 100644 --- a/internal/frontend/server.go +++ b/internal/frontend/server.go @@ -28,6 +28,7 @@ import ( "golang.org/x/pkgsite/internal/experiment" pagepkg "golang.org/x/pkgsite/internal/frontend/page" "golang.org/x/pkgsite/internal/frontend/serrors" + "golang.org/x/pkgsite/internal/frontend/urlinfo" "golang.org/x/pkgsite/internal/godoc/dochtml" "golang.org/x/pkgsite/internal/licenses" "golang.org/x/pkgsite/internal/log" @@ -329,12 +330,12 @@ func detailsTTLForPath(ctx context.Context, urlPath, tab string) time.Duration { if urlPath == "/" { return defaultTTL } - info, err := parseDetailsURLPath(urlPath) + info, err := urlinfo.ParseDetailsURLPath(urlPath) if err != nil { log.Errorf(ctx, "falling back to default TTL: %v", err) return defaultTTL } - if info.requestedVersion == version.Latest { + if info.RequestedVersion == version.Latest { return shortTTL } if tab == "importedby" || tab == "versions" { diff --git a/internal/frontend/unit.go b/internal/frontend/unit.go index 6eaac93d5..c9b3882a1 100644 --- a/internal/frontend/unit.go +++ b/internal/frontend/unit.go @@ -19,6 +19,7 @@ import ( "golang.org/x/pkgsite/internal/derrors" "golang.org/x/pkgsite/internal/frontend/page" "golang.org/x/pkgsite/internal/frontend/serrors" + "golang.org/x/pkgsite/internal/frontend/urlinfo" "golang.org/x/pkgsite/internal/log" "golang.org/x/pkgsite/internal/middleware/stats" "golang.org/x/pkgsite/internal/stdlib" @@ -106,7 +107,7 @@ type UnitPage struct { // serveUnitPage serves a unit page for a path. func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *http.Request, - ds internal.DataSource, info *urlPathInfo) (err error) { + ds internal.DataSource, info *urlinfo.URLPathInfo) (err error) { defer derrors.Wrap(&err, "serveUnitPage(ctx, w, r, ds, %v)", info) defer stats.Elapsed(ctx, "serveUnitPage")() @@ -121,12 +122,12 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht return nil } - um, err := ds.GetUnitMeta(ctx, info.fullPath, info.modulePath, info.requestedVersion) + um, err := ds.GetUnitMeta(ctx, info.FullPath, info.ModulePath, info.RequestedVersion) if err != nil { if !errors.Is(err, derrors.NotFound) { return err } - return s.servePathNotFoundPage(w, r, ds, info.fullPath, info.modulePath, info.requestedVersion) + return s.servePathNotFoundPage(w, r, ds, info.FullPath, info.ModulePath, info.RequestedVersion) } makeDepsDevURL := depsDevURLGenerator(ctx, um) @@ -137,7 +138,7 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht // It's also okay to provide just one (e.g. GOOS=windows), which will select // the first doc with that value, ignoring the other one. bc := internal.BuildContext{GOOS: r.FormValue("GOOS"), GOARCH: r.FormValue("GOARCH")} - d, err := fetchDetailsForUnit(ctx, r, tab, ds, um, info.requestedVersion, bc, s.vulnClient) + d, err := fetchDetailsForUnit(ctx, r, tab, ds, um, info.RequestedVersion, bc, s.vulnClient) if err != nil { return err } @@ -145,8 +146,8 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht return s.serveJSONPage(w, r, d) } - recordVersionTypeMetric(ctx, info.requestedVersion) - if _, ok := internal.DefaultBranches[info.requestedVersion]; ok { + recordVersionTypeMetric(ctx, info.RequestedVersion) + if _, ok := internal.DefaultBranches[info.RequestedVersion]; ok { // Since path@master is a moving target, we don't want it to be stale. // As a result, we enqueue every request of path@master to the frontend // task queue, which will initiate a fetch request depending on the @@ -160,17 +161,17 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht Err: err, Epage: &page.ErrorPage{ MessageData: fmt.Sprintf(`Default branches like "@%s" are not supported. Omit to get the current version.`, - info.requestedVersion), + info.RequestedVersion), }, } } go func() { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) defer cancel() - log.Infof(ctx, "serveUnitPage: Scheduling %q@%q to be fetched", um.ModulePath, info.requestedVersion) - if _, err := s.queue.ScheduleFetch(ctx, um.ModulePath, info.requestedVersion, nil); err != nil { + log.Infof(ctx, "serveUnitPage: Scheduling %q@%q to be fetched", um.ModulePath, info.RequestedVersion) + if _, err := s.queue.ScheduleFetch(ctx, um.ModulePath, info.RequestedVersion, nil); err != nil { log.Errorf(ctx, "serveUnitPage(%q): scheduling fetch for %q@%q: %v", - r.URL.Path, um.ModulePath, info.requestedVersion, err) + r.URL.Path, um.ModulePath, info.RequestedVersion, err) } }() } @@ -185,7 +186,7 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht // If we've already called GetUnitMeta for an unknown module path and the latest version, pass // it to GetLatestInfo to avoid a redundant call. var latestUnitMeta *internal.UnitMeta - if info.modulePath == internal.UnknownModulePath && info.requestedVersion == version.Latest { + if info.ModulePath == internal.UnknownModulePath && info.RequestedVersion == version.Latest { latestUnitMeta = um } latestInfo := s.GetLatestInfo(ctx, um.Path, um.ModulePath, latestUnitMeta) @@ -202,16 +203,16 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht if tabSettings.Name == "" { basePage.UseResponsiveLayout = true } - lv := linkVersion(um.ModulePath, info.requestedVersion, um.Version) + lv := linkVersion(um.ModulePath, info.RequestedVersion, um.Version) page := UnitPage{ BasePage: basePage, Unit: um, - Breadcrumb: displayBreadcrumb(um, info.requestedVersion), + Breadcrumb: displayBreadcrumb(um, info.RequestedVersion), Title: title, SelectedTab: tabSettings, - URLPath: constructUnitURL(um.Path, um.ModulePath, info.requestedVersion), - CanonicalURLPath: canonicalURLPath(um.Path, um.ModulePath, info.requestedVersion, um.Version), - DisplayVersion: displayVersion(um.ModulePath, info.requestedVersion, um.Version), + URLPath: constructUnitURL(um.Path, um.ModulePath, info.RequestedVersion), + CanonicalURLPath: canonicalURLPath(um.Path, um.ModulePath, info.RequestedVersion, um.Version), + DisplayVersion: displayVersion(um.ModulePath, info.RequestedVersion, um.Version), LinkVersion: lv, LatestURL: constructUnitURL(um.Path, um.ModulePath, version.Latest), LatestMinorClass: latestMinorClass(lv, latestInfo), diff --git a/internal/frontend/stdlib_test.go b/internal/frontend/urlinfo/stdlib_test.go similarity index 75% rename from internal/frontend/stdlib_test.go rename to internal/frontend/urlinfo/stdlib_test.go index 8cf62bf84..bc5b9ae02 100644 --- a/internal/frontend/stdlib_test.go +++ b/internal/frontend/urlinfo/stdlib_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package frontend +package urlinfo import ( "testing" @@ -42,13 +42,13 @@ func TestParseStdLibURLPath(t *testing.T) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - got, err := parseStdLibURLPath(test.url) + got, err := parseStdlibURLPath(test.url) if err != nil { - t.Fatalf("parseStdLibURLPath(%q): %v)", test.url, err) + t.Fatalf("parseStdlibURLPath(%q): %v)", test.url, err) } - if test.wantVersion != got.requestedVersion || test.wantPath != got.fullPath { - t.Fatalf("parseStdLibURLPath(%q): %q, %q, %v; want = %q, %q", - test.url, got.fullPath, got.requestedVersion, err, test.wantPath, test.wantVersion) + if test.wantVersion != got.RequestedVersion || test.wantPath != got.FullPath { + t.Fatalf("parseStdlibURLPath(%q): %q, %q, %v; want = %q, %q", + test.url, got.FullPath, got.RequestedVersion, err, test.wantPath, test.wantVersion) } }) } diff --git a/internal/frontend/urlinfo.go b/internal/frontend/urlinfo/urlinfo.go similarity index 50% rename from internal/frontend/urlinfo.go rename to internal/frontend/urlinfo/urlinfo.go index 0c2d8c4c0..b0898733d 100644 --- a/internal/frontend/urlinfo.go +++ b/internal/frontend/urlinfo/urlinfo.go @@ -2,64 +2,62 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package frontend +// Package urlinfo provides functions for extracting information out +// of url paths. +package urlinfo import ( - "context" "fmt" - "net/http" "strings" "golang.org/x/mod/module" "golang.org/x/mod/semver" "golang.org/x/pkgsite/internal" "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/stdlib" "golang.org/x/pkgsite/internal/version" ) -type urlPathInfo struct { - // fullPath is the full import path corresponding to the requested +// URLPathInfo contains the information about what unit is requested in a URL path. +type URLPathInfo struct { + // FullPath is the full import path corresponding to the requested // package/module/directory page. - fullPath string - // modulePath is the path of the module corresponding to the fullPath and + FullPath string + // ModulePath is the path of the module corresponding to the FullPath and // resolvedVersion. If unknown, it is set to internal.UnknownModulePath. - modulePath string + ModulePath string // requestedVersion is the version requested by the user, which will be one // of the following: "latest", "master", a Go version tag, or a semantic // version. - requestedVersion string + RequestedVersion string } -type userError struct { - userMessage string - err error +type UserError struct { + UserMessage string + Err error } -func (e *userError) Error() string { - return e.err.Error() +func (e *UserError) Error() string { + return e.Err.Error() } -func (e *userError) Unwrap() error { - return e.err +func (e *UserError) Unwrap() error { + return e.Err } -// extractURLPathInfo extracts information from a request to pkg.go.dev. +// ExtractURLPathInfo extracts information from a request to pkg.go.dev. // If an error is returned, the user will be served an http.StatusBadRequest. -func extractURLPathInfo(urlPath string) (_ *urlPathInfo, err error) { - defer derrors.Wrap(&err, "extractURLPathInfo(%q)", urlPath) +func ExtractURLPathInfo(urlPath string) (_ *URLPathInfo, err error) { + defer derrors.Wrap(&err, "ExtractURLPathInfo(%q)", urlPath) if m, _, _ := strings.Cut(strings.TrimPrefix(urlPath, "/"), "@"); stdlib.Contains(m) { - return parseStdLibURLPath(urlPath) + return parseStdlibURLPath(urlPath) } - return parseDetailsURLPath(urlPath) + return ParseDetailsURLPath(urlPath) } -// parseDetailsURLPath parses a URL path that refers (or may refer) to something +// ParseDetailsURLPath parses a URL path that refers (or may refer) to something // in the Go ecosystem. // // After trimming leading and trailing slashes, the path is expected to have one @@ -82,9 +80,9 @@ func extractURLPathInfo(urlPath string) (_ *urlPathInfo, err error) { // // In one case, we do a little more than parse the urlPath into parts: if the full path // could be a part of the standard library (because it has no '.'), we assume it -// is and set the modulePath to indicate the standard library. -func parseDetailsURLPath(urlPath string) (_ *urlPathInfo, err error) { - defer derrors.Wrap(&err, "parseDetailsURLPath(%q)", urlPath) +// is and set the ModulePath to indicate the standard library. +func ParseDetailsURLPath(urlPath string) (_ *URLPathInfo, err error) { + defer derrors.Wrap(&err, "ParseDetailsURLPath(%q)", urlPath) // This splits urlPath into either: // /[/] @@ -93,10 +91,10 @@ func parseDetailsURLPath(urlPath string) (_ *urlPathInfo, err error) { // or // //, @ modulePath, rest, found := strings.Cut(urlPath, "@") - info := &urlPathInfo{ - fullPath: strings.TrimSuffix(strings.TrimPrefix(modulePath, "/"), "/"), - modulePath: internal.UnknownModulePath, - requestedVersion: version.Latest, + info := &URLPathInfo{ + FullPath: strings.TrimSuffix(strings.TrimPrefix(modulePath, "/"), "/"), + ModulePath: internal.UnknownModulePath, + RequestedVersion: version.Latest, } if found { // The urlPath contains a "@". Parse the version and suffix from @@ -107,71 +105,71 @@ func parseDetailsURLPath(urlPath string) (_ *urlPathInfo, err error) { // The first path component after the '@' is the version. // You cannot explicitly write "latest" for the version. if endParts[0] == version.Latest { - return nil, &userError{ - err: fmt.Errorf("invalid version: %q", info.requestedVersion), - userMessage: fmt.Sprintf("%q is not a valid version", endParts[0]), + return nil, &UserError{ + Err: fmt.Errorf("invalid version: %q", info.RequestedVersion), + UserMessage: fmt.Sprintf("%q is not a valid version", endParts[0]), } } - info.requestedVersion = endParts[0] + info.RequestedVersion = endParts[0] // Parse the suffix following the "@version" from the urlPath. suffix := strings.Join(endParts[1:], "/") if suffix != "" { // If "@version" occurred in the middle of the path, the part before it // is the module path. - info.modulePath = info.fullPath - info.fullPath = info.fullPath + "/" + suffix + info.ModulePath = info.FullPath + info.FullPath = info.FullPath + "/" + suffix } } - if !isValidPath(info.fullPath) { - return nil, &userError{ - err: fmt.Errorf("isValidPath(%q) is false", info.fullPath), - userMessage: fmt.Sprintf("%q is not a valid import path", info.fullPath), + if !IsValidPath(info.FullPath) { + return nil, &UserError{ + Err: fmt.Errorf("IsValidPath(%q) is false", info.FullPath), + UserMessage: fmt.Sprintf("%q is not a valid import path", info.FullPath), } } return info, nil } -func parseStdLibURLPath(urlPath string) (_ *urlPathInfo, err error) { - defer derrors.Wrap(&err, "parseStdLibURLPath(%q)", urlPath) +func parseStdlibURLPath(urlPath string) (_ *URLPathInfo, err error) { + defer derrors.Wrap(&err, "parseStdlibURLPath(%q)", urlPath) // This splits urlPath into either: // /@ or / fullPath, tag, found := strings.Cut(urlPath, "@") fullPath = strings.TrimSuffix(strings.TrimPrefix(fullPath, "/"), "/") - if !isValidPath(fullPath) { - return nil, &userError{ - err: fmt.Errorf("isValidPath(%q) is false", fullPath), - userMessage: fmt.Sprintf("%q is not a valid import path", fullPath), + if !IsValidPath(fullPath) { + return nil, &UserError{ + Err: fmt.Errorf("IsValidPath(%q) is false", fullPath), + UserMessage: fmt.Sprintf("%q is not a valid import path", fullPath), } } - info := &urlPathInfo{ - fullPath: fullPath, - modulePath: stdlib.ModulePath, + info := &URLPathInfo{ + FullPath: fullPath, + ModulePath: stdlib.ModulePath, } if !found { - info.requestedVersion = version.Latest + info.RequestedVersion = version.Latest return info, nil } tag = strings.TrimSuffix(tag, "/") - info.requestedVersion = stdlib.VersionForTag(tag) - if info.requestedVersion == "" { + info.RequestedVersion = stdlib.VersionForTag(tag) + if info.RequestedVersion == "" { if tag == fetch.LocalVersion { // Special case: 0.0.0 is the version for a local stdlib - info.requestedVersion = fetch.LocalVersion + info.RequestedVersion = fetch.LocalVersion return info, nil } - return nil, &userError{ - err: fmt.Errorf("invalid Go tag for url: %q", urlPath), - userMessage: fmt.Sprintf("%q is not a valid tag for the standard library", tag), + return nil, &UserError{ + Err: fmt.Errorf("invalid Go tag for url: %q", urlPath), + UserMessage: fmt.Sprintf("%q is not a valid tag for the standard library", tag), } } return info, nil } -// isValidPath reports whether a requested path could be a valid unit. -func isValidPath(fullPath string) bool { +// IsValidPath reports whether a requested path could be a valid unit. +func IsValidPath(fullPath string) bool { if err := module.CheckImportPath(fullPath); err != nil { return false } @@ -195,24 +193,8 @@ func isValidPath(fullPath string) bool { return true } -func checkExcluded(ctx context.Context, ds internal.DataSource, fullPath string) error { - db, ok := ds.(internal.PostgresDB) - if !ok { - return nil - } - excluded, err := db.IsExcluded(ctx, fullPath) - if err != nil { - return err - } - if excluded { - // Return NotFound; don't let the user know that the package was excluded. - return &serrors.ServerError{Status: http.StatusNotFound} - } - return nil -} - -// isSupportedVersion reports whether the version is supported by the frontend. -func isSupportedVersion(fullPath, requestedVersion string) bool { +// IsSupportedVersion reports whether the version is supported by the frontend. +func IsSupportedVersion(fullPath, requestedVersion string) bool { if stdlib.Contains(fullPath) && stdlib.SupportedBranches[requestedVersion] { return true } @@ -221,43 +203,3 @@ func isSupportedVersion(fullPath, requestedVersion string) bool { } return requestedVersion == version.Latest || semver.IsValid(requestedVersion) } - -func setExperimentsFromQueryParam(ctx context.Context, r *http.Request) context.Context { - if err := r.ParseForm(); err != nil { - log.Errorf(ctx, "ParseForm: %v", err) - return ctx - } - return newContextFromExps(ctx, r.Form["exp"]) -} - -// newContextFromExps adds and removes experiments from the context's experiment -// set, creates a new set with the changes, and returns a context with the new -// set. Each string in expMods can be either an experiment name, which means -// that the experiment should be added, or "!" followed by an experiment name, -// meaning that it should be removed. -func newContextFromExps(ctx context.Context, expMods []string) context.Context { - var ( - exps []string - remove = map[string]bool{} - ) - set := experiment.FromContext(ctx) - for _, exp := range expMods { - if strings.HasPrefix(exp, "!") { - exp = exp[1:] - if set.IsActive(exp) { - remove[exp] = true - } - } else if !set.IsActive(exp) { - exps = append(exps, exp) - } - } - if len(exps) == 0 && len(remove) == 0 { - return ctx - } - for _, a := range set.Active() { - if !remove[a] { - exps = append(exps, a) - } - } - return experiment.NewContext(ctx, exps...) -} diff --git a/internal/frontend/urlinfo_test.go b/internal/frontend/urlinfo/urlinfo_test.go similarity index 55% rename from internal/frontend/urlinfo_test.go rename to internal/frontend/urlinfo/urlinfo_test.go index 7f6628620..db970ff9d 100644 --- a/internal/frontend/urlinfo_test.go +++ b/internal/frontend/urlinfo/urlinfo_test.go @@ -2,16 +2,13 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package frontend +package urlinfo import ( - "context" - "sort" "testing" "github.com/google/go-cmp/cmp" "golang.org/x/pkgsite/internal" - "golang.org/x/pkgsite/internal/experiment" "golang.org/x/pkgsite/internal/stdlib" "golang.org/x/pkgsite/internal/testing/sample" "golang.org/x/pkgsite/internal/version" @@ -20,87 +17,87 @@ import ( func TestExtractURLPathInfo(t *testing.T) { for _, test := range []struct { name, url string - want *urlPathInfo // nil => want non-nil error + want *URLPathInfo // nil => want non-nil error }{ { name: "path at latest", url: "/github.com/hashicorp/vault/api", - want: &urlPathInfo{ - modulePath: internal.UnknownModulePath, - fullPath: "github.com/hashicorp/vault/api", - requestedVersion: version.Latest, + want: &URLPathInfo{ + ModulePath: internal.UnknownModulePath, + FullPath: "github.com/hashicorp/vault/api", + RequestedVersion: version.Latest, }, }, { name: "path at version in nested module", url: "/github.com/hashicorp/vault/api@v1.0.3", - want: &urlPathInfo{ - modulePath: internal.UnknownModulePath, - fullPath: "github.com/hashicorp/vault/api", - requestedVersion: "v1.0.3", + want: &URLPathInfo{ + ModulePath: internal.UnknownModulePath, + FullPath: "github.com/hashicorp/vault/api", + RequestedVersion: "v1.0.3", }, }, { name: "package at version in parent module", url: "/github.com/hashicorp/vault@v1.0.3/api", - want: &urlPathInfo{ - modulePath: "github.com/hashicorp/vault", - fullPath: "github.com/hashicorp/vault/api", - requestedVersion: "v1.0.3", + want: &URLPathInfo{ + ModulePath: "github.com/hashicorp/vault", + FullPath: "github.com/hashicorp/vault/api", + RequestedVersion: "v1.0.3", }, }, { name: "package at version trailing slash", url: "/github.com/hashicorp/vault/api@v1.0.3/", - want: &urlPathInfo{ - modulePath: internal.UnknownModulePath, - fullPath: "github.com/hashicorp/vault/api", - requestedVersion: "v1.0.3", + want: &URLPathInfo{ + ModulePath: internal.UnknownModulePath, + FullPath: "github.com/hashicorp/vault/api", + RequestedVersion: "v1.0.3", }, }, { name: "stdlib module", url: "/std", - want: &urlPathInfo{ - modulePath: stdlib.ModulePath, - fullPath: "std", - requestedVersion: version.Latest, + want: &URLPathInfo{ + ModulePath: stdlib.ModulePath, + FullPath: "std", + RequestedVersion: version.Latest, }, }, { name: "stdlib module at version", url: "/std@go1.14", - want: &urlPathInfo{ - modulePath: stdlib.ModulePath, - fullPath: "std", - requestedVersion: "v1.14.0", + want: &URLPathInfo{ + ModulePath: stdlib.ModulePath, + FullPath: "std", + RequestedVersion: "v1.14.0", }, }, { name: "stdlib", url: "/net/http", - want: &urlPathInfo{ - modulePath: stdlib.ModulePath, - fullPath: "net/http", - requestedVersion: version.Latest, + want: &URLPathInfo{ + ModulePath: stdlib.ModulePath, + FullPath: "net/http", + RequestedVersion: version.Latest, }, }, { name: "stdlib at version", url: "/net/http@go1.14", - want: &urlPathInfo{ - modulePath: stdlib.ModulePath, - fullPath: "net/http", - requestedVersion: "v1.14.0", + want: &URLPathInfo{ + ModulePath: stdlib.ModulePath, + FullPath: "net/http", + RequestedVersion: "v1.14.0", }, }, } { t.Run(test.name, func(t *testing.T) { - got, err := extractURLPathInfo(test.url) + got, err := ExtractURLPathInfo(test.url) if err != nil { - t.Fatalf("extractURLPathInfo(%q): %v", test.url, err) + t.Fatalf("ExtractURLPathInfo(%q): %v", test.url, err) } - if diff := cmp.Diff(test.want, got, cmp.AllowUnexported(urlPathInfo{})); diff != "" { + if diff := cmp.Diff(test.want, got, cmp.AllowUnexported(URLPathInfo{})); diff != "" { t.Errorf("%q: mismatch (-want, +got):\n%s", test.url, diff) } }) @@ -140,46 +137,18 @@ func TestExtractURLPathInfo_Errors(t *testing.T) { } for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - got, err := extractURLPathInfo(test.url) + got, err := ExtractURLPathInfo(test.url) if (err != nil) != test.wantErr { - t.Fatalf("extractURLPathInfo(%q) error = (%v); want error %t)", test.url, err, test.wantErr) + t.Fatalf("ExtractURLPathInfo(%q) error = (%v); want error %t)", test.url, err, test.wantErr) } - if !test.wantErr && (test.wantModulePath != got.modulePath || test.wantVersion != got.requestedVersion || test.wantFullPath != got.fullPath) { - t.Fatalf("extractURLPathInfo(%q): %q, %q, %q, %v; want = %q, %q, %q, want err %t", - test.url, got.fullPath, got.modulePath, got.requestedVersion, err, test.wantFullPath, test.wantModulePath, test.wantVersion, test.wantErr) + if !test.wantErr && (test.wantModulePath != got.ModulePath || test.wantVersion != got.RequestedVersion || test.wantFullPath != got.FullPath) { + t.Fatalf("ExtractURLPathInfo(%q): %q, %q, %q, %v; want = %q, %q, %q, want err %t", + test.url, got.FullPath, got.ModulePath, got.RequestedVersion, err, test.wantFullPath, test.wantModulePath, test.wantVersion, test.wantErr) } }) } } -func TestNewContextFromExps(t *testing.T) { - for _, test := range []struct { - mods []string - want []string - }{ - { - mods: []string{"c", "a", "b"}, - want: []string{"a", "b", "c"}, - }, - { - mods: []string{"d", "a"}, - want: []string{"a", "b", "c", "d"}, - }, - { - mods: []string{"d", "!b", "!a", "c"}, - want: []string{"c", "d"}, - }, - } { - ctx := experiment.NewContext(context.Background(), "a", "b", "c") - ctx = newContextFromExps(ctx, test.mods) - got := experiment.FromContext(ctx).Active() - sort.Strings(got) - if !cmp.Equal(got, test.want) { - t.Errorf("mods=%v:\ngot %v\nwant %v", test.mods, got, test.want) - } - } -} - func TestIsValidPath(t *testing.T) { tests := []struct { path string @@ -200,9 +169,9 @@ func TestIsValidPath(t *testing.T) { {"gopkg.in/yaml.v2", true}, } for _, test := range tests { - got := isValidPath(test.path) + got := IsValidPath(test.path) if got != test.want { - t.Errorf("isValidPath(ctx, ds, %q) = %t, want %t", test.path, got, test.want) + t.Errorf("IsValidPath(ctx, ds, %q) = %t, want %t", test.path, got, test.want) } } } @@ -217,16 +186,16 @@ func TestIsSupportedVersion(t *testing.T) { {sample.ModulePath, "latest", true}, {sample.ModulePath, "master", true}, {sample.ModulePath, "main", true}, - {"net/http", "v1.2.3", true}, // isSupportedVersion expects the goTag is already converted to semver + {"net/http", "v1.2.3", true}, // IsSupportedVersion expects the goTag is already converted to semver {"net/http", "v1.2.3.bad", false}, {"net/http", "latest", true}, {"net/http", "master", true}, {"net/http", "main", false}, } for _, test := range tests { - got := isSupportedVersion(test.path, test.version) + got := IsSupportedVersion(test.path, test.version) if got != test.want { - t.Errorf("isSupportedVersion(ctx, ds, %q, %q) = %t, want %t", test.path, test.version, got, test.want) + t.Errorf("IsSupportedVersion(ctx, ds, %q, %q) = %t, want %t", test.path, test.version, got, test.want) } } }