Skip to content

Commit

Permalink
Use the same timestamp for etag as for last-modified
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mamhoff committed May 13, 2024
1 parent 7b1d334 commit 19f46bd
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
8 changes: 6 additions & 2 deletions app/controllers/alchemy/json_api/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion spec/requests/alchemy/json_api/pages_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 19f46bd

Please sign in to comment.