internal/postgres: don't insert into search if a prefix
In ReconcileSearch, don't insert a module into search_documents if
there is already a module path that is a suffix of the current one.
This avoids cases like the following:
1. github.com/araalinetworks/api, which has a golang package, is
fetched and added to search_documents.
2. github.com/araalinetworks/api/golang is fetched and added to
search_documents, correctly replacing github.com/araalinetworks/api
because it is a longer module path.
3. Another version of github.com/araalinetworks/api is fetched. It is
bad, so it isn't inserted in the usual way, but ReconcileSearch
re-inserts the latest good version, which overwrites the longer
module path inserted in step 2.
Change-Id: If8ae59acfde7838c0ea48882d6beb0a457e125db
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/342469
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/internal/postgres/insert_module.go b/internal/postgres/insert_module.go
index 073ab41..d977141 100644
--- a/internal/postgres/insert_module.go
+++ b/internal/postgres/insert_module.go
@@ -639,8 +639,9 @@
// ReconcileSearch reconciles the search data for modulePath. If the module is
// alternative or has no good versions, it removes search data. Otherwise, if
-// the latest good version doesn't match the version in search_documents, and it
-// inserts the latest good version into search_documents and imports_unique.
+// the latest good version doesn't match the version in search_documents,
+// and the module path is not a prefix of one already in search_documents,
+// it inserts the latest good version into search_documents and imports_unique.
// The version and status arguments should come from the module currently being
// fetched. They are used to determine if the module is alternative.
func (db *DB) ReconcileSearch(ctx context.Context, modulePath, version string, status int) (err error) {
@@ -685,18 +686,18 @@
log.Debugf(ctx, "ReconcileSearch(%q): alternative or no good version; removed from search_documents and imports_unique", modulePath)
return nil
}
- // Is the latest good version in search_documents?
+ // Is the latest good version in search_documents, or is there a longer module path?
var x int
switch err := tx.QueryRow(ctx, `
SELECT 1
FROM search_documents
- WHERE module_path = $1
- AND version = $2
+ WHERE (module_path = $1 AND version = $2)
+ OR module_path LIKE $1 || '/%'
`, modulePath, lmv.GoodVersion).Scan(&x); err {
case sql.ErrNoRows:
break
case nil:
- log.Debugf(ctx, "ReconcileSearch(%q): good version %s found in search_documents; doing nothing",
+ log.Debugf(ctx, "ReconcileSearch(%q): good version %s or suffix module path found in search_documents; doing nothing",
modulePath, lmv.GoodVersion)
return nil
default:
diff --git a/internal/postgres/insert_module_test.go b/internal/postgres/insert_module_test.go
index 42510ee..0e1ca99 100644
--- a/internal/postgres/insert_module_test.go
+++ b/internal/postgres/insert_module_test.go
@@ -620,8 +620,8 @@
defer release()
ctx := context.Background()
- insert := func(modulePath, version string, status int, imports []string, modfile string) {
- m := sample.Module(modulePath, version, "pkg")
+ insert := func(modulePath, version, suffix string, status int, imports []string, modfile string) {
+ m := sample.Module(modulePath, version, suffix)
pkg := m.Packages()[0]
pkg.Documentation[0].Synopsis = version
pkg.Imports = imports
@@ -645,15 +645,19 @@
}
}
- check := func(modPath, wantVersion string, wantImports []string) {
+ check := func(modPath, wantVersion, suffix string, wantImports []string) {
t.Helper()
+ pkgPath := modPath
+ if suffix != "" {
+ pkgPath += "/" + suffix
+ }
var gotVersion, gotSynopsis string
err := testDB.db.QueryRow(ctx, `
SELECT version, synopsis
FROM search_documents
WHERE module_path = $1
- AND package_path = $1 || '/pkg'
- `, modPath).Scan(&gotVersion, &gotSynopsis)
+ AND package_path = $2
+ `, modPath, pkgPath).Scan(&gotVersion, &gotSynopsis)
if errors.Is(err, sql.ErrNoRows) {
gotVersion = ""
gotSynopsis = ""
@@ -667,9 +671,9 @@
gotImports, err := testDB.db.CollectStrings(ctx, `
SELECT to_path
FROM imports_unique
- WHERE from_path = $1 || '/pkg'
- AND from_module_path = $1
- `, modPath)
+ WHERE from_path = $1
+ AND from_module_path = $2
+ `, pkgPath, modPath)
if err != nil {
t.Fatal(err)
}
@@ -683,31 +687,40 @@
imports1 := []string{"fmt", "log"}
const modPath1 = "m.com/a"
// Insert a good module. It should be in search_documents and imports_unique.
- insert(modPath1, "v1.1.0", 200, imports1, "")
- check(modPath1, "v1.1.0", imports1)
+ insert(modPath1, "v1.1.0", "pkg", 200, imports1, "")
+ check(modPath1, "v1.1.0", "pkg", imports1)
// Insert a higher, good version. It should replace the first.
imports2 := []string{"log", "strings"}
- insert(modPath1, "v1.2.0", 200, imports2, "")
- check(modPath1, "v1.2.0", imports2)
+ insert(modPath1, "v1.2.0", "pkg", 200, imports2, "")
+ check(modPath1, "v1.2.0", "pkg", imports2)
// Now an even higher, bad version comes along that retracts v1.2.0.
// The search_documents and imports_unique tables should go back to v1.1.0.
- insert(modPath1, "v1.3.0", 400, nil, "retract v1.2.0")
- check(modPath1, "v1.1.0", imports1)
+ insert(modPath1, "v1.3.0", "pkg", 400, nil, "retract v1.2.0")
+ check(modPath1, "v1.1.0", "pkg", imports1)
// Now a still higher version comes along that retracts everything. The
// module should no longer be in search_documents or imports_unique.
- insert(modPath1, "v1.4.0", 200, nil, "retract [v1.0.0, v1.4.0]")
- check(modPath1, "", nil)
+ insert(modPath1, "v1.4.0", "pkg", 200, nil, "retract [v1.0.0, v1.4.0]")
+ check(modPath1, "", "pkg", nil)
// Insert another good module.
const modPath2 = "m.com/b"
- insert(modPath2, "v1.1.0", 200, imports1, "")
- check(modPath2, "v1.1.0", imports1)
+ insert(modPath2, "v1.1.0", "pkg", 200, imports1, "")
+ check(modPath2, "v1.1.0", "pkg", imports1)
// A later version makes this an alternative module.
// The module should be removed from search.
- insert(modPath2, "v1.2.0", 491, imports1, "")
- check(modPath2, "", nil)
+ insert(modPath2, "v1.2.0", "pkg", 491, imports1, "")
+ check(modPath2, "", "pkg", nil)
+
+ // Insert a module that is its own package.
+ const modPath3 = "m.com/c/pkg"
+ insert(modPath3, "v1.0.0", "", 200, imports1, "")
+ check(modPath3, "v1.0.0", "", imports1)
+
+ // Insert a module that is a prefix. It shouldn't change anything.
+ insert("m.com/c", "v1.0.0", "pkg", 200, imports1, "")
+ check(modPath3, "v1.0.0", "", imports1)
}
