Add cache control to Files API static assets
Brand assets served at /api/files/v1/static had no cache headers. Introduce brand.Assets to own the embedded filesystem, content-hash ETags, and HTTP serving. Responses now carry Cache-Control and ETag so clients can cache and revalidate; stable email URLs stay revalidatable (max-age=3600, no immutable). Replace hardcoded Default*Path constants with StaticPathPrefix, logical filename constants, and StaticPath(). NewAssets validates required assets at startup so a rename fails fast instead of 404ing in sent emails. The files handler keeps routing and 404 rendering; ServeAssets sets cache headers and serves the file. Signed-off-by: Ludovic Vielle <ludovic@probo.com>
This commit is contained in:
@@ -17,7 +17,6 @@ package files_v1
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io/fs"
|
||||
"net/http"
|
||||
"time"
|
||||
|
||||
@@ -31,7 +30,7 @@ import (
|
||||
"go.probo.inc/probo/pkg/probo"
|
||||
"go.probo.inc/probo/pkg/securecookie"
|
||||
"go.probo.inc/probo/pkg/server/api/authn"
|
||||
"go.probo.inc/probo/pkg/server/jsonutil"
|
||||
"go.probo.inc/probo/pkg/server/jsonx"
|
||||
)
|
||||
|
||||
const presignedURLExpiry = 1 * time.Hour
|
||||
@@ -41,6 +40,7 @@ type Handler struct {
|
||||
fileSvc *filemanager.Service
|
||||
probo *probo.Service
|
||||
iamSvc *iam.Service
|
||||
assets *brand.Assets
|
||||
}
|
||||
|
||||
func NewMux(
|
||||
@@ -56,6 +56,7 @@ func NewMux(
|
||||
fileSvc: fileSvc,
|
||||
probo: proboSvc,
|
||||
iamSvc: iamSvc,
|
||||
assets: brand.NewAssets(),
|
||||
}
|
||||
|
||||
r := chi.NewRouter()
|
||||
@@ -77,12 +78,15 @@ func NewMux(
|
||||
func (h *Handler) handleGetStaticFile(w http.ResponseWriter, r *http.Request) {
|
||||
file := chi.URLParam(r, "file")
|
||||
|
||||
if _, statErr := fs.Stat(brand.Assets, file); statErr == nil {
|
||||
http.ServeFileFS(w, r, brand.Assets, file)
|
||||
if _, statErr := h.assets.Stat(file); statErr != nil {
|
||||
jsonx.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
return
|
||||
}
|
||||
|
||||
jsonutil.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
// ServeAssets honors the ETag header we set above for If-None-Match (and
|
||||
// If-Range), so it emits 304 Not Modified and handles range requests without
|
||||
// any extra conditional logic here.
|
||||
h.assets.ServeAssets(w, r, file)
|
||||
}
|
||||
|
||||
func (h *Handler) handleGetPublicFile(w http.ResponseWriter, r *http.Request) {
|
||||
@@ -90,14 +94,14 @@ func (h *Handler) handleGetPublicFile(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
fileID, err := gid.ParseGID(fileIDStr)
|
||||
if err != nil {
|
||||
jsonutil.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
jsonx.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
return
|
||||
}
|
||||
|
||||
file, err := h.fileSvc.GetPublicFile(r.Context(), fileID)
|
||||
if err != nil {
|
||||
if errors.Is(err, coredata.ErrResourceNotFound) {
|
||||
jsonutil.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
jsonx.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -107,7 +111,7 @@ func (h *Handler) handleGetPublicFile(w http.ResponseWriter, r *http.Request) {
|
||||
log.Error(err),
|
||||
log.String("file_id", fileIDStr),
|
||||
)
|
||||
jsonutil.RenderInternalServerError(w)
|
||||
jsonx.RenderInternalServerError(w)
|
||||
|
||||
return
|
||||
}
|
||||
@@ -120,7 +124,7 @@ func (h *Handler) handleGetPublicFile(w http.ResponseWriter, r *http.Request) {
|
||||
log.Error(err),
|
||||
log.String("file_id", fileIDStr),
|
||||
)
|
||||
jsonutil.RenderInternalServerError(w)
|
||||
jsonx.RenderInternalServerError(w)
|
||||
|
||||
return
|
||||
}
|
||||
@@ -133,7 +137,7 @@ func (h *Handler) handleGetFile(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
fileID, err := gid.ParseGID(fileIDStr)
|
||||
if err != nil {
|
||||
jsonutil.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
jsonx.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -153,19 +157,19 @@ func (h *Handler) handleGetFile(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
scope, err := h.iamSvc.Authorizer.Authorize(ctx, params)
|
||||
if err != nil {
|
||||
jsonutil.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
jsonx.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
return
|
||||
}
|
||||
|
||||
f, err := h.probo.Files.Get(ctx, scope, fileID)
|
||||
if err != nil {
|
||||
if errors.Is(err, coredata.ErrResourceNotFound) {
|
||||
jsonutil.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
jsonx.RenderNotFound(w, fmt.Errorf("file not found"))
|
||||
return
|
||||
}
|
||||
|
||||
h.logger.ErrorCtx(ctx, "cannot get file", log.Error(err), log.String("file_id", fileIDStr))
|
||||
jsonutil.RenderInternalServerError(w)
|
||||
jsonx.RenderInternalServerError(w)
|
||||
|
||||
return
|
||||
}
|
||||
@@ -173,7 +177,7 @@ func (h *Handler) handleGetFile(w http.ResponseWriter, r *http.Request) {
|
||||
presignedURL, err := h.fileSvc.GeneratePresignedURL(ctx, f, presignedURLExpiry)
|
||||
if err != nil {
|
||||
h.logger.ErrorCtx(ctx, "cannot generate file URL", log.Error(err), log.String("file_id", fileIDStr))
|
||||
jsonutil.RenderInternalServerError(w)
|
||||
jsonx.RenderInternalServerError(w)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
@@ -19,10 +19,12 @@ import (
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.gearno.de/kit/log"
|
||||
"go.probo.inc/probo/pkg/securecookie"
|
||||
)
|
||||
@@ -67,6 +69,43 @@ func TestHandleGetFile_InvalidGID(t *testing.T) {
|
||||
assert.Equal(t, http.StatusNotFound, rec.Code)
|
||||
}
|
||||
|
||||
func TestHandleGetStaticFile(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mux := NewMux(
|
||||
log.NewLogger(log.WithOutput(io.Discard)),
|
||||
nil,
|
||||
nil,
|
||||
nil,
|
||||
securecookie.Config{},
|
||||
"test-secret",
|
||||
)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/static/probo.png", nil)
|
||||
mux.ServeHTTP(rec, req)
|
||||
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
require.Contains(t, rec.Header().Get("Cache-Control"), "max-age=3600")
|
||||
|
||||
etag := rec.Header().Get("ETag")
|
||||
require.NotEmpty(t, etag)
|
||||
require.True(t, strings.HasPrefix(etag, `"`) && strings.HasSuffix(etag, `"`))
|
||||
|
||||
recNotModified := httptest.NewRecorder()
|
||||
reqNotModified := httptest.NewRequest(http.MethodGet, "/static/probo.png", nil)
|
||||
reqNotModified.Header.Set("If-None-Match", etag)
|
||||
mux.ServeHTTP(recNotModified, reqNotModified)
|
||||
|
||||
require.Equal(t, http.StatusNotModified, recNotModified.Code)
|
||||
|
||||
recMissing := httptest.NewRecorder()
|
||||
reqMissing := httptest.NewRequest(http.MethodGet, "/static/does-not-exist.png", nil)
|
||||
mux.ServeHTTP(recMissing, reqMissing)
|
||||
|
||||
require.Equal(t, http.StatusNotFound, recMissing.Code)
|
||||
}
|
||||
|
||||
func TestHandleGetFile_UnauthenticatedReturns401(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user