content,internal/frontend: update count on importedby page

When loading the imported by page, fetch the imported by count from
search_documents.num_imported_by, instead of limiting at 20,000.

This will use the same logic for the imported by count with the
main page and search page.

For golang/go#39138

Change-Id: Iae188d01129b6e35cfd82b79e51a0ef12938c459
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/288849
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>

content/static/html/helpers/_importedby.tmpl

@@ -7,9 +7,7 @@
 {{define "importedby"}}
   <div class="ImportedBy">
     {{if .ImportedBy}}
-      <p>
-        <b>Known {{pluralize .Total "importer"}}:</b> {{.Total}}{{if not .TotalIsExact}}+{{end}}
-      </p>
+      <b>Known {{pluralize .Total "importer"}}:</b> {{.NumImportedByDisplay}}
       {{template "sections" .ImportedBy}}
     {{else}}
       {{template "empty_content" "No known importers for this package!"}}

internal/frontend/imports.go

@@ -6,6 +6,7 @@ package frontend
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
 	"golang.org/x/pkgsite/internal"
@@ -64,6 +65,7 @@ func fetchImportsDetails(ctx context.Context, ds internal.DataSource, pkgPath, m
 // ImportedByDetails contains information for the collection of packages that
 // import a given package.
 type ImportedByDetails struct {
+	// ModulePath is the module path for the package referenced on this page.
 	ModulePath string
 
 	// ImportedBy is the collection of packages that import the
@@ -71,8 +73,12 @@ type ImportedByDetails struct {
 	// They are organized into a tree of sections by prefix.
 	ImportedBy []*Section
 
-	Total        int  // number of packages in ImportedBy
-	TotalIsExact bool // if false, then there may be more than Total
+	// NumImportedByDisplay is the display text at the top of the imported by
+	// tab section, which shows the imported by count and package limit.
+	NumImportedByDisplay string
+
+	// Total is the total number of importers.
+	Total int
 }
 
 var (
@@ -97,22 +103,20 @@ func fetchImportedByDetails(ctx context.Context, ds internal.DataSource, pkgPath
 	if err != nil {
 		return nil, err
 	}
-	importedByCount := len(importedBy)
-
-	// If we reached the query limit, then we don't know the total.
-	// Say so, and show one less than the limit.
-	// For example, if the limit is 101 and we get 101 results, then we'll
-	// say there are more than 100, and show the first 100.
-	totalIsExact := true
-	if importedByCount == tabImportedByLimit {
-		importedBy = importedBy[:len(importedBy)-1]
-		totalIsExact = false
+	numImportedBy, err := db.GetImportedByCount(ctx, pkgPath, modulePath)
+	if err != nil {
+		return nil, err
 	}
 	sections := Sections(importedBy, nextPrefixAccount)
+
+	display := formatImportedByCount(numImportedBy)
+	if numImportedBy >= tabImportedByLimit {
+		display += fmt.Sprintf(" (displaying %d packages)", tabImportedByLimit-1)
+	}
 	return &ImportedByDetails{
-		ModulePath:   modulePath,
-		ImportedBy:   sections,
-		Total:        importedByCount,
-		TotalIsExact: totalIsExact,
+		ModulePath:           modulePath,
+		ImportedBy:           sections,
+		NumImportedByDisplay: display,
+		Total:                numImportedBy,
 	}, nil
 }

internal/frontend/imports_test.go

@@ -6,7 +6,9 @@ package frontend
 
 import (
 	"context"
+	"fmt"
 	"path"
+	"strconv"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -110,15 +112,16 @@ func TestFetchImportedByDetails(t *testing.T) {
 		wantDetails *ImportedByDetails
 	}{
 		{
-			pkg:         pkg3,
-			wantDetails: &ImportedByDetails{TotalIsExact: true},
+			pkg: pkg3,
+			wantDetails: &ImportedByDetails{
+				NumImportedByDisplay: "0",
+			},
 		},
 		{
 			pkg: pkg2,
 			wantDetails: &ImportedByDetails{
-				ImportedBy:   []*Section{{Prefix: pkg3.Path, NumLines: 0}},
-				Total:        1,
-				TotalIsExact: true,
+				ImportedBy:           []*Section{{Prefix: pkg3.Path, NumLines: 0}},
+				NumImportedByDisplay: "0",
 			},
 		},
 		{
@@ -128,29 +131,58 @@ func TestFetchImportedByDetails(t *testing.T) {
 					{Prefix: pkg2.Path, NumLines: 0},
 					{Prefix: pkg3.Path, NumLines: 0},
 				},
-				Total:        2,
-				TotalIsExact: true,
+				NumImportedByDisplay: "0",
 			},
 		},
 	}
 
-	checkFetchImportedByDetails := func(ctx context.Context, pkg *internal.Unit, wantDetails *ImportedByDetails) {
-		got, err := fetchImportedByDetails(ctx, testDB, pkg.Path, pkg.ModulePath)
-		if err != nil {
-			t.Fatalf("fetchImportedByDetails(ctx, db, %q) = %v err = %v, want %v",
-				pkg.Path, got, err, wantDetails)
-		}
-		wantDetails.ModulePath = pkg.ModulePath
-		if diff := cmp.Diff(wantDetails, got); diff != "" {
-			t.Errorf("fetchImportedByDetails(ctx, db, %q) mismatch (-want +got):\n%s", pkg.Path, diff)
-		}
-	}
 	for _, test := range tests {
 		t.Run(test.pkg.Path, func(t *testing.T) {
 			otherVersion := newModule(path.Dir(test.pkg.Path), test.pkg)
 			otherVersion.Version = "v1.0.5"
 			pkg := otherVersion.Units[1]
-			checkFetchImportedByDetails(ctx, pkg, test.wantDetails)
+			checkFetchImportedByDetails(ctx, t, pkg, test.wantDetails)
 		})
 	}
 }
+
+func TestFetchImportedByDetails_ExceedsTabLimit(t *testing.T) {
+	defer postgres.ResetTestDB(testDB, t)
+	ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
+	defer cancel()
+
+	for _, count := range []int{tabImportedByLimit, 30000} {
+		t.Run(strconv.Itoa(count), func(t *testing.T) {
+			args := postgres.UpsertSearchDocumentArgs{
+				PackagePath: sample.PackagePath,
+				ModulePath:  sample.ModulePath,
+			}
+			if err := testDB.InsertModule(ctx, sample.Module(sample.ModulePath, sample.VersionString, sample.PackageName)); err != nil {
+				t.Fatal(err)
+			}
+			if err := testDB.UpsertSearchDocumentWithImportedByCount(ctx, args, count); err != nil {
+				t.Fatal(err)
+			}
+
+			pkg := sample.UnitForPackage(sample.PackagePath, sample.ModulePath, sample.VersionString, sample.PackageName, true)
+			wantDetails := &ImportedByDetails{
+				ModulePath:           sample.ModulePath,
+				NumImportedByDisplay: fmt.Sprintf("%s (displaying 20000 packages)", formatImportedByCount(count)),
+				Total:                count,
+			}
+			checkFetchImportedByDetails(ctx, t, pkg, wantDetails)
+		})
+	}
+}
+
+func checkFetchImportedByDetails(ctx context.Context, t *testing.T, pkg *internal.Unit, wantDetails *ImportedByDetails) {
+	got, err := fetchImportedByDetails(ctx, testDB, pkg.Path, pkg.ModulePath)
+	if err != nil {
+		t.Fatalf("fetchImportedByDetails(ctx, db, %q) = %v err = %v, want %v",
+			pkg.Path, got, err, wantDetails)
+	}
+	wantDetails.ModulePath = pkg.ModulePath
+	if diff := cmp.Diff(wantDetails, got); diff != "" {
+		t.Errorf("fetchImportedByDetails(ctx, db, %q) mismatch (-want +got):\n%s", pkg.Path, diff)
+	}
+}

internal/postgres/insert_module.go

@@ -148,7 +148,7 @@ func (db *DB) saveModule(ctx context.Context, m *internal.Module) (err error) {
 			return err
 		}
 		// Insert the module's packages into search_documents.
-		return db.upsertSearchDocuments(ctx, tx, m)
+		return upsertSearchDocuments(ctx, tx, m)
 	})
 }
 

internal/postgres/search.go

@@ -452,7 +452,7 @@ var upsertSearchStatement = fmt.Sprintf(`
 
 // upsertSearchDocuments adds search information for mod ot the search_documents table.
 // It assumes that all non-redistributable data has been removed from mod.
-func (db *DB) upsertSearchDocuments(ctx context.Context, ddb *database.DB, mod *internal.Module) (err error) {
+func upsertSearchDocuments(ctx context.Context, ddb *database.DB, mod *internal.Module) (err error) {
 	defer derrors.Wrap(&err, "upsertSearchDocuments(ctx, %q)", mod.ModulePath)
 	ctx, span := trace.StartSpan(ctx, "UpsertSearchDocuments")
 	defer span.End()
@@ -460,7 +460,7 @@ func (db *DB) upsertSearchDocuments(ctx context.Context, ddb *database.DB, mod *
 		if isInternalPackage(pkg.Path) {
 			continue
 		}
-		args := upsertSearchDocumentArgs{
+		args := UpsertSearchDocumentArgs{
 			PackagePath: pkg.Path,
 			ModulePath:  mod.ModulePath,
 		}
@@ -472,14 +472,14 @@ func (db *DB) upsertSearchDocuments(ctx context.Context, ddb *database.DB, mod *
 			args.ReadmeFilePath = pkg.Readme.Filepath
 			args.ReadmeContents = pkg.Readme.Contents
 		}
-		if err := db.UpsertSearchDocument(ctx, ddb, args); err != nil {
+		if err := UpsertSearchDocument(ctx, ddb, args); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-type upsertSearchDocumentArgs struct {
+type UpsertSearchDocumentArgs struct {
 	PackagePath    string
 	ModulePath     string
 	Synopsis       string
@@ -492,7 +492,7 @@ type upsertSearchDocumentArgs struct {
 //
 // The given module should have already been validated via a call to
 // validateModule.
-func (db *DB) UpsertSearchDocument(ctx context.Context, ddb *database.DB, args upsertSearchDocumentArgs) (err error) {
+func UpsertSearchDocument(ctx context.Context, ddb *database.DB, args UpsertSearchDocumentArgs) (err error) {
 	defer derrors.Wrap(&err, "DB.UpsertSearchDocument(ctx, ddb, %q, %q)", args.PackagePath, args.ModulePath)
 
 	// Only summarize the README if the package and module have the same path.
@@ -508,7 +508,7 @@ func (db *DB) UpsertSearchDocument(ctx context.Context, ddb *database.DB, args u
 
 // GetPackagesForSearchDocumentUpsert fetches search information for packages in search_documents
 // whose update time is before the given time.
-func (db *DB) GetPackagesForSearchDocumentUpsert(ctx context.Context, before time.Time, limit int) (argsList []upsertSearchDocumentArgs, err error) {
+func (db *DB) GetPackagesForSearchDocumentUpsert(ctx context.Context, before time.Time, limit int) (argsList []UpsertSearchDocumentArgs, err error) {
 	defer derrors.Wrap(&err, "GetPackagesForSearchDocumentUpsert(ctx, %s, %d)", before, limit)
 
 	query := `
@@ -535,7 +535,7 @@ func (db *DB) GetPackagesForSearchDocumentUpsert(ctx context.Context, before tim
 
 	collect := func(rows *sql.Rows) error {
 		var (
-			a      upsertSearchDocumentArgs
+			a      UpsertSearchDocumentArgs
 			redist bool
 		)
 		if err := rows.Scan(&a.PackagePath, &a.ModulePath, &a.Synopsis, &redist,
@@ -875,3 +875,17 @@ func (db *DB) DeleteOlderVersionFromSearchDocuments(ctx context.Context, moduleP
 		return nil
 	})
 }
+
+// UpsertSearchDocumentWithImportedByCount is the same as UpsertSearchDocument,
+// except it also updates the imported by count. This is only used for testing.
+func (db *DB) UpsertSearchDocumentWithImportedByCount(ctx context.Context, args UpsertSearchDocumentArgs, importedByCount int) (err error) {
+	defer derrors.Wrap(&err, "DB.UpsertSearchDocumentWithImportedByCount(ctx, ddb, %q, %q)", args.PackagePath, args.ModulePath)
+
+	if err := UpsertSearchDocument(ctx, db.db, args); err != nil {
+		return err
+	}
+	_, err = db.db.Exec(ctx,
+		`UPDATE search_documents SET imported_by_count=$1 WHERE package_path=$2;`,
+		importedByCount, args.PackagePath)
+	return err
+}

internal/postgres/search_test.go

@@ -989,7 +989,7 @@ func TestGetPackagesForSearchDocumentUpsert(t *testing.T) {
 		t.Fatal(err)
 	}
 	sort.Slice(got, func(i, j int) bool { return got[i].PackagePath < got[j].PackagePath })
-	want := []upsertSearchDocumentArgs{
+	want := []UpsertSearchDocumentArgs{
 		{
 			PackagePath:    moduleN.ModulePath,
 			ModulePath:     moduleN.ModulePath,

internal/worker/server.go

@@ -244,7 +244,7 @@ func (s *Server) handleRepopulateSearchDocuments(w http.ResponseWriter, r *http.
 	}
 
 	for _, args := range sdargs {
-		if err := s.db.UpsertSearchDocument(ctx, s.db.Underlying(), args); err != nil {
+		if err := postgres.UpsertSearchDocument(ctx, s.db.Underlying(), args); err != nil {
 			return err
 		}
 	}