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>
diff --git a/content/static/html/helpers/_importedby.tmpl b/content/static/html/helpers/_importedby.tmpl
index 8cff4f2..80022cb 100644
--- a/content/static/html/helpers/_importedby.tmpl
+++ b/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!"}}
diff --git a/internal/frontend/imports.go b/internal/frontend/imports.go
index eef9709..286c0eb 100644
--- a/internal/frontend/imports.go
+++ b/internal/frontend/imports.go
@@ -6,6 +6,7 @@
import (
"context"
+ "fmt"
"strings"
"golang.org/x/pkgsite/internal"
@@ -64,6 +65,7 @@
// 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 @@
// 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 @@
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
}
diff --git a/internal/frontend/imports_test.go b/internal/frontend/imports_test.go
index e93cc6d..dccf731 100644
--- a/internal/frontend/imports_test.go
+++ b/internal/frontend/imports_test.go
@@ -6,7 +6,9 @@
import (
"context"
+ "fmt"
"path"
+ "strconv"
"testing"
"github.com/google/go-cmp/cmp"
@@ -110,15 +112,16 @@
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 @@
{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)
+ }
+}
diff --git a/internal/postgres/insert_module.go b/internal/postgres/insert_module.go
index a241fc1..fad2ae4 100644
--- a/internal/postgres/insert_module.go
+++ b/internal/postgres/insert_module.go
@@ -148,7 +148,7 @@
return err
}
// Insert the module's packages into search_documents.
- return db.upsertSearchDocuments(ctx, tx, m)
+ return upsertSearchDocuments(ctx, tx, m)
})
}
diff --git a/internal/postgres/search.go b/internal/postgres/search.go
index decba91..bfd2b76 100644
--- a/internal/postgres/search.go
+++ b/internal/postgres/search.go
@@ -452,7 +452,7 @@
// 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 @@
if isInternalPackage(pkg.Path) {
continue
}
- args := upsertSearchDocumentArgs{
+ args := UpsertSearchDocumentArgs{
PackagePath: pkg.Path,
ModulePath: mod.ModulePath,
}
@@ -472,14 +472,14 @@
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 @@
//
// 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 @@
// 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 @@
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 @@
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
+}
diff --git a/internal/postgres/search_test.go b/internal/postgres/search_test.go
index db41c98..2ba034b 100644
--- a/internal/postgres/search_test.go
+++ b/internal/postgres/search_test.go
@@ -989,7 +989,7 @@
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,
diff --git a/internal/worker/server.go b/internal/worker/server.go
index 229591d..dfe0847 100644
--- a/internal/worker/server.go
+++ b/internal/worker/server.go
@@ -244,7 +244,7 @@
}
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
}
}
