From 69b3ffd964ddec27168d4acfa6499e475e586a1e Mon Sep 17 00:00:00 2001 From: Charlotte Brandhorst-Satzkorn Date: Wed, 2 Dec 2020 12:32:20 +0000 Subject: [PATCH] internal: direct user to latest major version in current package Currently, the pkgsite allows the user to click a link to be taken to the latest major version for that module. When a user is in a sub package, clicking this link transports them to the top of the directory, rather than the latest major version for that package. Changes the behaviour of the link by directing to the latest major version of the currently viewed package. If the package does not exist in the latest major version, it falls back to the root of the module. The direct proxy datasource implementation does not attempt to perform this resolution and maintains the current behaviour. Fixes golang/go#42292 Change-Id: I6c17978034f87ceddb0edeae3894ce2cd4913fd2 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/274413 Trust: Julie Qiu Trust: Jonathan Amsterdam Run-TryBot: Julie Qiu TryBot-Result: kokoro Reviewed-by: Julie Qiu --- content/static/html/helpers/_unit_header.tmpl | 2 +- internal/datasource.go | 10 +- internal/frontend/latest_version.go | 13 +- internal/frontend/server_test.go | 287 +++++++++++++----- internal/localdatasource/datasource.go | 7 +- internal/middleware/latestversion.go | 15 +- internal/middleware/latestversion_test.go | 33 +- internal/postgres/version.go | 40 +-- internal/postgres/version_test.go | 52 ++-- internal/proxydatasource/datasource.go | 20 +- internal/proxydatasource/datasource_test.go | 35 ++- internal/testing/pagecheck/pagecheck.go | 43 ++- 12 files changed, 386 insertions(+), 171 deletions(-) diff --git a/content/static/html/helpers/_unit_header.tmpl b/content/static/html/helpers/_unit_header.tmpl index c5f80b979..9068e7bf5 100644 --- a/content/static/html/helpers/_unit_header.tmpl +++ b/content/static/html/helpers/_unit_header.tmpl @@ -36,7 +36,7 @@ {{.}} {{end}} -
+
The highest tagged major version is $$GODISCOVERY_LATESTMAJORVERSION$$. diff --git a/internal/datasource.go b/internal/datasource.go index 6f886c177..bd16f0a3c 100644 --- a/internal/datasource.go +++ b/internal/datasource.go @@ -11,8 +11,14 @@ type DataSource interface { // See the internal/postgres package for further documentation of these // methods, particularly as they pertain to the main postgres implementation. - // GetLatestMajorVersion returns the latest major version of a series path. - GetLatestMajorVersion(ctx context.Context, seriesPath string) (_ string, err error) + // GetLatestMajorVersion returns the latest module path and the full package path + // of the latest version found, given the fullPath and the modulePath. + // For example, in the module path "github.com/casbin/casbin", there + // is another module path with a greater major version "github.com/casbin/casbin/v3". + // This function will return "github.com/casbin/casbin/v3" or the input module path + // if no later module path was found. It also returns the full package path at the + // latest module version if it exists. If not, it returns the module path. + GetLatestMajorVersion(ctx context.Context, fullPath, modulePath string) (_ string, _ string, err error) // GetNestedModules returns the latest major version of all nested modules // given a modulePath path prefix. GetNestedModules(ctx context.Context, modulePath string) ([]*ModuleInfo, error) diff --git a/internal/frontend/latest_version.go b/internal/frontend/latest_version.go index 24aa684cd..e0d24256a 100644 --- a/internal/frontend/latest_version.go +++ b/internal/frontend/latest_version.go @@ -13,19 +13,18 @@ import ( "golang.org/x/pkgsite/internal/log" ) -// GetLatestMajorVersion returns the major version of a package or module. -// If a module isn't found from the series path or an error ocurs, an empty string is returned +// GetLatestMajorVersion returns the latest module path and the full package path +// of any major version found given the fullPath and the modulePath. // It is intended to be used as an argument to middleware.LatestVersions. -func (s *Server) GetLatestMajorVersion(ctx context.Context, seriesPath string) string { - mv, err := s.getDataSource(ctx).GetLatestMajorVersion(ctx, seriesPath) +func (s *Server) GetLatestMajorVersion(ctx context.Context, fullPath, modulePath string) (_ string, _ string) { + latestModulePath, latestPackagePath, err := s.getDataSource(ctx).GetLatestMajorVersion(ctx, fullPath, modulePath) if err != nil { if !errors.Is(err, derrors.NotFound) { log.Errorf(ctx, "GetLatestMajorVersion: %v", err) } - return "" + return "", "" } - - return mv + return latestModulePath, latestPackagePath } // GetLatestMinorVersion returns the latest minor version of the package or module. diff --git a/internal/frontend/server_test.go b/internal/frontend/server_test.go index 707d1e90d..7b8bb061e 100644 --- a/internal/frontend/server_test.go +++ b/internal/frontend/server_test.go @@ -114,6 +114,52 @@ var testModules = []testModule{ }, }, }, + { + // A module with a greater major version available. + path: "github.com/v2major/module_name", + redistributable: true, + versions: []string{"v1.0.0"}, + packages: []testPackage{ + { + suffix: "bar", + doc: sample.DocumentationHTML.String(), + readmeContents: sample.ReadmeContents, + readmeFilePath: sample.ReadmeFilePath, + }, + { + suffix: "bar/directory/hello", + doc: `io.Writer`, + }, + { + suffix: "buz", + doc: sample.DocumentationHTML.String(), + readmeContents: sample.ReadmeContents, + readmeFilePath: sample.ReadmeFilePath, + }, + { + suffix: "buz/directory/hello", + doc: `io.Writer`, + }, + }, + }, + { + // A v2 of the previous module, with one version. + path: "github.com/v2major/module_name/v2", + redistributable: true, + versions: []string{"v2.0.0"}, + packages: []testPackage{ + { + suffix: "bar", + doc: sample.DocumentationHTML.String(), + readmeContents: sample.ReadmeContents, + readmeFilePath: sample.ReadmeFilePath, + }, + { + suffix: "bar/directory/hello", + doc: `io.Writer`, + }, + }, + }, { // A non-redistributable module. path: "github.com/non_redistributable", @@ -271,18 +317,59 @@ func serverTestCases() []serverTestCase { ) pkgV100 := &pagecheck.Page{ - Title: "foo", - ModulePath: sample.ModulePath, - Version: sample.VersionString, - FormattedVersion: sample.VersionString, - Suffix: sample.Suffix, - IsLatest: true, - LatestLink: "/" + sample.ModulePath + "@" + sample.VersionString + "/" + sample.Suffix, - LicenseType: sample.LicenseType, - LicenseFilePath: sample.LicenseFilePath, - PackageURLFormat: "/" + sample.ModulePath + "%s/" + sample.Suffix, - ModuleURL: "/" + sample.ModulePath, + Title: "foo", + ModulePath: sample.ModulePath, + Version: sample.VersionString, + FormattedVersion: sample.VersionString, + Suffix: sample.Suffix, + IsLatest: true, + LatestLink: "/" + sample.ModulePath + "@" + sample.VersionString + "/" + sample.Suffix, + LatestMajorVersionLink: "/" + sample.ModulePath + "/" + sample.Suffix, + LicenseType: sample.LicenseType, + LicenseFilePath: sample.LicenseFilePath, + PackageURLFormat: "/" + sample.ModulePath + "%s/" + sample.Suffix, + ModuleURL: "/" + sample.ModulePath, + } + + v2pkgV100 := &pagecheck.Page{ + Title: "bar", + ModulePath: "github.com/v2major/module_name", + Version: "v1.0.0", + FormattedVersion: "v1.0.0", + Suffix: "bar", + IsLatest: false, + LatestLink: "/github.com/v2major/module_name@v1.0.0/bar", + LatestMajorVersion: "v2", + LatestMajorVersionLink: "/github.com/v2major/module_name/v2/bar", + LicenseType: sample.LicenseType, + LicenseFilePath: sample.LicenseFilePath, + PackageURLFormat: "/github.com/v2major/module_name%s/bar", + ModuleURL: "/github.com/v2major/module_name", } + + v2pkgV1Buz := *v2pkgV100 + v2pkgV1Buz.Title = "buz" + v2pkgV1Buz.Suffix = "buz" + v2pkgV1Buz.LatestLink = "/github.com/v2major/module_name@v1.0.0/buz" + v2pkgV1Buz.LatestMajorVersionLink = "/github.com/v2major/module_name/v2" + v2pkgV1Buz.PackageURLFormat = "/github.com/v2major/module_name%s/buz" + + v2pkgV200 := &pagecheck.Page{ + Title: "bar", + ModulePath: "github.com/v2major/module_name/v2", + Version: "v2.0.0", + FormattedVersion: "v2.0.0", + Suffix: "bar", + IsLatest: true, + LatestLink: "/github.com/v2major/module_name/v2@v2.0.0/bar", + LatestMajorVersion: "v2", + LatestMajorVersionLink: "/github.com/v2major/module_name/v2/bar", + LicenseType: sample.LicenseType, + LicenseFilePath: sample.LicenseFilePath, + PackageURLFormat: "/github.com/v2major/module_name/v2%s/bar", + ModuleURL: "/github.com/v2major/module_name/v2", + } + p9 := *pkgV100 p9.Version = "v0.9.0" p9.FormattedVersion = "v0.9.0" @@ -296,54 +383,58 @@ func serverTestCases() []serverTestCase { pkgPseudo := &pp pkgInc := &pagecheck.Page{ - Title: "inc", - ModulePath: "github.com/incompatible", - Version: "v1.0.0+incompatible", - FormattedVersion: "v1.0.0+incompatible", - Suffix: "dir/inc", - IsLatest: true, - LatestLink: "/github.com/incompatible@v1.0.0+incompatible/dir/inc", - LicenseType: "MIT", - LicenseFilePath: "LICENSE", - PackageURLFormat: "/github.com/incompatible%s/dir/inc", - ModuleURL: "/github.com/incompatible", + Title: "inc", + ModulePath: "github.com/incompatible", + Version: "v1.0.0+incompatible", + FormattedVersion: "v1.0.0+incompatible", + Suffix: "dir/inc", + IsLatest: true, + LatestLink: "/github.com/incompatible@v1.0.0+incompatible/dir/inc", + LatestMajorVersionLink: "/github.com/incompatible/dir/inc", + LicenseType: "MIT", + LicenseFilePath: "LICENSE", + PackageURLFormat: "/github.com/incompatible%s/dir/inc", + ModuleURL: "/github.com/incompatible", } pkgNonRedist := &pagecheck.Page{ - Title: "bar", - ModulePath: "github.com/non_redistributable", - Version: "v1.0.0", - FormattedVersion: "v1.0.0", - Suffix: "bar", - IsLatest: true, - LatestLink: "/github.com/non_redistributable@v1.0.0/bar", - LicenseType: "", - PackageURLFormat: "/github.com/non_redistributable%s/bar", - ModuleURL: "/github.com/non_redistributable", + Title: "bar", + ModulePath: "github.com/non_redistributable", + Version: "v1.0.0", + FormattedVersion: "v1.0.0", + Suffix: "bar", + IsLatest: true, + LatestLink: "/github.com/non_redistributable@v1.0.0/bar", + LatestMajorVersionLink: "/github.com/non_redistributable/bar", + LicenseType: "", + PackageURLFormat: "/github.com/non_redistributable%s/bar", + ModuleURL: "/github.com/non_redistributable", } dir := &pagecheck.Page{ - Title: "directory/", - ModulePath: sample.ModulePath, - Version: "v1.0.0", - FormattedVersion: "v1.0.0", - Suffix: "foo/directory", - LicenseType: "MIT", - LicenseFilePath: "LICENSE", - ModuleURL: "/" + sample.ModulePath, - PackageURLFormat: "/" + sample.ModulePath + "%s/foo/directory", + Title: "directory/", + ModulePath: sample.ModulePath, + Version: "v1.0.0", + FormattedVersion: "v1.0.0", + Suffix: "foo/directory", + LicenseType: "MIT", + LicenseFilePath: "LICENSE", + ModuleURL: "/" + sample.ModulePath, + PackageURLFormat: "/" + sample.ModulePath + "%s/foo/directory", + LatestMajorVersionLink: "/github.com/valid/module_name/foo/directory", } mod := &pagecheck.Page{ - ModulePath: sample.ModulePath, - Title: "module_name", - ModuleURL: "/" + sample.ModulePath, - Version: "v1.0.0", - FormattedVersion: "v1.0.0", - LicenseType: "MIT", - LicenseFilePath: "LICENSE", - IsLatest: true, - LatestLink: "/" + sample.ModulePath + "@v1.0.0", + ModulePath: sample.ModulePath, + Title: "module_name", + ModuleURL: "/" + sample.ModulePath, + Version: "v1.0.0", + FormattedVersion: "v1.0.0", + LicenseType: "MIT", + LicenseFilePath: "LICENSE", + IsLatest: true, + LatestLink: "/" + sample.ModulePath + "@v1.0.0", + LatestMajorVersionLink: "/" + sample.ModulePath, } mp := *mod mp.Version = pseudoVersion @@ -351,42 +442,45 @@ func serverTestCases() []serverTestCase { mp.IsLatest = false dirPseudo := &pagecheck.Page{ - ModulePath: "github.com/pseudo", - Title: "dir/", - ModuleURL: "/github.com/pseudo", - LatestLink: "/github.com/pseudo@" + pseudoVersion + "/dir", - Suffix: "dir", - Version: pseudoVersion, - FormattedVersion: mp.FormattedVersion, - LicenseType: "MIT", - LicenseFilePath: "LICENSE", - IsLatest: true, - PackageURLFormat: "/github.com/pseudo%s/dir", + ModulePath: "github.com/pseudo", + Title: "dir/", + ModuleURL: "/github.com/pseudo", + LatestLink: "/github.com/pseudo@" + pseudoVersion + "/dir", + LatestMajorVersionLink: "/github.com/pseudo/dir", + Suffix: "dir", + Version: pseudoVersion, + FormattedVersion: mp.FormattedVersion, + LicenseType: "MIT", + LicenseFilePath: "LICENSE", + IsLatest: true, + PackageURLFormat: "/github.com/pseudo%s/dir", } dirCmd := &pagecheck.Page{ - Title: "cmd", - ModulePath: "std", - Version: "go1.13", - FormattedVersion: "go1.13", - Suffix: "cmd", - LicenseType: "MIT", - LicenseFilePath: "LICENSE", - ModuleURL: "/std", - PackageURLFormat: "/cmd%s", + Title: "cmd", + ModulePath: "std", + Version: "go1.13", + FormattedVersion: "go1.13", + Suffix: "cmd", + LicenseType: "MIT", + LicenseFilePath: "LICENSE", + ModuleURL: "/std", + PackageURLFormat: "/cmd%s", + LatestMajorVersionLink: "/cmd", } netHttp := &pagecheck.Page{ - Title: "http", - ModulePath: "http", - Version: "go1.13", - FormattedVersion: "go1.13", - LicenseType: sample.LicenseType, - LicenseFilePath: sample.LicenseFilePath, - ModuleURL: "/net/http", - PackageURLFormat: "/net/http%s", - IsLatest: true, - LatestLink: "/net/http@go1.13", + Title: "http", + ModulePath: "http", + Version: "go1.13", + FormattedVersion: "go1.13", + LicenseType: sample.LicenseType, + LicenseFilePath: sample.LicenseFilePath, + ModuleURL: "/net/http", + PackageURLFormat: "/net/http%s", + IsLatest: true, + LatestLink: "/net/http@go1.13", + LatestMajorVersionLink: "/net/http", } return []serverTestCase{ @@ -519,6 +613,39 @@ func serverTestCases() []serverTestCase { in(".UnitDetails-content", hasText(`not displayed due to license restrictions`)), ), }, + { + name: "v2 package at v1", + urlPath: fmt.Sprintf("/%s@%s/%s", v2pkgV100.ModulePath, v2pkgV100.Version, v2pkgV100.Suffix), + wantStatusCode: http.StatusOK, + want: in("", + pagecheck.UnitHeader(v2pkgV100, versioned, isPackage), + pagecheck.UnitReadme(), + pagecheck.UnitDoc(), + pagecheck.UnitDirectories(fmt.Sprintf("/%s@%s/%s/directory/hello", v2pkgV100.ModulePath, v2pkgV100.Version, v2pkgV100.Suffix), "directory/hello"), + pagecheck.CanonicalURLPath("/github.com/v2major/module_name@v1.0.0/bar")), + }, + { + name: "v2 module with v1 package that does not exist in v2", + urlPath: fmt.Sprintf("/%s@%s/%s", v2pkgV1Buz.ModulePath, v2pkgV1Buz.Version, v2pkgV1Buz.Suffix), + wantStatusCode: http.StatusOK, + want: in("", + pagecheck.UnitHeader(&v2pkgV1Buz, versioned, isPackage), + pagecheck.UnitReadme(), + pagecheck.UnitDoc(), + pagecheck.UnitDirectories(fmt.Sprintf("/%s@%s/%s/directory/hello", v2pkgV1Buz.ModulePath, v2pkgV1Buz.Version, v2pkgV1Buz.Suffix), "directory/hello"), + pagecheck.CanonicalURLPath("/github.com/v2major/module_name@v1.0.0/buz")), + }, + { + name: "v2 package at v2", + urlPath: fmt.Sprintf("/%s@%s/%s", v2pkgV200.ModulePath, v2pkgV200.Version, v2pkgV200.Suffix), + wantStatusCode: http.StatusOK, + want: in("", + pagecheck.UnitHeader(v2pkgV200, versioned, isPackage), + pagecheck.UnitReadme(), + pagecheck.UnitDoc(), + pagecheck.UnitDirectories(fmt.Sprintf("/%s@%s/%s/directory/hello", v2pkgV200.ModulePath, v2pkgV200.Version, v2pkgV200.Suffix), "directory/hello"), + pagecheck.CanonicalURLPath("/github.com/v2major/module_name/v2@v2.0.0/bar")), + }, { name: "package at version default", urlPath: fmt.Sprintf("/%s@%s/%s", sample.ModulePath, sample.VersionString, sample.Suffix), diff --git a/internal/localdatasource/datasource.go b/internal/localdatasource/datasource.go index 94ee63e9f..20a8ffdc6 100644 --- a/internal/localdatasource/datasource.go +++ b/internal/localdatasource/datasource.go @@ -170,11 +170,12 @@ func (ds *DataSource) findModule(pkgPath string) (_ string, err error) { return "", fmt.Errorf("%s not loaded: %w", pkgPath, derrors.NotFound) } -// GetLatestMajorVersion returns the latest major version of a series path. +// GetLatestMajorVersion returns the latest major version and the full package path +// of any major version found given the seriesPath and the v1Path. // When fetching local modules, version is not accounted for, so an empty // string is returned. -func (ds *DataSource) GetLatestMajorVersion(ctx context.Context, seriesPath string) (string, error) { - return "", nil +func (ds *DataSource) GetLatestMajorVersion(ctx context.Context, seriesPath string, v1Path string) (string, string, error) { + return "", "", nil } // GetNestedModules is not implemented. diff --git a/internal/middleware/latestversion.go b/internal/middleware/latestversion.go index 24f7f5b86..c4926842b 100644 --- a/internal/middleware/latestversion.go +++ b/internal/middleware/latestversion.go @@ -12,7 +12,6 @@ import ( "strings" "golang.org/x/mod/module" - "golang.org/x/pkgsite/internal" "golang.org/x/pkgsite/internal/log" ) @@ -28,7 +27,7 @@ const ( var latestInfoRegexp = regexp.MustCompile(`data-version="([^"]*)" data-mpath="([^"]*)" data-ppath="([^"]*)" data-pagetype="([^"]*)"`) type latestMinorFunc func(ctx context.Context, packagePath, modulePath, pageType string) string -type latestMajorFunc func(ctx context.Context, seriesPath string) string +type latestMajorFunc func(ctx context.Context, fullPath, modulePath string) (string, string) // LatestVersions replaces the HTML placeholder values for the badge and banner // that displays whether the version of the package or module being served is @@ -45,7 +44,6 @@ func LatestVersions(latestMinor latestMinorFunc, latestMajor latestMajorFunc) Mi // The template package converts '+' to its HTML entity. version = strings.Replace(version, "+", "+", -1) modulePath := string(matches[2]) - seriesPath := internal.SeriesPathForModule(modulePath) _, majorVersion, _ := module.SplitPathVersion(modulePath) packagePath := string(matches[3]) pageType := string(matches[4]) @@ -59,10 +57,11 @@ func LatestVersions(latestMinor latestMinorFunc, latestMajor latestMajorFunc) Mi default: latestMinorClass += "--goToLatest" } - latestMajorVersion := latestMajor(r.Context(), seriesPath) - latestMajorVersionText := latestMajorVersion - if len(latestMajorVersionText) > 0 { - latestMajorVersionText = latestMajorVersionText[1:] + latestModulePath, latestPackagePath := latestMajor(r.Context(), packagePath, modulePath) + _, latestMajorVersion, ok := module.SplitPathVersion(latestModulePath) + 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 @@ -76,7 +75,7 @@ func LatestVersions(latestMinor latestMinorFunc, latestMajor latestMajorFunc) Mi body = bytes.ReplaceAll(body, []byte(LatestMinorVersionPlaceholder), []byte(latestMinorVersion)) body = bytes.ReplaceAll(body, []byte(latestMajorClassPlaceholder), []byte(latestMajorClass)) body = bytes.ReplaceAll(body, []byte(LatestMajorVersionPlaceholder), []byte(latestMajorVersionText)) - body = bytes.ReplaceAll(body, []byte(LatestMajorVersionURL), []byte(seriesPath+latestMajorVersion)) + body = bytes.ReplaceAll(body, []byte(LatestMajorVersionURL), []byte(latestPackagePath)) } if _, err := w.Write(body); err != nil { log.Errorf(r.Context(), "LatestVersions, writing: %v", err) diff --git a/internal/middleware/latestversion_test.go b/internal/middleware/latestversion_test.go index e83f7f2af..0d326ff27 100644 --- a/internal/middleware/latestversion_test.go +++ b/internal/middleware/latestversion_test.go @@ -135,7 +135,7 @@ func TestLatestMinorVersion(t *testing.T) { handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, test.in) }) - latestMajor := func(context.Context, string) string { return "" } + latestMajor := func(context.Context, string, string) (string, string) { return "", "" } ts := httptest.NewServer(LatestVersions(test.latest, latestMajor)(handler)) defer ts.Close() resp, err := ts.Client().Get(ts.URL) @@ -163,8 +163,10 @@ func TestLatestMajorVersion(t *testing.T) { want string }{ { - name: "module path is not at latest", - latest: func(context.Context, string) string { return "/v3" }, + name: "module path is not at latest", + latest: func(context.Context, string, string) (string, string) { + return "foo.com/bar/v3", "foo.com/bar/v3" + }, modulePaths: []string{ "foo.com/bar", "foo.com/bar/v2", @@ -187,7 +189,7 @@ func TestLatestMajorVersion(t *testing.T) { }, { name: "module path is at latest", - latest: func(context.Context, string) string { return "/v3" }, + latest: func(context.Context, string, string) (string, string) { return "foo.com/bar/v3", "foo.com/bar/v3" }, modulePaths: []string{ "foo.com/bar", "foo.com/bar/v2", @@ -208,6 +210,29 @@ func TestLatestMajorVersion(t *testing.T) {

`, }, + { + name: "full path is not at the latest", + latest: func(context.Context, string, string) (string, string) { return "foo.com/bar/v3", "foo.com/bar/v3/far" }, + modulePaths: []string{ + "foo.com/bar", + "foo.com/bar/v2", + "foo.com/bar/v3", + }, + in: ` +
+ data-version="v1.0.0" data-mpath="foo.com/bar" data-ppath="foo.com/bar/far" data-pagetype="pkg"> +

+ The highest tagged major version is $$GODISCOVERY_LATESTMAJORVERSION$$. +

+
`, + want: ` +
+ data-version="v1.0.0" data-mpath="foo.com/bar" data-ppath="foo.com/bar/far" data-pagetype="pkg"> +

+ The highest tagged major version is v3. +

+
`, + }, } { t.Run(test.name, func(t *testing.T) { handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/postgres/version.go b/internal/postgres/version.go index ead9e0466..a37425cb2 100644 --- a/internal/postgres/version.go +++ b/internal/postgres/version.go @@ -11,7 +11,6 @@ import ( "strings" "github.com/Masterminds/squirrel" - "golang.org/x/mod/module" "golang.org/x/pkgsite/internal" "golang.org/x/pkgsite/internal/derrors" "golang.org/x/pkgsite/internal/version" @@ -103,30 +102,35 @@ func versionTypeExpr(vts []version.Type) string { return strings.Join(vs, ", ") } -// GetLatestMajorVersion returns the latest major version string of a module -// path. For example, in the module path "github.com/casbin/casbin", there -// is another path with a greater major version -// "github.com/casbin/casbin/v3". This function will return "/v3" or an -// empty string if there is no major version string at the end. -func (db *DB) GetLatestMajorVersion(ctx context.Context, seriesPath string) (_ string, err error) { - defer derrors.Wrap(&err, "DB.GetLatestMajorVersion(ctx, %q)", seriesPath) +// GetLatestMajorVersion returns the latest module path and the full package path +// of the latest version found, given the fullPath and the modulePath. +// For example, in the module path "github.com/casbin/casbin", there +// is another module path with a greater major version "github.com/casbin/casbin/v3". +// This function will return "github.com/casbin/casbin/v3" or the input module path +// if no later module path was found. It also returns the full package path at the +// latest module version if it exists. If not, it returns the module path. +func (db *DB) GetLatestMajorVersion(ctx context.Context, fullPath, modulePath string) (_ string, _ string, err error) { + defer derrors.Wrap(&err, "DB.GetLatestMajorVersion(ctx, %q, %q)", fullPath, modulePath) - var latestPath string - q, args, err := orderByLatest(squirrel.Select("m.module_path"). + seriesPath := internal.SeriesPathForModule(modulePath) + v1Path := internal.V1Path(fullPath, modulePath) + q, args, err := orderByLatest(squirrel.Select("m.module_path, u.path"). From("modules m"). + LeftJoin("units u ON u.module_id = m.id"). Where(squirrel.Eq{"m.series_path": seriesPath})). + OrderByClause(`CASE + WHEN u.v1_path = ? THEN 1 + ELSE 2 + END`, v1Path). Limit(1). ToSql() if err != nil { - return "", err + return "", "", err } + var latestModulePath, latestPackagePath string row := db.db.QueryRow(ctx, q, args...) - if err := row.Scan(&latestPath); err != nil { - return "", err + if err := row.Scan(&latestModulePath, &latestPackagePath); err != nil { + return "", "", err } - _, majorPath, ok := module.SplitPathVersion(latestPath) - if !ok { - return "", fmt.Errorf("module.SplitPathVersion(%q): %v", latestPath, majorPath) - } - return majorPath, nil + return latestModulePath, latestPackagePath, nil } diff --git a/internal/postgres/version_test.go b/internal/postgres/version_test.go index 3bae815fb..9e22452ba 100644 --- a/internal/postgres/version_test.go +++ b/internal/postgres/version_test.go @@ -236,37 +236,55 @@ func TestGetLatestMajorVersion(t *testing.T) { defer cancel() defer ResetTestDB(testDB, t) - for _, modulePath := range []string{ - "foo.com/bar", - "foo.com/bar/v2", - "foo.com/bar/v3", - "bar.com/foo", + for _, m := range []*internal.Module{ + sample.Module("foo.com/bar", "v1.1.1", "baz", "faz"), + sample.Module("foo.com/bar/v2", "v2.0.5", "baz", "faz"), + sample.Module("foo.com/bar/v3", "v3.0.1", "baz"), + sample.Module("bar.com/foo", sample.VersionString, sample.Suffix), } { - m := sample.Module(modulePath, sample.VersionString, sample.Suffix) if err := testDB.InsertModule(ctx, m); err != nil { t.Fatal(err) } } for _, test := range []struct { - seriesPath string - wantVersion string - wantErr error + fullPath string + modulePath string + wantModulePath string + wantPackagePath string + wantErr error }{ { - seriesPath: "foo.com/bar", - wantVersion: "/v3", + fullPath: "foo.com/bar", + modulePath: "foo.com/bar", + wantModulePath: "foo.com/bar/v3", + wantPackagePath: "foo.com/bar/v3", }, { - seriesPath: "bar.com/foo", - wantVersion: "", + fullPath: "bar.com/foo", + modulePath: "bar.com/foo", + wantModulePath: "bar.com/foo", + wantPackagePath: "bar.com/foo", }, { - seriesPath: "boo.com/far", + fullPath: "boo.com/far", + modulePath: "boo.com/far", wantErr: sql.ErrNoRows, }, + { + fullPath: "foo.com/bar/baz", + modulePath: "foo.com/bar", + wantModulePath: "foo.com/bar/v3", + wantPackagePath: "foo.com/bar/v3/baz", + }, + { + fullPath: "foo.com/bar/faz", + modulePath: "foo.com/bar", + wantModulePath: "foo.com/bar/v3", + wantPackagePath: "foo.com/bar/v3", + }, } { - gotVersion, err := testDB.GetLatestMajorVersion(ctx, test.seriesPath) + gotVersion, gotPath, err := testDB.GetLatestMajorVersion(ctx, test.fullPath, test.modulePath) if err != nil { if test.wantErr == nil { t.Fatalf("got unexpected error %v", err) @@ -275,8 +293,8 @@ func TestGetLatestMajorVersion(t *testing.T) { t.Errorf("got err = %v, want Is(%v)", err, test.wantErr) } } - if gotVersion != test.wantVersion { - t.Errorf("testDB.GetLatestMajorVersion(%v) = %v, want = %v", test.seriesPath, gotVersion, test.wantVersion) + if gotVersion != test.wantModulePath || gotPath != test.wantPackagePath { + t.Errorf("testDB.GetLatestMajorVersion(%v, %v) = (%v, %v), want = (%v, %v)", test.fullPath, test.modulePath, gotVersion, gotPath, test.wantModulePath, test.wantPackagePath) } } } diff --git a/internal/proxydatasource/datasource.go b/internal/proxydatasource/datasource.go index 65097e719..167517a7f 100644 --- a/internal/proxydatasource/datasource.go +++ b/internal/proxydatasource/datasource.go @@ -175,13 +175,16 @@ func (ds *DataSource) getUnit(ctx context.Context, fullPath, modulePath, version return nil, fmt.Errorf("%q missing from module %s: %w", fullPath, m.ModulePath, derrors.NotFound) } -// GetLatestMajorVersion finds the latest major version of a modulePath that -// is found in the proxy by iterating through vN versions. -func (ds *DataSource) GetLatestMajorVersion(ctx context.Context, seriesPath string) (_ string, err error) { - // We are checking if the series path is valid so that we can forward the error if not. +// GetLatestMajorVersion returns the latest module path and the full package path +// of the latest version found in the proxy by iterating through vN versions. +// This function does not attempt to find whether the full path exists +// in the new major version. +func (ds *DataSource) GetLatestMajorVersion(ctx context.Context, fullPath, modulePath string) (_ string, _ string, err error) { + // We are checking if the full path is valid so that we can forward the error if not. + seriesPath := internal.SeriesPathForModule(modulePath) _, err = ds.proxyClient.GetInfo(ctx, seriesPath, internal.LatestVersion) if err != nil { - return "", err + return "", "", err } const startVersion = 2 // We start checking versions from "/v2", since v1 and v0 versions don't @@ -192,12 +195,13 @@ func (ds *DataSource) GetLatestMajorVersion(ctx context.Context, seriesPath stri _, err := ds.proxyClient.GetInfo(ctx, query, internal.LatestVersion) if errors.Is(err, derrors.NotFound) { if v == 2 { - return "", nil + return modulePath, fullPath, nil } - return fmt.Sprintf("/v%d", v-1), nil + latestModulePath := fmt.Sprintf("%s/v%d", seriesPath, v-1) + return latestModulePath, latestModulePath, nil } if err != nil { - return "", err + return "", "", err } } } diff --git a/internal/proxydatasource/datasource_test.go b/internal/proxydatasource/datasource_test.go index 93a9fddb4..6306bc16c 100644 --- a/internal/proxydatasource/datasource_test.go +++ b/internal/proxydatasource/datasource_test.go @@ -220,24 +220,37 @@ func TestDataSource_GetLatestMajorVersion(t *testing.T) { ds := New(client) for _, test := range []struct { - seriesPath string - wantVersion string - wantErr error + fullPath string + modulePath string + wantModulePath string + wantPackagePath string + wantErr error }{ { - seriesPath: "foo.com/bar", - wantVersion: "/v3", + fullPath: "foo.com/bar", + modulePath: "foo.com/bar", + wantModulePath: "foo.com/bar/v3", + wantPackagePath: "foo.com/bar/v3", }, { - seriesPath: "bar.com/foo", - wantVersion: "", + fullPath: "bar.com/foo", + modulePath: "bar.com/foo", + wantModulePath: "bar.com/foo", + wantPackagePath: "bar.com/foo", }, { - seriesPath: "boo.com/far", + fullPath: "boo.com/far", + modulePath: "boo.com/far", wantErr: derrors.NotFound, }, + { + fullPath: "foo.com/bar/baz", + modulePath: "foo.com/bar", + wantModulePath: "foo.com/bar/v3", + wantPackagePath: "foo.com/bar/v3", + }, } { - gotVersion, err := ds.GetLatestMajorVersion(ctx, test.seriesPath) + gotVersion, gotPath, err := ds.GetLatestMajorVersion(ctx, test.fullPath, test.modulePath) if err != nil { if test.wantErr == nil { t.Fatalf("got unexpected error %v", err) @@ -246,8 +259,8 @@ func TestDataSource_GetLatestMajorVersion(t *testing.T) { t.Errorf("got err = %v, want Is(%v)", err, test.wantErr) } } - if gotVersion != test.wantVersion { - t.Errorf("GetLatestMajorVersion(%v) = %v, want %v", test.seriesPath, gotVersion, test.wantVersion) + if gotVersion != test.wantModulePath || gotPath != test.wantPackagePath { + t.Errorf("ds.GetLatestMajorVersion(%v, %v) = (%v, %v), want = (%v, %v)", test.fullPath, test.modulePath, gotVersion, gotPath, test.wantModulePath, test.wantPackagePath) } } } diff --git a/internal/testing/pagecheck/pagecheck.go b/internal/testing/pagecheck/pagecheck.go index 69fd7adfa..ec75536e1 100644 --- a/internal/testing/pagecheck/pagecheck.go +++ b/internal/testing/pagecheck/pagecheck.go @@ -19,18 +19,22 @@ import ( // Page describes a discovery site web page for a package, module or directory. type Page struct { - ModulePath string - Suffix string // package or directory path after module path; empty for a module - Version string - FormattedVersion string - Title string - LicenseType string - LicenseFilePath string - IsLatest bool // is this the latest version of this module? - LatestLink string // href of "Go to latest" link - PackageURLFormat string // the relative package URL, with one %s for "@version"; also used for dirs - ModuleURL string // the relative module URL - CommitTime string + ModulePath string + Suffix string // package or directory path after module path; empty for a module + Version string + FormattedVersion string + Title string + LicenseType string + LicenseFilePath string + IsLatest bool // is this the latest version of this module? + LatestLink string // href of "Go to latest" link + LatestMajorVersion string // is the suffix of the latest major version, empty if v0 or v1 + // link to the latest major version for this package, or if the package does not exist + // link to the latest major version + LatestMajorVersionLink string + PackageURLFormat string // the relative package URL, with one %s for "@version"; also used for dirs + ModuleURL string // the relative module URL + CommitTime string } // Overview describes the contents of the overview tab. @@ -141,9 +145,24 @@ func UnitHeader(p *Page, versionedURL bool, isPackage bool) htmlcheck.Checker { commitTime = time.Now().In(time.UTC).Format("Jan _2, 2006") } + versionBannerClass := "UnitHeader-versionBanner" + if p.IsLatest { + versionBannerClass += " DetailsHeader-banner--latest" + } + return in("header.UnitHeader", in(`[data-test-id="UnitHeader-breadcrumbCurrent"]`, text(curBreadcrumb)), in(`[data-test-id="UnitHeader-title"]`, text(p.Title)), + in(`[data-test-id="UnitHeader-versionBanner"]`, + attr("class", versionBannerClass), + in("span", + text("The highest tagged major version is "), + in("a", + href(p.LatestMajorVersionLink), + exactText(p.LatestMajorVersion), + ), + ), + ), in(`[data-test-id="UnitHeader-version"]`, in("a", href("?tab=versions"),