maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Comments API: fix caching issues (#4744) [deploy]

Browse source 
* Use ancestry in the index and add test

* Add tests for show action

* Return complete comments trees for index, kill n+1 and use correct edge cache key

* Return complete comments tree for show, kill n+1 and use correct edge cache key

* Refactor tests and add surrogate keys tests

* Preload users for the whole subtree

* Clarify explanation of the edge cache keys construction

* Refactor partials

* Remove action caching

* Replace .map.flatten with .flat_map

* Fix surrogate key prefix according to Fastly docs

* Add comment.purge to Comments::BustCacheJob to use Fastly-Rails

* Fix surrogate key spec
This commit is contained in:
rhymes 2020-01-15 16:45:56 +01:00 • committed by Ben Halpern
parent bbaa5902c7
commit bcd85452e9
8 changed files with 101 additions and 124 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
app/controllers/api/v0/comments_controller.rb
View file

@ -3,21 +3,42 @@ module Api
class CommentsController < ApiController
respond_to :json
caches_action :index,
cache_path: proc { |c| c.params.permit! },
expires_in: 10.minutes
caches_action :show,
cache_path: proc { |c| c.params.permit! },
expires_in: 10.minutes
before_action :set_cache_control_headers, only: %i[index show]
def index
article = Article.find(params[:a_id])
@comments = article.comments.roots.includes(:user).select(%i[id processed_html user_id ancestry])
@comments = article.comments.includes(:user).select(%i[id processed_html user_id ancestry]).arrange
set_surrogate_key_header "comments", edge_cache_keys(@comments)
end
def show
@comment = Comment.find(params[:id].to_i(26))
tree_with_root_comment = Comment.subtree_of(params[:id].to_i(26)).
includes(:user).
select(%i[id processed_html user_id ancestry]).
arrange
# being only one tree we know that the root comment is the first (and only) key
@comment = tree_with_root_comment.keys.first
@comments = tree_with_root_comment[@comment]
set_surrogate_key_header "comments", edge_cache_keys(tree_with_root_comment)
end
private
# ancestry wraps a single or multiple trees of comments into a single hash,
# in the case of an article comments, the hash has multiple keys (the root comments),
# in the case of a comment and its descendants, the hash has only one key.
# Either way, we need to use recursion to extract all the comment cache keys
# collecting both the keys of each level of root comments and their descendants
# NOTE: the objects are already loaded in memory by "ancestry",
# so no additional SQL query is performed during this extraction, avoiding N+1s
def edge_cache_keys(comments_trees)
comments_trees.keys.flat_map do |comment|
Array.wrap(comment.record_key) + edge_cache_keys(comments_trees[comment])
end
end
end
end

1
app/jobs/comments/bust_cache_job.rb
View file

@ -6,6 +6,7 @@ module Comments
comment = Comment.find_by(id: comment_id)
return unless comment&.commentable
comment.purge
service.call(comment.commentable)
end
end

6
app/views/api/v0/comments/_comment.json.jbuilder Normal file
View file

@ -0,0 +1,6 @@
json.type_of "comment"
json.id_code comment.id_code_generated
json.body_html comment.processed_html
json.partial! "api/v0/shared/user", user: comment.user

6
app/views/api/v0/comments/_comment_with_children.json.jbuilder Normal file
View file

@ -0,0 +1,6 @@
json.partial! "comment", comment: comment
# recursively render the comment subtree
json.children do
json.partial! "comments_with_children", comments: children
end

4
app/views/api/v0/comments/_comments_with_children.json.jbuilder Normal file
View file

@ -0,0 +1,4 @@
json.array! comments.keys do |root_comment|
# ancestry organizes root comments and their descendants in a hash structure
json.partial! "comment_with_children", comment: root_comment, children: comments[root_comment]
end

44
app/views/api/v0/comments/index.json.jbuilder
View file

@ -1,43 +1 @@
json.array! @comments do |comment|
json.type_of "comment"
json.id_code comment.id_code_generated
json.body_html comment.processed_html
json.user do
json.name comment.user.name
json.username comment.user.username
json.twitter_username comment.user.twitter_username
json.github_username comment.user.github_username
json.website_url comment.user.processed_website_url
json.profile_image ProfileImage.new(comment.user).get(640)
json.profile_image_90 ProfileImage.new(comment.user).get(90)
end
json.children comment.children.order(score: :desc).each do |children_comment|
json.id_code children_comment.id_code_generated
json.body_html children_comment.processed_html
json.user do
json.name children_comment.user.name
json.username children_comment.user.username
json.twitter_username children_comment.user.twitter_username
json.github_username children_comment.user.github_username
json.website_url children_comment.user.processed_website_url
json.profile_image ProfileImage.new(children_comment.user).get(640)
json.profile_image_90 ProfileImage.new(children_comment.user).get(90)
end
json.children children_comment.children.order(score: :desc).each do |grandchild_comment|
json.id_code grandchild_comment.id_code_generated
json.body_html grandchild_comment.processed_html
json.user do
json.name grandchild_comment.user.name
json.username grandchild_comment.user.username
json.twitter_username grandchild_comment.user.twitter_username
json.github_username grandchild_comment.user.github_username
json.website_url grandchild_comment.user.processed_website_url
json.profile_image ProfileImage.new(grandchild_comment.user).get(640)
json.profile_image_90 ProfileImage.new(grandchild_comment.user).get(90)
end
end
end
end
json.partial! "comments_with_children", comments: @comments

39
app/views/api/v0/comments/show.json.jbuilder
View file

@ -1,38 +1 @@
json.type_of "comment"
json.id_code @comment.id_code_generated
json.body_html @comment.processed_html
json.user do
json.name @comment.user.name
json.username @comment.user.username
json.twitter_username @comment.user.twitter_username
json.github_username @comment.user.github_username
json.website_url @comment.user.processed_website_url
json.profile_image ProfileImage.new(@comment.user).get(640)
json.profile_image_90 ProfileImage.new(@comment.user).get(90)
end
json.children @comment.children.order("score DESC").each do |children_comment|
json.id_code children_comment.id_code_generated
json.body_html children_comment.processed_html
json.user do
json.name children_comment.user.name
json.username children_comment.user.username
json.twitter_username children_comment.user.twitter_username
json.github_username children_comment.user.github_username
json.website_url children_comment.user.processed_website_url
json.profile_image ProfileImage.new(children_comment.user).get(640)
json.profile_image_90 ProfileImage.new(children_comment.user).get(90)
end
json.children children_comment.children.order("score DESC").each do |grandchild_comment|
json.id_code grandchild_comment.id_code_generated
json.body_html grandchild_comment.processed_html
json.user do
json.name grandchild_comment.user.name
json.username grandchild_comment.user.username
json.twitter_username grandchild_comment.user.twitter_username
json.github_username grandchild_comment.user.github_username
json.website_url grandchild_comment.user.processed_website_url
json.profile_image ProfileImage.new(grandchild_comment.user).get(640)
json.profile_image_90 ProfileImage.new(grandchild_comment.user).get(90)
end
end
end
json.partial! "comment_with_children", comment: @comment, children: @comments

86
spec/requests/api/v0/comments_spec.rb
View file

@ -6,12 +6,12 @@ RSpec.describe "Api::V0::Comments", type: :request do
end
let_it_be(:article) { create(:article) }
let_it_be(:root_comment) { create(:comment, commentable: article) }
let_it_be(:child_comment) { create(:comment, commentable: article, parent: root_comment) }
let_it_be(:grandchild_comment) { create(:comment, commentable: article, parent: child_comment) }
let_it_be(:great_grandchild_comment) { create(:comment, commentable: article, parent: grandchild_comment) }
describe "GET /api/comments" do
before do
create(:comment, commentable: article)
end
it "returns not found if wrong article id" do
get "/api/comments?a_id=gobbledygook"
expect(response).to have_http_status(:not_found)
@ -23,69 +23,87 @@ RSpec.describe "Api::V0::Comments", type: :request do
end
it "does not include children comments in the root list" do
# create child comment
create(:comment, commentable: article, parent: article.comments.first)
get "/api/comments?a_id=#{article.id}"
expected_ids = article.comments.roots.map(&:id_code_generated)
expect(json_response.map { |cm| cm["id_code"] }).to match_array(expected_ids)
end
it "includes children comments in the children list" do
# create child comment
parent_comment = article.comments.first
child_comment = create(:comment, commentable: article, parent: parent_comment)
get "/api/comments?a_id=#{article.id}"
comment_with_children = json_response.detect { |cm| cm["id_code"] == parent_comment.id_code_generated }
comment_with_children = json_response.detect { |cm| cm["id_code"] == root_comment.id_code_generated }
expect(comment_with_children["children"][0]["id_code"]).to eq(child_comment.id_code_generated)
end
it "includes granchildren comments in the children-children list" do
# create granchild comment
root_comment = article.comments.first
child_comment = create(:comment, commentable: article, parent: root_comment)
granchild_comment = create(:comment, commentable: article, parent: child_comment)
it "includes grandchildren comments in the children-children list" do
get "/api/comments?a_id=#{article.id}"
comment_with_descendants = json_response.detect { |cm| cm["id_code"] == root_comment.id_code_generated }
expect(comment_with_descendants["children"][0]["children"][0]["id_code"]).to eq(granchild_comment.id_code_generated)
expect(comment_with_descendants["children"][0]["children"][0]["id_code"]).to eq(grandchild_comment.id_code_generated)
end
it "includes great-grandchildren comments in the children-children-children list" do
get "/api/comments?a_id=#{article.id}"
comment_with_descendants = json_response.detect { |cm| cm["id_code"] == root_comment.id_code_generated }
json_great_grandchild_id_code = comment_with_descendants["children"][0]["children"][0]["children"][0]["id_code"]
expect(json_great_grandchild_id_code).to eq(great_grandchild_comment.id_code_generated)
end
it "sets the correct edge caching surrogate key for all the comments" do
sibling_root_comment = create(:comment, commentable: article)
get "/api/comments?a_id=#{article.id}"
expected_key = [
"comments", sibling_root_comment.record_key,
root_comment.record_key, child_comment.record_key,
grandchild_comment.record_key, great_grandchild_comment.record_key
].to_set
expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key)
end
end
describe "GET /api/comments/:id" do
let_it_be(:comment) { create(:comment, commentable: article) }
it "returns not found if wrong comment id" do
get "/api/comments/foobar"
expect(response).to have_http_status(:not_found)
end
it "returns the comment" do
get "/api/comments/#{comment.id_code_generated}"
expect(json_response["id_code"]).to eq(comment.id_code_generated)
get "/api/comments/#{root_comment.id_code_generated}"
expect(json_response["id_code"]).to eq(root_comment.id_code_generated)
end
it "includes children comments in the children list" do
# create child comment
child_comment = create(:comment, commentable: article, parent: comment)
get "/api/comments/#{root_comment.id_code_generated}"
get "/api/comments/#{comment.id_code_generated}"
comment_with_children = json_response
expect(comment_with_children["children"][0]["id_code"]).to eq(child_comment.id_code_generated)
end
it "includes granchildren comments in the children-children list" do
# create granchild comment
root_comment = comment
child_comment = create(:comment, commentable: article, parent: root_comment)
granchild_comment = create(:comment, commentable: article, parent: child_comment)
get "/api/comments/#{comment.id_code_generated}"
it "includes grandchildren comments in the children-children list" do
get "/api/comments/#{root_comment.id_code_generated}"
comment_with_descendants = json_response
expect(comment_with_descendants["children"][0]["children"][0]["id_code"]).to eq(granchild_comment.id_code_generated)
expect(comment_with_descendants["children"][0]["children"][0]["id_code"]).to eq(grandchild_comment.id_code_generated)
end
it "includes great-grandchildren comments in the children-children-children list" do
get "/api/comments/#{root_comment.id_code_generated}"
comment_with_descendants = json_response
json_great_grandchild_id_code = comment_with_descendants["children"][0]["children"][0]["children"][0]["id_code"]
expect(json_great_grandchild_id_code).to eq(great_grandchild_comment.id_code_generated)
end
it "sets the correct edge caching surrogate key for all the comments" do
get "/api/comments/#{root_comment.id_code_generated}"
expected_key = [
"comments", root_comment.record_key, child_comment.record_key,
grandchild_comment.record_key, great_grandchild_comment.record_key
].to_set
expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key)
end
end
end