interanl: add experiment not-at-v1
GetLatestMajorVersionForV1Path is taking a long time to execute
(up to 30s). This logic is added behind a feature flag for
investigation.
Change-Id: I6d0c1e9937a7d4690dfa6393f157b3f0be99a05b
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/283259
Trust: Julie Qiu <julie@golang.org>
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
diff --git a/internal/experiment.go b/internal/experiment.go
index 0c0d8c4..88a40a9 100644
--- a/internal/experiment.go
+++ b/internal/experiment.go
@@ -7,12 +7,14 @@
const (
ExperimentNotAtLatest = "not-at-latest"
+ ExperimentNotAtV1 = "not-at-v1"
)
// Experiments represents all of the active experiments in the codebase and
// a description of each experiment.
var Experiments = map[string]string{
ExperimentNotAtLatest: "Enable the display of a 'not at latest' badge.",
+ ExperimentNotAtV1: "Redirect requests to a path not at v1 to the highest major version of that path.",
}
// Experiment holds data associated with an experimental feature for frontend
diff --git a/internal/frontend/404.go b/internal/frontend/404.go
index 50a73ea..09ca6df 100644
--- a/internal/frontend/404.go
+++ b/internal/frontend/404.go
@@ -15,6 +15,7 @@
"github.com/google/safehtml/template/uncheckedconversions"
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/derrors"
+ "golang.org/x/pkgsite/internal/experiment"
"golang.org/x/pkgsite/internal/log"
"golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/stdlib"
@@ -137,25 +138,27 @@
}, fullPath, requestedVersion)
}
- // Check if the unit path exists at a higher major version.
- // For example, my.module might not exist, but my.module/v3 might.
- // Similarly, my.module/foo might not exist, but my.module/v3/foo might.
- // In either case, the user will be redirected to the highest major version
- // of the path.
- //
- // Do not bother to look for a specific version if this case. If
- // my.module/foo@v2.1.0 was requested, and my.module/foo/v2 exists, just
- // return the latest version of my.module/foo/v2.
- majPath, err := db.GetLatestMajorPathForV1Path(ctx, fullPath)
- if err != nil && err != derrors.NotFound {
- return nil, err
- }
- if majPath != "" {
- return &fetchResult{
- modulePath: majPath,
- goModPath: majPath,
- status: http.StatusFound,
- }, nil
+ if experiment.IsActive(ctx, internal.ExperimentNotAtV1) {
+ // Check if the unit path exists at a higher major version.
+ // For example, my.module might not exist, but my.module/v3 might.
+ // Similarly, my.module/foo might not exist, but my.module/v3/foo might.
+ // In either case, the user will be redirected to the highest major version
+ // of the path.
+ //
+ // Do not bother to look for a specific version if this case. If
+ // my.module/foo@v2.1.0 was requested, and my.module/foo/v2 exists, just
+ // return the latest version of my.module/foo/v2.
+ majPath, err := db.GetLatestMajorPathForV1Path(ctx, fullPath)
+ if err != nil && err != derrors.NotFound {
+ return nil, err
+ }
+ if majPath != "" {
+ return &fetchResult{
+ modulePath: majPath,
+ goModPath: majPath,
+ status: http.StatusFound,
+ }, nil
+ }
}
// The full path does not exist in our database, but its module might.
@@ -183,10 +186,14 @@
}
func fetchResultFromVersionMap(vm *internal.VersionMap) *fetchResult {
+ var err error
+ if vm.Error != "" {
+ err = errors.New(vm.Error)
+ }
return &fetchResult{
modulePath: vm.ModulePath,
goModPath: vm.GoModPath,
status: vm.Status,
- err: errors.New(vm.Error),
+ err: err,
}
}
diff --git a/internal/frontend/404_test.go b/internal/frontend/404_test.go
index 538a8f1..aa30240 100644
--- a/internal/frontend/404_test.go
+++ b/internal/frontend/404_test.go
@@ -8,11 +8,15 @@
"context"
"errors"
"net/http"
+ "strings"
"testing"
"github.com/google/go-cmp/cmp"
+ "github.com/google/go-cmp/cmp/cmpopts"
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/derrors"
+ "golang.org/x/pkgsite/internal/experiment"
+ "golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/testing/sample"
)
@@ -120,6 +124,23 @@
}
}
+ checkPath := func(ctx context.Context, t *testing.T, testDB *postgres.DB, path, wantPath string, status int) {
+ got, err := previousFetchStatusAndResponse(ctx, testDB, path, internal.LatestVersion)
+ if err != nil {
+ t.Fatal(err)
+ }
+ want := &fetchResult{
+ modulePath: wantPath,
+ goModPath: wantPath,
+ status: status,
+ }
+ if diff := cmp.Diff(want, got,
+ cmp.AllowUnexported(fetchResult{}),
+ cmpopts.IgnoreFields(fetchResult{}, "responseText")); diff != "" {
+ t.Errorf("mismatch (-want, +got):\n%s", diff)
+ }
+ }
+
for _, test := range []struct {
name, path, want string
}{
@@ -127,18 +148,12 @@
{"import path not at v1", sample.ModulePath + "/foo", sample.ModulePath + "/v4/foo"},
} {
t.Run(test.name, func(t *testing.T) {
- got, err := previousFetchStatusAndResponse(ctx, testDB, test.path, internal.LatestVersion)
- if err != nil {
- t.Fatal(err)
- }
- want := &fetchResult{
- modulePath: test.want,
- goModPath: test.want,
- status: http.StatusFound,
- }
- if diff := cmp.Diff(want, got, cmp.AllowUnexported(fetchResult{})); diff != "" {
- t.Errorf("mismatch (-want, +got):\n%s", diff)
- }
+ want := strings.ReplaceAll(test.want, "/v4", "")
+ checkPath(ctx, t, testDB, test.path, want, http.StatusNotFound)
+ })
+ t.Run(test.name+"not-at-v1", func(t *testing.T) {
+ ctx := experiment.NewContext(ctx, internal.ExperimentNotAtV1)
+ checkPath(ctx, t, testDB, test.path, test.want, http.StatusFound)
})
}
}
diff --git a/internal/frontend/server_test.go b/internal/frontend/server_test.go
index e82b4d2..06b6df6 100644
--- a/internal/frontend/server_test.go
+++ b/internal/frontend/server_test.go
@@ -1226,7 +1226,8 @@
}
}
- _, handler, _ := newTestServer(t, nil)
+ _, handler, _ := newTestServer(t, nil, internal.ExperimentNotAtV1)
+ _, handlerNoExp, _ := newTestServer(t, nil)
for _, test := range []struct {
name, path string
@@ -1244,6 +1245,14 @@
if w.Code != test.wantCode {
t.Errorf("%q: got status code = %d, want %d", test.path, w.Code, test.wantCode)
}
+
+ handlerNoExp.ServeHTTP(w, httptest.NewRequest("GET", test.path, nil))
+ if test.path == v1modpath || test.path == v1path {
+ test.wantCode = http.StatusNotFound
+ }
+ if w.Code != test.wantCode {
+ t.Errorf("%q: got status code = %d, want %d", test.path, w.Code, test.wantCode)
+ }
})
}
}
