internal/frontend,etc.: optimize away second GetUnitMeta call

If the frontend has called GetUnitMeta(unknown, latest), then
the GetLatestInfo doesn't have to repeat that call.

Change-Id: I61d1b733ded5a588468b8d18db1b66bd027c0fec
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/306594
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>

internal/datasource.go

@@ -24,7 +24,7 @@ type DataSource interface {
 
 	// GetLatestInfo gets information about the latest versions of a unit and module.
 	// See LatestInfo for documentation.
-	GetLatestInfo(ctx context.Context, unitPath, modulePath string) (LatestInfo, error)
+	GetLatestInfo(ctx context.Context, unitPath, modulePath string, latestUnitMeta *UnitMeta) (LatestInfo, error)
 }
 
 // LatestInfo holds information about the latest versions and paths.

internal/frontend/latest_version.go

@@ -18,12 +18,12 @@ import (
 //    fullPath and the modulePath.
 // It returns empty strings on error.
 // It is intended to be used as an argument to middleware.LatestVersions.
-func (s *Server) GetLatestInfo(ctx context.Context, unitPath, modulePath string) internal.LatestInfo {
+func (s *Server) GetLatestInfo(ctx context.Context, unitPath, modulePath string, latestUnitMeta *internal.UnitMeta) internal.LatestInfo {
 	// It is okay to use a different DataSource (DB connection) than the rest of the
 	// request, because this makes self-contained calls on the DB.
 	ds := s.getDataSource(ctx)
 
-	latest, err := ds.GetLatestInfo(ctx, unitPath, modulePath)
+	latest, err := ds.GetLatestInfo(ctx, unitPath, modulePath, latestUnitMeta)
 	if err != nil {
 		log.Errorf(ctx, "Server.GetLatestInfo: %v", err)
 	} else {

internal/frontend/latest_version_test.go

@@ -61,7 +61,7 @@ func TestLatestMinorVersion(t *testing.T) {
 	svr := &Server{getDataSource: func(context.Context) internal.DataSource { return testDB }}
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
-			got := svr.GetLatestInfo(ctx, tc.fullPath, tc.modulePath)
+			got := svr.GetLatestInfo(ctx, tc.fullPath, tc.modulePath, nil)
 			if got.MinorVersion != tc.wantMinorVersion {
 				t.Fatalf("got %q, want %q", tc.wantMinorVersion, got.MinorVersion)
 			}

internal/frontend/unit.go

@@ -159,7 +159,13 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht
 		return nil
 	}
 
-	latestInfo := s.GetLatestInfo(ctx, um.Path, um.ModulePath)
+	// 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 == internal.LatestVersion {
+		latestUnitMeta = um
+	}
+	latestInfo := s.GetLatestInfo(ctx, um.Path, um.ModulePath, latestUnitMeta)
 	var redirectPath string
 	redirectPath, err = cookie.Extract(w, r, cookie.AlternativeModuleFlash)
 	if err != nil {

internal/localdatasource/datasource.go

@@ -179,7 +179,7 @@ func (ds *DataSource) findModule(pkgPath string) (_ string, err error) {
 }
 
 // GetLatestInfo is not implemented.
-func (ds *DataSource) GetLatestInfo(ctx context.Context, unitPath, modulePath string) (internal.LatestInfo, error) {
+func (ds *DataSource) GetLatestInfo(ctx context.Context, unitPath, modulePath string, latestUnitMeta *internal.UnitMeta) (internal.LatestInfo, error) {
 	return internal.LatestInfo{}, nil
 }
 

internal/postgres/version.go

@@ -159,20 +159,27 @@ func populateLatestInfos(ctx context.Context, db *DB, mis []*internal.ModuleInfo
 
 // GetLatestInfo returns the latest information about the unit in the module.
 // See internal.LatestInfo for documentation about the returned values.
-func (db *DB) GetLatestInfo(ctx context.Context, unitPath, modulePath string) (latest internal.LatestInfo, err error) {
+// If latestUnitMeta is non-nil, it is the result of GetUnitMeta(unitPath, internal.UnknownModulePath, internal.LatestVersion).
+// That can save a redundant call to GetUnitMeta here.
+func (db *DB) GetLatestInfo(ctx context.Context, unitPath, modulePath string, latestUnitMeta *internal.UnitMeta) (latest internal.LatestInfo, err error) {
 	defer derrors.WrapStack(&err, "DB.GetLatestInfo(ctx, %q, %q)", unitPath, modulePath)
 
 	group, gctx := errgroup.WithContext(ctx)
 
-	group.Go(func() error {
-		um, err := db.GetUnitMeta(gctx, unitPath, internal.UnknownModulePath, internal.LatestVersion)
-		if err != nil {
-			return err
-		}
-		latest.MinorVersion = um.Version
-		latest.MinorModulePath = um.ModulePath
-		return nil
-	})
+	if latestUnitMeta != nil {
+		latest.MinorVersion = latestUnitMeta.Version
+		latest.MinorModulePath = latestUnitMeta.ModulePath
+	} else {
+		group.Go(func() error {
+			um, err := db.GetUnitMeta(gctx, unitPath, internal.UnknownModulePath, internal.LatestVersion)
+			if err != nil {
+				return err
+			}
+			latest.MinorVersion = um.Version
+			latest.MinorModulePath = um.ModulePath
+			return nil
+		})
+	}
 	group.Go(func() (err error) {
 		latest.MajorModulePath, latest.MajorUnitPath, err = db.getLatestMajorVersion(gctx, unitPath, modulePath)
 		return err

internal/postgres/version_test.go

@@ -386,7 +386,7 @@ func TestGetLatestInfo(t *testing.T) {
 		},
 	} {
 		t.Run(test.unit, func(t *testing.T) {
-			got, err := testDB.GetLatestInfo(ctx, test.unit, test.module)
+			got, err := testDB.GetLatestInfo(ctx, test.unit, test.module, nil)
 			if err != nil {
 				t.Fatal(err)
 			}

internal/proxydatasource/datasource.go

@@ -193,15 +193,17 @@ func (ds *DataSource) getUnit(ctx context.Context, fullPath, modulePath, version
 }
 
 // GetLatestInfo returns latest information for unitPath and modulePath.
-func (ds *DataSource) GetLatestInfo(ctx context.Context, unitPath, modulePath string) (latest internal.LatestInfo, err error) {
+func (ds *DataSource) GetLatestInfo(ctx context.Context, unitPath, modulePath string, latestUnitMeta *internal.UnitMeta) (latest internal.LatestInfo, err error) {
 	defer derrors.Wrap(&err, "GetLatestInfo(ctx, %q, %q)", unitPath, modulePath)
 
-	um, err := ds.GetUnitMeta(ctx, unitPath, internal.UnknownModulePath, internal.LatestVersion)
-	if err != nil {
-		return latest, err
+	if latestUnitMeta == nil {
+		latestUnitMeta, err = ds.GetUnitMeta(ctx, unitPath, internal.UnknownModulePath, internal.LatestVersion)
+		if err != nil {
+			return latest, err
+		}
 	}
-	latest.MinorVersion = um.Version
-	latest.MinorModulePath = um.ModulePath
+	latest.MinorVersion = latestUnitMeta.Version
+	latest.MinorModulePath = latestUnitMeta.ModulePath
 
 	latest.MajorModulePath, latest.MajorUnitPath, err = ds.getLatestMajorVersion(ctx, unitPath, modulePath)
 	if err != nil {

internal/proxydatasource/datasource_test.go

@@ -312,7 +312,7 @@ func TestGetLatestInfo(t *testing.T) {
 			wantPackagePath: "incompatible.com/bar/v3",
 		},
 	} {
-		gotLatest, err := ds.GetLatestInfo(ctx, test.fullPath, test.modulePath)
+		gotLatest, err := ds.GetLatestInfo(ctx, test.fullPath, test.modulePath, nil)
 		if err != nil {
 			if test.wantErr == nil {
 				t.Fatalf("got unexpected error %v", err)