internal/frontend: add an interface for creating request caches
This change adds a new Cacher interface that is used to create
middlewares for caching requests. This abstracts away the use of redis
so that the frontend doesn't depend on redis. The tests still depend
on redis for the 404 page testing logic, but the 404 page logic will
be moved out into a different package so those tests will go too.
The Expirer and Middleware interfaces are not present on the Cache
function so that the interface can be defined in package
internal/frontend without needing the dependency on the Middleware
package.
For golang/go#61399
Change-Id: I6518b2ed1d772cb4deda3308c4190f0f1b8a35a0
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/514518
kokoro-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
diff --git a/cmd/frontend/main.go b/cmd/frontend/main.go
index 3812db7..d7c3f19 100644
--- a/cmd/frontend/main.go
+++ b/cmd/frontend/main.go
@@ -132,17 +132,19 @@
}
router := dcensus.NewRouter(frontend.TagRoute)
- var cacheClient *redis.Client
+ var redisClient *redis.Client
+ var cacher frontend.Cacher
if cfg.RedisCacheHost != "" {
addr := cfg.RedisCacheHost + ":" + cfg.RedisCachePort
- cacheClient = redis.NewClient(&redis.Options{Addr: addr})
- if err := cacheClient.Ping(ctx).Err(); err != nil {
+ redisClient := redis.NewClient(&redis.Options{Addr: addr})
+ if err := redisClient.Ping(ctx).Err(); err != nil {
log.Errorf(ctx, "redis at %s: %v", addr, err)
} else {
log.Infof(ctx, "connected to redis at %s", addr)
}
+ cacher = middleware.NewCacher(redisClient)
}
- server.Install(router.Handle, cacheClient, cfg.AuthValues)
+ server.Install(router.Handle, cacher, cfg.AuthValues)
views := append(dcensus.ServerViews,
postgres.SearchLatencyDistribution,
postgres.SearchResponseCount,
@@ -184,7 +186,7 @@
middleware.AcceptRequests(http.MethodGet, http.MethodPost, http.MethodHead), // accept only GETs, POSTs and HEADs
middleware.BetaPkgGoDevRedirect(),
middleware.GodocOrgRedirect(),
- middleware.Quota(cfg.Quota, cacheClient),
+ middleware.Quota(cfg.Quota, redisClient),
middleware.SecureHeaders(!*disableCSP), // must come before any caching for nonces to work
middleware.Experiment(experimenter),
middleware.Panic(panicHandler),
diff --git a/internal/frontend/frontend_test.go b/internal/frontend/frontend_test.go
index 8b51c3e..ff35894 100644
--- a/internal/frontend/frontend_test.go
+++ b/internal/frontend/frontend_test.go
@@ -10,7 +10,6 @@
"testing"
"time"
- "github.com/go-redis/redis/v8"
"github.com/google/safehtml/template"
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/middleware"
@@ -46,7 +45,7 @@
docs []*internal.Documentation
}
-func newTestServer(t *testing.T, proxyModules []*proxytest.Module, redisClient *redis.Client, experimentNames ...string) (*Server, http.Handler, func()) {
+func newTestServer(t *testing.T, proxyModules []*proxytest.Module, cacher Cacher, experimentNames ...string) (*Server, http.Handler, func()) {
t.Helper()
proxyClient, teardown := proxytest.SetupTestClient(t, proxyModules)
sourceClient := source.NewClient(sourceTimeout)
@@ -72,7 +71,7 @@
t.Fatal(err)
}
mux := http.NewServeMux()
- s.Install(mux.Handle, redisClient, nil)
+ s.Install(mux.Handle, cacher, nil)
var exps []*internal.Experiment
for _, n := range experimentNames {
diff --git a/internal/frontend/server.go b/internal/frontend/server.go
index a22ac99..4ec4ca5 100644
--- a/internal/frontend/server.go
+++ b/internal/frontend/server.go
@@ -22,7 +22,6 @@
"time"
"cloud.google.com/go/errorreporting"
- "github.com/go-redis/redis/v8"
"github.com/google/safehtml"
"github.com/google/safehtml/template"
"golang.org/x/pkgsite/internal"
@@ -127,24 +126,36 @@
return s, nil
}
+// A Cacher is used to create request caches for http handlers.
+type Cacher interface {
+ // Cache returns a new middleware that caches every request.
+ // The name of the cache is used only for metrics.
+ // The expirer is a func that is used to map a new request to its TTL.
+ // authHeader is the header key used by the cache to know that a
+ // request should bypass the cache.
+ // authValues is the set of values that could be set on the authHeader in
+ // order to bypass the cache.
+ Cache(name string, expirer func(r *http.Request) time.Duration, authValues []string) func(http.Handler) http.Handler
+}
+
// Install registers server routes using the given handler registration func.
// authValues is the set of values that can be set on authHeader to bypass the
// cache.
-func (s *Server) Install(handle func(string, http.Handler), redisClient *redis.Client, authValues []string) {
+func (s *Server) Install(handle func(string, http.Handler), cacher Cacher, authValues []string) {
var (
detailHandler http.Handler = s.errorHandler(s.serveDetails)
fetchHandler http.Handler = s.errorHandler(s.serveFetch)
searchHandler http.Handler = s.errorHandler(s.serveSearch)
vulnHandler http.Handler = s.errorHandler(s.serveVuln)
)
- if redisClient != nil {
+ if cacher != nil {
// The cache middleware uses the URL string as the key for content served
// by the handlers it wraps. Be careful not to wrap the handler it returns
// with a handler that rewrites the URL in a way that could cause key
// collisions, like http.StripPrefix.
- detailHandler = middleware.Cache("details", redisClient, detailsTTL, authValues)(detailHandler)
- searchHandler = middleware.Cache("search", redisClient, searchTTL, authValues)(searchHandler)
- vulnHandler = middleware.Cache("vuln", redisClient, vulnTTL, authValues)(vulnHandler)
+ detailHandler = cacher.Cache("details", detailsTTL, authValues)(detailHandler)
+ searchHandler = cacher.Cache("search", searchTTL, authValues)(searchHandler)
+ vulnHandler = cacher.Cache("vuln", vulnTTL, authValues)(vulnHandler)
}
// Each AppEngine instance is created in response to a start request, which
// is an empty HTTP GET request to /_ah/start when scaling is set to manual
diff --git a/internal/frontend/server_test.go b/internal/frontend/server_test.go
index 1d9e7be..0bd94a1 100644
--- a/internal/frontend/server_test.go
+++ b/internal/frontend/server_test.go
@@ -31,6 +31,7 @@
"golang.org/x/pkgsite/internal/cookie"
"golang.org/x/pkgsite/internal/derrors"
"golang.org/x/pkgsite/internal/experiment"
+ "golang.org/x/pkgsite/internal/middleware"
"golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/testing/htmlcheck"
"golang.org/x/pkgsite/internal/testing/pagecheck"
@@ -1234,7 +1235,7 @@
}
defer rs.Close()
- _, handler, _ := newTestServer(t, nil, redis.NewClient(&redis.Options{Addr: rs.Addr()}))
+ _, handler, _ := newTestServer(t, nil, middleware.NewCacher(redis.NewClient(&redis.Options{Addr: rs.Addr()})))
for _, test := range []struct {
name, path string
@@ -1311,7 +1312,7 @@
}
defer rs.Close()
- _, handler, _ := newTestServer(t, nil, redis.NewClient(&redis.Options{Addr: rs.Addr()}))
+ _, handler, _ := newTestServer(t, nil, middleware.NewCacher(redis.NewClient(&redis.Options{Addr: rs.Addr()})))
for _, test := range []struct {
name, path, flash string
diff --git a/internal/middleware/caching.go b/internal/middleware/caching.go
index 4ef5e87..32cb0a8 100644
--- a/internal/middleware/caching.go
+++ b/internal/middleware/caching.go
@@ -98,13 +98,23 @@
// An Expirer computes the TTL that should be used when caching a page.
type Expirer func(r *http.Request) time.Duration
-// TTL returns an Expirer that expires all pages after the given TTL.
-func TTL(ttl time.Duration) Expirer {
+// ttl returns an Expirer that expires all pages after the given TTL.
+func ttl(ttl time.Duration) Expirer {
return func(r *http.Request) time.Duration {
return ttl
}
}
+// NewCacher returns a new Cacher, used for creating a middleware
+// that caches each request.
+func NewCacher(client *redis.Client) *cacher {
+ return &cacher{client: client}
+}
+
+type cacher struct {
+ client *redis.Client
+}
+
// Cache returns a new Middleware that caches every request.
// The name of the cache is used only for metrics.
// The expirer is a func that is used to map a new request to its TTL.
@@ -112,12 +122,12 @@
// request should bypass the cache.
// authValues is the set of values that could be set on the authHeader in
// order to bypass the cache.
-func Cache(name string, client *redis.Client, expirer Expirer, authValues []string) Middleware {
+func (c *cacher) Cache(name string, expirer func(r *http.Request) time.Duration, authValues []string) func(http.Handler) http.Handler {
return func(h http.Handler) http.Handler {
return &cache{
name: name,
authValues: authValues,
- cache: icache.New(client),
+ cache: icache.New(c.client),
delegate: h,
expirer: expirer,
}
diff --git a/internal/middleware/caching_test.go b/internal/middleware/caching_test.go
index 2f46b93..7b8dd2e 100644
--- a/internal/middleware/caching_test.go
+++ b/internal/middleware/caching_test.go
@@ -49,7 +49,7 @@
c := redis.NewClient(&redis.Options{Addr: s.Addr()})
mux := http.NewServeMux()
- mux.Handle("/A", Cache("A", c, TTL(1*time.Minute), []string{"yes"})(handler))
+ mux.Handle("/A", NewCacher(c).Cache("A", ttl(1*time.Minute), []string{"yes"})(handler))
mux.Handle("/B", handler)
ts := httptest.NewServer(mux)
view.Register(CacheResultCount)
diff --git a/internal/testing/integration/frontend_test.go b/internal/testing/integration/frontend_test.go
index f3cc6a4..ceaea33 100644
--- a/internal/testing/integration/frontend_test.go
+++ b/internal/testing/integration/frontend_test.go
@@ -46,7 +46,11 @@
t.Fatal(err)
}
mux := http.NewServeMux()
- s.Install(mux.Handle, rc, nil)
+ var cacher frontend.Cacher
+ if rc != nil {
+ cacher = middleware.NewCacher(rc)
+ }
+ s.Install(mux.Handle, cacher, nil)
// Get experiments from the context. Fully roll them out.
expNames := experiment.FromContext(ctx).Active()
