internal/frontend: use paths table to check for existence

A new experiment is added, ExperimentUsePathInfoToCheckExistence, which uses the paths table to check if a package/module/directory exists, regardless of whether we are using the old or new data model to render the pages. This is a first step to use the new paths-based data model. It also streamlines the flow for frontend fetches. Updates golang/go#39629 Change-Id: I6178776264cbe71ffb3fb87b3409df902b33b2d4 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/240680 Reviewed-by: Jonathan Amsterdam <[email protected]>
namaggarwal · Jul 1, 2020 · 6fed0de · 6fed0de
1 parent 993436f
commit 6fed0de
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 25 deletions.
diff --git a/internal/experiment.go b/internal/experiment.go
@@ -16,6 +16,7 @@ const (
 	ExperimentInsertSerializable          = "insert-serializable-txn"
 	ExperimentTeeProxyMakePkgGoDevRequest = "teeproxy-make-pkg-go-dev-request"
 	ExperimentUseDirectories              = "use-directories"
+	ExperimentUsePathInfoToCheckExistence = "use-path-info-to-check-existence"
 	ExperimentTranslateHTML               = "translate-html"
 )
 

diff --git a/internal/frontend/details.go b/internal/frontend/details.go
@@ -6,6 +6,7 @@ package frontend
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -85,22 +86,43 @@ func (s *Server) serveDetails(w http.ResponseWriter, r *http.Request) (err error
 
 	ctx := r.Context()
 	// Validate the fullPath and requestedVersion that were parsed.
-	if err := checkPathAndVersion(ctx, s.ds, fullPath, requestedVersion); err != nil {
+	if err := validatePathAndVersion(ctx, s.ds, fullPath, requestedVersion); err != nil {
 		return err
 	}
+	var resolvedModulePath string
+	if experiment.IsActive(ctx, internal.ExperimentUsePathInfoToCheckExistence) {
+		resolvedModulePath, _, _, err = s.ds.GetPathInfo(ctx, fullPath, modulePath, requestedVersion)
+		if err != nil {
+			if !errors.Is(err, derrors.NotFound) {
+				return err
+			}
+			// TODO(golang/go#39663) add a case for this to TestServer
+			path, err := s.stdlibPathForShortcut(ctx, fullPath)
+			if err != nil {
+				// Log the error, but prefer a "path not found" error for a
+				// better user experience.
+				log.Error(ctx, err)
+			}
+			if path != "" {
+				http.Redirect(w, r, path, http.StatusFound)
+				return nil
+			}
+			pathType := "package"
+			if isModule || fullPath == stdlib.ModulePath {
+				pathType = "module"
+			}
+			return pathNotFoundError(ctx, pathType, fullPath, requestedVersion)
+		}
+	}
 	if isActivePathAtMaster(ctx) && requestedVersion == internal.MasterVersion {
 		// 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
 		// last time we tried to fetch this module version.
 		go func() {
-			status, responseText := s.fetchAndPoll(r.Context(), modulePath, fullPath, requestedVersion)
-			logf := log.Infof
-			if status == http.StatusInternalServerError {
-				logf = log.Errorf
+			if err := s.queue.ScheduleFetch(ctx, resolvedModulePath, internal.MasterVersion, "", s.taskIDChangeInterval); err != nil {
+				log.Errorf(ctx, "serveDetails(%q): %v", r.URL.Path, err)
 			}
-			logf(ctx, "fetchAndPoll(%q, %q, %q) result from serveDetails(%q): %d %q",
-				modulePath, fullPath, requestedVersion, r.URL.Path, status, responseText)
 		}()
 	}
 	// Depending on what the request was for, return the module or package page.
@@ -191,9 +213,9 @@ func parseDetailsURLPath(urlPath string) (fullPath, modulePath, version string,
 	return fullPath, modulePath, version, nil
 }
 
-// checkPathAndVersion verifies that the requested path and version are
+// validatePathAndVersion verifies that the requested path and version are
 // acceptable. The given path may be a module or package path.
-func checkPathAndVersion(ctx context.Context, ds internal.DataSource, fullPath, requestedVersion string) error {
+func validatePathAndVersion(ctx context.Context, ds internal.DataSource, fullPath, requestedVersion string) error {
 	if !isSupportedVersion(ctx, requestedVersion) {
 		return &serverError{
 			status: http.StatusBadRequest,

diff --git a/internal/frontend/details_test.go b/internal/frontend/details_test.go
@@ -101,7 +101,7 @@ func TestParseDetailsURLPath(t *testing.T) {
 	}
 }
 
-func TestCheckPathAndVersion(t *testing.T) {
+func TestValidatePathAndVersion(t *testing.T) {
 	tests := []struct {
 		path, version string
 		want          int
@@ -111,7 +111,7 @@ func TestCheckPathAndVersion(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		err := checkPathAndVersion(context.Background(), fakeDataSource{}, test.path, test.version)
+		err := validatePathAndVersion(context.Background(), fakeDataSource{}, test.path, test.version)
 		var got int
 		if err == nil {
 			got = 200
@@ -121,7 +121,7 @@ func TestCheckPathAndVersion(t *testing.T) {
 			got = -1
 		}
 		if got != test.want {
-			t.Errorf("checkPathAndVersion(ctx, ds, %q, %q): got code %d, want %d", test.path, test.version, got, test.want)
+			t.Errorf("validatePathAndVersion(ctx, ds, %q, %q): got code %d, want %d", test.path, test.version, got, test.want)
 		}
 	}
 }

diff --git a/internal/frontend/fetch.go b/internal/frontend/fetch.go
@@ -466,7 +466,8 @@ func FetchAndUpdateState(ctx context.Context, modulePath, requestedVersion strin
 
 func isActiveFrontendFetch(ctx context.Context) bool {
 	return experiment.IsActive(ctx, internal.ExperimentFrontendFetch) &&
-		experiment.IsActive(ctx, internal.ExperimentInsertDirectories)
+		experiment.IsActive(ctx, internal.ExperimentInsertDirectories) &&
+		experiment.IsActive(ctx, internal.ExperimentUsePathInfoToCheckExistence)
 }
 
 func recordFrontendFetchMetric(status int, version string, latency time.Duration) {

diff --git a/internal/frontend/package.go b/internal/frontend/package.go
@@ -147,18 +147,6 @@ func (s *Server) servePackagePageNew(w http.ResponseWriter, r *http.Request, ful
 			if !isActiveUseDirectories(ctx) {
 				return pathNotFoundError(ctx, "package", fullPath, inVersion)
 			}
-			// TODO(golang/go#39663) add a case for this to TestServer, after we
-			// switch over to the paths-based data model.
-			path, err := s.stdlibPathForShortcut(ctx, fullPath)
-			if path == "" {
-				if err != nil {
-					// Log the error, but prefer a "path not found" error for a better user experience.
-					log.Error(ctx, err)
-				}
-				return pathNotFoundError(ctx, "package", fullPath, inVersion)
-			}
-			http.Redirect(w, r, path, http.StatusFound)
-			return nil
 		}
 		// We couldn't find a path at the given version, but if there's one at the latest version
 		// we can provide a link to it.