internal/postgres: filter excluded packages from search results
If we exclude a prefix, we won't show any pages for modules or
packages with that prefix. However, we were showing search results
with that prefix.
Remove excluded packages from the search results.
Fixes b/153152789.
Change-Id: Iae9294feaa68b318537faea83aac3fdda83cb1fd
Reviewed-on: https://team-review.git.corp.google.com/c/golang/discovery/+/769680
Reviewed-by: Julie Qiu <julieqiu@google.com>
diff --git a/internal/postgres/excluded.go b/internal/postgres/excluded.go
index a0a7cc1..9010f2c 100644
--- a/internal/postgres/excluded.go
+++ b/internal/postgres/excluded.go
@@ -46,9 +46,7 @@
prefix, user, reason)
if err != nil {
// Arrange to re-read the excluded_prefixes table on the next call to IsExcluded.
- excludedPrefixes.mu.Lock()
- excludedPrefixes.lastFetched = time.Time{}
- excludedPrefixes.mu.Unlock()
+ setExcludedPrefixesLastFetched(time.Time{})
}
return err
}
@@ -61,9 +59,15 @@
lastFetched time.Time
}
+func setExcludedPrefixesLastFetched(t time.Time) {
+ excludedPrefixes.mu.Lock()
+ excludedPrefixes.lastFetched = t
+ excludedPrefixes.mu.Unlock()
+}
+
const excludedPrefixesExpiration = time.Minute
-// refreshExcludedPrefixes makes sure the in-memory copy of the
+// ensureExcludedPrefixes makes sure the in-memory copy of the
// excluded_prefixes table is up to date.
func (db *DB) ensureExcludedPrefixes(ctx context.Context) {
excludedPrefixes.mu.Lock()
@@ -96,5 +100,6 @@
if err != nil {
return nil, err
}
+ setExcludedPrefixesLastFetched(time.Now())
return eps, nil
}
diff --git a/internal/postgres/search.go b/internal/postgres/search.go
index 1166c30..2076240 100644
--- a/internal/postgres/search.go
+++ b/internal/postgres/search.go
@@ -123,7 +123,18 @@
if err != nil {
return nil, err
}
- return resp.results, nil
+ // Filter out excluded paths.
+ var results []*internal.SearchResult
+ for _, r := range resp.results {
+ ex, err := db.IsExcluded(ctx, r.PackagePath)
+ if err != nil {
+ return nil, err
+ }
+ if !ex {
+ results = append(results, r)
+ }
+ }
+ return results, nil
}
// Penalties to search scores, applied as multipliers to the score.
diff --git a/internal/postgres/search_test.go b/internal/postgres/search_test.go
index 5626df4..a13bf80 100644
--- a/internal/postgres/search_test.go
+++ b/internal/postgres/search_test.go
@@ -621,6 +621,37 @@
}
}
+func TestExcludedFromSearch(t *testing.T) {
+ // Verify that excluded paths are omitted from search results.
+ ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
+ defer cancel()
+ defer ResetTestDB(testDB, t)
+
+ // Insert a module with two packages.
+ const domain = "exclude.com"
+ sm := sample.Module(domain, "v1.2.3", "pkg", "exclude")
+ if err := testDB.InsertModule(ctx, sm); err != nil {
+ t.Fatal(err)
+ }
+ // Exclude a prefix that matches one of the packages.
+ if err := testDB.InsertExcludedPrefix(ctx, domain+"/ex", "no user", "no reason"); err != nil {
+ t.Fatal(err)
+ }
+ // Search for both packages.
+ gotResults, err := testDB.Search(ctx, domain, 10, 0)
+ if err != nil {
+ t.Fatal(err)
+ }
+ var got []string
+ for _, g := range gotResults {
+ got = append(got, g.Name)
+ }
+ want := []string{"pkg"}
+ if !cmp.Equal(got, want) {
+ t.Errorf("got %v, want %v", got, want)
+ }
+}
+
type searchDocument struct {
packagePath string
modulePath string
diff --git a/internal/postgres/test_helper.go b/internal/postgres/test_helper.go
index cb85d20..b68b2e6 100644
--- a/internal/postgres/test_helper.go
+++ b/internal/postgres/test_helper.go
@@ -12,6 +12,7 @@
"os"
"path/filepath"
"testing"
+ "time"
"github.com/golang-migrate/migrate/v4"
"golang.org/x/pkgsite/internal/database"
@@ -116,6 +117,7 @@
if _, err := tx.Exec(ctx, `TRUNCATE excluded_prefixes;`); err != nil {
return err
}
+ setExcludedPrefixesLastFetched(time.Time{})
return nil
}); err != nil {
t.Fatalf("error resetting test DB: %v", err)
