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>

1 files changed, 33 insertions, 20 deletions

diff --git a/internal/postgres/insert_module_test.go b/internal/postgres/insert_module_test.go
index 42510eea..0e1ca99c 100644
--- a/internal/postgres/insert_module_test.go
+++ b/internal/postgres/insert_module_test.go
@@ -620,8 +620,8 @@ func TestReconcileSearch(t *testing.T) {
 	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 @@ func TestReconcileSearch(t *testing.T) {
 		}
 	}
 
-	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 @@ func TestReconcileSearch(t *testing.T) {
 		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 @@ func TestReconcileSearch(t *testing.T) {
 	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)
 }