internal/postgres: update GetPackagesForSearchDocumentUpsert to use readmes table
GetPackagesForSearchDocumentUpsert is updated to fetch readme data from
the readmes table instead of modules.readme_file_path and
modules.readme_contents.
For golang/go#258311
Change-Id: I8fbba5cd48643f583bcf56259096011322e46eee
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/258311
Trust: Julie Qiu <julie@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
diff --git a/internal/frontend/search_test.go b/internal/frontend/search_test.go
index 374e1ee..401dc09 100644
--- a/internal/frontend/search_test.go
+++ b/internal/frontend/search_test.go
@@ -19,6 +19,7 @@
func TestFetchSearchPage(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
defer cancel()
+ defer postgres.ResetTestDB(testDB, t)
var (
now = sample.NowTruncated()
@@ -30,15 +31,25 @@
CommitTime: now,
IsRedistributable: true,
},
- LegacyReadmeContents: "readme",
},
- LegacyPackages: []*internal.LegacyPackage{
+ Units: []*internal.Unit{
{
- Name: "foo",
- Path: "/path/to/foo",
- Synopsis: "foo is a package.",
- Licenses: sample.LicenseMetadata,
- IsRedistributable: true,
+ UnitMeta: internal.UnitMeta{
+ Name: "foo",
+ Path: "github.com/mod/foo",
+ Licenses: sample.LicenseMetadata,
+ CommitTime: now,
+ ModulePath: "github.com/mod/foo",
+ Version: "v1.0.0",
+ IsRedistributable: true,
+ },
+ Documentation: &internal.Documentation{
+ Synopsis: "foo is a package.",
+ },
+ Readme: &internal.Readme{
+ Filepath: "readme",
+ Contents: "readme",
+ },
},
},
}
@@ -50,29 +61,56 @@
CommitTime: now,
IsRedistributable: true,
},
- LegacyReadmeContents: "readme",
},
- LegacyPackages: []*internal.LegacyPackage{
+ Units: []*internal.Unit{
{
- Name: "bar",
- Path: "/path/to/bar",
- Synopsis: "bar is used by foo.",
- Licenses: sample.LicenseMetadata,
- IsRedistributable: true,
+ UnitMeta: internal.UnitMeta{
+ Name: "bar",
+ Path: "github.com/mod/bar",
+ Licenses: sample.LicenseMetadata,
+ CommitTime: now,
+ ModulePath: "github.com/mod/bar",
+ Version: "v1.0.0",
+ IsRedistributable: true,
+ },
+ Documentation: &internal.Documentation{
+ Synopsis: "bar is used by foo.",
+ },
+ Readme: &internal.Readme{
+ Filepath: "readme",
+ Contents: "readme",
+ },
},
},
}
)
+ for _, m := range []*internal.Module{moduleFoo, moduleBar} {
+ // TODO(golang/go#39629): Delete once packages table is fully deprecated.
+ for _, pkg := range m.Packages() {
+ m.LegacyPackages = append(m.LegacyPackages, &internal.LegacyPackage{
+ Path: pkg.Path,
+ Name: pkg.Name,
+ Synopsis: pkg.Documentation.Synopsis,
+ Licenses: sample.LicenseMetadata,
+ IsRedistributable: pkg.IsRedistributable,
+ GOOS: pkg.Documentation.GOOS,
+ GOARCH: pkg.Documentation.GOARCH,
+ })
+ }
+ if err := testDB.InsertModule(ctx, m); err != nil {
+ t.Fatal(err)
+ }
+ }
+
for _, tc := range []struct {
name, query string
modules []*internal.Module
wantSearchPage *SearchPage
}{
{
- name: "want expected search page",
- query: "foo bar",
- modules: []*internal.Module{moduleFoo, moduleBar},
+ name: "want expected search page",
+ query: "foo bar",
wantSearchPage: &SearchPage{
Pagination: pagination{
TotalCount: 1,
@@ -85,10 +123,10 @@
},
Results: []*SearchResult{
{
- Name: moduleBar.LegacyPackages[0].Name,
- PackagePath: moduleBar.LegacyPackages[0].Path,
+ Name: moduleBar.Packages()[0].Name,
+ PackagePath: moduleBar.Packages()[0].Path,
ModulePath: moduleBar.ModulePath,
- Synopsis: moduleBar.LegacyPackages[0].Synopsis,
+ Synopsis: moduleBar.Packages()[0].Documentation.Synopsis,
DisplayVersion: moduleBar.Version,
Licenses: []string{"MIT"},
CommitTime: elapsedTime(moduleBar.CommitTime),
@@ -98,9 +136,8 @@
},
},
{
- name: "want only foo search page",
- query: "package",
- modules: []*internal.Module{moduleFoo, moduleBar},
+ name: "want only foo search page",
+ query: "package",
wantSearchPage: &SearchPage{
Pagination: pagination{
TotalCount: 1,
@@ -113,10 +150,10 @@
},
Results: []*SearchResult{
{
- Name: moduleFoo.LegacyPackages[0].Name,
- PackagePath: moduleFoo.LegacyPackages[0].Path,
+ Name: moduleFoo.Packages()[0].Name,
+ PackagePath: moduleFoo.Packages()[0].Path,
ModulePath: moduleFoo.ModulePath,
- Synopsis: moduleFoo.LegacyPackages[0].Synopsis,
+ Synopsis: moduleFoo.Packages()[0].Documentation.Synopsis,
DisplayVersion: moduleFoo.Version,
Licenses: []string{"MIT"},
CommitTime: elapsedTime(moduleFoo.CommitTime),
@@ -127,14 +164,6 @@
},
} {
t.Run(tc.name, func(t *testing.T) {
- defer postgres.ResetTestDB(testDB, t)
-
- for _, v := range tc.modules {
- if err := testDB.InsertModule(ctx, v); err != nil {
- t.Fatal(err)
- }
- }
-
got, err := fetchSearchPage(ctx, testDB, tc.query, paginationParams{limit: 20, page: 1})
if err != nil {
t.Fatalf("fetchSearchPage(db, %q): %v", tc.query, err)
diff --git a/internal/postgres/search.go b/internal/postgres/search.go
index 9bc2956..5892436 100644
--- a/internal/postgres/search.go
+++ b/internal/postgres/search.go
@@ -441,18 +441,22 @@
defer derrors.Wrap(&err, "UpsertSearchDocuments(ctx, %q)", mod.ModulePath)
ctx, span := trace.StartSpan(ctx, "UpsertSearchDocuments")
defer span.End()
- for _, pkg := range mod.LegacyPackages {
+ for _, pkg := range mod.Packages() {
if isInternalPackage(pkg.Path) {
continue
}
- err := UpsertSearchDocument(ctx, db, upsertSearchDocumentArgs{
- PackagePath: pkg.Path,
- ModulePath: mod.ModulePath,
- Synopsis: pkg.Synopsis,
- ReadmeFilePath: mod.LegacyReadmeFilePath,
- ReadmeContents: mod.LegacyReadmeContents,
- })
- if err != nil {
+ args := upsertSearchDocumentArgs{
+ PackagePath: pkg.Path,
+ ModulePath: mod.ModulePath,
+ }
+ if pkg.Documentation != nil {
+ args.Synopsis = pkg.Documentation.Synopsis
+ }
+ if pkg.Readme != nil {
+ args.ReadmeFilePath = pkg.Readme.Filepath
+ args.ReadmeContents = pkg.Readme.Contents
+ }
+ if err := UpsertSearchDocument(ctx, db, args); err != nil {
return err
}
}
@@ -497,11 +501,17 @@
sd.module_path,
sd.synopsis,
sd.redistributable,
- m.readme_file_path,
- m.readme_contents
- FROM search_documents sd
- INNER JOIN modules m
- USING (module_path, version)
+ r.file_path,
+ r.contents
+ FROM modules m
+ INNER JOIN paths p
+ ON m.id = p.module_id
+ LEFT JOIN readmes r
+ ON p.id = r.path_id
+ INNER JOIN search_documents sd
+ ON sd.package_path = p.path
+ AND sd.module_path = m.module_path
+ AND sd.version = m.version
WHERE sd.updated_at < $1
LIMIT $2`
@@ -510,7 +520,8 @@
a upsertSearchDocumentArgs
redist bool
)
- if err := rows.Scan(&a.PackagePath, &a.ModulePath, &a.Synopsis, &redist, &a.ReadmeFilePath, &a.ReadmeContents); err != nil {
+ if err := rows.Scan(&a.PackagePath, &a.ModulePath, &a.Synopsis, &redist,
+ database.NullIsEmpty(&a.ReadmeFilePath), database.NullIsEmpty(&a.ReadmeContents)); err != nil {
return err
}
if !redist && !db.bypassLicenseCheck {
diff --git a/internal/postgres/search_test.go b/internal/postgres/search_test.go
index 36d5b83..899604f 100644
--- a/internal/postgres/search_test.go
+++ b/internal/postgres/search_test.go
@@ -154,10 +154,10 @@
// a single importing module.
func importGraph(popularPath, importerModule string, importerCount int) []*internal.Module {
m := sample.Module(popularPath, "v1.2.3", "")
- m.LegacyPackages[0].Imports = nil
+ m.Packages()[0].Imports = nil
// Try to improve the ts_rank of the 'foo' search term.
- m.LegacyPackages[0].Synopsis = "foo"
- m.LegacyReadmeContents = "foo"
+ m.Packages()[0].Documentation.Synopsis = "foo"
+ m.Units[0].Readme.Contents = "foo"
mods := []*internal.Module{m}
if importerCount > 0 {
m := sample.Module(importerModule, "v1.2.3")
@@ -286,8 +286,8 @@
defer ResetTestDB(testDB, t)
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
- for _, v := range test.modules {
- if err := testDB.InsertModule(ctx, v); err != nil {
+ for _, m := range test.modules {
+ if err := testDB.InsertModule(ctx, m); err != nil {
t.Fatal(err)
}
}
@@ -938,6 +938,17 @@
moduleA := sample.Module("mod.com", "v1.2.3",
"A", "A/notinternal", "A/internal", "A/internal/B")
+
+ // moduleA.Units[1] is mod.com/A.
+ moduleA.Units[1].Readme = &internal.Readme{
+ Filepath: sample.ReadmeFilePath,
+ Contents: sample.ReadmeContents,
+ }
+ // moduleA.Units[2] is mod.com/A/notinternal.
+ moduleA.Units[2].Readme = &internal.Readme{
+ Filepath: sample.ReadmeFilePath,
+ Contents: sample.ReadmeContents,
+ }
moduleN := nonRedistributableModule()
bypassDB := NewBypassingLicenseCheck(testDB.db)
for _, m := range []*internal.Module{moduleA, moduleN} {
diff --git a/internal/testing/sample/sample.go b/internal/testing/sample/sample.go
index b3fb6ea..4f6cad0 100644
--- a/internal/testing/sample/sample.go
+++ b/internal/testing/sample/sample.go
@@ -254,11 +254,9 @@
UnitMeta: *UnitMeta(m.ModulePath, m.ModulePath, m.Version, "", m.IsRedistributable),
LicenseContents: Licenses,
}
- if m.LegacyReadmeFilePath != "" {
- u.Readme = &internal.Readme{
- Filepath: m.LegacyReadmeFilePath,
- Contents: m.LegacyReadmeContents,
- }
+ u.Readme = &internal.Readme{
+ Filepath: ReadmeFilePath,
+ Contents: ReadmeContents,
}
return u
}
