From 0b28e436cbafeffabe0598038d24a7d42f54b60a Mon Sep 17 00:00:00 2001
From: Thomas von Deyen <thomas@vondeyen.com>
Date: Mon, 6 May 2024 17:25:26 +0200
Subject: [PATCH] fix(HTTP Cache): Use cache_key_with_version for etag

The cache_key is just the page id. We need to include
the cache_version (the page published_at date).
---
 .../alchemy/json_api/pages_controller.rb      |  4 ++--
 spec/requests/alchemy/json_api/pages_spec.rb  | 20 ++++++++++---------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/app/controllers/alchemy/json_api/pages_controller.rb b/app/controllers/alchemy/json_api/pages_controller.rb
index 850ee4c..ffaafdb 100644
--- a/app/controllers/alchemy/json_api/pages_controller.rb
+++ b/app/controllers/alchemy/json_api/pages_controller.rb
@@ -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
@@ -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
diff --git a/spec/requests/alchemy/json_api/pages_spec.rb b/spec/requests/alchemy/json_api/pages_spec.rb
index 114112d..1d908cf 100644
--- a/spec/requests/alchemy/json_api/pages_spec.rb
+++ b/spec/requests/alchemy/json_api/pages_spec.rb
@@ -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
 
@@ -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
 
@@ -46,7 +48,7 @@
               :alchemy_page,
               :public,
               :restricted,
-              published_at: DateTime.yesterday
+              published_at: published_at
             )
           end
 
@@ -62,7 +64,7 @@
               :alchemy_page,
               :public,
               page_layout: "contact",
-              published_at: DateTime.yesterday
+              published_at: published_at
             )
           end
 
@@ -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] }
@@ -196,7 +198,7 @@
                 :alchemy_page,
                 :public,
                 :restricted,
-                published_at: DateTime.yesterday
+                published_at: published_at
               )
             end
 
@@ -212,7 +214,7 @@
                 :alchemy_page,
                 :public,
                 page_layout: "contact",
-                published_at: DateTime.yesterday
+                published_at: published_at
               )
             end
 
@@ -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"})
@@ -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