internal/{frontend,postgres}: add GetSymbolHistory

postgres.GetPackageSymbol and symbol.IntroducedHistory are factored into a single GetSymbolHistory function, in preparation for optimizing this logic. For golang/go#37102 Change-Id: I04f726ae6f9731ee2e099b47e414aeaef90d5779 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/312049 Trust: Julie Qiu <julie@golang.org> Reviewed-by: Jonathan Amsterdam <jba@google.com>
golang · Apr 21, 2021 · 754bc2c · 754bc2c
1 parent 8185298
commit 754bc2c
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 14 deletions.
diff --git a/internal/frontend/main.go b/internal/frontend/main.go
@@ -19,7 +19,6 @@ import (
 	"golang.org/x/pkgsite/internal/log"
 	"golang.org/x/pkgsite/internal/middleware"
 	"golang.org/x/pkgsite/internal/postgres"
-	"golang.org/x/pkgsite/internal/symbol"
 	"golang.org/x/pkgsite/internal/version"
 )
 
@@ -144,11 +143,10 @@ func fetchMainDetails(ctx context.Context, ds internal.DataSource, um *internal.
 		if !ok {
 			return nil, proxydatasourceNotSupportedErr()
 		}
-		versionToNameToUnitSymbols, err := db.GetPackageSymbols(ctx, um.Path, um.ModulePath)
+		versionToNameToUnitSymbol, err = db.GetSymbolHistory(ctx, um.Path, um.ModulePath)
 		if err != nil {
 			return nil, err
 		}
-		versionToNameToUnitSymbol = symbol.IntroducedHistory(versionToNameToUnitSymbols)
 	}
 	nameToVersion := map[string]string{}
 	for v, nts := range versionToNameToUnitSymbol {

diff --git a/internal/frontend/versions.go b/internal/frontend/versions.go
@@ -18,7 +18,6 @@ import (
 	"golang.org/x/pkgsite/internal/log"
 	"golang.org/x/pkgsite/internal/postgres"
 	"golang.org/x/pkgsite/internal/stdlib"
-	"golang.org/x/pkgsite/internal/symbol"
 	"golang.org/x/pkgsite/internal/version"
 )
 
@@ -95,11 +94,10 @@ func fetchVersionsDetails(ctx context.Context, ds internal.DataSource, fullPath,
 
 	outVersionToNameToUnitSymbol := map[string]map[string]*internal.UnitSymbol{}
 	if experiment.IsActive(ctx, internal.ExperimentSymbolHistoryVersionsPage) {
-		versionToNameToUnitSymbols, err := db.GetPackageSymbols(ctx, fullPath, modulePath)
+		outVersionToNameToUnitSymbol, err = db.GetSymbolHistory(ctx, fullPath, modulePath)
 		if err != nil {
 			return nil, err
 		}
-		outVersionToNameToUnitSymbol = symbol.IntroducedHistory(versionToNameToUnitSymbols)
 	}
 	linkify := func(mi *internal.ModuleInfo) string {
 		// Here we have only version information, but need to construct the full

diff --git a/internal/postgres/symbol.go b/internal/postgres/symbol.go
@@ -390,7 +390,7 @@ func (db *DB) CompareStdLib(ctx context.Context) (map[string][]string, error) {
 	}
 	pkgToErrors := map[string][]string{}
 	for path := range apiVersions {
-		versionToNameToSymbol, err := db.GetPackageSymbols(ctx, path, stdlib.ModulePath)
+		versionToNameToSymbol, err := getPackageSymbols(ctx, db.db, path, stdlib.ModulePath)
 		if err != nil {
 			return nil, err
 		}

diff --git a/internal/postgres/symbol_history.go b/internal/postgres/symbol_history.go
@@ -10,16 +10,32 @@ import (
 	"fmt"
 
 	"golang.org/x/pkgsite/internal"
+	"golang.org/x/pkgsite/internal/database"
 	"golang.org/x/pkgsite/internal/derrors"
 	"golang.org/x/pkgsite/internal/middleware"
+	"golang.org/x/pkgsite/internal/symbol"
 )
 
-// GetPackageSymbols returns all of the symbols for a given package path and module path.
-func (db *DB) GetPackageSymbols(ctx context.Context, packagePath, modulePath string,
-) (_ map[string]map[string]*internal.UnitSymbol, err error) {
-	defer derrors.Wrap(&err, "GetPackageSymbols(ctx, db, %q, %q)", packagePath, modulePath)
-	defer middleware.ElapsedStat(ctx, "GetPackageSymbols")()
+// GetSymbolHistory returns a map of the first version when a symbol name is
+// added to the API, to the symbol name, to the UnitSymbol struct. The
+// UnitSymbol.Children field will always be empty, as children names are also
+// tracked.
+func (db *DB) GetSymbolHistory(ctx context.Context, packagePath, modulePath string,
+) (outVersionToNameToUnitSymbol map[string]map[string]*internal.UnitSymbol, err error) {
+	defer derrors.Wrap(&err, "GetSymbolHistory(ctx, %q, %q)", packagePath, modulePath)
+	defer middleware.ElapsedStat(ctx, "GetSymbolHistory")()
+	versionToNameToUnitSymbols, err := getPackageSymbols(ctx, db.db, packagePath, modulePath)
+	if err != nil {
+		return nil, err
+	}
+	return symbol.IntroducedHistory(versionToNameToUnitSymbols), nil
+}
 
+// getPackageSymbols returns all of the symbols for a given package path and module path.
+func getPackageSymbols(ctx context.Context, ddb *database.DB, packagePath, modulePath string,
+) (_ map[string]map[string]*internal.UnitSymbol, err error) {
+	defer derrors.Wrap(&err, "getPackageSymbols(ctx, ddb, %q, %q)", packagePath, modulePath)
+	defer middleware.ElapsedStat(ctx, "getPackageSymbols")()
 	query := `
 		SELECT
 			s1.name AS symbol_name,
@@ -86,7 +102,7 @@ func (db *DB) GetPackageSymbols(ctx context.Context, packagePath, modulePath str
 		us.AddBuildContext(build)
 		return nil
 	}
-	if err := db.db.RunQuery(ctx, query, collect, packagePath, modulePath); err != nil {
+	if err := ddb.RunQuery(ctx, query, collect, packagePath, modulePath); err != nil {
 		return nil, err
 	}
 	return versionToNameToUnitSymbol, nil

diff --git a/internal/postgres/symbol_test.go b/internal/postgres/symbol_test.go
@@ -336,7 +336,7 @@ func compareUnitSymbols(ctx context.Context, t *testing.T, testDB *DB,
 func comparePackageSymbols(ctx context.Context, t *testing.T, testDB *DB,
 	path, modulePath, version string, want map[string]map[string]*internal.UnitSymbol) {
 	t.Helper()
-	got, err := testDB.GetPackageSymbols(ctx, path, modulePath)
+	got, err := getPackageSymbols(ctx, testDB.db, path, modulePath)
 	if err != nil {
 		t.Fatal(err)
 	}