From 19f46bd9ec7543e4b168fc3cc42b185d18d1ffe9 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Mon, 13 May 2024 10:53:00 +0200 Subject: [PATCH] Use the same timestamp for etag as for last-modified According to MDN, the last-modified header is a fallback to the etag. Thus, we should be using the same value explicitly. Prior to this commit, the value was implicitly the same, because under the hood, Alchemy used the `published_at` timestamp. With this commit, it's explicitly the same and allows Alchemy to implement a `last_modified_at` method that will be respected. --- app/controllers/alchemy/json_api/pages_controller.rb | 8 ++++++-- spec/requests/alchemy/json_api/pages_spec.rb | 6 +++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/controllers/alchemy/json_api/pages_controller.rb b/app/controllers/alchemy/json_api/pages_controller.rb index ffaafdb..674312c 100644 --- a/app/controllers/alchemy/json_api/pages_controller.rb +++ b/app/controllers/alchemy/json_api/pages_controller.rb @@ -10,9 +10,12 @@ def index jsonapi_filter(page_scope, allowed) do |filtered_pages| @pages = filtered_pages.result + last_modified_page = @pages.select { |page| last_modified_for(page) }.max_by { |page| last_modified_for(page) } + last_modified_at = last_modified_for(last_modified_page) || Time.current + latest_etag = last_modified_page&.cache_key_with_version || Time.current.to_s if !@pages.all?(&:cache_page?) render_pages_json(allowed) && return - elsif stale?(last_modified: @pages.maximum(:published_at), etag: @pages.max_by(&:cache_key_with_version)&.cache_key_with_version) + elsif stale?(last_modified: last_modified_at, etag: latest_etag) render_pages_json(allowed) end end @@ -61,7 +64,8 @@ def load_page_for_cache_key end def last_modified_for(page) - page.published_at + return unless page + page.respond_to?(:last_modified_at) ? page.last_modified_at : page.published_at end def jsonapi_meta(pages) diff --git a/spec/requests/alchemy/json_api/pages_spec.rb b/spec/requests/alchemy/json_api/pages_spec.rb index 695d39d..c35d09b 100644 --- a/spec/requests/alchemy/json_api/pages_spec.rb +++ b/spec/requests/alchemy/json_api/pages_spec.rb @@ -43,7 +43,11 @@ it "sets public cache headers" do get alchemy_json_api.page_path(page) - expect(response.headers["Last-Modified"]).to eq(page.published_at.utc.httpdate) + if Alchemy.gem_version >= Gem::Version.new("7.2.0") + expect(response.headers["Last-Modified"]).to eq(page.public_version.updated_at.utc.httpdate) + else + expect(response.headers["Last-Modified"]).to eq(page.published_at.utc.httpdate) + end expect(response.headers["ETag"]).to eq('W/"0741fe32d81bfdabfeb47d9939c5f6b7"') expect(response.headers["Cache-Control"]).to eq("max-age=10800, public, must-revalidate") end