internl/postgres: move delete functions into delete.go
Pure code motion.
Change-Id: I80055dbb95f2341c72d3716b74e08335bdaa6fd4
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/341851
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/internal/postgres/delete.go b/internal/postgres/delete.go
index 2c9e789..4b36663 100644
--- a/internal/postgres/delete.go
+++ b/internal/postgres/delete.go
@@ -8,9 +8,13 @@
"context"
"database/sql"
+ "github.com/Masterminds/squirrel"
"github.com/lib/pq"
+ "golang.org/x/mod/semver"
+ "golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/database"
"golang.org/x/pkgsite/internal/derrors"
+ "golang.org/x/pkgsite/internal/log"
)
// DeleteModule deletes a Version from the database.
@@ -41,6 +45,85 @@
})
}
+// DeleteOlderVersionFromSearchDocuments deletes from search_documents every package with
+// the given module path whose version is older than the given version.
+// It is used when fetching a module with an alternative path. See internal/worker/fetch.go:fetchAndUpdateState.
+func (db *DB) DeleteOlderVersionFromSearchDocuments(ctx context.Context, modulePath, resolvedVersion string) (err error) {
+ defer derrors.WrapStack(&err, "DeleteOlderVersionFromSearchDocuments(ctx, %q, %q)", modulePath, resolvedVersion)
+
+ return db.db.Transact(ctx, sql.LevelDefault, func(tx *database.DB) error {
+ // Collect all package paths in search_documents with the given module path
+ // and an older version. (package_path is the primary key of search_documents.)
+ var ppaths []string
+ query := `
+ SELECT package_path, version
+ FROM search_documents
+ WHERE module_path = $1
+ `
+ err := tx.RunQuery(ctx, query, func(rows *sql.Rows) error {
+ var ppath, v string
+ if err := rows.Scan(&ppath, &v); err != nil {
+ return err
+ }
+ if semver.Compare(v, resolvedVersion) < 0 {
+ ppaths = append(ppaths, ppath)
+ }
+ return nil
+ }, modulePath)
+ if err != nil {
+ return err
+ }
+ if len(ppaths) == 0 {
+ return nil
+ }
+
+ // Delete all of those paths.
+ return deleteModuleOrPackagesInModuleFromSearchDocuments(ctx, tx, modulePath, ppaths)
+ })
+}
+
+// deleteOtherModulePackagesFromSearchDocuments deletes all packages from search
+// documents with the given module that are not in m.
+func deleteOtherModulePackagesFromSearchDocuments(ctx context.Context, tx *database.DB, m *internal.Module) error {
+ dbPkgs, err := tx.CollectStrings(ctx, `
+ SELECT package_path FROM search_documents WHERE module_path = $1
+ `, m.ModulePath)
+ if err != nil {
+ return err
+ }
+ pkgInModule := map[string]bool{}
+ for _, u := range m.Packages() {
+ pkgInModule[u.Path] = true
+ }
+ var otherPkgs []string
+ for _, p := range dbPkgs {
+ if !pkgInModule[p] {
+ otherPkgs = append(otherPkgs, p)
+ }
+ }
+ return deleteModuleOrPackagesInModuleFromSearchDocuments(ctx, tx, m.ModulePath, otherPkgs)
+}
+
+// deleteModuleOrPackagesInModuleFromSearchDocuments deletes module_path from
+// search_documents. If packages is non-empty, it only deletes those packages.
+func deleteModuleOrPackagesInModuleFromSearchDocuments(ctx context.Context, tx *database.DB, modulePath string, packages []string) error {
+ d := squirrel.Delete("search_documents").
+ Where(squirrel.Eq{"module_path": modulePath})
+ if len(packages) > 0 {
+ d = d.Where("package_path = ANY(?)", pq.Array(packages))
+ }
+ q, args, err := d.PlaceholderFormat(squirrel.Dollar).ToSql()
+ if err != nil {
+ return err
+ }
+ n, err := tx.Exec(ctx, q, args...)
+ if err != nil {
+ return err
+ }
+ log.Infof(ctx, "deleted %d rows of module %s from search_documents", n, modulePath)
+ return nil
+}
+
// DeletePseudoversionsExcept deletes all pseudoversions for the module except
// the provided resolvedVersion.
func (db *DB) DeletePseudoversionsExcept(ctx context.Context, modulePath, resolvedVersion string) (err error) {
diff --git a/internal/postgres/delete_test.go b/internal/postgres/delete_test.go
index f0e5fc7..ae15a73 100644
--- a/internal/postgres/delete_test.go
+++ b/internal/postgres/delete_test.go
@@ -6,12 +6,174 @@
import (
"context"
+ "database/sql"
+ "errors"
+ "sort"
+ "strings"
"testing"
+ "github.com/google/go-cmp/cmp"
+ "golang.org/x/pkgsite/internal/derrors"
"golang.org/x/pkgsite/internal/testing/sample"
"golang.org/x/pkgsite/internal/version"
)
+func TestDeleteModule(t *testing.T) {
+ t.Parallel()
+ testDB, release := acquire(t)
+ defer release()
+ ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
+ defer cancel()
+
+ v := sample.DefaultModule()
+
+ MustInsertModule(ctx, t, testDB, v)
+ if _, err := testDB.GetModuleInfo(ctx, v.ModulePath, v.Version); err != nil {
+ t.Fatal(err)
+ }
+
+ vm := sample.DefaultVersionMap()
+ if err := testDB.UpsertVersionMap(ctx, vm); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := testDB.GetVersionMap(ctx, v.ModulePath, v.Version); err != nil {
+ t.Fatal(err)
+ }
+
+ if err := testDB.DeleteModule(ctx, v.ModulePath, v.Version); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := testDB.GetModuleInfo(ctx, v.ModulePath, v.Version); !errors.Is(err, derrors.NotFound) {
+ t.Errorf("got %v, want NotFound", err)
+ }
+
+ var x int
+ err := testDB.Underlying().QueryRow(ctx, "SELECT 1 FROM imports_unique WHERE from_module_path = $1",
+ v.ModulePath).Scan(&x)
+ if err != sql.ErrNoRows {
+ t.Errorf("imports_unique: got %v, want ErrNoRows", err)
+ }
+ err = testDB.Underlying().QueryRow(
+ ctx,
+ "SELECT 1 FROM version_map WHERE module_path = $1 AND resolved_version = $2",
+ v.ModulePath, v.Version).Scan(&x)
+ if err != sql.ErrNoRows {
+ t.Errorf("version_map: got %v, want ErrNoRows", err)
+ }
+}
+
+func TestDeleteFromSearch(t *testing.T) {
+ t.Parallel()
+ ctx := context.Background()
+
+ const modulePath = "deleteme.com"
+
+ initial := []searchDocumentRow{
+ {modulePath + "/p1", modulePath, "v0.0.9", 0}, // oldest version of same module
+ {modulePath + "/p2", modulePath, "v1.1.0", 0}, // older version of same module
+ {modulePath + "/p4", modulePath, "v1.9.0", 0}, // newer version of same module
+ {"other.org/p2", "other.org", "v1.1.0", 0}, // older version of a different module
+ }
+
+ insertInitial := func(db *DB) {
+ t.Helper()
+ for _, r := range initial {
+ sm := sample.Module(r.ModulePath, r.Version, strings.TrimPrefix(r.PackagePath, r.ModulePath+"/"))
+ MustInsertModule(ctx, t, db, sm)
+ }
+ // Older versions are deleted by InsertModule, so only the newest versions are here.
+ checkSearchDocuments(ctx, t, db, initial[2:])
+ }
+
+ t.Run("DeleteOlderVersionFromSearchDocuments", func(t *testing.T) {
+ testDB, release := acquire(t)
+ defer release()
+ insertInitial(testDB)
+
+ if err := testDB.DeleteOlderVersionFromSearchDocuments(ctx, modulePath, "v1.2.3"); err != nil {
+ t.Fatal(err)
+ }
+
+ checkSearchDocuments(ctx, t, testDB, []searchDocumentRow{
+ {modulePath + "/p4", modulePath, "v1.9.0", 0}, // newer version not deleted
+ {"other.org/p2", "other.org", "v1.1.0", 0}, // other module not deleted
+ })
+ })
+ t.Run("deleteModuleFromSearchDocuments", func(t *testing.T) {
+ // Non-empty list of packages tested above. This tests an empty list.
+ testDB, release := acquire(t)
+ defer release()
+ insertInitial(testDB)
+
+ if err := deleteModuleOrPackagesInModuleFromSearchDocuments(ctx, testDB.db, modulePath, nil); err != nil {
+ t.Fatal(err)
+ }
+ checkSearchDocuments(ctx, t, testDB, []searchDocumentRow{
+ {"other.org/p2", "other.org", "v1.1.0", 0}, // other module not deleted
+ })
+ })
+ t.Run("deleteOtherModulePackagesFromSearchDocuments", func(t *testing.T) {
+ testDB, release := acquire(t)
+ defer release()
+
+ // Insert a module with two packages.
+ m0 := sample.Module(modulePath, "v1.0.0", "p1", "p2")
+ MustInsertModule(ctx, t, testDB, m0)
+ // Set the imported-by count to a non-zero value so we can tell which
+ // rows were deleted.
+ if _, err := testDB.db.Exec(ctx, `UPDATE search_documents SET imported_by_count = 1`); err != nil {
+ t.Fatal(err)
+ }
+ // Later version of module does not have p1.
+ m1 := sample.Module(modulePath, "v1.1.0", "p2", "p3")
+ MustInsertModule(ctx, t, testDB, m1)
+
+ // p1 should be gone, p2 should be there with the same
+ // imported_by_count, and p3 should be there with a zero count.
+ want := []searchDocumentRow{
+ {modulePath + "/p2", modulePath, "v1.1.0", 1},
+ {modulePath + "/p3", modulePath, "v1.1.0", 0},
+ }
+ checkSearchDocuments(ctx, t, testDB, want)
+ })
+}
+
+type searchDocumentRow struct {
+ PackagePath, ModulePath, Version string
+ ImportedByCount int
+}
+
+func readSearchDocuments(ctx context.Context, db *DB) ([]searchDocumentRow, error) {
+ var rows []searchDocumentRow
+ err := db.db.CollectStructs(ctx, &rows, `SELECT package_path, module_path, version, imported_by_count FROM search_documents`)
+ if err != nil {
+ return nil, err
+ }
+ return rows, err
+}
+
+func checkSearchDocuments(ctx context.Context, t *testing.T, db *DB, want []searchDocumentRow) {
+ t.Helper()
+ got, err := readSearchDocuments(ctx, db)
+ if err != nil {
+ t.Fatal(err)
+ }
+ less := func(r1, r2 searchDocumentRow) bool {
+ if r1.PackagePath != r2.PackagePath {
+ return r1.PackagePath < r2.PackagePath
+ }
+ if r1.ModulePath != r2.ModulePath {
+ return r1.ModulePath < r2.ModulePath
+ }
+ return r1.Version < r2.Version
+ }
+ sort.Slice(got, func(i, j int) bool { return less(got[i], got[j]) })
+ sort.Slice(want, func(i, j int) bool { return less(want[i], want[j]) })
+ if diff := cmp.Diff(want, got); diff != "" {
+ t.Errorf("search documents mismatch (-want, +got):\n%s", diff)
+ }
+}
+
func TestDeletePseudoversionsExcept(t *testing.T) {
t.Parallel()
ctx := context.Background()
diff --git a/internal/postgres/insert_module_test.go b/internal/postgres/insert_module_test.go
index 0bac9bb..76a9c6b 100644
--- a/internal/postgres/insert_module_test.go
+++ b/internal/postgres/insert_module_test.go
@@ -445,50 +445,6 @@
}
}
-func TestDeleteModule(t *testing.T) {
- t.Parallel()
- testDB, release := acquire(t)
- defer release()
- ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
- defer cancel()
-
- v := sample.DefaultModule()
-
- MustInsertModule(ctx, t, testDB, v)
- if _, err := testDB.GetModuleInfo(ctx, v.ModulePath, v.Version); err != nil {
- t.Fatal(err)
- }
-
- vm := sample.DefaultVersionMap()
- if err := testDB.UpsertVersionMap(ctx, vm); err != nil {
- t.Fatal(err)
- }
- if _, err := testDB.GetVersionMap(ctx, v.ModulePath, v.Version); err != nil {
- t.Fatal(err)
- }
-
- if err := testDB.DeleteModule(ctx, v.ModulePath, v.Version); err != nil {
- t.Fatal(err)
- }
- if _, err := testDB.GetModuleInfo(ctx, v.ModulePath, v.Version); !errors.Is(err, derrors.NotFound) {
- t.Errorf("got %v, want NotFound", err)
- }
-
- var x int
- err := testDB.Underlying().QueryRow(ctx, "SELECT 1 FROM imports_unique WHERE from_module_path = $1",
- v.ModulePath).Scan(&x)
- if err != sql.ErrNoRows {
- t.Errorf("imports_unique: got %v, want ErrNoRows", err)
- }
- err = testDB.Underlying().QueryRow(
- ctx,
- "SELECT 1 FROM version_map WHERE module_path = $1 AND resolved_version = $2",
- v.ModulePath, v.Version).Scan(&x)
- if err != sql.ErrNoRows {
- t.Errorf("version_map: got %v, want ErrNoRows", err)
- }
-}
-
func TestPostgres_NewerAlternative(t *testing.T) {
t.Parallel()
// Verify that packages are not added to search_documents if the module has a newer
diff --git a/internal/postgres/search.go b/internal/postgres/search.go
index fc788ed..0b70397 100644
--- a/internal/postgres/search.go
+++ b/internal/postgres/search.go
@@ -13,14 +13,12 @@
"strings"
"time"
- "github.com/Masterminds/squirrel"
"github.com/lib/pq"
"go.opencensus.io/plugin/ochttp"
"go.opencensus.io/stats"
"go.opencensus.io/stats/view"
"go.opencensus.io/tag"
"go.opencensus.io/trace"
- "golang.org/x/mod/semver"
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/database"
"golang.org/x/pkgsite/internal/dcensus"
@@ -1055,85 +1053,6 @@
return false
}
-// DeleteOlderVersionFromSearchDocuments deletes from search_documents every package with
-// the given module path whose version is older than the given version.
-// It is used when fetching a module with an alternative path. See internal/worker/fetch.go:fetchAndUpdateState.
-func (db *DB) DeleteOlderVersionFromSearchDocuments(ctx context.Context, modulePath, resolvedVersion string) (err error) {
- defer derrors.WrapStack(&err, "DeleteOlderVersionFromSearchDocuments(ctx, %q, %q)", modulePath, resolvedVersion)
-
- return db.db.Transact(ctx, sql.LevelDefault, func(tx *database.DB) error {
- // Collect all package paths in search_documents with the given module path
- // and an older version. (package_path is the primary key of search_documents.)
- var ppaths []string
- query := `
- SELECT package_path, version
- FROM search_documents
- WHERE module_path = $1
- `
- err := tx.RunQuery(ctx, query, func(rows *sql.Rows) error {
- var ppath, v string
- if err := rows.Scan(&ppath, &v); err != nil {
- return err
- }
- if semver.Compare(v, resolvedVersion) < 0 {
- ppaths = append(ppaths, ppath)
- }
- return nil
- }, modulePath)
- if err != nil {
- return err
- }
- if len(ppaths) == 0 {
- return nil
- }
-
- // Delete all of those paths.
- return deleteModuleOrPackagesInModuleFromSearchDocuments(ctx, tx, modulePath, ppaths)
- })
-}
-
-// deleteOtherModulePackagesFromSearchDocuments deletes all packages from search
-// documents with the given module that are not in m.
-func deleteOtherModulePackagesFromSearchDocuments(ctx context.Context, tx *database.DB, m *internal.Module) error {
- dbPkgs, err := tx.CollectStrings(ctx, `
- SELECT package_path FROM search_documents WHERE module_path = $1
- `, m.ModulePath)
- if err != nil {
- return err
- }
- pkgInModule := map[string]bool{}
- for _, u := range m.Packages() {
- pkgInModule[u.Path] = true
- }
- var otherPkgs []string
- for _, p := range dbPkgs {
- if !pkgInModule[p] {
- otherPkgs = append(otherPkgs, p)
- }
- }
- return deleteModuleOrPackagesInModuleFromSearchDocuments(ctx, tx, m.ModulePath, otherPkgs)
-}
-
-// deleteModuleOrPackagesInModuleFromSearchDocuments deletes module_path from
-// search_documents. If packages is non-empty, it only deletes those packages.
-func deleteModuleOrPackagesInModuleFromSearchDocuments(ctx context.Context, tx *database.DB, modulePath string, packages []string) error {
- d := squirrel.Delete("search_documents").
- Where(squirrel.Eq{"module_path": modulePath})
- if len(packages) > 0 {
- d = d.Where("package_path = ANY(?)", pq.Array(packages))
- }
- q, args, err := d.PlaceholderFormat(squirrel.Dollar).ToSql()
- if err != nil {
- return err
- }
- n, err := tx.Exec(ctx, q, args...)
- if err != nil {
- return err
- }
- log.Infof(ctx, "deleted %d rows of module %s from search_documents", n, modulePath)
- 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) {
diff --git a/internal/postgres/search_test.go b/internal/postgres/search_test.go
index 2190465..c1991fb 100644
--- a/internal/postgres/search_test.go
+++ b/internal/postgres/search_test.go
@@ -1334,118 +1334,6 @@
}
}
-func TestDeleteFromSearch(t *testing.T) {
- t.Parallel()
- ctx := context.Background()
-
- const modulePath = "deleteme.com"
-
- initial := []searchDocumentRow{
- {modulePath + "/p1", modulePath, "v0.0.9", 0}, // oldest version of same module
- {modulePath + "/p2", modulePath, "v1.1.0", 0}, // older version of same module
- {modulePath + "/p4", modulePath, "v1.9.0", 0}, // newer version of same module
- {"other.org/p2", "other.org", "v1.1.0", 0}, // older version of a different module
- }
-
- insertInitial := func(db *DB) {
- t.Helper()
- for _, r := range initial {
- sm := sample.Module(r.ModulePath, r.Version, strings.TrimPrefix(r.PackagePath, r.ModulePath+"/"))
- MustInsertModule(ctx, t, db, sm)
- }
- // Older versions are deleted by InsertModule, so only the newest versions are here.
- checkSearchDocuments(ctx, t, db, initial[2:])
- }
-
- t.Run("DeleteOlderVersionFromSearchDocuments", func(t *testing.T) {
- testDB, release := acquire(t)
- defer release()
- insertInitial(testDB)
-
- if err := testDB.DeleteOlderVersionFromSearchDocuments(ctx, modulePath, "v1.2.3"); err != nil {
- t.Fatal(err)
- }
-
- checkSearchDocuments(ctx, t, testDB, []searchDocumentRow{
- {modulePath + "/p4", modulePath, "v1.9.0", 0}, // newer version not deleted
- {"other.org/p2", "other.org", "v1.1.0", 0}, // other module not deleted
- })
- })
- t.Run("deleteModuleFromSearchDocuments", func(t *testing.T) {
- // Non-empty list of packages tested above. This tests an empty list.
- testDB, release := acquire(t)
- defer release()
- insertInitial(testDB)
-
- if err := deleteModuleOrPackagesInModuleFromSearchDocuments(ctx, testDB.db, modulePath, nil); err != nil {
- t.Fatal(err)
- }
- checkSearchDocuments(ctx, t, testDB, []searchDocumentRow{
- {"other.org/p2", "other.org", "v1.1.0", 0}, // other module not deleted
- })
- })
- t.Run("deleteOtherModulePackagesFromSearchDocuments", func(t *testing.T) {
- testDB, release := acquire(t)
- defer release()
-
- // Insert a module with two packages.
- m0 := sample.Module(modulePath, "v1.0.0", "p1", "p2")
- MustInsertModule(ctx, t, testDB, m0)
- // Set the imported-by count to a non-zero value so we can tell which
- // rows were deleted.
- if _, err := testDB.db.Exec(ctx, `UPDATE search_documents SET imported_by_count = 1`); err != nil {
- t.Fatal(err)
- }
- // Later version of module does not have p1.
- m1 := sample.Module(modulePath, "v1.1.0", "p2", "p3")
- MustInsertModule(ctx, t, testDB, m1)
-
- // p1 should be gone, p2 should be there with the same
- // imported_by_count, and p3 should be there with a zero count.
- want := []searchDocumentRow{
- {modulePath + "/p2", modulePath, "v1.1.0", 1},
- {modulePath + "/p3", modulePath, "v1.1.0", 0},
- }
- checkSearchDocuments(ctx, t, testDB, want)
- })
-}
-
-type searchDocumentRow struct {
- PackagePath, ModulePath, Version string
- ImportedByCount int
-}
-
-func readSearchDocuments(ctx context.Context, db *DB) ([]searchDocumentRow, error) {
- var rows []searchDocumentRow
- err := db.db.CollectStructs(ctx, &rows, `SELECT package_path, module_path, version, imported_by_count FROM search_documents`)
- if err != nil {
- return nil, err
- }
- return rows, err
-}
-
-func checkSearchDocuments(ctx context.Context, t *testing.T, db *DB, want []searchDocumentRow) {
- t.Helper()
- got, err := readSearchDocuments(ctx, db)
- if err != nil {
- t.Fatal(err)
- }
- less := func(r1, r2 searchDocumentRow) bool {
- if r1.PackagePath != r2.PackagePath {
- return r1.PackagePath < r2.PackagePath
- }
- if r1.ModulePath != r2.ModulePath {
- return r1.ModulePath < r2.ModulePath
- }
- return r1.Version < r2.Version
- }
- sort.Slice(got, func(i, j int) bool { return less(got[i], got[j]) })
- sort.Slice(want, func(i, j int) bool { return less(want[i], want[j]) })
- if diff := cmp.Diff(want, got); diff != "" {
- t.Errorf("search documents mismatch (-want, +got):\n%s", diff)
- }
-}
-
func TestGroupSearchResults(t *testing.T) {
rs := []*SearchResult{
{PackagePath: "m.com/p", ModulePath: "m.com", Score: 10},
