Skip to content

Commit

Permalink
fix(HTTP Cache): Use cache_key_with_version for etag
Browse files Browse the repository at this point in the history
The cache_key is just the page id. We need to include
the cache_version (the page published_at date).
  • Loading branch information
tvdeyen committed May 7, 2024
1 parent eb16066 commit 0b28e43
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 11 deletions.
4 changes: 2 additions & 2 deletions app/controllers/alchemy/json_api/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def index
@pages = filtered_pages.result
if !@pages.all?(&:cache_page?)
render_pages_json(allowed) && return
elsif stale?(last_modified: @pages.maximum(:published_at), etag: @pages.max_by(&:cache_key)&.cache_key)
elsif stale?(last_modified: @pages.maximum(:published_at), etag: @pages.max_by(&:cache_key_with_version)&.cache_key_with_version)
render_pages_json(allowed)
end
end
Expand All @@ -25,7 +25,7 @@ def show
render(jsonapi: api_page(load_page)) && return
end

if stale?(last_modified: last_modified_for(@page), etag: @page.cache_key)
if stale?(last_modified: last_modified_for(@page), etag: @page.cache_key_with_version)
# Only load page with all includes when browser cache is stale
render jsonapi: api_page(load_page)
end
Expand Down
20 changes: 11 additions & 9 deletions spec/requests/alchemy/json_api/pages_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@
)
end

let(:published_at) { DateTime.parse("2024-05-04 00:00:00") }

describe "GET /alchemy/json_api/pages/:id" do
context "a published page" do
let(:page) do
FactoryBot.create(
:alchemy_page,
:public,
published_at: DateTime.yesterday
published_at: published_at
)
end

Expand All @@ -36,7 +38,7 @@
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)
expect(response.headers["ETag"]).to match(/W\/".+"/)
expect(response.headers["ETag"]).to eq('W/"0741fe32d81bfdabfeb47d9939c5f6b7"')
expect(response.headers["Cache-Control"]).to eq("max-age=10800, public, must-revalidate")
end

Expand All @@ -46,7 +48,7 @@
:alchemy_page,
:public,
:restricted,
published_at: DateTime.yesterday
published_at: published_at
)
end

Expand All @@ -62,7 +64,7 @@
:alchemy_page,
:public,
page_layout: "contact",
published_at: DateTime.yesterday
published_at: published_at
)
end

Expand Down Expand Up @@ -172,7 +174,7 @@
context "with layoutpages and unpublished pages" do
let!(:layoutpage) { FactoryBot.create(:alchemy_page, :layoutpage, :public) }
let!(:non_public_page) { FactoryBot.create(:alchemy_page) }
let!(:public_page) { FactoryBot.create(:alchemy_page, :public, published_at: Date.yesterday) }
let!(:public_page) { FactoryBot.create(:alchemy_page, :public, published_at: published_at) }

context "as anonymous user" do
let!(:pages) { [public_page] }
Expand All @@ -196,7 +198,7 @@
:alchemy_page,
:public,
:restricted,
published_at: DateTime.yesterday
published_at: published_at
)
end

Expand All @@ -212,7 +214,7 @@
:alchemy_page,
:public,
page_layout: "contact",
published_at: DateTime.yesterday
published_at: published_at
)
end

Expand Down Expand Up @@ -265,7 +267,7 @@
context "with filters" do
let!(:standard_page) { FactoryBot.create(:alchemy_page, :public, published_at: 2.weeks.ago) }
let!(:news_page) { FactoryBot.create(:alchemy_page, :public, page_layout: "news", published_at: 1.week.ago) }
let!(:news_page2) { FactoryBot.create(:alchemy_page, :public, name: "News", page_layout: "news", published_at: Date.yesterday) }
let!(:news_page2) { FactoryBot.create(:alchemy_page, :public, name: "News", page_layout: "news", published_at: published_at) }

it "returns only matching pages by page_layout" do
get alchemy_json_api.pages_path(filter: {page_layout_eq: "news"})
Expand Down Expand Up @@ -299,7 +301,7 @@
it "sets cache headers of latest matching page" do
get alchemy_json_api.pages_path(filter: {page_layout_eq: "news"})
expect(response.headers["Last-Modified"]).to eq(news_page2.published_at.utc.httpdate)
expect(response.headers["ETag"]).to match(/W\/".+"/)
expect(response.headers["ETag"]).to eq('W/"e7a1c8beb22b58e94a605594d79766ad"')
expect(response.headers["Cache-Control"]).to eq("max-age=10800, public, must-revalidate")
end
end
Expand Down

0 comments on commit 0b28e43

Please sign in to comment.