internal/{frontend,vuln}: only fetch 5 most recent vulns on /vuln page
Previously, all vulns were fetched and all but 5 were discarded, an
expensive operation. With this change, only the 5 most recent vulns
are fetched.
Change-Id: Ia14992bbbdd4fb45a7512127e0f350fcb62b021d
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/484537
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Run-TryBot: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
diff --git a/internal/frontend/search.go b/internal/frontend/search.go
index 056c610..766d5a2 100644
--- a/internal/frontend/search.go
+++ b/internal/frontend/search.go
@@ -373,7 +373,7 @@
if mode != searchModeVuln || client == nil {
return nil, nil
}
- allEntries, err := client.Entries(ctx)
+ allEntries, err := client.Entries(ctx, -1)
if err != nil {
return nil, err
}
@@ -403,8 +403,6 @@
}
}
- sortVulnEntries(entries)
-
return &searchAction{
title: fmt.Sprintf("%s - Vulnerability Reports", query),
template: "vuln/list",
diff --git a/internal/frontend/vulns.go b/internal/frontend/vulns.go
index 16ae3ae..97e6afe 100644
--- a/internal/frontend/vulns.go
+++ b/internal/frontend/vulns.go
@@ -7,7 +7,6 @@
import (
"context"
"net/http"
- "sort"
"strings"
"golang.org/x/pkgsite/internal"
@@ -44,19 +43,16 @@
path := strings.TrimPrefix(r.URL.Path, "/vuln")
switch path {
case "/":
- // Serve a list of most recent entries.
- vulnListPage, err := newVulnListPage(r.Context(), s.vulnClient)
+ // Serve a list of the 5 most recent entries.
+ vulnListPage, err := newVulnListPage(r.Context(), s.vulnClient, 5)
if err != nil {
return &serverError{status: derrors.ToStatus(err)}
}
- if len(vulnListPage.Entries) > 5 {
- vulnListPage.Entries = vulnListPage.Entries[:5]
- }
vulnListPage.basePage = s.newBasePage(r, "Go Vulnerability Database")
s.servePage(r.Context(), w, "vuln/main", vulnListPage)
case "/list":
// Serve a list of all entries.
- vulnListPage, err := newVulnListPage(r.Context(), s.vulnClient)
+ vulnListPage, err := newVulnListPage(r.Context(), s.vulnClient, -1)
if err != nil {
return &serverError{status: derrors.ToStatus(err)}
}
@@ -99,20 +95,14 @@
}, nil
}
-func newVulnListPage(ctx context.Context, client *vuln.Client) (*VulnListPage, error) {
- entries, err := client.Entries(ctx)
+func newVulnListPage(ctx context.Context, client *vuln.Client, n int) (*VulnListPage, error) {
+ entries, err := client.Entries(ctx, n)
if err != nil {
return nil, derrors.VulnDBError
}
- sortVulnEntries(entries)
return &VulnListPage{Entries: entries}, nil
}
-// Sort vuln entries in descending order by Go ID.
-func sortVulnEntries(entries []*osv.Entry) {
- sort.Slice(entries, func(i, j int) bool { return entries[i].ID > entries[j].ID })
-}
-
// aliasLinks generates links to reference pages for vuln aliases.
func aliasLinks(e *osv.Entry) []link {
var links []link
diff --git a/internal/frontend/vulns_test.go b/internal/frontend/vulns_test.go
index ca3c844..07c1199 100644
--- a/internal/frontend/vulns_test.go
+++ b/internal/frontend/vulns_test.go
@@ -61,7 +61,7 @@
if err != nil {
t.Fatal(err)
}
- got, err := newVulnListPage(ctx, c)
+ got, err := newVulnListPage(ctx, c, -1)
if err != nil {
t.Fatal(err)
}
diff --git a/internal/vuln/client.go b/internal/vuln/client.go
index f2b1403..1b43079 100644
--- a/internal/vuln/client.go
+++ b/internal/vuln/client.go
@@ -214,15 +214,31 @@
return "", derrors.NotFound
}
-// Entries returns all entries in the database.
-func (c *Client) Entries(ctx context.Context) (_ []*osv.Entry, err error) {
- derrors.Wrap(&err, "Entries()")
+// Entries returns all entries in the database, sorted in descending
+// order by Go ID (most recent to least recent).
+// If n >= 0, only the n most recent entries are returned.
+func (c *Client) Entries(ctx context.Context, n int) (_ []*osv.Entry, err error) {
+ derrors.Wrap(&err, "Entries(n=%d)", n)
+
+ if n == 0 {
+ return nil, nil
+ }
ids, err := c.ids(ctx)
if err != nil {
return nil, err
}
+ sort.Slice(ids, func(i, j int) bool { return ids[i] > ids[j] })
+
+ if n >= 0 && len(ids) > n {
+ ids = ids[:n]
+ }
+
+ return c.byIDs(ctx, ids)
+}
+
+func (c *Client) byIDs(ctx context.Context, ids []string) (_ []*osv.Entry, err error) {
entries := make([]*osv.Entry, len(ids))
g, gctx := errgroup.WithContext(ctx)
g.SetLimit(4)
diff --git a/internal/vuln/client_test.go b/internal/vuln/client_test.go
index a19c200..be630b6 100644
--- a/internal/vuln/client_test.go
+++ b/internal/vuln/client_test.go
@@ -8,6 +8,7 @@
"bytes"
"context"
"encoding/json"
+ "fmt"
"reflect"
"strings"
"testing"
@@ -346,14 +347,42 @@
t.Fatal(err)
}
- got, err := c.Entries(ctx)
- if err != nil {
- t.Fatal(err)
+ tests := []struct {
+ n int
+ want []*osv.Entry
+ }{
+ {
+ n: -1,
+ want: []*osv.Entry{&testOSV3, &testOSV2, &testOSV1},
+ },
+ {
+ n: 0,
+ want: nil,
+ },
+ {
+ n: 2,
+ want: []*osv.Entry{&testOSV3, &testOSV2},
+ },
+ {
+ n: 3,
+ want: []*osv.Entry{&testOSV3, &testOSV2, &testOSV1},
+ },
+ {
+ n: 4,
+ want: []*osv.Entry{&testOSV3, &testOSV2, &testOSV1},
+ },
}
+ for _, tc := range tests {
+ t.Run(fmt.Sprintf("n=%d", tc.n), func(t *testing.T) {
+ got, err := c.Entries(ctx, tc.n)
+ if err != nil {
+ t.Fatal(err)
+ }
- want := []*osv.Entry{&testOSV1, &testOSV2, &testOSV3}
- if !reflect.DeepEqual(got, want) {
- t.Errorf("Entries = %#v, want %#v", got, want)
+ if !reflect.DeepEqual(got, tc.want) {
+ t.Errorf("Entries = %#v, want %#v", got, tc.want)
+ }
+ })
}
}
