diff --git a/api/error_response.go b/api/error_response.go index 169b715b..d0782e92 100644 --- a/api/error_response.go +++ b/api/error_response.go @@ -3,6 +3,7 @@ package api import ( "errors" "net/http" + "strings" "github.com/gofiber/fiber/v2" "github.com/jackc/pgx/v5" @@ -25,6 +26,9 @@ func errorHandler(logger *zap.Logger) func(*fiber.Ctx, error) error { logger.Error(err.Error(), zap.String("url", ctx.OriginalURL())) } + if strings.HasPrefix(ctx.Path(), "/sitemaps/") { + ctx.Set("Cache-Control", "no-store") + } return ctx.Status(code).JSON(&fiber.Map{ "code": code, diff --git a/api/server.go b/api/server.go index 61b13660..4284a5a8 100644 --- a/api/server.go +++ b/api/server.go @@ -170,9 +170,9 @@ func NewApiServer(config config.Config) *ApiServer { panic(err) } - // Entries carry their own freshness window so expired sitemap pages can be - // served stale while a background refresh rebuilds them. - sitemapPageCache, err := otter.MustBuilder[string, sitemapPageCacheEntry](256). + // Entries carry their own freshness window so expired sitemap XML can be + // served stale while a background refresh rebuilds it. + sitemapXMLCache, err := otter.MustBuilder[string, sitemapXMLCacheEntry](256). CollectStats(). Build() if err != nil { @@ -290,7 +290,7 @@ func NewApiServer(config config.Config) *ApiServer { qualifiedPlaylistsCache: &qualifiedPlaylistsCache, relatedUsersCache: &relatedUsersCache, genresPopularCache: &genresPopularCache, - sitemapPageCache: &sitemapPageCache, + sitemapXMLCache: &sitemapXMLCache, requestValidator: requestValidator, rewardAttester: rewardAttester, transactionSender: transactionSender, @@ -851,7 +851,7 @@ type ApiServer struct { qualifiedPlaylistsCache *otter.Cache[string, []int32] relatedUsersCache *otter.Cache[string, []int32] genresPopularCache *otter.Cache[string, []PopularGenre] - sitemapPageCache *otter.Cache[string, sitemapPageCacheEntry] + sitemapXMLCache *otter.Cache[string, sitemapXMLCacheEntry] requestValidator *RequestValidator rewardManagerClient *reward_manager.RewardManagerClient claimableTokensClient *claimable_tokens.ClaimableTokensClient diff --git a/api/v1_sitemaps.go b/api/v1_sitemaps.go index 09ffca98..a6a97975 100644 --- a/api/v1_sitemaps.go +++ b/api/v1_sitemaps.go @@ -18,7 +18,8 @@ import ( const sitemapLimit = 40_000 const sitemapCountCacheTTL = 1 * time.Hour -const sitemapPageCacheTTL = 1 * time.Hour +const sitemapXMLCacheTTL = 1 * time.Hour +const sitemapCacheControl = "public, max-age=3600, stale-while-revalidate=86400, stale-if-error=86400" var sitemapPageRegex = regexp.MustCompile(`^(\d+)\.xml$`) @@ -56,7 +57,7 @@ type cachedCount struct { refreshing bool // true if a background refresh is already in flight } -type sitemapPageCacheEntry struct { +type sitemapXMLCacheEntry struct { data []byte expiresAt time.Time refreshing bool @@ -94,42 +95,47 @@ func setCachedCount(key string, value int64) { } } -func (app *ApiServer) getCachedSitemapPage(key string) (data []byte, exists bool, needsRefresh bool) { - if app.sitemapPageCache == nil { +func (app *ApiServer) getCachedSitemapXML(key string) (data []byte, exists bool, needsRefresh bool) { + if app.sitemapXMLCache == nil { return nil, false, false } - entry, ok := app.sitemapPageCache.Get(key) + entry, ok := app.sitemapXMLCache.Get(key) if !ok { return nil, false, false } if time.Now().After(entry.expiresAt) && !entry.refreshing { entry.refreshing = true - app.sitemapPageCache.Set(key, entry) + app.sitemapXMLCache.Set(key, entry) return entry.data, true, true } return entry.data, true, false } -func (app *ApiServer) setCachedSitemapPage(key string, data []byte) { - if app.sitemapPageCache == nil { +func (app *ApiServer) setCachedSitemapXML(key string, data []byte) { + if app.sitemapXMLCache == nil { return } - app.sitemapPageCache.Set(key, sitemapPageCacheEntry{ + app.sitemapXMLCache.Set(key, sitemapXMLCacheEntry{ data: data, - expiresAt: time.Now().Add(sitemapPageCacheTTL), + expiresAt: time.Now().Add(sitemapXMLCacheTTL), }) } -func (app *ApiServer) clearSitemapPageRefreshing(key string) { - if app.sitemapPageCache == nil { +func (app *ApiServer) clearSitemapXMLRefreshing(key string) { + if app.sitemapXMLCache == nil { return } - entry, ok := app.sitemapPageCache.Get(key) + entry, ok := app.sitemapXMLCache.Get(key) if !ok { return } entry.refreshing = false - app.sitemapPageCache.Set(key, entry) + app.sitemapXMLCache.Set(key, entry) +} + +func setSitemapHeaders(c *fiber.Ctx) { + c.Set("Content-Type", "text/xml") + c.Set("Cache-Control", sitemapCacheControl) } type sitemapURL struct { @@ -254,7 +260,7 @@ func (app *ApiServer) sitemapDefault(c *fiber.Ctx) error { if err != nil { return err } - c.Set("Content-Type", "text/xml") + setSitemapHeaders(c) return c.Send(data) } @@ -264,7 +270,7 @@ func (app *ApiServer) sitemapDefaults(c *fiber.Ctx) error { if err != nil { return err } - c.Set("Content-Type", "text/xml") + setSitemapHeaders(c) return c.Send(data) } @@ -277,11 +283,56 @@ func (app *ApiServer) sitemapTypeIndex(c *fiber.Ctx) error { return fiber.NewError(400, fmt.Sprintf("Invalid sitemap type %s, should be one of track, playlist, user", entityType)) } + cacheKey := entityType + ":index.xml" + if data, ok, needsRefresh := app.getCachedSitemapXML(cacheKey); ok { + if needsRefresh { + go app.refreshSitemapTypeIndex(cacheKey, entityType) + } + setSitemapHeaders(c) + return c.Send(data) + } + count, err := app.getSitemapCount(c, entityType) if err != nil { return err } + data, err := app.buildSitemapTypeIndex(entityType, count) + if err != nil { + return err + } + app.setCachedSitemapXML(cacheKey, data) + setSitemapHeaders(c) + return c.Send(data) +} + +func (app *ApiServer) refreshSitemapTypeIndex(cacheKey string, entityType string) { + count, err := app.fetchSitemapCount(entityType) + if err != nil { + app.clearSitemapXMLRefreshing(cacheKey) + app.logger.Error( + "failed to refresh sitemap index", + zap.String("cache_key", cacheKey), + zap.String("type", entityType), + zap.Error(err), + ) + return + } + data, err := app.buildSitemapTypeIndex(entityType, count) + if err != nil { + app.clearSitemapXMLRefreshing(cacheKey) + app.logger.Error( + "failed to build sitemap index", + zap.String("cache_key", cacheKey), + zap.String("type", entityType), + zap.Error(err), + ) + return + } + app.setCachedSitemapXML(cacheKey, data) +} + +func (app *ApiServer) buildSitemapTypeIndex(entityType string, count int64) ([]byte, error) { pages := numPages(count, sitemapLimit) entries := make([]sitemapEntry, pages) for i := 1; i <= pages; i++ { @@ -292,10 +343,9 @@ func (app *ApiServer) sitemapTypeIndex(c *fiber.Ctx) error { data, err := buildSitemapIndex(entries) if err != nil { - return err + return nil, err } - c.Set("Content-Type", "text/xml") - return c.Send(data) + return data, nil } func (app *ApiServer) sitemapTypePage(c *fiber.Ctx) error { @@ -312,11 +362,11 @@ func (app *ApiServer) sitemapTypePage(c *fiber.Ctx) error { } cacheKey := entityType + ":" + fileName - if data, ok, needsRefresh := app.getCachedSitemapPage(cacheKey); ok { + if data, ok, needsRefresh := app.getCachedSitemapXML(cacheKey); ok { if needsRefresh { go app.refreshSitemapPage(cacheKey, entityType, pageNumber) } - c.Set("Content-Type", "text/xml") + setSitemapHeaders(c) return c.Send(data) } @@ -324,15 +374,15 @@ func (app *ApiServer) sitemapTypePage(c *fiber.Ctx) error { if err != nil { return err } - app.setCachedSitemapPage(cacheKey, data) - c.Set("Content-Type", "text/xml") + app.setCachedSitemapXML(cacheKey, data) + setSitemapHeaders(c) return c.Send(data) } func (app *ApiServer) refreshSitemapPage(cacheKey string, entityType string, pageNumber int) { data, err := app.buildSitemapPage(context.Background(), entityType, pageNumber) if err != nil { - app.clearSitemapPageRefreshing(cacheKey) + app.clearSitemapXMLRefreshing(cacheKey) app.logger.Error( "failed to refresh sitemap page", zap.String("cache_key", cacheKey), @@ -342,7 +392,7 @@ func (app *ApiServer) refreshSitemapPage(cacheKey string, entityType string, pag ) return } - app.setCachedSitemapPage(cacheKey, data) + app.setCachedSitemapXML(cacheKey, data) } func (app *ApiServer) buildSitemapPage(ctx context.Context, entityType string, pageNumber int) ([]byte, error) { diff --git a/api/v1_sitemaps_test.go b/api/v1_sitemaps_test.go index 030b4e52..771ee8d5 100644 --- a/api/v1_sitemaps_test.go +++ b/api/v1_sitemaps_test.go @@ -1,6 +1,7 @@ package api import ( + "net/http/httptest" "strings" "testing" "time" @@ -54,6 +55,12 @@ func sitemapTestApp(t *testing.T) *ApiServer { return app } +func clearCachedSitemapCount(entityType string) { + sitemapCountMu.Lock() + defer sitemapCountMu.Unlock() + delete(sitemapCountCache, entityType) +} + func TestSitemapDefault(t *testing.T) { app := sitemapTestApp(t) status, body := testGet(t, app, "/sitemaps/default.xml") @@ -82,10 +89,7 @@ func TestSitemapDefaults(t *testing.T) { func TestSitemapTrackIndex(t *testing.T) { app := sitemapTestApp(t) - // Clear the count cache so test gets fresh data - sitemapCountMu.Lock() - delete(sitemapCountCache, "track") - sitemapCountMu.Unlock() + clearCachedSitemapCount("track") status, body := testGet(t, app, "/sitemaps/track/index.xml") require.Equal(t, 200, status) @@ -95,6 +99,41 @@ func TestSitemapTrackIndex(t *testing.T) { assert.Contains(t, xml, "/sitemaps/track/1.xml") } +func TestSitemapTrackIndexCache(t *testing.T) { + app := sitemapTestApp(t) + + cached := []byte(`cached-track-index`) + app.setCachedSitemapXML("track:index.xml", cached) + + status, body := testGet(t, app, "/sitemaps/track/index.xml") + require.Equal(t, 200, status) + assert.Contains(t, string(body), "cached-track-index") +} + +func TestSitemapTrackIndexStaleCacheRefreshesInBackground(t *testing.T) { + app := sitemapTestApp(t) + clearCachedSitemapCount("track") + + stale := []byte(`stale-track-index`) + require.True(t, app.sitemapXMLCache.Set("track:index.xml", sitemapXMLCacheEntry{ + data: stale, + expiresAt: time.Now().Add(-time.Minute), + })) + + status, body := testGet(t, app, "/sitemaps/track/index.xml") + require.Equal(t, 200, status) + assert.Contains(t, string(body), "stale-track-index") + + require.Eventually(t, func() bool { + entry, ok := app.sitemapXMLCache.Get("track:index.xml") + return ok && + !entry.refreshing && + time.Now().Before(entry.expiresAt) && + strings.Contains(string(entry.data), "/sitemaps/track/1.xml") && + !strings.Contains(string(entry.data), "stale-track-index") + }, time.Second, 10*time.Millisecond) +} + func TestSitemapTrackPage(t *testing.T) { app := sitemapTestApp(t) status, body := testGet(t, app, "/sitemaps/track/1.xml") @@ -117,7 +156,7 @@ func TestSitemapTrackPageCache(t *testing.T) { app := sitemapTestApp(t) cached := []byte(`cached-track-page`) - app.setCachedSitemapPage("track:1.xml", cached) + app.setCachedSitemapXML("track:1.xml", cached) status, body := testGet(t, app, "/sitemaps/track/1.xml") require.Equal(t, 200, status) @@ -128,7 +167,7 @@ func TestSitemapTrackPageStaleCacheRefreshesInBackground(t *testing.T) { app := sitemapTestApp(t) stale := []byte(`stale-track-page`) - require.True(t, app.sitemapPageCache.Set("track:1.xml", sitemapPageCacheEntry{ + require.True(t, app.sitemapXMLCache.Set("track:1.xml", sitemapXMLCacheEntry{ data: stale, expiresAt: time.Now().Add(-time.Minute), })) @@ -138,7 +177,7 @@ func TestSitemapTrackPageStaleCacheRefreshesInBackground(t *testing.T) { assert.Contains(t, string(body), "stale-track-page") require.Eventually(t, func() bool { - entry, ok := app.sitemapPageCache.Get("track:1.xml") + entry, ok := app.sitemapXMLCache.Get("track:1.xml") return ok && !entry.refreshing && time.Now().Before(entry.expiresAt) && @@ -197,3 +236,27 @@ func TestSitemapContentType(t *testing.T) { require.Equal(t, 200, status) assert.Contains(t, string(body), "